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 14:00:42 +0000 [thread overview]
Message-ID: <875y25sqgl.fsf@redhat.com> (raw)
In-Reply-To: <878r71sr9r.fsf@redhat.com>
Andrew Burgess <aburgess@redhat.com> writes:
> 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 ® = 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 ® = 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 ® = 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.
As an initial experiment, I tried removing the float parameter after
applying every patch up to and including this one, and GDB still rebuilt
fine.
Then I applied every patch in the series, and removed the float
parameter, and again GDB built fine.
Maybe I'm missing something though...
Thanks,
Andrew
>
> 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
next prev parent reply other threads:[~2023-11-13 14:00 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
2023-11-13 14:00 ` Andrew Burgess [this message]
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=875y25sqgl.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).