[binutils-gdb] GDB: Only make data actually retrieved into value history available

[binutils-gdb] GDB: Only make data actually retrieved into value history available

[Maciej W. Rozycki]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=aaab5fce4f25e0d3b1a85edc7d6ac8f7d06f599c

commit aaab5fce4f25e0d3b1a85edc7d6ac8f7d06f599c
Author: Maciej W. Rozycki <macro@embecosm.com>
Date:   Fri Feb 10 23:49:19 2023 +0000

    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:
---
 gdb/testsuite/gdb.base/value-history-unavailable.c | 29 +++++++++
 .../gdb.base/value-history-unavailable.exp         | 73 ++++++++++++++++++++++
 gdb/valarith.c                                     | 15 +++++
 gdb/value.c                                        | 17 ++++-
 4 files changed, 133 insertions(+), 1 deletion(-)

diff --git a/gdb/testsuite/gdb.base/value-history-unavailable.c b/gdb/testsuite/gdb.base/value-history-unavailable.c
new file mode 100644
index 00000000000..b0c2f0afc0b
--- /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
index 00000000000..8949be0e1af
--- /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 1dce9d0f613..7312f0b5493 100644
--- 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 57625b005ac..fbdb07959c6 100644
--- 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...  */