[Lldb-commits] [lldb] [lldb-dap] Change the launch sequence (reland) (PR #138981)
via lldb-commits
lldb-commits at lists.llvm.org
Wed May 7 15:28:13 PDT 2025
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
- at 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
More information about the lldb-commits
mailing list