Skip to content

Revert "[CLANG][DWARF] Handle DIE offset collision in DW_IDX_parent" #95302

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 12, 2024

Conversation

fmayer
Copy link
Contributor

@fmayer fmayer commented Jun 12, 2024

Reverts #95039

This looks like it caused the ASan bot to fail: https://lab.llvm.org/buildbot/#/builders/168/builds/20912

Offending line was changed in this PR

@fmayer fmayer requested a review from vitalybuka June 12, 2024 20:26
@llvmbot
Copy link
Member

llvmbot commented Jun 12, 2024

@llvm/pr-subscribers-debuginfo

Author: Florian Mayer (fmayer)

Changes

Reverts llvm/llvm-project#95039

This looks like it caused the ASan bot to fail: https://lab.llvm.org/buildbot/#/builders/168/builds/20912

Offending line was changed in this PR


Full diff: https://github.com/llvm/llvm-project/pull/95302.diff

5 Files Affected:

  • (modified) llvm/include/llvm/CodeGen/AccelTable.h (+15-34)
  • (modified) llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp (+1-2)
  • (modified) llvm/lib/DWARFLinker/Classic/DWARFLinker.cpp (+3-6)
  • (modified) llvm/lib/DWARFLinker/Parallel/DWARFLinkerImpl.cpp (+3-4)
  • (removed) llvm/test/DebugInfo/X86/debug-names-types-die-offset-collision.ll (-69)
diff --git a/llvm/include/llvm/CodeGen/AccelTable.h b/llvm/include/llvm/CodeGen/AccelTable.h
index 622fcf019aad6..cff8fcbaf2cd7 100644
--- a/llvm/include/llvm/CodeGen/AccelTable.h
+++ b/llvm/include/llvm/CodeGen/AccelTable.h
@@ -257,38 +257,18 @@ class AppleAccelTableData : public AccelTableData {
 
 /// Helper class to identify an entry in DWARF5AccelTable based on their DIE
 /// offset and UnitID.
-struct OffsetAndUnitID {
-  uint64_t Offset = 0;
-  uint32_t UnitID = 0;
-  bool IsTU = false;
-  OffsetAndUnitID() = default;
-  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; }
-};
+struct OffsetAndUnitID : std::pair<uint64_t, uint32_t> {
+  using Base = std::pair<uint64_t, uint32_t>;
+  OffsetAndUnitID(Base B) : Base(B) {}
 
-template <> struct DenseMapInfo<OffsetAndUnitID> {
-  static inline OffsetAndUnitID getEmptyKey() {
-    OffsetAndUnitID Entry;
-    Entry.Offset = uint64_t(-1);
-    return Entry;
-  }
-  static inline OffsetAndUnitID getTombstoneKey() {
-    OffsetAndUnitID Entry;
-    Entry.Offset = uint64_t(-2);
-    return Entry;
-  }
-  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();
-  }
+  OffsetAndUnitID(uint64_t Offset, uint32_t UnitID) : Base(Offset, UnitID) {}
+  uint64_t offset() const { return first; };
+  uint32_t unitID() const { return second; };
 };
 
+template <>
+struct DenseMapInfo<OffsetAndUnitID> : DenseMapInfo<OffsetAndUnitID::Base> {};
+
 /// 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
 /// serialize itself. The complete serialization logic is in the
@@ -297,11 +277,12 @@ 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);
+  DWARF5AccelTableData(const DIE &Die, const uint32_t UnitID,
+                       const bool IsTU = false);
   DWARF5AccelTableData(const uint64_t DieOffset,
                        const std::optional<uint64_t> DefiningParentOffset,
                        const unsigned DieTag, const unsigned UnitID,
-                       const bool IsTU)
+                       const bool IsTU = false)
       : OffsetVal(DieOffset), ParentOffset(DefiningParentOffset),
         DieTag(DieTag), AbbrevNumber(0), IsTU(IsTU), UnitID(UnitID) {}
 
@@ -315,7 +296,7 @@ class DWARF5AccelTableData : public AccelTableData {
   }
 
   OffsetAndUnitID getDieOffsetAndUnitID() const {
-    return {getDieOffset(), getUnitID(), isTU()};
+    return {getDieOffset(), UnitID};
   }
 
   unsigned getDieTag() const { return DieTag; }
@@ -341,7 +322,7 @@ class DWARF5AccelTableData : public AccelTableData {
     assert(isNormalized() && "Accessing DIE Offset before normalizing.");
     if (!ParentOffset)
       return std::nullopt;
-    return OffsetAndUnitID(*ParentOffset, getUnitID(), isTU());
+    return OffsetAndUnitID(*ParentOffset, getUnitID());
   }
 
   /// Sets AbbrevIndex for an Entry.
@@ -435,7 +416,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(), Data->isTU());
+                Data->getUnitID(), true);
       }
     }
   }
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
index 7de9432325d8a..b9c02aed848cc 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
@@ -3592,8 +3592,7 @@ 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(),
-                    Unit.getUnitDie().getTag() == dwarf::DW_TAG_type_unit);
+    Current.addName(Ref, Die, Unit.getUniqueID());
     break;
   }
   case AccelTableKind::Default:
diff --git a/llvm/lib/DWARFLinker/Classic/DWARFLinker.cpp b/llvm/lib/DWARFLinker/Classic/DWARFLinker.cpp
index f6aaf88bb5fb7..2544d97eaafd0 100644
--- a/llvm/lib/DWARFLinker/Classic/DWARFLinker.cpp
+++ b/llvm/lib/DWARFLinker/Classic/DWARFLinker.cpp
@@ -2247,20 +2247,17 @@ void DWARFLinker::emitAcceleratorEntriesForUnit(CompileUnit &Unit) {
         DebugNames.addName(
             Namespace.Name, Namespace.Die->getOffset(),
             DWARF5AccelTableData::getDefiningParentDieOffset(*Namespace.Die),
-            Namespace.Die->getTag(), Unit.getUniqueID(),
-            Unit.getOutputUnitDIE()->getTag() == dwarf::DW_TAG_type_unit);
+            Namespace.Die->getTag(), Unit.getUniqueID());
       for (const auto &Pubname : Unit.getPubnames())
         DebugNames.addName(
             Pubname.Name, Pubname.Die->getOffset(),
             DWARF5AccelTableData::getDefiningParentDieOffset(*Pubname.Die),
-            Pubname.Die->getTag(), Unit.getUniqueID(),
-            Unit.getOutputUnitDIE()->getTag() == dwarf::DW_TAG_type_unit);
+            Pubname.Die->getTag(), Unit.getUniqueID());
       for (const auto &Pubtype : Unit.getPubtypes())
         DebugNames.addName(
             Pubtype.Name, Pubtype.Die->getOffset(),
             DWARF5AccelTableData::getDefiningParentDieOffset(*Pubtype.Die),
-            Pubtype.Die->getTag(), Unit.getUniqueID(),
-            Unit.getOutputUnitDIE()->getTag() == dwarf::DW_TAG_type_unit);
+            Pubtype.Die->getTag(), Unit.getUniqueID());
     } break;
     }
   }
diff --git a/llvm/lib/DWARFLinker/Parallel/DWARFLinkerImpl.cpp b/llvm/lib/DWARFLinker/Parallel/DWARFLinkerImpl.cpp
index a068ec1a3056d..e68bf0c227a0a 100644
--- a/llvm/lib/DWARFLinker/Parallel/DWARFLinkerImpl.cpp
+++ b/llvm/lib/DWARFLinker/Parallel/DWARFLinkerImpl.cpp
@@ -1356,10 +1356,9 @@ void DWARFLinkerImpl::emitDWARFv5DebugNamesSection(const Triple &TargetTriple) {
       case DwarfUnit::AccelType::Name:
       case DwarfUnit::AccelType::Namespace:
       case DwarfUnit::AccelType::Type: {
-        DebugNames->addName(
-            *DebugStrStrings.getExistingEntry(Info.String), Info.OutOffset,
-            std::nullopt /*ParentDIEOffset*/, Info.Tag, CU->getUniqueID(),
-            CU->getOutUnitDIE()->getTag() == dwarf::DW_TAG_type_unit);
+        DebugNames->addName(*DebugStrStrings.getExistingEntry(Info.String),
+                            Info.OutOffset, std::nullopt /*ParentDIEOffset*/,
+                            Info.Tag, CU->getUniqueID());
       } break;
 
       default:
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
deleted file mode 100644
index 104a166ffd3ad..0000000000000
--- a/llvm/test/DebugInfo/X86/debug-names-types-die-offset-collision.ll
+++ /dev/null
@@ -1,69 +0,0 @@
-; 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)

@fmayer fmayer merged commit 0cb2ae3 into main Jun 12, 2024
6 of 7 checks passed
@fmayer fmayer deleted the revert-95039-T191529541 branch June 12, 2024 20:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants