[lldb-dap] Add support for data breakpoint. #81541
Merged
Conversation
This file contains 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
|
@llvm/pr-subscribers-lldb Author: Zequan Wu (ZequanWu) ChangesThis implements functionality to handle It doesn't handle the case when This is based on top of #80753. Patch is 62.88 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/81541.diff 20 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 bb863bb8719176..2d48cfbd819366 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
@@ -501,6 +501,17 @@ def get_local_variable_value(self, name, frameIndex=0, threadId=None):
return variable["value"]
return None
+ def get_local_variable_child(self, name, child_name, frameIndex=0, threadId=None):
+ local = self.get_local_variable(name, frameIndex, threadId)
+ if local["variablesReference"] == 0:
+ return None
+ children = self.request_variables(local["variablesReference"])[
+ "body"]["variables"]
+ for child in children:
+ if child["name"] == child_name:
+ return child
+ return None
+
def replay_packets(self, replay_file_path):
f = open(replay_file_path, "r")
mode = "invalid"
@@ -895,6 +906,32 @@ 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}
+ command_dict = {
+ "command": "dataBreakpointInfo",
+ "type": "request",
+ "arguments": args_dict,
+ }
+ return self.send_recv(command_dict)
+
+ def request_setDataBreakpoint(self, dataBreakpoints):
+ """dataBreakpoints is a list of dictionary with following fields:
+ {
+ dataId: (address in hex)/(size in bytes)
+ accessType: read/write/readWrite
+ [condition]: string
+ [hitCondition]: string
+ }
+ """
+ args_dict = {"breakpoints": dataBreakpoints}
+ command_dict = {
+ "command": "setDataBreakpoints",
+ "type": "request",
+ "arguments": args_dict,
+ }
+ return self.send_recv(command_dict)
+
def request_compileUnits(self, moduleId):
args_dict = {"moduleId": moduleId}
command_dict = {
diff --git a/lldb/test/API/tools/lldb-dap/databreakpoint/Makefile b/lldb/test/API/tools/lldb-dap/databreakpoint/Makefile
new file mode 100644
index 00000000000000..99998b20bcb050
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/databreakpoint/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
diff --git a/lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py b/lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py
new file mode 100644
index 00000000000000..a13b4399a02f18
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py
@@ -0,0 +1,100 @@
+"""
+Test lldb-dap dataBreakpointInfo and setDataBreakpoints requests
+"""
+
+
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+import lldbdap_testcase
+
+
+class TestDAP_setDataBreakpoints(lldbdap_testcase.DAPTestCaseBase):
+ def setUp(self):
+ lldbdap_testcase.DAPTestCaseBase.setUp(self)
+ self.accessTypes = ["read", "write", "readWrite"]
+
+ @skipIfWindows
+ @skipIfRemote
+ def test_functionality(self):
+ """Tests setting data breakpoints.
+ """
+ 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.dap_server.get_local_variables()
+ # Test write watchpoints on x, arr[2]
+ response_x = self.dap_server.request_dataBreakpointInfo(1, "x")
+ arr = self.dap_server.get_local_variable("arr")
+ response_arr_2 = self.dap_server.request_dataBreakpointInfo(
+ arr["variablesReference"], "[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")
+ self.dap_server.request_setDataBreakpoint([])
+
+ # Test hit condition
+ second_loop_break_line = line_number(
+ source, "// second loop breakpoint")
+ breakpoint_ids = self.set_source_breakpoints(
+ source, [second_loop_break_line])
+ self.continue_to_breakpoints(breakpoint_ids)
+ dataBreakpoints = [
+ {
+ "dataId": response_x["body"]["dataId"],
+ "accessType": "write",
+ "hitCondition": "2"
+ }
+ ]
+ 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")
+ self.assertEquals(x_val, "3")
+
+ # Test condition
+ dataBreakpoints = [
+ {
+ "dataId": response_x["body"]["dataId"],
+ "accessType": "write",
+ "condition": "x==10"
+ }
+ ]
+ 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")
+ self.assertEquals(x_val, "10")
diff --git a/lldb/test/API/tools/lldb-dap/databreakpoint/main.cpp b/lldb/test/API/tools/lldb-dap/databreakpoint/main.cpp
new file mode 100644
index 00000000000000..8082fe02f3e534
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/databreakpoint/main.cpp
@@ -0,0 +1,17 @@
+int main(int argc, char const *argv[]) {
+ // Test for data breakpoint
+ int x = 0;
+ char arr[4] = {'a', 'b', 'c', 'd'};
+ for (int i = 0; i < 5; ++i) { // first loop breakpoint
+ if (i == 1) {
+ x = i + 1;
+ } else if (i == 2) {
+ arr[i] = 'z';
+ }
+ }
+
+ x = 1;
+ for (int i = 0; i < 10; ++i) { // second loop breakpoint
+ ++x;
+ }
+}
diff --git a/lldb/tools/lldb-dap/Breakpoint.cpp b/lldb/tools/lldb-dap/Breakpoint.cpp
new file mode 100644
index 00000000000000..0c33d4b114d760
--- /dev/null
+++ b/lldb/tools/lldb-dap/Breakpoint.cpp
@@ -0,0 +1,76 @@
+//===-- Breakpoint.cpp ----------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "Breakpoint.h"
+#include "DAP.h"
+#include "JSONUtils.h"
+#include "llvm/ADT/StringExtras.h"
+
+using namespace lldb_dap;
+
+void Breakpoint::SetCondition() { bp.SetCondition(condition.c_str()); }
+
+void Breakpoint::SetHitCondition() {
+ uint64_t hitCount = 0;
+ if (llvm::to_integer(hitCondition, hitCount))
+ bp.SetIgnoreCount(hitCount - 1);
+}
+
+void Breakpoint::CreateJsonObject(llvm::json::Object &object) {
+ // Each breakpoint location is treated as a separate breakpoint for VS code.
+ // They don't have the notion of a single breakpoint with multiple locations.
+ if (!bp.IsValid())
+ return;
+ object.try_emplace("verified", bp.GetNumResolvedLocations() > 0);
+ object.try_emplace("id", bp.GetID());
+ // VS Code DAP doesn't currently allow one breakpoint to have multiple
+ // locations so we just report the first one. If we report all locations
+ // then the IDE starts showing the wrong line numbers and locations for
+ // other source file and line breakpoints in the same file.
+
+ // Below we search for the first resolved location in a breakpoint and report
+ // this as the breakpoint location since it will have a complete location
+ // that is at least loaded in the current process.
+ lldb::SBBreakpointLocation bp_loc;
+ const auto num_locs = bp.GetNumLocations();
+ for (size_t i = 0; i < num_locs; ++i) {
+ bp_loc = bp.GetLocationAtIndex(i);
+ if (bp_loc.IsResolved())
+ break;
+ }
+ // If not locations are resolved, use the first location.
+ if (!bp_loc.IsResolved())
+ bp_loc = bp.GetLocationAtIndex(0);
+ auto bp_addr = bp_loc.GetAddress();
+
+ if (bp_addr.IsValid()) {
+ std::string formatted_addr =
+ "0x" + llvm::utohexstr(bp_addr.GetLoadAddress(g_dap.target));
+ object.try_emplace("instructionReference", formatted_addr);
+ auto line_entry = bp_addr.GetLineEntry();
+ const auto line = line_entry.GetLine();
+ if (line != UINT32_MAX)
+ object.try_emplace("line", line);
+ const auto column = line_entry.GetColumn();
+ if (column != 0)
+ object.try_emplace("column", column);
+ object.try_emplace("source", CreateSource(line_entry));
+ }
+}
+
+bool Breakpoint::MatchesName(const char *name) { return bp.MatchesName(name); }
+
+void Breakpoint::SetBreakpoint() {
+ // See comments in BreakpointBase::GetBreakpointLabel() for details of why
+ // we add a label to our breakpoints.
+ bp.AddName(GetBreakpointLabel());
+ if (!condition.empty())
+ SetCondition();
+ if (!hitCondition.empty())
+ SetHitCondition();
+}
diff --git a/lldb/tools/lldb-dap/Breakpoint.h b/lldb/tools/lldb-dap/Breakpoint.h
new file mode 100644
index 00000000000000..47a9d9c59ae2b7
--- /dev/null
+++ b/lldb/tools/lldb-dap/Breakpoint.h
@@ -0,0 +1,33 @@
+//===-- Breakpoint.h --------------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_TOOLS_LLDB_DAP_BREAKPOINT_H
+#define LLDB_TOOLS_LLDB_DAP_BREAKPOINT_H
+
+#include "BreakpointBase.h"
+
+namespace lldb_dap {
+
+struct Breakpoint : public BreakpointBase {
+ // The LLDB breakpoint associated wit this source breakpoint
+ lldb::SBBreakpoint bp;
+
+ Breakpoint() = default;
+ Breakpoint(const llvm::json::Object &obj) : BreakpointBase(obj){};
+ Breakpoint(lldb::SBBreakpoint bp) : bp(bp) {}
+
+ void SetCondition() override;
+ void SetHitCondition() override;
+ void CreateJsonObject(llvm::json::Object &object) override;
+
+ bool MatchesName(const char *name);
+ void SetBreakpoint();
+};
+} // namespace lldb_dap
+
+#endif
diff --git a/lldb/tools/lldb-dap/BreakpointBase.cpp b/lldb/tools/lldb-dap/BreakpointBase.cpp
index fb4b27fbe315fc..519729f5519ffc 100644
--- a/lldb/tools/lldb-dap/BreakpointBase.cpp
+++ b/lldb/tools/lldb-dap/BreakpointBase.cpp
@@ -8,306 +8,13 @@
#include "BreakpointBase.h"
#include "DAP.h"
-#include "JSONUtils.h"
#include "llvm/ADT/StringExtras.h"
using namespace lldb_dap;
BreakpointBase::BreakpointBase(const llvm::json::Object &obj)
: condition(std::string(GetString(obj, "condition"))),
- hitCondition(std::string(GetString(obj, "hitCondition"))),
- logMessage(std::string(GetString(obj, "logMessage"))) {}
-
-void BreakpointBase::SetCondition() { bp.SetCondition(condition.c_str()); }
-
-void BreakpointBase::SetHitCondition() {
- uint64_t hitCount = 0;
- if (llvm::to_integer(hitCondition, hitCount))
- bp.SetIgnoreCount(hitCount - 1);
-}
-
-lldb::SBError BreakpointBase::AppendLogMessagePart(llvm::StringRef part,
- bool is_expr) {
- if (is_expr) {
- logMessageParts.emplace_back(part, is_expr);
- } else {
- std::string formatted;
- lldb::SBError error = FormatLogText(part, formatted);
- if (error.Fail())
- return error;
- logMessageParts.emplace_back(formatted, is_expr);
- }
- return lldb::SBError();
-}
-
-// TODO: consolidate this code with the implementation in
-// FormatEntity::ParseInternal().
-lldb::SBError BreakpointBase::FormatLogText(llvm::StringRef text,
- std::string &formatted) {
- lldb::SBError error;
- while (!text.empty()) {
- size_t backslash_pos = text.find_first_of('\\');
- if (backslash_pos == std::string::npos) {
- formatted += text.str();
- return error;
- }
-
- formatted += text.substr(0, backslash_pos).str();
- // Skip the characters before and including '\'.
- text = text.drop_front(backslash_pos + 1);
-
- if (text.empty()) {
- error.SetErrorString(
- "'\\' character was not followed by another character");
- return error;
- }
-
- const char desens_char = text[0];
- text = text.drop_front(); // Skip the desensitized char character
- switch (desens_char) {
- case 'a':
- formatted.push_back('\a');
- break;
- case 'b':
- formatted.push_back('\b');
- break;
- case 'f':
- formatted.push_back('\f');
- break;
- case 'n':
- formatted.push_back('\n');
- break;
- case 'r':
- formatted.push_back('\r');
- break;
- case 't':
- formatted.push_back('\t');
- break;
- case 'v':
- formatted.push_back('\v');
- break;
- case '\'':
- formatted.push_back('\'');
- break;
- case '\\':
- formatted.push_back('\\');
- break;
- case '0':
- // 1 to 3 octal chars
- {
- if (text.empty()) {
- error.SetErrorString("missing octal number following '\\0'");
- return error;
- }
-
- // Make a string that can hold onto the initial zero char, up to 3
- // octal digits, and a terminating NULL.
- char oct_str[5] = {0, 0, 0, 0, 0};
-
- size_t i;
- for (i = 0;
- i < text.size() && i < 4 && (text[i] >= '0' && text[i] <= '7');
- ++i) {
- oct_str[i] = text[i];
- }
-
- text = text.drop_front(i);
- unsigned long octal_value = ::strtoul(oct_str, nullptr, 8);
- if (octal_value <= UINT8_MAX) {
- formatted.push_back((char)octal_value);
- } else {
- error.SetErrorString("octal number is larger than a single byte");
- return error;
- }
- }
- break;
-
- case 'x': {
- if (text.empty()) {
- error.SetErrorString("missing hex number following '\\x'");
- return error;
- }
- // hex number in the text
- if (isxdigit(text[0])) {
- // Make a string that can hold onto two hex chars plus a
- // NULL terminator
- char hex_str[3] = {0, 0, 0};
- hex_str[0] = text[0];
-
- text = text.drop_front();
-
- if (!text.empty() && isxdigit(text[0])) {
- hex_str[1] = text[0];
- text = text.drop_front();
- }
-
- unsigned long hex_value = strtoul(hex_str, nullptr, 16);
- if (hex_value <= UINT8_MAX) {
- formatted.push_back((char)hex_value);
- } else {
- error.SetErrorString("hex number is larger than a single byte");
- return error;
- }
- } else {
- formatted.push_back(desens_char);
- }
- break;
- }
-
- default:
- // Just desensitize any other character by just printing what came
- // after the '\'
- formatted.push_back(desens_char);
- break;
- }
- }
- return error;
-}
-
-// logMessage will be divided into array of LogMessagePart as two kinds:
-// 1. raw print text message, and
-// 2. interpolated expression for evaluation which is inside matching curly
-// braces.
-//
-// The function tries to parse logMessage into a list of LogMessageParts
-// for easy later access in BreakpointHitCallback.
-void BreakpointBase::SetLogMessage() {
- logMessageParts.clear();
-
- // Contains unmatched open curly braces indices.
- std::vector<int> unmatched_curly_braces;
-
- // Contains all matched curly braces in logMessage.
- // Loop invariant: matched_curly_braces_ranges are sorted by start index in
- // ascending order without any overlap between them.
- std::vector<std::pair<int, int>> matched_curly_braces_ranges;
-
- lldb::SBError error;
- // Part1 - parse matched_curly_braces_ranges.
- // locating all curly braced expression ranges in logMessage.
- // The algorithm takes care of nested and imbalanced curly braces.
- for (size_t i = 0; i < logMessage.size(); ++i) {
- if (logMessage[i] == '{') {
- unmatched_curly_braces.push_back(i);
- } else if (logMessage[i] == '}') {
- if (unmatched_curly_braces.empty())
- // Nothing to match.
- continue;
-
- int last_unmatched_index = unmatched_curly_braces.back();
- unmatched_curly_braces.pop_back();
-
- // Erase any matched ranges included in the new match.
- while (!matched_curly_braces_ranges.empty()) {
- assert(matched_curly_braces_ranges.back().first !=
- last_unmatched_index &&
- "How can a curley brace be matched twice?");
- if (matched_curly_braces_ranges.back().first < last_unmatched_index)
- break;
-
- // This is a nested range let's earse it.
- assert((size_t)matched_curly_braces_ranges.back().second < i);
- matched_curly_braces_ranges.pop_back();
- }
-
- // Assert invariant.
- assert(matched_curly_braces_ranges.empty() ||
- matched_curly_braces_ranges.back().first < last_unmatched_index);
- matched_curly_braces_ranges.emplace_back(last_unmatched_index, i);
- }
- }
-
- // Part2 - parse raw text and expresions parts.
- // All expression ranges have been parsed in matched_curly_braces_ranges.
- // The code below uses matched_curly_braces_ranges to divide logMessage
- // into raw text parts and expression parts.
- int last_raw_text_start = 0;
- for (const std::pair<int, int> &curly_braces_range :
- matched_curly_braces_ranges) {
- // Raw text before open curly brace.
- assert(curly_braces_range.first >= last_raw_text_start);
- size_t raw_text_len = curly_braces_range.first - last_raw_text_start;
- if (raw_text_len > 0) {
- error = AppendLogMessagePart(
- llvm::StringRef(logMessage.c_str() + last_raw_text_start,
- raw_text_len),
- /*is_expr=*/false);
- if (error.Fail()) {
- NotifyLogMessageError(error.GetCString());
- return;
- }
- }
-
- // Expression between curly braces.
- assert(curly_braces_range.second > curly_braces_range.first);
- size_t expr_len = curly_braces_range.second - curly_braces_range.first - 1;
- error = AppendLogMessagePart(
- llvm::StringRef(logMessage.c_str() + curly_braces_range.first + 1,
- expr_len),
- /*is_expr=*/true);
- if (error.Fail()) {
- NotifyLogMessageError(error.GetCString());
- return;
- }
-
- last_raw_text_start = curly_braces_range.second + 1;
- }
- // Trailing raw text after close curly brace.
- assert(last_raw_text_start >= 0);
- if (logMessage.size() > (size_t)last_raw_text_start) {
- error = AppendLogMessagePart(
- llvm::StringRef(logMessage.c_str() + las...
[truncated]
|
|
✅ With the latest revision this PR passed the Python code formatter. |
| return self.send_recv(command_dict) | ||
|
|
||
| def request_setDataBreakpoint(self, dataBreakpoints): | ||
| """dataBreakpoints is a list of dictionary with following fields: |
There was a problem hiding this comment.
list of dictionary sounds very weird. Did you mean something else?
There was a problem hiding this comment.
dataBreakpoints is a list of the dataBreakpoint which has following format:
{
dataId: (address in hex)/(size in bytes)
accessType: read/write/readWrite
[condition]: string
[hitCondition]: string
}
Yes, I already split it out: #80753 is the refactor pr. I'll rebase this one once that is merged. |
|
just ping me when this PR is rebased |
This implements functionality to handle DataBreakpointInfo request and SetDataBreakpoints request. It doesn't handle the case when name is an expression, see Todo comment for details.
ZequanWu
force-pushed
the
lldb-dap-watchpoint
branch
from
2743af5
to
a2d2869
Compare
Rebased. |
ZequanWu
changed the title
Comment on lines
+2696
to
+2697
| const auto variablesReference = | ||
| GetUnsigned(arguments, "variablesReference", 0); |
There was a problem hiding this comment.
it'd be better to use a non-zero default value, which might collide with an actual variable ref id
There was a problem hiding this comment.
If variablesReference is 0, it's treated as not provided as it's only meaningful if its value > 0. This is also how request_variables and request_setVariable handle it.
The spec https://microsoft.github.io/debug-adapter-protocol/specification#Types_Variable says: "If variablesReference is > 0, the variable is structured ...".
- Add support for setting data breakpoint with expression. If `variablesReference` is 0 or not provided, interprete `name` as `${number of bytes}@${expression}` to set data breakpoint at the given expression.
walter-erquinigo
approved these changes
Feb 13, 2024
|
I updated to add support for setting data breakpoint with expression. If |
|
I think using @ is fine, but we can revisit it later if we see any issues. |
clayborg
reviewed
Feb 13, 2024
Comment on lines
+575
to
+576
| std::string variable_name = CreateUniqueVariableNameForDisplay( | ||
| curr_variable, is_duplicated_variable_name); |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.IsValid()) { | ||
| addr = llvm::utohexstr(variable.GetLoadAddress()); |
There was a problem hiding this comment.
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().
There was a problem hiding this comment.
Sent a fix at #81680
| } | ||
| if (variable.IsValid()) { | ||
| addr = llvm::utohexstr(variable.GetLoadAddress()); | ||
| size = llvm::utostr(variable.GetByteSize()); |
There was a problem hiding this comment.
Need to check if the byte size is not zero and return an error if it is.
There was a problem hiding this comment.
Sent a fix at #81680
Comment on lines
+2747
to
+2749
| // 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}" |
There was a problem hiding this comment.
Is this format VS Code supported, or just something we are making up?
There was a problem hiding this comment.
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?
| ? std::string(error_cstr) | ||
| : "evaluation failed"); | ||
| } else | ||
| addr = llvm::utohexstr(value.GetValueAsUnsigned()); |
There was a problem hiding this comment.
We probably want to check this address by making sure it is valid. What if this return zero?
There was a problem hiding this comment.
Sent a fix at #81680
nico
reviewed
Feb 13, 2024
| @@ -52,5 +52,6 @@ executable("lldb-dap") { | |||
| "RunInTerminal.cpp", | |||
| "SourceBreakpoint.cpp", | |||
| "lldb-dap.cpp", | |||
| "Watchpoint.cpp" | |||
There was a problem hiding this comment.
Please don't update gn build files if you don't use the gn build. There's a bot that is able to do it most of the time, and it doesn't make mistakes like forgetting the trailing comma here :)
|
We (Fuchsia clang toolchain team) are facing a LLDB test failure in our ARM64 Clang toolchain builders and I suspect this change is causing the failure. I am building current ToT without this commit to verify if this failure goes away. |
Prabhuk
added a commit
to Prabhuk/llvm-project
that referenced
this pull request
Feb 15, 2024
This reverts commit 8c56e78. Reverting to address the LLDB test failure in ARM64.
|
From logs, looks like setting breakpoint with size being 1 byte failed on arm, and works with size being 4 bytes: |
Prabhuk
added a commit
that referenced
this pull request
Feb 15, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 implements functionality to handle
DataBreakpointInforequest andSetDataBreakpointsrequest.If variablesReference is 0 or not provided, interpret name as ${number of bytes}@${expression} to set data breakpoint at the given expression because the spec https://microsoft.github.io/debug-adapter-protocol/specification#Requests_DataBreakpointInfo doesn't say how the client could specify the number of bytes to watch.
This is based on top of #80753.