-
Notifications
You must be signed in to change notification settings - Fork 11.9k
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
[CLANG][DWARF] Handle DIE offset collision in DW_IDX_parent #95339
Conversation
@llvm/pr-subscribers-debuginfo Author: Alexander Yermolovich (ayermolo) ChangesThis fixes #93886. The UnitID is not Added a IsTU to the hash for parent chain. Full diff: https://github.com/llvm/llvm-project/pull/95339.diff 7 Files Affected:
diff --git a/llvm/include/llvm/CodeGen/AccelTable.h b/llvm/include/llvm/CodeGen/AccelTable.h
index cff8fcbaf2cd7..f98e994b3f998 100644
--- 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
index bfe544946fd90..cdb6f4a4443ab 100644
--- 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
index b9c02aed848cc..7de9432325d8a 100644
--- 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
index 2544d97eaafd0..c6312c387744a 100644
--- 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
index e68bf0c227a0a..c060f8f4c1718 100644
--- 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
index e16b82f696a2d..84757aea7045d 100644
--- 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
new file mode 100644
index 0000000000000..104a166ffd3ad
--- /dev/null
+++ 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)
|
This fixes llvm#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.
0860211
to
9973756
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the asan fix was replacing CU->getOutUnitDIE()->getTag() == dwarf::DW_TAG_type_unit
with CU->getTag() == dwarf::DW_TAG_type_unit)
?
Also noticed you deleted the default ctor here (which is good). Just to double check, this particular bit is unrelated to the asan issue right?
For the first yes. For parallel dwarf linker it was accessing freed UnitDie. It was allocated on local BumpAllocator in CompileUnit::cloneAndEmit. For followup PR might be a good idea to reset it to nullptr there? Yes ctor is unrelated. Just a bit of cleanup when I looked at the code "second time". :) |
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.