From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from barracuda.ebox.ca (barracuda.ebox.ca [96.127.255.19]) by sourceware.org (Postfix) with ESMTPS id 002473857C50 for ; Thu, 18 Nov 2021 21:30:00 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 002473857C50 X-ASG-Debug-ID: 1637270993-0c856e2e4717b260001-fS2M51 Received: from smtp.ebox.ca (smtp.ebox.ca [96.127.255.82]) by barracuda.ebox.ca with ESMTP id SmWOIUBCPJmQH545 (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Thu, 18 Nov 2021 16:29:53 -0500 (EST) X-Barracuda-Envelope-From: simon.marchi@efficios.com X-Barracuda-RBL-Trusted-Forwarder: 96.127.255.82 Received: from epycamd.internal.efficios.com (192-222-180-24.qc.cable.ebox.net [192.222.180.24]) by smtp.ebox.ca (Postfix) with ESMTP id 3DD8B441D65; Thu, 18 Nov 2021 16:29:53 -0500 (EST) From: Simon Marchi X-Barracuda-RBL-IP: 192.222.180.24 X-Barracuda-Effective-Source-IP: 192-222-180-24.qc.cable.ebox.net[192.222.180.24] X-Barracuda-Apparent-Source-IP: 192.222.180.24 To: gdb-patches@sourceware.org Subject: [PATCH v2 2/3] gdb: make extract_integer take an array_view Date: Thu, 18 Nov 2021 16:29:50 -0500 X-ASG-Orig-Subj: [PATCH v2 2/3] gdb: make extract_integer take an array_view Message-Id: <20211118212951.2547899-3-simon.marchi@efficios.com> X-Mailer: git-send-email 2.33.1 In-Reply-To: <20211118212951.2547899-1-simon.marchi@efficios.com> References: <20211118212951.2547899-1-simon.marchi@efficios.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Barracuda-Connect: smtp.ebox.ca[96.127.255.82] X-Barracuda-Start-Time: 1637270993 X-Barracuda-Encrypted: DHE-RSA-AES256-SHA X-Barracuda-URL: https://96.127.255.19:443/cgi-mod/mark.cgi X-Virus-Scanned: by bsmtpd at ebox.ca X-Barracuda-Scan-Msg-Size: 8375 X-Barracuda-BRTS-Status: 1 X-Barracuda-Spam-Score: 0.00 X-Barracuda-Spam-Status: No, SCORE=0.00 using global scores of TAG_LEVEL=1000.0 QUARANTINE_LEVEL=1000.0 KILL_LEVEL=8.0 tests= X-Barracuda-Spam-Report: Code version 3.2, rules version 3.2.3.94047 Rule breakdown below pts rule name description ---- ---------------------- -------------------------------------------------- X-Spam-Status: No, score=-19.5 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_NONE, KAM_DMARC_STATUS, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_SOFTFAIL, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 18 Nov 2021 21:30:02 -0000 From: Simon Marchi 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> -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 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 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/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. -- 2.33.1