Skip to content

Commit 6cdfa29

Browse files
Michael137Michael137
committed
[lldb][ClangExpression] Filter out non-root namespaces in FindNamespace
**Summary** In a program such as: ``` namespace A { namespace B { struct Bar {}; } } namespace B { struct Foo {}; } ``` ...LLDB would run into issues such as: ``` (lldb) expr ::B::Foo f error: expression failed to parse: error: <user expression 0>:1:6: no type named 'Foo' in namespace 'A::B' ::B::Foo f ~~~~~^ ``` This is because the `SymbolFileDWARF::FindNamespace` implementation will return *any* namespace it finds if the `parent_decl_ctx` provided is empty. In `FindExternalVisibleDecls` we use this API to find the namespace that symbol `B` refers to. If `A::B` happened to be the one that `SymbolFileDWARF::FindNamespace` looked at first, we would try to find `struct Foo` in `A::B`. Hence the error. This patch proposes a new `SymbolFileDWARF::FindNamespace` API that will only find a match for top-level namespaces, which is what `FindExternalVisibleDecls` is attempting anyway; it just never accounted for multiple namespaces of the same name. **Testing** * Added API test-case Differential Revision: https://reviews.llvm.org/D147436
1 parent d2f22fb commit 6cdfa29

File tree

15 files changed

+107
-36
lines changed

15 files changed

+107
-36
lines changed

‎lldb/include/lldb/Symbol/SymbolFile.h

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -327,8 +327,17 @@ class SymbolFile : public PluginInterface {
327327
virtual llvm::Expected<lldb::TypeSystemSP>
328328
GetTypeSystemForLanguage(lldb::LanguageType language) = 0;
329329

330+
/// Finds a namespace of name \ref name and whose parent
331+
/// context is \ref parent_decl_ctx.
332+
///
333+
/// If \code{.cpp} !parent_decl_ctx.IsValid() \endcode
334+
/// then this function will consider all namespaces that
335+
/// match the name. If \ref only_root_namespaces is
336+
/// true, only consider in the search those DIEs that
337+
/// represent top-level namespaces.
330338
virtual CompilerDeclContext
331-
FindNamespace(ConstString name, const CompilerDeclContext &parent_decl_ctx) {
339+
FindNamespace(ConstString name, const CompilerDeclContext &parent_decl_ctx,
340+
bool only_root_namespaces = false) {
332341
return CompilerDeclContext();
333342
}
334343

‎lldb/include/lldb/Symbol/SymbolFileOnDemand.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -171,9 +171,10 @@ class SymbolFileOnDemand : public lldb_private::SymbolFile {
171171
llvm::Expected<lldb::TypeSystemSP>
172172
GetTypeSystemForLanguage(lldb::LanguageType language) override;
173173

174-
lldb_private::CompilerDeclContext FindNamespace(
175-
lldb_private::ConstString name,
176-
const lldb_private::CompilerDeclContext &parent_decl_ctx) override;
174+
lldb_private::CompilerDeclContext
175+
FindNamespace(lldb_private::ConstString name,
176+
const lldb_private::CompilerDeclContext &parent_decl_ctx,
177+
bool only_root_namespaces) override;
177178

178179
std::vector<std::unique_ptr<lldb_private::CallEdge>>
179180
ParseCallEdgesInFunction(UserID func_id) override;

‎lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -700,7 +700,18 @@ void ClangASTSource::FillNamespaceMap(
700700
if (!symbol_file)
701701
continue;
702702

703-
found_namespace_decl = symbol_file->FindNamespace(name, namespace_decl);
703+
// If namespace_decl is not valid, 'FindNamespace' would look for
704+
// any namespace called 'name' (ignoring parent contexts) and return
705+
// the first one it finds. Thus if we're doing a qualified lookup only
706+
// consider root namespaces. E.g., in an expression ::A::B::Foo, the
707+
// lookup of ::A will result in a qualified lookup. Note, namespace
708+
// disambiguation for function calls are handled separately in
709+
// SearchFunctionsInSymbolContexts.
710+
const bool find_root_namespaces =
711+
context.m_decl_context &&
712+
context.m_decl_context->shouldUseQualifiedLookup();
713+
found_namespace_decl = symbol_file->FindNamespace(
714+
name, namespace_decl, /* only root namespaces */ find_root_namespaces);
704715

705716
if (found_namespace_decl) {
706717
context.m_namespace_map->push_back(

‎lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -134,9 +134,9 @@ class SymbolFileBreakpad : public SymbolFileCommon {
134134
llvm::inconvertibleErrorCode());
135135
}
136136

137-
CompilerDeclContext
138-
FindNamespace(ConstString name,
139-
const CompilerDeclContext &parent_decl_ctx) override {
137+
CompilerDeclContext FindNamespace(ConstString name,
138+
const CompilerDeclContext &parent_decl_ctx,
139+
bool only_root_namespaces) override {
140140
return CompilerDeclContext();
141141
}
142142

‎lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2324,12 +2324,19 @@ bool SymbolFileDWARF::ResolveFunction(const DWARFDIE &orig_die,
23242324
}
23252325

23262326
bool SymbolFileDWARF::DIEInDeclContext(const CompilerDeclContext &decl_ctx,
2327-
const DWARFDIE &die) {
2327+
const DWARFDIE &die,
2328+
bool only_root_namespaces) {
23282329
// If we have no parent decl context to match this DIE matches, and if the
23292330
// parent decl context isn't valid, we aren't trying to look for any
23302331
// particular decl context so any die matches.
2331-
if (!decl_ctx.IsValid())
2332+
if (!decl_ctx.IsValid()) {
2333+
// ...But if we are only checking root decl contexts, confirm that the
2334+
// 'die' is a top-level context.
2335+
if (only_root_namespaces)
2336+
return die.GetParent().Tag() == dwarf::DW_TAG_compile_unit;
2337+
23322338
return true;
2339+
}
23332340

23342341
if (die) {
23352342
if (DWARFASTParser *dwarf_ast = GetDWARFParser(*die.GetCU())) {
@@ -2632,7 +2639,8 @@ void SymbolFileDWARF::FindTypes(
26322639

26332640
CompilerDeclContext
26342641
SymbolFileDWARF::FindNamespace(ConstString name,
2635-
const CompilerDeclContext &parent_decl_ctx) {
2642+
const CompilerDeclContext &parent_decl_ctx,
2643+
bool only_root_namespaces) {
26362644
std::lock_guard<std::recursive_mutex> guard(GetModuleMutex());
26372645
Log *log = GetLog(DWARFLog::Lookups);
26382646

@@ -2648,7 +2656,7 @@ SymbolFileDWARF::FindNamespace(ConstString name,
26482656
return namespace_decl_ctx;
26492657

26502658
m_index->GetNamespaces(name, [&](DWARFDIE die) {
2651-
if (!DIEInDeclContext(parent_decl_ctx, die))
2659+
if (!DIEInDeclContext(parent_decl_ctx, die, only_root_namespaces))
26522660
return true; // The containing decl contexts don't match
26532661

26542662
DWARFASTParser *dwarf_ast = GetDWARFParser(*die.GetCU());

‎lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -214,9 +214,10 @@ class SymbolFileDWARF : public lldb_private::SymbolFileCommon {
214214
llvm::Expected<lldb::TypeSystemSP>
215215
GetTypeSystemForLanguage(lldb::LanguageType language) override;
216216

217-
lldb_private::CompilerDeclContext FindNamespace(
218-
lldb_private::ConstString name,
219-
const lldb_private::CompilerDeclContext &parent_decl_ctx) override;
217+
lldb_private::CompilerDeclContext
218+
FindNamespace(lldb_private::ConstString name,
219+
const lldb_private::CompilerDeclContext &parent_decl_ctx,
220+
bool only_root_namespaces) override;
220221

221222
void PreloadSymbols() override;
222223

@@ -274,7 +275,7 @@ class SymbolFileDWARF : public lldb_private::SymbolFileCommon {
274275

275276
static bool
276277
DIEInDeclContext(const lldb_private::CompilerDeclContext &parent_decl_ctx,
277-
const DWARFDIE &die);
278+
const DWARFDIE &die, bool only_root_namespaces = false);
278279

279280
std::vector<std::unique_ptr<lldb_private::CallEdge>>
280281
ParseCallEdgesInFunction(lldb_private::UserID func_id) override;

‎lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1256,13 +1256,14 @@ void SymbolFileDWARFDebugMap::FindTypes(
12561256
}
12571257

12581258
CompilerDeclContext SymbolFileDWARFDebugMap::FindNamespace(
1259-
lldb_private::ConstString name,
1260-
const CompilerDeclContext &parent_decl_ctx) {
1259+
lldb_private::ConstString name, const CompilerDeclContext &parent_decl_ctx,
1260+
bool only_root_namespaces) {
12611261
std::lock_guard<std::recursive_mutex> guard(GetModuleMutex());
12621262
CompilerDeclContext matching_namespace;
12631263

12641264
ForEachSymbolFile([&](SymbolFileDWARF *oso_dwarf) -> bool {
1265-
matching_namespace = oso_dwarf->FindNamespace(name, parent_decl_ctx);
1265+
matching_namespace =
1266+
oso_dwarf->FindNamespace(name, parent_decl_ctx, only_root_namespaces);
12661267

12671268
return (bool)matching_namespace;
12681269
});

‎lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -136,9 +136,10 @@ class SymbolFileDWARFDebugMap : public lldb_private::SymbolFileCommon {
136136
lldb_private::LanguageSet languages,
137137
llvm::DenseSet<lldb_private::SymbolFile *> &searched_symbol_files,
138138
lldb_private::TypeMap &types) override;
139-
lldb_private::CompilerDeclContext FindNamespace(
140-
lldb_private::ConstString name,
141-
const lldb_private::CompilerDeclContext &parent_decl_ctx) override;
139+
lldb_private::CompilerDeclContext
140+
FindNamespace(lldb_private::ConstString name,
141+
const lldb_private::CompilerDeclContext &parent_decl_ctx,
142+
bool only_root_namespaces) override;
142143
void GetTypes(lldb_private::SymbolContextScope *sc_scope,
143144
lldb::TypeClass type_mask,
144145
lldb_private::TypeList &type_list) override;

‎lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2133,9 +2133,8 @@ void SymbolFileNativePDB::GetTypes(lldb_private::SymbolContextScope *sc_scope,
21332133
TypeClass type_mask,
21342134
lldb_private::TypeList &type_list) {}
21352135

2136-
CompilerDeclContext
2137-
SymbolFileNativePDB::FindNamespace(ConstString name,
2138-
const CompilerDeclContext &parent_decl_ctx) {
2136+
CompilerDeclContext SymbolFileNativePDB::FindNamespace(
2137+
ConstString name, const CompilerDeclContext &parent_decl_ctx, bool) {
21392138
return {};
21402139
}
21412140

‎lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -152,9 +152,9 @@ class SymbolFileNativePDB : public SymbolFileCommon {
152152
llvm::Expected<lldb::TypeSystemSP>
153153
GetTypeSystemForLanguage(lldb::LanguageType language) override;
154154

155-
CompilerDeclContext
156-
FindNamespace(ConstString name,
157-
const CompilerDeclContext &parent_decl_ctx) override;
155+
CompilerDeclContext FindNamespace(ConstString name,
156+
const CompilerDeclContext &parent_decl_ctx,
157+
bool only_root_namespaces) override;
158158

159159
llvm::StringRef GetPluginName() override { return GetPluginNameStatic(); }
160160

‎lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1697,7 +1697,7 @@ PDBASTParser *SymbolFilePDB::GetPDBAstParser() {
16971697

16981698
lldb_private::CompilerDeclContext
16991699
SymbolFilePDB::FindNamespace(lldb_private::ConstString name,
1700-
const CompilerDeclContext &parent_decl_ctx) {
1700+
const CompilerDeclContext &parent_decl_ctx, bool) {
17011701
std::lock_guard<std::recursive_mutex> guard(GetModuleMutex());
17021702
auto type_system_or_err =
17031703
GetTypeSystemForLanguage(lldb::eLanguageTypeC_plus_plus);

‎lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -157,9 +157,10 @@ class SymbolFilePDB : public lldb_private::SymbolFileCommon {
157157
llvm::Expected<lldb::TypeSystemSP>
158158
GetTypeSystemForLanguage(lldb::LanguageType language) override;
159159

160-
lldb_private::CompilerDeclContext FindNamespace(
161-
lldb_private::ConstString name,
162-
const lldb_private::CompilerDeclContext &parent_decl_ctx) override;
160+
lldb_private::CompilerDeclContext
161+
FindNamespace(lldb_private::ConstString name,
162+
const lldb_private::CompilerDeclContext &parent_decl_ctx,
163+
bool only_root_namespaces) override;
163164

164165
llvm::StringRef GetPluginName() override { return GetPluginNameStatic(); }
165166

‎lldb/source/Symbol/SymbolFileOnDemand.cpp

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -483,13 +483,16 @@ SymbolFileOnDemand::GetTypeSystemForLanguage(LanguageType language) {
483483

484484
CompilerDeclContext
485485
SymbolFileOnDemand::FindNamespace(ConstString name,
486-
const CompilerDeclContext &parent_decl_ctx) {
486+
const CompilerDeclContext &parent_decl_ctx,
487+
bool only_root_namespaces) {
487488
if (!m_debug_info_enabled) {
488489
LLDB_LOG(GetLog(), "[{0}] {1}({2}) is skipped", GetSymbolFileName(),
489490
__FUNCTION__, name);
490-
return SymbolFile::FindNamespace(name, parent_decl_ctx);
491+
return SymbolFile::FindNamespace(name, parent_decl_ctx,
492+
only_root_namespaces);
491493
}
492-
return m_sym_file_impl->FindNamespace(name, parent_decl_ctx);
494+
return m_sym_file_impl->FindNamespace(name, parent_decl_ctx,
495+
only_root_namespaces);
493496
}
494497

495498
std::vector<std::unique_ptr<lldb_private::CallEdge>>

‎lldb/test/API/lang/cpp/namespace/TestNamespace.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,3 +225,11 @@ def test_with_run_command(self):
225225

226226
self.expect("expression variadic_sum", patterns=[
227227
'\(anonymous namespace\)::variadic_sum\(int, ...\)'])
228+
229+
self.expect_expr("::B::Bar b; b.x()", result_type="int", result_value="42")
230+
self.expect_expr("A::B::Bar b; b.y()", result_type="int", result_value="137")
231+
self.expect_expr("::NS1::NS2::Foo{}.bar() == -2 && ::NS2::Foo{}.bar() == -3",
232+
result_type="bool", result_value="true")
233+
# FIXME: C++ unqualified namespace lookups currently not supported when instantiating types.
234+
self.expect_expr("NS2::Foo{}.bar() == -3", result_type="bool", result_value="false")
235+
self.expect_expr("((::B::Bar*)&::B::bar)->x()", result_type="int", result_value="42")

‎lldb/test/API/lang/cpp/namespace/main.cpp

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,31 @@ int Foo::myfunc(int a)
102102
return myfunc2(3) + j + i + a + 2 + anon_uint + a_uint + b_uint + y_uint; // Set break point at this line.
103103
}
104104

105+
namespace B {
106+
struct Bar {
107+
int x() { return 42; }
108+
};
109+
Bar bar;
110+
} // namespace B
111+
112+
namespace A::B {
113+
struct Bar {
114+
int y() { return 137; }
115+
};
116+
} // namespace A::B
117+
118+
namespace NS1::NS2 {
119+
struct Foo {
120+
int bar() { return -2; }
121+
};
122+
} // namespace NS1::NS2
123+
124+
namespace NS2 {
125+
struct Foo {
126+
int bar() { return -3; }
127+
};
128+
} // namespace NS2
129+
105130
int
106131
main (int argc, char const *argv[])
107132
{
@@ -112,5 +137,8 @@ main (int argc, char const *argv[])
112137
A::B::test_lookup_at_nested_ns_scope_after_using();
113138
test_lookup_before_using_directive();
114139
test_lookup_after_using_directive();
115-
return Foo::myfunc(12);
140+
::B::Bar bb;
141+
A::B::Bar ab;
142+
return Foo::myfunc(12) + bb.x() + ab.y() + NS1::NS2::Foo{}.bar() +
143+
NS2::Foo{}.bar();
116144
}

0 commit comments

Comments
 (0)