[PATCH 3/3] Calculate max register size

[Luis Machado]

On 01/09/2017 04:58 AM, Alan Hayward wrote:
> Aarch64 SVE requires a max register size of 256. The current max size in gdb
> is 64. This is part of a series demonstrating the replacement of
> MAX_REGISTER_SIZE.
>
> In cases where a buffer is created to be used multiple times to hold different
> registers, then the maximum register size is required. Add a max register value
> to the regcache which is calculated on initialization.
>
> This patch is restricted to remote.c and regcache.c.
> Follow on patches will expand to other files.
>
> Tested on x86.
> Ok to commit?
>
> Thanks,
> Alan.
>
> 2017-01-09  Alan Hayward  <alan.hayward@arm.com>
>
> 	* regcache.c (struct regcache_descr): Add max_register_size
> 	(max_register_size): New.
> 	(init_regcache_descr): Find max register size.
> 	(regcache_save): Use max_register_size.
> 	(regcache_restore): Likewise.
> 	(regcache_dump): Likewise.
> 	* regcache.h (max_register_size): New.
> 	* remote.c (remote_prepare_to_store): Allocate buffer.
>
>
> diff --git a/gdb/regcache.h b/gdb/regcache.h
> index e5a7cf553279b8cc0d546ec1b8274cbf97e246d5..4db9517a9dd464d9c43be2af0573b767b86bfb56 100644
> --- a/gdb/regcache.h
> +++ b/gdb/regcache.h
> @@ -202,6 +202,9 @@ 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.  */
> +

Spurious newline.

> +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 9d28aa2c2114e0f1c52758bb2fbe9669a329c13e..46d017c7b2abcb18c9cdda005749071328735dbd 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;

Is this ever negative? Why not make it unsigned?

>  };
>
>  static void *
> @@ -125,7 +128,9 @@ init_regcache_descr (struct gdbarch *gdbarch)
>  	descr->sizeof_register[i] = TYPE_LENGTH (descr->register_type[i]);
>  	descr->register_offset[i] = offset;
>  	offset += descr->sizeof_register[i];
> -	gdb_assert (MAX_REGISTER_SIZE >= 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;
> @@ -135,7 +140,9 @@ init_regcache_descr (struct gdbarch *gdbarch)
>  	descr->sizeof_register[i] = TYPE_LENGTH (descr->register_type[i]);
>  	descr->register_offset[i] = offset;
>  	offset += descr->sizeof_register[i];
> -	gdb_assert (MAX_REGISTER_SIZE >= 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

Same as above, is this ever negative?

> +max_register_size (struct gdbarch *gdbarch)
> +{
> +  struct regcache_descr *descr = regcache_descr (gdbarch);

Is descr ever NULL?

> +  return descr->max_register_size;
> +}
> +
>  /* The register cache for storing raw register values.  */
>
>  struct regcache
> @@ -327,7 +341,7 @@ 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];
> +  gdb_byte *buf = (gdb_byte *) alloca (max_register_size (gdbarch));
>    int regnum;
>
>    /* The DST should be `read-only', if it wasn't then the save would
> @@ -369,7 +383,7 @@ regcache_restore (struct regcache *dst,
>  		  void *cooked_read_context)
>  {
>    struct gdbarch *gdbarch = dst->descr->gdbarch;
> -  gdb_byte buf[MAX_REGISTER_SIZE];
> +  gdb_byte *buf = (gdb_byte *) alloca (max_register_size (gdbarch));
>    int regnum;
>
>    /* The dst had better not be read-only.  If it is, the `restore'
> @@ -1279,7 +1293,7 @@ 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];
> +  gdb_byte *buf = (gdb_byte *) alloca (max_register_size (gdbarch));
>
>  #if 0
>    fprintf_unfiltered (file, "nr_raw_registers %d\n",
> diff --git a/gdb/remote.c b/gdb/remote.c
> index 9247d43b094925ff397eb36b450eaba521adfc99..86856e6a6aba1967faaa8ef547f8a48fcc63c383 100644
> --- a/gdb/remote.c
> +++ b/gdb/remote.c
> @@ -7752,9 +7752,10 @@ remote_fetch_registers (struct target_ops *ops,
>  static void
>  remote_prepare_to_store (struct target_ops *self, struct regcache *regcache)
>  {
> +  struct gdbarch *gdbarch = get_regcache_arch (regcache);
>    struct remote_arch_state *rsa = get_remote_arch_state ();
>    int i;
> -  gdb_byte buf[MAX_REGISTER_SIZE];
> +  gdb_byte *buf = (gdb_byte *) alloca (max_register_size (gdbarch));
>
>    /* Make sure the entire registers array is valid.  */
>    switch (packet_support (PACKET_P))
>
>
>

My comment is the same as 2/3. Should we use a different data structure 
that can grow/shrink as one wishes?