[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 ↗

(On Diff #360518)

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 ↗

(On Diff #360518)

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

441

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.

1007

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.cpp

 // 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;
-  }
+  if (offset == 0) {
+    // This means DW_AT_rnglists_base is missing and therefore DW_FORM_rnglistx
+    // cannot be handled. Returning a default-constructed ListTableType allows
+    // DW_FORM_sec_offset to be supported.
+    return ListTableType();
+  }
+
   ListTableType Table;
   if (llvm::Error E = Table.extractHeaderAndOffsets(data, &offset))
     return std::move(E);
   return Table;
 }
 
 void DWARFUnit::SetLoclistsBase(dw_addr_t loclists_base) {
   uint64_t offset = 0;
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");
 
-  auto range_list_or_error = GetRnglistTable()->findList(
-      m_dwarf.GetDWARFContext().getOrLoadRngListsData().GetAsLLVM(), offset);
+  llvm::DWARFDataExtractor data =
+      m_dwarf.GetDWARFContext().getOrLoadRngListsData().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 =
       range_list_or_error->getAbsoluteRanges(
           llvm::object::SectionedAddress{GetBaseAddress()},
           GetAddressByteSize(), [&](uint32_t index) {
             uint32_t index_size = GetAddressByteSize();

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: