[PATCH 0/7] More changes to DAP

[PATCH 0/7] More changes to DAP

[Tom Tromey]

This series fixes some DAP bugs and adds a few new features.

After this series, I think the main missing DAP feature is "data
breakpoints" (aka watchpoints).  I somewhat started work on these, but
haven't finished it.

Tested on x86-64 Fedora 36.

---
Tom Tromey (7):
      Fix type of DAP hitCondition
      Reuse breakpoints more frequently in DAP
      Handle exceptions when creating DAP breakpoints
      Implement type checking for DAP breakpoint requests
      Handle supportsVariablePaging in DAP
      Implement DAP logging breakpoints
      Implement DAP "hover" context

 gdb/python/lib/gdb/dap/breakpoint.py      | 275 +++++++++++++++++++++---------
 gdb/python/lib/gdb/dap/evaluate.py        |  20 ++-
 gdb/target.c                              |  12 +-
 gdb/testsuite/gdb.dap/catch-exception.exp |  24 ++-
 gdb/testsuite/gdb.dap/cond-bp.exp         |  21 ++-
 gdb/testsuite/gdb.dap/hover.c             |  30 ++++
 gdb/testsuite/gdb.dap/hover.exp           |  70 ++++++++
 gdb/testsuite/gdb.dap/log-message.c       |  31 ++++
 gdb/testsuite/gdb.dap/log-message.exp     |  51 ++++++
 gdb/testsuite/gdb.dap/scopes.exp          |  24 ++-
 gdb/testsuite/lib/dap-support.exp         |   3 +-
 11 files changed, 456 insertions(+), 105 deletions(-)
---
base-commit: 2e3aff27623b20b08ac58f8eaf73e97e58b4e67c
change-id: 20230612-more-dap-44e37ba18597

Best regards,
-- 
Tom Tromey <tromey@adacore.com>

[PATCH 1/7] Fix type of DAP hitCondition

[Tom Tromey]

DAP specifies a breakpoint's hitCondition as a string, meaning it is
an expression to be evaluated.  However, gdb implemented this as if it
were an integer instead.  This patch fixes this oversight.
---
 gdb/python/lib/gdb/dap/breakpoint.py | 11 +++++++----
 gdb/testsuite/gdb.dap/cond-bp.exp    |  2 +-
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/gdb/python/lib/gdb/dap/breakpoint.py b/gdb/python/lib/gdb/dap/breakpoint.py
index 20e65aa0e61..2cb8db68907 100644
--- a/gdb/python/lib/gdb/dap/breakpoint.py
+++ b/gdb/python/lib/gdb/dap/breakpoint.py
@@ -95,10 +95,13 @@ def _set_breakpoints_callback(kind, specs, creator):
             # FIXME handle exceptions here
             bp = creator(**spec)
 
-        if condition is not None:
-            bp.condition = condition
-        if hit_condition is not None:
-            bp.ignore_count = hit_condition
+        bp.condition = condition
+        if hit_condition is None:
+            bp.ignore_count = 0
+        else:
+            bp.ignore_count = int(
+                gdb.parse_and_eval(hit_condition, global_context=True)
+            )
 
         breakpoint_map[kind][keyspec] = bp
         result.append(breakpoint_descriptor(bp))
diff --git a/gdb/testsuite/gdb.dap/cond-bp.exp b/gdb/testsuite/gdb.dap/cond-bp.exp
index 376db4b3548..6369b6f579c 100644
--- a/gdb/testsuite/gdb.dap/cond-bp.exp
+++ b/gdb/testsuite/gdb.dap/cond-bp.exp
@@ -35,7 +35,7 @@ set obj [dap_check_request_and_response "set conditional breakpoint" \
 	     [format {o source [o path [%s]] \
 			  breakpoints [a [o line [i %d] \
 					      condition [s "i == 3"] \
-					      hitCondition [i 3]]]} \
+					      hitCondition [s 3]]]} \
 		  [list s $srcfile] $line]]
 set fn_bpno [dap_get_breakpoint_number $obj]
 

-- 
2.40.1

[PATCH 2/7] Reuse breakpoints more frequently in DAP

[Tom Tromey]

The DAP breakpoint code tries to reuse a breakpoint when possible.
Currently it uses the condition and the hit condition (aka ignore
count) when making this determination.  However, these attributes are
just going to be reset anyway, so this patch changes the code to
exclude these from the reuse decision.
---
 gdb/python/lib/gdb/dap/breakpoint.py | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/gdb/python/lib/gdb/dap/breakpoint.py b/gdb/python/lib/gdb/dap/breakpoint.py
index 2cb8db68907..f15f905c5f3 100644
--- a/gdb/python/lib/gdb/dap/breakpoint.py
+++ b/gdb/python/lib/gdb/dap/breakpoint.py
@@ -85,9 +85,11 @@ def _set_breakpoints_callback(kind, specs, creator):
     breakpoint_map[kind] = {}
     result = []
     for spec in specs:
-        keyspec = frozenset(spec.items())
-
+        # It makes sense to reuse a breakpoint even if the condition
+        # or ignore count differs, so remove these entries from the
+        # spec first.
         (condition, hit_condition) = _remove_entries(spec, "condition", "hitCondition")
+        keyspec = frozenset(spec.items())
 
         if keyspec in saved_map:
             bp = saved_map.pop(keyspec)

-- 
2.40.1

[PATCH 3/7] Handle exceptions when creating DAP breakpoints

[Tom Tromey]

When creating a DAP breakpoint, a failure should be returned by
setting "verified" to False.  gdb didn't properly implement this, and
there was a FIXME comment to this effect.  This patch fixes the
problem.
---
 gdb/python/lib/gdb/dap/breakpoint.py      | 89 +++++++++++++++++++------------
 gdb/testsuite/gdb.dap/catch-exception.exp | 24 ++++++---
 gdb/testsuite/gdb.dap/cond-bp.exp         | 19 +++++++
 3 files changed, 92 insertions(+), 40 deletions(-)

diff --git a/gdb/python/lib/gdb/dap/breakpoint.py b/gdb/python/lib/gdb/dap/breakpoint.py
index f15f905c5f3..3f40cfb7f63 100644
--- a/gdb/python/lib/gdb/dap/breakpoint.py
+++ b/gdb/python/lib/gdb/dap/breakpoint.py
@@ -20,7 +20,8 @@ import os
 from typing import Optional, Sequence
 
 from .server import request, capability
-from .startup import send_gdb_with_response, in_gdb_thread
+from .startup import send_gdb_with_response, in_gdb_thread, log_stack
+from .typecheck import type_check
 
 
 # Map from the breakpoint "kind" (like "function") to a second map, of
@@ -34,33 +35,35 @@ breakpoint_map = {}
 @in_gdb_thread
 def breakpoint_descriptor(bp):
     "Return the Breakpoint object descriptor given a gdb Breakpoint."
+    result = {
+        "id": bp.number,
+        # We always use True here, because this field just indicates
+        # that breakpoint creation was successful -- and if we have a
+        # breakpoint, the creation succeeded.
+        "verified": True,
+    }
     if bp.locations:
         # Just choose the first location, because DAP doesn't allow
         # multiple locations.  See
         # https://github.com/microsoft/debug-adapter-protocol/issues/13
         loc = bp.locations[0]
         (basename, line) = loc.source
-        result = {
-            "id": bp.number,
-            "verified": True,
-            "source": {
-                "name": os.path.basename(basename),
-                # We probably don't need this but it doesn't hurt to
-                # be explicit.
-                "sourceReference": 0,
-            },
-            "line": line,
-            "instructionReference": hex(loc.address),
-        }
+        result.update(
+            {
+                "source": {
+                    "name": os.path.basename(basename),
+                    # We probably don't need this but it doesn't hurt to
+                    # be explicit.
+                    "sourceReference": 0,
+                },
+                "line": line,
+                "instructionReference": hex(loc.address),
+            }
+        )
         path = loc.fullname
         if path is not None:
             result["source"]["path"] = path
-        return result
-    else:
-        return {
-            "id": bp.number,
-            "verified": False,
-        }
+    return result
 
 
 # Extract entries from a hash table and return a list of them.  Each
@@ -91,22 +94,42 @@ def _set_breakpoints_callback(kind, specs, creator):
         (condition, hit_condition) = _remove_entries(spec, "condition", "hitCondition")
         keyspec = frozenset(spec.items())
 
-        if keyspec in saved_map:
-            bp = saved_map.pop(keyspec)
-        else:
-            # FIXME handle exceptions here
-            bp = creator(**spec)
-
-        bp.condition = condition
-        if hit_condition is None:
-            bp.ignore_count = 0
-        else:
-            bp.ignore_count = int(
-                gdb.parse_and_eval(hit_condition, global_context=True)
+        # Create or reuse a breakpoint.  If asked, set the condition
+        # or the ignore count.  Catch errors coming from gdb and
+        # report these as an "unverified" breakpoint.
+        bp = None
+        try:
+            if keyspec in saved_map:
+                bp = saved_map.pop(keyspec)
+            else:
+                bp = creator(**spec)
+
+            bp.condition = condition
+            if hit_condition is None:
+                bp.ignore_count = 0
+            else:
+                bp.ignore_count = int(
+                    gdb.parse_and_eval(hit_condition, global_context=True)
+                )
+
+            # Reaching this spot means success.
+            breakpoint_map[kind][keyspec] = bp
+            result.append(breakpoint_descriptor(bp))
+        # Exceptions other than gdb.error are possible here.
+        except Exception as e:
+            log_stack()
+            # Maybe the breakpoint was made but setting an attribute
+            # failed.  We still want this to fail.
+            if bp is not None:
+                bp.delete()
+            # Breakpoint creation failed.
+            result.append(
+                {
+                    "verified": False,
+                    "message": str(e),
+                }
             )
 
-        breakpoint_map[kind][keyspec] = bp
-        result.append(breakpoint_descriptor(bp))
     # Delete any breakpoints that were not reused.
     for entry in saved_map.values():
         entry.delete()
diff --git a/gdb/testsuite/gdb.dap/catch-exception.exp b/gdb/testsuite/gdb.dap/catch-exception.exp
index 7f2e750b32e..95e65556cc6 100644
--- a/gdb/testsuite/gdb.dap/catch-exception.exp
+++ b/gdb/testsuite/gdb.dap/catch-exception.exp
@@ -31,22 +31,32 @@ if {[dap_launch $binfile] == ""} {
 
 set obj [dap_check_request_and_response "set exception catchpoints" \
 	     setExceptionBreakpoints \
-	     {o filters [a [s assert]] \
+	     {o filters [a [s nosuchfilter] [s assert]] \
 		  filterOptions [a [o filterId [s exception] \
 					condition [s "Global_Var = 23"]]]}]
 
 set bps [dict get [lindex $obj 0] body breakpoints]
-gdb_assert {[llength $bps] == 2} "two breakpoints"
+# We should get three responses, because we requested three
+# breakpoints.  However, one of them should fail.
+gdb_assert {[llength $bps] == 3} "three breakpoints"
 
 # The "path" should never be "null".
 set i 1
 foreach spec $bps {
-    # If "path" does not exist, then that is fine as well.
-    if {![dict exists $spec source path]} {
-	pass "breakpoint $i path"
+    if {$i == 1} {
+	# First one should fail.
+	gdb_assert {[dict get $spec verified] == "false"} \
+	    "invalid first exception"
+	gdb_assert {[dict get $spec message] != ""} \
+	    "first exception had message"
     } else {
-	gdb_assert {[dict get $spec source path] != "null"} \
-	    "breakpoint $i path"
+	# If "path" does not exist, then that is fine as well.
+	if {![dict exists $spec source path]} {
+	    pass "breakpoint $i path"
+	} else {
+	    gdb_assert {[dict get $spec source path] != "null"} \
+		"breakpoint $i path"
+	}
     }
     incr i
 }
diff --git a/gdb/testsuite/gdb.dap/cond-bp.exp b/gdb/testsuite/gdb.dap/cond-bp.exp
index 6369b6f579c..262ab9d26c8 100644
--- a/gdb/testsuite/gdb.dap/cond-bp.exp
+++ b/gdb/testsuite/gdb.dap/cond-bp.exp
@@ -30,6 +30,25 @@ if {[dap_launch $testfile] == ""} {
 }
 
 set line [gdb_get_line_number "STOP"]
+
+# Test some breakpoint-setting failure modes.
+set obj [dap_check_request_and_response "set invalid breakpoints" \
+	     setBreakpoints \
+	     [format {o source [o path [%s]] \
+			  breakpoints \
+			  [a [o line [i %d] condition [s "DEI@@++"]] \
+			       [o line [i %d] hitCondition [s "DEI@@++"]]]} \
+		  [list s $srcfile] $line $line]]
+
+set i 1
+foreach bp [dict get [lindex $obj 0] body breakpoints] {
+    gdb_assert {[dict get $bp verified] == "false"} \
+	"breakpoint $i invalid"
+    gdb_assert {[dict get $bp message] != ""} \
+	"breakpoint $i has message"
+    incr i
+}
+
 set obj [dap_check_request_and_response "set conditional breakpoint" \
 	     setBreakpoints \
 	     [format {o source [o path [%s]] \

-- 
2.40.1

[PATCH 4/7] Implement type checking for DAP breakpoint requests

[Tom Tromey]

I realized that with a small refactoring, it is possible to type-check
the parameters to the various DAP breakpoint requests.  This would
have caught the earlier bug involving hitCondition.
---
 gdb/python/lib/gdb/dap/breakpoint.py | 128 ++++++++++++++++++++++-------------
 1 file changed, 82 insertions(+), 46 deletions(-)

diff --git a/gdb/python/lib/gdb/dap/breakpoint.py b/gdb/python/lib/gdb/dap/breakpoint.py
index 3f40cfb7f63..227085ad73e 100644
--- a/gdb/python/lib/gdb/dap/breakpoint.py
+++ b/gdb/python/lib/gdb/dap/breakpoint.py
@@ -143,19 +143,28 @@ def _set_breakpoints(kind, specs):
     return _set_breakpoints_callback(kind, specs, gdb.Breakpoint)
 
 
-# Turn a DAP SourceBreakpoint, FunctionBreakpoint, or
-# InstructionBreakpoint into a "spec" that is used by
-# _set_breakpoints.  SPEC is a dictionary of parameters that is used
-# as the base of the result; it is modified in place.
-def _basic_spec(bp_info, spec):
-    for name in ("condition", "hitCondition"):
-        if name in bp_info:
-            spec[name] = bp_info[name]
-    return spec
+# A helper function that rewrites a SourceBreakpoint into the internal
+# form passed to the creator.  This function also allows for
+# type-checking of each SourceBreakpoint.
+@type_check
+def _rewrite_src_breakpoint(
+    *,
+    # This is a Source but we don't type-check it.
+    source,
+    line: int,
+    condition: Optional[str] = None,
+    hitCondition: Optional[str] = None,
+    **args,
+):
+    return {
+        "source": source["path"],
+        "line": line,
+        "condition": condition,
+        "hitCondition": hitCondition,
+    }
 
 
 # FIXME we do not specify a type for 'source'.
-# FIXME 'breakpoints' is really a list[SourceBreakpoint].
 @request("setBreakpoints")
 @capability("supportsHitConditionalBreakpoints")
 @capability("supportsConditionalBreakpoints")
@@ -163,17 +172,7 @@ def set_breakpoint(*, source, breakpoints: Sequence = (), **args):
     if "path" not in source:
         result = []
     else:
-        specs = []
-        for obj in breakpoints:
-            specs.append(
-                _basic_spec(
-                    obj,
-                    {
-                        "source": source["path"],
-                        "line": obj["line"],
-                    },
-                )
-            )
+        specs = [_rewrite_src_breakpoint(source=source, **bp) for bp in breakpoints]
         # Be sure to include the path in the key, so that we only
         # clear out breakpoints coming from this same source.
         key = "source:" + source["path"]
@@ -183,45 +182,64 @@ def set_breakpoint(*, source, breakpoints: Sequence = (), **args):
     }
 
 
+# A helper function that rewrites a FunctionBreakpoint into the
+# internal form passed to the creator.  This function also allows for
+# type-checking of each FunctionBreakpoint.
+@type_check
+def _rewrite_fn_breakpoint(
+    *,
+    name: str,
+    condition: Optional[str] = None,
+    hitCondition: Optional[str] = None,
+    **args,
+):
+    return {
+        "function": name,
+        "condition": condition,
+        "hitCondition": hitCondition,
+    }
+
+
 @request("setFunctionBreakpoints")
 @capability("supportsFunctionBreakpoints")
 def set_fn_breakpoint(*, breakpoints: Sequence, **args):
-    specs = []
-    for bp in breakpoints:
-        specs.append(
-            _basic_spec(
-                bp,
-                {
-                    "function": bp["name"],
-                },
-            )
-        )
+    specs = [_rewrite_fn_breakpoint(**bp) for bp in breakpoints]
     result = send_gdb_with_response(lambda: _set_breakpoints("function", specs))
     return {
         "breakpoints": result,
     }
 
 
+# A helper function that rewrites an InstructionBreakpoint into the
+# internal form passed to the creator.  This function also allows for
+# type-checking of each InstructionBreakpoint.
+@type_check
+def _rewrite_insn_breakpoint(
+    *,
+    instructionReference: str,
+    offset: Optional[int] = None,
+    condition: Optional[str] = None,
+    hitCondition: Optional[str] = None,
+    **args,
+):
+    # There's no way to set an explicit address breakpoint from
+    # Python, so we rely on "spec" instead.
+    val = "*" + instructionReference
+    if offset is not None:
+        val = val + " + " + str(offset)
+    return {
+        "spec": val,
+        "condition": condition,
+        "hitCondition": hitCondition,
+    }
+
+
 @request("setInstructionBreakpoints")
 @capability("supportsInstructionBreakpoints")
 def set_insn_breakpoints(
     *, breakpoints: Sequence, offset: Optional[int] = None, **args
 ):
-    specs = []
-    for bp in breakpoints:
-        # There's no way to set an explicit address breakpoint
-        # from Python, so we rely on "spec" instead.
-        val = "*" + bp["instructionReference"]
-        if offset is not None:
-            val = val + " + " + str(offset)
-        specs.append(
-            _basic_spec(
-                bp,
-                {
-                    "spec": val,
-                },
-            )
-        )
+    specs = [_rewrite_insn_breakpoint(**bp) for bp in breakpoints]
     result = send_gdb_with_response(lambda: _set_breakpoints("instruction", specs))
     return {
         "breakpoints": result,
@@ -249,6 +267,23 @@ def _set_exception_catchpoints(filter_options):
     return _set_breakpoints_callback("exception", filter_options, _catch_exception)
 
 
+# A helper function that rewrites an ExceptionFilterOptions into the
+# internal form passed to the creator.  This function also allows for
+# type-checking of each ExceptionFilterOptions.
+@type_check
+def _rewrite_exception_breakpoint(
+    *,
+    filterId: str,
+    condition: Optional[str] = None,
+    # Note that exception breakpoints do not support a hit count.
+    **args,
+):
+    return {
+        "filterId": filterId,
+        "condition": condition,
+    }
+
+
 @request("setExceptionBreakpoints")
 @capability("supportsExceptionFilterOptions")
 @capability(
@@ -272,6 +307,7 @@ def set_exception_breakpoints(
     # Convert the 'filters' to the filter-options style.
     options = [{"filterId": filter} for filter in filters]
     options.extend(filterOptions)
+    options = [_rewrite_exception_breakpoint(**bp) for bp in options]
     result = send_gdb_with_response(lambda: _set_exception_catchpoints(options))
     return {
         "breakpoints": result,

-- 
2.40.1

[PATCH 5/7] Handle supportsVariablePaging in DAP

[Tom Tromey]

A bug report about the supportsVariablePaging capability in DAP
resulted in a clarification: when this capability is not present, DAP
implementations should ignore the paging parameters to the "variables"
request.  This patch implements this clarification.
---
 gdb/python/lib/gdb/dap/evaluate.py |  7 ++++++-
 gdb/testsuite/gdb.dap/scopes.exp   | 24 +++++++++++++++++-------
 gdb/testsuite/lib/dap-support.exp  |  3 ++-
 3 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/gdb/python/lib/gdb/dap/evaluate.py b/gdb/python/lib/gdb/dap/evaluate.py
index af7bf43afd0..a0b199a1ed4 100644
--- a/gdb/python/lib/gdb/dap/evaluate.py
+++ b/gdb/python/lib/gdb/dap/evaluate.py
@@ -20,7 +20,7 @@ import gdb.printing
 from typing import Optional
 
 from .frames import frame_for_id
-from .server import capability, request
+from .server import capability, request, client_bool_capability
 from .startup import send_gdb_with_response, in_gdb_thread
 from .varref import find_variable, VariableReference
 
@@ -98,6 +98,11 @@ def _variables(ref, start, count):
 # Note that we ignore the 'filter' field.  That seems to be
 # specific to javascript.
 def variables(*, variablesReference: int, start: int = 0, count: int = 0, **args):
+    # This behavior was clarified here:
+    # https://github.com/microsoft/debug-adapter-protocol/pull/394
+    if not client_bool_capability("supportsVariablePaging"):
+        start = 0
+        count = 0
     result = send_gdb_with_response(
         lambda: _variables(variablesReference, start, count)
     )
diff --git a/gdb/testsuite/gdb.dap/scopes.exp b/gdb/testsuite/gdb.dap/scopes.exp
index cf9174f06a2..6937badcca0 100644
--- a/gdb/testsuite/gdb.dap/scopes.exp
+++ b/gdb/testsuite/gdb.dap/scopes.exp
@@ -67,13 +67,23 @@ gdb_assert {[dict get $reg_scope presentationHint] == "registers"} \
 gdb_assert {[dict get $reg_scope namedVariables] > 0} "at least one register"
 
 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] {
+# Send two requests and combine them, to verify that using a range
+# works.
+set refs1 [lindex [dap_check_request_and_response "fetch variables 0,1" \
+		       "variables" \
+		       [format {o variablesReference [i %d] count [i 2]} \
+			    $num]] \
+	       0]
+set refs2 [lindex [dap_check_request_and_response "fetch variables 2" \
+		       "variables" \
+		       [format {o variablesReference [i %d] \
+				    start [i 2] count [i 1]} \
+			    $num]] \
+	       0]
+
+set vars [concat [dict get $refs1 body variables] \
+	      [dict get $refs2 body variables]]
+foreach var $vars {
     set name [dict get $var name]
 
     if {$name != "dei"} {
diff --git a/gdb/testsuite/lib/dap-support.exp b/gdb/testsuite/lib/dap-support.exp
index 92484bfdb8d..e3750e1d016 100644
--- a/gdb/testsuite/lib/dap-support.exp
+++ b/gdb/testsuite/lib/dap-support.exp
@@ -232,7 +232,8 @@ proc _dap_initialize {name} {
     }
     return [dap_check_request_and_response $name initialize \
 		{o clientID [s "gdb testsuite"] \
-		     supportsVariableType [l true]}]
+		     supportsVariableType [l true] \
+		     supportsVariablePaging [l true]}]
 }
 
 # Start gdb, send a DAP initialize request, and then a launch request

-- 
2.40.1

[PATCH 6/7] Implement DAP logging breakpoints

[Tom Tromey]

DAP allows a source breakpoint to specify a log message.  When this is
done, the breakpoint acts more like gdb's dprintf: it logs a message
but does not cause a stop.

I looked into implement this using dprintf with the new %V printf
format.  However, my initial attempt at this did not work, because
when the inferior is continued, the dprintf output is captured by the
gdb.execute call.  Maybe this could be fixed by having all
inferior-continuation commands use the "&" form; the main benefit of
this would be that expressions are only parsed a single time.
---
 gdb/python/lib/gdb/dap/breakpoint.py  | 53 +++++++++++++++++++++++++++++++++--
 gdb/testsuite/gdb.dap/log-message.c   | 31 ++++++++++++++++++++
 gdb/testsuite/gdb.dap/log-message.exp | 51 +++++++++++++++++++++++++++++++++
 3 files changed, 132 insertions(+), 3 deletions(-)

diff --git a/gdb/python/lib/gdb/dap/breakpoint.py b/gdb/python/lib/gdb/dap/breakpoint.py
index 227085ad73e..27745ebfd2c 100644
--- a/gdb/python/lib/gdb/dap/breakpoint.py
+++ b/gdb/python/lib/gdb/dap/breakpoint.py
@@ -15,11 +15,12 @@
 
 import gdb
 import os
+import re
 
 # These are deprecated in 3.9, but required in older versions.
 from typing import Optional, Sequence
 
-from .server import request, capability
+from .server import request, capability, send_event
 from .startup import send_gdb_with_response, in_gdb_thread, log_stack
 from .typecheck import type_check
 
@@ -136,11 +137,54 @@ def _set_breakpoints_callback(kind, specs, creator):
     return result
 
 
-# Helper function to set odinary breakpoints according to a list of
+class _PrintBreakpoint(gdb.Breakpoint):
+    def __init__(self, logMessage, **args):
+        super().__init__(**args)
+        # Split the message up for easier processing.
+        self.message = re.split("{(.*?)}", logMessage)
+
+    def stop(self):
+        output = ""
+        for idx, item in enumerate(self.message):
+            if idx % 2 == 0:
+                # Even indices are plain text.
+                output += item
+            else:
+                # Odd indices are expressions to substitute.  The {}
+                # have already been stripped by the placement of the
+                # regex capture in the 'split' call.
+                try:
+                    val = gdb.parse_and_eval(item)
+                    output += str(val)
+                except Exception as e:
+                    output += "<" + str(e) + ">"
+        send_event(
+            "output",
+            {
+                "category": "console",
+                "output": output,
+            },
+        )
+        # Do not stop.
+        return False
+
+
+# Set a single breakpoint or a log point.  Returns the new breakpoint.
+# Note that not every spec will pass logMessage, so here we use a
+# default.
+@in_gdb_thread
+def _set_one_breakpoint(*, logMessage=None, **args):
+    if logMessage is not None:
+        return _PrintBreakpoint(logMessage, **args)
+    else:
+        return gdb.Breakpoint(**args)
+
+
+# Helper function to set ordinary breakpoints according to a list of
 # specifications.
 @in_gdb_thread
 def _set_breakpoints(kind, specs):
-    return _set_breakpoints_callback(kind, specs, gdb.Breakpoint)
+    return _set_breakpoints_callback(kind, specs, _set_one_breakpoint)
 
 
 # A helper function that rewrites a SourceBreakpoint into the internal
@@ -154,6 +198,7 @@ def _rewrite_src_breakpoint(
     line: int,
     condition: Optional[str] = None,
     hitCondition: Optional[str] = None,
+    logMessage: Optional[str] = None,
     **args,
 ):
     return {
@@ -161,6 +206,7 @@ def _rewrite_src_breakpoint(
         "line": line,
         "condition": condition,
         "hitCondition": hitCondition,
+        "logMessage": logMessage,
     }
 
 
@@ -168,6 +214,7 @@ def _rewrite_src_breakpoint(
 @request("setBreakpoints")
 @capability("supportsHitConditionalBreakpoints")
 @capability("supportsConditionalBreakpoints")
+@capability("supportsLogPoints")
 def set_breakpoint(*, source, breakpoints: Sequence = (), **args):
     if "path" not in source:
         result = []
diff --git a/gdb/testsuite/gdb.dap/log-message.c b/gdb/testsuite/gdb.dap/log-message.c
new file mode 100644
index 00000000000..d2c746033ad
--- /dev/null
+++ b/gdb/testsuite/gdb.dap/log-message.c
@@ -0,0 +1,31 @@
+/* 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;
+
+int
+func (int local)
+{
+  return global_variable - local; /* HERE */
+}
+
+int
+main ()
+{
+  global_variable = 23;
+  return func (23);
+}
diff --git a/gdb/testsuite/gdb.dap/log-message.exp b/gdb/testsuite/gdb.dap/log-message.exp
new file mode 100644
index 00000000000..31d5c4a11d4
--- /dev/null
+++ b/gdb/testsuite/gdb.dap/log-message.exp
@@ -0,0 +1,51 @@
+# 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 logging breakpoints.
+
+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 "HERE"]
+set obj [dap_check_request_and_response "set breakpoint" \
+	     setBreakpoints \
+	     [format {o source [o path [%s]] \
+			  breakpoints [a [o line [i %d] \
+					      logMessage [s "got {global_variable} - {local} = {global_variable - local}"]]]} \
+		  [list s $srcfile] $line]]
+set fn_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 "logging output" output \
+    {body category} console \
+    {body output} "got 23 - 23 = 0"
+
+# Check that the breakpoint did not cause a stop.
+dap_wait_for_event_and_check "inferior exited" exited
+
+dap_shutdown

-- 
2.40.1

[PATCH 7/7] Implement DAP "hover" context

[Tom Tromey]

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

Re: [PATCH 0/7] More changes to DAP

[Tom Tromey]

>>>>> "Tom" == Tom Tromey via Gdb-patches <gdb-patches@sourceware.org> writes:

Tom> This series fixes some DAP bugs and adds a few new features.
Tom> After this series, I think the main missing DAP feature is "data
Tom> breakpoints" (aka watchpoints).  I somewhat started work on these, but
Tom> haven't finished it.

I'm checking these in.

Tom