infrun.c:restore_selected_frame???

[Andrew Cagney]

>> Selected_frame_level is relative (0 is inner most) not absolute.  The above patch demonstrates this.
> 
> 
> If this is right, the way it is set (and the comment) at the beginning of select_frame is confusing:
> 
> /* Select frame FI, and note that its stack level is LEVEL.
>    LEVEL may be -1 if an actual level number is not known.  */
> 
> void
> select_frame (struct frame_info *fi, int level)
> {
>   register struct symtab *s;
> 
>   selected_frame = fi;
>   selected_frame_level = level;

I'm pretty sure it is.   My patch did two things;

- adds frame->level
- checks frame->level against selected_frame_level and throws an 
internal error if they clash

I then ran the testsuite until gdb stopped having internal errors.  As 
best I can tell, LEVEL is either fi->level or -1.  While probably not 
the best way, it was the only way I could think of to identify all cases :-)

I think LEVEL parameter is going to become redundant.

> Sounds like if I have a hold of the frame_info, it is sufficient to pass this and -1 in, but that is not right... 

Sounds like your existing patch to use frame->frame is best for now.  At 
least that way the correct level is restored.

> This is a collision with the varobj code.  varobj_create stashes away the current frame - in case the varobj that you are creating is in another frame, and then restores it by calling:
> 
>           select_frame (fi, -1);
> 
> According to the header comment for select frame, this is reasonable, because -1 is supposed to mean "I don't care about the level, but of course this doesn't work when you try to do restore_selected_frame with -1 from frame 0...

Ah!  Outch!

> We do this in Project Builder, but there we do it because a GUI has to present the WHOLE stack frame every time you step, which is slow, so we optimize this by sending frame+pc duples to PB using a special purpose command that just gets these and doesn't reconstruct the whole stack.
> 
> Don't know if you need this in gdb though.

If it hurts one GUI it is going to hurt others.

I think it might be better to try to modify GDB's frame code so that it 
isn't as heavy handed (fewer target accesses) when constructing the 
frame chain.  At present GDB typically fully analyzes one frame before 
going to construct a basic skeleton of the next one :-(

 >   When did you think this might be a problem?

I'm wondering if there can be false positives.  Consider a sequence like:

	o	in base
	o	save base's frame address
	o	do an inferior call to bar()
		which long-jumps over the return
		breakpoint
	o	eventually you hit a breakpoint
		in a random function

There is a chance that the frame will match base's frame giving a false 
positive.  I guess that is always there but the chance is so low it 
isn't worth worrying about.

enjoy,
Andrew