Another prototype fix for lld DWARF parsing of base address selection entries in range lists

[grimar]

Summary

This is based on David Blaikie's patch D36097 and
solves issue of wrong section index evaluating for ranges when
base address is used.

[grimar]

• Added testcase.

[dblaikie]

include/llvm/DebugInfo/DWARF/DWARFUnit.h

271–272 ↗

(On Diff #113255)

So why is this bit necessary? Should callers just not try to set a base address if they don't want to, rather than having setBaseAddress ignore the request?

lib/DebugInfo/DWARF/DWARFDebugRangeList.cpp

71 ↗

(On Diff #113255)

Is this clang-formatted? I'd expect no spaces inner sides of the {}

[grimar]

• Addressed review comments.

include/llvm/DebugInfo/DWARF/DWARFUnit.h

271–272 ↗

(On Diff #113255)

It is for dwarfdump-ranges-unrelocated.s testcase. It is based on following code:

test.cpp:
   void foo1() { }  
   void foo2() { }  
clang test.cpp -S -o test.s -gmlt -ffunction-sections

And output produced is:

	.section	.debug_info,"",@progbits
.Lcu_begin0:
	.long	38                      # 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:0x1f DW_TAG_compile_unit
	.long	.Linfo_string0          # DW_AT_producer
	.short	4                       # DW_AT_language
	.long	.Linfo_string1          # DW_AT_name
	.long	.Lline_table_start0     # DW_AT_stmt_list
	.long	.Linfo_string2          # DW_AT_comp_dir
	.quad	0                       # DW_AT_low_pc
	.long	.Ldebug_ranges0         # DW_AT_ranges
	.section	.debug_ranges,"",@progbits
.Ldebug_ranges0:
	.quad	.Lfunc_begin0
	.quad	.Lfunc_end0
	.quad	.Lfunc_begin1
	.quad	.Lfunc_end1
	.quad	0
	.quad	0

So it has ranges but also unrelocated zero base address is emited in debug info. Since it has no
corresponding relocation (SectionIndex == -1ULL), I am ignoring it to prevent using
-1ULL as section index name. That looks a bit hacky, but I currently see no other way to distinguish
between such "dummy" base address and real one.
In testcase it is shown that base address can be 0 after relocation is applied, so I think we need
to check both SectionIndex and relocated value, just like I do.

I moved that code to caller side, thats probably a bit more clear and localizes the hack.

lib/DebugInfo/DWARF/DWARFDebugRangeList.cpp

71 ↗

(On Diff #113255)

You right, fixed.

[grimar]

• Added testcase for executables.
• Updated code accordinng to discussion in corresponding thread.

[dblaikie]

Couple of minor things to address before you commit this (no need to send it for further review though - I'll catch anything else in post-commit), but otherwise looks good.

lib/DebugInfo/DWARF/DWARFFormValue.cpp

296 ↗

(On Diff #113529)

should getRelocatedValue handle this case (if there is no section, it coudl set the SectionIndex to -1ULL) so SectionIndex is welldefined after a call to getRelocatedValue, even when the section index is not known?

test/DebugInfo/X86/dwarfdump-ranges-baseaddr.s

65–68 ↗

(On Diff #113529)

Add a base address selection entry in this list as well (so let the first entry rely on the CU's default base address, then use a base address selection entry to switch base address (maybe switch section too to make it obvious in the CHECKs)), to test that codepath.

[grimar]

Thanks for review, David ! All your comments were addressed.

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

 #include "llvm/DebugInfo/DWARF/DWARFDataExtractor.h"
 #include "llvm/DebugInfo/DWARF/DWARFRelocMap.h"
 #include <cassert>
 #include <cstdint>
 #include <vector>
 
 namespace llvm {
 
+struct BaseAddress;
 class raw_ostream;
 
 struct DWARFAddressRange {
   uint64_t LowPC;
   uint64_t HighPC;
   uint64_t SectionIndex;
 };
 
public:
   void clear();
   void dump(raw_ostream &OS) const;
   bool extract(const DWARFDataExtractor &data, uint32_t *offset_ptr);
   const std::vector<RangeListEntry> &getEntries() { return Entries; }
 
   /// getAbsoluteRanges - Returns absolute address ranges defined by this range
   /// list. Has to be passed base address of the compile unit referencing this
   /// range list.
-  DWARFAddressRangesVector getAbsoluteRanges(uint64_t BaseAddress) const;
+  DWARFAddressRangesVector
+  getAbsoluteRanges(llvm::Optional<BaseAddress> BaseAddr) const;
 };
 
 } // end namespace llvm
 
 #endif // LLVM_DEBUGINFO_DWARF_DWARFDEBUGRANGELIST_H

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

while (Data.isValidOffset(Offset)) {
         break;
       this->push_back(std::move(U));
       Offset = this->back()->getNextUnitOffset();
     }
     Parsed = true;
   }
 };
 
+/// Represents base address of the CU.
+struct BaseAddress {
+  uint64_t Address;
+  uint64_t SectionIndex;
+};
+
 class DWARFUnit {
   DWARFContext &Context;
   /// Section containing this DWARFUnit.
   const DWARFSection &InfoSection;
 
   const DWARFDebugAbbrev *Abbrev;
   const DWARFSection *RangeSection;
   uint32_t RangeSectionBase;
class DWARFUnit {
 
   // Version, address size, and DWARF format.
   DWARFFormParams FormParams;
 
   uint32_t Offset;
   uint32_t Length;
   const DWARFAbbreviationDeclarationSet *Abbrevs;
   uint8_t UnitType;
-  uint64_t BaseAddr;
+  llvm::Optional<BaseAddress> BaseAddr;
   /// The compile unit debug information entry items.
   std::vector<DWARFDebugInfoEntry> DieArray;
 
   /// Map from range's start address to end address and corresponding DIE.
   /// IntervalMap does not support range removal, as a result, we use the
   /// std::map::upper_bound for address range lookup.
   std::map<uint64_t, std::pair<uint64_t, DWARFDie>> AddrDieMap;
 
case dwarf::DW_UT_split_compile:
       return 20;
     case dwarf::DW_UT_type:
     case dwarf::DW_UT_split_type:
       return 24;
     }
     llvm_unreachable("Invalid UnitType.");
   }
 
-  uint64_t getBaseAddress() const { return BaseAddr; }
+  llvm::Optional<BaseAddress> getBaseAddress() const { return BaseAddr; }
 
-  void setBaseAddress(uint64_t base_addr) {
-    BaseAddr = base_addr;
-  }
+  void setBaseAddress(BaseAddress BaseAddr) { this->BaseAddr = BaseAddr; }
 
   DWARFDie getUnitDIE(bool ExtractUnitDIEOnly = true) {
     extractDIEsIfNeeded(ExtractUnitDIEOnly);
     if (DieArray.empty())
       return DWARFDie();
     return DWARFDie(this, &DieArray[0]);
   }
 

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

 //===- DWARFDataExtractor.cpp ---------------------------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/DebugInfo/DWARF/DWARFDataExtractor.h"
 #include "llvm/DebugInfo/DWARF/DWARFContext.h"
 
 using namespace llvm;
 
 uint64_t DWARFDataExtractor::getRelocatedValue(uint32_t Size, uint32_t *Off,
                                                uint64_t *SecNdx) const {
+  if (SecNdx)
+    *SecNdx = -1ULL;
   if (!Section)
     return getUnsigned(Off, Size);
   Optional<RelocAddrEntry> Rel = Obj->find(*Section, *Off);
   if (!Rel)
     return getUnsigned(Off, Size);
   if (SecNdx)
     *SecNdx = Rel->SectionIndex;
   return getUnsigned(Off, Size) + Rel->Value;
 }

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

for (const RangeListEntry &RLE : Entries) {
     const char *format_str = (AddressSize == 4
                               ? "%08x %08"  PRIx64 " %08"  PRIx64 "\n"
                               : "%08x %016" PRIx64 " %016" PRIx64 "\n");
     OS << format(format_str, Offset, RLE.StartAddress, RLE.EndAddress);
   }
   OS << format("%08x <End of list>\n", Offset);
 }
 
-DWARFAddressRangesVector
-DWARFDebugRangeList::getAbsoluteRanges(uint64_t BaseAddress) const {
+DWARFAddressRangesVector DWARFDebugRangeList::getAbsoluteRanges(
+    llvm::Optional<BaseAddress> BaseAddr) const {
   DWARFAddressRangesVector Res;
   for (const RangeListEntry &RLE : Entries) {
     if (RLE.isBaseAddressSelectionEntry(AddressSize)) {
-      BaseAddress = RLE.EndAddress;
-    } else {
-      Res.push_back({BaseAddress + RLE.StartAddress,
-                     BaseAddress + RLE.EndAddress, RLE.SectionIndex});
+      BaseAddr = {RLE.EndAddress, RLE.SectionIndex};
+      continue;
     }
+
+    DWARFAddressRange E;
+    E.LowPC = RLE.StartAddress;
+    E.HighPC = RLE.EndAddress;
+    E.SectionIndex = RLE.SectionIndex;
+    // Base address of a range list entry is determined by the closest preceding
+    // base address selection entry in the same range list. It defaults to the
+    // base address of the compilation unit if there is no such entry.
+    if (BaseAddr) {
+      E.LowPC += BaseAddr->Address;
+      E.HighPC += BaseAddr->Address;
+      if (E.SectionIndex == -1ULL)
+        E.SectionIndex = BaseAddr->SectionIndex;
+    }
+    Res.push_back(E);
   }
   return Res;
 }

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

bool DWARFUnit::extractRangeList(uint32_t RangeListOffset,
   return RangeList.extract(RangesData, &ActualRangeListOffset);
 }
 
 void DWARFUnit::clear() {
   Offset = 0;
   Length = 0;
   Abbrevs = nullptr;
   FormParams = DWARFFormParams({0, 0, DWARF32});
-  BaseAddr = 0;
+  BaseAddr.reset();
   RangeSectionBase = 0;
   AddrOffsetSectionBase = 0;
   clearDIEs(false);
   DWO.reset();
 }
 
 const char *DWARFUnit::getCompilationDir() {
   return dwarf::toString(getUnitDIE().find(DW_AT_comp_dir), nullptr);
size_t DWARFUnit::extractDIEsIfNeeded(bool CUDieOnly) {
   extractDIEsToVector(!HasCUDie, !CUDieOnly, DieArray);
 
   if (DieArray.empty())
     return 0;
 
   // If CU DIE was just parsed, copy several attribute values from it.
   if (!HasCUDie) {
     DWARFDie UnitDie = getUnitDIE();
-    auto BaseAddr = toAddress(UnitDie.find({DW_AT_low_pc, DW_AT_entry_pc}));
-    if (BaseAddr)
-      setBaseAddress(*BaseAddr);
+    Optional<DWARFFormValue> PC = UnitDie.find({DW_AT_low_pc, DW_AT_entry_pc});
+    if (Optional<uint64_t> Addr = toAddress(PC))
+        setBaseAddress({*Addr, PC->getSectionIndex()});
+
     if (!isDWO) {
       assert(AddrOffsetSectionBase == 0);
       assert(RangeSectionBase == 0);
       AddrOffsetSectionBase =
           toSectionOffset(UnitDie.find(DW_AT_GNU_addr_base), 0);
       RangeSectionBase = toSectionOffset(UnitDie.find(DW_AT_rnglists_base), 0);
     }
 

llvm/trunk/test/DebugInfo/X86/dwarfdump-ranges-baseaddr-exe.s

+# RUN: llvm-dwarfdump %S/../Inputs/dwarfdump-ranges-baseaddr-exe.elf-x86-64 \
+# RUN:  | FileCheck %s
+
+## Executable binary for test produced from object built in 
+## dwarfdump-ranges-baseaddr.s testcase.
+
+# CHECK: .debug_info contents:
+# CHECK: 0x0000000b: DW_TAG_compile_unit [1]
+# CHECK:             DW_AT_low_pc [DW_FORM_addr]       (0x0000000000400078)
+# CHECK-NEXT:        DW_AT_ranges [DW_FORM_sec_offset] (0x00000000
+# CHECK-NEXT:    [0x0000000000400078 - 0x0000000000400079)
+# CHECK-NEXT:    [0x000000000040007b - 0x000000000040007e)
+# CHECK-NEXT:    [0x000000000040007f - 0x0000000000400080))

llvm/trunk/test/DebugInfo/X86/dwarfdump-ranges-baseaddr.s

+# RUN: llvm-mc -triple x86_64-pc-linux -filetype=obj %s -o %t
+# RUN: llvm-dwarfdump %t | FileCheck %s
+
+# CHECK: .debug_info contents:
+# CHECK: 0x0000000b: DW_TAG_compile_unit [1]
+# CHECK:             DW_AT_low_pc [DW_FORM_addr]       (0x0000000000000000)
+# CHECK-NEXT:        DW_AT_ranges [DW_FORM_sec_offset] (0x00000000
+# CHECK-NEXT:    [0x0000000000000000 - 0x0000000000000001) ".text"
+# CHECK-NEXT:    [0x0000000000000003 - 0x0000000000000006) ".text"
+# CHECK-NEXT:    [0x0000000000000001 - 0x0000000000000002) ".text.foo1")
+
+.text
+.globl foo
+.type foo,@function
+foo: 
+.Lfunc_begin0:
+  nop
+.Ltmp0:
+  nop
+  nop
+.Ltmp1:
+  nop
+  nop
+  nop
+.Ltmp2:
+
+.section .text.foo1,"ax",@progbits
+.Ltmp3:
+ nop
+.Ltmp4: 
+ nop
+.Ltmp5:
+
+.section .debug_abbrev,"",@progbits
+.byte 1                       # Abbreviation Code
+.byte 17                      # DW_TAG_compile_unit
+.byte 0                       # DW_CHILDREN_no
+.byte 37                      # DW_AT_producer
+.byte 14                      # DW_FORM_strp
+.byte 19                      # DW_AT_language
+.byte 5                       # DW_FORM_data2
+.byte 3                       # DW_AT_name
+.byte 14                      # DW_FORM_strp
+.byte 16                      # DW_AT_stmt_list
+.byte 23                      # DW_FORM_sec_offset
+.byte 27                      # DW_AT_comp_dir
+.byte 14                      # DW_FORM_strp
+.byte 17                      # DW_AT_low_pc
+.byte 1                       # DW_FORM_addr
+.byte 85                      # DW_AT_ranges
+.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 38                      # 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:0x1f DW_TAG_compile_unit
+.long 0                       # DW_AT_producer
+.short 4                      # DW_AT_language
+.long 0                       # DW_AT_name
+.long 0                       # DW_AT_stmt_list
+.long 0                       # DW_AT_comp_dir
+.quad .Lfunc_begin0           # DW_AT_low_pc
+.long .Ldebug_ranges0         # DW_AT_ranges
+
+.section .debug_ranges,"",@progbits
+.Ldebug_ranges0:
+ .quad .Lfunc_begin0-.Lfunc_begin0
+ .quad .Ltmp0-.Lfunc_begin0
+ .quad .Ltmp1-.Lfunc_begin0
+ .quad .Ltmp2-.Lfunc_begin0
+ .quad 0xFFFFFFFFFFFFFFFF
+ .quad .text.foo1
+ .quad .Ltmp4-.text.foo1
+ .quad .Ltmp5-.text.foo1
+ .quad 0
+ .quad 0