* dummy-frame.h (dummy_frame_pop): Add prototype.

	* dummy-frame.c: Include "observer.h".
	(dummy_frame_push): Do not check for stale frames.
	(dummy_frame_pop): New function.
	(cleanup_dummy_frames): New function.
	(_initialize_dummy_frame): Install it as inferior_created observer.

	* frame.h (struct frame_id): Update comments.
	(frame_id_inner): Remove prototype.
	* frame.c (frame_id_inner): Make static.  Add comments.
	(frame_find_by_id): Update frame_id_inner safety net check to avoid
	false positives for targets using non-contiguous stack ranges.
	(get_prev_frame_1): Update frame_id_inner safety net check.
	(frame_pop): Call dummy_frame_pop when popping a dummy frame.

	* stack.c (return_command): Directly pop the selected frame.
	* infrun.c (handle_inferior_event): Remove dead code.
	* i386-tdep.c (i386_push_dummy_call): Update comment.

8 files changed, 124 insertions, 109 deletions

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 0477f24f67c..2270a1c6edf 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,26 @@
 2008-08-26  Ulrich Weigand  <uweigand@de.ibm.com>
 
+	* dummy-frame.h (dummy_frame_pop): Add prototype.
+	* dummy-frame.c: Include "observer.h".
+	(dummy_frame_push): Do not check for stale frames.
+	(dummy_frame_pop): New function.
+	(cleanup_dummy_frames): New function.
+	(_initialize_dummy_frame): Install it as inferior_created observer.
+
+	* frame.h (struct frame_id): Update comments.
+	(frame_id_inner): Remove prototype.
+	* frame.c (frame_id_inner): Make static.  Add comments.
+	(frame_find_by_id): Update frame_id_inner safety net check to avoid
+	false positives for targets using non-contiguous stack ranges.
+	(get_prev_frame_1): Update frame_id_inner safety net check.
+	(frame_pop): Call dummy_frame_pop when popping a dummy frame.
+
+	* stack.c (return_command): Directly pop the selected frame.
+	* infrun.c (handle_inferior_event): Remove dead code.
+	* i386-tdep.c (i386_push_dummy_call): Update comment.
+
+2008-08-26  Ulrich Weigand  <uweigand@de.ibm.com>
+
 	* breakpoint.c (remove_breakpoint): Do not fail if unable to remove
 	breakpoint from shared library.
 
diff --git a/gdb/dummy-frame.c b/gdb/dummy-frame.c
index 4c044ef3df4..a27de2e28f7 100644
--- a/gdb/dummy-frame.c
+++ b/gdb/dummy-frame.c
@@ -30,6 +30,7 @@
 #include "command.h"
 #include "gdbcmd.h"
 #include "gdb_string.h"
+#include "observer.h"
 
 /* Dummy frame.  This saves the processor state just prior to setting
    up the inferior function call.  Older targets save the registers
@@ -87,26 +88,8 @@ void
 dummy_frame_push (struct regcache *caller_regcache,
 		  const struct frame_id *dummy_id)
 {
-  struct gdbarch *gdbarch = get_regcache_arch (caller_regcache);
   struct dummy_frame *dummy_frame;
 
-  /* Check to see if there are stale dummy frames, perhaps left over
-     from when a longjump took us out of a function that was called by
-     the debugger.  */
-  dummy_frame = dummy_frame_stack;
-  while (dummy_frame)
-    /* FIXME: cagney/2004-08-02: Should just test IDs.  */
-    if (frame_id_inner (gdbarch, dummy_frame->id, (*dummy_id)))
-      /* Stale -- destroy!  */
-      {
-	dummy_frame_stack = dummy_frame->next;
-	regcache_xfree (dummy_frame->regcache);
-	xfree (dummy_frame);
-	dummy_frame = dummy_frame_stack;
-      }
-    else
-      dummy_frame = dummy_frame->next;
-
   dummy_frame = XZALLOC (struct dummy_frame);
   dummy_frame->regcache = caller_regcache;
   dummy_frame->id = (*dummy_id);
@@ -114,6 +97,47 @@ dummy_frame_push (struct regcache *caller_regcache,
   dummy_frame_stack = dummy_frame;
 }
 
+/* Pop the dummy frame with ID dummy_id from the dummy-frame stack.  */
+
+void
+dummy_frame_pop (struct frame_id dummy_id)
+{
+  struct dummy_frame **dummy_ptr;
+
+  for (dummy_ptr = &dummy_frame_stack;
+       (*dummy_ptr) != NULL;
+       dummy_ptr = &(*dummy_ptr)->next)
+    {
+      struct dummy_frame *dummy = *dummy_ptr;
+      if (frame_id_eq (dummy->id, dummy_id))
+	{
+	  *dummy_ptr = dummy->next;
+	  regcache_xfree (dummy->regcache);
+	  xfree (dummy);
+	  break;
+	}
+    }
+}
+
+/* There may be stale dummy frames, perhaps left over from when a longjump took us
+   out of a function that was called by the debugger.  Clean them up at least once
+   whenever we start a new inferior.  */
+
+static void
+cleanup_dummy_frames (struct target_ops *target, int from_tty)
+{
+  struct dummy_frame *dummy, *next;
+
+  for (dummy = dummy_frame_stack; dummy; dummy = next)
+    {
+      next = dummy->next;
+      regcache_xfree (dummy->regcache);
+      xfree (dummy);
+    }
+
+  dummy_frame_stack = NULL;
+}
+
 /* Return the dummy frame cache, it contains both the ID, and a
    pointer to the regcache.  */
 struct dummy_frame_cache
@@ -258,4 +282,5 @@ _initialize_dummy_frame (void)
 	   _("Print the contents of the internal dummy-frame stack."),
 	   &maintenanceprintlist);
 
+  observer_attach_inferior_created (cleanup_dummy_frames);
 }
diff --git a/gdb/dummy-frame.h b/gdb/dummy-frame.h
index b299b6dd9c8..f7e05e6a1f4 100644
--- a/gdb/dummy-frame.h
+++ b/gdb/dummy-frame.h
@@ -41,6 +41,8 @@ struct frame_id;
 extern void dummy_frame_push (struct regcache *regcache,
 			      const struct frame_id *dummy_id);
 
+extern void dummy_frame_pop (struct frame_id dummy_id);
+
 /* If the PC falls in a dummy frame, return a dummy frame
    unwinder.  */
 
diff --git a/gdb/frame.c b/gdb/frame.c
index c4f85fe9c8a..55ded716825 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -368,7 +368,33 @@ frame_id_eq (struct frame_id l, struct frame_id r)
   return eq;
 }
 
-int
+/* Safety net to check whether frame ID L should be inner to
+   frame ID R, according to their stack addresses.
+
+   This method cannot be used to compare arbitrary frames, as the
+   ranges of valid stack addresses may be discontiguous (e.g. due
+   to sigaltstack).
+
+   However, it can be used as safety net to discover invalid frame
+   IDs in certain circumstances.
+
+   * If frame NEXT is the immediate inner frame to THIS, and NEXT
+     is a NORMAL frame, then the stack address of NEXT must be
+     inner-than-or-equal to the stack address of THIS.
+
+     Therefore, if frame_id_inner (THIS, NEXT) holds, some unwind
+     error has occurred.
+
+   * If frame NEXT is the immediate inner frame to THIS, and NEXT
+     is a NORMAL frame, and NEXT and THIS have different stack
+     addresses, no other frame in the frame chain may have a stack
+     address in between.
+
+     Therefore, if frame_id_inner (TEST, THIS) holds, but
+     frame_id_inner (TEST, NEXT) does not hold, TEST cannot refer
+     to a valid frame in the frame chain.   */
+
+static int
 frame_id_inner (struct gdbarch *gdbarch, struct frame_id l, struct frame_id r)
 {
   int inner;
@@ -395,28 +421,34 @@ frame_id_inner (struct gdbarch *gdbarch, struct frame_id l, struct frame_id r)
 struct frame_info *
 frame_find_by_id (struct frame_id id)
 {
-  struct frame_info *frame;
+  struct frame_info *frame, *prev_frame;
 
   /* ZERO denotes the null frame, let the caller decide what to do
      about it.  Should it instead return get_current_frame()?  */
   if (!frame_id_p (id))
     return NULL;
 
-  for (frame = get_current_frame ();
-       frame != NULL;
-       frame = get_prev_frame (frame))
+  for (frame = get_current_frame (); ; frame = prev_frame)
     {
       struct frame_id this = get_frame_id (frame);
       if (frame_id_eq (id, this))
 	/* An exact match.  */
 	return frame;
-      if (frame_id_inner (get_frame_arch (frame), id, this))
-	/* Gone to far.  */
+
+      prev_frame = get_prev_frame (frame);
+      if (!prev_frame)
+	return NULL;
+
+      /* As a safety net to avoid unnecessary backtracing while trying
+	 to find an invalid ID, we check for a common situation where
+	 we can detect from comparing stack addresses that no other
+	 frame in the current frame chain can have this ID.  See the
+	 comment at frame_id_inner for details.   */
+      if (get_frame_type (frame) == NORMAL_FRAME
+	  && !frame_id_inner (get_frame_arch (frame), id, this)
+	  && frame_id_inner (get_frame_arch (prev_frame), id,
+			     get_frame_id (prev_frame)))
 	return NULL;
-      /* Either we're not yet gone far enough out along the frame
-         chain (inner(this,id)), or we're comparing frameless functions
-         (same .base, different .func, no test available).  Struggle
-         on until we've definitly gone to far.  */
     }
   return NULL;
 }
@@ -517,6 +549,11 @@ frame_pop (struct frame_info *this_frame)
   scratch = frame_save_as_regcache (prev_frame);
   cleanups = make_cleanup_regcache_xfree (scratch);
 
+  /* If we are popping a dummy frame, clean up the associated
+     data as well.  */
+  if (get_frame_type (this_frame) == DUMMY_FRAME)
+    dummy_frame_pop (get_frame_id (this_frame));
+
   /* FIXME: cagney/2003-03-16: It should be possible to tell the
      target's register cache that it is about to be hit with a burst
      register transfer and that the sequence of register writes should
@@ -1207,11 +1244,10 @@ get_prev_frame_1 (struct frame_info *this_frame)
 
   /* Check that this frame's ID isn't inner to (younger, below, next)
      the next frame.  This happens when a frame unwind goes backwards.
-     Exclude signal trampolines (due to sigaltstack the frame ID can
-     go backwards) and sentinel frames (the test is meaningless).  */
-  if (this_frame->next->level >= 0
-      && this_frame->next->unwind->type != SIGTRAMP_FRAME
-      && frame_id_inner (get_frame_arch (this_frame), this_id,
+     This check is valid only if the next frame is NORMAL.  See the
+     comment at frame_id_inner for details.  */
+  if (this_frame->next->unwind->type == NORMAL_FRAME
+      && frame_id_inner (get_frame_arch (this_frame->next), this_id,
 			 get_frame_id (this_frame->next)))
     {
       if (frame_debug)
diff --git a/gdb/frame.h b/gdb/frame.h
index 1441e122282..231aa32e355 100644
--- a/gdb/frame.h
+++ b/gdb/frame.h
@@ -111,7 +111,7 @@ struct frame_id
      frames that do not change the stack but are still distinct and have 
      some form of distinct identifier (e.g. the ia64 which uses a 2nd 
      stack for registers).  This field is treated as unordered - i.e. will
-     not be used in frame ordering comparisons such as frame_id_inner().
+     not be used in frame ordering comparisons.
 
      This field is valid only if special_addr_p is true.  Otherwise, this
      frame is considered to have a wildcard special address, i.e. one that
@@ -124,22 +124,7 @@ struct frame_id
   unsigned int special_addr_p : 1;
 };
 
-/* Methods for constructing and comparing Frame IDs.
-
-   NOTE: Given stackless functions A and B, where A calls B (and hence
-   B is inner-to A).  The relationships: !eq(A,B); !eq(B,A);
-   !inner(A,B); !inner(B,A); all hold.
-
-   This is because, while B is inner-to A, B is not strictly inner-to A.  
-   Being stackless, they have an identical .stack_addr value, and differ 
-   only by their unordered .code_addr and/or .special_addr values.
-
-   Because frame_id_inner is only used as a safety net (e.g.,
-   detect a corrupt stack) the lack of strictness is not a problem.
-   Code needing to determine an exact relationship between two frames
-   must instead use frame_id_eq and frame_id_unwind.  For instance,
-   in the above, to determine that A stepped-into B, the equation
-   "A.id != B.id && A.id == id_unwind (B)" can be used.  */
+/* Methods for constructing and comparing Frame IDs.  */
 
 /* For convenience.  All fields are zero.  */
 extern const struct frame_id null_frame_id;
@@ -176,12 +161,6 @@ extern int frame_id_p (struct frame_id l);
    either L or R have a zero .func, then the same frame base.  */
 extern int frame_id_eq (struct frame_id l, struct frame_id r);
 
-/* Returns non-zero when L is strictly inner-than R (they have
-   different frame .bases).  Neither L, nor R can be `null'.  See note
-   above about frameless functions.  */
-extern int frame_id_inner (struct gdbarch *gdbarch, struct frame_id l,
-			   struct frame_id r);
-
 /* Write the internal representation of a frame ID on the specified
    stream.  */
 extern void fprint_frame_id (struct ui_file *file, struct frame_id id);
diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index 048454aae39..3e215fce294 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -1712,8 +1712,8 @@ i386_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
      (i386_frame_this_id, i386_sigtramp_frame_this_id,
      i386_dummy_id).  It's there, since all frame unwinders for
      a given target have to agree (within a certain margin) on the
-     definition of the stack address of a frame.  Otherwise
-     frame_id_inner() won't work correctly.  Since DWARF2/GCC uses the
+     definition of the stack address of a frame.  Otherwise frame id
+     comparison might not work correctly.  Since DWARF2/GCC uses the
      stack address *before* the function call as a frame's CFA.  On
      the i386, when %ebp is used as a frame pointer, the offset
      between the contents %ebp and the CFA as defined by GCC.  */
diff --git a/gdb/infrun.c b/gdb/infrun.c
index d6c49ab1391..2d31219901d 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -3323,33 +3323,6 @@ infrun: BPSTAT_WHAT_SET_LONGJMP_RESUME (!gdbarch_get_longjmp_target)\n");
   tss->current_line = stop_pc_sal.line;
   tss->current_symtab = stop_pc_sal.symtab;
 
-  /* In the case where we just stepped out of a function into the
-     middle of a line of the caller, continue stepping, but
-     step_frame_id must be modified to current frame */
-#if 0
-  /* NOTE: cagney/2003-10-16: I think this frame ID inner test is too
-     generous.  It will trigger on things like a step into a frameless
-     stackless leaf function.  I think the logic should instead look
-     at the unwound frame ID has that should give a more robust
-     indication of what happened.  */
-  if (step - ID == current - ID)
-    still stepping in same function;
-  else if (step - ID == unwind (current - ID))
-    stepped into a function;
-  else
-    stepped out of a function;
-  /* Of course this assumes that the frame ID unwind code is robust
-     and we're willing to introduce frame unwind logic into this
-     function.  Fortunately, those days are nearly upon us.  */
-#endif
-  {
-    struct frame_info *frame = get_current_frame ();
-    struct frame_id current_frame = get_frame_id (frame);
-    if (!(frame_id_inner (get_frame_arch (frame), current_frame,
-			  step_frame_id)))
-      step_frame_id = current_frame;
-  }
-
   if (debug_infrun)
      fprintf_unfiltered (gdb_stdlog, "infrun: keep going\n");
   keep_going (ecs);
diff --git a/gdb/stack.c b/gdb/stack.c
index d9c4f0a239a..61c799da526 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -1844,29 +1844,8 @@ If you continue, the return value that you specified will be ignored.\n";
 	error (_("Not confirmed"));
     }
 
-  /* NOTE: cagney/2003-01-18: Is this silly?  Rather than pop each
-     frame in turn, should this code just go straight to the relevant
-     frame and pop that?  */
-
-  /* First discard all frames inner-to the selected frame (making the
-     selected frame current).  */
-  {
-    struct frame_id selected_id = get_frame_id (get_selected_frame (NULL));
-    while (!frame_id_eq (selected_id, get_frame_id (get_current_frame ())))
-      {
-	struct frame_info *frame = get_current_frame ();
-	if (frame_id_inner (get_frame_arch (frame), selected_id,
-			    get_frame_id (frame)))
-	  /* Caught in the safety net, oops!  We've gone way past the
-             selected frame.  */
-	  error (_("Problem while popping stack frames (corrupt stack?)"));
-	frame_pop (get_current_frame ());
-      }
-  }
-
-  /* Second discard the selected frame (which is now also the current
-     frame).  */
-  frame_pop (get_current_frame ());
+  /* Discard the selected frame and all frames inner-to it.  */
+  frame_pop (get_selected_frame (NULL));
 
   /* Store RETURN_VALUE in the just-returned register set.  */
   if (return_value != NULL)