From: Simon Marchi via Gdb-patches <gdb-patches@sourceware.org>
To: gdb-patches@sourceware.org
Subject: [PATCH 3/4] gdb: make extract_integer take an array_view
Date: Mon, 8 Nov 2021 16:06:08 -0500 [thread overview]
Message-ID: <20211108210609.353208-3-simon.marchi@efficios.com> (raw)
In-Reply-To: <20211108210609.353208-1-simon.marchi@efficios.com>
From: Simon Marchi <simon.marchi@polymtl.ca>
I think it would make sense for extract_integer, extract_signed_integer
and extract_unsigned_integer to take an array_view. This way, when we
extract an integer, we can validate that we don't overflow the buffer
passed by the caller (e.g. ask to extract a 4-byte integer but pass a
2-byte buffer).
- Change extract_integer to take an array_view
- Add overloads of extract_signed_integer and extract_unsigned_integer
that take array_views. Keep the existing versions so we don't
need to change all callers, but make them call the array_view
versions.
This shortens some places like:
result = extract_unsigned_integer (value_contents (result_val).data (),
TYPE_LENGTH (value_type (result_val)),
byte_order);
into
result = extract_unsigned_integer (value_contents (result_val), byte_order);
value_contents returns an array view that is of length
`TYPE_LENGTH (value_type (result_val))` already, so the length is
implicitly communicated through the array view.
Change-Id: Ic1c1f98c88d5c17a8486393af316f982604d6c95
---
gdb/defs.h | 23 ++++++++++++++++---
gdb/dwarf2/expr.c | 4 +---
gdb/findvar.c | 35 ++++++++++++++---------------
gdb/regcache.c | 21 +++++++----------
gdb/unittests/gmp-utils-selftests.c | 2 +-
5 files changed, 47 insertions(+), 38 deletions(-)
diff --git a/gdb/defs.h b/gdb/defs.h
index f7e09eca9db..3b6a0e63905 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -63,6 +63,7 @@
#include "gdbsupport/host-defs.h"
#include "gdbsupport/enum-flags.h"
+#include "gdbsupport/array-view.h"
/* Scope types enumerator. List the types of scopes the compiler will
accept. */
@@ -500,20 +501,36 @@ enum symbol_needs_kind
/* In findvar.c. */
template<typename T, typename = RequireLongest<T>>
-T extract_integer (const gdb_byte *addr, int len, enum bfd_endian byte_order);
+T extract_integer (gdb::array_view<const gdb_byte>, enum bfd_endian byte_order);
+
+static inline LONGEST
+extract_signed_integer (gdb::array_view<const gdb_byte> buf,
+ enum bfd_endian byte_order)
+{
+ return extract_integer<LONGEST> (buf, byte_order);
+}
static inline LONGEST
extract_signed_integer (const gdb_byte *addr, int len,
enum bfd_endian byte_order)
{
- return extract_integer<LONGEST> (addr, len, byte_order);
+ return extract_signed_integer (gdb::array_view<const gdb_byte> (addr, len),
+ byte_order);
+}
+
+static inline ULONGEST
+extract_unsigned_integer (gdb::array_view<const gdb_byte> buf,
+ enum bfd_endian byte_order)
+{
+ return extract_integer<ULONGEST> (buf, byte_order);
}
static inline ULONGEST
extract_unsigned_integer (const gdb_byte *addr, int len,
enum bfd_endian byte_order)
{
- return extract_integer<ULONGEST> (addr, len, byte_order);
+ return extract_unsigned_integer (gdb::array_view<const gdb_byte> (addr, len),
+ byte_order);
}
extern int extract_long_unsigned_integer (const gdb_byte *, int,
diff --git a/gdb/dwarf2/expr.c b/gdb/dwarf2/expr.c
index e00a620406f..e308a50d5f5 100644
--- a/gdb/dwarf2/expr.c
+++ b/gdb/dwarf2/expr.c
@@ -1157,9 +1157,7 @@ dwarf_expr_context::fetch_address (int n)
ULONGEST result;
dwarf_require_integral (value_type (result_val));
- result = extract_unsigned_integer (value_contents (result_val).data (),
- TYPE_LENGTH (value_type (result_val)),
- byte_order);
+ result = extract_unsigned_integer (value_contents (result_val), byte_order);
/* For most architectures, calling extract_unsigned_integer() alone
is sufficient for extracting an address. However, some
diff --git a/gdb/findvar.c b/gdb/findvar.c
index d2b77133982..12134f88c0a 100644
--- a/gdb/findvar.c
+++ b/gdb/findvar.c
@@ -48,14 +48,11 @@ you lose
template<typename T, typename>
T
-extract_integer (const gdb_byte *addr, int len, enum bfd_endian byte_order)
+extract_integer (gdb::array_view<const gdb_byte> buf, enum bfd_endian byte_order)
{
typename std::make_unsigned<T>::type retval = 0;
- const unsigned char *p;
- const unsigned char *startaddr = addr;
- const unsigned char *endaddr = startaddr + len;
- if (len > (int) sizeof (T))
+ if (buf.size () > (int) sizeof (T))
error (_("\
That operation is not available on integers of more than %d bytes."),
(int) sizeof (T));
@@ -64,36 +61,38 @@ That operation is not available on integers of more than %d bytes."),
the least significant. */
if (byte_order == BFD_ENDIAN_BIG)
{
- p = startaddr;
+ size_t i = 0;
+
if (std::is_signed<T>::value)
{
/* Do the sign extension once at the start. */
- retval = ((LONGEST) * p ^ 0x80) - 0x80;
- ++p;
+ retval = ((LONGEST) buf[i] ^ 0x80) - 0x80;
+ ++i;
}
- for (; p < endaddr; ++p)
- retval = (retval << 8) | *p;
+ for (; i < buf.size (); ++i)
+ retval = (retval << 8) | buf[i];
}
else
{
- p = endaddr - 1;
+ ssize_t i = buf.size () - 1;
+
if (std::is_signed<T>::value)
{
/* Do the sign extension once at the start. */
- retval = ((LONGEST) * p ^ 0x80) - 0x80;
- --p;
+ retval = ((LONGEST) buf[i] ^ 0x80) - 0x80;
+ --i;
}
- for (; p >= startaddr; --p)
- retval = (retval << 8) | *p;
+ for (; i >= 0; --i)
+ retval = (retval << 8) | buf[i];
}
return retval;
}
/* Explicit instantiations. */
-template LONGEST extract_integer<LONGEST> (const gdb_byte *addr, int len,
+template LONGEST extract_integer<LONGEST> (gdb::array_view<const gdb_byte> buf,
enum bfd_endian byte_order);
-template ULONGEST extract_integer<ULONGEST> (const gdb_byte *addr, int len,
- enum bfd_endian byte_order);
+template ULONGEST extract_integer<ULONGEST>
+ (gdb::array_view<const gdb_byte> buf, enum bfd_endian byte_order);
/* Sometimes a long long unsigned integer can be extracted as a
LONGEST value. This is done so that we can print these values
diff --git a/gdb/regcache.c b/gdb/regcache.c
index 8457284c12a..b3ecba2fbe6 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -620,15 +620,12 @@ template<typename T, typename>
enum register_status
readable_regcache::raw_read (int regnum, T *val)
{
- gdb_byte *buf;
- enum register_status status;
-
assert_regnum (regnum);
- buf = (gdb_byte *) alloca (m_descr->sizeof_register[regnum]);
- status = raw_read (regnum, buf);
+ size_t len = m_descr->sizeof_register[regnum];
+ gdb_byte *buf = (gdb_byte *) alloca (len);
+ register_status status = raw_read (regnum, buf);
if (status == REG_VALID)
- *val = extract_integer<T> (buf,
- m_descr->sizeof_register[regnum],
+ *val = extract_integer<T> ({buf, len},
gdbarch_byte_order (m_descr->gdbarch));
else
*val = 0;
@@ -772,14 +769,12 @@ template<typename T, typename>
enum register_status
readable_regcache::cooked_read (int regnum, T *val)
{
- enum register_status status;
- gdb_byte *buf;
-
gdb_assert (regnum >= 0 && regnum < m_descr->nr_cooked_registers);
- buf = (gdb_byte *) alloca (m_descr->sizeof_register[regnum]);
- status = cooked_read (regnum, buf);
+ size_t len = m_descr->sizeof_register[regnum];
+ gdb_byte *buf = (gdb_byte *) alloca (len);
+ register_status status = cooked_read (regnum, buf);
if (status == REG_VALID)
- *val = extract_integer<T> (buf, m_descr->sizeof_register[regnum],
+ *val = extract_integer<T> ({buf, len},
gdbarch_byte_order (m_descr->gdbarch));
else
*val = 0;
diff --git a/gdb/unittests/gmp-utils-selftests.c b/gdb/unittests/gmp-utils-selftests.c
index f40666e4cd5..e48bb39166e 100644
--- a/gdb/unittests/gmp-utils-selftests.c
+++ b/gdb/unittests/gmp-utils-selftests.c
@@ -299,7 +299,7 @@ write_and_extract (T val, size_t buf_len, enum bfd_endian byte_order)
gdb_byte *buf = (gdb_byte *) alloca (buf_len);
v.write ({buf, buf_len}, byte_order, !std::is_signed<T>::value);
- return extract_integer<T> (buf, buf_len, byte_order);
+ return extract_integer<T> ({buf, buf_len}, byte_order);
}
/* Test the gdb_mpz::write method over a reasonable range of values.
--
2.33.0
next prev parent reply other threads:[~2021-11-08 21:06 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-11-08 21:06 [PATCH 1/4] gdb: fix length of array view returned by some value_contents functions Simon Marchi via Gdb-patches
2021-11-08 21:06 ` [PATCH 2/4] gdbsupport: add array_view copy function Simon Marchi via Gdb-patches
2021-11-08 21:11 ` Simon Marchi via Gdb-patches
2021-11-16 20:54 ` Tom Tromey
2021-11-16 20:53 ` Tom Tromey
2021-11-16 21:56 ` Simon Marchi via Gdb-patches
2021-11-17 0:20 ` Tom Tromey
2021-11-18 20:07 ` Simon Marchi
2021-11-08 21:06 ` Simon Marchi via Gdb-patches [this message]
2021-11-08 21:06 ` [PATCH 4/4] gdb: trivial changes to use array_view Simon Marchi via Gdb-patches
2021-11-16 20:41 ` [PATCH 1/4] gdb: fix length of array view returned by some value_contents functions Tom Tromey
2021-11-16 21:40 ` 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=20211108210609.353208-3-simon.marchi@efficios.com \
--to=gdb-patches@sourceware.org \
--cc=simon.marchi@efficios.com \
/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