[Lldb-commits] [lldb] [lldb-dap] Take two at refactoring the startup sequence. (PR #140331)
John Harrison via lldb-commits
lldb-commits at lists.llvm.org
Fri May 16 18:47:52 PDT 2025
https://github.com/ashgti created https://github.com/llvm/llvm-project/pull/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.
>From 71c7830f569c0b13045ad4b0301973e606a2f06e Mon Sep 17 00:00:00 2001
From: John Harrison <harjohn at google.com>
Date: Fri, 16 May 2025 18:35:28 -0700
Subject: [PATCH] [lldb-dap] Take two at refactoring the startup sequence.
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.
---
.../test/tools/lldb-dap/dap_server.py | 24 +++---
.../test/tools/lldb-dap/lldbdap_testcase.py | 70 +---------------
.../TestDAP_breakpointEvents.py | 2 +-
.../tools/lldb-dap/cancel/TestDAP_cancel.py | 4 +-
.../completions/TestDAP_completions.py | 17 ++--
.../tools/lldb-dap/console/TestDAP_console.py | 13 +--
.../console/TestDAP_redirection_to_console.py | 4 +-
.../lldb-dap/coreFile/TestDAP_coreFile.py | 1 +
.../lldb-dap/evaluate/TestDAP_evaluate.py | 1 -
.../tools/lldb-dap/launch/TestDAP_launch.py | 3 +-
.../module-event/TestDAP_module_event.py | 2 +-
.../tools/lldb-dap/module/TestDAP_module.py | 4 +-
.../repl-mode/TestDAP_repl_mode_detection.py | 2 +-
.../tools/lldb-dap/restart/TestDAP_restart.py | 5 +-
.../lldb-dap/send-event/TestDAP_sendEvent.py | 3 +-
.../lldb-dap/stackTrace/TestDAP_stackTrace.py | 2 +-
.../TestDAP_stackTraceDisassemblyDisplay.py | 2 +-
.../startDebugging/TestDAP_startDebugging.py | 2 +-
.../lldb-dap/stop-hooks/TestDAP_stop_hooks.py | 2 +-
.../tools/lldb-dap/threads/TestDAP_threads.py | 5 +-
.../children/TestDAP_variables_children.py | 5 +-
lldb/tools/lldb-dap/DAP.cpp | 29 +------
lldb/tools/lldb-dap/DAP.h | 1 -
.../lldb-dap/Handler/AttachRequestHandler.cpp | 19 +----
.../ConfigurationDoneRequestHandler.cpp | 82 ++++++++++---------
.../Handler/InitializeRequestHandler.cpp | 4 -
.../lldb-dap/Handler/LaunchRequestHandler.cpp | 20 +----
lldb/tools/lldb-dap/Handler/RequestHandler.h | 62 ++++++++------
lldb/tools/lldb-dap/Protocol/ProtocolBase.h | 3 +
.../lldb-dap/Protocol/ProtocolRequests.h | 7 ++
30 files changed, 150 insertions(+), 250 deletions(-)
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..d769ac51096b8 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",
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..cbcff179ed9d1 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,10 +131,8 @@ 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);
+ llvm::Expected<Args> arguments = parseArgs<Args>(request);
+ if (!arguments) {
HandleErrorResponse(
llvm::make_error<DAPError>(
llvm::formatv("arguments required for command '{0}' "
@@ -117,27 +144,14 @@ class RequestHandler : public BaseRequestHandler {
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);
- 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 +260,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 +314,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
More information about the lldb-commits
mailing list