-   Notifications  
You must be signed in to change notification settings  - Fork 15.1k
 
[lldb-dap] Correctly trigger 'entry' stop reasons. #165901
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
Conversation
This should ensure we correctly trigger the 'entry' stop reason when launching a process with `"stopOnEntry": true`. I've also updated the tests to ensure we receive the 'entry' stop reason to catch this regression.
|   @llvm/pr-subscribers-lldb Author: John Harrison (ashgti) ChangesNoticed this while looking into test stability that the 'entry' stop reason is not triggering correctly. This should ensure we correctly trigger the 'entry' stop reason when launching a process with  Full diff: https://github.com/llvm/llvm-project/pull/165901.diff 5 Files Affected: 
 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 29935bb8046ff..c6c4a3e2a4e1e 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 @@ -223,6 +223,16 @@ def verify_stop_exception_info(self, expected_description): return True return False + def verify_stop_on_entry(self) -> None: + """Waits for the process to be stopped and then verifies at least one + thread has the stop reason 'entry'.""" + self.dap_server.wait_for_stopped() + self.assertIn( + "entry", + (t["reason"] for t in self.dap_server.thread_stop_reasons.values()), + "Expected at least one thread to report stop reason 'entry' in {self.dap_server.thread_stop_reasons}", + ) + def verify_commands(self, flavor: str, output: str, commands: list[str]): self.assertTrue(output and len(output) > 0, "expect console output") lines = output.splitlines() 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 83faf276852f8..e8e07e1e86fc4 100644 --- a/lldb/test/API/tools/lldb-dap/restart/TestDAP_restart.py +++ b/lldb/test/API/tools/lldb-dap/restart/TestDAP_restart.py @@ -51,20 +51,8 @@ def test_stopOnEntry(self): 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( - len(self.dap_server.thread_stop_reasons) > 0, - "expected stopped event during launch", - ) - for _, body in self.dap_server.thread_stop_reasons.items(): - if "reason" in body: - reason = body["reason"] - self.assertNotEqual( - reason, "breakpoint", 'verify stop isn\'t "main" breakpoint' - ) + self.continue_to_next_stop() + self.verify_stop_on_entry() # Then, if we continue, we should hit the breakpoint at main. self.continue_to_breakpoints([bp_main]) @@ -73,17 +61,7 @@ def test_stopOnEntry(self): # main. resp = self.dap_server.request_restart() self.assertTrue(resp["success"]) - stopped_events = self.dap_server.wait_for_stopped() - for stopped_event in stopped_events: - if "body" in stopped_event: - body = stopped_event["body"] - if "reason" in body: - reason = body["reason"] - self.assertNotEqual( - reason, - "breakpoint", - 'verify stop after restart isn\'t "main" breakpoint', - ) + self.verify_stop_on_entry() @skipIfWindows def test_arguments(self): diff --git a/lldb/test/API/tools/lldb-dap/restart/TestDAP_restart_console.py b/lldb/test/API/tools/lldb-dap/restart/TestDAP_restart_console.py index e1ad1425a993d..7d4949907df0d 100644 --- a/lldb/test/API/tools/lldb-dap/restart/TestDAP_restart_console.py +++ b/lldb/test/API/tools/lldb-dap/restart/TestDAP_restart_console.py @@ -11,27 +11,6 @@ @skipIfBuildType(["debug"]) class TestDAP_restart_console(lldbdap_testcase.DAPTestCaseBase): - def verify_stopped_on_entry(self, stopped_events: List[Dict[str, Any]]): - seen_stopped_event = 0 - for stopped_event in stopped_events: - body = stopped_event.get("body") - if body is None: - continue - - reason = body.get("reason") - if reason is None: - continue - - self.assertNotEqual( - reason, - "breakpoint", - 'verify stop after restart isn\'t "main" breakpoint', - ) - if reason == "entry": - seen_stopped_event += 1 - - self.assertEqual(seen_stopped_event, 1, "expect only one stopped entry event.") - @skipIfAsan @skipIfWindows @skipIf(oslist=["linux"], archs=["arm$"]) # Always times out on buildbot @@ -92,11 +71,8 @@ def test_stopOnEntry(self): self.build_and_launch(program, console="integratedTerminal", stopOnEntry=True) [bp_main] = self.set_function_breakpoints(["main"]) - self.dap_server.request_continue() # sends configuration done - stopped_events = self.dap_server.wait_for_stopped() - # We should be stopped at the entry point. - self.assertGreaterEqual(len(stopped_events), 0, "expect stopped events") - self.verify_stopped_on_entry(stopped_events) + self.dap_server.request_configurationDone() + self.verify_stop_on_entry() # Then, if we continue, we should hit the breakpoint at main. self.dap_server.request_continue() @@ -105,8 +81,7 @@ def test_stopOnEntry(self): # Restart and check that we still get a stopped event before reaching # main. self.dap_server.request_restart() - stopped_events = self.dap_server.wait_for_stopped() - self.verify_stopped_on_entry(stopped_events) + self.verify_stop_on_entry() # continue to main self.dap_server.request_continue() diff --git a/lldb/tools/lldb-dap/EventHelper.cpp b/lldb/tools/lldb-dap/EventHelper.cpp index c5d5f2bb59b42..12d9e21c52ab3 100644 --- a/lldb/tools/lldb-dap/EventHelper.cpp +++ b/lldb/tools/lldb-dap/EventHelper.cpp @@ -176,7 +176,7 @@ llvm::Error SendThreadStoppedEvent(DAP &dap, bool on_entry) { llvm::DenseSet<lldb::tid_t> old_thread_ids; old_thread_ids.swap(dap.thread_ids); - uint32_t stop_id = process.GetStopID(); + uint32_t stop_id = on_entry ? 0 : process.GetStopID(); const uint32_t num_threads = process.GetNumThreads(); // First make a pass through the threads to see if the focused thread diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp index 2780a5b7748e8..1a3a6701b194d 100644 --- a/lldb/tools/lldb-dap/JSONUtils.cpp +++ b/lldb/tools/lldb-dap/JSONUtils.cpp @@ -711,7 +711,7 @@ llvm::json::Value CreateThreadStopped(DAP &dap, lldb::SBThread &thread, break; } if (stop_id == 0) - body.try_emplace("reason", "entry"); + body["reason"] = "entry"; const lldb::tid_t tid = thread.GetThreadID(); body.try_emplace("threadId", (int64_t)tid); // If no description has been set, then set it to the default thread stopped  |  
Noticed this while looking into test stability that the 'entry' stop reason is not triggering correctly. This should ensure we correctly trigger the 'entry' stop reason when launching a process with `"stopOnEntry": true`. I've also updated the tests to ensure we receive the 'entry' stop reason to catch this regression.
Noticed this while looking into test stability that the 'entry' stop reason is not triggering correctly. This should ensure we correctly trigger the 'entry' stop reason when launching a process with
"stopOnEntry": true. I've also updated the tests to ensure we receive the 'entry' stop reason to catch this regression.