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

Jump to bottom

[lldb-dap] Add support for data breakpoint. #81541

Merged
merged 2 commits into from
Feb 13, 2024
Merged

[lldb-dap] Add support for data breakpoint. #81541

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a2d2869
[lldb-dap] Add support for data breakpoint.
ZequanWu Feb 13, 2024
5e32cd2
- Address comments.
ZequanWu Feb 13, 2024
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
13 changes: 11 additions & 2 deletions lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -907,8 +907,17 @@ def request_setFunctionBreakpoints(self, names, condition=None, hitCondition=Non
}
return self.send_recv(command_dict)

def request_dataBreakpointInfo(self, variablesReference, name):
args_dict = {"variablesReference": variablesReference, "name": name}
def request_dataBreakpointInfo(
self, variablesReference, name, 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"],
}
command_dict = {
"command": "dataBreakpointInfo",
"type": "request",
Expand Down
57 changes: 46 additions & 11 deletions lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,55 @@ def setUp(self):
lldbdap_testcase.DAPTestCaseBase.setUp(self)
self.accessTypes = ["read", "write", "readWrite"]

@skipIfWindows
@skipIfRemote
def test_expression(self):
"""Tests setting data breakpoints on expression."""
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_stackFrame()
# Test setting write watchpoint using expressions: &x, arr+2
response_x = self.dap_server.request_dataBreakpointInfo(0, "4@&x")
response_arr_2 = self.dap_server.request_dataBreakpointInfo(0, "1@arr+2")
# Test response from dataBreakpointInfo request.
self.assertEquals(response_x["body"]["dataId"].split("/")[1], "4")
self.assertEquals(response_x["body"]["accessTypes"], self.accessTypes)
self.assertEquals(response_arr_2["body"]["dataId"].split("/")[1], "1")
self.assertEquals(response_arr_2["body"]["accessTypes"], self.accessTypes)
dataBreakpoints = [
{"dataId": response_x["body"]["dataId"], "accessType": "write"},
{"dataId": response_arr_2["body"]["dataId"], "accessType": "write"},
]
self.dap_server.request_setDataBreakpoint(dataBreakpoints)

self.dap_server.request_continue()
self.dap_server.wait_for_stopped()
x_val = self.dap_server.get_local_variable_value("x")
i_val = self.dap_server.get_local_variable_value("i")
self.assertEquals(x_val, "2")
self.assertEquals(i_val, "1")

self.dap_server.request_continue()
self.dap_server.wait_for_stopped()
arr_2 = self.dap_server.get_local_variable_child("arr", "[2]")
i_val = self.dap_server.get_local_variable_value("i")
self.assertEquals(arr_2["value"], "'z'")
self.assertEquals(i_val, "2")

@skipIfWindows
@skipIfRemote
def test_functionality(self):
"""Tests setting data breakpoints."""
"""Tests setting data breakpoints on variable."""
program = self.getBuildArtifact("a.out")
self.build_and_launch(program)
source = "main.cpp"
first_loop_break_line = line_number(source, "// first loop breakpoint")
breakpoint_ids = self.set_source_breakpoints(source, [first_loop_break_line])
self.continue_to_breakpoints(breakpoint_ids)
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")
Expand All @@ -41,15 +80,13 @@ def test_functionality(self):
]
self.dap_server.request_setDataBreakpoint(dataBreakpoints)

self.dap_server.request_continue()
self.dap_server.wait_for_stopped()
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.assertEquals(x_val, "2")
self.assertEquals(i_val, "1")

self.dap_server.request_continue()
self.dap_server.wait_for_stopped()
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.assertEquals(arr_2["value"], "'z'")
Expand All @@ -68,8 +105,7 @@ def test_functionality(self):
}
]
self.dap_server.request_setDataBreakpoint(dataBreakpoints)
self.dap_server.request_continue()
self.dap_server.wait_for_stopped()
self.continue_to_next_stop()
x_val = self.dap_server.get_local_variable_value("x")
self.assertEquals(x_val, "3")

Expand All @@ -82,7 +118,6 @@ def test_functionality(self):
}
]
self.dap_server.request_setDataBreakpoint(dataBreakpoints)
self.dap_server.request_continue()
self.dap_server.wait_for_stopped()
self.continue_to_next_stop()
x_val = self.dap_server.get_local_variable_value("x")
self.assertEquals(x_val, "10")
144 changes: 67 additions & 77 deletions lldb/tools/lldb-dap/lldb-dap.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -561,6 +561,46 @@ void EventThreadFunction() {
}
}

lldb::SBValue FindVariable(uint64_t variablesReference, llvm::StringRef name) {
lldb::SBValue variable;
if (lldb::SBValueList *top_scope = GetTopLevelScope(variablesReference)) {
bool is_duplicated_variable_name = name.contains(" @");
// variablesReference is one of our scopes, not an actual variable it is
// asking for a variable in locals or globals or registers
int64_t end_idx = top_scope->GetSize();
// Searching backward so that we choose the variable in closest scope
// among variables of the same name.
for (int64_t i = end_idx - 1; i >= 0; --i) {
lldb::SBValue curr_variable = top_scope->GetValueAtIndex(i);
std::string variable_name = CreateUniqueVariableNameForDisplay(
curr_variable, is_duplicated_variable_name);
Comment on lines +575 to +576
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to check the name that might include the "foo @ main.cpp:12" or just look for "foo"? Also, some members of a struct be anonymous unions, we probably want to make sure we can do the right thing with those

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This functions is shared between request_setVariable and request_dataBreakpointInfo, so it checks name that contains foo @ main.cpp:12. I tested this manually.

For anonymous unions inside struct, I also tested manually. If we watch either c or d, changes on either value stops the program as expected.

struct A {
  int a;
  int b;
  union {
    char c;
    int d;
  };
};

if (variable_name == name) {
variable = curr_variable;
break;
}
}
} else {
// This is not under the globals or locals scope, so there are no duplicated
// names.

// We have a named item within an actual variable so we need to find it
// withing the container variable by name.
lldb::SBValue container = g_dap.variables.GetVariable(variablesReference);
variable = container.GetChildMemberWithName(name.data());
if (!variable.IsValid()) {
if (name.starts_with("[")) {
llvm::StringRef index_str(name.drop_front(1));
uint64_t index = 0;
if (!index_str.consumeInteger(0, index)) {
if (index_str == "]")
variable = container.GetChildAtIndex(index);
}
}
}
}
return variable;
}

// Both attach and launch take a either a sourcePath or sourceMap
// argument (or neither), from which we need to set the target.source-map.
void SetSourceMapFromArguments(const llvm::json::Object &arguments) {
Expand Down Expand Up @@ -2697,58 +2737,41 @@ void request_dataBreakpointInfo(const llvm::json::Object &request) {
GetUnsigned(arguments, "variablesReference", 0);
llvm::StringRef name = GetString(arguments, "name");
lldb::SBFrame frame = g_dap.GetLLDBFrame(*arguments);
bool is_duplicated_variable_name = name.contains(" @");
lldb::SBValue variable = FindVariable(variablesReference, name);
std::string addr, size;

lldb::SBValue variable;
if (lldb::SBValueList *top_scope = GetTopLevelScope(variablesReference)) {
// variablesReference is one of our scopes, not an actual variable it is
// asking for a variable in locals or globals or registers
int64_t end_idx = top_scope->GetSize();
// Searching backward so that we choose the variable in closest scope
// among variables of the same name.
for (int64_t i = end_idx - 1; i >= 0; --i) {
lldb::SBValue curr_variable = top_scope->GetValueAtIndex(i);
std::string variable_name = CreateUniqueVariableNameForDisplay(
curr_variable, is_duplicated_variable_name);
if (variable_name == name) {
variable = curr_variable;
break;
}
}
if (variable.IsValid()) {
addr = llvm::utohexstr(variable.GetLoadAddress());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need some error checking here. variable.GetLoadAddress() will only return a valid address if the variable is actually in memory. So if variable.GetLoadAddress() returns LLDB_INVALID_ADDRESS we need to return an appropriate error like " does not exist in memory, its location is " where is the result of const char *SBValue::GetLocation().

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sent a fix at #81680

size = llvm::utostr(variable.GetByteSize());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to check if the byte size is not zero and return an error if it is.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sent a fix at #81680

} else if (variablesReference == 0 && frame.IsValid()) {
// Name might be an expression. In this case we assume that name is composed
// of the number of bytes to watch and expression, separated by '@':
// "${size}@${expression}"
Comment on lines +2747 to +2749
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this format VS Code supported, or just something we are making up?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm making this up, since the spec doesn't say how to specify the number of bytes to watch. Do you have better idea?

llvm::StringRef expr;
std::tie(size, expr) = name.split('@');
lldb::SBValue value = frame.EvaluateExpression(expr.data());
if (value.GetError().Fail()) {
lldb::SBError error = value.GetError();
const char *error_cstr = error.GetCString();
body.try_emplace("dataId", nullptr);
body.try_emplace("description", error_cstr && error_cstr[0]
? std::string(error_cstr)
: "evaluation failed");
} else
addr = llvm::utohexstr(value.GetValueAsUnsigned());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably want to check this address by making sure it is valid. What if this return zero?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sent a fix at #81680

} else {
// We are expanding a variable that has children, so we will return its
// children.
lldb::SBValue container = g_dap.variables.GetVariable(variablesReference);
variable = container.GetChildMemberWithName(name.data());
if (!variable.IsValid()) {
if (name.starts_with("[")) {
llvm::StringRef index_str(name.drop_front(1));
uint64_t index = 0;
if (!index_str.consumeInteger(0, index)) {
if (index_str == "]")
variable = container.GetChildAtIndex(index);
}
}
}
auto state = g_dap.target.GetProcess().GetState();
body.try_emplace("dataId", nullptr);
body.try_emplace("description",
"variable not found: " + llvm::utostr(state));
}

if (variable.IsValid()) {
std::string addr = llvm::utohexstr(variable.GetLoadAddress());
std::string size = llvm::utostr(variable.GetByteSize());
if (!body.getObject("dataId")) {
body.try_emplace("dataId", addr + "/" + size);
body.try_emplace("accessTypes", std::move(accessTypes));
body.try_emplace("description",
size + " bytes at " + addr + " " + name.str());
} else {
// TODO: name might be an expression if variablesReference == 0. In that
// case, we need to evaluate expression in the scope of given frame (or
// global scope if not given), but SBTarget::EvaluateExpression can only
// evalute expression on selected thread and frame. Also, it doesn't specify
// the number of bytes to watch.
body.try_emplace("dataId", nullptr);
body.try_emplace("description", "variable not found.");
}

response.try_emplace("body", std::move(body));
g_dap.SendJSON(llvm::json::Value(std::move(response)));
}
Expand Down Expand Up @@ -3319,7 +3342,6 @@ void request_setVariable(const llvm::json::Object &request) {
const auto variablesReference =
GetUnsigned(arguments, "variablesReference", 0);
llvm::StringRef name = GetString(arguments, "name");
bool is_duplicated_variable_name = name.contains(" @");

const auto value = GetString(arguments, "value");
// Set success to false just in case we don't find the variable by name
Expand All @@ -3340,40 +3362,8 @@ void request_setVariable(const llvm::json::Object &request) {
const auto id_value = GetUnsigned(arguments, "id", UINT64_MAX);
if (id_value != UINT64_MAX) {
variable = g_dap.variables.GetVariable(id_value);
} else if (lldb::SBValueList *top_scope =
GetTopLevelScope(variablesReference)) {
// variablesReference is one of our scopes, not an actual variable it is
// asking for a variable in locals or globals or registers
int64_t end_idx = top_scope->GetSize();
// Searching backward so that we choose the variable in closest scope
// among variables of the same name.
for (int64_t i = end_idx - 1; i >= 0; --i) {
lldb::SBValue curr_variable = top_scope->GetValueAtIndex(i);
std::string variable_name = CreateUniqueVariableNameForDisplay(
curr_variable, is_duplicated_variable_name);
if (variable_name == name) {
variable = curr_variable;
break;
}
}
} else {
// This is not under the globals or locals scope, so there are no duplicated
// names.

// We have a named item within an actual variable so we need to find it
// withing the container variable by name.
lldb::SBValue container = g_dap.variables.GetVariable(variablesReference);
variable = container.GetChildMemberWithName(name.data());
if (!variable.IsValid()) {
if (name.starts_with("[")) {
llvm::StringRef index_str(name.drop_front(1));
uint64_t index = 0;
if (!index_str.consumeInteger(0, index)) {
if (index_str == "]")
variable = container.GetChildAtIndex(index);
}
}
}
variable = FindVariable(variablesReference, name);
}

if (variable.IsValid()) {
Expand Down
Loading