llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Jonas Devlieghere (JDevlieghere) <details> <summary>Changes</summary> This PR changes how we treat the launch sequence in lldb-dap. - Send the initialized event after we finish handling the initialize request, rather than after we finish attaching or launching. - Delay handling the launch and attach request until we have handled the configurationDone request. The latter is now largely a NO-OP and only exists to signal lldb-dap that it can handle the launch and attach requests. - Delay handling the initial threads requests until we have handled the launch or attach request. - Make all attaching and launching synchronous, including when we have attach or launch commands. This removes the need to synchronize between the request and event thread. Background: https://discourse.llvm.org/t/reliability-of-the-lldb-dap-tests/86125 --- Patch is 54.05 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/138981.diff 30 Files Affected: - (modified) lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py (+34-31) - (modified) lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py (+55) - (modified) lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py (+2) - (modified) lldb/test/API/tools/lldb-dap/attach/TestDAP_attachByPortNum.py (+4-4) - (modified) lldb/test/API/tools/lldb-dap/breakpoint-events/TestDAP_breakpointEvents.py (+18-43) - (modified) lldb/test/API/tools/lldb-dap/completions/TestDAP_completions.py (+16-10) - (modified) lldb/test/API/tools/lldb-dap/console/TestDAP_console.py (+6-5) - (modified) lldb/test/API/tools/lldb-dap/console/TestDAP_redirection_to_console.py (+3-1) - (modified) lldb/test/API/tools/lldb-dap/disconnect/TestDAP_disconnect.py (+5-1) - (modified) lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py (+4-1) - (modified) lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py (+4-2) - (modified) lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py (+1-1) - (modified) lldb/test/API/tools/lldb-dap/repl-mode/TestDAP_repl_mode_detection.py (+1-1) - (modified) lldb/test/API/tools/lldb-dap/restart/TestDAP_restart.py (-1) - (modified) lldb/test/API/tools/lldb-dap/restart/TestDAP_restart_runInTerminal.py (-1) - (modified) lldb/test/API/tools/lldb-dap/send-event/TestDAP_sendEvent.py (+2-5) - (modified) lldb/test/API/tools/lldb-dap/stackTrace/TestDAP_stackTrace.py (+1-1) - (modified) lldb/test/API/tools/lldb-dap/stackTraceDisassemblyDisplay/TestDAP_stackTraceDisassemblyDisplay.py (+1-1) - (modified) lldb/test/API/tools/lldb-dap/startDebugging/TestDAP_startDebugging.py (+1-2) - (modified) lldb/test/API/tools/lldb-dap/stop-hooks/TestDAP_stop_hooks.py (+1-1) - (modified) lldb/test/API/tools/lldb-dap/variables/children/TestDAP_variables_children.py (+2-2) - (modified) lldb/tools/lldb-dap/DAP.cpp (+31-8) - (modified) lldb/tools/lldb-dap/DAP.h (+6-2) - (modified) lldb/tools/lldb-dap/EventHelper.cpp (+1-1) - (modified) lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp (+66-49) - (modified) lldb/tools/lldb-dap/Handler/ConfigurationDoneRequestHandler.cpp (+2-12) - (modified) lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp (+17-27) - (modified) lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp (+5-2) - (modified) lldb/tools/lldb-dap/Handler/RequestHandler.cpp (+40-27) - (modified) lldb/tools/lldb-dap/Handler/RequestHandler.h (+1) ``````````diff 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 6d9ab770684f1..e10342b72f4f0 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 @@ -132,7 +132,6 @@ def __init__(self, recv, send, init_commands, log_file=None): self.exit_status = None self.initialize_body = None self.thread_stop_reasons = {} - self.breakpoint_events = [] self.progress_events = [] self.reverse_requests = [] self.module_events = [] @@ -244,13 +243,6 @@ def handle_recv_packet(self, packet): self._process_stopped() tid = body["threadId"] self.thread_stop_reasons[tid] = body - elif event == "breakpoint": - # Breakpoint events come in when a breakpoint has locations - # added or removed. Keep track of them so we can look for them - # in tests. - self.breakpoint_events.append(packet) - # no need to add 'breakpoint' event packets to our packets list - return keepGoing elif event.startswith("progress"): # Progress events come in as 'progressStart', 'progressUpdate', # and 'progressEnd' events. Keep these around in case test @@ -412,6 +404,15 @@ def wait_for_stopped(self, timeout=None): self.threads = [] return stopped_events + def wait_for_breakpoint_events(self, timeout=None): + breakpoint_events = [] + while True: + event = self.wait_for_event("breakpoint", timeout=timeout) + if not event: + break + breakpoint_events.append(event) + return breakpoint_events + def wait_for_exited(self): event_dict = self.wait_for_event("exited") if event_dict is None: @@ -591,6 +592,7 @@ def request_attach( attachCommands=None, terminateCommands=None, coreFile=None, + stopOnAttach=True, postRunCommands=None, sourceMap=None, gdbRemotePort=None, @@ -620,6 +622,8 @@ def request_attach( args_dict["attachCommands"] = attachCommands if coreFile: args_dict["coreFile"] = coreFile + if stopOnAttach: + args_dict["stopOnEntry"] = stopOnAttach if postRunCommands: args_dict["postRunCommands"] = postRunCommands if sourceMap: @@ -632,7 +636,7 @@ def request_attach( response = self.send_recv(command_dict) if response["success"]: - self.wait_for_events(["process", "initialized"]) + self.wait_for_event("process") return response def request_breakpointLocations( @@ -666,10 +670,6 @@ def request_configurationDone(self): response = self.send_recv(command_dict) if response: self.configuration_done_sent = True - # Client requests the baseline of currently existing threads after - # a successful launch or attach. - # Kick off the threads request that follows - self.request_threads() return response def _process_stopped(self): @@ -887,7 +887,7 @@ def request_launch( response = self.send_recv(command_dict) if response["success"]: - self.wait_for_events(["process", "initialized"]) + self.wait_for_event("process") return response def request_next(self, threadId, granularity="statement"): @@ -1325,6 +1325,26 @@ def attach_options_specified(options): def run_vscode(dbg, args, options): dbg.request_initialize(options.sourceInitFile) + + if options.sourceBreakpoints: + source_to_lines = {} + for file_line in options.sourceBreakpoints: + (path, line) = file_line.split(":") + if len(path) == 0 or len(line) == 0: + print('error: invalid source with line "%s"' % (file_line)) + + else: + if path in source_to_lines: + source_to_lines[path].append(int(line)) + else: + source_to_lines[path] = [int(line)] + for source in source_to_lines: + dbg.request_setBreakpoints(source, source_to_lines[source]) + if options.funcBreakpoints: + dbg.request_setFunctionBreakpoints(options.funcBreakpoints) + + dbg.request_configurationDone() + if attach_options_specified(options): response = dbg.request_attach( program=options.program, @@ -1353,23 +1373,6 @@ def run_vscode(dbg, args, options): ) if response["success"]: - if options.sourceBreakpoints: - source_to_lines = {} - for file_line in options.sourceBreakpoints: - (path, line) = file_line.split(":") - if len(path) == 0 or len(line) == 0: - print('error: invalid source with line "%s"' % (file_line)) - - else: - if path in source_to_lines: - source_to_lines[path].append(int(line)) - else: - source_to_lines[path] = [int(line)] - for source in source_to_lines: - dbg.request_setBreakpoints(source, source_to_lines[source]) - if options.funcBreakpoints: - dbg.request_setFunctionBreakpoints(options.funcBreakpoints) - dbg.request_configurationDone() dbg.wait_for_stopped() else: if "message" in response: 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 2c14bb35162b5..c5a7eb76a58c7 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 @@ -340,6 +340,7 @@ def attach( exitCommands=None, attachCommands=None, coreFile=None, + stopOnAttach=True, disconnectAutomatically=True, terminateCommands=None, postRunCommands=None, @@ -348,6 +349,8 @@ def attach( expectFailure=False, gdbRemotePort=None, gdbRemoteHostname=None, + sourceBreakpoints=None, + functionBreakpoints=None, ): """Build the default Makefile target, create the DAP debug adapter, and attach to the process. @@ -364,6 +367,28 @@ def cleanup(): self.addTearDownHook(cleanup) # Initialize and launch the program self.dap_server.request_initialize(sourceInitFile) + self.dap_server.wait_for_event("initialized") + + # 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( program=program, pid=pid, @@ -376,6 +401,7 @@ def cleanup(): attachCommands=attachCommands, terminateCommands=terminateCommands, coreFile=coreFile, + stopOnAttach=stopOnAttach, postRunCommands=postRunCommands, sourceMap=sourceMap, gdbRemotePort=gdbRemotePort, @@ -419,6 +445,8 @@ def launch( commandEscapePrefix=None, customFrameFormat=None, customThreadFormat=None, + sourceBreakpoints=None, + functionBreakpoints=None, ): """Sending launch request to dap""" @@ -434,6 +462,29 @@ def cleanup(): # Initialize and launch the program self.dap_server.request_initialize(sourceInitFile) + self.dap_server.wait_for_event("initialized") + + # 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, args=args, @@ -504,6 +555,8 @@ def build_and_launch( customThreadFormat=None, launchCommands=None, expectFailure=False, + sourceBreakpoints=None, + functionBreakpoints=None, ): """Build the default Makefile target, create the DAP debug adapter, and launch the process. @@ -540,6 +593,8 @@ def build_and_launch( customThreadFormat=customThreadFormat, launchCommands=launchCommands, expectFailure=expectFailure, + sourceBreakpoints=sourceBreakpoints, + functionBreakpoints=functionBreakpoints, ) def getBuiltinDebugServerTool(self): 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 f48d5a7db3c50..741c011a3d692 100644 --- a/lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py +++ b/lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py @@ -27,6 +27,8 @@ def spawn_and_wait(program, delay): @skip class TestDAP_attach(lldbdap_testcase.DAPTestCaseBase): def set_and_hit_breakpoint(self, continueToExit=True): + self.dap_server.wait_for_stopped() + source = "main.c" breakpoint1_line = line_number(source, "// breakpoint 1") lines = [breakpoint1_line] 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 7f93b9f2a3a22..7250e67ebcd8c 100644 --- a/lldb/test/API/tools/lldb-dap/attach/TestDAP_attachByPortNum.py +++ b/lldb/test/API/tools/lldb-dap/attach/TestDAP_attachByPortNum.py @@ -18,17 +18,17 @@ import socket -@skip class TestDAP_attachByPortNum(lldbdap_testcase.DAPTestCaseBase): default_timeout = 20 def set_and_hit_breakpoint(self, continueToExit=True): + self.dap_server.wait_for_stopped() + source = "main.c" - main_source_path = os.path.join(os.getcwd(), source) - breakpoint1_line = line_number(main_source_path, "// breakpoint 1") + breakpoint1_line = line_number(source, "// breakpoint 1") lines = [breakpoint1_line] # Set breakpoint in the thread function so we can step the threads - breakpoint_ids = self.set_source_breakpoints(main_source_path, lines) + breakpoint_ids = self.set_source_breakpoints(source, lines) self.assertEqual( len(breakpoint_ids), len(lines), "expect correct number of breakpoints" ) 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 e5590e1b332a0..8581f10cef22a 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 @@ -81,52 +81,27 @@ def test_breakpoint_events(self): breakpoint["verified"], "expect foo breakpoint to not be verified" ) - # Get the stop at the entry point - self.continue_to_next_stop() + # Make sure we're stopped. + self.dap_server.wait_for_stopped() - # We are now stopped at the entry point to the program. Shared - # libraries are not loaded yet (at least on macOS they aren't) and only - # the breakpoint in the main executable should be resolved. - self.assertEqual(len(self.dap_server.breakpoint_events), 1) - event = self.dap_server.breakpoint_events[0] - body = event["body"] - self.assertEqual( - body["reason"], "changed", "breakpoint event should say changed" - ) - breakpoint = body["breakpoint"] - self.assertEqual(breakpoint["id"], main_bp_id) - self.assertTrue(breakpoint["verified"], "main breakpoint should be resolved") - - # Clear the list of breakpoint events so we don't see this one again. - self.dap_server.breakpoint_events.clear() + # Flush the breakpoint events. + self.dap_server.wait_for_breakpoint_events(timeout=5) # Continue to the breakpoint self.continue_to_breakpoints(dap_breakpoint_ids) - # When the process launches, we first expect to see both the main and - # foo breakpoint as unresolved. - for event in self.dap_server.breakpoint_events[:2]: - body = event["body"] - self.assertEqual( - body["reason"], "changed", "breakpoint event should say changed" - ) - breakpoint = body["breakpoint"] - self.assertIn(str(breakpoint["id"]), dap_breakpoint_ids) - self.assertFalse(breakpoint["verified"], "breakpoint should be unresolved") + verified_breakpoint_ids = [] + unverified_breakpoint_ids = [] + for breakpoint_event in self.dap_server.wait_for_breakpoint_events(timeout=5): + breakpoint = breakpoint_event["body"]["breakpoint"] + id = breakpoint["id"] + if breakpoint["verified"]: + verified_breakpoint_ids.append(id) + else: + unverified_breakpoint_ids.append(id) - # Then, once the dynamic loader has given us a load address, they - # should show up as resolved again. - for event in self.dap_server.breakpoint_events[3:]: - body = event["body"] - self.assertEqual( - body["reason"], "changed", "breakpoint event should say changed" - ) - breakpoint = body["breakpoint"] - self.assertIn(str(breakpoint["id"]), dap_breakpoint_ids) - self.assertTrue(breakpoint["verified"], "breakpoint should be resolved") - self.assertNotIn( - "source", - breakpoint, - "breakpoint event should not return a source object", - ) - self.assertIn("line", breakpoint, "breakpoint event should have line") + self.assertIn(main_bp_id, unverified_breakpoint_ids) + self.assertIn(foo_bp_id, unverified_breakpoint_ids) + + self.assertIn(main_bp_id, verified_breakpoint_ids) + self.assertIn(foo_bp_id, verified_breakpoint_ids) 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 210e591bff426..a94288c7a669e 100644 --- a/lldb/test/API/tools/lldb-dap/completions/TestDAP_completions.py +++ b/lldb/test/API/tools/lldb-dap/completions/TestDAP_completions.py @@ -2,7 +2,6 @@ Test lldb-dap completions request """ - import lldbdap_testcase import dap_server from lldbsuite.test import lldbutil @@ -32,6 +31,7 @@ variable_var1_completion = {"text": "var1", "label": "var1 -- int &"} variable_var2_completion = {"text": "var2", "label": "var2 -- int &"} + # Older version of libcxx produce slightly different typename strings for # templates like vector. @skipIf(compiler="clang", compiler_version=["<", "16.0"]) @@ -43,16 +43,22 @@ def verify_completions(self, actual_list, expected_list, not_expected_list=[]): for not_expected_item in not_expected_list: self.assertNotIn(not_expected_item, actual_list) - - def setup_debugee(self): + def setup_debugee(self, stopOnEntry=False): program = self.getBuildArtifact("a.out") - self.build_and_launch(program) - source = "main.cpp" - breakpoint1_line = line_number(source, "// breakpoint 1") - breakpoint2_line = line_number(source, "// breakpoint 2") - - self.set_source_breakpoints(source, [breakpoint1_line, breakpoint2_line]) + self.build_and_launch( + program, + stopOnEntry=stopOnEntry, + sourceBreakpoints=[ + ( + source, + [ + line_number(source, "// breakpoint 1"), + line_number(source, "// breakpoint 2"), + ], + ), + ], + ) def test_command_completions(self): """ @@ -235,7 +241,7 @@ def test_auto_completions(self): """ Tests completion requests in "repl-mode=auto" """ - self.setup_debugee() + self.setup_debugee(stopOnEntry=True) res = self.dap_server.request_evaluate( "`lldb-dap repl-mode auto", context="repl" 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 b07c4f871d73b..8642e317f9b3a 100644 --- a/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py +++ b/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py @@ -19,6 +19,7 @@ def get_subprocess(root_process, process_name): self.assertTrue(False, "No subprocess with name %s found" % process_name) + class TestDAP_console(lldbdap_testcase.DAPTestCaseBase): def check_lldb_command( self, lldb_command, contains_string, assert_msg, command_escape_prefix="`" @@ -52,7 +53,7 @@ def test_scopes_variables_setVariable_evaluate(self): character. """ program = self.getBuildArtifact("a.out") - self.build_and_launch(program) + self.build_and_launch(program, stopOnEntry=True) source = "main.cpp" breakpoint1_line = line_number(source, "// breakpoint 1") lines = [breakpoint1_line] @@ -81,7 +82,7 @@ def test_scopes_variables_setVariable_evaluate(self): def test_custom_escape_prefix(self): program = self.getBuildArtifact("a.out") - self.build_and_launch(program, commandEscapePrefix="::") + self.build_and_launch(program, stopOnEntry=True, commandEscapePrefix="::") source = "main.cpp" breakpoint1_line = line_number(source, "// breakpoint 1") breakpoint_ids = self.set_source_breakpoints(source, [breakpoint1_line]) @@ -96,7 +97,7 @@ def test_custom_escape_prefix(self): def test_empty_escape_prefix(self): program = self.getBuildArtifact("a.out") - self.build_and_launch(program, commandEscapePrefix="") + self.build_and_launch(program, stopOnEntry=True, commandEscapePrefix="") source = "main.cpp" breakpoint1_line = line_number(source, "// breakpoint 1") breakpoint_ids = self.set_source_breakpoints(source, [breakpoint1_line]) @@ -113,7 +114,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, commandEscapePrefix="") + self.build_and_launch(program, stopOnEntry=True, commandEscapePrefix="") breakpoint1_line = line_number(source, "// breakpoint 1") breakpoint_ids = self.set_source_breakpoints(source, [breakpoint1_line]) self.continue_to_breakpoints(breakpoint_ids) @@ -167,7 +168,7 @@ def test_exit_status_message_ok(self): de... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/138981 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
