Remove MAX_REGISTER_SIZE from remote-sim.c

Message ID E66B5C1B-FDC4-4A3F-937B-7A138DAFE01E@arm.com
State New, archived
Headers

Commit Message

Alan Hayward Feb. 24, 2017, 10:03 a.m. UTC
  Used a std::vector in the first case and struct value in the second.

Tested using make check with board files unix and native-gdbserver.

Ok to commit?

Alan.


2017-02-21  Alan Hayward  <alan.hayward@arm.com>

	* gdb/remote-sim.c (gdbsim_fetch_register): Use vec instead of array.
	(gdbsim_store_register): Read register using value.
  

Comments

Alan Hayward March 23, 2017, 2:12 p.m. UTC | #1
Ping.

> On 24 Feb 2017, at 10:03, Alan Hayward <Alan.Hayward@arm.com> wrote:
> 
> Used a std::vector in the first case and struct value in the second.
> 
> Tested using make check with board files unix and native-gdbserver.
> 
> Ok to commit?
> 
> Alan.
> 
> 
> 2017-02-21  Alan Hayward  <alan.hayward@arm.com>
> 
> 	* gdb/remote-sim.c (gdbsim_fetch_register): Use vec instead of array.
> 	(gdbsim_store_register): Read register using value.
> 
> 
> diff --git a/gdb/remote-sim.c b/gdb/remote-sim.c
> index b0c68c617e365c0ea18ac2eca525598b688ffbb5..aeceb06a557eb31778d0a85194fb47ea311eec09 100644
> --- a/gdb/remote-sim.c
> +++ b/gdb/remote-sim.c
> @@ -439,6 +439,9 @@ gdbsim_fetch_register (struct target_ops *ops,
>       return;
>     }
> 
> +  int regsize = register_size (gdbarch, regno);
> +  std::vector<gdb_byte> buf (regsize);
> +
>   switch (gdbarch_register_sim_regno (gdbarch, regno))
>     {
>     case LEGACY_SIM_REGNO_IGNORE:
> @@ -447,38 +450,32 @@ gdbsim_fetch_register (struct target_ops *ops,
>       {
> 	/* For moment treat a `does not exist' register the same way
> 	   as an ``unavailable'' register.  */
> -	gdb_byte buf[MAX_REGISTER_SIZE];
> 	int nr_bytes;
> 
> -	memset (buf, 0, MAX_REGISTER_SIZE);
> -	regcache_raw_supply (regcache, regno, buf);
> +	regcache_raw_supply (regcache, regno, buf.data ());
> 	break;
>       }
> 
>     default:
>       {
> 	static int warn_user = 1;
> -	gdb_byte buf[MAX_REGISTER_SIZE];
> 	int nr_bytes;
> 
> 	gdb_assert (regno >= 0 && regno < gdbarch_num_regs (gdbarch));
> -	memset (buf, 0, MAX_REGISTER_SIZE);
> 	nr_bytes = sim_fetch_register (sim_data->gdbsim_desc,
> 				       gdbarch_register_sim_regno
> 					 (gdbarch, regno),
> -				       buf,
> -				       register_size (gdbarch, regno));
> +				       buf.data (), regsize);
> 	if (nr_bytes > 0
> -	    && nr_bytes != register_size (gdbarch, regno) && warn_user)
> +	    && nr_bytes != regsize && warn_user)
> 	  {
> 	    fprintf_unfiltered (gdb_stderr,
> 				"Size of register %s (%d/%d) "
> 				"incorrect (%d instead of %d))",
> 				gdbarch_register_name (gdbarch, regno),
> 				regno,
> -				gdbarch_register_sim_regno
> -				  (gdbarch, regno),
> -				nr_bytes, register_size (gdbarch, regno));
> +				gdbarch_register_sim_regno (gdbarch, regno),
> +				nr_bytes, regsize);
> 	    warn_user = 0;
> 	  }
> 	/* FIXME: cagney/2002-05-27: Should check `nr_bytes == 0'
> @@ -486,13 +483,13 @@ gdbsim_fetch_register (struct target_ops *ops,
> 	   which registers are fetchable.  */
> 	/* Else if (nr_bytes < 0): an old simulator, that doesn't
> 	   think to return the register size.  Just assume all is ok.  */
> -	regcache_raw_supply (regcache, regno, buf);
> +	regcache_raw_supply (regcache, regno, buf.data ());
> 	if (remote_debug)
> 	  {
> 	    fprintf_unfiltered (gdb_stdlog,
> 				"gdbsim_fetch_register: %d", regno);
> 	    /* FIXME: We could print something more intelligible.  */
> -	    dump_mem (buf, register_size (gdbarch, regno));
> +	    dump_mem (buf.data (), regsize);
> 	  }
> 	break;
>       }
> @@ -516,15 +513,14 @@ gdbsim_store_register (struct target_ops *ops,
>     }
>   else if (gdbarch_register_sim_regno (gdbarch, regno) >= 0)
>     {
> -      gdb_byte tmp[MAX_REGISTER_SIZE];
> -      int nr_bytes;
> -
> -      regcache_cooked_read (regcache, regno, tmp);
> -      nr_bytes = sim_store_register (sim_data->gdbsim_desc,
> -				     gdbarch_register_sim_regno
> -				       (gdbarch, regno),
> -				     tmp, register_size (gdbarch, regno));
> -      if (nr_bytes > 0 && nr_bytes != register_size (gdbarch, regno))
> +      int regsize = register_size (gdbarch, regno);
> +      int sim_regno = gdbarch_register_sim_regno (gdbarch, regno);
> +
> +      struct value *value = regcache_cooked_read_value (regcache, regno);
> +      int nr_bytes = sim_store_register (sim_data->gdbsim_desc, sim_regno,
> +					 value_contents_all (value), regsize);
> +
> +      if (nr_bytes > 0 && nr_bytes != regsize)
> 	internal_error (__FILE__, __LINE__,
> 			_("Register size different to expected"));
>       if (nr_bytes < 0)
> @@ -538,8 +534,11 @@ gdbsim_store_register (struct target_ops *ops,
> 	{
> 	  fprintf_unfiltered (gdb_stdlog, "gdbsim_store_register: %d", regno);
> 	  /* FIXME: We could print something more intelligible.  */
> -	  dump_mem (tmp, register_size (gdbarch, regno));
> +	  dump_mem (value_contents_all (value), regsize);
> 	}
> +
> +      release_value (value);
> +      value_free (value);
>     }
> }
>
  
Yao Qi March 24, 2017, 10:41 a.m. UTC | #2
Alan Hayward <Alan.Hayward@arm.com> writes:

> Used a std::vector in the first case and struct value in the second.
>
> Tested using make check with board files unix and native-gdbserver.
>

What is your target triplet?  Do you see remote-sim.o in your build
directory?  remote-sim.c may not be built at all.

> Ok to commit?
>

It is good to me.  This patch is related to SIM, so I'd like Mike to
take a look too.
  
Alan Hayward March 24, 2017, 11:41 a.m. UTC | #3
> On 24 Mar 2017, at 10:41, Yao Qi <qiyaoltc@gmail.com> wrote:

> 

> Alan Hayward <Alan.Hayward@arm.com> writes:

> 

>> Used a std::vector in the first case and struct value in the second.

>> 

>> Tested using make check with board files unix and native-gdbserver.

>> 

> 

> What is your target triplet?  Do you see remote-sim.o in your build

> directory?  remote-sim.c may not be built at all.


You’re correct - I’m not building remote-sim at all!
Forcing SIM_OBS in Makefile.in gives me a compile error (just need to
cast result of value_contents_all to remove the const).

Is there an easy way to build and test remote-sim ?

> 

>> Ok to commit?

>> 

> 

> It is good to me.  This patch is related to SIM, so I'd like Mike to

> take a look too.

> 

> -- 

> Yao (齐尧)
  
Yao Qi March 24, 2017, 11:59 a.m. UTC | #4
On Fri, Mar 24, 2017 at 11:39 AM, Alan Hayward <Alan.Hayward@arm.com> wrote:
>>
>> What is your target triplet?  Do you see remote-sim.o in your build
>> directory?  remote-sim.c may not be built at all.
>
> You’re correct - I’m not building remote-sim at all!
> Forcing SIM_OBS in Makefile.in gives me a compile error (just need to
> cast result of value_contents_all to remove the const).
>
> Is there an easy way to build and test remote-sim ?
>

remote-sim.c is built if I configure gdb --target=arm-none-eabi.
I don't know how to test either, running sim test cases, Mike?
  

Patch

diff --git a/gdb/remote-sim.c b/gdb/remote-sim.c
index b0c68c617e365c0ea18ac2eca525598b688ffbb5..aeceb06a557eb31778d0a85194fb47ea311eec09 100644
--- a/gdb/remote-sim.c
+++ b/gdb/remote-sim.c
@@ -439,6 +439,9 @@  gdbsim_fetch_register (struct target_ops *ops,
       return;
     }

+  int regsize = register_size (gdbarch, regno);
+  std::vector<gdb_byte> buf (regsize);
+
   switch (gdbarch_register_sim_regno (gdbarch, regno))
     {
     case LEGACY_SIM_REGNO_IGNORE:
@@ -447,38 +450,32 @@  gdbsim_fetch_register (struct target_ops *ops,
       {
 	/* For moment treat a `does not exist' register the same way
 	   as an ``unavailable'' register.  */
-	gdb_byte buf[MAX_REGISTER_SIZE];
 	int nr_bytes;

-	memset (buf, 0, MAX_REGISTER_SIZE);
-	regcache_raw_supply (regcache, regno, buf);
+	regcache_raw_supply (regcache, regno, buf.data ());
 	break;
       }

     default:
       {
 	static int warn_user = 1;
-	gdb_byte buf[MAX_REGISTER_SIZE];
 	int nr_bytes;

 	gdb_assert (regno >= 0 && regno < gdbarch_num_regs (gdbarch));
-	memset (buf, 0, MAX_REGISTER_SIZE);
 	nr_bytes = sim_fetch_register (sim_data->gdbsim_desc,
 				       gdbarch_register_sim_regno
 					 (gdbarch, regno),
-				       buf,
-				       register_size (gdbarch, regno));
+				       buf.data (), regsize);
 	if (nr_bytes > 0
-	    && nr_bytes != register_size (gdbarch, regno) && warn_user)
+	    && nr_bytes != regsize && warn_user)
 	  {
 	    fprintf_unfiltered (gdb_stderr,
 				"Size of register %s (%d/%d) "
 				"incorrect (%d instead of %d))",
 				gdbarch_register_name (gdbarch, regno),
 				regno,
-				gdbarch_register_sim_regno
-				  (gdbarch, regno),
-				nr_bytes, register_size (gdbarch, regno));
+				gdbarch_register_sim_regno (gdbarch, regno),
+				nr_bytes, regsize);
 	    warn_user = 0;
 	  }
 	/* FIXME: cagney/2002-05-27: Should check `nr_bytes == 0'
@@ -486,13 +483,13 @@  gdbsim_fetch_register (struct target_ops *ops,
 	   which registers are fetchable.  */
 	/* Else if (nr_bytes < 0): an old simulator, that doesn't
 	   think to return the register size.  Just assume all is ok.  */
-	regcache_raw_supply (regcache, regno, buf);
+	regcache_raw_supply (regcache, regno, buf.data ());
 	if (remote_debug)
 	  {
 	    fprintf_unfiltered (gdb_stdlog,
 				"gdbsim_fetch_register: %d", regno);
 	    /* FIXME: We could print something more intelligible.  */
-	    dump_mem (buf, register_size (gdbarch, regno));
+	    dump_mem (buf.data (), regsize);
 	  }
 	break;
       }
@@ -516,15 +513,14 @@  gdbsim_store_register (struct target_ops *ops,
     }
   else if (gdbarch_register_sim_regno (gdbarch, regno) >= 0)
     {
-      gdb_byte tmp[MAX_REGISTER_SIZE];
-      int nr_bytes;
-
-      regcache_cooked_read (regcache, regno, tmp);
-      nr_bytes = sim_store_register (sim_data->gdbsim_desc,
-				     gdbarch_register_sim_regno
-				       (gdbarch, regno),
-				     tmp, register_size (gdbarch, regno));
-      if (nr_bytes > 0 && nr_bytes != register_size (gdbarch, regno))
+      int regsize = register_size (gdbarch, regno);
+      int sim_regno = gdbarch_register_sim_regno (gdbarch, regno);
+
+      struct value *value = regcache_cooked_read_value (regcache, regno);
+      int nr_bytes = sim_store_register (sim_data->gdbsim_desc, sim_regno,
+					 value_contents_all (value), regsize);
+
+      if (nr_bytes > 0 && nr_bytes != regsize)
 	internal_error (__FILE__, __LINE__,
 			_("Register size different to expected"));
       if (nr_bytes < 0)
@@ -538,8 +534,11 @@  gdbsim_store_register (struct target_ops *ops,
 	{
 	  fprintf_unfiltered (gdb_stdlog, "gdbsim_store_register: %d", regno);
 	  /* FIXME: We could print something more intelligible.  */
-	  dump_mem (tmp, register_size (gdbarch, regno));
+	  dump_mem (value_contents_all (value), regsize);
 	}
+
+      release_value (value);
+      value_free (value);
     }
 }