[patch/rfc] Add selected_frame->level

[Andrew Cagney <ac131313 at cygnus dot com>]

Hello,

The attached patch is the first shakey step towards eliminating the 
global variable selected_frame_level.

The patch makes two changes:

- adds the ``struct frame .level'' field so that each frame is self 
aware of its level within the stack (it could also be implemented as a 
function but that is another story).

- adds a complicated assertion to the select_frame() function to check 
that selected_frame_level,selected_frame->level and the level parameter 
are all consistent (or at least have expected values).

I've so far run the testsuite and not seen any regressions (or internal 
errors).  My intent is to commit this patch and then leave it for a week 
or so to see if anyone encounters problems with it.

If all appears safe I'll follow up with a second sequence of patches 
that remove the global and many many level parameters.

thoughts (especially on the strategy),
Andrew
2002-04-06  Andrew Cagney  <ac131313@redhat.com>

	* stack.c (select_frame): Check that selected_frame and the
	expected level are as expected.
	* blockframe.c (get_prev_frame): Set the `level' from next_frame.
	Update copyright.
	* frame.h (struct frame_info): Add field `level'.  Update
	copyright.

Index: blockframe.c
===================================================================
RCS file: /cvs/src/src/gdb/blockframe.c,v
retrieving revision 1.22
diff -u -r1.22 blockframe.c
--- blockframe.c	5 Apr 2002 22:04:40 -0000	1.22
+++ blockframe.c	7 Apr 2002 00:36:36 -0000
@@ -1,7 +1,9 @@
-/* Get info from stack frames;
-   convert between frames, blocks, functions and pc values.
-   Copyright 1986, 1987, 1988, 1989, 1990, 1991, 1992, 1993, 1994, 1995,
-   1996, 1997, 1998, 1999, 2000, 2001 Free Software Foundation, Inc.
+/* Get info from stack frames; convert between frames, blocks,
+   functions and pc values.
+
+   Copyright 1986, 1987, 1988, 1989, 1990, 1991, 1992, 1993, 1994,
+   1995, 1996, 1997, 1998, 1999, 2000, 2001, 2002 Free Software
+   Foundation, Inc.
 
    This file is part of GDB.
 
@@ -393,6 +395,7 @@
     next_frame->prev = prev;
   prev->next = next_frame;
   prev->frame = address;
+  prev->level = next_frame->level + 1;
 
 /* This change should not be needed, FIXME!  We should
    determine whether any targets *need* INIT_FRAME_PC to happen
Index: frame.h
===================================================================
RCS file: /cvs/src/src/gdb/frame.h,v
retrieving revision 1.10
diff -u -r1.10 frame.h
--- frame.h	5 Apr 2002 22:04:41 -0000	1.10
+++ frame.h	7 Apr 2002 00:36:37 -0000
@@ -1,6 +1,7 @@
 /* Definitions for dealing with stack frames, for GDB, the GNU debugger.
-   Copyright 1986, 1988, 1989, 1990, 1991, 1992, 1993, 1994, 1996, 1997,
-   1998, 1999, 2000, 2001 Free Software Foundation, Inc.
+
+   Copyright 1986, 1988, 1989, 1990, 1991, 1992, 1993, 1994, 1996,
+   1997, 1998, 1999, 2000, 2001, 2002 Free Software Foundation, Inc.
 
    This file is part of GDB.
 
@@ -62,6 +63,17 @@
        For the innermost frame, it's the current pc.
        For other frames, it is a pc saved in the next frame.  */
     CORE_ADDR pc;
+
+    /* Level of this frame.  The inner-most (youngest) frame is at
+       level 0.  As you move towards the outer-most (oldest) frame,
+       the level increases.  This is a cached value.  It could just as
+       easily be computed by counting back from the selected frame to
+       the inner most frame.  */
+    /* NOTE: cagney/2002-04-05: Perhaphs a level of ``-1'' should be
+       reserved to indicate a bogus frame - one that has been created
+       just to keep GDB happy (GDB always needs a frame).  For the
+       moment leave this as speculation.  */
+    int level;
 
     /* Nonzero if this is a frame associated with calling a signal handler.
 
Index: stack.c
===================================================================
RCS file: /cvs/src/src/gdb/stack.c,v
retrieving revision 1.31
diff -u -r1.31 stack.c
--- stack.c	5 Apr 2002 22:04:41 -0000	1.31
+++ stack.c	7 Apr 2002 00:39:40 -0000
@@ -1458,6 +1459,38 @@
 
   selected_frame = fi;
   selected_frame_level = level;
+  /* FIXME: cagney/2002-04-05: It can't be this easy (and looking at
+     the increasingly complex list of checkes, it wasn't)!  GDB is
+     dragging around, and constantly updating, the global variable
+     selected_frame_level.  Surely all that was needed was for the
+     level to be computed direct from the frame (by counting back to
+     the inner-most frame) or, as has been done here using a cached
+     value.  For moment, check that the expected and the actual level
+     are consistent.  If, after a few weeks, no one reports that this
+     assertion has failed, the global selected_frame_level and many
+     many parameters can all be deleted.  */
+  if (fi == NULL && level == -1)
+    /* Ok.  The target is clearing the selected frame as part of a
+       cache flush.  */
+    ;
+  else if (fi != NULL && fi->level == level)
+    /* Ok.  What you would expect.  Level is redundant.  */
+    ;
+  else if (fi != NULL && level == -1)
+    /* Ok.  See breakpoint.c.  The watchpoint code changes the
+       selected frame to the frame that contains the watchpoint and
+       then, later changes it back to the old value.  The -1 is used
+       as a marker so that the watchpoint code can easily detect that
+       things are not what they should be.  Why the watchpoint code
+       can't mindlessly save/restore the selected frame I don't know,
+       hopefully it can be simplified that way.  Hopefully the global
+       selected_frame can be replaced by a frame parameter, making
+       still more simplification possible.  */
+    ;
+  else
+    internal_error (__FILE__, __LINE__,
+		    "oops! fi=0x%p, fi->level=%d, level=%d",
+		    fi, fi ? fi->level : -1, level);
   if (selected_frame_level_changed_hook)
     selected_frame_level_changed_hook (level);