[Lldb-commits] [PATCH] D72489: [DWARF] Emit DW_AT_call_return_pc as an address

[Djordje Todorovic via Phabricator via lldb-commits]

djtodoro added inline comments.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp:984-991
   // from one function to another.
   if (DD->getDwarfVersion() == 4 && DD->tuneForGDB()) {
     assert(PCAddr && "Missing PC information for a call");
     addLabelAddress(CallSiteDIE, dwarf::DW_AT_low_pc, PCAddr);
   } else if (!IsTail || DD->tuneForGDB()) {
-    assert(PCOffset && "Missing return PC information for a call");
-    addAddressExpr(CallSiteDIE, dwarf::DW_AT_call_return_pc, PCOffset);
+    assert(PCAddr && "Missing return PC information for a call");
+    addLabelAddress(CallSiteDIE, dwarf::DW_AT_call_return_pc, PCAddr);
----------------
vsk wrote:
> dblaikie wrote:
> > Side question: How'd this end up like this? Why all these GDB tuning checks? Seems like it'd add another layer of complexity/variety that'll make it harder for us to all be evaluating the same things. 
> + @djtodoro, I'm not sure why AT_call_return_pc would be needed at a tail call site as the debugger must ignore it. As for emitting DW_AT_low_pc under gdb tuning, I think this might be an artifact from the original GNU implementation.
>I'm not sure why AT_call_return_pc would be needed at a tail call site as the debugger must ignore it.  As for emitting DW_AT_low_pc under gdb tuning, I think this might be an artifact from the original GNU implementation.

Yes, that is the GNU implementation's heritage (I cannot remember why GCC generated the low_pc info in the case of the tail calls), but GNU GDB needs the low_pc (as an address) in order to handle the call_site and call_site_parameters debug info for non-tail calls. To avoiding the pc address info in the case of tail calls makes sense to me, since debuggers should avoid that info.


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

https://reviews.llvm.org/D72489