[8/11] Add FRV_MAX_REGISTER_SIZE
Commit Message
> On 12 Apr 2017, at 09:27, Alan Hayward <Alan.Hayward@arm.com> wrote:
>
>
>> On 11 Apr 2017, at 11:02, Yao Qi <qiyaoltc@gmail.com> wrote:
>>
>> Alan Hayward <Alan.Hayward@arm.com> writes:
>>
>>> - char zerobuf[MAX_REGISTER_SIZE];
>>> + char zerobuf[FRV_MAX_REGISTER_SIZE];
>>>
>>> - memset (zerobuf, 0, MAX_REGISTER_SIZE);
>>> + memset (zerobuf, 0, FRV_MAX_REGISTER_SIZE);
>>>
>>> /* gr0 always contains 0. Also, the kernel passes the TBR value in
>>> this slot. */
>>
>> The code here fills some gr registers with zeros,
>>
>> /* gr0 always contains 0. Also, the kernel passes the TBR value in
>> this slot. */
>> regcache_raw_supply (regcache, first_gpr_regnum, zerobuf);
>>
>> /* Fill gr32, ..., gr63 with zeros. */
>> for (regi = first_gpr_regnum + 32; regi <= last_gpr_regnum; regi++)
>> regcache_raw_supply (regcache, regi, zerobuf);
>>
>> the size of these gr registers are know, 8 bytes. It won't be changed.
>> We can do,
>>
>> gdb_byte zerobuf[8] = { 0 };
>>
>> the code is still easy to read. If you really dislike magic number (IMO, 8
>> is not a magic number in this context), you can define FRR_GR_REGISTER_SIZE.
>>
>> Alternatively, you can add a new regache api, regcache_raw_supply_zero.
>> Many places can use this api, and some uses of MAX_REGISTER_SIZE can be
>> removed too, for example,
>>
>> regcache_raw_supply (regcache, MIPS_ZERO_REGNUM, zerobuf);
>>
>> --
>> Yao (齐尧)
>
> Went with the simpler solution.
>
> I don't have a FRV machine to test on.
> Tested on a --enable-targets=all build using make check with board files
> unix and native-gdbserver.
>
Looking again at my later patches, I agree that regcache_raw_supply_zero
would be useful in other places too.
I considered making regcache_raw_supply_zero call regcache_raw_supply, but
in the end it made more sense to make it completely separate.
I don't have a FRV machine to test on.
Tested on a --enable-targets=all build using make check with board files
unix and native-gdbserver.
Ok to commit?
Alan.
2017-04-27 Alan Hayward <alan.hayward@arm.com>
* gdb/frv-linux-tdep.c (frv_linux_supply_gregset): Call
regcache_raw_supply_zero
* gdb/regcache.c (regcache_raw_supply_zero): New.
* gdb/regcache.h (regcache_raw_supply_zero): New declaration.
Comments
Alan Hayward <Alan.Hayward@arm.com> writes:
Hi Alan,
regcache.c is updated, so please update your patch.
> I considered making regcache_raw_supply_zero call regcache_raw_supply, but
> in the end it made more sense to make it completely separate.
You can call raw_supply (regnum, NULL) and then set the status to REG_VALID.
On 05/03/2017 09:44 AM, Yao Qi wrote:
> Alan Hayward <Alan.Hayward@arm.com> writes:
>
> Hi Alan,
> regcache.c is updated, so please update your patch.
>
>> I considered making regcache_raw_supply_zero call regcache_raw_supply, but
>> in the end it made more sense to make it completely separate.
>
> You can call raw_supply (regnum, NULL) and then set the status to REG_VALID.
>
I think I agree with Alan -- if we defer to raw_supply, then I'd still prefer
that the memset is still done in regcache_raw_supply_zero, because whether
unavailable registers actually have a contents buffer at all is
implementation detail. We currently zero REG_UNVAILABLE registers in raw_supply,
but that could change. (And if we reuse raw_supply as is, then memset, we'll
memset twice.)
BTW, note that gdbserver has an equivalent function, called
"supply_register_zeroed".
Thanks,
Pedro Alves
@@ -413,17 +413,14 @@ frv_linux_supply_gregset (const struct regset *regset,
int regnum, const void *gregs, size_t len)
{
int regi;
- char zerobuf[MAX_REGISTER_SIZE];
-
- memset (zerobuf, 0, MAX_REGISTER_SIZE);
/* gr0 always contains 0. Also, the kernel passes the TBR value in
this slot. */
- regcache_raw_supply (regcache, first_gpr_regnum, zerobuf);
+ regcache_raw_supply_zero (regcache, first_gpr_regnum);
/* Fill gr32, ..., gr63 with zeros. */
for (regi = first_gpr_regnum + 32; regi <= last_gpr_regnum; regi++)
- regcache_raw_supply (regcache, regi, zerobuf);
+ regcache_raw_supply_zero (regcache, regi);
regcache_supply_regset (regset, regcache, regnum, gregs, len);
}
@@ -152,6 +152,12 @@ extern void regcache_raw_supply (struct regcache *regcache,
extern void regcache_raw_collect (const struct regcache *regcache,
int regnum, void *buf);
+/* Supply register REGNUM with zeroed value to REGCACHE. This is not the same
+ as calling regcache_raw_supply with NULL (which will set the state to
+ unavailable). */
+
+extern void regcache_raw_supply_zero (struct regcache *regcache, int regnum);
+
/* Collect register REGNUM from SOURCE_REGCACHE and store its contents to
DEST_REGCACHE. */
extern void regcache_raw_copy (const struct regcache *dest_regcache,
@@ -1125,6 +1125,27 @@ regcache_raw_collect (const struct regcache *regcache, int regnum, void *buf)
memcpy (buf, regbuf, size);
}
+/* Supply register REGNUM with zeroed value to REGCACHE. This is not the same
+ as calling regcache_raw_supply with NULL (which will set the state to
+ unavailable). */
+
+void
+regcache_raw_supply_zero (struct regcache *regcache, int regnum)
+{
+ void *regbuf;
+ size_t size;
+
+ gdb_assert (regcache != NULL);
+ gdb_assert (regnum >= 0 && regnum < regcache->descr->nr_raw_registers);
+ gdb_assert (!regcache->readonly_p);
+
+ regbuf = register_buffer (regcache, regnum);
+ size = regcache->descr->sizeof_register[regnum];
+
+ memset (regbuf, 0, size);
+ regcache->register_status[regnum] = REG_VALID;
+}
+
void
regcache_raw_copy (const struct regcache *dest_regcache, int regnum,
struct regcache *src_regcache)