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

[Antoine Tremblay <antoine.tremblay@ericsson.com>]

Yao Qi writes:

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

Ping, Pedro ?