public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: "Maciej W. Rozycki" <macro@embecosm.com>
To: gdb-patches@sourceware.org
Cc: Andrew Burgess <aburgess@redhat.com>, Tom Tromey <tom@tromey.com>,
	 Richard Bunt <Richard.Bunt@arm.com>
Subject: [PATCH v4 3/6] GDB: Fix the mess with value byte/bit range types
Date: Fri, 10 Feb 2023 14:19:23 +0000 (GMT)	[thread overview]
Message-ID: <alpine.DEB.2.20.2302092316350.7841@tpp.orcam.me.uk> (raw)
In-Reply-To: <alpine.DEB.2.20.2302092205020.7841@tpp.orcam.me.uk>

Consistently use the LONGEST and ULONGEST types for value byte/bit 
offsets and lengths respectively, avoiding silent truncation for ranges 
exceeding the 32-bit span, which may cause incorrect matching.  Also 
report a conversion overflow on byte ranges that cannot be expressed in 
terms of bits with these data types, e.g.:

  (gdb) print one_hundred[1LL << 58]
  Integer overflow in data location calculation
  (gdb) print one_hundred[(-1LL << 58) - 1]
  Integer overflow in data location calculation
  (gdb)

Previously such accesses would be let through with unpredictable results 
produced.
---
Changes from v3:

- s/slient/silent/ in change description.

No change from v2.

New change in v2.
---
 gdb/value.c |   43 ++++++++++++++++++++++++++-----------------
 gdb/value.h |    8 ++++----
 2 files changed, 30 insertions(+), 21 deletions(-)

gdb-value-range-types.diff
Index: src/gdb/value.c
===================================================================
--- src.orig/gdb/value.c
+++ src/gdb/value.c
@@ -74,7 +74,7 @@ struct range
   LONGEST offset;
 
   /* Length of the range.  */
-  LONGEST length;
+  ULONGEST length;
 
   /* Returns true if THIS is strictly less than OTHER, useful for
      searching.  We keep ranges sorted by offset and coalesce
@@ -97,10 +97,10 @@ struct range
    [offset2, offset2+len2) overlap.  */
 
 static int
-ranges_overlap (LONGEST offset1, LONGEST len1,
-		LONGEST offset2, LONGEST len2)
+ranges_overlap (LONGEST offset1, ULONGEST len1,
+		LONGEST offset2, ULONGEST len2)
 {
-  ULONGEST h, l;
+  LONGEST h, l;
 
   l = std::max (offset1, offset2);
   h = std::min (offset1 + len1, offset2 + len2);
@@ -112,7 +112,7 @@ ranges_overlap (LONGEST offset1, LONGEST
 
 static int
 ranges_contain (const std::vector<range> &ranges, LONGEST offset,
-		LONGEST length)
+		ULONGEST length)
 {
   range what;
 
@@ -380,7 +380,8 @@ get_value_arch (const struct value *valu
 }
 
 int
-value_bits_available (const struct value *value, LONGEST offset, LONGEST length)
+value_bits_available (const struct value *value,
+		      LONGEST offset, ULONGEST length)
 {
   gdb_assert (!value->lazy);
 
@@ -389,8 +390,16 @@ value_bits_available (const struct value
 
 int
 value_bytes_available (const struct value *value,
-		       LONGEST offset, LONGEST length)
+		       LONGEST offset, ULONGEST length)
 {
+  ULONGEST sign = (1ULL << (sizeof (ULONGEST) * 8 - 1)) / TARGET_CHAR_BIT;
+  ULONGEST mask = (sign << 1) - 1;
+
+  if (offset != ((offset & mask) ^ sign) - sign
+      || length != ((length & mask) ^ sign) - sign
+      || (length > 0 && (~offset & (offset + length - 1) & sign) != 0))
+    error (_("Integer overflow in data location calculation"));
+
   return value_bits_available (value,
 			       offset * TARGET_CHAR_BIT,
 			       length * TARGET_CHAR_BIT);
@@ -460,7 +469,7 @@ value_entirely_optimized_out (struct val
 
 static void
 insert_into_bit_range_vector (std::vector<range> *vectorp,
-			      LONGEST offset, LONGEST length)
+			      LONGEST offset, ULONGEST length)
 {
   range newr;
 
@@ -558,8 +567,8 @@ insert_into_bit_range_vector (std::vecto
       if (ranges_overlap (bef.offset, bef.length, offset, length))
 	{
 	  /* #1 */
-	  ULONGEST l = std::min (bef.offset, offset);
-	  ULONGEST h = std::max (bef.offset + bef.length, offset + length);
+	  LONGEST l = std::min (bef.offset, offset);
+	  LONGEST h = std::max (bef.offset + bef.length, offset + length);
 
 	  bef.offset = l;
 	  bef.length = h - l;
@@ -600,7 +609,7 @@ insert_into_bit_range_vector (std::vecto
 	  struct range &r = *i;
 	  if (r.offset <= t.offset + t.length)
 	    {
-	      ULONGEST l, h;
+	      LONGEST l, h;
 
 	      l = std::min (t.offset, r.offset);
 	      h = std::max (t.offset + t.length, r.offset + r.length);
@@ -626,14 +635,14 @@ insert_into_bit_range_vector (std::vecto
 
 void
 mark_value_bits_unavailable (struct value *value,
-			     LONGEST offset, LONGEST length)
+			     LONGEST offset, ULONGEST length)
 {
   insert_into_bit_range_vector (&value->unavailable, offset, length);
 }
 
 void
 mark_value_bytes_unavailable (struct value *value,
-			      LONGEST offset, LONGEST length)
+			      LONGEST offset, ULONGEST length)
 {
   mark_value_bits_unavailable (value,
 			       offset * TARGET_CHAR_BIT,
@@ -786,7 +795,7 @@ static int
 find_first_range_overlap_and_match (struct ranges_and_idx *rp1,
 				    struct ranges_and_idx *rp2,
 				    LONGEST offset1, LONGEST offset2,
-				    LONGEST length, ULONGEST *l, ULONGEST *h)
+				    ULONGEST length, ULONGEST *l, ULONGEST *h)
 {
   rp1->idx = find_first_range_overlap (rp1->ranges, rp1->idx,
 				       offset1, length);
@@ -1306,14 +1315,14 @@ value_contents_all (struct value *value)
 static void
 ranges_copy_adjusted (std::vector<range> *dst_range, int dst_bit_offset,
 		      const std::vector<range> &src_range, int src_bit_offset,
-		      int bit_length)
+		      unsigned int bit_length)
 {
   for (const range &r : src_range)
     {
-      ULONGEST h, l;
+      LONGEST h, l;
 
       l = std::max (r.offset, (LONGEST) src_bit_offset);
-      h = std::min (r.offset + r.length,
+      h = std::min ((LONGEST) (r.offset + r.length),
 		    (LONGEST) src_bit_offset + bit_length);
 
       if (l < h)
Index: src/gdb/value.h
===================================================================
--- src.orig/gdb/value.h
+++ src/gdb/value.h
@@ -505,7 +505,7 @@ extern int value_bits_synthetic_pointer
    byte is unavailable.  */
 
 extern int value_bytes_available (const struct value *value,
-				  LONGEST offset, LONGEST length);
+				  LONGEST offset, ULONGEST length);
 
 /* Given a value, determine whether the contents bits starting at
    OFFSET and extending for LENGTH bits are available.  This returns
@@ -513,7 +513,7 @@ extern int value_bytes_available (const
    bit is unavailable.  */
 
 extern int value_bits_available (const struct value *value,
-				 LONGEST offset, LONGEST length);
+				 LONGEST offset, ULONGEST length);
 
 /* Like value_bytes_available, but return false if any byte in the
    whole object is unavailable.  */
@@ -527,13 +527,13 @@ extern int value_entirely_unavailable (s
    LENGTH bytes as unavailable.  */
 
 extern void mark_value_bytes_unavailable (struct value *value,
-					  LONGEST offset, LONGEST length);
+					  LONGEST offset, ULONGEST length);
 
 /* Mark VALUE's content bits starting at OFFSET and extending for
    LENGTH bits as unavailable.  */
 
 extern void mark_value_bits_unavailable (struct value *value,
-					 LONGEST offset, LONGEST length);
+					 LONGEST offset, ULONGEST length);
 
 /* Compare LENGTH bytes of VAL1's contents starting at OFFSET1 with
    LENGTH bytes of VAL2's contents starting at OFFSET2.

  parent reply	other threads:[~2023-02-10 14:19 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-10 14:18 [PATCH v4 0/6] gdb: introduce limited array lengths while printing values Maciej W. Rozycki
2023-02-10 14:19 ` [PATCH v4 1/6] GDB: Switch to using C++ standard integer type limits Maciej W. Rozycki
2023-02-10 21:13   ` Tom Tromey
2023-02-10 14:19 ` [PATCH v4 2/6] GDB: Ignore `max-value-size' setting with value history accesses Maciej W. Rozycki
2023-02-10 14:19 ` Maciej W. Rozycki [this message]
2023-02-10 14:19 ` [PATCH v4 4/6] GDB: Only make data actually retrieved into value history available Maciej W. Rozycki
2023-02-10 21:16   ` Tom Tromey
2023-02-10 14:19 ` [PATCH v4 5/6] GDB/testsuite: Add `-nonl' option to `gdb_test' Maciej W. Rozycki
2023-02-10 14:19 ` [PATCH v4 6/6] GDB: Introduce limited array lengths while printing values Maciej W. Rozycki
2023-02-13 14:45   ` Simon Marchi
2023-02-14 19:20     ` Maciej W. Rozycki
2023-02-23 21:16       ` Maciej W. Rozycki
2023-02-10 21:17 ` [PATCH v4 0/6] gdb: introduce " Tom Tromey
2023-02-10 23:50   ` Maciej W. Rozycki

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=alpine.DEB.2.20.2302092316350.7841@tpp.orcam.me.uk \
    --to=macro@embecosm.com \
    --cc=Richard.Bunt@arm.com \
    --cc=aburgess@redhat.com \
    --cc=gdb-patches@sourceware.org \
    --cc=tom@tromey.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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).