Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Daniel Jacobowitz <drow@mvista.com>
To: Andrew Cagney <ac131313@redhat.com>
Cc: GDB Patches <gdb-patches@sources.redhat.com>
Subject: Re: [patch/rfc] Add a sentinel frame
Date: Wed, 19 Feb 2003 18:52:00 -0000	[thread overview]
Message-ID: <20030219185202.GA11371@nevyn.them.org> (raw)
In-Reply-To: <3E53CFB8.8070201@redhat.com>

On Wed, Feb 19, 2003 at 01:40:56PM -0500, Andrew Cagney wrote:
> >On Wed, Feb 19, 2003 at 12:51:18PM -0500, Andrew Cagney wrote:
> >
> >>
> >
> >>>>When you first committed that stuff, I warned you that would happen :-)
> >>>>The above test handled differently.
> >
> >>>
> >>>
> >>>Hey, you can't blame me for this bit.  I didn't add that check for
> >>>DEPRECATED_PC_IN_CALL_DUMMY, it was already there in
> >>>generic_frame_chain_valid.
> >
> >>
> >>I'm refering to frame_chain_valid(), a small part of which you changed. 
> >> The useful bits (your changes) were copied to the rewritten 
> >>get_prev_frame.  When frame_chain_valid() is deleted, that duplication 
> >>will go away.  To see what's wrong with frame_chain_valid() see 
> >>legacy_get_prev_frame.
> >
> >
> >I'm slow.  Could you explain the problem?  There's a comment about
> >things being deduced there which is no longer true, and a comment about
> >leaves of main that I can't make heads nor tails of but I don't think
> >it applies.
> 
> Where does one start?  it calls pc_unwind; it calls get_frame_pc; it 
> calls get_frame_base yet we passed in the frame base; it does tests in 
> the wrong order, carefully compare it to get_prev_frame; the lack of 
> frame-id; the fact that, on the sparc, the fp that is passed in was 
> bogus; knowing that all the function was ment to the general confusion 
> over unwinding the pc or fp first; knowing that frame_chain_valid() 
> started out as an equivalent to frame_id_p().

Offhand, we do _not_ pass in the frame base - we base in the base for
the next frame.  get_prev_frame makes the same get_frame_pc call.

You've lost the call to inside_entry_func.  Why?  You've changed the
inside_entry_file check to check the PC for the next frame instead of
the forthcoming frame, which is not at all the same thing.  Why?

You've lost the hook for an architecture-specific FRAME_CHAIN_VALID_P. 
I've asked you about this before and I still don't understand where you
want that logic to go.  The impression I've gotten is that you want it
to vanish, and that doesn't make any sense.

I'm not sure why the new order is better.

Duplicating the code has just made it harder for me to spot what I
consider under-explained changes in the logic.

> Contrast that to the new get_prev_frame() were everything is handled at 
> the one level.
> 
> As I said, to understand this, you're really going to have to study the 
> code.

I have, at length.  It hasn't been helping.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer


  reply	other threads:[~2003-02-19 18:52 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2003-01-23 20:54 Andrew Cagney
2003-01-27 21:42 ` Andrew Cagney
2003-02-10 23:36 ` Michal Ludvig
2003-02-11 16:48   ` Andrew Cagney
2003-02-18 11:21     ` Michal Ludvig
2003-02-19 13:27       ` Andrew Cagney
2003-02-19 14:04         ` Daniel Jacobowitz
2003-02-19 16:46           ` Andrew Cagney
2003-02-19 16:56             ` Daniel Jacobowitz
2003-02-19 17:11               ` Andrew Cagney
2003-02-19 17:17                 ` Daniel Jacobowitz
2003-02-19 17:46                   ` Andrew Cagney
2003-02-19 17:56                     ` Daniel Jacobowitz
2003-02-19 18:36                       ` Andrew Cagney
2003-02-19 18:52                         ` Daniel Jacobowitz [this message]
2003-02-19 20:22                           ` Andrew Cagney
2003-02-19 20:39                             ` Daniel Jacobowitz
2003-02-19 21:21                               ` Andrew Cagney
2003-02-20 19:32                                 ` Daniel Jacobowitz
2003-02-19 21:45                               ` Andrew Cagney
2003-02-20 19:32                                 ` Daniel Jacobowitz
2003-02-25 16:24         ` Michal Ludvig
2003-02-25 19:43           ` Andrew Cagney
2003-02-25 21:00             ` Michal Ludvig
2003-02-25 21:12               ` Andrew Cagney
2003-02-26  8:04                 ` Michal Ludvig
2003-02-27 18:27                   ` Andrew Cagney
2003-02-28 13:02                     ` Michal Ludvig
2003-02-28 15:48                       ` Andrew Cagney
2003-03-05 17:38                         ` Michal Ludvig
2003-03-05 18:25                           ` Andrew Cagney
2003-03-06 16:00                             ` Michal Ludvig
2003-03-06 20:13                               ` Andrew Cagney
2003-03-06 22:42                                 ` [RFA] Dummy frames on x86-64 Michal Ludvig
2003-03-07 14:50                                   ` Andrew Cagney
2003-02-25 22:41               ` [patch/rfc] Add a sentinel frame Andrew Cagney
2003-02-25 21:21 Michael Elizabeth Chastain

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=20030219185202.GA11371@nevyn.them.org \
    --to=drow@mvista.com \
    --cc=ac131313@redhat.com \
    --cc=gdb-patches@sources.redhat.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