[PATCH] Replace regbuf with regcache in record-full.c

[Alan Hayward]

PING

> On 14 Jun 2017, at 15:02, Alan Hayward <Alan.Hayward@arm.com> wrote:
> 
> In record-full.c, instead of backing up all the registers into a large
> buffer, duplicate the regcache.
> This enables the removal of an instance of MAX_REGISTER_SIZE.
> 
> Note that regcache_dup() create a read-only copy of a register cache,
> which ensures the new regcache cannot write back to the target.
> 
> Once created, we need to be able to copy registers between the two caches,
> which we want to do without creating a temporary buffer.
> 
> I've added regcache::raw_copy() to allow the copying of raw registers
> between two regcaches - either of which might be set as read-only.
> 
> Alternatively, I could make the new regcache as writable (by enabling a
> regcache copy constructor). But, I think this would be dangerous as it
> it then has the potential to write back to the target if the wrong function
> is called.
> 
> Tested on a --enable-targets=all build with board files unix and
> native-gdbserver.
> 
> 
> Alan.
> 
> 
> 2017-06-14  Alan Hayward  <alan.hayward@arm.com>
> 
> 	* gdb/record-full.c (record_full_core_regbuf): Remove.
> 	(record_full_core_regcache): New.
> 	(record_full_core_open_1): Duplicate regcache
> 	(record_full_close): Delete duplicated regcache.
> 	(record_full_core_fetch_registers): Use duplicated regcache.
> 	(record_full_core_store_registers): Likewise.
> 	* gdb/regcache.c (regcache::raw_copy): New function.
> 	* gdb/regcache.h (regcache::raw_copy): New declaration.
> 
> 
> diff --git a/gdb/record-full.c b/gdb/record-full.c
> index 4f73e2a5ad0d4a2407b31a1d391e813147e15798..00bbda213605110324a6529b9d4d538cd292c62f 100644
> --- a/gdb/record-full.c
> +++ b/gdb/record-full.c
> @@ -167,7 +167,7 @@ struct record_full_core_buf_entry
> };
> 
> /* Record buf with core target.  */
> -static gdb_byte *record_full_core_regbuf = NULL;
> +static struct regcache *record_full_core_regcache = NULL;
> static struct target_section *record_full_core_start;
> static struct target_section *record_full_core_end;
> static struct record_full_core_buf_entry *record_full_core_buf_list = NULL;
> @@ -793,22 +793,17 @@ static void
> record_full_core_open_1 (const char *name, int from_tty)
> {
>   struct regcache *regcache = get_current_regcache ();
> -  int regnum = gdbarch_num_regs (get_regcache_arch (regcache));
> -  int i;
> 
> -  /* Get record_full_core_regbuf.  */
> +  /* Get record_full_core_regcache.  */
>   target_fetch_registers (regcache, -1);
> -  record_full_core_regbuf = (gdb_byte *) xmalloc (MAX_REGISTER_SIZE * regnum);
> -  for (i = 0; i < regnum; i ++)
> -    regcache_raw_collect (regcache, i,
> -			  record_full_core_regbuf + MAX_REGISTER_SIZE * i);
> +  record_full_core_regcache = regcache_dup (regcache);
> 
>   /* Get record_full_core_start and record_full_core_end.  */
>   if (build_section_table (core_bfd, &record_full_core_start,
> 			   &record_full_core_end))
>     {
> -      xfree (record_full_core_regbuf);
> -      record_full_core_regbuf = NULL;
> +      regcache_xfree (record_full_core_regcache);
> +      record_full_core_regcache = NULL;
>       error (_("\"%s\": Can't find sections: %s"),
> 	     bfd_get_filename (core_bfd), bfd_errmsg (bfd_get_error ()));
>     }
> @@ -886,11 +881,11 @@ record_full_close (struct target_ops *self)
> 
>   record_full_list_release (record_full_list);
> 
> -  /* Release record_full_core_regbuf.  */
> -  if (record_full_core_regbuf)
> +  /* Release record_full_core_regcache.  */
> +  if (record_full_core_regcache)
>     {
> -      xfree (record_full_core_regbuf);
> -      record_full_core_regbuf = NULL;
> +      regcache_xfree (record_full_core_regcache);
> +      record_full_core_regcache = NULL;
>     }
> 
>   /* Release record_full_core_buf_list.  */
> @@ -2051,15 +2046,12 @@ record_full_core_fetch_registers (struct target_ops *ops,
>   if (regno < 0)
>     {
>       int num = gdbarch_num_regs (get_regcache_arch (regcache));
> -      int i;
> 
> -      for (i = 0; i < num; i ++)
> -        regcache_raw_supply (regcache, i,
> -                             record_full_core_regbuf + MAX_REGISTER_SIZE * i);
> +      for (int i = 0; i < num; i ++)
> +	regcache->raw_copy (i, record_full_core_regcache);
>     }
>   else
> -    regcache_raw_supply (regcache, regno,
> -                         record_full_core_regbuf + MAX_REGISTER_SIZE * regno);
> +    regcache->raw_copy (regno, record_full_core_regcache);
> }
> 
> /* "to_prepare_to_store" method for prec over corefile.  */
> @@ -2078,8 +2070,7 @@ record_full_core_store_registers (struct target_ops *ops,
>                              int regno)
> {
>   if (record_full_gdb_operation_disable)
> -    regcache_raw_collect (regcache, regno,
> -                          record_full_core_regbuf + MAX_REGISTER_SIZE * regno);
> +    record_full_core_regcache->raw_copy (regno, regcache);
>   else
>     error (_("You can't do that without a process to debug."));
> }
> diff --git a/gdb/regcache.h b/gdb/regcache.h
> index 00b729d9af1b9d9823fc84fb424047c2b918ec8d..24257d8eafd4f7168b622a48ec1e646b9027153e 100644
> --- a/gdb/regcache.h
> +++ b/gdb/regcache.h
> @@ -324,6 +324,8 @@ public:
> 
>   void raw_supply_zeroed (int regnum);
> 
> +  void raw_copy (int regnum, const struct regcache *src_regcache);
> +
>   enum register_status get_register_status (int regnum) const;
> 
>   void raw_set_cached_value (int regnum, const gdb_byte *buf);
> @@ -402,8 +404,8 @@ private:
>   /* Is this a read-only cache?  A read-only cache is used for saving
>      the target's register state (e.g, across an inferior function
>      call or just before forcing a function return).  A read-only
> -     cache can only be updated via the methods regcache_dup() and
> -     regcache_cpy().  The actual contents are determined by the
> +     cache can only be updated via the methods regcache_dup (), regcache_cpy ()
> +     and regcache::raw_copy ().  The actual contents are determined by the
>      reggroup_save and reggroup_restore methods.  */
>   bool m_readonly_p;
>   /* If this is a read-write cache, which thread's registers is
> diff --git a/gdb/regcache.c b/gdb/regcache.c
> index 4e8c1ee0e648fd17ce2a120c61ecd495ff2e2467..54454e3f039ff5d6ab436cd9e0f1da607476ce32 100644
> --- a/gdb/regcache.c
> +++ b/gdb/regcache.c
> @@ -1256,6 +1256,20 @@ regcache::raw_collect (int regnum, void *buf) const
>   memcpy (buf, regbuf, size);
> }
> 
> +/* Collect register REGNUM from SRC_REGCACHE and store its contents in
> +   REGCACHE.  Regcaches must have matching gdbarches.  */
> +
> +void
> +regcache::raw_copy (int regnum, const struct regcache *src_regcache)
> +{
> +  gdb_assert (src_regcache != NULL);
> +  gdb_assert (regnum >= 0 && regnum < m_descr->nr_raw_registers);
> +  gdb_assert (m_descr->gdbarch == src_regcache->m_descr->gdbarch);
> +
> +  src_regcache->raw_collect (regnum, register_buffer (regnum));
> +  m_register_status[regnum] = src_regcache->m_register_status[regnum];
> +}
> +
> /* Transfer a single or all registers belonging to a certain register
>    set to or from a buffer.  This is the main worker function for
>    regcache_supply_regset and regcache_collect_regset.  */
>