RE: [PATCH v2 3/3] btrace, frame: fix crash in get_frame_type

["Metzger, Markus T" <markus dot t dot metzger at intel dot com>]

> -----Original Message-----
> From: Pedro Alves [mailto:palves@redhat.com]
> Sent: Tuesday, February 9, 2016 11:02 PM
> To: Metzger, Markus T <markus.t.metzger@intel.com>
> Cc: gdb-patches@sourceware.org
> Subject: Re: [PATCH v2 3/3] btrace, frame: fix crash in get_frame_type
> 
> On 02/09/2016 02:42 PM, Metzger, Markus T wrote:
> 
> >>> CORE_ADDR  frame_unwind_pc (struct frame_info *this_frame)  {
> >>> +  if (this_frame == NULL)
> >>> +    throw_error (NOT_AVAILABLE_ERROR, _("PC not available"));
> >>
> >> How can this happen?
> >
> > One of its callers, frame_unwind_caller_pc, calls it with the result
> > of skip_artificial_frames like this:
> >
> > CORE_ADDR
> > frame_unwind_caller_pc (struct frame_info *this_frame) {
> >   return frame_unwind_pc (skip_artificial_frames (this_frame)); }
> >
> > Rather than handling the skip_artificial_frames() NULL return here, I
> > made frame_unwind_pc handle a NULL frame argument.
> >
> > I can move the check into frame_unwind_caller_pc if you prefer.
> 
> Yes, please.
> 
> Though, I think all these frame_unwind_caller_XXX methods should be
> consistent in how they handle skip_artificial_frames (this_frame) returning
> NULL, because they're all called together, assuming they're referring to the
> same frame.  If we throw error here, then I think we should throw in
> frame_unwind_caller_arch too, instead of having that one return the arch of
> the next frame.

get_frame_arch and frame_unwind_arch don't seem to throw any error today.
I'd rather not introduce new exceptions if not strictly necessary.  Its callers may
not be prepared to handle them.

In the absence of an arch unwinder, frame_unwind_arch uses the gdbarch of
the next frame.  And DWARF tailcall frames use the gdbarch of the bottom
non-tailcall frame.  This is consistent with the proposed changes.

We may want to return frame_unwind_arch (next_frame) instead of
get_frame_arch (next_frame).  OTOH gdb/dwarf2-frame-tailcall.c's
tailcall_frame_prev_arch returns get_frame_arch (cache->next_bottom_frame).
I'm currently mimicking that behavior.

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