https://github.com/Anthony-Eid updated
https://github.com/llvm/llvm-project/pull/124232
>From e741fc75ccb6e2a725b3b26dd4c503cdea6c5fbb Mon Sep 17 00:00:00 2001
From: Anthony Eid <he...@anthonyeid.me>
Date: Fri, 24 Jan 2025 00:43:39 -0500
Subject: [PATCH 1/2] Stop using replicated variable ids
---
lldb/tools/lldb-dap/DAP.cpp | 28 ++++++++++++---
lldb/tools/lldb-dap/DAP.h | 15 ++++++--
lldb/tools/lldb-dap/JSONUtils.cpp | 6 ++--
lldb/tools/lldb-dap/JSONUtils.h | 3 +-
lldb/tools/lldb-dap/ProgressEvent.h | 5 +++
lldb/tools/lldb-dap/lldb-dap.cpp | 54 ++++++++++++++++++-----------
6 files changed, 80 insertions(+), 31 deletions(-)
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index 35250d9eef608a..0836fbf9f32dd6 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -8,6 +8,7 @@
#include <chrono>
#include <cstdarg>
+#include <cstdint>
#include <fstream>
#include <mutex>
@@ -137,6 +138,15 @@ void DAP::PopulateExceptionBreakpoints() {
});
}
+std::optional<ScopeKind> DAP::ScopeKind(const int64_t variablesReference) {
+ auto iter = scope_kinds.find(variablesReference);
+ if (iter != scope_kinds.end()) {
+ return iter->second;
+ }
+
+ return std::nullopt;
+}
+
ExceptionBreakpoint *DAP::GetExceptionBreakpoint(const std::string &filter) {
// PopulateExceptionBreakpoints() is called after g_dap.debugger is created
// in a request-initialize.
@@ -476,12 +486,22 @@ lldb::SBFrame DAP::GetLLDBFrame(const llvm::json::Object
&arguments) {
llvm::json::Value DAP::CreateTopLevelScopes() {
llvm::json::Array scopes;
- scopes.emplace_back(
- CreateScope("Locals", VARREF_LOCALS, variables.locals.GetSize(), false));
- scopes.emplace_back(CreateScope("Globals", VARREF_GLOBALS,
+
+ scopes.emplace_back(CreateScope("Locals", variables.next_temporary_var_ref,
+ ScopeKind::Locals,
variables.locals.GetSize(),
+ false));
+ scope_kinds[variables.next_temporary_var_ref++] = ScopeKind::Locals;
+
+ scopes.emplace_back(CreateScope("Globals", variables.next_temporary_var_ref,
+ ScopeKind::Globals,
variables.globals.GetSize(), false));
- scopes.emplace_back(CreateScope("Registers", VARREF_REGS,
+ scope_kinds[variables.next_temporary_var_ref++] = ScopeKind::Globals;
+
+ scopes.emplace_back(CreateScope("Registers",
variables.next_temporary_var_ref,
+ ScopeKind::Registers,
variables.registers.GetSize(), false));
+ scope_kinds[variables.next_temporary_var_ref++] = ScopeKind::Registers;
+
return llvm::json::Value(std::move(scopes));
}
diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index ae496236f13369..68399e2e0410c7 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -13,6 +13,7 @@
#include <iosfwd>
#include <map>
#include <optional>
+#include <set>
#include <thread>
#include "llvm/ADT/DenseMap.h"
@@ -39,6 +40,7 @@
#include "InstructionBreakpoint.h"
#include "ProgressEvent.h"
#include "SourceBreakpoint.h"
+#include "lldb/API/SBValueList.h"
#define VARREF_LOCALS (int64_t)1
#define VARREF_GLOBALS (int64_t)2
@@ -86,6 +88,8 @@ struct Variables {
int64_t next_temporary_var_ref{VARREF_FIRST_VAR_IDX};
int64_t next_permanent_var_ref{PermanentVariableStartIndex};
+ std::set<uint32_t> added_frames;
+
/// Variables that are alive in this stop state.
/// Will be cleared when debuggee resumes.
llvm::DenseMap<int64_t, lldb::SBValue> referenced_variables;
@@ -117,25 +121,27 @@ struct Variables {
struct StartDebuggingRequestHandler : public lldb::SBCommandPluginInterface {
DAP &dap;
- explicit StartDebuggingRequestHandler(DAP &d) : dap(d) {};
+ explicit StartDebuggingRequestHandler(DAP &d) : dap(d){};
bool DoExecute(lldb::SBDebugger debugger, char **command,
lldb::SBCommandReturnObject &result) override;
};
struct ReplModeRequestHandler : public lldb::SBCommandPluginInterface {
DAP &dap;
- explicit ReplModeRequestHandler(DAP &d) : dap(d) {};
+ explicit ReplModeRequestHandler(DAP &d) : dap(d){};
bool DoExecute(lldb::SBDebugger debugger, char **command,
lldb::SBCommandReturnObject &result) override;
};
struct SendEventRequestHandler : public lldb::SBCommandPluginInterface {
DAP &dap;
- explicit SendEventRequestHandler(DAP &d) : dap(d) {};
+ explicit SendEventRequestHandler(DAP &d) : dap(d){};
bool DoExecute(lldb::SBDebugger debugger, char **command,
lldb::SBCommandReturnObject &result) override;
};
+enum ScopeKind { Locals, Globals, Registers };
+
struct DAP {
llvm::StringRef debug_adaptor_path;
InputStream input;
@@ -159,6 +165,7 @@ struct DAP {
std::vector<std::string> exit_commands;
std::vector<std::string> stop_commands;
std::vector<std::string> terminate_commands;
+ std::map<int, ScopeKind> scope_kinds;
// Map step in target id to list of function targets that user can choose.
llvm::DenseMap<lldb::addr_t, std::string> step_in_targets;
// A copy of the last LaunchRequest or AttachRequest so we can reuse its
@@ -231,6 +238,8 @@ struct DAP {
void PopulateExceptionBreakpoints();
+ std::optional<ScopeKind> ScopeKind(const int64_t variablesReference);
+
/// Attempt to determine if an expression is a variable expression or
/// lldb command using a heuristic based on the first term of the
/// expression.
diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp
b/lldb/tools/lldb-dap/JSONUtils.cpp
index 6ca4dfb4711a13..238343de055962 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -352,16 +352,16 @@ void FillResponse(const llvm::json::Object &request,
// "required": [ "name", "variablesReference", "expensive" ]
// }
llvm::json::Value CreateScope(const llvm::StringRef name,
- int64_t variablesReference,
+ int64_t variablesReference, ScopeKind kind,
int64_t namedVariables, bool expensive) {
llvm::json::Object object;
EmplaceSafeString(object, "name", name.str());
// TODO: Support "arguments" scope. At the moment lldb-dap includes the
// arguments into the "locals" scope.
- if (variablesReference == VARREF_LOCALS) {
+ if (kind == ScopeKind::Locals) {
object.try_emplace("presentationHint", "locals");
- } else if (variablesReference == VARREF_REGS) {
+ } else if (kind == ScopeKind::Registers) {
object.try_emplace("presentationHint", "registers");
}
diff --git a/lldb/tools/lldb-dap/JSONUtils.h b/lldb/tools/lldb-dap/JSONUtils.h
index db56d987773476..f117ad9fff792e 100644
--- a/lldb/tools/lldb-dap/JSONUtils.h
+++ b/lldb/tools/lldb-dap/JSONUtils.h
@@ -9,6 +9,7 @@
#ifndef LLDB_TOOLS_LLDB_DAP_JSONUTILS_H
#define LLDB_TOOLS_LLDB_DAP_JSONUTILS_H
+#include "DAP.h"
#include "DAPForward.h"
#include "lldb/API/SBCompileUnit.h"
#include "lldb/API/SBFileSpec.h"
@@ -319,7 +320,7 @@ CreateExceptionBreakpointFilter(const ExceptionBreakpoint
&bp);
/// A "Scope" JSON object with that follows the formal JSON
/// definition outlined by Microsoft.
llvm::json::Value CreateScope(const llvm::StringRef name,
- int64_t variablesReference,
+ int64_t variablesReference, ScopeKind kind,
int64_t namedVariables, bool expensive);
/// Create a "Source" JSON object as described in the debug adaptor definition.
diff --git a/lldb/tools/lldb-dap/ProgressEvent.h
b/lldb/tools/lldb-dap/ProgressEvent.h
index 72317b879c803a..e029831f8f330b 100644
--- a/lldb/tools/lldb-dap/ProgressEvent.h
+++ b/lldb/tools/lldb-dap/ProgressEvent.h
@@ -6,6 +6,9 @@
//
//===----------------------------------------------------------------------===//
+#ifndef PROGRESS_EVENT_H
+#define PROGRESS_EVENT_H
+
#include <atomic>
#include <chrono>
#include <mutex>
@@ -155,3 +158,5 @@ class ProgressEventReporter {
};
} // namespace lldb_dap
+
+#endif // PROGRESS_EVENT_H
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index 6b12569d90a831..52a31477d7092e 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -128,16 +128,19 @@ static void PrintWelcomeMessage(DAP &dap) {
}
lldb::SBValueList *GetTopLevelScope(DAP &dap, int64_t variablesReference) {
- switch (variablesReference) {
- case VARREF_LOCALS:
- return &dap.variables.locals;
- case VARREF_GLOBALS:
- return &dap.variables.globals;
- case VARREF_REGS:
- return &dap.variables.registers;
- default:
- return nullptr;
+ std::optional<ScopeKind> scope_kind = dap.ScopeKind(variablesReference);
+
+ if (scope_kind.has_value()) {
+ switch (scope_kind.value()) {
+ case lldb_dap::ScopeKind::Locals:
+ return &dap.variables.locals;
+ case lldb_dap::ScopeKind::Globals:
+ return &dap.variables.globals;
+ case lldb_dap::ScopeKind::Registers:
+ return &dap.variables.registers;
+ }
}
+ return nullptr;
}
SOCKET AcceptConnection(DAP &dap, int portno) {
@@ -2559,15 +2562,25 @@ void request_scopes(DAP &dap, const llvm::json::Object
&request) {
frame.GetThread().SetSelectedFrame(frame.GetFrameID());
}
- dap.variables.locals = frame.GetVariables(/*arguments=*/true,
- /*locals=*/true,
- /*statics=*/false,
- /*in_scope_only=*/true);
- dap.variables.globals = frame.GetVariables(/*arguments=*/false,
- /*locals=*/false,
- /*statics=*/true,
- /*in_scope_only=*/true);
- dap.variables.registers = frame.GetRegisters();
+ uint32_t frame_id = frame.GetFrameID();
+
+ if (dap.variables.added_frames.find(frame_id) ==
+ dap.variables.added_frames.end()) {
+ dap.variables.added_frames.insert(frame_id);
+
+ dap.variables.locals.Append(frame.GetVariables(/*arguments=*/true,
+ /*locals=*/true,
+ /*statics=*/false,
+ /*in_scope_only=*/true));
+
+ dap.variables.globals.Append(frame.GetVariables(/*arguments=*/false,
+ /*locals=*/false,
+ /*statics=*/true,
+ /*in_scope_only=*/true));
+
+ dap.variables.registers.Append(frame.GetRegisters());
+ }
+
body.try_emplace("scopes", dap.CreateTopLevelScopes());
response.try_emplace("body", std::move(body));
dap.SendJSON(llvm::json::Value(std::move(response)));
@@ -3945,7 +3958,7 @@ void request_variables(DAP &dap, const llvm::json::Object
&request) {
int64_t start_idx = 0;
int64_t num_children = 0;
- if (variablesReference == VARREF_REGS) {
+ if (dap.ScopeKind(variablesReference) == ScopeKind::Registers) {
// Change the default format of any pointer sized registers in the first
// register set to be the lldb::eFormatAddressInfo so we show the pointer
// and resolve what the pointer resolves to. Only change the format if
the
@@ -3965,7 +3978,8 @@ void request_variables(DAP &dap, const llvm::json::Object
&request) {
}
num_children = top_scope->GetSize();
- if (num_children == 0 && variablesReference == VARREF_LOCALS) {
+ if (num_children == 0 &&
+ dap.ScopeKind(variablesReference) == ScopeKind::Locals) {
// Check for an error in the SBValueList that might explain why we don't
// have locals. If we have an error display it as the sole value in the
// the locals.
>From 0b7aaed3da01adb6c1e1fa0d7836ca36bc1842da Mon Sep 17 00:00:00 2001
From: Anthony Eid <he...@anthonyeid.me>
Date: Fri, 24 Jan 2025 03:09:58 -0500
Subject: [PATCH 2/2] Fix bug where some scope requests would be invalid
---
lldb/tools/lldb-dap/DAP.cpp | 30 +++++++++++++---
lldb/tools/lldb-dap/DAP.h | 12 +++++--
lldb/tools/lldb-dap/lldb-dap.cpp | 62 +++++++++++++++++++-------------
3 files changed, 72 insertions(+), 32 deletions(-)
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index 0836fbf9f32dd6..0f36c0a578b3bf 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -141,7 +141,7 @@ void DAP::PopulateExceptionBreakpoints() {
std::optional<ScopeKind> DAP::ScopeKind(const int64_t variablesReference) {
auto iter = scope_kinds.find(variablesReference);
if (iter != scope_kinds.end()) {
- return iter->second;
+ return iter->second.first;
}
return std::nullopt;
@@ -484,23 +484,26 @@ lldb::SBFrame DAP::GetLLDBFrame(const llvm::json::Object
&arguments) {
return thread.GetFrameAtIndex(GetLLDBFrameID(frame_id));
}
-llvm::json::Value DAP::CreateTopLevelScopes() {
+llvm::json::Value DAP::CreateTopLevelScopes(uint32_t frame_id) {
llvm::json::Array scopes;
scopes.emplace_back(CreateScope("Locals", variables.next_temporary_var_ref,
ScopeKind::Locals,
variables.locals.GetSize(),
false));
- scope_kinds[variables.next_temporary_var_ref++] = ScopeKind::Locals;
+ scope_kinds[variables.next_temporary_var_ref++] =
+ std::make_pair(ScopeKind::Locals, frame_id);
scopes.emplace_back(CreateScope("Globals", variables.next_temporary_var_ref,
ScopeKind::Globals,
variables.globals.GetSize(), false));
- scope_kinds[variables.next_temporary_var_ref++] = ScopeKind::Globals;
+ scope_kinds[variables.next_temporary_var_ref++] =
+ std::make_pair(ScopeKind::Globals, frame_id);
scopes.emplace_back(CreateScope("Registers",
variables.next_temporary_var_ref,
ScopeKind::Registers,
variables.registers.GetSize(), false));
- scope_kinds[variables.next_temporary_var_ref++] = ScopeKind::Registers;
+ scope_kinds[variables.next_temporary_var_ref++] =
+ std::make_pair(ScopeKind::Registers, frame_id);
return llvm::json::Value(std::move(scopes));
}
@@ -846,11 +849,28 @@ lldb::SBError DAP::WaitForProcessToStop(uint32_t seconds)
{
return error;
}
+bool Variables::SwitchFrame(uint32_t frame_id) {
+ auto iter = frames.find(frame_id);
+
+ if (iter == frames.end()) {
+ return false;
+ }
+
+ auto [frame_locals, frame_globals, frame_registers] = iter->second;
+
+ locals = frame_locals;
+ globals = frame_globals;
+ registers = frame_registers;
+
+ return true;
+}
+
void Variables::Clear() {
locals.Clear();
globals.Clear();
registers.Clear();
referenced_variables.clear();
+ frames.clear();
}
int64_t Variables::GetNewVariableReference(bool is_permanent) {
diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index 68399e2e0410c7..a77e3c0389f5a1 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -15,6 +15,8 @@
#include <optional>
#include <set>
#include <thread>
+#include <tuple>
+#include <utility>
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/DenseSet.h"
@@ -88,7 +90,9 @@ struct Variables {
int64_t next_temporary_var_ref{VARREF_FIRST_VAR_IDX};
int64_t next_permanent_var_ref{PermanentVariableStartIndex};
- std::set<uint32_t> added_frames;
+ std::map<uint32_t,
+ std::tuple<lldb::SBValueList, lldb::SBValueList, lldb::SBValueList>>
+ frames;
/// Variables that are alive in this stop state.
/// Will be cleared when debuggee resumes.
@@ -115,6 +119,8 @@ struct Variables {
/// \return variableReference assigned to this expandable variable.
int64_t InsertVariable(lldb::SBValue variable, bool is_permanent);
+ bool SwitchFrame(uint32_t frame_id);
+
/// Clear all scope variables and non-permanent expandable variables.
void Clear();
};
@@ -165,7 +171,7 @@ struct DAP {
std::vector<std::string> exit_commands;
std::vector<std::string> stop_commands;
std::vector<std::string> terminate_commands;
- std::map<int, ScopeKind> scope_kinds;
+ std::map<int, std::pair<ScopeKind, uint32_t>> scope_kinds;
// Map step in target id to list of function targets that user can choose.
llvm::DenseMap<lldb::addr_t, std::string> step_in_targets;
// A copy of the last LaunchRequest or AttachRequest so we can reuse its
@@ -234,7 +240,7 @@ struct DAP {
lldb::SBFrame GetLLDBFrame(const llvm::json::Object &arguments);
- llvm::json::Value CreateTopLevelScopes();
+ llvm::json::Value CreateTopLevelScopes(uint32_t frame_id);
void PopulateExceptionBreakpoints();
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index 52a31477d7092e..f61b667a70220e 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -51,6 +51,7 @@
#include <sys/stat.h>
#include <sys/types.h>
#include <thread>
+#include <tuple>
#include <vector>
#if defined(_WIN32)
@@ -128,17 +129,25 @@ static void PrintWelcomeMessage(DAP &dap) {
}
lldb::SBValueList *GetTopLevelScope(DAP &dap, int64_t variablesReference) {
- std::optional<ScopeKind> scope_kind = dap.ScopeKind(variablesReference);
-
- if (scope_kind.has_value()) {
- switch (scope_kind.value()) {
- case lldb_dap::ScopeKind::Locals:
- return &dap.variables.locals;
- case lldb_dap::ScopeKind::Globals:
- return &dap.variables.globals;
- case lldb_dap::ScopeKind::Registers:
- return &dap.variables.registers;
- }
+ auto iter = dap.scope_kinds.find(variablesReference);
+ if (iter == dap.scope_kinds.end()) {
+ return nullptr;
+ }
+
+ ScopeKind scope_kind = iter->second.first;
+ uint32_t frame_id = iter->second.second;
+
+ if (!dap.variables.SwitchFrame(frame_id)) {
+ return nullptr;
+ }
+
+ switch (scope_kind) {
+ case lldb_dap::ScopeKind::Locals:
+ return &dap.variables.locals;
+ case lldb_dap::ScopeKind::Globals:
+ return &dap.variables.globals;
+ case lldb_dap::ScopeKind::Registers:
+ return &dap.variables.registers;
}
return nullptr;
}
@@ -2564,24 +2573,29 @@ void request_scopes(DAP &dap, const llvm::json::Object
&request) {
uint32_t frame_id = frame.GetFrameID();
- if (dap.variables.added_frames.find(frame_id) ==
- dap.variables.added_frames.end()) {
- dap.variables.added_frames.insert(frame_id);
+ if (dap.variables.frames.find(frame_id) == dap.variables.frames.end()) {
+
+ auto locals = frame.GetVariables(/*arguments=*/true,
+ /*locals=*/true,
+ /*statics=*/false,
+ /*in_scope_only=*/true);
+
+ auto globals = frame.GetVariables(/*arguments=*/false,
+ /*locals=*/false,
+ /*statics=*/true,
+ /*in_scope_only=*/true);
- dap.variables.locals.Append(frame.GetVariables(/*arguments=*/true,
- /*locals=*/true,
- /*statics=*/false,
- /*in_scope_only=*/true));
+ auto registers = frame.GetRegisters();
- dap.variables.globals.Append(frame.GetVariables(/*arguments=*/false,
- /*locals=*/false,
- /*statics=*/true,
- /*in_scope_only=*/true));
+ dap.variables.frames.insert(
+ std::make_pair(frame_id, std::make_tuple(locals, globals, registers)));
- dap.variables.registers.Append(frame.GetRegisters());
+ dap.variables.locals = locals;
+ dap.variables.globals = globals;
+ dap.variables.registers = registers;
}
- body.try_emplace("scopes", dap.CreateTopLevelScopes());
+ body.try_emplace("scopes", dap.CreateTopLevelScopes(frame_id));
response.try_emplace("body", std::move(body));
dap.SendJSON(llvm::json::Value(std::move(response)));
}
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
