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 <he...@anthonyeid.me> Date: Wed, 27 Aug 2025 13:04:26 -0400 Subject: [PATCH 1/4] 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 <he...@anthonyeid.me> Date: Wed, 27 Aug 2025 13:12:02 -0400 Subject: [PATCH 2/4] 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 <he...@anthonyeid.me> Date: Wed, 27 Aug 2025 18:19:44 -0400 Subject: [PATCH 3/4] 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); } >From c73d4c4f82afbee0399396909a0becf98aac257e Mon Sep 17 00:00:00 2001 From: Anthony <he...@anthonyeid.me> Date: Wed, 27 Aug 2025 18:31:25 -0400 Subject: [PATCH 4/4] remove unused GetScopedsKind func --- lldb/tools/lldb-dap/JSONUtils.cpp | 85 ------------------------------- lldb/tools/lldb-dap/Variables.h | 2 +- 2 files changed, 1 insertion(+), 86 deletions(-) diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp index 7c256481f8c71..ad1f035caaf54 100644 --- a/lldb/tools/lldb-dap/JSONUtils.cpp +++ b/lldb/tools/lldb-dap/JSONUtils.cpp @@ -293,91 +293,6 @@ void FillResponse(const llvm::json::Object &request, response.try_emplace("success", true); } -// "Scope": { -// "type": "object", -// "description": "A Scope is a named container for variables. Optionally -// a scope can map to a source or a range within a source.", -// "properties": { -// "name": { -// "type": "string", -// "description": "Name of the scope such as 'Arguments', 'Locals'." -// }, -// "presentationHint": { -// "type": "string", -// "description": "An optional hint for how to present this scope in the -// UI. If this attribute is missing, the scope is shown -// with a generic UI.", -// "_enum": [ "arguments", "locals", "registers" ], -// }, -// "variablesReference": { -// "type": "integer", -// "description": "The variables of this scope can be retrieved by -// passing the value of variablesReference to the -// VariablesRequest." -// }, -// "namedVariables": { -// "type": "integer", -// "description": "The number of named variables in this scope. The -// client can use this optional information to present -// the variables in a paged UI and fetch them in chunks." -// }, -// "indexedVariables": { -// "type": "integer", -// "description": "The number of indexed variables in this scope. The -// client can use this optional information to present -// the variables in a paged UI and fetch them in chunks." -// }, -// "expensive": { -// "type": "boolean", -// "description": "If true, the number of variables in this scope is -// large or expensive to retrieve." -// }, -// "source": { -// "$ref": "#/definitions/Source", -// "description": "Optional source for this scope." -// }, -// "line": { -// "type": "integer", -// "description": "Optional start line of the range covered by this -// scope." -// }, -// "column": { -// "type": "integer", -// "description": "Optional start column of the range covered by this -// scope." -// }, -// "endLine": { -// "type": "integer", -// "description": "Optional end line of the range covered by this scope." -// }, -// "endColumn": { -// "type": "integer", -// "description": "Optional end column of the range covered by this -// scope." -// } -// }, -// "required": [ "name", "variablesReference", "expensive" ] -// } -llvm::json::Value CreateScope(const llvm::StringRef name, - 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 (kind == ScopeKind::Locals) { - object.try_emplace("presentationHint", "locals"); - } else if (kind == ScopeKind::Registers) { - object.try_emplace("presentationHint", "registers"); - } - - object.try_emplace("variablesReference", variablesReference); - object.try_emplace("expensive", expensive); - object.try_emplace("namedVariables", namedVariables); - return llvm::json::Value(std::move(object)); -} - // "Event": { // "allOf": [ { "$ref": "#/definitions/ProtocolMessage" }, { // "type": "object", diff --git a/lldb/tools/lldb-dap/Variables.h b/lldb/tools/lldb-dap/Variables.h index 2f40f87e9b4bb..a8fad707e2e08 100644 --- a/lldb/tools/lldb-dap/Variables.h +++ b/lldb/tools/lldb-dap/Variables.h @@ -66,7 +66,7 @@ struct Variables { 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; + std::map<int64_t, std::pair<ScopeKind, uint32_t>> m_scope_kinds; /// Variables that are alive in this stop state. /// Will be cleared when debuggee resumes. _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits