labath added inline comments.

================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3740
+      if (tail_call)
+        call_inst_pc = low_pc;
+      else
----------------
vsk wrote:
> I think this needs to be `call_inst_pc = low_pc - 1`, see 
> `DwarfCompileUnit::constructCallSiteEntryDIE` for the rationale, and 
> `StackFrameList::SynthesizeTailCallFrames` for where we use this information. 
> The relevant part of the comment from SynthesizeTailCallFrames is:
> 
> "We do not want to subtract 1 from this PC, as it's the actual address of the 
> tail-calling branch instruction. This address is provided by the compiler via 
> DW_AT_call_pc."
> 
> In GNU+Dwarf4 mode, that's no longer true, the DW_AT_low_pc is a fake "return 
> address" for the tail call (really: the address of the instruction after the 
> tail-calling jump).
> 
> On x86_64, this test doesn't seem to stress this case, but the test breaks on 
> Darwin/arm64 without the adjustment.
Heh, you're right. I should've looked at what the code does instead of just 
trying to reverse engineer the logic from the output. I've now added the -1.
After looking that this code some more, I've come to realize that it's usage of 
the lack of DW_AT_call_return_pc to indicate a tail call is not correct -- I 
don't see anything preventing a producer from generating this attribute even 
for tail calls. I'm going to try refactoring this in another patch to store the 
tail-call-ness more explicitly. That should also make this part slightly 
cleaner.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80519



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to