Re: [PATCH 2/3] Allocate data in cached_reg_t

[Luis Machado <lgustavo at codesourcery dot com>]

On 01/09/2017 04:56 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 cached_reg_t the data is changed to a pointer, which is allocated using the
> size of the register being cached. This pointer must be manually freed when
> deleting a DEF_VEC of cached_reg_t's.
>
> Tested on x86.
> Ok to commit?
>
> Thanks,
> Alan.
>
> 2017-01-09  Alan Hayward  <alan.hayward@arm.com>
>
> 	* remote.c (struct cached_reg): Change data into a pointer.
> 	* (stop_reply_dtr): Free data pointers before deleting vector.
> 	(process_stop_reply): Likewise.
> 	(remote_parse_stop_reply): Allocate space for data
>
>
> diff --git a/gdb/remote.c b/gdb/remote.c
> index 6da6eb366ae442354fd6a37741335af9a4a5a056..9247d43b094925ff397eb36b450eaba521adfc99 100644
> --- a/gdb/remote.c
> +++ b/gdb/remote.c
> @@ -6306,7 +6306,7 @@ remote_console_output (char *msg)
>  typedef struct cached_reg
>  {
>    int num;
> -  gdb_byte data[MAX_REGISTER_SIZE];
> +  gdb_byte *data;

Would it make sense to go C++ and use a data structure that can take 
care of variable sizes? Just thinking if that would be easier than 
handling allocation/deallocation of the data.

>  } cached_reg_t;
>
>  DEF_VEC_O(cached_reg_t);
> @@ -6402,6 +6402,13 @@ static void
>  stop_reply_dtr (struct notif_event *event)
>  {
>    struct stop_reply *r = (struct stop_reply *) event;
> +  cached_reg_t *reg;
> +  int ix;
> +
> +  for (ix = 0;
> +       VEC_iterate(cached_reg_t, r->regcache, ix, reg);
> +       ix++)
> +    xfree (reg->data);
>
>    VEC_free (cached_reg_t, r->regcache);
>  }
> @@ -6974,6 +6981,7 @@ Packet: '%s'\n"),
>  		{
>  		  struct packet_reg *reg = packet_reg_from_pnum (rsa, pnum);
>  		  cached_reg_t cached_reg;
> +		  struct gdbarch *gdbarch = target_gdbarch ();
>
>  		  if (reg == NULL)
>  		    error (_("Remote sent bad register number %s: %s\n\
> @@ -6981,14 +6989,14 @@ Packet: '%s'\n"),
>  			   hex_string (pnum), p, buf);
>
>  		  cached_reg.num = reg->regnum;
> +		  cached_reg.data = (gdb_byte *)
> +		    xmalloc (register_size (gdbarch, reg->regnum));
>
>  		  p = p1 + 1;
>  		  fieldsize = hex2bin (p, cached_reg.data,
> -				       register_size (target_gdbarch (),
> -						      reg->regnum));
> +				       register_size (gdbarch, reg->regnum));
>  		  p += 2 * fieldsize;
> -		  if (fieldsize < register_size (target_gdbarch (),
> -						 reg->regnum))
> +		  if (fieldsize < register_size (gdbarch, reg->regnum))
>  		    warning (_("Remote reply is too short: %s"), buf);
>
>  		  VEC_safe_push (cached_reg_t, event->regcache, &cached_reg);
> @@ -7211,7 +7219,11 @@ process_stop_reply (struct stop_reply *stop_reply,
>  	  for (ix = 0;
>  	       VEC_iterate(cached_reg_t, stop_reply->regcache, ix, reg);
>  	       ix++)
> +	  {
>  	    regcache_raw_supply (regcache, reg->num, reg->data);
> +	    xfree (reg->data);
> +	  }
> +
>  	  VEC_free (cached_reg_t, stop_reply->regcache);
>  	}