RE: [PATCH] stack: fix gdb.dwarf2/dw2-undefined-ret-addr.exp regression

["Metzger, Markus T" <markus.t.metzger@intel.com>]

> -----Original Message-----
> From: Yao Qi [mailto:qiyaoltc@gmail.com]
> Sent: Thursday, October 6, 2016 11:32 AM
> To: Metzger, Markus T <markus.t.metzger@intel.com>
> Cc: gdb-patches@sourceware.org; qiyaoltc@gmail.com; palves@redhat.com
> Subject: Re: [PATCH] stack: fix gdb.dwarf2/dw2-undefined-ret-addr.exp
> regression

Hi Yao,

Thanks for your review.


> It is good to me.  I read this patch several times since July, because
> of something else in a038fa3e14a4.  Do we need to move
> frame_unwind_caller_id into try block? like this,
> 
>   TRY
>    {
>       if (!frame_id_p (frame_unwind_caller_id (fi)))
>          val_print_unavailable (gdb_stdout);
>       else
>        {
>           caller_pc = frame_unwind_caller_pc (fi);
>           caller_pc_p = 1;
>        }
>    }
>   CATCH (ex, ....)
> 
> frame_unwind_caller_id calls get_prev_frame_always which only catches
> MEMORY_ERROR.  IIUC, get_prev_frame_always and it callees may throw
> NOT_AVAILABLE_ERROR.

I didn't say that get_prev_frame_always may throw NOT_AVAILABLE_ERROR.
I said that I would treat the case where the trace ends with tailcall frames without
an actual caller frame like NOT_AVAILABLE_ERROR (and this patch is changing this
now to OPTIMIZED_OUT_ERROR).

The problem was that get_prev_frame_always may return NULL, which
skip_artificial_frames did not handle, causing GDB to crash.  We didn't really have
such a case before, but with record-btrace, you could end up with a bunch of
tailcall frames where the actual caller is not contained in the trace.

Pedro suggested that:  "
All the frame_unwind_caller_XXX methods should retrieve information
about the same frame that frame_unwind_caller_id returns.  They should
all really be guarded by a frame_unwind_caller_id check, like:

      if (frame_id_p (frame_unwind_caller_id (frame)))
"

As to whether the callers of frame_unwind_caller_id need to expect an exception,
I can't say for sure but I don't think it likely.  There are a few such checks in GDB
already and none of them seems to expect an exception.

Regards,
Markus.
Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928