Author: John Harrison Date: 2025-05-16T19:28:34-07:00 New Revision: 0e0b501bf53677105b539fa4f84cbfb76c46f74d
URL: https://github.com/llvm/llvm-project/commit/0e0b501bf53677105b539fa4f84cbfb76c46f74d DIFF: https://github.com/llvm/llvm-project/commit/0e0b501bf53677105b539fa4f84cbfb76c46f74d.diff LOG: [lldb-dap] Take two at refactoring the startup sequence. (#140331) This is more straight forward refactor of the startup sequence that reverts parts of ba29e60f9a2222bd5e883579bb78db13fc5a7588. Unlike my previous attempt, I ended up removing the pending request queue and not including an `AsyncReqeustHandler` because I don't think we actually need that at the moment. The key is that during the startup flow there are 2 parallel operations happening in the DAP that have different triggers. * The `initialize` request is sent and once the response is received the `launch` or `attach` is sent. * When the `initialized` event is recieved the `setBreakpionts` and other config requests are made followed by the `configurationDone` event. I moved the `initialized` event back to happen in the `PostRun` of the `launch` or `attach` request handlers. This ensures that we have a valid target by the time the configuration calls are made. I added also added a few extra validations that to the `configurationeDone` handler to ensure we're in an expected state. I've also fixed up the tests to match the new flow. With the other additional test fixes in 087a5d2ec7897cd99d3787820711fec76a8e1792 I think we've narrowed down the main source of test instability that motivated the startup sequence change. 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/breakpoint-events/TestDAP_breakpointEvents.py lldb/test/API/tools/lldb-dap/cancel/TestDAP_cancel.py lldb/test/API/tools/lldb-dap/completions/TestDAP_completions.py lldb/test/API/tools/lldb-dap/console/TestDAP_console.py lldb/test/API/tools/lldb-dap/console/TestDAP_redirection_to_console.py lldb/test/API/tools/lldb-dap/coreFile/TestDAP_coreFile.py lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py lldb/test/API/tools/lldb-dap/module-event/TestDAP_module_event.py lldb/test/API/tools/lldb-dap/module/TestDAP_module.py lldb/test/API/tools/lldb-dap/repl-mode/TestDAP_repl_mode_detection.py lldb/test/API/tools/lldb-dap/restart/TestDAP_restart.py lldb/test/API/tools/lldb-dap/send-event/TestDAP_sendEvent.py lldb/test/API/tools/lldb-dap/stackTrace/TestDAP_stackTrace.py lldb/test/API/tools/lldb-dap/stackTraceDisassemblyDisplay/TestDAP_stackTraceDisassemblyDisplay.py lldb/test/API/tools/lldb-dap/startDebugging/TestDAP_startDebugging.py lldb/test/API/tools/lldb-dap/stop-hooks/TestDAP_stop_hooks.py lldb/test/API/tools/lldb-dap/threads/TestDAP_threads.py lldb/test/API/tools/lldb-dap/variables/children/TestDAP_variables_children.py lldb/tools/lldb-dap/DAP.cpp 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/InitializeRequestHandler.cpp lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp lldb/tools/lldb-dap/Handler/RequestHandler.h lldb/tools/lldb-dap/Protocol/ProtocolBase.h lldb/tools/lldb-dap/Protocol/ProtocolRequests.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 d3589e78b6bc7..70fd0b0c419db 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 @@ -133,6 +133,7 @@ def __init__( self.output_condition = threading.Condition() self.output: dict[str, list[str]] = {} self.configuration_done_sent = False + self.initialized = False self.frame_scopes = {} self.init_commands = init_commands self.disassembled_instructions = {} @@ -235,6 +236,8 @@ def _handle_recv_packet(self, packet: Optional[ProtocolMessage]) -> bool: self.output_condition.release() # no need to add 'output' event packets to our packets list return keepGoing + elif event == "initialized": + self.initialized = True elif event == "process": # When a new process is attached or launched, remember the # details that are available in the body of the event @@ -602,7 +605,7 @@ def request_attach( exitCommands: Optional[list[str]] = None, terminateCommands: Optional[list[str]] = None, coreFile: Optional[str] = None, - stopOnAttach=True, + stopOnEntry=False, sourceMap: Optional[Union[list[tuple[str, str]], dict[str, str]]] = None, gdbRemotePort: Optional[int] = None, gdbRemoteHostname: Optional[str] = None, @@ -629,8 +632,8 @@ def request_attach( args_dict["attachCommands"] = attachCommands if coreFile: args_dict["coreFile"] = coreFile - if stopOnAttach: - args_dict["stopOnEntry"] = stopOnAttach + if stopOnEntry: + args_dict["stopOnEntry"] = stopOnEntry if postRunCommands: args_dict["postRunCommands"] = postRunCommands if sourceMap: @@ -640,11 +643,7 @@ def request_attach( if gdbRemoteHostname is not None: args_dict["gdb-remote-hostname"] = gdbRemoteHostname command_dict = {"command": "attach", "type": "request", "arguments": args_dict} - response = self.send_recv(command_dict) - - if response["success"]: - self.wait_for_event("process") - return response + return self.send_recv(command_dict) def request_breakpointLocations( self, file_path, line, end_line=None, column=None, end_column=None @@ -677,6 +676,7 @@ def request_configurationDone(self): response = self.send_recv(command_dict) if response: self.configuration_done_sent = True + self.request_threads() return response def _process_stopped(self): @@ -824,7 +824,7 @@ def request_launch( args: Optional[list[str]] = None, cwd: Optional[str] = None, env: Optional[dict[str, str]] = None, - stopOnEntry=True, + stopOnEntry=False, disableASLR=True, disableSTDIO=False, shellExpandArguments=False, @@ -894,11 +894,7 @@ def request_launch( if commandEscapePrefix is not None: args_dict["commandEscapePrefix"] = commandEscapePrefix command_dict = {"command": "launch", "type": "request", "arguments": args_dict} - response = self.send_recv(command_dict) - - if response["success"]: - self.wait_for_event("process") - return response + return self.send_recv(command_dict) def request_next(self, threadId, granularity="statement"): if self.exit_status is not None: 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 d7cf8e2864324..afdc746ed0d0d 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 @@ -56,7 +56,7 @@ def set_source_breakpoints(self, source_path, lines, data=None): It contains optional location/hitCondition/logMessage parameters. """ response = self.dap_server.request_setBreakpoints(source_path, lines, data) - if response is None: + if response is None or not response["success"]: return [] breakpoints = response["body"]["breakpoints"] breakpoint_ids = [] @@ -354,13 +354,9 @@ def disassemble(self, threadId=None, frameIndex=None): def attach( self, *, - stopOnAttach=True, disconnectAutomatically=True, sourceInitFile=False, expectFailure=False, - sourceBreakpoints=None, - functionBreakpoints=None, - timeout=DEFAULT_TIMEOUT, **kwargs, ): """Build the default Makefile target, create the DAP debug adapter, @@ -378,37 +374,13 @@ def cleanup(): self.addTearDownHook(cleanup) # Initialize and launch the program self.dap_server.request_initialize(sourceInitFile) - self.dap_server.wait_for_event("initialized", timeout) - - # Set source breakpoints as part of the launch sequence. - if sourceBreakpoints: - for source_path, lines in sourceBreakpoints: - response = self.dap_server.request_setBreakpoints(source_path, lines) - self.assertTrue( - response["success"], - "setBreakpoints failed (%s)" % (response), - ) - - # Set function breakpoints as part of the launch sequence. - if functionBreakpoints: - response = self.dap_server.request_setFunctionBreakpoints( - functionBreakpoints - ) - self.assertTrue( - response["success"], - "setFunctionBreakpoint failed (%s)" % (response), - ) - - self.dap_server.request_configurationDone() - response = self.dap_server.request_attach(stopOnAttach=stopOnAttach, **kwargs) + response = self.dap_server.request_attach(**kwargs) if expectFailure: return response if not (response and response["success"]): self.assertTrue( response["success"], "attach failed (%s)" % (response["message"]) ) - if stopOnAttach: - self.dap_server.wait_for_stopped(timeout) def launch( self, @@ -416,11 +388,7 @@ def launch( *, sourceInitFile=False, disconnectAutomatically=True, - sourceBreakpoints=None, - functionBreakpoints=None, expectFailure=False, - stopOnEntry=True, - timeout=DEFAULT_TIMEOUT, **kwargs, ): """Sending launch request to dap""" @@ -437,35 +405,7 @@ def cleanup(): # Initialize and launch the program self.dap_server.request_initialize(sourceInitFile) - self.dap_server.wait_for_event("initialized", timeout) - - # Set source breakpoints as part of the launch sequence. - if sourceBreakpoints: - for source_path, lines in sourceBreakpoints: - response = self.dap_server.request_setBreakpoints(source_path, lines) - self.assertTrue( - response["success"], - "setBreakpoints failed (%s)" % (response), - ) - - # Set function breakpoints as part of the launch sequence. - if functionBreakpoints: - response = self.dap_server.request_setFunctionBreakpoints( - functionBreakpoints - ) - self.assertTrue( - response["success"], - "setFunctionBreakpoint failed (%s)" % (response), - ) - - self.dap_server.request_configurationDone() - - response = self.dap_server.request_launch( - program, - stopOnEntry=stopOnEntry, - **kwargs, - ) - + response = self.dap_server.request_launch(program, **kwargs) if expectFailure: return response if not (response and response["success"]): @@ -473,10 +413,6 @@ def cleanup(): response["success"], "launch failed (%s)" % (response["body"]["error"]["format"]), ) - if stopOnEntry: - self.dap_server.wait_for_stopped(timeout) - - return response def build_and_launch( self, diff --git a/lldb/test/API/tools/lldb-dap/breakpoint-events/TestDAP_breakpointEvents.py b/lldb/test/API/tools/lldb-dap/breakpoint-events/TestDAP_breakpointEvents.py index 25f031db5cac5..d46fc31d797da 100644 --- a/lldb/test/API/tools/lldb-dap/breakpoint-events/TestDAP_breakpointEvents.py +++ b/lldb/test/API/tools/lldb-dap/breakpoint-events/TestDAP_breakpointEvents.py @@ -52,7 +52,7 @@ def test_breakpoint_events(self): # breakpoint events for these breakpoints but not for ones that are not # set via the command interpreter. bp_command = "breakpoint set --file foo.cpp --line %u" % (foo_bp2_line) - self.build_and_launch(program, stopOnEntry=True, preRunCommands=[bp_command]) + self.build_and_launch(program, preRunCommands=[bp_command]) main_bp_id = 0 foo_bp_id = 0 # Set breakpoints and verify that they got set correctly diff --git a/lldb/test/API/tools/lldb-dap/cancel/TestDAP_cancel.py b/lldb/test/API/tools/lldb-dap/cancel/TestDAP_cancel.py index 948c146d4da68..824ed8fe3bb97 100644 --- a/lldb/test/API/tools/lldb-dap/cancel/TestDAP_cancel.py +++ b/lldb/test/API/tools/lldb-dap/cancel/TestDAP_cancel.py @@ -44,7 +44,7 @@ def test_pending_request(self): Tests cancelling a pending request. """ program = self.getBuildArtifact("a.out") - self.build_and_launch(program, stopOnEntry=True) + self.build_and_launch(program) # Use a relatively short timeout since this is only to ensure the # following request is queued. @@ -76,7 +76,7 @@ def test_inflight_request(self): Tests cancelling an inflight request. """ program = self.getBuildArtifact("a.out") - self.build_and_launch(program, stopOnEntry=True) + self.build_and_launch(program) blocking_seq = self.async_blocking_request(duration=self.DEFAULT_TIMEOUT / 2) # Wait for the sleep to start to cancel the inflight request. diff --git a/lldb/test/API/tools/lldb-dap/completions/TestDAP_completions.py b/lldb/test/API/tools/lldb-dap/completions/TestDAP_completions.py index 75876c248f86c..04897acfcf85d 100644 --- a/lldb/test/API/tools/lldb-dap/completions/TestDAP_completions.py +++ b/lldb/test/API/tools/lldb-dap/completions/TestDAP_completions.py @@ -46,17 +46,12 @@ def verify_completions(self, actual_list, expected_list, not_expected_list=[]): def setup_debuggee(self): program = self.getBuildArtifact("a.out") source = "main.cpp" - self.build_and_launch( - program, - stopOnEntry=True, - sourceBreakpoints=[ - ( - source, - [ - line_number(source, "// breakpoint 1"), - line_number(source, "// breakpoint 2"), - ], - ), + self.build_and_launch(program) + self.set_source_breakpoints( + source, + [ + line_number(source, "// breakpoint 1"), + line_number(source, "// breakpoint 2"), ], ) diff --git a/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py b/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py index 1f810afdbb667..7b4d1adbb2071 100644 --- a/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py +++ b/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py @@ -53,7 +53,7 @@ def test_scopes_variables_setVariable_evaluate(self): character. """ program = self.getBuildArtifact("a.out") - self.build_and_launch(program, stopOnEntry=True) + self.build_and_launch(program) source = "main.cpp" breakpoint1_line = line_number(source, "// breakpoint 1") lines = [breakpoint1_line] @@ -66,6 +66,7 @@ def test_scopes_variables_setVariable_evaluate(self): # Cause a "scopes" to be sent for frame zero which should update the # selected thread and frame to frame 0. self.dap_server.get_local_variables(frameIndex=0) + # Verify frame #0 is selected in the command interpreter by running # the "frame select" command with no frame index which will print the # currently selected frame. @@ -74,15 +75,15 @@ def test_scopes_variables_setVariable_evaluate(self): # Cause a "scopes" to be sent for frame one which should update the # selected thread and frame to frame 1. self.dap_server.get_local_variables(frameIndex=1) + # Verify frame #1 is selected in the command interpreter by running # the "frame select" command with no frame index which will print the # currently selected frame. - self.check_lldb_command("frame select", "frame #1", "frame 1 is selected") def test_custom_escape_prefix(self): program = self.getBuildArtifact("a.out") - self.build_and_launch(program, stopOnEntry=True, commandEscapePrefix="::") + self.build_and_launch(program, commandEscapePrefix="::") source = "main.cpp" breakpoint1_line = line_number(source, "// breakpoint 1") breakpoint_ids = self.set_source_breakpoints(source, [breakpoint1_line]) @@ -97,7 +98,7 @@ def test_custom_escape_prefix(self): def test_empty_escape_prefix(self): program = self.getBuildArtifact("a.out") - self.build_and_launch(program, stopOnEntry=True, commandEscapePrefix="") + self.build_and_launch(program, commandEscapePrefix="") source = "main.cpp" breakpoint1_line = line_number(source, "// breakpoint 1") breakpoint_ids = self.set_source_breakpoints(source, [breakpoint1_line]) @@ -114,7 +115,7 @@ def test_empty_escape_prefix(self): def test_exit_status_message_sigterm(self): source = "main.cpp" program = self.getBuildArtifact("a.out") - self.build_and_launch(program, stopOnEntry=True, commandEscapePrefix="") + self.build_and_launch(program, commandEscapePrefix="") breakpoint1_line = line_number(source, "// breakpoint 1") breakpoint_ids = self.set_source_breakpoints(source, [breakpoint1_line]) self.continue_to_breakpoints(breakpoint_ids) @@ -168,7 +169,7 @@ def test_exit_status_message_ok(self): def test_diagnositcs(self): program = self.getBuildArtifact("a.out") - self.build_and_launch(program, stopOnEntry=True) + self.build_and_launch(program) core = self.getBuildArtifact("minidump.core") self.yaml2obj("minidump.yaml", core) diff --git a/lldb/test/API/tools/lldb-dap/console/TestDAP_redirection_to_console.py b/lldb/test/API/tools/lldb-dap/console/TestDAP_redirection_to_console.py index 23500bd6fe586..e367c327d4295 100644 --- a/lldb/test/API/tools/lldb-dap/console/TestDAP_redirection_to_console.py +++ b/lldb/test/API/tools/lldb-dap/console/TestDAP_redirection_to_console.py @@ -16,9 +16,7 @@ def test(self): """ program = self.getBuildArtifact("a.out") self.build_and_launch( - program, - stopOnEntry=True, - lldbDAPEnv={"LLDB_DAP_TEST_STDOUT_STDERR_REDIRECTION": ""}, + program, lldbDAPEnv={"LLDB_DAP_TEST_STDOUT_STDERR_REDIRECTION": ""} ) source = "main.cpp" 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 e678c5ee77fdc..db43dbaf515cf 100644 --- a/lldb/test/API/tools/lldb-dap/coreFile/TestDAP_coreFile.py +++ b/lldb/test/API/tools/lldb-dap/coreFile/TestDAP_coreFile.py @@ -19,6 +19,7 @@ def test_core_file(self): self.create_debug_adapter() self.attach(program=exe_file, coreFile=core_file) + self.dap_server.request_configurationDone() expected_frames = [ { diff --git a/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py b/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py index 372a9bb75e007..2166e88151986 100644 --- a/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py +++ b/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py @@ -43,7 +43,6 @@ def run_test_evaluate_expressions( self.build_and_launch( program, enableAutoVariableSummaries=enableAutoVariableSummaries, - stopOnEntry=True, ) source = "main.cpp" self.set_source_breakpoints( 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 0063954791fd5..8805ce50e6a21 100644 --- a/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py +++ b/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py @@ -87,7 +87,8 @@ def test_stopOnEntry(self): """ program = self.getBuildArtifact("a.out") self.build_and_launch(program, stopOnEntry=True) - + self.dap_server.request_configurationDone() + self.dap_server.wait_for_stopped() self.assertTrue( len(self.dap_server.thread_stop_reasons) > 0, "expected stopped event during launch", @@ -357,7 +358,6 @@ def test_commands(self): terminateCommands = ["expr 4+2"] self.build_and_launch( program, - stopOnEntry=True, initCommands=initCommands, preRunCommands=preRunCommands, postRunCommands=postRunCommands, diff --git a/lldb/test/API/tools/lldb-dap/module-event/TestDAP_module_event.py b/lldb/test/API/tools/lldb-dap/module-event/TestDAP_module_event.py index 19de35b60a3ef..1ef2f2a8235a4 100644 --- a/lldb/test/API/tools/lldb-dap/module-event/TestDAP_module_event.py +++ b/lldb/test/API/tools/lldb-dap/module-event/TestDAP_module_event.py @@ -10,7 +10,7 @@ class TestDAP_module_event(lldbdap_testcase.DAPTestCaseBase): @skipIfWindows def test_module_event(self): program = self.getBuildArtifact("a.out") - self.build_and_launch(program, stopOnEntry=True) + self.build_and_launch(program) source = "main.cpp" breakpoint1_line = line_number(source, "// breakpoint 1") diff --git a/lldb/test/API/tools/lldb-dap/module/TestDAP_module.py b/lldb/test/API/tools/lldb-dap/module/TestDAP_module.py index b333efd7bfb1f..3fc0f752ee39e 100644 --- a/lldb/test/API/tools/lldb-dap/module/TestDAP_module.py +++ b/lldb/test/API/tools/lldb-dap/module/TestDAP_module.py @@ -14,7 +14,7 @@ class TestDAP_module(lldbdap_testcase.DAPTestCaseBase): def run_test(self, symbol_basename, expect_debug_info_size): program_basename = "a.out.stripped" program = self.getBuildArtifact(program_basename) - self.build_and_launch(program, stopOnEntry=True) + self.build_and_launch(program) functions = ["foo"] breakpoint_ids = self.set_function_breakpoints(functions) self.assertEqual(len(breakpoint_ids), len(functions), "expect one breakpoint") @@ -108,7 +108,7 @@ def test_modules_dsym(self): @skipIfWindows def test_compile_units(self): program = self.getBuildArtifact("a.out") - self.build_and_launch(program, stopOnEntry=True) + self.build_and_launch(program) source = "main.cpp" main_source_path = self.getSourcePath(source) breakpoint1_line = line_number(source, "// breakpoint 1") diff --git a/lldb/test/API/tools/lldb-dap/repl-mode/TestDAP_repl_mode_detection.py b/lldb/test/API/tools/lldb-dap/repl-mode/TestDAP_repl_mode_detection.py index 81edcdf4bd0f9..c6f59949d668e 100644 --- a/lldb/test/API/tools/lldb-dap/repl-mode/TestDAP_repl_mode_detection.py +++ b/lldb/test/API/tools/lldb-dap/repl-mode/TestDAP_repl_mode_detection.py @@ -20,7 +20,7 @@ def assertEvaluate(self, expression, regex): def test_completions(self): program = self.getBuildArtifact("a.out") - self.build_and_launch(program, stopOnEntry=True) + self.build_and_launch(program) source = "main.cpp" breakpoint1_line = line_number(source, "// breakpoint 1") diff --git a/lldb/test/API/tools/lldb-dap/restart/TestDAP_restart.py b/lldb/test/API/tools/lldb-dap/restart/TestDAP_restart.py index 8681b31e8eb1b..83faf276852f8 100644 --- a/lldb/test/API/tools/lldb-dap/restart/TestDAP_restart.py +++ b/lldb/test/API/tools/lldb-dap/restart/TestDAP_restart.py @@ -35,7 +35,7 @@ def test_basic_functionality(self): # Restart then check we stop back at A and program state has been reset. resp = self.dap_server.request_restart() self.assertTrue(resp["success"]) - self.continue_to_breakpoints([bp_A]) + self.verify_breakpoint_hit([bp_A]) self.assertEqual( int(self.dap_server.get_local_variable_value("i")), 0, @@ -50,6 +50,9 @@ def test_stopOnEntry(self): program = self.getBuildArtifact("a.out") self.build_and_launch(program, stopOnEntry=True) [bp_main] = self.set_function_breakpoints(["main"]) + + self.dap_server.request_configurationDone() + self.dap_server.wait_for_stopped() # Once the "configuration done" event is sent, we should get a stopped # event immediately because of stopOnEntry. self.assertTrue( diff --git a/lldb/test/API/tools/lldb-dap/send-event/TestDAP_sendEvent.py b/lldb/test/API/tools/lldb-dap/send-event/TestDAP_sendEvent.py index 3e015186d4b81..a01845669666f 100644 --- a/lldb/test/API/tools/lldb-dap/send-event/TestDAP_sendEvent.py +++ b/lldb/test/API/tools/lldb-dap/send-event/TestDAP_sendEvent.py @@ -24,7 +24,6 @@ def test_send_event(self): } self.build_and_launch( program, - sourceBreakpoints=[(source, [breakpoint_line])], stopCommands=[ "lldb-dap send-event my-custom-event-no-body", "lldb-dap send-event my-custom-event '{}'".format( @@ -32,6 +31,8 @@ def test_send_event(self): ), ], ) + self.set_source_breakpoints(source, [breakpoint_line]) + self.continue_to_next_stop() custom_event = self.dap_server.wait_for_event( filter=["my-custom-event-no-body"] diff --git a/lldb/test/API/tools/lldb-dap/stackTrace/TestDAP_stackTrace.py b/lldb/test/API/tools/lldb-dap/stackTrace/TestDAP_stackTrace.py index 9c6f1d42feda2..abd469274ffd4 100644 --- a/lldb/test/API/tools/lldb-dap/stackTrace/TestDAP_stackTrace.py +++ b/lldb/test/API/tools/lldb-dap/stackTrace/TestDAP_stackTrace.py @@ -61,7 +61,7 @@ def test_stackTrace(self): Tests the 'stackTrace' packet and all its variants. """ program = self.getBuildArtifact("a.out") - self.build_and_launch(program, stopOnEntry=True) + self.build_and_launch(program) source = "main.c" self.source_path = os.path.join(os.getcwd(), source) self.recurse_end = line_number(source, "recurse end") diff --git a/lldb/test/API/tools/lldb-dap/stackTraceDisassemblyDisplay/TestDAP_stackTraceDisassemblyDisplay.py b/lldb/test/API/tools/lldb-dap/stackTraceDisassemblyDisplay/TestDAP_stackTraceDisassemblyDisplay.py index 963d711978534..08c225b3cada4 100644 --- a/lldb/test/API/tools/lldb-dap/stackTraceDisassemblyDisplay/TestDAP_stackTraceDisassemblyDisplay.py +++ b/lldb/test/API/tools/lldb-dap/stackTraceDisassemblyDisplay/TestDAP_stackTraceDisassemblyDisplay.py @@ -37,7 +37,7 @@ def build_and_run_until_breakpoint(self): breakpoint_line = line_number(other_source_file, "// Break here") program = self.getBuildArtifact("a.out") - self.build_and_launch(program, stopOnEntry=True, commandEscapePrefix="") + self.build_and_launch(program, commandEscapePrefix="") breakpoint_ids = self.set_source_breakpoints( other_source_file, [breakpoint_line] diff --git a/lldb/test/API/tools/lldb-dap/startDebugging/TestDAP_startDebugging.py b/lldb/test/API/tools/lldb-dap/startDebugging/TestDAP_startDebugging.py index e37cd36d7f283..b487257b6414d 100644 --- a/lldb/test/API/tools/lldb-dap/startDebugging/TestDAP_startDebugging.py +++ b/lldb/test/API/tools/lldb-dap/startDebugging/TestDAP_startDebugging.py @@ -15,7 +15,7 @@ def test_startDebugging(self): """ program = self.getBuildArtifact("a.out") source = "main.c" - self.build_and_launch(program, stopOnEntry=True) + self.build_and_launch(program) breakpoint_line = line_number(source, "// breakpoint") diff --git a/lldb/test/API/tools/lldb-dap/stop-hooks/TestDAP_stop_hooks.py b/lldb/test/API/tools/lldb-dap/stop-hooks/TestDAP_stop_hooks.py index 33e038408fa34..d630e1d14c3a0 100644 --- a/lldb/test/API/tools/lldb-dap/stop-hooks/TestDAP_stop_hooks.py +++ b/lldb/test/API/tools/lldb-dap/stop-hooks/TestDAP_stop_hooks.py @@ -15,7 +15,7 @@ def test_stop_hooks_before_run(self): """ program = self.getBuildArtifact("a.out") preRunCommands = ["target stop-hook add -o help"] - self.build_and_launch(program, stopOnEntry=True, preRunCommands=preRunCommands) + self.build_and_launch(program, preRunCommands=preRunCommands) breakpoint_ids = self.set_function_breakpoints(["main"]) # This request hangs if the race happens, because, in that case, the # command interpreter is in synchronous mode while lldb-dap expects diff --git a/lldb/test/API/tools/lldb-dap/threads/TestDAP_threads.py b/lldb/test/API/tools/lldb-dap/threads/TestDAP_threads.py index 6edb4b8e2a816..a4658da58ac94 100644 --- a/lldb/test/API/tools/lldb-dap/threads/TestDAP_threads.py +++ b/lldb/test/API/tools/lldb-dap/threads/TestDAP_threads.py @@ -50,7 +50,9 @@ def test_thread_format(self): """ program = self.getBuildArtifact("a.out") self.build_and_launch( - program, customThreadFormat="This is thread index #${thread.index}" + program, + customThreadFormat="This is thread index #${thread.index}", + stopCommands=["thread list"], ) source = "main.c" breakpoint_line = line_number(source, "// break here") @@ -63,5 +65,6 @@ def test_thread_format(self): self.continue_to_breakpoints(breakpoint_ids) # We are stopped at the second thread threads = self.dap_server.get_threads() + print("got thread", threads) self.assertEqual(threads[0]["name"], "This is thread index #1") self.assertEqual(threads[1]["name"], "This is thread index #2") diff --git a/lldb/test/API/tools/lldb-dap/variables/children/TestDAP_variables_children.py b/lldb/test/API/tools/lldb-dap/variables/children/TestDAP_variables_children.py index eb09649f387d7..75e75c4ad7c69 100644 --- a/lldb/test/API/tools/lldb-dap/variables/children/TestDAP_variables_children.py +++ b/lldb/test/API/tools/lldb-dap/variables/children/TestDAP_variables_children.py @@ -13,14 +13,11 @@ def test_get_num_children(self): program = self.getBuildArtifact("a.out") self.build_and_launch( program, - stopOnEntry=True, preRunCommands=[ "command script import '%s'" % self.getSourcePath("formatter.py") ], ) source = "main.cpp" - breakpoint1_line = line_number(source, "// break here") - breakpoint_ids = self.set_source_breakpoints( source, [line_number(source, "// break here")] ) @@ -47,7 +44,7 @@ def test_return_variable_with_children(self): Test the stepping out of a function with return value show the children correctly """ program = self.getBuildArtifact("a.out") - self.build_and_launch(program, stopOnEntry=True) + self.build_and_launch(program) function_name = "test_return_variable_with_children" breakpoint_ids = self.set_function_breakpoints([function_name]) diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp index 56a0c38b00037..0d5eba6c40961 100644 --- a/lldb/tools/lldb-dap/DAP.cpp +++ b/lldb/tools/lldb-dap/DAP.cpp @@ -916,20 +916,10 @@ llvm::Error DAP::Loop() { return errWrapper; } - // The launch sequence is special and we need to carefully handle - // packets in the right order. Until we've handled configurationDone, - bool add_to_pending_queue = false; - if (const protocol::Request *req = - std::get_if<protocol::Request>(&*next)) { - llvm::StringRef command = req->command; - if (command == "disconnect") - disconnecting = true; - if (!configuration_done) - add_to_pending_queue = - command != "initialize" && command != "configurationDone" && - command != "disconnect" && !command.ends_with("Breakpoints"); - } + std::get_if<protocol::Request>(&*next); + req && req->arguments == "disconnect") + disconnecting = true; const std::optional<CancelArguments> cancel_args = getArgumentsIfRequest<CancelArguments>(*next, "cancel"); @@ -956,8 +946,7 @@ llvm::Error DAP::Loop() { { std::lock_guard<std::mutex> guard(m_queue_mutex); - auto &queue = add_to_pending_queue ? m_pending_queue : m_queue; - queue.push_back(std::move(*next)); + m_queue.push_back(std::move(*next)); } m_queue_cv.notify_one(); } @@ -1255,16 +1244,6 @@ void DAP::SetConfiguration(const protocol::Configuration &config, SetThreadFormat(*configuration.customThreadFormat); } -void DAP::SetConfigurationDone() { - { - std::lock_guard<std::mutex> guard(m_queue_mutex); - std::copy(m_pending_queue.begin(), m_pending_queue.end(), - std::front_inserter(m_queue)); - configuration_done = true; - } - m_queue_cv.notify_all(); -} - void DAP::SetFrameFormat(llvm::StringRef format) { lldb::SBError error; frame_format = lldb::SBFormat(format.str().c_str(), error); diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index c1a1130b1e59f..8f24c6cf82924 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -444,7 +444,6 @@ struct DAP { /// Queue for all incoming messages. std::deque<protocol::Message> m_queue; - std::deque<protocol::Message> m_pending_queue; std::mutex m_queue_mutex; std::condition_variable m_queue_cv; diff --git a/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp index 0293ffbd0c922..371349a26866e 100644 --- a/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp @@ -140,24 +140,7 @@ Error AttachRequestHandler::Run(const AttachRequestArguments &args) const { } void AttachRequestHandler::PostRun() const { - if (!dap.target.GetProcess().IsValid()) - return; - - // Clients can request a baseline of currently existing threads after - // we acknowledge the configurationDone request. - // Client requests the baseline of currently existing threads after - // a successful or attach by sending a 'threads' request - // right after receiving the configurationDone response. - // Obtain the list of threads before we resume the process - dap.initial_thread_list = - GetThreads(dap.target.GetProcess(), dap.thread_format); - - SendProcessEvent(dap, Attach); - - if (dap.stop_at_entry) - SendThreadStoppedEvent(dap); - else - dap.target.GetProcess().Continue(); + 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 802c28d7b8904..1281857ef4b60 100644 --- a/lldb/tools/lldb-dap/Handler/ConfigurationDoneRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/ConfigurationDoneRequestHandler.cpp @@ -9,49 +9,53 @@ #include "DAP.h" #include "EventHelper.h" #include "JSONUtils.h" +#include "Protocol/ProtocolRequests.h" #include "RequestHandler.h" +#include "lldb/API/SBDebugger.h" + +using namespace llvm; +using namespace lldb_dap::protocol; namespace lldb_dap { -// "ConfigurationDoneRequest": { -// "allOf": [ { "$ref": "#/definitions/Request" }, { -// "type": "object", -// "description": "ConfigurationDone request; value of command field -// is 'configurationDone'.\nThe client of the debug protocol must -// send this request at the end of the sequence of configuration -// requests (which was started by the InitializedEvent).", -// "properties": { -// "command": { -// "type": "string", -// "enum": [ "configurationDone" ] -// }, -// "arguments": { -// "$ref": "#/definitions/ConfigurationDoneArguments" -// } -// }, -// "required": [ "command" ] -// }] -// }, -// "ConfigurationDoneArguments": { -// "type": "object", -// "description": "Arguments for 'configurationDone' request.\nThe -// configurationDone request has no standardized attributes." -// }, -// "ConfigurationDoneResponse": { -// "allOf": [ { "$ref": "#/definitions/Response" }, { -// "type": "object", -// "description": "Response to 'configurationDone' request. This is -// just an acknowledgement, so no body field is required." -// }] -// }, - -void ConfigurationDoneRequestHandler::operator()( - const llvm::json::Object &request) const { - dap.SetConfigurationDone(); - - llvm::json::Object response; - FillResponse(request, response); - dap.SendJSON(llvm::json::Value(std::move(response))); +/// This request indicates that the client has finished initialization of the +/// debug adapter. +/// +/// So it is the last request in the sequence of configuration requests (which +/// was started by the `initialized` event). +/// +/// Clients should only call this request if the corresponding capability +/// `supportsConfigurationDoneRequest` is true. +llvm::Error +ConfigurationDoneRequestHandler::Run(const ConfigurationDoneArguments &) const { + dap.configuration_done = true; + + // Ensure any command scripts did not leave us in an unexpected state. + lldb::SBProcess process = dap.target.GetProcess(); + if (!process.IsValid() || + !lldb::SBDebugger::StateIsStoppedState(process.GetState())) + return make_error<DAPError>( + "Expected process to be stopped.\r\n\r\nProcess is in an unexpected " + "state and may have missed an initial configuration. Please check that " + "any debugger command scripts are not resuming the process during the " + "launch sequence."); + + // Clients can request a baseline of currently existing threads after + // we acknowledge the configurationDone request. + // Client requests the baseline of currently existing threads after + // a successful or attach by sending a 'threads' request + // right after receiving the configurationDone response. + // Obtain the list of threads before we resume the process + dap.initial_thread_list = GetThreads(process, dap.thread_format); + + SendProcessEvent(dap, dap.is_attach ? Attach : Launch); + + if (dap.stop_at_entry) + SendThreadStoppedEvent(dap); + else + process.Continue(); + + return Error::success(); } } // namespace lldb_dap diff --git a/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp index b64987746b3d5..0a178406b5a69 100644 --- a/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp @@ -78,7 +78,3 @@ llvm::Expected<InitializeResponse> InitializeRequestHandler::Run( return dap.GetCapabilities(); } - -void InitializeRequestHandler::PostRun() const { - dap.SendJSON(CreateEventObject("initialized")); -} diff --git a/lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp index 22d1a090187d8..1d7b4b7009462 100644 --- a/lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp @@ -67,25 +67,7 @@ Error LaunchRequestHandler::Run(const LaunchRequestArguments &arguments) const { } void LaunchRequestHandler::PostRun() const { - if (!dap.target.GetProcess().IsValid()) - return; - - // Clients can request a baseline of currently existing threads after - // we acknowledge the configurationDone request. - // Client requests the baseline of currently existing threads after - // a successful or attach by sending a 'threads' request - // right after receiving the configurationDone response. - // Obtain the list of threads before we resume the process - dap.initial_thread_list = - GetThreads(dap.target.GetProcess(), dap.thread_format); - - // Attach happens when launching with runInTerminal. - SendProcessEvent(dap, dap.is_attach ? Attach : Launch); - - if (dap.stop_at_entry) - SendThreadStoppedEvent(dap); - else - dap.target.GetProcess().Continue(); + dap.SendJSON(CreateEventObject("initialized")); } } // namespace lldb_dap diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.h b/lldb/tools/lldb-dap/Handler/RequestHandler.h index 383f9e24a729a..e6bccfe12f402 100644 --- a/lldb/tools/lldb-dap/Handler/RequestHandler.h +++ b/lldb/tools/lldb-dap/Handler/RequestHandler.h @@ -21,6 +21,7 @@ #include "llvm/Support/JSON.h" #include <optional> #include <type_traits> +#include <variant> template <typename T> struct is_optional : std::false_type {}; @@ -87,6 +88,34 @@ class LegacyRequestHandler : public BaseRequestHandler { } }; +template <typename Args> +llvm::Expected<Args> parseArgs(const protocol::Request &request) { + if (!is_optional_v<Args> && !request.arguments) + return llvm::make_error<DAPError>( + llvm::formatv("arguments required for command '{0}' " + "but none received", + request.command) + .str()); + + Args arguments; + llvm::json::Path::Root root("arguments"); + if (request.arguments && !fromJSON(*request.arguments, arguments, root)) { + std::string parse_failure; + llvm::raw_string_ostream OS(parse_failure); + OS << "invalid arguments for request '" << request.command + << "': " << llvm::toString(root.getError()) << "\n"; + root.printErrorContext(*request.arguments, OS); + return llvm::make_error<DAPError>(parse_failure); + } + + return arguments; +} +template <> +inline llvm::Expected<protocol::EmptyArguments> +parseArgs(const protocol::Request &request) { + return std::nullopt; +} + /// Base class for handling DAP requests. Handlers should declare their /// arguments and response body types like: /// @@ -102,42 +131,21 @@ class RequestHandler : public BaseRequestHandler { response.request_seq = request.seq; response.command = request.command; - if (!is_optional_v<Args> && !request.arguments) { - DAP_LOG(dap.log, - "({0}) malformed request {1}, expected arguments but got none", - dap.transport.GetClientName(), request.command); - HandleErrorResponse( - llvm::make_error<DAPError>( - llvm::formatv("arguments required for command '{0}' " - "but none received", - request.command) - .str()), - response); - dap.Send(response); - return; - } - - Args arguments; - llvm::json::Path::Root root("arguments"); - if (request.arguments && !fromJSON(*request.arguments, arguments, root)) { - std::string parse_failure; - llvm::raw_string_ostream OS(parse_failure); - OS << "invalid arguments for request '" << request.command - << "': " << llvm::toString(root.getError()) << "\n"; - root.printErrorContext(*request.arguments, OS); - HandleErrorResponse(llvm::make_error<DAPError>(parse_failure), response); + llvm::Expected<Args> arguments = parseArgs<Args>(request); + if (llvm::Error err = arguments.takeError()) { + HandleErrorResponse(std::move(err), response); dap.Send(response); return; } if constexpr (std::is_same_v<Resp, llvm::Error>) { - if (llvm::Error err = Run(arguments)) { + if (llvm::Error err = Run(*arguments)) { HandleErrorResponse(std::move(err), response); } else { response.success = true; } } else { - Resp body = Run(arguments); + Resp body = Run(*arguments); if (llvm::Error err = body.takeError()) { HandleErrorResponse(std::move(err), response); } else { @@ -246,14 +254,17 @@ class ContinueRequestHandler Run(const protocol::ContinueArguments &args) const override; }; -class ConfigurationDoneRequestHandler : public LegacyRequestHandler { +class ConfigurationDoneRequestHandler + : public RequestHandler<protocol::ConfigurationDoneArguments, + protocol::ConfigurationDoneResponse> { public: - using LegacyRequestHandler::LegacyRequestHandler; + using RequestHandler::RequestHandler; static llvm::StringLiteral GetCommand() { return "configurationDone"; } FeatureSet GetSupportedFeatures() const override { return {protocol::eAdapterFeatureConfigurationDoneRequest}; } - void operator()(const llvm::json::Object &request) const override; + protocol::ConfigurationDoneResponse + Run(const protocol::ConfigurationDoneArguments &) const override; }; class DisconnectRequestHandler @@ -297,7 +308,6 @@ class InitializeRequestHandler static llvm::StringLiteral GetCommand() { return "initialize"; } llvm::Expected<protocol::InitializeResponse> Run(const protocol::InitializeRequestArguments &args) const override; - void PostRun() const override; }; class LaunchRequestHandler diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolBase.h b/lldb/tools/lldb-dap/Protocol/ProtocolBase.h index bad0e886d94d2..1cb9cb13dd0da 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolBase.h +++ b/lldb/tools/lldb-dap/Protocol/ProtocolBase.h @@ -148,6 +148,9 @@ struct ErrorResponseBody { }; llvm::json::Value toJSON(const ErrorResponseBody &); +/// This is a placehold for requests with an empty, null or undefined arguments. +using EmptyArguments = std::optional<std::monostate>; + /// This is just an acknowledgement, so no body field is required. using VoidResponse = llvm::Error; diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h index 4e08b4728453b..b421c631344de 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h +++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h @@ -370,6 +370,13 @@ struct ContinueResponseBody { }; llvm::json::Value toJSON(const ContinueResponseBody &); +/// Arguments for `configurationDone` request. +using ConfigurationDoneArguments = EmptyArguments; + +/// Response to `configurationDone` request. This is just an acknowledgement, so +/// no body field is required. +using ConfigurationDoneResponse = VoidResponse; + /// Arguments for `setVariable` request. struct SetVariableArguments { /// The reference of the variable container. The `variablesReference` must _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
