public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Jan Kratochvil <jan.kratochvil@redhat.com>
To: Pedro Alves <pedro@codesourcery.com>
Cc: gdb-patches@sourceware.org
Subject: Re: [patch 04/12] entryval: Virtual tail call frames
Date: Fri, 29 Jul 2011 15:54:00 -0000	[thread overview]
Message-ID: <20110729153040.GE14528@host1.jankratochvil.net> (raw)
In-Reply-To: <201107191917.05433.pedro@codesourcery.com>

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

      reply	other threads:[~2011-07-29 15:31 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-07-18 20:19 Jan Kratochvil
2011-07-18 21:04 ` Daniel Jacobowitz
2011-07-19 14:48 ` Tom Tromey
2011-07-29 15:29   ` Jan Kratochvil
2011-07-19 18:28 ` Pedro Alves
2011-07-29 15:54   ` Jan Kratochvil [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110729153040.GE14528@host1.jankratochvil.net \
    --to=jan.kratochvil@redhat.com \
    --cc=gdb-patches@sourceware.org \
    --cc=pedro@codesourcery.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).