[patch 3/3] Fix stale tp->step_resume_breakpoint

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

Hi,

this is a follow-up to:
	Re: [patch] Fix stale tp->step_resume_breakpoint
	http://sourceware.org/ml/gdb-patches/2010-11/msg00011.html


On Tue, 02 Nov 2010 02:05:18 +0100, Pedro Alves wrote:
> I think this raises an obvious question, and hints at
> a larger issue: if you find you you need to tuck away step_resume_breakpoint,
> then, how come you don't need to do the same for all the other execution
> command state?  (step_range_start, step_range_end, step_frame_id,
> continuations, etc.).

Some of them were already saved, just not into `struct inferior_thread_state'
but into `struct inferior_status'.  So extended now further the latter.

I find these two redundant, they could be merged?  I did not try to.
This could be a different patch.


> I'd assume that in the use case you trip on step_resume_breakpoint
> troubles, you'd also be losing thread stepping state (or state
> for any other execution command), thus your thread would end up
> running free, forgetting about the previous command that was
> going on before the infcall.  Is that not the case?

Tried to fix the fields I thought should be saved.  Some fields I am not sure.

For example `stop_signal' IMO should be saved+reset+restored but
gdb.base/call-signal-resume.exp tests it is persisent across infcalls so
I kept it that way.


Thanks,
Jan


gdb/
2010-11-24  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* defs.h (discard_all_specified_continuations): New prototype.
	* infrun.c (struct inferior_status) <step_resume_breakpoint)
	(tp_continuations, intermediate_continuations, step_multi)
	(inf_continuations): New fields.
	(save_inferior_status): Save step_resume_breakpoint, tp_continuations,
	intermediate_continuations, step_multi and inf_continuations.
	(restore_inferior_status): Restore step_resume_breakpoint,
	tp_continuations, intermediate_continuations, step_multi and
	inf_continuations.
	(discard_inferior_status): Discard step_resume_breakpoint,
	tp_continuations, intermediate_continuations and inf_continuations.
	* utils.c (discard_all_specified_continuations): New function.
	(discard_all_continuations_thread_callback)
	(discard_all_intermediate_continuations_thread_callback): Use it.

gdb/testsuite/
2010-11-24  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.base/step-resume-infcall.exp: New file.
	* gdb.base/step-resume-infcall.c: New file.

--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -734,6 +734,8 @@ extern void add_continuation (struct thread_info *,
 			      void (*)(void *));
 extern void do_all_continuations (void);
 extern void do_all_continuations_thread (struct thread_info *);
+extern void
+  discard_all_specified_continuations (struct continuation **continuationsp);
 extern void discard_all_continuations (void);
 extern void discard_all_continuations_thread (struct thread_info *);
 
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -6167,19 +6167,26 @@ get_inferior_thread_state_regcache (struct inferior_thread_state *inf_state)
 struct inferior_status
 {
   /* Direct copies of the struct thread_info fields:  */
+  struct breakpoint *step_resume_breakpoint;
   CORE_ADDR step_range_start;
   CORE_ADDR step_range_end;
   struct frame_id step_frame_id;
   struct frame_id step_stack_frame_id;
   int trap_expected;
   int proceed_to_finish;
+  struct continuation *tp_continuations;                   
+  struct continuation *intermediate_continuations;
   int in_infcall;
   enum step_over_calls_kind step_over_calls;
   int stop_step;
+  int step_multi;
+  /* STOP_SIGNAL should not be saved/restored as tested by:
+     gdb.base/call-signal-resume.exp  */
   bpstat stop_bpstat;
 
   /* Direct copies of the struct inferior fields:  */
   int stop_soon;
+  struct continuation *inf_continuations;
 
   /* Other fields:  */
   enum stop_stack_kind stop_stack_dummy;
@@ -6201,15 +6208,23 @@ save_inferior_status (void)
   struct inferior *inf = current_inferior ();
 
   /* Direct copies of the struct thread_info fields:  */
+  inf_status->step_resume_breakpoint = tp->step_resume_breakpoint;
+  tp->step_resume_breakpoint = NULL;
   inf_status->step_range_start = tp->step_range_start;
   inf_status->step_range_end = tp->step_range_end;
   inf_status->step_frame_id = tp->step_frame_id;
   inf_status->step_stack_frame_id = tp->step_stack_frame_id;
   inf_status->trap_expected = tp->trap_expected;
   inf_status->proceed_to_finish = tp->proceed_to_finish;
+  inf_status->tp_continuations = tp->continuations;
+  tp->continuations = NULL;
+  inf_status->intermediate_continuations = tp->intermediate_continuations;
+  tp->intermediate_continuations = NULL;
   inf_status->in_infcall = tp->in_infcall;
   inf_status->step_over_calls = tp->step_over_calls;
   inf_status->stop_step = tp->stop_step;
+  inf_status->step_multi = tp->step_multi;
+  tp->step_multi = 0;
 
   /* Save original bpstat chain here; replace it with copy of chain.
      If caller's caller is walking the chain, they'll be happier if we
@@ -6220,6 +6235,8 @@ save_inferior_status (void)
 
   /* Direct copies of the struct inferior fields:  */
   inf_status->stop_soon = inf->stop_soon;
+  inf_status->inf_continuations = inf->continuations;
+  inf->continuations = NULL;
 
   /* Other fields:  */
   inf_status->stop_stack_dummy = stop_stack_dummy;
@@ -6261,15 +6278,23 @@ restore_inferior_status (struct inferior_status *inf_status)
   struct inferior *inf = current_inferior ();
 
   /* Direct copies of the struct thread_info fields:  */
+  if (tp->step_resume_breakpoint)
+    tp->step_resume_breakpoint->disposition = disp_del_at_next_stop;
+  tp->step_resume_breakpoint = inf_status->step_resume_breakpoint;
   tp->step_range_start = inf_status->step_range_start;
   tp->step_range_end = inf_status->step_range_end;
   tp->step_frame_id = inf_status->step_frame_id;
   tp->step_stack_frame_id = inf_status->step_stack_frame_id;
   tp->trap_expected = inf_status->trap_expected;
   tp->proceed_to_finish = inf_status->proceed_to_finish;
+  discard_all_continuations_thread (tp);
+  tp->continuations = inf_status->tp_continuations;
+  discard_all_intermediate_continuations_thread (tp);
+  tp->intermediate_continuations = inf_status->intermediate_continuations;
   tp->in_infcall = inf_status->in_infcall;
   tp->step_over_calls = inf_status->step_over_calls;
   tp->stop_step = inf_status->stop_step;
+  tp->step_multi = inf_status->step_multi;
 
   /* Handle the bpstat_copy of the chain.  */
   bpstat_clear (&tp->stop_bpstat);
@@ -6278,6 +6303,8 @@ restore_inferior_status (struct inferior_status *inf_status)
 
   /* Direct copies of the struct inferior fields:  */
   inf->stop_soon = inf_status->stop_soon;
+  discard_all_inferior_continuations (inf);
+  inf->continuations = inf_status->inf_continuations;
 
   /* Other fields:  */
   stop_stack_dummy = inf_status->stop_stack_dummy;
@@ -6316,8 +6343,19 @@ make_cleanup_restore_inferior_status (struct inferior_status *inf_status)
 void
 discard_inferior_status (struct inferior_status *inf_status)
 {
+  /* Direct copies of the struct thread_info fields:  */
+  if (inf_status->step_resume_breakpoint)
+    inf_status->step_resume_breakpoint->disposition = disp_del_at_next_stop;
+
+  discard_all_specified_continuations (&inf_status->tp_continuations);
+  discard_all_specified_continuations (&inf_status->intermediate_continuations);
+
+  /* Direct copies of the struct inferior fields:  */
+  discard_all_specified_continuations (&inf_status->inf_continuations);
+
   /* See save_inferior_status for info on stop_bpstat. */
   bpstat_clear (&inf_status->stop_bpstat);
+
   xfree (inf_status);
 }
 
--- /dev/null
+++ b/gdb/testsuite/gdb.base/step-resume-infcall.c
@@ -0,0 +1,45 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2005, 2007, 2008, 2009, 2010 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 <stdio.h>
+
+int
+cond (void)
+{
+  puts ("cond-hit");
+
+  return 1;
+}
+
+int
+func (void)
+{
+  return 0;	/* in-func */
+}
+
+int
+main (void)
+{
+  /* Variable is used so that there is always at least one instruction on the
+     same line after FUNC returns.  */
+  int i = func ();	/* call-func */
+
+  /* Dummy calls.  */
+  cond ();
+  func ();
+  return 0;
+}
--- /dev/null
+++ b/gdb/testsuite/gdb.base/step-resume-infcall.exp
@@ -0,0 +1,49 @@
+# Copyright (C) 2010 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 "step-resume-infcall"
+
+if { [prepare_for_testing ${testfile}.exp ${testfile}] } {
+    return -1
+}
+
+if ![runto_main] {
+    return -1
+}
+
+gdb_test "step" " in-func .*"
+gdb_test "up" " call-func .*"
+gdb_test_no_output {set $b=$pc}
+
+if ![runto_main] {
+    return -1
+}
+
+gdb_breakpoint {*$b if cond ()}
+
+set test {print $bpnum}
+gdb_test_multiple $test $test {
+    -re " = (\[0-9\]+)\r\n$gdb_prompt $" {
+	set caller_bp $expect_out(1,string)
+	pass $test
+    }
+}
+
+# Debug only:
+gdb_test "disass/m" ".*"
+gdb_test "info breakpoints" ".*"
+
+# `cond-hit' is now printed twice; but it may not be in the future.
+gdb_test "next" "cond-hit.*\r\nBreakpoint $caller_bp, .* call-func .*"
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -840,15 +840,22 @@ do_all_continuations (void)
   iterate_over_threads (do_all_continuations_thread_callback, NULL);
 }
 
+/* Get rid of all the continuations tracked by *CONTINUATIONSP.  */
+void
+discard_all_specified_continuations (struct continuation **continuationsp)
+{
+  struct cleanup *continuation_ptr = &(*continuationsp)->base;
+
+  discard_my_cleanups (&continuation_ptr, NULL);
+  *continuationsp = NULL;
+}
+
 /* Callback for iterate over threads.  */
 static int
 discard_all_continuations_thread_callback (struct thread_info *thread,
 					   void *data)
 {
-  struct cleanup *continuation_ptr = &thread->continuations->base;
-
-  discard_my_cleanups (&continuation_ptr, NULL);
-  thread->continuations = NULL;
+  discard_all_specified_continuations (&thread->continuations);
   return 0;
 }
 
@@ -922,10 +929,7 @@ static int
 discard_all_intermediate_continuations_thread_callback (struct thread_info *thread,
 							void *data)
 {
-  struct cleanup *continuation_ptr = &thread->intermediate_continuations->base;
-
-  discard_my_cleanups (&continuation_ptr, NULL);
-  thread->intermediate_continuations = NULL;
+  discard_all_specified_continuations (&thread->intermediate_continuations);
   return 0;
 }