[DWARF/AccelTable] Remove getDIESectionOffset for DWARF v5 entries

[labath]

Summary

This method was not correct for entries in DWO files as it assumed it
could just add up the CU and DIE offsets to get the absolute DIE offset.
This is not correct for the DWO files, as here the CU offset will
reference the skeleton unit, whereas the DIE offset will be the offset
in the full unit in the DWO file.

Unfortunately, this means that we are not able to determine the absolute
DIE offset using the information in the .debug_names section alone,
which means we have to offload some of this work to the users of this
class.

To demonstrate how this can be done, I've added/fixed the ability to
lookup entries using accelerator tables in DWO files in llvm-dwarfdump.
To make this happen, I've needed to make two extra changes in other
classes:

• made the DWARFContext method to lookup a CU based on the section offset public. I've needed this functionality to lookup a CU, and this seems like a useful thing in general.
• made DWARFUnit::getDWOId call extractDIEsIfNeeded. Before this, the DWOId was filled in only if the root DIE happened to be parsed before we called the accessor. Since the lazy parsing is supposed to happen under the hood, calling extractDIEsIfNeeded seems appropriate.

[dblaikie]

Seems good to me

lib/DebugInfo/DWARF/DWARFVerifier.cpp

1267 ↗

(On Diff #150707)

Generally I'd suggest just using [&] (capture all by reference) when using a lambda that doesn't escape its scope - as it doesn't tend to add a lot of value to enumerate the specific variables used within a lambda (in the same way that other local scopes in C++ (if/while/for/etc) don't enumerate which variables will be used within them)

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

public:
 
 
   public:
     /// Returns the Offset of the Compilation Unit associated with this
     /// Accelerator Entry or None if the Compilation Unit offset is not recorded
     /// in this Accelerator Entry.
     virtual Optional<uint64_t> getCUOffset() const = 0;
 
-    /// Returns the Section Offset of the Debug Info Entry associated with this
-    /// Accelerator Entry or None if the DIE offset is not recorded in this
-    /// Accelerator Entry. The returned offset is relative to the start of the
-    /// Section containing the DIE.
-    virtual Optional<uint64_t> getDIESectionOffset() const = 0;
-
     /// Returns the Tag of the Debug Info Entry associated with this
     /// Accelerator Entry or None if the Tag is not recorded in this
     /// Accelerator Entry.
     virtual Optional<dwarf::Tag> getTag() const = 0;
 
     /// Returns the raw values of fields in the Accelerator Entry. In general,
     /// these can only be interpreted with the help of the metadata in the
     /// owning Accelerator Table.
class Entry final : public DWARFAcceleratorTable::Entry {
 
     Entry(const HeaderData &Data);
     Entry() = default;
 
     void extract(const AppleAcceleratorTable &AccelTable, uint32_t *Offset);
 
   public:
     Optional<uint64_t> getCUOffset() const override;
-    Optional<uint64_t> getDIESectionOffset() const override;
+
+    /// Returns the Section Offset of the Debug Info Entry associated with this
+    /// Accelerator Entry or None if the DIE offset is not recorded in this
+    /// Accelerator Entry. The returned offset is relative to the start of the
+    /// Section containing the DIE.
+    Optional<uint64_t> getDIESectionOffset() const;
+
     Optional<dwarf::Tag> getTag() const override;
 
     /// Returns the value of the Atom in this Accelerator Entry, if the Entry
     /// contains such Atom.
     Optional<DWARFFormValue> lookup(HeaderData::AtomType Atom) const;
 
     friend class AppleAcceleratorTable;
     friend class ValueIterator;
public:
   class Entry final : public DWARFAcceleratorTable::Entry {
     const NameIndex *NameIdx;
     const Abbrev *Abbr;
 
     Entry(const NameIndex &NameIdx, const Abbrev &Abbr);
 
   public:
     Optional<uint64_t> getCUOffset() const override;
-    Optional<uint64_t> getDIESectionOffset() const override;
     Optional<dwarf::Tag> getTag() const override { return tag(); }
 
     /// Returns the Index into the Compilation Unit list of the owning Name
     /// Index or None if this Accelerator Entry does not have an associated
     /// Compilation Unit. It is up to the user to verify that the returned Index
     /// is valid in the owning NameIndex (or use getCUOffset(), which will
     /// handle that check itself). Note that entries in NameIndexes which index
     /// just a single Compilation Unit are implicitly associated with that unit,

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

public:
   /// Get the compile unit at the specified index for the DWO compile units.
   DWARFCompileUnit *getDWOCompileUnitAtIndex(unsigned index) {
     parseDWOCompileUnits();
     return DWOCUs[index].get();
   }
 
   DWARFCompileUnit *getDWOCompileUnitForHash(uint64_t Hash);
 
+  /// Return the compile unit that includes an offset (relative to .debug_info).
+  DWARFCompileUnit *getCompileUnitForOffset(uint32_t Offset);
+
   /// Get a DIE given an exact offset.
   DWARFDie getDIEForOffset(uint32_t Offset);
 
   unsigned getMaxVersion() const { return MaxVersion; }
 
   void setMaxVersionIfGreater(unsigned Version) {
     if (Version > MaxVersion)
       MaxVersion = Version;
create(const StringMap<std::unique_ptr<MemoryBuffer>> &Sections,
          uint8_t AddrSize, bool isLittleEndian = sys::IsLittleEndianHost);
 
   /// Loads register info for the architecture of the provided object file.
   /// Improves readability of dumped DWARF expressions. Requires the caller to
   /// have initialized the relevant target descriptions.
   Error loadRegisterInfo(const object::ObjectFile &Obj);
 
 private:
-  /// Return the compile unit that includes an offset (relative to .debug_info).
-  DWARFCompileUnit *getCompileUnitForOffset(uint32_t Offset);
-
   /// Return the compile unit which contains instruction with provided
   /// address.
   DWARFCompileUnit *getCompileUnitForAddress(uint64_t Address);
 };
 
 } // end namespace llvm
 
 #endif // LLVM_DEBUGINFO_DWARF_DWARFCONTEXT_H

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

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() const { return getHeader().getDWOId(); }
+  Optional<uint64_t> getDWOId() {
+    extractDIEsIfNeeded(/*CUDieOnly*/ true);
+    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

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

 
 Optional<uint64_t> DWARFDebugNames::Entry::getCUOffset() const {
   Optional<uint64_t> Index = getCUIndex();
   if (!Index || *Index >= NameIdx->getCUCount())
     return None;
   return NameIdx->getCUOffset(*Index);
 }
 
-Optional<uint64_t> DWARFDebugNames::Entry::getDIESectionOffset() const {
-  Optional<uint64_t> CUOff = getCUOffset();
-  Optional<uint64_t> DIEOff = getDIEUnitOffset();
-  if (CUOff && DIEOff)
-    return *CUOff + *DIEOff;
-  return None;
-}
-
 void DWARFDebugNames::Entry::dump(ScopedPrinter &W) const {
   W.printHex("Abbrev", Abbr->Code);
   W.startLine() << formatv("Tag: {0}\n", Abbr->Tag);
   assert(Abbr->Attributes.size() == Values.size());
   for (const auto &Tuple : zip_first(Abbr->Attributes, Values)) {
     W.startLine() << formatv("{0}: ", std::get<0>(Tuple).Index);
     std::get<1>(Tuple).dump(W.getOStream());
     W.getOStream() << '\n';

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

for (; EntryOr; ++NumEntries, EntryID = NextEntryID,
     if (CUIndex > NI.getCUCount()) {
       error() << formatv("Name Index @ {0:x}: Entry @ {1:x} contains an "
                          "invalid CU index ({2}).\n",
                          NI.getUnitOffset(), EntryID, CUIndex);
       ++NumErrors;
       continue;
     }
     uint32_t CUOffset = NI.getCUOffset(CUIndex);
-    uint64_t DIEOffset = *EntryOr->getDIESectionOffset();
+    uint64_t DIEOffset = CUOffset + *EntryOr->getDIEUnitOffset();
     DWARFDie DIE = DCtx.getDIEForOffset(DIEOffset);
     if (!DIE) {
       error() << formatv("Name Index @ {0:x}: Entry @ {1:x} references a "
                          "non-existing DIE @ {2:x}.\n",
                          NI.getUnitOffset(), EntryID, DIEOffset);
       ++NumErrors;
       continue;
     }
unsigned DWARFVerifier::verifyNameIndexCompleteness(
 
   default:
     break;
   }
 
   // Now we know that our Die should be present in the Index. Let's check if
   // that's the case.
   unsigned NumErrors = 0;
+  uint64_t DieUnitOffset = Die.getOffset() - Die.getDwarfUnit()->getOffset();
   for (StringRef Name : EntryNames) {
-    if (none_of(NI.equal_range(Name), [&Die](const DWARFDebugNames::Entry &E) {
-          return E.getDIESectionOffset() == uint64_t(Die.getOffset());
+    if (none_of(NI.equal_range(Name), [&](const DWARFDebugNames::Entry &E) {
+          return E.getDIEUnitOffset() == DieUnitOffset;
         })) {
       error() << formatv("Name Index @ {0:x}: Entry for DIE @ {1:x} ({2}) with "
                          "name {3} missing.\n",
                          NI.getUnitOffset(), Die.getOffset(), Die.getTag(),
                          Name);
       ++NumErrors;
     }
   }

llvm/trunk/test/tools/llvm-dwarfdump/X86/debug-names-find-dwo.s

+# RUN: llvm-mc -triple x86_64-pc-linux %s -filetype=obj | \
+# RUN:   llvm-dwarfdump -find=foobar - | FileCheck %s
+
+# CHECK: DW_TAG_variable
+# CHECK-NEXT: DW_AT_name ("foobar")
+
+  .text
+  .file "<stdin>"
+  .file 1 "/tmp/cu1.c"
+  .type foobar,@object          # @foobar
+  .comm foobar,8,8
+  .section  .debug_str,"MS",@progbits,1
+.Lskel_string0:
+  .asciz  "foo.dwo"               # string offset=0
+.Lskel_string1:
+  .asciz  "/tmp"                  # string offset=8
+.Lskel_string2:
+  .asciz  "foobar"                # string offset=13
+  .section  .debug_loc.dwo,"",@progbits
+  .section  .debug_abbrev,"",@progbits
+  .byte 1                       # Abbreviation Code
+  .byte 17                      # DW_TAG_compile_unit
+  .byte 0                       # DW_CHILDREN_no
+  .byte 16                      # DW_AT_stmt_list
+  .byte 23                      # DW_FORM_sec_offset
+  .ascii  "\260B"                 # DW_AT_GNU_dwo_name
+  .byte 14                      # DW_FORM_strp
+  .byte 27                      # DW_AT_comp_dir
+  .byte 14                      # DW_FORM_strp
+  .ascii  "\261B"                 # DW_AT_GNU_dwo_id
+  .byte 7                       # DW_FORM_data8
+  .ascii  "\263B"                 # DW_AT_GNU_addr_base
+  .byte 23                      # DW_FORM_sec_offset
+  .byte 0                       # EOM(1)
+  .byte 0                       # EOM(2)
+  .byte 0                       # EOM(3)
+  .section  .debug_info,"",@progbits
+.Lcu_begin0:
+  .long 32                      # Length of Unit
+  .short  4                       # DWARF version number
+  .long .debug_abbrev           # Offset Into Abbrev. Section
+  .byte 8                       # Address Size (in bytes)
+  .byte 1                       # Abbrev [1] 0xb:0x19 DW_TAG_compile_unit
+  .long 0                       # DW_AT_stmt_list
+  .long .Lskel_string0          # DW_AT_GNU_dwo_name
+  .long .Lskel_string1          # DW_AT_comp_dir
+  .quad -1328675031687321003    # DW_AT_GNU_dwo_id
+  .long .debug_addr             # DW_AT_GNU_addr_base
+  .section  .debug_ranges,"",@progbits
+  .section  .debug_macinfo,"",@progbits
+  .byte 0                       # End Of Macro List Mark
+  .section  .debug_str.dwo,"MS",@progbits,1
+.Linfo_string0:
+  .asciz  "foo.dwo"               # string offset=0
+.Linfo_string1:
+  .asciz  "clang version 7.0.0 (trunk 325496) (llvm/trunk 325732)" # string offset=8
+.Linfo_string2:
+  .asciz  "/tmp/cu1.c"            # string offset=63
+.Linfo_string3:
+  .asciz  "foobar"                # string offset=74
+  .section  .debug_str_offsets.dwo,"",@progbits
+  .long 0
+  .long 8
+  .long 63
+  .long 74
+  .section  .debug_info.dwo,"",@progbits
+  .long 34                      # Length of Unit
+  .short  4                       # DWARF version number
+  .long 0                       # Offset Into Abbrev. Section
+  .byte 8                       # Address Size (in bytes)
+  .byte 1                       # Abbrev [1] 0xb:0x1b DW_TAG_compile_unit
+  .byte 0                       # DW_AT_GNU_dwo_name
+  .byte 1                       # DW_AT_producer
+  .short  12                      # DW_AT_language
+  .byte 2                       # DW_AT_name
+  .quad -1328675031687321003    # DW_AT_GNU_dwo_id
+  .byte 2                       # Abbrev [2] 0x19:0xb DW_TAG_variable
+  .byte 3                       # DW_AT_name
+  .long 36                      # DW_AT_type
+                                        # DW_AT_external
+  .byte 1                       # DW_AT_decl_file
+  .byte 1                       # DW_AT_decl_line
+  .byte 2                       # DW_AT_location
+  .byte 251
+  .byte 0
+  .byte 3                       # Abbrev [3] 0x24:0x1 DW_TAG_pointer_type
+  .byte 0                       # End Of Children Mark
+  .section  .debug_abbrev.dwo,"",@progbits
+  .byte 1                       # Abbreviation Code
+  .byte 17                      # DW_TAG_compile_unit
+  .byte 1                       # DW_CHILDREN_yes
+  .ascii  "\260B"                 # DW_AT_GNU_dwo_name
+  .ascii  "\202>"                 # DW_FORM_GNU_str_index
+  .byte 37                      # DW_AT_producer
+  .ascii  "\202>"                 # DW_FORM_GNU_str_index
+  .byte 19                      # DW_AT_language
+  .byte 5                       # DW_FORM_data2
+  .byte 3                       # DW_AT_name
+  .ascii  "\202>"                 # DW_FORM_GNU_str_index
+  .ascii  "\261B"                 # DW_AT_GNU_dwo_id
+  .byte 7                       # DW_FORM_data8
+  .byte 0                       # EOM(1)
+  .byte 0                       # EOM(2)
+  .byte 2                       # Abbreviation Code
+  .byte 52                      # DW_TAG_variable
+  .byte 0                       # DW_CHILDREN_no
+  .byte 3                       # DW_AT_name
+  .ascii  "\202>"                 # DW_FORM_GNU_str_index
+  .byte 73                      # DW_AT_type
+  .byte 19                      # DW_FORM_ref4
+  .byte 63                      # DW_AT_external
+  .byte 25                      # DW_FORM_flag_present
+  .byte 58                      # DW_AT_decl_file
+  .byte 11                      # DW_FORM_data1
+  .byte 59                      # DW_AT_decl_line
+  .byte 11                      # DW_FORM_data1
+  .byte 2                       # DW_AT_location
+  .byte 24                      # DW_FORM_exprloc
+  .byte 0                       # EOM(1)
+  .byte 0                       # EOM(2)
+  .byte 3                       # Abbreviation Code
+  .byte 15                      # DW_TAG_pointer_type
+  .byte 0                       # DW_CHILDREN_no
+  .byte 0                       # EOM(1)
+  .byte 0                       # EOM(2)
+  .byte 0                       # EOM(3)
+  .section  .debug_addr,"",@progbits
+  .quad foobar
+  .section  .debug_names,"",@progbits
+  .long .Lnames_end0-.Lnames_start0 # Header: unit length
+.Lnames_start0:
+  .short  5                       # Header: version
+  .short  0                       # Header: padding
+  .long 1                       # Header: compilation unit count
+  .long 0                       # Header: local type unit count
+  .long 0                       # Header: foreign type unit count
+  .long 1                       # Header: bucket count
+  .long 1                       # Header: name count
+  .long .Lnames_abbrev_end0-.Lnames_abbrev_start0 # Header: abbreviation table size
+  .long 8                       # Header: augmentation string size
+  .ascii  "LLVM0700"              # Header: augmentation string
+  .long .Lcu_begin0             # Compilation unit 0
+  .long 1                       # Bucket 0
+  .long -35364674               # Hash in Bucket 0
+  .long .Lskel_string2          # String in Bucket 0: foobar
+  .long .Lnames0-.Lnames_entries0 # Offset in Bucket 0
+.Lnames_abbrev_start0:
+  .byte 52                      # Abbrev code
+  .byte 52                      # DW_TAG_variable
+  .byte 3                       # DW_IDX_die_offset
+  .byte 19                      # DW_FORM_ref4
+  .byte 0                       # End of abbrev
+  .byte 0                       # End of abbrev
+  .byte 0                       # End of abbrev list
+.Lnames_abbrev_end0:
+.Lnames_entries0:
+.Lnames0:
+  .byte 52                      # Abbreviation code
+  .long 25                      # DW_IDX_die_offset
+  .long 0                       # End of list: foobar
+  .p2align  2
+.Lnames_end0:

llvm/trunk/tools/llvm-dwarfdump/llvm-dwarfdump.cpp

for (const auto &Entry : CU->dies()) {
         // Match full text.
         else if (Names.count(Name))
           Die.dump(OS, 0, getDumpOpts());
       }
     }
 
 }
 
-template <typename AccelTable>
-static void getDIEOffset(const AccelTable &Accel, StringRef Name,
-                         SmallVectorImpl<uint64_t> &Offsets) {
-  for (const auto &Entry : Accel.equal_range(Name))
-    if (llvm::Optional<uint64_t> Off = Entry.getDIESectionOffset())
-      Offsets.push_back(*Off);
+static void getDies(DWARFContext &DICtx, const AppleAcceleratorTable &Accel,
+                    StringRef Name, SmallVectorImpl<DWARFDie> &Dies) {
+  for (const auto &Entry : Accel.equal_range(Name)) {
+    if (llvm::Optional<uint64_t> Off = Entry.getDIESectionOffset()) {
+      if (DWARFDie Die = DICtx.getDIEForOffset(*Off))
+        Dies.push_back(Die);
+    }
+  }
+}
+
+static DWARFDie toDie(const DWARFDebugNames::Entry &Entry,
+                      DWARFContext &DICtx) {
+  llvm::Optional<uint64_t> CUOff = Entry.getCUOffset();
+  llvm::Optional<uint64_t> Off = Entry.getDIEUnitOffset();
+  if (!CUOff || !Off)
+    return DWARFDie();
+
+  DWARFCompileUnit *CU = DICtx.getCompileUnitForOffset(*CUOff);
+  if (!CU)
+    return DWARFDie();
+
+  if (Optional<uint64_t> DWOId = CU->getDWOId()) {
+    // This is a skeleton unit. Look up the DIE in the DWO unit.
+    CU = DICtx.getDWOCompileUnitForHash(*DWOId);
+    if (!CU)
+      return DWARFDie();
+  }
+
+  return CU->getDIEForOffset(CU->getOffset() + *Off);
+}
+
+static void getDies(DWARFContext &DICtx, const DWARFDebugNames &Accel,
+                    StringRef Name, SmallVectorImpl<DWARFDie> &Dies) {
+  for (const auto &Entry : Accel.equal_range(Name)) {
+    if (DWARFDie Die = toDie(Entry, DICtx))
+      Dies.push_back(Die);
+  }
 }
 
 /// Print only DIEs that have a certain name.
 static void filterByAccelName(ArrayRef<std::string> Names, DWARFContext &DICtx,
                               raw_ostream &OS) {
-  SmallVector<uint64_t, 4> Offsets;
+  SmallVector<DWARFDie, 4> Dies;
   for (const auto &Name : Names) {
-    getDIEOffset(DICtx.getAppleNames(), Name, Offsets);
-    getDIEOffset(DICtx.getAppleTypes(), Name, Offsets);
-    getDIEOffset(DICtx.getAppleNamespaces(), Name, Offsets);
-    getDIEOffset(DICtx.getDebugNames(), Name, Offsets);
+    getDies(DICtx, DICtx.getAppleNames(), Name, Dies);
+    getDies(DICtx, DICtx.getAppleTypes(), Name, Dies);
+    getDies(DICtx, DICtx.getAppleNamespaces(), Name, Dies);
+    getDies(DICtx, DICtx.getDebugNames(), Name, Dies);
   }
-  llvm::sort(Offsets.begin(), Offsets.end());
-  Offsets.erase(std::unique(Offsets.begin(), Offsets.end()), Offsets.end());
+  llvm::sort(Dies.begin(), Dies.end());
+  Dies.erase(std::unique(Dies.begin(), Dies.end()), Dies.end());
 
-  for (uint64_t Off: Offsets) {
-    DWARFDie Die = DICtx.getDIEForOffset(Off);
+  for (DWARFDie Die : Dies)
     Die.dump(OS, 0, getDumpOpts());
-  }
-}
 
 /// Handle the --lookup option and dump the DIEs and line info for the given
 /// address.
 static bool lookup(DWARFContext &DICtx, uint64_t Address, raw_ostream &OS) {
   auto DIEsForAddr = DICtx.getDIEsForAddress(Lookup);
 
   if (!DIEsForAddr)
     return false;