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

Antoine Tremblay
2016-02-24T17:57:56Z

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 ?