[offbyone] Eliminate pop method

[Andrew Cagney <ac131313 at redhat dot com>]

(Hopefully this is the last thing for that branch)

The attatched is a proof-of-concept for using strictly generic code to 
pop stack frames.

Making the assumption that the register unwind code works (...), it pops 
a frame in two steps:

- Unwinds the frame's registers saving them in a scratch regcache
(the unwound registers are in the save_reggroup)
- Copies that scratch regcache into the targets registers
(the copied registers are in the restore_reggroup)

It works with the d10v.

It eliminates the per-frame pop method which means that a new 
traditional frame unwinder is down to just two methods.

(just don't look too closely at how I managed to do the register save, 
it pulled a few swifties :-)

I'll follow this one up next week,
Andrew

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

	* d10v-tdep.c (d10v_frame_pop): Delete function.
	(d10v_frame_unwind): Update.
	* frame.c: Include "reggroups.h".
	* regcache.c (regcache_raw_write): Allow writes to a readonly
	regcache.
	* frame.c (frame_saved_regs_pop): Delete function.
	(trad_frame_unwinder): Update
	(frame_pop): Rewrite.
	* sentinel-frame.c (sentinel_frame_pop): Delete function.
	(sentinel_frame_unwinder): Update.
	* dummy-frame.c (dummy_frame_pop): Delete function.
	(dummy_frame_unwind): Update.
	* frame-unwind.h (frame_unwind_pop_ftype): Delete definition.
	(struct frame_unwind): Update.

Index: d10v-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/d10v-tdep.c,v
retrieving revision 1.80.2.3
diff -u -r1.80.2.3 d10v-tdep.c
--- d10v-tdep.c	6 Mar 2003 19:21:30 -0000	1.80.2.3
+++ d10v-tdep.c	7 Mar 2003 14:23:18 -0000
@@ -1578,42 +1578,7 @@
 			 lvalp, addrp, realnump, bufferp);
 }
 
-static void
-d10v_frame_pop (struct frame_info *next_frame, void **this_cache,
-		struct regcache *regcache)
-{
-  struct d10v_unwind_cache *info
-    = d10v_frame_unwind_cache (next_frame, this_cache);
-  CORE_ADDR fp;
-  int regnum;
-  char raw_buffer[8];
-
-  /* now update the current registers with the old values */
-  for (regnum = A0_REGNUM; regnum < A0_REGNUM + NR_A_REGS; regnum++)
-    {
-      saved_regs_unwind (next_frame, info->saved_regs, regnum, raw_buffer);
-      regcache_cooked_write (regcache, regnum, raw_buffer);
-    }
-  for (regnum = 0; regnum < SP_REGNUM; regnum++)
-    {
-      saved_regs_unwind (next_frame, info->saved_regs, regnum, raw_buffer);
-      regcache_cooked_write (regcache, regnum, raw_buffer);
-    }
-  saved_regs_unwind (next_frame, info->saved_regs, PSW_REGNUM, raw_buffer);
-  regcache_cooked_write (regcache, PSW_REGNUM, raw_buffer);
-
-  saved_regs_unwind (next_frame, info->saved_regs, LR_REGNUM, raw_buffer);
-  regcache_cooked_write (regcache, PC_REGNUM, raw_buffer);
-
-  saved_regs_unwind (next_frame, info->saved_regs, SP_REGNUM, raw_buffer);
-  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.10.4.2
diff -u -r1.10.4.2 dummy-frame.c
--- dummy-frame.c	6 Mar 2003 19:21:30 -0000	1.10.4.2
+++ dummy-frame.c	7 Mar 2003 14:23:18 -0000
@@ -288,37 +288,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 *next_frame, void **this_cache,
-		 struct regcache *regcache)
-{
-  struct dummy_frame *dummy = cached_find_dummy_frame (next_frame, this_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)
 {
@@ -405,7 +374,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.3.6.2
diff -u -r1.3.6.2 frame-unwind.h
--- frame-unwind.h	6 Mar 2003 19:21:30 -0000	1.3.6.2
+++ frame-unwind.h	7 Mar 2003 14:23:18 -0000
@@ -102,35 +102,11 @@
 				       CORE_ADDR *addrp,
 				       int *realnump, void *valuep);
 
-/* Assuming the frame chain: (outer) prev <-> this <-> next (inner);
-   use the NEXT frame, and its register unwind method, to unwind THIS
-   frame's entire stack, writing PREV's frames register values into
-   REGCACHE.
-
-   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.
-
-   FIXME: cagney/2003-03-04: Isn't this entire function redundant?
-   Shouldn't the code instead just iterate through the restore
-   reggroup unwinding those registers?  */
-
-typedef void (frame_unwind_pop_ftype) (struct frame_info *next_frame,
-				       void **this_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.71.2.4
diff -u -r1.71.2.4 frame.c
--- frame.c	6 Mar 2003 19:21:30 -0000	1.71.2.4
+++ frame.c	7 Mar 2003 14:23:22 -0000
@@ -38,6 +38,7 @@
 #include "frame-unwind.h"
 #include "command.h"
 #include "gdbcmd.h"
+#include "reggroups.h"
 
 /* Flag to control debugging.  */
 
@@ -135,9 +136,9 @@
 }
 
 CORE_ADDR
-frame_pc_unwind (struct frame_info *next_frame)
+frame_pc_unwind (struct frame_info *this_frame)
 {
-  if (!next_frame->pc_unwind_cache_p)
+  if (!this_frame->pc_unwind_cache_p)
     {
       CORE_ADDR pc;
       if (gdbarch_unwind_pc_p (current_gdbarch))
@@ -148,7 +149,7 @@
 	     the value of this frame's PC (resume address).  A typical
 	     implementation is no more than:
 	   
-	     frame_unwind_register (next_frame, ISA_PC_REGNUM, buf);
+	     frame_unwind_register (this_frame, ISA_PC_REGNUM, buf);
 	     return extract_address (buf, size of ISA_PC_REGNUM);
 
 	     Note: this method is very heavily dependent on a correct
@@ -158,9 +159,9 @@
 	     frame.  This is all in stark contrast to the old
 	     FRAME_SAVED_PC which would try to directly handle all the
 	     different ways that a PC could be unwound.  */
-	  pc = gdbarch_unwind_pc (current_gdbarch, next_frame);
+	  pc = gdbarch_unwind_pc (current_gdbarch, this_frame);
 	}
-      else if (next_frame->level < 0)
+      else if (this_frame->level < 0)
 	{
 	  /* FIXME: cagney/2003-03-06: Old code and and a sentinel
              frame.  Do like was always done.  Fetch the PC's value
@@ -175,26 +176,52 @@
              frame.  Do like was always done.  Note that this method,
              unlike unwind_pc(), tries to handle all the different
              frame cases directly.  It fails.  */
-	  pc = FRAME_SAVED_PC (next_frame);
+	  pc = FRAME_SAVED_PC (this_frame);
 	}
       else
 	internal_error (__FILE__, __LINE__, "No gdbarch_unwind_pc method");
-      next_frame->pc_unwind_cache = pc;
-      next_frame->pc_unwind_cache_p = 1;
+      this_frame->pc_unwind_cache = pc;
+      this_frame->pc_unwind_cache_p = 1;
     }
-  return next_frame->pc_unwind_cache;
+  return this_frame->pc_unwind_cache;
 }
 
 void
-frame_pop (struct frame_info *frame)
+frame_pop (struct frame_info *this_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->next, &frame->unwind_cache, current_regcache);
+  if (POP_FRAME_P ())
+    {
+      POP_FRAME;
+    }
+  else
+    {
+      /* Note, the dummy-frame code does something very similar to
+         this.  Perhaphs a common routine is in order.  */
+      struct regcache *scratch_regcache = regcache_xmalloc (current_gdbarch);
+      struct cleanup *cleanups = make_cleanup_regcache_xfree (scratch_regcache);
+      void *buf = alloca (max_register_size (current_gdbarch));
+      int regnum;
+
+      /* Copy over any registers (identified by their membership in
+	 the save_reggroup) and mark them as valid.  The full [0
+	 .. NUM_REGS+NUM_PSEUDO_REGS) range is checked since some
+	 architectures need to save/restore `cooked' registers that
+	 live in memory.  */
+      for (regnum = 0; regnum < NUM_REGS + NUM_PSEUDO_REGS; regnum++)
+	{
+	  if (gdbarch_register_reggroup_p (current_gdbarch, regnum,
+					   save_reggroup))
+	    {
+	      frame_unwind_register (this_frame, regnum, buf);
+	      regcache_cooked_write (scratch_regcache, regnum, buf);
+	    }
+	}
+
+      /* Now write the unwound registers, en-mass, back into the
+         regcache.  */
+      regcache_cpy (current_regcache, scratch_regcache);
+      do_cleanups (cleanups);
+    }
   flush_cached_frames ();
 }
 
@@ -770,16 +797,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: regcache.c
===================================================================
RCS file: /cvs/src/src/gdb/regcache.c,v
retrieving revision 1.71
diff -u -r1.71 regcache.c
--- regcache.c	3 Mar 2003 20:50:19 -0000	1.71
+++ regcache.c	7 Mar 2003 14:23:22 -0000
@@ -929,7 +929,14 @@
 {
   gdb_assert (regcache != NULL && buf != NULL);
   gdb_assert (regnum >= 0 && regnum < regcache->descr->nr_raw_registers);
-  gdb_assert (!regcache->readonly_p);
+
+  if (regcache->readonly_p)
+    {
+      memcpy (register_buffer (regcache, regnum), buf,
+	      regcache->descr->sizeof_register[regnum]);
+      regcache->register_valid_p[regnum] = 1;
+      return;
+    }
 
   if (regcache->descr->legacy_p)
     {
Index: sentinel-frame.c
===================================================================
RCS file: /cvs/src/src/gdb/sentinel-frame.c,v
retrieving revision 1.2.6.2
diff -u -r1.2.6.2 sentinel-frame.c
--- sentinel-frame.c	6 Mar 2003 19:21:30 -0000	1.2.6.2
+++ sentinel-frame.c	7 Mar 2003 14:23:22 -0000
@@ -84,17 +84,8 @@
   internal_error (__FILE__, __LINE__, "sentinel_frame_id_unwind called");
 }
 
-static void
-sentinel_frame_pop (struct frame_info *next_frame,
-		    void **this_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
 };