From 2a50938ab740296a1d6df67feea9401e57e4d90e Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Mon, 25 Oct 2021 23:29:34 -0400 Subject: [PATCH] gdb: make extract_integer take an array_view 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/amd64-linux-tdep.c | 2 +- gdb/defs.h | 23 ++++++++++++++++--- gdb/dwarf2/expr.c | 7 ++---- gdb/fbsd-tdep.c | 6 ++--- gdb/findvar.c | 35 ++++++++++++++--------------- gdb/frame.c | 4 +--- gdb/frv-tdep.c | 2 +- gdb/hppa-bsd-tdep.c | 2 +- gdb/hppa-linux-tdep.c | 2 +- gdb/i386-linux-tdep.c | 2 +- gdb/ia64-tdep.c | 4 ++-- gdb/regcache.c | 21 +++++++---------- gdb/unittests/gmp-utils-selftests.c | 2 +- gdb/valops.c | 3 +-- 14 files changed, 60 insertions(+), 55 deletions(-) diff --git a/gdb/amd64-linux-tdep.c b/gdb/amd64-linux-tdep.c index 817a197ceaa..3fe3d394932 100644 --- a/gdb/amd64-linux-tdep.c +++ b/gdb/amd64-linux-tdep.c @@ -237,7 +237,7 @@ amd64_linux_get_syscall_number (struct gdbarch *gdbarch, is stored at %rax register. */ regcache->cooked_read (AMD64_LINUX_ORIG_RAX_REGNUM, buf); - ret = extract_signed_integer (buf, 8, byte_order); + ret = extract_signed_integer (buf, byte_order); return ret; } 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> -T extract_integer (const gdb_byte *addr, int len, enum bfd_endian byte_order); +T extract_integer (gdb::array_view, enum bfd_endian byte_order); + +static inline LONGEST +extract_signed_integer (gdb::array_view buf, + enum bfd_endian byte_order) +{ + return extract_integer (buf, byte_order); +} static inline LONGEST extract_signed_integer (const gdb_byte *addr, int len, enum bfd_endian byte_order) { - return extract_integer (addr, len, byte_order); + return extract_signed_integer (gdb::array_view (addr, len), + byte_order); +} + +static inline ULONGEST +extract_unsigned_integer (gdb::array_view buf, + enum bfd_endian byte_order) +{ + return extract_integer (buf, byte_order); } static inline ULONGEST extract_unsigned_integer (const gdb_byte *addr, int len, enum bfd_endian byte_order) { - return extract_integer (addr, len, byte_order); + return extract_unsigned_integer (gdb::array_view (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 592dbe19d56..b267785ef51 100644 --- a/gdb/dwarf2/expr.c +++ b/gdb/dwarf2/expr.c @@ -577,8 +577,7 @@ indirect_pieced_value (value *value) encode address spaces and other things in CORE_ADDR. */ bfd_endian byte_order = gdbarch_byte_order (get_frame_arch (frame)); LONGEST byte_offset - = extract_signed_integer (value_contents (value).data (), - TYPE_LENGTH (type), byte_order); + = extract_signed_integer (value_contents (value), byte_order); byte_offset += piece->v.ptr.offset; return indirect_synthetic_pointer (piece->v.ptr.die_sect_off, @@ -1157,9 +1156,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/fbsd-tdep.c b/gdb/fbsd-tdep.c index 07cd844c818..4da7798544b 100644 --- a/gdb/fbsd-tdep.c +++ b/gdb/fbsd-tdep.c @@ -611,7 +611,7 @@ fbsd_core_xfer_siginfo (struct gdbarch *gdbarch, gdb_byte *readbuf, LWPINFO_OFFSET + LWPINFO_PL_FLAGS, 4)) return -1; - int pl_flags = extract_signed_integer (buf, 4, gdbarch_byte_order (gdbarch)); + int pl_flags = extract_signed_integer (buf, gdbarch_byte_order (gdbarch)); if (!(pl_flags & PL_FLAG_SI)) return -1; @@ -1933,7 +1933,7 @@ fbsd_read_integer_by_name (struct gdbarch *gdbarch, const char *name) if (target_read_memory (BMSYMBOL_VALUE_ADDRESS (ms), buf, sizeof buf) != 0) error (_("Unable to read value of '%s'"), name); - return extract_signed_integer (buf, sizeof buf, gdbarch_byte_order (gdbarch)); + return extract_signed_integer (buf, gdbarch_byte_order (gdbarch)); } /* Lookup offsets of fields in the runtime linker's 'Obj_Entry' @@ -2004,7 +2004,7 @@ fbsd_get_tls_index (struct gdbarch *gdbarch, CORE_ADDR lm_addr) throw_error (TLS_GENERIC_ERROR, _("Cannot find thread-local variables on this target")); - return extract_signed_integer (buf, sizeof buf, gdbarch_byte_order (gdbarch)); + return extract_signed_integer (buf, gdbarch_byte_order (gdbarch)); } /* See fbsd-tdep.h. */ 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 T -extract_integer (const gdb_byte *addr, int len, enum bfd_endian byte_order) +extract_integer (gdb::array_view buf, enum bfd_endian byte_order) { typename std::make_unsigned::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::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::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 (const gdb_byte *addr, int len, +template LONGEST extract_integer (gdb::array_view buf, enum bfd_endian byte_order); -template ULONGEST extract_integer (const gdb_byte *addr, int len, - enum bfd_endian byte_order); +template ULONGEST extract_integer + (gdb::array_view 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/frame.c b/gdb/frame.c index 2a899fc494f..7944d1edef8 100644 --- a/gdb/frame.c +++ b/gdb/frame.c @@ -1288,7 +1288,6 @@ frame_unwind_register_signed (frame_info *next_frame, int regnum) { struct gdbarch *gdbarch = frame_unwind_arch (next_frame); enum bfd_endian byte_order = gdbarch_byte_order (gdbarch); - int size = register_size (gdbarch, regnum); struct value *value = frame_unwind_register_value (next_frame, regnum); gdb_assert (value != NULL); @@ -1304,8 +1303,7 @@ frame_unwind_register_signed (frame_info *next_frame, int regnum) _("Register %d is not available"), regnum); } - LONGEST r = extract_signed_integer (value_contents_all (value).data (), size, - byte_order); + LONGEST r = extract_signed_integer (value_contents_all (value), byte_order); release_value (value); return r; diff --git a/gdb/frv-tdep.c b/gdb/frv-tdep.c index 09274d54a0e..3e2729f6c91 100644 --- a/gdb/frv-tdep.c +++ b/gdb/frv-tdep.c @@ -596,7 +596,7 @@ frv_analyze_prologue (struct gdbarch *gdbarch, CORE_ADDR pc, if (target_read_memory (pc, buf, sizeof buf) != 0) break; - op = extract_signed_integer (buf, sizeof buf, byte_order); + op = extract_signed_integer (buf, byte_order); next_pc = pc + 4; diff --git a/gdb/hppa-bsd-tdep.c b/gdb/hppa-bsd-tdep.c index f4567b48f82..9021414a460 100644 --- a/gdb/hppa-bsd-tdep.c +++ b/gdb/hppa-bsd-tdep.c @@ -75,7 +75,7 @@ hppabsd_find_global_pointer (struct gdbarch *gdbarch, struct value *function) if (target_read_memory (addr, buf, sizeof buf) != 0) break; - tag = extract_signed_integer (buf, sizeof buf, byte_order); + tag = extract_signed_integer (buf, byte_order); if (tag == DT_PLTGOT) { CORE_ADDR pltgot; diff --git a/gdb/hppa-linux-tdep.c b/gdb/hppa-linux-tdep.c index 1dd6993ab09..6bb8580aa62 100644 --- a/gdb/hppa-linux-tdep.c +++ b/gdb/hppa-linux-tdep.c @@ -384,7 +384,7 @@ hppa_linux_find_global_pointer (struct gdbarch *gdbarch, status = target_read_memory (addr, buf, sizeof (buf)); if (status != 0) break; - tag = extract_signed_integer (buf, sizeof (buf), byte_order); + tag = extract_signed_integer (buf, byte_order); if (tag == DT_PLTGOT) { diff --git a/gdb/i386-linux-tdep.c b/gdb/i386-linux-tdep.c index 898b73f632c..7c6274589c9 100644 --- a/gdb/i386-linux-tdep.c +++ b/gdb/i386-linux-tdep.c @@ -549,7 +549,7 @@ i386_linux_get_syscall_number_from_regcache (struct regcache *regcache) is stored at %eax register. */ regcache->cooked_read (I386_LINUX_ORIG_EAX_REGNUM, buf); - ret = extract_signed_integer (buf, 4, byte_order); + ret = extract_signed_integer (buf, byte_order); return ret; } diff --git a/gdb/ia64-tdep.c b/gdb/ia64-tdep.c index 829909dab18..f1af7cb0fec 100644 --- a/gdb/ia64-tdep.c +++ b/gdb/ia64-tdep.c @@ -3450,7 +3450,7 @@ ia64_find_global_pointer_from_dynamic_section (struct gdbarch *gdbarch, status = target_read_memory (addr, buf, sizeof (buf)); if (status != 0) break; - tag = extract_signed_integer (buf, sizeof (buf), byte_order); + tag = extract_signed_integer (buf, byte_order); if (tag == DT_PLTGOT) { @@ -3531,7 +3531,7 @@ find_extant_func_descr (struct gdbarch *gdbarch, CORE_ADDR faddr) status = target_read_memory (addr, buf, sizeof (buf)); if (status != 0) break; - faddr2 = extract_signed_integer (buf, sizeof (buf), byte_order); + faddr2 = extract_signed_integer (buf, byte_order); if (faddr == faddr2) return addr; 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 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 (buf, - m_descr->sizeof_register[regnum], + *val = extract_integer ({buf, len}, gdbarch_byte_order (m_descr->gdbarch)); else *val = 0; @@ -772,14 +769,12 @@ template 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 (buf, m_descr->sizeof_register[regnum], + *val = extract_integer ({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::value); - return extract_integer (buf, buf_len, byte_order); + return extract_integer ({buf, buf_len}, byte_order); } /* Test the gdb_mpz::write method over a reasonable range of values. diff --git a/gdb/valops.c b/gdb/valops.c index ca71c128de9..779ca93edd7 100644 --- a/gdb/valops.c +++ b/gdb/valops.c @@ -587,8 +587,7 @@ value_cast (struct type *type, struct value *arg2) bits. */ if (code2 == TYPE_CODE_PTR) longest = extract_unsigned_integer - (value_contents (arg2).data (), TYPE_LENGTH (type2), - type_byte_order (type2)); + (value_contents (arg2), type_byte_order (type2)); else longest = value_as_long (arg2); return value_from_longest (to_type, convert_to_boolean ? -- 2.30.2