llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Ilia Kuklin (kuilpd) <details> <summary>Changes</summary> Remove unused code and unnecessary function calls, optimize global variable search. Add more test cases. --- Full diff: https://github.com/llvm/llvm-project/pull/146094.diff 6 Files Affected: - (modified) lldb/include/lldb/ValueObject/DILEval.h (+2-4) - (modified) lldb/source/ValueObject/DILEval.cpp (+52-120) - (modified) lldb/test/API/commands/frame/var-dil/basics/GlobalVariableLookup/Makefile (+1-1) - (modified) lldb/test/API/commands/frame/var-dil/basics/GlobalVariableLookup/TestFrameVarDILGlobalVariableLookup.py (+11-11) - (added) lldb/test/API/commands/frame/var-dil/basics/GlobalVariableLookup/extern.cpp (+10) - (modified) lldb/test/API/commands/frame/var-dil/basics/GlobalVariableLookup/main.cpp (+3) ``````````diff diff --git a/lldb/include/lldb/ValueObject/DILEval.h b/lldb/include/lldb/ValueObject/DILEval.h index 2a0cb548a810f..45e29b3ddcd7b 100644 --- a/lldb/include/lldb/ValueObject/DILEval.h +++ b/lldb/include/lldb/ValueObject/DILEval.h @@ -25,8 +25,7 @@ namespace lldb_private::dil { /// evaluating). lldb::ValueObjectSP LookupIdentifier(llvm::StringRef name_ref, std::shared_ptr<StackFrame> frame_sp, - lldb::DynamicValueType use_dynamic, - CompilerType *scope_ptr = nullptr); + lldb::DynamicValueType use_dynamic); /// Given the name of an identifier, check to see if it matches the name of a /// global variable. If so, find the ValueObject for that global variable, and @@ -35,8 +34,7 @@ lldb::ValueObjectSP LookupIdentifier(llvm::StringRef name_ref, lldb::ValueObjectSP LookupGlobalIdentifier(llvm::StringRef name_ref, std::shared_ptr<StackFrame> frame_sp, lldb::TargetSP target_sp, - lldb::DynamicValueType use_dynamic, - CompilerType *scope_ptr = nullptr); + lldb::DynamicValueType use_dynamic); class Interpreter : Visitor { public: diff --git a/lldb/source/ValueObject/DILEval.cpp b/lldb/source/ValueObject/DILEval.cpp index b2bb4e20ddc24..f75f104ba2bc5 100644 --- a/lldb/source/ValueObject/DILEval.cpp +++ b/lldb/source/ValueObject/DILEval.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "lldb/ValueObject/DILEval.h" +#include "lldb/Symbol/CompileUnit.h" #include "lldb/Symbol/VariableList.h" #include "lldb/Target/RegisterContext.h" #include "lldb/ValueObject/DILAST.h" @@ -18,70 +19,26 @@ namespace lldb_private::dil { -static lldb::ValueObjectSP LookupStaticIdentifier( - VariableList &variable_list, std::shared_ptr<StackFrame> exe_scope, - llvm::StringRef name_ref, llvm::StringRef unqualified_name) { - // First look for an exact match to the (possibly) qualified name. - for (const lldb::VariableSP &var_sp : variable_list) { - lldb::ValueObjectSP valobj_sp( - ValueObjectVariable::Create(exe_scope.get(), var_sp)); - if (valobj_sp && valobj_sp->GetVariable() && - (valobj_sp->GetVariable()->NameMatches(ConstString(name_ref)))) - return valobj_sp; - } - - // If the qualified name is the same as the unqualfied name, there's nothing - // more to be done. - if (name_ref == unqualified_name) - return nullptr; - - // We didn't match the qualified name; try to match the unqualified name. - for (const lldb::VariableSP &var_sp : variable_list) { - lldb::ValueObjectSP valobj_sp( - ValueObjectVariable::Create(exe_scope.get(), var_sp)); - if (valobj_sp && valobj_sp->GetVariable() && - (valobj_sp->GetVariable()->NameMatches(ConstString(unqualified_name)))) - return valobj_sp; - } - - return nullptr; -} - static lldb::VariableSP DILFindVariable(ConstString name, - lldb::VariableListSP variable_list) { + VariableList &variable_list) { lldb::VariableSP exact_match; std::vector<lldb::VariableSP> possible_matches; - for (lldb::VariableSP var_sp : *variable_list) { + for (lldb::VariableSP var_sp : variable_list) { llvm::StringRef str_ref_name = var_sp->GetName().GetStringRef(); - // Check for global vars, which might start with '::'. - str_ref_name.consume_front("::"); - if (str_ref_name == name.GetStringRef()) - possible_matches.push_back(var_sp); - else if (var_sp->NameMatches(name)) - possible_matches.push_back(var_sp); - } - - // Look for exact matches (favors local vars over global vars) - auto exact_match_it = - llvm::find_if(possible_matches, [&](lldb::VariableSP var_sp) { - return var_sp->GetName() == name; - }); - - if (exact_match_it != possible_matches.end()) - return *exact_match_it; - - // Look for a global var exact match. - for (auto var_sp : possible_matches) { - llvm::StringRef str_ref_name = var_sp->GetName().GetStringRef(); str_ref_name.consume_front("::"); + // Check for the exact same match if (str_ref_name == name.GetStringRef()) return var_sp; + + // Check for possible matches by base name + if (var_sp->NameMatches(name)) + possible_matches.push_back(var_sp); } - // If there's a single non-exact match, take it. - if (possible_matches.size() == 1) + // If there's a non-exact match, take it. + if (possible_matches.size() > 0) return possible_matches[0]; return nullptr; @@ -89,38 +46,23 @@ static lldb::VariableSP DILFindVariable(ConstString name, lldb::ValueObjectSP LookupGlobalIdentifier( llvm::StringRef name_ref, std::shared_ptr<StackFrame> stack_frame, - lldb::TargetSP target_sp, lldb::DynamicValueType use_dynamic, - CompilerType *scope_ptr) { - // First look for match in "local" global variables. - lldb::VariableListSP variable_list(stack_frame->GetInScopeVariableList(true)); - name_ref.consume_front("::"); + lldb::TargetSP target_sp, lldb::DynamicValueType use_dynamic) { + // Get a global variables list without the locals from the current frame + SymbolContext symbol_context = + stack_frame->GetSymbolContext(lldb::eSymbolContextCompUnit); + lldb::VariableListSP variable_list = + symbol_context.comp_unit->GetVariableList(true); + name_ref.consume_front("::"); lldb::ValueObjectSP value_sp; if (variable_list) { lldb::VariableSP var_sp = - DILFindVariable(ConstString(name_ref), variable_list); + DILFindVariable(ConstString(name_ref), *variable_list); if (var_sp) value_sp = stack_frame->GetValueObjectForFrameVariable(var_sp, use_dynamic); } - if (value_sp) - return value_sp; - - // Also check for static global vars. - if (variable_list) { - const char *type_name = ""; - if (scope_ptr) - type_name = scope_ptr->GetCanonicalType().GetTypeName().AsCString(); - std::string name_with_type_prefix = - llvm::formatv("{0}::{1}", type_name, name_ref).str(); - value_sp = LookupStaticIdentifier(*variable_list, stack_frame, - name_with_type_prefix, name_ref); - if (!value_sp) - value_sp = LookupStaticIdentifier(*variable_list, stack_frame, name_ref, - name_ref); - } - if (value_sp) return value_sp; @@ -129,28 +71,22 @@ lldb::ValueObjectSP LookupGlobalIdentifier( target_sp->GetImages().FindGlobalVariables( ConstString(name_ref), std::numeric_limits<uint32_t>::max(), modules_var_list); - if (modules_var_list.Empty()) - return nullptr; - for (const lldb::VariableSP &var_sp : modules_var_list) { - std::string qualified_name = llvm::formatv("::{0}", name_ref).str(); - if (var_sp->NameMatches(ConstString(name_ref)) || - var_sp->NameMatches(ConstString(qualified_name))) { + if (!modules_var_list.Empty()) { + lldb::VariableSP var_sp = + DILFindVariable(ConstString(name_ref), modules_var_list); + if (var_sp) value_sp = ValueObjectVariable::Create(stack_frame.get(), var_sp); - break; - } - } - - if (value_sp) - return value_sp; + if (value_sp) + return value_sp; + } return nullptr; } lldb::ValueObjectSP LookupIdentifier(llvm::StringRef name_ref, std::shared_ptr<StackFrame> stack_frame, - lldb::DynamicValueType use_dynamic, - CompilerType *scope_ptr) { + lldb::DynamicValueType use_dynamic) { // Support $rax as a special syntax for accessing registers. // Will return an invalid value in case the requested register doesn't exist. if (name_ref.consume_front("$")) { @@ -164,38 +100,34 @@ lldb::ValueObjectSP LookupIdentifier(llvm::StringRef name_ref, return nullptr; } - lldb::VariableListSP variable_list( - stack_frame->GetInScopeVariableList(false)); - if (!name_ref.contains("::")) { - if (!scope_ptr || !scope_ptr->IsValid()) { - // Lookup in the current frame. - // Try looking for a local variable in current scope. - lldb::ValueObjectSP value_sp; - if (variable_list) { - lldb::VariableSP var_sp = - DILFindVariable(ConstString(name_ref), variable_list); - if (var_sp) - value_sp = - stack_frame->GetValueObjectForFrameVariable(var_sp, use_dynamic); - } - if (!value_sp) - value_sp = stack_frame->FindVariable(ConstString(name_ref)); - - if (value_sp) - return value_sp; - - // Try looking for an instance variable (class member). - SymbolContext sc = stack_frame->GetSymbolContext( - lldb::eSymbolContextFunction | lldb::eSymbolContextBlock); - llvm::StringRef ivar_name = sc.GetInstanceVariableName(); - value_sp = stack_frame->FindVariable(ConstString(ivar_name)); - if (value_sp) - value_sp = value_sp->GetChildMemberWithName(name_ref); - - if (value_sp) - return value_sp; + // Lookup in the current frame. + // Try looking for a local variable in current scope. + lldb::VariableListSP variable_list( + stack_frame->GetInScopeVariableList(false)); + + lldb::ValueObjectSP value_sp; + if (variable_list) { + lldb::VariableSP var_sp = + variable_list->FindVariable(ConstString(name_ref)); + if (var_sp) + value_sp = + stack_frame->GetValueObjectForFrameVariable(var_sp, use_dynamic); } + + if (value_sp) + return value_sp; + + // Try looking for an instance variable (class member). + SymbolContext sc = stack_frame->GetSymbolContext( + lldb::eSymbolContextFunction | lldb::eSymbolContextBlock); + llvm::StringRef ivar_name = sc.GetInstanceVariableName(); + value_sp = stack_frame->FindVariable(ConstString(ivar_name)); + if (value_sp) + value_sp = value_sp->GetChildMemberWithName(name_ref); + + if (value_sp) + return value_sp; } return nullptr; } diff --git a/lldb/test/API/commands/frame/var-dil/basics/GlobalVariableLookup/Makefile b/lldb/test/API/commands/frame/var-dil/basics/GlobalVariableLookup/Makefile index 99998b20bcb05..c39bce9a94dcf 100644 --- a/lldb/test/API/commands/frame/var-dil/basics/GlobalVariableLookup/Makefile +++ b/lldb/test/API/commands/frame/var-dil/basics/GlobalVariableLookup/Makefile @@ -1,3 +1,3 @@ -CXX_SOURCES := main.cpp +CXX_SOURCES := main.cpp extern.cpp include Makefile.rules diff --git a/lldb/test/API/commands/frame/var-dil/basics/GlobalVariableLookup/TestFrameVarDILGlobalVariableLookup.py b/lldb/test/API/commands/frame/var-dil/basics/GlobalVariableLookup/TestFrameVarDILGlobalVariableLookup.py index edb013c7b3526..8a8f068c19466 100644 --- a/lldb/test/API/commands/frame/var-dil/basics/GlobalVariableLookup/TestFrameVarDILGlobalVariableLookup.py +++ b/lldb/test/API/commands/frame/var-dil/basics/GlobalVariableLookup/TestFrameVarDILGlobalVariableLookup.py @@ -32,20 +32,20 @@ def test_frame_var(self): self.expect_var_path("::globalPtr", type="int *") self.expect_var_path("::globalRef", type="int &") - self.expect( - "frame variable 'externGlobalVar'", - error=True, - substrs=["use of undeclared identifier"], - ) # 0x00C0FFEE - self.expect( - "frame variable '::externGlobalVar'", - error=True, - substrs=["use of undeclared identifier"], - ) # ["12648430"]) - self.expect_var_path("ns::globalVar", value="13") self.expect_var_path("ns::globalPtr", type="int *") self.expect_var_path("ns::globalRef", type="int &") self.expect_var_path("::ns::globalVar", value="13") self.expect_var_path("::ns::globalPtr", type="int *") self.expect_var_path("::ns::globalRef", type="int &") + + self.expect_var_path("externGlobalVar", value="2") + self.expect_var_path("::externGlobalVar", value="2") + self.expect_var_path("ext::externGlobalVar", value="4") + self.expect_var_path("::ext::externGlobalVar", value="4") + + self.expect_var_path("ExtStruct::static_inline", value="16") + + # Test local variable priority over global + self.expect_var_path("foo", value="1") + self.expect_var_path("::foo", value="2") diff --git a/lldb/test/API/commands/frame/var-dil/basics/GlobalVariableLookup/extern.cpp b/lldb/test/API/commands/frame/var-dil/basics/GlobalVariableLookup/extern.cpp new file mode 100644 index 0000000000000..c451191dafc57 --- /dev/null +++ b/lldb/test/API/commands/frame/var-dil/basics/GlobalVariableLookup/extern.cpp @@ -0,0 +1,10 @@ +int externGlobalVar = 2; + +namespace ext { +int externGlobalVar = 4; +} // namespace ext + +struct ExtStruct { +private: + static constexpr inline int static_inline = 16; +} es; diff --git a/lldb/test/API/commands/frame/var-dil/basics/GlobalVariableLookup/main.cpp b/lldb/test/API/commands/frame/var-dil/basics/GlobalVariableLookup/main.cpp index 5bae4fd423e32..a8ecbe2d8fc44 100644 --- a/lldb/test/API/commands/frame/var-dil/basics/GlobalVariableLookup/main.cpp +++ b/lldb/test/API/commands/frame/var-dil/basics/GlobalVariableLookup/main.cpp @@ -10,6 +10,9 @@ int *globalPtr = &globalVar; int &globalRef = globalVar; } // namespace ns +int foo = 2; + int main(int argc, char **argv) { + int foo = 1; return 0; // Set a breakpoint here } `````````` </details> https://github.com/llvm/llvm-project/pull/146094 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
