This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch 04/12] entryval: Virtual tail call frames


On Tue, 19 Jul 2011 20:17:05 +0200, Pedro Alves wrote:
> > +static void
> > +tailcall_frame_this_id (struct frame_info *this_frame, void **this_cache,
> > +                       struct frame_id *this_id)
[...]
> Should probably preserve frame_id->special_addr as well, for ia64.
+
> > +  (*this_id) = frame_id_build (this_frame_base, get_frame_pc (this_frame));

Yes, done.


> > @@ -1285,6 +1297,15 @@ dwarf2_frame_prev_register (struct frame_info *this_frame, void **this_cache,
> >    CORE_ADDR addr;
> >    int realnum;
> >  
> > +  /* Virtual tail call frames report different values only for PC.  Non-bottom
> > +     frames of a virtual tail call frames chain use
> > +     dwarf2_tailcall_frame_unwind unwinder so this code does not apply for
> > +     them.  */
> > +  if (cache->tailcall_cache && regnum == gdbarch_pc_regnum (gdbarch))
> > +    return dwarf2_tailcall_frame_unwind.prev_register (this_frame,
> > +                                                      &cache->tailcall_cache,
> > +                                                      regnum);
> > +
> 
> This looked strange.  Isn't it breaking some abstraction?
> I would have expected dwarf2_tailcall_frame_unwind.prev_register
> to be called _first_ (automatically by frame_prev_register) and then
> have that defer to dwarf2_frame_prev_register when necessary.

This is now slightly changed.  Besides simulated PC there must be also
simulated SP (for the pushed return address in tail call frames).  So that
exception has been moved into dwarf2-frame-tailcall.c now.

Or do you question the current layout?  Without this patch:
bottom frame (NORMAL_FRAME)
top frame (NORMAL_FRAME)

with this patch for the same inferior state:
bottom frame (NORMAL_FRAME unwind with patched PC+SP into tail call frame #1)
tail call frame #1 (TAILCALL_FRAME)
tail call frame #2 (TAILCALL_FRAME)
top frame (NORMAL_FRAME)

It is questionable whether "bottom frame" should be NORMAL_FRAME or
TAILCALL_FRAME.  I find it more like NORMAL_FRAME as frame is about unwinding
registers and "bottom frame" does (almost) the normal register unwind.

"tail call frame #1+2" has register set of "top frame" which suggests this
frame types layout IMO.


It will be in a new patchset resubmit.


Thanks,
Jan


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]