Skip to content
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.

Sign up for GitHub

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

[lldb-dap] Correctly report breakpoints as resolved on macOS #129589

Merged
merged 2 commits into from
Mar 4, 2025
+70 −68
Merged

[lldb-dap] Correctly report breakpoints as resolved on macOS #129589

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2706528
[lldb-dap] Correctly report breakpoints as resolved on macOS
JDevlieghere Mar 3, 2025
427c132
Check the event mask before the label
JDevlieghere Mar 3, 2025
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
110 changes: 55 additions & 55 deletions lldb/test/API/tools/lldb-dap/breakpoint-events/TestDAP_breakpointEvents.py
View file
Original file line number Diff line number Diff line change
@@ -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")
28 changes: 15 additions & 13 deletions lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp
View file
Original file line number Diff line number Diff line change
@@ -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)));
}
}