Author: John Harrison Date: 2025-12-22T14:52:46-08:00 New Revision: 3f08fafbee7170c6d6e1f5dc73a943ebde0658db
URL: https://github.com/llvm/llvm-project/commit/3f08fafbee7170c6d6e1f5dc73a943ebde0658db DIFF: https://github.com/llvm/llvm-project/commit/3f08fafbee7170c6d6e1f5dc73a943ebde0658db.diff LOG: [lldb-dap] Adjusting the initialize/launch flow to better match the spec. (#171549) In https://github.com/llvm/llvm-project/pull/170523 it was pointed out that the spec does specifically specify that launch/attach should not respond until configurationDone is handled. This means we do need to support async request handlers. To better align with the spec, I've added a new `lldb_dap::AsyncRequestHandler`. This is an additional handler type that allows us to respond at a later point. Additionally, I refactored `launch` and `attach` to only respond once the `configurationDone` is complete, specifically during the `PostRun` operation of the `configurationDone` handler. I merged some of the common behavior between `RequestHandler` and `AsyncRequestHandler` into their common `BaseRequestHandler`. The flow should now be: ``` <-> initialize request / response --> launch/attach request <-- event initialized ... optionally ... <-> setBreakpoints request / response <-> setFunctionBreakpoints request / response <-> setExceptionBreakpoints request / response <-> setInstructionBreakpoints request / response ... finally ... <-> configurationDone request / response <-- launch/attach response ``` --------- Co-authored-by: Jonas Devlieghere <[email protected]> Added: Modified: lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py lldb/test/API/tools/lldb-dap/attach-commands/TestDAP_attachCommands.py lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py lldb/test/API/tools/lldb-dap/attach/TestDAP_attachByPortNum.py lldb/test/API/tools/lldb-dap/breakpoint-assembly/TestDAP_breakpointAssembly.py lldb/test/API/tools/lldb-dap/commands/TestDAP_commands.py lldb/test/API/tools/lldb-dap/coreFile/TestDAP_coreFile.py lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py lldb/tools/lldb-dap/DAP.h lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp lldb/tools/lldb-dap/Handler/ConfigurationDoneRequestHandler.cpp lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp lldb/tools/lldb-dap/Handler/RequestHandler.cpp lldb/tools/lldb-dap/Handler/RequestHandler.h Removed: ################################################################################ diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py index afa3dbde6dc5f..992e75244b67a 100644 --- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py +++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py @@ -54,7 +54,7 @@ class Event(TypedDict): body: Any -class Request(TypedDict, total=False): +class Request(TypedDict): type: Literal["request"] seq: int command: str @@ -533,6 +533,7 @@ def _handle_reverse_request(self, request: Request) -> None: "seq": 0, "request_seq": request["seq"], "success": True, + "message": None, "command": "runInTerminal", "body": body, } @@ -558,7 +559,7 @@ def _process_continued(self, all_threads_continued: bool): if all_threads_continued: self.thread_stop_reasons = {} - def _update_verified_breakpoints(self, breakpoints: list[Breakpoint]): + def _update_verified_breakpoints(self, breakpoints: List[Breakpoint]): for bp in breakpoints: # If no id is set, we cannot correlate the given breakpoint across # requests, ignore it. @@ -592,7 +593,7 @@ def send_packet(self, packet: ProtocolMessage) -> int: return packet["seq"] - def _send_recv(self, request: Request) -> Optional[Response]: + def _send_recv(self, request: Request) -> Response: """Send a command python dictionary as JSON and receive the JSON response. Validates that the response is the correct sequence and command in the reply. Any events that are received are added to the @@ -653,7 +654,7 @@ def wait_for_breakpoint_events(self): breakpoint_events.append(event) return breakpoint_events - def wait_for_breakpoints_to_be_verified(self, breakpoint_ids: list[str]): + def wait_for_breakpoints_to_be_verified(self, breakpoint_ids: List[str]): """Wait for all breakpoints to be verified. Return all unverified breakpoints.""" while any(id not in self.resolved_breakpoints for id in breakpoint_ids): breakpoint_event = self.wait_for_event(["breakpoint"]) @@ -879,7 +880,7 @@ def request_attach( sourceMap: Optional[Union[list[tuple[str, str]], dict[str, str]]] = None, gdbRemotePort: Optional[int] = None, gdbRemoteHostname: Optional[str] = None, - ): + ) -> int: args_dict = {} if pid is not None: args_dict["pid"] = pid @@ -917,7 +918,7 @@ def request_attach( if gdbRemoteHostname is not None: args_dict["gdb-remote-hostname"] = gdbRemoteHostname command_dict = {"command": "attach", "type": "request", "arguments": args_dict} - return self._send_recv(command_dict) + return self.send_packet(command_dict) def request_breakpointLocations( self, file_path, line, end_line=None, column=None, end_column=None @@ -948,10 +949,10 @@ def request_configurationDone(self): "arguments": {}, } response = self._send_recv(command_dict) - if response: + if response and response["success"]: self.configuration_done_sent = True stopped_on_entry = self.is_stopped - self.request_threads() + threads_response = self.request_threads() if not stopped_on_entry: # Drop the initial cached threads if we did not stop-on-entry. # In VSCode, immediately following 'configurationDone', a @@ -1079,10 +1080,10 @@ def request_evaluate( threadId=None, context=None, is_hex: Optional[bool] = None, - ): + ) -> Response: stackFrame = self.get_stackFrame(frameIndex=frameIndex, threadId=threadId) if stackFrame is None: - return [] + raise ValueError("invalid frameIndex") args_dict = { "expression": expression, "frameId": stackFrame["id"], @@ -1165,7 +1166,7 @@ def request_launch( commandEscapePrefix: Optional[str] = None, customFrameFormat: Optional[str] = None, customThreadFormat: Optional[str] = None, - ): + ) -> int: args_dict = {"program": program} if args: args_dict["args"] = args @@ -1216,7 +1217,7 @@ def request_launch( if commandEscapePrefix is not None: args_dict["commandEscapePrefix"] = commandEscapePrefix command_dict = {"command": "launch", "type": "request", "arguments": args_dict} - return self._send_recv(command_dict) + return self.send_packet(command_dict) def request_next(self, threadId, granularity="statement"): if self.exit_status is not None: @@ -1276,6 +1277,7 @@ def request_setBreakpoints(self, source: Source, line_array, data=None): Each parameter object is 1:1 mapping with entries in line_entry. It contains optional location/hitCondition/logMessage parameters. """ + assert self.initialized, "cannot setBreakpoints before initialized" args_dict = { "source": source, "sourceModified": False, @@ -1313,6 +1315,7 @@ def request_setBreakpoints(self, source: Source, line_array, data=None): def request_setExceptionBreakpoints( self, *, filters: list[str] = [], filter_options: list[dict] = [] ): + assert self.initialized, "cannot setExceptionBreakpoints before initialized" args_dict = {"filters": filters} if filter_options: args_dict["filterOptions"] = filter_options @@ -1324,6 +1327,7 @@ def request_setExceptionBreakpoints( return self._send_recv(command_dict) def request_setFunctionBreakpoints(self, names, condition=None, hitCondition=None): + assert self.initialized, "cannot setFunctionBreakpoints before initialized" breakpoints = [] for name in names: bp = {"name": name} @@ -1372,6 +1376,7 @@ def request_setDataBreakpoint(self, dataBreakpoints): [hitCondition]: string } """ + assert self.initialized, "cannot setDataBreakpoints before initialized" args_dict = {"breakpoints": dataBreakpoints} command_dict = { "command": "setDataBreakpoints", diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py index 73cc7fbc32715..ac10d6d593805 100644 --- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py +++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py @@ -7,7 +7,7 @@ import uuid import dap_server -from dap_server import Source +from dap_server import Source, Response from lldbsuite.test.decorators import skipIf from lldbsuite.test.lldbtest import * from lldbsuite.test import lldbplatformutil @@ -500,9 +500,9 @@ def attach( *, disconnectAutomatically=True, sourceInitFile=False, - expectFailure=False, + waitForResponse=False, **kwargs, - ): + ) -> Optional[Response]: """Build the default Makefile target, create the DAP debug adapter, and attach to the process. """ @@ -518,20 +518,20 @@ def cleanup(): self.addTearDownHook(cleanup) # Initialize and launch the program self.dap_server.request_initialize(sourceInitFile) - response = self.dap_server.request_attach(**kwargs) - if expectFailure: - return response - if not (response and response["success"]): - error_msg = self._build_error_message("attach failed", response) - self.assertTrue(response and response["success"], error_msg) + attach_seq = self.dap_server.request_attach(**kwargs) + self.dap_server.wait_for_event(["initialized"]) + if waitForResponse: + self.dap_server.request_configurationDone() + return self.dap_server.receive_response(attach_seq) + return None def launch( self, - program=None, + program: str, *, sourceInitFile=False, disconnectAutomatically=True, - expectFailure=False, + waitForResponse=False, **kwargs, ): """Sending launch request to dap""" @@ -548,12 +548,12 @@ def cleanup(): # Initialize and launch the program self.dap_server.request_initialize(sourceInitFile) - response = self.dap_server.request_launch(program, **kwargs) - if expectFailure: - return response - if not (response and response["success"]): - error_msg = self._build_error_message("launch failed", response) - self.assertTrue(response and response["success"], error_msg) + launch_seq = self.dap_server.request_launch(program, **kwargs) + self.dap_server.wait_for_event(["initialized"]) + if waitForResponse: + self.dap_server.request_configurationDone() + return self.dap_server.receive_response(launch_seq) + return launch_seq def build_and_launch( self, @@ -570,6 +570,13 @@ def build_and_launch( return self.launch(program, **kwargs) + def verify_configuration_done(self, expectedResult=True): + resp = self.dap_server.request_configurationDone() + if expectedResult: + self.assertTrue(resp["success"]) + else: + self.assertFalse(resp["success"]) + def getBuiltinDebugServerTool(self): # Tries to find simulation/lldb-server/gdbserver tool path. server_tool = None diff --git a/lldb/test/API/tools/lldb-dap/attach-commands/TestDAP_attachCommands.py b/lldb/test/API/tools/lldb-dap/attach-commands/TestDAP_attachCommands.py index 9e29f07db80f1..b4daf9fd171c2 100644 --- a/lldb/test/API/tools/lldb-dap/attach-commands/TestDAP_attachCommands.py +++ b/lldb/test/API/tools/lldb-dap/attach-commands/TestDAP_attachCommands.py @@ -105,7 +105,7 @@ def test_attach_command_process_failures(self): resp = self.attach( program=program, attachCommands=attachCommands, - expectFailure=True, + waitForResponse=True, ) self.assertFalse(resp["success"]) self.assertIn( diff --git a/lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py b/lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py index ce1b965d3cd0d..c7174ce879606 100644 --- a/lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py +++ b/lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py @@ -85,7 +85,7 @@ def test_attach_with_missing_debuggerId_or_targetId(self): self.build_and_create_debug_adapter() # Test with only targetId specified (no debuggerId) - resp = self.attach(targetId=99999, expectFailure=True) + resp = self.attach(targetId=99999, waitForResponse=True) self.assertFalse(resp["success"]) self.assertIn( "Both 'debuggerId' and 'targetId' must be specified together", @@ -102,7 +102,7 @@ def test_attach_with_invalid_debuggerId_and_targetId(self): # Attach with both debuggerId=9999 and targetId=99999 (both invalid). # Since debugger ID 9999 likely doesn't exist in the global registry, # we expect a validation error. - resp = self.attach(debuggerId=9999, targetId=99999, expectFailure=True) + resp = self.attach(debuggerId=9999, targetId=99999, waitForResponse=True) self.assertFalse(resp["success"]) error_msg = resp["body"]["error"]["format"] # Either error is acceptable - both indicate the debugger reuse diff --git a/lldb/test/API/tools/lldb-dap/attach/TestDAP_attachByPortNum.py b/lldb/test/API/tools/lldb-dap/attach/TestDAP_attachByPortNum.py index edb87a9314d78..e8f3f0fbc3fd3 100644 --- a/lldb/test/API/tools/lldb-dap/attach/TestDAP_attachByPortNum.py +++ b/lldb/test/API/tools/lldb-dap/attach/TestDAP_attachByPortNum.py @@ -99,7 +99,7 @@ def test_fails_if_both_port_and_pid_are_set(self): pid=pid, gdbRemotePort=port, sourceInitFile=True, - expectFailure=True, + waitForResponse=True, ) self.assertFalse( response["success"], "The user can't specify both pid and port" @@ -115,7 +115,10 @@ def test_by_invalid_port(self): port = 0 response = self.attach( - program=program, gdbRemotePort=port, sourceInitFile=True, expectFailure=True + program=program, + gdbRemotePort=port, + sourceInitFile=True, + waitForResponse=True, ) self.assertFalse( response["success"], @@ -138,7 +141,10 @@ def test_by_illegal_port(self): ) response = self.attach( - program=program, gdbRemotePort=port, sourceInitFile=True, expectFailure=True + program=program, + gdbRemotePort=port, + sourceInitFile=True, + waitForResponse=True, ) self.assertFalse( response["success"], diff --git a/lldb/test/API/tools/lldb-dap/breakpoint-assembly/TestDAP_breakpointAssembly.py b/lldb/test/API/tools/lldb-dap/breakpoint-assembly/TestDAP_breakpointAssembly.py index fab109c93a17b..cbbea9ea9540b 100644 --- a/lldb/test/API/tools/lldb-dap/breakpoint-assembly/TestDAP_breakpointAssembly.py +++ b/lldb/test/API/tools/lldb-dap/breakpoint-assembly/TestDAP_breakpointAssembly.py @@ -94,9 +94,10 @@ def test_persistent_assembly_breakpoint(self): try: self.dap_server.request_initialize() self.dap_server.request_launch(program) + self.dap_server.wait_for_event(["initialized"]) - assmebly_func_breakpoints = self.set_function_breakpoints(["assembly_func"]) - self.continue_to_breakpoints(assmebly_func_breakpoints) + assembly_func_breakpoints = self.set_function_breakpoints(["assembly_func"]) + self.continue_to_breakpoints(assembly_func_breakpoints) assembly_func_frame = self.get_stackFrames()[0] source_reference = assembly_func_frame["source"]["sourceReference"] @@ -137,6 +138,8 @@ def test_persistent_assembly_breakpoint(self): try: self.dap_server.request_initialize() self.dap_server.request_launch(program) + self.dap_server.wait_for_event(["initialized"]) + new_session_breakpoints_ids = self.set_source_breakpoints_from_source( Source(persistent_breakpoint_source), [persistent_breakpoint_line], diff --git a/lldb/test/API/tools/lldb-dap/commands/TestDAP_commands.py b/lldb/test/API/tools/lldb-dap/commands/TestDAP_commands.py index f53813a8a48f6..bde53f53ac318 100644 --- a/lldb/test/API/tools/lldb-dap/commands/TestDAP_commands.py +++ b/lldb/test/API/tools/lldb-dap/commands/TestDAP_commands.py @@ -47,7 +47,7 @@ def do_test_abort_on_error( launchCommands=commands if use_launch_commands else None, preRunCommands=commands if use_pre_run_commands else None, postRunCommands=commands if use_post_run_commands else None, - expectFailure=True, + waitForResponse=True, ) full_output = self.collect_console( pattern=command_abort_on_error, @@ -76,7 +76,7 @@ def test_command_directive_abort_on_error_attach_commands(self): resp = self.attach( program=program, attachCommands=["?!" + command_quiet, "!" + command_abort_on_error], - expectFailure=True, + waitForResponse=True, ) self.assertFalse(resp["success"], "expected 'attach' failure") full_output = self.collect_console(pattern=command_abort_on_error) diff --git a/lldb/test/API/tools/lldb-dap/coreFile/TestDAP_coreFile.py b/lldb/test/API/tools/lldb-dap/coreFile/TestDAP_coreFile.py index d56a8a45ebf1e..174b61cbede57 100644 --- a/lldb/test/API/tools/lldb-dap/coreFile/TestDAP_coreFile.py +++ b/lldb/test/API/tools/lldb-dap/coreFile/TestDAP_coreFile.py @@ -67,7 +67,7 @@ def test_wrong_core_file(self): self.create_debug_adapter() resp = self.attach( - program=exe_file, coreFile=wrong_core_file, expectFailure=True + program=exe_file, coreFile=wrong_core_file, waitForResponse=True ) self.assertIsNotNone(resp) self.assertFalse(resp["success"], "Expected failure in response {resp!r}") diff --git a/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py b/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py index 647ce055a32f8..f143f7f3fdbc3 100644 --- a/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py +++ b/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py @@ -37,7 +37,7 @@ def test_failing_launch_program(self): """ program = self.getBuildArtifact("a.out") self.create_debug_adapter() - response = self.launch(program, expectFailure=True) + response = self.launch(program, waitForResponse=True) self.assertFalse(response["success"]) self.assertEqual( "'{0}' does not exist".format(program), response["body"]["error"]["format"] @@ -53,7 +53,7 @@ def test_failing_launch_commands_and_console(self): program, launchCommands=["a b c"], console="integratedTerminal", - expectFailure=True, + waitForResponse=True, ) self.assertFalse(response["success"]) self.assertTrue(self.get_dict_value(response, ["body", "error", "showUser"])) @@ -68,7 +68,7 @@ def test_failing_console(self): """ program = self.getBuildArtifact("a.out") self.create_debug_adapter() - response = self.launch(program, console="invalid", expectFailure=True) + response = self.launch(program, console="invalid", waitForResponse=True) self.assertFalse(response["success"]) self.assertTrue(self.get_dict_value(response, ["body", "error", "showUser"])) self.assertRegex( @@ -156,6 +156,7 @@ def test_debuggerRoot(self): self.build_and_launch( program, debuggerRoot=program_parent_dir, initCommands=commands ) + self.continue_to_exit() output = self.get_console() self.assertTrue(output and len(output) > 0, "expect console output") lines = output.splitlines() @@ -171,7 +172,6 @@ def test_debuggerRoot(self): % (program_parent_dir, line[len(prefix) :]), ) self.assertTrue(found, "verified lldb-dap working directory") - self.continue_to_exit() def test_sourcePath(self): """ @@ -180,6 +180,7 @@ def test_sourcePath(self): program = self.getBuildArtifact("a.out") program_dir = os.path.dirname(program) self.build_and_launch(program, sourcePath=program_dir) + self.continue_to_exit() output = self.get_console() self.assertTrue(output and len(output) > 0, "expect console output") lines = output.splitlines() @@ -195,7 +196,6 @@ def test_sourcePath(self): "lldb-dap working dir %s == %s" % (quoted_path, line[6:]), ) self.assertTrue(found, 'found "sourcePath" in console output') - self.continue_to_exit() @skipIfWindows def test_disableSTDIO(self): @@ -355,7 +355,7 @@ def test_commands(self): launch. "initCommands" are a list of LLDB commands that get executed - before the targt is created. + before the target is created. "preRunCommands" are a list of LLDB commands that get executed after the target has been created and before the launch. "stopCommands" are a list of LLDB commands that get executed each @@ -440,28 +440,28 @@ def test_extra_launch_commands(self): first_line = line_number(source, "// breakpoint 1") second_line = line_number(source, "// breakpoint 2") # Set target binary and 2 breakpoints - # then we can varify the "launchCommands" get run + # then we can verify the "launchCommands" get run # also we can verify that "stopCommands" get run as the # breakpoints get hit launchCommands = [ 'target create "%s"' % (program), - "breakpoint s -f main.c -l %d" % first_line, - "breakpoint s -f main.c -l %d" % second_line, "process launch --stop-at-entry", ] - initCommands = ["target list", "platform list"] preRunCommands = ["image list a.out", "image dump sections a.out"] + postRunCommands = ['script print("hello world")'] stopCommands = ["frame variable", "bt"] exitCommands = ["expr 2+3", "expr 3+4"] self.launch( program, initCommands=initCommands, preRunCommands=preRunCommands, + postRunCommands=postRunCommands, stopCommands=stopCommands, exitCommands=exitCommands, launchCommands=launchCommands, ) + self.set_source_breakpoints("main.c", [first_line, second_line]) # Get output from the console. This should contain both the # "initCommands" and the "preRunCommands". @@ -474,6 +474,7 @@ def test_extra_launch_commands(self): # Verify all "launchCommands" were found in console output # After execution, program should launch self.verify_commands("launchCommands", output, launchCommands) + self.verify_commands("postRunCommands", output, postRunCommands) # Verify the "stopCommands" here self.continue_to_next_stop() output = self.get_console() @@ -511,7 +512,7 @@ def test_failing_launch_commands(self): initCommands=initCommands, preRunCommands=preRunCommands, launchCommands=launchCommands, - expectFailure=True, + waitForResponse=True, ) self.assertFalse(response["success"]) @@ -608,7 +609,8 @@ def test_no_lldbinit_flag(self): initCommands = ["settings show stop-disassembly-display"] # Launch with initCommands to check the setting - self.launch(program, initCommands=initCommands, stopOnEntry=True) + self.launch(program, initCommands=initCommands) + self.continue_to_exit() # Get console output to verify the setting was NOT set from .lldbinit output = self.get_console() diff --git a/lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py b/lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py index a9ddc1ac55ffe..d7e0168239f34 100644 --- a/lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py +++ b/lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py @@ -105,7 +105,7 @@ def test_runInTerminalInvalidTarget(self): console="integratedTerminal", args=["foobar"], env=["FOO=bar"], - expectFailure=True, + waitForResponse=True, ) self.assertFalse(response["success"]) self.assertIn( diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index 9500c82a853c8..e1cd5c29a3907 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -36,6 +36,7 @@ #include "lldb/lldb-types.h" #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/DenseSet.h" +#include "llvm/ADT/FunctionExtras.h" #include "llvm/ADT/SmallSet.h" #include "llvm/ADT/StringMap.h" #include "llvm/ADT/StringRef.h" @@ -145,6 +146,7 @@ struct DAP final : public DAPTransport::MessageHandler { ReplMode repl_mode; lldb::SBFormat frame_format; lldb::SBFormat thread_format; + llvm::unique_function<void()> on_configuration_done; /// This is used to allow request_evaluate to handle empty expressions /// (ie the user pressed 'return' and expects the previous expression to diff --git a/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp index 1c14dcd16e5e6..2261924af9ccb 100644 --- a/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp @@ -8,7 +8,6 @@ #include "DAP.h" #include "EventHelper.h" -#include "JSONUtils.h" #include "LLDBUtils.h" #include "Protocol/ProtocolRequests.h" #include "RequestHandler.h" @@ -17,7 +16,6 @@ #include "lldb/lldb-defines.h" #include "llvm/Support/Error.h" #include "llvm/Support/FileSystem.h" -#include <cstdint> using namespace llvm; using namespace lldb_dap::protocol; @@ -54,7 +52,7 @@ Error AttachRequestHandler::Run(const AttachRequestArguments &args) const { sys::fs::set_current_path(dap.configuration.debuggerRoot); // Run any initialize LLDB commands the user specified in the launch.json - if (llvm::Error err = dap.RunInitCommands()) + if (Error err = dap.RunInitCommands()) return err; dap.ConfigureSourceMaps(); @@ -85,12 +83,11 @@ Error AttachRequestHandler::Run(const AttachRequestArguments &args) const { if ((args.pid == LLDB_INVALID_PROCESS_ID || args.gdbRemotePort == LLDB_DAP_INVALID_PORT) && - args.waitFor) { + args.waitFor) dap.SendOutput(OutputType::Console, llvm::formatv("Waiting to attach to \"{0}\"...", dap.target.GetExecutable().GetFilename()) .str()); - } { // Perform the launch in synchronous mode so that we don't have to worry @@ -133,6 +130,7 @@ Error AttachRequestHandler::Run(const AttachRequestArguments &args) const { attach_info.SetWaitForLaunch(args.waitFor, /*async=*/false); dap.target.Attach(attach_info, error); } + if (error.Fail()) return ToError(error); } @@ -150,8 +148,4 @@ Error AttachRequestHandler::Run(const AttachRequestArguments &args) const { return Error::success(); } -void AttachRequestHandler::PostRun() const { - dap.SendJSON(CreateEventObject("initialized")); -} - } // namespace lldb_dap diff --git a/lldb/tools/lldb-dap/Handler/ConfigurationDoneRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/ConfigurationDoneRequestHandler.cpp index 1bfe7b7f6ef5c..bdcdd7904524f 100644 --- a/lldb/tools/lldb-dap/Handler/ConfigurationDoneRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/ConfigurationDoneRequestHandler.cpp @@ -9,7 +9,6 @@ #include "DAP.h" #include "EventHelper.h" #include "LLDBUtils.h" -#include "Protocol/ProtocolEvents.h" #include "Protocol/ProtocolRequests.h" #include "ProtocolUtils.h" #include "RequestHandler.h" @@ -66,4 +65,13 @@ ConfigurationDoneRequestHandler::Run(const ConfigurationDoneArguments &) const { return ToError(process.Continue()); } +void ConfigurationDoneRequestHandler::PostRun() const { + if (!dap.on_configuration_done) + return; + + dap.on_configuration_done(); + // Clear the callback to ensure any captured resources are released. + dap.on_configuration_done = nullptr; +} + } // namespace lldb_dap diff --git a/lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp index 33563cc7a5cee..476040cfc3b66 100644 --- a/lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp @@ -8,7 +8,6 @@ #include "DAP.h" #include "EventHelper.h" -#include "JSONUtils.h" #include "LLDBUtils.h" #include "Protocol/ProtocolRequests.h" #include "RequestHandler.h" @@ -65,8 +64,4 @@ Error LaunchRequestHandler::Run(const LaunchRequestArguments &arguments) const { return Error::success(); } -void LaunchRequestHandler::PostRun() const { - dap.SendJSON(CreateEventObject("initialized")); -} - } // namespace lldb_dap diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.cpp b/lldb/tools/lldb-dap/Handler/RequestHandler.cpp index d67437ad5b3ae..c60c86219f67d 100644 --- a/lldb/tools/lldb-dap/Handler/RequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/RequestHandler.cpp @@ -18,6 +18,7 @@ #include "lldb/API/SBDefines.h" #include "lldb/API/SBEnvironment.h" #include "llvm/Support/Error.h" +#include "llvm/Support/JSON.h" #include <mutex> #if !defined(_WIN32) @@ -257,7 +258,7 @@ llvm::Error BaseRequestHandler::LaunchProcess( void BaseRequestHandler::PrintWelcomeMessage() const { #ifdef LLDB_DAP_WELCOME_MESSAGE - dap.SendOutput(OutputType::Console, LLDB_DAP_WELCOME_MESSAGE); + dap.SendOutput(eOutputCategoryConsole, LLDB_DAP_WELCOME_MESSAGE); #endif } @@ -268,4 +269,72 @@ bool BaseRequestHandler::HasInstructionGranularity( return false; } +void BaseRequestHandler::BuildErrorResponse( + llvm::Error err, protocol::Response &response) const { + // Handle the ErrorSuccess case. + if (!err) { + response.success = true; + return; + } + + response.success = false; + + llvm::handleAllErrors( + std::move(err), + [&](const NotStoppedError &err) { + response.message = lldb_dap::protocol::eResponseMessageNotStopped; + }, + [&](const DAPError &err) { + protocol::ErrorMessage error_message; + error_message.sendTelemetry = false; + error_message.format = err.getMessage(); + error_message.showUser = err.getShowUser(); + error_message.id = err.convertToErrorCode().value(); + error_message.url = err.getURL(); + error_message.urlLabel = err.getURLLabel(); + protocol::ErrorResponseBody body; + body.error = error_message; + + response.body = body; + }, + [&](const llvm::ErrorInfoBase &err) { + protocol::ErrorMessage error_message; + error_message.showUser = true; + error_message.sendTelemetry = false; + error_message.format = err.message(); + error_message.id = err.convertToErrorCode().value(); + protocol::ErrorResponseBody body; + body.error = error_message; + + response.body = body; + }); +} + +void BaseRequestHandler::SendError(llvm::Error err, + protocol::Response &response) const { + BuildErrorResponse(std::move(err), response); + Send(response); +} + +void BaseRequestHandler::SendSuccess( + protocol::Response &response, std::optional<llvm::json::Value> body) const { + response.success = true; + if (body) + response.body = std::move(*body); + + Send(response); +} + +void BaseRequestHandler::Send(protocol::Response &response) const { + // Mark the request as 'cancelled' if the debugger was interrupted while + // evaluating this handler. + if (dap.debugger.InterruptRequested()) { + response.success = false; + response.message = protocol::eResponseMessageCancelled; + response.body = std::nullopt; + } + + dap.Send(response); +} + } // namespace lldb_dap diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.h b/lldb/tools/lldb-dap/Handler/RequestHandler.h index a18a8049c804d..ff0ff4decf0f0 100644 --- a/lldb/tools/lldb-dap/Handler/RequestHandler.h +++ b/lldb/tools/lldb-dap/Handler/RequestHandler.h @@ -11,17 +11,17 @@ #include "DAP.h" #include "DAPError.h" -#include "DAPLog.h" #include "Protocol/ProtocolBase.h" #include "Protocol/ProtocolRequests.h" #include "Protocol/ProtocolTypes.h" #include "llvm/ADT/DenseSet.h" +#include "llvm/ADT/FunctionExtras.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/Error.h" #include "llvm/Support/JSON.h" +#include <memory> #include <optional> #include <type_traits> -#include <variant> #include <vector> template <typename T> struct is_optional : std::false_type {}; @@ -76,6 +76,20 @@ class BaseRequestHandler { /// @} + /// Builds an error response from the given error. + void BuildErrorResponse(llvm::Error, protocol::Response &) const; + + /// Sends an error response from the current handler. + void SendError(llvm::Error, protocol::Response &) const; + + /// Sends a successful response, with an optional body from the current + /// handler. + void SendSuccess(protocol::Response &, + std::optional<llvm::json::Value> = std::nullopt) const; + + /// Send a response to the client. + void Send(protocol::Response &response) const; + DAP &dap; }; @@ -133,42 +147,27 @@ class RequestHandler : public BaseRequestHandler { response.command = request.command; llvm::Expected<Args> arguments = parseArgs<Args>(request); - if (llvm::Error err = arguments.takeError()) { - HandleErrorResponse(std::move(err), response); - dap.Send(response); - return; - } + if (llvm::Error err = arguments.takeError()) + return SendError(std::move(err), response); if constexpr (std::is_same_v<Resp, llvm::Error>) { - if (llvm::Error err = Run(*arguments)) { - HandleErrorResponse(std::move(err), response); - } else { - response.success = true; - } + if (llvm::Error err = Run(*arguments)) + SendError(std::move(err), response); + else + SendSuccess(response); } else { Resp body = Run(*arguments); - if (llvm::Error err = body.takeError()) { - HandleErrorResponse(std::move(err), response); - } else { - response.success = true; - response.body = std::move(*body); - } + if (llvm::Error err = body.takeError()) + SendError(std::move(err), response); + else + SendSuccess(response, std::move(*body)); } - // Mark the request as 'cancelled' if the debugger was interrupted while - // evaluating this handler. - if (dap.debugger.InterruptRequested()) { - dap.debugger.CancelInterruptRequest(); - response.success = false; - response.message = protocol::eResponseMessageCancelled; - response.body = std::nullopt; - } - - dap.Send(response); - PostRun(); }; +protected: + /// Run the request handler. virtual Resp Run(const Args &) const = 0; /// A hook for a request handler to run additional operations after the @@ -177,48 +176,50 @@ class RequestHandler : public BaseRequestHandler { /// *NOTE*: PostRun will be invoked even if the `Run` operation returned an /// error. virtual void PostRun() const {}; +}; - void HandleErrorResponse(llvm::Error err, - protocol::Response &response) const { - response.success = false; - llvm::handleAllErrors( - std::move(err), - [&](const NotStoppedError &err) { - response.message = lldb_dap::protocol::eResponseMessageNotStopped; - }, - [&](const DAPError &err) { - protocol::ErrorMessage error_message; - error_message.sendTelemetry = false; - error_message.format = err.getMessage(); - error_message.showUser = err.getShowUser(); - error_message.id = err.convertToErrorCode().value(); - error_message.url = err.getURL(); - error_message.urlLabel = err.getURLLabel(); - protocol::ErrorResponseBody body; - body.error = error_message; - response.body = body; - }, - [&](const llvm::ErrorInfoBase &err) { - protocol::ErrorMessage error_message; - error_message.showUser = true; - error_message.sendTelemetry = false; - error_message.format = err.message(); - error_message.id = err.convertToErrorCode().value(); - protocol::ErrorResponseBody body; - body.error = error_message; - response.body = body; - }); - } +/// A specialized base class for attach and launch requests that delays sending +/// the response until 'configurationDone' is received. +template <typename Args, typename Resp> +class DelayedResponseRequestHandler : public BaseRequestHandler { + using BaseRequestHandler::BaseRequestHandler; + + void operator()(const protocol::Request &request) const override { + // Only support void responses for now. + static_assert(std::is_same_v<Resp, llvm::Error>); + + protocol::Response response; + response.request_seq = request.seq; + response.command = request.command; + + llvm::Expected<Args> arguments = parseArgs<Args>(request); + if (llvm::Error err = arguments.takeError()) + return SendError(std::move(err), response); + + BuildErrorResponse(Run(*arguments), response); + + dap.on_configuration_done = [this, response]() mutable { Send(response); }; + + // The 'configurationDone' request is not sent until after 'initialized' + // triggers the breakpoints being sent and 'configurationDone' is the last + // message in the chain. + protocol::Event initialized{"initialized"}; + dap.Send(initialized); + }; + +protected: + /// Run the request handler. + virtual Resp Run(const Args &) const = 0; }; class AttachRequestHandler - : public RequestHandler<protocol::AttachRequestArguments, - protocol::AttachResponse> { + : public DelayedResponseRequestHandler<protocol::AttachRequestArguments, + protocol::AttachResponse> { public: - using RequestHandler::RequestHandler; + using DelayedResponseRequestHandler::DelayedResponseRequestHandler; static llvm::StringLiteral GetCommand() { return "attach"; } - llvm::Error Run(const protocol::AttachRequestArguments &args) const override; - void PostRun() const override; + protocol::AttachResponse + Run(const protocol::AttachRequestArguments &args) const override; }; class BreakpointLocationsRequestHandler @@ -277,6 +278,7 @@ class ConfigurationDoneRequestHandler } protocol::ConfigurationDoneResponse Run(const protocol::ConfigurationDoneArguments &) const override; + void PostRun() const override; }; class DisconnectRequestHandler @@ -330,14 +332,13 @@ class InitializeRequestHandler }; class LaunchRequestHandler - : public RequestHandler<protocol::LaunchRequestArguments, - protocol::LaunchResponse> { + : public DelayedResponseRequestHandler<protocol::LaunchRequestArguments, + protocol::LaunchResponse> { public: - using RequestHandler::RequestHandler; + using DelayedResponseRequestHandler::DelayedResponseRequestHandler; static llvm::StringLiteral GetCommand() { return "launch"; } - llvm::Error + protocol::LaunchResponse Run(const protocol::LaunchRequestArguments &arguments) const override; - void PostRun() const override; }; class RestartRequestHandler _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
