From 87e3cc466e8ea352639beb6db40a36e339d608d1 Mon Sep 17 00:00:00 2001 From: Tom Tromey Date: Thu, 5 Oct 2023 06:27:50 -0600 Subject: [PATCH] Implement DAP setVariable request This patch implements the DAP setVariable request. setVariable is a bit odd in that it specifies the variable to modify by passing in the variable's container and the name of the variable. This approach can't handle variable shadowing (there are a couple of open DAP bugs on this topic), so this patch renames duplicates to avoid the problem. --- gdb/python/lib/gdb/dap/evaluate.py | 21 +++++ gdb/python/lib/gdb/dap/scopes.py | 3 + gdb/python/lib/gdb/dap/varref.py | 97 ++++++++++++++++++----- gdb/testsuite/gdb.dap/assign.c | 35 +++++++++ gdb/testsuite/gdb.dap/assign.exp | 122 +++++++++++++++++++++++++++++ gdb/testsuite/gdb.dap/assign.py | 54 +++++++++++++ 6 files changed, 311 insertions(+), 21 deletions(-) create mode 100644 gdb/testsuite/gdb.dap/assign.c create mode 100644 gdb/testsuite/gdb.dap/assign.exp create mode 100644 gdb/testsuite/gdb.dap/assign.py diff --git a/gdb/python/lib/gdb/dap/evaluate.py b/gdb/python/lib/gdb/dap/evaluate.py index 405e6fc2410..ea5a1e61a08 100644 --- a/gdb/python/lib/gdb/dap/evaluate.py +++ b/gdb/python/lib/gdb/dap/evaluate.py @@ -141,3 +141,24 @@ def set_expression( return send_gdb_with_response( lambda: _set_expression(expression, value, frameId, format) ) + + +# Helper function to perform an assignment. +@in_gdb_thread +def _set_variable(ref, name, value, value_format): + with apply_format(value_format): + var = find_variable(ref) + lhs = var.find_child_by_name(name) + rhs = gdb.parse_and_eval(value) + lhs.assign(rhs) + return lhs.to_object() + + +@capability("supportsSetVariable") +@request("setVariable") +def set_variable( + *, variablesReference: int, name: str, value: str, format=None, **args +): + return send_gdb_with_response( + lambda: _set_variable(variablesReference, name, value, format) + ) diff --git a/gdb/python/lib/gdb/dap/scopes.py b/gdb/python/lib/gdb/dap/scopes.py index 5dde060f44f..87f2ed7547f 100644 --- a/gdb/python/lib/gdb/dap/scopes.py +++ b/gdb/python/lib/gdb/dap/scopes.py @@ -59,6 +59,9 @@ class _ScopeReference(BaseReference): # FIXME construct a Source object return result + def has_children(self): + return True + def child_count(self): return len(self.var_list) diff --git a/gdb/python/lib/gdb/dap/varref.py b/gdb/python/lib/gdb/dap/varref.py index 5dfe6e0edb9..8f0a070498c 100644 --- a/gdb/python/lib/gdb/dap/varref.py +++ b/gdb/python/lib/gdb/dap/varref.py @@ -16,7 +16,8 @@ import gdb from .startup import in_gdb_thread from .server import client_bool_capability -from abc import abstractmethod +from abc import ABC, abstractmethod +from collections import defaultdict from contextlib import contextmanager @@ -52,7 +53,7 @@ def apply_format(value_format): return _null() -class BaseReference: +class BaseReference(ABC): """Represent a variable or a scope. This class is just a base class, some methods must be implemented in @@ -72,7 +73,7 @@ class BaseReference: all_variables.append(self) self.ref = len(all_variables) self.name = name - self.children = None + self.reset_children() @in_gdb_thread def to_object(self): @@ -80,16 +81,27 @@ class BaseReference: 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, - } + result = {"variablesReference": self.ref if self.has_children() else 0} if self.name is not None: result["name"] = str(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 has_children(self): + """Return True if this object has children.""" + return False + + def reset_children(self): + """Reset any cached information about the children of this object.""" + # A list of all the children. Each child is a BaseReference + # of some kind. + self.children = None + # Map from the name of a child to a BaseReference. + self.by_name = {} + # Keep track of how many duplicates there are of a given name, + # so that unique names can be generated. Map from base name + # to a count. + self.name_counts = defaultdict(lambda: 1) @abstractmethod def fetch_one_child(self, index): @@ -105,23 +117,55 @@ class BaseReference: """Return the number of children of this variable.""" return + # Helper method to compute the final name for a child whose base + # name is given. Updates the name_counts map. This is used to + # handle shadowing -- in DAP, the adapter is responsible for + # making sure that all the variables in a a given container have + # unique names. See + # https://github.com/microsoft/debug-adapter-protocol/issues/141 + # and + # https://github.com/microsoft/debug-adapter-protocol/issues/149 + def _compute_name(self, name): + if name in self.by_name: + self.name_counts[name] += 1 + # In theory there's no safe way to compute a name, because + # a pretty-printer might already be generating names of + # that form. In practice I think we should not worry too + # much. + name = name + " #" + str(self.name_counts[name]) + return name + @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.""" + COUNT is the number to return, with 0 meaning return all. + Returns an iterable of some kind.""" 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 + name = self._compute_name(name) + var = VariableReference(name, value) + self.children[idx] = var + self.by_name[name] = var + yield self.children[idx] + + @in_gdb_thread + def find_child_by_name(self, name): + """Find a child of this variable, given its name. + + Returns the value of the child, or throws if not found.""" + # A lookup by name can only be done using names previously + # provided to the client, so we can simply rely on the by-name + # map here. + if name in self.by_name: + return self.by_name[name] + raise Exception("no variable named '" + name + "'") class VariableReference(BaseReference): @@ -135,16 +179,27 @@ class VariableReference(BaseReference): RESULT_NAME can be used to change how the simple string result is emitted in the result dictionary.""" super().__init__(name) - self.value = value - self.printer = gdb.printing.make_visualizer(value) self.result_name = result_name - # We cache all the children we create. + self.value = value + self._update_value() + + # Internal method to update local data when the value changes. + def _update_value(self): + self.reset_children() + self.printer = gdb.printing.make_visualizer(self.value) self.child_cache = None - if not hasattr(self.printer, "children"): - self.no_children() - self.count = None - else: + if self.has_children(): self.count = -1 + else: + self.count = None + + def assign(self, value): + """Assign VALUE to this object and update.""" + self.value.assign(value) + self._update_value() + + def has_children(self): + return hasattr(self.printer, "children") def cache_children(self): if self.child_cache is None: diff --git a/gdb/testsuite/gdb.dap/assign.c b/gdb/testsuite/gdb.dap/assign.c new file mode 100644 index 00000000000..c3bc3d3f107 --- /dev/null +++ b/gdb/testsuite/gdb.dap/assign.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 . */ + +struct special_type +{ + /* Discriminator used by the pretty-printer. When zero, no fields + are shown; when non-zero, shows the datum. */ + int disc; + int datum; +}; + +struct special_type empty = { 0, 23 }; +struct special_type full = { 1, 23 }; + +int +main () +{ + struct special_type value = full; + + return 0; /* STOP */ +} diff --git a/gdb/testsuite/gdb.dap/assign.exp b/gdb/testsuite/gdb.dap/assign.exp new file mode 100644 index 00000000000..e1c412eca5f --- /dev/null +++ b/gdb/testsuite/gdb.dap/assign.exp @@ -0,0 +1,122 @@ +# 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 . + +# Test the setVariable request. + +require allow_dap_tests + +load_lib dap-support.exp + +standard_testfile + +if {[build_executable ${testfile}.exp $testfile] == -1} { + return +} + +set remote_python_file [gdb_remote_download host \ + ${srcdir}/${subdir}/${testfile}.py] + +save_vars GDBFLAGS { + append GDBFLAGS " -iex \"source $remote_python_file\"" + + if {[dap_launch $testfile] == ""} { + 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 "start inferior" configurationDone + +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] + +set desc [dict get $refs body variables] +gdb_assert {[llength $desc] == 1} "only one variable fetched" +set desc [lindex $desc 0] + +set saved_ref [dict get $desc variablesReference] + +proc check_vars {prefix var varref summary} { + with_test_prefix $prefix { + gdb_assert {[dict get $var name] == "value"} "variable name" + gdb_assert {[dict get $var variablesReference] == $varref} \ + "variable reference" + gdb_assert {[dict get $var value] == $summary} \ + "variable value" + } +} + +check_vars initial $desc $saved_ref full + +set refs [lindex [dap_check_request_and_response "assign empty to variable" \ + "setVariable" \ + [format {o variablesReference [i %d] name [s value] \ + value [s empty]} \ + $num]] \ + 0] +check_vars "assign empty" [dict get $refs body] 0 empty + +set refs [lindex [dap_check_request_and_response "assign full to variable" \ + "setVariable" \ + [format {o variablesReference [i %d] name [s value] \ + value [s full]} \ + $num]] \ + 0] +check_vars "assign full" [dict get $refs body] $saved_ref full + +# Now fetch the children of the variable, to check that the shadowing +# workaround works. +gdb_assert {[dict get $refs body namedVariables] == 2} \ + "two children of variable" + +set num [dict get $refs body variablesReference] +set refs [lindex [dap_check_request_and_response \ + "fetch children of variable" "variables" \ + [format {o variablesReference [i %d] count [i 2]} \ + $num]] \ + 0] + +lassign [dict get $refs body variables] one two +gdb_assert {[dict get $one name] != [dict get $two name]} \ + "children have different names" + +dap_shutdown diff --git a/gdb/testsuite/gdb.dap/assign.py b/gdb/testsuite/gdb.dap/assign.py new file mode 100644 index 00000000000..d7b4868455d --- /dev/null +++ b/gdb/testsuite/gdb.dap/assign.py @@ -0,0 +1,54 @@ +# Copyright (C) 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 +# 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 . + + +import gdb + + +class EmptyPrinter(gdb.ValuePrinter): + """Pretty print a structure""" + + def __init__(self, val): + self._val = val + + def to_string(self): + return "empty" + + +class FullPrinter(gdb.ValuePrinter): + """Pretty print a structure""" + + def __init__(self, val): + self._val = val + + def to_string(self): + return "full" + + def children(self): + # This is used to test the renaming code. + yield "datum", self._val["datum"] + yield "datum", self._val["datum"] + + +def lookup_function(val): + if val.type.tag == "special_type": + if val["disc"] == 0: + return EmptyPrinter(val) + else: + return FullPrinter(val) + return None + + +gdb.pretty_printers.append(lookup_function) -- 2.30.2