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 01/17] 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 02/17] 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 03/17] 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 04/17] 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. >From 2712abdb5a21e466a4ee8a289c52c8bc9e537c63 Mon Sep 17 00:00:00 2001 From: Anthony <he...@anthonyeid.me> Date: Thu, 28 Aug 2025 00:25:00 -0400 Subject: [PATCH 05/17] Fix hardcoded scope references in DAP variable handling The test and API changes now get scope references dynamically instead of using hardcoded values. This fixes the below tests: - testDAP_memory - TestDAP_variables - GetTopLevelScope_ReturnsCorrectScope --- .../test/tools/lldb-dap/lldbdap_testcase.py | 33 ++++++++++++++++-- .../tools/lldb-dap/memory/TestDAP_memory.py | 4 ++- .../lldb-dap/variables/TestDAP_variables.py | 17 +++++----- lldb/unittests/DAP/VariablesTest.cpp | 34 ++++++++++++++++--- 4 files changed, 73 insertions(+), 15 deletions(-) diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py index b28a78792c70f..a26be6e5d1bfb 100644 --- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py +++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py @@ -351,11 +351,40 @@ def get_local_as_int(self, name, threadId=None): def set_local(self, name, value, id=None): """Set a top level local variable only.""" - return self.dap_server.request_setVariable(1, name, str(value), id=id) + # Get the locals scope reference dynamically + locals_ref = self.get_locals_scope_reference() + if locals_ref is None: + return None + return self.dap_server.request_setVariable(locals_ref, name, str(value), id=id) def set_global(self, name, value, id=None): """Set a top level global variable only.""" - return self.dap_server.request_setVariable(2, name, str(value), id=id) + # Get the globals scope reference dynamically + stackFrame = self.dap_server.get_stackFrame() + if stackFrame is None: + return None + frameId = stackFrame["id"] + scopes_response = self.dap_server.request_scopes(frameId) + frame_scopes = scopes_response["body"]["scopes"] + for scope in frame_scopes: + if scope["name"] == "Globals": + varRef = scope["variablesReference"] + return self.dap_server.request_setVariable(varRef, name, str(value), id=id) + return None + + + def get_locals_scope_reference(self): + """Get the variablesReference for the locals scope.""" + stackFrame = self.dap_server.get_stackFrame() + if stackFrame is None: + return None + frameId = stackFrame["id"] + scopes_response = self.dap_server.request_scopes(frameId) + frame_scopes = scopes_response["body"]["scopes"] + for scope in frame_scopes: + if scope["name"] == "Locals": + return scope["variablesReference"] + return None def stepIn( self, diff --git a/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py b/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py index f51056d7020c6..5e1dc8d30ff8d 100644 --- a/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py +++ b/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py @@ -70,9 +70,11 @@ def test_memory_refs_set_variable(self): self.continue_to_next_stop() ptr_value = self.get_local_as_int("rawptr") + locals_ref = self.get_locals_scope_reference() + self.assertIsNotNone(locals_ref, "Failed to get locals scope reference") self.assertIn( "memoryReference", - self.dap_server.request_setVariable(1, "rawptr", ptr_value + 2)[ + self.dap_server.request_setVariable(locals_ref, "rawptr", ptr_value + 2)[ "body" ].keys(), ) diff --git a/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py b/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py index a3a4bdaaf40a6..a51d1f0ee179c 100644 --- a/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py +++ b/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py @@ -341,24 +341,25 @@ def do_test_scopes_variables_setVariable_evaluate( self.verify_variables(verify_locals, self.dap_server.get_local_variables()) # Now we verify that we correctly change the name of a variable with and without differentiator suffix - self.assertFalse(self.dap_server.request_setVariable(1, "x2", 9)["success"]) + local_scope_ref = self.get_locals_scope_reference() + self.assertFalse(self.dap_server.request_setVariable(local_scope_ref, "x2", 9)["success"]) self.assertFalse( - self.dap_server.request_setVariable(1, "x @ main.cpp:0", 9)["success"] + self.dap_server.request_setVariable(local_scope_ref, "x @ main.cpp:0", 9)["success"] ) self.assertTrue( - self.dap_server.request_setVariable(1, "x @ main.cpp:19", 19)["success"] + self.dap_server.request_setVariable(local_scope_ref, "x @ main.cpp:19", 19)["success"] ) self.assertTrue( - self.dap_server.request_setVariable(1, "x @ main.cpp:21", 21)["success"] + self.dap_server.request_setVariable(local_scope_ref, "x @ main.cpp:21", 21)["success"] ) self.assertTrue( - self.dap_server.request_setVariable(1, "x @ main.cpp:23", 23)["success"] + self.dap_server.request_setVariable(local_scope_ref, "x @ main.cpp:23", 23)["success"] ) # The following should have no effect self.assertFalse( - self.dap_server.request_setVariable(1, "x @ main.cpp:23", "invalid")[ + self.dap_server.request_setVariable(local_scope_ref, "x @ main.cpp:23", "invalid")[ "success" ] ) @@ -370,7 +371,7 @@ def do_test_scopes_variables_setVariable_evaluate( self.verify_variables(verify_locals, self.dap_server.get_local_variables()) # The plain x variable shold refer to the innermost x - self.assertTrue(self.dap_server.request_setVariable(1, "x", 22)["success"]) + self.assertTrue(self.dap_server.request_setVariable(local_scope_ref, "x", 22)["success"]) verify_locals["x @ main.cpp:23"]["equals"]["value"] = "22" self.verify_variables(verify_locals, self.dap_server.get_local_variables()) @@ -709,7 +710,7 @@ def test_return_variables(self): break self.assertFalse( - self.dap_server.request_setVariable(1, "(Return Value)", 20)["success"] + self.dap_server.request_setVariable(self.get_locals_scope_reference(), "(Return Value)", 20)["success"] ) @skipIfWindows diff --git a/lldb/unittests/DAP/VariablesTest.cpp b/lldb/unittests/DAP/VariablesTest.cpp index 6b14fc6c3945d..40884e89e0c54 100644 --- a/lldb/unittests/DAP/VariablesTest.cpp +++ b/lldb/unittests/DAP/VariablesTest.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "Variables.h" +#include "lldb/API/SBFrame.h" #include "lldb/API/SBValue.h" #include "lldb/API/SBValueList.h" #include "gtest/gtest.h" @@ -66,20 +67,45 @@ TEST_F(VariablesTest, Clear_RemovesTemporaryKeepsPermanent) { } TEST_F(VariablesTest, GetTopLevelScope_ReturnsCorrectScope) { + lldb::SBFrame frame; + uint32_t frame_id = 0; + + vars.ReadyFrame(frame_id, frame); + vars.SwitchFrame(frame_id); + vars.locals.Append(lldb::SBValue()); vars.globals.Append(lldb::SBValue()); vars.registers.Append(lldb::SBValue()); - EXPECT_EQ(vars.GetTopLevelScope(VARREF_LOCALS), &vars.locals); - EXPECT_EQ(vars.GetTopLevelScope(VARREF_GLOBALS), &vars.globals); - EXPECT_EQ(vars.GetTopLevelScope(VARREF_REGS), &vars.registers); + int64_t locals_ref = vars.GetNewVariableReference(false); + vars.AddScopeKind(locals_ref, ScopeKind::Locals, frame_id); + + int64_t globals_ref = vars.GetNewVariableReference(false); + vars.AddScopeKind(globals_ref, ScopeKind::Globals, frame_id); + + int64_t registers_ref = vars.GetNewVariableReference(false); + vars.AddScopeKind(registers_ref, ScopeKind::Registers, frame_id); + + EXPECT_EQ(vars.GetTopLevelScope(locals_ref), &vars.locals); + EXPECT_EQ(vars.GetTopLevelScope(globals_ref), &vars.globals); + EXPECT_EQ(vars.GetTopLevelScope(registers_ref), &vars.registers); EXPECT_EQ(vars.GetTopLevelScope(9999), nullptr); } TEST_F(VariablesTest, FindVariable_LocalsByName) { + lldb::SBFrame frame; + uint32_t frame_id = 0; + + vars.ReadyFrame(frame_id, frame); + vars.SwitchFrame(frame_id); + lldb::SBValue dummy; vars.locals.Append(dummy); - lldb::SBValue found = vars.FindVariable(VARREF_LOCALS, ""); + + int64_t locals_ref = vars.GetNewVariableReference(false); + vars.AddScopeKind(locals_ref, ScopeKind::Locals, frame_id); + + lldb::SBValue found = vars.FindVariable(locals_ref, ""); EXPECT_EQ(found.IsValid(), dummy.IsValid()); } >From d0f1e86dd1cd081427329c5b53b262274653556c Mon Sep 17 00:00:00 2001 From: Anthony <he...@anthonyeid.me> Date: Thu, 28 Aug 2025 00:54:37 -0400 Subject: [PATCH 06/17] Fix set data breakpoint test by using the correct variable reference Variable reference 1 was hard coded to always be the local scope but since the var_ref of local scope is dynamic now it uses a helper function in the test --- .../lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py b/lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py index a542a318050dd..211d71d43d155 100644 --- a/lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py +++ b/lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py @@ -106,8 +106,10 @@ def test_functionality(self): self.set_source_breakpoints(source, [first_loop_break_line]) self.continue_to_next_stop() self.dap_server.get_local_variables() + locals_ref = self.get_locals_scope_reference() + self.assertIsNotNone(locals_ref, "Failed to get locals scope reference") # Test write watchpoints on x, arr[2] - response_x = self.dap_server.request_dataBreakpointInfo(1, "x") + response_x = self.dap_server.request_dataBreakpointInfo(locals_ref, "x") arr = self.dap_server.get_local_variable("arr") response_arr_2 = self.dap_server.request_dataBreakpointInfo( arr["variablesReference"], "[2]" >From 875b926e8b04b072d56dbf2d75ce9f5732341382 Mon Sep 17 00:00:00 2001 From: Anthony <he...@anthonyeid.me> Date: Thu, 28 Aug 2025 02:16:27 -0400 Subject: [PATCH 07/17] Have llm (Claude Opus 4) Generate a regression test --- .../lldb-dap/variables/TestDAP_variables.py | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py b/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py index a51d1f0ee179c..342dc4de7b895 100644 --- a/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py +++ b/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py @@ -832,3 +832,44 @@ def test_value_format(self): self.assertEqual(var_pt_x["value"], "11") var_pt_y = self.dap_server.get_local_variable_child("pt", "y", is_hex=is_hex) self.assertEqual(var_pt_y["value"], "22") + + @skipIfWindows + def test_variable_id_uniqueness_simple(self): + """ + Simple regression test for variable ID uniqueness across frames. + Ensures variable IDs are not reused between different scopes/frames. + """ + program = self.getBuildArtifact("a.out") + self.build_and_launch(program) + source = "main.cpp" + + # Set breakpoint at test_indexedVariables call to get multiple frames + bp_line = line_number(source, "// breakpoint 3") + self.set_source_breakpoints(source, [bp_line]) + self.continue_to_next_stop() + + # Get stack frames + frames = self.get_stackFrames() + self.assertGreaterEqual(len(frames), 2, "Need at least 2 frames") + + # Track all variable references for uniqueness check + all_refs = set() + + # Check first 2-3 frames + for i in range(min(3, len(frames))): + frame_id = frames[i]['id'] + scopes = self.dap_server.request_scopes(frame_id)["body"]["scopes"] + + for scope in scopes: + ref = scope['variablesReference'] + if ref != 0: # 0 means no variables + # Ensure this reference hasn't been used before + self.assertNotIn(ref, all_refs, + f"Variable reference {ref} was reused!") + all_refs.add(ref) + + # Verify we collected references and they're still accessible + self.assertGreater(len(all_refs), 0, "Should have found variable references") + for ref in all_refs: + response = self.dap_server.request_variables(ref) + self.assertTrue(response['success'], f"Failed to access reference {ref}") >From 4dae801003ed014378cfd95bfea1b6ddac0664b2 Mon Sep 17 00:00:00 2001 From: Anthony <he...@anthonyeid.me> Date: Thu, 28 Aug 2025 02:26:15 -0400 Subject: [PATCH 08/17] Format test files with darker --- .../lldb-dap/variables/TestDAP_variables.py | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py b/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py index 342dc4de7b895..705863edbc861 100644 --- a/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py +++ b/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py @@ -843,33 +843,28 @@ def test_variable_id_uniqueness_simple(self): self.build_and_launch(program) source = "main.cpp" - # Set breakpoint at test_indexedVariables call to get multiple frames bp_line = line_number(source, "// breakpoint 3") self.set_source_breakpoints(source, [bp_line]) self.continue_to_next_stop() - # Get stack frames frames = self.get_stackFrames() self.assertGreaterEqual(len(frames), 2, "Need at least 2 frames") - # Track all variable references for uniqueness check all_refs = set() - # Check first 2-3 frames for i in range(min(3, len(frames))): - frame_id = frames[i]['id'] + frame_id = frames[i]["id"] scopes = self.dap_server.request_scopes(frame_id)["body"]["scopes"] for scope in scopes: - ref = scope['variablesReference'] - if ref != 0: # 0 means no variables - # Ensure this reference hasn't been used before - self.assertNotIn(ref, all_refs, - f"Variable reference {ref} was reused!") + ref = scope["variablesReference"] + if ref != 0: + self.assertNotIn( + ref, all_refs, f"Variable reference {ref} was reused!" + ) all_refs.add(ref) - # Verify we collected references and they're still accessible self.assertGreater(len(all_refs), 0, "Should have found variable references") for ref in all_refs: response = self.dap_server.request_variables(ref) - self.assertTrue(response['success'], f"Failed to access reference {ref}") + self.assertTrue(response["success"], f"Failed to access reference {ref}") >From a52fe0777cc872932f583926aa6c9098962d84aa Mon Sep 17 00:00:00 2001 From: Anthony <he...@anthonyeid.me> Date: Thu, 28 Aug 2025 02:33:27 -0400 Subject: [PATCH 09/17] Remove unused global defines and set First Var Ref to 1 --- lldb/tools/lldb-dap/Variables.h | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/lldb/tools/lldb-dap/Variables.h b/lldb/tools/lldb-dap/Variables.h index a8fad707e2e08..2ea386d8d50c9 100644 --- a/lldb/tools/lldb-dap/Variables.h +++ b/lldb/tools/lldb-dap/Variables.h @@ -14,10 +14,7 @@ #include "llvm/ADT/DenseMap.h" #include <map> -#define VARREF_FIRST_VAR_IDX (int64_t)4 -#define VARREF_LOCALS (int64_t)1 -#define VARREF_GLOBALS (int64_t)2 -#define VARREF_REGS (int64_t)3 +#define VARREF_FIRST_VAR_IDX (int64_t)1 namespace lldb_dap { >From 5f47586f82d1df6f7c3a4f8335c46cd191262bfa Mon Sep 17 00:00:00 2001 From: Anthony <he...@anthonyeid.me> Date: Thu, 28 Aug 2025 18:12:23 -0400 Subject: [PATCH 10/17] Use scopekind instead of hardcoded comparisons in CreateScope --- .../lldb-dap/Handler/ScopesRequestHandler.cpp | 25 +++++++++++++------ 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/lldb/tools/lldb-dap/Handler/ScopesRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/ScopesRequestHandler.cpp index 553a7b30adba2..2073eb6f9fc76 100644 --- a/lldb/tools/lldb-dap/Handler/ScopesRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/ScopesRequestHandler.cpp @@ -8,6 +8,7 @@ #include "DAP.h" #include "RequestHandler.h" +#include "Variables.h" using namespace lldb_dap::protocol; namespace lldb_dap { @@ -29,10 +30,9 @@ namespace lldb_dap { /// /// \return /// A `protocol::Scope` -Scope CreateScope(const llvm::StringRef name, int64_t variablesReference, +Scope CreateScope(const ScopeKind kind, int64_t variablesReference, int64_t namedVariables, bool expensive) { Scope scope; - scope.name = name; // TODO: Support "arguments" and "return value" scope. // At the moment lldb-dap includes the arguments and return_value into the @@ -41,10 +41,21 @@ 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 (name == "Locals") + switch (kind) { + case ScopeKind::Locals: scope.presentationHint = Scope::eScopePresentationHintLocals; - else if (name == "Registers") + scope.name = "Locals"; + break; + case ScopeKind::Globals: + scope.name = "Globals"; + break; + case ScopeKind::Registers: scope.presentationHint = Scope::eScopePresentationHintRegisters; + scope.name = "Registers"; + break; + default: + llvm_unreachable("Unhandled ScopeKind"); + } scope.variablesReference = variablesReference; scope.namedVariables = namedVariables; @@ -84,18 +95,18 @@ ScopesRequestHandler::Run(const ScopesArguments &args) const { std::vector<Scope> scopes = {}; int64_t variable_reference = dap.variables.GetNewVariableReference(false); - scopes.push_back(CreateScope("Locals", variable_reference, + scopes.push_back(CreateScope(ScopeKind::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, + scopes.push_back(CreateScope(ScopeKind::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, + scopes.push_back(CreateScope(ScopeKind::Registers, variable_reference, dap.variables.registers.GetSize(), false)); dap.variables.AddScopeKind(variable_reference, ScopeKind::Registers, >From b7548f1e8d1bff888b3987d78ad694db1a9e310f Mon Sep 17 00:00:00 2001 From: Anthony <he...@anthonyeid.me> Date: Fri, 29 Aug 2025 01:43:41 -0400 Subject: [PATCH 11/17] Stop storing duplicate scopes in variables I got rid of a variables locals, globals, and register fields and added some helper methods to access scopes based on a var_ref or a frame_id and a ScopeKind variant --- .../Handler/EvaluateRequestHandler.cpp | 1 - .../lldb-dap/Handler/ScopesRequestHandler.cpp | 25 ++++---- .../Handler/VariablesRequestHandler.cpp | 13 ++-- lldb/tools/lldb-dap/SourceBreakpoint.cpp | 1 - lldb/tools/lldb-dap/Variables.cpp | 64 +++++++++++++------ lldb/tools/lldb-dap/Variables.h | 21 ++++-- 6 files changed, 79 insertions(+), 46 deletions(-) diff --git a/lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp index e207f830183b6..e1556846dff19 100644 --- a/lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp @@ -162,7 +162,6 @@ 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 2073eb6f9fc76..1f4641340d234 100644 --- a/lldb/tools/lldb-dap/Handler/ScopesRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/ScopesRequestHandler.cpp @@ -30,8 +30,8 @@ namespace lldb_dap { /// /// \return /// A `protocol::Scope` -Scope CreateScope(const ScopeKind kind, int64_t variablesReference, - int64_t namedVariables, bool expensive) { +static Scope CreateScope(const ScopeKind kind, int64_t variablesReference, + int64_t namedVariables, bool expensive) { Scope scope; // TODO: Support "arguments" and "return value" scope. @@ -53,8 +53,6 @@ Scope CreateScope(const ScopeKind kind, int64_t variablesReference, scope.presentationHint = Scope::eScopePresentationHintRegisters; scope.name = "Registers"; break; - default: - llvm_unreachable("Unhandled ScopeKind"); } scope.variablesReference = variablesReference; @@ -90,24 +88,27 @@ ScopesRequestHandler::Run(const ScopesArguments &args) const { uint32_t frame_id = frame.GetFrameID(); dap.variables.ReadyFrame(frame_id, frame); - dap.variables.SwitchFrame(frame_id); - std::vector<Scope> scopes = {}; + std::vector<protocol::Scope> scopes = {}; int64_t variable_reference = dap.variables.GetNewVariableReference(false); - scopes.push_back(CreateScope(ScopeKind::Locals, variable_reference, - dap.variables.locals.GetSize(), false)); + scopes.push_back(CreateScope( + ScopeKind::Locals, variable_reference, + dap.variables.GetScope(frame_id, ScopeKind::Locals)->GetSize(), false)); dap.variables.AddScopeKind(variable_reference, ScopeKind::Locals, frame_id); variable_reference = dap.variables.GetNewVariableReference(false); - scopes.push_back(CreateScope(ScopeKind::Globals, variable_reference, - dap.variables.globals.GetSize(), false)); + scopes.push_back(CreateScope( + ScopeKind::Globals, variable_reference, + dap.variables.GetScope(frame_id, ScopeKind::Globals)->GetSize(), false)); dap.variables.AddScopeKind(variable_reference, ScopeKind::Globals, frame_id); variable_reference = dap.variables.GetNewVariableReference(false); - scopes.push_back(CreateScope(ScopeKind::Registers, variable_reference, - dap.variables.registers.GetSize(), false)); + scopes.push_back(CreateScope( + ScopeKind::Registers, variable_reference, + dap.variables.GetScope(frame_id, ScopeKind::Registers)->GetSize(), + false)); dap.variables.AddScopeKind(variable_reference, ScopeKind::Registers, frame_id); diff --git a/lldb/tools/lldb-dap/Handler/VariablesRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/VariablesRequestHandler.cpp index 3d99983ac0c83..ca5da1e78a79f 100644 --- a/lldb/tools/lldb-dap/Handler/VariablesRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/VariablesRequestHandler.cpp @@ -11,6 +11,7 @@ #include "Handler/RequestHandler.h" #include "JSONUtils.h" #include "ProtocolUtils.h" +#include "Variables.h" using namespace llvm; using namespace lldb_dap::protocol; @@ -38,15 +39,16 @@ VariablesRequestHandler::Run(const VariablesArguments &arguments) const { int64_t start_idx = 0; int64_t num_children = 0; - std::optional<ScopeKind> scope_kind = dap.variables.GetScopeKind(var_ref); - if (scope_kind && *scope_kind == ScopeKind::Registers) { + std::optional<ScopeData> scope_data = dap.variables.GetScopeKind(var_ref); + if (scope_data.has_value() && scope_data->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 // format was set to the default format or if it was hex as some registers // have formats set for them. const uint32_t addr_size = dap.target.GetProcess().GetAddressByteSize(); - lldb::SBValue reg_set = dap.variables.registers.GetValueAtIndex(0); + lldb::SBValue reg_set = scope_data->scope.GetValueAtIndex(0); const uint32_t num_regs = reg_set.GetNumChildren(); for (uint32_t reg_idx = 0; reg_idx < num_regs; ++reg_idx) { lldb::SBValue reg = reg_set.GetChildAtIndex(reg_idx); @@ -59,7 +61,8 @@ VariablesRequestHandler::Run(const VariablesArguments &arguments) const { } num_children = top_scope->GetSize(); - if (num_children == 0 && scope_kind && *scope_kind == ScopeKind::Locals) { + if (num_children == 0 && scope_data && + scope_data->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. @@ -95,7 +98,7 @@ VariablesRequestHandler::Run(const VariablesArguments &arguments) const { } // Show return value if there is any ( in the local top frame ) - if (scope_kind && *scope_kind == ScopeKind::Locals) { + if (scope_data && scope_data->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/SourceBreakpoint.cpp b/lldb/tools/lldb-dap/SourceBreakpoint.cpp index 843a5eb09c7ae..6f776f4d5f429 100644 --- a/lldb/tools/lldb-dap/SourceBreakpoint.cpp +++ b/lldb/tools/lldb-dap/SourceBreakpoint.cpp @@ -10,7 +10,6 @@ #include "BreakpointBase.h" #include "DAP.h" #include "JSONUtils.h" -#include "ProtocolUtils.h" #include "lldb/API/SBBreakpoint.h" #include "lldb/API/SBFileSpec.h" #include "lldb/API/SBFileSpecList.h" diff --git a/lldb/tools/lldb-dap/Variables.cpp b/lldb/tools/lldb-dap/Variables.cpp index b5cd9d69d385f..82e8950c2deb9 100644 --- a/lldb/tools/lldb-dap/Variables.cpp +++ b/lldb/tools/lldb-dap/Variables.cpp @@ -9,6 +9,9 @@ #include "Variables.h" #include "JSONUtils.h" #include "lldb/API/SBFrame.h" +#include "lldb/API/SBValueList.h" +#include <cstdint> +#include <optional> using namespace lldb_dap; @@ -21,26 +24,24 @@ lldb::SBValueList *Variables::GetTopLevelScope(int64_t variablesReference) { ScopeKind scope_kind = iter->second.first; uint32_t frame_id = iter->second.second; - if (!SwitchFrame(frame_id)) { + auto frame_iter = m_frames.find(frame_id); + if (frame_iter == m_frames.end()) { return nullptr; } switch (scope_kind) { case lldb_dap::ScopeKind::Locals: - return &locals; + return &std::get<0>(frame_iter->second); case lldb_dap::ScopeKind::Globals: - return &globals; + return &std::get<1>(frame_iter->second); case lldb_dap::ScopeKind::Registers: - return ®isters; + return &std::get<2>(frame_iter->second); } return nullptr; } void Variables::Clear() { - locals.Clear(); - globals.Clear(); - registers.Clear(); m_referencedvariables.clear(); m_scope_kinds.clear(); m_frames.clear(); @@ -120,30 +121,51 @@ lldb::SBValue Variables::FindVariable(uint64_t variablesReference, return variable; } -std::optional<ScopeKind> +std::optional<ScopeData> Variables::GetScopeKind(const int64_t variablesReference) { - auto iter = m_scope_kinds.find(variablesReference); - if (iter != m_scope_kinds.end()) { - return iter->second.first; + auto scope_kind_iter = m_scope_kinds.find(variablesReference); + if (scope_kind_iter == m_scope_kinds.end()) { + return std::nullopt; + } + + auto scope_iter = m_frames.find(scope_kind_iter->second.second); + if (scope_iter == m_frames.end()) { + return std::nullopt; + } + + switch (scope_kind_iter->second.first) { + case lldb_dap::ScopeKind::Locals: + return ScopeData(scope_kind_iter->second.first, + std::get<0>(scope_iter->second)); + case lldb_dap::ScopeKind::Globals: + return ScopeData(scope_kind_iter->second.first, + std::get<1>(scope_iter->second)); + case lldb_dap::ScopeKind::Registers: + return ScopeData(scope_kind_iter->second.first, + std::get<2>(scope_iter->second)); } return std::nullopt; } -bool Variables::SwitchFrame(const uint32_t frame_id) { - auto iter = m_frames.find(frame_id); +lldb::SBValueList *Variables::GetScope(const uint32_t frame_id, + const ScopeKind kind) { - if (iter == m_frames.end()) { - return false; + auto frame = m_frames.find(frame_id); + if (m_frames.find(frame_id) == m_frames.end()) { + return nullptr; } - auto [frame_locals, frame_globals, frame_registers] = iter->second; - - locals = frame_locals; - globals = frame_globals; - registers = frame_registers; + switch (kind) { + case ScopeKind::Locals: + return &std::get<0>(frame->second); + case ScopeKind::Globals: + return &std::get<1>(frame->second); + case ScopeKind::Registers: + return &std::get<2>(frame->second); + } - return true; + return nullptr; } void Variables::ReadyFrame(uint32_t frame_id, lldb::SBFrame &frame) { diff --git a/lldb/tools/lldb-dap/Variables.h b/lldb/tools/lldb-dap/Variables.h index 2ea386d8d50c9..e001d688bd039 100644 --- a/lldb/tools/lldb-dap/Variables.h +++ b/lldb/tools/lldb-dap/Variables.h @@ -13,6 +13,7 @@ #include "lldb/API/SBValueList.h" #include "llvm/ADT/DenseMap.h" #include <map> +#include <utility> #define VARREF_FIRST_VAR_IDX (int64_t)1 @@ -20,11 +21,15 @@ namespace lldb_dap { enum ScopeKind { Locals, Globals, Registers }; -struct Variables { - lldb::SBValueList locals; - lldb::SBValueList globals; - lldb::SBValueList registers; +struct ScopeData { + ScopeKind kind; + lldb::SBValueList scope; + + ScopeData(ScopeKind kind, lldb::SBValueList scope) + : kind(kind), scope(scope) {} +}; +struct Variables { /// Check if \p var_ref points to a variable that should persist for the /// entire duration of the debug session, e.g. repl expandable variables static bool IsPermanentVariableReference(int64_t var_ref); @@ -39,6 +44,8 @@ struct Variables { /// If \p var_ref is invalid an empty SBValue is returned. lldb::SBValue GetVariable(int64_t var_ref) const; + lldb::SBValueList *GetScope(const uint32_t frame_id, const ScopeKind kind); + /// Insert a new \p variable. /// \return variableReference assigned to this expandable variable. int64_t InsertVariable(lldb::SBValue variable, bool is_permanent); @@ -47,10 +54,9 @@ 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); + std::optional<ScopeData> GetScopeKind(const int64_t variablesReference); /// Clear all scope variables and non-permanent expandable variables. void Clear(); @@ -63,6 +69,7 @@ struct Variables { static constexpr int64_t PermanentVariableStartIndex = (1ll << 32); int64_t m_next_temporary_var_ref{VARREF_FIRST_VAR_IDX}; + // Variable Reference, frame_id std::map<int64_t, std::pair<ScopeKind, uint32_t>> m_scope_kinds; /// Variables that are alive in this stop state. @@ -73,6 +80,8 @@ struct Variables { /// These are the variables evaluated from debug console REPL. llvm::DenseMap<int64_t, lldb::SBValue> m_referencedpermanent_variables; + /// Key = frame_id + /// Value = (locals, globals Registers) scopes std::map<uint32_t, std::tuple<lldb::SBValueList, lldb::SBValueList, lldb::SBValueList>> m_frames; >From c8196a27ea8ff5b89a5d83aa3ee85f227858a4fe Mon Sep 17 00:00:00 2001 From: Anthony <he...@anthonyeid.me> Date: Fri, 29 Aug 2025 02:01:04 -0400 Subject: [PATCH 12/17] Apply Python formatting to fix CI darker checks --- .../test/tools/lldb-dap/lldbdap_testcase.py | 5 +-- .../lldb-dap/variables/TestDAP_variables.py | 34 +++++++++++++------ 2 files changed, 27 insertions(+), 12 deletions(-) diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py index a26be6e5d1bfb..9c363e54e747c 100644 --- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py +++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py @@ -369,10 +369,11 @@ def set_global(self, name, value, id=None): for scope in frame_scopes: if scope["name"] == "Globals": varRef = scope["variablesReference"] - return self.dap_server.request_setVariable(varRef, name, str(value), id=id) + return self.dap_server.request_setVariable( + varRef, name, str(value), id=id + ) return None - def get_locals_scope_reference(self): """Get the variablesReference for the locals scope.""" stackFrame = self.dap_server.get_stackFrame() diff --git a/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py b/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py index 705863edbc861..6d83661ede922 100644 --- a/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py +++ b/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py @@ -342,26 +342,36 @@ def do_test_scopes_variables_setVariable_evaluate( # Now we verify that we correctly change the name of a variable with and without differentiator suffix local_scope_ref = self.get_locals_scope_reference() - self.assertFalse(self.dap_server.request_setVariable(local_scope_ref, "x2", 9)["success"]) self.assertFalse( - self.dap_server.request_setVariable(local_scope_ref, "x @ main.cpp:0", 9)["success"] + self.dap_server.request_setVariable(local_scope_ref, "x2", 9)["success"] + ) + self.assertFalse( + self.dap_server.request_setVariable(local_scope_ref, "x @ main.cpp:0", 9)[ + "success" + ] ) self.assertTrue( - self.dap_server.request_setVariable(local_scope_ref, "x @ main.cpp:19", 19)["success"] + self.dap_server.request_setVariable(local_scope_ref, "x @ main.cpp:19", 19)[ + "success" + ] ) self.assertTrue( - self.dap_server.request_setVariable(local_scope_ref, "x @ main.cpp:21", 21)["success"] + self.dap_server.request_setVariable(local_scope_ref, "x @ main.cpp:21", 21)[ + "success" + ] ) self.assertTrue( - self.dap_server.request_setVariable(local_scope_ref, "x @ main.cpp:23", 23)["success"] + self.dap_server.request_setVariable(local_scope_ref, "x @ main.cpp:23", 23)[ + "success" + ] ) # The following should have no effect self.assertFalse( - self.dap_server.request_setVariable(local_scope_ref, "x @ main.cpp:23", "invalid")[ - "success" - ] + self.dap_server.request_setVariable( + local_scope_ref, "x @ main.cpp:23", "invalid" + )["success"] ) verify_locals["x @ main.cpp:19"]["equals"]["value"] = "19" @@ -371,7 +381,9 @@ def do_test_scopes_variables_setVariable_evaluate( self.verify_variables(verify_locals, self.dap_server.get_local_variables()) # The plain x variable shold refer to the innermost x - self.assertTrue(self.dap_server.request_setVariable(local_scope_ref, "x", 22)["success"]) + self.assertTrue( + self.dap_server.request_setVariable(local_scope_ref, "x", 22)["success"] + ) verify_locals["x @ main.cpp:23"]["equals"]["value"] = "22" self.verify_variables(verify_locals, self.dap_server.get_local_variables()) @@ -710,7 +722,9 @@ def test_return_variables(self): break self.assertFalse( - self.dap_server.request_setVariable(self.get_locals_scope_reference(), "(Return Value)", 20)["success"] + self.dap_server.request_setVariable( + self.get_locals_scope_reference(), "(Return Value)", 20 + )["success"] ) @skipIfWindows >From 23879526b6304f21dac5ea4e584e347f6d8fe0a5 Mon Sep 17 00:00:00 2001 From: Anthony <he...@anthonyeid.me> Date: Fri, 29 Aug 2025 02:30:46 -0400 Subject: [PATCH 13/17] Hopefully fix failing tests (They're not running on my end) --- lldb/unittests/DAP/VariablesTest.cpp | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/lldb/unittests/DAP/VariablesTest.cpp b/lldb/unittests/DAP/VariablesTest.cpp index 40884e89e0c54..46b171a443586 100644 --- a/lldb/unittests/DAP/VariablesTest.cpp +++ b/lldb/unittests/DAP/VariablesTest.cpp @@ -71,11 +71,6 @@ TEST_F(VariablesTest, GetTopLevelScope_ReturnsCorrectScope) { uint32_t frame_id = 0; vars.ReadyFrame(frame_id, frame); - vars.SwitchFrame(frame_id); - - vars.locals.Append(lldb::SBValue()); - vars.globals.Append(lldb::SBValue()); - vars.registers.Append(lldb::SBValue()); int64_t locals_ref = vars.GetNewVariableReference(false); vars.AddScopeKind(locals_ref, ScopeKind::Locals, frame_id); @@ -86,9 +81,12 @@ TEST_F(VariablesTest, GetTopLevelScope_ReturnsCorrectScope) { int64_t registers_ref = vars.GetNewVariableReference(false); vars.AddScopeKind(registers_ref, ScopeKind::Registers, frame_id); - EXPECT_EQ(vars.GetTopLevelScope(locals_ref), &vars.locals); - EXPECT_EQ(vars.GetTopLevelScope(globals_ref), &vars.globals); - EXPECT_EQ(vars.GetTopLevelScope(registers_ref), &vars.registers); + EXPECT_EQ(vars.GetTopLevelScope(locals_ref), + &vars.GetScope(frame_id, lldb_dap::ScopeKind::Locals)); + EXPECT_EQ(vars.GetTopLevelScope(globals_ref), + &vars.GetScope(frame_id, lldb_dap::ScopeKind::Globals)); + EXPECT_EQ(vars.GetTopLevelScope(registers_ref), + &vars.GetScope(frame_id, lldb_dap::ScopeKind::Registers)); EXPECT_EQ(vars.GetTopLevelScope(9999), nullptr); } @@ -97,10 +95,6 @@ TEST_F(VariablesTest, FindVariable_LocalsByName) { uint32_t frame_id = 0; vars.ReadyFrame(frame_id, frame); - vars.SwitchFrame(frame_id); - - lldb::SBValue dummy; - vars.locals.Append(dummy); int64_t locals_ref = vars.GetNewVariableReference(false); vars.AddScopeKind(locals_ref, ScopeKind::Locals, frame_id); >From bc0032595afe685a75c25dab514f7d4100c8b440 Mon Sep 17 00:00:00 2001 From: Anthony <he...@anthonyeid.me> Date: Fri, 29 Aug 2025 12:20:49 -0400 Subject: [PATCH 14/17] Actually fix the failing tests for real --- lldb/unittests/DAP/VariablesTest.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lldb/unittests/DAP/VariablesTest.cpp b/lldb/unittests/DAP/VariablesTest.cpp index 46b171a443586..2c5e608c30675 100644 --- a/lldb/unittests/DAP/VariablesTest.cpp +++ b/lldb/unittests/DAP/VariablesTest.cpp @@ -82,11 +82,11 @@ TEST_F(VariablesTest, GetTopLevelScope_ReturnsCorrectScope) { vars.AddScopeKind(registers_ref, ScopeKind::Registers, frame_id); EXPECT_EQ(vars.GetTopLevelScope(locals_ref), - &vars.GetScope(frame_id, lldb_dap::ScopeKind::Locals)); + vars.GetScope(frame_id, lldb_dap::ScopeKind::Locals)); EXPECT_EQ(vars.GetTopLevelScope(globals_ref), - &vars.GetScope(frame_id, lldb_dap::ScopeKind::Globals)); + vars.GetScope(frame_id, lldb_dap::ScopeKind::Globals)); EXPECT_EQ(vars.GetTopLevelScope(registers_ref), - &vars.GetScope(frame_id, lldb_dap::ScopeKind::Registers)); + vars.GetScope(frame_id, lldb_dap::ScopeKind::Registers)); EXPECT_EQ(vars.GetTopLevelScope(9999), nullptr); } @@ -100,6 +100,7 @@ TEST_F(VariablesTest, FindVariable_LocalsByName) { vars.AddScopeKind(locals_ref, ScopeKind::Locals, frame_id); lldb::SBValue found = vars.FindVariable(locals_ref, ""); + lldb::SBValue dummy; EXPECT_EQ(found.IsValid(), dummy.IsValid()); } >From facb48958e9f426e075ebef7d95614ff6d020ff4 Mon Sep 17 00:00:00 2001 From: Anthony <he...@anthonyeid.me> Date: Tue, 2 Sep 2025 10:47:57 -0400 Subject: [PATCH 15/17] Simplify Scopes request handler Merge ReadyFrame and Switch Frame functions. Also get rid of AddScopeKind function in favor of inlining it in ReadyFrame --- .../lldb-dap/Handler/ScopesRequestHandler.cpp | 77 +------------------ lldb/tools/lldb-dap/JSONUtils.cpp | 1 - lldb/tools/lldb-dap/JSONUtils.h | 1 - lldb/tools/lldb-dap/Variables.cpp | 72 +++++++++++++++-- lldb/tools/lldb-dap/Variables.h | 25 +++++- 5 files changed, 90 insertions(+), 86 deletions(-) diff --git a/lldb/tools/lldb-dap/Handler/ScopesRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/ScopesRequestHandler.cpp index 1f4641340d234..0f0c80383bf2d 100644 --- a/lldb/tools/lldb-dap/Handler/ScopesRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/ScopesRequestHandler.cpp @@ -13,55 +13,6 @@ using namespace lldb_dap::protocol; namespace lldb_dap { -/// Creates a `protocol::Scope` struct. -/// -/// -/// \param[in] name -/// The value to place into the "name" key -/// -/// \param[in] variablesReference -/// The value to place into the "variablesReference" key -/// -/// \param[in] namedVariables -/// The value to place into the "namedVariables" key -/// -/// \param[in] expensive -/// The value to place into the "expensive" key -/// -/// \return -/// A `protocol::Scope` -static Scope CreateScope(const ScopeKind kind, int64_t variablesReference, - int64_t namedVariables, bool expensive) { - Scope scope; - - // TODO: Support "arguments" and "return value" scope. - // At the moment lldb-dap includes the arguments and return_value into the - // "locals" scope. - // vscode only expands the first non-expensive scope, this causes friction - // 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. - switch (kind) { - case ScopeKind::Locals: - scope.presentationHint = Scope::eScopePresentationHintLocals; - scope.name = "Locals"; - break; - case ScopeKind::Globals: - scope.name = "Globals"; - break; - case ScopeKind::Registers: - scope.presentationHint = Scope::eScopePresentationHintRegisters; - scope.name = "Registers"; - break; - } - - scope.variablesReference = variablesReference; - scope.namedVariables = namedVariables; - scope.expensive = expensive; - - return scope; -} - llvm::Expected<ScopesResponseBody> ScopesRequestHandler::Run(const ScopesArguments &args) const { lldb::SBFrame frame = dap.GetLLDBFrame(args.frameId); @@ -86,32 +37,8 @@ ScopesRequestHandler::Run(const ScopesArguments &args) const { } uint32_t frame_id = frame.GetFrameID(); - - dap.variables.ReadyFrame(frame_id, frame); - - std::vector<protocol::Scope> scopes = {}; - - int64_t variable_reference = dap.variables.GetNewVariableReference(false); - scopes.push_back(CreateScope( - ScopeKind::Locals, variable_reference, - dap.variables.GetScope(frame_id, ScopeKind::Locals)->GetSize(), false)); - - dap.variables.AddScopeKind(variable_reference, ScopeKind::Locals, frame_id); - - variable_reference = dap.variables.GetNewVariableReference(false); - scopes.push_back(CreateScope( - ScopeKind::Globals, variable_reference, - dap.variables.GetScope(frame_id, ScopeKind::Globals)->GetSize(), false)); - dap.variables.AddScopeKind(variable_reference, ScopeKind::Globals, frame_id); - - variable_reference = dap.variables.GetNewVariableReference(false); - scopes.push_back(CreateScope( - ScopeKind::Registers, variable_reference, - dap.variables.GetScope(frame_id, ScopeKind::Registers)->GetSize(), - false)); - - dap.variables.AddScopeKind(variable_reference, ScopeKind::Registers, - frame_id); + std::vector<protocol::Scope> scopes = + dap.variables.ReadyFrame(frame_id, frame); return ScopesResponseBody{std::move(scopes)}; } diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp index ad1f035caaf54..a4c42346d31f8 100644 --- a/lldb/tools/lldb-dap/JSONUtils.cpp +++ b/lldb/tools/lldb-dap/JSONUtils.cpp @@ -11,7 +11,6 @@ #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" diff --git a/lldb/tools/lldb-dap/JSONUtils.h b/lldb/tools/lldb-dap/JSONUtils.h index 6575411acd878..e9094f67b94ec 100644 --- a/lldb/tools/lldb-dap/JSONUtils.h +++ b/lldb/tools/lldb-dap/JSONUtils.h @@ -9,7 +9,6 @@ #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 82e8950c2deb9..4566931d0e7cc 100644 --- a/lldb/tools/lldb-dap/Variables.cpp +++ b/lldb/tools/lldb-dap/Variables.cpp @@ -8,13 +8,49 @@ #include "Variables.h" #include "JSONUtils.h" +#include "Protocol/ProtocolTypes.h" #include "lldb/API/SBFrame.h" #include "lldb/API/SBValueList.h" #include <cstdint> #include <optional> +#include <vector> using namespace lldb_dap; +namespace lldb_dap { + +protocol::Scope CreateScope(const ScopeKind kind, int64_t variablesReference, + int64_t namedVariables, bool expensive) { + protocol::Scope scope; + + // TODO: Support "arguments" and "return value" scope. + // At the moment lldb-he arguments and return_value into the + // "locals" scope. + // vscode only expands the first non-expensive scope, this causes friction + // 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. + switch (kind) { + case ScopeKind::Locals: + scope.presentationHint = protocol::Scope::eScopePresentationHintLocals; + scope.name = "Locals"; + break; + case ScopeKind::Globals: + scope.name = "Globals"; + break; + case ScopeKind::Registers: + scope.presentationHint = protocol::Scope::eScopePresentationHintRegisters; + scope.name = "Registers"; + break; + } + + scope.variablesReference = variablesReference; + scope.namedVariables = namedVariables; + scope.expensive = expensive; + + return scope; +} + lldb::SBValueList *Variables::GetTopLevelScope(int64_t variablesReference) { auto iter = m_scope_kinds.find(variablesReference); if (iter == m_scope_kinds.end()) { @@ -168,7 +204,9 @@ lldb::SBValueList *Variables::GetScope(const uint32_t frame_id, return nullptr; } -void Variables::ReadyFrame(uint32_t frame_id, lldb::SBFrame &frame) { +std::vector<protocol::Scope> Variables::ReadyFrame(uint32_t frame_id, + lldb::SBFrame &frame) { + if (m_frames.find(frame_id) == m_frames.end()) { auto locals = frame.GetVariables(/*arguments=*/true, @@ -186,10 +224,34 @@ void Variables::ReadyFrame(uint32_t frame_id, lldb::SBFrame &frame) { 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) { + std::vector<protocol::Scope> scopes = {}; + + int64_t variable_reference = GetNewVariableReference(false); + + scopes.push_back(CreateScope(ScopeKind::Locals, variable_reference, + GetScope(frame_id, ScopeKind::Locals)->GetSize(), + false)); - m_scope_kinds[variable_reference] = std::make_pair(kind, frame_id); + m_scope_kinds[variable_reference] = + std::make_pair(ScopeKind::Locals, frame_id); + + variable_reference = GetNewVariableReference(false); + scopes.push_back( + CreateScope(ScopeKind::Globals, variable_reference, + GetScope(frame_id, ScopeKind::Globals)->GetSize(), false)); + m_scope_kinds[variable_reference] = + std::make_pair(ScopeKind::Globals, frame_id); + + variable_reference = GetNewVariableReference(false); + scopes.push_back( + CreateScope(ScopeKind::Registers, variable_reference, + GetScope(frame_id, ScopeKind::Registers)->GetSize(), false)); + + m_scope_kinds[variable_reference] = + std::make_pair(ScopeKind::Registers, frame_id); + + return scopes; } + +} // namespace lldb_dap diff --git a/lldb/tools/lldb-dap/Variables.h b/lldb/tools/lldb-dap/Variables.h index e001d688bd039..2470cd030b47d 100644 --- a/lldb/tools/lldb-dap/Variables.h +++ b/lldb/tools/lldb-dap/Variables.h @@ -9,6 +9,7 @@ #ifndef LLDB_TOOLS_LLDB_DAP_VARIABLES_H #define LLDB_TOOLS_LLDB_DAP_VARIABLES_H +#include "Protocol/ProtocolTypes.h" #include "lldb/API/SBValue.h" #include "lldb/API/SBValueList.h" #include "llvm/ADT/DenseMap.h" @@ -20,6 +21,24 @@ namespace lldb_dap { enum ScopeKind { Locals, Globals, Registers }; +/// Creates a `protocol::Scope` struct. +/// +/// \param[in] kind +/// The kind of scope to create +/// +/// \param[in] variablesReference +/// The value to place into the "variablesReference" key +/// +/// \param[in] namedVariables +/// The value to place into the "namedVariables" key +/// +/// \param[in] expensive +/// The value to place into the "expensive" key +/// +/// \return +/// A `protocol::Scope` +protocol::Scope CreateScope(const ScopeKind kind, int64_t variablesReference, + int64_t namedVariables, bool expensive); struct ScopeData { ScopeKind kind; @@ -55,15 +74,13 @@ struct Variables { lldb::SBValue FindVariable(uint64_t variablesReference, llvm::StringRef name); /// Initialize a frame if it hasn't been already, otherwise do nothing - void ReadyFrame(uint32_t frame_id, lldb::SBFrame &frame); + std::vector<protocol::Scope> ReadyFrame(uint32_t frame_id, + lldb::SBFrame &frame); std::optional<ScopeData> 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); >From c4df0a8aaca50ef1c473d28643f8788b691e6694 Mon Sep 17 00:00:00 2001 From: Anthony <he...@anthonyeid.me> Date: Tue, 2 Sep 2025 14:20:09 -0400 Subject: [PATCH 16/17] Fix variables test --- lldb/unittests/DAP/VariablesTest.cpp | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/lldb/unittests/DAP/VariablesTest.cpp b/lldb/unittests/DAP/VariablesTest.cpp index 2c5e608c30675..8857d420ced20 100644 --- a/lldb/unittests/DAP/VariablesTest.cpp +++ b/lldb/unittests/DAP/VariablesTest.cpp @@ -72,20 +72,13 @@ TEST_F(VariablesTest, GetTopLevelScope_ReturnsCorrectScope) { vars.ReadyFrame(frame_id, frame); - int64_t locals_ref = vars.GetNewVariableReference(false); - vars.AddScopeKind(locals_ref, ScopeKind::Locals, frame_id); - - int64_t globals_ref = vars.GetNewVariableReference(false); - vars.AddScopeKind(globals_ref, ScopeKind::Globals, frame_id); - - int64_t registers_ref = vars.GetNewVariableReference(false); - vars.AddScopeKind(registers_ref, ScopeKind::Registers, frame_id); + int64_t next_variable_ref = vars.GetNewVariableReference(false); - EXPECT_EQ(vars.GetTopLevelScope(locals_ref), + EXPECT_EQ(vars.GetTopLevelScope(next_variable_ref - 3), vars.GetScope(frame_id, lldb_dap::ScopeKind::Locals)); - EXPECT_EQ(vars.GetTopLevelScope(globals_ref), + EXPECT_EQ(vars.GetTopLevelScope(next_variable_ref - 2), vars.GetScope(frame_id, lldb_dap::ScopeKind::Globals)); - EXPECT_EQ(vars.GetTopLevelScope(registers_ref), + EXPECT_EQ(vars.GetTopLevelScope(next_variable_ref - 1), vars.GetScope(frame_id, lldb_dap::ScopeKind::Registers)); EXPECT_EQ(vars.GetTopLevelScope(9999), nullptr); } @@ -97,9 +90,8 @@ TEST_F(VariablesTest, FindVariable_LocalsByName) { vars.ReadyFrame(frame_id, frame); int64_t locals_ref = vars.GetNewVariableReference(false); - vars.AddScopeKind(locals_ref, ScopeKind::Locals, frame_id); - lldb::SBValue found = vars.FindVariable(locals_ref, ""); + lldb::SBValue found = vars.FindVariable(locals_ref - 1, ""); lldb::SBValue dummy; EXPECT_EQ(found.IsValid(), dummy.IsValid()); >From 348f21ca0507d08dd30c5ebb4929b5007a098a85 Mon Sep 17 00:00:00 2001 From: Anthony <he...@anthonyeid.me> Date: Fri, 5 Sep 2025 01:47:01 -0400 Subject: [PATCH 17/17] Simplify ScopeData and improve naming in ready frame function --- lldb/tools/lldb-dap/Variables.cpp | 36 +++++++++++++++---------------- lldb/tools/lldb-dap/Variables.h | 3 --- 2 files changed, 18 insertions(+), 21 deletions(-) diff --git a/lldb/tools/lldb-dap/Variables.cpp b/lldb/tools/lldb-dap/Variables.cpp index 4566931d0e7cc..8f0c48f4e1ca6 100644 --- a/lldb/tools/lldb-dap/Variables.cpp +++ b/lldb/tools/lldb-dap/Variables.cpp @@ -169,16 +169,19 @@ Variables::GetScopeKind(const int64_t variablesReference) { return std::nullopt; } + ScopeData scope_data = ScopeData(); + scope_data.kind = scope_kind_iter->second.first; + switch (scope_kind_iter->second.first) { case lldb_dap::ScopeKind::Locals: - return ScopeData(scope_kind_iter->second.first, - std::get<0>(scope_iter->second)); + scope_data.scope = std::get<0>(scope_iter->second); + return scope_data; case lldb_dap::ScopeKind::Globals: - return ScopeData(scope_kind_iter->second.first, - std::get<1>(scope_iter->second)); + scope_data.scope = std::get<1>(scope_iter->second); + return scope_data; case lldb_dap::ScopeKind::Registers: - return ScopeData(scope_kind_iter->second.first, - std::get<2>(scope_iter->second)); + scope_data.scope = std::get<2>(scope_iter->second); + return scope_data; } return std::nullopt; @@ -227,29 +230,26 @@ std::vector<protocol::Scope> Variables::ReadyFrame(uint32_t frame_id, std::vector<protocol::Scope> scopes = {}; - int64_t variable_reference = GetNewVariableReference(false); + int64_t locals_ref = GetNewVariableReference(false); - scopes.push_back(CreateScope(ScopeKind::Locals, variable_reference, + scopes.push_back(CreateScope(ScopeKind::Locals, locals_ref, GetScope(frame_id, ScopeKind::Locals)->GetSize(), false)); - m_scope_kinds[variable_reference] = - std::make_pair(ScopeKind::Locals, frame_id); + m_scope_kinds[locals_ref] = std::make_pair(ScopeKind::Locals, frame_id); - variable_reference = GetNewVariableReference(false); + int64_t globals_ref = GetNewVariableReference(false); scopes.push_back( - CreateScope(ScopeKind::Globals, variable_reference, + CreateScope(ScopeKind::Globals, globals_ref, GetScope(frame_id, ScopeKind::Globals)->GetSize(), false)); - m_scope_kinds[variable_reference] = - std::make_pair(ScopeKind::Globals, frame_id); + m_scope_kinds[globals_ref] = std::make_pair(ScopeKind::Globals, frame_id); - variable_reference = GetNewVariableReference(false); + int64_t registers_ref = GetNewVariableReference(false); scopes.push_back( - CreateScope(ScopeKind::Registers, variable_reference, + CreateScope(ScopeKind::Registers, registers_ref, GetScope(frame_id, ScopeKind::Registers)->GetSize(), false)); - m_scope_kinds[variable_reference] = - std::make_pair(ScopeKind::Registers, frame_id); + m_scope_kinds[registers_ref] = std::make_pair(ScopeKind::Registers, frame_id); return scopes; } diff --git a/lldb/tools/lldb-dap/Variables.h b/lldb/tools/lldb-dap/Variables.h index 2470cd030b47d..c3ce14c1661fe 100644 --- a/lldb/tools/lldb-dap/Variables.h +++ b/lldb/tools/lldb-dap/Variables.h @@ -43,9 +43,6 @@ protocol::Scope CreateScope(const ScopeKind kind, int64_t variablesReference, struct ScopeData { ScopeKind kind; lldb::SBValueList scope; - - ScopeData(ScopeKind kind, lldb::SBValueList scope) - : kind(kind), scope(scope) {} }; struct Variables { _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits