public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Tom Tromey <tromey@adacore.com>
To: gdb-patches@sourceware.org
Subject: [PATCH 7/7] Implement DAP "hover" context
Date: Mon, 12 Jun 2023 13:36:34 -0600	[thread overview]
Message-ID: <20230612-more-dap-v1-7-ad868f1a4cc0@adacore.com> (raw)
In-Reply-To: <20230612-more-dap-v1-0-ad868f1a4cc0@adacore.com>

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 | 13 +++++++
 gdb/target.c                       | 12 +++----
 gdb/testsuite/gdb.dap/hover.c      | 30 ++++++++++++++++
 gdb/testsuite/gdb.dap/hover.exp    | 70 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 119 insertions(+), 6 deletions(-)

diff --git a/gdb/python/lib/gdb/dap/evaluate.py b/gdb/python/lib/gdb/dap/evaluate.py
index a0b199a1ed4..651a4046a34 100644
--- a/gdb/python/lib/gdb/dap/evaluate.py
+++ b/gdb/python/lib/gdb/dap/evaluate.py
@@ -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:
diff --git a/gdb/target.c b/gdb/target.c
index 80d2c80d4bf..fecbc89d3ca 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -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
index 00000000000..f85339612cd
--- /dev/null
+++ b/gdb/testsuite/gdb.dap/hover.c
@@ -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
index 00000000000..4caf105b347
--- /dev/null
+++ b/gdb/testsuite/gdb.dap/hover.exp
@@ -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

-- 
2.40.1


  parent reply	other threads:[~2023-06-12 19:36 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-12 19:36 [PATCH 0/7] More changes to DAP Tom Tromey
2023-06-12 19:36 ` [PATCH 1/7] Fix type of DAP hitCondition Tom Tromey
2023-06-12 19:36 ` [PATCH 2/7] Reuse breakpoints more frequently in DAP Tom Tromey
2023-06-12 19:36 ` [PATCH 3/7] Handle exceptions when creating DAP breakpoints Tom Tromey
2023-06-12 19:36 ` [PATCH 4/7] Implement type checking for DAP breakpoint requests Tom Tromey
2023-06-12 19:36 ` [PATCH 5/7] Handle supportsVariablePaging in DAP Tom Tromey
2023-06-12 19:36 ` [PATCH 6/7] Implement DAP logging breakpoints Tom Tromey
2023-06-12 19:36 ` Tom Tromey [this message]
2023-06-22 15:46 ` [PATCH 0/7] More changes to DAP Tom Tromey

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230612-more-dap-v1-7-ad868f1a4cc0@adacore.com \
    --to=tromey@adacore.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).