Implement DAP "hover" context
authorTom Tromey <tromey@adacore.com>
Thu, 1 Jun 2023 17:54:17 +0000 (11:54 -0600)
committerTom Tromey <tromey@adacore.com>
Thu, 22 Jun 2023 15:46:24 +0000 (09:46 -0600)
A DAP client can request that an expression be evaluated in "hover"
context, meaning that it should not cause side effects.  In gdb, this
can be implemented by temporarily setting a few "may-" parameters to
"off".

In order to make this work, I had to also change "may-write-registers"
so that it can be changed while the program is running.  I don't think
there was any reason for this prohibition in the first place.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30476

gdb/python/lib/gdb/dap/evaluate.py
gdb/target.c
gdb/testsuite/gdb.dap/hover.c [new file with mode: 0644]
gdb/testsuite/gdb.dap/hover.exp [new file with mode: 0644]

index a0b199a1ed43224dc2980250f396aed54dd0cbc4..651a4046a348047179768536ed262a70c3bb4f89 100644 (file)
@@ -43,6 +43,16 @@ def _evaluate(expr, frame_id):
     return ref.to_object()
 
 
+# Like _evaluate but ensure that the expression cannot cause side
+# effects.
+@in_gdb_thread
+def _eval_for_hover(expr, frame_id):
+    with gdb.with_parameter("may-write-registers", "off"):
+        with gdb.with_parameter("may-write-memory", "off"):
+            with gdb.with_parameter("may-call-functions", "off"):
+                return _evaluate(expr, frame_id)
+
+
 # Helper function to perform an assignment.
 @in_gdb_thread
 def _set_expression(expression, value, frame_id):
@@ -71,6 +81,7 @@ def _repl(command, frame_id):
 
 
 @request("evaluate")
+@capability("supportsEvaluateForHovers")
 def eval_request(
     *,
     expression: str,
@@ -81,6 +92,8 @@ def eval_request(
     if context in ("watch", "variables"):
         # These seem to be expression-like.
         return send_gdb_with_response(lambda: _evaluate(expression, frameId))
+    elif context == "hover":
+        return send_gdb_with_response(lambda: _eval_for_hover(expression, frameId))
     elif context == "repl":
         return send_gdb_with_response(lambda: _repl(expression, frameId))
     else:
index 80d2c80d4bfb8306671716054d79bbcc3fa4afaa..fecbc89d3ca13526b3f0f99a178ed208d717dec2 100644 (file)
@@ -4494,7 +4494,6 @@ set_target_permissions (const char *args, int from_tty,
     }
 
   /* Make the real values match the user-changed values.  */
-  may_write_registers = may_write_registers_1;
   may_insert_breakpoints = may_insert_breakpoints_1;
   may_insert_tracepoints = may_insert_tracepoints_1;
   may_insert_fast_tracepoints = may_insert_fast_tracepoints_1;
@@ -4502,14 +4501,15 @@ set_target_permissions (const char *args, int from_tty,
   update_observer_mode ();
 }
 
-/* Set memory write permission independently of observer mode.  */
+/* Set some permissions independently of observer mode.  */
 
 static void
-set_write_memory_permission (const char *args, int from_tty,
-                       struct cmd_list_element *c)
+set_write_memory_registers_permission (const char *args, int from_tty,
+                                      struct cmd_list_element *c)
 {
   /* Make the real values match the user-changed values.  */
   may_write_memory = may_write_memory_1;
+  may_write_registers = may_write_registers_1;
   update_observer_mode ();
 }
 
@@ -4578,7 +4578,7 @@ Set permission to write into registers."), _("\
 Show permission to write into registers."), _("\
 When this permission is on, GDB may write into the target's registers.\n\
 Otherwise, any sort of write attempt will result in an error."),
-                          set_target_permissions, NULL,
+                          set_write_memory_registers_permission, NULL,
                           &setlist, &showlist);
 
   add_setshow_boolean_cmd ("may-write-memory", class_support,
@@ -4587,7 +4587,7 @@ Set permission to write into target memory."), _("\
 Show permission to write into target memory."), _("\
 When this permission is on, GDB may write into the target's memory.\n\
 Otherwise, any sort of write attempt will result in an error."),
-                          set_write_memory_permission, NULL,
+                          set_write_memory_registers_permission, NULL,
                           &setlist, &showlist);
 
   add_setshow_boolean_cmd ("may-insert-breakpoints", class_support,
diff --git a/gdb/testsuite/gdb.dap/hover.c b/gdb/testsuite/gdb.dap/hover.c
new file mode 100644 (file)
index 0000000..f853396
--- /dev/null
@@ -0,0 +1,30 @@
+/* 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/>.  */
+
+int global_variable = 23;
+
+int
+increment ()
+{
+  return ++global_variable;
+}
+
+int
+main ()
+{
+  return 0;                    /* BREAK */
+}
diff --git a/gdb/testsuite/gdb.dap/hover.exp b/gdb/testsuite/gdb.dap/hover.exp
new file mode 100644 (file)
index 0000000..4caf105
--- /dev/null
@@ -0,0 +1,70 @@
+# 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 "hover" evaluation.
+
+require allow_dap_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 breakpoint" stopped \
+    "body reason" breakpoint \
+    "body hitBreakpointIds" $line_bpno
+
+set obj [dap_check_request_and_response "evaluate global" \
+            evaluate {o expression [s global_variable]}]
+dap_match_values "global value in function" [lindex $obj 0] \
+    "body result" 23
+
+set obj [dap_request_and_response \
+            evaluate {o context [s hover] expression [s increment()]}]
+gdb_assert {[dict get [lindex $obj 0] success] == "false"} \
+    "increment was rejected in hover mode"
+
+dap_check_request_and_response "call increment" \
+    evaluate {o expression [s increment()]}
+
+set obj [dap_request_and_response \
+            evaluate {o context [s hover] \
+                          expression [s "global_variable = -1"]}]
+gdb_assert {[dict get [lindex $obj 0] success] == "false"} \
+    "assignment was rejected in hover mode"
+
+set obj [dap_check_request_and_response "evaluate global again" \
+            evaluate {o expression [s global_variable]}]
+dap_match_values "global value incremented once" [lindex $obj 0] \
+    "body result" 24
+
+dap_shutdown