[Lldb-commits] [lldb] Make breakpoint stop reason more accurate for function breakpoints (PR #130841)
via lldb-commits
lldb-commits at lists.llvm.org
Wed Mar 12 10:42:04 PDT 2025
https://github.com/satyajanga updated https://github.com/llvm/llvm-project/pull/130841
>From 55d3b7d0ecf103cc97942c1406926853c35356c1 Mon Sep 17 00:00:00 2001
From: satya janga <satyajanga at fb.com>
Date: Tue, 11 Mar 2025 13:39:08 -0700
Subject: [PATCH 1/2] Make breakpoint stop reason more accurate
---
.../test/tools/lldb-dap/lldbdap_testcase.py | 3 ++-
lldb/tools/lldb-dap/DAP.cpp | 26 +++++++++++++++++++
lldb/tools/lldb-dap/DAP.h | 12 ++++++---
lldb/tools/lldb-dap/JSONUtils.cpp | 12 ++++++---
4 files changed, 46 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 70b04b051e0ec..5faf83ca826a0 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
@@ -86,6 +86,7 @@ def verify_breakpoint_hit(self, breakpoint_ids):
if (
body["reason"] != "breakpoint"
and body["reason"] != "instruction breakpoint"
+ and body["reason"] != "function breakpoint"
):
continue
if "description" not in body:
@@ -100,7 +101,7 @@ def verify_breakpoint_hit(self, breakpoint_ids):
# location.
description = body["description"]
for breakpoint_id in breakpoint_ids:
- match_desc = "breakpoint %s." % (breakpoint_id)
+ match_desc = "%s %s." % (body["reason"], breakpoint_id)
if match_desc in description:
return
self.assertTrue(False, "breakpoint not hit")
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index edd3b31be8ff7..485c420dc59e8 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -1128,6 +1128,32 @@ DAP::GetInstructionBPFromStopReason(lldb::SBThread &thread) {
return inst_bp;
}
+FunctionBreakpoint *DAP::GetFunctionBPFromStopReason(lldb::SBThread &thread) {
+ const auto num = thread.GetStopReasonDataCount();
+ FunctionBreakpoint *func_bp = nullptr;
+ for (size_t i = 0; i < num; i += 2) {
+ // thread.GetStopReasonDataAtIndex(i) will return the bp ID and
+ // thread.GetStopReasonDataAtIndex(i+1) will return the location
+ // within that breakpoint. We only care about the bp ID so we can
+ // see if this is an function breakpoint that is getting hit.
+ lldb::break_id_t bp_id = thread.GetStopReasonDataAtIndex(i);
+ func_bp = GetFunctionBreakPoint(bp_id);
+ // If any breakpoint is not an function breakpoint, then stop and
+ // report this as a normal breakpoint
+ if (func_bp == nullptr)
+ return nullptr;
+ }
+ return func_bp;
+}
+
+FunctionBreakpoint *DAP::GetFunctionBreakPoint(const lldb::break_id_t bp_id) {
+ for (auto &bp : function_breakpoints) {
+ if (bp.second.bp.GetID() == bp_id)
+ return &bp.second;
+ }
+ return nullptr;
+}
+
lldb::SBValueList *Variables::GetTopLevelScope(int64_t variablesReference) {
switch (variablesReference) {
case VARREF_LOCALS:
diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index 3ff1992b61f5b..ea72b41d02516 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -125,21 +125,21 @@ struct Variables {
struct StartDebuggingRequestHandler : public lldb::SBCommandPluginInterface {
DAP &dap;
- explicit StartDebuggingRequestHandler(DAP &d) : dap(d) {};
+ explicit StartDebuggingRequestHandler(DAP &d) : dap(d){};
bool DoExecute(lldb::SBDebugger debugger, char **command,
lldb::SBCommandReturnObject &result) override;
};
struct ReplModeRequestHandler : public lldb::SBCommandPluginInterface {
DAP &dap;
- explicit ReplModeRequestHandler(DAP &d) : dap(d) {};
+ explicit ReplModeRequestHandler(DAP &d) : dap(d){};
bool DoExecute(lldb::SBDebugger debugger, char **command,
lldb::SBCommandReturnObject &result) override;
};
struct SendEventRequestHandler : public lldb::SBCommandPluginInterface {
DAP &dap;
- explicit SendEventRequestHandler(DAP &d) : dap(d) {};
+ explicit SendEventRequestHandler(DAP &d) : dap(d){};
bool DoExecute(lldb::SBDebugger debugger, char **command,
lldb::SBCommandReturnObject &result) override;
};
@@ -383,6 +383,12 @@ struct DAP {
InstructionBreakpoint *GetInstructionBPFromStopReason(lldb::SBThread &thread);
+ FunctionBreakpoint *GetFunctionBPFromStopReason(lldb::SBThread &thread);
+
+ FunctionBreakpoint *GetFunctionBreakPoint(const lldb::break_id_t bp_id);
+
+ void WaitWorkerThreadsToExit();
+
private:
// Send the JSON in "json_str" to the "out" stream. Correctly send the
// "Content-Length:" field followed by the length, followed by the raw
diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp
index 932145b1799bd..cd0d518cd4f7d 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -967,17 +967,23 @@ llvm::json::Value CreateThreadStopped(DAP &dap, lldb::SBThread &thread,
body.try_emplace("reason", "exception");
EmplaceSafeString(body, "description", exc_bp->label);
} else {
+ std::string reason = "breakpoint";
InstructionBreakpoint *inst_bp =
dap.GetInstructionBPFromStopReason(thread);
if (inst_bp) {
- body.try_emplace("reason", "instruction breakpoint");
+ reason = "instruction breakpoint";
} else {
- body.try_emplace("reason", "breakpoint");
+ FunctionBreakpoint *function_bp =
+ dap.GetFunctionBPFromStopReason(thread);
+ if (function_bp) {
+ reason = "function breakpoint";
+ }
}
+ body.try_emplace("reason", reason);
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);
+ llvm::formatv("{0} {1}.{2}", reason, bp_id, bp_loc_id);
body.try_emplace("hitBreakpointIds",
llvm::json::Array{llvm::json::Value(bp_id)});
EmplaceSafeString(body, "description", desc_str);
>From 4bcddcf9cebccb56fd3264e3436d6c16fc772d7d Mon Sep 17 00:00:00 2001
From: satya janga <satyajanga at fb.com>
Date: Tue, 11 Mar 2025 13:39:08 -0700
Subject: [PATCH 2/2] Make breakpoint stop reason more accurate
---
.../TestDAP_setDataBreakpoints.py | 32 ++++++++++
lldb/tools/lldb-dap/DAP.cpp | 58 -------------------
lldb/tools/lldb-dap/DAP.h | 54 ++++++++++++++++-
.../Handler/ExceptionInfoRequestHandler.cpp | 3 +-
lldb/tools/lldb-dap/JSONUtils.cpp | 19 ++++--
5 files changed, 100 insertions(+), 66 deletions(-)
diff --git a/lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py b/lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py
index a542a318050dd..f5ff971261b6d 100644
--- a/lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py
+++ b/lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py
@@ -171,3 +171,35 @@ def test_functionality(self):
self.continue_to_next_stop()
x_val = self.dap_server.get_local_variable_value("x")
self.assertEqual(x_val, "10")
+
+ @skipIfWindows
+ def test_breakpoint_reason(self):
+ """Tests setting data breakpoints on variable. Verify that the breakpoint has the correct reason and description in the stopped event."""
+ program = self.getBuildArtifact("a.out")
+ self.build_and_launch(program)
+ source = "main.cpp"
+ first_loop_break_line = line_number(source, "// first loop breakpoint")
+ self.set_source_breakpoints(source, [first_loop_break_line])
+ self.continue_to_next_stop()
+ self.dap_server.get_local_variables()
+ # Test write watchpoints on x, arr[2]
+ response_x = self.dap_server.request_dataBreakpointInfo(1, "x")
+ # Test response from dataBreakpointInfo request.
+ self.assertEqual(response_x["body"]["dataId"].split("/")[1], "4")
+ self.assertEqual(response_x["body"]["accessTypes"], self.accessTypes)
+ dataBreakpoints = [
+ {"dataId": response_x["body"]["dataId"], "accessType": "write"},
+ ]
+ set_response = self.dap_server.request_setDataBreakpoint(dataBreakpoints)
+ self.assertEqual(
+ set_response["body"]["breakpoints"],
+ [{"verified": True}],
+ )
+
+ stopped_events = self.continue_to_next_stop()
+ self.assertEqual(len(stopped_events), 1)
+ stopped_event = stopped_events[0]
+ self.assertEqual(stopped_event["body"]["reason"], "data breakpoint")
+ self.assertEqual(stopped_event["body"]["description"], "data breakpoint 1.1")
+ x_val = self.dap_server.get_local_variable_value("x")
+ self.assertEqual(x_val, "2")
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index 485c420dc59e8..4929205a4bfe9 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -183,17 +183,6 @@ ExceptionBreakpoint *DAP::GetExceptionBreakpoint(const std::string &filter) {
return nullptr;
}
-ExceptionBreakpoint *DAP::GetExceptionBreakpoint(const lldb::break_id_t bp_id) {
- // See comment in the other GetExceptionBreakpoint().
- PopulateExceptionBreakpoints();
-
- for (auto &bp : *exception_breakpoints) {
- if (bp.bp.GetID() == bp_id)
- return &bp;
- }
- return nullptr;
-}
-
llvm::Error DAP::ConfigureIO(std::FILE *overrideOut, std::FILE *overrideErr) {
in = lldb::SBFile(std::fopen(DEV_NULL, "r"), /*transfer_ownership=*/true);
@@ -475,27 +464,6 @@ DAP::SendFormattedOutput(OutputType o, const char *format, ...) {
o, llvm::StringRef(buffer, std::min<int>(actual_length, sizeof(buffer))));
}
-ExceptionBreakpoint *DAP::GetExceptionBPFromStopReason(lldb::SBThread &thread) {
- const auto num = thread.GetStopReasonDataCount();
- // Check to see if have hit an exception breakpoint and change the
- // reason to "exception", but only do so if all breakpoints that were
- // hit are exception breakpoints.
- ExceptionBreakpoint *exc_bp = nullptr;
- for (size_t i = 0; i < num; i += 2) {
- // thread.GetStopReasonDataAtIndex(i) will return the bp ID and
- // thread.GetStopReasonDataAtIndex(i+1) will return the location
- // within that breakpoint. We only care about the bp ID so we can
- // see if this is an exception breakpoint that is getting hit.
- lldb::break_id_t bp_id = thread.GetStopReasonDataAtIndex(i);
- exc_bp = GetExceptionBreakpoint(bp_id);
- // If any breakpoint is not an exception breakpoint, then stop and
- // report this as a normal breakpoint
- if (exc_bp == nullptr)
- return nullptr;
- }
- return exc_bp;
-}
-
lldb::SBThread DAP::GetLLDBThread(const llvm::json::Object &arguments) {
auto tid = GetInteger<int64_t>(arguments, "threadId")
.value_or(LLDB_INVALID_THREAD_ID);
@@ -1128,32 +1096,6 @@ DAP::GetInstructionBPFromStopReason(lldb::SBThread &thread) {
return inst_bp;
}
-FunctionBreakpoint *DAP::GetFunctionBPFromStopReason(lldb::SBThread &thread) {
- const auto num = thread.GetStopReasonDataCount();
- FunctionBreakpoint *func_bp = nullptr;
- for (size_t i = 0; i < num; i += 2) {
- // thread.GetStopReasonDataAtIndex(i) will return the bp ID and
- // thread.GetStopReasonDataAtIndex(i+1) will return the location
- // within that breakpoint. We only care about the bp ID so we can
- // see if this is an function breakpoint that is getting hit.
- lldb::break_id_t bp_id = thread.GetStopReasonDataAtIndex(i);
- func_bp = GetFunctionBreakPoint(bp_id);
- // If any breakpoint is not an function breakpoint, then stop and
- // report this as a normal breakpoint
- if (func_bp == nullptr)
- return nullptr;
- }
- return func_bp;
-}
-
-FunctionBreakpoint *DAP::GetFunctionBreakPoint(const lldb::break_id_t bp_id) {
- for (auto &bp : function_breakpoints) {
- if (bp.second.bp.GetID() == bp_id)
- return &bp.second;
- }
- return nullptr;
-}
-
lldb::SBValueList *Variables::GetTopLevelScope(int64_t variablesReference) {
switch (variablesReference) {
case VARREF_LOCALS:
diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index ea72b41d02516..0a42d2be4bd5e 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -379,9 +379,59 @@ struct DAP {
void SetThreadFormat(llvm::StringRef format);
- InstructionBreakpoint *GetInstructionBreakpoint(const lldb::break_id_t bp_id);
+ template <typename BreakpointType>
+ BreakpointType *GetBreakpointFromStopReason(lldb::SBThread &thread) {
+ // Check to see if have hit the <BreakpointType> breakpoint and change the
+ // reason accordingly, but only do so if all breakpoints that were
+ // hit are of <BreakpointType>.
+ const auto num = thread.GetStopReasonDataCount();
+ BreakpointType *bp = nullptr;
+ for (size_t i = 0; i < num; i += 2) {
+ lldb::break_id_t bp_id = thread.GetStopReasonDataAtIndex(i);
+ // If any breakpoint is not the <BreakpointType>, then stop and
+ // report this as a normal breakpoint
+ bp = GetBreakpoint<BreakpointType>(bp_id);
+ if (bp == nullptr)
+ return nullptr;
+ }
+ return bp;
+ }
+
+ template <typename BreakpointType>
+ BreakpointType *GetBreakpoint(const lldb::break_id_t bp_id);
+
+ template <>
+ FunctionBreakpoint *
+ GetBreakpoint<FunctionBreakpoint>(const lldb::break_id_t bp_id) {
+ for (auto &bp : function_breakpoints) {
+ if (bp.second.bp.GetID() == bp_id)
+ return &bp.second;
+ }
+ return nullptr;
+ }
- InstructionBreakpoint *GetInstructionBPFromStopReason(lldb::SBThread &thread);
+ template <>
+ InstructionBreakpoint *
+ GetBreakpoint<InstructionBreakpoint>(const lldb::break_id_t bp_id) {
+ for (auto &bp : instruction_breakpoints) {
+ if (bp.second.bp.GetID() == bp_id)
+ return &bp.second;
+ }
+ return nullptr;
+ }
+
+ template <>
+ ExceptionBreakpoint *
+ GetBreakpoint<ExceptionBreakpoint>(const lldb::break_id_t bp_id) {
+ // See comment in the other GetExceptionBreakpoint().
+ PopulateExceptionBreakpoints();
+
+ for (auto &bp : *exception_breakpoints) {
+ if (bp.bp.GetID() == bp_id)
+ return &bp;
+ }
+ return nullptr;
+ }
FunctionBreakpoint *GetFunctionBPFromStopReason(lldb::SBThread &thread);
diff --git a/lldb/tools/lldb-dap/Handler/ExceptionInfoRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/ExceptionInfoRequestHandler.cpp
index 2f4d4efd1b189..974b340bb960c 100644
--- a/lldb/tools/lldb-dap/Handler/ExceptionInfoRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/ExceptionInfoRequestHandler.cpp
@@ -123,7 +123,8 @@ void ExceptionInfoRequestHandler::operator()(
if (stopReason == lldb::eStopReasonSignal)
body.try_emplace("exceptionId", "signal");
else if (stopReason == lldb::eStopReasonBreakpoint) {
- ExceptionBreakpoint *exc_bp = dap.GetExceptionBPFromStopReason(thread);
+ ExceptionBreakpoint *exc_bp =
+ dap.GetBreakpointFromStopReason<ExceptionBreakpoint>(thread);
if (exc_bp) {
EmplaceSafeString(body, "exceptionId", exc_bp->filter);
EmplaceSafeString(body, "description", exc_bp->label);
diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp
index cd0d518cd4f7d..1561c73ff5f2a 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -962,19 +962,20 @@ llvm::json::Value CreateThreadStopped(DAP &dap, lldb::SBThread &thread,
body.try_emplace("reason", "step");
break;
case lldb::eStopReasonBreakpoint: {
- ExceptionBreakpoint *exc_bp = dap.GetExceptionBPFromStopReason(thread);
+ ExceptionBreakpoint *exc_bp =
+ dap.GetBreakpointFromStopReason<ExceptionBreakpoint>(thread);
if (exc_bp) {
body.try_emplace("reason", "exception");
EmplaceSafeString(body, "description", exc_bp->label);
} else {
- std::string reason = "breakpoint";
+ llvm::StringRef reason = "breakpoint";
InstructionBreakpoint *inst_bp =
- dap.GetInstructionBPFromStopReason(thread);
+ dap.GetBreakpointFromStopReason<InstructionBreakpoint>(thread);
if (inst_bp) {
reason = "instruction breakpoint";
} else {
FunctionBreakpoint *function_bp =
- dap.GetFunctionBPFromStopReason(thread);
+ dap.GetBreakpointFromStopReason<FunctionBreakpoint>(thread);
if (function_bp) {
reason = "function breakpoint";
}
@@ -989,7 +990,15 @@ llvm::json::Value CreateThreadStopped(DAP &dap, lldb::SBThread &thread,
EmplaceSafeString(body, "description", desc_str);
}
} break;
- case lldb::eStopReasonWatchpoint:
+ case lldb::eStopReasonWatchpoint: {
+ // Assuming that all watch points are data breakpoints.
+ body.try_emplace("reason", "data 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("data breakpoint {0}.{1}", bp_id, bp_loc_id);
+ EmplaceSafeString(body, "description", desc_str);
+ } break;
case lldb::eStopReasonInstrumentation:
body.try_emplace("reason", "breakpoint");
break;
More information about the lldb-commits
mailing list