[lldb] Improve invalid DWARF DW_AT_ranges error reporting

[jankratochvil]

Summary

In D98289#inline-939112 @dblaikie said:

Perhaps this could be more informative about what makes the range list index of 0 invalid? "index 0 out of range of range list table (with range list base 0xXXX) with offset entry count of XX (valid indexes 0-(XX-1))" Maybe that's too verbose/not worth worrying about since this'll only be relevant to DWARF producers trying to debug their DWARFv5, maybe no one will ever see this message in practice. Just a thought.

If this is OK I will need to submit the LLVM part separately.

[dblaikie]

Looks good to me - at least includes more values from the file to point to the problem. Though phrasing might be better (I guess some other code prints the "DW_AT_ranges(0x0) attribute" part, which to me seems a bit weird (doesn't include the form, which might be useful for instance) and the "00000000: " prefix on the "DW_FORM_rnglistx cannot be used" part is maybe a bit of a strange way to print the CU offset (nothing in the diagnostic says what this number is) - maybe the higher level error handling could print these high level details out as a suffix ("Failure in CU that begins at offset <blah>, in DIE at offset <blah>, at attribute DW_AT_*, encoded with DW_FORM_*, with value blah").

But probably not a big deal - if someone's getting sufficiently bogus DWARF to reach this, they can probably figure it out. Maybe other LLDB folks have some ideas about how best to structure the error handling more generally in this regard.

[jankratochvil]

Is the message format OK now?

[dblaikie]

Yeah, looks pretty good!

lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp

static DWARFRangeList GetRangesOrReportError(DWARFUnit &unit,
                                              const DWARFFormValue &value) {
   llvm::Expected<DWARFRangeList> expected_ranges =
       (value.Form() == DW_FORM_rnglistx)
           ? unit.FindRnglistFromIndex(value.Unsigned())
           : unit.FindRnglistFromOffset(value.Unsigned());
   if (expected_ranges)
     return std::move(*expected_ranges);
   unit.GetSymbolFileDWARF().GetObjectFile()->GetModule()->ReportError(
-      "{0x%8.8x}: DIE has DW_AT_ranges(0x%" PRIx64 ") attribute, but "
+      "{0x%8.8x}: DIE has DW_AT_ranges(%s 0x%" PRIx64 ") attribute, but "
       "range extraction failed (%s), please file a bug "
       "and attach the file at the start of this error message",
-      die.GetOffset(), value.Unsigned(),
-      toString(expected_ranges.takeError()).c_str());
+      die.GetOffset(),
+      llvm::dwarf::FormEncodingString(value.Form()).str().c_str(),
+      value.Unsigned(), toString(expected_ranges.takeError()).c_str());
   return DWARFRangeList();
 }
 
 // GetDIENamesAndRanges
 //
 // Gets the valid address ranges for a given DIE by looking for a
 // DW_AT_low_pc/DW_AT_high_pc pair, DW_AT_entry_pc, or DW_AT_ranges attributes.
 bool DWARFDebugInfoEntry::GetDIENamesAndRanges(

lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h

public:
   /// Return a list 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::Expected<DWARFRangeList> FindRnglistFromIndex(uint32_t index);
 
   /// Return a rangelist's offset based on an index. The index designates
   /// an entry in the rangelist table's offset array and is supplied by
   /// DW_FORM_rnglistx.
-  llvm::Optional<uint64_t> GetRnglistOffset(uint32_t Index);
+  llvm::Expected<uint64_t> GetRnglistOffset(uint32_t Index);
 
   llvm::Optional<uint64_t> GetLoclistOffset(uint32_t Index) {
     if (!m_loclist_table_header)
       return llvm::None;
 
     llvm::Optional<uint64_t> Offset = m_loclist_table_header->getOffsetEntry(
         m_dwarf.GetDWARFContext().getOrLoadLocListsData().GetAsLLVM(), Index);
     if (!Offset)

lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp

else
       GetSymbolFileDWARF().GetObjectFile()->GetModule()->ReportError(
           "Failed to extract range list table at offset 0x%" PRIx64 ": %s",
           m_ranges_base, toString(table_or_error.takeError()).c_str());
   }
   return m_rnglist_table;
 }
 
 // This function is called only for DW_FORM_rnglistx.
-llvm::Optional<uint64_t> DWARFUnit::GetRnglistOffset(uint32_t Index) {
+llvm::Expected<uint64_t> DWARFUnit::GetRnglistOffset(uint32_t Index) {
   if (!GetRnglist())
-    return llvm::None;
-  if (!m_ranges_base) {
-    GetSymbolFileDWARF().GetObjectFile()->GetModule()->ReportError(
-        "%8.8x: DW_FORM_rnglistx cannot be used without DW_AT_rnglists_base",
+    return llvm::createStringError(errc::invalid_argument,
+                                   "missing or invalid range list table");
+  if (!m_ranges_base)
+    return llvm::createStringError(errc::invalid_argument,
+                                   "DW_FORM_rnglistx cannot be used without "
+                                   "DW_AT_rnglists_base for CU at 0x%8.8x",
                                    GetOffset());
-    return llvm::None;
-  }
   if (llvm::Optional<uint64_t> off = GetRnglist()->getOffsetEntry(
           m_dwarf.GetDWARFContext().getOrLoadRngListsData().GetAsLLVM(), Index))
     return *off + m_ranges_base;
-  return llvm::None;
+  return llvm::createStringError(
+      errc::invalid_argument,
+      "invalid range list table index %u; OffsetEntryCount is %u, "
+      "DW_AT_rnglists_base is %" PRIu64,
+      Index, GetRnglist()->getOffsetEntryCount(), m_ranges_base);
 }
 
 void DWARFUnit::SetStrOffsetsBase(dw_offset_t str_offsets_base) {
   m_str_offsets_base = str_offsets_base;
 }
 
 // It may be called only with m_die_array_mutex held R/W.
 void DWARFUnit::ClearDIEsRWLocked() {
for (const llvm::DWARFAddressRange &llvm_range : *llvm_ranges) {
     ranges.Append(DWARFRangeList::Entry(llvm_range.LowPC,
                                         llvm_range.HighPC - llvm_range.LowPC));
   }
   return ranges;
 }
 
 llvm::Expected<DWARFRangeList>
 DWARFUnit::FindRnglistFromIndex(uint32_t index) {
-  if (llvm::Optional<uint64_t> offset = GetRnglistOffset(index))
-    return FindRnglistFromOffset(*offset);
-  if (GetRnglist())
-    return llvm::createStringError(errc::invalid_argument,
-                                   "invalid range list table index %d", index);
-
-  return llvm::createStringError(errc::invalid_argument,
-                                 "missing or invalid range list table");
+  llvm::Expected<uint64_t> maybe_offset = GetRnglistOffset(index);
+  if (!maybe_offset)
+    return maybe_offset.takeError();
+  return FindRnglistFromOffset(*maybe_offset);
 }

lldb/test/Shell/SymbolFile/DWARF/DW_AT_range-DW_FORM_sec_offset.s

 # CHECK-NEXT:       id = {0x0000003f}, range = [0x00000001-0x00000002)
 
 # RUN: llvm-mc -triple=x86_64-pc-linux -filetype=obj \
 # RUN:   --defsym RNGLISTX=0 %s > %t-rnglistx
 # RUN: %lldb %t-rnglistx -o "image lookup -v -s lookup_rnglists" \
 # RUN:   -o exit 2>&1 | FileCheck --check-prefix=RNGLISTX %s
 
 # RNGLISTX-LABEL: image lookup -v -s lookup_rnglists
-# RNGLISTX: error: {{.*}} 00000000: DW_FORM_rnglistx cannot be used without DW_AT_rnglists_base
+# RNGLISTX: error: {{.*}} {0x0000003f}: DIE has DW_AT_ranges(DW_FORM_rnglistx 0x0) attribute, but range extraction failed (DW_FORM_rnglistx cannot be used without DW_AT_rnglists_base for CU at 0x00000000), please file a bug and attach the file at the start of this error message
 
 # RUN: llvm-mc -triple=x86_64-pc-linux -filetype=obj \
 # RUN:   --defsym RNGLISTX=0 --defsym RNGLISTBASE=0 %s > %t-rnglistbase
 # RUN: %lldb %t-rnglistbase -o "image lookup -v -s lookup_rnglists" \
 # RUN:   -o exit 2>&1 | FileCheck --check-prefix=RNGLISTBASE %s
 
 # RNGLISTBASE-LABEL: image lookup -v -s lookup_rnglists
-# RNGLISTBASE: error: {{.*}}-rnglistbase {0x00000043}: DIE has DW_AT_ranges(0x0) attribute, but range extraction failed (invalid range list table index 0), please file a bug and attach the file at the start of this error message
+# RNGLISTBASE: error: {{.*}}-rnglistbase {0x00000043}: DIE has DW_AT_ranges(DW_FORM_rnglistx 0x0) attribute, but range extraction failed (invalid range list table index 0; OffsetEntryCount is 0, DW_AT_rnglists_base is 12), please file a bug and attach the file at the start of this error message
 
         .text
 rnglists:
         nop
 .Lblock1_begin:
 lookup_rnglists:
         nop
 .Lblock1_end:

lldb/test/Shell/SymbolFile/DWARF/x86/debug_ranges-missing-section.s

 # RUN: llvm-mc -triple=x86_64-pc-linux -filetype=obj %s > %t
 # RUN: %lldb %t -o "image lookup -v -s lookup_ranges" -o exit 2>&1 | FileCheck %s
 
-# CHECK: DIE has DW_AT_ranges(0x47) attribute, but range extraction failed (No debug_ranges section),
+# CHECK: DIE has DW_AT_ranges(DW_FORM_sec_offset 0x47) attribute, but range extraction failed (No debug_ranges section),
 # CHECK:  Function: id = {0x0000001c}, name = "ranges", range = [0x0000000000000000-0x0000000000000004)
 # CHECK:    Blocks: id = {0x0000001c}, range = [0x00000000-0x00000004)
 
         .text
         .p2align 12
         .globl  ranges
         .type   ranges,@function
 ranges:                                    # @ranges

llvm/include/llvm/DebugInfo/DWARF/DWARFListTable.h

public:
 
   void clear() {
     HeaderData = {};
   }
   uint64_t getHeaderOffset() const { return HeaderOffset; }
   uint8_t getAddrSize() const { return HeaderData.AddrSize; }
   uint64_t getLength() const { return HeaderData.Length; }
   uint16_t getVersion() const { return HeaderData.Version; }
+  uint32_t getOffsetEntryCount() const { return HeaderData.OffsetEntryCount; }
   StringRef getSectionName() const { return SectionName; }
   StringRef getListTypeString() const { return ListTypeString; }
   dwarf::DwarfFormat getFormat() const { return Format; }
 
   /// Return the size of the table header including the length but not including
   /// the offsets.
   static uint8_t getHeaderSize(dwarf::DwarfFormat Format) {
     switch (Format) {
public:
   Error extract(DWARFDataExtractor Data, uint64_t *OffsetPtr);
   /// Look up a list based on a given offset. Extract it and enter it into the
   /// list map if necessary.
   Expected<DWARFListType> findList(DWARFDataExtractor Data,
                                    uint64_t Offset) const;
 
   uint64_t getHeaderOffset() const { return Header.getHeaderOffset(); }
   uint8_t getAddrSize() const { return Header.getAddrSize(); }
+  uint32_t getOffsetEntryCount() const { return Header.getOffsetEntryCount(); }
   dwarf::DwarfFormat getFormat() const { return Header.getFormat(); }
 
   void dump(DWARFDataExtractor Data, raw_ostream &OS,
             llvm::function_ref<Optional<object::SectionedAddress>(uint32_t)>
                 LookupPooledAddress,
             DIDumpOptions DumpOpts = {}) const;
 
   /// Return the contents of the offset entry designated by a given index.