Re: frame unwinding patches

[Mark Wielaard <mark@klomp.org>]

On Tue, 2017-04-11 at 12:16 +0200, Ulf Hermann wrote:
> > I do agree with Jan that frame pointer unwinding is notoriously
> > untrustworthy. Even with some sanity checks it is hard to know whether
> > you are doing a correct unwind. gdb gets away with it through pretty
> > advanced frame sniffers, which take a lot of low-level compiler
> > generation knowledge to get correct (and even then...). You only restore
> > the pc, stack and frame pointer registers (maybe incorrectly). So
> > afterwards, even if you got valid CFI data you might be stuck.
> 
> Yes, especially with mixed stack traces, where part of the stack has
> CFI and part of it doesn't, we quickly run into guesswork. I've
> regenerated the binaries as suggested, with the result being that
> raise() from libc actually has CFI, but doesn't set a frame pointer.
> So, the frame pointer unwinder can find raise() in the link register,
> but it sets up the FP register with the wrong value. Then raise() is
> unwound using CFI, which mixes up the registers some more. At that
> point we're lost. I don't see an easy way out of this.

That might just mean that the testcase is slightly unrealistic.
Getting a reliable backtrace through signal handlers when not having
full CFI is probably not something we can expect to work. That doesn't
mean having a frame pointer based fallback is a bad thing. We probably
should find a more realistic testcase. And maybe in the future add an
interface to allow people to unwind through "pure CFI" or mixed mode
with frame markers that tell the caller whether the registers can be
trusted or not.

> I will keep a version of the frame unwinding for perfparser as it's
> still better than not unwinding at all, but I do understand that it's
> not really suitable for mainline elfutils.

Really I do think it would be nice to have. I certainly didn't mean it
isn't suitable for mainline. We just should be realistic about the
expectations. IMHO if at all possible we should get this upstream so you
don't have to carry extra patches.

Cheers,

Mark