[PATCH, RFC] Changes to sim_store_register API

[PATCH, RFC] Changes to sim_store_register API

[Andrew Burgess]

[-- Attachment #1: Type: text/plain, Size: 1322 bytes --]

The current API to sim_store_register returns the length of the register that was updated. However if the length returned is <= 0 then this is acceptable. Some simulators return 0 or -1 even on a successful register write.

I would like to propose the following changes,

(1) As the length of the register to be written is already passed into sim_store_register all simulators should return that length if they successfully update the register.
(2) If a length of 0 is returned this indicates that the simulator understood the request but couldn't update the register, this might mean that the register is read only, or just that updating of this register has not been implemented yet; in this case gdb will give a warning that the register has not been updated and continue.
(3) If a length of -1 is returned this indicates that the simulator didn't understand the request or some other non-recoverable error has occurred; in this case gdb will quit with an error.

The attached patch implements this change and updates the existing simulators to either return the length in the case of success or to return 0 for errors. 

I would welcome your feedback and suggestions.

I would be happy to update the fetch register case in a similar fashion but thought I'd start small to begin with.

Thanks,
Andrew


[-- Attachment #2: sim-store-register.patch --]
[-- Type: application/octet-stream, Size: 4381 bytes --]

diff -pru clean/gdb-7.2.50.20101027/gdb/remote-sim.c working/gdb-7.2.50.20101027/gdb/remote-sim.c
--- clean/gdb-7.2.50.20101027/gdb/remote-sim.c	2010-08-10 05:39:26.000000000 +0100
+++ working/gdb-7.2.50.20101027/gdb/remote-sim.c	2010-11-07 16:47:01.998498944 +0000
@@ -527,9 +527,13 @@ gdbsim_store_register (struct target_ops
       if (nr_bytes > 0 && nr_bytes != register_size (gdbarch, regno))
 	internal_error (__FILE__, __LINE__,
 			_("Register size different to expected"));
-      /* FIXME: cagney/2002-05-27: Should check `nr_bytes == 0'
-	 indicating that GDB and the SIM have different ideas about
-	 which registers are fetchable.  */
+      if (nr_bytes < 0)
+        internal_error (__FILE__, __LINE__,
+			_("Register %d not updated"), regno);
+      if (nr_bytes == 0)
+        warning (_("Register not updated"),
+                 gdbarch_register_name (gdbarch, regno));
+      
       if (remote_debug)
 	{
 	  printf_filtered ("gdbsim_store_register: %d", regno);
Only in working/gdb-7.2.50.20101027/gdb: remote-sim.c~
diff -pru clean/gdb-7.2.50.20101027/sim/erc32/interf.c working/gdb-7.2.50.20101027/sim/erc32/interf.c
--- clean/gdb-7.2.50.20101027/sim/erc32/interf.c	2010-05-11 15:18:20.000000000 +0100
+++ working/gdb-7.2.50.20101027/sim/erc32/interf.c	2010-10-27 16:56:12.607262027 +0100
@@ -330,7 +330,7 @@ sim_store_register(sd, regno, value, len
 	regval = (value[3] << 24) | (value[2] << 16)
 		 | (value[1] << 8) | value[0];
     set_regi(&sregs, regno, regval);
-    return -1;
+    return length;
 }
 
 
diff -pru clean/gdb-7.2.50.20101027/sim/h8300/compile.c working/gdb-7.2.50.20101027/sim/h8300/compile.c
--- clean/gdb-7.2.50.20101027/sim/h8300/compile.c	2010-04-14 08:38:04.000000000 +0100
+++ working/gdb-7.2.50.20101027/sim/h8300/compile.c	2010-10-27 16:56:28.193119915 +0100
@@ -4715,7 +4715,7 @@ sim_store_register (SIM_DESC sd, int rn,
       h8_set_ticks (sd, longval);
       break;
     }
-  return -1;
+  return length;
 }
 
 int
diff -pru clean/gdb-7.2.50.20101027/sim/m32c/gdb-if.c working/gdb-7.2.50.20101027/sim/m32c/gdb-if.c
--- clean/gdb-7.2.50.20101027/sim/m32c/gdb-if.c	2010-04-14 08:38:04.000000000 +0100
+++ working/gdb-7.2.50.20101027/sim/m32c/gdb-if.c	2010-10-27 16:57:29.622058082 +0100
@@ -502,7 +502,7 @@ sim_store_register (SIM_DESC sd, int reg
 	default:
 	  fprintf (stderr, "m32c minisim: unrecognized register number: %d\n",
 		   regno);
-	  return -1;
+	  return 0;
 	}
     }
 
diff -pru clean/gdb-7.2.50.20101027/sim/mn10300/interp.c working/gdb-7.2.50.20101027/sim/mn10300/interp.c
--- clean/gdb-7.2.50.20101027/sim/mn10300/interp.c	2004-06-26 23:18:18.000000000 +0100
+++ working/gdb-7.2.50.20101027/sim/mn10300/interp.c	2010-10-27 16:59:00.810068620 +0100
@@ -410,7 +410,7 @@ sim_store_register (SIM_DESC sd,
 		    int length)
 {
   State.regs[rn] = get_word (memory);
-  return -1;
+  return length;
 }
 
 
diff -pru clean/gdb-7.2.50.20101027/sim/ppc/gdb-sim.c working/gdb-7.2.50.20101027/sim/ppc/gdb-sim.c
--- clean/gdb-7.2.50.20101027/sim/ppc/gdb-sim.c	2010-01-01 10:03:33.000000000 +0000
+++ working/gdb-7.2.50.20101027/sim/ppc/gdb-sim.c	2010-10-27 17:00:16.559040694 +0100
@@ -1289,7 +1289,7 @@ sim_store_register (SIM_DESC sd, int reg
   const char *regname = regnum2name (regno);
 
   if (simulator == NULL || regname == NULL)
-    return -1;
+    return 0;
 
   TRACE(trace_gdb, ("sim_store_register(regno=%d(%s), buf=0x%lx)\n",
 		    regno, regname, (long)buf));
diff -pru clean/gdb-7.2.50.20101027/sim/rx/gdb-if.c working/gdb-7.2.50.20101027/sim/rx/gdb-if.c
--- clean/gdb-7.2.50.20101027/sim/rx/gdb-if.c	2010-09-24 00:05:28.000000000 +0100
+++ working/gdb-7.2.50.20101027/sim/rx/gdb-if.c	2010-10-27 16:59:37.090066882 +0100
@@ -630,7 +630,7 @@ sim_store_register (SIM_DESC sd, int reg
     default:
       fprintf (stderr, "rx minisim: unrecognized register number: %d\n",
 	       regno);
-      return -1;
+      return 0;
     }
 
   return size;
diff -pru clean/gdb-7.2.50.20101027/sim/v850/interp.c working/gdb-7.2.50.20101027/sim/v850/interp.c
--- clean/gdb-7.2.50.20101027/sim/v850/interp.c	2010-03-31 00:43:03.000000000 +0100
+++ working/gdb-7.2.50.20101027/sim/v850/interp.c	2010-10-27 17:00:02.016083444 +0100
@@ -327,7 +327,7 @@ sim_store_register (sd, rn, memory, leng
      int length;
 {
   State.regs[rn] = T2H_4 (*(unsigned32*)memory);
-  return -1;
+  return length;
 }
 
 void

RE: [PATCH, RFC] Changes to sim_store_register API

[Andrew Burgess]

[-- Attachment #1: Type: text/plain, Size: 3472 bytes --]

Hi,

Updated patch, I also realise I forgot a changelog entry - I've included one below.

I'm currently using this code in a company branch of gdb. For this patch off mainline I've tested building a few different gdb targets and am currently working on setting up a cross-compiler tool chain so I can run some simulator tests.
While I'm doing that if anyone would like to review this and give any feedback, then I can make any changes needed.

Thanks,

Andrew

ChangeLog Entries:

gdb/ChangeLog : 

2010-11-16  Andrew Burgess <aburgess@broadcom.com>

	* remote-sim.c (gdbsim_store_register): Update API to 
	  sim_store_register to check more error conditions.

sim/erc32/ChangeLog :

2010-11-16  Andrew Burgess <aburgess@broadcom.com>

	* interf.c (sim_store_register): Update return value to 
	  match new API. 

sim/h8300/ChangeLog :

2010-11-16  Andrew Burgess <aburgess@broadcom.com>

	* compile.c (sim_store_register): Update return value to 
	  match new API. 

sim/m32c/ChangesLog : 

2010-11-16  Andrew Burgess <aburgess@broadcom.com>

	* gdb-if.c (sim_store_register): Update return value to 
	  match new API. 

sim/mn10300/ChangeLog : 

2010-11-16  Andrew Burgess <aburgess@broadcom.com>

	* interp.c (sim_store_register): Update return value to 
	  match new API. 

sim/ppc/ChangeLog : 

2010-11-16  Andrew Burgess <aburgess@broadcom.com>

	* gdb-sim.c (sim_store_register): Update return value to 
	  match new API. 

sim/rx/ChangeLog :

2010-11-16  Andrew Burgess  <aburgess@broadcom.com>

	* gdb-if.c (sim_store_register): Update return value to
	  match new API.

sim/v850/ChangeLog : 

2010-11-16  Andrew Burgess  <aburgess@broadcom.com>

	* interp.c (sim_store_register): Update return value to
	  match new API.

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] On Behalf Of Andrew Burgess
> Sent: 08 November 2010 08:12
> To: gdb-patches@sourceware.org
> Subject: [PATCH, RFC] Changes to sim_store_register API
> 
> The current API to sim_store_register returns the length of the
> register that was updated. However if the length returned is <= 0 then
> this is acceptable. Some simulators return 0 or -1 even on a successful
> register write.
> 
> I would like to propose the following changes,
> 
> (1) As the length of the register to be written is already passed into
> sim_store_register all simulators should return that length if they
> successfully update the register.
> (2) If a length of 0 is returned this indicates that the simulator
> understood the request but couldn't update the register, this might
> mean that the register is read only, or just that updating of this
> register has not been implemented yet; in this case gdb will give a
> warning that the register has not been updated and continue.
> (3) If a length of -1 is returned this indicates that the simulator
> didn't understand the request or some other non-recoverable error has
> occurred; in this case gdb will quit with an error.
> 
> The attached patch implements this change and updates the existing
> simulators to either return the length in the case of success or to
> return 0 for errors.
> 
> I would welcome your feedback and suggestions.
> 
> I would be happy to update the fetch register case in a similar fashion
> but thought I'd start small to begin with.
> 
> Thanks,
> Andrew


[-- Attachment #2: gdb-sim-store.patch --]
[-- Type: application/octet-stream, Size: 4064 bytes --]

diff -rwu clean/gdb-7.2.50.20101027/gdb/remote-sim.c working/gdb-7.2.50.20101027/gdb/remote-sim.c
--- clean/gdb-7.2.50.20101027/gdb/remote-sim.c	2010-08-10 05:39:26.000000000 +0100
+++ working/gdb-7.2.50.20101027/gdb/remote-sim.c	2010-11-16 11:07:54.398861201 +0000
@@ -527,9 +527,13 @@
       if (nr_bytes > 0 && nr_bytes != register_size (gdbarch, regno))
 	internal_error (__FILE__, __LINE__,
 			_("Register size different to expected"));
-      /* FIXME: cagney/2002-05-27: Should check `nr_bytes == 0'
-	 indicating that GDB and the SIM have different ideas about
-	 which registers are fetchable.  */
+      if (nr_bytes < 0)
+        internal_error (__FILE__, __LINE__,
+			_("Register %d not updated"), regno);
+      if (nr_bytes == 0)
+        warning (_("Register %s not updated"),
+                 gdbarch_register_name (gdbarch, regno));
+      
       if (remote_debug)
 	{
 	  printf_filtered ("gdbsim_store_register: %d", regno);
Only in working/gdb-7.2.50.20101027/gdb: remote-sim.c~
diff -rwu clean/gdb-7.2.50.20101027/sim/erc32/interf.c working/gdb-7.2.50.20101027/sim/erc32/interf.c
--- clean/gdb-7.2.50.20101027/sim/erc32/interf.c	2010-05-11 15:18:20.000000000 +0100
+++ working/gdb-7.2.50.20101027/sim/erc32/interf.c	2010-10-27 16:56:12.607262027 +0100
@@ -330,7 +330,7 @@
 	regval = (value[3] << 24) | (value[2] << 16)
 		 | (value[1] << 8) | value[0];
     set_regi(&sregs, regno, regval);
-    return -1;
+    return length;
 }
 
 
diff -rwu clean/gdb-7.2.50.20101027/sim/h8300/compile.c working/gdb-7.2.50.20101027/sim/h8300/compile.c
--- clean/gdb-7.2.50.20101027/sim/h8300/compile.c	2010-04-14 08:38:04.000000000 +0100
+++ working/gdb-7.2.50.20101027/sim/h8300/compile.c	2010-10-27 16:56:28.193119915 +0100
@@ -4715,7 +4715,7 @@
       h8_set_ticks (sd, longval);
       break;
     }
-  return -1;
+  return length;
 }
 
 int
diff -rwu clean/gdb-7.2.50.20101027/sim/m32c/gdb-if.c working/gdb-7.2.50.20101027/sim/m32c/gdb-if.c
--- clean/gdb-7.2.50.20101027/sim/m32c/gdb-if.c	2010-04-14 08:38:04.000000000 +0100
+++ working/gdb-7.2.50.20101027/sim/m32c/gdb-if.c	2010-10-27 16:57:29.622058082 +0100
@@ -502,7 +502,7 @@
 	default:
 	  fprintf (stderr, "m32c minisim: unrecognized register number: %d\n",
 		   regno);
-	  return -1;
+	  return 0;
 	}
     }
 
diff -rwu clean/gdb-7.2.50.20101027/sim/mn10300/interp.c working/gdb-7.2.50.20101027/sim/mn10300/interp.c
--- clean/gdb-7.2.50.20101027/sim/mn10300/interp.c	2004-06-26 23:18:18.000000000 +0100
+++ working/gdb-7.2.50.20101027/sim/mn10300/interp.c	2010-10-27 16:59:00.810068620 +0100
@@ -410,7 +410,7 @@
 		    int length)
 {
   State.regs[rn] = get_word (memory);
-  return -1;
+  return length;
 }
 
 
diff -rwu clean/gdb-7.2.50.20101027/sim/ppc/gdb-sim.c working/gdb-7.2.50.20101027/sim/ppc/gdb-sim.c
--- clean/gdb-7.2.50.20101027/sim/ppc/gdb-sim.c	2010-01-01 10:03:33.000000000 +0000
+++ working/gdb-7.2.50.20101027/sim/ppc/gdb-sim.c	2010-10-27 17:00:16.559040694 +0100
@@ -1289,7 +1289,7 @@
   const char *regname = regnum2name (regno);
 
   if (simulator == NULL || regname == NULL)
-    return -1;
+    return 0;
 
   TRACE(trace_gdb, ("sim_store_register(regno=%d(%s), buf=0x%lx)\n",
 		    regno, regname, (long)buf));
diff -rwu clean/gdb-7.2.50.20101027/sim/rx/gdb-if.c working/gdb-7.2.50.20101027/sim/rx/gdb-if.c
--- clean/gdb-7.2.50.20101027/sim/rx/gdb-if.c	2010-09-24 00:05:28.000000000 +0100
+++ working/gdb-7.2.50.20101027/sim/rx/gdb-if.c	2010-10-27 16:59:37.090066882 +0100
@@ -630,7 +630,7 @@
     default:
       fprintf (stderr, "rx minisim: unrecognized register number: %d\n",
 	       regno);
-      return -1;
+      return 0;
     }
 
   return size;
diff -rwu clean/gdb-7.2.50.20101027/sim/v850/interp.c working/gdb-7.2.50.20101027/sim/v850/interp.c
--- clean/gdb-7.2.50.20101027/sim/v850/interp.c	2010-03-31 00:43:03.000000000 +0100
+++ working/gdb-7.2.50.20101027/sim/v850/interp.c	2010-10-27 17:00:02.016083444 +0100
@@ -327,7 +327,7 @@
      int length;
 {
   State.regs[rn] = T2H_4 (*(unsigned32*)memory);
-  return -1;
+  return length;
 }
 
 void

Re: [PATCH, RFC] Changes to sim_store_register API

[Joel Brobecker]

Andrew,

Thanks for doing this work.

Just a small request: Can you generate your diffs using the -p option?
This allows us to see the name of the function being modified by each
hunk, thus making review a little easier...

> 2010-11-16  Andrew Burgess <aburgess@broadcom.com>
> 
> 	* remote-sim.c (gdbsim_store_register): Update API to 
> 	  sim_store_register to check more error conditions.

This change needs a corresponding documentation update in
include/gdb/remote-sim.h.

> 2010-11-16  Andrew Burgess <aburgess@broadcom.com>
> 
> 	* interf.c (sim_store_register): Update return value to 
> 	  match new API. 
> 
> sim/h8300/ChangeLog :
> 
> 2010-11-16  Andrew Burgess <aburgess@broadcom.com>
> 
> 	* compile.c (sim_store_register): Update return value to 
> 	  match new API. 
> 
> sim/m32c/ChangesLog : 
> 
> 2010-11-16  Andrew Burgess <aburgess@broadcom.com>
> 
> 	* gdb-if.c (sim_store_register): Update return value to 
> 	  match new API. 

I think that this function should return -1 if the check_regno() check
fails.

> sim/mn10300/ChangeLog : 
> 
> 2010-11-16  Andrew Burgess <aburgess@broadcom.com>
> 
> 	* interp.c (sim_store_register): Update return value to 
> 	  match new API. 
> 
> sim/ppc/ChangeLog : 
> 
> 2010-11-16  Andrew Burgess <aburgess@broadcom.com>
> 
> 	* gdb-sim.c (sim_store_register): Update return value to 
> 	  match new API. 
> 
> sim/rx/ChangeLog :
> 
> 2010-11-16  Andrew Burgess  <aburgess@broadcom.com>
> 
> 	* gdb-if.c (sim_store_register): Update return value to
> 	  match new API.

Same as above, here.

> sim/v850/ChangeLog : 
> 
> 2010-11-16  Andrew Burgess  <aburgess@broadcom.com>
> 
> 	* interp.c (sim_store_register): Update return value to
> 	  match new API.

Overall, this seems OK to me.  Can you make the adjustments requested
above, and then resubmit?

>  	default:
>  	  fprintf (stderr, "m32c minisim: unrecognized register number: %d\n",
>  		   regno);
> -	  return -1;
> +	  return 0;

I had to think twice about this one - on the one hand, reaching this
code means an implementation error/hole somewhere.  But then, assuming
that the register number cannot be off-limits (because of the check_regno()
test above), it's probably because of the sim code missing the handling
for a given register. Which would be best in that case? A warning and
continue, or an internal-error? I'll follow your suggestion on a
warning, hoping that the failed operation does not change the behavior
too much (the warnings might help reduce confusion).

-- 
Joel

Re: [PATCH, RFC] Changes to sim_store_register API

[Andrew Burgess]

Joel,

Thanks for taking the time to review this patch.

On 14/12/2010 06:49, Joel Brobecker wrote:
>
> Overall, this seems OK to me.  Can you make the adjustments requested
> above, and then resubmit?
>

I believe I have addressed all the issues raised.

Thanks,

Andrew


gdb/ChangeLog :

2010-11-16  Andrew Burgess <aburgess@broadcom.com>

	* remote-sim.c (gdbsim_store_register): Update API to
	  sim_store_register to check more error conditions.

include/gdb/ChangeLog :

2010-11-16  Andrew Burgess <aburgess@broadcom.com>

         * remote-sim.h (sim_store_register): Update the API
           documentation for this function.

sim/erc32/ChangeLog :

2010-11-16  Andrew Burgess <aburgess@broadcom.com>

	* interf.c (sim_store_register): Update return value to
	  match new API.

sim/h8300/ChangeLog :

2010-11-16  Andrew Burgess <aburgess@broadcom.com>

	* compile.c (sim_store_register): Update return value to
	  match new API.

sim/m32c/ChangesLog :

2010-11-16  Andrew Burgess <aburgess@broadcom.com>

	* gdb-if.c (sim_store_register): Update return value to
	  match new API.

sim/mn10300/ChangeLog :

2010-11-16  Andrew Burgess <aburgess@broadcom.com>

	* interp.c (sim_store_register): Update return value to
	  match new API.

sim/ppc/ChangeLog :

2010-11-16  Andrew Burgess <aburgess@broadcom.com>

	* gdb-sim.c (sim_store_register): Update return value to
	  match new API.

sim/rx/ChangeLog :

2010-11-16  Andrew Burgess  <aburgess@broadcom.com>

	* gdb-if.c (sim_store_register): Update return value to
	  match new API.

sim/v850/ChangeLog :

2010-11-16  Andrew Burgess  <aburgess@broadcom.com>

	* interp.c (sim_store_register): Update return value to
	  match new API.


diff -rwup clean/gdb-7.2.50.20101027/gdb/remote-sim.c working/gdb-7.2.50.20101027/gdb/remote-sim.c
--- clean/gdb-7.2.50.20101027/gdb/remote-sim.c	2010-08-10 05:39:26.000000000 +0100
+++ working/gdb-7.2.50.20101027/gdb/remote-sim.c	2010-11-16 11:07:54.398861201 +0000
@@ -527,9 +527,13 @@ gdbsim_store_register (struct target_ops
        if (nr_bytes > 0 && nr_bytes != register_size (gdbarch, regno))
  	internal_error (__FILE__, __LINE__,
  			_("Register size different to expected"));
-      /* FIXME: cagney/2002-05-27: Should check `nr_bytes == 0'
-	 indicating that GDB and the SIM have different ideas about
-	 which registers are fetchable.  */
+      if (nr_bytes < 0)
+        internal_error (__FILE__, __LINE__,
+			_("Register %d not updated"), regno);
+      if (nr_bytes == 0)
+        warning (_("Register %s not updated"),
+                 gdbarch_register_name (gdbarch, regno));
+
        if (remote_debug)
  	{
  	  printf_filtered ("gdbsim_store_register: %d", regno);
diff -rwup clean/gdb-7.2.50.20101027/include/gdb/remote-sim.h working/gdb-7.2.50.20101027/include/gdb/remote-sim.h
--- clean/gdb-7.2.50.20101027/include/gdb/remote-sim.h	2010-04-13 21:39:44.000000000 +0100
+++ working/gdb-7.2.50.20101027/include/gdb/remote-sim.h	2010-12-14 08:44:50.186806811 +0000
@@ -191,13 +191,15 @@ int sim_fetch_register (SIM_DESC sd, int


  /* Store register REGNO from the raw (target endian) value in BUF.
-   Return the actual size of the register or zero if REGNO is not
-   applicable.

-   Legacy implementations ignore LENGTH and always return -1.
+   Return the actual size of the register, any size not equal to
+   LENGTH indicates the register was not updated correctly.

-   If LENGTH does not match the size of REGNO no data is transfered
-   (the actual register size is still returned). */
+   Return a LENGTH of -1 to indicate the register was not updated
+   and an error has occurred.
+
+   Return a LENGTH of 0 to indicate the register was not updated
+   but no error has occurred. */

  int sim_store_register (SIM_DESC sd, int regno, unsigned char *buf, int length);

diff -rwup clean/gdb-7.2.50.20101027/sim/erc32/interf.c working/gdb-7.2.50.20101027/sim/erc32/interf.c
--- clean/gdb-7.2.50.20101027/sim/erc32/interf.c	2010-05-11 15:18:20.000000000 +0100
+++ working/gdb-7.2.50.20101027/sim/erc32/interf.c	2010-10-27 16:56:12.607262027 +0100
@@ -330,7 +330,7 @@ sim_store_register(sd, regno, value, len
  	regval = (value[3] << 24) | (value[2] << 16)
  		 | (value[1] << 8) | value[0];
      set_regi(&sregs, regno, regval);
-    return -1;
+    return length;
  }


diff -rwup clean/gdb-7.2.50.20101027/sim/h8300/compile.c working/gdb-7.2.50.20101027/sim/h8300/compile.c
--- clean/gdb-7.2.50.20101027/sim/h8300/compile.c	2010-04-14 08:38:04.000000000 +0100
+++ working/gdb-7.2.50.20101027/sim/h8300/compile.c	2010-10-27 16:56:28.193119915 +0100
@@ -4715,7 +4715,7 @@ sim_store_register (SIM_DESC sd, int rn,
        h8_set_ticks (sd, longval);
        break;
      }
-  return -1;
+  return length;
  }

  int
diff -rwup clean/gdb-7.2.50.20101027/sim/m32c/gdb-if.c working/gdb-7.2.50.20101027/sim/m32c/gdb-if.c
--- clean/gdb-7.2.50.20101027/sim/m32c/gdb-if.c	2010-04-14 08:38:04.000000000 +0100
+++ working/gdb-7.2.50.20101027/sim/m32c/gdb-if.c	2010-12-14 08:46:04.058808380 +0000
@@ -405,7 +405,7 @@ sim_store_register (SIM_DESC sd, int reg
    check_desc (sd);

    if (!check_regno (regno))
-    return 0;
+    return -1;

    size = reg_size (regno);

@@ -502,7 +502,7 @@ sim_store_register (SIM_DESC sd, int reg
  	default:
  	  fprintf (stderr, "m32c minisim: unrecognized register number: %d\n",
  		   regno);
-	  return -1;
+	  return 0;
  	}
      }

diff -rwup clean/gdb-7.2.50.20101027/sim/mn10300/interp.c working/gdb-7.2.50.20101027/sim/mn10300/interp.c
--- clean/gdb-7.2.50.20101027/sim/mn10300/interp.c	2004-06-26 23:18:18.000000000 +0100
+++ working/gdb-7.2.50.20101027/sim/mn10300/interp.c	2010-10-27 16:59:00.810068620 +0100
@@ -410,7 +410,7 @@ sim_store_register (SIM_DESC sd,
  		    int length)
  {
    State.regs[rn] = get_word (memory);
-  return -1;
+  return length;
  }


diff -rwup clean/gdb-7.2.50.20101027/sim/ppc/gdb-sim.c working/gdb-7.2.50.20101027/sim/ppc/gdb-sim.c
--- clean/gdb-7.2.50.20101027/sim/ppc/gdb-sim.c	2010-01-01 10:03:33.000000000 +0000
+++ working/gdb-7.2.50.20101027/sim/ppc/gdb-sim.c	2010-10-27 17:00:16.559040694 +0100
@@ -1289,7 +1289,7 @@ sim_store_register (SIM_DESC sd, int reg
    const char *regname = regnum2name (regno);

    if (simulator == NULL || regname == NULL)
-    return -1;
+    return 0;

    TRACE(trace_gdb, ("sim_store_register(regno=%d(%s), buf=0x%lx)\n",
  		    regno, regname, (long)buf));
diff -rwup clean/gdb-7.2.50.20101027/sim/rx/gdb-if.c working/gdb-7.2.50.20101027/sim/rx/gdb-if.c
--- clean/gdb-7.2.50.20101027/sim/rx/gdb-if.c	2010-09-24 00:05:28.000000000 +0100
+++ working/gdb-7.2.50.20101027/sim/rx/gdb-if.c	2010-12-14 08:48:41.402812553 +0000
@@ -534,12 +534,12 @@ sim_store_register (SIM_DESC sd, int reg
    check_desc (sd);

    if (!check_regno (regno))
-    return 0;
+    return -1;

    size = reg_size (regno);

    if (length != size)
-    return 0;
+    return -1;

    if (rx_big_endian)
      val = get_be (buf, length);
@@ -630,7 +630,7 @@ sim_store_register (SIM_DESC sd, int reg
      default:
        fprintf (stderr, "rx minisim: unrecognized register number: %d\n",
  	       regno);
-      return -1;
+      return 0;
      }

    return size;
diff -rwup clean/gdb-7.2.50.20101027/sim/v850/interp.c working/gdb-7.2.50.20101027/sim/v850/interp.c
--- clean/gdb-7.2.50.20101027/sim/v850/interp.c	2010-03-31 00:43:03.000000000 +0100
+++ working/gdb-7.2.50.20101027/sim/v850/interp.c	2010-10-27 17:00:02.016083444 +0100
@@ -327,7 +327,7 @@ sim_store_register (sd, rn, memory, leng
       int length;
  {
    State.regs[rn] = T2H_4 (*(unsigned32*)memory);
-  return -1;
+  return length;
  }

  void

Re: [PATCH, RFC] Changes to sim_store_register API

[Joel Brobecker]

Andrew,

> gdb/ChangeLog :
> 
> 2010-11-16  Andrew Burgess <aburgess@broadcom.com>
> 
> 	* remote-sim.c (gdbsim_store_register): Update API to
> 	  sim_store_register to check more error conditions.
> 
> include/gdb/ChangeLog :
> 
> 2010-11-16  Andrew Burgess <aburgess@broadcom.com>
> 
>          * remote-sim.h (sim_store_register): Update the API
>            documentation for this function.
> 
> sim/erc32/ChangeLog :
> 
> 2010-11-16  Andrew Burgess <aburgess@broadcom.com>
> 
> 	* interf.c (sim_store_register): Update return value to
> 	  match new API.
> 
> sim/h8300/ChangeLog :
> 
> 2010-11-16  Andrew Burgess <aburgess@broadcom.com>
> 
> 	* compile.c (sim_store_register): Update return value to
> 	  match new API.
> 
> sim/m32c/ChangesLog :
> 
> 2010-11-16  Andrew Burgess <aburgess@broadcom.com>
> 
> 	* gdb-if.c (sim_store_register): Update return value to
> 	  match new API.
> 
> sim/mn10300/ChangeLog :
> 
> 2010-11-16  Andrew Burgess <aburgess@broadcom.com>
> 
> 	* interp.c (sim_store_register): Update return value to
> 	  match new API.
> 
> sim/ppc/ChangeLog :
> 
> 2010-11-16  Andrew Burgess <aburgess@broadcom.com>
> 
> 	* gdb-sim.c (sim_store_register): Update return value to
> 	  match new API.
> 
> sim/rx/ChangeLog :
> 
> 2010-11-16  Andrew Burgess  <aburgess@broadcom.com>
> 
> 	* gdb-if.c (sim_store_register): Update return value to
> 	  match new API.
> 
> sim/v850/ChangeLog :
> 
> 2010-11-16  Andrew Burgess  <aburgess@broadcom.com>
> 
> 	* interp.c (sim_store_register): Update return value to
> 	  match new API.

This looks good to me. Please remember to update the alignement in
your ChangeLog entries before committing (everything should be aligned
on the same column).

-- 
Joel

Re: [PATCH, RFC] Changes to sim_store_register API

[Andrew Burgess]

On 28/12/2010 05:53, Joel Brobecker wrote:
>
> This looks good to me. Please remember to update the alignement in
> your ChangeLog entries before committing (everything should be aligned
> on the same column).

I don't have commit access. I'm hoping someone will commit for me.

Thanks,

Andrew

Re: [PATCH, RFC] [commit] Changes to sim_store_register API

[Andrew Burgess]

On 28/12/2010 05:53, Joel Brobecker wrote:
> Andrew,

<snip patch>

>
> This looks good to me. Please remember to update the alignement in
> your ChangeLog entries before committing (everything should be aligned
> on the same column).

This is now committed.

First commit so I really hope I've not broken the world.

Andrew