GDB: Only make data actually retrieved into value history available

While it makes sense to allow accessing out-of-bounds elements in the
debuggee and see whatever there might happen to be there in memory (we
are a debugger and not a programming rules enforcement facility and we
want to make people's life easier in chasing bugs), e.g.:

  (gdb) print one_hundred[-1]
  $1 = 0
  (gdb) print one_hundred[100]
  $2 = 0
  (gdb)

we shouldn't really pretend that we have any meaningful data around
values recorded in history (what these commands really retrieve are
current debuggee memory contents outside the original data accessed,
really confusing in my opinion).  Mark values recorded in history as
such then and verify accesses to be in-range for them:

  (gdb) print one_hundred[-1]
  $1 = <unavailable>
  (gdb) print one_hundred[100]
  $2 = <unavailable>

Add a suitable test case, which also covers integer overflows in data
location calculation.

Approved-By: Tom Tromey <tom@tromey.com>

diff --git a/gdb/testsuite/gdb.base/value-history-unavailable.c b/gdb/testsuite/gdb.base/value-history-unavailable.c
new file mode 100644 (file)
index 0000000..b0c2f0a
--- /dev/null
+++ b/gdb/testsuite/gdb.base/value-history-unavailable.c
@@ -0,0 +1,29 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright (C) 2023 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+struct
+{
+  unsigned char x[1024];
+  unsigned char a[1024];
+  unsigned char y[1024];
+} a = { {-1} };
+
+int
+main ()
+{
+  return 0;
+}

diff --git a/gdb/testsuite/gdb.base/value-history-unavailable.exp b/gdb/testsuite/gdb.base/value-history-unavailable.exp
new file mode 100644 (file)
index 0000000..8949be0
--- /dev/null
+++ b/gdb/testsuite/gdb.base/value-history-unavailable.exp
@@ -0,0 +1,73 @@
+# Copyright (C) 2023 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test GDB's value availability ranges.
+
+standard_testfile
+
+if { [prepare_for_testing "failed to prepare" $testfile $srcfile] } {
+    return -1
+}
+
+if ![runto_main] then {
+    perror "couldn't run to breakpoint"
+    continue
+}
+
+set target_char_mask [get_valueof "/u" "a.x\[0]" "255" "get target char mask"]
+set target_char_bit 0
+for {set i $target_char_mask} {$i > 0} {set i [expr $i >> 1]} {
+    incr target_char_bit
+}
+set target_char_rank -1
+for {set i $target_char_bit} {$i > 0} {set i [expr $i >> 1]} {
+    incr target_char_rank
+}
+
+# Verify accesses to original inferior data.
+gdb_test "print a.a" "\\\$2 = '\\\\000' <repeats 1023 times>"
+gdb_test "print a.a\[-1\]" "\\\$3 = 0 '\\\\000'"
+gdb_test "print a.a\[1024\]" "\\\$4 = 0 '\\\\000'"
+
+# Verify in-range value history accesses.
+gdb_test "print \$2" "\\\$5 = '\\\\000' <repeats 1023 times>"
+gdb_test "print \$2\[0\]" "\\\$6 = 0 '\\\\000'"
+gdb_test "print \$2\[1023\]" "\\\$7 = 0 '\\\\000'"
+
+# Values outside the array recorded will have not been retrieved.
+gdb_test "print \$2\[-1\]" "\\\$8 = <unavailable>"
+gdb_test "print \$2\[1024\]" "\\\$9 = <unavailable>"
+gdb_test "print \$2\[-1LL << 63 - $target_char_rank\]" \
+       "\\\$10 = <unavailable>"
+gdb_test "print \$2\[(1LL << 63 - $target_char_rank) - 1\]" \
+       "\\\$11 = <unavailable>"
+
+# Accesses through pointers in history go straight to the inferior though.
+gdb_test "print \$2\[0\]@1" "\\\$12 = \"\""
+gdb_test "print \$2\[-1\]@1" "\\\$13 = \"\""
+gdb_test "print \$2\[1024\]@1" "\\\$14 = \"\""
+
+# Verify out-of-range value history accesses.
+gdb_test "print \$2\[(-1LL << 63 - $target_char_rank) - 1\]" \
+    "Integer overflow in data location calculation"
+gdb_test "print \$2\[(1LL << 63 - $target_char_rank)\]" \
+    "Integer overflow in data location calculation"
+gdb_test "print \$2\[-1LL << 63\]" \
+    "Integer overflow in data location calculation"
+gdb_test "print \$2\[(1ULL << 63) - 1\]" \
+    "Integer overflow in data location calculation"
+
+# Sanity-check a copy of an unavailable value.
+gdb_test "print \$11" "\\\$15 = <unavailable>"

diff --git a/gdb/valarith.c b/gdb/valarith.c
index 1dce9d0f613820ba01c3c7876ff66980fbd977f9..7312f0b549389a405b389e8a5fbc740591cbbf58 100644 (file)
--- a/gdb/valarith.c
+++ b/gdb/valarith.c
@@ -182,6 +182,21 @@ value_subscript (struct value *array, LONGEST index)
        }
 
       index -= *lowerbound;
+
+      /* Do not try to dereference a pointer to an unavailable value.
+        Instead mock up a new one and give it the original address.  */
+      struct type *elt_type = check_typedef (tarray->target_type ());
+      LONGEST elt_size = type_length_units (elt_type);
+      if (!value_lazy (array)
+         && !value_bytes_available (array, elt_size * index, elt_size))
+       {
+         struct value *val = allocate_value (elt_type);
+         mark_value_bytes_unavailable (val, 0, elt_size);
+         VALUE_LVAL (val) = lval_memory;
+         set_value_address (val, value_address (array) + elt_size * index);
+         return val;
+       }
+
       array = value_coerce_array (array);
     }
 

diff --git a/gdb/value.c b/gdb/value.c
index 57625b005aca614b40978f8509607abea86d3da9..fbdb07959c6e8f68e07d95665fa79ec409703a73 100644 (file)
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -185,6 +185,7 @@ struct value
       initialized (1),
       stack (0),
       is_zero (false),
+      in_history (false),
       type (type_),
       enclosing_type (type_)
   {
@@ -239,6 +240,9 @@ struct value
      otherwise.  */
   bool is_zero : 1;
 
+  /* True if this a value recorded in value history; false otherwise.  */
+  bool in_history : 1;
+
   /* Location of value (if lval).  */
   union
   {
@@ -385,7 +389,13 @@ value_bits_available (const struct value *value,
 {
   gdb_assert (!value->lazy);
 
-  return !ranges_contain (value->unavailable, offset, length);
+  /* Don't pretend we have anything available there in the history beyond
+     the boundaries of the value recorded.  It's not like inferior memory
+     where there is actual stuff underneath.  */
+  ULONGEST val_len = TARGET_CHAR_BIT * value_enclosing_type (value)->length ();
+  return !((value->in_history
+           && (offset < 0 || offset + length > val_len))
+          || ranges_contain (value->unavailable, offset, length));
 }
 
 int
@@ -1797,6 +1807,7 @@ value_copy (const value *arg)
   val->modifiable = arg->modifiable;
   val->stack = arg->stack;
   val->is_zero = arg->is_zero;
+  val->in_history = arg->in_history;
   val->initialized = arg->initialized;
   val->unavailable = arg->unavailable;
   val->optimized_out = arg->optimized_out;
@@ -1950,6 +1961,10 @@ record_latest_value (struct value *val)
      a value on the value history never changes.  */
   if (value_lazy (val))
     value_fetch_lazy (val);
+
+  /* Mark the value as recorded in the history for the availability check.  */
+  val->in_history = true;
+
   /* We preserve VALUE_LVAL so that the user can find out where it was fetched
      from.  This is a bit dubious, because then *&$1 does not just return $1
      but the current contents of that location.  c'est la vie...  */