]> git.ipfire.org Git - thirdparty/binutils-gdb.git/commitdiff
Automatically run (most) DAP requests in gdb thread
authorTom Tromey <tromey@adacore.com>
Mon, 6 Nov 2023 20:31:17 +0000 (13:31 -0700)
committerTom Tromey <tromey@adacore.com>
Fri, 17 Nov 2023 15:26:02 +0000 (08:26 -0700)
Nearly every DAP request implementation forwards its work to the gdb
thread, using send_gdb_with_response.  This patch refactors the
'request' decorator to make this automatic, and to provide some
parameters so that the unusual requests can express their needs as
well.

In a few spots this simplifies the code by removing an unnecessary
helper function.  This could be done in more places as well if we
wanted.

The main motivation for this patch is that I thought it would be
helpful for cancellation.  I am still working on that, but meanwhile
the parameterization of 'request' makes it easy to handle the
'notStopped' response as well.

Reviewed-by: Kévin Le Gouguec <legouguec@adacore.com>
14 files changed:
gdb/python/lib/gdb/dap/breakpoint.py
gdb/python/lib/gdb/dap/bt.py
gdb/python/lib/gdb/dap/disassemble.py
gdb/python/lib/gdb/dap/evaluate.py
gdb/python/lib/gdb/dap/launch.py
gdb/python/lib/gdb/dap/locations.py
gdb/python/lib/gdb/dap/memory.py
gdb/python/lib/gdb/dap/modules.py
gdb/python/lib/gdb/dap/next.py
gdb/python/lib/gdb/dap/pause.py
gdb/python/lib/gdb/dap/scopes.py
gdb/python/lib/gdb/dap/server.py
gdb/python/lib/gdb/dap/sources.py
gdb/python/lib/gdb/dap/threads.py

index ae3a5027f35217cd86e5e0feef7403b794526e16..33aa18e65bcfc8f2c2da93e25b833f92984f0776 100644 (file)
@@ -24,7 +24,7 @@ from typing import Optional, Sequence
 
 from .server import request, capability, send_event
 from .sources import make_source
-from .startup import send_gdb_with_response, in_gdb_thread, log_stack
+from .startup import in_gdb_thread, log_stack
 from .typecheck import type_check
 
 
@@ -287,7 +287,7 @@ def set_breakpoint(*, source, breakpoints: Sequence = (), **args):
         # 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"]
-        result = send_gdb_with_response(lambda: _set_breakpoints(key, specs))
+        result = _set_breakpoints(key, specs)
     return {
         "breakpoints": result,
     }
@@ -315,9 +315,8 @@ def _rewrite_fn_breakpoint(
 @capability("supportsFunctionBreakpoints")
 def set_fn_breakpoint(*, breakpoints: Sequence, **args):
     specs = [_rewrite_fn_breakpoint(**bp) for bp in breakpoints]
-    result = send_gdb_with_response(lambda: _set_breakpoints("function", specs))
     return {
-        "breakpoints": result,
+        "breakpoints": _set_breakpoints("function", specs),
     }
 
 
@@ -351,9 +350,8 @@ def set_insn_breakpoints(
     *, breakpoints: Sequence, offset: Optional[int] = None, **args
 ):
     specs = [_rewrite_insn_breakpoint(**bp) for bp in breakpoints]
-    result = send_gdb_with_response(lambda: _set_breakpoints("instruction", specs))
     return {
-        "breakpoints": result,
+        "breakpoints": _set_breakpoints("instruction", specs),
     }
 
 
@@ -432,7 +430,6 @@ def set_exception_breakpoints(
     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,
+        "breakpoints": _set_exception_catchpoints(options),
     }
index c63ca6d13b504ca3fce82f895c160adcb2fd3d66..a770b2548371b41af06008bfcb4d0fd6552ab8c3 100644 (file)
@@ -24,7 +24,7 @@ from .modules import module_id
 from .scopes import symbol_value
 from .server import request, capability
 from .sources import make_source
-from .startup import send_gdb_with_response, in_gdb_thread
+from .startup import in_gdb_thread
 from .state import set_thread
 from .typecheck import type_check
 from .varref import apply_format
@@ -147,6 +147,4 @@ def stacktrace(
     if format is None:
         format = {}
     format = check_stack_frame(**format)
-    return send_gdb_with_response(
-        lambda: _backtrace(threadId, levels, startFrame, format)
-    )
+    return _backtrace(threadId, levels, startFrame, format)
index dda2f43b5a3cb86cf9c213413de44452df66aa92..069549eb7f86bad65930ad82716b798f58684a15 100644 (file)
@@ -16,7 +16,7 @@
 import gdb
 
 from .server import request, capability
-from .startup import send_gdb_with_response, in_gdb_thread
+from .startup import in_gdb_thread
 
 
 @in_gdb_thread
@@ -54,6 +54,4 @@ def disassemble(
     **extra
 ):
     pc = int(memoryReference, 0) + offset
-    return send_gdb_with_response(
-        lambda: _disassemble(pc, instructionOffset, instructionCount)
-    )
+    return _disassemble(pc, instructionOffset, instructionCount)
index ea5a1e61a08a82ff6bfb4a134c090673ccc3386a..67e103e2ca74bf859b37aa2cb358994f06d5ba74 100644 (file)
@@ -20,7 +20,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 .startup import in_gdb_thread
 from .varref import find_variable, VariableReference, apply_format
 
 
@@ -96,14 +96,12 @@ def eval_request(
 ):
     if context in ("watch", "variables"):
         # These seem to be expression-like.
-        return send_gdb_with_response(lambda: _evaluate(expression, frameId, format))
+        return _evaluate(expression, frameId, format)
     elif context == "hover":
-        return send_gdb_with_response(
-            lambda: _eval_for_hover(expression, frameId, format)
-        )
+        return _eval_for_hover(expression, frameId, format)
     elif context == "repl":
         # Ignore the format for repl evaluation.
-        return send_gdb_with_response(lambda: _repl(expression, frameId))
+        return _repl(expression, frameId)
     else:
         raise Exception('unknown evaluate context "' + context + '"')
 
@@ -127,10 +125,7 @@ def variables(
     if not client_bool_capability("supportsVariablePaging"):
         start = 0
         count = 0
-    result = send_gdb_with_response(
-        lambda: _variables(variablesReference, start, count, format)
-    )
-    return {"variables": result}
+    return {"variables": _variables(variablesReference, start, count, format)}
 
 
 @capability("supportsSetExpression")
@@ -138,9 +133,7 @@ def variables(
 def set_expression(
     *, expression: str, value: str, frameId: Optional[int] = None, format=None, **args
 ):
-    return send_gdb_with_response(
-        lambda: _set_expression(expression, value, frameId, format)
-    )
+    return _set_expression(expression, value, frameId, format)
 
 
 # Helper function to perform an assignment.
@@ -159,6 +152,4 @@ def _set_variable(ref, name, value, value_format):
 def set_variable(
     *, variablesReference: int, name: str, value: str, format=None, **args
 ):
-    return send_gdb_with_response(
-        lambda: _set_variable(variablesReference, name, value, format)
-    )
+    return _set_variable(variablesReference, name, value, format)
index d13037fa476fd19f633d5df9a289528aca0a92b2..e81d2849a8ef70db0fcbb109d3e61c495a989057 100644 (file)
@@ -20,11 +20,11 @@ from typing import Mapping, Optional, Sequence
 
 from .events import ExecutionInvoker
 from .server import request, capability
-from .startup import send_gdb, send_gdb_with_response, in_gdb_thread, exec_and_log
+from .startup import in_gdb_thread, exec_and_log
 
 
-# The program being launched, or None.  This should only be access
-# from the DAP thread.
+# The program being launched, or None.  This should only be accessed
+# from the gdb thread.
 _program = None
 
 
@@ -49,7 +49,7 @@ def _launch_setup(program, cwd, args, env, stopAtBeginningOfMainSubprogram):
 # Any parameters here are necessarily extensions -- DAP requires this
 # from implementations.  Any additions or changes here should be
 # documented in the gdb manual.
-@request("launch")
+@request("launch", response=False)
 def launch(
     *,
     program: Optional[str] = None,
@@ -61,9 +61,7 @@ def launch(
 ):
     global _program
     _program = program
-    send_gdb(
-        lambda: _launch_setup(program, cwd, args, env, stopAtBeginningOfMainSubprogram)
-    )
+    _launch_setup(program, cwd, args, env, stopAtBeginningOfMainSubprogram)
 
 
 @request("attach")
@@ -77,17 +75,14 @@ def attach(*, pid: Optional[int] = None, target: Optional[str] = None, **args):
         cmd = "target remote " + target
     else:
         raise Exception("attach requires either 'pid' or 'target'")
-    # Use send_gdb_with_response to ensure we get an error if the
-    # attach fails.
-    send_gdb_with_response(cmd)
-    return None
+    exec_and_log(cmd)
 
 
 @capability("supportsConfigurationDoneRequest")
-@request("configurationDone")
+@request("configurationDone", response=False)
 def config_done(**args):
     global _program
     if _program is not None:
         # Suppress the continue event, but don't set any particular
         # expected stop.
-        send_gdb(ExecutionInvoker("run", None))
+        ExecutionInvoker("run", None)()
index a299e8da9594f612b316dc2729f2dafb4e015529..032174df9c8828f024808d63d72fdae0f9097852 100644 (file)
@@ -20,7 +20,7 @@ from typing import Optional
 
 from .server import capability, request
 from .sources import decode_source
-from .startup import in_gdb_thread, send_gdb_with_response
+from .startup import in_gdb_thread
 
 
 @in_gdb_thread
@@ -46,4 +46,4 @@ def _find_lines(source, start_line, end_line):
 def breakpoint_locations(*, source, line: int, endLine: Optional[int] = None, **extra):
     if endLine is None:
         endLine = line
-    return send_gdb_with_response(lambda: _find_lines(source, line, endLine))
+    return _find_lines(source, line, endLine)
index 85948bda9f4759b7fb85b832fd77f93fa872d77c..6b94f4130450099ab10f9cc937e8d3bafecdbf40 100644 (file)
@@ -17,35 +17,22 @@ import base64
 import gdb
 
 from .server import request, capability
-from .startup import send_gdb_with_response, in_gdb_thread
-
-
-@in_gdb_thread
-def _read_memory(addr, count):
-    buf = gdb.selected_inferior().read_memory(addr, count)
-    return base64.b64encode(buf).decode("ASCII")
 
 
 @request("readMemory")
 @capability("supportsReadMemoryRequest")
 def read_memory(*, memoryReference: str, offset: int = 0, count: int, **extra):
     addr = int(memoryReference, 0) + offset
-    buf = send_gdb_with_response(lambda: _read_memory(addr, count))
+    buf = gdb.selected_inferior().read_memory(addr, count)
     return {
         "address": hex(addr),
-        "data": buf,
+        "data": base64.b64encode(buf).decode("ASCII"),
     }
 
 
-@in_gdb_thread
-def _write_memory(addr, contents):
-    buf = base64.b64decode(contents)
-    gdb.selected_inferior().write_memory(addr, buf)
-
-
 @request("writeMemory")
 @capability("supportsWriteMemoryRequest")
 def write_memory(*, memoryReference: str, offset: int = 0, data: str, **extra):
     addr = int(memoryReference, 0) + offset
-    send_gdb_with_response(lambda: _write_memory(addr, data))
-    return {}
+    buf = base64.b64decode(data)
+    gdb.selected_inferior().write_memory(addr, buf)
index 1aec1cba0acfafab96da9b0305efaf889e76477c..87a4f6be6693fca547d8106847b1eaa053613a05 100644 (file)
@@ -16,7 +16,7 @@
 import gdb
 
 from .server import capability, request
-from .startup import in_gdb_thread, send_gdb_with_response
+from .startup import in_gdb_thread
 
 
 @in_gdb_thread
@@ -63,4 +63,4 @@ def _modules(start, count):
 @capability("supportsModulesRequest")
 @request("modules")
 def modules(*, startModule: int = 0, moduleCount: int = 0, **args):
-    return send_gdb_with_response(lambda: _modules(startModule, moduleCount))
+    return _modules(startModule, moduleCount)
index e5bb8d64da0953e259c3335ab57bf6ee76d88726..431020e32e12028a0dcee1b6485c1a3700cff89f 100644 (file)
@@ -49,37 +49,42 @@ def _handle_thread_step(thread_id, single_thread, select=False):
     return result
 
 
-@request("next")
+@request("next", response=False)
 def next(
     *, threadId: int, singleThread: bool = False, granularity: str = "statement", **args
 ):
-    send_gdb(lambda: _handle_thread_step(threadId, singleThread))
+    _handle_thread_step(threadId, singleThread)
     cmd = "next"
     if granularity == "instruction":
         cmd += "i"
-    send_gdb(ExecutionInvoker(cmd, StopKinds.STEP))
+    ExecutionInvoker(cmd, StopKinds.STEP)()
 
 
 @capability("supportsSteppingGranularity")
 @capability("supportsSingleThreadExecutionRequests")
-@request("stepIn")
+@request("stepIn", response=False)
 def step_in(
     *, threadId: int, singleThread: bool = False, granularity: str = "statement", **args
 ):
-    send_gdb(lambda: _handle_thread_step(threadId, singleThread))
+    _handle_thread_step(threadId, singleThread)
     cmd = "step"
     if granularity == "instruction":
         cmd += "i"
-    send_gdb(ExecutionInvoker(cmd, StopKinds.STEP))
+    ExecutionInvoker(cmd, StopKinds.STEP)()
 
 
-@request("stepOut")
+@request("stepOut", response=False)
 def step_out(*, threadId: int, singleThread: bool = False, **args):
-    send_gdb(lambda: _handle_thread_step(threadId, singleThread, True))
-    send_gdb(ExecutionInvoker("finish", StopKinds.STEP))
+    _handle_thread_step(threadId, singleThread, True)
+    ExecutionInvoker("finish", StopKinds.STEP)()
 
 
-@request("continue")
+# This is a server-side request because it is funny: it wants to
+# 'continue' but also return a result, which precludes using
+# response=False.  Using 'continue &' would mostly work ok, but this
+# yields races when a stop occurs before the response is sent back to
+# the client.
+@request("continue", on_dap_thread=True)
 def continue_request(*, threadId: int, singleThread: bool = False, **args):
     locked = send_gdb_with_response(lambda: _handle_thread_step(threadId, singleThread))
     send_gdb(ExecutionInvoker("continue", None))
index 1e59d6305235488c88db6fc8c38fc87c6ec06e76..d96172c0757f5e522b29a05bdb2bf2ca2e7133b6 100644 (file)
@@ -15,9 +15,8 @@
 
 from .events import StopKinds, ExecutionInvoker
 from .server import request
-from .startup import send_gdb
 
 
-@request("pause")
+@request("pause", response=False)
 def pause(**args):
-    send_gdb(ExecutionInvoker("interrupt -a", StopKinds.PAUSE))
+    ExecutionInvoker("interrupt -a", StopKinds.PAUSE)()
index e526dfb518a8e83af2c4b038dedd26b5a44d708a..63cd3255b8a2f2038cfe5b5ed80c08ace2a6e5f8 100644 (file)
@@ -16,7 +16,7 @@
 import gdb
 
 from .frames import frame_for_id
-from .startup import send_gdb_with_response, in_gdb_thread
+from .startup import in_gdb_thread
 from .server import request
 from .varref import BaseReference
 
@@ -133,5 +133,4 @@ def _get_scope(id):
 
 @request("scopes")
 def scopes(*, frameId: int, **extra):
-    scopes = send_gdb_with_response(lambda: _get_scope(frameId))
-    return {"scopes": scopes}
+    return {"scopes": _get_scope(frameId)}
index 62bf240c1e925a8dba4b6b3a1d87610d9fa1c0f2..4430d2aba5fc88e182cd675fe42d0ea41b1ce852 100644 (file)
@@ -20,11 +20,14 @@ import sys
 
 from .io import start_json_writer, read_json
 from .startup import (
+    exec_and_log,
     in_dap_thread,
+    in_gdb_thread,
+    send_gdb,
+    send_gdb_with_response,
     start_thread,
     log,
     log_stack,
-    send_gdb_with_response,
 )
 from .typecheck import type_check
 
@@ -160,12 +163,27 @@ def send_event(event, body=None):
     _server.send_event(event, body)
 
 
-def request(name):
-    """A decorator that indicates that the wrapper function implements
-    the DAP request NAME."""
+def request(name: str, *, response: bool = True, on_dap_thread: bool = False):
+    """A decorator for DAP requests.
+
+    This registers the function as the implementation of the DAP
+    request NAME.  By default, the function is invoked in the gdb
+    thread, and its result is returned as the 'body' of the DAP
+    response.
+
+    Some keyword arguments are provided as well:
+
+    If RESPONSE is False, the result of the function will not be
+    waited for and no 'body' will be in the response.
+
+    If ON_DAP_THREAD is True, the function will be invoked in the DAP
+    thread.  When ON_DAP_THREAD is True, RESPONSE may not be False.
+    """
+
+    # Validate the parameters.
+    assert not on_dap_thread or response
 
     def wrap(func):
-        global _commands
         code = func.__code__
         # We don't permit requests to have positional arguments.
         try:
@@ -176,11 +194,32 @@ def request(name):
         assert code.co_argcount == 0
         # A request must have a **args parameter.
         assert code.co_flags & inspect.CO_VARKEYWORDS
-        # All requests must run in the DAP thread.
-        # Also type-check the calls.
-        func = in_dap_thread(type_check(func))
-        _commands[name] = func
-        return func
+
+        # Type-check the calls.
+        func = type_check(func)
+
+        # Verify that the function is run on the correct thread.
+        if on_dap_thread:
+            cmd = in_dap_thread(func)
+        else:
+            func = in_gdb_thread(func)
+
+            if response:
+
+                def sync_call(**args):
+                    return send_gdb_with_response(lambda: func(**args))
+
+                cmd = sync_call
+            else:
+
+                def non_sync_call(**args):
+                    return send_gdb(lambda: func(**args))
+
+                cmd = non_sync_call
+
+        global _commands
+        _commands[name] = cmd
+        return cmd
 
     return wrap
 
@@ -208,7 +247,7 @@ def client_bool_capability(name):
     return False
 
 
-@request("initialize")
+@request("initialize", on_dap_thread=True)
 def initialize(**args):
     global _server, _capabilities
     _server.config = args
@@ -219,14 +258,12 @@ def initialize(**args):
 @request("terminate")
 @capability("supportsTerminateRequest")
 def terminate(**args):
-    # We can ignore the result here, because we only really need to
-    # synchronize.
-    send_gdb_with_response("kill")
+    exec_and_log("kill")
 
 
-@request("disconnect")
+@request("disconnect", on_dap_thread=True)
 @capability("supportTerminateDebuggee")
 def disconnect(*, terminateDebuggee: bool = False, **args):
     if terminateDebuggee:
-        terminate()
+        send_gdb_with_response("kill")
     _server.shutdown()
index 00a70701d26932d5ec6c65beebd9e4169b6c8d4c..821205cedd1d50bab29c96e93c42f1ca02c9a377 100644 (file)
@@ -18,7 +18,7 @@ import os
 import gdb
 
 from .server import request, capability
-from .startup import send_gdb_with_response, in_gdb_thread
+from .startup import in_gdb_thread
 
 
 # The next available source reference ID.  Must be greater than 0.
@@ -76,8 +76,9 @@ def decode_source(source):
     return _id_map[ref]["path"]
 
 
-@in_gdb_thread
-def _sources():
+@request("loadedSources")
+@capability("supportsLoadedSourcesRequest")
+def loaded_sources(**extra):
     result = []
     for elt in gdb.execute_mi("-file-list-exec-source-files")["files"]:
         result.append(make_source(elt["fullname"], elt["file"]))
@@ -86,24 +87,6 @@ def _sources():
     }
 
 
-@request("loadedSources")
-@capability("supportsLoadedSourcesRequest")
-def loaded_sources(**extra):
-    return send_gdb_with_response(_sources)
-
-
-# This helper is needed because we must only access the globals here
-# from the gdb thread.
-@in_gdb_thread
-def _get_source(source):
-    filename = decode_source(source)
-    with open(filename) as f:
-        content = f.read()
-    return {
-        "content": content,
-    }
-
-
 @request("source")
 def source(*, source=None, sourceReference: int, **extra):
     # The 'sourceReference' parameter is required by the spec, but is
@@ -111,4 +94,9 @@ def source(*, source=None, sourceReference: int, **extra):
     # 'source' is preferred.
     if source is None:
         source = {"sourceReference": sourceReference}
-    return send_gdb_with_response(lambda: _get_source(source))
+    filename = decode_source(source)
+    with open(filename) as f:
+        content = f.read()
+    return {
+        "content": content,
+    }
index a4ca9e61488ea01cd7b6ec95970c75960eacd46b..515966761ce89316560154af61d3a1abde791f46 100644 (file)
@@ -16,7 +16,6 @@
 import gdb
 
 from .server import request
-from .startup import send_gdb_with_response, in_gdb_thread
 
 
 def _thread_name(thr):
@@ -27,9 +26,8 @@ def _thread_name(thr):
     return None
 
 
-# A helper function to construct the list of threads.
-@in_gdb_thread
-def _get_threads():
+@request("threads")
+def threads(**args):
     result = []
     for thr in gdb.selected_inferior().threads():
         one_result = {
@@ -39,12 +37,6 @@ def _get_threads():
         if name is not None:
             one_result["name"] = name
         result.append(one_result)
-    return result
-
-
-@request("threads")
-def threads(**args):
-    result = send_gdb_with_response(_get_threads)
     return {
         "threads": result,
     }