diff mbox

[8/11] Add FRV_MAX_REGISTER_SIZE

Message ID CE496FA1-0D8A-47DC-B8E5-91598F30C93E@arm.com
State New
Headers show

Commit Message

Alan Hayward May 3, 2017, 10:56 a.m. UTC 
> On 3 May 2017, at 10:27, Pedro Alves <palves@redhat.com> wrote:
> 
> 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".
> 

Agreed. I didn't want somebody in the future to change raw_supply and cause
raw_supply_zero to break.

Patch updated to head and raw_supply_zero moved into regcache class.

Tested on a --enable-targets=all using make check with board files
unix and native-gdbserver.

Ok to commit?

Alan.

2017-05-03  Alan Hayward  <alan.hayward@arm.com>

	* frv-linux-tdep.c (frv_linux_supply_gregset): Use raw_supply_zero.
	* regcache.c (regcache::raw_supply_zero): New function.
	* regcache.h (regcache::raw_supply_zero: New declaration.

Comments

Pedro Alves May 3, 2017, 11:23 a.m. UTC | #1 
On 05/03/2017 11:56 AM, Alan Hayward wrote:

>> BTW, note that gdbserver has an equivalent function, called
>> "supply_register_zeroed".

> +/* Supply register REGNUM with zeroed value to REGCACHE.  This is not the same
> +   as calling raw_supply with NULL (which will set the state to
> +   unavailable).  */
> +
> +void
> +regcache::raw_supply_zero (int regnum)

A very minor detail, but I'd prefer it if gdb and gdbserver agreed 
on terminology.  Should we call this one "zeroed" too, or
rename gdbserver's to "zero"?

Thanks,
Pedro Alves
Alan Hayward May 3, 2017, 11:36 a.m. UTC | #2 
> On 3 May 2017, at 12:23, Pedro Alves <palves@redhat.com> wrote:

> 

> On 05/03/2017 11:56 AM, Alan Hayward wrote:

> 

>>> BTW, note that gdbserver has an equivalent function, called

>>> "supply_register_zeroed".

> 

>> +/* Supply register REGNUM with zeroed value to REGCACHE.  This is not the same

>> +   as calling raw_supply with NULL (which will set the state to

>> +   unavailable).  */

>> +

>> +void

>> +regcache::raw_supply_zero (int regnum)

> 

> A very minor detail, but I'd prefer it if gdb and gdbserver agreed 

> on terminology.  Should we call this one "zeroed" too, or

> rename gdbserver's to "zero"?

> 

> Thanks,

> Pedro Alves

> 


I’m happy to update raw_supply to raw_supply_zeroed.
I think that sounds better too.


Alan.
Yao Qi May 3, 2017, 1:34 p.m. UTC | #3 
Alan Hayward <Alan.Hayward@arm.com> writes:

Hi Alan,

> +/* Supply register REGNUM with zeroed value to REGCACHE.  This is not the same

REGCACHE don't have to be capitalized.  This line is too long.

> +   as calling raw_supply with NULL (which will set the state to
> +   unavailable).  */
> +
> +void
> +regcache::raw_supply_zero (int regnum)

s/raw_supply_zero/raw_supply_zeroed/

OK with these changes.
diff mbox

Patch

diff --git a/gdb/frv-linux-tdep.c b/gdb/frv-linux-tdep.c
index eb87f93058b0287e8f05c585d1b6aa1ff2bffb78..30e5bf00c199a0193ea6cac3a14070445492d3fe 100644
--- a/gdb/frv-linux-tdep.c
+++ b/gdb/frv-linux-tdep.c
@@ -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 (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 (regi);

   regcache_supply_regset (regset, regcache, regnum, gregs, len);
 }
diff --git a/gdb/regcache.h b/gdb/regcache.h
index 346d290b752d8809f78b72c104ccbc2f5574d83c..ee949597d5691b692b1162a92bc328266fdc160a 100644
--- a/gdb/regcache.h
+++ b/gdb/regcache.h
@@ -296,6 +296,8 @@  public:

   void raw_supply (int regnum, const void *buf);

+  void raw_supply_zero (int regnum);
+
   void raw_copy (int regnum, struct regcache *src_regcache);

   enum register_status get_register_status (int regnum) const;
diff --git a/gdb/regcache.c b/gdb/regcache.c
index 748c30e2d4c5f572e9741f73a39c4bc555d1d663..89c2eb55e29af9a806a684382dc928c307ff934b 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -1208,6 +1208,26 @@  regcache::raw_supply (int regnum, const void *buf)
     }
 }

+/* Supply register REGNUM with zeroed value to REGCACHE.  This is not the same
+   as calling raw_supply with NULL (which will set the state to
+   unavailable).  */
+
+void
+regcache::raw_supply_zero (int regnum)
+{
+  void *regbuf;
+  size_t size;
+
+  gdb_assert (regnum >= 0 && regnum < m_descr->nr_raw_registers);
+  gdb_assert (!m_readonly_p);
+
+  regbuf = register_buffer (regnum);
+  size = m_descr->sizeof_register[regnum];
+
+  memset (regbuf, 0, size);
+  m_register_status[regnum] = REG_VALID;
+}
+
 /* Collect register REGNUM from REGCACHE and store its contents in BUF.  */

 void