Author: Anthony Eid Date: 2026-02-06T09:30:49Z New Revision: 6e3e74afd473bea8b4238ae58576940e218d7a08
URL: https://github.com/llvm/llvm-project/commit/6e3e74afd473bea8b4238ae58576940e218d7a08 DIFF: https://github.com/llvm/llvm-project/commit/6e3e74afd473bea8b4238ae58576940e218d7a08.diff LOG: lldb-dap: Stop using replicated variable ids (#124232) Closes #119784 Probably closes #147105 as well, but I couldn't test due to #156473: This PR fixes two bugs: 1. It generates unique variable reference IDs per suspended debuggee state. 2. It stores all created variables in a stopped state instead of dropping variables in unselected scopes. So it can properly handle all scope/variable requests It does this by storing all variables in their respective scopes and using that mapping in request handlers that relied on the old mapping. It dynamically creates new variable/scope IDs instead of resetting IDs whenever a new scope is created. I also removed some unused code as well. --------- Co-authored-by: Med Ismail Bennani <[email protected]> Co-authored-by: Jonas Devlieghere <[email protected]> Co-authored-by: Ebuka Ezike <[email protected]> (cherry picked from commit 6ca8f79fc88971412e6e5692ea86d680cd438fe3) Added: Modified: lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py lldb/tools/lldb-dap/Handler/ScopesRequestHandler.cpp lldb/tools/lldb-dap/Handler/VariablesRequestHandler.cpp lldb/tools/lldb-dap/JSONUtils.cpp lldb/tools/lldb-dap/SourceBreakpoint.cpp lldb/tools/lldb-dap/Variables.cpp lldb/tools/lldb-dap/Variables.h lldb/unittests/DAP/VariablesTest.cpp Removed: ################################################################################ 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 e4a3e1c786ffe..5dd2405a3389b 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 @@ -379,11 +379,39 @@ def set_variable(self, varRef, name, value, id=None): def set_local(self, name, value, id=None): """Set a top level local variable only.""" - return self.set_variable(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.set_variable(locals_ref, name, str(value), id=id) def set_global(self, name, value, id=None): """Set a top level global variable only.""" - return self.set_variable(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.set_variable(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/databreakpoint/TestDAP_setDataBreakpoints.py b/lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py index df029ca16d667..0c8ba4e5d07af 100644 --- a/lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py +++ b/lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py @@ -111,8 +111,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]" 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 05445af40aea5..1dbb0143e7a55 100644 --- a/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py +++ b/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py @@ -834,3 +834,39 @@ 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 diff erent scopes/frames. + """ + program = self.getBuildArtifact("a.out") + self.build_and_launch(program) + source = "main.cpp" + + bp_line = line_number(source, "// breakpoint 3") + self.set_source_breakpoints(source, [bp_line]) + self.continue_to_next_stop() + + frames = self.get_stackFrames() + self.assertGreaterEqual(len(frames), 2, "Need at least 2 frames") + + all_refs = set() + + 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: + self.assertNotIn( + ref, all_refs, f"Variable reference {ref} was reused!" + ) + all_refs.add(ref) + + 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}") diff --git a/lldb/tools/lldb-dap/Handler/ScopesRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/ScopesRequestHandler.cpp index aaad0e20f9c21..251711ca62406 100644 --- a/lldb/tools/lldb-dap/Handler/ScopesRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/ScopesRequestHandler.cpp @@ -8,51 +8,11 @@ #include "DAP.h" #include "RequestHandler.h" +#include "Variables.h" 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 llvm::StringRef name, 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 - // "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. - if (variablesReference == VARREF_LOCALS) - scope.presentationHint = Scope::eScopePresentationHintLocals; - else if (variablesReference == VARREF_REGS) - scope.presentationHint = Scope::eScopePresentationHintRegisters; - - 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); @@ -75,22 +35,9 @@ 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)}; + std::vector<protocol::Scope> scopes = + dap.variables.CreateScopes(args.frameId, frame); return ScopesResponseBody{std::move(scopes)}; } diff --git a/lldb/tools/lldb-dap/Handler/VariablesRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/VariablesRequestHandler.cpp index 0177df8b754ab..ee7cb525d9c29 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; @@ -30,20 +31,22 @@ VariablesRequestHandler::Run(const VariablesArguments &arguments) const { std::vector<Variable> variables; - if (lldb::SBValueList *top_scope = dap.variables.GetTopLevelScope(var_ref)) { + std::optional<ScopeData> scope_data = dap.variables.GetTopLevelScope(var_ref); + if (scope_data) { // variablesReference is one of our scopes, not an actual variable it is // asking for the list of args, locals or globals. int64_t start_idx = 0; int64_t num_children = 0; - if (var_ref == VARREF_REGS) { + if (scope_data->kind == eScopeKindRegisters) { + // 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); @@ -55,15 +58,15 @@ VariablesRequestHandler::Run(const VariablesArguments &arguments) const { } } - num_children = top_scope->GetSize(); - if (num_children == 0 && var_ref == VARREF_LOCALS) { + num_children = scope_data->scope.GetSize(); + if (num_children == 0 && scope_data->kind == eScopeKindLocals) { // 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. // "error" owns the error string so we must keep it alive as long as we // want to use the returns "const char *" - lldb::SBError error = top_scope->GetError(); + lldb::SBError error = scope_data->scope.GetError(); const char *var_err = error.GetCString(); if (var_err) { // Create a fake variable named "error" to explain why variables were @@ -85,14 +88,14 @@ VariablesRequestHandler::Run(const VariablesArguments &arguments) const { // We first find out which variable names are duplicated std::map<llvm::StringRef, int> variable_name_counts; for (auto i = start_idx; i < end_idx; ++i) { - lldb::SBValue variable = top_scope->GetValueAtIndex(i); + lldb::SBValue variable = scope_data->scope.GetValueAtIndex(i); if (!variable.IsValid()) break; variable_name_counts[GetNonNullVariableName(variable)]++; } // Show return value if there is any ( in the local top frame ) - if (var_ref == VARREF_LOCALS) { + if (scope_data && scope_data->kind == eScopeKindLocals) { auto process = dap.target.GetProcess(); auto selected_thread = process.GetSelectedThread(); lldb::SBValue stop_return_value = selected_thread.GetStopReturnValue(); @@ -116,7 +119,7 @@ VariablesRequestHandler::Run(const VariablesArguments &arguments) const { // Now we construct the result with unique display variable names for (auto i = start_idx; i < end_idx; ++i) { - lldb::SBValue variable = top_scope->GetValueAtIndex(i); + lldb::SBValue variable = scope_data->scope.GetValueAtIndex(i); if (!variable.IsValid()) break; diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp index 8851af65d1c4f..530927a4a9ef5 100644 --- a/lldb/tools/lldb-dap/JSONUtils.cpp +++ b/lldb/tools/lldb-dap/JSONUtils.cpp @@ -299,91 +299,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, - 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) { - object.try_emplace("presentationHint", "locals"); - } else if (variablesReference == VARREF_REGS) { - 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/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 777e3183d8c0d..6d49113243799 100644 --- a/lldb/tools/lldb-dap/Variables.cpp +++ b/lldb/tools/lldb-dap/Variables.cpp @@ -8,27 +8,77 @@ #include "Variables.h" #include "JSONUtils.h" +#include "Protocol/ProtocolTypes.h" +#include "lldb/API/SBFrame.h" +#include "lldb/API/SBValue.h" +#include "lldb/API/SBValueList.h" +#include <cstdint> +#include <optional> +#include <vector> using namespace lldb_dap; -lldb::SBValueList *Variables::GetTopLevelScope(int64_t variablesReference) { - switch (variablesReference) { - case VARREF_LOCALS: - return &locals; - case VARREF_GLOBALS: - return &globals; - case VARREF_REGS: - return ®isters; - default: - return nullptr; +namespace lldb_dap { + +protocol::Scope CreateScope(const ScopeKind kind, int64_t variablesReference, + int64_t namedVariables, bool expensive) { + protocol::Scope scope; + scope.variablesReference = variablesReference; + scope.namedVariables = namedVariables; + scope.expensive = expensive; + + // TODO: Support "arguments" and "return value" scope. + // At the moment lldb-dap includes the arguments and return_value into the + // "locals" scope. + // VS Code 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 eScopeKindLocals: + scope.presentationHint = protocol::Scope::eScopePresentationHintLocals; + scope.name = "Locals"; + break; + case eScopeKindGlobals: + scope.name = "Globals"; + break; + case eScopeKindRegisters: + scope.presentationHint = protocol::Scope::eScopePresentationHintRegisters; + scope.name = "Registers"; + break; } + + return scope; +} + +std::optional<ScopeData> +Variables::GetTopLevelScope(int64_t variablesReference) { + auto scope_kind_iter = m_scope_kinds.find(variablesReference); + if (scope_kind_iter == m_scope_kinds.end()) + return std::nullopt; + + ScopeKind scope_kind = scope_kind_iter->second.first; + uint64_t dap_frame_id = scope_kind_iter->second.second; + + auto frame_iter = m_frames.find(dap_frame_id); + if (frame_iter == m_frames.end()) + return std::nullopt; + + lldb::SBValueList *scope = frame_iter->second.GetScope(scope_kind); + if (scope == nullptr) + return std::nullopt; + + ScopeData scope_data; + scope_data.kind = scope_kind; + scope_data.scope = *scope; + return scope_data; } void Variables::Clear() { - locals.Clear(); - globals.Clear(); - registers.Clear(); m_referencedvariables.clear(); + m_scope_kinds.clear(); + m_frames.clear(); + m_next_temporary_var_ref = TemporaryVariableStartIndex; } int64_t Variables::GetNewVariableReference(bool is_permanent) { @@ -66,15 +116,16 @@ int64_t Variables::InsertVariable(lldb::SBValue variable, bool is_permanent) { lldb::SBValue Variables::FindVariable(uint64_t variablesReference, llvm::StringRef name) { lldb::SBValue variable; - if (lldb::SBValueList *top_scope = GetTopLevelScope(variablesReference)) { + if (std::optional<ScopeData> scope_data = + GetTopLevelScope(variablesReference)) { bool is_duplicated_variable_name = name.contains(" @"); // variablesReference is one of our scopes, not an actual variable it is // asking for a variable in locals or globals or registers - int64_t end_idx = top_scope->GetSize(); + int64_t end_idx = scope_data->scope.GetSize(); // Searching backward so that we choose the variable in closest scope // among variables of the same name. for (int64_t i = end_idx - 1; i >= 0; --i) { - lldb::SBValue curr_variable = top_scope->GetValueAtIndex(i); + lldb::SBValue curr_variable = scope_data->scope.GetValueAtIndex(i); std::string variable_name = CreateUniqueVariableNameForDisplay( curr_variable, is_duplicated_variable_name); if (variable_name == name) { @@ -103,3 +154,52 @@ lldb::SBValue Variables::FindVariable(uint64_t variablesReference, } return variable; } + +lldb::SBValueList *Variables::GetScope(const uint64_t dap_frame_id, + const ScopeKind kind) { + + auto frame = m_frames.find(dap_frame_id); + if (frame == m_frames.end()) { + return nullptr; + } + + return frame->second.GetScope(kind); +} + +std::vector<protocol::Scope> +Variables::CreateScopes(const uint64_t dap_frame_id, lldb::SBFrame &frame) { + auto iter = m_frames.find(dap_frame_id); + if (iter == 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(); + + iter = + m_frames.emplace(dap_frame_id, FrameScopes{locals, globals, registers}) + .first; + } + + const FrameScopes &frame_scopes = iter->second; + + auto create_scope = [&](ScopeKind kind, uint32_t size) { + int64_t ref = GetNewVariableReference(false); + m_scope_kinds.try_emplace(ref, kind, dap_frame_id); + return CreateScope(kind, ref, size, false); + }; + + return { + create_scope(eScopeKindLocals, frame_scopes.locals.GetSize()), + create_scope(eScopeKindGlobals, frame_scopes.globals.GetSize()), + create_scope(eScopeKindRegisters, frame_scopes.registers.GetSize()), + }; +} + +} // namespace lldb_dap diff --git a/lldb/tools/lldb-dap/Variables.h b/lldb/tools/lldb-dap/Variables.h index 0ed84b36aef99..9814cabc7036c 100644 --- a/lldb/tools/lldb-dap/Variables.h +++ b/lldb/tools/lldb-dap/Variables.h @@ -9,22 +9,67 @@ #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" - -#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 +#include <map> +#include <optional> +#include <utility> namespace lldb_dap { -struct Variables { +enum ScopeKind : unsigned { + eScopeKindLocals, + eScopeKindGlobals, + eScopeKindRegisters +}; +/// 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; + lldb::SBValueList scope; +}; + +/// Stores the three scope variable lists for a single stack frame. +struct FrameScopes { lldb::SBValueList locals; lldb::SBValueList globals; lldb::SBValueList registers; + /// Returns a pointer to the scope corresponding to the given kind. + lldb::SBValueList *GetScope(ScopeKind kind) { + switch (kind) { + case eScopeKindLocals: + return &locals; + case eScopeKindGlobals: + return &globals; + case eScopeKindRegisters: + return ®isters; + } + + llvm_unreachable("unknown scope kind"); + } +}; + +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,21 +84,37 @@ 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 uint64_t dap_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); - lldb::SBValueList *GetTopLevelScope(int64_t variablesReference); + std::optional<ScopeData> GetTopLevelScope(int64_t variablesReference); lldb::SBValue FindVariable(uint64_t variablesReference, llvm::StringRef name); + /// Initialize a frame if it hasn't been already, otherwise do nothing + std::vector<protocol::Scope> CreateScopes(const uint64_t dap_frame_id, + lldb::SBFrame &frame); + /// Clear all scope variables and non-permanent expandable variables. void Clear(); private: - /// Variable_reference start index of permanent expandable variable. + /// Variable reference start index of temporary variables. + static constexpr int64_t TemporaryVariableStartIndex = 1; + + /// Variable reference start index of permanent expandable variable. static constexpr int64_t PermanentVariableStartIndex = (1ll << 32); + int64_t m_next_permanent_var_ref{PermanentVariableStartIndex}; + int64_t m_next_temporary_var_ref{TemporaryVariableStartIndex}; + + // Variable Reference, dap_frame_id + std::map<int64_t, std::pair<ScopeKind, uint64_t>> m_scope_kinds; + /// Variables that are alive in this stop state. /// Will be cleared when debuggee resumes. llvm::DenseMap<int64_t, lldb::SBValue> m_referencedvariables; @@ -62,8 +123,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}; - int64_t m_next_permanent_var_ref{PermanentVariableStartIndex}; + /// Key = dap_frame_id (encodes both thread index ID and frame ID) + /// Value = scopes for the frame (locals, globals, registers) + std::map<uint64_t, FrameScopes> m_frames; }; } // namespace lldb_dap diff --git a/lldb/unittests/DAP/VariablesTest.cpp b/lldb/unittests/DAP/VariablesTest.cpp index 6b14fc6c3945d..c7ffe42202090 100644 --- a/lldb/unittests/DAP/VariablesTest.cpp +++ b/lldb/unittests/DAP/VariablesTest.cpp @@ -7,6 +7,8 @@ //===----------------------------------------------------------------------===// #include "Variables.h" +#include "Protocol/ProtocolTypes.h" +#include "lldb/API/SBFrame.h" #include "lldb/API/SBValue.h" #include "lldb/API/SBValueList.h" #include "gtest/gtest.h" @@ -17,6 +19,15 @@ class VariablesTest : public ::testing::Test { protected: enum : bool { Permanent = true, Temporary = false }; Variables vars; + + static const protocol::Scope * + FindScope(const std::vector<protocol::Scope> &scopes, llvm::StringRef name) { + for (const auto &scope : scopes) { + if (scope.name == name) + return &scope; + } + return nullptr; + } }; TEST_F(VariablesTest, GetNewVariableReference_UniqueAndRanges) { @@ -66,20 +77,46 @@ TEST_F(VariablesTest, Clear_RemovesTemporaryKeepsPermanent) { } TEST_F(VariablesTest, GetTopLevelScope_ReturnsCorrectScope) { - 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); - EXPECT_EQ(vars.GetTopLevelScope(9999), nullptr); + lldb::SBFrame frame; + uint32_t frame_id = 0; + + std::vector<protocol::Scope> scopes = vars.CreateScopes(frame_id, frame); + + const protocol::Scope *locals_scope = FindScope(scopes, "Locals"); + const protocol::Scope *globals_scope = FindScope(scopes, "Globals"); + const protocol::Scope *registers_scope = FindScope(scopes, "Registers"); + + ASSERT_NE(locals_scope, nullptr); + ASSERT_NE(globals_scope, nullptr); + ASSERT_NE(registers_scope, nullptr); + + auto locals_data = vars.GetTopLevelScope(locals_scope->variablesReference); + auto globals_data = vars.GetTopLevelScope(globals_scope->variablesReference); + auto registers_data = + vars.GetTopLevelScope(registers_scope->variablesReference); + + ASSERT_TRUE(locals_data.has_value()); + ASSERT_TRUE(globals_data.has_value()); + ASSERT_TRUE(registers_data.has_value()); + + EXPECT_EQ(locals_data->kind, eScopeKindLocals); + EXPECT_EQ(globals_data->kind, eScopeKindGlobals); + EXPECT_EQ(registers_data->kind, eScopeKindRegisters); + + EXPECT_FALSE(vars.GetTopLevelScope(9999).has_value()); } TEST_F(VariablesTest, FindVariable_LocalsByName) { + lldb::SBFrame frame; + uint32_t frame_id = 0; + + std::vector<protocol::Scope> scopes = vars.CreateScopes(frame_id, frame); + + const protocol::Scope *locals_scope = FindScope(scopes, "Locals"); + ASSERT_NE(locals_scope, nullptr); + + lldb::SBValue found = vars.FindVariable(locals_scope->variablesReference, ""); lldb::SBValue dummy; - vars.locals.Append(dummy); - lldb::SBValue found = vars.FindVariable(VARREF_LOCALS, ""); EXPECT_EQ(found.IsValid(), dummy.IsValid()); } _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
