Re: [PATCH] Remove MAX_REGISTER_SIZE from regcache.c

[Alan Hayward <Alan.Hayward@arm.com>]

Ping.

> On 24 Feb 2017, at 10:19, Alan Hayward <Alan.Hayward@arm.com> wrote:
> 
> Also adds functionality to calculate maximum register size.
> 
> Cannot remove buffer from regcache_restore becuase we need to keep the
> register value if the read fails.
> 
> Leaves two asserts check with MAX_REGISTER_SIZE which must be kept
> until MAX_REGISTER_SIZE is fully removed from all files.
> 
> Tested using make check with board files unix and native-gdbserver.
> 
> Ok to commit?
> 
> Alan.
> 
> 2017-02-24  Alan Hayward  <alan.hayward@arm.com>
> 
> 	* regcache.c (struct regcache_descr): Add max_register_size
> 	(init_regcache_descr): Calculate maximum register size.
> 	(max_register_size): New function.
> 	(regcache_save): Avoid buffer use.
> 	(regcache_restore): Use vec instead of array.
> 	(regcache_dump): Avoid buffer use.
> 	* regcache.h (struct regcache_descr): Add max_register_size
> 	(max_register_size): New function.
> 
> 
> index 59233308f926ebd52db9958cba168daacc77c1ee..a4a47c8f738ee5df69074375a3f169558891cf0e 100644
> --- a/gdb/regcache.h
> +++ b/gdb/regcache.h
> @@ -206,6 +206,8 @@ extern struct type *register_type (struct gdbarch *gdbarch, int regnum);
> 
> extern int register_size (struct gdbarch *gdbarch, int regnum);
> 
> +/* Return the size of the largest register.  */
> +extern long max_register_size (struct gdbarch *gdbarch);
> 
> /* Save/restore a register cache.  The set of registers saved /
>    restored into the DST regcache determined by the save_reggroup /
> diff --git a/gdb/regcache.c b/gdb/regcache.c
> index 31aa1baf7ef69c27c00e45e3c8d4eb3c41dc4203..0da184cb7c6ea3a171139a40676658322e62ad2e 100644
> --- a/gdb/regcache.c
> +++ b/gdb/regcache.c
> @@ -73,6 +73,9 @@ struct regcache_descr
> 
>   /* Cached table containing the type of each register.  */
>   struct type **register_type;
> +
> +  /* Size of the largest register.  */
> +  long max_register_size;
> };
> 
> static void *
> @@ -126,6 +129,8 @@ init_regcache_descr (struct gdbarch *gdbarch)
> 	descr->register_offset[i] = offset;
> 	offset += descr->sizeof_register[i];
> 	gdb_assert (MAX_REGISTER_SIZE >= descr->sizeof_register[i]);
> +	descr->max_register_size = std::max (descr->max_register_size,
> +					     descr->sizeof_register[i]);
>       }
>     /* Set the real size of the raw register cache buffer.  */
>     descr->sizeof_raw_registers = offset;
> @@ -136,6 +141,8 @@ init_regcache_descr (struct gdbarch *gdbarch)
> 	descr->register_offset[i] = offset;
> 	offset += descr->sizeof_register[i];
> 	gdb_assert (MAX_REGISTER_SIZE >= descr->sizeof_register[i]);
> +	descr->max_register_size = std::max (descr->max_register_size,
> +					     descr->sizeof_register[i]);
>       }
>     /* Set the real size of the readonly register cache buffer.  */
>     descr->sizeof_cooked_registers = offset;
> @@ -187,6 +194,13 @@ regcache_register_size (const struct regcache *regcache, int n)
>   return register_size (get_regcache_arch (regcache), n);
> }
> 
> +long
> +max_register_size (struct gdbarch *gdbarch)
> +{
> +  struct regcache_descr *descr = regcache_descr (gdbarch);
> +  return descr->max_register_size;
> +}
> +
> /* The register cache for storing raw register values.  */
> 
> struct regcache
> @@ -327,7 +341,6 @@ regcache_save (struct regcache *dst, regcache_cooked_read_ftype *cooked_read,
> 	       void *src)
> {
>   struct gdbarch *gdbarch = dst->descr->gdbarch;
> -  gdb_byte buf[MAX_REGISTER_SIZE];
>   int regnum;
> 
>   /* The DST should be `read-only', if it wasn't then the save would
> @@ -346,17 +359,13 @@ regcache_save (struct regcache *dst, regcache_cooked_read_ftype *cooked_read,
>     {
>       if (gdbarch_register_reggroup_p (gdbarch, regnum, save_reggroup))
> 	{
> -	  enum register_status status = cooked_read (src, regnum, buf);
> +	  gdb_byte *dst_buf = register_buffer (dst, regnum);
> +	  enum register_status status = cooked_read (src, regnum, dst_buf);
> 
> -	  if (status == REG_VALID)
> -	    memcpy (register_buffer (dst, regnum), buf,
> -		    register_size (gdbarch, regnum));
> -	  else
> +	  if (status != REG_VALID)
> 	    {
> 	      gdb_assert (status != REG_UNKNOWN);
> -
> -	      memset (register_buffer (dst, regnum), 0,
> -		      register_size (gdbarch, regnum));
> +	      memset (dst_buf, 0, register_size (gdbarch, regnum));
> 	    }
> 	  dst->register_status[regnum] = status;
> 	}
> @@ -369,7 +378,7 @@ regcache_restore (struct regcache *dst,
> 		  void *cooked_read_context)
> {
>   struct gdbarch *gdbarch = dst->descr->gdbarch;
> -  gdb_byte buf[MAX_REGISTER_SIZE];
> +  std::vector<gdb_byte> buf (max_register_size (gdbarch));
>   int regnum;
> 
>   /* The dst had better not be read-only.  If it is, the `restore'
> @@ -385,9 +394,9 @@ regcache_restore (struct regcache *dst,
> 	{
> 	  enum register_status status;
> 
> -	  status = cooked_read (cooked_read_context, regnum, buf);
> +	  status = cooked_read (cooked_read_context, regnum, buf.data ());
> 	  if (status == REG_VALID)
> -	    regcache_cooked_write (dst, regnum, buf);
> +	    regcache_cooked_write (dst, regnum, buf.data ());
> 	}
>     }
> }
> @@ -1324,7 +1333,6 @@ regcache_dump (struct regcache *regcache, struct ui_file *file,
>   int footnote_register_offset = 0;
>   int footnote_register_type_name_null = 0;
>   long register_offset = 0;
> -  gdb_byte buf[MAX_REGISTER_SIZE];
> 
> #if 0
>   fprintf_unfiltered (file, "nr_raw_registers %d\n",
> @@ -1451,8 +1459,8 @@ regcache_dump (struct regcache *regcache, struct ui_file *file,
> 	    fprintf_unfiltered (file, "<unavailable>");
> 	  else
> 	    {
> -	      regcache_raw_read (regcache, regnum, buf);
> -	      print_hex_chars (file, buf,
> +	      regcache_raw_update (regcache, regnum);
> +	      print_hex_chars (file, register_buffer (regcache, regnum),
> 			       regcache->descr->sizeof_register[regnum],
> 			       gdbarch_byte_order (gdbarch));
> 	    }
> @@ -1465,17 +1473,19 @@ regcache_dump (struct regcache *regcache, struct ui_file *file,
> 	    fprintf_unfiltered (file, "Cooked value");
> 	  else
> 	    {
> -	      enum register_status status;
> +	      struct value *value = regcache_cooked_read_value (regcache,
> +								regnum);
> 
> -	      status = regcache_cooked_read (regcache, regnum, buf);
> -	      if (status == REG_UNKNOWN)
> -		fprintf_unfiltered (file, "<invalid>");
> -	      else if (status == REG_UNAVAILABLE)
> +	      if (value_optimized_out (value)
> +		  || !value_entirely_available (value))
> 		fprintf_unfiltered (file, "<unavailable>");
> 	      else
> -		print_hex_chars (file, buf,
> +		print_hex_chars (file, value_contents_all (value),
> 				 regcache->descr->sizeof_register[regnum],
> 				 gdbarch_byte_order (gdbarch));
> +
> +	      release_value (value);
> +	      value_free (value);
> 	    }
> 	}
>