[PATCH] D52168: [DWARF] - Emit the correct value for DW_AT_addr_base.

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: lib/CodeGen/AsmPrinter/AddressPool.cpp:43-44
+  // It is referenced via DW_AT_addr_base.
+  if (AddrBase)
+    Asm.OutStreamer->EmitLabel(AddrBase);
 }
----------------
dblaikie wrote:
> Is there any reason to support not labeling the address pool? It doesn't look like this should be conditional.
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.


================
Comment at: lib/CodeGen/AsmPrinter/DwarfFile.h:59
+  /// address table.
+  MCSymbol *AddressTableBaseSym = nullptr;
+
----------------
dblaikie wrote:
> 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.
Nice, done. (Originally I put it here for consistency, near the `StringOffsetsStartSym` and `RnglistsTableBaseSym`).


================
Comment at: lib/CodeGen/AsmPrinter/DwarfUnit.cpp:1655-1656
+  MCSymbol *Base = DU->getAddrTableBaseSym();
+  if (!Base)
+    Base = TLOF.getDwarfAddrSection()->getBeginSymbol();
+  addSectionLabel(getUnitDie(), dwarf::DW_AT_GNU_addr_base, Base,
----------------
dblaikie wrote:
> 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?
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.


https://reviews.llvm.org/D52168