[PATCH] Implement ValueFormat for DAP

[PATCH] Implement ValueFormat for DAP

[Tom Tromey]

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.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30469
---
 gdb/python/lib/gdb/dap/bt.py       | 102 +++++++++++++++--------------
 gdb/python/lib/gdb/dap/evaluate.py |  79 ++++++++++++----------
 gdb/python/lib/gdb/dap/varref.py   |  18 +++++
 gdb/testsuite/gdb.dap/hover.exp    |   5 ++
 4 files changed, 123 insertions(+), 81 deletions(-)

diff --git a/gdb/python/lib/gdb/dap/bt.py b/gdb/python/lib/gdb/dap/bt.py
index 975c88f8208..4d1d89a81c3 100644
--- a/gdb/python/lib/gdb/dap/bt.py
+++ b/gdb/python/lib/gdb/dap/bt.py
@@ -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)
+    )
diff --git a/gdb/python/lib/gdb/dap/evaluate.py b/gdb/python/lib/gdb/dap/evaluate.py
index 63e80331b24..6c920577f32 100644
--- a/gdb/python/lib/gdb/dap/evaluate.py
+++ b/gdb/python/lib/gdb/dap/evaluate.py
@@ -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,26 @@ 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):
-    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)
+def _eval_for_hover(expr, frame_id, value_format):
+    with apply_format(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)
 
 
 class _SetResult(VariableReference):
@@ -65,15 +67,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.
@@ -90,42 +93,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}
 
@@ -133,6 +144,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)
+    )
diff --git a/gdb/python/lib/gdb/dap/varref.py b/gdb/python/lib/gdb/dap/varref.py
index 213151fd3d3..9465198d01c 100644
--- a/gdb/python/lib/gdb/dap/varref.py
+++ b/gdb/python/lib/gdb/dap/varref.py
@@ -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.
 
diff --git a/gdb/testsuite/gdb.dap/hover.exp b/gdb/testsuite/gdb.dap/hover.exp
index 4caf105b347..1600984981c 100644
--- a/gdb/testsuite/gdb.dap/hover.exp
+++ b/gdb/testsuite/gdb.dap/hover.exp
@@ -48,6 +48,11 @@ 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_request_and_response \
 	     evaluate {o context [s hover] expression [s increment()]}]
 gdb_assert {[dict get [lindex $obj 0] success] == "false"} \
-- 
2.40.1