[patch rfc] generic POP_FRAME

[Andrew Cagney <ac131313 at redhat dot com>]

Hello,

As with the change replacing FRAME_SAVED_PC() with gdbarch_unwind_pc(), 
the attatched relies on there being a correctly working 
frame_register_unwind().

The patch, when there isn't a POP_FRAME() method, implements the frame 
pop by unwinding the registers of the frame being discarded and then 
writing those unwound values to the current_regcache.  In addition to 
making the code generic, this change eliminates a long standing race 
condition where the code would be trying to simultaneously read old and 
write new values into the current_regcache (outch)!

When there is a POP_FRAME() method, it does things the old way (a follow 
up patch will deprecate POP_FRAME).

Tested on d10v (which doesn't have POP_FRAME) and i386 (which does have 
POP_FRAME).

I'll look to commit this in a few days.

Andrew
2003-03-10  Andrew Cagney  <cagney at redhat dot com>

	Eliminate the need for POP_FRAME.
	* frame-unwind.h (frame_unwind_pop_ftype): Delete definition.
	(struct frame_unwind): Update.
	* frame.c (do_frame_unwind_register): New function.
	(frame_pop): When no POP_FRAME, pop the frame using register
	unwind and a scratch regcache.
	(frame_saved_regs_pop): Delete function.
	(trad_frame_unwinder): Update.
	* d10v-tdep.c (d10v_frame_pop): Delete function.
	(d10v_frame_unwind): Update.
	* sentinel-frame.c (sentinel_frame_pop): Delete function.
	(sentinel_frame_unwinder): Update.
	* dummy-frame.c (dummy_frame_pop): Delete function.
	(dummy_frame_unwind): Update.

Index: d10v-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/d10v-tdep.c,v
retrieving revision 1.85
diff -u -r1.85 d10v-tdep.c
--- d10v-tdep.c	10 Mar 2003 15:28:40 -0000	1.85
+++ d10v-tdep.c	10 Mar 2003 20:08:37 -0000
@@ -1572,45 +1572,7 @@
 }
 
 
-static void
-d10v_frame_pop (struct frame_info *fi, void **unwind_cache,
-		struct regcache *regcache)
-{
-  struct d10v_unwind_cache *info = d10v_frame_unwind_cache (fi, unwind_cache);
-  CORE_ADDR fp;
-  int regnum;
-  char raw_buffer[8];
-
-  fp = get_frame_base (fi);
-
-  /* now update the current registers with the old values */
-  for (regnum = A0_REGNUM; regnum < A0_REGNUM + NR_A_REGS; regnum++)
-    {
-      frame_unwind_register (fi, regnum, raw_buffer);
-      regcache_cooked_write (regcache, regnum, raw_buffer);
-    }
-  for (regnum = 0; regnum < SP_REGNUM; regnum++)
-    {
-      frame_unwind_register (fi, regnum, raw_buffer);
-      regcache_cooked_write (regcache, regnum, raw_buffer);
-    }
-  frame_unwind_register (fi, PSW_REGNUM, raw_buffer);
-  regcache_cooked_write (regcache, PSW_REGNUM, raw_buffer);
-
-  frame_unwind_register (fi, PC_REGNUM, raw_buffer);
-  regcache_cooked_write (regcache, PC_REGNUM, raw_buffer);
-
-  store_unsigned_integer (raw_buffer,
-			  register_size (current_gdbarch, SP_REGNUM),
-			  fp + info->size);
-  regcache_cooked_write (regcache, SP_REGNUM, raw_buffer);
-
-  target_store_registers (-1);
-  flush_cached_frames ();
-}
-
 static struct frame_unwind d10v_frame_unwind = {
-  d10v_frame_pop,
   d10v_frame_id_unwind,
   d10v_frame_register_unwind
 };
Index: dummy-frame.c
===================================================================
RCS file: /cvs/src/src/gdb/dummy-frame.c,v
retrieving revision 1.11
diff -u -r1.11 dummy-frame.c
--- dummy-frame.c	10 Mar 2003 15:28:40 -0000	1.11
+++ dummy-frame.c	10 Mar 2003 20:08:37 -0000
@@ -282,37 +282,6 @@
   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)
 {
@@ -390,7 +359,6 @@
 
 static struct frame_unwind dummy_frame_unwind =
 {
-  dummy_frame_pop,
   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.4
diff -u -r1.4 frame-unwind.h
--- frame-unwind.h	10 Mar 2003 15:28:40 -0000	1.4
+++ frame-unwind.h	10 Mar 2003 20:08:37 -0000
@@ -76,27 +76,11 @@
 				      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_id_ftype *id;
   frame_unwind_reg_ftype *reg;
 };
Index: frame.c
===================================================================
RCS file: /cvs/src/src/gdb/frame.c,v
retrieving revision 1.75
diff -u -r1.75 frame.c
--- frame.c	10 Mar 2003 15:28:40 -0000	1.75
+++ frame.c	10 Mar 2003 20:08:38 -0000
@@ -185,16 +185,44 @@
   return this_frame->pc_unwind_cache;
 }
 
+static int
+do_frame_unwind_register (void *src, int regnum, void *buf)
+{
+  frame_unwind_register (src, regnum, buf);
+  return 1;
+}
+
 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);
+  struct regcache *scratch_regcache;
+  struct cleanup *cleanups;
+
+  if (POP_FRAME_P ())
+    {
+      /* A legacy architecture that has implemented a custom pop
+	 function.  All new architectures should instead be using the
+	 generic code below.  */
+      POP_FRAME;
+    }
+  else
+    {
+      /* Make a copy of all the register values unwound from this
+	 frame.  Save them in a scratch buffer so that there isn't a
+	 race betweening trying to extract the old values from the
+	 current_regcache while, at the same time writing new values
+	 into that same cache.  */
+      struct regcache *scratch = regcache_xmalloc (current_gdbarch);
+      struct cleanup *cleanups = make_cleanup_regcache_xfree (scratch);
+      regcache_save (scratch, do_frame_unwind_register, frame);
+      /* Now copy those saved registers into the current regcache.
+         Here, regcache_cpy() calls regcache_restore().  */
+      regcache_cpy (current_regcache, scratch);
+      do_cleanups (cleanups);
+    }
+  /* We've made right mess of GDB's local state, just discard
+     everything.  */
+  target_store_registers (-1);
   flush_cached_frames ();
 }
 
@@ -768,16 +796,7 @@
   id->base = base;
 }
 	
-static void
-frame_saved_regs_pop (struct frame_info *fi, void **cache,
-		      struct regcache *regcache)
-{
-  gdb_assert (POP_FRAME_P ());
-  POP_FRAME;
-}
-
 const struct frame_unwind trad_frame_unwinder = {
-  frame_saved_regs_pop,
   frame_saved_regs_id_unwind,
   frame_saved_regs_register_unwind
 };
Index: sentinel-frame.c
===================================================================
RCS file: /cvs/src/src/gdb/sentinel-frame.c,v
retrieving revision 1.3
diff -u -r1.3 sentinel-frame.c
--- sentinel-frame.c	10 Mar 2003 15:28:41 -0000	1.3
+++ sentinel-frame.c	10 Mar 2003 20:08:39 -0000
@@ -83,17 +83,8 @@
   id->pc = read_pc ();
 }
 
-static void
-sentinel_frame_pop (struct frame_info *frame,
-		    void **cache,
-		    struct regcache *regcache)
-{
-  internal_error (__FILE__, __LINE__, "Function sentinal_frame_pop called");
-}
-
 const struct frame_unwind sentinel_frame_unwinder =
 {
-  sentinel_frame_pop,
   sentinel_frame_id_unwind,
   sentinel_frame_register_unwind
 };