From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-il1-x130.google.com (mail-il1-x130.google.com [IPv6:2607:f8b0:4864:20::130]) by sourceware.org (Postfix) with ESMTPS id CC550385AF8F for ; Thu, 27 Jul 2023 19:20:36 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org CC550385AF8F Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=adacore.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=adacore.com Received: by mail-il1-x130.google.com with SMTP id e9e14a558f8ab-348c6696960so5069195ab.2 for ; Thu, 27 Jul 2023 12:20:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=adacore.com; s=google; t=1690485636; x=1691090436; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=IpBM/zEA3mwNLMCpkEgq/OUbEKobThUZ13CVVc+jQH0=; b=IHxj104AMy0jvxRPmveE52R2PkITNA9V2UP+vSChBcqwqVFvp4lfH0Ho0+ASLE5y7q tc6Bps8kqHkmZDSU3rZomAfzytvMphABDUZiwnM6Melk/Yin/OQ1IaKlzV+HWf9FZMeL ftEeKeE8XyIOsepwEEiZxq680XRxlZUGcL6tGlzsBrsvbWdP9c2dwuqvBkHHVPtW8R8M qPNMDdhu/jSCZcZjZgfvnEBYd2huxSI2f57gMI6b2GpA6h6wNE450ukiEHQFI5mS/CKY ZYooDeDGvmZQa14Wp7gQUvIuD4gwwvaQHi9ROy5KlDK3oRm8t2KmUzc1MKaVkh3MLlZF ZK+g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1690485636; x=1691090436; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=IpBM/zEA3mwNLMCpkEgq/OUbEKobThUZ13CVVc+jQH0=; b=EhZt+yqhy6tSKLu6K4kaTOtTz8oci+ue0OToAHKNghTPTsShrsnn4G6vv7oFJNfy4B DfP2vp/B70qiEFJhkwbgHVRdjCePmkMx2FZqQWGJPT4KcUYzoK8nzt06Y6wsyPmqBJv7 bFDWoVogTB+dsguta6JxbTjPmRpLPK7t7/bSe57tTpZAqKDhJT12nwrRSwuHyCyAsRr4 6cPSk63ddLZQFnxQi12p2dRdFLqTQJhwcCz7SmtAhHQhVgI7rZ1MZQsIWX6XEW0DyOhU CFTqGjMyCPSMmx1tgcTkpfx3d2qtdc+bx6r3ijMxipdXPQtNgTTZ6GQHQkM9Gl2GO30Y QJDg== X-Gm-Message-State: ABy/qLY8RZSRMXGjsCN9i14bTxpgGts9u/4iVeiIeI01PE+ibUK5X7F3 7XcVHI6wLLaPiyIM3kiot78LJx5j6+jQLXAHIXLCJA== X-Google-Smtp-Source: APBJJlE8dqXGoMReqFB3Iqwe4nWW4q6frPeLt2og4u3uF6E/UQDPheoWAax4DHDK/t4kd5TG7tNpkA== X-Received: by 2002:a05:6e02:1a05:b0:346:56c2:495c with SMTP id s5-20020a056e021a0500b0034656c2495cmr457120ild.5.1690485635830; Thu, 27 Jul 2023 12:20:35 -0700 (PDT) Received: from localhost.localdomain (75-166-135-140.hlrn.qwest.net. [75.166.135.140]) by smtp.gmail.com with ESMTPSA id p11-20020a92da4b000000b00345d00dc3fdsm679897ilq.78.2023.07.27.12.20.34 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 27 Jul 2023 12:20:35 -0700 (PDT) From: Tom Tromey To: gdb-patches@sourceware.org Cc: Tom Tromey Subject: [PATCH v2] Implement ValueFormat for DAP Date: Thu, 27 Jul 2023 13:20:24 -0600 Message-Id: <20230727192024.2472123-1-tromey@adacore.com> X-Mailer: git-send-email 2.40.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-11.6 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: 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 | 102 +++++++++++++++-------------- gdb/python/lib/gdb/dap/evaluate.py | 72 +++++++++++--------- gdb/python/lib/gdb/dap/varref.py | 18 +++++ gdb/testsuite/gdb.dap/hover.exp | 10 +++ 4 files changed, 124 insertions(+), 78 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..c9ad61f1ff3 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,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): @@ -65,15 +66,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 +92,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 +143,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..5186c4e5831 100644 --- a/gdb/testsuite/gdb.dap/hover.exp +++ b/gdb/testsuite/gdb.dap/hover.exp @@ -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"} \ -- 2.40.1