[PATCH] D64993: Fix PC adjustment in StackFrame::GetSymbolContext

[Joseph Tremoulet via Phabricator via llvm-commits]

JosephTremoulet marked an inline comment as done.
JosephTremoulet added inline comments.


================
Comment at: lldb/source/Plugins/Process/Utility/RegisterContextLLDB.cpp:1760
+void RegisterContextLLDB::PropagateTrapHandlerFlag(
+    lldb::UnwindPlanSP unwind_plan) {
+  if (unwind_plan->GetUnwindPlanForSignalTrap() != eLazyBoolYes) {
----------------
jasonmolenda wrote:
> clayborg wrote:
> > JosephTremoulet wrote:
> > > I'm a bit ambivalent about adding this part -- the downside is that it's not concretely helping today, because if an eh_frame record has the 'S' flag in its augmentation (which is what `unwind_Plan->GetUnwindPlanForSignalTrap()` reports), we'll have already decremented pc and generated unwind plans based on the prior function when initializing the register context.  But the upside is that this connects the dots between the otherwise-unused bit on the unwind plan and the frame type, and will be in place should we subsequently add code to try the second function's unwind plan as a fallback.
> > I will let Jason comment on this one.
> Yeah, this was my impression of the S augmentation flag in the eh_frame too -- we can't really use it in lldb today without forcing a scan of eh_frame entries the first time we unwind a function from that Module, and that would be unfortunate.  But I like to see the flag being parsed and recorded; at some point in the future we may find a good way to use it.
> Yeah, this was my impression of the S augmentation flag in the eh_frame too -- we can't really use it in lldb today without forcing a scan of eh_frame entries the first time we unwind a function from that Module

"Thinking out lout" a bit... we could consider doing the check only when the pc is at the very start of a function -- in that case, either it's one of these signal frames where it's worth paying the price to check eh_frame, or it's a call at the end of the previous function... am I understanding correctly that the non-signal case is specifically when a noreturn non-tail call gets laid out at the end of a function and there's no alignment padding between it and the next function?  I'd naively think that's an uncommon case, is there reason to believe otherwise?  I don't think this will be a high enough priority for me to actually investigate any time soon, but if I were going to investigate, do we have some standard test suite for assessing that sort of thing (either statically how often that code pattern happens or dynamically what slowdown is observed from a particular lldb source change)?

Thanks!


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64993