[llvm][dsymutil] Use the DW_AT_name of the uniqued DIE for insertion into .debug_names

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,

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<DeclContext *> {
       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;
   }

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<DWARFFormValue> 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);

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<DeclContext *, 1>(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<DeclContext *, 1>(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,23 +155,25 @@ 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.
   if (IsAnonymousNamespace)
     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;

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 }

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")
 

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
 

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

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<decltype([]{})>();
+}

llvm/test/tools/dsymutil/Inputs/odr-uniquing-DW_AT_name-conflict/lib1.h

@@ -0,0 +1,3 @@
+struct Foo {
+  template<typename T> void func() {}
+};

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<decltype([]{})>();
+}

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();
+}