[Lldb-commits] [PATCH] D140630: [lldb-vscode] Add data breakpoint support
cimacmillan added a comment. Ping @clayborg would you mind having another look? Thanks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140630/new/ https://reviews.llvm.org/D140630 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D140630: [lldb-vscode] Add data breakpoint support
cimacmillan created this revision.
Herald added a project: All.
cimacmillan requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D140630
Files:
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
lldb/test/API/tools/lldb-vscode/breakpoint_data/Makefile
lldb/test/API/tools/lldb-vscode/breakpoint_data/TestVSCode_setDataBreakpoints.py
lldb/test/API/tools/lldb-vscode/breakpoint_data/main.cpp
lldb/tools/lldb-vscode/CMakeLists.txt
lldb/tools/lldb-vscode/VSCode.h
lldb/tools/lldb-vscode/Watchpoint.cpp
lldb/tools/lldb-vscode/Watchpoint.h
lldb/tools/lldb-vscode/lldb-vscode.cpp
Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -41,6 +41,7 @@
#include
#include
#include
+#include
#include
#include "llvm/ADT/ArrayRef.h"
@@ -1541,6 +1542,8 @@
body.try_emplace("supportsProgressReporting", true);
// The debug adapter supports 'logMessage' in breakpoint.
body.try_emplace("supportsLogPoints", true);
+ // The debug adapter supports data breakpoints
+ body.try_emplace("supportsDataBreakpoints", true);
response.try_emplace("body", std::move(body));
g_vsc.SendJSON(llvm::json::Value(std::move(response)));
@@ -2117,6 +2120,337 @@
g_vsc.SendJSON(llvm::json::Value(std::move(response)));
}
+static WatchpointType get_watchpoint_type_from_request(std::string type) {
+ if (type == "write") {
+return WatchpointType::Write;
+ } else if (type == "read") {
+return WatchpointType::Read;
+ } else if (type == "readWrite") {
+return WatchpointType::ReadWrite;
+ }
+ std::string unknown_type_error = "Unknown watchpoint type: ";
+ unknown_type_error.append(type);
+ unknown_type_error.append(". Defaulting to ReadWrite.");
+ g_vsc.SendOutput(OutputType::Console, unknown_type_error);
+ return WatchpointType::ReadWrite;
+}
+
+static std::string set_data_breakpoint(const llvm::json::Object *breakpoint) {
+ std::string id = GetString(breakpoint, "id").str();
+ const char *data_c_str = GetString(breakpoint, "dataId").data();
+ bool enabled = GetBoolean(breakpoint, "enabled", false);
+ WatchpointType watchpoint_type = get_watchpoint_type_from_request(
+ GetString(breakpoint, "accessType").str());
+ uint64_t v_address;
+ size_t v_size;
+ sscanf(data_c_str, "%llu/%zu", &v_address, &v_size);
+
+ if (g_vsc.watchpoints.find(id) != g_vsc.watchpoints.end()) {
+auto existing_watchpoint = g_vsc.watchpoints.at(id);
+if (existing_watchpoint.is_enabled() != enabled)
+ existing_watchpoint.set_enabled(enabled);
+return id;
+ }
+
+ g_vsc.watchpoints.emplace(
+ id, Watchpoint(v_address, v_size, enabled, watchpoint_type));
+ return id;
+}
+
+// "SetDataBreakpointsRequest": {
+// "allOf": [ { "$ref": "#/definitions/Request" }, {
+// "type": "object",
+// "description": "Replaces all existing data breakpoints with new data
+// breakpoints.\nTo clear all data breakpoints, specify an empty
+// array.\nWhen a data breakpoint is hit, a `stopped` event (with reason
+// `data breakpoint`) is generated.\nClients should only call this request
+// if the corresponding capability `supportsDataBreakpoints` is true.",
+// "properties": {
+// "command": {
+// "type": "string",
+// "enum": [ "setDataBreakpoints" ]
+// },
+// "arguments": {
+// "$ref": "#/definitions/SetDataBreakpointsArguments"
+// }
+// },
+// "required": [ "command", "arguments" ]
+// }]
+// },
+// "SetDataBreakpointsArguments": {
+// "type": "object",
+// "description": "Arguments for `setDataBreakpoints` request.",
+// "properties": {
+// "breakpoints": {
+// "type": "array",
+// "items": {
+// "$ref": "#/definitions/DataBreakpoint"
+// },
+// "description": "The contents of this array replaces all existing data
+// breakpoints. An empty array clears all data breakpoints."
+// }
+// },
+// "required": [ "breakpoints" ]
+// },
+// "SetDataBreakpointsResponse": {
+// "allOf": [ { "$ref": "#/definitions/Response" }, {
+// "type": "object",
+// "description": "Response to `setDataBreakpoints` request.\nReturned is
+// information about each breakpoint created by this request.",
+// "properties": {
+// "body": {
+// "type": "object",
+// "properties": {
+// "breakpoints": {
+// "type": "array",
+// "items": {
+// "$ref": "#/definitions/Breakpoint"
+// },
+// "description": "Information about the data breakpoints. The array
+// elements correspond to the elements of the input argument
+//
[Lldb-commits] [PATCH] D140630: [lldb-vscode] Add data breakpoint support
cimacmillan added a comment. Ping! :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140630/new/ https://reviews.llvm.org/D140630 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D140630: [lldb-vscode] Add data breakpoint support
cimacmillan added a comment. Ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140630/new/ https://reviews.llvm.org/D140630 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D140630: [lldb-vscode] Add data breakpoint support
cimacmillan added a comment.
@clayborg Thanks for your feedback. I'm part the way through implementing your
changes. Specifically about this point:
> I seem to remember that it will disable this watchpoint as soon as a local
> variable goes out of scope, though I might not be remember things correctly
This is a behaviour I'd like to address, where for instance watchpoints are
triggered in different functions because the stack frame addresses align. I
have this example, I can add a test for:
int test_a() {
int x = 10; <- watchpoint set here
x = 20; <- watchpoint triggered here
}
int test_b() {
int b = 10; <- watchpoint triggered here also
b = 20;
}
int main() {
test_a();
test_b();
I've refactored the Watchpoint class to use "SBValue::Watch", and I can still
reproduce this behaviour. I also can't find where this watchpoint disable on
scope change might be implemented. Do you have any suggestions for this? Thanks
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D140630/new/
https://reviews.llvm.org/D140630
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D140630: [lldb-vscode] Add data breakpoint support
cimacmillan updated this revision to Diff 490077.
cimacmillan edited the summary of this revision.
cimacmillan added a comment.
Add handling for const and register cases. Setting watchpoint on SBValue rather
than the
address.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D140630/new/
https://reviews.llvm.org/D140630
Files:
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
lldb/test/API/tools/lldb-vscode/breakpoint_data/Makefile
lldb/test/API/tools/lldb-vscode/breakpoint_data/TestVSCode_setDataBreakpoints.py
lldb/test/API/tools/lldb-vscode/breakpoint_data/main.cpp
lldb/test/API/tools/lldb-vscode/breakpoint_data_optimized/Makefile
lldb/test/API/tools/lldb-vscode/breakpoint_data_optimized/TestVSCode_setDataBreakpoints.py
lldb/test/API/tools/lldb-vscode/breakpoint_data_optimized/main.cpp
lldb/tools/lldb-vscode/CMakeLists.txt
lldb/tools/lldb-vscode/VSCode.h
lldb/tools/lldb-vscode/Watchpoint.cpp
lldb/tools/lldb-vscode/Watchpoint.h
lldb/tools/lldb-vscode/lldb-vscode.cpp
Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -41,6 +41,7 @@
#include
#include
#include
+#include
#include
#include "llvm/ADT/ArrayRef.h"
@@ -56,6 +57,8 @@
#include "llvm/Support/PrettyStackTrace.h"
#include "llvm/Support/raw_ostream.h"
+#include "lldb/API/SBMemoryRegionInfo.h"
+
#include "JSONUtils.h"
#include "LLDBUtils.h"
#include "OutputRedirector.h"
@@ -1541,6 +1544,8 @@
body.try_emplace("supportsProgressReporting", true);
// The debug adapter supports 'logMessage' in breakpoint.
body.try_emplace("supportsLogPoints", true);
+ // The debug adapter supports data breakpoints
+ body.try_emplace("supportsDataBreakpoints", true);
response.try_emplace("body", std::move(body));
g_vsc.SendJSON(llvm::json::Value(std::move(response)));
@@ -2117,6 +2122,365 @@
g_vsc.SendJSON(llvm::json::Value(std::move(response)));
}
+static WatchpointType get_watchpoint_type_from_request(std::string type) {
+ if (type == "write") {
+return WatchpointType::Write;
+ } else if (type == "read") {
+return WatchpointType::Read;
+ } else if (type == "readWrite") {
+return WatchpointType::ReadWrite;
+ }
+ std::string unknown_type_error = "Unknown watchpoint type: ";
+ unknown_type_error.append(type);
+ unknown_type_error.append(". Defaulting to ReadWrite.");
+ g_vsc.SendOutput(OutputType::Console, unknown_type_error);
+ return WatchpointType::ReadWrite;
+}
+
+static lldb::SBValue get_variable(std::string variable_name,
+ uint32_t variables_reference) {
+ bool is_duplicated_variable_name =
+ variable_name.find(" @") != llvm::StringRef::npos;
+ lldb::SBValue variable;
+
+ if (lldb::SBValueList *top_scope = GetTopLevelScope(variables_reference)) {
+// 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 local_variable = CreateUniqueVariableNameForDisplay(
+ curr_variable, is_duplicated_variable_name);
+ if (variable_name == local_variable) {
+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_vsc.variables.GetVariable(variables_reference);
+if (!container.IsValid()) {
+ return variable;
+}
+
+variable = container.GetChildMemberWithName(variable_name.data());
+if (!variable.IsValid()) {
+ if (variable_name.size() > 0 && variable_name[0] == '[') {
+llvm::StringRef index_str(std::move(variable_name.substr(1)));
+uint64_t index = 0;
+if (!index_str.consumeInteger(0, index)) {
+ if (index_str == "]")
+variable = container.GetChildAtIndex(index);
+}
+ }
+}
+ }
+
+ return variable;
+}
+
+static std::optional set_data_breakpoint(const llvm::json::Object *breakpoint) {
+ std::string breakpoint_id = GetString(breakpoint, "id").str();
+ std::string data_id = GetString(breakpoint, "dataId").str();
+ bool enabled = GetBoolean(breakpoint, "enabled", false);
+ WatchpointType watchpoint_type = get_watchpoint_type_from_request(
+ GetString(breakpoint, "accessType").str());
+
+ if (g_vsc.watchpoints.find
[Lldb-commits] [PATCH] D140630: [lldb-vscode] Add data breakpoint support
cimacmillan marked 6 inline comments as done.
cimacmillan added inline comments.
Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:2188
+
+static std::optional set_data_breakpoint(const llvm::json::Object
*breakpoint) {
+ std::string breakpoint_id = GetString(breakpoint, "id").str();
clayborg wrote:
> This function should probably be able to return an error for each watchpoint
> if anything fails so this can be reported back as a description or error
> response in the "setDataBreakpoints" response? We shouldn't be printing to
> the console for errors in the "breakpoint" object arguments or if the
> watchpoint actually fails to resolve (was in a register). An error message
> should be returned somehow if this is possible?
Thanks for your feedback, working through comments now. About this and
> Is there a way to return an error for a specific data breakpoint instead of
> us printing to the console?
We can return whether the breakpoint is verified, which displays a greyed dot
similar to when a line breakpoint can't be set. There doesn't seem to be a way
to display the error in VS code with the message. I think the logging would be
useful in that case, but I can also include it in the message response in case
this is added to VS code.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D140630/new/
https://reviews.llvm.org/D140630
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D140630: [lldb-vscode] Add data breakpoint support
cimacmillan updated this revision to Diff 493936.
cimacmillan added a comment.
Refactored Watchpoint class and addressed comments
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D140630/new/
https://reviews.llvm.org/D140630
Files:
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
lldb/test/API/tools/lldb-vscode/breakpoint_data/Makefile
lldb/test/API/tools/lldb-vscode/breakpoint_data/TestVSCode_setDataBreakpoints.py
lldb/test/API/tools/lldb-vscode/breakpoint_data/main.cpp
lldb/test/API/tools/lldb-vscode/breakpoint_data_optimized/Makefile
lldb/test/API/tools/lldb-vscode/breakpoint_data_optimized/TestVSCode_setDataBreakpoints.py
lldb/test/API/tools/lldb-vscode/breakpoint_data_optimized/main.cpp
lldb/tools/lldb-vscode/CMakeLists.txt
lldb/tools/lldb-vscode/VSCode.h
lldb/tools/lldb-vscode/Watchpoint.cpp
lldb/tools/lldb-vscode/Watchpoint.h
lldb/tools/lldb-vscode/lldb-vscode.cpp
Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -41,6 +41,7 @@
#include
#include
#include
+#include
#include
#include "llvm/ADT/ArrayRef.h"
@@ -56,6 +57,8 @@
#include "llvm/Support/PrettyStackTrace.h"
#include "llvm/Support/raw_ostream.h"
+#include "lldb/API/SBMemoryRegionInfo.h"
+
#include "JSONUtils.h"
#include "LLDBUtils.h"
#include "OutputRedirector.h"
@@ -1541,6 +1544,8 @@
body.try_emplace("supportsProgressReporting", true);
// The debug adapter supports 'logMessage' in breakpoint.
body.try_emplace("supportsLogPoints", true);
+ // The debug adapter supports data breakpoints
+ body.try_emplace("supportsDataBreakpoints", true);
response.try_emplace("body", std::move(body));
g_vsc.SendJSON(llvm::json::Value(std::move(response)));
@@ -2117,6 +2122,334 @@
g_vsc.SendJSON(llvm::json::Value(std::move(response)));
}
+static lldb::SBValue get_variable(std::string variable_name,
+ uint32_t variables_reference) {
+ bool is_duplicated_variable_name =
+ variable_name.find(" @") != llvm::StringRef::npos;
+ lldb::SBValue variable;
+
+ if (lldb::SBValueList *top_scope = GetTopLevelScope(variables_reference)) {
+// 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 local_variable = CreateUniqueVariableNameForDisplay(
+ curr_variable, is_duplicated_variable_name);
+ if (variable_name == local_variable) {
+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_vsc.variables.GetVariable(variables_reference);
+if (!container.IsValid()) {
+ return variable;
+}
+
+variable = container.GetChildMemberWithName(variable_name.data());
+if (!variable.IsValid()) {
+ if (variable_name.size() > 0 && variable_name[0] == '[') {
+llvm::StringRef index_str(std::move(variable_name.substr(1)));
+uint64_t index = 0;
+if (!index_str.consumeInteger(0, index)) {
+ if (index_str == "]")
+variable = container.GetChildAtIndex(index);
+}
+ }
+}
+ }
+
+ return variable;
+}
+
+// "SetDataBreakpointsRequest": {
+// "allOf": [ { "$ref": "#/definitions/Request" }, {
+// "type": "object",
+// "description": "Replaces all existing data breakpoints with new data
+// breakpoints.\nTo clear all data breakpoints, specify an empty
+// array.\nWhen a data breakpoint is hit, a `stopped` event (with reason
+// `data breakpoint`) is generated.\nClients should only call this request
+// if the corresponding capability `supportsDataBreakpoints` is true.",
+// "properties": {
+// "command": {
+// "type": "string",
+// "enum": [ "setDataBreakpoints" ]
+// },
+// "arguments": {
+// "$ref": "#/definitions/SetDataBreakpointsArguments"
+// }
+// },
+// "required": [ "command", "arguments" ]
+// }]
+// },
+// "SetDataBreakpointsArguments": {
+// "type": "object",
+// "description": "Arguments for `setDataBreakpoints` request.",
+// "properties": {
+// "breakpoints": {
+// "type": "array",
+// "items": {
+// "$ref": "#/definitions/Da
[Lldb-commits] [PATCH] D140630: [lldb-vscode] Add data breakpoint support
cimacmillan marked 12 inline comments as done. cimacmillan added a comment. @clayborg Thanks for your feedback. I've refactored the Watchpoint class slightly so that it encapsulates the parsing of the setDataBreakpoints request and be used to get the Breakpoint info to attach to the response. This is more similar to how the other breakpoint classes are written, and encapsulates things a bit better. Comment at: lldb/test/API/tools/lldb-vscode/breakpoint_data/TestVSCode_setDataBreakpoints.py:73 +num_a = array_find(locals, lambda x: x['name'] == 'num_a') +self.assertIsNotNone(num_a) + clayborg wrote: > might be better as suggested? There isn't an IsValid function in this case as it's not the SBValue type. I suppose is the variable was invalid, the rest of the test would fail. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140630/new/ https://reviews.llvm.org/D140630 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D140630: [lldb-vscode] Add data breakpoint support
cimacmillan added a comment. Ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140630/new/ https://reviews.llvm.org/D140630 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D140630: [lldb-vscode] Add data breakpoint support
cimacmillan added a comment. Ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140630/new/ https://reviews.llvm.org/D140630 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D140630: [lldb-vscode] Add data breakpoint support
cimacmillan added a comment. Ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140630/new/ https://reviews.llvm.org/D140630 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D140630: [lldb-vscode] Add data breakpoint support
cimacmillan added a comment. Ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140630/new/ https://reviews.llvm.org/D140630 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
