[lldb] Fix#2 of DW_AT_ranges DW_FORM_sec_offset not using DW_AT_rnglists_base (used by GCC)

[jankratochvil]

Summary

Fix D98289 so that it works even for 2nd..nth compilation unit (.debug_rnglists).

[ikudrin]

As far as I understand it, you need a specially constructed llvm::DWARFDebugRnglistTable object so that DWARFUnit::FindRnglistFromOffset() can call its findList() method and get a list of records located at a specific offset. It looks like a newly created llvm::DWARFDebugRnglistTable object has its Header.Length set to 0, so it already works as required for the usage, and there is no need to fill its fields with artificial values that do not represent real content of the section. Am I right?

llvm/lib/DebugInfo/DWARF/DWARFListTable.cpp

54

The line looks suspicious because Data is a local variable that is destroyed right after the statement.

[jankratochvil]

In D106466#2922549, @ikudrin wrote:

As far as I understand it, you need a specially constructed llvm::DWARFDebugRnglistTable object so that DWARFUnit::FindRnglistFromOffset() can call its findList() method and get a list of records located at a specific offset.

Yes.

It looks like a newly created llvm::DWARFDebugRnglistTable object has its Header.Length set to 0,

But that is wrong Length, the correct one is to cover the whole section which I set by:

HeaderData.Length = Data.size() - dwarf::getUnitLengthFieldByteSize(Format);

so it already works as required for the usage, and there is no need to fill its fields with artificial values that do not represent real content of the section. Am I right?

One needs to set at least AddrSize and OffsetEntryCount as callers do use it. Not sure now with the other fields but in such case one should remove them also from existing DWARFListTableHeader::extract. DWARFListTableHeader::create should be as much similar to DWARFListTableHeader::extract as possible. Maybe we could make some [nfc] patches to make DWARFListTableHeader::Header smaller but it is now only 16 bytes already so I do not find that much useful. It would be also orthogonal to this patch.

llvm/lib/DebugInfo/DWARF/DWARFListTable.cpp

54

You are right. Although existing DWARFListTableHeader::extract had the same problem from where I did copy-paste it. Filed for it D107470.

[ikudrin]

In D106466#2926185, @jankratochvil wrote:

In D106466#2922549, @ikudrin wrote:

As far as I understand it, you need a specially constructed llvm::DWARFDebugRnglistTable object so that DWARFUnit::FindRnglistFromOffset() can call its findList() method and get a list of records located at a specific offset.

Yes.

It looks like a newly created llvm::DWARFDebugRnglistTable object has its Header.Length set to 0,

But that is wrong Length, the correct one is to cover the whole section which I set by:

HeaderData.Length = Data.size() - dwarf::getUnitLengthFieldByteSize(Format);

This value is not incorrect, but a special one, with custom handling. DWARFUnit::findRnglistFromOffset(uint64_t Offset) fetches the list without creating a fake header.

so it already works as required for the usage, and there is no need to fill its fields with artificial values that do not represent real content of the section. Am I right?

One needs to set at least AddrSize and OffsetEntryCount as callers do use it.

Could you please point me to that usage? I am not that familiar with the LLDB code.

[jankratochvil]

In D106466#2939551, @ikudrin wrote:

In D106466#2926185, @jankratochvil wrote:

One needs to set at least AddrSize and OffsetEntryCount as callers do use it.

Could you please point me to that usage? I am not that familiar with the LLDB code.

Those are used by DWARFUnit::GetRnglistOffset->llvm::DWARFDebugRnglistTable::getOffsetEntry. But in such case llvm::DWARFDebugRnglistTable::extractHeaderAndOffsets were already called that time.
So that is not the code path I need for DWARF not using DW_AT_rnglists_base.

I understand now the Header.length==0 case so the fix is simple now, thanks for showing it to me.

[ikudrin]

The code looks good, but please improve the comments and wait for approval from a more LLDB-knowledgeable person than me,

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

437

Please extend the comment to emphasize that even if DW_AT_rnglists_base is missing and DW_FORM_rnglistx cannot be handled, returning a default-constructed Table allows DW_FORM_sec_offset to be supported.

999

This comment is quite misleading as it references offset which is the argument of the method.

[jankratochvil]

@dblaikie do you think it is OK for check-in now?

[dblaikie]

I assume there's already test coverage for rnglistx in debug_info.dwo/split unit? (because in that case there's no rnglists_base, but rnglistx is usable) - could you explain how this code avoids treating the split unit rnglists_base == 0 case as "there is no rnglists_base and so rnglistx isn't usable"?

[jankratochvil]

In D106466#2947710, @dblaikie wrote:

I assume there's already test coverage for rnglistx in debug_info.dwo/split unit? (because in that case there's no rnglists_base, but rnglistx is usable)

I admit I did not check it, thanks for catching it. But it is already tested by lldb/test/Shell/SymbolFile/DWARF/x86/debug_rnglists-dwo.s implemented by Pavel Labath in 2019. It is using DWO DW_TAG_lexical_block->DW_AT_ranges->DW_FORM_rnglistx.

• could you explain how this code avoids treating the split unit rnglists_base == 0 case as "there is no rnglists_base and so rnglistx isn't usable"?

m_ranges_base is not zero in such case as it has been set from the skeleton.

[dblaikie]

In D106466#2950268, @jankratochvil wrote:

In D106466#2947710, @dblaikie wrote:

I assume there's already test coverage for rnglistx in debug_info.dwo/split unit? (because in that case there's no rnglists_base, but rnglistx is usable)

I admit I did not check it, thanks for catching it. But it is already tested by lldb/test/Shell/SymbolFile/DWARF/x86/debug_rnglists-dwo.s implemented by Pavel Labath in 2019. It is using DWO DW_TAG_lexical_block->DW_AT_ranges->DW_FORM_rnglistx.

• could you explain how this code avoids treating the split unit rnglists_base == 0 case as "there is no rnglists_base and so rnglistx isn't usable"?

m_ranges_base is not zero in such case as it has been set from the skeleton.

ah, OK - was going to say that setting it from the skeleton would be incorrect (for DWARF 5, at least) - but I see that's handled a few lines down: https://github.com/llvm/llvm-project/blob/f58a642da19c64cb6ee1badb0f176a872c1d7a0a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp#L110

Looks all good to me, then.

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

protected:
   // Get all DWARF debug informration entries. Parse all DIEs if needed.
   const DWARFDebugInfoEntry *DIEPtr() {
     ExtractDIEsIfNeeded();
     if (m_die_array.empty())
       return NULL;
     return &m_die_array[0];
   }
 
+  llvm::Expected<llvm::DWARFDebugRnglistTable>
+  CreateListTableHeader(const llvm::DWARFDataExtractor &data, uint64_t offset,
+                        DwarfFormat format) const;
+
   const llvm::Optional<llvm::DWARFDebugRnglistTable> &GetRnglistTable();
 
   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

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

 void DWARFUnit::SetAddrBase(dw_addr_t addr_base) { m_addr_base = addr_base; }
 
 // Parse the rangelist table header, including the optional array of offsets
 // following it (DWARF v5 and later).
 template <typename ListTableType>
 static llvm::Expected<ListTableType>
 ParseListTableHeader(const llvm::DWARFDataExtractor &data, 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
-  // of the header.
-  if (offset > 0) {
-    uint64_t HeaderSize = llvm::DWARFListTableHeader::getHeaderSize(format);
-    if (offset < HeaderSize)
-      return llvm::createStringError(errc::invalid_argument,
-                                     "did not detect a valid"
-                                     " list table with base = 0x%" PRIx64 "\n",
-                                     offset);
-    offset -= HeaderSize;
-  }
+  // Correct Offset so that it points to the start of the header.
   ListTableType Table;
   if (llvm::Error E = Table.extractHeaderAndOffsets(data, &offset))
     return std::move(E);
   return Table;
 }
 
+llvm::Expected<llvm::DWARFDebugRnglistTable>
+DWARFUnit::CreateListTableHeader(const llvm::DWARFDataExtractor &data,
+                                 uint64_t offset, DwarfFormat format) const {
+  if (offset > 0)
+    return ParseListTableHeader<llvm::DWARFDebugRnglistTable>(data, offset,
+                                                              format);
+
+  // Unspecified Offset means only DW_FORM_sec_offset (and not DW_FORM_rnglistx)
+  // will be used. Make up a header covering the whole .debug_rnglists section.
+  llvm::DWARFDebugRnglistTable Table;
+  if (llvm::Error E = Table.createHeaderAndOffsets(
+          data, m_header.GetVersion(), m_header.GetAddressByteSize(), DWARF32))
+    return std::move(E);
+  return Table;
+}
+
 void DWARFUnit::SetLoclistsBase(dw_addr_t loclists_base) {
   m_loclists_base = loclists_base;
 
   uint64_t header_size = llvm::DWARFListTableHeader::getHeaderSize(DWARF32);
   if (loclists_base < header_size)
     return;
 
   m_loclist_table_header.emplace(".debug_loclists", "locations");
void DWARFUnit::SetRangesBase(dw_addr_t ranges_base) {
 
   m_ranges_base = ranges_base;
 }
 
 const llvm::Optional<llvm::DWARFDebugRnglistTable> &
 DWARFUnit::GetRnglistTable() {
   if (GetVersion() >= 5 && !m_rnglist_table_done) {
     m_rnglist_table_done = true;
-    if (auto table_or_error =
-            ParseListTableHeader<llvm::DWARFDebugRnglistTable>(
-                m_dwarf.GetDWARFContext().getOrLoadRngListsData().GetAsLLVM(),
-                m_ranges_base, DWARF32))
+    if (auto table_or_error = CreateListTableHeader(
       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());
   }
   return m_rnglist_table;
 }
DWARFUnit::FindRnglistFromOffset(dw_offset_t offset) {
 
   if (!GetRnglistTable())
     return llvm::createStringError(errc::invalid_argument,
                                    "missing or invalid range list table");
 
   llvm::DWARFDataExtractor data =
       m_dwarf.GetDWARFContext().getOrLoadRngListsData().GetAsLLVM();
   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 =
       range_list_or_error->getAbsoluteRanges(
           llvm::object::SectionedAddress{GetBaseAddress()},
           GetAddressByteSize(), [&](uint32_t index) {

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 12), 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 24), 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:
.Ldebug_info_start0:
         .byte   8                       # Address Size (in bytes)
         .long   .debug_abbrev           # Offset Into Abbrev. Section
         .byte   1                       # Abbrev [1] 0xc:0x5f DW_TAG_compile_unit
         .asciz  "Hand-written DWARF"    # DW_AT_producer
         .byte   0                       # DW_AT_low_pc
         .long   .Lrnglists_end-rnglists # DW_AT_high_pc
         .long   .Laddr_table_base0      # DW_AT_addr_base
 .ifdef RNGLISTBASE
-        .long   .Ldebug_ranges0         # DW_AT_rnglists_base
+        .long   .Ldebug_ranges1         # DW_AT_rnglists_base
 .endif
         .byte   2                       # Abbrev [2] 0x2b:0x37 DW_TAG_subprogram
         .quad   rnglists                # DW_AT_low_pc
         .long   .Lrnglists_end-rnglists # DW_AT_high_pc
         .asciz  "rnglists"              # DW_AT_name
         .byte   5                       # Abbrev [5] 0x52:0xf DW_TAG_lexical_block
 .ifndef RNGLISTX
-        .long   .Ldebug_ranges0         # DW_AT_ranges DW_FORM_sec_offset
+        .long   .Ldebug_ranges1         # DW_AT_ranges DW_FORM_sec_offset
 .else
         .uleb128 0                      # DW_AT_ranges DW_FORM_rnglistx
 .endif
         .byte   0                       # End Of Children Mark
         .byte   0                       # End Of Children Mark
 .Ldebug_info_end0:
 
         .section        .debug_addr,"",@progbits
         .long   .Ldebug_addr_end0-.Ldebug_addr_start0 # Length of contribution
 .Ldebug_addr_start0:
         .short  5                       # DWARF version number
         .byte   8                       # Address size
         .byte   0                       # Segment selector size
 .Laddr_table_base0:
         .quad   rnglists
 .Ldebug_addr_end0:
 
         .section        .debug_rnglists,"",@progbits
         .long   .Ldebug_rnglist_table_end0-.Ldebug_rnglist_table_start0 # Length
 .Ldebug_rnglist_table_start0:
         .short  5                       # Version
         .byte   8                       # Address size
         .byte   0                       # Segment selector size
         .long   0                       # Offset entry count
-.Ldebug_ranges0:
+.Ldebug_rnglist_table_end0:
+
+        .long   .Ldebug_rnglist_table_end1-.Ldebug_rnglist_table_start1 # Length
+.Ldebug_rnglist_table_start1:
+        .short  5                       # Version
+        .byte   8                       # Address size
+        .byte   0                       # Segment selector size
+        .long   0                       # Offset entry count
+.Ldebug_ranges1:
         .byte   4                       # DW_RLE_offset_pair
         .uleb128 .Lblock1_begin-rnglists  #   starting offset
         .uleb128 .Lblock1_end-rnglists    #   ending offset
         .byte   0                       # DW_RLE_end_of_list
-.Ldebug_rnglist_table_end0:
+.Ldebug_rnglist_table_end1:

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

static Optional<uint64_t> getOffsetEntry(DataExtractor Data,
     uint64_t Offset = OffsetTableOffset + OffsetByteSize * Index;
     auto R = Data.getUnsigned(&Offset, OffsetByteSize);
     return R;
   }
 
   /// Extract the table header and the array of offsets.
   Error extract(DWARFDataExtractor Data, uint64_t *OffsetPtr);
 
+  // Make up a header from DWARFUnit header to cover the whole Data section.
+  Error create(DWARFDataExtractor Data, unsigned Version, unsigned AddrSize,
+               dwarf::DwarfFormat FormatParam);
+
   /// Returns the length of the table, including the length field, or 0 if the
   /// length has not been determined (e.g. because the table has not yet been
   /// parsed, or there was a problem in parsing).
   uint64_t length() const;
 };
 
 /// A class representing a table of lists as specified in the DWARF v5
 /// standard for location lists and range lists. The table consists of a header
public:
   void clear() {
     Header.clear();
     ListMap.clear();
   }
   /// Extract the table header and the array of offsets.
   Error extractHeaderAndOffsets(DWARFDataExtractor Data, uint64_t *OffsetPtr) {
     return Header.extract(Data, OffsetPtr);
   }
+  // Make up a header from DWARFUnit header to cover the whole Data section.
+  Error createHeaderAndOffsets(DWARFDataExtractor Data, unsigned Version,
+                               unsigned AddrSize, dwarf::DwarfFormat Format) {
+    return Header.create(Data, Version, AddrSize, Format);
+  }
   /// Extract an entire table, including all list entries.
   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(); }

llvm/lib/DebugInfo/DWARF/DWARFListTable.cpp

return createStringError(errc::not_supported,
                              "unsupported address size %" PRIu8, AddrSize);
   if (SegSize != 0)
     return createStringError(errc::not_supported,
                              "unsupported segment selector size %" PRIu8,
                              SegSize);
   return Error::success();
 }
 
+Error DWARFListTableHeader::create(DWARFDataExtractor Data, unsigned Version,
+                                   unsigned AddrSize,
+                                   dwarf::DwarfFormat FormatParam) {
+  Format = FormatParam;
+  HeaderOffset = 0;
+  if (Data.size() < getHeaderSize(Format))
+    return createStringError(errc::invalid_argument,
+                             "%s table at offset 0x%" PRIx64
+                             " has too small length (0x%" PRIx64
+                             ") to contain a complete header",
+                             SectionName.data(), HeaderOffset, Data.size());
+
+  HeaderData.Length = Data.size() - dwarf::getUnitLengthFieldByteSize(Format);
+  HeaderData.Version = Version;
+  HeaderData.AddrSize = AddrSize;
+  HeaderData.SegSize = 0;
+  HeaderData.OffsetEntryCount = 0;
+  if (Error Err = HeaderData.verify())
+    return createStringError(
+        errc::invalid_argument, "parsing %s table at offset 0x%" PRIx64 ": %s",
+        SectionName.data(), HeaderOffset, toString(std::move(Err)).c_str());
+
+  return Error::success();
+}
+
 Error DWARFListTableHeader::extract(DWARFDataExtractor Data,
                                     uint64_t *OffsetPtr) {
   HeaderOffset = *OffsetPtr;
   Error Err = Error::success();
 
   std::tie(HeaderData.Length, Format) = Data.getInitialLength(OffsetPtr, &Err);
   if (Err)
     return createStringError(