This is an archive of the discontinued LLVM Phabricator instance.

[lldb/DWARF] Fix PC value for artificial tail call frames for the "GNU" case
ClosedPublic

Authored by labath on Jun 2 2020, 7:45 AM.

Details

Summary

The way that the support for the GNU dialect of tail call frames was
implemented in D80519 meant that the were reporting very bogus PC values
which pointed into the middle of an instruction: the -1 trick is
necessary for the address to resolve to the right function, but we
should still be reporting a more realistic PC value -- I say "realistic"
and not "real", because it's very debatable what should be the correct
PC value for frames like this.

This patch achieves that my moving the -1 from SymbolFileDWARF into the
stack frame computation code. The idea is that SymbolFileDWARF will
merely report whether it has provided an address of the instruction
after the tail call, or the address of the call instruction itself. The
StackFrameList machinery uses this information to set the "behaves like
frame zero" property of the artificial frames (the main thing this flag
does is it controls the -1 subtraction when looking up the function
address).

This required a moderate refactor of the CallEdge class, because it was
implicitly assuming that edges pointing after the call were real calls
and those pointing the the call insn were tail calls. The class now
carries this information explicitly -- it carries three mostly
independent pieces of information:

  • an address of interest in the caller
  • a bit saying whether this address points to the call insn or after it
  • whether this is a tail call

Diff Detail

Event Timeline

labath created this revision.Jun 2 2020, 7:45 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 2 2020, 7:45 AM
vsk added a comment.Jun 2 2020, 9:59 AM

Thanks, this ended up being a lot cleaner than I expected!

lldb/include/lldb/Symbol/Function.h
325–328

It seems like there's going to be around sizeof(void *) bytes worth of padding here. Maybe it'd be simpler to store these flags as AddrType and bool respectively? Alternatively, the struct can be marked 'packed'.

labath marked an inline comment as done.Jun 3 2020, 2:04 AM
labath added inline comments.
lldb/include/lldb/Symbol/Function.h
325–328

Indeed. I was optimizing prematurely.

labath updated this revision to Diff 268102.Jun 3 2020, 2:14 AM
  • Unpack the CallEdge structure
vsk accepted this revision.Jun 3 2020, 10:48 AM

Lgtm.

lldb/source/Symbol/Function.cpp
320–321

Could you please update this comment?

This revision is now accepted and ready to land.Jun 3 2020, 10:48 AM
This revision was automatically updated to reflect the committed changes.