[DWARFv5] Put DWO ID in its place

[probinson]

Summary

In v5, the DWO ID is in the (split/skeleton) CU header, not an attribute
on the CU DIE.

This makes header sizes more different, so used the parsed size whenever
we have one, for simplicity.

[probinson]

Contrary to my usual practice, I did the emission and reading parts in the same patch, it was just easier to manage that way.
I could probably do the use-the-parsed-size stuff as a separate NFC if people want, but it didn't seem like that big a deal.

[aprantl]

Seems reasonable to me.

llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h

64 ↗

(On Diff #147857)

This is probably a silly micro-optimization, but for better packing we could move the DWOId here (assuming that it is 9 bytes large on a 64-bit architecture). Alternatively: is 0 a valid DWOId?

llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h

86 ↗

(On Diff #147857)

///

llvm/lib/DebugInfo/DWARF/DWARFContext.cpp

572 ↗

(On Diff #147857)

// No DWO ID?
continue;

[probinson]

llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h

64 ↗

(On Diff #147857)

LLVM generates DWOId using MD5, so in principle yes 0 is a valid ID. I can move it; didn't think about it occupying +1 byte.

llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h

86 ↗

(On Diff #147857)

This header is inconsistent (might be nice to tidy that up someday) but sure.

llvm/lib/DebugInfo/DWARF/DWARFContext.cpp

572 ↗

(On Diff #147857)

Would it then be multiline and require braces? :-)
Sure.

[JDevlieghere]

LGTM

llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h

227 ↗

(On Diff #147857)

Nit: This looks a little awkward. I probably would've moved the ternary operator into a separate variable and used that in the return DwarfUnit::getHeaderSize() + DwoIDSize;

[probinson]

Addressed review comments prior to commit.

llvm/trunk/include/llvm/DebugInfo/DWARF/DWARFTypeUnit.h

DWARFTypeUnit(DWARFContext &Context, const DWARFSection &Section,
                 const DWARFUnitHeader &Header,
                 const DWARFDebugAbbrev *DA, const DWARFSection *RS,
                 StringRef SS, const DWARFSection &SOS, const DWARFSection *AOS,
                 const DWARFSection &LS, bool LE, bool IsDWO,
                 const DWARFUnitSectionBase &UnitSection)
       : DWARFUnit(Context, Section, Header, DA, RS, SS, SOS, AOS, LS, LE, IsDWO,
                   UnitSection) {}
 
-  uint32_t getHeaderSize() const override {
-    return DWARFUnit::getHeaderSize() + 12;
-  }
   uint64_t getTypeHash() const { return getHeader().getTypeHash(); }
   uint32_t getTypeOffset() const { return getHeader().getTypeOffset(); }
 
   void dump(raw_ostream &OS, DIDumpOptions DumpOpts = {});
   static const DWARFSectionKind Section = DW_SECT_TYPES;
 };
 
 } // end namespace llvm
 
 #endif // LLVM_DEBUGINFO_DWARF_DWARFTYPEUNIT_H

llvm/trunk/include/llvm/DebugInfo/DWARF/DWARFUnit.h

class DWARFUnitHeader {
 
   // For DWO units only.
   const DWARFUnitIndex::Entry *IndexEntry = nullptr;
 
   // For type units only.
   uint64_t TypeHash = 0;
   uint32_t TypeOffset = 0;
 
+  // For v5 split or skeleton compile units only.
+  Optional<uint64_t> DWOId;
+
   // Unit type as parsed, or derived from the section kind.
   uint8_t UnitType = 0;
 
+  // Size as parsed. uint8_t for compactness.
+  uint8_t Size = 0;
+
 public:
   /// Parse a unit header from \p debug_info starting at \p offset_ptr.
   bool extract(DWARFContext &Context, const DWARFDataExtractor &debug_info,
                uint32_t *offset_ptr, DWARFSectionKind Kind = DW_SECT_INFO,
                const DWARFUnitIndex *Index = nullptr);
   uint32_t getOffset() const { return Offset; }
   const dwarf::FormParams &getFormParams() const { return FormParams; }
   uint16_t getVersion() const { return FormParams.Version; }
   dwarf::DwarfFormat getFormat() const { return FormParams.Format; }
   uint8_t getAddressByteSize() const { return FormParams.AddrSize; }
   uint8_t getRefAddrByteSize() const { return FormParams.getRefAddrByteSize(); }
   uint8_t getDwarfOffsetByteSize() const {
     return FormParams.getDwarfOffsetByteSize();
   }
   uint32_t getLength() const { return Length; }
   uint64_t getAbbrOffset() const { return AbbrOffset; }
+  Optional<uint64_t> getDWOId() const { return DWOId; }
+  void setDWOId(uint64_t Id) {
+    assert((!DWOId || *DWOId == Id) && "setting DWOId to a different value");
+    DWOId = Id;
+  }
   const DWARFUnitIndex::Entry *getIndexEntry() const { return IndexEntry; }
   uint64_t getTypeHash() const { return TypeHash; }
   uint32_t getTypeOffset() const { return TypeOffset; }
   uint8_t getUnitType() const { return UnitType; }
   bool isTypeUnit() const {
     return UnitType == dwarf::DW_UT_type || UnitType == dwarf::DW_UT_split_type;
   }
+  uint8_t getSize() const { return Size; }
   // FIXME: Support DWARF64.
   uint32_t getNextUnitOffset() const { return Offset + Length + 4; }
 };
 
 /// Base class for all DWARFUnitSection classes. This provides the
 /// functionality common to all unit types.
 class DWARFUnitSectionBase {
 public:
uint32_t getDIEIndex(const DWARFDebugInfoEntry *Die) {
     auto First = DieArray.data();
     assert(Die >= First && Die < First + DieArray.size());
     return Die - First;
   }
 
 protected:
   const DWARFUnitHeader &getHeader() const { return Header; }
 
-  /// Size in bytes of the unit header.
-  virtual uint32_t getHeaderSize() const { return getVersion() <= 4 ? 11 : 12; }
+  /// Size in bytes of the parsed unit header.
+  uint32_t getHeaderSize() const { return Header.getSize(); }
 
   /// Find the unit's contribution to the string offsets table and determine its
   /// length and form. The given offset is expected to be derived from the unit
   /// DIE's DW_AT_str_offsets_base attribute.
   Optional<StrOffsetsContributionDescriptor>
   determineStringOffsetsTableContribution(DWARFDataExtractor &DA,
                                           uint64_t Offset);
 
public:
   DWARFDie getUnitDIE(bool ExtractUnitDIEOnly = true) {
     extractDIEsIfNeeded(ExtractUnitDIEOnly);
     if (DieArray.empty())
       return DWARFDie();
     return DWARFDie(this, &DieArray[0]);
   }
 
   const char *getCompilationDir();
-  Optional<uint64_t> getDWOId();
+  Optional<uint64_t> getDWOId() const { return getHeader().getDWOId(); }
+  void setDWOId(uint64_t NewID) { Header.setDWOId(NewID); }
 
   /// Return a vector of address ranges resulting from a (possibly encoded)
   /// range list starting at a given offset in the appropriate ranges section.
   DWARFAddressRangesVector findRnglistFromOffset(uint32_t Offset);
 
   /// Return a vector of address ranges retrieved from an encoded range
   /// list whose offset is found via a table lookup given an index (DWARF v5
   /// and later).

llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h

class DwarfCompileUnit final : public DwarfUnit {
 
   // The base address of this unit, if any. Used for relative references in
   // ranges/locs.
   const MCSymbol *BaseAddress = nullptr;
 
   DenseMap<const MDNode *, DIE *> AbstractSPDies;
   DenseMap<const MDNode *, std::unique_ptr<DbgVariable>> AbstractVariables;
 
+  /// DWO ID for correlating skeleton and split units.
+  uint64_t DWOId = 0;
+
   /// Construct a DIE for the given DbgVariable without initializing the
   /// DbgVariable's DIE reference.
   DIE *constructVariableDIEImpl(const DbgVariable &DV, bool Abstract);
 
   bool isDwoUnit() const override;
 
   DenseMap<const MDNode *, DIE *> &getAbstractSPDies() {
     if (isDwoUnit() && !DD->shareAcrossDWOCUs())
public:
   DbgVariable *getExistingAbstractVariable(InlinedVariable IV,
                                            const DILocalVariable *&Cleansed);
   DbgVariable *getExistingAbstractVariable(InlinedVariable IV);
   void createAbstractVariable(const DILocalVariable *DV, LexicalScope *Scope);
 
   /// Set the skeleton unit associated with this unit.
   void setSkeleton(DwarfCompileUnit &Skel) { Skeleton = &Skel; }
 
+  unsigned getHeaderSize() const override {
+    // DWARF v5 added the DWO ID to the header for split/skeleton units.
+    unsigned DWOIdSize =
+        DD->getDwarfVersion() >= 5 && DD->useSplitDwarf() ? sizeof(uint64_t)
+                                                          : 0;
+    return DwarfUnit::getHeaderSize() + DWOIdSize;
+  }
   unsigned getLength() {
     return sizeof(uint32_t) + // Length field
         getHeaderSize() + getUnitDie().getSize();
   }
 
   void emitHeader(bool UseOffsets) override;
 
   MCSymbol *getLabelBegin() const {
public:
 
   /// getRanges - Get the list of ranges for this unit.
   const SmallVectorImpl<RangeSpan> &getRanges() const { return CURanges; }
   SmallVector<RangeSpan, 2> takeRanges() { return std::move(CURanges); }
 
   void setBaseAddress(const MCSymbol *Base) { BaseAddress = Base; }
   const MCSymbol *getBaseAddress() const { return BaseAddress; }
 
+  uint64_t getDWOId() const { return DWOId; }
+  void setDWOId(uint64_t DwoId) { DWOId = DwoId; }
+
   bool hasDwarfPubSections() const;
 };
 
 } // end namespace llvm
 
 #endif // LLVM_LIB_CODEGEN_ASMPRINTER_DWARFCOMPILEUNIT_H

llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp

if (!Skeleton && !DD->useSectionsAsReferences()) {
     LabelBegin = Asm->createTempSymbol("cu_begin");
     Asm->OutStreamer->EmitLabel(LabelBegin);
   }
 
   dwarf::UnitType UT = Skeleton ? dwarf::DW_UT_split_compile
                                 : DD->useSplitDwarf() ? dwarf::DW_UT_skeleton
                                                       : dwarf::DW_UT_compile;
   DwarfUnit::emitCommonHeader(UseOffsets, UT);
+  if (DD->getDwarfVersion() >= 5 && UT != dwarf::DW_UT_compile)
+    Asm->emitInt64(getDWOId());
 }
 
 bool DwarfCompileUnit::hasDwarfPubSections() const {
   // Opting in to GNU Pubnames/types overrides the default to ensure these are
   // generated for things like Gold's gdb_index generation.
   if (CUNode->getGnuPubnames())
     return true;
 

llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp

for (const auto &P : CUMap) {
     // Add CU specific attributes if we need to add any.
     // If we're splitting the dwarf out now that we've got the entire
     // CU then add the dwo id to it.
     auto *SkCU = TheCU.getSkeleton();
     if (useSplitDwarf()) {
       // Emit a unique identifier for this CU.
       uint64_t ID =
           DIEHash(Asm).computeCUSignature(DWOName, TheCU.getUnitDie());
+      if (getDwarfVersion() >= 5) {
+        TheCU.setDWOId(ID);
+        SkCU->setDWOId(ID);
+      } else {
         TheCU.addUInt(TheCU.getUnitDie(), dwarf::DW_AT_GNU_dwo_id,
                       dwarf::DW_FORM_data8, ID);
         SkCU->addUInt(SkCU->getUnitDie(), dwarf::DW_AT_GNU_dwo_id,
                       dwarf::DW_FORM_data8, ID);
-
+      }
       // We don't keep track of which addresses are used in which CU so this
       // is a bit pessimistic under LTO.
       if (!AddrPool.isEmpty()) {
         const MCSymbol *Sym = TLOF.getDwarfAddrSection()->getBeginSymbol();
         SkCU->addSectionLabel(SkCU->getUnitDie(), dwarf::DW_AT_GNU_addr_base,
                               Sym, Sym);
       }
       if (!SkCU->getRangeLists().empty()) {

llvm/trunk/lib/DebugInfo/DWARF/DWARFCompileUnit.cpp

 
 void DWARFCompileUnit::dump(raw_ostream &OS, DIDumpOptions DumpOpts) {
   OS << format("0x%08x", getOffset()) << ": Compile Unit:"
      << " length = " << format("0x%08x", getLength())
      << " version = " << format("0x%04x", getVersion());
   if (getVersion() >= 5)
     OS << " unit_type = " << dwarf::UnitTypeString(getUnitType());
   OS << " abbr_offset = " << format("0x%04x", getAbbreviations()->getOffset())
-     << " addr_size = " << format("0x%02x", getAddressByteSize())
-     << " (next unit at " << format("0x%08x", getNextUnitOffset())
-     << ")\n";
+     << " addr_size = " << format("0x%02x", getAddressByteSize());
+  if (getVersion() >= 5 && getUnitType() != dwarf::DW_UT_compile)
+    OS << " DWO_id = " << format("0x%016" PRIx64, *getDWOId());
+  OS << " (next unit at " << format("0x%08x", getNextUnitOffset()) << ")\n";
 
   if (DWARFDie CUDie = getUnitDIE(false))
     CUDie.dump(OS, 0, DumpOpts);
   else
     OS << "<compile unit can't be parsed!>\n\n";
 }
 
 // VTable anchor.
 DWARFCompileUnit::~DWARFCompileUnit() = default;

llvm/trunk/lib/DebugInfo/DWARF/DWARFContext.cpp

if (const auto *R = CUI.getFromHash(Hash))
       return DWOCUs.getUnitForIndexEntry(*R);
     return nullptr;
   }
 
   // If there's no index, just search through the CUs in the DWO - there's
   // probably only one unless this is something like LTO - though an in-process
   // built/cached lookup table could be used in that case to improve repeated
   // lookups of different CUs in the DWO.
-  for (const auto &DWOCU : dwo_compile_units())
+  for (const auto &DWOCU : dwo_compile_units()) {
+    // Might not have parsed DWO ID yet.
+    if (!DWOCU->getDWOId()) {
+      if (Optional<uint64_t> DWOId =
+          toUnsigned(DWOCU->getUnitDIE().find(DW_AT_GNU_dwo_id)))
+        DWOCU->setDWOId(*DWOId);
+      else
+        // No DWO ID?
+        continue;
+    }
     if (DWOCU->getDWOId() == Hash)
       return DWOCU.get();
+  }
   return nullptr;
 }
 
 DWARFDie DWARFContext::getDIEForOffset(uint32_t Offset) {
   parseCompileUnits();
   if (auto *CU = CUs.getUnitForOffset(Offset))
     return CU->getDIEForOffset(Offset);
   return DWARFDie();

llvm/trunk/lib/DebugInfo/DWARF/DWARFUnit.cpp

if (IndexEntry) {
     auto *UnitContrib = IndexEntry->getOffset();
     if (!UnitContrib || UnitContrib->Length != (Length + 4))
       return false;
     auto *AbbrEntry = IndexEntry->getOffset(DW_SECT_ABBREV);
     if (!AbbrEntry)
       return false;
     AbbrOffset = AbbrEntry->Offset;
   }
-  bool TypeOffsetOK = true;
   if (isTypeUnit()) {
     TypeHash = debug_info.getU64(offset_ptr);
     TypeOffset = debug_info.getU32(offset_ptr);
-    // Type offset is unit-relative; should be after the header and before
-    // the end of the current unit.
-    TypeOffsetOK = TypeOffset >= *offset_ptr - Offset &&
-                   TypeOffset < getLength() + SizeOfLength;
-  }
+  } else if (UnitType == DW_UT_split_compile || UnitType == DW_UT_skeleton)
+    DWOId = debug_info.getU64(offset_ptr);
+
+  // Header fields all parsed, capture the size of this unit header.
+  assert(*offset_ptr - Offset <= 255 && "unexpected header size");
+  Size = uint8_t(*offset_ptr - Offset);
+
+  bool TypeOffsetOK =
+      !isTypeUnit()
+          ? true
+          : TypeOffset >= Size && TypeOffset < getLength() + SizeOfLength;
   bool LengthOK = debug_info.isValidOffset(getNextUnitOffset() - 1);
   bool VersionOK = DWARFContext::isSupportedVersion(getVersion());
   bool AddrSizeOK = getAddressByteSize() == 4 || getAddressByteSize() == 8;
 
   if (!LengthOK || !VersionOK || !AddrSizeOK || !TypeOffsetOK)
     return false;
 
   // Keep track of the highest DWARF version we encounter across all units.
void DWARFUnit::clear() {
   clearDIEs(false);
   DWO.reset();
 }
 
 const char *DWARFUnit::getCompilationDir() {
   return dwarf::toString(getUnitDIE().find(DW_AT_comp_dir), nullptr);
 }
 
-Optional<uint64_t> DWARFUnit::getDWOId() {
-  return toUnsigned(getUnitDIE().find(DW_AT_GNU_dwo_id));
-}
-
 void DWARFUnit::extractDIEsToVector(
     bool AppendCUDie, bool AppendNonCUDies,
     std::vector<DWARFDebugInfoEntry> &Dies) const {
   if (!AppendCUDie && !AppendNonCUDies)
     return;
 
   // Set the offset to that of the first DIE and calculate the start of the
   // next compilation unit header.
size_t DWARFUnit::extractDIEsIfNeeded(bool CUDieOnly) {
   extractDIEsToVector(!HasCUDie, !CUDieOnly, DieArray);
 
   if (DieArray.empty())
     return 0;
 
   // If CU DIE was just parsed, copy several attribute values from it.
   if (!HasCUDie) {
     DWARFDie UnitDie = getUnitDIE();
+    if (Optional<uint64_t> DWOId = toUnsigned(UnitDie.find(DW_AT_GNU_dwo_id)))
+      Header.setDWOId(*DWOId);
     if (!isDWO) {
       assert(AddrOffsetSectionBase == 0);
       assert(RangeSectionBase == 0);
       AddrOffsetSectionBase =
           toSectionOffset(UnitDie.find(DW_AT_GNU_addr_base), 0);
       RangeSectionBase = toSectionOffset(UnitDie.find(DW_AT_rnglists_base), 0);
     }
 

llvm/trunk/test/CodeGen/X86/dwarf-headers.ll

 
 ; RUN: llc -split-dwarf-file=foo.dwo -split-dwarf-output=%t.dwo \
 ; RUN:     -dwarf-version=5 -generate-type-units \
 ; RUN:     -filetype=obj -O0 -mtriple=x86_64-unknown-linux-gnu < %s \
 ; RUN:     | llvm-dwarfdump -v - | FileCheck %s --check-prefix=O-5
 ; RUN: llvm-dwarfdump -v %t.dwo | FileCheck %s --check-prefix=DWO-5
 
 ; Looking for DWARF headers to be generated correctly.
-; There are 7 variants: v4 CU, v4 TU, v5 (normal/skeleton/split) CU,
-; v5 (normal/split) TU.  The v5 CU variants and TU variants differ
-; only in the type-unit code.
+; There are 8 variants with 5 formats: v4 CU, v4 TU, v5 normal/partial CU,
+; v5 skeleton/split CU, v5 normal/split TU.  Some v5 variants differ only
+; in the unit_type code, and the skeleton/split CU differs from normal/partial
+; by having one extra field (dwo_id).
 ; (v2 thru v4 CUs are all the same, and TUs were invented in v4,
 ; so we don't bother checking older versions.)
 
 ; Test case built from:
 ;struct S {
 ;  int s1;
 ;};
 ;

 ; SINGLE-5: .debug_types contents:
 ; SINGLE-5: 0x00000000: Type Unit: {{.*}} version = 0x0005 unit_type = DW_UT_type abbr_offset
 ; SINGLE-5: 0x00000018: DW_TAG_type_unit
 
 ; Verify the v5 split headers.
 ;
 ; O-5: .debug_info contents:
 ; O-5: 0x00000000: Compile Unit: {{.*}} version = 0x0005 unit_type = DW_UT_skeleton abbr_offset
-; O-5: 0x0000000c: DW_TAG_compile_unit
+; O-5-SAME:        DWO_id = 0x4ed74084f749d96b
+; O-5: 0x00000014: DW_TAG_compile_unit
 ;
 ; DWO-5: .debug_info.dwo contents:
 ; DWO-5: 0x00000000: Compile Unit: {{.*}} version = 0x0005 unit_type = DW_UT_split_compile abbr_offset
-; DWO-5: 0x0000000c: DW_TAG_compile_unit
+; DWO-5-SAME:        DWO_id = 0x4ed74084f749d96b
+; DWO-5: 0x00000014: DW_TAG_compile_unit
 ;
 ; FIXME: V5 wants type units in .debug_info.dwo not .debug_types.dwo.
 ; DWO-5: .debug_types.dwo contents:
 ; DWO-5: 0x00000000: Type Unit: {{.*}} version = 0x0005 unit_type = DW_UT_split_type abbr_offset
 ; DWO-5: 0x00000018: DW_TAG_type_unit
 
 
 ; ModuleID = 't.cpp'

llvm/trunk/test/DebugInfo/X86/dwarfdump-header.s

 
 # DWARF v5 split CU header.
         .long  CU_split_5_end-CU_split_5_version # Length of Unit
 CU_split_5_version:
         .short 5                # DWARF version number
         .byte 5                 # DWARF Unit Type
         .byte 8                 # Address Size (in bytes)
         .long .debug_abbrev.dwo # Offset Into Abbrev. Section
+        .quad 0x5a              # DWO ID
 # The split compile-unit DIE, with DW_AT_producer, DW_AT_name, DW_AT_stmt_list.
         .byte 1
         .long dwo_producer
         .long dwo_CU_5
         .long dwo_LH_5_start
         .byte 0 # NULL
 CU_split_5_end:
 
-# CHECK: 0x00000000: Compile Unit: length = 0x00000016 version = 0x0005 unit_type = DW_UT_split_compile abbr_offset = 0x0000 addr_size = 0x08 (next unit at 0x0000001a)
-# CHECK: 0x0000000c: DW_TAG_compile_unit
+# CHECK: 0x00000000: Compile Unit: length = 0x0000001e version = 0x0005 unit_type = DW_UT_split_compile abbr_offset = 0x0000 addr_size = 0x08 DWO_id = 0x000000000000005a (next unit at 0x00000022)
+# CHECK: 0x00000014: DW_TAG_compile_unit
 # CHECK-NEXT: DW_AT_producer {{.*}} "Handmade DWO producer"
 # CHECK-NEXT: DW_AT_name {{.*}} "V5_dwo_compile_unit"
 
         .section .debug_types,"",@progbits
 # CHECK-LABEL: .debug_types contents:
 
 # DWARF v4 Type unit header. Normal/split are identical so we do only one.
 TU_4_start: