Skip to content

Commit 03aa5c0

Browse files
authored
[LLDB] Optimize identifier lookup in DIL (#146094)
Remove unused code and unnecessary function calls, optimize global variable search. Add more test cases.
1 parent 438863a commit 03aa5c0

File tree

6 files changed

+79
-136
lines changed

6 files changed

+79
-136
lines changed

lldb/include/lldb/ValueObject/DILEval.h

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,7 @@ namespace lldb_private::dil {
2525
/// evaluating).
2626
lldb::ValueObjectSP LookupIdentifier(llvm::StringRef name_ref,
2727
std::shared_ptr<StackFrame> frame_sp,
28-
lldb::DynamicValueType use_dynamic,
29-
CompilerType *scope_ptr = nullptr);
28+
lldb::DynamicValueType use_dynamic);
3029

3130
/// Given the name of an identifier, check to see if it matches the name of a
3231
/// global variable. If so, find the ValueObject for that global variable, and
@@ -35,8 +34,7 @@ lldb::ValueObjectSP LookupIdentifier(llvm::StringRef name_ref,
3534
lldb::ValueObjectSP LookupGlobalIdentifier(llvm::StringRef name_ref,
3635
std::shared_ptr<StackFrame> frame_sp,
3736
lldb::TargetSP target_sp,
38-
lldb::DynamicValueType use_dynamic,
39-
CompilerType *scope_ptr = nullptr);
37+
lldb::DynamicValueType use_dynamic);
4038

4139
class Interpreter : Visitor {
4240
public:

lldb/source/ValueObject/DILEval.cpp

Lines changed: 52 additions & 120 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
//===----------------------------------------------------------------------===//
88

99
#include "lldb/ValueObject/DILEval.h"
10+
#include "lldb/Symbol/CompileUnit.h"
1011
#include "lldb/Symbol/VariableList.h"
1112
#include "lldb/Target/RegisterContext.h"
1213
#include "lldb/ValueObject/DILAST.h"
@@ -18,109 +19,50 @@
1819

1920
namespace lldb_private::dil {
2021

21-
static lldb::ValueObjectSP LookupStaticIdentifier(
22-
VariableList &variable_list, std::shared_ptr<StackFrame> exe_scope,
23-
llvm::StringRef name_ref, llvm::StringRef unqualified_name) {
24-
// First look for an exact match to the (possibly) qualified name.
25-
for (const lldb::VariableSP &var_sp : variable_list) {
26-
lldb::ValueObjectSP valobj_sp(
27-
ValueObjectVariable::Create(exe_scope.get(), var_sp));
28-
if (valobj_sp && valobj_sp->GetVariable() &&
29-
(valobj_sp->GetVariable()->NameMatches(ConstString(name_ref))))
30-
return valobj_sp;
31-
}
32-
33-
// If the qualified name is the same as the unqualfied name, there's nothing
34-
// more to be done.
35-
if (name_ref == unqualified_name)
36-
return nullptr;
37-
38-
// We didn't match the qualified name; try to match the unqualified name.
39-
for (const lldb::VariableSP &var_sp : variable_list) {
40-
lldb::ValueObjectSP valobj_sp(
41-
ValueObjectVariable::Create(exe_scope.get(), var_sp));
42-
if (valobj_sp && valobj_sp->GetVariable() &&
43-
(valobj_sp->GetVariable()->NameMatches(ConstString(unqualified_name))))
44-
return valobj_sp;
45-
}
46-
47-
return nullptr;
48-
}
49-
5022
static lldb::VariableSP DILFindVariable(ConstString name,
51-
lldb::VariableListSP variable_list) {
23+
VariableList &variable_list) {
5224
lldb::VariableSP exact_match;
5325
std::vector<lldb::VariableSP> possible_matches;
5426

55-
for (lldb::VariableSP var_sp : *variable_list) {
27+
for (lldb::VariableSP var_sp : variable_list) {
5628
llvm::StringRef str_ref_name = var_sp->GetName().GetStringRef();
57-
// Check for global vars, which might start with '::'.
58-
str_ref_name.consume_front("::");
5929

60-
if (str_ref_name == name.GetStringRef())
61-
possible_matches.push_back(var_sp);
62-
else if (var_sp->NameMatches(name))
63-
possible_matches.push_back(var_sp);
64-
}
65-
66-
// Look for exact matches (favors local vars over global vars)
67-
auto exact_match_it =
68-
llvm::find_if(possible_matches, [&](lldb::VariableSP var_sp) {
69-
return var_sp->GetName() == name;
70-
});
71-
72-
if (exact_match_it != possible_matches.end())
73-
return *exact_match_it;
74-
75-
// Look for a global var exact match.
76-
for (auto var_sp : possible_matches) {
77-
llvm::StringRef str_ref_name = var_sp->GetName().GetStringRef();
7830
str_ref_name.consume_front("::");
31+
// Check for the exact same match
7932
if (str_ref_name == name.GetStringRef())
8033
return var_sp;
34+
35+
// Check for possible matches by base name
36+
if (var_sp->NameMatches(name))
37+
possible_matches.push_back(var_sp);
8138
}
8239

83-
// If there's a single non-exact match, take it.
84-
if (possible_matches.size() == 1)
40+
// If there's a non-exact match, take it.
41+
if (possible_matches.size() > 0)
8542
return possible_matches[0];
8643

8744
return nullptr;
8845
}
8946

9047
lldb::ValueObjectSP LookupGlobalIdentifier(
9148
llvm::StringRef name_ref, std::shared_ptr<StackFrame> stack_frame,
92-
lldb::TargetSP target_sp, lldb::DynamicValueType use_dynamic,
93-
CompilerType *scope_ptr) {
94-
// First look for match in "local" global variables.
95-
lldb::VariableListSP variable_list(stack_frame->GetInScopeVariableList(true));
96-
name_ref.consume_front("::");
49+
lldb::TargetSP target_sp, lldb::DynamicValueType use_dynamic) {
50+
// Get a global variables list without the locals from the current frame
51+
SymbolContext symbol_context =
52+
stack_frame->GetSymbolContext(lldb::eSymbolContextCompUnit);
53+
lldb::VariableListSP variable_list =
54+
symbol_context.comp_unit->GetVariableList(true);
9755

56+
name_ref.consume_front("::");
9857
lldb::ValueObjectSP value_sp;
9958
if (variable_list) {
10059
lldb::VariableSP var_sp =
101-
DILFindVariable(ConstString(name_ref), variable_list);
60+
DILFindVariable(ConstString(name_ref), *variable_list);
10261
if (var_sp)
10362
value_sp =
10463
stack_frame->GetValueObjectForFrameVariable(var_sp, use_dynamic);
10564
}
10665

107-
if (value_sp)
108-
return value_sp;
109-
110-
// Also check for static global vars.
111-
if (variable_list) {
112-
const char *type_name = "";
113-
if (scope_ptr)
114-
type_name = scope_ptr->GetCanonicalType().GetTypeName().AsCString();
115-
std::string name_with_type_prefix =
116-
llvm::formatv("{0}::{1}", type_name, name_ref).str();
117-
value_sp = LookupStaticIdentifier(*variable_list, stack_frame,
118-
name_with_type_prefix, name_ref);
119-
if (!value_sp)
120-
value_sp = LookupStaticIdentifier(*variable_list, stack_frame, name_ref,
121-
name_ref);
122-
}
123-
12466
if (value_sp)
12567
return value_sp;
12668

@@ -129,28 +71,22 @@ lldb::ValueObjectSP LookupGlobalIdentifier(
12971
target_sp->GetImages().FindGlobalVariables(
13072
ConstString(name_ref), std::numeric_limits<uint32_t>::max(),
13173
modules_var_list);
132-
if (modules_var_list.Empty())
133-
return nullptr;
13474

135-
for (const lldb::VariableSP &var_sp : modules_var_list) {
136-
std::string qualified_name = llvm::formatv("::{0}", name_ref).str();
137-
if (var_sp->NameMatches(ConstString(name_ref)) ||
138-
var_sp->NameMatches(ConstString(qualified_name))) {
75+
if (!modules_var_list.Empty()) {
76+
lldb::VariableSP var_sp =
77+
DILFindVariable(ConstString(name_ref), modules_var_list);
78+
if (var_sp)
13979
value_sp = ValueObjectVariable::Create(stack_frame.get(), var_sp);
140-
break;
141-
}
142-
}
143-
144-
if (value_sp)
145-
return value_sp;
14680

81+
if (value_sp)
82+
return value_sp;
83+
}
14784
return nullptr;
14885
}
14986

15087
lldb::ValueObjectSP LookupIdentifier(llvm::StringRef name_ref,
15188
std::shared_ptr<StackFrame> stack_frame,
152-
lldb::DynamicValueType use_dynamic,
153-
CompilerType *scope_ptr) {
89+
lldb::DynamicValueType use_dynamic) {
15490
// Support $rax as a special syntax for accessing registers.
15591
// Will return an invalid value in case the requested register doesn't exist.
15692
if (name_ref.consume_front("$")) {
@@ -164,38 +100,34 @@ lldb::ValueObjectSP LookupIdentifier(llvm::StringRef name_ref,
164100
return nullptr;
165101
}
166102

167-
lldb::VariableListSP variable_list(
168-
stack_frame->GetInScopeVariableList(false));
169-
170103
if (!name_ref.contains("::")) {
171-
if (!scope_ptr || !scope_ptr->IsValid()) {
172-
// Lookup in the current frame.
173-
// Try looking for a local variable in current scope.
174-
lldb::ValueObjectSP value_sp;
175-
if (variable_list) {
176-
lldb::VariableSP var_sp =
177-
DILFindVariable(ConstString(name_ref), variable_list);
178-
if (var_sp)
179-
value_sp =
180-
stack_frame->GetValueObjectForFrameVariable(var_sp, use_dynamic);
181-
}
182-
if (!value_sp)
183-
value_sp = stack_frame->FindVariable(ConstString(name_ref));
184-
185-
if (value_sp)
186-
return value_sp;
187-
188-
// Try looking for an instance variable (class member).
189-
SymbolContext sc = stack_frame->GetSymbolContext(
190-
lldb::eSymbolContextFunction | lldb::eSymbolContextBlock);
191-
llvm::StringRef ivar_name = sc.GetInstanceVariableName();
192-
value_sp = stack_frame->FindVariable(ConstString(ivar_name));
193-
if (value_sp)
194-
value_sp = value_sp->GetChildMemberWithName(name_ref);
195-
196-
if (value_sp)
197-
return value_sp;
104+
// Lookup in the current frame.
105+
// Try looking for a local variable in current scope.
106+
lldb::VariableListSP variable_list(
107+
stack_frame->GetInScopeVariableList(false));
108+
109+
lldb::ValueObjectSP value_sp;
110+
if (variable_list) {
111+
lldb::VariableSP var_sp =
112+
variable_list->FindVariable(ConstString(name_ref));
113+
if (var_sp)
114+
value_sp =
115+
stack_frame->GetValueObjectForFrameVariable(var_sp, use_dynamic);
198116
}
117+
118+
if (value_sp)
119+
return value_sp;
120+
121+
// Try looking for an instance variable (class member).
122+
SymbolContext sc = stack_frame->GetSymbolContext(
123+
lldb::eSymbolContextFunction | lldb::eSymbolContextBlock);
124+
llvm::StringRef ivar_name = sc.GetInstanceVariableName();
125+
value_sp = stack_frame->FindVariable(ConstString(ivar_name));
126+
if (value_sp)
127+
value_sp = value_sp->GetChildMemberWithName(name_ref);
128+
129+
if (value_sp)
130+
return value_sp;
199131
}
200132
return nullptr;
201133
}
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
1-
CXX_SOURCES := main.cpp
1+
CXX_SOURCES := main.cpp extern.cpp
22

33
include Makefile.rules

lldb/test/API/commands/frame/var-dil/basics/GlobalVariableLookup/TestFrameVarDILGlobalVariableLookup.py

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -32,20 +32,20 @@ def test_frame_var(self):
3232
self.expect_var_path("::globalPtr", type="int *")
3333
self.expect_var_path("::globalRef", type="int &")
3434

35-
self.expect(
36-
"frame variable 'externGlobalVar'",
37-
error=True,
38-
substrs=["use of undeclared identifier"],
39-
) # 0x00C0FFEE
40-
self.expect(
41-
"frame variable '::externGlobalVar'",
42-
error=True,
43-
substrs=["use of undeclared identifier"],
44-
) # ["12648430"])
45-
4635
self.expect_var_path("ns::globalVar", value="13")
4736
self.expect_var_path("ns::globalPtr", type="int *")
4837
self.expect_var_path("ns::globalRef", type="int &")
4938
self.expect_var_path("::ns::globalVar", value="13")
5039
self.expect_var_path("::ns::globalPtr", type="int *")
5140
self.expect_var_path("::ns::globalRef", type="int &")
41+
42+
self.expect_var_path("externGlobalVar", value="2")
43+
self.expect_var_path("::externGlobalVar", value="2")
44+
self.expect_var_path("ext::externGlobalVar", value="4")
45+
self.expect_var_path("::ext::externGlobalVar", value="4")
46+
47+
self.expect_var_path("ExtStruct::static_inline", value="16")
48+
49+
# Test local variable priority over global
50+
self.expect_var_path("foo", value="1")
51+
self.expect_var_path("::foo", value="2")
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
int externGlobalVar = 2;
2+
3+
namespace ext {
4+
int externGlobalVar = 4;
5+
} // namespace ext
6+
7+
struct ExtStruct {
8+
private:
9+
static constexpr inline int static_inline = 16;
10+
} es;

lldb/test/API/commands/frame/var-dil/basics/GlobalVariableLookup/main.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,9 @@ int *globalPtr = &globalVar;
1010
int &globalRef = globalVar;
1111
} // namespace ns
1212

13+
int foo = 2;
14+
1315
int main(int argc, char **argv) {
16+
int foo = 1;
1417
return 0; // Set a breakpoint here
1518
}

0 commit comments

Comments
 (0)