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 11573eba06907..e5590e1b332a0 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 @@ -2,7 +2,6 @@ Test lldb-dap setBreakpoints request """ - import dap_server from lldbsuite.test.decorators import * from lldbsuite.test.lldbtest import * @@ -12,9 +11,7 @@ class TestDAP_breakpointEvents(lldbdap_testcase.DAPTestCaseBase): - @skipIfWindows @skipUnlessDarwin - @expectedFailureAll(macos_version=[">=", "13.0"]) def test_breakpoint_events(self): """ This test sets a breakpoint in a shared library and runs and stops @@ -63,70 +60,73 @@ def test_breakpoint_events(self): response = self.dap_server.request_setBreakpoints( main_source_path, [main_bp_line] ) - if response: - breakpoints = response["body"]["breakpoints"] - for breakpoint in breakpoints: - main_bp_id = breakpoint["id"] - dap_breakpoint_ids.append("%i" % (main_bp_id)) - # line = breakpoint['line'] - self.assertTrue( - breakpoint["verified"], "expect main breakpoint to be verified" - ) + self.assertTrue(response) + breakpoints = response["body"]["breakpoints"] + for breakpoint in breakpoints: + main_bp_id = breakpoint["id"] + dap_breakpoint_ids.append("%i" % (main_bp_id)) + self.assertTrue( + breakpoint["verified"], "expect main breakpoint to be verified" + ) response = self.dap_server.request_setBreakpoints( foo_source_path, [foo_bp1_line] ) - if response: - breakpoints = response["body"]["breakpoints"] - for breakpoint in breakpoints: - foo_bp_id = breakpoint["id"] - dap_breakpoint_ids.append("%i" % (foo_bp_id)) - self.assertFalse( - breakpoint["verified"], "expect foo breakpoint to not be verified" - ) + self.assertTrue(response) + breakpoints = response["body"]["breakpoints"] + for breakpoint in breakpoints: + foo_bp_id = breakpoint["id"] + dap_breakpoint_ids.append("%i" % (foo_bp_id)) + self.assertFalse( + breakpoint["verified"], "expect foo breakpoint to not be verified" + ) # Get the stop at the entry point self.continue_to_next_stop() # 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 any - # breakpoints set in foo.cpp should not be resolved. + # 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( - len(self.dap_server.breakpoint_events), - 0, - "no breakpoint events when stopped at entry point", + 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() # Continue to the breakpoint self.continue_to_breakpoints(dap_breakpoint_ids) - # Make sure we only get an event for the breakpoint we set via a call - # to self.dap_server.request_setBreakpoints(...), not the breakpoint - # we set with with a LLDB command in preRunCommands. - self.assertEqual( - len(self.dap_server.breakpoint_events), - 1, - "make sure we got a breakpoint event", - ) - event = self.dap_server.breakpoint_events[0] - # Verify the details of the breakpoint changed notification. - body = event["body"] - self.assertEqual( - body["reason"], "changed", "breakpoint event is says breakpoint is changed" - ) - breakpoint = body["breakpoint"] - self.assertTrue( - breakpoint["verified"], "breakpoint event is says it is verified" - ) - self.assertEqual( - breakpoint["id"], - foo_bp_id, - "breakpoint event is for breakpoint %i" % (foo_bp_id), - ) - self.assertTrue( - "line" in breakpoint and breakpoint["line"] > 0, - "breakpoint event is has a line number", - ) - self.assertNotIn( - "source", breakpoint, "breakpoint event should not return a source object" - ) + # 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") + + # 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") diff --git a/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp index e9041f3985523..d0196088a59b7 100644 --- a/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp @@ -137,27 +137,29 @@ static void EventThreadFunction(DAP &dap) { lldb::SBBreakpoint::GetBreakpointEventTypeFromEvent(event); auto bp = Breakpoint( dap, lldb::SBBreakpoint::GetBreakpointFromEvent(event)); - // If the breakpoint was originated from the IDE, it will have the - // BreakpointBase::GetBreakpointLabel() label attached. Regardless - // of wether the locations were added or removed, the breakpoint - // ins't going away, so we the reason is always "changed". + // If the breakpoint was set through DAP, it will have the + // BreakpointBase::GetBreakpointLabel() label. Regardless + // of whether locations were added, removed, or resolved, the + // breakpoint isn't going away and the reason is always "changed". if ((event_type & lldb::eBreakpointEventTypeLocationsAdded || - event_type & lldb::eBreakpointEventTypeLocationsRemoved) && + event_type & lldb::eBreakpointEventTypeLocationsRemoved || + event_type & lldb::eBreakpointEventTypeLocationsResolved) && bp.MatchesName(BreakpointBase::GetBreakpointLabel())) { - auto bp_event = CreateEventObject("breakpoint"); - llvm::json::Object body; - // As VSCode already knows the path of this breakpoint, we don't - // need to send it back as part of a "changed" event. This - // prevent us from sending to VSCode paths that should be source - // mapped. Note that CreateBreakpoint doesn't apply source mapping. - // Besides, the current implementation of VSCode ignores the - // "source" element of breakpoint events. + // As the DAP client already knows the path of this breakpoint, we + // don't need to send it back as part of the "changed" event. This + // avoids sending paths that should be source mapped. Note that + // CreateBreakpoint doesn't apply source mapping and certain + // implementation ignore the source part of this event anyway. llvm::json::Value source_bp = CreateBreakpoint(&bp); source_bp.getAsObject()->erase("source"); + llvm::json::Object body; body.try_emplace("breakpoint", source_bp); body.try_emplace("reason", "changed"); + + llvm::json::Object bp_event = CreateEventObject("breakpoint"); bp_event.try_emplace("body", std::move(body)); + dap.SendJSON(llvm::json::Value(std::move(bp_event))); } }