[lldb] Improve error handling around GetRngListData()

[kimanh]

Summary

This adds more informative error handling around the
usage of GetRngListData.

[kimanh]

I've split up the other CL (https://reviews.llvm.org/D107456) into the part that was reviewed, and this: the error handling that was requested on that CL. Please take a look whenever you have time!

[shafik]

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

522

If I am reading this correctly, it looks like this will only be set on the non-error case which will leave contribution_off in the caller uninitialized in the cases you care about logging.

[kimanh]

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

522

Sorry for the very late reply (I had a long vacation)! And thanks for the comment.

If GetRnglistData didn't work, the error will be thrown below (line 526) in here. However, if it is successful, but another problem occurs (callers of GetRnglistData throw an error) the Offset will be initialized, and it can be logged. Example: ParseListTableHeader using it in ll. 548. Does that make sense?

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

const DWARFDebugInfoEntry *DIEPtr() {
     ExtractDIEsIfNeeded();
     if (m_die_array.empty())
       return NULL;
     return &m_die_array[0];
   }
 
   const llvm::Optional<llvm::DWARFDebugRnglistTable> &GetRnglistTable();
 
-  lldb_private::DWARFDataExtractor GetRnglistData() const;
+  lldb_private::DWARFDataExtractor GetRnglistData(uint32_t &Offset) const;
 
   SymbolFileDWARF &m_dwarf;
   std::shared_ptr<DWARFUnit> m_dwo;
   DWARFUnitHeader m_header;
   const DWARFAbbreviationDeclarationSet *m_abbrevs = nullptr;
   void *m_user_data = nullptr;
   // The compile unit debug information entry item
   DWARFDebugInfoEntry::collection m_die_array;

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

if (const auto *contribution = entry->getContribution(
             GetVersion() >= 5 ? llvm::DW_SECT_LOCLISTS : llvm::DW_SECT_EXT_LOC))
       return DWARFDataExtractor(data, contribution->Offset,
                                 contribution->Length);
     return DWARFDataExtractor();
   }
   return data;
 }
 
-DWARFDataExtractor DWARFUnit::GetRnglistData() const {
+DWARFDataExtractor DWARFUnit::GetRnglistData(uint32_t &Offset) const {
   DWARFContext &Ctx = GetSymbolFileDWARF().GetDWARFContext();
   const DWARFDataExtractor &data = Ctx.getOrLoadRngListsData();
   if (const llvm::DWARFUnitIndex::Entry *entry = m_header.GetIndexEntry()) {
     if (const auto *contribution =
-            entry->getContribution(llvm::DW_SECT_RNGLISTS))
+            entry->getContribution(llvm::DW_SECT_RNGLISTS)) {
+      Offset = contribution->Offset;
       return DWARFDataExtractor(data, contribution->Offset,
                                 contribution->Length);
+    }
     GetSymbolFileDWARF().GetObjectFile()->GetModule()->ReportError(
         "Failed to find range list contribution for CU with signature "
         "0x%" PRIx64,
         entry->getSignature());
 
     return DWARFDataExtractor();
   }
   return data;
 }
 
 void DWARFUnit::SetRangesBase(dw_addr_t ranges_base) {
   lldbassert(!m_rnglist_table_done);
 
   m_ranges_base = ranges_base;
 }
 
 const llvm::Optional<llvm::DWARFDebugRnglistTable> &
 DWARFUnit::GetRnglistTable() {
   if (GetVersion() >= 5 && !m_rnglist_table_done) {
     m_rnglist_table_done = true;
+    uint32_t contribution_off;
     if (auto table_or_error =
             ParseListTableHeader<llvm::DWARFDebugRnglistTable>(
-                GetRnglistData().GetAsLLVM(), m_ranges_base, DWARF32))
+                GetRnglistData(contribution_off).GetAsLLVM(), m_ranges_base,
+                DWARF32))
       m_rnglist_table = std::move(table_or_error.get());
     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());
+          "Failed to extract range list table at offset 0x%" PRIx64
+          " (DW_AT_rnglists_base is %" PRIu64 "): %s",
+          m_ranges_base + contribution_off, m_ranges_base,
+          toString(table_or_error.takeError()).c_str());
   }
   return m_rnglist_table;
 }
 
 // This function is called only for DW_FORM_rnglistx.
 llvm::Expected<uint64_t> DWARFUnit::GetRnglistOffset(uint32_t Index) {
   if (!GetRnglistTable())
     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());
+  uint32_t contribution_off;
   if (llvm::Optional<uint64_t> off = GetRnglistTable()->getOffsetEntry(
-          GetRnglistData().GetAsLLVM(), Index))
+          GetRnglistData(contribution_off).GetAsLLVM(), Index))
     return *off + m_ranges_base;
   return llvm::createStringError(
       errc::invalid_argument,
       "invalid range list table index %u; OffsetEntryCount is %u, "
-      "DW_AT_rnglists_base is %" PRIu64,
-      Index, GetRnglistTable()->getOffsetEntryCount(), m_ranges_base);
+      "DW_AT_rnglists_base is %" PRIu64
+      ", offset to range list table is 0x%" PRIx64,
+      Index, GetRnglistTable()->getOffsetEntryCount(), m_ranges_base,
+      m_ranges_base + contribution_off);
 }
 
 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() {
if (GetVersion() <= 4) {
     debug_ranges->FindRanges(this, offset, ranges);
     return ranges;
   }
 
   if (!GetRnglistTable())
     return llvm::createStringError(errc::invalid_argument,
                                    "missing or invalid range list table");
 
-  llvm::DWARFDataExtractor data = GetRnglistData().GetAsLLVM();
+  uint32_t contribution_offset;
+  llvm::DWARFDataExtractor data =
+      GetRnglistData(contribution_offset).GetAsLLVM();
 
   // As DW_AT_rnglists_base may be missing we need to call setAddressSize.
   data.setAddressSize(m_header.GetAddressByteSize());
   auto range_list_or_error = GetRnglistTable()->findList(data, offset);
   if (!range_list_or_error)
     return range_list_or_error.takeError();
 
   llvm::Expected<llvm::DWARFAddressRangesVector> llvm_ranges =

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

 # 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(DW_FORM_rnglistx 0x0) attribute, but range extraction failed (invalid range list table index 0; OffsetEntryCount is 0, DW_AT_rnglists_base is 24), please file a bug and attach the file at the start of this error message
+# RNGLISTBASE: error: DW_AT_range-DW_FORM_sec_offset.s.tmp-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 24, offset to range list table is 0x18), 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: