2004-03-31 Andrew Cagney <cagney@redhat.com>

	* frame.h (frame_unwind_id): Declare.
	* frame.c (frame_unwind_id): New function.
	(get_prev_frame_1): New function.
	(frame_debug_got_null_frame): New function.
	(get_prev_frame): Use frame_debug_got_null_frame.  Move unwind
	code proper to prev_frame, update description.
	* infrun.c (step_over_function): Use frame_unwind_id.

4 files changed, 168 insertions, 121 deletions

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 924a256..6ffac47 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,13 @@
+2004-03-31  Andrew Cagney  <cagney@redhat.com>
+
+	* frame.h (frame_unwind_id): Declare.
+	* frame.c (frame_unwind_id): New function.
+	(get_prev_frame_1): New function.
+	(frame_debug_got_null_frame): New function.
+	(get_prev_frame): Use frame_debug_got_null_frame.  Move unwind
+	code proper to prev_frame, update description.
+	* infrun.c (step_over_function): Use frame_unwind_id.
+
 2004-04-31  J. Brobecker  <brobecker@gnat.com>
 
 	* hppa-tdep.c (hppa32_push_dummy_call): Set the Stack Pointer.
diff --git a/gdb/frame.c b/gdb/frame.c
index f6aa5cf..d725e87 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -40,6 +40,8 @@
 #include "command.h"
 #include "gdbcmd.h"
 
+static struct frame_info *get_prev_frame_1 (struct frame_info *this_frame);
+
 /* We keep a cache of stack frames, each of which is a "struct
    frame_info".  The innermost one gets allocated (in
    wait_for_inferior) each time the inferior stops; current_frame
@@ -250,6 +252,16 @@ get_frame_id (struct frame_info *fi)
   return fi->this_id.value;
 }
 
+struct frame_id
+frame_unwind_id (struct frame_info *next_frame)
+{
+  /* Use prev_frame, and not get_prev_frame.  The latter will truncate
+     the frame chain, leading to this function unintentionally
+     returning a null_frame_id (e.g., when a caller requests the frame
+     ID of "main()"s caller.  */
+  return get_frame_id (get_prev_frame_1 (next_frame));
+}
+
 const struct frame_id null_frame_id; /* All zeros.  */
 
 struct frame_id
@@ -1720,23 +1732,22 @@ legacy_get_prev_frame (struct frame_info *this_frame)
   return prev;
 }
 
-/* Return a structure containing various interesting information
-   about the frame that called THIS_FRAME.  Returns NULL
-   if there is no such frame.
+/* Return a "struct frame_info" corresponding to the frame that called
+   THIS_FRAME.  Returns NULL if there is no such frame.
 
-   This function tests some target-independent conditions that should
-   terminate the frame chain, such as unwinding past main().  It
-   should not contain any target-dependent tests, such as checking
-   whether the program-counter is zero.  */
+   Unlike get_prev_frame, this function always tries to unwind the
+   frame.  */
 
-struct frame_info *
-get_prev_frame (struct frame_info *this_frame)
+static struct frame_info *
+get_prev_frame_1 (struct frame_info *this_frame)
 {
   struct frame_info *prev_frame;
 
+  gdb_assert (this_frame != NULL);
+
   if (frame_debug)
     {
-      fprintf_unfiltered (gdb_stdlog, "{ get_prev_frame (this_frame=");
+      fprintf_unfiltered (gdb_stdlog, "{ get_prev_frame_1 (this_frame=");
       if (this_frame != NULL)
 	fprintf_unfiltered (gdb_stdlog, "%d", this_frame->level);
       else
@@ -1744,6 +1755,136 @@ get_prev_frame (struct frame_info *this_frame)
       fprintf_unfiltered (gdb_stdlog, ") ");
     }
 
+  /* Only try to do the unwind once.  */
+  if (this_frame->prev_p)
+    {
+      if (frame_debug)
+	{
+	  fprintf_unfiltered (gdb_stdlog, "-> ");
+	  fprint_frame (gdb_stdlog, this_frame->prev);
+	  fprintf_unfiltered (gdb_stdlog, " // cached \n");
+	}
+      return this_frame->prev;
+    }
+  this_frame->prev_p = 1;
+
+  /* If any of the old frame initialization methods are around, use
+     the legacy get_prev_frame method.  */
+  if (legacy_frame_p (current_gdbarch))
+    {
+      prev_frame = legacy_get_prev_frame (this_frame);
+      return prev_frame;
+    }
+
+  /* Check that this frame's ID was valid.  If it wasn't, don't try to
+     unwind to the prev frame.  Be careful to not apply this test to
+     the sentinel frame.  */
+  if (this_frame->level >= 0 && !frame_id_p (get_frame_id (this_frame)))
+    {
+      if (frame_debug)
+	{
+	  fprintf_unfiltered (gdb_stdlog, "-> ");
+	  fprint_frame (gdb_stdlog, NULL);
+	  fprintf_unfiltered (gdb_stdlog, " // this ID is NULL }\n");
+	}
+      return NULL;
+    }
+
+  /* Check that this frame's ID isn't inner to (younger, below, next)
+     the next frame.  This happens when a frame unwind goes backwards.
+     Since the sentinel frame doesn't really exist, don't compare the
+     inner-most against that sentinel.  */
+  if (this_frame->level > 0
+      && frame_id_inner (get_frame_id (this_frame),
+			 get_frame_id (this_frame->next)))
+    error ("Previous frame inner to this frame (corrupt stack?)");
+
+  /* Check that this and the next frame are not identical.  If they
+     are, there is most likely a stack cycle.  As with the inner-than
+     test above, avoid comparing the inner-most and sentinel frames.  */
+  if (this_frame->level > 0
+      && frame_id_eq (get_frame_id (this_frame),
+		      get_frame_id (this_frame->next)))
+    error ("Previous frame identical to this frame (corrupt stack?)");
+
+  /* Allocate the new frame but do not wire it in to the frame chain.
+     Some (bad) code in INIT_FRAME_EXTRA_INFO tries to look along
+     frame->next to pull some fancy tricks (of course such code is, by
+     definition, recursive).  Try to prevent it.
+
+     There is no reason to worry about memory leaks, should the
+     remainder of the function fail.  The allocated memory will be
+     quickly reclaimed when the frame cache is flushed, and the `we've
+     been here before' check above will stop repeated memory
+     allocation calls.  */
+  prev_frame = FRAME_OBSTACK_ZALLOC (struct frame_info);
+  prev_frame->level = this_frame->level + 1;
+
+  /* Don't yet compute ->unwind (and hence ->type).  It is computed
+     on-demand in get_frame_type, frame_register_unwind, and
+     get_frame_id.  */
+
+  /* Don't yet compute the frame's ID.  It is computed on-demand by
+     get_frame_id().  */
+
+  /* The unwound frame ID is validate at the start of this function,
+     as part of the logic to decide if that frame should be further
+     unwound, and not here while the prev frame is being created.
+     Doing this makes it possible for the user to examine a frame that
+     has an invalid frame ID.
+
+     Some very old VAX code noted: [...]  For the sake of argument,
+     suppose that the stack is somewhat trashed (which is one reason
+     that "info frame" exists).  So, return 0 (indicating we don't
+     know the address of the arglist) if we don't know what frame this
+     frame calls.  */
+
+  /* Link it in.  */
+  this_frame->prev = prev_frame;
+  prev_frame->next = this_frame;
+
+  if (frame_debug)
+    {
+      fprintf_unfiltered (gdb_stdlog, "-> ");
+      fprint_frame (gdb_stdlog, prev_frame);
+      fprintf_unfiltered (gdb_stdlog, " }\n");
+    }
+
+  return prev_frame;
+}
+
+/* Debug routine to print a NULL frame being returned.  */
+
+static void
+frame_debug_got_null_frame (struct ui_file *file,
+			    struct frame_info *this_frame,
+			    const char *reason)
+{
+  if (frame_debug)
+    {
+      fprintf_unfiltered (gdb_stdlog, "{ get_prev_frame (this_frame=");
+      if (this_frame != NULL)
+	fprintf_unfiltered (gdb_stdlog, "%d", this_frame->level);
+      else
+	fprintf_unfiltered (gdb_stdlog, "<NULL>");
+      fprintf_unfiltered (gdb_stdlog, ") -> // %s}\n", reason);
+    }
+}
+
+/* Return a structure containing various interesting information about
+   the frame that called THIS_FRAME.  Returns NULL if there is entier
+   no such frame or the frame fails any of a set of target-independent
+   condition that should terminate the frame chain (e.g., as unwinding
+   past main()).
+
+   This function should not contain target-dependent tests, such as
+   checking whether the program-counter is zero.  */
+
+struct frame_info *
+get_prev_frame (struct frame_info *this_frame)
+{
+  struct frame_info *prev_frame;
+
   /* Return the inner-most frame, when the caller passes in NULL.  */
   /* NOTE: cagney/2002-11-09: Not sure how this would happen.  The
      caller should have previously obtained a valid frame using
@@ -1776,6 +1917,7 @@ get_prev_frame (struct frame_info *this_frame)
 
          Per the above, this code shouldn't even be called with a NULL
          THIS_FRAME.  */
+      frame_debug_got_null_frame (gdb_stdlog, this_frame, "this_frame NULL");
       return current_frame;
     }
 
@@ -1796,8 +1938,7 @@ get_prev_frame (struct frame_info *this_frame)
        previously unwound.  That way if the user later decides to
        allow unwinds past main(), that just happens.  */
     {
-      if (frame_debug)
-	fprintf_unfiltered (gdb_stdlog, "-> NULL // inside main func }\n");
+      frame_debug_got_null_frame (gdb_stdlog, this_frame, "inside main func");
       return NULL;
     }
 
@@ -1836,28 +1977,10 @@ get_prev_frame (struct frame_info *this_frame)
       && this_frame->type != DUMMY_FRAME && this_frame->level >= 0
       && inside_entry_func (this_frame))
     {
-      if (frame_debug)
-	{
-	  fprintf_unfiltered (gdb_stdlog, "-> ");
-	  fprint_frame (gdb_stdlog, NULL);
-	  fprintf_unfiltered (gdb_stdlog, "// inside entry func }\n");
-	}
+      frame_debug_got_null_frame (gdb_stdlog, this_frame, "inside entry func");
       return NULL;
     }
 
-  /* Only try to do the unwind once.  */
-  if (this_frame->prev_p)
-    {
-      if (frame_debug)
-	{
-	  fprintf_unfiltered (gdb_stdlog, "-> ");
-	  fprint_frame (gdb_stdlog, this_frame->prev);
-	  fprintf_unfiltered (gdb_stdlog, " // cached \n");
-	}
-      return this_frame->prev;
-    }
-  this_frame->prev_p = 1;
-
   /* If we're inside the entry file, it isn't valid.  Don't apply this
      test to a dummy frame - dummy frame PC's typically land in the
      entry file.  Don't apply this test to the sentinel frame.
@@ -1883,98 +2006,11 @@ get_prev_frame (struct frame_info *this_frame)
       && this_frame->type != DUMMY_FRAME && this_frame->level >= 0
       && deprecated_inside_entry_file (get_frame_pc (this_frame)))
     {
-      if (frame_debug)
-	{
-	  fprintf_unfiltered (gdb_stdlog, "-> ");
-	  fprint_frame (gdb_stdlog, NULL);
-	  fprintf_unfiltered (gdb_stdlog, " // inside entry file }\n");
-	}
+      frame_debug_got_null_frame (gdb_stdlog, this_frame, "inside entry file");
       return NULL;
     }
 
-  /* If any of the old frame initialization methods are around, use
-     the legacy get_prev_frame method.  */
-  if (legacy_frame_p (current_gdbarch))
-    {
-      prev_frame = legacy_get_prev_frame (this_frame);
-      return prev_frame;
-    }
-
-  /* Check that this frame's ID was valid.  If it wasn't, don't try to
-     unwind to the prev frame.  Be careful to not apply this test to
-     the sentinel frame.  */
-  if (this_frame->level >= 0 && !frame_id_p (get_frame_id (this_frame)))
-    {
-      if (frame_debug)
-	{
-	  fprintf_unfiltered (gdb_stdlog, "-> ");
-	  fprint_frame (gdb_stdlog, NULL);
-	  fprintf_unfiltered (gdb_stdlog, " // this ID is NULL }\n");
-	}
-      return NULL;
-    }
-
-  /* Check that this frame's ID isn't inner to (younger, below, next)
-     the next frame.  This happens when a frame unwind goes backwards.
-     Since the sentinel frame doesn't really exist, don't compare the
-     inner-most against that sentinel.  */
-  if (this_frame->level > 0
-      && frame_id_inner (get_frame_id (this_frame),
-			 get_frame_id (this_frame->next)))
-    error ("Previous frame inner to this frame (corrupt stack?)");
-
-  /* Check that this and the next frame are not identical.  If they
-     are, there is most likely a stack cycle.  As with the inner-than
-     test above, avoid comparing the inner-most and sentinel frames.  */
-  if (this_frame->level > 0
-      && frame_id_eq (get_frame_id (this_frame),
-		      get_frame_id (this_frame->next)))
-    error ("Previous frame identical to this frame (corrupt stack?)");
-
-  /* Allocate the new frame but do not wire it in to the frame chain.
-     Some (bad) code in INIT_FRAME_EXTRA_INFO tries to look along
-     frame->next to pull some fancy tricks (of course such code is, by
-     definition, recursive).  Try to prevent it.
-
-     There is no reason to worry about memory leaks, should the
-     remainder of the function fail.  The allocated memory will be
-     quickly reclaimed when the frame cache is flushed, and the `we've
-     been here before' check above will stop repeated memory
-     allocation calls.  */
-  prev_frame = FRAME_OBSTACK_ZALLOC (struct frame_info);
-  prev_frame->level = this_frame->level + 1;
-
-  /* Don't yet compute ->unwind (and hence ->type).  It is computed
-     on-demand in get_frame_type, frame_register_unwind, and
-     get_frame_id.  */
-
-  /* Don't yet compute the frame's ID.  It is computed on-demand by
-     get_frame_id().  */
-
-  /* The unwound frame ID is validate at the start of this function,
-     as part of the logic to decide if that frame should be further
-     unwound, and not here while the prev frame is being created.
-     Doing this makes it possible for the user to examine a frame that
-     has an invalid frame ID.
-
-     Some very old VAX code noted: [...]  For the sake of argument,
-     suppose that the stack is somewhat trashed (which is one reason
-     that "info frame" exists).  So, return 0 (indicating we don't
-     know the address of the arglist) if we don't know what frame this
-     frame calls.  */
-
-  /* Link it in.  */
-  this_frame->prev = prev_frame;
-  prev_frame->next = this_frame;
-
-  if (frame_debug)
-    {
-      fprintf_unfiltered (gdb_stdlog, "-> ");
-      fprint_frame (gdb_stdlog, prev_frame);
-      fprintf_unfiltered (gdb_stdlog, " }\n");
-    }
-
-  return prev_frame;
+  return get_prev_frame_1 (this_frame);
 }
 
 CORE_ADDR
diff --git a/gdb/frame.h b/gdb/frame.h
index 028167d..3e38c61 100644
--- a/gdb/frame.h
+++ b/gdb/frame.h
@@ -312,6 +312,7 @@ extern CORE_ADDR get_frame_base (struct frame_info *);
    frame after a frame cache flush (and other similar operations).  If
    FI is NULL, return the null_frame_id.  */
 extern struct frame_id get_frame_id (struct frame_info *fi);
+extern struct frame_id frame_unwind_id (struct frame_info *next_frame);
 
 /* Assuming that a frame is `normal', return its base-address, or 0 if
    the information isn't available.  NOTE: This address is really only
diff --git a/gdb/infrun.c b/gdb/infrun.c
index aaf6ac8..b1d03e3 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -2959,7 +2959,7 @@ step_over_function (struct execution_control_state *ecs)
 	sr_id = get_frame_id (get_current_frame ());
     }
   else
-    sr_id = get_frame_id (get_prev_frame (get_current_frame ()));
+    sr_id = frame_unwind_id (get_current_frame ());
 
   step_resume_breakpoint = set_momentary_breakpoint (sr_sal, sr_id, bp_step_resume);