[DebugInfo] Fix reading DWARFv5 type units in DWP.

[ikudrin]

Summary

In DWARFv5, type units are stored in .debug_info sections, along with compilation units, and they are distinguished by the unit_type field in the header, not by the name of the section. It is impossible to associate the correct index section of a DWP file with the unit before the unit's header is read. This patch fixes reading DWARFv5 type units by parsing the header first and then applying the index entry according to the actual unit type.

[dblaikie]

Looks good - thanks!

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

class DWARFUnitHeader {
   // 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.
+  /// Note that \p SectionKind is used as a hint to guess the unit type
+  /// for DWARF formats prior to DWARFv5. In DWARFv5 the unit type is
+  /// explicitly defined in the header and the hint is ignored.
   bool extract(DWARFContext &Context, const DWARFDataExtractor &debug_info,
-               uint64_t *offset_ptr, DWARFSectionKind Kind = DW_SECT_INFO,
-               const DWARFUnitIndex *Index = nullptr,
-               const DWARFUnitIndex::Entry *Entry = nullptr);
+               uint64_t *offset_ptr, DWARFSectionKind SectionKind);
+  // For units in DWARF Package File, remember the index entry and update
+  // the abbreviation offset read by extract().
+  bool applyIndexEntry(const DWARFUnitIndex::Entry *Entry);
   uint64_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();

llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp

Parser = [=, &Context, &Obj, &Section, &SOS,
               &LS](uint64_t Offset, DWARFSectionKind SectionKind,
                    const DWARFSection *CurSection,
                    const DWARFUnitIndex::Entry *IndexEntry)
         -> std::unique_ptr<DWARFUnit> {
       const DWARFSection &InfoSection = CurSection ? *CurSection : Section;
       DWARFDataExtractor Data(Obj, InfoSection, LE, 0);
       if (!Data.isValidOffset(Offset))
         return nullptr;
-      const DWARFUnitIndex *Index = nullptr;
-      if (IsDWO)
-        Index = &getDWARFUnitIndex(Context, SectionKind);
       DWARFUnitHeader Header;
-      if (!Header.extract(Context, Data, &Offset, SectionKind, Index,
-                          IndexEntry))
+      if (!Header.extract(Context, Data, &Offset, SectionKind))
+        return nullptr;
+      if (!IndexEntry && IsDWO) {
+        const DWARFUnitIndex &Index = getDWARFUnitIndex(
+            Context, Header.isTypeUnit() ? DW_SECT_EXT_TYPES : DW_SECT_INFO);
+        IndexEntry = Index.getFromOffset(Header.getOffset());
+      }
+      if (IndexEntry && !Header.applyIndexEntry(IndexEntry))
         return nullptr;
       std::unique_ptr<DWARFUnit> U;
       if (Header.isTypeUnit())
         U = std::make_unique<DWARFTypeUnit>(Context, InfoSection, Header, DA,
                                              RS, LocSection, SS, SOS, AOS, LS,
                                              LE, IsDWO, *this);
       else
         U = std::make_unique<DWARFCompileUnit>(Context, InfoSection, Header,
Optional<uint64_t> DWARFUnit::getStringOffsetSectionItem(uint32_t Index) const {
   DWARFDataExtractor DA(Context.getDWARFObj(), StringOffsetSection,
                         isLittleEndian, 0);
   return DA.getRelocatedValue(ItemSize, &Offset);
 }
 
 bool DWARFUnitHeader::extract(DWARFContext &Context,
                               const DWARFDataExtractor &debug_info,
                               uint64_t *offset_ptr,
-                              DWARFSectionKind SectionKind,
-                              const DWARFUnitIndex *Index,
-                              const DWARFUnitIndex::Entry *Entry) {
+                              DWARFSectionKind SectionKind) {
   Offset = *offset_ptr;
   Error Err = Error::success();
-  IndexEntry = Entry;
-  if (!IndexEntry && Index)
-    IndexEntry = Index->getFromOffset(*offset_ptr);
+  IndexEntry = nullptr;
   std::tie(Length, FormParams.Format) =
       debug_info.getInitialLength(offset_ptr, &Err);
   FormParams.Version = debug_info.getU16(offset_ptr, &Err);
   if (FormParams.Version >= 5) {
     UnitType = debug_info.getU8(offset_ptr, &Err);
     FormParams.AddrSize = debug_info.getU8(offset_ptr, &Err);
     AbbrOffset = debug_info.getRelocatedValue(
         FormParams.getDwarfOffsetByteSize(), offset_ptr, nullptr, &Err);
if (isTypeUnit()) {
     TypeOffset = debug_info.getUnsigned(
         offset_ptr, FormParams.getDwarfOffsetByteSize(), &Err);
   } else if (UnitType == DW_UT_split_compile || UnitType == DW_UT_skeleton)
     DWOId = debug_info.getU64(offset_ptr, &Err);
 
   if (errorToBool(std::move(Err)))
     return false;
 
-  if (IndexEntry) {
-    if (AbbrOffset)
-      return false;
-    auto *UnitContrib = IndexEntry->getContribution();
-    if (!UnitContrib ||
-        UnitContrib->Length != (Length + getUnitLengthFieldByteSize()))
-      return false;
-    auto *AbbrEntry = IndexEntry->getContribution(DW_SECT_ABBREV);
-    if (!AbbrEntry)
-      return false;
-    AbbrOffset = AbbrEntry->Offset;
-  }
-
   // 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);
 
   // Type offset is unit-relative; should be after the header and before
   // the end of the current unit.
   bool TypeOffsetOK =
       !isTypeUnit()
           ? true
           : TypeOffset >= Size &&
                 TypeOffset < getLength() + getUnitLengthFieldByteSize();
   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.
   Context.setMaxVersionIfGreater(getVersion());
   return true;
 }
 
+bool DWARFUnitHeader::applyIndexEntry(const DWARFUnitIndex::Entry *Entry) {
+  assert(Entry);
+  assert(!IndexEntry);
+  IndexEntry = Entry;
+  if (AbbrOffset)
+    return false;
+  auto *UnitContrib = IndexEntry->getContribution();
+  if (!UnitContrib ||
+      UnitContrib->Length != (getLength() + getUnitLengthFieldByteSize()))
+    return false;
+  auto *AbbrEntry = IndexEntry->getContribution(DW_SECT_ABBREV);
+  if (!AbbrEntry)
+    return false;
+  AbbrOffset = AbbrEntry->Offset;
+  return true;
+}
+
 // Parse the rangelist table header, including the optional array of offsets
 // following it (DWARF v5 and later).
 template<typename ListTableType>
 static Expected<ListTableType>
 parseListTableHeader(DWARFDataExtractor &DA, uint64_t Offset,
                         DwarfFormat Format) {
   // We are expected to be called with Offset 0 or pointing just past the table
   // header. Correct Offset in the latter case so that it points to the start

llvm/test/DebugInfo/X86/dwp-v5-tu.s

+## The test checks that we can read DWARFv5 type units in DWP files.
+
+# RUN: llvm-mc -triple x86_64-unknown-linux %s -filetype=obj -o - | \
+# RUN:   llvm-dwarfdump -debug-info - | \
+# RUN:   FileCheck %s
+
+# CHECK:      .debug_info.dwo contents:
+# CHECK:      Type Unit:
+# CHECK:      DW_TAG_type_unit
+# CHECK-NEXT:   DW_AT_visibility (DW_VIS_local)
+# CHECK:      DW_TAG_structure_type
+# CHECK-NEXT:   DW_AT_name ("foo")
+
+    .section .debug_abbrev.dwo, "e", @progbits
+## Reserve some space in the section so that the abbreviation table for the type
+## unit does not start at the beginning of the section and thus the base offset
+## from the index section should be added to find the correct offset.
+    .space 16
+.LAbbrevBegin:
+    .uleb128 1                              # Abbrev code
+    .uleb128 0x41                           # DW_TAG_type_unit
+    .byte 1                                 # DW_CHILDREN_yes
+    .uleb128 0x17                           # DW_AT_visibility
+    .uleb128 0x0b                           # DW_FORM_data1
+    .byte 0                                 # EOM(1)
+    .byte 0                                 # EOM(2)
+    .uleb128 2                              # Abbrev code
+    .uleb128 0x13                           # DW_TAG_structure_type
+    .byte 0                                 # DW_CHILDREN_no
+    .uleb128 0x03                           # DW_AT_name
+    .uleb128 0x08                           # DW_FORM_string
+    .byte 0                                 # EOM(1)
+    .byte 0                                 # EOM(2)
+    .byte 0                                 # EOM(3)
+.LAbbrevEnd:
+
+    .section .debug_info.dwo, "e", @progbits
+.LTUBegin:
+    .long .LTUEnd-.LTUVersion               # Length of Unit
+.LTUVersion:
+    .short 5                                # DWARF version number
+    .byte 6                                 # DW_UT_split_type
+    .byte 8                                 # Address Size (in bytes)
+    .long 0                                 # Offset Into Abbrev. Section
+    .quad 0x1100001122222222                # Type Signature
+    .long .LTUType-.LTUBegin                # Type offset
+    .uleb128 1                              # Abbrev [1] DW_TAG_type_unit
+    .byte 1                                 # DW_AT_visibility
+.LTUType:
+    .uleb128 2                              # Abbrev [2] DW_TAG_structure_type
+    .asciz "foo"                            # DW_AT_name
+.LTUEnd:
+
+    .section .debug_tu_index, "", @progbits
+## Header:
+    .short 5                                # Version
+    .space 2                                # Padding
+    .long 2                                 # Section count
+    .long 1                                 # Unit count
+    .long 2                                 # Slot count
+## Hash Table of Signatures:
+    .quad 0x1100001122222222
+    .quad 0
+## Parallel Table of Indexes:
+    .long 1
+    .long 0
+## Table of Section Offsets:
+## Row 0:
+    .long 1                                 # DW_SECT_INFO
+    .long 3                                 # DW_SECT_ABBREV
+## Row 1:
+    .long 0                                 # Offset in .debug_info.dwo
+    .long .LAbbrevBegin-.debug_abbrev.dwo   # Offset in .debug_abbrev.dwo
+## Table of Section Sizes:
+    .long .LTUEnd-.LTUBegin                 # Size in .debug_info.dwo
+    .long .LAbbrevEnd-.LAbbrevBegin         # Size in .debug_abbrev.dwo

llvm/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp

TEST(DWARFDebugInfo, TestDWARF64UnitLength) {
   auto Context = DWARFContext::create(Sections, /* AddrSize = */ 4,
                                       /* isLittleEndian = */ true);
   const auto &Obj = Context->getDWARFObj();
   Obj.forEachInfoSections([&](const DWARFSection &Sec) {
     DWARFUnitHeader Header;
     DWARFDataExtractor Data(Obj, Sec, /* IsLittleEndian = */ true,
                             /* AddressSize = */ 4);
     uint64_t Offset = 0;
-    EXPECT_FALSE(Header.extract(*Context, Data, &Offset));
+    EXPECT_FALSE(Header.extract(*Context, Data, &Offset, DW_SECT_INFO));
     // Header.extract() returns false because there is not enough space
     // in the section for the declared length. Anyway, we can check that
     // the properties are read correctly.
     ASSERT_EQ(DwarfFormat::DWARF64, Header.getFormat());
     ASSERT_EQ(0x1122334455667788ULL, Header.getLength());
     ASSERT_EQ(5, Header.getVersion());
     ASSERT_EQ(DW_UT_compile, Header.getUnitType());
     ASSERT_EQ(4, Header.getAddressByteSize());