Skip to content

[CLANG][DWARF] Handle DIE offset collision in DW_IDX_parent #95039

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.

Sign up for GitHub

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
Merged

[CLANG][DWARF] Handle DIE offset collision in DW_IDX_parent #95039

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 34 additions & 15 deletions llvm/include/llvm/CodeGen/AccelTable.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,17 +257,37 @@ 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> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would it make it sense to just use tuple here to avoid having to define the dense map boilerplate?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really have a strong opinion. Poking around I don't really see tuple being used with DenseMap. More standard approach seems this boilerplate code.
@dwblaikie to break a tie? :)

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

template <>
struct DenseMapInfo<OffsetAndUnitID> : DenseMapInfo<OffsetAndUnitID::Base> {};
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();
}
};

/// 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
Expand All @@ -277,12 +297,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) {}

Expand All @@ -296,7 +315,7 @@ class DWARF5AccelTableData : public AccelTableData {
}

OffsetAndUnitID getDieOffsetAndUnitID() const {
return {getDieOffset(), UnitID};
return {getDieOffset(), getUnitID(), isTU()};
}

unsigned getDieTag() const { return DieTag; }
Expand All @@ -322,7 +341,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.
Expand Down Expand Up @@ -416,7 +435,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());
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
9 changes: 6 additions & 3 deletions llvm/lib/DWARFLinker/Classic/DWARFLinker.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.getOutputUnitDIE()->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.getOutputUnitDIE()->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.getOutputUnitDIE()->getTag() == dwarf::DW_TAG_type_unit);
} break;
}
}
Expand Down
7 changes: 4 additions & 3 deletions llvm/lib/DWARFLinker/Parallel/DWARFLinkerImpl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1356,9 +1356,10 @@ 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());
DebugNames->addName(
*DebugStrStrings.getExistingEntry(Info.String), Info.OutOffset,
std::nullopt /*ParentDIEOffset*/, Info.Tag, CU->getUniqueID(),
CU->getOutUnitDIE()->getTag() == dwarf::DW_TAG_type_unit);
} break;

default:
Expand Down
69 changes: 69 additions & 0 deletions llvm/test/DebugInfo/X86/debug-names-types-die-offset-collision.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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)
Loading