[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

43–44

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

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

43–44

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

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:
   AddressPool() = default;
 
   /// Returns the index into the address pool with the given
   /// label/symbol.
   unsigned getIndex(const MCSymbol *Sym, bool TLS = false);
 
-  void emit(AsmPrinter &Asm, MCSection *AddrSection);
+  void emit(AsmPrinter &Asm, MCSection *AddrSection, MCSymbol *AddrBase);
 
   bool isEmpty() { return Pool.empty(); }
 
   bool hasBeenUsed() const { return HasBeenUsed; }
 
   void resetUsedFlag() { HasBeenUsed = false; }
 
 private:
-  void emitHeader(AsmPrinter &Asm, MCSection *Section);
+  void emitHeader(AsmPrinter &Asm, MCSection *Section, MCSymbol *AddrBase);
 };
 
 } // end namespace llvm
 
 #endif // LLVM_LIB_CODEGEN_ASMPRINTER_ADDRESSPOOL_H

lib/CodeGen/AsmPrinter/AddressPool.cpp

 
 unsigned AddressPool::getIndex(const MCSymbol *Sym, bool TLS) {
   HasBeenUsed = true;
   auto IterBool =
       Pool.insert(std::make_pair(Sym, AddressPoolEntry(Pool.size(), TLS)));
   return IterBool.first->second.Number;
 }
 
-
-void AddressPool::emitHeader(AsmPrinter &Asm, MCSection *Section) {
+void AddressPool::emitHeader(AsmPrinter &Asm, MCSection *Section,
+                             MCSymbol *AddrBase) {
   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.
+
+  // Define the symbol that marks the start of the contribution.
+  // It is referenced via DW_AT_addr_base.
+  if (AddrBase)
+    Asm.OutStreamer->EmitLabel(AddrBase);
 }
 
 // Emit addresses into the section given.
-void AddressPool::emit(AsmPrinter &Asm, MCSection *AddrSection) {
+void AddressPool::emit(AsmPrinter &Asm, MCSection *AddrSection,
+                       MCSymbol *AddrBase) {
   if (Asm.getDwarfVersion() >= 5)
-    emitHeader(Asm, AddrSection);
+    emitHeader(Asm, AddrSection, AddrBase);
 
   if (Pool.empty())
     return;
 
   // Start the dwarf addr section.
   Asm.OutStreamer->SwitchSection(AddrSection);
 
   // Order the address pool entries by ID

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.
+  if (getDwarfVersion() >= 5 && useSplitDwarf())
+    SkeletonHolder.setAddrTableBaseSym(
+        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
void DwarfDebug::emitDebugStrDWO() {
   MCSection *OffSec = Asm->getObjFileLowering().getDwarfStrOffDWOSection();
   InfoHolder.emitStrings(Asm->getObjFileLowering().getDwarfStrDWOSection(),
                          OffSec, /* UseRelativeOffsets = */ false);
 }
 
 // Emit DWO addresses.
 void DwarfDebug::emitDebugAddr() {
   assert(useSplitDwarf() && "No split dwarf?");
-  AddrPool.emit(*Asm, Asm->getObjFileLowering().getDwarfAddrSection());
+  AddrPool.emit(*Asm, Asm->getObjFileLowering().getDwarfAddrSection(),
+                SkeletonHolder.getAddrTableBaseSym());
 }
 
 MCDwarfDwoLineTable *DwarfDebug::getDwoLineTable(const DwarfCompileUnit &CU) {
   if (!useSplitDwarf())
     return nullptr;
   const DICompileUnit *DIUnit = CU.getCUNode();
   SplitTypeUnitFileTable.maybeSetRootFile(
       DIUnit->getDirectory(), DIUnit->getFilename(),

lib/CodeGen/AsmPrinter/DwarfFile.h

class DwarfFile {
   /// DWARF v5: The symbol that designates the start of the contribution to
   /// the string offsets table. The contribution is shared by all units.
   MCSymbol *StringOffsetsStartSym = nullptr;
 
   /// DWARF v5: The symbol that designates the base of the range list table.
   /// The table is shared by all units.
   MCSymbol *RnglistsTableBaseSym = nullptr;
 
+  /// DWARF v5: The symbol that designates the start of the contribution to the
+  /// address table.
+  MCSymbol *AddressTableBaseSym = nullptr;
+
   /// The variables of a lexical scope.
   struct ScopeVars {
     /// We need to sort Args by ArgNo and check for duplicates. This could also
     /// be implemented as a list or vector + std::lower_bound().
     std::map<unsigned, DbgVariable *> Args;
     SmallVector<DbgVariable *, 8> Locals;
   };
   /// Collection of DbgVariables of each lexical scope.
public:
   MCSymbol *getStringOffsetsStartSym() const { return StringOffsetsStartSym; }
 
   void setStringOffsetsStartSym(MCSymbol *Sym) { StringOffsetsStartSym = Sym; }
 
   MCSymbol *getRnglistsTableBaseSym() const { return RnglistsTableBaseSym; }
 
   void setRnglistsTableBaseSym(MCSymbol *Sym) { RnglistsTableBaseSym = Sym; }
 
+  MCSymbol *getAddrTableBaseSym() const { return AddressTableBaseSym; }
+  void setAddrTableBaseSym(MCSymbol *Sym) { AddressTableBaseSym = Sym; }
+
   /// \returns false if the variable was merged with a previous one.
   bool addScopeVariable(LexicalScope *LS, DbgVariable *Var);
 
   void addScopeLabel(LexicalScope *LS, DbgLabel *Label);
 
   DenseMap<LexicalScope *, ScopeVars> &getScopeVariables() {
     return ScopeVariables;
   }

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 *Base = DU->getAddrTableBaseSym();
+  if (!Base)
+    Base = TLOF.getDwarfAddrSection()->getBeginSymbol();
+  addSectionLabel(getUnitDie(), dwarf::DW_AT_GNU_addr_base, Base,
+                  TLOF.getDwarfAddrSection()->getBeginSymbol());
+}

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'