[Lldb-commits] [PATCH] D63667: Support __kernel_rt_sigreturn in frame initialization

Mon Jul 15 20:11:46 PDT 2019

JosephTremoulet added a comment.

> I'm guessing on this Linux system, you've got a trap receiver function on the stack that is on its first instruction or something? So backing up the pc value for *that* frame is the problem you're solving.
> ...
>  Just just to check, we've got a backtrace like...

Yes, it's just like you describe.

> I think Unwind::GetFrameInfoAtIndex can take a new output argument, bool behaves_like_zeroth_frame. It would be set to true if idx == 0 or if m_frames[idx - 1].IsTrapHandlerFrame() is true.
> StackFrame.h would pick up a new ivar m_behaves_like_zeroth_frame and accessor method BehavesLikeZerothFrame().
> StackFrameList::GetFramesUpTo will fetch the value via unwinder->GetFrameInfoAtIndex() and pass it as an argument to the StackFrame::StackFrame ctor.
> Then in StackFrame::GetSymbolContext we can use the m_behaves_like_zeroth_frame to fix the symbol lookup logic.
> I'm not wedded to the behaves_like_zeroth_frame nomenclature. But I think this approach makes sense.

Makes sense to me too, thanks for the pointers.  I think I'll upload that as a separate patch to keep the reviews incremental, but maybe commit them back-to-back since the second would improve any diagnostics if the first fails on other platforms.

> Anyway this looks fine to me.

Is that a green light to commit the change (modulo figuring out the issue that Jan reported which I'm not able to reproduce), or does that mean LGTY but I should get sign-off from someone else as well?  (Sorry if I'm being dense, just haven't committed changes to lldb before and want to make sure I understand the workflow, and noticed you didn't mark the Phab review as "accepted", wondering if that's significant)

Thanks!

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63667