* [PATCH 1/2] Export "finish" return value to Python
2024-02-12 18:47 [PATCH 0/2] Add "finish" return value to DAP Tom Tromey
@ 2024-02-12 18:47 ` Tom Tromey
2024-02-12 19:24 ` Eli Zaretskii
2024-02-12 18:47 ` [PATCH 2/2] Add return value to DAP scope Tom Tromey
1 sibling, 1 reply; 7+ messages in thread
From: Tom Tromey @ 2024-02-12 18:47 UTC (permalink / raw)
To: gdb-patches
This patch changes the Python "stop" event emission code to also add
the function return value, if it is known. This happens when the stop
comes from a "finish" command and when the value can be fetched.
The test is in the next patch.
---
gdb/doc/python.texi | 6 ++++++
gdb/infcmd.c | 17 -----------------
gdb/python/py-stopevent.c | 23 ++++++++++++++++++++++-
gdb/thread-fsm.h | 19 +++++++++++++++++++
4 files changed, 47 insertions(+), 18 deletions(-)
diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index ece9038844c..21a84fabb34 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -3813,6 +3813,12 @@ corresponding MI output (@pxref{GDB/MI Async Records}).
A dictionary was used for this (rather than adding attributes directly
to the event object) so that the MI keys could be used unchanged.
+
+When a @code{StopEvent} results from a @code{finish} command, it will
+also hold the return value from the function, if that is available.
+This will be an entry named @samp{return-value} in the @code{details}
+dictionary. The value of this entry will be a @code{gdb.Value}
+object.
@end defvar
Emits @code{gdb.SignalEvent}, which extends @code{gdb.StopEvent}.
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 4e17a6b0629..68f64f0d2b3 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -1484,23 +1484,6 @@ get_return_value (struct symbol *func_symbol, struct value *function)
return value;
}
-/* The captured function return value/type and its position in the
- value history. */
-
-struct return_value_info
-{
- /* The captured return value. May be NULL if we weren't able to
- retrieve it. See get_return_value. */
- struct value *value;
-
- /* The return type. In some cases, we'll not be able extract the
- return value, but we always know the type. */
- struct type *type;
-
- /* If we captured a value, this is the value history index. */
- int value_history_index;
-};
-
/* Helper for print_return_value. */
static void
diff --git a/gdb/python/py-stopevent.c b/gdb/python/py-stopevent.c
index 61d93727b31..fcaebe26f13 100644
--- a/gdb/python/py-stopevent.c
+++ b/gdb/python/py-stopevent.c
@@ -20,6 +20,7 @@
#include "defs.h"
#include "py-stopevent.h"
#include "py-uiout.h"
+#include "thread-fsm.h"
gdbpy_ref<>
create_stop_event_object (PyTypeObject *py_type, const gdbpy_ref<> &dict)
@@ -45,6 +46,7 @@ static gdbpy_ref<>
py_print_bpstat (bpstat *bs, enum gdb_signal stop_signal)
{
py_ui_out uiout;
+ struct value *return_value = nullptr;
try
{
@@ -55,6 +57,10 @@ py_print_bpstat (bpstat *bs, enum gdb_signal stop_signal)
{
async_reply_reason reason = tp->thread_fsm ()->async_reply_reason ();
uiout.field_string ("reason", async_reason_lookup (reason));
+
+ return_value_info *rvinfo = tp->thread_fsm ()->return_value ();
+ if (rvinfo != nullptr && rvinfo->value != nullptr)
+ return_value = rvinfo->value;
}
if (stop_signal != GDB_SIGNAL_0 && stop_signal != GDB_SIGNAL_TRAP)
@@ -73,7 +79,22 @@ py_print_bpstat (bpstat *bs, enum gdb_signal stop_signal)
return nullptr;
}
- return uiout.result ();
+ gdbpy_ref<> dict = uiout.result ();
+ if (dict == nullptr)
+ return nullptr;
+
+ /* This has to be done separately to avoid error issues, and because
+ there's no API to add generic Python objects to a py_ui_out. */
+ if (return_value != nullptr)
+ {
+ gdbpy_ref<> val (value_to_value_object (return_value));
+ if (val == nullptr)
+ return nullptr;
+ if (PyDict_SetItemString (dict.get (), "finish-value", val.get ()) < 0)
+ return nullptr;
+ }
+
+ return dict;
}
/* Callback observers when a stop event occurs. This function will create a
diff --git a/gdb/thread-fsm.h b/gdb/thread-fsm.h
index 90abb5c27f6..ed117719c0d 100644
--- a/gdb/thread-fsm.h
+++ b/gdb/thread-fsm.h
@@ -23,6 +23,25 @@
struct return_value_info;
struct thread_fsm_ops;
+struct type;
+struct value;
+
+/* The captured function return value/type and its position in the
+ value history. */
+
+struct return_value_info
+{
+ /* The captured return value. May be NULL if we weren't able to
+ retrieve it. See get_return_value. */
+ struct value *value;
+
+ /* The return type. In some cases, we'll not be able extract the
+ return value, but we always know the type. */
+ struct type *type;
+
+ /* If we captured a value, this is the value history index. */
+ int value_history_index;
+};
/* A thread finite-state machine structure contains the necessary info
and callbacks to manage the state machine protocol of a thread's
--
2.43.0
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 2/2] Add return value to DAP scope
2024-02-12 18:47 [PATCH 0/2] Add "finish" return value to DAP Tom Tromey
2024-02-12 18:47 ` [PATCH 1/2] Export "finish" return value to Python Tom Tromey
@ 2024-02-12 18:47 ` Tom Tromey
2024-02-13 15:01 ` Kévin Le Gouguec
1 sibling, 1 reply; 7+ messages in thread
From: Tom Tromey @ 2024-02-12 18:47 UTC (permalink / raw)
To: gdb-patches
A bug report in the DAP specification repository pointed out that it
is typical for DAP implementations to put a function's return value
into the outermost scope.
This patch changes gdb to follow this convention.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31341
---
gdb/python/lib/gdb/dap/events.py | 3 ++
gdb/python/lib/gdb/dap/scopes.py | 45 ++++++++++++++++++++-
gdb/testsuite/gdb.dap/step-out.c | 36 +++++++++++++++++
gdb/testsuite/gdb.dap/step-out.exp | 82 ++++++++++++++++++++++++++++++++++++++
4 files changed, 164 insertions(+), 2 deletions(-)
diff --git a/gdb/python/lib/gdb/dap/events.py b/gdb/python/lib/gdb/dap/events.py
index 41302229ee5..928f23fd3ff 100644
--- a/gdb/python/lib/gdb/dap/events.py
+++ b/gdb/python/lib/gdb/dap/events.py
@@ -15,6 +15,7 @@
import gdb
+from .scopes import set_finish_value
from .server import send_event
from .startup import exec_and_log, in_gdb_thread, log
from .modules import is_module, make_module
@@ -218,6 +219,8 @@ def _on_stop(event):
}
if isinstance(event, gdb.BreakpointEvent):
obj["hitBreakpointIds"] = [x.number for x in event.breakpoints]
+ if hasattr(event, "details") and "finish-value" in event.details:
+ set_finish_value(event.details["finish-value"])
global _expected_pause
global _expected_stop_reason
diff --git a/gdb/python/lib/gdb/dap/scopes.py b/gdb/python/lib/gdb/dap/scopes.py
index ff553259ea9..f3bbe4fc614 100644
--- a/gdb/python/lib/gdb/dap/scopes.py
+++ b/gdb/python/lib/gdb/dap/scopes.py
@@ -25,17 +25,32 @@ from .varref import BaseReference
frame_to_scope = {}
+# If the most recent stop was due to a 'finish', and the return value
+# could be computed, then this holds that value. Otherwise it holds
+# None.
+_last_return_value = None
+
+
# When the inferior is re-started, we erase all scope references. See
# the section "Lifetime of Objects References" in the spec.
@in_gdb_thread
def clear_scopes(event):
global frame_to_scope
frame_to_scope = {}
+ global _last_return_value
+ _last_return_value = None
gdb.events.cont.connect(clear_scopes)
+@in_gdb_thread
+def set_finish_value(val):
+ """Set the current 'finish' value on a stop."""
+ global _last_return_value
+ _last_return_value = val
+
+
# A helper function to compute the value of a symbol. SYM is either a
# gdb.Symbol, or an object implementing the SymValueWrapper interface.
# FRAME is a frame wrapper, as produced by a frame filter. Returns a
@@ -76,7 +91,7 @@ class _ScopeReference(BaseReference):
result["presentationHint"] = self.hint
# How would we know?
result["expensive"] = False
- result["namedVariables"] = len(self.var_list)
+ result["namedVariables"] = self.child_count()
if self.line is not None:
result["line"] = self.line
# FIXME construct a Source object
@@ -93,6 +108,28 @@ class _ScopeReference(BaseReference):
return symbol_value(self.var_list[idx], self.frame)
+# A _ScopeReference that prepends the most recent return value, if one
+# is available.
+class _FinishScopeReference(_ScopeReference):
+ def __init__(self, *args):
+ super().__init__(*args)
+
+ def child_count(self):
+ result = super().child_count()
+ global _last_return_value
+ if _last_return_value is not None:
+ result = result + 1
+ return result
+
+ def fetch_one_child(self, idx):
+ global _last_return_value
+ if _last_return_value is not None:
+ if idx == 0:
+ return ("(return)", _last_return_value)
+ idx = idx - 1
+ return super().fetch_one_child(idx)
+
+
class _RegisterReference(_ScopeReference):
def __init__(self, name, frame):
super().__init__(
@@ -109,6 +146,7 @@ class _RegisterReference(_ScopeReference):
@request("scopes")
def scopes(*, frameId: int, **extra):
+ global _last_return_value
global frame_to_scope
if frameId in frame_to_scope:
scopes = frame_to_scope[frameId]
@@ -120,10 +158,13 @@ def scopes(*, frameId: int, **extra):
args = tuple(frame.frame_args() or ())
if args:
scopes.append(_ScopeReference("Arguments", "arguments", frame, args))
+ has_return_value = frameId == 0 and _last_return_value is not None
# Make sure to handle the None case as well as the empty
# iterator case.
locs = tuple(frame.frame_locals() or ())
- if locs:
+ if has_return_value:
+ scopes.append(_FinishScopeReference("Locals", "locals", frame, locs))
+ elif locs:
scopes.append(_ScopeReference("Locals", "locals", frame, locs))
scopes.append(_RegisterReference("Registers", frame))
frame_to_scope[frameId] = scopes
diff --git a/gdb/testsuite/gdb.dap/step-out.c b/gdb/testsuite/gdb.dap/step-out.c
new file mode 100644
index 00000000000..8c7e6942342
--- /dev/null
+++ b/gdb/testsuite/gdb.dap/step-out.c
@@ -0,0 +1,36 @@
+/* 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/>. */
+
+struct result
+{
+ int x;
+};
+
+struct result
+function_breakpoint_here (int q)
+{
+ struct result val;
+ val.x = q;
+ return val;
+}
+
+int
+main ()
+{
+ int q = 23;
+ return function_breakpoint_here (q).x - q;
+}
diff --git a/gdb/testsuite/gdb.dap/step-out.exp b/gdb/testsuite/gdb.dap/step-out.exp
new file mode 100644
index 00000000000..757f4ebdaca
--- /dev/null
+++ b/gdb/testsuite/gdb.dap/step-out.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 that stepOut puts the return value in scope.
+
+require allow_dap_tests
+
+load_lib dap-support.exp
+
+standard_testfile
+
+if {[build_executable ${testfile}.exp $testfile] == -1} {
+ return
+}
+
+if {[dap_initialize] == ""} {
+ return
+}
+
+set obj [dap_check_request_and_response "set breakpoint on function" \
+ setFunctionBreakpoints \
+ {o breakpoints [a [o name [s function_breakpoint_here]]]}]
+set fn_bpno [dap_get_breakpoint_number $obj]
+
+dap_check_request_and_response "configurationDone" configurationDone
+
+if {[dap_launch $testfile] == ""} {
+ return
+}
+dap_wait_for_event_and_check "inferior started" thread "body reason" started
+
+dap_wait_for_event_and_check "stopped at function breakpoint" stopped \
+ "body reason" breakpoint \
+ "body hitBreakpointIds" $fn_bpno
+
+dap_check_request_and_response "return from function" stepOut \
+ {o threadId [i 1]}
+dap_wait_for_event_and_check "stopped after return" stopped \
+ "body reason" step
+
+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] == 2} "two scopes"
+
+lassign $scopes scope reg_scope
+gdb_assert {[dict get $scope name] == "Locals"} "scope is locals"
+gdb_assert {[dict get $scope presentationHint] == "locals"} \
+ "locals presentation hint"
+gdb_assert {[dict get $scope namedVariables] == 2} "two vars in scope"
+
+set num [dict get $scope variablesReference]
+set refs [lindex [dap_check_request_and_response "fetch arguments" \
+ "variables" \
+ [format {o variablesReference [i %d]} $num]] \
+ 0]
+set varlist [lindex [dict get $refs body variables] 0]
+
+gdb_assert {[dict get $varlist variablesReference] > 0} \
+ "variable has children"
+gdb_assert {[dict get $varlist name] == "(return)"} \
+ "variable is return value"
+
+dap_shutdown
--
2.43.0
^ permalink raw reply [flat|nested] 7+ messages in thread