Author: Michael Buch Date: 2023-04-14T17:11:30+01:00 New Revision: 6cdfa295743729178ff6f15a8dcd36f8f7d27c2c
URL: https://github.com/llvm/llvm-project/commit/6cdfa295743729178ff6f15a8dcd36f8f7d27c2c DIFF: https://github.com/llvm/llvm-project/commit/6cdfa295743729178ff6f15a8dcd36f8f7d27c2c.diff LOG: [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 Added: Modified: lldb/include/lldb/Symbol/SymbolFile.h lldb/include/lldb/Symbol/SymbolFileOnDemand.h lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h lldb/source/Symbol/SymbolFileOnDemand.cpp lldb/test/API/lang/cpp/namespace/TestNamespace.py lldb/test/API/lang/cpp/namespace/main.cpp Removed: ################################################################################ diff --git a/lldb/include/lldb/Symbol/SymbolFile.h b/lldb/include/lldb/Symbol/SymbolFile.h index 90162b7827d3..8de752816cf9 100644 --- a/lldb/include/lldb/Symbol/SymbolFile.h +++ b/lldb/include/lldb/Symbol/SymbolFile.h @@ -327,8 +327,17 @@ class SymbolFile : public PluginInterface { virtual llvm::Expected<lldb::TypeSystemSP> GetTypeSystemForLanguage(lldb::LanguageType language) = 0; + /// Finds a namespace of name \ref name and whose parent + /// context is \ref parent_decl_ctx. + /// + /// If \code{.cpp} !parent_decl_ctx.IsValid() \endcode + /// then this function will consider all namespaces that + /// match the name. If \ref only_root_namespaces is + /// true, only consider in the search those DIEs that + /// represent top-level namespaces. virtual CompilerDeclContext - FindNamespace(ConstString name, const CompilerDeclContext &parent_decl_ctx) { + FindNamespace(ConstString name, const CompilerDeclContext &parent_decl_ctx, + bool only_root_namespaces = false) { return CompilerDeclContext(); } diff --git a/lldb/include/lldb/Symbol/SymbolFileOnDemand.h b/lldb/include/lldb/Symbol/SymbolFileOnDemand.h index 825fba755e99..adf1017ce73c 100644 --- a/lldb/include/lldb/Symbol/SymbolFileOnDemand.h +++ b/lldb/include/lldb/Symbol/SymbolFileOnDemand.h @@ -171,9 +171,10 @@ class SymbolFileOnDemand : public lldb_private::SymbolFile { llvm::Expected<lldb::TypeSystemSP> GetTypeSystemForLanguage(lldb::LanguageType language) override; - lldb_private::CompilerDeclContext FindNamespace( - lldb_private::ConstString name, - const lldb_private::CompilerDeclContext &parent_decl_ctx) override; + lldb_private::CompilerDeclContext + FindNamespace(lldb_private::ConstString name, + const lldb_private::CompilerDeclContext &parent_decl_ctx, + bool only_root_namespaces) override; std::vector<std::unique_ptr<lldb_private::CallEdge>> ParseCallEdgesInFunction(UserID func_id) override; diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp index 108566b066d7..76cbfc4c05f8 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp @@ -700,7 +700,18 @@ void ClangASTSource::FillNamespaceMap( if (!symbol_file) continue; - found_namespace_decl = symbol_file->FindNamespace(name, namespace_decl); + // If namespace_decl is not valid, 'FindNamespace' would look for + // any namespace called 'name' (ignoring parent contexts) and return + // the first one it finds. Thus if we're doing a qualified lookup only + // consider root namespaces. E.g., in an expression ::A::B::Foo, the + // lookup of ::A will result in a qualified lookup. Note, namespace + // disambiguation for function calls are handled separately in + // SearchFunctionsInSymbolContexts. + const bool find_root_namespaces = + context.m_decl_context && + context.m_decl_context->shouldUseQualifiedLookup(); + found_namespace_decl = symbol_file->FindNamespace( + name, namespace_decl, /* only root namespaces */ find_root_namespaces); if (found_namespace_decl) { context.m_namespace_map->push_back( diff --git a/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h b/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h index 6d9336966517..3942f4f7ae24 100644 --- a/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h +++ b/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h @@ -134,9 +134,9 @@ class SymbolFileBreakpad : public SymbolFileCommon { llvm::inconvertibleErrorCode()); } - CompilerDeclContext - FindNamespace(ConstString name, - const CompilerDeclContext &parent_decl_ctx) override { + CompilerDeclContext FindNamespace(ConstString name, + const CompilerDeclContext &parent_decl_ctx, + bool only_root_namespaces) override { return CompilerDeclContext(); } diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp index 837d87cdebbb..0bde202f280f 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp @@ -2324,12 +2324,19 @@ bool SymbolFileDWARF::ResolveFunction(const DWARFDIE &orig_die, } bool SymbolFileDWARF::DIEInDeclContext(const CompilerDeclContext &decl_ctx, - const DWARFDIE &die) { + const DWARFDIE &die, + bool only_root_namespaces) { // If we have no parent decl context to match this DIE matches, and if the // parent decl context isn't valid, we aren't trying to look for any // particular decl context so any die matches. - if (!decl_ctx.IsValid()) + if (!decl_ctx.IsValid()) { + // ...But if we are only checking root decl contexts, confirm that the + // 'die' is a top-level context. + if (only_root_namespaces) + return die.GetParent().Tag() == dwarf::DW_TAG_compile_unit; + return true; + } if (die) { if (DWARFASTParser *dwarf_ast = GetDWARFParser(*die.GetCU())) { @@ -2632,7 +2639,8 @@ void SymbolFileDWARF::FindTypes( CompilerDeclContext SymbolFileDWARF::FindNamespace(ConstString name, - const CompilerDeclContext &parent_decl_ctx) { + const CompilerDeclContext &parent_decl_ctx, + bool only_root_namespaces) { std::lock_guard<std::recursive_mutex> guard(GetModuleMutex()); Log *log = GetLog(DWARFLog::Lookups); @@ -2648,7 +2656,7 @@ SymbolFileDWARF::FindNamespace(ConstString name, return namespace_decl_ctx; m_index->GetNamespaces(name, [&](DWARFDIE die) { - if (!DIEInDeclContext(parent_decl_ctx, die)) + if (!DIEInDeclContext(parent_decl_ctx, die, only_root_namespaces)) return true; // The containing decl contexts don't match DWARFASTParser *dwarf_ast = GetDWARFParser(*die.GetCU()); diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h index 0616846d8dc6..d685e4df6d85 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h @@ -214,9 +214,10 @@ class SymbolFileDWARF : public lldb_private::SymbolFileCommon { llvm::Expected<lldb::TypeSystemSP> GetTypeSystemForLanguage(lldb::LanguageType language) override; - lldb_private::CompilerDeclContext FindNamespace( - lldb_private::ConstString name, - const lldb_private::CompilerDeclContext &parent_decl_ctx) override; + lldb_private::CompilerDeclContext + FindNamespace(lldb_private::ConstString name, + const lldb_private::CompilerDeclContext &parent_decl_ctx, + bool only_root_namespaces) override; void PreloadSymbols() override; @@ -274,7 +275,7 @@ class SymbolFileDWARF : public lldb_private::SymbolFileCommon { static bool DIEInDeclContext(const lldb_private::CompilerDeclContext &parent_decl_ctx, - const DWARFDIE &die); + const DWARFDIE &die, bool only_root_namespaces = false); std::vector<std::unique_ptr<lldb_private::CallEdge>> ParseCallEdgesInFunction(lldb_private::UserID func_id) override; diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp index a07807241deb..1d79b13a6633 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp @@ -1256,13 +1256,14 @@ void SymbolFileDWARFDebugMap::FindTypes( } CompilerDeclContext SymbolFileDWARFDebugMap::FindNamespace( - lldb_private::ConstString name, - const CompilerDeclContext &parent_decl_ctx) { + lldb_private::ConstString name, const CompilerDeclContext &parent_decl_ctx, + bool only_root_namespaces) { std::lock_guard<std::recursive_mutex> guard(GetModuleMutex()); CompilerDeclContext matching_namespace; ForEachSymbolFile([&](SymbolFileDWARF *oso_dwarf) -> bool { - matching_namespace = oso_dwarf->FindNamespace(name, parent_decl_ctx); + matching_namespace = + oso_dwarf->FindNamespace(name, parent_decl_ctx, only_root_namespaces); return (bool)matching_namespace; }); diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h index 0313063119c7..881fd4c45ff0 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h @@ -136,9 +136,10 @@ class SymbolFileDWARFDebugMap : public lldb_private::SymbolFileCommon { lldb_private::LanguageSet languages, llvm::DenseSet<lldb_private::SymbolFile *> &searched_symbol_files, lldb_private::TypeMap &types) override; - lldb_private::CompilerDeclContext FindNamespace( - lldb_private::ConstString name, - const lldb_private::CompilerDeclContext &parent_decl_ctx) override; + lldb_private::CompilerDeclContext + FindNamespace(lldb_private::ConstString name, + const lldb_private::CompilerDeclContext &parent_decl_ctx, + bool only_root_namespaces) override; void GetTypes(lldb_private::SymbolContextScope *sc_scope, lldb::TypeClass type_mask, lldb_private::TypeList &type_list) override; diff --git a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp index df558c842f9c..01756e47e917 100644 --- a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp +++ b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp @@ -2133,9 +2133,8 @@ void SymbolFileNativePDB::GetTypes(lldb_private::SymbolContextScope *sc_scope, TypeClass type_mask, lldb_private::TypeList &type_list) {} -CompilerDeclContext -SymbolFileNativePDB::FindNamespace(ConstString name, - const CompilerDeclContext &parent_decl_ctx) { +CompilerDeclContext SymbolFileNativePDB::FindNamespace( + ConstString name, const CompilerDeclContext &parent_decl_ctx, bool) { return {}; } diff --git a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h index 90cd5251679e..bf64cd330c1f 100644 --- a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h +++ b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h @@ -152,9 +152,9 @@ class SymbolFileNativePDB : public SymbolFileCommon { llvm::Expected<lldb::TypeSystemSP> GetTypeSystemForLanguage(lldb::LanguageType language) override; - CompilerDeclContext - FindNamespace(ConstString name, - const CompilerDeclContext &parent_decl_ctx) override; + CompilerDeclContext FindNamespace(ConstString name, + const CompilerDeclContext &parent_decl_ctx, + bool only_root_namespaces) override; llvm::StringRef GetPluginName() override { return GetPluginNameStatic(); } diff --git a/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp b/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp index 613e36207127..7f07eec1e464 100644 --- a/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp +++ b/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp @@ -1697,7 +1697,7 @@ PDBASTParser *SymbolFilePDB::GetPDBAstParser() { lldb_private::CompilerDeclContext SymbolFilePDB::FindNamespace(lldb_private::ConstString name, - const CompilerDeclContext &parent_decl_ctx) { + const CompilerDeclContext &parent_decl_ctx, bool) { std::lock_guard<std::recursive_mutex> guard(GetModuleMutex()); auto type_system_or_err = GetTypeSystemForLanguage(lldb::eLanguageTypeC_plus_plus); diff --git a/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h b/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h index 992bafd6a377..5b98c6e8b486 100644 --- a/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h +++ b/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h @@ -157,9 +157,10 @@ class SymbolFilePDB : public lldb_private::SymbolFileCommon { llvm::Expected<lldb::TypeSystemSP> GetTypeSystemForLanguage(lldb::LanguageType language) override; - lldb_private::CompilerDeclContext FindNamespace( - lldb_private::ConstString name, - const lldb_private::CompilerDeclContext &parent_decl_ctx) override; + lldb_private::CompilerDeclContext + FindNamespace(lldb_private::ConstString name, + const lldb_private::CompilerDeclContext &parent_decl_ctx, + bool only_root_namespaces) override; llvm::StringRef GetPluginName() override { return GetPluginNameStatic(); } diff --git a/lldb/source/Symbol/SymbolFileOnDemand.cpp b/lldb/source/Symbol/SymbolFileOnDemand.cpp index aa3d23ccbf4d..d3694580194f 100644 --- a/lldb/source/Symbol/SymbolFileOnDemand.cpp +++ b/lldb/source/Symbol/SymbolFileOnDemand.cpp @@ -483,13 +483,16 @@ SymbolFileOnDemand::GetTypeSystemForLanguage(LanguageType language) { CompilerDeclContext SymbolFileOnDemand::FindNamespace(ConstString name, - const CompilerDeclContext &parent_decl_ctx) { + const CompilerDeclContext &parent_decl_ctx, + bool only_root_namespaces) { if (!m_debug_info_enabled) { LLDB_LOG(GetLog(), "[{0}] {1}({2}) is skipped", GetSymbolFileName(), __FUNCTION__, name); - return SymbolFile::FindNamespace(name, parent_decl_ctx); + return SymbolFile::FindNamespace(name, parent_decl_ctx, + only_root_namespaces); } - return m_sym_file_impl->FindNamespace(name, parent_decl_ctx); + return m_sym_file_impl->FindNamespace(name, parent_decl_ctx, + only_root_namespaces); } std::vector<std::unique_ptr<lldb_private::CallEdge>> diff --git a/lldb/test/API/lang/cpp/namespace/TestNamespace.py b/lldb/test/API/lang/cpp/namespace/TestNamespace.py index 114d70dc0ec9..ad534cd58ac0 100644 --- a/lldb/test/API/lang/cpp/namespace/TestNamespace.py +++ b/lldb/test/API/lang/cpp/namespace/TestNamespace.py @@ -225,3 +225,11 @@ def test_with_run_command(self): self.expect("expression variadic_sum", patterns=[ '\(anonymous namespace\)::variadic_sum\(int, ...\)']) + + self.expect_expr("::B::Bar b; b.x()", result_type="int", result_value="42") + self.expect_expr("A::B::Bar b; b.y()", result_type="int", result_value="137") + self.expect_expr("::NS1::NS2::Foo{}.bar() == -2 && ::NS2::Foo{}.bar() == -3", + result_type="bool", result_value="true") + # FIXME: C++ unqualified namespace lookups currently not supported when instantiating types. + self.expect_expr("NS2::Foo{}.bar() == -3", result_type="bool", result_value="false") + self.expect_expr("((::B::Bar*)&::B::bar)->x()", result_type="int", result_value="42") diff --git a/lldb/test/API/lang/cpp/namespace/main.cpp b/lldb/test/API/lang/cpp/namespace/main.cpp index fc5dacdaf8f5..6a8efa160766 100644 --- a/lldb/test/API/lang/cpp/namespace/main.cpp +++ b/lldb/test/API/lang/cpp/namespace/main.cpp @@ -102,6 +102,31 @@ int Foo::myfunc(int a) return myfunc2(3) + j + i + a + 2 + anon_uint + a_uint + b_uint + y_uint; // Set break point at this line. } +namespace B { +struct Bar { + int x() { return 42; } +}; +Bar bar; +} // namespace B + +namespace A::B { +struct Bar { + int y() { return 137; } +}; +} // namespace A::B + +namespace NS1::NS2 { +struct Foo { + int bar() { return -2; } +}; +} // namespace NS1::NS2 + +namespace NS2 { +struct Foo { + int bar() { return -3; } +}; +} // namespace NS2 + int main (int argc, char const *argv[]) { @@ -112,5 +137,8 @@ main (int argc, char const *argv[]) A::B::test_lookup_at_nested_ns_scope_after_using(); test_lookup_before_using_directive(); test_lookup_after_using_directive(); - return Foo::myfunc(12); + ::B::Bar bb; + A::B::Bar ab; + return Foo::myfunc(12) + bb.x() + ab.y() + NS1::NS2::Foo{}.bar() + + NS2::Foo{}.bar(); } _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
