diff --git a/llvm/include/llvm/DWARFLinker/Classic/DWARFLinker.h b/llvm/include/llvm/DWARFLinker/Classic/DWARFLinker.h index 5b9535380aebf..d6f5d926f022c 100644 --- a/llvm/include/llvm/DWARFLinker/Classic/DWARFLinker.h +++ b/llvm/include/llvm/DWARFLinker/Classic/DWARFLinker.h @@ -708,7 +708,11 @@ class LLVM_ABI DWARFLinker : public DWARFLinkerBase { /// already there. /// \returns is a name was found. bool getDIENames(const DWARFDie &Die, AttributesInfo &Info, - OffsetsStringPool &StringPool, bool StripTemplate = false); + OffsetsStringPool &StringPool, const DWARFFile &File, + CompileUnit &Unit, bool StripTemplate = false); + + llvm::StringRef getCanonicalDIEName(DWARFDie Die, const DWARFFile &File, + CompileUnit *Unit); uint32_t hashFullyQualifiedName(DWARFDie DIE, CompileUnit &U, const DWARFFile &File, diff --git a/llvm/include/llvm/DWARFLinker/Classic/DWARFLinkerDeclContext.h b/llvm/include/llvm/DWARFLinker/Classic/DWARFLinkerDeclContext.h index 9fb1b3f80e2ff..5ced6d05cc231 100644 --- a/llvm/include/llvm/DWARFLinker/Classic/DWARFLinkerDeclContext.h +++ b/llvm/include/llvm/DWARFLinker/Classic/DWARFLinkerDeclContext.h @@ -84,11 +84,13 @@ class DeclContext { DeclContext() : DefinedInClangModule(0), Parent(*this) {} DeclContext(unsigned Hash, uint32_t Line, uint32_t ByteSize, uint16_t Tag, - StringRef Name, StringRef File, const DeclContext &Parent, - DWARFDie LastSeenDIE = DWARFDie(), unsigned CUId = 0) + StringRef Name, StringRef NameForUniquing, StringRef File, + const DeclContext &Parent, DWARFDie LastSeenDIE = DWARFDie(), + unsigned CUId = 0) : QualifiedNameHash(Hash), Line(Line), ByteSize(ByteSize), Tag(Tag), - DefinedInClangModule(0), Name(Name), File(File), Parent(Parent), - LastSeenDIE(LastSeenDIE), LastSeenCompileUnitID(CUId) {} + DefinedInClangModule(0), Name(Name), NameForUniquing(NameForUniquing), + File(File), Parent(Parent), LastSeenDIE(LastSeenDIE), + LastSeenCompileUnitID(CUId) {} uint32_t getQualifiedNameHash() const { return QualifiedNameHash; } @@ -100,6 +102,7 @@ class DeclContext { uint32_t getCanonicalDIEOffset() const { return CanonicalDIEOffset; } void setCanonicalDIEOffset(uint32_t Offset) { CanonicalDIEOffset = Offset; } + llvm::StringRef getCanonicalName() const { return Name; } bool isDefinedInClangModule() const { return DefinedInClangModule; } void setDefinedInClangModule(bool Val) { DefinedInClangModule = Val; } @@ -115,6 +118,7 @@ class DeclContext { uint16_t Tag = dwarf::DW_TAG_compile_unit; unsigned DefinedInClangModule : 1; StringRef Name; + StringRef NameForUniquing; StringRef File; const DeclContext &Parent; DWARFDie LastSeenDIE; @@ -180,7 +184,7 @@ struct DeclMapInfo : private DenseMapInfo { return RHS == LHS; return LHS->QualifiedNameHash == RHS->QualifiedNameHash && LHS->Line == RHS->Line && LHS->ByteSize == RHS->ByteSize && - LHS->Name.data() == RHS->Name.data() && + LHS->NameForUniquing.data() == RHS->NameForUniquing.data() && LHS->File.data() == RHS->File.data() && LHS->Parent.QualifiedNameHash == RHS->Parent.QualifiedNameHash; } diff --git a/llvm/lib/DWARFLinker/Classic/DWARFLinker.cpp b/llvm/lib/DWARFLinker/Classic/DWARFLinker.cpp index 8637b55c78f9c..daf3788639451 100644 --- a/llvm/lib/DWARFLinker/Classic/DWARFLinker.cpp +++ b/llvm/lib/DWARFLinker/Classic/DWARFLinker.cpp @@ -151,22 +151,84 @@ static bool isTypeTag(uint16_t Tag) { return false; } -bool DWARFLinker::DIECloner::getDIENames(const DWARFDie &Die, - AttributesInfo &Info, - OffsetsStringPool &StringPool, - bool StripTemplate) { +/// Recurse through the input DIE's canonical references until we find a +/// DW_AT_name. +llvm::StringRef +DWARFLinker::DIECloner::getCanonicalDIEName(DWARFDie Die, const DWARFFile &File, + CompileUnit *Unit) { + if (!Die) + return {}; + + std::optional Ref; + + auto GetDieName = [](const DWARFDie &D) -> llvm::StringRef { + auto NameForm = D.find(llvm::dwarf::DW_AT_name); + if (!NameForm) + return {}; + + auto NameOrErr = NameForm->getAsCString(); + if (!NameOrErr) { + llvm::consumeError(NameOrErr.takeError()); + return {}; + } + + return *NameOrErr; + }; + + llvm::StringRef Name = GetDieName(Die); + if (!Name.empty()) + return Name; + + while (true) { + if (!(Ref = Die.find(llvm::dwarf::DW_AT_specification)) && + !(Ref = Die.find(llvm::dwarf::DW_AT_abstract_origin))) + break; + + Die = Linker.resolveDIEReference(File, CompileUnits, *Ref, Die, Unit); + if (!Die) + break; + + assert(Unit); + + unsigned SpecIdx = Unit->getOrigUnit().getDIEIndex(Die); + CompileUnit::DIEInfo &SpecInfo = Unit->getInfo(SpecIdx); + if (SpecInfo.Ctxt && SpecInfo.Ctxt->hasCanonicalDIE()) { + if (!SpecInfo.Ctxt->getCanonicalName().empty()) { + Name = SpecInfo.Ctxt->getCanonicalName(); + break; + } + } + + Name = GetDieName(Die); + if (!Name.empty()) + break; + } + + return Name; +} + +bool DWARFLinker::DIECloner::getDIENames( + const DWARFDie &Die, AttributesInfo &Info, OffsetsStringPool &StringPool, + const DWARFFile &File, CompileUnit &Unit, bool StripTemplate) { // This function will be called on DIEs having low_pcs and // ranges. As getting the name might be more expansive, filter out // blocks directly. if (Die.getTag() == dwarf::DW_TAG_lexical_block) return false; + // The mangled name of an specification DIE will by virtue of the + // uniquing algorithm be the same as the one it got uniqued into. + // So just use the input DIE's linkage name. if (!Info.MangledName) if (const char *MangledName = Die.getLinkageName()) Info.MangledName = StringPool.getEntry(MangledName); + // For subprograms with linkage names, we unique on the linkage name, + // so DW_AT_name's may differ between the input and canonical DIEs. + // Use the name of the canonical DIE. if (!Info.Name) - if (const char *Name = Die.getShortName()) + if (llvm::StringRef Name = getCanonicalDIEName(Die, File, &Unit); + !Name.empty()) Info.Name = StringPool.getEntry(Name); if (!Info.MangledName) @@ -1939,7 +2001,7 @@ DIE *DWARFLinker::DIECloner::cloneDIE(const DWARFDie &InputDIE, // accelerator tables too. For now stick with dsymutil's behavior. if ((Info.InDebugMap || AttrInfo.HasLowPc || AttrInfo.HasRanges) && Tag != dwarf::DW_TAG_compile_unit && - getDIENames(InputDIE, AttrInfo, DebugStrPool, + getDIENames(InputDIE, AttrInfo, DebugStrPool, File, Unit, Tag != dwarf::DW_TAG_inlined_subroutine)) { if (AttrInfo.MangledName && AttrInfo.MangledName != AttrInfo.Name) Unit.addNameAccelerator(Die, AttrInfo.MangledName, @@ -1962,7 +2024,7 @@ DIE *DWARFLinker::DIECloner::cloneDIE(const DWARFDie &InputDIE, } else if (Tag == dwarf::DW_TAG_imported_declaration && AttrInfo.Name) { Unit.addNamespaceAccelerator(Die, AttrInfo.Name); } else if (isTypeTag(Tag) && !AttrInfo.IsDeclaration) { - bool Success = getDIENames(InputDIE, AttrInfo, DebugStrPool); + bool Success = getDIENames(InputDIE, AttrInfo, DebugStrPool, File, Unit); uint64_t RuntimeLang = dwarf::toUnsigned(InputDIE.find(dwarf::DW_AT_APPLE_runtime_class)) .value_or(0); diff --git a/llvm/lib/DWARFLinker/Classic/DWARFLinkerDeclContext.cpp b/llvm/lib/DWARFLinker/Classic/DWARFLinkerDeclContext.cpp index c9c8dddce9c44..66a1ba9c6711f 100644 --- a/llvm/lib/DWARFLinker/Classic/DWARFLinkerDeclContext.cpp +++ b/llvm/lib/DWARFLinker/Classic/DWARFLinkerDeclContext.cpp @@ -84,24 +84,26 @@ DeclContextTree::getChildDeclContext(DeclContext &Context, const DWARFDie &DIE, break; } - StringRef NameRef; + StringRef Name = DIE.getShortName(); + StringRef NameForUniquing; StringRef FileRef; if (const char *LinkageName = DIE.getLinkageName()) - NameRef = StringPool.internString(LinkageName); - else if (const char *ShortName = DIE.getShortName()) - NameRef = StringPool.internString(ShortName); + NameForUniquing = StringPool.internString(LinkageName); + else if (!Name.empty()) + NameForUniquing = StringPool.internString(Name); - bool IsAnonymousNamespace = NameRef.empty() && Tag == dwarf::DW_TAG_namespace; + bool IsAnonymousNamespace = + NameForUniquing.empty() && Tag == dwarf::DW_TAG_namespace; if (IsAnonymousNamespace) { // FIXME: For dsymutil-classic compatibility. I think uniquing within // anonymous namespaces is wrong. There is no ODR guarantee there. - NameRef = "(anonymous namespace)"; + NameForUniquing = "(anonymous namespace)"; } if (Tag != dwarf::DW_TAG_class_type && Tag != dwarf::DW_TAG_structure_type && Tag != dwarf::DW_TAG_union_type && - Tag != dwarf::DW_TAG_enumeration_type && NameRef.empty()) + Tag != dwarf::DW_TAG_enumeration_type && NameForUniquing.empty()) return PointerIntPair(nullptr); unsigned Line = 0; @@ -140,10 +142,10 @@ DeclContextTree::getChildDeclContext(DeclContext &Context, const DWARFDie &DIE, } } - if (!Line && NameRef.empty()) + if (!Line && NameForUniquing.empty()) return PointerIntPair(nullptr); - // We hash NameRef, which is the mangled name, in order to get most + // We hash NameForUniquing, which is the mangled name, in order to get most // overloaded functions resolve correctly. // // Strictly speaking, hashing the Tag is only necessary for a @@ -153,7 +155,8 @@ DeclContextTree::getChildDeclContext(DeclContext &Context, const DWARFDie &DIE, // FIXME: dsymutil-classic won't unique the same type presented // once as a struct and once as a class. Using the Tag in the fully // qualified name hash to get the same effect. - unsigned Hash = hash_combine(Context.getQualifiedNameHash(), Tag, NameRef); + unsigned Hash = + hash_combine(Context.getQualifiedNameHash(), Tag, NameForUniquing); // FIXME: dsymutil-classic compatibility: when we don't have a name, // use the filename. @@ -161,15 +164,16 @@ DeclContextTree::getChildDeclContext(DeclContext &Context, const DWARFDie &DIE, Hash = hash_combine(Hash, FileRef); // Now look if this context already exists. - DeclContext Key(Hash, Line, ByteSize, Tag, NameRef, FileRef, Context); + DeclContext Key(Hash, Line, ByteSize, Tag, Name, NameForUniquing, FileRef, + Context); auto ContextIter = Contexts.find(&Key); if (ContextIter == Contexts.end()) { // The context wasn't found. bool Inserted; - DeclContext *NewContext = - new (Allocator) DeclContext(Hash, Line, ByteSize, Tag, NameRef, FileRef, - Context, DIE, U.getUniqueID()); + DeclContext *NewContext = new (Allocator) + DeclContext(Hash, Line, ByteSize, Tag, Name, NameForUniquing, FileRef, + Context, DIE, U.getUniqueID()); std::tie(ContextIter, Inserted) = Contexts.insert(NewContext); assert(Inserted && "Failed to insert DeclContext"); (void)Inserted; diff --git a/llvm/test/tools/dsymutil/AArch64/dummy-debug-map-arm64.map b/llvm/test/tools/dsymutil/AArch64/dummy-debug-map-arm64.map index 50d860290422c..bd2b2014ee22c 100644 --- a/llvm/test/tools/dsymutil/AArch64/dummy-debug-map-arm64.map +++ b/llvm/test/tools/dsymutil/AArch64/dummy-debug-map-arm64.map @@ -11,9 +11,13 @@ objects: - filename: 1.o symbols: - { sym: _bar, objAddr: 0x0, binAddr: 0x10000, size: 0x10 } + - { sym: __Z13lib1_internalv, objAddr: 0x0, binAddr: 0x10020, size: 0x20 } + - { sym: __ZN3Foo4funcIZ13lib1_internalvE3$_0EEvv, objAddr: 0x0, binAddr: 0x10040, size: 0x20 } - filename: 2.o symbols: - { sym: __Z3foov, objAddr: 0x0, binAddr: 0x20000, size: 0x10 } + - { sym: __Z13lib1_internalv, objAddr: 0x0, binAddr: 0x20020, size: 0x20 } + - { sym: __ZN3Foo4funcIZ13lib1_internalvE3$_0EEvv, objAddr: 0x0, binAddr: 0x20040, size: 0x20 } - filename: 3.o symbols: - { sym: __Z3foov, objAddr: 0x0, binAddr: 0x30000, size: 0x10 } diff --git a/llvm/test/tools/dsymutil/AArch64/dwarf5-str-offsets-base-strx.test b/llvm/test/tools/dsymutil/AArch64/dwarf5-str-offsets-base-strx.test index c0c4fe835682f..c5110a873c603 100644 --- a/llvm/test/tools/dsymutil/AArch64/dwarf5-str-offsets-base-strx.test +++ b/llvm/test/tools/dsymutil/AArch64/dwarf5-str-offsets-base-strx.test @@ -98,7 +98,7 @@ CHECK: DW_AT_str_offsets_base [DW_FORM_sec_offset] (0x000000 CHECK: DW_AT_comp_dir [DW_FORM_strx] (indexed (00000004) string = "/Users/shubham/Development/test109275485") CHECK: DW_TAG_subprogram -CHECK: DW_AT_low_pc [DW_FORM_addrx] (indexed (00000000) address = 0x0000000000010000) +CHECK: DW_AT_low_pc [DW_FORM_addrx] (indexed (00000000) address = 0x0000000000010040) CHECK: DW_AT_linkage_name [DW_FORM_strx] (indexed (00000005) string = "_Z4foo2i") CHECK: DW_AT_name [DW_FORM_strx] (indexed (00000006) string = "foo2") diff --git a/llvm/test/tools/dsymutil/AArch64/inlined-low_pc.c b/llvm/test/tools/dsymutil/AArch64/inlined-low_pc.c index d2d36f675e8b7..b89a6f99ebcb3 100644 --- a/llvm/test/tools/dsymutil/AArch64/inlined-low_pc.c +++ b/llvm/test/tools/dsymutil/AArch64/inlined-low_pc.c @@ -10,10 +10,10 @@ int bar(int a) { return foo(a); } // RUN: llvm-dwarfdump - | FileCheck %s // CHECK: DW_TAG_subprogram -// CHECK: DW_AT_low_pc{{.*}}0x0000000000010000 +// CHECK: DW_AT_low_pc{{.*}}0x0000000000010040 // CHECK: DW_AT_name{{.*}}"bar" // CHECK-NOT: NULL // CHECK: DW_TAG_inlined_subroutine // CHECK-NEXT: DW_AT_abstract_origin{{.*}}"foo" -// CHECK-NEXT: DW_AT_low_pc{{.*}}0x0000000000010000 +// CHECK-NEXT: DW_AT_low_pc{{.*}}0x0000000000010040 diff --git a/llvm/test/tools/dsymutil/AArch64/odr-uniquing-DW_AT_name-conflict.test b/llvm/test/tools/dsymutil/AArch64/odr-uniquing-DW_AT_name-conflict.test new file mode 100644 index 0000000000000..b6edb8bca3194 --- /dev/null +++ b/llvm/test/tools/dsymutil/AArch64/odr-uniquing-DW_AT_name-conflict.test @@ -0,0 +1,28 @@ +# Tests the case where a DW_TAG_subprogram for a method declaration +# got uniqued into a DW_TAG_subprogram with the same linkage name (but +# different DW_AT_name). Make sure the DW_TAG_subprogram DIE for the +# definition, which previously pointed to the now de-deduplicated declaration, +# gets inserted into the .debug_names table using the DW_AT_name of the canonical +# declaration DW_TAG_subprogram. +# +# Object files compiled as follows: +# clang -g -c -o 1.o Inputs/odr-uniquing-DW_AT_name-conflict/lib1.cpp +# clang -g -c -o 2.o Inputs/odr-uniquing-DW_AT_name-conflict/lib2.cpp + +# RUN: dsymutil -f -oso-prepend-path=%p/../Inputs/odr-uniquing-DW_AT_name-conflict -y %p/dummy-debug-map-arm64.map -o - \ +# RUN: | llvm-dwarfdump --verify - | FileCheck %s + +# RUN: dsymutil --linker parallel -f -oso-prepend-path=%p/../Inputs/odr-uniquing-DW_AT_name-conflict -y %p/dummy-debug-map-arm64.map -o - \ +# RUN: | not llvm-dwarfdump --verify - | FileCheck %s --check-prefix=PARALLEL-ODR + +# RUN: dsymutil -f -oso-prepend-path=%p/../Inputs/odr-uniquing-DW_AT_name-conflict -y %p/dummy-debug-map-arm64.map -no-odr -o - \ +# RUN: | llvm-dwarfdump --verify - | FileCheck %s + +# RUN: dsymutil --linker parallel -f -oso-prepend-path=%p/../Inputs/odr-uniquing-DW_AT_name-conflict -y %p/dummy-debug-map-arm64.map -no-odr -o - \ +# RUN: | llvm-dwarfdump --verify - | FileCheck %s + +# CHECK: No errors. + +# FIXME: parallel DWARFLinker uses wrong DW_AT_name when inserting uniqued subprogram into .debug_names +# PARALLEL-ODR: Verifying .debug_names... +# PARALLEL-ODR-NEXT: error: Name Index {{.*}} mismatched Name of DIE diff --git a/llvm/test/tools/dsymutil/Inputs/odr-uniquing-DW_AT_name-conflict/1.o b/llvm/test/tools/dsymutil/Inputs/odr-uniquing-DW_AT_name-conflict/1.o new file mode 100644 index 0000000000000..5932a3c89aaeb Binary files /dev/null and b/llvm/test/tools/dsymutil/Inputs/odr-uniquing-DW_AT_name-conflict/1.o differ diff --git a/llvm/test/tools/dsymutil/Inputs/odr-uniquing-DW_AT_name-conflict/2.o b/llvm/test/tools/dsymutil/Inputs/odr-uniquing-DW_AT_name-conflict/2.o new file mode 100644 index 0000000000000..607b47059e982 Binary files /dev/null and b/llvm/test/tools/dsymutil/Inputs/odr-uniquing-DW_AT_name-conflict/2.o differ diff --git a/llvm/test/tools/dsymutil/Inputs/odr-uniquing-DW_AT_name-conflict/lib1.cpp b/llvm/test/tools/dsymutil/Inputs/odr-uniquing-DW_AT_name-conflict/lib1.cpp new file mode 100644 index 0000000000000..4cf90f071ee8c --- /dev/null +++ b/llvm/test/tools/dsymutil/Inputs/odr-uniquing-DW_AT_name-conflict/lib1.cpp @@ -0,0 +1,5 @@ +#include "lib1.h" + +[[gnu::weak]] void lib1_internal() { + Foo{}.func(); +} diff --git a/llvm/test/tools/dsymutil/Inputs/odr-uniquing-DW_AT_name-conflict/lib1.h b/llvm/test/tools/dsymutil/Inputs/odr-uniquing-DW_AT_name-conflict/lib1.h new file mode 100644 index 0000000000000..3b3cefbaeac17 --- /dev/null +++ b/llvm/test/tools/dsymutil/Inputs/odr-uniquing-DW_AT_name-conflict/lib1.h @@ -0,0 +1,3 @@ +struct Foo { + template void func() {} +}; diff --git a/llvm/test/tools/dsymutil/Inputs/odr-uniquing-DW_AT_name-conflict/lib2.cpp b/llvm/test/tools/dsymutil/Inputs/odr-uniquing-DW_AT_name-conflict/lib2.cpp new file mode 100644 index 0000000000000..4cf90f071ee8c --- /dev/null +++ b/llvm/test/tools/dsymutil/Inputs/odr-uniquing-DW_AT_name-conflict/lib2.cpp @@ -0,0 +1,5 @@ +#include "lib1.h" + +[[gnu::weak]] void lib1_internal() { + Foo{}.func(); +} diff --git a/llvm/test/tools/dsymutil/Inputs/odr-uniquing-DW_AT_name-conflict/main.cpp b/llvm/test/tools/dsymutil/Inputs/odr-uniquing-DW_AT_name-conflict/main.cpp new file mode 100644 index 0000000000000..77f2cc4c8affe --- /dev/null +++ b/llvm/test/tools/dsymutil/Inputs/odr-uniquing-DW_AT_name-conflict/main.cpp @@ -0,0 +1,6 @@ +[[gnu::weak]] void lib1_internal(); + +int main() { + lib1_internal(); + __builtin_debugtrap(); +}