Skip to content

[lldb-dap] Change the launch sequence #138219

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 6, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 34 additions & 31 deletions lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = []
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -591,6 +592,7 @@ def request_attach(
attachCommands=None,
terminateCommands=None,
coreFile=None,
stopOnAttach=True,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we default to False for attach requests?

postRunCommands=None,
sourceMap=None,
gdbRemotePort=None,
Expand Down Expand Up @@ -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:
Expand All @@ -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(
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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"):
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,7 @@ def attach(
exitCommands=None,
attachCommands=None,
coreFile=None,
stopOnAttach=True,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we default this to False?

Or should we have the DAPTestCaseBase case attach and launch helpers take an initialSourceBreakpoints, initialFunctionBreakpoints, initialExceptionBreakpoints parameter and we help the tests by sending these as part of the setup flow, kind of how we have disconnectAutomatically as a flag to handle shutdown flows automatically.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The way the tests are currently written, they expect to be stopped after the launch (presumably because these tests were never sending the configurationDone request). I went with this default to keep the changes minimal, but I like the idea of the initial breakpoints as a way to improve this in a subsequent PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good, we can follow up with some other improvements to tests.

disconnectAutomatically=True,
terminateCommands=None,
postRunCommands=None,
Expand All @@ -364,6 +365,8 @@ def cleanup():
self.addTearDownHook(cleanup)
# Initialize and launch the program
self.dap_server.request_initialize(sourceInitFile)
self.dap_server.wait_for_event("initialized")
self.dap_server.request_configurationDone()
response = self.dap_server.request_attach(
program=program,
pid=pid,
Expand All @@ -376,6 +379,7 @@ def cleanup():
attachCommands=attachCommands,
terminateCommands=terminateCommands,
coreFile=coreFile,
stopOnAttach=stopOnAttach,
postRunCommands=postRunCommands,
sourceMap=sourceMap,
gdbRemotePort=gdbRemotePort,
Expand Down Expand Up @@ -434,6 +438,9 @@ def cleanup():

# Initialize and launch the program
self.dap_server.request_initialize(sourceInitFile)
self.dap_server.wait_for_event("initialized")
self.dap_server.request_configurationDone()

response = self.dap_server.request_launch(
program,
args=args,
Expand Down
2 changes: 2 additions & 0 deletions lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,9 @@ def verify_completions(self, actual_list, expected_list, 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)
self.build_and_launch(program, stopOnEntry=stopOnEntry)

source = "main.cpp"
breakpoint1_line = line_number(source, "// breakpoint 1")
Expand Down Expand Up @@ -235,7 +235,7 @@ def test_auto_completions(self):
"""
Tests completion requests in "repl-mode=auto"
"""
self.setup_debugee()
self.setup_debugee(stopOnEntry=True)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason why we are changing the test behavior. I believe currently they will be false.


res = self.dap_server.request_evaluate(
"`lldb-dap repl-mode auto", context="repl"
Expand Down
2 changes: 1 addition & 1 deletion lldb/test/API/tools/lldb-dap/console/TestDAP_console.py
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ def test_exit_status_message_ok(self):

def test_diagnositcs(self):
program = self.getBuildArtifact("a.out")
self.build_and_launch(program)
self.build_and_launch(program, stopOnEntry=True)

core = self.getBuildArtifact("minidump.core")
self.yaml2obj("minidump.yaml", core)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,19 @@ def test_launch(self):
created.
"""
program = self.getBuildArtifact("a.out")
self.build_and_launch(program, disconnectAutomatically=False)
self.build_and_launch(program, stopOnEntry=True, disconnectAutomatically=False)

# We set a breakpoint right before the side effect file is created
self.set_source_breakpoints(
self.source, [line_number(self.source, "// breakpoint")]
)
self.continue_to_next_stop()

# verify we haven't produced the side effect file yet
self.assertFalse(os.path.exists(program + ".side_effect"))

self.dap_server.request_disconnect()

# verify we didn't produce the side effect file
time.sleep(1)
self.assertFalse(os.path.exists(program + ".side_effect"))
Expand Down
5 changes: 4 additions & 1 deletion lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from lldbsuite.test.decorators import *
from lldbsuite.test.lldbtest import *


# DAP tests are flakey, see https://github.com/llvm/llvm-project/issues/137660.
@skip
class TestDAP_evaluate(lldbdap_testcase.DAPTestCaseBase):
Expand Down Expand Up @@ -42,7 +43,9 @@ def run_test_evaluate_expressions(
self.context = context
program = self.getBuildArtifact("a.out")
self.build_and_launch(
program, enableAutoVariableSummaries=enableAutoVariableSummaries
program,
enableAutoVariableSummaries=enableAutoVariableSummaries,
stopOnEntry=True,
)
source = "main.cpp"
self.set_source_breakpoints(
Expand Down
4 changes: 2 additions & 2 deletions lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,8 @@ def test_stopOnEntry(self):
"""
program = self.getBuildArtifact("a.out")
self.build_and_launch(program, stopOnEntry=True)
self.set_function_breakpoints(["main"])
stopped_events = self.continue_to_next_stop()

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: probs should update the line 98 & 99 below

                    self.assertEqual(
                        reason, "singal", 'verify stop reason is SIGSTOP'
                    )

stopped_events = self.dap_server.wait_for_stopped()
for stopped_event in stopped_events:
if "body" in stopped_event:
body = stopped_event["body"]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ def verify_progress_events(
@skipIfWindows
def test(self):
program = self.getBuildArtifact("a.out")
self.build_and_launch(program)
self.build_and_launch(program, stopOnEntry=True)
progress_emitter = os.path.join(os.getcwd(), "Progress_emitter.py")
self.dap_server.request_evaluate(
f"`command script import {progress_emitter}", context="repl"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def assertEvaluate(self, expression, regex):

def test_completions(self):
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")
Expand Down
1 change: 0 additions & 1 deletion lldb/test/API/tools/lldb-dap/restart/TestDAP_restart.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ def test_basic_functionality(self):
[bp_A, bp_B] = self.set_source_breakpoints("main.c", [line_A, line_B])

# Verify we hit A, then B.
self.dap_server.request_configurationDone()
self.verify_breakpoint_hit([bp_A])
self.dap_server.request_continue()
self.verify_breakpoint_hit([bp_B])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ def test_stopOnEntry(self):
program = self.getBuildArtifact("a.out")
self.build_and_launch(program, runInTerminal=True, stopOnEntry=True)
[bp_main] = self.set_function_breakpoints(["main"])
self.dap_server.request_configurationDone()

# When using stopOnEntry, configurationDone doesn't result in a running
# process, we should immediately get a stopped event instead.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ def test_stop_hooks_before_run(self):
self.build_and_launch(program, stopOnEntry=True, preRunCommands=preRunCommands)

# The first stop is on entry.
self.continue_to_next_stop()
self.dap_server.wait_for_stopped()

breakpoint_ids = self.set_function_breakpoints(["main"])
# This request hangs if the race happens, because, in that case, the
Expand Down
Loading
Loading