From 38f64ddb3f09b20c9a28e24dad99b93449a163c9 Mon Sep 17 00:00:00 2001 From: Felipe de Azevedo Piovezan Date: Tue, 9 Jan 2024 09:49:53 -0300 Subject: [PATCH 1/6] [AsmPrinter][DebugNames] Implement DW_IDX_parent entries This implements the ideas discussed in [1]. To summarize, this commit changes AsmPrinter so that it outputs DW_IDX_parent information for debug_name entries. It will enable debuggers to speed up queries for fully qualified types (based on a DWARFDeclContext) significantly, as debuggers will no longer need to parse the entire CU in order to inspect the parent chain of a DIE. Instead, a debugger can simply take the parent DIE offset from the accelerator table and peek at its name in the debug_info/debug_str sections. The implementation uses two types of DW_FORM for the DW_IDX_parent attribute: 1. DW_FORM_ref4, which points to the accelerator table entry for the parent. 2. DW_FORM_flag_present, when the entry has a parent that is not in the table (that is, the parent doesn't have a name, or isn't allowed to be in the table as per the DWARF spec). This is space-efficient, since it takes 0 bytes. The implementation works by: 1. Changing how abbreviations are encoded (so that they encode which form, if any, was used to encode IDX_Parent) 2. Creating an MCLabel per accelerator table entry, so that they may be referred by IDX_parent references. When all patches related to this are merged, we are able to show that evaluating an expression such as: ``` lldb --batch -o 'b CodeGenFunction::GenerateCode' -o run -o 'expr Fn' -- \ clang++ -c -g test.cpp -o /dev/null ``` is far faster: from ~5000 ms to ~1500ms. Building llvm-project + clang with and without this patch, and looking at its impact on object file size: ``` ls -la $(find build_stage2_Debug_idx_parent_assert_dwarf5 -name \*.cpp.o) | awk '{s+=$5} END {printf "%\047d\n", s}' 11,507,327,592 -la $(find build_stage2_Debug_no_idx_parent_assert_dwarf5 -name \*.cpp.o) | awk '{s+=$5} END {printf "%\047d\n", s}' 11,436,446,616 ``` That is, an increase of 0.62% in total object file size. Looking only at debug_names: ``` $stage1_build/bin/llvm-objdump --section-headers $(find build_stage2_Debug_idx_parent_assert_dwarf5 -name \*.cpp.o) | grep __debug_names | awk '{s+="0x"$3} END {printf "%\047d\n", s}' 440,772,348 $stage1_build/bin/llvm-objdump --section-headers $(find build_stage2_Debug_no_idx_parent_assert_dwarf5 -name \*.cpp.o) | grep __debug_names | awk '{s+="0x"$3} END {printf "%\047d\n", s}' 369,867,920 ``` That is an increase of 19%. DWARF Linkers need to be changed in order to support this. This commit already brings support to "base" linker, but it does not attempt to modify the parallel linker. Accelerator entries refer to the corresponding DIE offset, and this patch also requires the parent DIE offset -- it's not clear how the parallel linker can access this. It may be obvious to someone familiar with it, but it would be nice to get help from its authors. [1]: https://discourse.llvm.org/t/rfc-improve-dwarf-5-debug-names-type-lookup-parsing-speed/74151/ --- llvm/include/llvm/CodeGen/AccelTable.h | 23 +++- llvm/lib/CodeGen/AsmPrinter/AccelTable.cpp | 118 +++++++++++++++--- llvm/lib/DWARFLinker/Classic/DWARFLinker.cpp | 8 ++ .../DWARFLinker/Parallel/DWARFLinkerImpl.cpp | 3 +- llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp | 16 ++- .../test/DebugInfo/X86/debug-names-dwarf64.ll | 16 ++- .../DebugInfo/X86/debug-names-end-of-list.ll | 6 +- llvm/test/DebugInfo/X86/debug-names-types.ll | 58 ++++++--- .../ARM/accel-imported-declarations.test | 2 + .../ARM/dwarf5-dwarf4-combination-macho.test | 12 +- 10 files changed, 212 insertions(+), 50 deletions(-) diff --git a/llvm/include/llvm/CodeGen/AccelTable.h b/llvm/include/llvm/CodeGen/AccelTable.h index 0638fbffda4f3..12ed123ee90a0 100644 --- a/llvm/include/llvm/CodeGen/AccelTable.h +++ b/llvm/include/llvm/CodeGen/AccelTable.h @@ -270,9 +270,12 @@ class DWARF5AccelTableData : public AccelTableData { DWARF5AccelTableData(const DIE &Die, const uint32_t UnitID, const bool IsTU = false); - DWARF5AccelTableData(const uint64_t DieOffset, const unsigned DieTag, - const unsigned UnitID, const bool IsTU = false) - : OffsetVal(DieOffset), DieTag(DieTag), UnitID(UnitID), IsTU(IsTU) {} + DWARF5AccelTableData(const uint64_t DieOffset, + const std::optional ParentOffset, + const unsigned DieTag, const unsigned UnitID, + const bool IsTU = false) + : OffsetVal(DieOffset), ParentOffset(ParentOffset), DieTag(DieTag), + UnitID(UnitID), IsTU(IsTU) {} #ifndef NDEBUG void print(raw_ostream &OS) const override; @@ -287,14 +290,23 @@ class DWARF5AccelTableData : public AccelTableData { bool isTU() const { return IsTU; } void normalizeDIEToOffset() { assert(!isNormalized() && "Accessing offset after normalizing."); - OffsetVal = std::get(OffsetVal)->getOffset(); + const DIE *Entry = std::get(OffsetVal); + ParentOffset = Entry->getParent() ? Entry->getParent()->getOffset() + : std::optional(); + OffsetVal = Entry->getOffset(); } bool isNormalized() const { return std::holds_alternative(OffsetVal); } + std::optional getParentDieOffset() const { + assert(isNormalized() && "Accessing DIE Offset before normalizing."); + return ParentOffset; + } + protected: std::variant OffsetVal; + std::optional ParentOffset; uint32_t DieTag : 16; uint32_t UnitID : 15; uint32_t IsTU : 1; @@ -341,7 +353,8 @@ class DWARF5AccelTable : public AccelTable { void addTypeEntries(DWARF5AccelTable &Table) { for (auto &Entry : Table.getEntries()) { for (auto *Data : Entry.second.getValues()) { - addName(Entry.second.Name, Data->getDieOffset(), Data->getDieTag(), + addName(Entry.second.Name, Data->getDieOffset(), + Data->getParentDieOffset(), Data->getDieTag(), Data->getUnitID(), true); } } diff --git a/llvm/lib/CodeGen/AsmPrinter/AccelTable.cpp b/llvm/lib/CodeGen/AsmPrinter/AccelTable.cpp index b72c17aa6f54a..c1f214e06a4b6 100644 --- a/llvm/lib/CodeGen/AsmPrinter/AccelTable.cpp +++ b/llvm/lib/CodeGen/AsmPrinter/AccelTable.cpp @@ -13,6 +13,7 @@ #include "llvm/CodeGen/AccelTable.h" #include "DwarfCompileUnit.h" #include "DwarfUnit.h" +#include "llvm/ADT/DenseSet.h" #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/Twine.h" #include "llvm/BinaryFormat/Dwarf.h" @@ -207,7 +208,7 @@ class Dwarf5AccelTableWriter : public AccelTableWriter { }; Header Header; - DenseMap> + DenseMap> Abbreviations; ArrayRef> CompUnits; ArrayRef> TypeUnits; @@ -220,6 +221,8 @@ class Dwarf5AccelTableWriter : public AccelTableWriter { MCSymbol *EntryPool = Asm->createTempSymbol("names_entries"); // Indicates if this module is built with Split Dwarf enabled. bool IsSplitDwarf = false; + // Stores the DIE offsets which are indexed by this table. + DenseSet IndexedOffsets; void populateAbbrevsMap(); @@ -228,8 +231,11 @@ class Dwarf5AccelTableWriter : public AccelTableWriter { void emitBuckets() const; void emitStringOffsets() const; void emitAbbrevs() const; - void emitEntry(const DWARF5AccelTableData &Entry) const; - void emitData() const; + void + emitEntry(const DWARF5AccelTableData &Entry, + const DenseMap &DIEOffsetToAccelEntryLabel, + DenseSet &EmittedAccelEntrySymbols) const; + void emitData(); public: Dwarf5AccelTableWriter( @@ -395,23 +401,65 @@ void Dwarf5AccelTableWriter::Header::emit(Dwarf5AccelTableWriter &Ctx) { Asm->OutStreamer->emitBytes({AugmentationString, AugmentationStringSize}); } -static uint32_t constexpr LowerBitSize = dwarf::DW_IDX_type_hash; +enum IdxParentEncoding : uint8_t { + NoIndexedParent = 0, // Parent information present but parent isn't indexed. + Ref4 = 1, // Parent information present and parent is indexed. + NoParent = 2, // Parent information missing. +}; + +static uint32_t constexpr NumBitsIdxParent = 2; + +uint8_t encodeIdxParent(std::optional MaybeParentForm) { + if (!MaybeParentForm) + return NoParent; + switch (*MaybeParentForm) { + case dwarf::Form::DW_FORM_flag_present: + return NoIndexedParent; + case dwarf::Form::DW_FORM_ref4: + return Ref4; + default: + break; + } + // This is not crashing on bad input: we should only reach this if the + // internal compiler logic is faulty; see getFormForIdxParent. + llvm_unreachable("Bad form for IDX_parent"); +} + +static uint32_t constexpr ParentBitOffset = dwarf::DW_IDX_type_hash; +static uint32_t constexpr TagBitOffset = ParentBitOffset + NumBitsIdxParent; static uint32_t getTagFromAbbreviationTag(const uint32_t AbbrvTag) { - return AbbrvTag >> LowerBitSize; + return AbbrvTag >> TagBitOffset; } /// Constructs a unique AbbrevTag that captures what a DIE accesses. /// Using this tag we can emit a unique abbreviation for each DIE. static uint32_t constructAbbreviationTag( const unsigned Tag, - const std::optional &EntryRet) { + const std::optional &EntryRet, + std::optional MaybeParentForm) { uint32_t AbbrvTag = 0; if (EntryRet) AbbrvTag |= 1 << EntryRet->Encoding.Index; AbbrvTag |= 1 << dwarf::DW_IDX_die_offset; - AbbrvTag |= Tag << LowerBitSize; + AbbrvTag |= 1 << dwarf::DW_IDX_parent; + AbbrvTag |= encodeIdxParent(MaybeParentForm) << ParentBitOffset; + AbbrvTag |= Tag << TagBitOffset; return AbbrvTag; } + +static std::optional +getFormForIdxParent(const DenseSet &IndexedOffsets, + std::optional ParentOffset) { + // No parent information + if (!ParentOffset) + return std::nullopt; + // Parent is indexed by this table. + if (IndexedOffsets.contains(*ParentOffset)) + return dwarf::Form::DW_FORM_ref4; + // Parent is not indexed by this table. + return dwarf::Form::DW_FORM_flag_present; +} + void Dwarf5AccelTableWriter::populateAbbrevsMap() { for (auto &Bucket : Contents.getBuckets()) { for (auto *Hash : Bucket) { @@ -419,12 +467,17 @@ void Dwarf5AccelTableWriter::populateAbbrevsMap() { std::optional EntryRet = getIndexForEntry(*Value); unsigned Tag = Value->getDieTag(); - uint32_t AbbrvTag = constructAbbreviationTag(Tag, EntryRet); + std::optional MaybeParentForm = + getFormForIdxParent(IndexedOffsets, Value->getParentDieOffset()); + uint32_t AbbrvTag = + constructAbbreviationTag(Tag, EntryRet, MaybeParentForm); if (Abbreviations.count(AbbrvTag) == 0) { - SmallVector UA; + SmallVector UA; if (EntryRet) UA.push_back(EntryRet->Encoding); UA.push_back({dwarf::DW_IDX_die_offset, dwarf::DW_FORM_ref4}); + if (MaybeParentForm) + UA.push_back({dwarf::DW_IDX_parent, *MaybeParentForm}); Abbreviations.try_emplace(AbbrvTag, UA); } } @@ -496,15 +549,32 @@ void Dwarf5AccelTableWriter::emitAbbrevs() const { } void Dwarf5AccelTableWriter::emitEntry( - const DWARF5AccelTableData &Entry) const { + const DWARF5AccelTableData &Entry, + const DenseMap &DIEOffsetToAccelEntryLabel, + DenseSet &EmittedAccelEntrySymbols) const { std::optional EntryRet = getIndexForEntry(Entry); - uint32_t AbbrvTag = constructAbbreviationTag(Entry.getDieTag(), EntryRet); + std::optional MaybeParentOffset = Entry.getParentDieOffset(); + std::optional MaybeParentForm = + getFormForIdxParent(IndexedOffsets, MaybeParentOffset); + uint32_t AbbrvTag = + constructAbbreviationTag(Entry.getDieTag(), EntryRet, MaybeParentForm); auto AbbrevIt = Abbreviations.find(AbbrvTag); assert(AbbrevIt != Abbreviations.end() && "Why wasn't this abbrev generated?"); assert(getTagFromAbbreviationTag(AbbrevIt->first) == Entry.getDieTag() && "Invalid Tag"); + + auto EntrySymbolIt = DIEOffsetToAccelEntryLabel.find(Entry.getDieOffset()); + assert(EntrySymbolIt != DIEOffsetToAccelEntryLabel.end()); + MCSymbol *EntrySymbol = EntrySymbolIt->getSecond(); + + // Emit the label for this Entry, so that IDX_parents may refer to it. + // Note: a DIE may have multiple accelerator Entries; this check avoids + // creating/emitting multiple labels for the same DIE. + if (EmittedAccelEntrySymbols.insert(EntrySymbol).second) + Asm->OutStreamer->emitLabel(EntrySymbol); + Asm->emitULEB128(AbbrevIt->first, "Abbreviation code"); for (const auto &AttrEnc : AbbrevIt->second) { @@ -520,20 +590,34 @@ void Dwarf5AccelTableWriter::emitEntry( assert(AttrEnc.Form == dwarf::DW_FORM_ref4); Asm->emitInt32(Entry.getDieOffset()); break; + case dwarf::DW_IDX_parent: { + if (AttrEnc.Form == dwarf::Form::DW_FORM_flag_present) + break; + auto ParentSymbolIt = DIEOffsetToAccelEntryLabel.find(*MaybeParentOffset); + assert(ParentSymbolIt != DIEOffsetToAccelEntryLabel.end()); + Asm->emitLabelDifference(ParentSymbolIt->getSecond(), EntryPool, 4); + break; + } default: llvm_unreachable("Unexpected index attribute!"); } } } -void Dwarf5AccelTableWriter::emitData() const { +void Dwarf5AccelTableWriter::emitData() { + DenseMap DIEOffsetToAccelEntryLabel; + + for (auto Offset : IndexedOffsets) + DIEOffsetToAccelEntryLabel[Offset] = Asm->createTempSymbol("symbol"); + Asm->OutStreamer->emitLabel(EntryPool); + DenseSet EmittedAccelEntrySymbols; for (auto &Bucket : Contents.getBuckets()) { for (auto *Hash : Bucket) { // Remember to emit the label for our offset. Asm->OutStreamer->emitLabel(Hash->Sym); - for (const auto *Value : Hash->Values) - emitEntry(*static_cast(Value)); + for (const auto *Value : Hash->getValues()) + emitEntry(*Value, DIEOffsetToAccelEntryLabel, EmittedAccelEntrySymbols); Asm->OutStreamer->AddComment("End of list: " + Hash->Name.getString()); Asm->emitInt8(0); } @@ -555,6 +639,12 @@ Dwarf5AccelTableWriter::Dwarf5AccelTableWriter( CompUnits(CompUnits), TypeUnits(TypeUnits), getIndexForEntry(std::move(getIndexForEntry)), IsSplitDwarf(IsSplitDwarf) { + + for (auto &Bucket : Contents.getBuckets()) + for (auto *Hash : Bucket) + for (auto *Value : Hash->getValues()) + IndexedOffsets.insert(Value->getDieOffset()); + populateAbbrevsMap(); } diff --git a/llvm/lib/DWARFLinker/Classic/DWARFLinker.cpp b/llvm/lib/DWARFLinker/Classic/DWARFLinker.cpp index 8d76c3bcf672e..66d9b0ae9841f 100644 --- a/llvm/lib/DWARFLinker/Classic/DWARFLinker.cpp +++ b/llvm/lib/DWARFLinker/Classic/DWARFLinker.cpp @@ -2252,14 +2252,22 @@ void DWARFLinker::emitAcceleratorEntriesForUnit(CompileUnit &Unit) { TheDwarfEmitter->emitPubTypesForUnit(Unit); } break; case AccelTableKind::DebugNames: { + auto ParentOffsetOrNull = [](const DIE *Die) -> std::optional { + if (const DIE *Parent = Die->getParent()) + return Die->getParent()->getOffset(); + return std::nullopt; + }; for (const auto &Namespace : Unit.getNamespaces()) DebugNames.addName(Namespace.Name, Namespace.Die->getOffset(), + ParentOffsetOrNull(Namespace.Die), Namespace.Die->getTag(), Unit.getUniqueID()); for (const auto &Pubname : Unit.getPubnames()) DebugNames.addName(Pubname.Name, Pubname.Die->getOffset(), + ParentOffsetOrNull(Pubname.Die), Pubname.Die->getTag(), Unit.getUniqueID()); for (const auto &Pubtype : Unit.getPubtypes()) DebugNames.addName(Pubtype.Name, Pubtype.Die->getOffset(), + ParentOffsetOrNull(Pubtype.Die), Pubtype.Die->getTag(), Unit.getUniqueID()); } break; } diff --git a/llvm/lib/DWARFLinker/Parallel/DWARFLinkerImpl.cpp b/llvm/lib/DWARFLinker/Parallel/DWARFLinkerImpl.cpp index bb59cbfdb3477..790229e213b2b 100644 --- a/llvm/lib/DWARFLinker/Parallel/DWARFLinkerImpl.cpp +++ b/llvm/lib/DWARFLinker/Parallel/DWARFLinkerImpl.cpp @@ -1374,7 +1374,8 @@ void DWARFLinkerImpl::emitDWARFv5DebugNamesSection(const Triple &TargetTriple) { case DwarfUnit::AccelType::Namespace: case DwarfUnit::AccelType::Type: { DebugNames->addName(*DebugStrStrings.getExistingEntry(Info.String), - Info.OutOffset, Info.Tag, CU->getUniqueID()); + Info.OutOffset, std::nullopt /*ParentDIEOffset*/, + Info.Tag, CU->getUniqueID()); } break; default: diff --git a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp index d25b732fdba3f..c4c14f5e2c9d3 100644 --- a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp +++ b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "llvm/DebugInfo/DWARF/DWARFVerifier.h" #include "llvm/ADT/IntervalMap.h" +#include "llvm/ADT/STLExtras.h" #include "llvm/ADT/SmallSet.h" #include "llvm/BinaryFormat/Dwarf.h" #include "llvm/DebugInfo/DWARF/DWARFAbbreviationDeclaration.h" @@ -1272,6 +1273,20 @@ unsigned DWARFVerifier::verifyNameIndexAttribute( NI.getUnitOffset(), Abbr.Code, AttrEnc.Form, dwarf::DW_FORM_data8); return 1; } + return 0; + } + + if (AttrEnc.Index == dwarf::DW_IDX_parent) { + constexpr static auto AllowedForms = {dwarf::Form::DW_FORM_flag_present, + dwarf::Form::DW_FORM_ref4}; + if (!is_contained(AllowedForms, AttrEnc.Form)) { + error() << formatv("NameIndex @ {0:x}: Abbreviation {1:x}: DW_IDX_parent " + "uses an unexpected form {2} (should be " + "DW_FORM_ref4 or DW_FORM_flag_present).\n", + NI.getUnitOffset(), Abbr.Code, AttrEnc.Form); + return 1; + } + return 0; } // A list of known index attributes and their expected form classes. @@ -1286,7 +1301,6 @@ unsigned DWARFVerifier::verifyNameIndexAttribute( {dwarf::DW_IDX_compile_unit, DWARFFormValue::FC_Constant, {"constant"}}, {dwarf::DW_IDX_type_unit, DWARFFormValue::FC_Constant, {"constant"}}, {dwarf::DW_IDX_die_offset, DWARFFormValue::FC_Reference, {"reference"}}, - {dwarf::DW_IDX_parent, DWARFFormValue::FC_Constant, {"constant"}}, }; ArrayRef TableRef(Table); diff --git a/llvm/test/DebugInfo/X86/debug-names-dwarf64.ll b/llvm/test/DebugInfo/X86/debug-names-dwarf64.ll index b94bdbcb12010..a6855d77a34ac 100644 --- a/llvm/test/DebugInfo/X86/debug-names-dwarf64.ll +++ b/llvm/test/DebugInfo/X86/debug-names-dwarf64.ll @@ -30,21 +30,25 @@ ; CHECK-NEXT: CU[0]: 0x00000000 ; CHECK-NEXT: ] ; CHECK-NEXT: Abbreviations [ +; CHECK-NEXT: Abbreviation [[ABBREV_LABEL:0x[0-9a-f]*]] { +; CHECK-NEXT: Tag: DW_TAG_label +; CHECK-NEXT: DW_IDX_die_offset: DW_FORM_ref4 +; CHECK-NEXT: DW_IDX_parent: DW_FORM_ref4 +; CHECK-NEXT: } ; CHECK-NEXT: Abbreviation [[ABBREV:0x[0-9a-f]*]] { ; CHECK-NEXT: Tag: DW_TAG_base_type ; CHECK-NEXT: DW_IDX_die_offset: DW_FORM_ref4 +; CHECK-NEXT: DW_IDX_parent: DW_FORM_flag_present ; CHECK-NEXT: } ; CHECK-NEXT: Abbreviation [[ABBREV1:0x[0-9a-f]*]] { ; CHECK-NEXT: Tag: DW_TAG_variable ; CHECK-NEXT: DW_IDX_die_offset: DW_FORM_ref4 +; CHECK-NEXT: DW_IDX_parent: DW_FORM_flag_present ; CHECK-NEXT: } ; CHECK-NEXT: Abbreviation [[ABBREV_SP:0x[0-9a-f]*]] { ; CHECK-NEXT: Tag: DW_TAG_subprogram ; CHECK-NEXT: DW_IDX_die_offset: DW_FORM_ref4 -; CHECK-NEXT: } -; CHECK-NEXT: Abbreviation [[ABBREV_LABEL:0x[0-9a-f]*]] { -; CHECK-NEXT: Tag: DW_TAG_label -; CHECK-NEXT: DW_IDX_die_offset: DW_FORM_ref4 +; CHECK-NEXT: DW_IDX_parent: DW_FORM_flag_present ; CHECK-NEXT: } ; CHECK-NEXT: ] ; CHECK-NEXT: Bucket 0 [ @@ -55,6 +59,7 @@ ; CHECK-NEXT: Abbrev: [[ABBREV]] ; CHECK-NEXT: Tag: DW_TAG_base_type ; CHECK-NEXT: DW_IDX_die_offset: [[TYPEDIE]] +; CHECK-NEXT: DW_IDX_parent: true ; CHECK-NEXT: } ; CHECK-NEXT: } ; CHECK-NEXT: ] @@ -66,6 +71,7 @@ ; CHECK-NEXT: Abbrev: [[ABBREV1]] ; CHECK-NEXT: Tag: DW_TAG_variable ; CHECK-NEXT: DW_IDX_die_offset: [[VARDIE]] +; CHECK-NEXT: DW_IDX_parent: true ; CHECK-NEXT: } ; CHECK-NEXT: } ; CHECK-NEXT: Name 3 { @@ -75,6 +81,7 @@ ; CHECK-NEXT: Abbrev: [[ABBREV_SP]] ; CHECK-NEXT: Tag: DW_TAG_subprogram ; CHECK-NEXT: DW_IDX_die_offset: [[SPDIE]] +; CHECK-NEXT: DW_IDX_parent: true ; CHECK-NEXT: } ; CHECK-NEXT: } ; CHECK-NEXT: ] @@ -89,6 +96,7 @@ ; CHECK-NEXT: Abbrev: [[ABBREV_LABEL]] ; CHECK-NEXT: Tag: DW_TAG_label ; CHECK-NEXT: DW_IDX_die_offset: [[LABELDIE]] +; CHECK-NEXT: DW_IDX_parent: 0x{{.*}} ; CHECK-NEXT: } ; CHECK-NEXT: } ; CHECK-NEXT: ] diff --git a/llvm/test/DebugInfo/X86/debug-names-end-of-list.ll b/llvm/test/DebugInfo/X86/debug-names-end-of-list.ll index f57168a8eff38..b8d4046201c34 100644 --- a/llvm/test/DebugInfo/X86/debug-names-end-of-list.ll +++ b/llvm/test/DebugInfo/X86/debug-names-end-of-list.ll @@ -6,8 +6,10 @@ ; CHECK: .section .debug_names,"",@progbits ; CHECK: .Lnames_entries0: -; CHECK: .byte 0 # End of list: int -; CHECK: .byte 0 # End of list: foo +; CHECK: .byte 0 +; CHECK-NEXT: # End of list: int +; CHECK: .byte 0 +; CHECK-NEXT: # End of list: foo @foo = common dso_local global i32 0, align 4, !dbg !5 diff --git a/llvm/test/DebugInfo/X86/debug-names-types.ll b/llvm/test/DebugInfo/X86/debug-names-types.ll index 501b7efd88eb9..c44e82578f14a 100644 --- a/llvm/test/DebugInfo/X86/debug-names-types.ll +++ b/llvm/test/DebugInfo/X86/debug-names-types.ll @@ -37,27 +37,32 @@ ; CHECK-NEXT: LocalTU[0]: 0x00000000 ; CHECK-NEXT: ] ; CHECK: Abbreviations [ -; CHECK-NEXT: Abbreviation [[ABBREV1:0x[0-9a-f]*]] { +; CHECK-NEXT: Abbreviation [[ABBREV3:0x[0-9a-f]*]] { ; CHECK-NEXT: Tag: DW_TAG_structure_type +; CHECK-NEXT: DW_IDX_type_unit: DW_FORM_data1 ; CHECK-NEXT: DW_IDX_die_offset: DW_FORM_ref4 +; CHECK-NEXT: DW_IDX_parent: DW_FORM_flag_present ; CHECK-NEXT: } -; CHECK-NEXT: Abbreviation [[ABBREV3:0x[0-9a-f]*]] { -; CHECK-NEXT: Tag: DW_TAG_structure_type +; CHECK-NEXT: Abbreviation [[ABBREV4:0x[0-9a-f]*]] { +; CHECK-NEXT: Tag: DW_TAG_base_type ; CHECK-NEXT: DW_IDX_type_unit: DW_FORM_data1 ; CHECK-NEXT: DW_IDX_die_offset: DW_FORM_ref4 +; CHECK-NEXT: DW_IDX_parent: DW_FORM_flag_present ; CHECK-NEXT: } ; CHECK-NEXT: Abbreviation [[ABBREV:0x[0-9a-f]*]] { ; CHECK-NEXT: Tag: DW_TAG_base_type ; CHECK-NEXT: DW_IDX_die_offset: DW_FORM_ref4 +; CHECK-NEXT: DW_IDX_parent: DW_FORM_flag_present ; CHECK-NEXT: } -; CHECK-NEXT: Abbreviation [[ABBREV2:0x[0-9a-f]*]] { -; CHECK-NEXT: Tag: DW_TAG_subprogram +; CHECK-NEXT: Abbreviation [[ABBREV1:0x[0-9a-f]*]] { +; CHECK-NEXT: Tag: DW_TAG_structure_type ; CHECK-NEXT: DW_IDX_die_offset: DW_FORM_ref4 +; CHECK-NEXT: DW_IDX_parent: DW_FORM_flag_present ; CHECK-NEXT: } -; CHECK-NEXT: Abbreviation [[ABBREV4:0x[0-9a-f]*]] { -; CHECK-NEXT: Tag: DW_TAG_base_type -; CHECK-NEXT: DW_IDX_type_unit: DW_FORM_data1 +; CHECK-NEXT: Abbreviation [[ABBREV2:0x[0-9a-f]*]] { +; CHECK-NEXT: Tag: DW_TAG_subprogram ; CHECK-NEXT: DW_IDX_die_offset: DW_FORM_ref4 +; CHECK-NEXT: DW_IDX_parent: DW_FORM_flag_present ; CHECK-NEXT: } ; CHECK-NEXT: ] ; CHECK-NEXT: Bucket 0 [ @@ -68,6 +73,7 @@ ; CHECK-NEXT: Abbrev: [[ABBREV]] ; CHECK-NEXT: Tag: DW_TAG_base_type ; CHECK-NEXT: DW_IDX_die_offset: 0x0000003e +; CHECK-NEXT: DW_IDX_parent: true ; CHECK-NEXT: } ; CHECK-NEXT: } ; CHECK-NEXT: ] @@ -80,11 +86,13 @@ ; CHECK-NEXT: Tag: DW_TAG_structure_type ; CHECK-NEXT: DW_IDX_type_unit: 0x00 ; CHECK-NEXT: DW_IDX_die_offset: 0x00000023 +; CHECK-NEXT: DW_IDX_parent: true ; CHECK-NEXT: } -; CHECK-NEXT: Entry @ 0xaa { +; CHECK-NEXT: Entry @ {{.+}} { ; CHECK-NEXT: Abbrev: [[ABBREV1]] ; CHECK-NEXT: Tag: DW_TAG_structure_type ; CHECK-NEXT: DW_IDX_die_offset: 0x00000042 +; CHECK-NEXT: DW_IDX_parent: true ; CHECK-NEXT: } ; CHECK-NEXT: } ; CHECK-NEXT: ] @@ -96,6 +104,7 @@ ; CHECK-NEXT: Abbrev: [[ABBREV2]] ; CHECK-NEXT: Tag: DW_TAG_subprogram ; CHECK-NEXT: DW_IDX_die_offset: 0x00000023 +; CHECK-NEXT: DW_IDX_parent: true ; CHECK-NEXT: } ; CHECK-NEXT: } ; CHECK-NEXT: ] @@ -108,6 +117,7 @@ ; CHECK-NEXT: Tag: DW_TAG_base_type ; CHECK-NEXT: DW_IDX_type_unit: 0x00 ; CHECK-NEXT: DW_IDX_die_offset: 0x00000038 +; CHECK-NEXT: DW_IDX_parent: true ; CHECK-NEXT: } ; CHECK-NEXT: } ; CHECK-NEXT: ] @@ -120,7 +130,7 @@ ; CHECK-SPLIT: Foreign TU count: 1 ; CHECK-SPLIT-NEXT: Bucket count: 4 ; CHECK-SPLIT-NEXT: Name count: 4 -; CHECK-SPLIT-NEXT: Abbreviations table size: 0x28 +; CHECK-SPLIT-NEXT: Abbreviations table size: 0x32 ; CHECK-SPLIT-NEXT: Augmentation: 'LLVM0700' ; CHECK-SPLIT-NEXT: } ; CHECK-SPLIT-NEXT: Compilation Unit offsets [ @@ -130,27 +140,32 @@ ; CHECK-SPLIT-NEXT: ForeignTU[0]: 0x675d23e4f33235f2 ; CHECK-SPLIT-NEXT: ] ; CHECK-SPLIT-NEXT: Abbreviations [ -; CHECK-SPLIT-NEXT: Abbreviation [[ABBREV:0x[0-9a-f]*]] { +; CHECK-SPLIT-NEXT: Abbreviation [[ABBREV1:0x[0-9a-f]*]] { ; CHECK-SPLIT-NEXT: Tag: DW_TAG_structure_type +; CHECK-SPLIT-NEXT: DW_IDX_type_unit: DW_FORM_data1 ; CHECK-SPLIT-NEXT: DW_IDX_die_offset: DW_FORM_ref4 +; CHECK-SPLIT-NEXT: DW_IDX_parent: DW_FORM_flag_present ; CHECK-SPLIT-NEXT: } -; CHECK-SPLIT-NEXT: Abbreviation [[ABBREV1:0x[0-9a-f]*]] { -; CHECK-SPLIT-NEXT: Tag: DW_TAG_structure_type +; CHECK-SPLIT-NEXT: Abbreviation [[ABBREV4:0x[0-9a-f]*]] { +; CHECK-SPLIT-NEXT: Tag: DW_TAG_base_type ; CHECK-SPLIT-NEXT: DW_IDX_type_unit: DW_FORM_data1 ; CHECK-SPLIT-NEXT: DW_IDX_die_offset: DW_FORM_ref4 +; CHECK-SPLIT-NEXT: DW_IDX_parent: DW_FORM_flag_present ; CHECK-SPLIT-NEXT: } ; CHECK-SPLIT-NEXT: Abbreviation [[ABBREV2:0x[0-9a-f]*]] { ; CHECK-SPLIT-NEXT: Tag: DW_TAG_base_type ; CHECK-SPLIT-NEXT: DW_IDX_die_offset: DW_FORM_ref4 +; CHECK-SPLIT-NEXT: DW_IDX_parent: DW_FORM_flag_present ; CHECK-SPLIT-NEXT: } -; CHECK-SPLIT-NEXT: Abbreviation [[ABBREV3:0x[0-9a-f]*]] { -; CHECK-SPLIT-NEXT: Tag: DW_TAG_subprogram +; CHECK-SPLIT-NEXT: Abbreviation [[ABBREV:0x[0-9a-f]*]] { +; CHECK-SPLIT-NEXT: Tag: DW_TAG_structure_type ; CHECK-SPLIT-NEXT: DW_IDX_die_offset: DW_FORM_ref4 +; CHECK-SPLIT-NEXT: DW_IDX_parent: DW_FORM_flag_present ; CHECK-SPLIT-NEXT: } -; CHECK-SPLIT-NEXT: Abbreviation [[ABBREV4:0x[0-9a-f]*]] { -; CHECK-SPLIT-NEXT: Tag: DW_TAG_base_type -; CHECK-SPLIT-NEXT: DW_IDX_type_unit: DW_FORM_data1 +; CHECK-SPLIT-NEXT: Abbreviation [[ABBREV3:0x[0-9a-f]*]] { +; CHECK-SPLIT-NEXT: Tag: DW_TAG_subprogram ; CHECK-SPLIT-NEXT: DW_IDX_die_offset: DW_FORM_ref4 +; CHECK-SPLIT-NEXT: DW_IDX_parent: DW_FORM_flag_present ; CHECK-SPLIT-NEXT: } ; CHECK-SPLIT-NEXT: ] ; CHECK-SPLIT-NEXT: Bucket 0 [ @@ -161,6 +176,7 @@ ; CHECK-SPLIT-NEXT: Abbrev: [[ABBREV2]] ; CHECK-SPLIT-NEXT: Tag: DW_TAG_base_type ; CHECK-SPLIT-NEXT: DW_IDX_die_offset: 0x00000035 +; CHECK-SPLIT-NEXT: DW_IDX_parent: true ; CHECK-SPLIT-NEXT: } ; CHECK-SPLIT-NEXT: } ; CHECK-SPLIT-NEXT: ] @@ -173,11 +189,13 @@ ; CHECK-SPLIT-NEXT: Tag: DW_TAG_structure_type ; CHECK-SPLIT-NEXT: DW_IDX_type_unit: 0x00 ; CHECK-SPLIT-NEXT: DW_IDX_die_offset: 0x00000021 +; CHECK-SPLIT-NEXT: DW_IDX_parent: true ; CHECK-SPLIT-NEXT: } -; CHECK-SPLIT-NEXT: Entry @ 0xae { +; CHECK-SPLIT-NEXT: Entry @ {{.*}} { ; CHECK-SPLIT-NEXT: Abbrev: [[ABBREV]] ; CHECK-SPLIT-NEXT: Tag: DW_TAG_structure_type ; CHECK-SPLIT-NEXT: DW_IDX_die_offset: 0x00000039 +; CHECK-SPLIT-NEXT: DW_IDX_parent: true ; CHECK-SPLIT-NEXT: } ; CHECK-SPLIT-NEXT: } ; CHECK-SPLIT-NEXT: ] @@ -189,6 +207,7 @@ ; CHECK-SPLIT-NEXT: Abbrev: [[ABBREV3]] ; CHECK-SPLIT-NEXT: Tag: DW_TAG_subprogram ; CHECK-SPLIT-NEXT: DW_IDX_die_offset: 0x0000001a +; CHECK-SPLIT-NEXT: DW_IDX_parent: true ; CHECK-SPLIT-NEXT: } ; CHECK-SPLIT-NEXT: } ; CHECK-SPLIT-NEXT: ] @@ -201,6 +220,7 @@ ; CHECK-SPLIT-NEXT: Tag: DW_TAG_base_type ; CHECK-SPLIT-NEXT: DW_IDX_type_unit: 0x00 ; CHECK-SPLIT-NEXT: DW_IDX_die_offset: 0x00000036 +; CHECK-SPLIT-NEXT: DW_IDX_parent: true ; CHECK-SPLIT-NEXT: } ; CHECK-SPLIT-NEXT: } ; CHECK-SPLIT-NEXT: ] diff --git a/llvm/test/tools/dsymutil/ARM/accel-imported-declarations.test b/llvm/test/tools/dsymutil/ARM/accel-imported-declarations.test index 057e89d060b1d..a8bf191e036f7 100644 --- a/llvm/test/tools/dsymutil/ARM/accel-imported-declarations.test +++ b/llvm/test/tools/dsymutil/ARM/accel-imported-declarations.test @@ -23,11 +23,13 @@ DWARF-NEXT: Entry {{.*}} { DWARF-NEXT: Abbrev: {{.*}} DWARF-NEXT: Tag: DW_TAG_namespace DWARF: DW_IDX_die_offset: [[NAMESPACE]] +DWARF-NEXT: DW_IDX_parent: 0x{{.*}} DWARF-NEXT: } DWARF-NEXT: Entry {{.*}} { DWARF-NEXT: Abbrev: {{.*}} DWARF: Tag: DW_TAG_imported_declaration DWARF: DW_IDX_die_offset: 0x0000005c +DWARF-NEXT: DW_IDX_parent: 0x{{.*}} DWARF-NEXT: } DWARF-NEXT: } diff --git a/llvm/test/tools/dsymutil/ARM/dwarf5-dwarf4-combination-macho.test b/llvm/test/tools/dsymutil/ARM/dwarf5-dwarf4-combination-macho.test index d5b78bd1487e1..1570ea922e955 100644 --- a/llvm/test/tools/dsymutil/ARM/dwarf5-dwarf4-combination-macho.test +++ b/llvm/test/tools/dsymutil/ARM/dwarf5-dwarf4-combination-macho.test @@ -31,14 +31,14 @@ RUN: rm -rf %t.dir && mkdir -p %t.dir RUN: dsymutil -y %p/dummy-debug-map-amr64.map -oso-prepend-path=%p/../Inputs/DWARF5-DWARF4-combination -o %t.dir/dwarf5-dwarf4-combination-macho.dSYM -RUN: llvm-dwarfdump %t.dir/dwarf5-dwarf4-combination-macho.dSYM -a --verbose | FileCheck %s +RUN: llvm-dwarfdump %t.dir/dwarf5-dwarf4-combination-macho.dSYM -a --verbose | FileCheck %s --check-prefixes=CHECK,WITH-PARENTS RUN: rm -rf %t.dir && mkdir -p %t.dir RUN: dsymutil --no-odr --linker llvm -y %p/dummy-debug-map-amr64.map \ RUN: -oso-prepend-path=%p/../Inputs/DWARF5-DWARF4-combination \ RUN: -o %t.dir/dwarf5-dwarf4-combination-macho.dSYM RUN: llvm-dwarfdump %t.dir/dwarf5-dwarf4-combination-macho.dSYM \ -RUN: -a --verbose | FileCheck %s +RUN: -a --verbose | FileCheck %s --check-prefixes=CHECK,NO-PARENTS ### Uncomment following when llvm-dwarfdump will dump address ranges ### correctly for severall compile units case. @@ -219,7 +219,10 @@ CHECK-NEXT: 0x00000028: 000000dc "int" CHECK: .debug_names contents: CHECK-NEXT: Name Index @ 0x0 { CHECK-NEXT: Header { -CHECK-NEXT: Length: 0xC4 +; FIXME: when the parallel dwarf linker is able to generate DW_IDX_parent, +; these headers should be the same. +WITH-PARENTS-NEXT: Length: 0xC8 +NO-PARENTS-NEXT: Length: 0xC4 CHECK-NEXT: Format: DWARF32 CHECK-NEXT: Version: 5 CHECK-NEXT: CU count: 2 @@ -227,6 +230,7 @@ CHECK-NEXT: Local TU count: 0 CHECK-NEXT: Foreign TU count: 0 CHECK-NEXT: Bucket count: 5 CHECK-NEXT: Name count: 5 -CHECK-NEXT: Abbreviations table size: 0x13 +WITH-PARENTS-NEXT: Abbreviations table size: 0x17 +NO-PARENTS-NEXT: Abbreviations table size: 0x13 CHECK-NEXT: Augmentation: 'LLVM0700' CHECK-NEXT: } From e30a9134312e969e7b2e20ae2522dc9929ac07ff Mon Sep 17 00:00:00 2001 From: Felipe de Azevedo Piovezan Date: Wed, 10 Jan 2024 10:42:19 -0300 Subject: [PATCH 2/6] fixup! NFC review comments --- llvm/lib/CodeGen/AsmPrinter/AccelTable.cpp | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/llvm/lib/CodeGen/AsmPrinter/AccelTable.cpp b/llvm/lib/CodeGen/AsmPrinter/AccelTable.cpp index c1f214e06a4b6..04d75598f8330 100644 --- a/llvm/lib/CodeGen/AsmPrinter/AccelTable.cpp +++ b/llvm/lib/CodeGen/AsmPrinter/AccelTable.cpp @@ -221,7 +221,7 @@ class Dwarf5AccelTableWriter : public AccelTableWriter { MCSymbol *EntryPool = Asm->createTempSymbol("names_entries"); // Indicates if this module is built with Split Dwarf enabled. bool IsSplitDwarf = false; - // Stores the DIE offsets which are indexed by this table. + /// Stores the DIE offsets which are indexed by this table. DenseSet IndexedOffsets; void populateAbbrevsMap(); @@ -402,14 +402,14 @@ void Dwarf5AccelTableWriter::Header::emit(Dwarf5AccelTableWriter &Ctx) { } enum IdxParentEncoding : uint8_t { - NoIndexedParent = 0, // Parent information present but parent isn't indexed. - Ref4 = 1, // Parent information present and parent is indexed. - NoParent = 2, // Parent information missing. + NoIndexedParent = 0, /// Parent information present but parent isn't indexed. + Ref4 = 1, /// Parent information present and parent is indexed. + NoParent = 2, /// Parent information missing. }; static uint32_t constexpr NumBitsIdxParent = 2; -uint8_t encodeIdxParent(std::optional MaybeParentForm) { +uint8_t encodeIdxParent(const std::optional MaybeParentForm) { if (!MaybeParentForm) return NoParent; switch (*MaybeParentForm) { @@ -418,11 +418,10 @@ uint8_t encodeIdxParent(std::optional MaybeParentForm) { case dwarf::Form::DW_FORM_ref4: return Ref4; default: - break; + // This is not crashing on bad input: we should only reach this if the + // internal compiler logic is faulty; see getFormForIdxParent. + llvm_unreachable("Bad form for IDX_parent"); } - // This is not crashing on bad input: we should only reach this if the - // internal compiler logic is faulty; see getFormForIdxParent. - llvm_unreachable("Bad form for IDX_parent"); } static uint32_t constexpr ParentBitOffset = dwarf::DW_IDX_type_hash; @@ -608,7 +607,7 @@ void Dwarf5AccelTableWriter::emitData() { DenseMap DIEOffsetToAccelEntryLabel; for (auto Offset : IndexedOffsets) - DIEOffsetToAccelEntryLabel[Offset] = Asm->createTempSymbol("symbol"); + DIEOffsetToAccelEntryLabel.insert({Offset, Asm->createTempSymbol("")}); Asm->OutStreamer->emitLabel(EntryPool); DenseSet EmittedAccelEntrySymbols; From aa17884a27a7f14666c44e974a755203a02800e6 Mon Sep 17 00:00:00 2001 From: Felipe de Azevedo Piovezan Date: Wed, 10 Jan 2024 13:24:46 -0300 Subject: [PATCH 3/6] fixup! Don't use IDX_Parent if the parent is a declaration --- llvm/include/llvm/CodeGen/AccelTable.h | 13 ++++--- llvm/lib/CodeGen/AsmPrinter/AccelTable.cpp | 10 +++++ llvm/lib/DWARFLinker/Classic/DWARFLinker.cpp | 26 ++++++------- .../DebugInfo/Generic/debug-names-one-cu.ll | 37 ++++++++++++++++++- 4 files changed, 65 insertions(+), 21 deletions(-) diff --git a/llvm/include/llvm/CodeGen/AccelTable.h b/llvm/include/llvm/CodeGen/AccelTable.h index 12ed123ee90a0..132cc83865655 100644 --- a/llvm/include/llvm/CodeGen/AccelTable.h +++ b/llvm/include/llvm/CodeGen/AccelTable.h @@ -271,11 +271,11 @@ class DWARF5AccelTableData : public AccelTableData { DWARF5AccelTableData(const DIE &Die, const uint32_t UnitID, const bool IsTU = false); DWARF5AccelTableData(const uint64_t DieOffset, - const std::optional ParentOffset, + const std::optional DefiningParentOffset, const unsigned DieTag, const unsigned UnitID, const bool IsTU = false) - : OffsetVal(DieOffset), ParentOffset(ParentOffset), DieTag(DieTag), - UnitID(UnitID), IsTU(IsTU) {} + : OffsetVal(DieOffset), ParentOffset(DefiningParentOffset), + DieTag(DieTag), UnitID(UnitID), IsTU(IsTU) {} #ifndef NDEBUG void print(raw_ostream &OS) const override; @@ -291,8 +291,7 @@ class DWARF5AccelTableData : public AccelTableData { void normalizeDIEToOffset() { assert(!isNormalized() && "Accessing offset after normalizing."); const DIE *Entry = std::get(OffsetVal); - ParentOffset = Entry->getParent() ? Entry->getParent()->getOffset() - : std::optional(); + ParentOffset = getDefiningParentDieOffset(*Entry); OffsetVal = Entry->getOffset(); } bool isNormalized() const { @@ -304,6 +303,10 @@ class DWARF5AccelTableData : public AccelTableData { return ParentOffset; } + /// If `Die` has a non-null parent and the parent is not a declaration, + /// return its offset. + static std::optional getDefiningParentDieOffset(const DIE &Die); + protected: std::variant OffsetVal; std::optional ParentOffset; diff --git a/llvm/lib/CodeGen/AsmPrinter/AccelTable.cpp b/llvm/lib/CodeGen/AsmPrinter/AccelTable.cpp index 04d75598f8330..f0a85818eb466 100644 --- a/llvm/lib/CodeGen/AsmPrinter/AccelTable.cpp +++ b/llvm/lib/CodeGen/AsmPrinter/AccelTable.cpp @@ -401,6 +401,16 @@ void Dwarf5AccelTableWriter::Header::emit(Dwarf5AccelTableWriter &Ctx) { Asm->OutStreamer->emitBytes({AugmentationString, AugmentationStringSize}); } +std::optional +DWARF5AccelTableData::getDefiningParentDieOffset(const DIE &Die) { + auto *Parent = Die.getParent(); + if (!Parent) + return {}; + if (Parent->findAttribute(dwarf::Attribute::DW_AT_declaration)) + return {}; + return Parent->getOffset(); +} + enum IdxParentEncoding : uint8_t { NoIndexedParent = 0, /// Parent information present but parent isn't indexed. Ref4 = 1, /// Parent information present and parent is indexed. diff --git a/llvm/lib/DWARFLinker/Classic/DWARFLinker.cpp b/llvm/lib/DWARFLinker/Classic/DWARFLinker.cpp index 66d9b0ae9841f..62e0b184fd7f9 100644 --- a/llvm/lib/DWARFLinker/Classic/DWARFLinker.cpp +++ b/llvm/lib/DWARFLinker/Classic/DWARFLinker.cpp @@ -2252,23 +2252,21 @@ void DWARFLinker::emitAcceleratorEntriesForUnit(CompileUnit &Unit) { TheDwarfEmitter->emitPubTypesForUnit(Unit); } break; case AccelTableKind::DebugNames: { - auto ParentOffsetOrNull = [](const DIE *Die) -> std::optional { - if (const DIE *Parent = Die->getParent()) - return Die->getParent()->getOffset(); - return std::nullopt; - }; for (const auto &Namespace : Unit.getNamespaces()) - DebugNames.addName(Namespace.Name, Namespace.Die->getOffset(), - ParentOffsetOrNull(Namespace.Die), - Namespace.Die->getTag(), Unit.getUniqueID()); + DebugNames.addName( + Namespace.Name, Namespace.Die->getOffset(), + DWARF5AccelTableData::getDefiningParentDieOffset(*Namespace.Die), + Namespace.Die->getTag(), Unit.getUniqueID()); for (const auto &Pubname : Unit.getPubnames()) - DebugNames.addName(Pubname.Name, Pubname.Die->getOffset(), - ParentOffsetOrNull(Pubname.Die), - Pubname.Die->getTag(), Unit.getUniqueID()); + DebugNames.addName( + Pubname.Name, Pubname.Die->getOffset(), + DWARF5AccelTableData::getDefiningParentDieOffset(*Pubname.Die), + Pubname.Die->getTag(), Unit.getUniqueID()); for (const auto &Pubtype : Unit.getPubtypes()) - DebugNames.addName(Pubtype.Name, Pubtype.Die->getOffset(), - ParentOffsetOrNull(Pubtype.Die), - Pubtype.Die->getTag(), Unit.getUniqueID()); + DebugNames.addName( + Pubtype.Name, Pubtype.Die->getOffset(), + DWARF5AccelTableData::getDefiningParentDieOffset(*Pubtype.Die), + Pubtype.Die->getTag(), Unit.getUniqueID()); } break; } } diff --git a/llvm/test/DebugInfo/Generic/debug-names-one-cu.ll b/llvm/test/DebugInfo/Generic/debug-names-one-cu.ll index 469be45c312e8..92212a24bd153 100644 --- a/llvm/test/DebugInfo/Generic/debug-names-one-cu.ll +++ b/llvm/test/DebugInfo/Generic/debug-names-one-cu.ll @@ -7,22 +7,49 @@ ; CHECK: CU count: 1 ; CHECK: Local TU count: 0 ; CHECK: Foreign TU count: 0 -; CHECK: Name count: 1 +; CHECK: Name count: 3 ; CHECK: CU[0]: 0x{{[0-9a-f]*}} +; CHECK: Abbreviation [[ABBREV_STRUCT:0x[0-9a-f]*]] { +; CHECK-NEXT: Tag: DW_TAG_structure_type +; CHECK-NEXT: DW_IDX_die_offset: DW_FORM_ref4 +; CHECK-NEXT: } + ; CHECK: Abbreviation [[ABBREV:0x[0-9a-f]*]] ; CHECK-NEXT: Tag: DW_TAG_variable ; CHECK-NEXT: DW_IDX_die_offset: DW_FORM_ref4 +; CHECK-NEXT: DW_IDX_parent: DW_FORM_flag_present +; CHECK-NEXT: } + +; The entry for A::B must not have an IDX_Parent, since A is only a forward +; declaration. +; CHECK: String: 0x{{[0-9a-f]*}} "B" +; CHECK-NEXT: Entry +; CHECK-NEXT: Abbrev: [[ABBREV_STRUCT]] +; CHECK-NEXT: Tag: DW_TAG_structure_type +; CHECK-NEXT: DW_IDX_die_offset: 0x{{[0-9a-f]*}} +; CHECK-NEXT: } + +; CHECK: String: 0x{{[0-9a-f]*}} "someA_B" +; CHECK-NEXT: Entry +; CHECK-NEXT: Abbrev: [[ABBREV]] +; CHECK-NEXT: Tag: DW_TAG_variable +; CHECK-NEXT: DW_IDX_die_offset: 0x{{[0-9a-f]*}} +; CHECK-NEXT: DW_IDX_parent: true +; CHECK-NEXT: } ; CHECK: String: 0x{{[0-9a-f]*}} "foobar" ; CHECK-NEXT: Entry ; CHECK-NEXT: Abbrev: [[ABBREV]] ; CHECK-NEXT: Tag: DW_TAG_variable ; CHECK-NEXT: DW_IDX_die_offset: 0x{{[0-9a-f]*}} +; CHECK-NEXT: DW_IDX_parent: true +; CHECK-NEXT: } ; VERIFY: No errors. @foobar = common dso_local global ptr null, align 8, !dbg !0 +@someA_B = common dso_local global ptr null, align 8, !dbg !18 !llvm.dbg.cu = !{!2} !llvm.module.flags = !{!7, !8, !9} @@ -33,9 +60,15 @@ !2 = distinct !DICompileUnit(language: DW_LANG_C99, file: !3, producer: "clang version 7.0.0 (trunk 325496) (llvm/trunk 325732)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !4, globals: !5) !3 = !DIFile(filename: "/tmp/cu1.c", directory: "/tmp") !4 = !{} -!5 = !{!0} +!5 = !{!0, !18} !6 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: null, size: 64) !7 = !{i32 2, !"Dwarf Version", i32 4} !8 = !{i32 2, !"Debug Info Version", i32 3} !9 = !{i32 1, !"wchar_size", i32 4} !10 = !{!"clang version 7.0.0 (trunk 325496) (llvm/trunk 325732)"} + +!13 = !{} +!15 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "B", scope: !16, file: !3, line: 3, size: 8, elements: !13, identifier: "type_A::B") +!16 = !DICompositeType(tag: DW_TAG_structure_type, name: "A", file: !3, line: 1, size: 8, flags: DIFlagFwdDecl, identifier: "type_A") +!17 = distinct !DIGlobalVariable(name: "someA_B", scope: !2, file: !3, line: 1, type: !15, isLocal: false, isDefinition: true) +!18 = !DIGlobalVariableExpression(var: !17, expr: !DIExpression()) From 2a5a5107090b03fcb9feb6d728e247ef8c2312da Mon Sep 17 00:00:00 2001 From: Felipe de Azevedo Piovezan Date: Wed, 10 Jan 2024 15:51:20 -0300 Subject: [PATCH 4/6] fixup! Consider the UnitID when identifying entries --- llvm/include/llvm/CodeGen/AccelTable.h | 30 +++++++++++++++++++- llvm/lib/CodeGen/AsmPrinter/AccelTable.cpp | 32 ++++++++++++---------- 2 files changed, 46 insertions(+), 16 deletions(-) diff --git a/llvm/include/llvm/CodeGen/AccelTable.h b/llvm/include/llvm/CodeGen/AccelTable.h index 132cc83865655..5747897a11416 100644 --- a/llvm/include/llvm/CodeGen/AccelTable.h +++ b/llvm/include/llvm/CodeGen/AccelTable.h @@ -255,6 +255,20 @@ class AppleAccelTableData : public AccelTableData { static uint32_t hash(StringRef Buffer) { return djbHash(Buffer); } }; +/// Helper class to identify an entry in DWARF5AccelTable based on their DIE +/// offset and UnitID. +struct OffsetAndUnitID : std::pair { + using Base = std::pair; + 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; }; +}; + +template <> +struct DenseMapInfo : DenseMapInfo {}; + /// 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 @@ -285,6 +299,11 @@ class DWARF5AccelTableData : public AccelTableData { assert(isNormalized() && "Accessing DIE Offset before normalizing."); return std::get(OffsetVal); } + + OffsetAndUnitID getDieOffsetAndUnitID() const { + return {getDieOffset(), UnitID}; + } + unsigned getDieTag() const { return DieTag; } unsigned getUnitID() const { return UnitID; } bool isTU() const { return IsTU; } @@ -299,8 +318,17 @@ class DWARF5AccelTableData : public AccelTableData { } std::optional getParentDieOffset() const { + auto OffsetAndId = getParentDieOffsetAndUnitID(); + if (!OffsetAndId) + return {}; + return OffsetAndId->offset(); + } + + std::optional getParentDieOffsetAndUnitID() const { assert(isNormalized() && "Accessing DIE Offset before normalizing."); - return ParentOffset; + if (!ParentOffset) + return std::nullopt; + return OffsetAndUnitID(*ParentOffset, getUnitID()); } /// If `Die` has a non-null parent and the parent is not a declaration, diff --git a/llvm/lib/CodeGen/AsmPrinter/AccelTable.cpp b/llvm/lib/CodeGen/AsmPrinter/AccelTable.cpp index f0a85818eb466..09f4a0f8a7882 100644 --- a/llvm/lib/CodeGen/AsmPrinter/AccelTable.cpp +++ b/llvm/lib/CodeGen/AsmPrinter/AccelTable.cpp @@ -222,7 +222,7 @@ class Dwarf5AccelTableWriter : public AccelTableWriter { // Indicates if this module is built with Split Dwarf enabled. bool IsSplitDwarf = false; /// Stores the DIE offsets which are indexed by this table. - DenseSet IndexedOffsets; + DenseSet IndexedOffsets; void populateAbbrevsMap(); @@ -231,10 +231,10 @@ class Dwarf5AccelTableWriter : public AccelTableWriter { void emitBuckets() const; void emitStringOffsets() const; void emitAbbrevs() const; - void - emitEntry(const DWARF5AccelTableData &Entry, - const DenseMap &DIEOffsetToAccelEntryLabel, - DenseSet &EmittedAccelEntrySymbols) const; + void emitEntry( + const DWARF5AccelTableData &Entry, + const DenseMap &DIEOffsetToAccelEntryLabel, + DenseSet &EmittedAccelEntrySymbols) const; void emitData(); public: @@ -457,8 +457,8 @@ static uint32_t constructAbbreviationTag( } static std::optional -getFormForIdxParent(const DenseSet &IndexedOffsets, - std::optional ParentOffset) { +getFormForIdxParent(const DenseSet &IndexedOffsets, + std::optional ParentOffset) { // No parent information if (!ParentOffset) return std::nullopt; @@ -476,8 +476,8 @@ void Dwarf5AccelTableWriter::populateAbbrevsMap() { std::optional EntryRet = getIndexForEntry(*Value); unsigned Tag = Value->getDieTag(); - std::optional MaybeParentForm = - getFormForIdxParent(IndexedOffsets, Value->getParentDieOffset()); + std::optional MaybeParentForm = getFormForIdxParent( + IndexedOffsets, Value->getParentDieOffsetAndUnitID()); uint32_t AbbrvTag = constructAbbreviationTag(Tag, EntryRet, MaybeParentForm); if (Abbreviations.count(AbbrvTag) == 0) { @@ -559,11 +559,12 @@ void Dwarf5AccelTableWriter::emitAbbrevs() const { void Dwarf5AccelTableWriter::emitEntry( const DWARF5AccelTableData &Entry, - const DenseMap &DIEOffsetToAccelEntryLabel, + const DenseMap &DIEOffsetToAccelEntryLabel, DenseSet &EmittedAccelEntrySymbols) const { std::optional EntryRet = getIndexForEntry(Entry); - std::optional MaybeParentOffset = Entry.getParentDieOffset(); + std::optional MaybeParentOffset = + Entry.getParentDieOffsetAndUnitID(); std::optional MaybeParentForm = getFormForIdxParent(IndexedOffsets, MaybeParentOffset); uint32_t AbbrvTag = @@ -574,7 +575,8 @@ void Dwarf5AccelTableWriter::emitEntry( assert(getTagFromAbbreviationTag(AbbrevIt->first) == Entry.getDieTag() && "Invalid Tag"); - auto EntrySymbolIt = DIEOffsetToAccelEntryLabel.find(Entry.getDieOffset()); + auto EntrySymbolIt = + DIEOffsetToAccelEntryLabel.find(Entry.getDieOffsetAndUnitID()); assert(EntrySymbolIt != DIEOffsetToAccelEntryLabel.end()); MCSymbol *EntrySymbol = EntrySymbolIt->getSecond(); @@ -614,9 +616,9 @@ void Dwarf5AccelTableWriter::emitEntry( } void Dwarf5AccelTableWriter::emitData() { - DenseMap DIEOffsetToAccelEntryLabel; + DenseMap DIEOffsetToAccelEntryLabel; - for (auto Offset : IndexedOffsets) + for (OffsetAndUnitID Offset : IndexedOffsets) DIEOffsetToAccelEntryLabel.insert({Offset, Asm->createTempSymbol("")}); Asm->OutStreamer->emitLabel(EntryPool); @@ -652,7 +654,7 @@ Dwarf5AccelTableWriter::Dwarf5AccelTableWriter( for (auto &Bucket : Contents.getBuckets()) for (auto *Hash : Bucket) for (auto *Value : Hash->getValues()) - IndexedOffsets.insert(Value->getDieOffset()); + IndexedOffsets.insert(Value->getDieOffsetAndUnitID()); populateAbbrevsMap(); } From b4ce5d5cbc6bd403e8b71100ca5477b9f3bfa452 Mon Sep 17 00:00:00 2001 From: Felipe de Azevedo Piovezan Date: Tue, 16 Jan 2024 13:52:32 -0800 Subject: [PATCH 5/6] fixup! Add test for multiple TUs --- .../X86/debug-names-parents-same-offset.ll | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 llvm/test/DebugInfo/X86/debug-names-parents-same-offset.ll diff --git a/llvm/test/DebugInfo/X86/debug-names-parents-same-offset.ll b/llvm/test/DebugInfo/X86/debug-names-parents-same-offset.ll new file mode 100644 index 0000000000000..9170cfc8508fa --- /dev/null +++ b/llvm/test/DebugInfo/X86/debug-names-parents-same-offset.ll @@ -0,0 +1,66 @@ +; RUN: llc -mtriple=x86_64 -generate-type-units -dwarf-version=5 -filetype=obj %s -o %t +; RUN: llvm-dwarfdump -debug-names %t | FileCheck %s + +; Two structs that have different names but are structurally identical: +; namespace MyNamespace { +; struct MyStruct1 { +; char c1; +; }; +; struct MyStruct2 { +; char c2; +; }; +; } // namespace MyNamespace +; MyNamespace::MyStruct1 gv1; +; MyNamespace::MyStruct2 gv2; + +; Using two TUs, this should produce DIE structures with the same offset. +; We test that accelerator table generation works with this. + +; CHECK: String: {{.*}} "MyStruct2" +; CHECK-NEXT: Entry @ {{.*}} { +; CHECK-NEXT: Abbrev: [[ABBREV:0x.*]] +; CHECK-NEXT: Tag: DW_TAG_structure_type +; CHECK-NEXT: DW_IDX_type_unit: 0x01 +; CHECK-NEXT: DW_IDX_die_offset: [[DieOffset:0x.*]] +; CHECK-NEXT: DW_IDX_parent: [[Parent1:0x.*]] +; CHECK: String: {{.*}} "MyStruct1" +; CHECK-NEXT: Entry @ {{.*}} { +; CHECK-NEXT: Abbrev: [[ABBREV]] +; CHECK-NEXT: Tag: DW_TAG_structure_type +; CHECK-NEXT: DW_IDX_type_unit: 0x00 +; CHECK-NEXT: DW_IDX_die_offset: [[DieOffset:0x.*]] +; CHECK-NOT: DW_IDX_parent: [[Parent1]] +; CHECK-NEXT: DW_IDX_parent: 0x{{.*}} + + +%"struct.MyNamespace::MyStruct1" = type { i8 } +%"struct.MyNamespace::MyStruct2" = type { i8 } + +@gv1 = dso_local global %"struct.MyNamespace::MyStruct1" zeroinitializer, align 1, !dbg !0 +@gv2 = dso_local global %"struct.MyNamespace::MyStruct2" zeroinitializer, align 1, !dbg !5 + +!llvm.dbg.cu = !{!2} +!llvm.module.flags = !{!15, !16, !17, !18, !19} +!llvm.ident = !{!20} + +!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression()) +!1 = distinct !DIGlobalVariable(name: "gv1", scope: !2, file: !3, line: 10, type: !12, isLocal: false, isDefinition: true) +!2 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !3, producer: "blah", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, globals: !4, splitDebugInlining: false) +!3 = !DIFile(filename: "two_tus.cpp", directory: "blah", checksumkind: CSK_MD5, checksum: "69acf04f32811fe7fd35449b58c3f5b1") +!4 = !{!0, !5} +!5 = !DIGlobalVariableExpression(var: !6, expr: !DIExpression()) +!6 = distinct !DIGlobalVariable(name: "gv2", scope: !2, file: !3, line: 11, type: !7, isLocal: false, isDefinition: true) +!7 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "MyStruct2", scope: !8, file: !3, line: 5, size: 8, flags: DIFlagTypePassByValue, elements: !9, identifier: "_ZTSN11MyNamespace9MyStruct2E") +!8 = !DINamespace(name: "MyNamespace", scope: null) +!9 = !{!10} +!10 = !DIDerivedType(tag: DW_TAG_member, name: "c2", scope: !7, file: !3, line: 6, baseType: !11, size: 8) +!11 = !DIBasicType(name: "char", size: 8, encoding: DW_ATE_signed_char) +!12 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "MyStruct1", scope: !8, file: !3, line: 2, size: 8, flags: DIFlagTypePassByValue, elements: !13, identifier: "_ZTSN11MyNamespace9MyStruct1E") +!13 = !{!14} +!14 = !DIDerivedType(tag: DW_TAG_member, name: "c1", scope: !12, file: !3, line: 3, baseType: !11, size: 8) +!15 = !{i32 7, !"Dwarf Version", i32 5} +!16 = !{i32 2, !"Debug Info Version", i32 3} +!17 = !{i32 1, !"wchar_size", i32 4} +!18 = !{i32 7, !"uwtable", i32 2} +!19 = !{i32 7, !"frame-pointer", i32 2} +!20 = !{!"blah"} From 990d15b78368f4d88128e5e1e549e0e6a462ccb0 Mon Sep 17 00:00:00 2001 From: Felipe de Azevedo Piovezan Date: Wed, 17 Jan 2024 10:12:14 -0800 Subject: [PATCH 6/6] fixup! NFC review comments --- llvm/include/llvm/CodeGen/AccelTable.h | 7 +++---- llvm/lib/CodeGen/AsmPrinter/AccelTable.cpp | 10 ++++------ 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/llvm/include/llvm/CodeGen/AccelTable.h b/llvm/include/llvm/CodeGen/AccelTable.h index 5747897a11416..e6a661696354b 100644 --- a/llvm/include/llvm/CodeGen/AccelTable.h +++ b/llvm/include/llvm/CodeGen/AccelTable.h @@ -318,10 +318,9 @@ class DWARF5AccelTableData : public AccelTableData { } std::optional getParentDieOffset() const { - auto OffsetAndId = getParentDieOffsetAndUnitID(); - if (!OffsetAndId) - return {}; - return OffsetAndId->offset(); + if (auto OffsetAndId = getParentDieOffsetAndUnitID()) + return OffsetAndId->offset(); + return {}; } std::optional getParentDieOffsetAndUnitID() const { diff --git a/llvm/lib/CodeGen/AsmPrinter/AccelTable.cpp b/llvm/lib/CodeGen/AsmPrinter/AccelTable.cpp index 09f4a0f8a7882..1024aabf2ab0f 100644 --- a/llvm/lib/CodeGen/AsmPrinter/AccelTable.cpp +++ b/llvm/lib/CodeGen/AsmPrinter/AccelTable.cpp @@ -403,12 +403,10 @@ void Dwarf5AccelTableWriter::Header::emit(Dwarf5AccelTableWriter &Ctx) { std::optional DWARF5AccelTableData::getDefiningParentDieOffset(const DIE &Die) { - auto *Parent = Die.getParent(); - if (!Parent) - return {}; - if (Parent->findAttribute(dwarf::Attribute::DW_AT_declaration)) - return {}; - return Parent->getOffset(); + if (auto *Parent = Die.getParent(); + Parent && !Parent->findAttribute(dwarf::Attribute::DW_AT_declaration)) + return Parent->getOffset(); + return {}; } enum IdxParentEncoding : uint8_t {