[CLANG][DWARF] Handle DIE offset collision in DW_IDX_parent (#95339)

This fixes #93886. The UnitID is not unique between CUs and TUs. This led to DW_IDX_parent to point ot an entry for a DIE in CU if it had the same relative offset as TU die. Added a IsTU to the hash for parent chain.
llvm · Jun 14, 2024 · 9b7b1be · 9b7b1be
1 parent 74fe1da
commit 9b7b1be
Show file tree

Hide file tree

Showing 7 changed files with 120 additions and 21 deletions.
diff --git a/llvm/include/llvm/CodeGen/AccelTable.h b/llvm/include/llvm/CodeGen/AccelTable.h
@@ -257,17 +257,33 @@ class AppleAccelTableData : public AccelTableData {
 
 /// Helper class to identify an entry in DWARF5AccelTable based on their DIE
 /// offset and UnitID.
-struct OffsetAndUnitID : std::pair<uint64_t, uint32_t> {
-  using Base = std::pair<uint64_t, uint32_t>;
-  OffsetAndUnitID(Base B) : Base(B) {}
-
-  OffsetAndUnitID(uint64_t Offset, uint32_t UnitID) : Base(Offset, UnitID) {}
-  uint64_t offset() const { return first; };
-  uint32_t unitID() const { return second; };
+struct OffsetAndUnitID {
+  uint64_t Offset = 0;
+  uint32_t UnitID = 0;
+  bool IsTU = false;
+  OffsetAndUnitID() = delete;
+  OffsetAndUnitID(uint64_t Offset, uint32_t UnitID, bool IsTU)
+      : Offset(Offset), UnitID(UnitID), IsTU(IsTU) {}
+  uint64_t offset() const { return Offset; };
+  uint32_t unitID() const { return UnitID; };
+  bool isTU() const { return IsTU; }
 };
 
-template <>
-struct DenseMapInfo<OffsetAndUnitID> : DenseMapInfo<OffsetAndUnitID::Base> {};
+template <> struct DenseMapInfo<OffsetAndUnitID> {
+  static inline OffsetAndUnitID getEmptyKey() {
+    return OffsetAndUnitID(-1, -1, false);
+  }
+  static inline OffsetAndUnitID getTombstoneKey() {
+    return OffsetAndUnitID(-2, -2, false);
+  }
+  static unsigned getHashValue(const OffsetAndUnitID &Val) {
+    return (unsigned)llvm::hash_combine(Val.offset(), Val.unitID(), Val.IsTU);
+  }
+  static bool isEqual(const OffsetAndUnitID &LHS, const OffsetAndUnitID &RHS) {
+    return LHS.offset() == RHS.offset() && LHS.unitID() == RHS.unitID() &&
+           LHS.IsTU == RHS.isTU();
+  }
+};
 
 /// The Data class implementation for DWARF v5 accelerator table. Unlike the
 /// Apple Data classes, this class is just a DIE wrapper, and does not know to
@@ -277,12 +293,11 @@ class DWARF5AccelTableData : public AccelTableData {
 public:
   static uint32_t hash(StringRef Name) { return caseFoldingDjbHash(Name); }
 
-  DWARF5AccelTableData(const DIE &Die, const uint32_t UnitID,
-                       const bool IsTU = false);
+  DWARF5AccelTableData(const DIE &Die, const uint32_t UnitID, const bool IsTU);
   DWARF5AccelTableData(const uint64_t DieOffset,
                        const std::optional<uint64_t> DefiningParentOffset,
                        const unsigned DieTag, const unsigned UnitID,
-                       const bool IsTU = false)
+                       const bool IsTU)
       : OffsetVal(DieOffset), ParentOffset(DefiningParentOffset),
         DieTag(DieTag), AbbrevNumber(0), IsTU(IsTU), UnitID(UnitID) {}
 
@@ -296,7 +311,7 @@ class DWARF5AccelTableData : public AccelTableData {
   }
 
   OffsetAndUnitID getDieOffsetAndUnitID() const {
-    return {getDieOffset(), UnitID};
+    return {getDieOffset(), getUnitID(), isTU()};
   }
 
   unsigned getDieTag() const { return DieTag; }
@@ -322,7 +337,7 @@ class DWARF5AccelTableData : public AccelTableData {
     assert(isNormalized() && "Accessing DIE Offset before normalizing.");
     if (!ParentOffset)
       return std::nullopt;
-    return OffsetAndUnitID(*ParentOffset, getUnitID());
+    return OffsetAndUnitID(*ParentOffset, getUnitID(), isTU());
   }
 
   /// Sets AbbrevIndex for an Entry.
@@ -416,7 +431,7 @@ class DWARF5AccelTable : public AccelTable<DWARF5AccelTableData> {
       for (auto *Data : Entry.second.getValues<DWARF5AccelTableData *>()) {
         addName(Entry.second.Name, Data->getDieOffset(),
                 Data->getParentDieOffset(), Data->getDieTag(),
-                Data->getUnitID(), true);
+                Data->getUnitID(), Data->isTU());
       }
     }
   }

diff --git a/llvm/include/llvm/DWARFLinker/Classic/DWARFLinkerCompileUnit.h b/llvm/include/llvm/DWARFLinker/Classic/DWARFLinkerCompileUnit.h
@@ -137,6 +137,8 @@ class CompileUnit {
     return nullptr;
   }
 
+  dwarf::Tag getTag() const { return OrigUnit.getUnitDIE().getTag(); }
+
   bool hasODR() const { return HasODR; }
   bool isClangModule() const { return !ClangModuleName.empty(); }
   uint16_t getLanguage();

diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
@@ -3592,7 +3592,8 @@ void DwarfDebug::addAccelNameImpl(
                "Kind is TU but CU is being processed.");
     // The type unit can be discarded, so need to add references to final
     // acceleration table once we know it's complete and we emit it.
-    Current.addName(Ref, Die, Unit.getUniqueID());
+    Current.addName(Ref, Die, Unit.getUniqueID(),
+                    Unit.getUnitDie().getTag() == dwarf::DW_TAG_type_unit);
     break;
   }
   case AccelTableKind::Default:

diff --git a/llvm/lib/DWARFLinker/Classic/DWARFLinker.cpp b/llvm/lib/DWARFLinker/Classic/DWARFLinker.cpp
@@ -2247,17 +2247,20 @@ void DWARFLinker::emitAcceleratorEntriesForUnit(CompileUnit &Unit) {
         DebugNames.addName(
             Namespace.Name, Namespace.Die->getOffset(),
             DWARF5AccelTableData::getDefiningParentDieOffset(*Namespace.Die),
-            Namespace.Die->getTag(), Unit.getUniqueID());
+            Namespace.Die->getTag(), Unit.getUniqueID(),
+            Unit.getTag() == dwarf::DW_TAG_type_unit);
       for (const auto &Pubname : Unit.getPubnames())
         DebugNames.addName(
             Pubname.Name, Pubname.Die->getOffset(),
             DWARF5AccelTableData::getDefiningParentDieOffset(*Pubname.Die),
-            Pubname.Die->getTag(), Unit.getUniqueID());
+            Pubname.Die->getTag(), Unit.getUniqueID(),
+            Unit.getTag() == dwarf::DW_TAG_type_unit);
       for (const auto &Pubtype : Unit.getPubtypes())
         DebugNames.addName(
             Pubtype.Name, Pubtype.Die->getOffset(),
             DWARF5AccelTableData::getDefiningParentDieOffset(*Pubtype.Die),
-            Pubtype.Die->getTag(), Unit.getUniqueID());
+            Pubtype.Die->getTag(), Unit.getUniqueID(),
+            Unit.getTag() == dwarf::DW_TAG_type_unit);
     } break;
     }
   }

diff --git a/llvm/lib/DWARFLinker/Parallel/DWARFLinkerImpl.cpp b/llvm/lib/DWARFLinker/Parallel/DWARFLinkerImpl.cpp
@@ -1358,7 +1358,8 @@ void DWARFLinkerImpl::emitDWARFv5DebugNamesSection(const Triple &TargetTriple) {
       case DwarfUnit::AccelType::Type: {
         DebugNames->addName(*DebugStrStrings.getExistingEntry(Info.String),
                             Info.OutOffset, std::nullopt /*ParentDIEOffset*/,
-                            Info.Tag, CU->getUniqueID());
+                            Info.Tag, CU->getUniqueID(),
+                            CU->getTag() == dwarf::DW_TAG_type_unit);
       } break;
 
       default:

diff --git a/llvm/lib/DWARFLinker/Parallel/DWARFLinkerUnit.h b/llvm/lib/DWARFLinker/Parallel/DWARFLinkerUnit.h
@@ -77,10 +77,15 @@ class DwarfUnit : public OutputSections {
   void setOutUnitDIE(DIE *UnitDie) {
     OutUnitDIE = UnitDie;
 
-    if (OutUnitDIE != nullptr)
+    if (OutUnitDIE != nullptr) {
       UnitSize = getDebugInfoHeaderSize() + OutUnitDIE->getSize();
+      UnitTag = OutUnitDIE->getTag();
+    }
   }
 
+  /// Returns unit DWARF tag.
+  dwarf::Tag getTag() const { return UnitTag; }
+
   /// \defgroup Methods used to emit unit's debug info:
   ///
   /// @{
@@ -180,6 +185,9 @@ class DwarfUnit : public OutputSections {
 
   uint64_t UnitSize = 0;
 
+  /// DWARF unit tag.
+  dwarf::Tag UnitTag = dwarf::DW_TAG_null;
+
   /// true if current unit references_to/is_referenced by other unit.
   std::atomic<bool> IsInterconnectedCU = {false};
 

diff --git a/llvm/test/DebugInfo/X86/debug-names-types-die-offset-collision.ll b/llvm/test/DebugInfo/X86/debug-names-types-die-offset-collision.ll
@@ -0,0 +1,69 @@
+; UNSUPPORTED: system-windows
+
+;; This test checks that DW_IDX_parent is generated correctly when there is DIE relative offset collision between CU and TU.
+
+; RUN: llc -mtriple=x86_64 -generate-type-units -dwarf-version=5 -filetype=obj %s -o %t
+; RUN: llvm-dwarfdump -debug-info -debug-names %t | FileCheck %s
+
+; CHECK: .debug_info contents:
+; CHECK:        0x00000023:   DW_TAG_namespace
+; CHECK-NEXT:                   DW_AT_name  ("B")
+; CHECK:        0x00000023:   DW_TAG_subprogram
+; CHECK-NEXT:                   DW_AT_low_pc
+; CHECK-NEXT:                   DW_AT_high_pc
+; CHECK-NEXT:                   DW_AT_frame_base
+; CHECK-NEXT:                   DW_AT_linkage_name  ("_Z9get_statev")
+; CHECK-NEXT:                   DW_AT_name  ("get_state")
+
+; CHECK: .debug_names contents:
+; CHECK:  String: {{.*}} "B"
+; CHECK:        Entry @ [[ENTRY:0x[0-9a-f]*]]
+; CHECK:  String: {{.*}} "State"
+; CHECK:        Entry @ 0xd3 {
+; CHECK:          Abbrev: 0x4
+; CHECK:          Tag: DW_TAG_structure_type
+; CHECK:          DW_IDX_type_unit: 0x00
+; CHECK:          DW_IDX_die_offset: 0x00000025
+; CHECK:          DW_IDX_parent: Entry @ [[ENTRY:0x[0-9a-f]*]]
+; CHECK:        }
+
+
+;; namespace B { struct State { class InnerState{}; }; }
+;; B::State::InnerState get_state() { return B::State::InnerState(); }
+;; clang++ main.cpp -g2 -O0 -fdebug-types-section -gpubnames
+
+; ModuleID = 'main.cpp'
+source_filename = "main.cpp"
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Function Attrs: mustprogress noinline nounwind optnone uwtable
+define dso_local void @_Z9get_statev() #0 !dbg !10 {
+entry:
+  ret void, !dbg !17
+}
+
+attributes #0 = { mustprogress noinline nounwind optnone uwtable "frame-pointer"="all" "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cmov,+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!2, !3, !4, !5, !6, !7, !8}
+!llvm.ident = !{!9}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang version 19.0.0git", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false)
+!1 = !DIFile(filename: "main.cpp", directory: "/folder", checksumkind: CSK_MD5, checksum: "a84fe2e4ecb77633f6c33f3b6833b9e7")
+!2 = !{i32 7, !"Dwarf Version", i32 5}
+!3 = !{i32 2, !"Debug Info Version", i32 3}
+!4 = !{i32 1, !"wchar_size", i32 4}
+!5 = !{i32 8, !"PIC Level", i32 2}
+!6 = !{i32 7, !"PIE Level", i32 2}
+!7 = !{i32 7, !"uwtable", i32 2}
+!8 = !{i32 7, !"frame-pointer", i32 2}
+!9 = !{!"clang version 19.0.0git"}
+!10 = distinct !DISubprogram(name: "get_state", linkageName: "_Z9get_statev", scope: !1, file: !1, line: 2, type: !11, scopeLine: 2, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0)
+!11 = !DISubroutineType(types: !12)
+!12 = !{!13}
+!13 = distinct !DICompositeType(tag: DW_TAG_class_type, name: "InnerState", scope: !14, file: !1, line: 1, size: 8, flags: DIFlagTypePassByValue, elements: !16, identifier: "_ZTSN1B5State10InnerStateE")
+!14 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "State", scope: !15, file: !1, line: 1, size: 8, flags: DIFlagTypePassByValue, elements: !16, identifier: "_ZTSN1B5StateE")
+!15 = !DINamespace(name: "B", scope: null)
+!16 = !{}
+!17 = !DILocation(line: 2, column: 36, scope: !10)