llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Ebuka Ezike (da-viper) <details> <summary>Changes</summary> 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 | ``` --- Patch is 52.12 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/179262.diff 13 Files Affected: - (modified) lldb/tools/lldb-dap/DAP.h (+2-2) - (modified) lldb/tools/lldb-dap/Handler/DataBreakpointInfoRequestHandler.cpp (+1-1) - (modified) lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp (+1-1) - (modified) lldb/tools/lldb-dap/Handler/LocationsRequestHandler.cpp (+1-1) - (modified) lldb/tools/lldb-dap/Handler/ScopesRequestHandler.cpp (+1-1) - (modified) lldb/tools/lldb-dap/Handler/SetVariableRequestHandler.cpp (+4-3) - (modified) lldb/tools/lldb-dap/Handler/VariablesRequestHandler.cpp (+6-144) - (modified) lldb/tools/lldb-dap/Protocol/ProtocolRequests.h (+5-5) - (modified) lldb/tools/lldb-dap/Protocol/ProtocolTypes.h (+3-3) - (modified) lldb/tools/lldb-dap/SBAPIExtras.h (+5) - (modified) lldb/tools/lldb-dap/Variables.cpp (+287-117) - (modified) lldb/tools/lldb-dap/Variables.h (+182-56) - (modified) lldb/unittests/DAP/VariablesTest.cpp (+33-29) ``````````diff diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index 8d56a226dbb28..2f5fcd7783d55 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; @@ -378,7 +378,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 ec26bb66e8aec..e828dbb970dd5 100644 --- a/lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp @@ -93,7 +93,7 @@ EvaluateRequestHandler::Run(const EvaluateArguments &arguments) const { body.type = desc.display_type_name; if (value.MightHaveChildren() || ValuePointsToCode(value)) - body.variablesReference = dap.variables.InsertVariable( + body.variablesReference = dap.reference_storage.InsertVariable( value, /*is_permanent=*/arguments.context == eEvaluateContextRepl); if (lldb::addr_t addr = value.GetLoadAddress(); addr != LLDB_INVALID_ADDRESS) 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 0745b2e043c21..4f5320fcfbf1d 100644 --- a/lldb/tools/lldb-dap/SBAPIExtras.h +++ b/lldb/tools/lldb-dap/SBAPIExtras.h @@ -46,6 +46,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); - ... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/179262 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
