[commit] Convert pop() to a per-frame method

[Andrew Cagney <ac131313 at redhat dot com>]

Hello,

The attached changes POP_FRAME() into a per-frame pop method 
frame_pop().  It keeps the old code working by providing a default that 
calls the existing POP_FRAME().

Several things to note:

- The code retains a long standing problem with the register cache.
Since the pop code is simultaneously reading/writing to the same cache, 
it is pretty easy to override a regcache entry that is needed to restore 
another register.

- The command `return' could be greatly simplified.
At present it does a separate pop for each frame back to the selected 
frame.  It could (I suspect if the SPARC is ignored) just directly pop 
the last frame.  Things to investigate one day....  Wonder if SIGTRAMP 
frames hit this theory for six.

- It might be possible to (again if the SPARC is ignored) simply 
implement pop() using a scratch register cache and the register unwind 
method.  I think this is one to worry about after all the frame rewrite 
mess has been cleaned up.  However, does anyone feel lucky?

This change also means that POP_FRAME is going to be deprecated.

Tested on i386 and PPC.

committed,
Andrew
2003-01-19  Andrew Cagney  <ac131313@redhat.com>

	* frame-unwind.h (frame_unwind_pop_ftype): Declare.
	(struct frame_unwind): Add field pop.
	* frame.h (frame_pop): Declare.
	* frame.c (frame_saved_regs_pop): New function.
	(trad_frame_unwinder): Add frame_saved_regs_pop.
	(frame_pop): New function.
	* dummy-frame.c (dummy_frame_pop): New function.
	(discard_innermost_dummy): New function.
	(generic_pop_dummy_frame): Use discard_innermost_dummy.
	(dummy_frame_unwind): Add dummy_frame_pop.
	* infrun.c (normal_stop): Call frame_pop instead of POP_FRAME.
	* valops.c (hand_function_call): Ditto.
	* stack.c (return_command): Ditto.

Index: dummy-frame.c
===================================================================
RCS file: /cvs/src/src/gdb/dummy-frame.c,v
retrieving revision 1.9
diff -u -r1.9 dummy-frame.c
--- dummy-frame.c	18 Jan 2003 17:25:23 -0000	1.9
+++ dummy-frame.c	19 Jan 2003 17:30:17 -0000
@@ -270,8 +270,48 @@
     (*popper) (frame);
 }
 
-/* Function: pop_dummy_frame
-   Restore the machine state from a saved dummy stack frame. */
+/* Discard the innermost dummy frame from the dummy frame stack
+   (passed in as a parameter).  */
+
+static void
+discard_innermost_dummy (struct dummy_frame **stack)
+{
+  struct dummy_frame *tbd = (*stack);
+  (*stack) = (*stack)->next;
+  regcache_xfree (tbd->regcache);
+  xfree (tbd);
+}
+
+/* Function: dummy_frame_pop.  Restore the machine state from a saved
+   dummy stack frame. */
+
+static void
+dummy_frame_pop (struct frame_info *fi, void **cache,
+		 struct regcache *regcache)
+{
+  struct dummy_frame *dummy = cached_find_dummy_frame (fi, cache);
+
+  /* If it isn't, what are we even doing here?  */
+  gdb_assert (get_frame_type (fi) == DUMMY_FRAME);
+
+  if (dummy == NULL)
+    error ("Can't pop dummy frame!");
+
+  /* Discard all dummy frames up-to but not including this one.  */
+  while (dummy_frame_stack != dummy)
+    discard_innermost_dummy (&dummy_frame_stack);
+
+  /* Restore this one.  */
+  regcache_cpy (regcache, dummy->regcache);
+  flush_cached_frames ();
+
+  /* Now discard it.  */
+  discard_innermost_dummy (&dummy_frame_stack);
+
+  /* Note: target changed would be better.  Registers, memory and
+     frame are all invalid.  */
+  flush_cached_frames ();
+}
 
 void
 generic_pop_dummy_frame (void)
@@ -283,12 +323,10 @@
 
   if (!dummy_frame)
     error ("Can't pop dummy frame!");
-  dummy_frame_stack = dummy_frame->next;
   regcache_cpy (current_regcache, dummy_frame->regcache);
   flush_cached_frames ();
 
-  regcache_xfree (dummy_frame->regcache);
-  xfree (dummy_frame);
+  discard_innermost_dummy (&dummy_frame_stack);
 }
 
 /* Function: fix_call_dummy
@@ -369,6 +407,7 @@
 
 static struct frame_unwind dummy_frame_unwind =
 {
+  dummy_frame_pop,
   dummy_frame_pc_unwind,
   dummy_frame_id_unwind,
   dummy_frame_register_unwind
Index: frame-unwind.h
===================================================================
RCS file: /cvs/src/src/gdb/frame-unwind.h,v
retrieving revision 1.2
diff -u -r1.2 frame-unwind.h
--- frame-unwind.h	18 Jan 2003 17:25:23 -0000	1.2
+++ frame-unwind.h	19 Jan 2003 17:30:18 -0000
@@ -82,12 +82,27 @@
 				      void **unwind_cache,
 				      struct frame_id * id);
 
+/* Discard the frame by restoring the registers (in regcache) back to
+   that of the caller.  */
+/* NOTE: cagney/2003-01-19: While at present the callers all pop each
+   frame in turn, the implementor should try to code things so that
+   any frame can be popped directly.  */
+/* FIXME: cagney/2003-01-19: Since both FRAME and REGCACHE refer to a
+   common register cache, care must be taken when restoring the
+   registers.  The `correct fix' is to first first save the registers
+   in a scratch cache, and second write that scratch cache back to to
+   the real register cache.  */
+
+typedef void (frame_unwind_pop_ftype) (struct frame_info *frame,
+				       void **unwind_cache,
+				       struct regcache *regcache);
 
 struct frame_unwind
 {
   /* Should the frame's type go here? */
   /* Should an attribute indicating the frame's address-in-block go
      here?  */
+  frame_unwind_pop_ftype *pop;
   frame_unwind_pc_ftype *pc;
   frame_unwind_id_ftype *id;
   frame_unwind_reg_ftype *reg;
Index: frame.c
===================================================================
RCS file: /cvs/src/src/gdb/frame.c,v
retrieving revision 1.59
diff -u -r1.59 frame.c
--- frame.c	18 Jan 2003 17:25:23 -0000	1.59
+++ frame.c	19 Jan 2003 17:30:32 -0000
@@ -145,6 +145,18 @@
   return frame->id_unwind_cache;
 }
 
+void
+frame_pop (struct frame_info *frame)
+{
+  /* FIXME: cagney/2003-01-18: There is probably a chicken-egg problem
+     with passing in current_regcache.  The pop function needs to be
+     written carefully so as to not overwrite registers whose [old]
+     values are needed to restore other registers.  Instead, this code
+     should pass in a scratch cache and, as a second step, restore the
+     registers using that.  */
+  frame->unwind->pop (frame, &frame->unwind_cache, current_regcache);
+  flush_cached_frames ();
+}
 
 void
 frame_register_unwind (struct frame_info *frame, int regnum,
@@ -715,7 +727,15 @@
   id->base = base;
 }
 	
+static void
+frame_saved_regs_pop (struct frame_info *fi, void **cache,
+		      struct regcache *regcache)
+{
+  POP_FRAME;
+}
+
 const struct frame_unwind trad_frame_unwinder = {
+  frame_saved_regs_pop,
   frame_saved_regs_pc_unwind,
   frame_saved_regs_id_unwind,
   frame_saved_regs_register_unwind
Index: frame.h
===================================================================
RCS file: /cvs/src/src/gdb/frame.h,v
retrieving revision 1.62
diff -u -r1.62 frame.h
--- frame.h	18 Jan 2003 17:25:23 -0000	1.62
+++ frame.h	19 Jan 2003 17:30:41 -0000
@@ -306,6 +306,10 @@
    caller's frame.  */
 extern struct frame_id frame_id_unwind (struct frame_info *frame);
 
+/* Discard the specified frame.  Restoring the registers to the state
+   of the caller.  */
+extern void frame_pop (struct frame_info *frame);
+
 /* Describe the saved registers of a frame.  */
 
 #if defined (EXTRA_FRAME_INFO) || defined (FRAME_FIND_SAVED_REGS)
Index: infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.94
diff -u -r1.94 infrun.c
--- infrun.c	14 Jan 2003 00:07:43 -0000	1.94
+++ infrun.c	19 Jan 2003 17:31:26 -0000
@@ -3109,10 +3109,10 @@
 
   if (stop_stack_dummy)
     {
-      /* Pop the empty frame that contains the stack dummy.
-         POP_FRAME ends with a setting of the current frame, so we
-         can use that next. */
-      POP_FRAME;
+      /* Pop the empty frame that contains the stack dummy.  POP_FRAME
+         ends with a setting of the current frame, so we can use that
+         next. */
+      frame_pop (get_current_frame ());
       /* Set stop_pc to what it was before we called the function.
          Can't rely on restore_inferior_status because that only gets
          called if we don't stop in the called function.  */
Index: stack.c
===================================================================
RCS file: /cvs/src/src/gdb/stack.c,v
retrieving revision 1.62
diff -u -r1.62 stack.c
--- stack.c	18 Jan 2003 15:55:52 -0000	1.62
+++ stack.c	19 Jan 2003 17:31:46 -0000
@@ -1625,6 +1625,10 @@
 	error ("Not confirmed.");
     }
 
+  /* FIXME: cagney/2003-01-18: Rather than pop each frame in turn,
+     this code should just go straight to the relevant frame and pop
+     that.  */
+
   /* Do the real work.  Pop until the specified frame is current.  We
      use this method because the deprecated_selected_frame is not valid after
      a POP_FRAME.  The pc comparison makes this work even if the
@@ -1632,11 +1636,11 @@
 
   while (selected_frame_addr != get_frame_base (frame = get_current_frame ())
 	 || selected_frame_pc != get_frame_pc (frame))
-    POP_FRAME;
+    frame_pop (get_current_frame ());
 
   /* Then pop that frame.  */
 
-  POP_FRAME;
+  frame_pop (get_current_frame ());
 
   /* Compute the return value (if any) and store in the place
      for return values.  */
@@ -1646,9 +1650,14 @@
 
   /* If we are at the end of a call dummy now, pop the dummy frame too.  */
 
+  /* FIXME: cagney/2003-01-18: This is silly.  Instead of popping all
+     the frames except the dummy, and then, as an afterthought,
+     popping the dummy frame, this code should just pop through to the
+     dummy frame.  */
+  
   if (CALL_DUMMY_HAS_COMPLETED (read_pc(), read_sp (),
 				get_frame_base (get_current_frame ())))
-    POP_FRAME;
+    frame_pop (get_current_frame ());
 
   /* If interactive, print the frame that is now current.  */
 
Index: valops.c
===================================================================
RCS file: /cvs/src/src/gdb/valops.c,v
retrieving revision 1.86
diff -u -r1.86 valops.c
--- valops.c	4 Jan 2003 22:37:47 -0000	1.86
+++ valops.c	19 Jan 2003 17:32:23 -0000
@@ -1711,8 +1711,9 @@
 	  {
 	    /* The user wants the context restored. */
 
-            /* We must get back to the frame we were before the dummy call. */
-            POP_FRAME;
+            /* We must get back to the frame we were before the dummy
+               call. */
+	    frame_pop (get_current_frame ());
 
 	    /* FIXME: Insert a bunch of wrap_here; name can be very long if it's
 	       a C++ name with arguments and stuff.  */