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

[Lassi Tuura]

Hi,

>> 1. The new signal frame detection code has a potential issue. The dwarf 
>> frame information is only available after calling unw_step(), so if you 
>> chance to set things up so you start unwinding at the kernel signal return 
>> frame, it will get confused. I don't know how to fix this without major 
>> change how unw_init_* work relative to unw_is_signal_frame(). I don't think 
>> it's significant, as chances of constructing such a stack frame are slim; I 
>> don't think it's possible with local unwind at all, and I don't even know 
>> how to create the situation with remote (ptrace) unwinding.
> 
> 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.

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.

>> 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