clayborg updated this revision to Diff 243385.
clayborg added a comment.
Fixed Pavel's issues. I thoroughly tested this and found that we should be only
sending "changed" events for breakpoints since breakpoints never go away.
Changed in this patch:
- add a "vscode" name to each breakpoint so we know which breakpoints were set
in the IDE UI. Any breakpoints that don't have this won't get updates to the
IDE since the IDE doesn't know about them. This is in case people set
breakpoints in the debugger console. I tried reporting such breakpoints as
"new" breakpoints, but the IDE UI doesn't do anything with new breakpoint that
weren't created in the UI.
- Fixed logic to always report "changed" as the breakpoint event reason. LLDB
breakpoints never go away, so they are never "new" or "removed".
- If a breakpoint has no resolved locations, then the breakpoint is marked as
"verified": false in the breakpoint responses and events. When a breakpoint
resolves then it will get updated.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73665/new/
https://reviews.llvm.org/D73665
Files:
lldb/tools/lldb-vscode/BreakpointBase.cpp
lldb/tools/lldb-vscode/BreakpointBase.h
lldb/tools/lldb-vscode/ExceptionBreakpoint.cpp
lldb/tools/lldb-vscode/FunctionBreakpoint.cpp
lldb/tools/lldb-vscode/JSONUtils.cpp
lldb/tools/lldb-vscode/JSONUtils.h
lldb/tools/lldb-vscode/LLDBUtils.cpp
lldb/tools/lldb-vscode/LLDBUtils.h
lldb/tools/lldb-vscode/SourceBreakpoint.cpp
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
@@ -379,27 +379,20 @@
if (event_mask & lldb::SBTarget::eBroadcastBitBreakpointChanged) {
auto event_type =
lldb::SBBreakpoint::GetBreakpointEventTypeFromEvent(event);
- const auto num_locs =
- lldb::SBBreakpoint::GetNumBreakpointLocationsFromEvent(event);
auto bp = lldb::SBBreakpoint::GetBreakpointFromEvent(event);
- bool added = event_type & lldb::eBreakpointEventTypeLocationsAdded;
- bool removed =
- event_type & lldb::eBreakpointEventTypeLocationsRemoved;
- if (added || removed) {
- for (size_t i = 0; i < num_locs; ++i) {
- auto bp_loc =
- lldb::SBBreakpoint::GetBreakpointLocationAtIndexFromEvent(
- event, i);
- auto bp_event = CreateEventObject("breakpoint");
- llvm::json::Object body;
- body.try_emplace("breakpoint", CreateBreakpoint(bp_loc));
- if (added)
- body.try_emplace("reason", "new");
- else
- body.try_emplace("reason", "removed");
- bp_event.try_emplace("body", std::move(body));
- g_vsc.SendJSON(llvm::json::Value(std::move(bp_event)));
- }
+ // If the breakpoint was originated from the IDE, it will have the
+ // BreakpointBase::GetBreakpointLabel() label attached. Regardless
+ // of wether the locations were added or removed, the breakpoint
+ // ins't going away, so we the reason is always "changed".
+ if ((event_type & lldb::eBreakpointEventTypeLocationsAdded ||
+ event_type & lldb::eBreakpointEventTypeLocationsRemoved) &&
+ bp.MatchesName(BreakpointBase::GetBreakpointLabel())) {
+ auto bp_event = CreateEventObject("breakpoint");
+ llvm::json::Object body;
+ body.try_emplace("breakpoint", CreateBreakpoint(bp));
+ body.try_emplace("reason", "changed");
+ bp_event.try_emplace("body", std::move(body));
+ g_vsc.SendJSON(llvm::json::Value(std::move(bp_event)));
}
}
} else if (event.BroadcasterMatchesRef(g_vsc.broadcaster)) {
Index: lldb/tools/lldb-vscode/SourceBreakpoint.cpp
===================================================================
--- lldb/tools/lldb-vscode/SourceBreakpoint.cpp
+++ lldb/tools/lldb-vscode/SourceBreakpoint.cpp
@@ -17,6 +17,9 @@
void SourceBreakpoint::SetBreakpoint(const llvm::StringRef source_path) {
bp = g_vsc.target.BreakpointCreateByLocation(source_path.str().c_str(), line);
+ // 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())
Index: lldb/tools/lldb-vscode/LLDBUtils.h
===================================================================
--- lldb/tools/lldb-vscode/LLDBUtils.h
+++ lldb/tools/lldb-vscode/LLDBUtils.h
@@ -106,46 +106,6 @@
/// The LLDB frame index ID.
uint32_t GetLLDBFrameID(uint64_t dap_frame_id);
-/// Given a LLDB breakpoint, make a breakpoint ID that is unique to a
-/// specific breakpoint and breakpoint location.
-///
-/// VSCode requires a Breakpoint "id" to be unique, so we use the
-/// breakpoint ID in the lower BREAKPOINT_ID_SHIFT bits and the
-/// breakpoint location ID in the upper BREAKPOINT_ID_SHIFT bits.
-///
-/// \param[in] bp_loc
-/// The LLDB break point location.
-///
-/// \return
-/// A unique integer that allows us to easily find the right
-/// stack frame within a thread on subsequent VS code requests.
-int64_t MakeVSCodeBreakpointID(lldb::SBBreakpointLocation &bp_loc);
-
-/// Given a VSCode breakpoint ID, convert to a LLDB breakpoint ID.
-///
-/// VSCode requires a Breakpoint "id" to be unique, so we use the
-/// breakpoint ID in the lower BREAKPOINT_ID_SHIFT bits and the
-/// breakpoint location ID in the upper BREAKPOINT_ID_SHIFT bits.
-///
-/// \param[in] dap_breakpoint_id
-/// The VSCode breakpoint ID to convert to an LLDB breakpoint ID.
-///
-/// \return
-/// The LLDB breakpoint ID.
-uint32_t GetLLDBBreakpointID(uint64_t dap_breakpoint_id);
-
-/// Given a VSCode breakpoint ID, convert to a LLDB breakpoint location ID.
-///
-/// VSCode requires a Breakpoint "id" to be unique, so we use the
-/// breakpoint ID in the lower BREAKPOINT_ID_SHIFT bits and the
-/// breakpoint location ID in the upper BREAKPOINT_ID_SHIFT bits.
-///
-/// \param[in] dap_breakpoint_id
-/// The VSCode frame ID to convert to a breakpoint location ID.
-///
-/// \return
-/// The LLDB breakpoint location ID.
-uint32_t GetLLDBBreakpointLocationID(uint64_t dap_breakpoint_id);
} // namespace lldb_vscode
#endif
Index: lldb/tools/lldb-vscode/LLDBUtils.cpp
===================================================================
--- lldb/tools/lldb-vscode/LLDBUtils.cpp
+++ lldb/tools/lldb-vscode/LLDBUtils.cpp
@@ -79,19 +79,4 @@
frame.GetFrameID());
}
-static uint32_t constexpr BREAKPOINT_ID_SHIFT = 22;
-
-uint32_t GetLLDBBreakpointID(uint64_t dap_breakpoint_id) {
- return dap_breakpoint_id >> BREAKPOINT_ID_SHIFT;
-}
-
-uint32_t GetLLDBBreakpointLocationID(uint64_t dap_breakpoint_id) {
- return dap_breakpoint_id & ((1u << BREAKPOINT_ID_SHIFT) - 1);
-}
-
-int64_t MakeVSCodeBreakpointID(lldb::SBBreakpointLocation &bp_loc) {
- return (int64_t)(bp_loc.GetBreakpoint().GetID() << BREAKPOINT_ID_SHIFT |
- bp_loc.GetID());
-}
-
} // namespace lldb_vscode
Index: lldb/tools/lldb-vscode/JSONUtils.h
===================================================================
--- lldb/tools/lldb-vscode/JSONUtils.h
+++ lldb/tools/lldb-vscode/JSONUtils.h
@@ -199,13 +199,13 @@
/// Converts breakpoint location to a Visual Studio Code "Breakpoint"
/// JSON object and appends it to the \a breakpoints array.
///
-/// \param[in] bp_loc
-/// A LLDB breakpoint location object to convert into a JSON value
+/// \param[in] bp
+/// A LLDB breakpoint object to convert into a JSON value
///
/// \return
/// A "Breakpoint" JSON object with that follows the formal JSON
/// definition outlined by Microsoft.
-llvm::json::Value CreateBreakpoint(lldb::SBBreakpointLocation &bp_loc);
+llvm::json::Value CreateBreakpoint(lldb::SBBreakpoint &bp);
/// Create a "Event" JSON object using \a event_name as the event name
///
Index: lldb/tools/lldb-vscode/JSONUtils.cpp
===================================================================
--- lldb/tools/lldb-vscode/JSONUtils.cpp
+++ lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -281,16 +281,33 @@
// },
// "required": [ "verified" ]
// }
-llvm::json::Value CreateBreakpoint(lldb::SBBreakpointLocation &bp_loc) {
+llvm::json::Value CreateBreakpoint(lldb::SBBreakpoint &bp) {
// 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_loc.IsValid())
+ if (!bp.IsValid())
return llvm::json::Value(std::move(object));
- object.try_emplace("verified", true);
- const auto vs_id = MakeVSCodeBreakpointID(bp_loc);
- object.try_emplace("id", vs_id);
+ 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()) {
auto line_entry = bp_addr.GetLineEntry();
@@ -303,15 +320,7 @@
}
void AppendBreakpoint(lldb::SBBreakpoint &bp, llvm::json::Array &breakpoints) {
- if (!bp.IsValid())
- return;
- const auto num_locations = bp.GetNumLocations();
- if (num_locations == 0)
- return;
- for (size_t i = 0; i < num_locations; ++i) {
- auto bp_loc = bp.GetLocationAtIndex(i);
- breakpoints.emplace_back(CreateBreakpoint(bp_loc));
- }
+ breakpoints.emplace_back(CreateBreakpoint(bp));
}
// "Event": {
@@ -870,4 +879,3 @@
}
} // namespace lldb_vscode
-
Index: lldb/tools/lldb-vscode/FunctionBreakpoint.cpp
===================================================================
--- lldb/tools/lldb-vscode/FunctionBreakpoint.cpp
+++ lldb/tools/lldb-vscode/FunctionBreakpoint.cpp
@@ -18,6 +18,9 @@
if (functionName.empty())
return;
bp = g_vsc.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())
Index: lldb/tools/lldb-vscode/ExceptionBreakpoint.cpp
===================================================================
--- lldb/tools/lldb-vscode/ExceptionBreakpoint.cpp
+++ lldb/tools/lldb-vscode/ExceptionBreakpoint.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "ExceptionBreakpoint.h"
+#include "BreakpointBase.h"
#include "VSCode.h"
namespace lldb_vscode {
@@ -18,6 +19,9 @@
bool throw_value = filter.find("_throw") != std::string::npos;
bp = g_vsc.target.BreakpointCreateForException(language, catch_value,
throw_value);
+ // See comments in BreakpointBase::GetBreakpointLabel() for details of why
+ // we add a label to our breakpoints.
+ bp.AddName(BreakpointBase::GetBreakpointLabel());
}
void ExceptionBreakpoint::ClearBreakpoint() {
@@ -28,4 +32,3 @@
}
} // namespace lldb_vscode
-
Index: lldb/tools/lldb-vscode/BreakpointBase.h
===================================================================
--- lldb/tools/lldb-vscode/BreakpointBase.h
+++ lldb/tools/lldb-vscode/BreakpointBase.h
@@ -15,7 +15,7 @@
#include <string>
namespace lldb_vscode {
-
+
struct BreakpointBase {
// An optional expression for conditional breakpoints.
@@ -36,6 +36,7 @@
void SetCondition();
void SetHitCondition();
void UpdateBreakpoint(const BreakpointBase &request_bp);
+ static const char *GetBreakpointLabel();
};
} // namespace lldb_vscode
Index: lldb/tools/lldb-vscode/BreakpointBase.cpp
===================================================================
--- lldb/tools/lldb-vscode/BreakpointBase.cpp
+++ lldb/tools/lldb-vscode/BreakpointBase.cpp
@@ -18,7 +18,7 @@
void BreakpointBase::SetCondition() { bp.SetCondition(condition.c_str()); }
-void BreakpointBase::SetHitCondition() {
+void BreakpointBase::SetHitCondition() {
uint64_t hitCount = 0;
if (llvm::to_integer(hitCondition, hitCount))
bp.SetIgnoreCount(hitCount - 1);
@@ -34,3 +34,19 @@
SetHitCondition();
}
}
+
+const char *BreakpointBase::GetBreakpointLabel() {
+ // Breakpoints in LLDB can have names added to them which are kind of like
+ // labels or categories. All breakpoints that are set through the IDE UI get
+ // sent through the various VS code DAP set*Breakpoint packets, and these
+ // breakpoints will be labeled with this name so if breakpoint update events
+ // come in for breakpoints that the IDE doesn't know about, like if a
+ // breakpoint is set manually using the debugger console, we won't report any
+ // updates on them and confused the IDE. This function gets called by all of
+ // the breakpoint classes after they set breakpoints to mark a breakpoint as
+ // a UI breakpoint. We can later check a lldb::SBBreakpoint object that comes
+ // in via LLDB breakpoint changed events and check the breakpoint by calling
+ // "bool lldb::SBBreakpoint::MatchesName(const char *)" to check if a
+ // breakpoint in one of the UI breakpoints that we should report changes for.
+ return "vscode";
+}
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
