[binutils-gdb] GDB: Ignore `max-value-size' setting with value history accesses

[binutils-gdb] GDB: Ignore `max-value-size' setting with value history accesses

[Maciej W. Rozycki]

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

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

    GDB: Ignore `max-value-size' setting with value history accesses
    
    We have an inconsistency in value history accesses where array element
    accesses cause an error for entries exceeding the currently selected
    `max-value-size' setting even where such accesses successfully complete
    for elements located in the inferior, e.g.:
    
      (gdb) p/d one
      $1 = 0
      (gdb) p/d one_hundred
      $2 = {0 <repeats 100 times>}
      (gdb) p/d one_hundred[99]
      $3 = 0
      (gdb) set max-value-size 25
      (gdb) p/d one_hundred
      value requires 100 bytes, which is more than max-value-size
      (gdb) p/d one_hundred[99]
      $7 = 0
      (gdb) p/d $2
      value requires 100 bytes, which is more than max-value-size
      (gdb) p/d $2[99]
      value requires 100 bytes, which is more than max-value-size
      (gdb)
    
    According to our documentation the `max-value-size' setting is a safety
    guard against allocating an overly large amount of memory.  Moreover a
    statement in documentation says, concerning this setting, that: "Setting
    this variable does not affect values that have already been allocated
    within GDB, only future allocations."  While in the implementer-speak
    the sentence may be unambiguous I think the outside user may well infer
    that the setting does not apply to values previously printed.
    
    Therefore rather than just fixing this inconsistency it seems reasonable
    to lift the setting for value history accesses, under an implication
    that by having been retrieved from the debuggee they have already passed
    the safety check.  Do it then, by suppressing the value size check in
    `value_copy' -- under an observation that if the original value has been
    already loaded (i.e. it's not lazy), then it must have previously passed
    said check -- making the last two commands succeed:
    
      (gdb) p/d $2
      $8 = {0 <repeats 100 times>}
      (gdb) p/d $2 [99]
      $9 = 0
      (gdb)
    
    Expand the testsuite accordingly, covering both value history handling
    and the use of `value_copy' by `make_cv_value', used by Python code.

Diff:
---
 gdb/testsuite/gdb.base/max-value-size.exp |  3 +++
 gdb/testsuite/gdb.python/py-xmethods.exp  | 17 ++++++++++++++
 gdb/testsuite/gdb.python/py-xmethods.py   | 17 ++++++++++++++
 gdb/value.c                               | 38 +++++++++++++++++++------------
 4 files changed, 61 insertions(+), 14 deletions(-)

diff --git a/gdb/testsuite/gdb.base/max-value-size.exp b/gdb/testsuite/gdb.base/max-value-size.exp
index cc9ae775e38..199067f5914 100644
--- a/gdb/testsuite/gdb.base/max-value-size.exp
+++ b/gdb/testsuite/gdb.base/max-value-size.exp
@@ -53,6 +53,9 @@ proc do_value_printing { max_value_size test_prefix } {
 	    gdb_test "p/d one_hundred" " = \\{0 <repeats 100 times>\\}"
 	}
 	gdb_test "p/d one_hundred \[99\]" " = 0"
+	# Verify that accessing value history is undisturbed.
+	gdb_test "p/d \$2" " = \\{0 <repeats 100 times>\\}"
+	gdb_test "p/d \$2 \[99\]" " = 0"
     }
 }
 
diff --git a/gdb/testsuite/gdb.python/py-xmethods.exp b/gdb/testsuite/gdb.python/py-xmethods.exp
index 97d560476fc..0174b179e36 100644
--- a/gdb/testsuite/gdb.python/py-xmethods.exp
+++ b/gdb/testsuite/gdb.python/py-xmethods.exp
@@ -160,3 +160,20 @@ gdb_test_no_output "enable xmethod progspace E_methods;method_int"
 gdb_test "pt e.method('a')" "type = void"
 gdb_test "pt e.method(10)" \
     "NotImplementedError.*Error while fetching result type of an xmethod worker defined in Python."
+
+# Verify `max-value-size' obedience with `gdb.Value.const_value'.
+gdb_test "p a1.getarray()" \
+  "From Python <A_getarray>.* = \\{0, 1, 2, 3, 4, 5, 6, 7, 8, 9\\}" \
+  "after: a1.getarray"
+gdb_test "p b1.getarray()" \
+  "From Python <B_getarray>.* = \\{0, 1, 2, 3, 4, 5, 6, 7, 8, 9\\}" \
+  "after: b1.getarray"
+gdb_test_no_output "python gdb.set_parameter('max-value-size', 16)"
+gdb_test "p a1.getarray()" \
+  "From Python <A_getarray>.*value requires $decimal bytes,\
+   which is more than max-value-size" \
+  "after: a1.getarray (max-value-size)"
+gdb_test "p b1.getarray()" \
+  "From Python <B_getarray>.*value requires $decimal bytes,\
+   which is more than max-value-size" \
+  "after: b1.getarray (max-value-size)"
diff --git a/gdb/testsuite/gdb.python/py-xmethods.py b/gdb/testsuite/gdb.python/py-xmethods.py
index 61d28535e94..20dc397b924 100644
--- a/gdb/testsuite/gdb.python/py-xmethods.py
+++ b/gdb/testsuite/gdb.python/py-xmethods.py
@@ -39,6 +39,11 @@ def A_geta(obj):
     return obj["a"]
 
 
+def A_getarray(obj):
+    print("From Python <A_getarray>:")
+    return obj["array"]
+
+
 def A_getarrayind(obj, index):
     print("From Python <A_getarrayind>:")
     return obj["array"][index]
@@ -48,12 +53,18 @@ def A_indexoper(obj, index):
     return obj["array"][index].reference_value()
 
 
+def B_getarray(obj):
+    print("From Python <B_getarray>:")
+    return obj["array"].const_value()
+
+
 def B_indexoper(obj, index):
     return obj["array"][index].const_value().reference_value()
 
 
 type_A = gdb.parse_and_eval("(dop::A *) 0").type.target()
 type_B = gdb.parse_and_eval("(dop::B *) 0").type.target()
+type_array = gdb.parse_and_eval("(int[10] *) 0").type.target()
 type_int = gdb.parse_and_eval("(int *) 0").type.target()
 
 
@@ -210,12 +221,18 @@ global_dm_list = [
     ),
     SimpleXMethodMatcher(r"plus_plus_A", r"^dop::A$", r"operator\+\+", plus_plus_A),
     SimpleXMethodMatcher(r"A_geta", r"^dop::A$", r"^geta$", A_geta),
+    SimpleXMethodMatcher(
+        r"A_getarray", r"^dop::A$", r"^getarray$", A_getarray, type_array
+    ),
     SimpleXMethodMatcher(
         r"A_getarrayind", r"^dop::A$", r"^getarrayind$", A_getarrayind, type_int
     ),
     SimpleXMethodMatcher(
         r"A_indexoper", r"^dop::A$", r"operator\[\]", A_indexoper, type_int
     ),
+    SimpleXMethodMatcher(
+        r"B_getarray", r"^dop::B$", r"^getarray$", B_getarray, type_array
+    ),
     SimpleXMethodMatcher(
         r"B_indexoper", r"^dop::B$", r"operator\[\]", B_indexoper, type_int
     ),
diff --git a/gdb/value.c b/gdb/value.c
index e3f60e7e989..09e10b9a8b1 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -1034,31 +1034,42 @@ check_type_length_before_alloc (const struct type *type)
     }
 }
 
-/* Allocate the contents of VAL if it has not been allocated yet.  */
+/* Allocate the contents of VAL if it has not been allocated yet.
+   If CHECK_SIZE is true, then apply the usual max-value-size checks.  */
 
 static void
-allocate_value_contents (struct value *val)
+allocate_value_contents (struct value *val, bool check_size)
 {
   if (!val->contents)
     {
-      check_type_length_before_alloc (val->enclosing_type);
+      if (check_size)
+	check_type_length_before_alloc (val->enclosing_type);
       val->contents.reset
 	((gdb_byte *) xzalloc (val->enclosing_type->length ()));
     }
 }
 
-/* Allocate a  value  and its contents for type TYPE.  */
+/* Allocate a value and its contents for type TYPE.  If CHECK_SIZE is true,
+   then apply the usual max-value-size checks.  */
 
-struct value *
-allocate_value (struct type *type)
+static struct value *
+allocate_value (struct type *type, bool check_size)
 {
   struct value *val = allocate_value_lazy (type);
 
-  allocate_value_contents (val);
+  allocate_value_contents (val, check_size);
   val->lazy = 0;
   return val;
 }
 
+/* Allocate a value and its contents for type TYPE.  */
+
+struct value *
+allocate_value (struct type *type)
+{
+  return allocate_value (type, true);
+}
+
 /* Allocate a  value  that has the correct length
    for COUNT repetitions of type TYPE.  */
 
@@ -1169,7 +1180,7 @@ value_contents_raw (struct value *value)
   struct gdbarch *arch = get_value_arch (value);
   int unit_size = gdbarch_addressable_memory_unit_size (arch);
 
-  allocate_value_contents (value);
+  allocate_value_contents (value, true);
 
   ULONGEST length = value_type (value)->length ();
   return gdb::make_array_view
@@ -1179,7 +1190,7 @@ value_contents_raw (struct value *value)
 gdb::array_view<gdb_byte>
 value_contents_all_raw (struct value *value)
 {
-  allocate_value_contents (value);
+  allocate_value_contents (value, true);
 
   ULONGEST length = value_enclosing_type (value)->length ();
   return gdb::make_array_view (value->contents.get (), length);
@@ -1752,9 +1763,8 @@ value_release_to_mark (const struct value *mark)
   return result;
 }
 
-/* Return a copy of the value ARG.
-   It contains the same contents, for same memory address,
-   but it's a different block of storage.  */
+/* Return a copy of the value ARG.  It contains the same contents,
+   for the same memory address, but it's a different block of storage.  */
 
 struct value *
 value_copy (const value *arg)
@@ -1765,7 +1775,7 @@ value_copy (const value *arg)
   if (value_lazy (arg))
     val = allocate_value_lazy (encl_type);
   else
-    val = allocate_value (encl_type);
+    val = allocate_value (encl_type, false);
   val->type = arg->type;
   VALUE_LVAL (val) = arg->lval;
   val->location = arg->location;
@@ -4162,7 +4172,7 @@ void
 value_fetch_lazy (struct value *val)
 {
   gdb_assert (value_lazy (val));
-  allocate_value_contents (val);
+  allocate_value_contents (val, true);
   /* A value is either lazy, or fully fetched.  The
      availability/validity is only established as we try to fetch a
      value.  */