Re: [patch/rfc] Add a sentinel frame

Andrew Cagney
2003-02-19T20:22:52Z

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

Oops, yes. Anyway.

The old code would `randomly' call either frame_saved_pc or frame_chain.
I mean randomly, you'd think you had it licked and then discover that
for some edge case the two calls were reversed.

The new get_prev_frame carefully orders these calls so that the sequence:

frame_pc_unwind()
then
frame_id_unwind()

always occures.

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

Dig up old notes.

This is the test you added. It stops the unwind past main:

if (next_frame->level >= 0
&& !backtrace_below_main
&& inside_main_func (get_frame_pc (next_frame)))
/* Don't unwind past main(), bug always unwind the sentinel frame.
Note, this is done _before_ the frame has been marked as
previously unwound. That way if the user later decides to
allow unwinds past main(), that just happens. */
return NULL;

It occures first (as it should). It occures before any
frame_id_unwind() as needed by frame_chain_valid. It also occures
before the test:

/* Only try to do the unwind once. */
if (next_frame->prev_p)
return next_frame->prev;
next_frame->prev_p = 1;

so that frame flush code was eliminated (ya!).

On the other hand, if GDB is to unwind past main (presumably, if
s/backtrace_below_main/unwind_past_main/ is false) it does the test:

if (inside_entry_file (get_frame_pc (next_frame)))

(note the comments about how, if this becomes optional, it should also
be moved to before `next_frame->prev_p = 1').

Anyway, now that missing test. frame_chain_valid() also contained:

/* If we're inside the entry file, it isn't valid. */
if (inside_entry_file (frame_pc_unwind (fi)))
return 0;

Note the frame_pc_unwind(). This test is looking one level along the
stack frame to determine if it should unwind to that level. That is,
when FI->prev->pc is in the entry_file, don't unwind to FI->prev. The
problem is, FI->prev->pc is in entry_file when FI->pc is in main.

Even when unwind-past-main is disabled, GDB refuses to unwind past main!
Consequently, on the branch, I dropped the test.

(It also unwinds the PC when we're probably not ready).

--

As things progress, and more targets switch to the new code, the tests
in get_prev_frame will most likely evolve. However, I don't know that
we want to be adding tests without hard evidence that they are needed :-/

Having said that, sanity checks that the frame didn't go backwards:
!frame_id_inner (frame_id, get_frame_id (next_frame))?
and that they changed:
!frame_id_eq (frame_id, get_frame_id (next_frame))?
probably wouldn't hurt.

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

If the frame isn't valid, the per architecture frame_id_unwind() returns
a null frame ID (tested using frame_id_p()). No need for that redundant
test.

Andrew