-
Notifications
You must be signed in to change notification settings - Fork 11.8k
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.
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
[lldb-dap][NFC] Add Breakpoint struct to share common logic. #80753
Conversation
@llvm/pr-subscribers-lldb Author: Zequan Wu (ZequanWu) ChangesThis adds a layer between I basically moved the Patch is 27.36 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/80753.diff 13 Files Affected:
diff --git a/lldb/tools/lldb-dap/Breakpoint.cpp b/lldb/tools/lldb-dap/Breakpoint.cpp
new file mode 100644
index 0000000000000..4ccf353b06ccc
--- /dev/null
+++ b/lldb/tools/lldb-dap/Breakpoint.cpp
@@ -0,0 +1,182 @@
+//===-- Breakpoint.cpp ------------------------------------------*- 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
+//
+//===----------------------------------------------------------------------===//
+
+#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);
+}
+
+// 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 Breakpoint::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() + last_raw_text_start,
+ logMessage.size() - last_raw_text_start),
+ /*is_expr=*/false);
+ if (error.Fail()) {
+ NotifyLogMessageError(error.GetCString());
+ return;
+ }
+ }
+
+ bp.SetCallback(BreakpointBase::BreakpointHitCallback, this);
+}
+
+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();
+ if (!logMessage.empty())
+ SetLogMessage();
+}
diff --git a/lldb/tools/lldb-dap/Breakpoint.h b/lldb/tools/lldb-dap/Breakpoint.h
new file mode 100644
index 0000000000000..a668e29f3d1e8
--- /dev/null
+++ b/lldb/tools/lldb-dap/Breakpoint.h
@@ -0,0 +1,34 @@
+//===-- 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 SetLogMessage() 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 fb4b27fbe315f..088ccc6b27f8e 100644
--- a/lldb/tools/lldb-dap/BreakpointBase.cpp
+++ b/lldb/tools/lldb-dap/BreakpointBase.cpp
@@ -8,7 +8,6 @@
#include "BreakpointBase.h"
#include "DAP.h"
-#include "JSONUtils.h"
#include "llvm/ADT/StringExtras.h"
using namespace lldb_dap;
@@ -18,14 +17,6 @@ BreakpointBase::BreakpointBase(const llvm::json::Object &obj)
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) {
@@ -164,110 +155,6 @@ lldb::SBError BreakpointBase::FormatLogText(llvm::StringRef text,
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() + last_raw_text_start,
- logMessage.size() - last_raw_text_start),
- /*is_expr=*/false);
- if (error.Fail()) {
- NotifyLogMessageError(error.GetCString());
- return;
- }
- }
-
- bp.SetCallback(BreakpointBase::BreakpointHitCallback, this);
-}
-
void BreakpointBase::NotifyLogMessageError(llvm::StringRef error) {
std::string message = "Log message has error: ";
message += error;
diff --git a/lldb/tools/lldb-dap/BreakpointBase.h b/lldb/tools/lldb-dap/BreakpointBase.h
index 41787f7861021..a4184bfe65d27 100644
--- a/lldb/tools/lldb-dap/BreakpointBase.h
+++ b/lldb/tools/lldb-dap/BreakpointBase.h
@@ -9,7 +9,6 @@
#ifndef LLDB_TOOLS_LLDB_DAP_BREAKPOINTBASE_H
#define LLDB_TOOLS_LLDB_DAP_BREAKPOINTBASE_H
-#include "JSONUtils.h"
#include "lldb/API/SBBreakpoint.h"
#include "llvm/Support/JSON.h"
#include <string>
@@ -35,15 +34,16 @@ struct BreakpointBase {
// interpolated.
std::string logMessage;
std::vector<LogMessagePart> logMessageParts;
- // The LLDB breakpoint associated wit this source breakpoint
- lldb::SBBreakpoint bp;
BreakpointBase() = default;
BreakpointBase(const llvm::json::Object &obj);
+ virtual ~BreakpointBase() = default;
+
+ virtual void SetCondition() = 0;
+ virtual void SetHitCondition() = 0;
+ virtual void SetLogMessage() = 0;
+ virtual void CreateJsonObject(llvm::json::Object &object) = 0;
- void SetCondition();
- void SetHitCondition();
- void SetLogMessage();
void UpdateBreakpoint(const BreakpointBase &request_bp);
// Format \param text and return formatted text in \param formatted.
diff --git a/lldb/tools/lldb-dap/CMakeLists.txt b/lldb/tools/lldb-dap/CMakeLists.txt
index 554567eb3b0e2..f8c0e4ecf36c2 100644
--- a/lldb/tools/lldb-dap/CMakeLists.txt
+++ b/lldb/tools/lldb-dap/CMakeLists.txt
@@ -24,6 +24,7 @@ tablegen(LLVM Options.inc -gen-opt-parser-defs)
add_public_tablegen_target(LLDBDAPOptionsTableGen)
add_lldb_tool(lldb-dap
lldb-dap.cpp
+ Breakpoint.cpp
BreakpointBase.cpp
ExceptionBreakpoint.cpp
FifoFiles.cpp
diff --git a/lldb/tools/lldb-dap/FunctionBreakpoint.cpp b/lldb/tools/lldb-dap/FunctionBreakpoint.cpp
index d4bdb976500ec..21743bf908706 100644
--- a/lldb/tools/lldb-dap/FunctionBreakpoint.cpp
+++ b/lldb/tools/lldb-dap/FunctionBreakpoint.cpp
@@ -12,21 +12,13 @@
namespace lldb_dap {
FunctionBreakpoint::FunctionBreakpoint(const llvm::json::Object &obj)
- : BreakpointBase(obj), functionName(std::string(GetString(obj, "name"))) {}
+ : Breakpoint(obj), functionName(std::string(GetString(obj, "name"))) {}
void FunctionBreakpoint::SetBreakpoint() {
if (functionName.empty())
return;
bp = g_dap.target.BreakpointCreateByName(functionName.c_str());
- // 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();
- if (!logMessage.empty())
- SetLogMessage();
+ Breakpoint::SetBreakpoint();
}
} // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/FunctionBreakpoint.h b/lldb/tools/lldb-dap/FunctionBreakpoint.h
index fc23e94e12876..b15ff1931a6b2 100644
--- a/lldb/tools/lldb-dap/FunctionBreakpoint.h
+++ b/lldb/tools/lldb-dap/FunctionBreakpoint.h
@@ -9,11 +9,11 @@
#ifndef LLDB_TOOLS_LLDB_DAP_FUNCTIONBREAKPOINT_H
#define LLDB_TOOLS_LLDB_DAP_FUNCTIONBREAKPOINT_H
-#include "BreakpointBase.h"
+#include "Breakpoint.h"
namespace lldb_dap {
-struct FunctionBreakpoint : public BreakpointBase {
+struct FunctionBreakpoint : public Breakpoint {
std::string functionName;
FunctionBreakpoint() = default;
diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp
index a8b438d9d6df3..878449a91aa66 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -364,54 +364,14 @@ llvm::json::Value CreateScope(const llvm::StringRef name,
// },
// "required": [ "verified" ]
// }
-llvm::json::Value CreateBreakpoint(lldb::SBBreakpoint &bp,
+llvm::json::Value CreateBreakpoint(BreakpointBase *bp,
std::optional<llvm::StringRef> request_path,
std::optional<uint32_t> request_line,
std::optional<uint32_t> request_column) {
- // 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.
llvm::json::Object object;
- if (!bp.IsValid())
- return llvm::json::Value(std::move(object));
-
- 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 (request_path)
object.try_emplace("source", CreateSource(*request_path));
-
- 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));
- }
+ bp->CreateJsonObject(object);
// We try to add request_line as a fallback
if (request_line)
object.try_emplace("line", *request_line);
@@ -506,7 +466,7 @@ llvm::json::Value CreateModule(lldb::SBModule &module) {
return llvm::json::Value(std::move(object));
}
-void AppendBreakpoint(lldb::SBBreakpoint &bp, llvm::json::Array &breakpoints,
+void AppendBreakpoint(BreakpointBase *bp, llvm::json::Array &breakpoints,
std::optional<llvm::StringRef> request_path,
std::optional<uint32_t> request_line) {
breakpoints.emplace_back(CreateBreakpoint(bp, request_path, request_line));
diff --git a/lldb/tools/lldb-dap/JSONUtils.h b/lldb/tools/lldb-dap/JSONUtils.h
index 62338548890c0..1515f5ba2e5f4 100644
--- a/lldb/tools/l...
[truncated]
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
…only show up in `SourceBreakpoint` json object.
I uploaded #81541 as reference for reviewers to understand the motivation of this patch. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks!
This implements functionality to handle `DataBreakpointInfo` request and `SetDataBreakpoints` request. 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.
) This adds a layer between `SounceBreakpoint`/`FunctionBreakpoint` and `BreakpointBase` to have better separation and encapsulation so we are not directly operating on `SBBreakpoint`. I basically moved the `SBBreakpoint` and the methods that requires it from `BreakpointBase` to `Breakpoint`. This allows adding support for data watchpoint easier by sharing the logic inside `BreakpointBase`.
This adds a layer between
SounceBreakpoint
/FunctionBreakpoint
andBreakpointBase
to have better separation and encapsulation so we are not directly operating onSBBreakpoint
.I basically moved the
SBBreakpoint
and the methods that requires it fromBreakpointBase
toBreakpoint
. This allows adding support for data watchpoint easier by sharing the logic insideBreakpointBase
.