Re: [PATCH 1/4] Teach arm unwinders to terminate gracefully

[Yao Qi <qiyaoltc at gmail dot com>]

Antoine Tremblay <antoine.tremblay@ericsson.com> writes:

Hi Antoine,

> The reason for this is that the target's stack pointer is unavailable
> when examining the trace buffer.  What we are seeing is due to the
> 'tfind' command creating a sentinel frame and unwinding it.  If an
> exception is thrown, we are left with the sentinel frame being displayed
> at level #-1.  The exception is thrown when the prologue unwinder tries
> to read the stack pointer to construct an ID for the frame.
>
> This patch fixes this and similar issues by making all the arm unwinders
> catch NOT_AVAILABLE_ERROR exceptions when either register or memory is
> unreadable and report back to the frame core code with UNWIND_UNAVAILABLE.
>
> Note this commit log adapted from 7dfa3edc033c443036d9f2a3e01120f7fb54f498
> which fixed a similar issue for aarch64.

It is right to follow aarch64 patch, but I am wondering whether we can
do it better.

Nowadays, the unwind termination due to unavailable memory is handled in
unwinders in each arch backend.  However, as we support more and more
arch for tracepoint, can we handle the unwind termination in target
independent code?

The initial work of unwind termination due to unavailable memory was
done by Pedro https://www.sourceware.org/ml/gdb-patches/2011-02/msg00611.html
in a way that each unwinder was taught to terminate with
UNWIND_UNAVAILABLE.  At that moment, only x86 supports tracepoint, so it
was reasonable to handle UNWIND_UNAVAILABLE inside unwinders of one arch.  Now,
the situation changes, because we have more and more arch need
tracepoint support, if we can handle UNWIND_UNAVAILABLE in the callers
of each unwinder, each unwinder doesn't have to worry about the
unavailable at all.  In fact, GDB has done that way when calling unwinder->sniffer,
in frame_unwind_try_unwinder

  TRY
    {
      res = unwinder->sniffer (unwinder, this_frame, this_cache);
    }
  CATCH (ex, RETURN_MASK_ERROR)
    {
      if (ex.error == NOT_AVAILABLE_ERROR)
	{
	  /* This usually means that not even the PC is available,
	     thus most unwinders aren't able to determine if they're
	     the best fit.  Keep trying.  Fallback prologue unwinders
	     should always accept the frame.  */
	  do_cleanups (old_cleanup);
	  return 0;
	}
      throw_exception (ex);
    }
  END_CATCH

we can wrap methods of 'struct frame_unwind' with try/catch, and handle
NOT_AVAILABLE_ERROR properly.  In this way, each unwinder doesn't have
to worry about unavailable memory at all.

Pedro, what do you think?  Did you try this approach in the rest of 9
different ways :) (you said you "implemented this differently in about
10 different ways" in your email) ?

-- 
Yao (éå)