[4/4] Introduce NoOpStringPrinter

Message ID 20241120-noop-string-printer-v1-4-40c44526fb4a@adacore.com
State New
Headers
Series Fix string display in DAP |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Test passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Test passed

Commit Message

Tom Tromey Nov. 20, 2024, 4:31 p.m. UTC
  We discovered that attempting to print a very large string-like array
would succeed on the CLI, but in DAP would cause the "variables"
request to fail with:

  value requires 67038491 bytes, which is more than max-value-size

This turns out to be a limitation in Value.format_string, which
de-lazy-ifies the value.

This patch fixes this problem by introducing a new NoOpStringPrinter
class, and then using it for string-like values.  This printer returns
a lazy string, which solves the problem.

Note there are some special cases where we do not want to return a
lazy string.  I've documented these in the code.  I considered making
gdb.Value.lazy_string handle these cases -- for example it could
return 'self' rather than a lazy string in some situations -- but this
approach was simpler.
---
 gdb/python/lib/gdb/printing.py     | 40 ++++++++++++++++++-
 gdb/testsuite/gdb.dap/max-size.c   | 25 ++++++++++++
 gdb/testsuite/gdb.dap/max-size.exp | 82 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 146 insertions(+), 1 deletion(-)
  

Patch

diff --git a/gdb/python/lib/gdb/printing.py b/gdb/python/lib/gdb/printing.py
index 55ba43585eca13809385e559ad27ee441d653498..0635993cac3cfc484744e1816c644c48531d364e 100644
--- a/gdb/python/lib/gdb/printing.py
+++ b/gdb/python/lib/gdb/printing.py
@@ -281,6 +281,44 @@  class NoOpScalarPrinter(gdb.ValuePrinter):
         return self.__value.format_string(raw=True)
 
 
+class NoOpStringPrinter(gdb.ValuePrinter):
+    """A no-op pretty printer that wraps a string value."""
+
+    def __init__(self, ty, value):
+        self.__ty = ty
+        self.__value = value
+
+    def to_string(self):
+        # We need some special cases here.
+        #
+        # * If the gdb.Value was created from a Python string, it will
+        #   be a non-lazy array -- but will have address 0 and so the
+        #   contents will be lost on conversion to lazy string.
+        #   (Weirdly, the .address attribute will not be 0 though.)
+        #   Since conversion to lazy string is to avoid fetching too
+        #   much data, and since the array is already non-lazy, just
+        #   return it.
+        #
+        # * To avoid weird printing for a C "string" that is just a
+        #   NULL pointer, special case this as well.
+        #
+        # * Lazy strings only understand arrays and pointers; other
+        #   string-like objects (like a Rust &str) should simply be
+        #   returned.
+        code = self.__ty.code
+        if code == gdb.TYPE_CODE_ARRAY and not self.__value.is_lazy:
+            return self.__value
+        elif code == gdb.TYPE_CODE_PTR and self.__value == 0:
+            return self.__value
+        elif code != gdb.TYPE_CODE_PTR and code != gdb.TYPE_CODE_ARRAY:
+            return self.__value
+        else:
+            return self.__value.lazy_string()
+
+    def display_hint(self):
+        return "string"
+
+
 class NoOpPointerReferencePrinter(gdb.ValuePrinter):
     """A no-op pretty printer that wraps a pointer or reference."""
 
@@ -368,7 +406,7 @@  def make_visualizer(value):
     else:
         ty = value.type.strip_typedefs()
         if ty.is_string_like:
-            result = NoOpScalarPrinter(value)
+            result = NoOpStringPrinter(ty, value)
         elif ty.code == gdb.TYPE_CODE_ARRAY:
             result = NoOpArrayPrinter(ty, value)
         elif ty.is_array_like:
diff --git a/gdb/testsuite/gdb.dap/max-size.c b/gdb/testsuite/gdb.dap/max-size.c
new file mode 100644
index 0000000000000000000000000000000000000000..6bc2e3698f70f2d374809ee10224f826d88e4802
--- /dev/null
+++ b/gdb/testsuite/gdb.dap/max-size.c
@@ -0,0 +1,25 @@ 
+/* Copyright 2024 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   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/>.  */
+
+int
+main ()
+{
+  /* Note that this is an array to ensure that is_string_like returns
+     True for the type.  */
+  const char the_string[] = "this is a string longer than 16 bytes";
+  return 0;			/* STOP */
+}
diff --git a/gdb/testsuite/gdb.dap/max-size.exp b/gdb/testsuite/gdb.dap/max-size.exp
new file mode 100644
index 0000000000000000000000000000000000000000..3bcbfdb6bc586bef7c63ef15cd435486d7cce58c
--- /dev/null
+++ b/gdb/testsuite/gdb.dap/max-size.exp
@@ -0,0 +1,82 @@ 
+# Copyright 2024 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 printing of string-like objects larger than the maximum value
+# size.
+
+require allow_dap_tests
+
+load_lib dap-support.exp
+
+standard_testfile
+
+if {[build_executable ${testfile}.exp $testfile] == -1} {
+    return
+}
+
+save_vars GDBFLAGS {
+    append GDBFLAGS " -iex \"set max-value-size 16\""
+
+    if {[dap_initialize] == ""} {
+	return
+    }
+}
+
+set line [gdb_get_line_number "STOP"]
+set obj [dap_check_request_and_response "set breakpoint by line number" \
+	     setBreakpoints \
+	     [format {o source [o path [%s]] breakpoints [a [o line [i %d]]]} \
+		  [list s $srcfile] $line]]
+set line_bpno [dap_get_breakpoint_number $obj]
+
+dap_check_request_and_response "configurationDone" configurationDone
+
+if {[dap_launch $testfile] == ""} {
+    return
+}
+dap_wait_for_event_and_check "stopped at line breakpoint" stopped \
+    "body reason" breakpoint \
+    "body hitBreakpointIds" $line_bpno
+
+set bt [lindex [dap_check_request_and_response "backtrace" stackTrace \
+		    {o threadId [i 1]}] \
+	    0]
+set frame_id [dict get [lindex [dict get $bt body stackFrames] 0] id]
+
+set scopes [dap_check_request_and_response "get scopes" scopes \
+		[format {o frameId [i %d]} $frame_id]]
+set scopes [dict get [lindex $scopes 0] body scopes]
+
+lassign $scopes scope reg_scope
+gdb_assert {[dict get $scope name] == "Locals"} "scope is locals"
+gdb_assert {[dict get $scope namedVariables] == 1} "one var in scope"
+
+set num [dict get $scope variablesReference]
+set refs [lindex [dap_check_request_and_response "fetch variable" \
+		      "variables" \
+		      [format {o variablesReference [i %d] count [i 1]} \
+			   $num]] \
+	      0]
+
+foreach var [dict get $refs body variables] {
+    gdb_assert {[dict get $var name] == "the_string"} "variable name"
+    # The result looks strange here, but only because TON does not
+    # handle the backslash-quote sequence properly when decoding the
+    # JSON.  The actual JSON is: "value": "\"DEI\"".
+    gdb_assert {[dict get $var value] == "\\\"this is a string longer than 16 bytes\\\""} \
+	"variable value"
+}
+
+dap_shutdown