[lldb-dap] Change the launch sequence (reland) #138981
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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
|
@llvm/pr-subscribers-lldb Author: Jonas Devlieghere (JDevlieghere) ChangesThis PR changes how we treat the launch sequence in lldb-dap.
Background: 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:
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]
|
JDevlieghere
force-pushed
the
lldb-dap-launch-sequence-reland
branch
from
3e7b245 to
9b091ec
Compare
JDevlieghere
force-pushed
the
lldb-dap-launch-sequence-reland
branch
from
9b091ec to
1ac9193
Compare
DavidSpickett
added a commit
that referenced
this pull request
May 8, 2025
Since #138981 / aeeb9a3 were landed and tests re-enabled, these tests have been failing on our Windows on Arm bot: https://lab.llvm.org/buildbot/#/builders/141/builds/8523 ******************** Unresolved Tests (1): lldb-api :: tools/lldb-dap/send-event/TestDAP_sendEvent.py ******************** Failed Tests (2): lldb-api :: tools/lldb-dap/launch/TestDAP_launch.py lldb-api :: tools/lldb-dap/stackTrace/TestDAP_stackTrace.py
|
A few tests still failing on Windows - https://lab.llvm.org/buildbot/#/builders/141/builds/8523 Looks like your previous theory, does the test need updating somehow? Seems like it was expecting one kind of response, but got a different one. Again could be what you said, if the program never stopped that explains why there are no stack frames here. |
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
May 8, 2025
Since llvm/llvm-project#138981 / llvm/llvm-project@aeeb9a3 were landed and tests re-enabled, these tests have been failing on our Windows on Arm bot: https://lab.llvm.org/buildbot/#/builders/141/builds/8523 ******************** Unresolved Tests (1): lldb-api :: tools/lldb-dap/send-event/TestDAP_sendEvent.py ******************** Failed Tests (2): lldb-api :: tools/lldb-dap/launch/TestDAP_launch.py lldb-api :: tools/lldb-dap/stackTrace/TestDAP_stackTrace.py
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR changes how we treat the launch sequence in lldb-dap.
request, rather than after we finish attaching or launching.
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.
the launch or attach request.
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