Re: [PATCH v2 1/3] Add min size to regset section iterations

[Simon Marchi <simon.marchi@ericsson.com>]

On 2018-07-30 05:25 AM, Alan Hayward wrote:
> When using the regset section iteration functions, the size parameter is used
> in different ways.
> 
> With collect, size is used to create the buffer in which to write the regset.
> (see linux-tdep.c::linux_collect_regset_section_cb).
> 
> With supply, size is used to confirm the existing regset is the correct size.
> If REGSET_VARIABLE_SIZE is set then the regset can be bigger than size.
> Effectively, size is the minimum possible size of the regset.
> (see corelow.c::get_core_register_section).
> 
> There are currently no targets with both REGSET_VARIABLE_SIZE and a collect
> function.
> 
> To allow support of collects for REGSET_VARIABLE_SIZE we need two sizes.
> Min_size is the minimum allowed size for the regset, and size is used as the
> size to use when creating new regsets. For all targets that are not
> REGSET_VARIABLE_SIZE then these two sizes are equal.

Hi Alan,

I am still a bit confused by how these two sizes are used.  Please document
carefully what they each mean, for both the collect and supply case.

Part of my confusion comes the fact that for Aarch64 SVE, we seem to know
in advance the exact size the SVE register section should have, based on
vq (which we read in aarch64_linux_core_read_vq).  In this case, the single
size parameter would be enough for collecting and supplying, since it's the
exact size (not a minimum size).

> diff --git a/gdb/corelow.c b/gdb/corelow.c
> index 059ce2f6eb..32a054ee3e 100644
> --- a/gdb/corelow.c
> +++ b/gdb/corelow.c
> @@ -107,6 +107,7 @@ public:
>  				  const struct regset *regset,
>  				  const char *name,
>  				  int min_size,
> +				  int size,
>  				  int which,
>  				  const char *human_name,
>  				  bool required);
> @@ -570,12 +571,13 @@ core_target::get_core_register_section (struct regcache *regcache,
>  					const struct regset *regset,
>  					const char *name,
>  					int min_size,
> +					int size,
>  					int which,
>  					const char *human_name,
>  					bool required)
>  {>    struct bfd_section *section;
> -  bfd_size_type size;
> +  bfd_size_type core_size;

I would suggest naming this "section_size", or "reg_section_size".  At first I thought it meant
the size of the whole core file.  You could push this rename as an obvious patch right now to
reduce the noise in this patch, since it's a good change on its own.

Simon