[v2,2/3] gdb: make extract_integer take an array_view

Message ID 20211118212951.2547899-3-simon.marchi@efficios.com
State New
Headers show
Series
  • Add array_view copy function and more
Related show

Commit Message

Mike Frysinger via Gdb-patches Nov. 18, 2021, 9:29 p.m.
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(-)

-- 
2.33.1

Patch

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 f7e632809d0..a0031d2dadd 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.