[Lldb-commits] [lldb] WIP: Stop using replicated variable ids (PR #124232)
Anthony Eid via lldb-commits
lldb-commits at lists.llvm.org
Wed Aug 27 15:20:23 PDT 2025
https://github.com/Anthony-Eid updated https://github.com/llvm/llvm-project/pull/124232
>From 30658e994b18b7c0db114a297036421c8de2dea3 Mon Sep 17 00:00:00 2001
From: Anthony <hello at anthonyeid.me>
Date: Wed, 27 Aug 2025 13:04:26 -0400
Subject: [PATCH 1/3] Fix variable request from reusing variable_ids
---
.../lldb-dap/Handler/ScopesRequestHandler.cpp | 45 ++++++-----
lldb/tools/lldb-dap/JSONUtils.h | 1 +
lldb/tools/lldb-dap/Variables.cpp | 80 +++++++++++++++++--
lldb/tools/lldb-dap/Variables.h | 18 ++++-
4 files changed, 119 insertions(+), 25 deletions(-)
diff --git a/lldb/tools/lldb-dap/Handler/ScopesRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/ScopesRequestHandler.cpp
index aaad0e20f9c21..160d8e264d089 100644
--- a/lldb/tools/lldb-dap/Handler/ScopesRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/ScopesRequestHandler.cpp
@@ -29,8 +29,8 @@ namespace lldb_dap {
///
/// \return
/// A `protocol::Scope`
-static Scope CreateScope(const llvm::StringRef name, int64_t variablesReference,
- int64_t namedVariables, bool expensive) {
+Scope CreateScope(const llvm::StringRef name, int64_t variablesReference,
+ int64_t namedVariables, bool expensive) {
Scope scope;
scope.name = name;
@@ -75,22 +75,31 @@ ScopesRequestHandler::Run(const ScopesArguments &args) const {
frame.GetThread().GetProcess().SetSelectedThread(frame.GetThread());
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();
-
- std::vector scopes = {CreateScope("Locals", VARREF_LOCALS,
- dap.variables.locals.GetSize(), false),
- CreateScope("Globals", VARREF_GLOBALS,
- dap.variables.globals.GetSize(), false),
- CreateScope("Registers", VARREF_REGS,
- dap.variables.registers.GetSize(), false)};
+
+ uint32_t frame_id = frame.GetFrameID();
+
+ dap.variables.ReadyFrame(frame_id, frame);
+ dap.variables.SwitchFrame(frame_id);
+
+ std::vector<Scope> scopes = {};
+
+ int64_t variable_reference = dap.variables.GetNewVariableReference(false);
+ scopes.push_back(CreateScope("Locals", variable_reference,
+ dap.variables.locals.GetSize(), false));
+
+ dap.variables.AddScopeKind(variable_reference, ScopeKind::Locals, frame_id);
+
+ variable_reference = dap.variables.GetNewVariableReference(false);
+ scopes.push_back(CreateScope("Globals", variable_reference,
+ dap.variables.globals.GetSize(), false));
+ dap.variables.AddScopeKind(variable_reference, ScopeKind::Globals, frame_id);
+
+ variable_reference = dap.variables.GetNewVariableReference(false);
+ scopes.push_back(CreateScope("Registers", variable_reference,
+ dap.variables.registers.GetSize(), false));
+
+ dap.variables.AddScopeKind(variable_reference, ScopeKind::Registers,
+ frame_id);
return ScopesResponseBody{std::move(scopes)};
}
diff --git a/lldb/tools/lldb-dap/JSONUtils.h b/lldb/tools/lldb-dap/JSONUtils.h
index e9094f67b94ec..6575411acd878 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 "Protocol/ProtocolTypes.h"
#include "lldb/API/SBCompileUnit.h"
diff --git a/lldb/tools/lldb-dap/Variables.cpp b/lldb/tools/lldb-dap/Variables.cpp
index 777e3183d8c0d..9ed3773df817d 100644
--- a/lldb/tools/lldb-dap/Variables.cpp
+++ b/lldb/tools/lldb-dap/Variables.cpp
@@ -8,20 +8,33 @@
#include "Variables.h"
#include "JSONUtils.h"
+#include "lldb/API/SBFrame.h"
using namespace lldb_dap;
lldb::SBValueList *Variables::GetTopLevelScope(int64_t variablesReference) {
- switch (variablesReference) {
- case VARREF_LOCALS:
+ auto iter = m_scope_kinds.find(variablesReference);
+ if (iter == m_scope_kinds.end()) {
+ return nullptr;
+ }
+
+ ScopeKind scope_kind = iter->second.first;
+ uint32_t frame_id = iter->second.second;
+
+ if (!SwitchFrame(frame_id)) {
+ return nullptr;
+ }
+
+ switch (scope_kind) {
+ case lldb_dap::ScopeKind::Locals:
return &locals;
- case VARREF_GLOBALS:
+ case lldb_dap::ScopeKind::Globals:
return &globals;
- case VARREF_REGS:
+ case lldb_dap::ScopeKind::Registers:
return ®isters;
- default:
- return nullptr;
}
+
+ return nullptr;
}
void Variables::Clear() {
@@ -29,6 +42,8 @@ void Variables::Clear() {
globals.Clear();
registers.Clear();
m_referencedvariables.clear();
+ m_frames.clear();
+ m_next_temporary_var_ref = VARREF_FIRST_VAR_IDX;
}
int64_t Variables::GetNewVariableReference(bool is_permanent) {
@@ -103,3 +118,56 @@ lldb::SBValue Variables::FindVariable(uint64_t variablesReference,
}
return variable;
}
+
+std::optional<ScopeKind>
+Variables::GetScopeKind(const int64_t variablesReference) {
+ auto iter = m_scope_kinds.find(variablesReference);
+ if (iter != m_scope_kinds.end()) {
+ return iter->second.first;
+ }
+
+ return std::nullopt;
+}
+
+bool Variables::SwitchFrame(const uint32_t frame_id) {
+ auto iter = m_frames.find(frame_id);
+
+ if (iter == m_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::ReadyFrame(uint32_t frame_id, lldb::SBFrame &frame) {
+ if (m_frames.find(frame_id) == m_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);
+
+ auto registers = frame.GetRegisters();
+
+ m_frames.insert(
+ std::make_pair(frame_id, std::make_tuple(locals, globals, registers)));
+ }
+}
+
+void Variables::AddScopeKind(int64_t variable_reference, ScopeKind kind,
+ uint32_t frame_id) {
+
+ m_scope_kinds[variable_reference] =
+ std::make_pair(ScopeKind::Globals, frame_id);
+}
diff --git a/lldb/tools/lldb-dap/Variables.h b/lldb/tools/lldb-dap/Variables.h
index 0ed84b36aef99..2f40f87e9b4bb 100644
--- a/lldb/tools/lldb-dap/Variables.h
+++ b/lldb/tools/lldb-dap/Variables.h
@@ -12,6 +12,7 @@
#include "lldb/API/SBValue.h"
#include "lldb/API/SBValueList.h"
#include "llvm/ADT/DenseMap.h"
+#include <map>
#define VARREF_FIRST_VAR_IDX (int64_t)4
#define VARREF_LOCALS (int64_t)1
@@ -20,6 +21,8 @@
namespace lldb_dap {
+enum ScopeKind { Locals, Globals, Registers };
+
struct Variables {
lldb::SBValueList locals;
lldb::SBValueList globals;
@@ -47,12 +50,23 @@ struct Variables {
lldb::SBValue FindVariable(uint64_t variablesReference, llvm::StringRef name);
+ bool SwitchFrame(uint32_t frame_id);
+ /// Initialize a frame if it hasn't been already, otherwise do nothing
+ void ReadyFrame(uint32_t frame_id, lldb::SBFrame &frame);
+ std::optional<ScopeKind> GetScopeKind(const int64_t variablesReference);
+
/// Clear all scope variables and non-permanent expandable variables.
void Clear();
+ void AddScopeKind(int64_t variable_reference, ScopeKind kind,
+ uint32_t frame_id);
+
private:
/// Variable_reference start index of permanent expandable variable.
static constexpr int64_t PermanentVariableStartIndex = (1ll << 32);
+ int64_t m_next_temporary_var_ref{VARREF_FIRST_VAR_IDX};
+
+ std::map<int, std::pair<ScopeKind, uint32_t>> m_scope_kinds;
/// Variables that are alive in this stop state.
/// Will be cleared when debuggee resumes.
@@ -62,7 +76,9 @@ struct Variables {
/// These are the variables evaluated from debug console REPL.
llvm::DenseMap<int64_t, lldb::SBValue> m_referencedpermanent_variables;
- int64_t m_next_temporary_var_ref{VARREF_FIRST_VAR_IDX};
+ std::map<uint32_t,
+ std::tuple<lldb::SBValueList, lldb::SBValueList, lldb::SBValueList>>
+ m_frames;
int64_t m_next_permanent_var_ref{PermanentVariableStartIndex};
};
>From 9707978154cc43ee6f7e3d54d0159a0c40d5bbf0 Mon Sep 17 00:00:00 2001
From: Anthony <hello at anthonyeid.me>
Date: Wed, 27 Aug 2025 13:12:02 -0400
Subject: [PATCH 2/3] Add createScope changes
---
lldb/tools/lldb-dap/JSONUtils.cpp | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp
index 4f26599a49bac..7c256481f8c71 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -11,6 +11,7 @@
#include "ExceptionBreakpoint.h"
#include "LLDBUtils.h"
#include "ProtocolUtils.h"
+#include "Variables.h"
#include "lldb/API/SBAddress.h"
#include "lldb/API/SBCompileUnit.h"
#include "lldb/API/SBDeclaration.h"
@@ -358,16 +359,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");
}
>From 50230c201673a58236bf5bccfe3036ca5d14cc95 Mon Sep 17 00:00:00 2001
From: Anthony <hello at anthonyeid.me>
Date: Wed, 27 Aug 2025 18:19:44 -0400
Subject: [PATCH 3/3] Fix some bugs
---
lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp | 1 +
lldb/tools/lldb-dap/Handler/ScopesRequestHandler.cpp | 4 ++--
lldb/tools/lldb-dap/Handler/VariablesRequestHandler.cpp | 7 ++++---
lldb/tools/lldb-dap/Variables.cpp | 4 ++--
4 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp
index e1556846dff19..e207f830183b6 100644
--- a/lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp
@@ -162,6 +162,7 @@ void EvaluateRequestHandler::operator()(
// focus_tid to the current frame for any thread related events.
if (frame.IsValid()) {
dap.focus_tid = frame.GetThread().GetThreadID();
+ dap.variables.SwitchFrame(frame.GetFrameID());
}
bool required_command_failed = false;
diff --git a/lldb/tools/lldb-dap/Handler/ScopesRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/ScopesRequestHandler.cpp
index 160d8e264d089..553a7b30adba2 100644
--- a/lldb/tools/lldb-dap/Handler/ScopesRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/ScopesRequestHandler.cpp
@@ -41,9 +41,9 @@ Scope CreateScope(const llvm::StringRef name, int64_t variablesReference,
// if we add the arguments above the local scope as the locals scope will not
// be expanded if we enter a function with arguments. It becomes more
// annoying when the scope has arguments, return_value and locals.
- if (variablesReference == VARREF_LOCALS)
+ if (name == "Locals")
scope.presentationHint = Scope::eScopePresentationHintLocals;
- else if (variablesReference == VARREF_REGS)
+ else if (name == "Registers")
scope.presentationHint = Scope::eScopePresentationHintRegisters;
scope.variablesReference = variablesReference;
diff --git a/lldb/tools/lldb-dap/Handler/VariablesRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/VariablesRequestHandler.cpp
index 5fa2b1ef5e20d..3d99983ac0c83 100644
--- a/lldb/tools/lldb-dap/Handler/VariablesRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/VariablesRequestHandler.cpp
@@ -38,7 +38,8 @@ VariablesRequestHandler::Run(const VariablesArguments &arguments) const {
int64_t start_idx = 0;
int64_t num_children = 0;
- if (var_ref == VARREF_REGS) {
+ std::optional<ScopeKind> scope_kind = dap.variables.GetScopeKind(var_ref);
+ if (scope_kind && *scope_kind == 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
@@ -58,7 +59,7 @@ VariablesRequestHandler::Run(const VariablesArguments &arguments) const {
}
num_children = top_scope->GetSize();
- if (num_children == 0 && var_ref == VARREF_LOCALS) {
+ if (num_children == 0 && scope_kind && *scope_kind == 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.
@@ -94,7 +95,7 @@ VariablesRequestHandler::Run(const VariablesArguments &arguments) const {
}
// Show return value if there is any ( in the local top frame )
- if (var_ref == VARREF_LOCALS) {
+ if (scope_kind && *scope_kind == ScopeKind::Locals) {
auto process = dap.target.GetProcess();
auto selected_thread = process.GetSelectedThread();
lldb::SBValue stop_return_value = selected_thread.GetStopReturnValue();
diff --git a/lldb/tools/lldb-dap/Variables.cpp b/lldb/tools/lldb-dap/Variables.cpp
index 9ed3773df817d..b5cd9d69d385f 100644
--- a/lldb/tools/lldb-dap/Variables.cpp
+++ b/lldb/tools/lldb-dap/Variables.cpp
@@ -42,6 +42,7 @@ void Variables::Clear() {
globals.Clear();
registers.Clear();
m_referencedvariables.clear();
+ m_scope_kinds.clear();
m_frames.clear();
m_next_temporary_var_ref = VARREF_FIRST_VAR_IDX;
}
@@ -168,6 +169,5 @@ void Variables::ReadyFrame(uint32_t frame_id, lldb::SBFrame &frame) {
void Variables::AddScopeKind(int64_t variable_reference, ScopeKind kind,
uint32_t frame_id) {
- m_scope_kinds[variable_reference] =
- std::make_pair(ScopeKind::Globals, frame_id);
+ m_scope_kinds[variable_reference] = std::make_pair(kind, frame_id);
}
More information about the lldb-commits
mailing list