Re: [Libunwind-devel] [PATCH 0/8] libunwind accuracy improvements for x86-64

[Andrew Cagney]

>> As in back-trace from the signal trampoline?   Single-stepping a
>> program in a signal-handler will eventually get it back to the signal
>> trampoline.  Since code trying to single-step out of a handler relies
>> on correct back-traces, it also turns out it is significant (and users
>> do notice if the back-trace is wrong and complain :-).
>
> The problem I was referring to is that unw_init_{local,remote} don't retrieve 
> any frame information for the starting frame. So if you call something other 
> than unw_step() after creating context, it cannot know what's going on. 
> Previously it didn't matter, but it matters with my patch: if you initialise 
> libunwind with address in kernel signal trampoline itself, then ask 
> unw_is_signal_frame(), you'll get "no", incorrectly. Back-tracing 
> (unw_step()) will still work just fine.

ah, yes; looking at one of the earliest e-mails where it was suggested
adding is_interrupted:

  <signal>     is_interrupted=1 is_signal=1 -- inner
  handler()    is_interrupted=1 is_signal=0
  <signal>     is_interrupted=0 is_signal=1
  foo()        is_interrupted=1 is_signal=0
  main()       is_interrupted=0 is_signal=0 -- outer

then it may not be an issue.   Here "use_prev_insn" === ! is_interrupted.

> This seems almost impossible to run into with local unwinding - it's not 
> possible for unw_getcontext() to have signal trampoline as the caller. I 
> didn't think of single stepping, so yes it might be an issue for remote 
> unwinding. But even then the consequence is that unw_is_signal_frame() will 
> incorrectly say no, but unwinding itself will work.
>
> From my perspective that seems less troublesome than the havoc with the 
> heuristic signal walker I had :-) I am happy to fix it, I just don't know how 
> to twist current architecture so unw_is_signal_frame() becomes informed about 
> current frame's dwarf info.
>
> Hm, slightly related to this, perhaps I should mention I used a different 
> default for local and remote unwinding for use_prev_instr: local defaults to 
> previous (= getcontext call itself), remote to next instruction, as the 
> latter is what I understood ptrace single stepping based context to mean. I 
> couldn't work out what the semantics for remote unwind should be.

Sounds right; the inner most frame, for ptrace, was interrupted and
therefore, use_prev_instr would be false.

Having "is_signal_frame" wrong for the inner most frame would lead to
back-trace code that relied on it not displaying something like
<signal-trampoline> for that frame.  A user would notice, for
instance, that when stepping out-of a handler and into the
signal-trampoline, that the trampoline frame's name confusingly
changes.

However, perhaps the thing to do here is accept, document, and even
deprecate is_signal_frame since it isn't reliable.  Instead,
reflecting the underlying implementation's need to do a step to the
caller and add a flag - along the lines of use_prev_instr or
is_interrupted - to indicate the state of the callee.

thoughts?

>
>>> 2. As previously discussed the changes are partially based on earlier work 
>>> in frysk. The final code isn't really the same - libunwind has changed, I 
>>> did many things differently, and code in frysk wasn't a complete solution 
>>> in my tests - but it was useful to look at the frysk version for ideas! 
>>> Sorry I forgot to add the credit in the original mail; if someone from 
>>> frysk cares they might want to ask to be added to copyright list.
>>
>> Mark Wielaard did the frysk work so perhaps a nod to him in the news
>> or somewhere?
>
> Sounds appropriate to me.
>
> Lassi
>
>