From 773cff4106a9c28300847fcc9612ea25cc0cd44d Mon Sep 17 00:00:00 2001 From: Stephen Tozer Date: Wed, 16 Jul 2025 17:06:39 +0100 Subject: [PATCH 1/4] [lldb-dap] Allow returning multiple breakpoints in "stopped" event Currently, the "stopped" event returned when a breakpoint is hit will always return only the ID of first breakpoint returned from `GetStopReasonDataAtIndex`. This is slightly different from the behaviour in `lldb`, where multiple breakpoints can exist at a single instruction address and all are returned as part of the stop reason when that address is hit. This patch allows all multiple hit breakpoints to be returned in the "stopped" event, both in the hitBreakpointIds field and in the description, using the same formatting as lldb e.g. "breakpoint 1.1 2.1". --- .../test/tools/lldb-dap/lldbdap_testcase.py | 23 +++++++++++++++++++ .../breakpoint/TestDAP_setBreakpoints.py | 20 ++++++++++++++++ .../API/tools/lldb-dap/breakpoint/main.cpp | 2 +- lldb/tools/lldb-dap/JSONUtils.cpp | 17 +++++++++----- 4 files changed, 55 insertions(+), 7 deletions(-) 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 d823126e3e2fd..9b227c67bb859 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 @@ -173,6 +173,29 @@ def verify_breakpoint_hit(self, breakpoint_ids, timeout=DEFAULT_TIMEOUT): return self.assertTrue(False, f"breakpoint not hit, stopped_events={stopped_events}") + def verify_all_breakpoints_hit(self, breakpoint_ids, timeout=DEFAULT_TIMEOUT): + """Wait for the process we are debugging to stop, and verify we hit + all of the breakpoint locations in the "breakpoint_ids" array. + "breakpoint_ids" should be a list of breakpoint ID strings + (["1", "2"]).""" + stopped_events = self.dap_server.wait_for_stopped(timeout) + for stopped_event in stopped_events: + if "body" in stopped_event: + body = stopped_event["body"] + if "reason" not in body: + continue + if ( + body["reason"] != "breakpoint" + and body["reason"] != "instruction breakpoint" + ): + continue + if "hitBreakpointIds" not in body: + continue + hit_bps = body["hitBreakpointIds"] + if all(int(breakpoint_id) in hit_bps for breakpoint_id in breakpoint_ids): + return + self.assertTrue(False, f"breakpoints not hit, stopped_events={stopped_events}") + def verify_stop_exception_info(self, expected_description, timeout=DEFAULT_TIMEOUT): """Wait for the process we are debugging to stop, and verify the stop reason is 'exception' and that the description matches diff --git a/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setBreakpoints.py b/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setBreakpoints.py index 831edd6494c1e..121933c672bf7 100644 --- a/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setBreakpoints.py +++ b/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setBreakpoints.py @@ -398,3 +398,23 @@ def test_column_breakpoints(self): self.stepIn() func_name = self.get_stackFrames()[0]["name"] self.assertEqual(func_name, "a::fourteen(int)") + + @skipIfWindows + def test_hit_multiple_breakpoints(self): + """Test that if we hit multiple breakpoints at the same address, they + all appear in the stop reason.""" + breakpoint_lines = [ + line_number("main.cpp", "// end of foo check"), + line_number("main.cpp", "// before loop") + ] + + program = self.getBuildArtifact("a.out") + self.build_and_launch(program) + + # Set a pair of breakpoints that will both resolve to the same address. + breakpoint_ids = self.set_source_breakpoints(self.main_path, breakpoint_lines) + self.assertEqual(len(breakpoint_ids), 2, "expected two breakpoints") + self.dap_server.request_continue() + print(breakpoint_ids) + # Verify we hit both of the breakpoints we just set + self.verify_all_breakpoints_hit(breakpoint_ids) diff --git a/lldb/test/API/tools/lldb-dap/breakpoint/main.cpp b/lldb/test/API/tools/lldb-dap/breakpoint/main.cpp index a84546a95af15..a9a16f240e37d 100644 --- a/lldb/test/API/tools/lldb-dap/breakpoint/main.cpp +++ b/lldb/test/API/tools/lldb-dap/breakpoint/main.cpp @@ -33,7 +33,7 @@ int main(int argc, char const *argv[]) { if (foo == nullptr) { fprintf(stderr, "%s\n", dlerror()); exit(2); - } + } // end of foo check foo(12); // before loop for (int i = 0; i < 10; ++i) { diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp index 41ca29a405ac9..30e6ea900d919 100644 --- a/lldb/tools/lldb-dap/JSONUtils.cpp +++ b/lldb/tools/lldb-dap/JSONUtils.cpp @@ -654,12 +654,17 @@ llvm::json::Value CreateThreadStopped(DAP &dap, lldb::SBThread &thread, } else { body.try_emplace("reason", "breakpoint"); } - lldb::break_id_t bp_id = thread.GetStopReasonDataAtIndex(0); - lldb::break_id_t bp_loc_id = thread.GetStopReasonDataAtIndex(1); - std::string desc_str = - llvm::formatv("breakpoint {0}.{1}", bp_id, bp_loc_id); - body.try_emplace("hitBreakpointIds", - llvm::json::Array{llvm::json::Value(bp_id)}); + std::vector bp_ids; + std::ostringstream desc_sstream; + desc_sstream << "breakpoint"; + for (size_t idx = 0; idx < thread.GetStopReasonDataCount(); idx += 2) { + lldb::break_id_t bp_id = thread.GetStopReasonDataAtIndex(idx); + lldb::break_id_t bp_loc_id = thread.GetStopReasonDataAtIndex(idx + 1); + bp_ids.push_back(llvm::json::Value(bp_id)); + desc_sstream << " " << bp_id << "." << bp_loc_id; + } + std::string desc_str = desc_sstream.str(); + body.try_emplace("hitBreakpointIds", llvm::json::Array(bp_ids)); EmplaceSafeString(body, "description", desc_str); } } break; From 71a817e1c69dbd90064e973c69ede2259ca65d67 Mon Sep 17 00:00:00 2001 From: Stephen Tozer Date: Wed, 16 Jul 2025 17:54:33 +0100 Subject: [PATCH 2/4] Some review comments --- .../Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py | 4 +++- .../API/tools/lldb-dap/breakpoint/TestDAP_setBreakpoints.py | 2 +- lldb/tools/lldb-dap/JSONUtils.cpp | 4 ++-- 3 files changed, 6 insertions(+), 4 deletions(-) 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 9b227c67bb859..029b8d665d91d 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 @@ -192,7 +192,9 @@ def verify_all_breakpoints_hit(self, breakpoint_ids, timeout=DEFAULT_TIMEOUT): if "hitBreakpointIds" not in body: continue hit_bps = body["hitBreakpointIds"] - if all(int(breakpoint_id) in hit_bps for breakpoint_id in breakpoint_ids): + if all( + int(breakpoint_id) in hit_bps for breakpoint_id in breakpoint_ids + ): return self.assertTrue(False, f"breakpoints not hit, stopped_events={stopped_events}") diff --git a/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setBreakpoints.py b/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setBreakpoints.py index 121933c672bf7..3a32fd98138df 100644 --- a/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setBreakpoints.py +++ b/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setBreakpoints.py @@ -405,7 +405,7 @@ def test_hit_multiple_breakpoints(self): all appear in the stop reason.""" breakpoint_lines = [ line_number("main.cpp", "// end of foo check"), - line_number("main.cpp", "// before loop") + line_number("main.cpp", "// before loop"), ] program = self.getBuildArtifact("a.out") diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp index 30e6ea900d919..f42c50236f19e 100644 --- a/lldb/tools/lldb-dap/JSONUtils.cpp +++ b/lldb/tools/lldb-dap/JSONUtils.cpp @@ -654,13 +654,13 @@ llvm::json::Value CreateThreadStopped(DAP &dap, lldb::SBThread &thread, } else { body.try_emplace("reason", "breakpoint"); } - std::vector bp_ids; + std::vector bp_ids; std::ostringstream desc_sstream; desc_sstream << "breakpoint"; for (size_t idx = 0; idx < thread.GetStopReasonDataCount(); idx += 2) { lldb::break_id_t bp_id = thread.GetStopReasonDataAtIndex(idx); lldb::break_id_t bp_loc_id = thread.GetStopReasonDataAtIndex(idx + 1); - bp_ids.push_back(llvm::json::Value(bp_id)); + bp_ids.push_back(bp_id); desc_sstream << " " << bp_id << "." << bp_loc_id; } std::string desc_str = desc_sstream.str(); From c789b42293a64e02c2897720b6dbd5797d0b2443 Mon Sep 17 00:00:00 2001 From: Stephen Tozer Date: Thu, 17 Jul 2025 21:16:59 +0100 Subject: [PATCH 3/4] Address further review comments --- .../lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py | 7 ++----- .../tools/lldb-dap/breakpoint/TestDAP_setBreakpoints.py | 7 +++++-- lldb/test/API/tools/lldb-dap/breakpoint/main.cpp | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) 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 029b8d665d91d..1567462839748 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 @@ -176,8 +176,7 @@ def verify_breakpoint_hit(self, breakpoint_ids, timeout=DEFAULT_TIMEOUT): def verify_all_breakpoints_hit(self, breakpoint_ids, timeout=DEFAULT_TIMEOUT): """Wait for the process we are debugging to stop, and verify we hit all of the breakpoint locations in the "breakpoint_ids" array. - "breakpoint_ids" should be a list of breakpoint ID strings - (["1", "2"]).""" + "breakpoint_ids" should be a list of int breakpoint IDs ([1, 2]).""" stopped_events = self.dap_server.wait_for_stopped(timeout) for stopped_event in stopped_events: if "body" in stopped_event: @@ -192,9 +191,7 @@ def verify_all_breakpoints_hit(self, breakpoint_ids, timeout=DEFAULT_TIMEOUT): if "hitBreakpointIds" not in body: continue hit_bps = body["hitBreakpointIds"] - if all( - int(breakpoint_id) in hit_bps for breakpoint_id in breakpoint_ids - ): + if all(breakpoint_id in hit_bps for breakpoint_id in breakpoint_ids): return self.assertTrue(False, f"breakpoints not hit, stopped_events={stopped_events}") diff --git a/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setBreakpoints.py b/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setBreakpoints.py index 3a32fd98138df..2e860ff5d5e17 100644 --- a/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setBreakpoints.py +++ b/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setBreakpoints.py @@ -404,7 +404,7 @@ def test_hit_multiple_breakpoints(self): """Test that if we hit multiple breakpoints at the same address, they all appear in the stop reason.""" breakpoint_lines = [ - line_number("main.cpp", "// end of foo check"), + line_number("main.cpp", "// break non-breakpointable line"), line_number("main.cpp", "// before loop"), ] @@ -412,7 +412,10 @@ def test_hit_multiple_breakpoints(self): self.build_and_launch(program) # Set a pair of breakpoints that will both resolve to the same address. - breakpoint_ids = self.set_source_breakpoints(self.main_path, breakpoint_lines) + breakpoint_ids = [ + int(bp_id) + for bp_id in self.set_source_breakpoints(self.main_path, breakpoint_lines) + ] self.assertEqual(len(breakpoint_ids), 2, "expected two breakpoints") self.dap_server.request_continue() print(breakpoint_ids) diff --git a/lldb/test/API/tools/lldb-dap/breakpoint/main.cpp b/lldb/test/API/tools/lldb-dap/breakpoint/main.cpp index a9a16f240e37d..6e16e75b9a3b9 100644 --- a/lldb/test/API/tools/lldb-dap/breakpoint/main.cpp +++ b/lldb/test/API/tools/lldb-dap/breakpoint/main.cpp @@ -33,7 +33,7 @@ int main(int argc, char const *argv[]) { if (foo == nullptr) { fprintf(stderr, "%s\n", dlerror()); exit(2); - } // end of foo check + } // break non-breakpointable line foo(12); // before loop for (int i = 0; i < 10; ++i) { From 83ad725542dacabae636df2a516ca247cdfb1cfa Mon Sep 17 00:00:00 2001 From: Stephen Tozer Date: Thu, 17 Jul 2025 21:23:00 +0100 Subject: [PATCH 4/4] crush clang-format --- lldb/test/API/tools/lldb-dap/breakpoint/main.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/test/API/tools/lldb-dap/breakpoint/main.cpp b/lldb/test/API/tools/lldb-dap/breakpoint/main.cpp index 6e16e75b9a3b9..2206b07f19494 100644 --- a/lldb/test/API/tools/lldb-dap/breakpoint/main.cpp +++ b/lldb/test/API/tools/lldb-dap/breakpoint/main.cpp @@ -33,7 +33,7 @@ int main(int argc, char const *argv[]) { if (foo == nullptr) { fprintf(stderr, "%s\n", dlerror()); exit(2); - } // break non-breakpointable line + } // break non-breakpointable line foo(12); // before loop for (int i = 0; i < 10; ++i) {