[patch rfc] Add the frame's type to the unwinder

[Andrew Cagney <ac131313 at redhat dot com>]

Hello,

The attached patch, moves the frame's type into `struct frame_unwind' 
and in doing so makes it possible to combine the sequence:

	prev->type = frame_type_from_pc()
	prev->unwind = frame_unwind_find_by_pc()

into a single:

	prev->unwind = frame_unwind_find_by_pc();
	// prev->type == prev->unwind->type;

and hence eliminate the the call to frame_type_from_pc().


Fine print:

While the above is true for new code, old code continues to use the same 
confused combination of frame_type_from_pc(), frame_unwind_find_by_pc(), 
deprecated_set_frame_type(), and legacy_get_prev_frame().  A legacy 
frame's type is initialized to UNKNOWN_FRAME, and then zero or more of 
those functions will override it.  get_frame_type() mapping an 
UNKNOWN_FRAME onto a NORMAL_FRAME.

Importantly, if a legacy frame's unwinder returns a known frame type 
(!UNKNOWN_FRAME) then that will be used in preference to any other 
value.  This should make it possible for old architectures to 
incrementally implement new unwinders.


Tested on d10v, ppc, and i386.  I'll commit in a few days,

Andrew
2003-04-02  Andrew Cagney  <cagney at redhat dot com>

	* frame.c (get_prev_frame): Do not call frame_type_from_pc.  Set
	the frame's type from the unwinder.
	(get_frame_type): Map UNKNOWN_FRAME onto NORMAL_FRAME.
	(create_new_frame, legacy_get_prev_frame): When the unwinder's
	type isn't UNKNOWN_FRAME, initalize "type" from the unwinder.
	(get_frame_base_address): Use get_frame_type.
	(get_frame_locals_address, get_frame_args_address): Ditto.
	(legacy_saved_regs_unwinder): Set the type to UNKNOWN_TYPE.
	* frame.h (enum frame_type): Add UNKNOWN_FRAME.
	(struct frame_info): Add comment explaining why the frame contains
	a "type" field.
	* dummy-frame.c (dummy_frame_unwind): Set the type to DUMMY_FRAME.
	* d10v-tdep.c (d10v_frame_unwind): Set the type to NORMAL_FRAME.
	* sentinel-frame.c (sentinel_frame_unwinder): Set the type to
	NORMAL_FRAME.
	* frame-unwind.h: Include "frame.h".
	(struct frame_unwind): Add "type" field.
	* Makefile.in (frame_unwind_h): Add $(frame_h).
	
Index: Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/Makefile.in,v
retrieving revision 1.357
diff -u -r1.357 Makefile.in
--- Makefile.in	2 Apr 2003 03:02:46 -0000	1.357
+++ Makefile.in	2 Apr 2003 20:39:27 -0000
@@ -641,7 +641,7 @@
 expression_h = expression.h $(symtab_h) $(doublest_h)
 f_lang_h = f-lang.h
 frame_h = frame.h
-frame_unwind_h = frame-unwind.h
+frame_unwind_h = frame-unwind.h $(frame_h)
 frame_base_h = frame-base.h
 gdb_events_h = gdb-events.h
 gdb_stabs_h = gdb-stabs.h
Index: d10v-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/d10v-tdep.c,v
retrieving revision 1.101
diff -u -r1.101 d10v-tdep.c
--- d10v-tdep.c	1 Apr 2003 19:55:03 -0000	1.101
+++ d10v-tdep.c	2 Apr 2003 20:39:27 -0000
@@ -1553,6 +1553,7 @@
 }
 
 static const struct frame_unwind d10v_frame_unwind = {
+  NORMAL_FRAME,
   d10v_frame_this_id,
   d10v_frame_prev_register
 };
Index: dummy-frame.c
===================================================================
RCS file: /cvs/src/src/gdb/dummy-frame.c,v
retrieving revision 1.15
diff -u -r1.15 dummy-frame.c
--- dummy-frame.c	31 Mar 2003 23:52:37 -0000	1.15
+++ dummy-frame.c	2 Apr 2003 20:39:27 -0000
@@ -403,6 +403,7 @@
 
 static struct frame_unwind dummy_frame_unwind =
 {
+  DUMMY_FRAME,
   dummy_frame_this_id,
   dummy_frame_prev_register
 };
Index: frame-unwind.h
===================================================================
RCS file: /cvs/src/src/gdb/frame-unwind.h,v
retrieving revision 1.6
diff -u -r1.6 frame-unwind.h
--- frame-unwind.h	17 Mar 2003 14:23:49 -0000	1.6
+++ frame-unwind.h	2 Apr 2003 20:39:27 -0000
@@ -28,6 +28,8 @@
 struct gdbarch;
 struct regcache;
 
+#include "frame.h"		/* For enum frame_type.  */
+
 /* Return the frame unwind methods for the function that contains PC,
    or NULL if this this unwinder can't handle this frame.  */
 
@@ -128,7 +130,9 @@
 
 struct frame_unwind
 {
-  /* Should the frame's type go here? */
+  /* The frame's type.  Should this instead be a collection of
+     predicates that test the frame for various attributes?  */
+  enum frame_type type;
   /* Should an attribute indicating the frame's address-in-block go
      here?  */
   frame_this_id_ftype *this_id;
Index: frame.c
===================================================================
RCS file: /cvs/src/src/gdb/frame.c,v
retrieving revision 1.93
diff -u -r1.93 frame.c
--- frame.c	1 Apr 2003 19:11:01 -0000	1.93
+++ frame.c	2 Apr 2003 20:39:28 -0000
@@ -815,6 +815,8 @@
 }
 	
 const struct frame_unwind legacy_saved_regs_unwinder = {
+  /* Not really.  It gets overridden by legacy_get_prev_frame.  */
+  UNKNOWN_FRAME,
   legacy_saved_regs_this_id,
   legacy_saved_regs_prev_register
 };
@@ -958,14 +960,18 @@
   fi->frame = addr;
   fi->pc = pc;
   fi->next = create_sentinel_frame (current_regcache);
-  fi->type = frame_type_from_pc (pc);
+
+  /* Select/initialize both the unwind function and the frame's type
+     based on the PC.  */
+  fi->unwind = frame_unwind_find_by_pc (current_gdbarch, fi->pc);
+  if (fi->unwind->type != UNKNOWN_FRAME)
+    fi->type = fi->unwind->type;
+  else
+    fi->type = frame_type_from_pc (pc);
 
   if (DEPRECATED_INIT_EXTRA_FRAME_INFO_P ())
     DEPRECATED_INIT_EXTRA_FRAME_INFO (0, fi);
 
-  /* Select/initialize an unwind function.  */
-  fi->unwind = frame_unwind_find_by_pc (current_gdbarch, fi->pc);
-
   return fi;
 }
 
@@ -1043,7 +1049,7 @@
      Unfortunatly those same work-arounds rely on the type defaulting
      to NORMAL_FRAME.  Ulgh!  The new frame code does not have this
      problem.  */
-  prev->type = NORMAL_FRAME;
+  prev->type = UNKNOWN_FRAME;
 
   /* A legacy frame's ID is always computed here.  Mark it as valid.  */
   prev->id_p = 1;
@@ -1079,10 +1085,15 @@
 				"Outermost frame - unwound PC zero\n");
 	  return NULL;
 	}
-      prev->type = frame_type_from_pc (prev->pc);
 
-      /* Set the unwind functions based on that identified PC.  */
+      /* Set the unwind functions based on that identified PC.  Ditto
+         for the "type" but strongly prefer the unwinder's frame
+         type.  */
       prev->unwind = frame_unwind_find_by_pc (current_gdbarch, prev->pc);
+      if (prev->unwind->type == UNKNOWN_FRAME)
+	prev->type = frame_type_from_pc (prev->pc);
+      else
+	prev->type = prev->unwind->type;
 
       /* Find the prev's frame's ID.  */
       if (prev->type == DUMMY_FRAME
@@ -1325,37 +1336,44 @@
      use that to decide how the frame should be unwound.  */
   prev->unwind = frame_unwind_find_by_pc (current_gdbarch, prev->pc);
 
-  /* NOTE: cagney/2002-11-18: The code segments, found in
-     create_new_frame and get_prev_frame(), that initializes the
-     frames type is subtly different.  The latter only updates ->type
-     when it encounters a SIGTRAMP_FRAME or DUMMY_FRAME.  This stops
-     get_prev_frame() overriding the frame's type when the INIT code
-     has previously set it.  This is really somewhat bogus.  The
-     initialization, as seen in create_new_frame(), should occur
-     before the INIT function has been called.  */
-  if (DEPRECATED_USE_GENERIC_DUMMY_FRAMES
-      && (DEPRECATED_PC_IN_CALL_DUMMY_P ()
-	  ? DEPRECATED_PC_IN_CALL_DUMMY (prev->pc, 0, 0)
-	  : pc_in_dummy_frame (prev->pc)))
-    prev->type = DUMMY_FRAME;
+  /* If the unwinder provides a frame type, use it.  Otherwize fall
+     back to the heuristic mess.  */
+  if (prev->unwind->type != UNKNOWN_FRAME)
+    prev->type = prev->unwind->type;
   else
     {
-      /* FIXME: cagney/2002-11-10: This should be moved to before the
-	 INIT code above so that the INIT code knows what the frame's
-	 type is (in fact, for a [generic] dummy-frame, the type can
-	 be set and then the entire initialization can be skipped.
-	 Unforunatly, its the INIT code that sets the PC (Hmm, catch
-	 22).  */
-      char *name;
-      find_pc_partial_function (prev->pc, &name, NULL, NULL);
-      if (PC_IN_SIGTRAMP (prev->pc, name))
-	prev->type = SIGTRAMP_FRAME;
-      /* FIXME: cagney/2002-11-11: Leave prev->type alone.  Some
-         architectures are forcing the frame's type in INIT so we
-         don't want to override it here.  Remember, NORMAL_FRAME == 0,
-         so it all works (just :-/).  Once this initialization is
-         moved to the start of this function, all this nastness will
-         go away.  */
+      /* NOTE: cagney/2002-11-18: The code segments, found in
+	 create_new_frame and get_prev_frame(), that initializes the
+	 frames type is subtly different.  The latter only updates
+	 ->type when it encounters a SIGTRAMP_FRAME or DUMMY_FRAME.
+	 This stops get_prev_frame() overriding the frame's type when
+	 the INIT code has previously set it.  This is really somewhat
+	 bogus.  The initialization, as seen in create_new_frame(),
+	 should occur before the INIT function has been called.  */
+      if (DEPRECATED_USE_GENERIC_DUMMY_FRAMES
+	  && (DEPRECATED_PC_IN_CALL_DUMMY_P ()
+	      ? DEPRECATED_PC_IN_CALL_DUMMY (prev->pc, 0, 0)
+	      : pc_in_dummy_frame (prev->pc)))
+	prev->type = DUMMY_FRAME;
+      else
+	{
+	  /* FIXME: cagney/2002-11-10: This should be moved to before
+	     the INIT code above so that the INIT code knows what the
+	     frame's type is (in fact, for a [generic] dummy-frame,
+	     the type can be set and then the entire initialization
+	     can be skipped.  Unforunatly, its the INIT code that sets
+	     the PC (Hmm, catch 22).  */
+	  char *name;
+	  find_pc_partial_function (prev->pc, &name, NULL, NULL);
+	  if (PC_IN_SIGTRAMP (prev->pc, name))
+	    prev->type = SIGTRAMP_FRAME;
+	  /* FIXME: cagney/2002-11-11: Leave prev->type alone.  Some
+	     architectures are forcing the frame's type in INIT so we
+	     don't want to override it here.  Remember, NORMAL_FRAME
+	     == 0, so it all works (just :-/).  Once this
+	     initialization is moved to the start of this function,
+	     all this nastness will go away.  */
+	}
     }
 
   return prev;
@@ -1561,12 +1579,19 @@
 			    "Outermost frame - unwound PC zero\n");
       return NULL;
     }
-  prev_frame->type = frame_type_from_pc (prev_frame->pc);
 
   /* Set the unwind functions based on that identified PC.  */
   prev_frame->unwind = frame_unwind_find_by_pc (current_gdbarch,
 						prev_frame->pc);
 
+  /* FIXME: cagney/2003-04-02: Rather than storing the frame's type in
+     the frame, the unwinder's type should be returned directly.
+     Unfortunatly, legacy code, called by legacy_get_prev_frame,
+     explicitly set the frames type using the method
+     deprecated_set_frame_type().  */
+  gdb_assert (prev_frame->unwind->type != UNKNOWN_FRAME);
+  prev_frame->type = prev_frame->unwind->type;
+
   /* The prev's frame's ID is computed by demand in get_frame_id().  */
 
   /* The unwound frame ID is validate at the start of this function,
@@ -1636,7 +1661,7 @@
 CORE_ADDR
 get_frame_base_address (struct frame_info *fi)
 {
-  if (fi->type != NORMAL_FRAME)
+  if (get_frame_type (fi) != NORMAL_FRAME)
     return 0;
   if (fi->base == NULL)
     fi->base = frame_base_find_by_pc (current_gdbarch, get_frame_pc (fi));
@@ -1651,7 +1676,7 @@
 get_frame_locals_address (struct frame_info *fi)
 {
   void **cache;
-  if (fi->type != NORMAL_FRAME)
+  if (get_frame_type (fi) != NORMAL_FRAME)
     return 0;
   /* If there isn't a frame address method, find it.  */
   if (fi->base == NULL)
@@ -1669,7 +1694,7 @@
 get_frame_args_address (struct frame_info *fi)
 {
   void **cache;
-  if (fi->type != NORMAL_FRAME)
+  if (get_frame_type (fi) != NORMAL_FRAME)
     return 0;
   /* If there isn't a frame address method, find it.  */
   if (fi->base == NULL)
@@ -1703,7 +1728,10 @@
   if (!DEPRECATED_USE_GENERIC_DUMMY_FRAMES
       && deprecated_frame_in_dummy (frame))
     return DUMMY_FRAME;
-  return frame->type;
+  if (frame->type == UNKNOWN_FRAME)
+    return NORMAL_FRAME;
+  else
+    return frame->type;
 }
 
 void
Index: frame.h
===================================================================
RCS file: /cvs/src/src/gdb/frame.h,v
retrieving revision 1.80
diff -u -r1.80 frame.h
--- frame.h	1 Apr 2003 19:26:52 -0000	1.80
+++ frame.h	2 Apr 2003 20:39:28 -0000
@@ -225,6 +225,11 @@
 
 enum frame_type
 {
+  /* The frame's type hasn't yet been defined.  This is a catch-all
+     for legacy code that uses really strange technicques, such as
+     deprecated_set_frame_type, to set the frame's type.  New code
+     should not use this value.  */
+  UNKNOWN_FRAME,
   /* A true stack frame, created by the target program during normal
      execution.  */
   NORMAL_FRAME,
@@ -367,6 +372,10 @@
     int level;
 
     /* The frame's type.  */
+    /* FIXME: cagney/2003-04-02: Should instead be returning
+       ->unwind->type.  Unfortunatly, legacy code is still explicitly
+       setting the type using the method deprecated_set_frame_type.
+       Eliminate that method and this field can be eliminated.  */
     enum frame_type type;
 
     /* For each register, address of where it was saved on entry to
Index: sentinel-frame.c
===================================================================
RCS file: /cvs/src/src/gdb/sentinel-frame.c,v
retrieving revision 1.5
diff -u -r1.5 sentinel-frame.c
--- sentinel-frame.c	17 Mar 2003 14:23:50 -0000	1.5
+++ sentinel-frame.c	2 Apr 2003 20:39:28 -0000
@@ -83,6 +83,8 @@
 
 const struct frame_unwind sentinel_frame_unwinder =
 {
+  /* Should the sentinel frame be given a special type?  */
+  NORMAL_FRAME,
   sentinel_frame_this_id,
   sentinel_frame_prev_register
 };