[PATCH v3 01/17] Fix and test "checkpoint" in non-stop mode

[Pedro Alves <palves at redhat dot com>]

Letting a "checkpoint" run to exit with "set non-stop on" behaves
differently compared to the default all-stop mode ("set non-stop
off").

Currently, in non-stop mode:

  (gdb) start
  Temporary breakpoint 1 at 0x40086b: file src/gdb/testsuite/gdb.base/checkpoint.c, line 28.
  Starting program: build/gdb/testsuite/gdb.base/checkpoint

  Temporary breakpoint 1, main () at src/gdb/testsuite/gdb.base/checkpoint.c:28
  28        char *tmp = &linebuf[0];
  (gdb) checkpoint
  checkpoint 1: fork returned pid 24948.
  (gdb) c
  Continuing.
  Copy complete.
  Deleting copy.
  [Inferior 1 (process 24944) exited normally]
  [Switching to process 24948]
  (gdb) info threads
    Id   Target Id         Frame
    1    process 24948 "checkpoint" (running)

  No selected thread.  See `help thread'.
  (gdb) c
  The program is not being run.
  (gdb)

Two issues above:

 1. Thread 1 got stuck in "(running)" state (it isn't really running)

 2. While checkpoints try to preserve the illusion that the thread is
    still the same when the process exits, GDB switched to "No thread
    selected." instead of staying with thread 1 selected.

Problem #1 is caused by handle_inferior_event and normal_stop not
considering that when a
TARGET_WAITKIND_SIGNALLED/TARGET_WAITKIND_EXITED event is reported,
and the inferior is mourned, the target may still have execution.

Problem #2 is caused by the make_cleanup_restore_current_thread
cleanup installed by fetch_inferior_event not being able to find the
original thread 1's ptid in the thread list, thus not being able to
restore thread 1 as selected thread.  The fix is to make the cleanup
installed by make_cleanup_restore_current_thread aware of thread ptid
changes, by installing a thread_ptid_changed observer that adjusts the
cleanup's data.

After the patch, we get the same in all-stop and non-stop modes:

  (gdb) c
  Continuing.
  Copy complete.
  Deleting copy.
  [Inferior 1 (process 25109) exited normally]
  [Switching to process 25113]
  (gdb) info threads
    Id   Target Id         Frame
  * 1    process 25113 "checkpoint" main () at src/gdb/testsuite/gdb.base/checkpoint.c:28
  (gdb)

Turns out the whole checkpoints.exp file can run in non-stop mode
unmodified.  I thought of moving most of the test file's contents to a
procedure that can be called twice, once in non-stop mode and another
in all-stop mode.  But then, the test already takes over 30 seconds to
run on my machine, so I thought it'd be nicer to run all-stop and
non-stop mode in parallel.  Thus I added a new checkpoint-ns.exp file
that just sources checkpoint.exp, and sets a knob that checkpoint.exp
reads to know it should test non-stop mode.  No other test in the tree
currently uses this mechanism, but I can't see a reason we shouldn't
do this.

gdb/ChangeLog:
2015-04-17  Pedro Alves  <palves@redhat.com>

	* infrun.c (handle_inferior_event): If we get
	TARGET_WAITKIND_SIGNALLED or TARGET_WAITKIND_EXITED in non-stop
	mode, mark all threads of the exiting process as not-executing.
	(normal_stop): If we get TARGET_WAITKIND_SIGNALLED or
	TARGET_WAITKIND_EXITED in non-stop mode, finish all threads of the
	exiting process, if inferior_ptid still points at a process.
	* thread.c (struct current_thread_cleanup) <next>: New field.
	(current_thread_cleanup_chain): New global.
	(restore_current_thread_ptid_changed): New function.
	(restore_current_thread_cleanup_dtor): Remove the cleanup from the
	current_thread_cleanup_chain list.
	(make_cleanup_restore_current_thread): Add the cleanup data to the
	current_thread_cleanup_chain list.
	(_initialize_thread): Install restore_current_thread_ptid_changed
	as thread_ptid_changed observer.

gdb/testsuite/ChangeLog:
2015-04-17  Pedro Alves  <palves@redhat.com>

	* gdb.base/checkpoint-ns.exp: New file.
	* gdb.base/checkpoint.exp (checkpoint_non_stop): New input
	variable.  If set, test in non-stop mode.  Pass explicit
	"checkpoint.c" to standard_testfile.

v3:
	No changes.
---
 gdb/infrun.c                             | 31 +++++++++++++++++++++-----
 gdb/testsuite/gdb.base/checkpoint-ns.exp | 27 +++++++++++++++++++++++
 gdb/testsuite/gdb.base/checkpoint.exp    | 31 +++++++++++++++++++++-----
 gdb/thread.c                             | 38 ++++++++++++++++++++++++++++++++
 4 files changed, 117 insertions(+), 10 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/checkpoint-ns.exp

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 7870f70..9c1fdc5 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -3804,8 +3804,18 @@ handle_inferior_event (struct execution_control_state *ecs)
      any other process were left running.  */
   if (!non_stop)
     set_executing (minus_one_ptid, 0);
-  else if (ecs->ws.kind != TARGET_WAITKIND_SIGNALLED
-	   && ecs->ws.kind != TARGET_WAITKIND_EXITED)
+  else if (ecs->ws.kind == TARGET_WAITKIND_SIGNALLED
+	   && ecs->ws.kind == TARGET_WAITKIND_EXITED)
+    {
+      ptid_t pid_ptid;
+
+      /* Some targets still have execution when a process exits.
+	 E.g., for "checkpoint", when when a fork exits and is
+	 mourned, linux-fork.c switches to another fork.  */
+      pid_ptid = pid_to_ptid (ptid_get_pid (ecs->ptid));
+      set_executing (pid_ptid, 0);
+    }
+  else
     set_executing (ecs->ptid, 0);
 
   switch (ecs->ws.kind)
@@ -6550,6 +6560,7 @@ normal_stop (void)
   struct target_waitstatus last;
   ptid_t last_ptid;
   struct cleanup *old_chain = make_cleanup (null_cleanup, NULL);
+  ptid_t pid_ptid;
 
   get_last_target_status (&last_ptid, &last);
 
@@ -6559,9 +6570,19 @@ normal_stop (void)
      here, so do this before any filtered output.  */
   if (!non_stop)
     make_cleanup (finish_thread_state_cleanup, &minus_one_ptid);
-  else if (last.kind != TARGET_WAITKIND_SIGNALLED
-	   && last.kind != TARGET_WAITKIND_EXITED
-	   && last.kind != TARGET_WAITKIND_NO_RESUMED)
+  else if (last.kind == TARGET_WAITKIND_SIGNALLED
+	   || last.kind == TARGET_WAITKIND_EXITED)
+    {
+      /* Some targets still have execution when a process exits.
+	 E.g., for "checkpoint", when when a fork exits and is
+	 mourned, linux-fork.c switches to another fork.  */
+      if (!ptid_equal (inferior_ptid, null_ptid))
+	{
+	  pid_ptid = pid_to_ptid (ptid_get_pid (inferior_ptid));
+	  make_cleanup (finish_thread_state_cleanup, &pid_ptid);
+	}
+    }
+  else if (last.kind != TARGET_WAITKIND_NO_RESUMED)
     make_cleanup (finish_thread_state_cleanup, &inferior_ptid);
 
   /* As we're presenting a stop, and potentially removing breakpoints,
diff --git a/gdb/testsuite/gdb.base/checkpoint-ns.exp b/gdb/testsuite/gdb.base/checkpoint-ns.exp
new file mode 100644
index 0000000..03a1747
--- /dev/null
+++ b/gdb/testsuite/gdb.base/checkpoint-ns.exp
@@ -0,0 +1,27 @@
+# Copyright 2015 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/>.  */
+
+# Test gdb checkpoint and restart in non-stop mode.
+
+# We drive non-stop mode from a separate file because the whole test
+# takes a while to run.  This way, we can test both modes in parallel.
+
+# checkpoint.exp reads this variable.
+set checkpoint_non_stop 1
+
+source $srcdir/$subdir/checkpoint.exp
+
+# Unset to avoid problems with non-parallel mode.
+unset checkpoint_non_stop
diff --git a/gdb/testsuite/gdb.base/checkpoint.exp b/gdb/testsuite/gdb.base/checkpoint.exp
index 6d94ab6..c297b50 100644
--- a/gdb/testsuite/gdb.base/checkpoint.exp
+++ b/gdb/testsuite/gdb.base/checkpoint.exp
@@ -13,6 +13,20 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
+#
+# This tests gdb checkpoint and restart.
+#
+
+# Note this file is also sourced by checkpoint-ns.exp to run all the
+# tests below in non-stop mode.  That's driven by a separate file so
+# testing both all-stop and non-stop can be done in parallel.
+
+# This is the variable checkpoint-ns.exp sets to request non-stop
+# mode.  When not set, default to all-stop mode.
+if {![info exists checkpoint_non_stop]} {
+    set checkpoint_non_stop 0
+}
+
 if { [is_remote target] || ![isnative] } then {
     continue
 }
@@ -24,8 +38,10 @@ if {![istarget "*-*-linux*"]} then {
     continue
 }
 
-
-standard_testfile .c
+# Must name the source file explicitly, otherwise when driven by
+# checkpoints-ns.exp, we'd try compiling checkpoints-ns.c, which
+# doesn't exist.
+standard_testfile checkpoint.c
 
 set pi_txt [gdb_remote_download host ${srcdir}/${subdir}/pi.txt]
 if {[is_remote host]} {
@@ -42,9 +58,9 @@ if {[prepare_for_testing ${testfile}.exp $testfile $srcfile \
 
 global gdb_prompt
 
-#
-# This tests gdb checkpoint and restart.
-#
+if {$checkpoint_non_stop} {
+    gdb_test_no_output "set non-stop on"
+}
 
 runto_main
 set break1_loc [gdb_get_line_number "breakpoint 1"]
@@ -327,6 +343,11 @@ gdb_start
 gdb_reinitialize_dir $srcdir/$subdir
 gdb_load ${binfile}
 
+if {$checkpoint_non_stop} {
+    gdb_test_no_output "set non-stop on" \
+	"set non-stop on, for many checkpoints"
+}
+
 runto_main
 gdb_breakpoint $break1_loc
 
diff --git a/gdb/thread.c b/gdb/thread.c
index 23dfcc9..46b5947 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -1279,8 +1279,16 @@ restore_selected_frame (struct frame_id a_frame_id, int frame_level)
     }
 }
 
+/* Data used by the cleanup installed by
+   'make_cleanup_restore_current_thread'.  */
+
 struct current_thread_cleanup
 {
+  /* Next in list of currently installed 'struct
+     current_thread_cleanup' cleanups.  See
+     'current_thread_cleanup_chain' below.  */
+  struct current_thread_cleanup *next;
+
   ptid_t inferior_ptid;
   struct frame_id selected_frame_id;
   int selected_frame_level;
@@ -1289,6 +1297,29 @@ struct current_thread_cleanup
   int was_removable;
 };
 
+/* A chain of currently installed 'struct current_thread_cleanup'
+   cleanups.  Restoring the previously selected thread looks up the
+   old thread in the thread list by ptid.  If the thread changes ptid,
+   we need to update the cleanup's thread structure so the look up
+   succeeds.  */
+static struct current_thread_cleanup *current_thread_cleanup_chain;
+
+/* A thread_ptid_changed observer.  Update all currently installed
+   current_thread_cleanup cleanups that want to switch back to
+   OLD_PTID to switch back to NEW_PTID instead.  */
+
+static void
+restore_current_thread_ptid_changed (ptid_t old_ptid, ptid_t new_ptid)
+{
+  struct current_thread_cleanup *it;
+
+  for (it = current_thread_cleanup_chain; it != NULL; it = it->next)
+    {
+      if (ptid_equal (it->inferior_ptid, old_ptid))
+	it->inferior_ptid = new_ptid;
+    }
+}
+
 static void
 do_restore_current_thread_cleanup (void *arg)
 {
@@ -1329,6 +1360,8 @@ restore_current_thread_cleanup_dtor (void *arg)
   struct thread_info *tp;
   struct inferior *inf;
 
+  current_thread_cleanup_chain = current_thread_cleanup_chain->next;
+
   tp = find_thread_ptid (old->inferior_ptid);
   if (tp)
     tp->refcount--;
@@ -1362,6 +1395,9 @@ make_cleanup_restore_current_thread (void)
   old->inf_id = current_inferior ()->num;
   old->was_removable = current_inferior ()->removable;
 
+  old->next = current_thread_cleanup_chain;
+  current_thread_cleanup_chain = old;
+
   if (!ptid_equal (inferior_ptid, null_ptid))
     {
       old->was_stopped = is_stopped (inferior_ptid);
@@ -1815,4 +1851,6 @@ Show printing of thread events (such as thread start and exit)."), NULL,
          &setprintlist, &showprintlist);
 
   create_internalvar_type_lazy ("_thread", &thread_funcs, NULL);
+
+  observer_attach_thread_ptid_changed (restore_current_thread_ptid_changed);
 }
-- 
1.9.3