https://github.com/da-viper updated https://github.com/llvm/llvm-project/pull/179262
>From 33a821727b92c3b9fd855bb597e8f4c07053c268 Mon Sep 17 00:00:00 2001 From: Ebuka Ezike <[email protected]> Date: Mon, 2 Feb 2026 15:33:16 +0000 Subject: [PATCH 1/5] [lldb-dap] Refactor variablesReference storage and scope management. This commit refactors the Variables class into a VariableReferenceStorage with variableReferences of different ReferenceKinds. The variablesReference is now a uint32_t. The most significant byte (bits 24 - 31) holds the reference kind and the remaining 3 bytes (bits 0 -23) holds the actual reference. We have (at the moment) 3 reference kinds. Temporary => 0b0000 => 0x00 Permanent => 0b0001 => 0x01 Scope => 0b0010 => 0x03 The actual variablesReference can be used to get a `VariableStore`. VariableStore holds variables in a group. It has two implementations: - ScopeStore: Holds variables within frame scopes (locals, globals, registers). This is lazy-loaded and only fetched when variable(s) in the scope is requested. - ExpandableValueStore: Holds SBValue and fetches it's variable children when requested. ReferenceKindPool: The variablesReference created starts from 1 with the mask of the Reference kind applied. It holds vector of VariableStore of one Referencekind, This allows constant lookup of a reference Example: ```md | maskedVariablesReference | Mask | variablesReference | RefrenceKind | VariableStore | |--------------------------|------|--------------------|--------------|---------------| | 20 -> 0x00000014 | 0x00 | 20 -> 0x00000014 | temporary | ValueStore | | 268435476 -> 0x01000014 | 0x01 | 20 -> 0x00000014 | permanent | ValueStore | | 536870932 -> 0x01000014 | 0x02 | 20 -> 0x00000014 | scope | ScopeStore | ``` --- lldb/tools/lldb-dap/DAP.h | 4 +- .../DataBreakpointInfoRequestHandler.cpp | 2 +- .../Handler/EvaluateRequestHandler.cpp | 4 +- .../Handler/LocationsRequestHandler.cpp | 2 +- .../lldb-dap/Handler/ScopesRequestHandler.cpp | 2 +- .../Handler/SetVariableRequestHandler.cpp | 7 +- .../Handler/VariablesRequestHandler.cpp | 150 +------ .../lldb-dap/Protocol/ProtocolRequests.h | 10 +- lldb/tools/lldb-dap/Protocol/ProtocolTypes.h | 6 +- lldb/tools/lldb-dap/SBAPIExtras.h | 5 + lldb/tools/lldb-dap/Variables.cpp | 404 +++++++++++++----- lldb/tools/lldb-dap/Variables.h | 238 ++++++++--- lldb/unittests/DAP/VariablesTest.cpp | 62 +-- 13 files changed, 532 insertions(+), 364 deletions(-) diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index 657105c8dd6b9..9c4bdd589b74f 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -102,7 +102,7 @@ struct DAP final : public DAPTransport::MessageHandler { /// The target instance for this DAP session. lldb::SBTarget target; - Variables variables; + VariableReferenceStorage reference_storage; lldb::SBBroadcaster broadcaster; FunctionBreakpointMap function_breakpoints; InstructionBreakpointMap instruction_breakpoints; @@ -377,7 +377,7 @@ struct DAP final : public DAPTransport::MessageHandler { protocol::Capabilities GetCustomCapabilities(); /// Debuggee will continue from stopped state. - void WillContinue() { variables.Clear(); } + void WillContinue() { reference_storage.Clear(); } /// Poll the process to wait for it to reach the eStateStopped state. /// diff --git a/lldb/tools/lldb-dap/Handler/DataBreakpointInfoRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/DataBreakpointInfoRequestHandler.cpp index 245d92c18e59e..1061c3fa12848 100644 --- a/lldb/tools/lldb-dap/Handler/DataBreakpointInfoRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/DataBreakpointInfoRequestHandler.cpp @@ -41,7 +41,7 @@ llvm::Expected<protocol::DataBreakpointInfoResponseBody> DataBreakpointInfoRequestHandler::Run( const protocol::DataBreakpointInfoArguments &args) const { protocol::DataBreakpointInfoResponseBody response; - lldb::SBValue variable = dap.variables.FindVariable( + lldb::SBValue variable = dap.reference_storage.FindVariable( args.variablesReference.value_or(0), args.name); std::string addr, size; diff --git a/lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp index 0eb5c57732350..a69618b22178c 100644 --- a/lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp @@ -133,8 +133,8 @@ EvaluateRequestHandler::Run(const EvaluateArguments &arguments) const { body.type = desc.display_type_name; if (value.MightHaveChildren() || ValuePointsToCode(value)) - body.variablesReference = - dap.variables.InsertVariable(value, /*is_permanent=*/is_repl_context); + body.variablesReference = dap.reference_storage.InsertVariable( + value, /*is_permanent=*/is_repl_context); if (lldb::addr_t addr = value.GetLoadAddress(); addr != LLDB_INVALID_ADDRESS) body.memoryReference = EncodeMemoryReference(addr); diff --git a/lldb/tools/lldb-dap/Handler/LocationsRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/LocationsRequestHandler.cpp index 923c8b1aefa34..3b8599b59d1ac 100644 --- a/lldb/tools/lldb-dap/Handler/LocationsRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/LocationsRequestHandler.cpp @@ -27,7 +27,7 @@ LocationsRequestHandler::Run(const protocol::LocationsArguments &args) const { // We use the lowest bit to distinguish between value location and declaration // location auto [var_ref, is_value_location] = UnpackLocation(args.locationReference); - lldb::SBValue variable = dap.variables.GetVariable(var_ref); + lldb::SBValue variable = dap.reference_storage.GetVariable(var_ref); if (!variable.IsValid()) return llvm::make_error<DAPError>("Invalid variable reference"); diff --git a/lldb/tools/lldb-dap/Handler/ScopesRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/ScopesRequestHandler.cpp index 251711ca62406..3786d583e0e87 100644 --- a/lldb/tools/lldb-dap/Handler/ScopesRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/ScopesRequestHandler.cpp @@ -37,7 +37,7 @@ ScopesRequestHandler::Run(const ScopesArguments &args) const { } std::vector<protocol::Scope> scopes = - dap.variables.CreateScopes(args.frameId, frame); + dap.reference_storage.CreateScopes(frame); return ScopesResponseBody{std::move(scopes)}; } diff --git a/lldb/tools/lldb-dap/Handler/SetVariableRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/SetVariableRequestHandler.cpp index b9ae28d6772ac..293594dc2c90a 100644 --- a/lldb/tools/lldb-dap/Handler/SetVariableRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/SetVariableRequestHandler.cpp @@ -10,6 +10,7 @@ #include "EventHelper.h" #include "JSONUtils.h" #include "Protocol/ProtocolEvents.h" +#include "Protocol/ProtocolTypes.h" #include "RequestHandler.h" using namespace lldb_dap::protocol; @@ -27,7 +28,7 @@ llvm::Expected<SetVariableResponseBody> SetVariableRequestHandler::Run(const SetVariableArguments &args) const { const auto args_name = llvm::StringRef(args.name); - if (args.variablesReference == UINT64_MAX) { + if (args.variablesReference == LLDB_DAP_INVALID_VAR_REF) { return llvm::make_error<DAPError>( llvm::formatv("invalid reference {}", args.variablesReference).str(), llvm::inconvertibleErrorCode(), @@ -40,7 +41,7 @@ SetVariableRequestHandler::Run(const SetVariableArguments &args) const { "cannot change the value of the return value"); lldb::SBValue variable = - dap.variables.FindVariable(args.variablesReference, args_name); + dap.reference_storage.FindVariable(args.variablesReference, args_name); if (!variable.IsValid()) return llvm::make_error<DAPError>("could not find variable in scope"); @@ -62,7 +63,7 @@ SetVariableRequestHandler::Run(const SetVariableArguments &args) const { // is_permanent is false because debug console does not support // setVariable request. const int64_t new_var_ref = - dap.variables.InsertVariable(variable, /*is_permanent=*/false); + dap.reference_storage.InsertVariable(variable, /*is_permanent=*/false); if (variable.MightHaveChildren()) { body.variablesReference = new_var_ref; if (desc.type_obj.IsArrayType()) diff --git a/lldb/tools/lldb-dap/Handler/VariablesRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/VariablesRequestHandler.cpp index ee7cb525d9c29..cf7697500d3de 100644 --- a/lldb/tools/lldb-dap/Handler/VariablesRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/VariablesRequestHandler.cpp @@ -9,8 +9,6 @@ #include "DAP.h" #include "EventHelper.h" #include "Handler/RequestHandler.h" -#include "JSONUtils.h" -#include "ProtocolUtils.h" #include "Variables.h" using namespace llvm; @@ -24,150 +22,14 @@ namespace lldb_dap { /// indexed children. Expected<VariablesResponseBody> VariablesRequestHandler::Run(const VariablesArguments &arguments) const { - const uint64_t var_ref = arguments.variablesReference; - const uint64_t count = arguments.count; - const uint64_t start = arguments.start; - const bool hex = arguments.format ? arguments.format->hex : false; + const uint32_t var_ref = arguments.variablesReference; - std::vector<Variable> variables; + VariableStore *store = dap.reference_storage.GetVariableStore(var_ref); + if (!store) + return VariablesResponseBody{}; - 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 (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 = 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); - const lldb::Format format = reg.GetFormat(); - if (format == lldb::eFormatDefault || format == lldb::eFormatHex) { - if (reg.GetByteSize() == addr_size) - reg.SetFormat(lldb::eFormatAddressInfo); - } - } - } - - 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 = scope_data->scope.GetError(); - const char *var_err = error.GetCString(); - if (var_err) { - // Create a fake variable named "error" to explain why variables were - // not available. This new error will help let users know when there was - // a problem that kept variables from being available for display and - // allow users to fix this issue instead of seeing no variables. The - // errors are only set when there is a problem that the user could - // fix, so no error will show up when you have no debug info, only when - // we do have debug info and something that is fixable can be done. - Variable var; - var.name = "<error>"; - var.type = "const char *"; - var.value = var_err; - variables.emplace_back(var); - } - } - const int64_t end_idx = start_idx + ((count == 0) ? num_children : count); - - // 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 = 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 (scope_data && scope_data->kind == eScopeKindLocals) { - auto process = dap.target.GetProcess(); - auto selected_thread = process.GetSelectedThread(); - lldb::SBValue stop_return_value = selected_thread.GetStopReturnValue(); - - if (stop_return_value.IsValid() && - (selected_thread.GetSelectedFrame().GetFrameID() == 0)) { - auto renamed_return_value = stop_return_value.Clone("(Return Value)"); - int64_t return_var_ref = 0; - - if (stop_return_value.MightHaveChildren() || - stop_return_value.IsSynthetic()) { - return_var_ref = dap.variables.InsertVariable(stop_return_value, - /*is_permanent=*/false); - } - variables.emplace_back(CreateVariable( - renamed_return_value, return_var_ref, hex, - dap.configuration.enableAutoVariableSummaries, - dap.configuration.enableSyntheticChildDebugging, false)); - } - } - - // Now we construct the result with unique display variable names - for (auto i = start_idx; i < end_idx; ++i) { - lldb::SBValue variable = scope_data->scope.GetValueAtIndex(i); - - if (!variable.IsValid()) - break; - - const int64_t frame_var_ref = - dap.variables.InsertVariable(variable, /*is_permanent=*/false); - variables.emplace_back(CreateVariable( - variable, frame_var_ref, hex, - dap.configuration.enableAutoVariableSummaries, - dap.configuration.enableSyntheticChildDebugging, - variable_name_counts[GetNonNullVariableName(variable)] > 1)); - } - } else { - // We are expanding a variable that has children, so we will return its - // children. - lldb::SBValue variable = dap.variables.GetVariable(var_ref); - if (variable.IsValid()) { - const bool is_permanent = - dap.variables.IsPermanentVariableReference(var_ref); - auto addChild = [&](lldb::SBValue child, - std::optional<llvm::StringRef> custom_name = {}) { - if (!child.IsValid()) - return; - const int64_t child_var_ref = - dap.variables.InsertVariable(child, is_permanent); - variables.emplace_back( - CreateVariable(child, child_var_ref, hex, - dap.configuration.enableAutoVariableSummaries, - dap.configuration.enableSyntheticChildDebugging, - /*is_name_duplicated=*/false, custom_name)); - }; - const int64_t num_children = variable.GetNumChildren(); - const int64_t end_idx = start + ((count == 0) ? num_children : count); - int64_t i = start; - for (; i < end_idx && i < num_children; ++i) - addChild(variable.GetChildAtIndex(i)); - - // If we haven't filled the count quota from the request, we insert a new - // "[raw]" child that can be used to inspect the raw version of a - // synthetic member. That eliminates the need for the user to go to the - // debug console and type `frame var <variable> to get these values. - if (dap.configuration.enableSyntheticChildDebugging && - variable.IsSynthetic() && i == num_children) - addChild(variable.GetNonSyntheticValue(), "[raw]"); - } - } - - return VariablesResponseBody{std::move(variables)}; + return VariablesResponseBody{ + store->GetVariables(dap.reference_storage, dap.configuration, arguments)}; } } // namespace lldb_dap diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h index 28c9f48200e0c..312a9dd8714a0 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h +++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h @@ -435,7 +435,7 @@ struct SetVariableArguments { /// The reference of the variable container. The `variablesReference` must /// have been obtained in the current suspended state. See 'Lifetime of Object /// References' in the Overview section for details. - uint64_t variablesReference = UINT64_MAX; + uint32_t variablesReference = LLDB_DAP_INVALID_VAR_REF; /// The name of the variable in the container. std::string name; @@ -466,7 +466,7 @@ struct SetVariableResponseBody { /// If this property is included in the response, any `variablesReference` /// previously associated with the updated variable, and those of its /// children, are no longer valid. - uint64_t variablesReference = 0; + uint32_t variablesReference = 0; /// The number of named child variables. /// The client can use this information to present the variables in a paged @@ -727,7 +727,7 @@ struct DataBreakpointInfoArguments { /// for a child of the container. The `variablesReference` must have been /// obtained in the current suspended state.See 'Lifetime of Object /// References' in the Overview section for details. - std::optional<int64_t> variablesReference; + std::optional<uint32_t> variablesReference; /// The name of the variable's child to obtain data breakpoint information /// for. If `variablesReference` isn't specified, this can be an expression, @@ -953,7 +953,7 @@ struct VariablesArguments { /// The variable for which to retrieve its children. The `variablesReference` /// must have been obtained in the current suspended state. See 'Lifetime of /// Object References' in the Overview section for details. - uint64_t variablesReference; + uint32_t variablesReference = LLDB_DAP_INVALID_VAR_REF; enum VariablesFilter : unsigned { eVariablesFilterBoth = 0, @@ -1158,7 +1158,7 @@ struct EvaluateResponseBody { /// children can be retrieved by passing `variablesReference` to the /// `variables` request as long as execution remains suspended. See 'Lifetime /// of Object References' in the Overview section for details. - int64_t variablesReference = 0; + uint32_t variablesReference = 0; /// The number of named child variables. /// The client can use this information to present the variables in a paged diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h index 71046d24c9787..0a9d1ebaccd8d 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h +++ b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h @@ -29,7 +29,7 @@ #include <optional> #include <string> -#define LLDB_DAP_INVALID_VAR_REF INT64_MAX +#define LLDB_DAP_INVALID_VAR_REF UINT32_MAX #define LLDB_DAP_INVALID_SRC_REF 0 #define LLDB_DAP_INVALID_VALUE_LOC 0 #define LLDB_DAP_INVALID_STACK_FRAME_ID UINT64_MAX @@ -464,7 +464,7 @@ struct Scope { /// remains suspended. See 'Lifetime of Object References' in the Overview /// section for details. //// - uint64_t variablesReference = LLDB_DAP_INVALID_VAR_REF; + uint32_t variablesReference = LLDB_DAP_INVALID_VAR_REF; /// The number of named variables in this scope. /// The client can use this information to present the variables in a paged UI @@ -963,7 +963,7 @@ struct Variable { /// children can be retrieved by passing `variablesReference` to the /// `variables` request as long as execution remains suspended. See 'Lifetime /// of Object References' in the Overview section for details. - uint64_t variablesReference = 0; + uint32_t variablesReference = 0; /// The number of named child variables. /// diff --git a/lldb/tools/lldb-dap/SBAPIExtras.h b/lldb/tools/lldb-dap/SBAPIExtras.h index eb59cb08ea4fd..33599e1cddd1d 100644 --- a/lldb/tools/lldb-dap/SBAPIExtras.h +++ b/lldb/tools/lldb-dap/SBAPIExtras.h @@ -55,6 +55,11 @@ using frame_iter = inline frame_iter begin(SBThread T) { return {T, 0}; } inline frame_iter end(SBThread T) { return {T, T.GetNumFrames()}; } +/// SBValue value iterator +using value_iter = iter<SBValue, SBValue, uint32_t, &SBValue::GetChildAtIndex>; +inline value_iter begin(SBValue &T) { return {T, 0}; } +inline value_iter end(SBValue &T) { return {T, T.GetNumChildren()}; } + // llvm::raw_ostream print helpers. inline llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, SBStream &stream) { diff --git a/lldb/tools/lldb-dap/Variables.cpp b/lldb/tools/lldb-dap/Variables.cpp index 6d49113243799..98dc0417e5242 100644 --- a/lldb/tools/lldb-dap/Variables.cpp +++ b/lldb/tools/lldb-dap/Variables.cpp @@ -8,20 +8,27 @@ #include "Variables.h" #include "JSONUtils.h" +#include "Protocol/ProtocolRequests.h" #include "Protocol/ProtocolTypes.h" +#include "ProtocolUtils.h" +#include "SBAPIExtras.h" #include "lldb/API/SBFrame.h" +#include "lldb/API/SBProcess.h" #include "lldb/API/SBValue.h" #include "lldb/API/SBValueList.h" +#include "llvm/ADT/Sequence.h" +#include "llvm/Support/ErrorHandling.h" #include <cstdint> #include <optional> #include <vector> using namespace lldb_dap; +using namespace lldb_dap::protocol; namespace lldb_dap { -protocol::Scope CreateScope(const ScopeKind kind, int64_t variablesReference, - int64_t namedVariables, bool expensive) { +protocol::Scope CreateScope(ScopeKind kind, uint32_t variablesReference, + uint32_t namedVariables, bool expensive) { protocol::Scope scope; scope.variablesReference = variablesReference; scope.namedVariables = namedVariables; @@ -51,155 +58,318 @@ protocol::Scope CreateScope(const ScopeKind kind, int64_t variablesReference, 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; +std::vector<Variable> +ScopeStore::GetVariables(VariableReferenceStorage &storage, + const Configuration &config, + const VariablesArguments &args) { + LoadVariables(); + if (m_kind == lldb_dap::eScopeKindRegisters) + SetRegistersFormat(); - ScopeKind scope_kind = scope_kind_iter->second.first; - uint64_t dap_frame_id = scope_kind_iter->second.second; + const bool format_hex = args.format ? args.format->hex : false; + std::vector<Variable> variables; + if (m_kind == eScopeKindLocals) + AddReturnValue(storage, config, variables, format_hex); - auto frame_iter = m_frames.find(dap_frame_id); - if (frame_iter == m_frames.end()) - return std::nullopt; + const uint64_t count = args.count; + const uint32_t start_idx = 0; + const uint32_t num_children = m_children.GetSize(); + const uint32_t end_idx = start_idx + ((count == 0) ? num_children : count); - lldb::SBValueList *scope = frame_iter->second.GetScope(scope_kind); - if (scope == nullptr) - return std::nullopt; + // We first find out which variable names are duplicated + std::map<llvm::StringRef, uint32_t> variable_name_counts; + for (auto i = start_idx; i < end_idx; ++i) { + lldb::SBValue variable = m_children.GetValueAtIndex(i); + if (!variable.IsValid()) + break; + variable_name_counts[GetNonNullVariableName(variable)]++; + } + + // Now we construct the result with unique display variable names + for (auto i = start_idx; i < end_idx; ++i) { + lldb::SBValue variable = m_children.GetValueAtIndex(i); + + if (!variable.IsValid()) + break; - ScopeData scope_data; - scope_data.kind = scope_kind; - scope_data.scope = *scope; - return scope_data; + const uint32_t frame_var_ref = + storage.InsertVariable(variable, /*is_permanent=*/false); + variables.emplace_back(CreateVariable( + variable, frame_var_ref, format_hex, config.enableAutoVariableSummaries, + config.enableSyntheticChildDebugging, + variable_name_counts[GetNonNullVariableName(variable)] > 1)); + } + return variables; } -void Variables::Clear() { - m_referencedvariables.clear(); - m_scope_kinds.clear(); - m_frames.clear(); - m_next_temporary_var_ref = TemporaryVariableStartIndex; +lldb::SBValue ScopeStore::FindVariable(llvm::StringRef name) { + LoadVariables(); + + lldb::SBValue variable; + const bool is_name_duplicated = name.contains(" @"); + // variablesReference is one of our scopes, not an actual variable it is + // asking for a variable in locals or globals or registers + const uint32_t end_idx = m_children.GetSize(); + // Searching backward so that we choose the variable in closest scope + // among variables of the same name. + for (const uint32_t i : llvm::reverse(llvm::seq<uint32_t>(0, end_idx))) { + lldb::SBValue curr_variable = m_children.GetValueAtIndex(i); + std::string variable_name = + CreateUniqueVariableNameForDisplay(curr_variable, is_name_duplicated); + if (variable_name == name) { + variable = curr_variable; + break; + } + } + return variable; } -int64_t Variables::GetNewVariableReference(bool is_permanent) { - if (is_permanent) - return m_next_permanent_var_ref++; - return m_next_temporary_var_ref++; +void ScopeStore::LoadVariables() { + if (m_variables_loaded) + return; + + m_variables_loaded = true; + switch (m_kind) { + case eScopeKindLocals: + m_children = m_frame.GetVariables(/*arguments=*/true, + /*locals=*/true, + /*statics=*/false, + /*in_scope_only=*/true); + break; + case eScopeKindGlobals: + m_children = m_frame.GetVariables(/*arguments=*/false, + /*locals=*/false, + /*statics=*/true, + /*in_scope_only=*/true); + break; + case eScopeKindRegisters: + m_children = m_frame.GetRegisters(); + } } -bool Variables::IsPermanentVariableReference(int64_t var_ref) { - return var_ref >= PermanentVariableStartIndex; +void ScopeStore::SetRegistersFormat() { + // 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 = + m_frame.GetThread().GetProcess().GetAddressByteSize(); + for (lldb::SBValue reg : m_children.GetValueAtIndex(0)) { + const lldb::Format format = reg.GetFormat(); + if (format == lldb::eFormatDefault || format == lldb::eFormatHex) { + if (reg.GetByteSize() == addr_size) + reg.SetFormat(lldb::eFormatAddressInfo); + } + } } -lldb::SBValue Variables::GetVariable(int64_t var_ref) const { - if (IsPermanentVariableReference(var_ref)) { - auto pos = m_referencedpermanent_variables.find(var_ref); - if (pos != m_referencedpermanent_variables.end()) - return pos->second; - } else { - auto pos = m_referencedvariables.find(var_ref); - if (pos != m_referencedvariables.end()) - return pos->second; +void ScopeStore::AddReturnValue(VariableReferenceStorage &storage, + const Configuration &config, + std::vector<Variable> &variables, + bool format_hex) { + assert(m_kind == eScopeKindLocals && + "Return Value Should only be in local scope"); + if (m_children.GetSize() == 0) { + // 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 = m_children.GetError(); + if (const char *var_err = error.GetCString()) { + // Create a fake variable named "error" to explain why variables were + // not available. This new error will help let users know when there was + // a problem that kept variables from being available for display and + // allow users to fix this issue instead of seeing no variables. The + // errors are only set when there is a problem that the user could + // fix, so no error will show up when you have no debug info, only when + // we do have debug info and something that is fixable can be done. + Variable err_var; + err_var.name = "<error>"; + err_var.type = "const char *"; + err_var.value = var_err; + variables.push_back(std::move(err_var)); + } + return; + } + + // Show return value if there is any ( in the local top frame ) + auto selected_thread = m_frame.GetThread(); + lldb::SBValue stop_return_value = selected_thread.GetStopReturnValue(); + + if (stop_return_value.IsValid() && + (selected_thread.GetSelectedFrame().GetFrameID() == 0)) { + auto renamed_return_value = stop_return_value.Clone("(Return Value)"); + uint32_t return_var_ref = 0; + + if (stop_return_value.MightHaveChildren() || + stop_return_value.IsSynthetic()) { + return_var_ref = storage.InsertVariable(stop_return_value, + /*is_permanent=*/false); + } + variables.emplace_back( + CreateVariable(renamed_return_value, return_var_ref, format_hex, + config.enableAutoVariableSummaries, + config.enableSyntheticChildDebugging, false)); } - return lldb::SBValue(); } -int64_t Variables::InsertVariable(lldb::SBValue variable, bool is_permanent) { - int64_t var_ref = GetNewVariableReference(is_permanent); - if (is_permanent) - m_referencedpermanent_variables.insert(std::make_pair(var_ref, variable)); - else - m_referencedvariables.insert(std::make_pair(var_ref, variable)); - return var_ref; +std::vector<Variable> +ExpandableValueStore::GetVariables(VariableReferenceStorage &storage, + const Configuration &config, + const VariablesArguments &args) { + const uint32_t var_ref = args.variablesReference; + const uint64_t count = args.count; + const uint64_t start = args.start; + const bool hex = args.format ? args.format->hex : false; + + lldb::SBValue variable = storage.GetVariable(var_ref); + if (!variable) + return {}; + + // We are expanding a variable that has children, so we will return its + // children. + std::vector<Variable> variables; + const bool is_permanent = + VariableReferenceStorage::IsPermanentVariableReference(var_ref); + auto addChild = [&](lldb::SBValue child, + std::optional<llvm::StringRef> custom_name = {}) { + if (!child.IsValid()) + return; + const uint32_t child_var_ref = storage.InsertVariable(child, is_permanent); + variables.emplace_back(CreateVariable( + child, child_var_ref, hex, config.enableAutoVariableSummaries, + config.enableSyntheticChildDebugging, + /*is_name_duplicated=*/false, custom_name)); + }; + const uint32_t num_children = variable.GetNumChildren(); + const uint32_t end_idx = start + ((count == 0) ? num_children : count); + uint32_t i = start; + for (; i < end_idx && i < num_children; ++i) + addChild(variable.GetChildAtIndex(i)); + + // If we haven't filled the count quota from the request, we insert a new + // "[raw]" child that can be used to inspect the raw version of a + // synthetic member. That eliminates the need for the user to go to the + // debug console and type `frame var <variable> to get these values. + if (config.enableSyntheticChildDebugging && variable.IsSynthetic() && + i == num_children) + addChild(variable.GetNonSyntheticValue(), "[raw]"); + + return variables; } -lldb::SBValue Variables::FindVariable(uint64_t variablesReference, - llvm::StringRef name) { - lldb::SBValue variable; - 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 = 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 = scope_data->scope.GetValueAtIndex(i); - std::string variable_name = CreateUniqueVariableNameForDisplay( - curr_variable, is_duplicated_variable_name); - if (variable_name == name) { - variable = curr_variable; - break; - } - } - } else { - // This is not under the globals or locals scope, so there are no - // duplicated names. - - // We have a named item within an actual variable so we need to find it - // withing the container variable by name. - lldb::SBValue container = GetVariable(variablesReference); - variable = container.GetChildMemberWithName(name.data()); - if (!variable.IsValid()) { - if (name.starts_with("[")) { - llvm::StringRef index_str(name.drop_front(1)); - uint64_t index = 0; - if (!index_str.consumeInteger(0, index)) { - if (index_str == "]") - variable = container.GetChildAtIndex(index); - } +lldb::SBValue ExpandableValueStore::FindVariable(llvm::StringRef name) { + lldb::SBValue variable = m_value.GetChildMemberWithName(name.data()); + if (!variable.IsValid()) { + if (name.starts_with('[') && name.ends_with(']')) { + llvm::StringRef index_str(name.drop_front().drop_back()); + uint64_t index = 0; + if (!index_str.consumeInteger(0, index)) { + variable = m_value.GetChildAtIndex(index); } } } + return variable; } -lldb::SBValueList *Variables::GetScope(const uint64_t dap_frame_id, - const ScopeKind kind) { +bool VariableReferenceStorage::IsPermanentVariableReference(uint32_t var_ref) { + return GetReferenceKind(var_ref) == eReferenceKindPermanent; +} - auto frame = m_frames.find(dap_frame_id); - if (frame == m_frames.end()) { - return nullptr; - } +uint32_t VariableReferenceStorage::CreateVariableReference(bool is_permanent) { + if (is_permanent) + return m_permanent_kind_pool.NextReference(); - return frame->second.GetScope(kind); + return m_temporary_kind_pool.NextReference(); } -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); +lldb::SBValue VariableReferenceStorage::GetVariable(uint32_t masked_var_ref) { + const ReferenceKind kind = GetReferenceKind(masked_var_ref); - auto registers = frame.GetRegisters(); + if (kind == eReferenceKindTemporary) { + if (auto *store = m_temporary_kind_pool.GetVariableStore(masked_var_ref)) + return store->GetVariable(); + } - iter = - m_frames.emplace(dap_frame_id, FrameScopes{locals, globals, registers}) - .first; + if (kind == eReferenceKindPermanent) { + if (auto *store = m_permanent_kind_pool.GetVariableStore(masked_var_ref)) + return store->GetVariable(); } - const FrameScopes &frame_scopes = iter->second; + return {}; +} - 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); - }; +uint32_t VariableReferenceStorage::InsertVariable(const lldb::SBValue &variable, + bool is_permanent) { + if (is_permanent) { + const uint32_t masked_var_ref = m_permanent_kind_pool.NextReference(); + const uint32_t var_ref = m_permanent_kind_pool.ClearMask(masked_var_ref); + m_permanent_kind_pool.Add(variable); + assert(var_ref == m_permanent_kind_pool.Size() && + "New variablesReference must be the size of the reference pool"); + return masked_var_ref; + } - return { - create_scope(eScopeKindLocals, frame_scopes.locals.GetSize()), - create_scope(eScopeKindGlobals, frame_scopes.globals.GetSize()), - create_scope(eScopeKindRegisters, frame_scopes.registers.GetSize()), + const uint32_t masked_var_ref = m_temporary_kind_pool.NextReference(); + const uint32_t var_ref = m_temporary_kind_pool.ClearMask(masked_var_ref); + m_temporary_kind_pool.Add(variable); + assert(var_ref == m_temporary_kind_pool.Size() && + "New variablesReference must be the size of the reference pool"); + return masked_var_ref; +} + +lldb::SBValue +VariableReferenceStorage::FindVariable(uint32_t variablesReference, + llvm::StringRef name) { + if (VariableStore *store = GetVariableStore(variablesReference)) + return store->FindVariable(name); + + return {}; +} + +std::vector<protocol::Scope> +VariableReferenceStorage::CreateScopes(lldb::SBFrame &frame) { + auto create_scope = [&](ScopeKind kind) { + const uint32_t masked_ref = m_scope_kind_pool.NextReference(); + const uint32_t raw_ref = m_scope_kind_pool.ClearMask(masked_ref); + m_scope_kind_pool.Add(kind, frame); + assert(m_scope_kind_pool.Size() == raw_ref && + "New scopes ref must be the size of the pool"); + + return CreateScope(kind, masked_ref, 0, false); }; + + return {create_scope(eScopeKindLocals), create_scope(eScopeKindGlobals), + create_scope(eScopeKindRegisters)}; } +VariableStore * +VariableReferenceStorage::GetVariableStore(uint32_t masked_var_ref) { + const ReferenceKind kind = GetReferenceKind(masked_var_ref); + switch (kind) { + case eReferenceKindPermanent: + return m_permanent_kind_pool.GetVariableStore(masked_var_ref); + case eReferenceKindTemporary: + return m_temporary_kind_pool.GetVariableStore(masked_var_ref); + case eReferenceKindScope: + return m_scope_kind_pool.GetVariableStore(masked_var_ref); + } + llvm_unreachable("Unknown reference kind."); +} + +ReferenceKind VariableReferenceStorage::GetReferenceKind(uint32_t var_ref) { + const uint8_t reference_kind_byte = var_ref >> k_reference_kind_bit_offset; + + if ((reference_kind_byte & k_scope_kind_byte) == k_scope_kind_byte) + return eReferenceKindScope; + + if ((reference_kind_byte & k_permanent_kind_byte) == k_permanent_kind_byte) + return eReferenceKindPermanent; + + return eReferenceKindTemporary; +} } // namespace lldb_dap diff --git a/lldb/tools/lldb-dap/Variables.h b/lldb/tools/lldb-dap/Variables.h index 9814cabc7036c..be47bca02c784 100644 --- a/lldb/tools/lldb-dap/Variables.h +++ b/lldb/tools/lldb-dap/Variables.h @@ -9,21 +9,28 @@ #ifndef LLDB_TOOLS_LLDB_DAP_VARIABLES_H #define LLDB_TOOLS_LLDB_DAP_VARIABLES_H +#include "Protocol/ProtocolRequests.h" #include "Protocol/ProtocolTypes.h" +#include "lldb/API/SBFrame.h" #include "lldb/API/SBValue.h" #include "lldb/API/SBValueList.h" -#include "llvm/ADT/DenseMap.h" -#include <map> -#include <optional> -#include <utility> +#include <limits> namespace lldb_dap { +struct VariableReferenceStorage; enum ScopeKind : unsigned { eScopeKindLocals, eScopeKindGlobals, eScopeKindRegisters }; + +enum ReferenceKind : uint32_t { + eReferenceKindPermanent, + eReferenceKindTemporary, + eReferenceKindScope +}; + /// Creates a `protocol::Scope` struct. /// /// \param[in] kind @@ -40,92 +47,211 @@ enum ScopeKind : unsigned { /// /// \return /// A `protocol::Scope` -protocol::Scope CreateScope(const ScopeKind kind, int64_t variablesReference, - int64_t namedVariables, bool expensive); +protocol::Scope CreateScope(ScopeKind kind, uint32_t variablesReference, + uint32_t namedVariables, bool expensive); + +/// An Interface to get or find specific variables by name. +class VariableStore { +public: + explicit VariableStore() = default; + virtual ~VariableStore() = default; + + virtual std::vector<protocol::Variable> + GetVariables(VariableReferenceStorage &storage, + const protocol::Configuration &config, + const protocol::VariablesArguments &args) = 0; + virtual lldb::SBValue FindVariable(llvm::StringRef name) = 0; -struct ScopeData { - ScopeKind kind; - lldb::SBValueList scope; + // Not copyable. + VariableStore(const VariableStore &) = delete; + VariableStore operator=(const VariableStore &) = delete; + VariableStore(VariableStore &&) = default; + VariableStore &operator=(VariableStore &&) = default; }; -/// 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; - } +/// A Variable store for fetching variables within a specific scope (locals, +/// globals, or registers) for a given stack frame. +class ScopeStore : public VariableStore { +public: + explicit ScopeStore(ScopeKind kind, const lldb::SBFrame &frame) + : m_frame(frame), m_kind(kind) {} - llvm_unreachable("unknown scope kind"); - } + std::vector<protocol::Variable> + GetVariables(VariableReferenceStorage &storage, + const protocol::Configuration &config, + const protocol::VariablesArguments &args) final; + lldb::SBValue FindVariable(llvm::StringRef name) final; + +private: + void LoadVariables(); + void SetRegistersFormat(); + void AddReturnValue(VariableReferenceStorage &storage, + const protocol::Configuration &config, + std::vector<protocol::Variable> &variables, + bool format_hex); + lldb::SBFrame m_frame; + lldb::SBValueList m_children; + ScopeKind m_kind; + bool m_variables_loaded = false; }; -struct Variables { +/// Variable store for expandable values. +/// +/// Manages children variables of complex types (structs, arrays, pointers, +/// etc.) that can be expanded in the debugger UI. +class ExpandableValueStore : public VariableStore { + +public: + explicit ExpandableValueStore(const lldb::SBValue &value) : m_value(value) {} + + std::vector<protocol::Variable> + GetVariables(VariableReferenceStorage &storage, + const protocol::Configuration &config, + const protocol::VariablesArguments &args) final; + lldb::SBValue FindVariable(llvm::StringRef name) final; + [[nodiscard]] lldb::SBValue GetVariable() const { return m_value; }; + +private: + lldb::SBValue m_value; +}; + +struct VariableReferenceStorage { /// 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); + static bool IsPermanentVariableReference(uint32_t var_ref); /// \return a new variableReference. /// Specify is_permanent as true for variable that should persist entire /// debug session. - int64_t GetNewVariableReference(bool is_permanent); + uint32_t CreateVariableReference(bool is_permanent); /// \return the expandable variable corresponding with variableReference /// value of \p value. /// 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); + lldb::SBValue GetVariable(uint32_t var_ref); /// Insert a new \p variable. /// \return variableReference assigned to this expandable variable. - int64_t InsertVariable(lldb::SBValue variable, bool is_permanent); + uint32_t InsertVariable(const lldb::SBValue &variable, bool is_permanent); + + lldb::SBValue FindVariable(uint32_t variablesReference, llvm::StringRef name); - std::optional<ScopeData> GetTopLevelScope(int64_t variablesReference); + std::vector<protocol::Scope> CreateScopes(lldb::SBFrame &frame); - lldb::SBValue FindVariable(uint64_t variablesReference, llvm::StringRef name); + void Clear() { + m_temporary_kind_pool.Clear(); + m_scope_kind_pool.Clear(); + } - /// 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); + VariableStore *GetVariableStore(uint32_t var_ref); - /// Clear all scope variables and non-permanent expandable variables. - void Clear(); + static ReferenceKind GetReferenceKind(uint32_t var_ref); private: /// Variable reference start index of temporary variables. - static constexpr int64_t TemporaryVariableStartIndex = 1; + static constexpr uint8_t k_temporary_kind_byte = 0b0000; + /// Permanent reference mask for permanent expandable variables. + static constexpr uint8_t k_permanent_kind_byte = 0b0001; + /// Scope reference mask for scope types. + static constexpr uint8_t k_scope_kind_byte = 0b0010; + static constexpr uint32_t max_reference_value = + std::numeric_limits<int32_t>::max(); + + /// The most significant byte is used for storing the reference kind. + static constexpr uint32_t k_reference_kind_bit_size = + std::numeric_limits<uint8_t>::digits; + /// The start bit offset in the variable reference used to store the reference + /// kind. + static constexpr uint32_t k_reference_kind_bit_offset = + std::numeric_limits<uint32_t>::digits - k_reference_kind_bit_size; + // we should be able to store at least 8 million variables for each store + // type at every stopped state. + static constexpr uint32_t k_min_number_of_variables = 8'000'000; + // Sanity check of the amount of variables we can store. + static_assert(((max_reference_value >> k_reference_kind_bit_size) > + k_min_number_of_variables) && + "Not enough space to store 8 million variables"); + // Ensure reference kind bytes are unique. + static_assert(k_temporary_kind_byte != k_permanent_kind_byte, + "Temporary and permanent kind bytes must be different"); + static_assert(k_temporary_kind_byte != k_scope_kind_byte, + "Temporary kind and scope bytes must be different"); + static_assert(k_permanent_kind_byte != k_scope_kind_byte, + "Permanent kind and scope bytes must be different"); + + /// Template class for managing pools of variable stores. + /// All references created starts from zero with the Reference kind mask + /// applied, the mask is then removed when fetching a variable store + /// + /// \tparam VariableStoreType + /// The type of variable store to use. + /// + /// \tparam ReferenceKindByte + /// The bitmask used to identify this pool's references + template <typename VariableStoreType, uint8_t ReferenceKindByte> + class ReferenceKindPool { + + public: + static constexpr uint32_t k_reference_mask = ReferenceKindByte + << k_reference_kind_bit_offset; + + static_assert( + ((k_reference_mask + k_min_number_of_variables) < + max_reference_value) && + "Cannot store minimum number of variables without overflowing"); + + explicit ReferenceKindPool() : m_value(k_reference_mask) {} + uint32_t NextReference() { + m_value++; + return m_value; + } + + /// Resets the produced reference and clears all scope and temporary + /// expandable kind pools. + void Clear() { + m_value = k_reference_mask; + m_pool.clear(); + } - /// Variable reference start index of permanent expandable variable. - static constexpr int64_t PermanentVariableStartIndex = (1ll << 32); + VariableStoreType *GetVariableStore(uint32_t masked_var_ref) { + const uint32_t var_ref = ClearMask(masked_var_ref); - int64_t m_next_permanent_var_ref{PermanentVariableStartIndex}; - int64_t m_next_temporary_var_ref{TemporaryVariableStartIndex}; + if (var_ref != 0 && var_ref <= m_pool.size()) + return &m_pool[var_ref - 1]; + return nullptr; + } + + uint32_t ClearMask(uint32_t masked_var_ref) { + return masked_var_ref & ~k_reference_mask; + } + + template <typename... Args> void Add(Args &&...args) { + m_pool.emplace_back(std::forward<Args>(args)...); + } - // Variable Reference, dap_frame_id - std::map<int64_t, std::pair<ScopeKind, uint64_t>> m_scope_kinds; + size_t Size() { return m_pool.size(); } + + // non copyable and movable. + ReferenceKindPool(const ReferenceKindPool &) = delete; + ReferenceKindPool &operator=(const ReferenceKindPool &) = delete; + ReferenceKindPool(ReferenceKindPool &&) = delete; + ReferenceKindPool &operator=(ReferenceKindPool &&) = delete; + ~ReferenceKindPool() = default; + + private: + uint32_t m_value = ReferenceKindByte; + std::vector<VariableStoreType> m_pool; + }; /// Variables that are alive in this stop state. /// Will be cleared when debuggee resumes. - llvm::DenseMap<int64_t, lldb::SBValue> m_referencedvariables; - + ReferenceKindPool<ExpandableValueStore, k_temporary_kind_byte> + m_temporary_kind_pool; /// Variables that persist across entire debug session. /// These are the variables evaluated from debug console REPL. - llvm::DenseMap<int64_t, lldb::SBValue> m_referencedpermanent_variables; - - /// 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; + ReferenceKindPool<ExpandableValueStore, k_permanent_kind_byte> + m_permanent_kind_pool; + ReferenceKindPool<ScopeStore, k_scope_kind_byte> m_scope_kind_pool; }; } // namespace lldb_dap diff --git a/lldb/unittests/DAP/VariablesTest.cpp b/lldb/unittests/DAP/VariablesTest.cpp index c7ffe42202090..d27e3164b6ad8 100644 --- a/lldb/unittests/DAP/VariablesTest.cpp +++ b/lldb/unittests/DAP/VariablesTest.cpp @@ -10,7 +10,6 @@ #include "Protocol/ProtocolTypes.h" #include "lldb/API/SBFrame.h" #include "lldb/API/SBValue.h" -#include "lldb/API/SBValueList.h" #include "gtest/gtest.h" using namespace lldb_dap; @@ -18,7 +17,7 @@ using namespace lldb_dap; class VariablesTest : public ::testing::Test { protected: enum : bool { Permanent = true, Temporary = false }; - Variables vars; + VariableReferenceStorage vars; static const protocol::Scope * FindScope(const std::vector<protocol::Scope> &scopes, llvm::StringRef name) { @@ -31,10 +30,10 @@ class VariablesTest : public ::testing::Test { }; TEST_F(VariablesTest, GetNewVariableReference_UniqueAndRanges) { - const int64_t temp1 = vars.GetNewVariableReference(Temporary); - const int64_t temp2 = vars.GetNewVariableReference(Temporary); - const int64_t perm1 = vars.GetNewVariableReference(Permanent); - const int64_t perm2 = vars.GetNewVariableReference(Permanent); + const int64_t temp1 = vars.CreateVariableReference(Temporary); + const int64_t temp2 = vars.CreateVariableReference(Temporary); + const int64_t perm1 = vars.CreateVariableReference(Permanent); + const int64_t perm2 = vars.CreateVariableReference(Permanent); EXPECT_NE(temp1, temp2); EXPECT_NE(perm1, perm2); @@ -59,11 +58,11 @@ TEST_F(VariablesTest, InsertAndGetVariable_Permanent) { } TEST_F(VariablesTest, IsPermanentVariableReference) { - const int64_t perm = vars.GetNewVariableReference(Permanent); - const int64_t temp = vars.GetNewVariableReference(Temporary); + const int64_t perm = vars.CreateVariableReference(Permanent); + const int64_t temp = vars.CreateVariableReference(Temporary); - EXPECT_TRUE(Variables::IsPermanentVariableReference(perm)); - EXPECT_FALSE(Variables::IsPermanentVariableReference(temp)); + EXPECT_TRUE(VariableReferenceStorage::IsPermanentVariableReference(perm)); + EXPECT_FALSE(VariableReferenceStorage::IsPermanentVariableReference(temp)); } TEST_F(VariablesTest, Clear_RemovesTemporaryKeepsPermanent) { @@ -76,11 +75,10 @@ TEST_F(VariablesTest, Clear_RemovesTemporaryKeepsPermanent) { EXPECT_EQ(vars.GetVariable(perm).IsValid(), dummy.IsValid()); } -TEST_F(VariablesTest, GetTopLevelScope_ReturnsCorrectScope) { +TEST_F(VariablesTest, VariablesStore) { lldb::SBFrame frame; - uint32_t frame_id = 0; - std::vector<protocol::Scope> scopes = vars.CreateScopes(frame_id, frame); + std::vector<protocol::Scope> scopes = vars.CreateScopes(frame); const protocol::Scope *locals_scope = FindScope(scopes, "Locals"); const protocol::Scope *globals_scope = FindScope(scopes, "Globals"); @@ -90,27 +88,33 @@ TEST_F(VariablesTest, GetTopLevelScope_ReturnsCorrectScope) { 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()); + auto *locals_store = vars.GetVariableStore(locals_scope->variablesReference); + auto *globals_store = + vars.GetVariableStore(globals_scope->variablesReference); + auto *registers_store = + vars.GetVariableStore(registers_scope->variablesReference); + + ASSERT_NE(locals_store, nullptr); + ASSERT_NE(globals_store, nullptr); + ASSERT_NE(registers_store, nullptr); + + const uint32_t local_ref = locals_scope->variablesReference; + const uint32_t global_ref = globals_scope->variablesReference; + const uint32_t register_ref = registers_scope->variablesReference; + ASSERT_EQ(VariableReferenceStorage::GetReferenceKind(global_ref), + eReferenceKindScope); + ASSERT_EQ(VariableReferenceStorage::GetReferenceKind(local_ref), + eReferenceKindScope); + ASSERT_EQ(VariableReferenceStorage::GetReferenceKind(register_ref), + eReferenceKindScope); + + EXPECT_EQ(vars.GetVariableStore(9999), nullptr); } TEST_F(VariablesTest, FindVariable_LocalsByName) { lldb::SBFrame frame; - uint32_t frame_id = 0; - std::vector<protocol::Scope> scopes = vars.CreateScopes(frame_id, frame); + std::vector<protocol::Scope> scopes = vars.CreateScopes(frame); const protocol::Scope *locals_scope = FindScope(scopes, "Locals"); ASSERT_NE(locals_scope, nullptr); >From b92fc1d28bf5e29b4a5087ad51a60710fe8f591e Mon Sep 17 00:00:00 2001 From: Ebuka Ezike <[email protected]> Date: Mon, 2 Feb 2026 20:10:06 +0000 Subject: [PATCH 2/5] [lldb-dap] Disable function Clear for permanent types --- lldb/tools/lldb-dap/Variables.h | 29 ++++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/lldb/tools/lldb-dap/Variables.h b/lldb/tools/lldb-dap/Variables.h index be47bca02c784..ace82f015400e 100644 --- a/lldb/tools/lldb-dap/Variables.h +++ b/lldb/tools/lldb-dap/Variables.h @@ -14,6 +14,7 @@ #include "lldb/API/SBFrame.h" #include "lldb/API/SBValue.h" #include "lldb/API/SBValueList.h" +#include "llvm/Support/ErrorHandling.h" #include <limits> namespace lldb_dap { @@ -188,11 +189,23 @@ struct VariableReferenceStorage { /// /// \tparam ReferenceKindByte /// The bitmask used to identify this pool's references - template <typename VariableStoreType, uint8_t ReferenceKindByte> + template <typename VariableStoreType, ReferenceKind Kind> class ReferenceKindPool { public: - static constexpr uint32_t k_reference_mask = ReferenceKindByte + static constexpr uint8_t k_reference_kind_byte = []() { + switch (Kind) { + case eReferenceKindPermanent: + return k_permanent_kind_byte; + case eReferenceKindTemporary: + return k_temporary_kind_byte; + case eReferenceKindScope: + return k_scope_kind_byte; + } + llvm_unreachable("Unknown reference kind"); + }(); + + static constexpr uint32_t k_reference_mask = k_reference_kind_byte << k_reference_kind_bit_offset; static_assert( @@ -208,7 +221,9 @@ struct VariableReferenceStorage { /// Resets the produced reference and clears all scope and temporary /// expandable kind pools. - void Clear() { + template <ReferenceKind LHS = Kind, + ReferenceKind RHS = eReferenceKindPermanent> + std::enable_if_t<LHS != RHS, void> Clear() { m_value = k_reference_mask; m_pool.clear(); } @@ -239,19 +254,19 @@ struct VariableReferenceStorage { ~ReferenceKindPool() = default; private: - uint32_t m_value = ReferenceKindByte; + uint32_t m_value = k_reference_kind_byte; std::vector<VariableStoreType> m_pool; }; /// Variables that are alive in this stop state. /// Will be cleared when debuggee resumes. - ReferenceKindPool<ExpandableValueStore, k_temporary_kind_byte> + ReferenceKindPool<ExpandableValueStore, eReferenceKindTemporary> m_temporary_kind_pool; /// Variables that persist across entire debug session. /// These are the variables evaluated from debug console REPL. - ReferenceKindPool<ExpandableValueStore, k_permanent_kind_byte> + ReferenceKindPool<ExpandableValueStore, eReferenceKindPermanent> m_permanent_kind_pool; - ReferenceKindPool<ScopeStore, k_scope_kind_byte> m_scope_kind_pool; + ReferenceKindPool<ScopeStore, eReferenceKindScope> m_scope_kind_pool; }; } // namespace lldb_dap >From 1c1e3eac0832ce124bd4d6b1a1ffb0462a057d9a Mon Sep 17 00:00:00 2001 From: Ebuka Ezike <[email protected]> Date: Mon, 2 Feb 2026 21:21:54 +0000 Subject: [PATCH 3/5] add var_ref_t type --- .../Handler/SetVariableRequestHandler.cpp | 2 +- .../Handler/VariablesRequestHandler.cpp | 4 +- lldb/tools/lldb-dap/Protocol/DAPTypes.h | 9 ++++ .../lldb-dap/Protocol/ProtocolRequests.h | 10 ++--- lldb/tools/lldb-dap/Protocol/ProtocolTypes.h | 4 +- lldb/tools/lldb-dap/Variables.cpp | 44 +++++++++---------- lldb/tools/lldb-dap/Variables.h | 26 +++++------ lldb/unittests/DAP/VariablesTest.cpp | 18 ++++---- 8 files changed, 64 insertions(+), 53 deletions(-) diff --git a/lldb/tools/lldb-dap/Handler/SetVariableRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/SetVariableRequestHandler.cpp index 293594dc2c90a..4b6bdc3159512 100644 --- a/lldb/tools/lldb-dap/Handler/SetVariableRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/SetVariableRequestHandler.cpp @@ -62,7 +62,7 @@ SetVariableRequestHandler::Run(const SetVariableArguments &args) const { // so always insert a new one to get its variablesReference. // is_permanent is false because debug console does not support // setVariable request. - const int64_t new_var_ref = + const var_ref_t new_var_ref = dap.reference_storage.InsertVariable(variable, /*is_permanent=*/false); if (variable.MightHaveChildren()) { body.variablesReference = new_var_ref; diff --git a/lldb/tools/lldb-dap/Handler/VariablesRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/VariablesRequestHandler.cpp index cf7697500d3de..306438b574bf5 100644 --- a/lldb/tools/lldb-dap/Handler/VariablesRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/VariablesRequestHandler.cpp @@ -9,7 +9,9 @@ #include "DAP.h" #include "EventHelper.h" #include "Handler/RequestHandler.h" +#include "Protocol/ProtocolRequests.h" #include "Variables.h" +#include "llvm/Support/ErrorExtras.h" using namespace llvm; using namespace lldb_dap::protocol; @@ -26,7 +28,7 @@ VariablesRequestHandler::Run(const VariablesArguments &arguments) const { VariableStore *store = dap.reference_storage.GetVariableStore(var_ref); if (!store) - return VariablesResponseBody{}; + return llvm::createStringErrorV("invalid variableReference: {}", var_ref); return VariablesResponseBody{ store->GetVariables(dap.reference_storage, dap.configuration, arguments)}; diff --git a/lldb/tools/lldb-dap/Protocol/DAPTypes.h b/lldb/tools/lldb-dap/Protocol/DAPTypes.h index 23ba93946bdee..38f565a4e9a2d 100644 --- a/lldb/tools/lldb-dap/Protocol/DAPTypes.h +++ b/lldb/tools/lldb-dap/Protocol/DAPTypes.h @@ -22,6 +22,15 @@ #include <optional> #include <string> +namespace lldb_dap { +/// The var_ref_t hold two values, the `ReferenceKind` and the +/// `variablesReference`. +/// +/// The most significant byte encodes the `ReferenceKind` and +/// remaining three bytes has the `variablesReference`. +using var_ref_t = uint32_t; +} // namespace lldb_dap + namespace lldb_dap::protocol { /// Data used to help lldb-dap resolve breakpoints persistently across different diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h index 312a9dd8714a0..c00392360158e 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h +++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h @@ -435,7 +435,7 @@ struct SetVariableArguments { /// The reference of the variable container. The `variablesReference` must /// have been obtained in the current suspended state. See 'Lifetime of Object /// References' in the Overview section for details. - uint32_t variablesReference = LLDB_DAP_INVALID_VAR_REF; + var_ref_t variablesReference = LLDB_DAP_INVALID_VAR_REF; /// The name of the variable in the container. std::string name; @@ -466,7 +466,7 @@ struct SetVariableResponseBody { /// If this property is included in the response, any `variablesReference` /// previously associated with the updated variable, and those of its /// children, are no longer valid. - uint32_t variablesReference = 0; + var_ref_t variablesReference = 0; /// The number of named child variables. /// The client can use this information to present the variables in a paged @@ -727,7 +727,7 @@ struct DataBreakpointInfoArguments { /// for a child of the container. The `variablesReference` must have been /// obtained in the current suspended state.See 'Lifetime of Object /// References' in the Overview section for details. - std::optional<uint32_t> variablesReference; + std::optional<var_ref_t> variablesReference; /// The name of the variable's child to obtain data breakpoint information /// for. If `variablesReference` isn't specified, this can be an expression, @@ -953,7 +953,7 @@ struct VariablesArguments { /// The variable for which to retrieve its children. The `variablesReference` /// must have been obtained in the current suspended state. See 'Lifetime of /// Object References' in the Overview section for details. - uint32_t variablesReference = LLDB_DAP_INVALID_VAR_REF; + var_ref_t variablesReference = LLDB_DAP_INVALID_VAR_REF; enum VariablesFilter : unsigned { eVariablesFilterBoth = 0, @@ -1158,7 +1158,7 @@ struct EvaluateResponseBody { /// children can be retrieved by passing `variablesReference` to the /// `variables` request as long as execution remains suspended. See 'Lifetime /// of Object References' in the Overview section for details. - uint32_t variablesReference = 0; + var_ref_t variablesReference = 0; /// The number of named child variables. /// The client can use this information to present the variables in a paged diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h index 0a9d1ebaccd8d..067e79bcb3a20 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h +++ b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h @@ -464,7 +464,7 @@ struct Scope { /// remains suspended. See 'Lifetime of Object References' in the Overview /// section for details. //// - uint32_t variablesReference = LLDB_DAP_INVALID_VAR_REF; + var_ref_t variablesReference = LLDB_DAP_INVALID_VAR_REF; /// The number of named variables in this scope. /// The client can use this information to present the variables in a paged UI @@ -963,7 +963,7 @@ struct Variable { /// children can be retrieved by passing `variablesReference` to the /// `variables` request as long as execution remains suspended. See 'Lifetime /// of Object References' in the Overview section for details. - uint32_t variablesReference = 0; + var_ref_t variablesReference = 0; /// The number of named child variables. /// diff --git a/lldb/tools/lldb-dap/Variables.cpp b/lldb/tools/lldb-dap/Variables.cpp index 98dc0417e5242..5676b782fd8eb 100644 --- a/lldb/tools/lldb-dap/Variables.cpp +++ b/lldb/tools/lldb-dap/Variables.cpp @@ -27,7 +27,7 @@ using namespace lldb_dap::protocol; namespace lldb_dap { -protocol::Scope CreateScope(ScopeKind kind, uint32_t variablesReference, +protocol::Scope CreateScope(ScopeKind kind, var_ref_t variablesReference, uint32_t namedVariables, bool expensive) { protocol::Scope scope; scope.variablesReference = variablesReference; @@ -92,7 +92,7 @@ ScopeStore::GetVariables(VariableReferenceStorage &storage, if (!variable.IsValid()) break; - const uint32_t frame_var_ref = + const var_ref_t frame_var_ref = storage.InsertVariable(variable, /*is_permanent=*/false); variables.emplace_back(CreateVariable( variable, frame_var_ref, format_hex, config.enableAutoVariableSummaries, @@ -202,7 +202,7 @@ void ScopeStore::AddReturnValue(VariableReferenceStorage &storage, if (stop_return_value.IsValid() && (selected_thread.GetSelectedFrame().GetFrameID() == 0)) { auto renamed_return_value = stop_return_value.Clone("(Return Value)"); - uint32_t return_var_ref = 0; + var_ref_t return_var_ref = 0; if (stop_return_value.MightHaveChildren() || stop_return_value.IsSynthetic()) { @@ -220,7 +220,7 @@ std::vector<Variable> ExpandableValueStore::GetVariables(VariableReferenceStorage &storage, const Configuration &config, const VariablesArguments &args) { - const uint32_t var_ref = args.variablesReference; + const var_ref_t var_ref = args.variablesReference; const uint64_t count = args.count; const uint64_t start = args.start; const bool hex = args.format ? args.format->hex : false; @@ -238,7 +238,7 @@ ExpandableValueStore::GetVariables(VariableReferenceStorage &storage, std::optional<llvm::StringRef> custom_name = {}) { if (!child.IsValid()) return; - const uint32_t child_var_ref = storage.InsertVariable(child, is_permanent); + const var_ref_t child_var_ref = storage.InsertVariable(child, is_permanent); variables.emplace_back(CreateVariable( child, child_var_ref, hex, config.enableAutoVariableSummaries, config.enableSyntheticChildDebugging, @@ -276,18 +276,18 @@ lldb::SBValue ExpandableValueStore::FindVariable(llvm::StringRef name) { return variable; } -bool VariableReferenceStorage::IsPermanentVariableReference(uint32_t var_ref) { +bool VariableReferenceStorage::IsPermanentVariableReference(var_ref_t var_ref) { return GetReferenceKind(var_ref) == eReferenceKindPermanent; } -uint32_t VariableReferenceStorage::CreateVariableReference(bool is_permanent) { +var_ref_t VariableReferenceStorage::CreateVariableReference(bool is_permanent) { if (is_permanent) return m_permanent_kind_pool.NextReference(); return m_temporary_kind_pool.NextReference(); } -lldb::SBValue VariableReferenceStorage::GetVariable(uint32_t masked_var_ref) { +lldb::SBValue VariableReferenceStorage::GetVariable(var_ref_t masked_var_ref) { const ReferenceKind kind = GetReferenceKind(masked_var_ref); if (kind == eReferenceKindTemporary) { @@ -303,29 +303,29 @@ lldb::SBValue VariableReferenceStorage::GetVariable(uint32_t masked_var_ref) { return {}; } -uint32_t VariableReferenceStorage::InsertVariable(const lldb::SBValue &variable, - bool is_permanent) { +var_ref_t +VariableReferenceStorage::InsertVariable(const lldb::SBValue &variable, + bool is_permanent) { if (is_permanent) { - const uint32_t masked_var_ref = m_permanent_kind_pool.NextReference(); - const uint32_t var_ref = m_permanent_kind_pool.ClearMask(masked_var_ref); + const var_ref_t masked_var_ref = m_permanent_kind_pool.NextReference(); + const var_ref_t var_ref = m_permanent_kind_pool.ClearMask(masked_var_ref); m_permanent_kind_pool.Add(variable); assert(var_ref == m_permanent_kind_pool.Size() && "New variablesReference must be the size of the reference pool"); return masked_var_ref; } - const uint32_t masked_var_ref = m_temporary_kind_pool.NextReference(); - const uint32_t var_ref = m_temporary_kind_pool.ClearMask(masked_var_ref); + const var_ref_t masked_var_ref = m_temporary_kind_pool.NextReference(); + const var_ref_t var_ref = m_temporary_kind_pool.ClearMask(masked_var_ref); m_temporary_kind_pool.Add(variable); assert(var_ref == m_temporary_kind_pool.Size() && "New variablesReference must be the size of the reference pool"); return masked_var_ref; } -lldb::SBValue -VariableReferenceStorage::FindVariable(uint32_t variablesReference, - llvm::StringRef name) { - if (VariableStore *store = GetVariableStore(variablesReference)) +lldb::SBValue VariableReferenceStorage::FindVariable(var_ref_t masked_var_ref, + llvm::StringRef name) { + if (VariableStore *store = GetVariableStore(masked_var_ref)) return store->FindVariable(name); return {}; @@ -334,8 +334,8 @@ VariableReferenceStorage::FindVariable(uint32_t variablesReference, std::vector<protocol::Scope> VariableReferenceStorage::CreateScopes(lldb::SBFrame &frame) { auto create_scope = [&](ScopeKind kind) { - const uint32_t masked_ref = m_scope_kind_pool.NextReference(); - const uint32_t raw_ref = m_scope_kind_pool.ClearMask(masked_ref); + const var_ref_t masked_ref = m_scope_kind_pool.NextReference(); + const var_ref_t raw_ref = m_scope_kind_pool.ClearMask(masked_ref); m_scope_kind_pool.Add(kind, frame); assert(m_scope_kind_pool.Size() == raw_ref && "New scopes ref must be the size of the pool"); @@ -348,7 +348,7 @@ VariableReferenceStorage::CreateScopes(lldb::SBFrame &frame) { } VariableStore * -VariableReferenceStorage::GetVariableStore(uint32_t masked_var_ref) { +VariableReferenceStorage::GetVariableStore(var_ref_t masked_var_ref) { const ReferenceKind kind = GetReferenceKind(masked_var_ref); switch (kind) { case eReferenceKindPermanent: @@ -361,7 +361,7 @@ VariableReferenceStorage::GetVariableStore(uint32_t masked_var_ref) { llvm_unreachable("Unknown reference kind."); } -ReferenceKind VariableReferenceStorage::GetReferenceKind(uint32_t var_ref) { +ReferenceKind VariableReferenceStorage::GetReferenceKind(var_ref_t var_ref) { const uint8_t reference_kind_byte = var_ref >> k_reference_kind_bit_offset; if ((reference_kind_byte & k_scope_kind_byte) == k_scope_kind_byte) diff --git a/lldb/tools/lldb-dap/Variables.h b/lldb/tools/lldb-dap/Variables.h index ace82f015400e..dd8cb124aa094 100644 --- a/lldb/tools/lldb-dap/Variables.h +++ b/lldb/tools/lldb-dap/Variables.h @@ -48,7 +48,7 @@ enum ReferenceKind : uint32_t { /// /// \return /// A `protocol::Scope` -protocol::Scope CreateScope(ScopeKind kind, uint32_t variablesReference, +protocol::Scope CreateScope(ScopeKind kind, var_ref_t variablesReference, uint32_t namedVariables, bool expensive); /// An Interface to get or find specific variables by name. @@ -119,23 +119,23 @@ class ExpandableValueStore : public VariableStore { struct VariableReferenceStorage { /// 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(uint32_t var_ref); + static bool IsPermanentVariableReference(var_ref_t var_ref); /// \return a new variableReference. /// Specify is_permanent as true for variable that should persist entire /// debug session. - uint32_t CreateVariableReference(bool is_permanent); + var_ref_t CreateVariableReference(bool is_permanent); /// \return the expandable variable corresponding with variableReference /// value of \p value. /// If \p var_ref is invalid an empty SBValue is returned. - lldb::SBValue GetVariable(uint32_t var_ref); + lldb::SBValue GetVariable(var_ref_t var_ref); /// Insert a new \p variable. /// \return variableReference assigned to this expandable variable. - uint32_t InsertVariable(const lldb::SBValue &variable, bool is_permanent); + var_ref_t InsertVariable(const lldb::SBValue &variable, bool is_permanent); - lldb::SBValue FindVariable(uint32_t variablesReference, llvm::StringRef name); + lldb::SBValue FindVariable(var_ref_t masked_var_ref, llvm::StringRef name); std::vector<protocol::Scope> CreateScopes(lldb::SBFrame &frame); @@ -144,9 +144,9 @@ struct VariableReferenceStorage { m_scope_kind_pool.Clear(); } - VariableStore *GetVariableStore(uint32_t var_ref); + VariableStore *GetVariableStore(var_ref_t var_ref); - static ReferenceKind GetReferenceKind(uint32_t var_ref); + static ReferenceKind GetReferenceKind(var_ref_t var_ref); private: /// Variable reference start index of temporary variables. @@ -214,7 +214,7 @@ struct VariableReferenceStorage { "Cannot store minimum number of variables without overflowing"); explicit ReferenceKindPool() : m_value(k_reference_mask) {} - uint32_t NextReference() { + var_ref_t NextReference() { m_value++; return m_value; } @@ -228,15 +228,15 @@ struct VariableReferenceStorage { m_pool.clear(); } - VariableStoreType *GetVariableStore(uint32_t masked_var_ref) { - const uint32_t var_ref = ClearMask(masked_var_ref); + VariableStoreType *GetVariableStore(var_ref_t masked_var_ref) { + const var_ref_t var_ref = ClearMask(masked_var_ref); if (var_ref != 0 && var_ref <= m_pool.size()) return &m_pool[var_ref - 1]; return nullptr; } - uint32_t ClearMask(uint32_t masked_var_ref) { + var_ref_t ClearMask(var_ref_t masked_var_ref) { return masked_var_ref & ~k_reference_mask; } @@ -254,7 +254,7 @@ struct VariableReferenceStorage { ~ReferenceKindPool() = default; private: - uint32_t m_value = k_reference_kind_byte; + var_ref_t m_value = k_reference_kind_byte; std::vector<VariableStoreType> m_pool; }; diff --git a/lldb/unittests/DAP/VariablesTest.cpp b/lldb/unittests/DAP/VariablesTest.cpp index d27e3164b6ad8..01ea6bc0f9114 100644 --- a/lldb/unittests/DAP/VariablesTest.cpp +++ b/lldb/unittests/DAP/VariablesTest.cpp @@ -43,7 +43,7 @@ TEST_F(VariablesTest, GetNewVariableReference_UniqueAndRanges) { TEST_F(VariablesTest, InsertAndGetVariable_Temporary) { lldb::SBValue dummy; - const int64_t ref = vars.InsertVariable(dummy, Temporary); + const var_ref_t ref = vars.InsertVariable(dummy, Temporary); lldb::SBValue out = vars.GetVariable(ref); EXPECT_EQ(out.IsValid(), dummy.IsValid()); @@ -51,15 +51,15 @@ TEST_F(VariablesTest, InsertAndGetVariable_Temporary) { TEST_F(VariablesTest, InsertAndGetVariable_Permanent) { lldb::SBValue dummy; - const int64_t ref = vars.InsertVariable(dummy, Permanent); + const var_ref_t ref = vars.InsertVariable(dummy, Permanent); lldb::SBValue out = vars.GetVariable(ref); EXPECT_EQ(out.IsValid(), dummy.IsValid()); } TEST_F(VariablesTest, IsPermanentVariableReference) { - const int64_t perm = vars.CreateVariableReference(Permanent); - const int64_t temp = vars.CreateVariableReference(Temporary); + const var_ref_t perm = vars.CreateVariableReference(Permanent); + const var_ref_t temp = vars.CreateVariableReference(Temporary); EXPECT_TRUE(VariableReferenceStorage::IsPermanentVariableReference(perm)); EXPECT_FALSE(VariableReferenceStorage::IsPermanentVariableReference(temp)); @@ -67,8 +67,8 @@ TEST_F(VariablesTest, IsPermanentVariableReference) { TEST_F(VariablesTest, Clear_RemovesTemporaryKeepsPermanent) { lldb::SBValue dummy; - const int64_t temp = vars.InsertVariable(dummy, Temporary); - const int64_t perm = vars.InsertVariable(dummy, Permanent); + const var_ref_t temp = vars.InsertVariable(dummy, Temporary); + const var_ref_t perm = vars.InsertVariable(dummy, Permanent); vars.Clear(); EXPECT_FALSE(vars.GetVariable(temp).IsValid()); @@ -98,9 +98,9 @@ TEST_F(VariablesTest, VariablesStore) { ASSERT_NE(globals_store, nullptr); ASSERT_NE(registers_store, nullptr); - const uint32_t local_ref = locals_scope->variablesReference; - const uint32_t global_ref = globals_scope->variablesReference; - const uint32_t register_ref = registers_scope->variablesReference; + const var_ref_t local_ref = locals_scope->variablesReference; + const var_ref_t global_ref = globals_scope->variablesReference; + const var_ref_t register_ref = registers_scope->variablesReference; ASSERT_EQ(VariableReferenceStorage::GetReferenceKind(global_ref), eReferenceKindScope); ASSERT_EQ(VariableReferenceStorage::GetReferenceKind(local_ref), >From eb76df897f24e8d057b428eb9c9294706c2afb7d Mon Sep 17 00:00:00 2001 From: Ebuka Ezike <[email protected]> Date: Mon, 2 Feb 2026 23:21:57 +0000 Subject: [PATCH 4/5] add review changes --- .../lldb-dap/variables/TestDAP_variables.py | 23 +++++++++++-- .../Handler/VariablesRequestHandler.cpp | 7 ++-- lldb/tools/lldb-dap/Variables.cpp | 31 +++-------------- lldb/tools/lldb-dap/Variables.h | 34 ++++++++++++------- lldb/unittests/DAP/VariablesTest.cpp | 12 +++---- 5 files changed, 58 insertions(+), 49 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 1dbb0143e7a55..351ec9d3ef262 100644 --- a/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py +++ b/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py @@ -99,6 +99,11 @@ def verify_values(self, verify_dict, actual, varref_dict=None, expression=None): ) def verify_variables(self, verify_dict, variables, varref_dict=None): + self.assertGreaterEqual( + len(variables), + 1, + f"No variables to verify, verify_dict={json.dumps(verify_dict, indent=4)}", + ) for variable in variables: name = variable["name"] if not name.startswith("std::"): @@ -542,6 +547,7 @@ def do_test_scopes_and_evaluate_expansion(self, enableAutoVariableSummaries: boo # Evaluate from known contexts. expr_varref_dict = {} + permanent_expandable_ref = None for context, verify_dict in expandable_expression["context"].items(): response = self.dap_server.request_evaluate( expandable_expression["name"], @@ -549,9 +555,15 @@ def do_test_scopes_and_evaluate_expansion(self, enableAutoVariableSummaries: boo threadId=None, context=context, ) + + response_body = response["body"] + if context == "repl": # save the variablesReference + self.assertIn("variablesReference", response_body) + permanent_expandable_ref = response_body["variablesReference"] + self.verify_values( verify_dict, - response["body"], + response_body, expr_varref_dict, expandable_expression["name"], ) @@ -578,7 +590,7 @@ def do_test_scopes_and_evaluate_expansion(self, enableAutoVariableSummaries: boo ) self.continue_to_breakpoints(breakpoint_ids) - var_ref = expr_varref_dict[expandable_expression["name"]] + var_ref = permanent_expandable_ref response = self.dap_server.request_variables(var_ref) self.verify_variables( expandable_expression["children"], response["body"]["variables"] @@ -598,6 +610,13 @@ def do_test_scopes_and_evaluate_expansion(self, enableAutoVariableSummaries: boo if scope["name"] == "Registers": self.assertEqual(scope.get("presentationHint"), "registers") + # Test invalid variablesReference. + for wrong_var_ref in (-6000, -1, 4000): + response = self.dap_server.request_variables(wrong_var_ref) + self.assertFalse(response["success"]) + error_msg: str = response["body"]["error"]["format"] + self.assertTrue(error_msg.startswith("Invalid variablesReference")) + def test_scopes_and_evaluate_expansion(self): self.do_test_scopes_and_evaluate_expansion(enableAutoVariableSummaries=False) diff --git a/lldb/tools/lldb-dap/Handler/VariablesRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/VariablesRequestHandler.cpp index 306438b574bf5..ca02b0a109c0a 100644 --- a/lldb/tools/lldb-dap/Handler/VariablesRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/VariablesRequestHandler.cpp @@ -10,6 +10,7 @@ #include "EventHelper.h" #include "Handler/RequestHandler.h" #include "Protocol/ProtocolRequests.h" +#include "Protocol/ProtocolTypes.h" #include "Variables.h" #include "llvm/Support/ErrorExtras.h" @@ -24,11 +25,13 @@ namespace lldb_dap { /// indexed children. Expected<VariablesResponseBody> VariablesRequestHandler::Run(const VariablesArguments &arguments) const { - const uint32_t var_ref = arguments.variablesReference; + const var_ref_t var_ref = arguments.variablesReference; + if (var_ref == LLDB_DAP_INVALID_VAR_REF) + return llvm::createStringErrorV("Invalid variablesReference: {}", var_ref); VariableStore *store = dap.reference_storage.GetVariableStore(var_ref); if (!store) - return llvm::createStringErrorV("invalid variableReference: {}", var_ref); + return llvm::createStringErrorV("Invalid variablesReference: {}", var_ref); return VariablesResponseBody{ store->GetVariables(dap.reference_storage, dap.configuration, arguments)}; diff --git a/lldb/tools/lldb-dap/Variables.cpp b/lldb/tools/lldb-dap/Variables.cpp index 5676b782fd8eb..5c8fe1badcb81 100644 --- a/lldb/tools/lldb-dap/Variables.cpp +++ b/lldb/tools/lldb-dap/Variables.cpp @@ -280,13 +280,6 @@ bool VariableReferenceStorage::IsPermanentVariableReference(var_ref_t var_ref) { return GetReferenceKind(var_ref) == eReferenceKindPermanent; } -var_ref_t VariableReferenceStorage::CreateVariableReference(bool is_permanent) { - if (is_permanent) - return m_permanent_kind_pool.NextReference(); - - return m_temporary_kind_pool.NextReference(); -} - lldb::SBValue VariableReferenceStorage::GetVariable(var_ref_t masked_var_ref) { const ReferenceKind kind = GetReferenceKind(masked_var_ref); @@ -306,21 +299,10 @@ lldb::SBValue VariableReferenceStorage::GetVariable(var_ref_t masked_var_ref) { var_ref_t VariableReferenceStorage::InsertVariable(const lldb::SBValue &variable, bool is_permanent) { - if (is_permanent) { - const var_ref_t masked_var_ref = m_permanent_kind_pool.NextReference(); - const var_ref_t var_ref = m_permanent_kind_pool.ClearMask(masked_var_ref); - m_permanent_kind_pool.Add(variable); - assert(var_ref == m_permanent_kind_pool.Size() && - "New variablesReference must be the size of the reference pool"); - return masked_var_ref; - } + if (is_permanent) + return m_permanent_kind_pool.Add(variable); - const var_ref_t masked_var_ref = m_temporary_kind_pool.NextReference(); - const var_ref_t var_ref = m_temporary_kind_pool.ClearMask(masked_var_ref); - m_temporary_kind_pool.Add(variable); - assert(var_ref == m_temporary_kind_pool.Size() && - "New variablesReference must be the size of the reference pool"); - return masked_var_ref; + return m_temporary_kind_pool.Add(variable); } lldb::SBValue VariableReferenceStorage::FindVariable(var_ref_t masked_var_ref, @@ -334,12 +316,7 @@ lldb::SBValue VariableReferenceStorage::FindVariable(var_ref_t masked_var_ref, std::vector<protocol::Scope> VariableReferenceStorage::CreateScopes(lldb::SBFrame &frame) { auto create_scope = [&](ScopeKind kind) { - const var_ref_t masked_ref = m_scope_kind_pool.NextReference(); - const var_ref_t raw_ref = m_scope_kind_pool.ClearMask(masked_ref); - m_scope_kind_pool.Add(kind, frame); - assert(m_scope_kind_pool.Size() == raw_ref && - "New scopes ref must be the size of the pool"); - + const var_ref_t masked_ref = m_scope_kind_pool.Add(kind, frame); return CreateScope(kind, masked_ref, 0, false); }; diff --git a/lldb/tools/lldb-dap/Variables.h b/lldb/tools/lldb-dap/Variables.h index dd8cb124aa094..fbacf570776ce 100644 --- a/lldb/tools/lldb-dap/Variables.h +++ b/lldb/tools/lldb-dap/Variables.h @@ -205,26 +205,23 @@ struct VariableReferenceStorage { llvm_unreachable("Unknown reference kind"); }(); - static constexpr uint32_t k_reference_mask = k_reference_kind_byte - << k_reference_kind_bit_offset; + static constexpr var_ref_t k_reference_mask = + k_reference_kind_byte << k_reference_kind_bit_offset; + static constexpr var_ref_t k_reference_max_size = + (1 << k_reference_kind_bit_offset) - 1; static_assert( ((k_reference_mask + k_min_number_of_variables) < max_reference_value) && "Cannot store minimum number of variables without overflowing"); - explicit ReferenceKindPool() : m_value(k_reference_mask) {} - var_ref_t NextReference() { - m_value++; - return m_value; - } + explicit ReferenceKindPool() = default; - /// Resets the produced reference and clears all scope and temporary - /// expandable kind pools. + /// Resets the count to zero and clears the pool. template <ReferenceKind LHS = Kind, ReferenceKind RHS = eReferenceKindPermanent> std::enable_if_t<LHS != RHS, void> Clear() { - m_value = k_reference_mask; + reference_count = 0; m_pool.clear(); } @@ -240,8 +237,16 @@ struct VariableReferenceStorage { return masked_var_ref & ~k_reference_mask; } - template <typename... Args> void Add(Args &&...args) { + template <typename... Args> var_ref_t Add(Args &&...args) { + assert(reference_count == m_pool.size() && + "Current reference_count must be the size of the pool"); + m_pool.emplace_back(std::forward<Args>(args)...); + const var_ref_t next_var_ref = NextReference(); + + assert(next_var_ref < k_reference_max_size && + "Exceeds the maximum variablesReference"); + return next_var_ref | k_reference_mask; } size_t Size() { return m_pool.size(); } @@ -254,7 +259,12 @@ struct VariableReferenceStorage { ~ReferenceKindPool() = default; private: - var_ref_t m_value = k_reference_kind_byte; + var_ref_t NextReference() { + reference_count++; + return reference_count; + } + + var_ref_t reference_count = 0; std::vector<VariableStoreType> m_pool; }; diff --git a/lldb/unittests/DAP/VariablesTest.cpp b/lldb/unittests/DAP/VariablesTest.cpp index 01ea6bc0f9114..8b10723442f85 100644 --- a/lldb/unittests/DAP/VariablesTest.cpp +++ b/lldb/unittests/DAP/VariablesTest.cpp @@ -30,10 +30,10 @@ class VariablesTest : public ::testing::Test { }; TEST_F(VariablesTest, GetNewVariableReference_UniqueAndRanges) { - const int64_t temp1 = vars.CreateVariableReference(Temporary); - const int64_t temp2 = vars.CreateVariableReference(Temporary); - const int64_t perm1 = vars.CreateVariableReference(Permanent); - const int64_t perm2 = vars.CreateVariableReference(Permanent); + const var_ref_t temp1 = vars.InsertVariable(lldb::SBValue(), Temporary); + const var_ref_t temp2 = vars.InsertVariable(lldb::SBValue(), Temporary); + const var_ref_t perm1 = vars.InsertVariable(lldb::SBValue(), Permanent); + const var_ref_t perm2 = vars.InsertVariable(lldb::SBValue(), Permanent); EXPECT_NE(temp1, temp2); EXPECT_NE(perm1, perm2); @@ -58,8 +58,8 @@ TEST_F(VariablesTest, InsertAndGetVariable_Permanent) { } TEST_F(VariablesTest, IsPermanentVariableReference) { - const var_ref_t perm = vars.CreateVariableReference(Permanent); - const var_ref_t temp = vars.CreateVariableReference(Temporary); + const var_ref_t perm = vars.InsertVariable(lldb::SBValue(), Permanent); + const var_ref_t temp = vars.InsertVariable(lldb::SBValue(), Temporary); EXPECT_TRUE(VariableReferenceStorage::IsPermanentVariableReference(perm)); EXPECT_FALSE(VariableReferenceStorage::IsPermanentVariableReference(temp)); >From de2a9056a8c415449cbb10d73ef000cae8c29b6d Mon Sep 17 00:00:00 2001 From: Ebuka Ezike <[email protected]> Date: Tue, 3 Feb 2026 00:29:20 +0000 Subject: [PATCH 5/5] mark non local scope as expensive add review changes add review changes --- lldb/tools/lldb-dap/Variables.cpp | 6 ++--- lldb/tools/lldb-dap/Variables.h | 37 ++++++++++++++----------------- 2 files changed, 20 insertions(+), 23 deletions(-) diff --git a/lldb/tools/lldb-dap/Variables.cpp b/lldb/tools/lldb-dap/Variables.cpp index 5c8fe1badcb81..f8f0691bc4adf 100644 --- a/lldb/tools/lldb-dap/Variables.cpp +++ b/lldb/tools/lldb-dap/Variables.cpp @@ -28,10 +28,9 @@ using namespace lldb_dap::protocol; namespace lldb_dap { protocol::Scope CreateScope(ScopeKind kind, var_ref_t variablesReference, - uint32_t namedVariables, bool expensive) { + bool expensive) { protocol::Scope scope; scope.variablesReference = variablesReference; - scope.namedVariables = namedVariables; scope.expensive = expensive; // TODO: Support "arguments" and "return value" scope. @@ -317,7 +316,8 @@ std::vector<protocol::Scope> VariableReferenceStorage::CreateScopes(lldb::SBFrame &frame) { auto create_scope = [&](ScopeKind kind) { const var_ref_t masked_ref = m_scope_kind_pool.Add(kind, frame); - return CreateScope(kind, masked_ref, 0, false); + const bool is_expensive = kind != eScopeKindLocals; + return CreateScope(kind, masked_ref, is_expensive); }; return {create_scope(eScopeKindLocals), create_scope(eScopeKindGlobals), diff --git a/lldb/tools/lldb-dap/Variables.h b/lldb/tools/lldb-dap/Variables.h index fbacf570776ce..64552628e7ea4 100644 --- a/lldb/tools/lldb-dap/Variables.h +++ b/lldb/tools/lldb-dap/Variables.h @@ -26,7 +26,7 @@ enum ScopeKind : unsigned { eScopeKindRegisters }; -enum ReferenceKind : uint32_t { +enum ReferenceKind : unsigned { eReferenceKindPermanent, eReferenceKindTemporary, eReferenceKindScope @@ -40,16 +40,13 @@ enum ReferenceKind : uint32_t { /// \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(ScopeKind kind, var_ref_t variablesReference, - uint32_t namedVariables, bool expensive); + bool expensive); /// An Interface to get or find specific variables by name. class VariableStore { @@ -72,7 +69,7 @@ class VariableStore { /// A Variable store for fetching variables within a specific scope (locals, /// globals, or registers) for a given stack frame. -class ScopeStore : public VariableStore { +class ScopeStore final : public VariableStore { public: explicit ScopeStore(ScopeKind kind, const lldb::SBFrame &frame) : m_frame(frame), m_kind(kind) {} @@ -80,8 +77,8 @@ class ScopeStore : public VariableStore { std::vector<protocol::Variable> GetVariables(VariableReferenceStorage &storage, const protocol::Configuration &config, - const protocol::VariablesArguments &args) final; - lldb::SBValue FindVariable(llvm::StringRef name) final; + const protocol::VariablesArguments &args) override; + lldb::SBValue FindVariable(llvm::StringRef name) override; private: void LoadVariables(); @@ -100,7 +97,7 @@ class ScopeStore : public VariableStore { /// /// Manages children variables of complex types (structs, arrays, pointers, /// etc.) that can be expanded in the debugger UI. -class ExpandableValueStore : public VariableStore { +class ExpandableValueStore final : public VariableStore { public: explicit ExpandableValueStore(const lldb::SBValue &value) : m_value(value) {} @@ -108,8 +105,8 @@ class ExpandableValueStore : public VariableStore { std::vector<protocol::Variable> GetVariables(VariableReferenceStorage &storage, const protocol::Configuration &config, - const protocol::VariablesArguments &args) final; - lldb::SBValue FindVariable(llvm::StringRef name) final; + const protocol::VariablesArguments &args) override; + lldb::SBValue FindVariable(llvm::StringRef name) override; [[nodiscard]] lldb::SBValue GetVariable() const { return m_value; }; private: @@ -155,8 +152,6 @@ struct VariableReferenceStorage { static constexpr uint8_t k_permanent_kind_byte = 0b0001; /// Scope reference mask for scope types. static constexpr uint8_t k_scope_kind_byte = 0b0010; - static constexpr uint32_t max_reference_value = - std::numeric_limits<int32_t>::max(); /// The most significant byte is used for storing the reference kind. static constexpr uint32_t k_reference_kind_bit_size = @@ -168,9 +163,13 @@ struct VariableReferenceStorage { // we should be able to store at least 8 million variables for each store // type at every stopped state. static constexpr uint32_t k_min_number_of_variables = 8'000'000; + // Sanity check of the amount of variables we can store. - static_assert(((max_reference_value >> k_reference_kind_bit_size) > - k_min_number_of_variables) && + static constexpr uint32_t max_reference_value = + std::numeric_limits<int32_t>::max(); + static constexpr var_ref_t k_reference_max_size = + max_reference_value >> k_reference_kind_bit_size; + static_assert((k_reference_max_size > k_min_number_of_variables) && "Not enough space to store 8 million variables"); // Ensure reference kind bytes are unique. static_assert(k_temporary_kind_byte != k_permanent_kind_byte, @@ -193,7 +192,7 @@ struct VariableReferenceStorage { class ReferenceKindPool { public: - static constexpr uint8_t k_reference_kind_byte = []() { + static constexpr uint8_t k_reference_kind_byte = [] { switch (Kind) { case eReferenceKindPermanent: return k_permanent_kind_byte; @@ -207,8 +206,6 @@ struct VariableReferenceStorage { static constexpr var_ref_t k_reference_mask = k_reference_kind_byte << k_reference_kind_bit_offset; - static constexpr var_ref_t k_reference_max_size = - (1 << k_reference_kind_bit_offset) - 1; static_assert( ((k_reference_mask + k_min_number_of_variables) < @@ -233,7 +230,7 @@ struct VariableReferenceStorage { return nullptr; } - var_ref_t ClearMask(var_ref_t masked_var_ref) { + [[nodiscard]] var_ref_t ClearMask(var_ref_t masked_var_ref) const { return masked_var_ref & ~k_reference_mask; } @@ -249,7 +246,7 @@ struct VariableReferenceStorage { return next_var_ref | k_reference_mask; } - size_t Size() { return m_pool.size(); } + [[nodiscard]] size_t Size() const { return m_pool.size(); } // non copyable and movable. ReferenceKindPool(const ReferenceKindPool &) = delete; _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
