[rfc] frame->register_unwind() - different way of unwinding a frame

[Andrew Cagney <ac131313 at cygnus dot com>]

Ref:
http://sources.redhat.com/ml/gdb/2002-04/msg00245.html

The attached is a revised frame->unwind and frame cache patch.  I think 
this one is looking much better - it now drops into place for existing 
targets.

It introduces two changes:


frame->register_unwind()

Per other e-mail, there is a per-frame unwind function.  Two 
implementations are provided:
	- a generic dummy frame
	- the traditional ->saved_regs frame
The method for each frame is currently selected according to the PC 
(override mechanism?).  Those functions can make use of a cacheing 
mechanism (although in the patch it is #if 0'd out).  Running 
restore.exp native suggests a 2% reduction in the system time (which 
makes sense :-).  Testing continues.

I suspect the interface to this method will evolve over time.


``info frame''

I've updated this to use the register-unwind function.  This eliminates 
the problem of using ->saved_regs to determine the location of all the 
registers.  In the case of the SP, it will now also indicate a value on 
the stack or in another register.


Doesn't show any regressions for the rs6000.


I'll have to try it on a few other targets.

Andrew
2002-04-20  Andrew Cagney  <ac131313@redhat.com>

	* stack.c (frame_info): Use frame_register_unwind instead of
	saved_regs.  Mention when the SP is on the stack or in a register.

	* frame.h (frame_register_unwind_ftype): Define.  Document.
	(struct frame_info): Add field register_unwind and
	register_unwind_cache.
	(frame_register_unwind): Declare.
	(generic_unwind_get_saved_register): Declare.

	* frame.c (frame_register_unwind): New function.
	(generic_unwind_get_saved_register): New function.

	* blockframe.c (generic_call_dummy_register_unwind): New function.
	(frame_saved_regs_register_unwind): New function.
	(set_unwind_by_pc): New function.
	(create_new_frame): New function.
	(get_prev_frame): New function.

Index: blockframe.c
===================================================================
RCS file: /cvs/src/src/gdb/blockframe.c,v
retrieving revision 1.24
diff -u -r1.24 blockframe.c
--- blockframe.c	14 Apr 2002 13:38:06 -0000	1.24
+++ blockframe.c	21 Apr 2002 04:31:37 -0000
@@ -34,9 +34,28 @@
 #include "inferior.h"		/* for read_pc */
 #include "annotate.h"
 #include "regcache.h"
+#include "gdb_assert.h"
 
 /* Prototypes for exported functions. */
 
+static void generic_call_dummy_register_unwind (struct frame_info *frame,
+						void **cache,
+						int regnum,
+						int *optimized,
+						enum lval_type *lval,
+						CORE_ADDR *addrp,
+						int *realnum,
+						void *raw_buffer);
+static void frame_saved_regs_register_unwind (struct frame_info *frame,
+					      void **cache,
+					      int regnum,
+					      int *optimized,
+					      enum lval_type *lval,
+					      CORE_ADDR *addrp,
+					      int *realnum,
+					      void *buffer);
+
+
 void _initialize_blockframe (void);
 
 /* A default FRAME_CHAIN_VALID, in the form that is suitable for most
@@ -208,6 +227,27 @@
   current_frame = frame;
 }
 
+
+/* Using the PC, select a mechanism for unwinding a frame returning
+   the previous frame.  The register unwind function should, on
+   demand, initialize the ->context object.  */
+
+static void
+set_unwind_by_pc (CORE_ADDR pc, CORE_ADDR fp,
+		  frame_register_unwind_ftype **unwind)
+{
+  if (!USE_GENERIC_DUMMY_FRAMES)
+    /* Still need to set this to something.  The ``info frame'' code
+       calls this function to find out where the saved registers are.
+       Hopefully this is robust enough to stop any core dumps and
+       return vaguely correct values..  */
+    *unwind = frame_saved_regs_register_unwind;
+  else if (PC_IN_CALL_DUMMY (pc, fp, fp))
+    *unwind = generic_call_dummy_register_unwind;
+  else
+    *unwind = frame_saved_regs_register_unwind;
+}
+
 /* Create an arbitrary (i.e. address specified by user) or innermost frame.
    Always returns a non-NULL value.  */
 
@@ -232,6 +272,9 @@
   if (INIT_EXTRA_FRAME_INFO_P ())
     INIT_EXTRA_FRAME_INFO (0, fi);
 
+  /* Select/initialize an unwind function.  */
+  set_unwind_by_pc (fi->pc, fi->frame, &fi->register_unwind);
+
   return fi;
 }
 
@@ -456,6 +499,12 @@
 	}
     }
 
+  /* Initialize the code used to unwind the frame PREV based on the PC
+     (and probably other architectural information).  The PC lets you
+     check things like the debug info at that point (dwarf2cfi?) and
+     use that to decide how the frame should be unwound.  */
+  set_unwind_by_pc (prev->pc, prev->frame, &prev->register_unwind);
+
   find_pc_partial_function (prev->pc, &name,
 			    (CORE_ADDR *) NULL, (CORE_ADDR *) NULL);
   if (IN_SIGTRAMP (prev->pc, name))
@@ -1241,6 +1290,141 @@
 			struct value **args, struct type *type, int gcc_p)
 {
   return;
+}
+
+/* Given a call-dummy dummy-frame, return the registers.  Here the
+   register value is taken from the local copy of the register buffer.  */
+
+static void
+generic_call_dummy_register_unwind (struct frame_info *frame, void **cache,
+				    int regnum, int *optimized,
+				    enum lval_type *lvalp, CORE_ADDR *addrp,
+				    int *realnum, void *bufferp)
+{
+  gdb_assert (frame != NULL);
+  gdb_assert (PC_IN_CALL_DUMMY (frame->pc, frame->frame, frame->frame));
+
+  /* Describe the register's location.  Generic dummy frames always
+     have the register value in an ``expression''.  */
+  *optimized = 0;
+  *lvalp = not_lval;
+  *addrp = 0;
+  *realnum = -1;
+
+  /* If needed, find and return the value of the register.  */
+  if (bufferp != NULL)
+    {
+      char *registers;
+#if 0
+      /* Get the address of the register buffer that contains all the
+	 saved registers for this dummy frame.  Cache that address.  */
+      registers = (*cache);
+      if (registers == NULL)
+	{
+	  registers = generic_find_dummy_frame (frame->pc, frame->frame);
+	  (*cache) = registers;
+	}
+#else
+      /* Get the address of the register buffer that contains the
+         saved registers and then extract the value from that.  */
+      registers = generic_find_dummy_frame (frame->pc, frame->frame);
+#endif
+      gdb_assert (registers != NULL);
+      /* Return the actual value.  */
+      memcpy (bufferp, registers + REGISTER_BYTE (regnum),
+	      REGISTER_RAW_SIZE (regnum));
+    }
+}
+
+/* Return the register saved in the simplistic ``saved_regs'' cache.
+   If the value isn't here AND a value is needed, try the next inner
+   most frame.  */
+
+static void
+frame_saved_regs_register_unwind (struct frame_info *frame, void **cache,
+				  int regnum, int *optimizedp,
+				  enum lval_type *lvalp, CORE_ADDR *addrp,
+				  int *realnump, void *bufferp)
+{
+  /* There is always a frame at this point.  And THIS is the frame
+     we're interested in.  */
+  gdb_assert (frame != NULL);
+  gdb_assert (!PC_IN_CALL_DUMMY (frame->pc, frame->frame, frame->frame));
+
+  /* Load the saved_regs register cache.  */
+  if (frame->saved_regs == NULL)
+    FRAME_INIT_SAVED_REGS (frame);
+
+  if (frame->saved_regs != NULL
+      && frame->saved_regs[regnum] != 0)
+    {
+      if (regnum == SP_REGNUM)
+	{
+	  /* SP register treated specially.  */
+	  *optimizedp = 0;
+	  *lvalp = not_lval;
+	  *addrp = 0;
+	  *realnump = -1;
+	  if (bufferp != NULL)
+	    store_address (bufferp, REGISTER_RAW_SIZE (regnum),
+			   frame->saved_regs[regnum]);
+	}
+      else
+	{
+	  /* Any other register is saved in memory, fetch it but cache
+             a local copy of its value.  */
+	  *optimizedp = 0;
+	  *lvalp = lval_memory;
+	  *addrp = frame->saved_regs[regnum];
+	  *realnump = -1;
+	  if (bufferp != NULL)
+	    {
+#if 0
+	      /* Save each register value, as it is read in, in a
+                 frame based cache.  */
+	      void **regs = (*cache);
+	      if (regs == NULL)
+		{
+		  int sizeof_cache = ((NUM_REGS + NUM_PSEUDO_REGS)
+				      * sizeof (void *));
+		  regs = frame_obstack_alloc (sizeof_cache);
+		  memset (regs, 0, sizeof_cache);
+		  (*cache) = regs;
+		}
+	      if (regs[regnum] == NULL)
+		{
+		  regs[regnum]
+		    = frame_obstack_alloc (REGISTER_RAW_SIZE (regnum));
+		  read_memory (frame->saved_regs[regnum], regs[regnum],
+			       REGISTER_RAW_SIZE (regnum));
+		}
+	      memcpy (bufferp, regs[regnum], REGISTER_RAW_SIZE (regnum));
+#else
+	      /* Read the value in from memory.  */
+	      read_memory (frame->saved_regs[regnum], bufferp,
+			   REGISTER_RAW_SIZE (regnum));
+#endif
+	    }
+	}
+      return;
+    }
+
+  /* No luck, assume this and the next frame have the same register
+     value.  If a value is needed, pass the request on down the chain;
+     otherwise just return an indication that the value is in the same
+     register as the next frame.  */
+  if (bufferp == NULL)
+    {
+      *optimizedp = 0;
+      *lvalp = lval_register;
+      *addrp = 0;
+      *realnump = regnum;
+    }
+  else
+    {
+      frame_register_unwind (frame->next, regnum, optimizedp, lvalp, addrp,
+			     realnump, bufferp);
+    }
 }
 
 /* Function: get_saved_register
Index: frame.c
===================================================================
RCS file: /cvs/src/src/gdb/frame.c,v
retrieving revision 1.7
diff -u -r1.7 frame.c
--- frame.c	17 Apr 2002 21:55:12 -0000	1.7
+++ frame.c	21 Apr 2002 04:31:38 -0000
@@ -1,4 +1,4 @@
-/* Cache and manage the values of registers for GDB, the GNU debugger.
+/* Cache and manage frames for GDB, the GNU debugger.
 
    Copyright 1986, 1987, 1989, 1991, 1994, 1995, 1996, 1998, 2000,
    2001, 2002 Free Software Foundation, Inc.
@@ -26,6 +26,7 @@
 #include "value.h"
 #include "inferior.h"	/* for inferior_ptid */
 #include "regcache.h"
+#include "gdb_assert.h"
 
 /* FIND_SAVED_REGISTER ()
 
@@ -157,6 +158,95 @@
     }
   if (addrp != NULL)
     *addrp = addr;
+}
+
+void
+frame_register_unwind (struct frame_info *frame, int regnum,
+		       int *optimizedp, enum lval_type *lvalp,
+		       CORE_ADDR *addrp, int *realnump, void *bufferp)
+{
+  struct frame_unwind_cache *cache;
+
+  /* Require all but BUFFERP to be valid.  A NULL BUFFERP indicates
+     that the value proper does not need to be fetched.  */
+  gdb_assert (optimizedp != NULL);
+  gdb_assert (lvalp != NULL);
+  gdb_assert (addrp != NULL);
+  gdb_assert (realnump != NULL);
+  /* gdb_assert (bufferp != NULL); */
+
+  /* NOTE: cagney/2002-04-14: It would be nice if, instead of a
+     special case, there was always an inner frame dedicated to the
+     hardware registers.  Unfortunatly, there is too much unwind code
+     around that looks up/down the frame chain while making the
+     assumption that each frame level is using the same unwind code.  */
+
+  if (frame == NULL)
+    {
+      /* We're in the inner-most frame, get the value direct from the
+	 register cache.  */
+      *optimizedp = 0;
+      *lvalp = lval_register;
+      *addrp = 0;
+      /* Should this code test ``register_cached (regnum) < 0'' and do
+         something like set realnum to -1 when the register isn't
+         available?  */
+      *realnump = regnum;
+      if (bufferp)
+	read_register_gen (regnum, bufferp);
+      return;
+    }
+
+  /* Ask this frame to unwind its register.  */
+  frame->register_unwind (frame, &frame->register_unwind_cache, regnum,
+			  optimizedp, lvalp, addrp, realnump, bufferp);
+}
+
+
+void
+generic_unwind_get_saved_register (char *raw_buffer,
+				   int *optimizedp,
+				   CORE_ADDR *addrp,
+				   struct frame_info *frame,
+				   int regnum,
+				   enum lval_type *lvalp)
+{
+  int optimizedx;
+  CORE_ADDR addrx;
+  int realnumx;
+  enum lval_type lvalx;
+
+  if (!target_has_registers)
+    error ("No registers.");
+
+  /* Keep things simple, ensure that all the pointers (except valuep)
+     are non NULL.  */
+  if (optimizedp == NULL)
+    optimizedp = &optimizedx;
+  if (lvalp == NULL)
+    lvalp = &lvalx;
+  if (addrp == NULL)
+    addrp = &addrx;
+
+  /* Reached the the bottom (youngest, inner most) of the frame chain
+     (youngest, inner most) frame, go direct to the hardware register
+     cache (do not pass go, do not try to cache the value, ...).  The
+     unwound value would have been cached in frame->next but that
+     doesn't exist.  This doesn't matter as the hardware register
+     cache is stopping any unnecessary accesses to the target.  */
+
+  /* NOTE: cagney/2002-04-14: It would be nice if, instead of a
+     special case, there was always an inner frame dedicated to the
+     hardware registers.  Unfortunatly, there is too much unwind code
+     around that looks up/down the frame chain while making the
+     assumption that each frame level is using the same unwind code.  */
+
+  if (frame == NULL)
+    frame_register_unwind (NULL, regnum, optimizedp, lvalp, addrp, &realnumx,
+			   raw_buffer);
+  else
+    frame_register_unwind (frame->next, regnum, optimizedp, lvalp, addrp,
+			   &realnumx, raw_buffer);
 }
 
 #if !defined (GET_SAVED_REGISTER)
Index: frame.h
===================================================================
RCS file: /cvs/src/src/gdb/frame.h,v
retrieving revision 1.12
diff -u -r1.12 frame.h
--- frame.h	12 Apr 2002 18:18:57 -0000	1.12
+++ frame.h	21 Apr 2002 04:31:40 -0000
@@ -23,6 +23,29 @@
 #if !defined (FRAME_H)
 #define FRAME_H 1
 
+/* Return the location (and possibly value) of REGNUM for the previous
+   (older, up) frame.  All parameters except VALUEP can be assumed to
+   be non NULL.  When VALUEP is NULL, just the location of the
+   register should be returned.
+
+   UNWIND_CACHE is provided as mechanism for implementing a per-frame
+   local cache.  It's initial value being NULL.  Memory for that cache
+   should be allocated using frame_obstack_alloc().
+
+   Register window architectures (eg SPARC) should note that REGNUM
+   identifies the register for the previous frame.  For instance, a
+   request for the value of "o1" for the previous frame would be found
+   in the register "i1" in this FRAME.  */
+
+typedef void (frame_register_unwind_ftype) (struct frame_info *frame,
+					    void **unwind_cache,
+					    int regnum,
+					    int *optimized,
+					    enum lval_type *lvalp,
+					    CORE_ADDR *addrp,
+					    int *realnump,
+					    void *valuep);
+
 /* Describe the saved registers of a frame.  */
 
 #if defined (EXTRA_FRAME_INFO) || defined (FRAME_FIND_SAVED_REGS)
@@ -112,6 +135,11 @@
        related unwind data.  */
     struct unwind_contect *context;
 
+    /* See description above.  Return the register value for the
+       previous frame.  */
+    frame_register_unwind_ftype *register_unwind;
+    void *register_unwind_cache;
+
     /* Pointers to the next (down, inner) and previous (up, outer)
        frame_info's in the frame cache.  */
     struct frame_info *next; /* down, inner */
@@ -276,6 +304,22 @@
 extern void generic_get_saved_register (char *, int *, CORE_ADDR *,
 					struct frame_info *, int,
 					enum lval_type *);
+
+extern void generic_unwind_get_saved_register (char *raw_buffer,
+					       int *optimized,
+					       CORE_ADDR * addrp,
+					       struct frame_info *frame,
+					       int regnum,
+					       enum lval_type *lval);
+
+/* Unwind the stack frame so that the value of REGNUM, in the previous
+   frame is returned.  If VALUEP is NULL, don't fetch/compute the
+   value.  Instead just return the location of the value.  */
+
+extern void frame_register_unwind (struct frame_info *frame, int regnum,
+				   int *optimizedp, enum lval_type *lvalp,
+				   CORE_ADDR *addrp, int *realnump,
+				   void *valuep);
 
 extern void get_saved_register (char *raw_buffer, int *optimized,
 				CORE_ADDR * addrp,
Index: rs6000-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/rs6000-tdep.c,v
retrieving revision 1.59
diff -u -r1.59 rs6000-tdep.c
--- rs6000-tdep.c	20 Apr 2002 03:09:28 -0000	1.59
+++ rs6000-tdep.c	21 Apr 2002 04:31:47 -0000
@@ -2655,7 +2655,7 @@
   set_gdbarch_pc_in_call_dummy (gdbarch, generic_pc_in_call_dummy);
   set_gdbarch_call_dummy_p (gdbarch, 1);
   set_gdbarch_call_dummy_stack_adjust_p (gdbarch, 0);
-  set_gdbarch_get_saved_register (gdbarch, generic_get_saved_register);
+  set_gdbarch_get_saved_register (gdbarch, generic_unwind_get_saved_register);
   set_gdbarch_fix_call_dummy (gdbarch, rs6000_fix_call_dummy);
   set_gdbarch_push_dummy_frame (gdbarch, generic_push_dummy_frame);
   set_gdbarch_save_dummy_frame_tos (gdbarch, generic_save_dummy_frame_tos);
Index: stack.c
===================================================================
RCS file: /cvs/src/src/gdb/stack.c,v
retrieving revision 1.33
diff -u -r1.33 stack.c
--- stack.c	10 Apr 2002 23:32:33 -0000	1.33
+++ stack.c	21 Apr 2002 04:31:54 -0000
@@ -918,39 +918,84 @@
       }
   }
 
-  FRAME_INIT_SAVED_REGS (fi);
-  if (fi->saved_regs != NULL)
-    {
-      /* The sp is special; what's returned isn't the save address, but
-         actually the value of the previous frame's sp.  */
-      printf_filtered (" Previous frame's sp is ");
-      print_address_numeric (fi->saved_regs[SP_REGNUM], 1, gdb_stdout);
-      printf_filtered ("\n");
-      count = 0;
-      numregs = NUM_REGS + NUM_PSEUDO_REGS;
-      for (i = 0; i < numregs; i++)
-	if (fi->saved_regs[i] && i != SP_REGNUM)
+  if (fi->saved_regs == NULL)
+    FRAME_INIT_SAVED_REGS (fi);
+  /* Print as much information as possible on the location of all the
+     registers.  */
+  {
+    enum lval_type lval;
+    int optimized;
+    CORE_ADDR addr;
+    int realnum;
+    int count;
+    int i;
+    int need_nl = 1;
+
+    /* The sp is special; what's displayed isn't the save address, but
+       the value of the previous frame's sp.  This is a legacy thing,
+       at one stage the frame cached the previous frame's SP instead
+       of its address, hence it was easiest to just display the cached
+       value.  */
+    if (SP_REGNUM >= 0)
+      {
+	/* Find out the location of the saved stack pointer with out
+           actually evaluating it.  */
+	frame_register_unwind (fi, SP_REGNUM, &optimized, &lval, &addr,
+			       &realnum, NULL);
+	if (!optimized && lval == not_lval)
 	  {
-	    if (count == 0)
-	      puts_filtered (" Saved registers:\n ");
-	    else
-	      puts_filtered (",");
-	    wrap_here (" ");
-	    printf_filtered (" %s at ", REGISTER_NAME (i));
-	    print_address_numeric (fi->saved_regs[i], 1, gdb_stdout);
-	    count++;
+	    void *value = alloca (MAX_REGISTER_RAW_SIZE);
+	    CORE_ADDR sp;
+	    frame_register_unwind (fi, SP_REGNUM, &optimized, &lval, &addr,
+				   &realnum, value);
+	    sp = extract_address (value, REGISTER_RAW_SIZE (SP_REGNUM));
+	    printf_filtered (" Previous frame's sp is ");
+	    print_address_numeric (sp, 1, gdb_stdout);
+	    printf_filtered ("\n");
+	    need_nl = 0;
 	  }
-      if (count)
-	puts_filtered ("\n");
-    }
-  else
-    {
-      /* We could get some information about saved registers by
-         calling get_saved_register on each register.  Which info goes
-         with which frame is necessarily lost, however, and I suspect
-         that the users don't care whether they get the info.  */
+	else if (!optimized && lval == lval_memory)
+	  {
+	    printf_filtered (" Previous frame's sp at ");
+	    print_address_numeric (addr, 1, gdb_stdout);
+	    printf_filtered ("\n");
+	    need_nl = 0;
+	  }
+	else if (!optimized && lval == lval_register)
+	  {
+	    printf_filtered (" Previous frame's sp in %s\n",
+			     REGISTER_NAME (realnum));
+	    need_nl = 0;
+	  }
+	/* else keep quiet.  */
+      }
+
+    count = 0;
+    numregs = NUM_REGS + NUM_PSEUDO_REGS;
+    for (i = 0; i < numregs; i++)
+      if (i != SP_REGNUM)
+	{
+	  /* Find out the location of the saved register without
+             fetching the corresponding value.  */
+	  frame_register_unwind (fi, i, &optimized, &lval, &addr, &realnum,
+				 NULL);
+	  /* For moment, only display registers that were saved on the
+	     stack.  */
+	  if (!optimized && lval == lval_memory)
+	    {
+	      if (count == 0)
+		puts_filtered (" Saved registers:\n ");
+	      else
+		puts_filtered (",");
+	      wrap_here (" ");
+	      printf_filtered (" %s at ", REGISTER_NAME (i));
+	      print_address_numeric (addr, 1, gdb_stdout);
+	      count++;
+	    }
+	}
+    if (count || need_nl)
       puts_filtered ("\n");
-    }
+  }
 }
 
 #if 0