public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Andrew Burgess <aburgess@redhat.com>
To: Simon Marchi <simon.marchi@polymtl.ca>, gdb-patches@sourceware.org
Cc: Simon Marchi <simon.marchi@efficios.com>
Subject: Re: [PATCH 05/24] gdb: change regcache interface to use array_view
Date: Mon, 13 Nov 2023 13:43:12 +0000	[thread overview]
Message-ID: <878r71sr9r.fsf@redhat.com> (raw)
In-Reply-To: <20231108051222.1275306-6-simon.marchi@polymtl.ca>


I accidentally sent this email only to Simon originally, dropping
gdb-patches.  Re-sending it now, including the list.

Simon Marchi <simon.marchi@polymtl.ca> writes:

> From: Simon Marchi <simon.marchi@efficios.com>
>
> Change most of regcache (and base classes) to use array_view when
> possible, instead of raw pointers.  By propagating the use of array_view
> further, it enables having some runtime checks to make sure the what we
> read from or write to regcaches has the expected length (such as the one
> in the `copy(array_view, array_view)` function.  It also integrates well
> when connecting with other APIs already using gdb::array_view.
>
> Add some overloads of the methods using raw pointers to avoid having to
> change all call sites at once (which is both a lot of work and risky).
>
> I tried to do this change in small increments, but since many of these
> functions use each other, it ended up simpler to do it in one shot than
> having a lot of intermediary / transient changes.
>
> This change extends into gdbserver as well, because there is some part
> of the regcache interface that is shared.
>
> Changing the reg_buffer_common interface to use array_view caused some
> build failures in nat/aarch64-scalable-linux-ptrace.c.  That file
> currently "takes advantage" of the fact that
> reg_buffer_common::{raw_supply,raw_collect} operates on `void *`, which
> IMO is dangerous.  It uses raw_supply/raw_collect directly on
> uint64_t's, which I guess is fine because it is expected that native
> code will have the same endianness as the debugged process.  To
> accomodate that, add some overloads of raw_collect and raw_supply that
> work on uint64_t.
>
> This file also uses raw_collect and raw_supply on `char` pointers.
> Change it to use `gdb_byte` pointers instead.  Add overloads of
> raw_collect and raw_supply that work on `gdb_byte *` and make an
> array_view on the fly using the register's size.  Those call sites could
> be converted to use array_view with not much work, in which case these
> overloads could be removed, but I didn't want to do it in this patch, to
> avoid starting to dig in arch-specific code.
>
> Change-Id: I9005f04114543ddff738949e12d85a31855304c2
> ---
>  gdb/frame.c                             |   4 +-
>  gdb/nat/aarch64-scalable-linux-ptrace.c |  20 +-
>  gdb/regcache.c                          | 478 ++++++++++++++----------
>  gdb/regcache.h                          | 113 ++++--
>  gdbserver/regcache.cc                   |  50 +--
>  gdbserver/regcache.h                    |   6 +-
>  gdbsupport/common-regcache.h            |  41 +-
>  gdbsupport/rsp-low.cc                   |   8 +
>  gdbsupport/rsp-low.h                    |   2 +
>  9 files changed, 451 insertions(+), 271 deletions(-)
>
> diff --git a/gdb/frame.c b/gdb/frame.c
> index 7077016ccba4..afadb8ac4e73 100644
> --- a/gdb/frame.c
> +++ b/gdb/frame.c
> @@ -1107,9 +1107,9 @@ get_frame_func (frame_info_ptr this_frame)
>  std::unique_ptr<readonly_detached_regcache>
>  frame_save_as_regcache (frame_info_ptr this_frame)
>  {
> -  auto cooked_read = [this_frame] (int regnum, gdb_byte *buf)
> +  auto cooked_read = [this_frame] (int regnum, gdb::array_view<gdb_byte> buf)
>      {
> -      if (!deprecated_frame_register_read (this_frame, regnum, buf))
> +      if (!deprecated_frame_register_read (this_frame, regnum, buf.data ()))
>  	return REG_UNAVAILABLE;
>        else
>  	return REG_VALID;
> diff --git a/gdb/nat/aarch64-scalable-linux-ptrace.c b/gdb/nat/aarch64-scalable-linux-ptrace.c
> index dc0e45fa91ee..b8fb317edaca 100644
> --- a/gdb/nat/aarch64-scalable-linux-ptrace.c
> +++ b/gdb/nat/aarch64-scalable-linux-ptrace.c
> @@ -613,7 +613,7 @@ aarch64_sve_regs_copy_to_reg_buf (int tid, struct reg_buffer_common *reg_buf)
>  {
>    gdb::byte_vector sve_state = aarch64_fetch_sve_regset (tid);
>  
> -  char *base = (char *) sve_state.data ();
> +  gdb_byte *base = sve_state.data ();
>    struct user_sve_header *header
>      = (struct user_sve_header *) sve_state.data ();
>  
> @@ -684,8 +684,10 @@ aarch64_sve_regs_copy_to_reg_buf (int tid, struct reg_buffer_common *reg_buf)
>  	  reg_buf->raw_supply (AARCH64_SVE_Z0_REGNUM + i, reg);
>  	}
>  
> -      reg_buf->raw_supply (AARCH64_FPSR_REGNUM, &fpsimd->fpsr);
> -      reg_buf->raw_supply (AARCH64_FPCR_REGNUM, &fpsimd->fpcr);
> +      reg_buf->raw_supply (AARCH64_FPSR_REGNUM,
> +			   (const gdb_byte *) &fpsimd->fpsr);
> +      reg_buf->raw_supply (AARCH64_FPCR_REGNUM,
> +			   (const gdb_byte *) &fpsimd->fpcr);
>  
>        /* Clear the SVE only registers.  */
>        memset (reg, 0, SVE_PT_SVE_ZREG_SIZE (vq));
> @@ -720,7 +722,7 @@ aarch64_sve_regs_copy_from_reg_buf (int tid,
>    gdb::byte_vector new_state (SVE_PT_SIZE (32, SVE_PT_REGS_SVE), 0);
>    memcpy (new_state.data (), sve_state.data (), sve_state.size ());
>    header = (struct user_sve_header *) new_state.data ();
> -  char *base = (char *) new_state.data ();
> +  gdb_byte *base = new_state.data ();
>  
>    /* Sanity check the data in the header.  */
>    if (!sve_vl_valid (header->vl)
> @@ -805,9 +807,11 @@ aarch64_sve_regs_copy_from_reg_buf (int tid,
>  	    }
>  
>  	  if (REG_VALID == reg_buf->get_register_status (AARCH64_FPSR_REGNUM))
> -	    reg_buf->raw_collect (AARCH64_FPSR_REGNUM, &fpsimd->fpsr);
> +	    reg_buf->raw_collect (AARCH64_FPSR_REGNUM,
> +				  (gdb_byte *) &fpsimd->fpsr);
>  	  if (REG_VALID == reg_buf->get_register_status (AARCH64_FPCR_REGNUM))
> -	    reg_buf->raw_collect (AARCH64_FPCR_REGNUM, &fpsimd->fpcr);
> +	    reg_buf->raw_collect (AARCH64_FPCR_REGNUM,
> +				  (gdb_byte *) &fpsimd->fpcr);
>  
>  	  /* At this point we have collected all the data from the register
>  	     cache and we are ready to update the FPSIMD register content
> @@ -894,7 +898,7 @@ aarch64_za_regs_copy_to_reg_buf (int tid, struct reg_buffer_common *reg_buf,
>    /* Sanity check.  */
>    gdb_assert (!za_state.empty ());
>  
> -  char *base = (char *) za_state.data ();
> +  gdb_byte *base = za_state.data ();
>    struct user_za_header *header = (struct user_za_header *) base;
>  
>    /* If we have ZA state, read it.  Otherwise, make the contents of ZA
> @@ -1027,7 +1031,7 @@ aarch64_za_regs_copy_from_reg_buf (int tid,
>        /* Fetch the current ZA state from the thread.  */
>        gdb::byte_vector za_state = aarch64_fetch_za_regset (tid);
>  
> -      char *base = (char *) za_state.data ();
> +      gdb_byte *base = za_state.data ();
>        struct user_za_header *za_header = (struct user_za_header *) base;
>        uint64_t svq = sve_vq_from_vl (za_header->vl);
>  
> diff --git a/gdb/regcache.c b/gdb/regcache.c
> index ae5411ba5563..4f3881386f34 100644
> --- a/gdb/regcache.c
> +++ b/gdb/regcache.c
> @@ -221,10 +221,9 @@ regcache::regcache (inferior *inf_for_target_calls, gdbarch *gdbarch,
>  
>  readonly_detached_regcache::readonly_detached_regcache (regcache &src)
>    : readonly_detached_regcache (src.arch (),
> -				[&src] (int regnum, gdb_byte *buf)
> -				  {
> -				    return src.cooked_read (regnum, buf);
> -				  })
> +				[&src] (int regnum,
> +					gdb::array_view<gdb_byte> buf)
> +				  { return src.cooked_read (regnum, buf, 1.0f); })
>  {
>  }
>  
> @@ -234,19 +233,36 @@ reg_buffer::arch () const
>    return m_descr->gdbarch;
>  }
>  
> -/* Return  a pointer to register REGNUM's buffer cache.  */
> +template<typename ElemType>
> +gdb::array_view<ElemType>
> +reg_buffer::register_buffer (int regnum) const
> +{
> +  assert_regnum (regnum);
> +  ElemType *start = &m_registers[m_descr->register_offset[regnum]];
> +  int size = m_descr->sizeof_register[regnum];
> +  return gdb::array_view<ElemType> (start, size);
> +}
>  
> -gdb_byte *
> +/* See regcache.h.  */
> +
> +gdb::array_view<const gdb_byte>
>  reg_buffer::register_buffer (int regnum) const
>  {
> -  return m_registers.get () + m_descr->register_offset[regnum];
> +  return register_buffer<const gdb_byte> (regnum);
> +}
> +
> +/* See regcache.h.  */
> +
> +gdb::array_view<gdb_byte>
> +reg_buffer::register_buffer (int regnum)
> +{
> +  return register_buffer<gdb_byte> (regnum);
>  }
>  
>  void
>  reg_buffer::save (register_read_ftype cooked_read)
>  {
>    struct gdbarch *gdbarch = m_descr->gdbarch;
> -  int regnum;
>  
>    /* It should have pseudo registers.  */
>    gdb_assert (m_has_pseudo);
> @@ -257,17 +273,17 @@ reg_buffer::save (register_read_ftype cooked_read)
>       save_reggroup) and mark them as valid.  The full [0 .. gdbarch_num_regs +
>       gdbarch_num_pseudo_regs) range is checked since some architectures need
>       to save/restore `cooked' registers that live in memory.  */
> -  for (regnum = 0; regnum < m_descr->nr_cooked_registers; regnum++)
> +  for (int regnum = 0; regnum < m_descr->nr_cooked_registers; regnum++)
>      {
>        if (gdbarch_register_reggroup_p (gdbarch, regnum, save_reggroup))
>  	{
> -	  gdb_byte *dst_buf = register_buffer (regnum);
> -	  enum register_status status = cooked_read (regnum, dst_buf);
> +	  gdb::array_view<gdb_byte> dst_buf = register_buffer (regnum);
> +	  register_status status = cooked_read (regnum, dst_buf);
>  
>  	  gdb_assert (status != REG_UNKNOWN);
>  
>  	  if (status != REG_VALID)
> -	    memset (dst_buf, 0, register_size (gdbarch, regnum));
> +	    memset (dst_buf.data (), 0, dst_buf.size ());
>  
>  	  m_register_status[regnum] = status;
>  	}
> @@ -294,7 +310,7 @@ regcache::restore (readonly_detached_regcache *src)
>        if (gdbarch_register_reggroup_p (gdbarch, regnum, restore_reggroup))
>  	{
>  	  if (src->m_register_status[regnum] == REG_VALID)
> -	    cooked_write (regnum, src->register_buffer (regnum));
> +	    cooked_write (regnum, src->register_buffer (regnum), 1.0f);
>  	}
>      }
>  }
> @@ -610,21 +626,30 @@ regcache::raw_update (int regnum)
>      }
>  }
>  
> -enum register_status
> -readable_regcache::raw_read (int regnum, gdb_byte *buf)
> +register_status
> +readable_regcache::raw_read (int regnum, gdb::array_view<gdb_byte> dst, float)
>  {
> -  gdb_assert (buf != NULL);
> +  assert_regnum (regnum);
> +  gdb_assert (dst.size () == m_descr->sizeof_register[regnum]);
> +
>    raw_update (regnum);
>  
>    if (m_register_status[regnum] != REG_VALID)
> -    memset (buf, 0, m_descr->sizeof_register[regnum]);
> +    memset (dst.data (), 0, dst.size ());
>    else
> -    memcpy (buf, register_buffer (regnum),
> -	    m_descr->sizeof_register[regnum]);
> +    copy (register_buffer (regnum), dst);
>  
>    return m_register_status[regnum];
>  }
>  
> +register_status
> +readable_regcache::raw_read (int regnum, gdb_byte *dst)
> +{
> +  assert_regnum (regnum);
> +  int size = m_descr->sizeof_register[regnum];
> +  return raw_read (regnum, gdb::make_array_view (dst, size), 1.0f);
> +}
> +
>  enum register_status
>  regcache_raw_read_signed (struct regcache *regcache, int regnum, LONGEST *val)
>  {
> @@ -639,12 +664,14 @@ readable_regcache::raw_read (int regnum, T *val)
>    assert_regnum (regnum);
>    size_t len = m_descr->sizeof_register[regnum];
>    gdb_byte *buf = (gdb_byte *) alloca (len);
> -  register_status status = raw_read (regnum, buf);
> +  auto view = gdb::make_array_view (buf, len);
> +  register_status status = raw_read (regnum, view, 1.0f);
> +
>    if (status == REG_VALID)
> -    *val = extract_integer<T> ({buf, len},
> -			       gdbarch_byte_order (m_descr->gdbarch));
> +    *val = extract_integer<T> (view, gdbarch_byte_order (m_descr->gdbarch));
>    else
>      *val = 0;
> +
>    return status;
>  }
>  
> @@ -668,13 +695,13 @@ template<typename T, typename>
>  void
>  regcache::raw_write (int regnum, T val)
>  {
> -  gdb_byte *buf;
> -
>    assert_regnum (regnum);
> -  buf = (gdb_byte *) alloca (m_descr->sizeof_register[regnum]);
> -  store_integer (buf, m_descr->sizeof_register[regnum],
> -		 gdbarch_byte_order (m_descr->gdbarch), val);
> -  raw_write (regnum, buf);
> +
> +  int len = m_descr->sizeof_register[regnum];
> +  gdb_byte *buf = (gdb_byte *) alloca (m_descr->sizeof_register[regnum]);
> +  auto view = gdb::make_array_view (buf, len);
> +  store_integer (view, gdbarch_byte_order (m_descr->gdbarch), val);
> +  raw_write (regnum, view, 1.0f);
>  }
>  
>  void
> @@ -698,47 +725,61 @@ regcache_raw_get_signed (struct regcache *regcache, int regnum)
>    return value;
>  }
>  
> -enum register_status
> -readable_regcache::cooked_read (int regnum, gdb_byte *buf)
> +/* See regcache.h.  */
> +
> +register_status
> +readable_regcache::cooked_read (int regnum, gdb::array_view<gdb_byte> dst,
> +				float)
>  {
>    gdb_assert (regnum >= 0);
>    gdb_assert (regnum < m_descr->nr_cooked_registers);
> +
>    if (regnum < num_raw_registers ())
> -    return raw_read (regnum, buf);
> -  else if (m_has_pseudo
> -	   && m_register_status[regnum] != REG_UNKNOWN)
> +    return raw_read (regnum, dst, 1.0f);
> +
> +  gdb_assert (dst.size () == m_descr->sizeof_register[regnum]);
> +
> +  if (m_has_pseudo && m_register_status[regnum] != REG_UNKNOWN)
>      {
>        if (m_register_status[regnum] == REG_VALID)
> -	memcpy (buf, register_buffer (regnum),
> -		m_descr->sizeof_register[regnum]);
> +	copy (register_buffer (regnum), dst);
>        else
> -	memset (buf, 0, m_descr->sizeof_register[regnum]);
> +	memset (dst.data (), 0, dst.size ());
>  
>        return m_register_status[regnum];
>      }
>    else if (gdbarch_pseudo_register_read_value_p (m_descr->gdbarch))
>      {
> -      struct value *computed;
> -      enum register_status result = REG_VALID;
> -
> +      register_status result = REG_VALID;
>        scoped_value_mark mark;
> +      value *computed
> +	= gdbarch_pseudo_register_read_value (m_descr->gdbarch, this, regnum);
>  
> -      computed = gdbarch_pseudo_register_read_value (m_descr->gdbarch,
> -						     this, regnum);
>        if (computed->entirely_available ())
> -	memcpy (buf, computed->contents_raw ().data (),
> -		m_descr->sizeof_register[regnum]);
> +	copy (computed->contents_raw (), dst);
>        else
>  	{
> -	  memset (buf, 0, m_descr->sizeof_register[regnum]);
> +	  memset (dst.data (), 0, dst.size ());
>  	  result = REG_UNAVAILABLE;
>  	}
>  
>        return result;
>      }
>    else
> -    return gdbarch_pseudo_register_read (m_descr->gdbarch, this,
> -					 regnum, buf);
> +    return gdbarch_pseudo_register_read (m_descr->gdbarch, this, regnum,
> +					 dst.data ());
> +}
> +
> +/* See regcache.h.  */
> +
> +register_status
> +readable_regcache::cooked_read (int regnum, gdb_byte *dst)
> +{
> +  gdb_assert (regnum >= 0);
> +  gdb_assert (regnum < m_descr->nr_cooked_registers);
> +
> +  int size = m_descr->sizeof_register[regnum];
> +  return cooked_read (regnum, gdb::make_array_view (dst, size), 1.0f);
>  }
>  
>  struct value *
> @@ -760,8 +801,8 @@ readable_regcache::cooked_read_value (int regnum)
>        /* It is more efficient in general to do this delegation in this
>  	 direction than in the other one, even though the value-based
>  	 API is preferred.  */
> -      if (cooked_read (regnum,
> -		       result->contents_raw ().data ()) == REG_UNAVAILABLE)
> +      if (cooked_read (regnum, result->contents_raw (), 1.0f)
> +	  == REG_UNAVAILABLE)
>  	result->mark_bytes_unavailable (0,
>  					result->type ()->length ());
>  
> @@ -787,10 +828,10 @@ readable_regcache::cooked_read (int regnum, T *val)
>    gdb_assert (regnum >= 0 && regnum < m_descr->nr_cooked_registers);
>    size_t len = m_descr->sizeof_register[regnum];
>    gdb_byte *buf = (gdb_byte *) alloca (len);
> -  register_status status = cooked_read (regnum, buf);
> +  auto view = gdb::make_array_view (buf, len);
> +  register_status status = cooked_read (regnum, view, 1.0f);
>    if (status == REG_VALID)
> -    *val = extract_integer<T> ({buf, len},
> -			       gdbarch_byte_order (m_descr->gdbarch));
> +    *val = extract_integer<T> (view, gdbarch_byte_order (m_descr->gdbarch));
>    else
>      *val = 0;
>    return status;
> @@ -816,13 +857,14 @@ template<typename T, typename>
>  void
>  regcache::cooked_write (int regnum, T val)
>  {
> -  gdb_byte *buf;
> +  gdb_assert (regnum >= 0);
> +  gdb_assert (regnum < m_descr->nr_cooked_registers);
>  
> -  gdb_assert (regnum >=0 && regnum < m_descr->nr_cooked_registers);
> -  buf = (gdb_byte *) alloca (m_descr->sizeof_register[regnum]);
> -  store_integer (buf, m_descr->sizeof_register[regnum],
> -		 gdbarch_byte_order (m_descr->gdbarch), val);
> -  cooked_write (regnum, buf);
> +  int size = m_descr->sizeof_register[regnum];
> +  gdb_byte *buf = (gdb_byte *) alloca (size);
> +  auto view = gdb::make_array_view (buf, size);
> +  store_integer (view, gdbarch_byte_order (m_descr->gdbarch), val);
> +  cooked_write (regnum, view, 1.0f);
>  }
>  
>  void
> @@ -834,11 +876,10 @@ regcache_cooked_write_unsigned (struct regcache *regcache, int regnum,
>  }
>  
>  void
> -regcache::raw_write (int regnum, const gdb_byte *buf)
> +regcache::raw_write (int regnum, gdb::array_view<const gdb_byte> src, float)
>  {
> -
> -  gdb_assert (buf != NULL);
>    assert_regnum (regnum);
> +  gdb_assert (src.size () == m_descr->sizeof_register[regnum]);
>  
>    /* On the sparc, writing %g0 is a no-op, so we don't even want to
>       change the registers array if something writes to this register.  */
> @@ -848,15 +889,15 @@ regcache::raw_write (int regnum, const gdb_byte *buf)
>    /* If we have a valid copy of the register, and new value == old
>       value, then don't bother doing the actual store.  */
>    if (get_register_status (regnum) == REG_VALID
> -      && (memcmp (register_buffer (regnum), buf,
> -		  m_descr->sizeof_register[regnum]) == 0))
> +      && (memcmp (register_buffer (regnum).data (), src.data (), src.size ())
> +	  == 0))
>      return;
>  
>    gdb::optional<scoped_restore_current_thread> maybe_restore_thread
>      = maybe_switch_inferior (m_inf_for_target_calls);
>  
>    target_prepare_to_store (this);
> -  raw_supply (regnum, buf);
> +  raw_supply (regnum, src, 1.0f);
>  
>    /* Invalidate the register after it is written, in case of a
>       failure.  */
> @@ -871,211 +912,248 @@ regcache::raw_write (int regnum, const gdb_byte *buf)
>  }
>  
>  void
> -regcache::cooked_write (int regnum, const gdb_byte *buf)
> +regcache::raw_write (int regnum, const gdb_byte *src)
> +{
> +  assert_regnum (regnum);
> +
> +  int size = m_descr->sizeof_register[regnum];
> +  raw_write (regnum, gdb::make_array_view (src, size), 1.0f);
> +}
> +
> +/* See regcache.h.  */
> +
> +void
> +regcache::cooked_write (int regnum, gdb::array_view<const gdb_byte> src, float)
>  {
>    gdb_assert (regnum >= 0);
>    gdb_assert (regnum < m_descr->nr_cooked_registers);
> +
>    if (regnum < num_raw_registers ())
> -    raw_write (regnum, buf);
> +    raw_write (regnum, src, 1.0f);
>    else
> -    gdbarch_pseudo_register_write (m_descr->gdbarch, this,
> -				   regnum, buf);
> +    gdbarch_pseudo_register_write (m_descr->gdbarch, this, regnum,
> +				   src.data ());
>  }
>  
>  /* See regcache.h.  */
>  
> -enum register_status
> -readable_regcache::read_part (int regnum, int offset, int len,
> -			      gdb_byte *out, bool is_raw)
> +void
> +regcache::cooked_write (int regnum, const gdb_byte *src)
> +{
> +  gdb_assert (regnum >= 0);
> +  gdb_assert (regnum < m_descr->nr_cooked_registers);
> +
> +  int size = m_descr->sizeof_register[regnum];
> +  return cooked_write (regnum, gdb::make_array_view (src, size), 1.0f);
> +}
> +
> +/* See regcache.h.  */
> +
> +register_status
> +readable_regcache::read_part (int regnum, int offset,
> +			      gdb::array_view<gdb_byte> dst, bool is_raw)
>  {
>    int reg_size = register_size (arch (), regnum);
>  
> -  gdb_assert (out != NULL);
>    gdb_assert (offset >= 0);
> -  gdb_assert (len >= 0 && offset + len <= reg_size);
> +  gdb_assert (offset + dst.size () <= reg_size);
>  
> -  if (len == 0)
> +  if (dst.size () == 0)
>      {
>        /* Nothing to do.  */
>        return REG_VALID;
>      }
>  
> -  if (len == reg_size)
> +  if (dst.size () == reg_size)
>      {
>        /* Read the full register.  */
> -      return (is_raw) ? raw_read (regnum, out) : cooked_read (regnum, out);
> +      if (is_raw)
> +	return raw_read (regnum, dst, 1.0f);
> +      else
> +	return cooked_read (regnum, dst, 1.0f);
>      }
>  
> -  enum register_status status;
> -  gdb_byte *reg = (gdb_byte *) alloca (reg_size);
> -
>    /* Read full register to buffer.  */
> -  status = (is_raw) ? raw_read (regnum, reg) : cooked_read (regnum, reg);
> +  register_status status;
> +  gdb_byte *reg_buf = (gdb_byte *) alloca (reg_size);
> +  auto reg = gdb::make_array_view (reg_buf, reg_size);
> +
> +  if (is_raw)
> +    status = raw_read (regnum, reg, 1.0f);
> +  else
> +    status = cooked_read (regnum, reg, 1.0f);
> +
>    if (status != REG_VALID)
>      return status;
>  
>    /* Copy out.  */
> -  memcpy (out, reg + offset, len);
> +  copy (reg.slice (offset, dst.size ()), dst);
>    return REG_VALID;
>  }
>  
>  /* See regcache.h.  */
>  
>  void
> -reg_buffer::raw_collect_part (int regnum, int offset, int len,
> -			      gdb_byte *out) const
> +reg_buffer::raw_collect_part (int regnum, int offset,
> +			      gdb::array_view<gdb_byte> dst) const
>  {
>    int reg_size = register_size (arch (), regnum);
>  
> -  gdb_assert (out != nullptr);
>    gdb_assert (offset >= 0);
> -  gdb_assert (len >= 0 && offset + len <= reg_size);
> +  gdb_assert (offset + dst.size () <= reg_size);
>  
> -  if (len == 0)
> +  if (dst.size () == 0)
>      {
>        /* Nothing to do.  */
>        return;
>      }
>  
> -  if (len == reg_size)
> +  if (dst.size () == reg_size)
>      {
>        /* Collect the full register.  */
> -      return raw_collect (regnum, out);
> +      return raw_collect (regnum, dst, 1.0f);
>      }
>  
>    /* Read to buffer, then write out.  */
> -  gdb_byte *reg = (gdb_byte *) alloca (reg_size);
> -  raw_collect (regnum, reg);
> -  memcpy (out, reg + offset, len);
> +  gdb_byte *reg_buf = (gdb_byte *) alloca (reg_size);
> +  auto reg = gdb::make_array_view (reg_buf, reg_size);
> +  raw_collect (regnum, reg, 1.0f);
> +  copy (reg.slice (offset, dst.size ()), dst);
>  }
>  
>  /* See regcache.h.  */
>  
> -enum register_status
> -regcache::write_part (int regnum, int offset, int len,
> -		      const gdb_byte *in, bool is_raw)
> +register_status
> +regcache::write_part (int regnum, int offset,
> +		      gdb::array_view<const gdb_byte> src, bool is_raw)
>  {
>    int reg_size = register_size (arch (), regnum);
>  
> -  gdb_assert (in != NULL);
>    gdb_assert (offset >= 0);
> -  gdb_assert (len >= 0 && offset + len <= reg_size);
> +  gdb_assert (offset + src.size () <= reg_size);
>  
> -  if (len == 0)
> +  if (src.size () == 0)
>      {
>        /* Nothing to do.  */
>        return REG_VALID;
>      }
>  
> -  if (len == reg_size)
> +  if (src.size () == reg_size)
>      {
>        /* Write the full register.  */
> -      (is_raw) ? raw_write (regnum, in) : cooked_write (regnum, in);
> +      if (is_raw)
> +	raw_write (regnum, src, 1.0f);
> +      else
> +	cooked_write (regnum, src, 1.0f);
> +
>        return REG_VALID;
>      }
>  
> -  enum register_status status;
> -  gdb_byte *reg = (gdb_byte *) alloca (reg_size);
> -
>    /* Read existing register to buffer.  */
> -  status = (is_raw) ? raw_read (regnum, reg) : cooked_read (regnum, reg);
> +  register_status status;
> +  gdb_byte *reg_buf = (gdb_byte *) alloca (reg_size);
> +  auto reg = gdb::make_array_view (reg_buf, reg_size);
> +
> +  if (is_raw)
> +    status = raw_read (regnum, reg, 1.0f);
> +  else
> +    status = cooked_read (regnum, reg, 1.0f);
> +
>    if (status != REG_VALID)
>      return status;
>  
>    /* Update buffer, then write back to regcache.  */
> -  memcpy (reg + offset, in, len);
> -  is_raw ? raw_write (regnum, reg) : cooked_write (regnum, reg);
> +  copy (src, reg.slice (offset, src.size ()));
> +
> +  if (is_raw)
> +    raw_write (regnum, reg, 1.0f);
> +  else
> +    cooked_write (regnum, reg, 1.0f);
> +
>    return REG_VALID;
>  }
>  
>  /* See regcache.h.  */
>  
>  void
> -reg_buffer::raw_supply_part (int regnum, int offset, int len,
> -			     const gdb_byte *in)
> +reg_buffer::raw_supply_part (int regnum, int offset,
> +			     gdb::array_view<const gdb_byte> src)
>  {
>    int reg_size = register_size (arch (), regnum);
>  
> -  gdb_assert (in != nullptr);
>    gdb_assert (offset >= 0);
> -  gdb_assert (len >= 0 && offset + len <= reg_size);
> +  gdb_assert (offset + src.size () <= reg_size);
>  
> -  if (len == 0)
> +  if (src.size () == 0)
>      {
>        /* Nothing to do.  */
>        return;
>      }
>  
> -  if (len == reg_size)
> +  if (src.size () == reg_size)
>      {
>        /* Supply the full register.  */
> -      return raw_supply (regnum, in);
> +      return raw_supply (regnum, src, 1.0f);
>      }
>  
> -  gdb_byte *reg = (gdb_byte *) alloca (reg_size);
> -
>    /* Read existing value to buffer.  */
> -  raw_collect (regnum, reg);
> +  gdb_byte *reg_buf = (gdb_byte *) alloca (reg_size);
> +  auto reg = gdb::make_array_view (reg_buf, reg_size);
> +  raw_collect (regnum, reg, 1.0f);
>  
>    /* Write to buffer, then write out.  */
> -  memcpy (reg + offset, in, len);
> -  raw_supply (regnum, reg);
> +  copy (src, reg.slice (offset, src.size ()));
> +  raw_supply (regnum, reg, 1.0f);
>  }
>  
> -enum register_status
> -readable_regcache::raw_read_part (int regnum, int offset, int len,
> -				  gdb_byte *buf)
> +register_status
> +readable_regcache::raw_read_part (int regnum, int offset,
> +				  gdb::array_view<gdb_byte> dst)
>  {
>    assert_regnum (regnum);
> -  return read_part (regnum, offset, len, buf, true);
> +  return read_part (regnum, offset, dst, true);
>  }
>  
>  /* See regcache.h.  */
>  
>  void
> -regcache::raw_write_part (int regnum, int offset, int len,
> -			  const gdb_byte *buf)
> +regcache::raw_write_part (int regnum, int offset,
> +			  gdb::array_view<const gdb_byte> src)
>  {
>    assert_regnum (regnum);
> -  write_part (regnum, offset, len, buf, true);
> +  write_part (regnum, offset, src, true);
>  }
>  
>  /* See regcache.h.  */
>  
> -enum register_status
> -readable_regcache::cooked_read_part (int regnum, int offset, int len,
> -				     gdb_byte *buf)
> +register_status
> +readable_regcache::cooked_read_part (int regnum, int offset,
> +				     gdb::array_view<gdb_byte> dst)
>  {
>    gdb_assert (regnum >= 0 && regnum < m_descr->nr_cooked_registers);
> -  return read_part (regnum, offset, len, buf, false);
> +  return read_part (regnum, offset, dst, false);
>  }
>  
>  /* See regcache.h.  */
>  
>  void
> -regcache::cooked_write_part (int regnum, int offset, int len,
> -			     const gdb_byte *buf)
> +regcache::cooked_write_part (int regnum, int offset,
> +			     gdb::array_view<const gdb_byte> src)
>  {
>    gdb_assert (regnum >= 0 && regnum < m_descr->nr_cooked_registers);
> -  write_part (regnum, offset, len, buf, false);
> +  write_part (regnum, offset, src, false);
>  }
>  
>  /* See gdbsupport/common-regcache.h.  */
>  
>  void
> -reg_buffer::raw_supply (int regnum, const void *buf)
> +reg_buffer::raw_supply (int regnum, gdb::array_view<const gdb_byte> src, float)
>  {
> -  void *regbuf;
> -  size_t size;
> -
> -  assert_regnum (regnum);
> -
> -  regbuf = register_buffer (regnum);
> -  size = m_descr->sizeof_register[regnum];
> +  gdb::array_view<gdb_byte> dst = register_buffer (regnum);
>  
> -  if (buf)
> +  if (src.data () != nullptr)
>      {
> -      memcpy (regbuf, buf, size);
> +      copy (src, dst);
>        m_register_status[regnum] = REG_VALID;
>      }
>    else
> @@ -1083,7 +1161,7 @@ reg_buffer::raw_supply (int regnum, const void *buf)
>        /* This memset not strictly necessary, but better than garbage
>  	 in case the register value manages to escape somewhere (due
>  	 to a bug, no less).  */
> -      memset (regbuf, 0, size);
> +      memset (dst.data (), 0, dst.size ());
>        m_register_status[regnum] = REG_UNAVAILABLE;
>      }
>  }
> @@ -1091,19 +1169,25 @@ reg_buffer::raw_supply (int regnum, const void *buf)
>  /* See regcache.h.  */
>  
>  void
> -reg_buffer::raw_supply_integer (int regnum, const gdb_byte *addr,
> -				int addr_len, bool is_signed)
> +reg_buffer::raw_supply (int regnum, const void *src)
>  {
> -  enum bfd_endian byte_order = gdbarch_byte_order (m_descr->gdbarch);
> -  gdb_byte *regbuf;
> -  size_t regsize;
> -
>    assert_regnum (regnum);
>  
> -  regbuf = register_buffer (regnum);
> -  regsize = m_descr->sizeof_register[regnum];
> +  int size = m_descr->sizeof_register[regnum];
> +  raw_supply (regnum, gdb::make_array_view ((const gdb_byte *) src, size),
> +	      1.0f);
> +}
> +
> +/* See regcache.h.  */
> +
> +void
> +reg_buffer::raw_supply_integer (int regnum, const gdb_byte *addr, int addr_len,
> +				bool is_signed)
> +{
> +  gdb::array_view<gdb_byte> dst = register_buffer (regnum);
> +  bfd_endian byte_order = gdbarch_byte_order (m_descr->gdbarch);
>  
> -  copy_integer_to_size (regbuf, regsize, addr, addr_len, is_signed,
> +  copy_integer_to_size (dst.data (), dst.size (), addr, addr_len, is_signed,
>  			byte_order);
>    m_register_status[regnum] = REG_VALID;
>  }
> @@ -1113,32 +1197,31 @@ reg_buffer::raw_supply_integer (int regnum, const gdb_byte *addr,
>  void
>  reg_buffer::raw_supply_zeroed (int regnum)
>  {
> -  void *regbuf;
> -  size_t size;
> -
> -  assert_regnum (regnum);
> -
> -  regbuf = register_buffer (regnum);
> -  size = m_descr->sizeof_register[regnum];
> -
> -  memset (regbuf, 0, size);
> +  gdb::array_view<gdb_byte> dst = register_buffer (regnum);
> +  memset (dst.data (), 0, dst.size ());
>    m_register_status[regnum] = REG_VALID;
>  }
>  
>  /* See gdbsupport/common-regcache.h.  */
>  
>  void
> -reg_buffer::raw_collect (int regnum, void *buf) const
> +reg_buffer::raw_collect (int regnum, gdb::array_view<gdb_byte> dst,
> +			 float) const
>  {
> -  const void *regbuf;
> -  size_t size;
> +  gdb::array_view<const gdb_byte> src = register_buffer (regnum);
> +  copy (src, dst);
> +}
>  
> -  gdb_assert (buf != NULL);
> +/* See regcache.h.  */
> +
> +void
> +reg_buffer::raw_collect (int regnum, void *dst) const
> +{
>    assert_regnum (regnum);
>  
> -  regbuf = register_buffer (regnum);
> -  size = m_descr->sizeof_register[regnum];
> -  memcpy (buf, regbuf, size);
> +  int size = m_descr->sizeof_register[regnum];
> +  return raw_collect (regnum, gdb::make_array_view ((gdb_byte *) dst, size),
> +		      1.0f);
>  }
>  
>  /* See regcache.h.  */
> @@ -1147,16 +1230,9 @@ void
>  reg_buffer::raw_collect_integer (int regnum, gdb_byte *addr, int addr_len,
>  				 bool is_signed) const
>  {
> -  enum bfd_endian byte_order = gdbarch_byte_order (m_descr->gdbarch);
> -  const gdb_byte *regbuf;
> -  size_t regsize;
> -
> -  assert_regnum (regnum);
> -
> -  regbuf = register_buffer (regnum);
> -  regsize = m_descr->sizeof_register[regnum];
> -
> -  copy_integer_to_size (addr, addr_len, regbuf, regsize, is_signed,
> +  gdb::array_view<const gdb_byte> dst = register_buffer (regnum);
> +  bfd_endian byte_order = gdbarch_byte_order (m_descr->gdbarch);
> +  copy_integer_to_size (addr, addr_len, dst.data (), dst.size (), is_signed,
>  			byte_order);
>  }
>  
> @@ -1175,7 +1251,8 @@ regcache::transfer_regset_register (struct regcache *out_regcache, int regnum,
>  
>    if (out_buf != nullptr)
>      {
> -      raw_collect_part (regnum, 0, reg_size, out_buf + offs);
> +      raw_collect_part (regnum, 0,
> +			gdb::make_array_view (out_buf + offs, reg_size));
>  
>        /* Ensure any additional space is cleared.  */
>        if (slot_size > reg_size)
> @@ -1186,12 +1263,14 @@ regcache::transfer_regset_register (struct regcache *out_regcache, int regnum,
>        /* Zero-extend the register value if the slot is smaller than the register.  */
>        if (slot_size < register_size (gdbarch, regnum))
>  	out_regcache->raw_supply_zeroed (regnum);
> -      out_regcache->raw_supply_part (regnum, 0, reg_size, in_buf + offs);
> +      out_regcache->raw_supply_part (regnum, 0,
> +				     gdb::make_array_view (in_buf + offs,
> +							   reg_size));
>      }
>    else
>      {
>        /* Invalidate the register.  */
> -      out_regcache->raw_supply (regnum, nullptr);
> +      out_regcache->raw_supply (regnum, {});
>      }
>  }
>  
> @@ -1322,13 +1401,12 @@ bool
>  reg_buffer::raw_compare (int regnum, const void *buf, int offset) const
>  {
>    gdb_assert (buf != NULL);
> -  assert_regnum (regnum);
>  
> -  const char *regbuf = (const char *) register_buffer (regnum);
> -  size_t size = m_descr->sizeof_register[regnum];
> -  gdb_assert (size >= offset);
> +  gdb::array_view<const gdb_byte> regbuf = register_buffer (regnum);
> +  gdb_assert (offset < regbuf.size ());
> +  regbuf = regbuf.slice (offset);
>  
> -  return (memcmp (buf, regbuf + offset, size - offset) == 0);
> +  return memcmp (buf, regbuf.data (), regbuf.size ()) == 0;
>  }
>  
>  /* Special handling for register PC.  */
> @@ -1417,17 +1495,15 @@ regcache::debug_print_register (const char *func,  int regno)
>    if (regno >= 0 && regno < gdbarch_num_regs (gdbarch))
>      {
>        enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> -      int size = register_size (gdbarch, regno);
> -      gdb_byte *buf = register_buffer (regno);
> +      gdb::array_view<gdb_byte> buf = register_buffer (regno);
>  
>        gdb_printf (gdb_stdlog, " = ");
> -      for (int i = 0; i < size; i++)
> -	{
> -	  gdb_printf (gdb_stdlog, "%02x", buf[i]);
> -	}
> -      if (size <= sizeof (LONGEST))
> +      for (gdb_byte byte : buf)
> +	gdb_printf (gdb_stdlog, "%02x", byte);
> +
> +      if (buf.size () <= sizeof (LONGEST))
>  	{
> -	  ULONGEST val = extract_unsigned_integer (buf, size, byte_order);
> +	  ULONGEST val = extract_unsigned_integer (buf, byte_order);
>  
>  	  gdb_printf (gdb_stdlog, " %s %s",
>  		      core_addr_to_string_nz (val), plongest (val));
> @@ -1889,7 +1965,7 @@ cooked_read_test (struct gdbarch *gdbarch)
>    readwrite.set_ptid (mockctx.mock_ptid);
>    gdb::byte_vector buf (register_size (gdbarch, nonzero_regnum));
>  
> -  readwrite.raw_read (nonzero_regnum, buf.data ());
> +  readwrite.raw_read (nonzero_regnum, buf, 1.0f);
>  
>    /* raw_read calls target_fetch_registers.  */
>    SELF_CHECK (mockctx.mock_target.fetch_registers_called > 0);
> @@ -1910,9 +1986,8 @@ cooked_read_test (struct gdbarch *gdbarch)
>  
>        gdb::byte_vector inner_buf (register_size (gdbarch, regnum));
>  
> -      SELF_CHECK (REG_VALID == readwrite.cooked_read (regnum,
> -						      inner_buf.data ()));
> -
> +      SELF_CHECK (REG_VALID
> +		  == readwrite.cooked_read (regnum, inner_buf, 1.0f));
>        SELF_CHECK (mockctx.mock_target.fetch_registers_called == 0);
>        SELF_CHECK (mockctx.mock_target.store_registers_called == 0);
>        SELF_CHECK (mockctx.mock_target.xfer_partial_called == 0);
> @@ -1932,8 +2007,7 @@ cooked_read_test (struct gdbarch *gdbarch)
>  	continue;
>  
>        gdb::byte_vector inner_buf (register_size (gdbarch, regnum));
> -      enum register_status status = readonly.cooked_read (regnum,
> -							  inner_buf.data ());
> +      register_status status = readonly.cooked_read (regnum, inner_buf, 1.0f);
>  
>        if (regnum < gdbarch_num_regs (gdbarch))
>  	{
> @@ -2023,8 +2097,8 @@ cooked_write_test (struct gdbarch *gdbarch)
>  	      && regnum <= gdbarch_num_regs (gdbarch) + 4))
>  	continue;
>  
> -      std::vector<gdb_byte> expected (register_size (gdbarch, regnum), 0);
> -      std::vector<gdb_byte> buf (register_size (gdbarch, regnum), 0);
> +      gdb::byte_vector expected (register_size (gdbarch, regnum), 0);
> +      gdb::byte_vector buf (register_size (gdbarch, regnum), 0);
>        const auto type = register_type (gdbarch, regnum);
>  
>        if (type->code () == TYPE_CODE_FLT
> @@ -2079,9 +2153,9 @@ cooked_write_test (struct gdbarch *gdbarch)
>  	  SELF_CHECK (0);
>  	}
>  
> -      readwrite.cooked_write (regnum, expected.data ());
> +      readwrite.cooked_write (regnum, expected, 1.0f);
>  
> -      SELF_CHECK (readwrite.cooked_read (regnum, buf.data ()) == REG_VALID);
> +      SELF_CHECK (readwrite.cooked_read (regnum, buf, 1.0f) == REG_VALID);
>        SELF_CHECK (expected == buf);
>      }
>  }
> diff --git a/gdb/regcache.h b/gdb/regcache.h
> index 57ddac465f09..123c9248c345 100644
> --- a/gdb/regcache.h
> +++ b/gdb/regcache.h
> @@ -20,6 +20,7 @@
>  #ifndef REGCACHE_H
>  #define REGCACHE_H
>  
> +#include "gdbsupport/array-view.h"
>  #include "gdbsupport/common-regcache.h"
>  #include "gdbsupport/function-view.h"
>  
> @@ -172,8 +173,8 @@ extern struct type *register_type (struct gdbarch *gdbarch, int regnum);
>     
>  extern int register_size (struct gdbarch *gdbarch, int regnum);
>  
> -typedef gdb::function_view<register_status (int regnum, gdb_byte *buf)>
> -  register_read_ftype;
> +using register_read_ftype
> +  = gdb::function_view<register_status (int, gdb::array_view<gdb_byte>)>;
>  
>  /* A (register_number, register_value) pair.  */
>  
> @@ -199,7 +200,11 @@ class reg_buffer : public reg_buffer_common
>    enum register_status get_register_status (int regnum) const override;
>  
>    /* See gdbsupport/common-regcache.h.  */
> -  void raw_collect (int regnum, void *buf) const override;
> +  void raw_collect (int regnum, gdb::array_view<gdb_byte> dst,
> +		    float) const override;
> +
> +  /* Deprecated overload of the above.  */
> +  void raw_collect (int regnum, void *dst) const;
>  
>    /* Collect register REGNUM from REGCACHE.  Store collected value as an integer
>       at address ADDR, in target endian, with length ADDR_LEN and sign IS_SIGNED.
> @@ -209,17 +214,24 @@ class reg_buffer : public reg_buffer_common
>    void raw_collect_integer (int regnum, gdb_byte *addr, int addr_len,
>  			    bool is_signed) const;
>  
> -  /* Collect register REGNUM from REGCACHE, starting at OFFSET in register,
> -     reading only LEN.  */
> -  void raw_collect_part (int regnum, int offset, int len, gdb_byte *out) const;
> +  /* Collect part of register REGNUM from this register buffer.  Start at OFFSET
> +     in register.  The size is given by the size of DST.  */
> +  void raw_collect_part (int regnum, int offset,
> +			 gdb::array_view<gdb_byte> dst) const;
> +
> +  /* Deprecated overload of the above.  */
> +  void raw_collect_part (int regnum, int offset, int len, gdb_byte *dst) const
> +  { raw_collect_part (regnum, offset, gdb::make_array_view (dst, len)); }
>  
>    /* See gdbsupport/common-regcache.h.  */
> -  void raw_supply (int regnum, const void *buf) override;
> +  void raw_supply (int regnum, gdb::array_view<const gdb_byte> src,
> +		   float) override;
> +
> +  /* Deprecated overload of the above.  */
> +  void raw_supply (int regnum, const void *src);
>  
>    void raw_supply (int regnum, const reg_buffer &src)
> -  {
> -    raw_supply (regnum, src.register_buffer (regnum));
> -  }
> +  { raw_supply (regnum, src.register_buffer (regnum), 1.0f); }
>  
>    /* Supply register REGNUM to REGCACHE.  Value to supply is an integer stored
>       at address ADDR, in target endian, with length ADDR_LEN and sign IS_SIGNED.
> @@ -234,9 +246,11 @@ class reg_buffer : public reg_buffer_common
>       unavailable).  */
>    void raw_supply_zeroed (int regnum);
>  
> -  /* Supply register REGNUM to REGCACHE, starting at OFFSET in register, writing
> -     only LEN, without editing the rest of the register.  */
> -  void raw_supply_part (int regnum, int offset, int len, const gdb_byte *in);
> +  /* Supply part of register REGNUM to this register buffer.  Start at OFFSET in
> +     the register.  The size is given by the size of SRC.  The rest of the
> +     register left untouched.  */
> +  void raw_supply_part (int regnum, int offset,
> +			gdb::array_view<const gdb_byte> src);
>  
>    void invalidate (int regnum);
>  
> @@ -251,7 +265,11 @@ class reg_buffer : public reg_buffer_common
>  
>    int num_raw_registers () const;
>  
> -  gdb_byte *register_buffer (int regnum) const;
> +  /* Return a view on register REGNUM's buffer cache.  */
> +  template <typename ElemType>
> +  gdb::array_view<ElemType> register_buffer (int regnum) const;
> +  gdb::array_view<const gdb_byte> register_buffer (int regnum) const;
> +  gdb::array_view<gdb_byte> register_buffer (int regnum);
>  
>    /* Save a register cache.  The set of registers saved into the
>       regcache determined by the save_reggroup.  COOKED_READ returns
> @@ -281,27 +299,42 @@ class readable_regcache : public reg_buffer
>  
>    /* Transfer a raw register [0..NUM_REGS) from core-gdb to this regcache,
>       return its value in *BUF and return its availability status.  */
> +  register_status raw_read (int regnum, gdb::array_view<gdb_byte> dst, float);
> +
> +  /* Deprecated overload of the above.  */
> +  register_status raw_read (int regnum, gdb_byte *dst);
>  
> -  enum register_status raw_read (int regnum, gdb_byte *buf);
>    template<typename T, typename = RequireLongest<T>>
> -  enum register_status raw_read (int regnum, T *val);
> +  register_status raw_read (int regnum, T *val);
>  
>    /* Partial transfer of raw registers.  Return the status of the register.  */
> -  enum register_status raw_read_part (int regnum, int offset, int len,
> -				      gdb_byte *buf);
> +  register_status raw_read_part (int regnum, int offset,
> +				 gdb::array_view<gdb_byte> dst);
> +
> +  /* Deprecated overload of the above.  */
> +  register_status raw_read_part (int regnum, int offset, int len,
> +				 gdb_byte *dst)
> +  { return raw_read_part (regnum, offset, gdb::make_array_view (dst, len)); }
>  
>    /* Make certain that the register REGNUM is up-to-date.  */
>    virtual void raw_update (int regnum) = 0;
>  
>    /* Transfer a raw register [0..NUM_REGS+NUM_PSEUDO_REGS) from core-gdb to
> -     this regcache, return its value in *BUF and return its availability status.  */
> -  enum register_status cooked_read (int regnum, gdb_byte *buf);
> +     this regcache, return its value in DST and return its availability status.  */
> +  register_status cooked_read (int regnum, gdb::array_view<gdb_byte> dst,
> +			       float);
> +  register_status cooked_read (int regnum, gdb_byte *dst);
> +
>    template<typename T, typename = RequireLongest<T>>
> -  enum register_status cooked_read (int regnum, T *val);
> +  register_status cooked_read (int regnum, T *val);
>  
>    /* Partial transfer of a cooked register.  */
> -  enum register_status cooked_read_part (int regnum, int offset, int len,
> -					 gdb_byte *buf);
> +  register_status cooked_read_part (int regnum, int offset,
> +				    gdb::array_view<gdb_byte> dst);
> +
> +  /* Deprecated overload of the above.  */
> +  register_status cooked_read_part (int regnum, int offset, int len, gdb_byte *src)
> +  { return cooked_read_part (regnum, offset, gdb::make_array_view (src, len)); }
>  
>    /* Read register REGNUM from the regcache and return a new value.  This
>       will call mark_value_bytes_unavailable as appropriate.  */
> @@ -311,8 +344,8 @@ class readable_regcache : public reg_buffer
>  
>    /* Perform a partial register transfer using a read, modify, write
>       operation.  Will fail if register is currently invalid.  */
> -  enum register_status read_part (int regnum, int offset, int len,
> -				  gdb_byte *out, bool is_raw);
> +  register_status read_part (int regnum, int offset,
> +			     gdb::array_view<gdb_byte> dst, bool is_raw);
>  };
>  
>  /* Buffer of registers, can be read and written.  */
> @@ -354,13 +387,19 @@ class regcache : public detached_regcache
>    /* Update the value of raw register REGNUM (in the range [0..NUM_REGS)) and
>       transfer its value to core-gdb.  */
>  
> -  void raw_write (int regnum, const gdb_byte *buf);
> +  void raw_write (int regnum, gdb::array_view<const gdb_byte> src, float);
> +
> +  /* Deprecated overload of the above.  */
> +  void raw_write (int regnum, const gdb_byte *src);
>  
>    template<typename T, typename = RequireLongest<T>>
>    void raw_write (int regnum, T val);
>  
>    /* Transfer of pseudo-registers.  */
> -  void cooked_write (int regnum, const gdb_byte *buf);
> +  void cooked_write (int regnum, gdb::array_view<const gdb_byte> src, float);
> +
> +  /* Deprecated overload of the above.  */
> +  void cooked_write (int regnum, const gdb_byte *src);
>  
>    template<typename T, typename = RequireLongest<T>>
>    void cooked_write (int regnum, T val);
> @@ -369,12 +408,21 @@ class regcache : public detached_regcache
>  
>    /* Partial transfer of raw registers.  Perform read, modify, write style
>       operations.  */
> -  void raw_write_part (int regnum, int offset, int len, const gdb_byte *buf);
> +  void raw_write_part (int regnum, int offset,
> +		       gdb::array_view<const gdb_byte> src);
> +
> +  /* Deprecated overload of the above.  */
> +  void raw_write_part (int regnum, int offset, int len, const gdb_byte *src)
> +  { raw_write_part (regnum, offset, gdb::make_array_view (src, len)); }
>  
>    /* Partial transfer of a cooked register.  Perform read, modify, write style
>       operations.  */
> -  void cooked_write_part (int regnum, int offset, int len,
> -			  const gdb_byte *buf);
> +  void cooked_write_part (int regnum, int offset,
> +			  gdb::array_view<const gdb_byte> src);
> +
> +  /* Deprecated overload of the above.  */
> +  void cooked_write_part (int regnum, int offset, int len, const gdb_byte *src)
> +  { cooked_write_part (regnum, offset, gdb::make_array_view (src, len)); }
>  
>    /* Transfer a set of registers (as described by REGSET) between
>       REGCACHE and BUF.  If REGNUM == -1, transfer all registers
> @@ -442,8 +490,9 @@ class regcache : public detached_regcache
>  
>    /* Perform a partial register transfer using a read, modify, write
>       operation.  */
> -  enum register_status write_part (int regnum, int offset, int len,
> -				   const gdb_byte *in, bool is_raw);
> +  register_status write_part (int regnum, int offset,
> +			      gdb::array_view<const gdb_byte> src,
> +			      bool is_raw);
>  
>    /* The address space of this register cache (for registers where it
>       makes sense, like PC or SP).  */
> diff --git a/gdbserver/regcache.cc b/gdbserver/regcache.cc
> index 4ac0fb659c3b..e1c174b56cb6 100644
> --- a/gdbserver/regcache.cc
> +++ b/gdbserver/regcache.cc
> @@ -315,27 +315,33 @@ regcache_register_size (const reg_buffer_common *regcache, int n)
>      (gdb::checked_static_cast<const struct regcache *> (regcache)->tdesc, n);
>  }
>  
> -static unsigned char *
> +static gdb::array_view<gdb_byte>
>  register_data (const struct regcache *regcache, int n)
>  {
> -  return (regcache->registers
> -	  + find_register_by_number (regcache->tdesc, n).offset / 8);
> +  const gdb::reg &reg = find_register_by_number (regcache->tdesc, n);
> +  return gdb::make_array_view (regcache->registers + reg.offset / 8,
> +			       reg.size / 8);
>  }
>  
>  void
> -supply_register (struct regcache *regcache, int n, const void *buf)
> +supply_register (struct regcache *regcache, int n, const void *vbuf)
>  {
> -  return regcache->raw_supply (n, buf);
> +  const gdb::reg &reg = find_register_by_number (regcache->tdesc, n);
> +  const gdb_byte *buf = static_cast<const gdb_byte *> (vbuf);
> +  return regcache->raw_supply (n, gdb::make_array_view (buf, reg.size / 8),
> +			       1.0f);
>  }
>  
>  /* See gdbsupport/common-regcache.h.  */
>  
>  void
> -regcache::raw_supply (int n, const void *buf)
> +regcache::raw_supply (int n, gdb::array_view<const gdb_byte> src, float)
>  {
> -  if (buf)
> +  auto dst = register_data (this, n);
> +
> +  if (src.data () != nullptr)
>      {
> -      memcpy (register_data (this, n), buf, register_size (tdesc, n));
> +      copy (src, dst);
>  #ifndef IN_PROCESS_AGENT
>        if (register_status != NULL)
>  	register_status[n] = REG_VALID;
> @@ -343,7 +349,7 @@ regcache::raw_supply (int n, const void *buf)
>      }
>    else
>      {
> -      memset (register_data (this, n), 0, register_size (tdesc, n));
> +      memset (dst.data (), 0, dst.size ());
>  #ifndef IN_PROCESS_AGENT
>        if (register_status != NULL)
>  	register_status[n] = REG_UNAVAILABLE;
> @@ -356,8 +362,8 @@ regcache::raw_supply (int n, const void *buf)
>  void
>  supply_register_zeroed (struct regcache *regcache, int n)
>  {
> -  memset (register_data (regcache, n), 0,
> -	  register_size (regcache->tdesc, n));
> +  auto dst = register_data (regcache, n);
> +  memset (dst.data (), 0, dst.size ());
>  #ifndef IN_PROCESS_AGENT
>    if (regcache->register_status != NULL)
>      regcache->register_status[n] = REG_VALID;
> @@ -426,17 +432,20 @@ supply_register_by_name (struct regcache *regcache,
>  #endif
>  
>  void
> -collect_register (struct regcache *regcache, int n, void *buf)
> +collect_register (struct regcache *regcache, int n, void *vbuf)
>  {
> -  regcache->raw_collect (n, buf);
> +  const gdb::reg &reg = find_register_by_number (regcache->tdesc, n);
> +  gdb_byte *buf = static_cast<gdb_byte *> (vbuf);
> +  regcache->raw_collect (n, gdb::make_array_view (buf, reg.size / 8), 1.0f);
>  }
>  
>  /* See gdbsupport/common-regcache.h.  */
>  
>  void
> -regcache::raw_collect (int n, void *buf) const
> +regcache::raw_collect (int n, gdb::array_view<gdb_byte> dst, float) const
>  {
> -  memcpy (buf, register_data (this, n), register_size (tdesc, n));
> +  auto src = register_data (this, n);
> +  copy (src, dst);
>  }
>  
>  enum register_status
> @@ -476,8 +485,7 @@ regcache_raw_get_unsigned_by_name (struct regcache *regcache,
>  void
>  collect_register_as_string (struct regcache *regcache, int n, char *buf)
>  {
> -  bin2hex (register_data (regcache, n), buf,
> -	   register_size (regcache->tdesc, n));
> +  bin2hex (register_data (regcache, n), buf);
>  }
>  
>  void
> @@ -524,9 +532,9 @@ regcache::raw_compare (int regnum, const void *buf, int offset) const
>  {
>    gdb_assert (buf != NULL);
>  
> -  const unsigned char *regbuf = register_data (this, regnum);
> -  int size = register_size (tdesc, regnum);
> -  gdb_assert (size >= offset);
> +  gdb::array_view<const gdb_byte> regbuf = register_data (this, regnum);
> +  gdb_assert (offset < regbuf.size ());
> +  regbuf = regbuf.slice (offset);
>  
> -  return (memcmp (buf, regbuf + offset, size - offset) == 0);
> +  return memcmp (buf, regbuf.data (), regbuf.size ()) == 0;
>  }
> diff --git a/gdbserver/regcache.h b/gdbserver/regcache.h
> index 7248bcf5808a..ba093c97c1f0 100644
> --- a/gdbserver/regcache.h
> +++ b/gdbserver/regcache.h
> @@ -50,10 +50,12 @@ struct regcache : public reg_buffer_common
>    enum register_status get_register_status (int regnum) const override;
>  
>    /* See gdbsupport/common-regcache.h.  */
> -  void raw_supply (int regnum, const void *buf) override;
> +  void raw_supply (int regnum, gdb::array_view<const gdb_byte> src,
> +		   float) override;
>  
>    /* See gdbsupport/common-regcache.h.  */
> -  void raw_collect (int regnum, void *buf) const override;
> +  void raw_collect (int regnum, gdb::array_view<gdb_byte> dst,
> +		    float) const override;
>  
>    /* See gdbsupport/common-regcache.h.  */
>    bool raw_compare (int regnum, const void *buf, int offset) const override;
> diff --git a/gdbsupport/common-regcache.h b/gdbsupport/common-regcache.h
> index 6d98ca8c92ed..75d209f7cbc0 100644
> --- a/gdbsupport/common-regcache.h
> +++ b/gdbsupport/common-regcache.h
> @@ -78,11 +78,44 @@ struct reg_buffer_common
>       buffer.  */
>    virtual register_status get_register_status (int regnum) const = 0;
>  
> -  /* Supply register REGNUM, whose contents are stored in BUF, to REGCACHE.  */
> -  virtual void raw_supply (int regnum, const void *buf) = 0;
> +  /* Supply register REGNUM, whose contents are stored in SRC, to this register
> +     buffer.  */
> +  virtual void raw_supply (int regnum, gdb::array_view<const gdb_byte> src,
> +			   float) = 0;

The addition of the 'float' argument is pretty unexpected here.  It's
not mentioned in the commit message or the function comment, nor is it
actually used in any of the function implementations as far as I can
tell.  I'm assuming this is resolving some C++ overload problem.

Sorry if I've missed something ... I've only looked at the diff so far.
I thought I'd reach out before I start playing with the code trying to
figure out what's going on.

If this is an overload workaround then I think this needs a comment
explaining what the problem is that it's working around.

If I'm just missing something, then the comment here still needs an
explanation of what the extra parameter is for.

Thanks,
Andrew


> +
> +  void raw_supply (int regnum, const uint64_t *src)
> +  {
> +    raw_supply (regnum,
> +		gdb::make_array_view ((const gdb_byte *) src, sizeof (*src)),
> +		1.0f);
> +  }
> +
> +  void raw_supply (int regnum, const gdb_byte *src)
> +  {
> +    raw_supply (regnum,
> +		gdb::make_array_view (src,
> +				      regcache_register_size (this, regnum)),
> +		1.0f);
> +  }
> +
> +  /* Collect register REGNUM from this register buffer and store its contents in
> +     DST.  */
> +  virtual void raw_collect (int regnum, gdb::array_view<gdb_byte> dst,
> +			    float) const = 0;
> +
> +  void raw_collect (int regnum, uint64_t *dst) const
> +  {
> +    raw_collect (regnum,
> +		 gdb::make_array_view ((gdb_byte *) dst, sizeof (*dst)), 1.0f);
> +  };
>  
> -  /* Collect register REGNUM from REGCACHE and store its contents in BUF.  */
> -  virtual void raw_collect (int regnum, void *buf) const = 0;
> +  void raw_collect (int regnum, gdb_byte *dst)
> +  {
> +    raw_collect (regnum,
> +		 gdb::make_array_view (dst,
> +				       regcache_register_size (this, regnum)),
> +		 1.0f);
> +  }
>  
>    /* Compare the contents of the register stored in the regcache (ignoring the
>       first OFFSET bytes) to the contents of BUF (without any offset).  Returns
> diff --git a/gdbsupport/rsp-low.cc b/gdbsupport/rsp-low.cc
> index 3d8c2002956e..632be265c00c 100644
> --- a/gdbsupport/rsp-low.cc
> +++ b/gdbsupport/rsp-low.cc
> @@ -143,6 +143,14 @@ bin2hex (const gdb_byte *bin, char *hex, int count)
>  
>  /* See rsp-low.h.  */
>  
> +int
> +bin2hex (gdb::array_view<gdb_byte> bin, char *hex)
> +{
> +  return bin2hex (bin.data (), hex, bin.size ());
> +}
> +
> +/* See rsp-low.h.  */
> +
>  std::string
>  bin2hex (const gdb_byte *bin, int count)
>  {
> diff --git a/gdbsupport/rsp-low.h b/gdbsupport/rsp-low.h
> index 327d5f3a0947..1fc2572a7f5c 100644
> --- a/gdbsupport/rsp-low.h
> +++ b/gdbsupport/rsp-low.h
> @@ -54,6 +54,8 @@ extern std::string hex2str (const char *hex, int count);
>  
>  extern int bin2hex (const gdb_byte *bin, char *hex, int count);
>  
> +extern int bin2hex (gdb::array_view<gdb_byte> bin, char *hex);
> +
>  /* Overloaded version of bin2hex that returns a std::string.  */
>  
>  extern std::string bin2hex (const gdb_byte *bin, int count);
> -- 
> 2.42.1


  reply	other threads:[~2023-11-13 13:43 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-08  5:00 [PATCH 00/24] Fix reading and writing pseudo registers in non-current frames Simon Marchi
2023-11-08  5:00 ` [PATCH 01/24] gdb: don't handle i386 k registers as pseudo registers Simon Marchi
2023-11-11 19:29   ` John Baldwin
2023-11-08  5:00 ` [PATCH 02/24] gdb: use reg_buffer_common throughout gdbsupport/common-regcache.h Simon Marchi
2023-11-11 19:42   ` John Baldwin
2023-11-08  5:00 ` [PATCH 03/24] gdb: make store_integer take an array_view Simon Marchi
2023-11-08  5:00 ` [PATCH 04/24] gdb: simplify conditions in regcache::{read,write,raw_collect,raw_supply}_part Simon Marchi
2023-11-08  5:00 ` [PATCH 05/24] gdb: change regcache interface to use array_view Simon Marchi
2023-11-13 13:43   ` Andrew Burgess [this message]
2023-11-13 14:00     ` Andrew Burgess
2023-11-13 16:47       ` Simon Marchi
2023-11-08  5:00 ` [PATCH 06/24] gdb: fix bugs in {get,put}_frame_register_bytes Simon Marchi
2023-11-13 15:00   ` Andrew Burgess
2023-11-13 19:51     ` Simon Marchi
2023-11-08  5:00 ` [PATCH 07/24] gdb: make put_frame_register take an array_view Simon Marchi
2023-11-08  5:00 ` [PATCH 08/24] gdb: change value_of_register and value_of_register_lazy to take the next frame Simon Marchi
2023-11-08  5:00 ` [PATCH 09/24] gdb: remove frame_register Simon Marchi
2023-11-08  5:00 ` [PATCH 10/24] gdb: make put_frame_register take the next frame Simon Marchi
2023-11-08  5:00 ` [PATCH 11/24] gdb: make put_frame_register_bytes " Simon Marchi
2023-11-08  5:00 ` [PATCH 12/24] gdb: make get_frame_register_bytes " Simon Marchi
2023-11-08  5:00 ` [PATCH 13/24] gdb: add value::allocate_register Simon Marchi
2023-11-08  5:00 ` [PATCH 14/24] gdb: read pseudo register through frame Simon Marchi
2023-11-11 20:11   ` John Baldwin
2023-11-08  5:00 ` [PATCH 15/24] gdb: change parameter name in frame_unwind_register_unsigned declaration Simon Marchi
2023-11-08  5:01 ` [PATCH 16/24] gdb: rename gdbarch_pseudo_register_write to gdbarch_deprecated_pseudo_register_write Simon Marchi
2023-11-14 12:12   ` Andrew Burgess
2023-11-14 15:16     ` Simon Marchi
2023-11-08  5:01 ` [PATCH 17/24] gdb: add gdbarch_pseudo_register_write that takes a frame Simon Marchi
2023-11-14 12:20   ` Andrew Burgess
2023-11-14 15:20     ` Simon Marchi
2023-11-08  5:01 ` [PATCH 18/24] gdb: migrate i386 and amd64 to the new gdbarch_pseudo_register_write Simon Marchi
2023-11-11 20:16   ` John Baldwin
2023-11-13  2:59     ` Simon Marchi
2023-11-08  5:01 ` [PATCH 19/24] gdb: make aarch64_za_offsets_from_regnum return za_offsets Simon Marchi
2023-11-08  5:01 ` [PATCH 20/24] gdb: add missing raw register read in aarch64_sme_pseudo_register_write Simon Marchi
2023-11-08  5:01 ` [PATCH 21/24] gdb: migrate aarch64 to new gdbarch_pseudo_register_write Simon Marchi
2023-11-08  5:01 ` [PATCH 22/24] gdb: migrate arm to gdbarch_pseudo_register_read_value Simon Marchi
2023-11-08  5:01 ` [PATCH 23/24] gdb: migrate arm to new gdbarch_pseudo_register_write Simon Marchi
2023-11-08  5:01 ` [PATCH 24/24] gdb/testsuite: add tests for unwinding of pseudo registers Simon Marchi
2023-11-08  5:16 ` [PATCH 00/24] Fix reading and writing pseudo registers in non-current frames Simon Marchi
2023-11-09  3:05   ` Simon Marchi
2023-11-08 11:57 ` Luis Machado
2023-11-08 15:47   ` Simon Marchi
2023-11-08 17:08     ` Luis Machado
2023-11-08 19:34       ` Simon Marchi
2023-11-09 19:04         ` Simon Marchi
2023-11-13 13:10           ` Luis Machado
2023-11-13 15:08             ` Luis Machado
2023-11-11 20:26 ` John Baldwin
2023-11-13  3:03   ` Simon Marchi
2023-12-01 16:27 Simon Marchi
2023-12-01 16:27 ` [PATCH 05/24] gdb: change regcache interface to use array_view Simon Marchi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=878r71sr9r.fsf@redhat.com \
    --to=aburgess@redhat.com \
    --cc=gdb-patches@sourceware.org \
    --cc=simon.marchi@efficios.com \
    --cc=simon.marchi@polymtl.ca \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).