[patch 1/3] Remove stale dummy frames (gdb2495.exp regression)

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

Hi,

this is extended variant of:
	[patch 1/2] Fix gdb.cp/gdb2495.exp regression with gcc-4.7 #5
	http://sourceware.org/ml/gdb-patches/2012-03/msg00357.html

I am not completely sure if the new functions should be placed to breakpoint.c
or dummy-frame.c.  The belong more to dummy-frame.c but they would have
difficulties accessing private things from breakpoint.c.

stale-infcall.exp FAILs with FSF GDB HEAD only partially, the fix prevents
functionality regression of stale-infcall.c that would happen with ON_STACK
with [patch 3/3].

No regressions on {x86_64,x86_64-m32,i686}-fedora17-linux-gnu.


Thanks,
Jan


gdb/
2012-06-13  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Remove stale dummy frames.
	* breakpoint.c: Include dummy-frame.h.
	(longjmp_breakpoint_ops): New variable.
	(update_breakpoints_after_exec, breakpoint_init_inferior): Delete also
	bp_longjmp_call_dummy.
	(bpstat_what, bptype_string, print_one_breakpoint_location)
	(init_bp_location): Support bp_longjmp_call_dummy.
	(set_longjmp_breakpoint): Use longjmp_breakpoint_ops.  Comment why.
	(set_longjmp_breakpoint_for_call_dummy)
	(check_longjmp_breakpoint_for_call_dummy, longjmp_bkpt_dtor): New
	functions.
	(initialize_breakpoint_ops): Initialize longjmp_breakpoint_ops.
	* breakpoint.h (enum bptype): New item bp_longjmp_call_dummy.  Delete
	FIXME comment and extend the other comment for bp_call_dummy.
	(set_longjmp_breakpoint_for_call_dummy)
	(check_longjmp_breakpoint_for_call_dummy): New declarations.
	* dummy-frame.c: Include gdbthread.h.
	(pop_dummy_frame_bpt): New function.
	(pop_dummy_frame): Call pop_dummy_frame_bpt.
	(dummy_frame_discard): New function.
	(cleanup_dummy_frames): Update the comment about longjmps.
	* dummy-frame.h (dummy_frame_discard): New declaration.
	* gdbthread.h (struct thread_info): Extend initiating_frame comment.
	* infcall.c (call_function_by_hand): New variable longjmp_b.  Call
	set_longjmp_breakpoint_for_call_dummy.  Chain its breakpoints with BPT.
	* infrun.c (handle_inferior_event) <BPSTAT_WHAT_CLEAR_LONGJMP_RESUME>:
	Add case 4 comment.  Call check_longjmp_breakpoint_for_call_dummy and
	keep_going if IS_LONGJMP and there is no other reason to stop.

Gdb/testsuite/
2012-06-13  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Remove stale dummy frames.
	* gdb.base/call-signal-resume.exp (maintenance print dummy-frames)
	(maintenance info breakpoints): New tests.
	* gdb.base/stale-infcall.c: New file.
	* gdb.base/stale-infcall.exp: New file.

--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -68,6 +68,7 @@
 #include "skip.h"
 #include "gdb_regex.h"
 #include "ax-gdb.h"
+#include "dummy-frame.h"
 
 /* readline include files */
 #include "readline/readline.h"
@@ -287,6 +288,9 @@ static struct breakpoint_ops internal_breakpoint_ops;
 /* Momentary breakpoints class type.  */
 static struct breakpoint_ops momentary_breakpoint_ops;
 
+/* Momentary breakpoints for bp_longjmp and bp_exception class type.  */
+static struct breakpoint_ops longjmp_breakpoint_ops;
+
 /* The breakpoint_ops structure to be used in regular user created
    breakpoints.  */
 struct breakpoint_ops bkpt_breakpoint_ops;
@@ -3148,6 +3152,7 @@ update_breakpoints_after_exec (void)
     /* Longjmp and longjmp-resume breakpoints are also meaningless
        after an exec.  */
     if (b->type == bp_longjmp || b->type == bp_longjmp_resume
+	|| b->type == bp_longjmp_call_dummy
 	|| b->type == bp_exception || b->type == bp_exception_resume)
       {
 	delete_breakpoint (b);
@@ -3439,6 +3444,7 @@ breakpoint_init_inferior (enum inf_context context)
     switch (b->type)
       {
       case bp_call_dummy:
+      case bp_longjmp_call_dummy:
 
 	/* If the call dummy breakpoint is at the entry point it will
 	   cause problems when the inferior is rerun, so we better get
@@ -5098,9 +5104,10 @@ bpstat_what (bpstat bs_head)
 	    }
 	  break;
 	case bp_longjmp:
+	case bp_longjmp_call_dummy:
 	case bp_exception:
 	  this_action = BPSTAT_WHAT_SET_LONGJMP_RESUME;
-	  retval.is_longjmp = bptype == bp_longjmp;
+	  retval.is_longjmp = bptype != bp_exception;
 	  break;
 	case bp_longjmp_resume:
 	case bp_exception_resume:
@@ -5433,6 +5440,7 @@ bptype_string (enum bptype type)
     {bp_access_watchpoint, "acc watchpoint"},
     {bp_longjmp, "longjmp"},
     {bp_longjmp_resume, "longjmp resume"},
+    {bp_longjmp_call_dummy, "longjmp for call dummy"},
     {bp_exception, "exception"},
     {bp_exception_resume, "exception resume"},
     {bp_step_resume, "step resume"},
@@ -5575,6 +5583,7 @@ print_one_breakpoint_location (struct breakpoint *b,
       case bp_finish:
       case bp_longjmp:
       case bp_longjmp_resume:
+      case bp_longjmp_call_dummy:
       case bp_exception:
       case bp_exception_resume:
       case bp_step_resume:
@@ -6438,6 +6447,7 @@ init_bp_location (struct bp_location *loc, const struct bp_location_ops *ops,
     case bp_finish:
     case bp_longjmp:
     case bp_longjmp_resume:
+    case bp_longjmp_call_dummy:
     case bp_exception:
     case bp_exception_resume:
     case bp_step_resume:
@@ -6741,8 +6751,10 @@ set_longjmp_breakpoint (struct thread_info *tp, struct frame_id frame)
 	enum bptype type = b->type == bp_longjmp_master ? bp_longjmp : bp_exception;
 	struct breakpoint *clone;
 
+	/* longjmp_breakpoint_ops ensures INITIATING_FRAME is cleared again
+	   after their removal.  */
 	clone = momentary_breakpoint_from_master (b, type,
-						  &momentary_breakpoint_ops);
+						  &longjmp_breakpoint_ops);
 	clone->thread = thread;
       }
 
@@ -6776,6 +6788,75 @@ delete_longjmp_breakpoint_at_next_stop (int thread)
       }
 }
 
+/* Place breakpoints of type bp_longjmp_call_dummy to catch longjmp for
+   INFERIOR_PTID thread.  Chain them all by RELATED_BREAKPOINT and return
+   pointer to any of them.  Return NULL if this system cannot place longjmp
+   breakpoints.  */
+
+struct breakpoint *
+set_longjmp_breakpoint_for_call_dummy (void)
+{
+  struct breakpoint *b, *retval = NULL;
+
+  ALL_BREAKPOINTS (b)
+    if (b->pspace == current_program_space && b->type == bp_longjmp_master)
+      {
+	struct breakpoint *new_b;
+
+	new_b = momentary_breakpoint_from_master (b, bp_longjmp_call_dummy,
+						  &momentary_breakpoint_ops);
+	new_b->thread = pid_to_thread_id (inferior_ptid);
+
+	/* Link NEW_B into the chain of RETVAL breakpoints.  */
+
+	gdb_assert (new_b->related_breakpoint == new_b);
+	if (retval == NULL)
+	  retval = new_b;
+	new_b->related_breakpoint = retval;
+	while (retval->related_breakpoint != new_b->related_breakpoint)
+	  retval = retval->related_breakpoint;
+	retval->related_breakpoint = new_b;
+      }
+
+  return retval;
+}
+
+/* Verify all existing dummy frames and their associated breakpoints for
+   THREAD.  Remove those which can no longer be found in the current frame
+   stack.
+
+   You should call this function only at places where it is safe to currently
+   unwind the whole stack.  Failed stack unwind would discard live dummy
+   frames.  */
+
+void
+check_longjmp_breakpoint_for_call_dummy (int thread)
+{
+  struct breakpoint *b, *b_tmp;
+
+  ALL_BREAKPOINTS_SAFE (b, b_tmp)
+    if (b->type == bp_longjmp_call_dummy && b->thread == thread)
+      {
+	struct breakpoint *dummy_b = b->related_breakpoint;
+
+	while (dummy_b != b && dummy_b->type != bp_call_dummy)
+	  dummy_b = dummy_b->related_breakpoint;
+	if (dummy_b->type != bp_call_dummy
+	    || frame_find_by_id (dummy_b->frame_id) != NULL)
+	  continue;
+	
+	dummy_frame_discard (dummy_b->frame_id);
+
+	while (b->related_breakpoint != b)
+	  {
+	    if (b_tmp == b->related_breakpoint)
+	      b_tmp = b->related_breakpoint->next;
+	    delete_breakpoint (b->related_breakpoint);
+	  }
+	delete_breakpoint (b);
+      }
+}
+
 void
 enable_overlay_breakpoints (void)
 {
@@ -12765,6 +12846,22 @@ momentary_bkpt_print_mention (struct breakpoint *b)
   /* Nothing to mention.  These breakpoints are internal.  */
 }
 
+/* Ensure INITIATING_FRAME is cleared when no such breakpoint exists.
+
+   It gets cleared already on the removal of the first one of such placed
+   breakpoints.  This is OK as they get all removed altogether.  */
+
+static void
+longjmp_bkpt_dtor (struct breakpoint *self)
+{
+  struct thread_info *tp = find_thread_id (self->thread);
+
+  if (tp)
+    tp->initiating_frame = null_frame_id;
+
+  momentary_breakpoint_ops.dtor (self);
+}
+
 /* Specific methods for probe breakpoints.  */
 
 static int
@@ -15354,6 +15451,11 @@ initialize_breakpoint_ops (void)
   ops->print_it = momentary_bkpt_print_it;
   ops->print_mention = momentary_bkpt_print_mention;
 
+  /* Momentary breakpoints for bp_longjmp and bp_exception.  */
+  ops = &longjmp_breakpoint_ops;
+  *ops = momentary_breakpoint_ops;
+  ops->dtor = longjmp_bkpt_dtor;
+
   /* Probe breakpoints.  */
   ops = &bkpt_probe_breakpoint_ops;
   *ops = bkpt_breakpoint_ops;
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -64,6 +64,12 @@ enum bptype
     bp_longjmp,			/* secret breakpoint to find longjmp() */
     bp_longjmp_resume,		/* secret breakpoint to escape longjmp() */
 
+    /* Breakpoint placed to the same location(s) like bp_longjmp but used to
+       protect against stale DUMMY_FRAME.  Multiple bp_longjmp_call_dummy and
+       one bp_call_dummy are chained together by related_breakpoint for each
+       DUMMY_FRAME.  */
+    bp_longjmp_call_dummy,
+
     /* An internal breakpoint that is installed on the unwinder's
        debug hook.  */
     bp_exception,
@@ -93,14 +99,8 @@ enum bptype
        3) It can never be disabled.  */
     bp_watchpoint_scope,
 
-    /* The breakpoint at the end of a call dummy.  */
-    /* FIXME: What if the function we are calling longjmp()s out of
-       the call, or the user gets out with the "return" command?  We
-       currently have no way of cleaning up the breakpoint in these
-       (obscure) situations.  (Probably can solve this by noticing
-       longjmp, "return", etc., it's similar to noticing when a
-       watchpoint on a local variable goes out of scope (with hardware
-       support for watchpoints)).  */
+    /* The breakpoint at the end of a call dummy.  See bp_longjmp_call_dummy it
+       is chained with by related_breakpoint.  */
     bp_call_dummy,
 
     /* A breakpoint set on std::terminate, that is used to catch
@@ -1287,6 +1287,9 @@ extern void delete_longjmp_breakpoint (int thread);
 /* Mark all longjmp breakpoints from THREAD for later deletion.  */
 extern void delete_longjmp_breakpoint_at_next_stop (int thread);
 
+extern struct breakpoint *set_longjmp_breakpoint_for_call_dummy (void);
+extern void check_longjmp_breakpoint_for_call_dummy (int thread);
+
 extern void enable_overlay_breakpoints (void);
 extern void disable_overlay_breakpoints (void);
 
--- a/gdb/dummy-frame.c
+++ b/gdb/dummy-frame.c
@@ -29,6 +29,7 @@
 #include "gdbcmd.h"
 #include "gdb_string.h"
 #include "observer.h"
+#include "gdbthread.h"
 
 /* Dummy frame.  This saves the processor state just prior to setting
    up the inferior function call.  Older targets save the registers
@@ -108,19 +109,44 @@ remove_dummy_frame (struct dummy_frame **dummy_ptr)
   xfree (dummy);
 }
 
+/* Delete any breakpoint B which is a momentary breakpoint for return from
+   inferior call matching DUMMY_VOIDP.  */
+
+static int
+pop_dummy_frame_bpt (struct breakpoint *b, void *dummy_voidp)
+{
+  struct dummy_frame *dummy = dummy_voidp;
+
+  if (b->thread == pid_to_thread_id (inferior_ptid)
+      && b->disposition == disp_del && frame_id_eq (b->frame_id, dummy->id))
+    {
+      while (b->related_breakpoint != b)
+	delete_breakpoint (b->related_breakpoint);
+
+      delete_breakpoint (b);
+
+      /* Stop the traversal.  */
+      return 1;
+    }
+
+  /* Continue the traversal.  */
+  return 0;
+}
+
 /* Pop *DUMMY_PTR, restoring program state to that before the
    frame was created.  */
 
 static void
 pop_dummy_frame (struct dummy_frame **dummy_ptr)
 {
-  struct dummy_frame *dummy;
+  struct dummy_frame *dummy = *dummy_ptr;
+
+  restore_infcall_suspend_state (dummy->caller_state);
 
-  restore_infcall_suspend_state ((*dummy_ptr)->caller_state);
+  iterate_over_breakpoints (pop_dummy_frame_bpt, dummy);
 
   /* restore_infcall_control_state frees inf_state,
      all that remains is to pop *dummy_ptr.  */
-  dummy = *dummy_ptr;
   *dummy_ptr = dummy->next;
   xfree (dummy);
 
@@ -166,9 +192,22 @@ dummy_frame_pop (struct frame_id dummy_id)
   pop_dummy_frame (dp);
 }
 
-/* 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.  */
+/* Drop dummy frame DUMMY_ID.  Do nothing if it is not found.  Do not restore
+   its state into inferior, just free its memory.  */
+
+void
+dummy_frame_discard (struct frame_id dummy_id)
+{
+  struct dummy_frame **dp;
+
+  dp = lookup_dummy_frame (dummy_id);
+  if (dp)
+    remove_dummy_frame (dp);
+}
+
+/* There may be stale dummy frames, perhaps left over from when an uncaught
+   longjmp 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)
--- a/gdb/dummy-frame.h
+++ b/gdb/dummy-frame.h
@@ -52,6 +52,8 @@ extern void dummy_frame_push (struct infcall_suspend_state *caller_state,
 
 extern void dummy_frame_pop (struct frame_id dummy_id);
 
+extern void dummy_frame_discard (struct frame_id dummy_id);
+
 /* If the PC falls in a dummy frame, return a dummy frame
    unwinder.  */
 
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -216,7 +216,9 @@ struct thread_info
   int stop_requested;
 
   /* The initiating frame of a nexting operation, used for deciding
-     which exceptions to intercept.  */
+     which exceptions to intercept.  If it is null_frame_id no
+     bp_longjmp or bp_exception but longjmp has been caught just for
+     bp_longjmp_call_dummy.  */
   struct frame_id initiating_frame;
 
   /* Private data used by the target vector implementation.  */
--- a/gdb/infcall.c
+++ b/gdb/infcall.c
@@ -744,7 +744,7 @@ call_function_by_hand (struct value *function, int nargs, struct value **args)
      inferior.  That way it breaks when it returns.  */
 
   {
-    struct breakpoint *bpt;
+    struct breakpoint *bpt, *longjmp_b;
     struct symtab_and_line sal;
 
     init_sal (&sal);		/* initialize to zeroes */
@@ -760,6 +760,17 @@ call_function_by_hand (struct value *function, int nargs, struct value **args)
     frame = NULL;
 
     bpt->disposition = disp_del;
+    gdb_assert (bpt->related_breakpoint == bpt);
+
+    longjmp_b = set_longjmp_breakpoint_for_call_dummy ();
+    if (longjmp_b)
+      {
+	/* Link BPT into the chain of LONGJMP_B.  */
+	bpt->related_breakpoint = longjmp_b;
+	while (longjmp_b->related_breakpoint != bpt->related_breakpoint)
+	  longjmp_b = longjmp_b->related_breakpoint;
+	longjmp_b->related_breakpoint = bpt;
+      }
   }
 
   /* Create a breakpoint in std::terminate.
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -4438,18 +4438,34 @@ process_event_stop_test:
 	     3. The initiating frame exists and is different from the
 	     current frame.  This means the exception or longjmp has
 	     been caught beneath the initiating frame, so keep
-	     going.  */
+	     going.
+
+	     4. longjmp breakpoint has been placed just to protect
+	     against stale dummy frames and user is not interested in
+	     stopping around longjmps.  */
 
 	  if (debug_infrun)
 	    fprintf_unfiltered (gdb_stdlog,
 				"infrun: BPSTAT_WHAT_CLEAR_LONGJMP_RESUME\n");
 
-	  init_frame = frame_find_by_id (ecs->event_thread->initiating_frame);
-
 	  gdb_assert (ecs->event_thread->control.exception_resume_breakpoint
 		      != NULL);
 	  delete_exception_resume_breakpoint (ecs->event_thread);
 
+	  if (what.is_longjmp)
+	    {
+	      check_longjmp_breakpoint_for_call_dummy (ecs->event_thread->num);
+
+	      if (!frame_id_p (ecs->event_thread->initiating_frame))
+		{
+		  /* Case 4.  */
+		  keep_going (ecs);
+		  return;
+		}
+	    }
+
+	  init_frame = frame_find_by_id (ecs->event_thread->initiating_frame);
+
 	  if (init_frame)
 	    {
 	      struct frame_id current_id
--- a/gdb/testsuite/gdb.base/call-signal-resume.exp
+++ b/gdb/testsuite/gdb.base/call-signal-resume.exp
@@ -101,6 +101,18 @@ gdb_test "frame $frame_number" ".*"
 gdb_test_no_output "set confirm off"
 gdb_test "return" ""
 
+# Verify there are no remains of the dummy frame.
+gdb_test_no_output "maintenance print dummy-frames"
+set test "maintenance info breakpoints"
+gdb_test_multiple $test $test {
+    -re " call dummy .*\r\n$gdb_prompt $" {
+	fail $test
+    }
+    -re "\r\n$gdb_prompt $" {
+	pass $test
+    }
+}
+
 # Resume execution, the program should continue without any signal.
 
 gdb_test "break stop_two" "Breakpoint \[0-9\]* at .*"
--- /dev/null
+++ b/gdb/testsuite/gdb.base/stale-infcall.c
@@ -0,0 +1,63 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2012 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <setjmp.h>
+#include <string.h>
+#include <stdlib.h>
+
+#define BUFSIZE 0x1000
+
+static jmp_buf jmp;
+
+void
+infcall (void)
+{
+  longjmp (jmp, 1);
+}
+
+static void
+run1 (void)
+{
+  char buf[BUFSIZE / 2];
+  int dummy = 0;
+
+  dummy++; /* break-run1 */
+}
+
+static char buf_zero[BUFSIZE];
+
+static void
+run2 (void)
+{
+  char buf[BUFSIZE];
+
+  memset (buf, 0, sizeof (buf));
+
+  if (memcmp (buf, buf_zero, sizeof (buf)) != 0) /* break-run2 */
+    abort (); /* break-fail */
+}
+
+int
+main ()
+{
+  if (setjmp (jmp) == 0)
+    run1 ();
+  else
+    run2 ();
+
+  return 0; /* break-exit */
+}
--- /dev/null
+++ b/gdb/testsuite/gdb.base/stale-infcall.exp
@@ -0,0 +1,57 @@
+# Copyright (C) 2012 Free Software Foundation # Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+set testfile stale-infcall
+set srcfile ${testfile}.c
+if { [prepare_for_testing $testfile.exp $testfile $srcfile] } {
+    return -1
+}
+
+if ![runto_main] {
+    return -1
+}
+
+gdb_breakpoint [gdb_get_line_number "break-run1"]
+gdb_breakpoint [gdb_get_line_number "break-run2"]
+gdb_breakpoint [gdb_get_line_number "break-exit"]
+gdb_breakpoint [gdb_get_line_number "break-fail"]
+
+gdb_continue_to_breakpoint "break-run1" ".* break-run1 .*"
+
+gdb_test "print infcall ()" " break-run2 .*The program being debugged stopped while in a function called from GDB\\..*When the function is done executing, GDB will silently stop\\."
+
+set test "stack corrupted"
+gdb_test_multiple "continue" $test {
+    -re " break-exit .*\r\n$gdb_prompt $" {
+	pass $test
+    }
+    -re " break-fail .*\r\n$gdb_prompt $" {
+	fail $test
+    }
+}
+
+gdb_test "bt" "#0 \[^\r\n\]* main \[^\r\n\]*"
+
+# Verify there are no remains of the dummy frame.
+gdb_test_no_output "maintenance print dummy-frames"
+set test "maintenance info breakpoints"
+gdb_test_multiple $test $test {
+    -re " call dummy .*\r\n$gdb_prompt $" {
+	fail $test
+    }
+    -re "\r\n$gdb_prompt $" {
+	pass $test
+    }
+}