From 3d0c616ce30cdc3d9c26dda8fdc608a6c85f00a6 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Sun, 18 May 2025 10:57:16 -0700 Subject: [PATCH] [lldb-dap] Move the Variables struct into its own file (#140393) Move the Variables struct out of DAP.h and into its own file to reduce the complexity of the latter. This PR also makes the members that are implementation details private and adds a handful of basic unit tests. --- lldb/tools/lldb-dap/CMakeLists.txt | 1 + lldb/tools/lldb-dap/DAP.cpp | 93 ------------------------ lldb/tools/lldb-dap/DAP.h | 51 +------------ lldb/tools/lldb-dap/Variables.cpp | 105 +++++++++++++++++++++++++++ lldb/tools/lldb-dap/Variables.h | 71 ++++++++++++++++++ lldb/unittests/DAP/CMakeLists.txt | 1 + lldb/unittests/DAP/VariablesTest.cpp | 85 ++++++++++++++++++++++ 7 files changed, 264 insertions(+), 143 deletions(-) create mode 100644 lldb/tools/lldb-dap/Variables.cpp create mode 100644 lldb/tools/lldb-dap/Variables.h create mode 100644 lldb/unittests/DAP/VariablesTest.cpp diff --git a/lldb/tools/lldb-dap/CMakeLists.txt b/lldb/tools/lldb-dap/CMakeLists.txt index 5dedee8a87f4..f8e81eaff860 100644 --- a/lldb/tools/lldb-dap/CMakeLists.txt +++ b/lldb/tools/lldb-dap/CMakeLists.txt @@ -23,6 +23,7 @@ add_lldb_library(lldbDAP RunInTerminal.cpp SourceBreakpoint.cpp Transport.cpp + Variables.cpp Watchpoint.cpp Handler/ResponseHandler.cpp diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp index d813cdb0b907..886813162224 100644 --- a/lldb/tools/lldb-dap/DAP.cpp +++ b/lldb/tools/lldb-dap/DAP.cpp @@ -1017,45 +1017,6 @@ lldb::SBError DAP::WaitForProcessToStop(std::chrono::seconds seconds) { return error; } -void Variables::Clear() { - locals.Clear(); - globals.Clear(); - registers.Clear(); - referenced_variables.clear(); -} - -int64_t Variables::GetNewVariableReference(bool is_permanent) { - if (is_permanent) - return next_permanent_var_ref++; - return next_temporary_var_ref++; -} - -bool Variables::IsPermanentVariableReference(int64_t var_ref) { - return var_ref >= PermanentVariableStartIndex; -} - -lldb::SBValue Variables::GetVariable(int64_t var_ref) const { - if (IsPermanentVariableReference(var_ref)) { - auto pos = referenced_permanent_variables.find(var_ref); - if (pos != referenced_permanent_variables.end()) - return pos->second; - } else { - auto pos = referenced_variables.find(var_ref); - if (pos != referenced_variables.end()) - return pos->second; - } - return lldb::SBValue(); -} - -int64_t Variables::InsertVariable(lldb::SBValue variable, bool is_permanent) { - int64_t var_ref = GetNewVariableReference(is_permanent); - if (is_permanent) - referenced_permanent_variables.insert(std::make_pair(var_ref, variable)); - else - referenced_variables.insert(std::make_pair(var_ref, variable)); - return var_ref; -} - bool StartDebuggingRequestHandler::DoExecute( lldb::SBDebugger debugger, char **command, lldb::SBCommandReturnObject &result) { @@ -1292,60 +1253,6 @@ DAP::GetInstructionBPFromStopReason(lldb::SBThread &thread) { return inst_bp; } -lldb::SBValueList *Variables::GetTopLevelScope(int64_t variablesReference) { - switch (variablesReference) { - case VARREF_LOCALS: - return &locals; - case VARREF_GLOBALS: - return &globals; - case VARREF_REGS: - return ®isters; - default: - return nullptr; - } -} - -lldb::SBValue Variables::FindVariable(uint64_t variablesReference, - llvm::StringRef name) { - lldb::SBValue variable; - if (lldb::SBValueList *top_scope = GetTopLevelScope(variablesReference)) { - bool is_duplicated_variable_name = name.contains(" @"); - // variablesReference is one of our scopes, not an actual variable it is - // asking for a variable in locals or globals or registers - int64_t end_idx = top_scope->GetSize(); - // Searching backward so that we choose the variable in closest scope - // among variables of the same name. - for (int64_t i = end_idx - 1; i >= 0; --i) { - lldb::SBValue curr_variable = top_scope->GetValueAtIndex(i); - 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); - } - } - } - } - return variable; -} - protocol::Capabilities DAP::GetCapabilities() { protocol::Capabilities capabilities; diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index 975e2b290e1f..eafc92776fca 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -20,6 +20,7 @@ #include "Protocol/ProtocolTypes.h" #include "SourceBreakpoint.h" #include "Transport.h" +#include "Variables.h" #include "lldb/API/SBBroadcaster.h" #include "lldb/API/SBCommandInterpreter.h" #include "lldb/API/SBDebugger.h" @@ -30,8 +31,6 @@ #include "lldb/API/SBMutex.h" #include "lldb/API/SBTarget.h" #include "lldb/API/SBThread.h" -#include "lldb/API/SBValue.h" -#include "lldb/API/SBValueList.h" #include "lldb/lldb-types.h" #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/DenseSet.h" @@ -52,10 +51,6 @@ #include #include -#define VARREF_LOCALS (int64_t)1 -#define VARREF_GLOBALS (int64_t)2 -#define VARREF_REGS (int64_t)3 -#define VARREF_FIRST_VAR_IDX (int64_t)4 #define NO_TYPENAME "" namespace lldb_dap { @@ -88,50 +83,6 @@ enum class PacketStatus { enum class ReplMode { Variable = 0, Command, Auto }; -struct Variables { - /// Variable_reference start index of permanent expandable variable. - static constexpr int64_t PermanentVariableStartIndex = (1ll << 32); - - lldb::SBValueList locals; - lldb::SBValueList globals; - lldb::SBValueList registers; - - int64_t next_temporary_var_ref{VARREF_FIRST_VAR_IDX}; - int64_t next_permanent_var_ref{PermanentVariableStartIndex}; - - /// Variables that are alive in this stop state. - /// Will be cleared when debuggee resumes. - llvm::DenseMap referenced_variables; - /// Variables that persist across entire debug session. - /// These are the variables evaluated from debug console REPL. - llvm::DenseMap referenced_permanent_variables; - - /// Check if \p var_ref points to a variable that should persist for the - /// entire duration of the debug session, e.g. repl expandable variables - static bool IsPermanentVariableReference(int64_t var_ref); - - /// \return a new variableReference. - /// Specify is_permanent as true for variable that should persist entire - /// debug session. - int64_t GetNewVariableReference(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; - - /// Insert a new \p variable. - /// \return variableReference assigned to this expandable variable. - int64_t InsertVariable(lldb::SBValue variable, bool is_permanent); - - lldb::SBValueList *GetTopLevelScope(int64_t variablesReference); - - lldb::SBValue FindVariable(uint64_t variablesReference, llvm::StringRef name); - - /// Clear all scope variables and non-permanent expandable variables. - void Clear(); -}; - struct StartDebuggingRequestHandler : public lldb::SBCommandPluginInterface { DAP &dap; explicit StartDebuggingRequestHandler(DAP &d) : dap(d) {}; diff --git a/lldb/tools/lldb-dap/Variables.cpp b/lldb/tools/lldb-dap/Variables.cpp new file mode 100644 index 000000000000..777e3183d8c0 --- /dev/null +++ b/lldb/tools/lldb-dap/Variables.cpp @@ -0,0 +1,105 @@ +//===-- Variables.cpp ---------------------------------------------------*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "Variables.h" +#include "JSONUtils.h" + +using namespace lldb_dap; + +lldb::SBValueList *Variables::GetTopLevelScope(int64_t variablesReference) { + switch (variablesReference) { + case VARREF_LOCALS: + return &locals; + case VARREF_GLOBALS: + return &globals; + case VARREF_REGS: + return ®isters; + default: + return nullptr; + } +} + +void Variables::Clear() { + locals.Clear(); + globals.Clear(); + registers.Clear(); + m_referencedvariables.clear(); +} + +int64_t Variables::GetNewVariableReference(bool is_permanent) { + if (is_permanent) + return m_next_permanent_var_ref++; + return m_next_temporary_var_ref++; +} + +bool Variables::IsPermanentVariableReference(int64_t var_ref) { + return var_ref >= PermanentVariableStartIndex; +} + +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; + } + 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; +} + +lldb::SBValue Variables::FindVariable(uint64_t variablesReference, + llvm::StringRef name) { + lldb::SBValue variable; + if (lldb::SBValueList *top_scope = GetTopLevelScope(variablesReference)) { + bool is_duplicated_variable_name = name.contains(" @"); + // variablesReference is one of our scopes, not an actual variable it is + // asking for a variable in locals or globals or registers + int64_t end_idx = top_scope->GetSize(); + // Searching backward so that we choose the variable in closest scope + // among variables of the same name. + for (int64_t i = end_idx - 1; i >= 0; --i) { + lldb::SBValue curr_variable = top_scope->GetValueAtIndex(i); + 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); + } + } + } + } + return variable; +} diff --git a/lldb/tools/lldb-dap/Variables.h b/lldb/tools/lldb-dap/Variables.h new file mode 100644 index 000000000000..0ed84b36aef9 --- /dev/null +++ b/lldb/tools/lldb-dap/Variables.h @@ -0,0 +1,71 @@ +//===-- Variables.h -----------------------------------------------------*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLDB_TOOLS_LLDB_DAP_VARIABLES_H +#define LLDB_TOOLS_LLDB_DAP_VARIABLES_H + +#include "lldb/API/SBValue.h" +#include "lldb/API/SBValueList.h" +#include "llvm/ADT/DenseMap.h" + +#define VARREF_FIRST_VAR_IDX (int64_t)4 +#define VARREF_LOCALS (int64_t)1 +#define VARREF_GLOBALS (int64_t)2 +#define VARREF_REGS (int64_t)3 + +namespace lldb_dap { + +struct Variables { + lldb::SBValueList locals; + lldb::SBValueList globals; + lldb::SBValueList registers; + + /// 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); + + /// \return a new variableReference. + /// Specify is_permanent as true for variable that should persist entire + /// debug session. + int64_t GetNewVariableReference(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; + + /// Insert a new \p variable. + /// \return variableReference assigned to this expandable variable. + int64_t InsertVariable(lldb::SBValue variable, bool is_permanent); + + lldb::SBValueList *GetTopLevelScope(int64_t variablesReference); + + lldb::SBValue FindVariable(uint64_t variablesReference, llvm::StringRef name); + + /// Clear all scope variables and non-permanent expandable variables. + void Clear(); + +private: + /// Variable_reference start index of permanent expandable variable. + static constexpr int64_t PermanentVariableStartIndex = (1ll << 32); + + /// Variables that are alive in this stop state. + /// Will be cleared when debuggee resumes. + llvm::DenseMap m_referencedvariables; + + /// Variables that persist across entire debug session. + /// These are the variables evaluated from debug console REPL. + llvm::DenseMap m_referencedpermanent_variables; + + int64_t m_next_temporary_var_ref{VARREF_FIRST_VAR_IDX}; + int64_t m_next_permanent_var_ref{PermanentVariableStartIndex}; +}; + +} // namespace lldb_dap + +#endif diff --git a/lldb/unittests/DAP/CMakeLists.txt b/lldb/unittests/DAP/CMakeLists.txt index af7d11e2e95e..429a12e9fb50 100644 --- a/lldb/unittests/DAP/CMakeLists.txt +++ b/lldb/unittests/DAP/CMakeLists.txt @@ -6,6 +6,7 @@ add_lldb_unittest(DAPTests ProtocolTypesTest.cpp TestBase.cpp TransportTest.cpp + VariablesTest.cpp LINK_LIBS lldbDAP diff --git a/lldb/unittests/DAP/VariablesTest.cpp b/lldb/unittests/DAP/VariablesTest.cpp new file mode 100644 index 000000000000..6b14fc6c3945 --- /dev/null +++ b/lldb/unittests/DAP/VariablesTest.cpp @@ -0,0 +1,85 @@ +//===-- VariablesTest.cpp -------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "Variables.h" +#include "lldb/API/SBValue.h" +#include "lldb/API/SBValueList.h" +#include "gtest/gtest.h" + +using namespace lldb_dap; + +class VariablesTest : public ::testing::Test { +protected: + enum : bool { Permanent = true, Temporary = false }; + Variables vars; +}; + +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); + + EXPECT_NE(temp1, temp2); + EXPECT_NE(perm1, perm2); + EXPECT_LT(temp1, perm1); + EXPECT_LT(temp2, perm1); +} + +TEST_F(VariablesTest, InsertAndGetVariable_Temporary) { + lldb::SBValue dummy; + const int64_t ref = vars.InsertVariable(dummy, Temporary); + lldb::SBValue out = vars.GetVariable(ref); + + EXPECT_EQ(out.IsValid(), dummy.IsValid()); +} + +TEST_F(VariablesTest, InsertAndGetVariable_Permanent) { + lldb::SBValue dummy; + const int64_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.GetNewVariableReference(Permanent); + const int64_t temp = vars.GetNewVariableReference(Temporary); + + EXPECT_TRUE(Variables::IsPermanentVariableReference(perm)); + EXPECT_FALSE(Variables::IsPermanentVariableReference(temp)); +} + +TEST_F(VariablesTest, Clear_RemovesTemporaryKeepsPermanent) { + lldb::SBValue dummy; + const int64_t temp = vars.InsertVariable(dummy, Temporary); + const int64_t perm = vars.InsertVariable(dummy, Permanent); + vars.Clear(); + + EXPECT_FALSE(vars.GetVariable(temp).IsValid()); + EXPECT_EQ(vars.GetVariable(perm).IsValid(), dummy.IsValid()); +} + +TEST_F(VariablesTest, GetTopLevelScope_ReturnsCorrectScope) { + vars.locals.Append(lldb::SBValue()); + vars.globals.Append(lldb::SBValue()); + vars.registers.Append(lldb::SBValue()); + + EXPECT_EQ(vars.GetTopLevelScope(VARREF_LOCALS), &vars.locals); + EXPECT_EQ(vars.GetTopLevelScope(VARREF_GLOBALS), &vars.globals); + EXPECT_EQ(vars.GetTopLevelScope(VARREF_REGS), &vars.registers); + EXPECT_EQ(vars.GetTopLevelScope(9999), nullptr); +} + +TEST_F(VariablesTest, FindVariable_LocalsByName) { + lldb::SBValue dummy; + vars.locals.Append(dummy); + lldb::SBValue found = vars.FindVariable(VARREF_LOCALS, ""); + + EXPECT_EQ(found.IsValid(), dummy.IsValid()); +}