From 1aff68c333c9a12a4b67cca717eba6d96d4afb5a Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Wed, 20 Nov 2024 14:37:37 +0100 Subject: [PATCH 1/3] [lldb] Fix lookup of types in anonymous namespaces with -gsimple-template-names Anonymous namespaces are supposed to be optional when looking up types. This was not working in combination with -gsimple-template-names, because the way it was constructing the complete (with template args) name scope (i.e., by generating thescope as a string and then reparsing it) did not preserve the information about the scope kinds. Essentially what the code wants here is to call `GetTypeLookupContext` (that's the function used to get the context in the "regular" code path), but to embelish each name with the template arguments (if they don't have them already). This PR implements exactly that by adding an argument to control which kind of names are we interested in. This should also make the lookup faster as it avoids parsing of the long string, but I haven't attempted to benchmark that. I believe this function can also be used in some other places where we're manually appending template names, but I'm leaving that for another patch. --- .../Plugins/SymbolFile/DWARF/DWARFDIE.cpp | 44 +++++++++++++------ .../Plugins/SymbolFile/DWARF/DWARFDIE.h | 20 ++++++++- .../SymbolFile/DWARF/SymbolFileDWARF.cpp | 19 +++----- .../cpp/nested-template/TestNestedTemplate.py | 5 +++ .../API/lang/cpp/nested-template/main.cpp | 7 +++ 5 files changed, 65 insertions(+), 30 deletions(-) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp index 6857878b354a0..f2b02d3d6898d 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp @@ -19,6 +19,8 @@ #include "llvm/ADT/iterator.h" #include "llvm/BinaryFormat/Dwarf.h" #include "llvm/DebugInfo/DWARF/DWARFAddressRange.h" +#include "llvm/DebugInfo/DWARF/DWARFTypePrinter.h" +#include "llvm/Support/raw_ostream.h" using namespace lldb_private; using namespace lldb_private::dwarf; @@ -376,7 +378,8 @@ lldb_private::Type *DWARFDIE::ResolveTypeUID(const DWARFDIE &die) const { return nullptr; } -static CompilerContext GetContextEntry(DWARFDIE die) { +static CompilerContext GetContextEntry(DWARFDIE die, + bool complete_template_names) { auto ctx = [die](CompilerContextKind kind) { return CompilerContext(kind, ConstString(die.GetName())); }; @@ -386,11 +389,6 @@ static CompilerContext GetContextEntry(DWARFDIE die) { return ctx(CompilerContextKind::Module); case DW_TAG_namespace: return ctx(CompilerContextKind::Namespace); - case DW_TAG_class_type: - case DW_TAG_structure_type: - return ctx(CompilerContextKind::ClassOrStruct); - case DW_TAG_union_type: - return ctx(CompilerContextKind::Union); case DW_TAG_enumeration_type: return ctx(CompilerContextKind::Enum); case DW_TAG_subprogram: @@ -401,12 +399,28 @@ static CompilerContext GetContextEntry(DWARFDIE die) { return ctx(CompilerContextKind::Typedef); case DW_TAG_base_type: return ctx(CompilerContextKind::Builtin); + case DW_TAG_class_type: + case DW_TAG_structure_type: + case DW_TAG_union_type: { + CompilerContextKind kind = die.Tag() == DW_TAG_union_type + ? CompilerContextKind::Union + : CompilerContextKind::ClassOrStruct; + llvm::StringRef name = die.GetName(); + if (!complete_template_names || name.contains('<')) + return CompilerContext(kind, ConstString(name)); + + std::string name_storage = name.str(); + llvm::raw_string_ostream os(name_storage); + llvm::DWARFTypePrinter(os).appendAndTerminateTemplateParameters( + die); + return CompilerContext(kind, ConstString(os.str())); + } default: llvm_unreachable("Check tag type in the caller!"); } } -static void GetDeclContextImpl(DWARFDIE die, +static void GetDeclContextImpl(DWARFDIE die, bool complete_template_names, llvm::SmallSet &seen, std::vector &context) { // Stop if we hit a cycle. @@ -428,7 +442,7 @@ static void GetDeclContextImpl(DWARFDIE die, case DW_TAG_subprogram: case DW_TAG_variable: case DW_TAG_typedef: - context.push_back(GetContextEntry(die)); + context.push_back(GetContextEntry(die, complete_template_names)); break; default: break; @@ -438,15 +452,16 @@ static void GetDeclContextImpl(DWARFDIE die, } } -std::vector DWARFDIE::GetDeclContext() const { +std::vector +DWARFDIE::GetDeclContext(bool complete_template_names) const { llvm::SmallSet seen; std::vector context; - GetDeclContextImpl(*this, seen, context); + GetDeclContextImpl(*this, complete_template_names, seen, context); std::reverse(context.begin(), context.end()); return context; } -static void GetTypeLookupContextImpl(DWARFDIE die, +static void GetTypeLookupContextImpl(DWARFDIE die, bool complete_template_names, llvm::SmallSet &seen, std::vector &context) { // Stop if we hit a cycle. @@ -461,7 +476,7 @@ static void GetTypeLookupContextImpl(DWARFDIE die, case DW_TAG_variable: case DW_TAG_typedef: case DW_TAG_base_type: - context.push_back(GetContextEntry(die)); + context.push_back(GetContextEntry(die, complete_template_names)); break; // If any of the tags below appear in the parent chain, stop the decl @@ -484,10 +499,11 @@ static void GetTypeLookupContextImpl(DWARFDIE die, } } -std::vector DWARFDIE::GetTypeLookupContext() const { +std::vector +DWARFDIE::GetTypeLookupContext(bool complete_template_names) const { llvm::SmallSet seen; std::vector context; - GetTypeLookupContextImpl(*this, seen, context); + GetTypeLookupContextImpl(*this, complete_template_names, seen, context); std::reverse(context.begin(), context.end()); return context; } diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h index c3239b5b121f9..b2df9a3c43a75 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h @@ -73,7 +73,15 @@ class DWARFDIE : public DWARFBaseDIE { /// Return this DIE's decl context as it is needed to look up types /// in Clang modules. This context will include any modules or functions that /// the type is declared in so an exact module match can be efficiently made. - std::vector GetDeclContext() const; + /// + /// \param[in] complete_template_names + /// If true, augments the returned names with template arguments derived + /// from the child DIEs, if the names don't contained template arguments + /// already. If false, the returned context will contain the names exactly + /// as they are spelled in the debug info, regardless of whether that + /// includes template arguments or not. + std::vector + GetDeclContext(bool complete_template_names = false) const; /// Get a context to a type so it can be looked up. /// @@ -85,7 +93,15 @@ class DWARFDIE : public DWARFBaseDIE { /// appropriate time, like either the translation unit or at a function /// context. This is designed to allow users to efficiently look for types /// using a full or partial CompilerContext array. - std::vector GetTypeLookupContext() const; + /// + /// \param[in] complete_template_names + /// If true, augments the returned names with template arguments derived + /// from the child DIEs, if the names don't contained template arguments + /// already. If false, the returned context will contain the names exactly + /// as they are spelled in the debug info, regardless of whether that + /// includes template arguments or not. + std::vector + GetTypeLookupContext(bool complete_template_names = false) const; DWARFDeclContext GetDWARFDeclContext() const; diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp index 360dbaa1beb5e..0821aa0050fc6 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp @@ -10,10 +10,8 @@ #include "llvm/ADT/STLExtras.h" #include "llvm/DebugInfo/DWARF/DWARFAddressRange.h" #include "llvm/DebugInfo/DWARF/DWARFDebugLoc.h" -#include "llvm/DebugInfo/DWARF/DWARFTypePrinter.h" #include "llvm/Support/Casting.h" #include "llvm/Support/FileUtilities.h" -#include "llvm/Support/Format.h" #include "llvm/Support/FormatAdapters.h" #include "llvm/Support/Threading.h" @@ -2740,18 +2738,11 @@ void SymbolFileDWARF::FindTypes(const TypeQuery &query, TypeResults &results) { // Copy our match's context and update the basename we are looking for // so we can use this only to compare the context correctly. m_index->GetTypesWithQuery(query_simple, [&](DWARFDIE die) { - // Check the language, but only if we have a language filter. - if (query.HasLanguage()) { - if (!query.LanguageMatches(GetLanguageFamily(*die.GetCU()))) - return true; // Keep iterating over index types, language mismatch. - } - - std::string qualified_name; - llvm::raw_string_ostream os(qualified_name); - llvm::DWARFTypePrinter type_printer(os); - type_printer.appendQualifiedName(die); - TypeQuery die_query(qualified_name, e_exact_match); - if (query.ContextMatches(die_query.GetContextRef())) + std::vector qualified_context = + query.GetModuleSearch() + ? die.GetDeclContext(/*complete_template_names=*/true) + : die.GetTypeLookupContext(/*complete_template_names=*/true); + if (query.ContextMatches(qualified_context)) if (Type *matching_type = ResolveType(die, true, true)) results.InsertUnique(matching_type->shared_from_this()); return !results.Done(query); // Keep iterating if we aren't done. diff --git a/lldb/test/API/lang/cpp/nested-template/TestNestedTemplate.py b/lldb/test/API/lang/cpp/nested-template/TestNestedTemplate.py index 42db060529a81..7a73619555fa0 100644 --- a/lldb/test/API/lang/cpp/nested-template/TestNestedTemplate.py +++ b/lldb/test/API/lang/cpp/nested-template/TestNestedTemplate.py @@ -17,6 +17,11 @@ def do_test(self, debug_flags): DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"], ) + self.expect( + "image lookup -A -t 'NS::Struct'", + DATA_TYPES_DISPLAYED_CORRECTLY, + substrs=["1 match found"], + ) @skipIf(compiler=no_match("clang")) @skipIf(compiler_version=["<", "15.0"]) diff --git a/lldb/test/API/lang/cpp/nested-template/main.cpp b/lldb/test/API/lang/cpp/nested-template/main.cpp index 06d1094880964..eef7c021e22ea 100644 --- a/lldb/test/API/lang/cpp/nested-template/main.cpp +++ b/lldb/test/API/lang/cpp/nested-template/main.cpp @@ -5,6 +5,13 @@ struct Outer { struct Inner {}; }; +namespace NS { +namespace { +template struct Struct {}; +} // namespace +} // namespace NS + int main() { Outer::Inner oi; + NS::Struct ns_struct; } From 1241bec3327e376b76b0daf827137e933a455c1d Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Wed, 15 Jan 2025 17:00:54 +0100 Subject: [PATCH 2/3] test unions --- lldb/test/API/lang/cpp/nested-template/TestNestedTemplate.py | 5 +++++ lldb/test/API/lang/cpp/nested-template/main.cpp | 2 ++ 2 files changed, 7 insertions(+) diff --git a/lldb/test/API/lang/cpp/nested-template/TestNestedTemplate.py b/lldb/test/API/lang/cpp/nested-template/TestNestedTemplate.py index 7a73619555fa0..055a8e6e21042 100644 --- a/lldb/test/API/lang/cpp/nested-template/TestNestedTemplate.py +++ b/lldb/test/API/lang/cpp/nested-template/TestNestedTemplate.py @@ -22,6 +22,11 @@ def do_test(self, debug_flags): DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"], ) + self.expect( + "image lookup -A -t 'NS::Union'", + DATA_TYPES_DISPLAYED_CORRECTLY, + substrs=["1 match found"], + ) @skipIf(compiler=no_match("clang")) @skipIf(compiler_version=["<", "15.0"]) diff --git a/lldb/test/API/lang/cpp/nested-template/main.cpp b/lldb/test/API/lang/cpp/nested-template/main.cpp index eef7c021e22ea..9bef73052825f 100644 --- a/lldb/test/API/lang/cpp/nested-template/main.cpp +++ b/lldb/test/API/lang/cpp/nested-template/main.cpp @@ -8,10 +8,12 @@ struct Outer { namespace NS { namespace { template struct Struct {}; +template struct Union {}; } // namespace } // namespace NS int main() { Outer::Inner oi; NS::Struct ns_struct; + NS::Union ns_union; } From d26f1b0d2f2e294d8b4477230a3318fd330d0af3 Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Wed, 15 Jan 2025 17:02:10 +0100 Subject: [PATCH 3/3] rename arg --- .../Plugins/SymbolFile/DWARF/DWARFDIE.cpp | 20 +++++++++---------- .../Plugins/SymbolFile/DWARF/DWARFDIE.h | 8 ++++---- .../SymbolFile/DWARF/SymbolFileDWARF.cpp | 4 ++-- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp index f2b02d3d6898d..1e2564cb22f25 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp @@ -379,7 +379,7 @@ lldb_private::Type *DWARFDIE::ResolveTypeUID(const DWARFDIE &die) const { } static CompilerContext GetContextEntry(DWARFDIE die, - bool complete_template_names) { + bool derive_template_names) { auto ctx = [die](CompilerContextKind kind) { return CompilerContext(kind, ConstString(die.GetName())); }; @@ -406,7 +406,7 @@ static CompilerContext GetContextEntry(DWARFDIE die, ? CompilerContextKind::Union : CompilerContextKind::ClassOrStruct; llvm::StringRef name = die.GetName(); - if (!complete_template_names || name.contains('<')) + if (!derive_template_names || name.contains('<')) return CompilerContext(kind, ConstString(name)); std::string name_storage = name.str(); @@ -420,7 +420,7 @@ static CompilerContext GetContextEntry(DWARFDIE die, } } -static void GetDeclContextImpl(DWARFDIE die, bool complete_template_names, +static void GetDeclContextImpl(DWARFDIE die, bool derive_template_names, llvm::SmallSet &seen, std::vector &context) { // Stop if we hit a cycle. @@ -442,7 +442,7 @@ static void GetDeclContextImpl(DWARFDIE die, bool complete_template_names, case DW_TAG_subprogram: case DW_TAG_variable: case DW_TAG_typedef: - context.push_back(GetContextEntry(die, complete_template_names)); + context.push_back(GetContextEntry(die, derive_template_names)); break; default: break; @@ -453,15 +453,15 @@ static void GetDeclContextImpl(DWARFDIE die, bool complete_template_names, } std::vector -DWARFDIE::GetDeclContext(bool complete_template_names) const { +DWARFDIE::GetDeclContext(bool derive_template_names) const { llvm::SmallSet seen; std::vector context; - GetDeclContextImpl(*this, complete_template_names, seen, context); + GetDeclContextImpl(*this, derive_template_names, seen, context); std::reverse(context.begin(), context.end()); return context; } -static void GetTypeLookupContextImpl(DWARFDIE die, bool complete_template_names, +static void GetTypeLookupContextImpl(DWARFDIE die, bool derive_template_names, llvm::SmallSet &seen, std::vector &context) { // Stop if we hit a cycle. @@ -476,7 +476,7 @@ static void GetTypeLookupContextImpl(DWARFDIE die, bool complete_template_names, case DW_TAG_variable: case DW_TAG_typedef: case DW_TAG_base_type: - context.push_back(GetContextEntry(die, complete_template_names)); + context.push_back(GetContextEntry(die, derive_template_names)); break; // If any of the tags below appear in the parent chain, stop the decl @@ -500,10 +500,10 @@ static void GetTypeLookupContextImpl(DWARFDIE die, bool complete_template_names, } std::vector -DWARFDIE::GetTypeLookupContext(bool complete_template_names) const { +DWARFDIE::GetTypeLookupContext(bool derive_template_names) const { llvm::SmallSet seen; std::vector context; - GetTypeLookupContextImpl(*this, complete_template_names, seen, context); + GetTypeLookupContextImpl(*this, derive_template_names, seen, context); std::reverse(context.begin(), context.end()); return context; } diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h index b2df9a3c43a75..8785ac09b1f14 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h @@ -74,14 +74,14 @@ class DWARFDIE : public DWARFBaseDIE { /// in Clang modules. This context will include any modules or functions that /// the type is declared in so an exact module match can be efficiently made. /// - /// \param[in] complete_template_names + /// \param[in] derive_template_names /// If true, augments the returned names with template arguments derived /// from the child DIEs, if the names don't contained template arguments /// already. If false, the returned context will contain the names exactly /// as they are spelled in the debug info, regardless of whether that /// includes template arguments or not. std::vector - GetDeclContext(bool complete_template_names = false) const; + GetDeclContext(bool derive_template_names = false) const; /// Get a context to a type so it can be looked up. /// @@ -94,14 +94,14 @@ class DWARFDIE : public DWARFBaseDIE { /// context. This is designed to allow users to efficiently look for types /// using a full or partial CompilerContext array. /// - /// \param[in] complete_template_names + /// \param[in] derive_template_names /// If true, augments the returned names with template arguments derived /// from the child DIEs, if the names don't contained template arguments /// already. If false, the returned context will contain the names exactly /// as they are spelled in the debug info, regardless of whether that /// includes template arguments or not. std::vector - GetTypeLookupContext(bool complete_template_names = false) const; + GetTypeLookupContext(bool derive_template_names = false) const; DWARFDeclContext GetDWARFDeclContext() const; diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp index 0821aa0050fc6..2f451d173c4dd 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp @@ -2740,8 +2740,8 @@ void SymbolFileDWARF::FindTypes(const TypeQuery &query, TypeResults &results) { m_index->GetTypesWithQuery(query_simple, [&](DWARFDIE die) { std::vector qualified_context = query.GetModuleSearch() - ? die.GetDeclContext(/*complete_template_names=*/true) - : die.GetTypeLookupContext(/*complete_template_names=*/true); + ? die.GetDeclContext(/*derive_template_names=*/true) + : die.GetTypeLookupContext(/*derive_template_names=*/true); if (query.ContextMatches(qualified_context)) if (Type *matching_type = ResolveType(die, true, true)) results.InsertUnique(matching_type->shared_from_this());