Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Alan Hayward <Alan.Hayward@arm.com>
To: Yao Qi <qiyaoltc@gmail.com>
Cc: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>,
	nd <nd@arm.com>
Subject: Re: [PATCH 8/11] Add FRV_MAX_REGISTER_SIZE
Date: Thu, 27 Apr 2017 08:55:00 -0000	[thread overview]
Message-ID: <22BD364F-A9A9-4E21-BC47-96A320760640@arm.com> (raw)
In-Reply-To: <EBF2EEFF-20D5-43F3-B35D-5BB37C3CB153@arm.com>


> 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.


diff --git a/gdb/frv-linux-tdep.c b/gdb/frv-linux-tdep.c
index eb87f93058b0287e8f05c585d1b6aa1ff2bffb78..d52eb2163437e32029c989c2caecce8533f11a65 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 (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);
 }
diff --git a/gdb/regcache.h b/gdb/regcache.h
index f201f0c084f40ccf276a7e1ba19050cbc11208ad..ff6e583a48f9200f4a46ea5fcca69bfdd2862dac 100644
--- a/gdb/regcache.h
+++ b/gdb/regcache.h
@@ -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,
diff --git a/gdb/regcache.c b/gdb/regcache.c
index 20e43f486ff02f15000bebee2a4eb03db53111d4..03f7d216f7d2fb8dd2e5b1189c9743ed37facb30 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -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)




  parent reply	other threads:[~2017-04-27  8:55 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-04 10:14 Alan Hayward
2017-04-11 10:02 ` Yao Qi
     [not found]   ` <EBF2EEFF-20D5-43F3-B35D-5BB37C3CB153@arm.com>
2017-04-27  8:55     ` Alan Hayward [this message]
2017-05-03  8:44       ` Yao Qi
2017-05-03  9:27         ` Pedro Alves
     [not found]           ` <CE496FA1-0D8A-47DC-B8E5-91598F30C93E@arm.com>
2017-05-03 11:23             ` Pedro Alves
2017-05-03 11:36               ` Alan Hayward
2017-05-03 13:34             ` Yao Qi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=22BD364F-A9A9-4E21-BC47-96A320760640@arm.com \
    --to=alan.hayward@arm.com \
    --cc=gdb-patches@sourceware.org \
    --cc=nd@arm.com \
    --cc=qiyaoltc@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox