[Lldb-commits] [lldb] [LLDB] Optimize identifier lookup in DIL (PR #146094)
via lldb-commits
lldb-commits at lists.llvm.org
Fri Jun 27 08:30:32 PDT 2025
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
More information about the lldb-commits
mailing list