This is the mail archive of the gdb-patches@sources.redhat.com 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/rfc] Add a sentinel frame


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


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