[llvm] [lldb] [lldb-dap][NFC] Add Breakpoint struct to share common logic. (PR #80753)
via llvm-commits
llvm-commits at lists.llvm.org
Mon Feb 5 14:39:17 PST 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: Zequan Wu (ZequanWu)
<details>
<summary>Changes</summary>
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`.
---
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:
- (added) lldb/tools/lldb-dap/Breakpoint.cpp (+182)
- (added) lldb/tools/lldb-dap/Breakpoint.h (+34)
- (modified) lldb/tools/lldb-dap/BreakpointBase.cpp (-113)
- (modified) lldb/tools/lldb-dap/BreakpointBase.h (+6-6)
- (modified) lldb/tools/lldb-dap/CMakeLists.txt (+1)
- (modified) lldb/tools/lldb-dap/FunctionBreakpoint.cpp (+2-10)
- (modified) lldb/tools/lldb-dap/FunctionBreakpoint.h (+2-2)
- (modified) lldb/tools/lldb-dap/JSONUtils.cpp (+3-43)
- (modified) lldb/tools/lldb-dap/JSONUtils.h (+3-2)
- (modified) lldb/tools/lldb-dap/SourceBreakpoint.cpp (+2-10)
- (modified) lldb/tools/lldb-dap/SourceBreakpoint.h (+3-3)
- (modified) lldb/tools/lldb-dap/lldb-dap.cpp (+9-8)
- (modified) llvm/utils/gn/secondary/lldb/tools/lldb-dap/BUILD.gn (+1)
``````````diff
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]
``````````
</details>
https://github.com/llvm/llvm-project/pull/80753
More information about the llvm-commits
mailing list