RE: [patch v8 21/24] record-btrace: extend unwinder

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

> -----Original Message-----
> From: Pedro Alves [mailto:palves@redhat.com]
> Sent: Friday, December 13, 2013 8:45 PM


> > @@ -1511,6 +1512,11 @@ dwarf2_frame_base_sniffer (struct frame_info
> *this_frame)
> >  CORE_ADDR
> >  dwarf2_frame_cfa (struct frame_info *this_frame)
> >  {
> > +  if (frame_unwinder_is (this_frame,
> &record_btrace_tailcall_frame_unwind)
> > +      || frame_unwinder_is (this_frame, &record_btrace_frame_unwind))
> > +    throw_error (NOT_AVAILABLE_ERROR,
> > +		 _("cfa not available for record btrace target"));
> 
> Kind of odd abstraction boundaries broken here.  On the one hand,
> we go through the target for the unwind sniffing, but OTOH, here
> we refer to the unwinders directly.  Hmm.
> 
> Ah, I don't think it really matters -- if you convert
> to the frame_info.stack_status == FID_STACK_UNAVAILABLE
> mechanism, then this can check for that instead, and the
> get_frame_unwind_stop_reason check can even be eliminated.
> Perhaps it should be even get_frame_base the one that
> throws.

We need that check before checking for dwarf2 unwinders.
Get_frame_base is called later but only for dwarf2 unwinders.

I will try to replace the check with a check for stack availability.


> >  /* Level of the selected frame: 0 for innermost, 1 for its caller, ...
> > diff --git a/gdb/frame.h b/gdb/frame.h
> > index 71f07dd..b20b69f 100644
> > --- a/gdb/frame.h
> > +++ b/gdb/frame.h
> > @@ -222,7 +222,11 @@ enum frame_type
> >    ARCH_FRAME,
> >    /* Sentinel or registers frame.  This frame obtains register values
> >       direct from the inferior's registers.  */
> > -  SENTINEL_FRAME
> > +  SENTINEL_FRAME,
> > +  /* A branch tracing frame.  */
> > +  BTRACE_FRAME,
> > +  /* A branch tracing tail call frame.  */
> > +  BTRACE_TAILCALL_FRAME
> >  };
> 
> Hmm?  Why were these needed?  Missing patch rationale...  And missing
> comments, I suppose.

The alternative would be to pretend that they are NORMAL_FRAME
and TAILCALL_FRAME, respectively.  I thought it cleaner to add a new
frame type.  Those frames don't have available stack.

Do you want me to use NORMAL_FRAME and TAILCALL_FRAME, instead?

Regards,
Markus.

Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052