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

Metzger, Markus T
2013-12-16T12:42:46Z

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