[lldb-dap] Add data breakpoints for bytes #167237
Open
+94
−19
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
|
@llvm/pr-subscribers-lldb Author: Sergei Druzhkov (DrSergei) ChangesThis patch adds support for Full diff: https://github.com/llvm/llvm-project/pull/167237.diff 4 Files Affected:
diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
index ac550962cfb85..aaa6f76001aa3 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
@@ -1234,16 +1234,24 @@ def request_setFunctionBreakpoints(self, names, condition=None, hitCondition=Non
return response
def request_dataBreakpointInfo(
- self, variablesReference, name, frameIndex=0, threadId=None
+ self, variablesReference, name, size=None, frameIndex=0, threadId=None
):
stackFrame = self.get_stackFrame(frameIndex=frameIndex, threadId=threadId)
if stackFrame is None:
return []
- args_dict = {
- "variablesReference": variablesReference,
- "name": name,
- "frameId": stackFrame["id"],
- }
+ args_dict = (
+ {
+ "variablesReference": variablesReference,
+ "name": name,
+ "frameId": stackFrame["id"],
+ }
+ if size is None
+ else {
+ "bytes": size,
+ "name": name,
+ "asAddress": True,
+ }
+ )
command_dict = {
"command": "dataBreakpointInfo",
"type": "request",
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..7afc42f89ce55 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,55 @@ 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_bytes(self):
+ """Tests setting data breakpoints on memory range."""
+ 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()
+ # Test write watchpoints on x, arr[2]
+ x = self.dap_server.get_local_variable("x")
+ response_x = self.dap_server.request_dataBreakpointInfo(
+ 0, x["memoryReference"], 4
+ )
+ arr_2 = self.dap_server.get_local_variable_child("arr", "[2]")
+ response_arr_2 = self.dap_server.request_dataBreakpointInfo(
+ 0, arr_2["memoryReference"], 4
+ )
+
+ # Test response from dataBreakpointInfo request.
+ self.assertEqual(
+ response_x["body"]["dataId"].split("/"), [x["memoryReference"][2:], "4"]
+ )
+ self.assertEqual(response_x["body"]["accessTypes"], self.accessTypes)
+ self.assertEqual(
+ response_arr_2["body"]["dataId"].split("/"),
+ [arr_2["memoryReference"][2:], "4"],
+ )
+ self.assertEqual(response_arr_2["body"]["accessTypes"], self.accessTypes)
+ dataBreakpoints = [
+ {"dataId": response_x["body"]["dataId"], "accessType": "write"},
+ {"dataId": response_arr_2["body"]["dataId"], "accessType": "write"},
+ ]
+ set_response = self.dap_server.request_setDataBreakpoint(dataBreakpoints)
+ self.assertEqual(
+ set_response["body"]["breakpoints"],
+ [{"verified": True}, {"verified": True}],
+ )
+
+ self.continue_to_next_stop()
+ x_val = self.dap_server.get_local_variable_value("x")
+ i_val = self.dap_server.get_local_variable_value("i")
+ self.assertEqual(x_val, "2")
+ self.assertEqual(i_val, "1")
+
+ self.continue_to_next_stop()
+ arr_2 = self.dap_server.get_local_variable_child("arr", "[2]")
+ i_val = self.dap_server.get_local_variable_value("i")
+ self.assertEqual(arr_2["value"], "42")
+ self.assertEqual(i_val, "2")
+ self.dap_server.request_setDataBreakpoint([])
diff --git a/lldb/tools/lldb-dap/Handler/DataBreakpointInfoRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/DataBreakpointInfoRequestHandler.cpp
index 87b93fc999ecd..c55b165c6b783 100644
--- a/lldb/tools/lldb-dap/Handler/DataBreakpointInfoRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/DataBreakpointInfoRequestHandler.cpp
@@ -16,6 +16,20 @@
namespace lldb_dap {
+static bool CheckAddress(DAP &dap, lldb::addr_t load_addr) {
+ lldb::SBMemoryRegionInfo region;
+ lldb::SBError err =
+ dap.target.GetProcess().GetMemoryRegionInfo(load_addr, region);
+ // Only lldb-server supports "qMemoryRegionInfo". So, don't fail this
+ // request if SBProcess::GetMemoryRegionInfo returns error.
+ if (err.Success()) {
+ if (!(region.IsReadable() || region.IsWritable())) {
+ return false;
+ }
+ }
+ return true;
+}
+
/// Obtains information on a possible data breakpoint that could be set on an
/// expression or variable. Clients should only call this request if the
/// corresponding capability supportsDataBreakpoints is true.
@@ -23,7 +37,6 @@ llvm::Expected<protocol::DataBreakpointInfoResponseBody>
DataBreakpointInfoRequestHandler::Run(
const protocol::DataBreakpointInfoArguments &args) const {
protocol::DataBreakpointInfoResponseBody response;
- lldb::SBFrame frame = dap.GetLLDBFrame(args.frameId);
lldb::SBValue variable = dap.variables.FindVariable(
args.variablesReference.value_or(0), args.name);
std::string addr, size;
@@ -43,7 +56,8 @@ DataBreakpointInfoRequestHandler::Run(
addr = llvm::utohexstr(load_addr);
size = llvm::utostr(byte_size);
}
- } else if (args.variablesReference.value_or(0) == 0 && frame.IsValid()) {
+ } else if (lldb::SBFrame frame = dap.GetLLDBFrame(args.frameId);
+ args.variablesReference.value_or(0) == 0 && frame.IsValid()) {
lldb::SBValue value = frame.EvaluateExpression(args.name.c_str());
if (value.GetError().Fail()) {
lldb::SBError error = value.GetError();
@@ -58,17 +72,10 @@ DataBreakpointInfoRequestHandler::Run(
if (data.IsValid()) {
size = llvm::utostr(data.GetByteSize());
addr = llvm::utohexstr(load_addr);
- lldb::SBMemoryRegionInfo region;
- lldb::SBError err =
- dap.target.GetProcess().GetMemoryRegionInfo(load_addr, region);
- // Only lldb-server supports "qMemoryRegionInfo". So, don't fail this
- // request if SBProcess::GetMemoryRegionInfo returns error.
- if (err.Success()) {
- if (!(region.IsReadable() || region.IsWritable())) {
- is_data_ok = false;
- response.description = "memory region for address " + addr +
- " has no read or write permissions";
- }
+ if (!CheckAddress(dap, load_addr)) {
+ is_data_ok = false;
+ response.description = "memory region for address " + addr +
+ " has no read or write permissions";
}
} else {
is_data_ok = false;
@@ -76,6 +83,17 @@ DataBreakpointInfoRequestHandler::Run(
"unable to get byte size for expression: " + args.name;
}
}
+ } else if (args.asAddress) {
+ size = llvm::utostr(args.bytes.value_or(1));
+ if (llvm::StringRef(args.name).starts_with("0x"))
+ addr = args.name.substr(2);
+ else
+ addr = llvm::utohexstr(std::stoull(args.name));
+ if (!CheckAddress(dap, std::stoull(addr, 0, 16))) {
+ is_data_ok = false;
+ response.description = "memory region for address " + addr +
+ " has no read or write permissions";
+ }
} else {
is_data_ok = false;
response.description = "variable not found: " + args.name;
diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.h b/lldb/tools/lldb-dap/Handler/RequestHandler.h
index bc22133d92453..c9cafd33327a8 100644
--- a/lldb/tools/lldb-dap/Handler/RequestHandler.h
+++ b/lldb/tools/lldb-dap/Handler/RequestHandler.h
@@ -432,6 +432,9 @@ class DataBreakpointInfoRequestHandler
public:
using RequestHandler::RequestHandler;
static llvm::StringLiteral GetCommand() { return "dataBreakpointInfo"; }
+ FeatureSet GetSupportedFeatures() const override {
+ return {protocol::eAdapterFeatureDataBreakpointBytes};
+ }
llvm::Expected<protocol::DataBreakpointInfoResponseBody>
Run(const protocol::DataBreakpointInfoArguments &args) const override;
};
|
ashgti
reviewed
Nov 10, 2025
|
|
||
| namespace lldb_dap { | ||
|
|
||
| static bool CheckAddress(DAP &dap, lldb::addr_t load_addr) { |
Contributor
There was a problem hiding this comment.
nit: Can we give this a more descriptive name? Like IsRW or IsMutable I think thats what this is checking...?
| "name": name, | ||
| "frameId": stackFrame["id"], | ||
| } | ||
| if size is None |
Contributor
There was a problem hiding this comment.
Can we lift this to a statement instead of an inline expression like this?
This is a bit complex for an inline if
ashgti
approved these changes
Nov 10, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch adds support for
dataBreakpointInfoBytescapability from DAP. You can test this feature in VSCode (Add data breakpoint at addressbutton in breakpoints tab).