Implement ValueFormat for DAP
authorTom Tromey <tromey@adacore.com>
Tue, 25 Jul 2023 16:13:52 +0000 (10:13 -0600)
committerTom Tromey <tromey@adacore.com>
Tue, 1 Aug 2023 19:03:31 +0000 (13:03 -0600)
This patch implements ValueFormat for DAP.  Currently this only means
supporting "hex".

Note that StackFrameFormat is defined to have many more options, but
none are currently recognized.  It isn't entirely clear how these
should be handled.  I'll file a new gdb bug for this, and perhaps an
upstream DAP bug as well.

New in v2:
- I realized that the "hover" context was broken, and furthermore
  that we only had tests for "hover" failing, not for it succeeding.
  This version fixes the oversight and adds a test.

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

gdb/python/lib/gdb/dap/bt.py
gdb/python/lib/gdb/dap/evaluate.py
gdb/python/lib/gdb/dap/varref.py
gdb/testsuite/gdb.dap/hover.exp

index 975c88f8208289e6ff04ff71d52bca579ba763d0..4d1d89a81c3c6ef646f1eb1023031fab36c519a4 100644 (file)
@@ -22,60 +22,66 @@ from .modules import module_id
 from .server import request, capability
 from .startup import send_gdb_with_response, in_gdb_thread
 from .state import set_thread
+from .varref import apply_format
 
 
 # Helper function to compute a stack trace.
 @in_gdb_thread
-def _backtrace(thread_id, levels, startFrame):
-    set_thread(thread_id)
-    frames = []
-    if levels == 0:
-        # Zero means all remaining frames.
-        high = -1
-    else:
-        # frame_iterator uses an inclusive range, so subtract one.
-        high = startFrame + levels - 1
-    try:
-        frame_iter = frame_iterator(gdb.newest_frame(), startFrame, high)
-    except gdb.error:
-        frame_iter = ()
-    for current_frame in frame_iter:
-        pc = current_frame.address()
-        newframe = {
-            "id": frame_id(current_frame),
-            "name": current_frame.function(),
-            # This must always be supplied, but we will set it
-            # correctly later if that is possible.
-            "line": 0,
-            # GDB doesn't support columns.
-            "column": 0,
-            "instructionPointerReference": hex(pc),
-        }
-        objfile = gdb.current_progspace().objfile_for_address(pc)
-        if objfile is not None:
-            newframe["moduleId"] = module_id(objfile)
-        line = current_frame.line()
-        if line is not None:
-            newframe["line"] = line
-        filename = current_frame.filename()
-        if filename is not None:
-            newframe["source"] = {
-                "name": os.path.basename(filename),
-                "path": filename,
-                # We probably don't need this but it doesn't hurt
-                # to be explicit.
-                "sourceReference": 0,
+def _backtrace(thread_id, levels, startFrame, value_format):
+    with apply_format(value_format):
+        set_thread(thread_id)
+        frames = []
+        if levels == 0:
+            # Zero means all remaining frames.
+            high = -1
+        else:
+            # frame_iterator uses an inclusive range, so subtract one.
+            high = startFrame + levels - 1
+        try:
+            frame_iter = frame_iterator(gdb.newest_frame(), startFrame, high)
+        except gdb.error:
+            frame_iter = ()
+        for current_frame in frame_iter:
+            pc = current_frame.address()
+            newframe = {
+                "id": frame_id(current_frame),
+                "name": current_frame.function(),
+                # This must always be supplied, but we will set it
+                # correctly later if that is possible.
+                "line": 0,
+                # GDB doesn't support columns.
+                "column": 0,
+                "instructionPointerReference": hex(pc),
             }
-        frames.append(newframe)
-    # Note that we do not calculate totalFrames here.  Its absence
-    # tells the client that it may simply ask for frames until a
-    # response yields fewer frames than requested.
-    return {
-        "stackFrames": frames,
-    }
+            objfile = gdb.current_progspace().objfile_for_address(pc)
+            if objfile is not None:
+                newframe["moduleId"] = module_id(objfile)
+            line = current_frame.line()
+            if line is not None:
+                newframe["line"] = line
+            filename = current_frame.filename()
+            if filename is not None:
+                newframe["source"] = {
+                    "name": os.path.basename(filename),
+                    "path": filename,
+                    # We probably don't need this but it doesn't hurt
+                    # to be explicit.
+                    "sourceReference": 0,
+                }
+            frames.append(newframe)
+        # Note that we do not calculate totalFrames here.  Its absence
+        # tells the client that it may simply ask for frames until a
+        # response yields fewer frames than requested.
+        return {
+            "stackFrames": frames,
+        }
 
 
 @request("stackTrace")
 @capability("supportsDelayedStackTraceLoading")
-def stacktrace(*, levels: int = 0, startFrame: int = 0, threadId: int, **extra):
-    return send_gdb_with_response(lambda: _backtrace(threadId, levels, startFrame))
+def stacktrace(
+    *, levels: int = 0, startFrame: int = 0, threadId: int, format=None, **extra
+):
+    return send_gdb_with_response(
+        lambda: _backtrace(threadId, levels, startFrame, format)
+    )
index 9fa94e08121fb4c99186ee883a1674b9975d7238..405e6fc24104a9cd91eb667136c372d4e9d906ea 100644 (file)
@@ -21,7 +21,7 @@ from typing import Optional
 from .frames import select_frame
 from .server import capability, request, client_bool_capability
 from .startup import send_gdb_with_response, in_gdb_thread
-from .varref import find_variable, VariableReference
+from .varref import find_variable, VariableReference, apply_format
 
 
 class EvaluateResult(VariableReference):
@@ -31,24 +31,25 @@ class EvaluateResult(VariableReference):
 
 # Helper function to evaluate an expression in a certain frame.
 @in_gdb_thread
-def _evaluate(expr, frame_id):
-    global_context = True
-    if frame_id is not None:
-        select_frame(frame_id)
-        global_context = False
-    val = gdb.parse_and_eval(expr, global_context=global_context)
-    ref = EvaluateResult(val)
-    return ref.to_object()
+def _evaluate(expr, frame_id, value_format):
+    with apply_format(value_format):
+        global_context = True
+        if frame_id is not None:
+            select_frame(frame_id)
+            global_context = False
+        val = gdb.parse_and_eval(expr, global_context=global_context)
+        ref = EvaluateResult(val)
+        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):
+def _eval_for_hover(expr, frame_id, value_format):
     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)
+                return _evaluate(expr, frame_id, value_format)
 
 
 class _SetResult(VariableReference):
@@ -58,15 +59,16 @@ class _SetResult(VariableReference):
 
 # Helper function to perform an assignment.
 @in_gdb_thread
-def _set_expression(expression, value, frame_id):
-    global_context = True
-    if frame_id is not None:
-        select_frame(frame_id)
-        global_context = False
-    lhs = gdb.parse_and_eval(expression, global_context=global_context)
-    rhs = gdb.parse_and_eval(value, global_context=global_context)
-    lhs.assign(rhs)
-    return _SetResult(lhs).to_object()
+def _set_expression(expression, value, frame_id, value_format):
+    with apply_format(value_format):
+        global_context = True
+        if frame_id is not None:
+            select_frame(frame_id)
+            global_context = False
+        lhs = gdb.parse_and_eval(expression, global_context=global_context)
+        rhs = gdb.parse_and_eval(value, global_context=global_context)
+        lhs.assign(rhs)
+        return _SetResult(lhs).to_object()
 
 
 # Helper function to evaluate a gdb command in a certain frame.
@@ -83,42 +85,50 @@ def _repl(command, frame_id):
 
 @request("evaluate")
 @capability("supportsEvaluateForHovers")
+@capability("supportsValueFormattingOptions")
 def eval_request(
     *,
     expression: str,
     frameId: Optional[int] = None,
     context: str = "variables",
+    format=None,
     **args,
 ):
     if context in ("watch", "variables"):
         # These seem to be expression-like.
-        return send_gdb_with_response(lambda: _evaluate(expression, frameId))
+        return send_gdb_with_response(lambda: _evaluate(expression, frameId, format))
     elif context == "hover":
-        return send_gdb_with_response(lambda: _eval_for_hover(expression, frameId))
+        return send_gdb_with_response(
+            lambda: _eval_for_hover(expression, frameId, format)
+        )
     elif context == "repl":
+        # Ignore the format for repl evaluation.
         return send_gdb_with_response(lambda: _repl(expression, frameId))
     else:
         raise Exception('unknown evaluate context "' + context + '"')
 
 
 @in_gdb_thread
-def _variables(ref, start, count):
-    var = find_variable(ref)
-    children = var.fetch_children(start, count)
-    return [x.to_object() for x in children]
+def _variables(ref, start, count, value_format):
+    with apply_format(value_format):
+        var = find_variable(ref)
+        children = var.fetch_children(start, count)
+        return [x.to_object() for x in children]
 
 
 @request("variables")
 # 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):
+def variables(
+    *, variablesReference: int, start: int = 0, count: int = 0, format=None, **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)
+        lambda: _variables(variablesReference, start, count, format)
     )
     return {"variables": result}
 
@@ -126,6 +136,8 @@ def variables(*, variablesReference: int, start: int = 0, count: int = 0, **args
 @capability("supportsSetExpression")
 @request("setExpression")
 def set_expression(
-    *, expression: str, value: str, frameId: Optional[int] = None, **args
+    *, expression: str, value: str, frameId: Optional[int] = None, format=None, **args
 ):
-    return send_gdb_with_response(lambda: _set_expression(expression, value, frameId))
+    return send_gdb_with_response(
+        lambda: _set_expression(expression, value, frameId, format)
+    )
index 0e0d92a883205b1686c16ef548eb43131f5115aa..c1e5ba8686d5b1644541856a09ca1fe45785f9ee 100644 (file)
@@ -17,6 +17,7 @@ import gdb
 from .startup import in_gdb_thread
 from .server import client_bool_capability
 from abc import abstractmethod
+from contextlib import contextmanager
 
 
 # A list of all the variable references created during this pause.
@@ -34,6 +35,23 @@ def clear_vars(event):
 gdb.events.cont.connect(clear_vars)
 
 
+# A null context manager.  Python supplies one, starting in 3.7.
+@contextmanager
+def _null(**ignore):
+    yield
+
+
+@in_gdb_thread
+def apply_format(value_format):
+    """Temporarily apply the DAP ValueFormat.
+
+    This returns a new context manager that applies the given DAP
+    ValueFormat object globally, then restores gdb's state when finished."""
+    if value_format is not None and "hex" in value_format and value_format["hex"]:
+        return gdb.with_parameter("output-radix", 16)
+    return _null()
+
+
 class BaseReference:
     """Represent a variable or a scope.
 
index 4caf105b347670205608b6eeb516eedb0b3541f9..5186c4e583174cfb465f1a21428b988580914a39 100644 (file)
@@ -48,6 +48,16 @@ set obj [dap_check_request_and_response "evaluate global" \
 dap_match_values "global value in function" [lindex $obj 0] \
     "body result" 23
 
+set obj [dap_check_request_and_response "evaluate global as hex" \
+            evaluate {o expression [s global_variable] format [o hex [l true]]}]
+dap_match_values "global value in function as hex" [lindex $obj 0] \
+    "body result" 0x17
+
+set obj [dap_check_request_and_response "evaluate global in hover mode" \
+            evaluate {o context [s hover] expression [s global_variable]}]
+dap_match_values "global value in hover mode" [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"} \