Implement DAP variables, scopes, and evaluate requests

Message ID 20230223200806.2647349-1-tromey@adacore.com
State New
Headers
Series Implement DAP variables, scopes, and evaluate requests |

Commit Message

Tom Tromey Feb. 23, 2023, 8:08 p.m. UTC
  The DAP code already claimed to implement "scopes" and "evaluate", but
this wasn't done completely correctly.  This patch implements these
and also implements the "variables" request.

After this patch, variables and scopes correctly report their
sub-structure.  This also interfaces with the gdb pretty-printer API,
so the output of pretty-printers is available.
---
 gdb/data-directory/Makefile.in     |   1 +
 gdb/python/lib/gdb/dap/evaluate.py |  38 ++++--
 gdb/python/lib/gdb/dap/scopes.py   |  63 +++++++---
 gdb/python/lib/gdb/dap/varref.py   | 178 +++++++++++++++++++++++++++++
 gdb/python/lib/gdb/printing.py     |  66 +++++++++++
 gdb/testsuite/gdb.dap/scopes.c     |  35 ++++++
 gdb/testsuite/gdb.dap/scopes.exp   | 101 ++++++++++++++++
 7 files changed, 460 insertions(+), 22 deletions(-)
 create mode 100644 gdb/python/lib/gdb/dap/varref.py
 create mode 100644 gdb/testsuite/gdb.dap/scopes.c
 create mode 100644 gdb/testsuite/gdb.dap/scopes.exp
  

Comments

Tom Tromey Feb. 23, 2023, 8:21 p.m. UTC | #1
>>>>> "Tom" == Tom Tromey <tromey@adacore.com> writes:

Tom> The DAP code already claimed to implement "scopes" and "evaluate", but
Tom> this wasn't done completely correctly.  This patch implements these
Tom> and also implements the "variables" request.

Tom> diff --git a/gdb/testsuite/gdb.dap/scopes.exp b/gdb/testsuite/gdb.dap/scopes.exp

Tom> +# Test DAP 'bt' through a function without debuginfo.

After I sent it I noticed this copy-paste comment.
I've updated it locally.

Tom
  
Tom Tromey March 14, 2023, 3:07 p.m. UTC | #2
>>>>> "Tom" == Tom Tromey via Gdb-patches <gdb-patches@sourceware.org> writes:

Tom> The DAP code already claimed to implement "scopes" and "evaluate", but
Tom> this wasn't done completely correctly.  This patch implements these
Tom> and also implements the "variables" request.

Tom> After this patch, variables and scopes correctly report their
Tom> sub-structure.  This also interfaces with the gdb pretty-printer API,
Tom> so the output of pretty-printers is available.

I'm going to check this in now.

Tom
  

Patch

diff --git a/gdb/data-directory/Makefile.in b/gdb/data-directory/Makefile.in
index f1139291eed..ff1340c44c0 100644
--- a/gdb/data-directory/Makefile.in
+++ b/gdb/data-directory/Makefile.in
@@ -103,6 +103,7 @@  PYTHON_FILE_LIST = \
 	gdb/dap/startup.py \
 	gdb/dap/state.py \
 	gdb/dap/threads.py \
+	gdb/dap/varref.py \
 	gdb/function/__init__.py \
 	gdb/function/as_string.py \
 	gdb/function/caller_is.py \
diff --git a/gdb/python/lib/gdb/dap/evaluate.py b/gdb/python/lib/gdb/dap/evaluate.py
index f01bf0f33c9..d04ac169a8e 100644
--- a/gdb/python/lib/gdb/dap/evaluate.py
+++ b/gdb/python/lib/gdb/dap/evaluate.py
@@ -14,10 +14,17 @@ 
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 import gdb
+import gdb.printing
 
 from .frames import frame_for_id
 from .server import request
 from .startup import send_gdb_with_response, in_gdb_thread
+from .varref import find_variable, VariableReference
+
+
+class EvaluateResult(VariableReference):
+    def __init__(self, value):
+        super().__init__(None, value, "result")
 
 
 # Helper function to evaluate an expression in a certain frame.
@@ -26,17 +33,32 @@  def _evaluate(expr, frame_id):
     if frame_id is not None:
         frame = frame_for_id(frame_id)
         frame.select()
-    return str(gdb.parse_and_eval(expr))
+    val = gdb.parse_and_eval(expr)
+    ref = EvaluateResult(val)
+    return ref.to_object()
 
 
 # FIXME 'format' & hex
-# FIXME return a structured response using pretty-printers / varobj
 # FIXME supportsVariableType handling
+# FIXME "repl"
 @request("evaluate")
 def eval_request(*, expression, frameId=None, **args):
-    result = send_gdb_with_response(lambda: _evaluate(expression, frameId))
-    return {
-        "result": result,
-        # FIXME
-        "variablesReference": -1,
-    }
+    return send_gdb_with_response(lambda: _evaluate(expression, frameId))
+
+
+@in_gdb_thread
+def _variables(ref, start, count):
+    var = find_variable(ref)
+    children = var.fetch_children(start, count)
+    return [x.to_object() for x in children]
+
+
+@request("variables")
+# Note that we ignore the 'filter' field.  That seems to be
+# specific to javascript.
+# FIXME: implement format
+def variables(*, variablesReference, start=0, count=0, **args):
+    result = send_gdb_with_response(
+        lambda: _variables(variablesReference, start, count)
+    )
+    return {"variables": result}
diff --git a/gdb/python/lib/gdb/dap/scopes.py b/gdb/python/lib/gdb/dap/scopes.py
index 0c887db91ad..78bdeeea900 100644
--- a/gdb/python/lib/gdb/dap/scopes.py
+++ b/gdb/python/lib/gdb/dap/scopes.py
@@ -1,4 +1,4 @@ 
-# Copyright 2022 Free Software Foundation, Inc.
+# Copyright 2022, 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
@@ -18,6 +18,7 @@  import gdb
 from .frames import frame_for_id
 from .startup import send_gdb_with_response, in_gdb_thread
 from .server import request
+from .varref import BaseReference
 
 
 # Helper function to return a frame's block without error.
@@ -29,17 +30,53 @@  def _safe_block(frame):
         return None
 
 
-# Helper function to return a list of variables of block, up to the
-# enclosing function.
+# Helper function to return two lists of variables of block, up to the
+# enclosing function.  The result is of the form (ARGS, LOCALS), where
+# each element is itself a list.
 @in_gdb_thread
 def _block_vars(block):
-    result = []
+    args = []
+    locs = []
     while True:
-        result += list(block)
+        for var in block:
+            if var.is_argument:
+                args.append(var)
+            else:
+                locs.append(var)
         if block.function is not None:
             break
         block = block.superblock
-    return result
+    return (args, locs)
+
+
+class ScopeReference(BaseReference):
+    def __init__(self, name, frame, var_list):
+        super().__init__(name)
+        self.frame = frame
+        self.func = frame.function()
+        self.var_list = var_list
+
+    def to_object(self):
+        result = super().to_object()
+        # How would we know?
+        result["expensive"] = False
+        result["namedVariables"] = len(self.var_list)
+        if self.func is not None:
+            result["line"] = self.func.line
+            # FIXME construct a Source object
+        return result
+
+    def child_count(self):
+        return len(self.var_list)
+
+    @in_gdb_thread
+    def fetch_one_child(self, idx):
+        sym = self.var_list[idx]
+        if sym.needs_frame:
+            val = sym.value(self.frame)
+        else:
+            val = sym.value()
+        return (sym.print_name, val)
 
 
 # Helper function to create a DAP scopes for a given frame ID.
@@ -49,14 +86,12 @@  def _get_scope(id):
     block = _safe_block(frame)
     scopes = []
     if block is not None:
-        new_scope = {
-            # FIXME
-            "name": "Locals",
-            "expensive": False,
-            "namedVariables": len(_block_vars(block)),
-        }
-        scopes.append(new_scope)
-    return scopes
+        (args, locs) = _block_vars(block)
+        if args:
+            scopes.append(ScopeReference("Arguments", frame, args))
+        if locs:
+            scopes.append(ScopeReference("Locals", frame, locs))
+    return [x.to_object() for x in scopes]
 
 
 @request("scopes")
diff --git a/gdb/python/lib/gdb/dap/varref.py b/gdb/python/lib/gdb/dap/varref.py
new file mode 100644
index 00000000000..888415a322d
--- /dev/null
+++ b/gdb/python/lib/gdb/dap/varref.py
@@ -0,0 +1,178 @@ 
+# Copyright 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/>.
+
+import gdb
+from .startup import in_gdb_thread
+from abc import abstractmethod
+
+
+# A list of all the variable references created during this pause.
+all_variables = []
+
+
+# When the inferior is re-started, we erase all variable references.
+# See the section "Lifetime of Objects References" in the spec.
+@in_gdb_thread
+def clear_vars(event):
+    global all_variables
+    all_variables = []
+
+
+gdb.events.cont.connect(clear_vars)
+
+
+class BaseReference:
+    """Represent a variable or a scope.
+
+    This class is just a base class, some methods must be implemented in
+    subclasses.
+
+    The 'ref' field can be used as the variablesReference in the protocol.
+    """
+
+    @in_gdb_thread
+    def __init__(self, name):
+        """Create a new variable reference with the given name.
+
+        NAME is a string or None.  None means this does not have a
+        name, e.g., the result of expression evaluation."""
+
+        global all_variables
+        all_variables.append(self)
+        self.ref = len(all_variables)
+        self.name = name
+        self.children = None
+
+    @in_gdb_thread
+    def to_object(self):
+        """Return a dictionary that describes this object for DAP.
+
+        The resulting object is a starting point that can be filled in
+        further.  See the Scope or Variable types in the spec"""
+        result = {
+            "variablesReference": self.ref,
+        }
+        if self.name is not None:
+            result["name"] = self.name
+        return result
+
+    def no_children(self):
+        """Call this to declare that this variable or scope has no children."""
+        self.ref = 0
+
+    @abstractmethod
+    def fetch_one_child(self, index):
+        """Fetch one child of this variable.
+
+        INDEX is the index of the child to fetch.
+        This should return a tuple of the form (NAME, VALUE), where
+        NAME is the name of the variable, and VALUE is a gdb.Value."""
+        return
+
+    @abstractmethod
+    def child_count(self):
+        """Return the number of children of this variable."""
+        return
+
+    @in_gdb_thread
+    def fetch_children(self, start, count):
+        """Fetch children of this variable.
+
+        START is the starting index.
+        COUNT is the number to return, with 0 meaning return all."""
+        if count == 0:
+            count = self.child_count()
+        if self.children is None:
+            self.children = [None] * self.child_count()
+        result = []
+        for idx in range(start, start + count):
+            if self.children[idx] is None:
+                (name, value) = self.fetch_one_child(idx)
+                self.children[idx] = VariableReference(name, value)
+            result.append(self.children[idx])
+        return result
+
+
+class VariableReference(BaseReference):
+    """Concrete subclass of BaseReference that handles gdb.Value."""
+
+    def __init__(self, name, value, result_name="value"):
+        """Initializer.
+
+        NAME is the name of this reference, see superclass.
+        VALUE is a gdb.Value that holds the value.
+        RESULT_NAME can be used to change how the simple string result
+        is emitted in the result dictionary."""
+        super().__init__(name)
+        self.printer = gdb.printing.make_visualizer(value)
+        self.result_name = result_name
+        # We cache all the children we create.
+        self.child_cache = None
+        if not hasattr(self.printer, "children"):
+            self.no_children()
+            self.count = None
+        else:
+            self.count = -1
+
+    def cache_children(self):
+        if self.child_cache is None:
+            # This discards all laziness.  This could be improved
+            # slightly by lazily evaluating children, but because this
+            # code also generally needs to know the number of
+            # children, it probably wouldn't help much.  A real fix
+            # would require an update to gdb's pretty-printer protocol
+            # (though of course that is probably also inadvisable).
+            self.child_cache = list(self.printer.children())
+        return self.child_cache
+
+    def child_count(self):
+        if self.count is None:
+            return None
+        if self.count == -1:
+            if hasattr(self.printer, "num_children"):
+                num_children = self.printer.num_children
+            else:
+                num_children = len(self.cache_children())
+            self.count = num_children
+        return self.count
+
+    def to_object(self):
+        result = super().to_object()
+        result[self.result_name] = self.printer.to_string()
+        num_children = self.child_count()
+        if num_children is not None:
+            if (
+                hasattr(self.printer, "display_hint")
+                and self.printer.display_hint() == "array"
+            ):
+                result["indexedVariables"] = num_children
+            else:
+                result["namedVariables"] = num_children
+        return result
+
+    @in_gdb_thread
+    def fetch_one_child(self, idx):
+        return self.cache_children()[idx]
+
+
+@in_gdb_thread
+def find_variable(ref):
+    """Given a variable reference, return the corresponding variable object."""
+    global all_variables
+    # Variable references are offset by 1.
+    ref = ref - 1
+    if ref < 0 or ref > len(all_variables):
+        raise Exception("invalid variablesReference")
+    return all_variables[ref]
diff --git a/gdb/python/lib/gdb/printing.py b/gdb/python/lib/gdb/printing.py
index 740a96e60eb..6403ef1a6c1 100644
--- a/gdb/python/lib/gdb/printing.py
+++ b/gdb/python/lib/gdb/printing.py
@@ -269,6 +269,72 @@  class FlagEnumerationPrinter(PrettyPrinter):
             return None
 
 
+class NoOpScalarPrinter:
+    """A no-op pretty printer that wraps a scalar value."""
+    def __init__(self, value):
+        self.value = value
+
+    def to_string(self):
+        return self.value.format_string(raw=True)
+
+
+class NoOpArrayPrinter:
+    """A no-op pretty printer that wraps an array value."""
+    def __init__(self, value):
+        self.value = value
+        (low, high) = self.value.type.range()
+        self.low = low
+        self.high = high
+        # This is a convenience to the DAP code and perhaps other
+        # users.
+        self.num_children = high - low + 1
+
+    def to_string(self):
+        return ""
+
+    def display_hint(self):
+        return "array"
+
+    def children(self):
+        for i in range(self.low, self.high):
+            yield (i, self.value[i])
+
+
+class NoOpStructPrinter:
+    """A no-op pretty printer that wraps a struct or union value."""
+    def __init__(self, value):
+        self.value = value
+
+    def to_string(self):
+        return ""
+
+    def children(self):
+        for field in self.value.type.fields():
+            if field.name is not None:
+                yield (field.name, self.value[field])
+
+
+def make_visualizer(value):
+    """Given a gdb.Value, wrap it in a pretty-printer.
+
+    If a pretty-printer is found by the usual means, it is returned.
+    Otherwise, VALUE will be wrapped in a no-op visualizer."""
+
+    result = gdb.default_visualizer(value)
+    if result is not None:
+        # Found a pretty-printer.
+        pass
+    elif value.type.code == gdb.TYPE_CODE_ARRAY:
+        result = gdb.printing.NoOpArrayPrinter(value)
+        (low, high) = value.type.range()
+        result.n_children = high - low + 1
+    elif value.type.code in (gdb.TYPE_CODE_STRUCT, gdb.TYPE_CODE_UNION):
+        result = gdb.printing.NoOpStructPrinter(value)
+    else:
+        result = gdb.printing.NoOpScalarPrinter(value)
+    return result
+
+
 # Builtin pretty-printers.
 # The set is defined as empty, and files in printing/*.py add their printers
 # to this with add_builtin_pretty_printer.
diff --git a/gdb/testsuite/gdb.dap/scopes.c b/gdb/testsuite/gdb.dap/scopes.c
new file mode 100644
index 00000000000..7b35036cce1
--- /dev/null
+++ b/gdb/testsuite/gdb.dap/scopes.c
@@ -0,0 +1,35 @@ 
+/* Copyright 2023 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/>.  */
+
+struct dei_struct
+{
+  int x;
+  int more[5];
+};
+
+int main ()
+{
+  struct dei_struct dei = { 2, { 3, 5, 7, 11, 13 } };
+
+  static int scalar = 23;
+
+  {
+    const char *inner = "inner block";
+
+    return 0;			/* BREAK */
+  }
+}
diff --git a/gdb/testsuite/gdb.dap/scopes.exp b/gdb/testsuite/gdb.dap/scopes.exp
new file mode 100644
index 00000000000..257b300a30e
--- /dev/null
+++ b/gdb/testsuite/gdb.dap/scopes.exp
@@ -0,0 +1,101 @@ 
+# Copyright 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 DAP 'bt' through a function without debuginfo.
+
+require allow_python_tests
+
+load_lib dap-support.exp
+
+standard_testfile
+
+if {[build_executable ${testfile}.exp $testfile] == -1} {
+    return
+}
+
+if {[dap_launch $testfile] == ""} {
+    return
+}
+
+set line [gdb_get_line_number "BREAK"]
+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 "start inferior" configurationDone
+dap_wait_for_event_and_check "inferior started" thread "body reason" started
+
+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]
+
+gdb_assert {[llength $scopes] == 1} "single scope"
+
+set scope [lindex $scopes 0]
+gdb_assert {[dict get $scope name] == "Locals"} "scope is locals"
+gdb_assert {[dict get $scope namedVariables] == 3} "three vars in scope"
+
+set num [dict get $scope variablesReference]
+set refs [lindex [dap_check_request_and_response "fetch variables" \
+		      "variables" \
+		      [format {o variablesReference [i %d] count [i 3]} \
+			   $num]] \
+	      0]
+
+foreach var [dict get $refs body variables] {
+    set name [dict get $var name]
+
+    if {$name != "dei"} {
+	gdb_assert {[dict get $var variablesReference] == 0} \
+	    "$name has no structure"
+    }
+
+    switch $name {
+	"inner" {
+	    gdb_assert {[string match "*inner block*" [dict get $var value]]} \
+		"check value of inner"
+	}
+	"dei" {
+	    gdb_assert {[dict get $var value] == ""} "check value of dei"
+	    set dei_ref [dict get $var variablesReference]
+	}
+	"scalar" {
+	    gdb_assert {[dict get $var value] == 23} "check value of scalar"
+	}
+	default {
+	    fail "unknown variable $name"
+	}
+    }
+}
+
+set refs [lindex [dap_check_request_and_response "fetch contents of dei" \
+		      "variables" \
+		      [format {o variablesReference [i %d]} $dei_ref]] \
+	      0]
+set deivals [dict get $refs body variables]
+gdb_assert {[llength $deivals] == 2} "dei has two members"
+
+dap_shutdown