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

[Joseph Tremoulet via Phabricator via lldb-commits]

JosephTremoulet added a comment.

I've updated this with code to recognize the 'S' in the eh_frame augmentation and record it in a new bool member of UnwindPlan, `m_plan_is_for_signal_trap`.  I haven't hooked up any consumers of the new bit; as you say, with the current code flow we don't parse the eh_frame info until after we've decided whether the current frame is a trap handler frame or not.

I took a look at `behaves_like_zeroth_frame`.  In my case, we're not getting to the point of consulting it, because before checking it, if we are processing a trap handler frame or if AlwaysRelyOnEHInfo returns true (both of which are true in my case), we use the EH Frame unwind plan if it is valid at the current address, and that valid at current address check is already passing.  I'm somewhat reluctant to fiddle with the behaves_like_zeroth_frame bit speculatively.  Note that with this change, it will get set for the next frame above __kernel_rt_sigreturn (because the current test checks if the next-lower frame is an eTrapHandler frame), which I think is where we want the async processing.

I see that we also do this "subtract 1 from the pc" thing in StackFrame::GetSymbolContext, and I believe this is why even with my change here I'm seeing the wrong function name in backtraces (e.g., in the new test, it shows "abort_caller -> [function that happened to precede __kernel_rt_sigreturn] -> handler" rather than "abort_caller -> kernel_rt_sigreturn -> handler").  I'm not sure how best to address that, since the base StackFrame and RegisterContext classes don't have this notion of "frame type" / "trap handler frame" that the "-LLDB" subclasses have -- seems like we'd need to have GetFrameInfoAtIndex have another out parameter to specify frame type, and UnwindLLDB's implementation could then pull it from the RegisterContextLLDB -- would that be a good change?  I don't actually need that for my current use case, the "wrong" name in the backtrace is fine...

Thanks for the notes on the asynchronous cases.  I'd certainly be interested in getting those to work, though I haven't sorted out from where the unwinder can find the above-trap frame's register values (happy for pointers here!).  I think I'm running into what this comment from GetFullUnwindPlanForFrame describes:

  // If we're in _sigtramp(), unwinding past this frame requires special
  // knowledge.  On Mac OS X this knowledge is properly encoded in the eh_frame
  // section, so prefer that if available. On other platforms we may need to
  // provide a platform-specific UnwindPlan which encodes the details of how to
  // unwind out of sigtramp.

I'd like to get the synchronous case support working as a first step regardless.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63667