https://github.com/Anthony-Eid created
https://github.com/llvm/llvm-project/pull/124232
Closes #119784
This is still a work in progress, but I've technically fixed the bug where
lldb-dap would reuse variable IDs within the same stopped state. However, I
found another bug that is still prohibiting the Zed dap client from working
correctly with lldb-dap.
Whenever there's a scope request, lldb-dap resets their variable list to only
variables from that scope. So variable requests fail when
`request.variable_reference_id` represents a variable from a different scope
than `dap.variables` has been reset to.
I would be happy to fix the second bug in this issue or open a new PR that
addresses that.
>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] 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.
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
