[PATCH] D72489: [DWARF] Emit DW_AT_call_return_pc as an address

Fri Jan 10 12:52:18 PST 2020

vsk marked an inline comment as done.
vsk added a comment.

In D72489#1813533 <https://reviews.llvm.org/D72489#1813533>, @dblaikie wrote:

> Thanks for looking into this!
>
> Could you measure the size of the object files of, for example, the clang binary before/after this change - and, if possible, on Linux (where relocations are required to fixup these addresses)? I'm concerned this might increase the size significantly.

Hm, there's something I'm missing here. I verified that this change causes one extra X86_64_RELOC_UNSIGNED relocation to be emitted per call site entry in a MachO. For a stage2 build of clang on Darwin (2281 .o's), this patch increases the total file size of the .o's by 0.04% (6,970,706,668 bytes -> 6,973,643,092 bytes, a ~3MB increase). That size increase is much smaller than expected, because clang has ~5.5 million call site entries, and (IIUC) an X86_64_RELOC_UNSIGNED relocation takes 8 bytes. So I'd expect a ~42MB size increase. Does anyone have any insight into this discrepancy? Are relocations compressed, or did the previous subtraction encoding take up space elsewhere?

> Could you check if this does the right thing for Split DWARF or DWARFv5? (I suspect it does - if you're using the same API as is used for DW_TAG_label's low_pc, for instance - which I imagine you are) - in terms of adding the address to debug_addr, and using addrx encoding.

I've added a test in test/DebugInfo/X86/debug_addr.ll.

> I was going to suggest we use a non-standard extension to preserve the subprogram-relative behavior, by using a different DW_FORM (such as data4) to communicate that this is a subprogram-relative value. But that won't be sufficient for PROPELLER, for instance, which fragments subprograms so there's no unique low_pc for it to be relative to. So that leaves us maybe really wanting the addr+offset encoding discussed here: http://lists.llvm.org/pipermail/llvm-dev/2020-January/138029.html

I don't really understand the addr+offset encoding, I'll follow up on llvm-dev.

================
Comment at: lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_dso/Makefile:10
+
+lib_One: lib_Two
+
----------------
aprantl wrote:
> It seems like one of the lib_Two dependencies is redundant? Presumably the one in a.out?
Ah, yep. If it's all right I'd rather just leave the redundant lib_Two dependency for a.out, to make it really explicit.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72489/new/

https://reviews.llvm.org/D72489