[DWARF] - Emit the correct value for DW_AT_addr_base.

[grimar]

Summary

Currently, we emit DW_AT_addr_base that points to the beginning of
the .debug_addr section. That is not correct for the DWARF5 case because address
table contains the header and the attribute should point to the first entry
following the header.

This is currently the reason why LLDB does not work with such executables correctly.

[dblaikie]

lib/CodeGen/AsmPrinter/AddressPool.cpp

41–42

Is there any reason to support not labeling the address pool? It doesn't look like this should be conditional.

lib/CodeGen/AsmPrinter/DwarfFile.h

59 ↗

(On Diff #165747)

Since the DWO file can't have an address table itself (so its AddressTableBaseSym would always be null) - perhaps this symbol should be tracked in either the AddressPool itself, or in DwarfDebug? I'd slightly favor the AddressPool handling this. Using something like "DD->getAddressPool().getLabel()" to retrieve it in the DWARFCompileUnit & no need to pass it in when the AddressPool is emitting its header - it already has it/can attach it there.

lib/CodeGen/AsmPrinter/DwarfUnit.cpp

1655–1656

Could you explain what this case is for? I would (naively) have thought this case wouldn't come up once the label was being appropriately managed/emitted at the end of the header? In what situations is there no computed addrTableBase, but this fallback is correct?

[grimar]

• Addressed review comments.

[grimar]

lib/CodeGen/AsmPrinter/AddressPool.cpp

41–42

Labeling of the address pool really needed only for dwarf5 because dwarf4 does not have address table header.
For simplicity of the code, I changed this. With that, I had to change the 2 more tests though.
I am not sure what is better, for dwarf4 output labeling is not useful, but the code is a bit simpler now.

lib/CodeGen/AsmPrinter/DwarfFile.h

59 ↗

(On Diff #165747)

Nice, done. (Originally I put it here for consistency, near the StringOffsetsStartSym and RnglistsTableBaseSym).

lib/CodeGen/AsmPrinter/DwarfUnit.cpp

1655–1656

My code created a base symbol for dwarf5 only:

if (getDwarfVersion() >= 5 && useSplitDwarf())
  SkeletonHolder.setAddrTableBaseSym(
      Asm->createTempSymbol("addr_table_base"));

So this fallback was for dwarf4.

[dblaikie]

test/DebugInfo/X86/debug_addr.ll

34

This looks suspicious, I think? This test is for DWARF4 so why does it have a non-zero addr_base?

[grimar]

test/DebugInfo/X86/debug_addr.ll

34

No :) This test file is for both DWARF4 and DWARF5. See the corresponding check tags. For DWARF4 it is zero (line 20).
This place is exactly what needed to be fixed to show that this patch does the right thing now.

[dblaikie]

Looks good - thanks!

lib/CodeGen/AsmPrinter/AddressPool.h

public:
   void emit(AsmPrinter &Asm, MCSection *AddrSection);
 
   bool isEmpty() { return Pool.empty(); }
 
   bool hasBeenUsed() const { return HasBeenUsed; }
 
   void resetUsedFlag() { HasBeenUsed = false; }
 
+  MCSymbol *getLabel() { return AddressTableBaseSym; }
+  void setLabel(MCSymbol *Sym) { AddressTableBaseSym = Sym; }
+
 private:
   void emitHeader(AsmPrinter &Asm, MCSection *Section);
+
+  /// Symbol designates the start of the contribution to the address table.
+  MCSymbol *AddressTableBaseSym = nullptr;
 };
 
 } // end namespace llvm
 
 #endif // LLVM_LIB_CODEGEN_ASMPRINTER_ADDRESSPOOL_H

lib/CodeGen/AsmPrinter/AddressPool.cpp

unsigned AddressPool::getIndex(const MCSymbol *Sym, bool TLS) {
   auto IterBool =
       Pool.insert(std::make_pair(Sym, AddressPoolEntry(Pool.size(), TLS)));
   return IterBool.first->second.Number;
 }
 
 
 void AddressPool::emitHeader(AsmPrinter &Asm, MCSection *Section) {
   static const uint8_t AddrSize = Asm.getDataLayout().getPointerSize();
-  Asm.OutStreamer->SwitchSection(Section);
-
   uint64_t Length = sizeof(uint16_t) // version
                   + sizeof(uint8_t)  // address_size
                   + sizeof(uint8_t)  // segment_selector_size
                   + AddrSize * Pool.size(); // entries
   Asm.emitInt32(Length); // TODO: Support DWARF64 format.
   Asm.emitInt16(Asm.getDwarfVersion());
   Asm.emitInt8(AddrSize);
   Asm.emitInt8(0); // TODO: Support non-zero segment_selector_size.
 }
 
 // Emit addresses into the section given.
 void AddressPool::emit(AsmPrinter &Asm, MCSection *AddrSection) {
+  // Start the dwarf addr section.
+  Asm.OutStreamer->SwitchSection(AddrSection);
+
   if (Asm.getDwarfVersion() >= 5)
     emitHeader(Asm, AddrSection);
 
+  // Define the symbol that marks the start of the contribution.
+  // It is referenced via DW_AT_addr_base.
+  Asm.OutStreamer->EmitLabel(AddressTableBaseSym);
+
   if (Pool.empty())
     return;
 
-  // Start the dwarf addr section.
-  Asm.OutStreamer->SwitchSection(AddrSection);
-
   // Order the address pool entries by ID
   SmallVector<const MCExpr *, 64> Entries(Pool.size());
 
   for (const auto &I : Pool)
     Entries[I.second.Number] =
         I.second.TLS
             ? Asm.getObjFileLowering().getDebugThreadLocalSymbol(I.first)
             : MCSymbolRefExpr::create(I.first, Asm.OutContext);
 
   for (const MCExpr *Entry : Entries)
     Asm.OutStreamer->EmitValue(Entry, Asm.getDataLayout().getPointerSize());
 }

lib/CodeGen/AsmPrinter/DwarfDebug.cpp

(useSplitDwarf() ? SkeletonHolder : InfoHolder)
         .setStringOffsetsStartSym(Asm->createTempSymbol("str_offsets_base"));
 
   // Create the symbol that designates the start of the DWARF v5 range list
   // table. It is located past the header and before the offsets table.
   if (getDwarfVersion() >= 5)
     (useSplitDwarf() ? SkeletonHolder : InfoHolder)
         .setRnglistsTableBaseSym(Asm->createTempSymbol("rnglists_table_base"));
 
+  // Create the symbol that points to the first entry following the debug
+  // address table (.debug_addr) header.
+  AddrPool.setLabel(Asm->createTempSymbol("addr_table_base"));
+
   for (DICompileUnit *CUNode : M->debug_compile_units()) {
     // FIXME: Move local imported entities into a list attached to the
     // subprogram, then this search won't be needed and a
     // getImportedEntities().empty() test should go below with the rest.
     bool HasNonLocalImportedEntities = llvm::any_of(
         CUNode->getImportedEntities(), [](const DIImportedEntity *IE) {
           return !isa<DILocalScope>(IE->getScope());
         });
if (useSplitDwarf()) {
       } else {
         TheCU.addUInt(TheCU.getUnitDie(), dwarf::DW_AT_GNU_dwo_id,
                       dwarf::DW_FORM_data8, ID);
         SkCU->addUInt(SkCU->getUnitDie(), dwarf::DW_AT_GNU_dwo_id,
                       dwarf::DW_FORM_data8, ID);
       }
       // We don't keep track of which addresses are used in which CU so this
       // is a bit pessimistic under LTO.
-      if (!AddrPool.isEmpty()) {
-        const MCSymbol *Sym = TLOF.getDwarfAddrSection()->getBeginSymbol();
-        SkCU->addSectionLabel(SkCU->getUnitDie(), dwarf::DW_AT_GNU_addr_base,
-                              Sym, Sym);
-      }
+      if (!AddrPool.isEmpty())
+        SkCU->addAddrTableBase();
+
       if (getDwarfVersion() < 5 && !SkCU->getRangeLists().empty()) {
         const MCSymbol *Sym = TLOF.getDwarfRangesSection()->getBeginSymbol();
         SkCU->addSectionLabel(SkCU->getUnitDie(), dwarf::DW_AT_GNU_ranges_base,
                               Sym, Sym);
       }
     }
 
     // If we have code split among multiple sections or non-contiguous

lib/CodeGen/AsmPrinter/DwarfUnit.h

public:
   virtual void emitHeader(bool UseOffsets) = 0;
 
   /// Add the DW_AT_str_offsets_base attribute to the unit DIE.
   void addStringOffsetsStart();
 
   /// Add the DW_AT_rnglists_base attribute to the unit DIE.
   void addRnglistsBase();
 
+  /// Add the DW_AT_addr_base attribute to the unit DIE.
+  void addAddrTableBase();
+
   virtual DwarfCompileUnit &getCU() = 0;
 
   void constructTypeDIE(DIE &Buffer, const DICompositeType *CTy);
 
   /// addSectionDelta - Add a label delta attribute data and value.
   DIE::value_iterator addSectionDelta(DIE &Die, dwarf::Attribute Attribute,
                                       const MCSymbol *Hi, const MCSymbol *Lo);
 

lib/CodeGen/AsmPrinter/DwarfUnit.cpp

 void DwarfUnit::addRnglistsBase() {
   assert(DD->getDwarfVersion() >= 5 &&
          "DW_AT_rnglists_base requires DWARF version 5 or later");
   const TargetLoweringObjectFile &TLOF = Asm->getObjFileLowering();
   addSectionLabel(getUnitDie(), dwarf::DW_AT_rnglists_base,
                   DU->getRnglistsTableBaseSym(),
                   TLOF.getDwarfRnglistsSection()->getBeginSymbol());
 }
+
+void DwarfUnit::addAddrTableBase() {
+  const TargetLoweringObjectFile &TLOF = Asm->getObjFileLowering();
+  MCSymbol *Label = DD->getAddressPool().getLabel();
+  addSectionLabel(getUnitDie(), dwarf::DW_AT_GNU_addr_base, Label,
+                  TLOF.getDwarfAddrSection()->getBeginSymbol());
+}

test/DebugInfo/PowerPC/tls-fission.ll

 ; RUN: llc -split-dwarf-file=foo.dwo -mtriple=powerpc64-unknown-linux-gnu -O0 -filetype=asm < %s | FileCheck %s
 
 ; FIXME: add relocation and DWARF expression support to llvm-dwarfdump & use
 ; that here instead of raw assembly printing
 
 ; CHECK: debug_info.dwo
 ; 3 bytes of data in this DW_FORM_block1 representation of the location of 'tls'
 ; CHECK: .byte 3{{ *}}# DW_AT_location
 ; DW_OP_const_index (0xfx == 252) to refer to the debug_addr table
 ; CHECK-NEXT: .byte 252
 ; an index of zero into the debug_addr table
 ; CHECK-NEXT: .byte 0
 ; DW_OP_GNU_push_tls_address
 ; CHECK-NEXT: .byte 224
 ; check that the expected TLS address description is the first thing in the debug_addr section
 ; CHECK: .section .debug_addr,"",@progbits
+; CHECK-NEXT: .Laddr_table_base0:
 ; CHECK-NEXT: .quad tls@DTPREL+32768
 
 source_filename = "test/DebugInfo/PowerPC/tls-fission.ll"
 
 @tls = thread_local global i32 0, align 4, !dbg !0
 
 !llvm.dbg.cu = !{!4}
 !llvm.module.flags = !{!7, !8}

test/DebugInfo/X86/debug_addr.ll

 ; DWARF4-NEXT: ]
 
 ; DWARF5: .debug_info contents:
 ; DWARF5: Compile Unit:{{.*}}version = 0x0005
 ; DWARF5-NOT: Compile Unit
 ; DWARF5: DW_TAG_compile_unit
 ; DWARF5-NOT: DW_TAG_{{.*}}
 ; DWARF5: DW_AT_GNU_dwo_name{{.*}}test.dwo
-; DWARF5: DW_AT_GNU_addr_base{{.*}}0x00000000
+; DWARF5: DW_AT_GNU_addr_base{{.*}}0x00000008
 ; DWARF5: .debug_addr contents:
 ; DWARF5-NEXT: 0x00000000: Addr Section: length = 0x0000000c, version = 0x0005, addr_size = 0x04, seg_size = 0x00
 ; DWARF5-NEXT: Addrs: [
 ; DWARF5-NEXT: 0x00000000
 ; DWARF5-NEXT: 0x00000010
 ; DWARF5-NEXT: ]
 
 ; ModuleID = './test.c'

test/DebugInfo/X86/tls.ll

 ; DW_OP_GNU_addr_index
 ; FISSION-NEXT: .byte 251
 ; FISSION-NEXT: .byte 1
 ; DW_OP_stack_value
 ; FISSION-NEXT: .byte 159
 
 ; check that the expected TLS address description is the first thing in the debug_addr section
 ; FISSION: .section    .debug_addr
+; FISSION-NEXT: .Laddr_table_base0:
 ; FISSION-NEXT: .quad  tls@DTPOFF
 ; FISSION-NEXT: .quad  glbl
 ; FISSION-NOT: .quad  glbl
 
 ; Generated from:
 
 ; __thread int tls;
 ; int glbl;