[doc,RFA] : Fix pr 14236

Message ID 21305.45035.310308.253836@ruffy.mtv.corp.google.com
State New, archived
Headers

Commit Message

Doug Evans March 31, 2014, 6:11 p.m. UTC
  Doug Evans writes:
 > Hi.
 > 
 > This patch is a proposed(!) fix to bug 14236.
 > The problem here is that the "interrupt" command (figuratively speaking)
 > takes an implicit & as an argument (it is always asynchronous) and
 > users aren't expecting this.
 > 
 > This patch fixes the problem by changing the behaviour by making
 > "interrupt" synchronous by default and supporting a new option &,
 > to be consistent with every other gdb command (which is what I think
 > every user would expect, including this user).
 > 
 > Alas, while I think this is the correct fix (*1), if one could set aside
 > breaking existing code, the latter, obviously, is important too.
 > I had to update a few testcases that expected the old behaviour,
 > but I don't, in this particular case, have a feel for whether the
 > change in behaviour would break anything outside of gdb's testsuite.
 > Without a "wait" command the current behaviour is completely useless
 > in scripts, for example.
 > 
 > I have patches in the works to fix this in other ways.
 > E.g., regardless of how "interrupt" is fixed I think we also want a
 > "wait" command.
 > 
 > But for now I'd like to know what the community thinks of this approach.
 > 
 > (*1): gdb's infrun state machine doesn't support waiting for all
 > threads to stop in non-stop mode, AFAICT, and I'm loathe to make it more
 > complicated just to handle this.  Therefore the continuation that handles
 > completing the interrupt command waits for any remaining threads itself.
 > Alas, I'm not sure if there's a technical reason why this patch
 > can't work.  It is derived, in part, from the attach support, and it
 > too waits for threads outside of the main event loop.

Hi.  Sorry for the followup.
Gotta love test-driven-development.
I had a TODO of another testcase and sure enough ...

This patch replaces the previous one.
No doc changes, just a new testcase and a change to wait
for the current thread to stop in non-stop mode.

2014-03-30  Doug Evans  <xdje42@gmail.com>

	PR gdb/14236
	* NEWS: Mention change in default behaviour of "interrupt" command.
	* infcmd.c (struct interrupt_command_continuation_args): New struct.
	(interrupt_command_continuation_free_args): New function.
	(wait_thread_stopped): New function.
	(interrupt_command_post_wait): New function.
	(interrupt_command_continuation): New function.
	(add_interrupt_continuation): New function.
	(interrupt_command): Rewrite.

	doc/
	* gdb.texinfo (Background Execution): Document new option "&" for
	"interrupt" command.

	testsuite/
	* gdb.base/async-shell.exp: Add "&" to "interrupt" command.
	* gdb.base/dprintf-non-stop.exp: Ditto.
	* gdb.base/interrupt-noterm.exp: Ditto.
	* gdb.threads/interrupt-command.c: New file.
	* gdb.threads/interrupt-command.exp: New file.
  

Comments

Doug Evans July 7, 2014, 4:08 p.m. UTC | #1
Doug Evans writes:
 > Doug Evans writes:
 >  > Hi.
 >  > 
 >  > This patch is a proposed(!) fix to bug 14236.
 >  > The problem here is that the "interrupt" command (figuratively speaking)
 >  > takes an implicit & as an argument (it is always asynchronous) and
 >  > users aren't expecting this.
 >  > 
 >  > This patch fixes the problem by changing the behaviour by making
 >  > "interrupt" synchronous by default and supporting a new option &,
 >  > to be consistent with every other gdb command (which is what I think
 >  > every user would expect, including this user).
 >  > 
 >  > Alas, while I think this is the correct fix (*1), if one could set aside
 >  > breaking existing code, the latter, obviously, is important too.
 >  > I had to update a few testcases that expected the old behaviour,
 >  > but I don't, in this particular case, have a feel for whether the
 >  > change in behaviour would break anything outside of gdb's testsuite.
 >  > Without a "wait" command the current behaviour is completely useless
 >  > in scripts, for example.
 >  > 
 >  > I have patches in the works to fix this in other ways.
 >  > E.g., regardless of how "interrupt" is fixed I think we also want a
 >  > "wait" command.
 >  > 
 >  > But for now I'd like to know what the community thinks of this approach.
 >  > 
 >  > (*1): gdb's infrun state machine doesn't support waiting for all
 >  > threads to stop in non-stop mode, AFAICT, and I'm loathe to make it more
 >  > complicated just to handle this.  Therefore the continuation that handles
 >  > completing the interrupt command waits for any remaining threads itself.
 >  > Alas, I'm not sure if there's a technical reason why this patch
 >  > can't work.  It is derived, in part, from the attach support, and it
 >  > too waits for threads outside of the main event loop.
 > 
 > Hi.  Sorry for the followup.
 > Gotta love test-driven-development.
 > I had a TODO of another testcase and sure enough ...
 > 
 > This patch replaces the previous one.
 > No doc changes, just a new testcase and a change to wait
 > for the current thread to stop in non-stop mode.
 > 
 > 2014-03-30  Doug Evans  <xdje42@gmail.com>
 > 
 > 	PR gdb/14236
 > 	* NEWS: Mention change in default behaviour of "interrupt" command.
 > 	* infcmd.c (struct interrupt_command_continuation_args): New struct.
 > 	(interrupt_command_continuation_free_args): New function.
 > 	(wait_thread_stopped): New function.
 > 	(interrupt_command_post_wait): New function.
 > 	(interrupt_command_continuation): New function.
 > 	(add_interrupt_continuation): New function.
 > 	(interrupt_command): Rewrite.
 > 
 > 	doc/
 > 	* gdb.texinfo (Background Execution): Document new option "&" for
 > 	"interrupt" command.
 > 
 > 	testsuite/
 > 	* gdb.base/async-shell.exp: Add "&" to "interrupt" command.
 > 	* gdb.base/dprintf-non-stop.exp: Ditto.
 > 	* gdb.base/interrupt-noterm.exp: Ditto.
 > 	* gdb.threads/interrupt-command.c: New file.
 > 	* gdb.threads/interrupt-command.exp: New file.

Hi.
It's been three months.
If there are no comments I will press ahead with this.
The existing asynchronous behaviour of "interrupt" without an "&" is a bug worth fixing IMO.

I can certainly make the new functionality dependent on some flag, if people think it's worth preserving the broken behaviour as the default.
  
Pedro Alves July 10, 2014, 6:41 p.m. UTC | #2
Hi Doug,

(FYI, the subject line made this get out of my radar.)

On 07/07/2014 05:08 PM, Doug Evans wrote:

> Hi.
> It's been three months.
> If there are no comments I will press ahead with this.
> The existing asynchronous behaviour of "interrupt" without an "&" is a bug worth fixing IMO.
> 
> I can certainly make the new functionality dependent on some flag, if people think it's worth preserving the broken behaviour as the default.

I'd like to take a better look at this.  I browsed it quickly when you first posted it,
and I recall thinking that you may have a problem with new threads appearing (or
not listed because nothing has refreshed the list).  I address this somewhat with
the existing target methods in the all-stop-on-top-of-nonstop patch (you can see
it on the list a while ago).  More comments soon after I take a better look.
Meanwhile you know what I'll probably comment on.  :-)
  
Doug Evans July 10, 2014, 7:07 p.m. UTC | #3
On Thu, Jul 10, 2014 at 11:41 AM, Pedro Alves <palves@redhat.com> wrote:
> [...] in the all-stop-on-top-of-nonstop patch (you can see
> it on the list a while ago).

Got URL so there's no confusion?
TIA
  

Patch

diff --git a/gdb/NEWS b/gdb/NEWS
index 2a384ba..01d8094 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -3,6 +3,9 @@ 
 
 *** Changes since GDB 7.7
 
+* The "interrupt" command now waits for the inferior to stop by default.
+  To avoid waiting for the inferior to stop pass "&".
+
 * Guile scripting
 
   GDB now has support for scripting using Guile.  Whether this is
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 0bf33b7..f3747fd 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -5871,13 +5871,15 @@  using the @code{interrupt} command.
 
 @table @code
 @kindex interrupt
-@item interrupt
-@itemx interrupt -a
+@item interrupt @r{[}-a@r{]} @r{[}&@r{]}
 
 Suspend execution of the running program.  In all-stop mode,
 @code{interrupt} stops the whole process, but in non-stop mode, it stops
 only the current thread.  To stop the whole program in non-stop mode,
-use @code{interrupt -a}.
+use @code{interrupt -a}.  The @code{-a} option is ignored in all-stop mode.
+By default, the @code{interrupt} command waits for the thread to stop,
+or if the @code{-a} option is specified all threads.
+To not wait for the thread to stop pass @code{&}.
 @end table
 
 @node Thread-Specific Breakpoints
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 920169d..b5535bb 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -2700,29 +2700,178 @@  interrupt_target_1 (int all_threads)
     set_stop_requested (ptid, 1);
 }
 
-/* interrupt [-a]
-   Stop the execution of the target while running in async mode, in
-   the backgound.  In all-stop, stop the whole process.  In non-stop
-   mode, stop the current thread only by default, or stop all threads
-   if the `-a' switch is used.  */
+/* Data associated with an "interrupt" command continuation.  */
+
+struct interrupt_command_continuation_args
+{
+  /* This is the thread to wait for or minus_one_ptid to wait for all
+     threads.  */
+  ptid_t wait_for_ptid;
+};
+
+/* continuation_free_arg_ftype function for the interrupt command.  */
+
+static void
+interrupt_command_continuation_free_args (void *args)
+{
+  struct interrupt_command_continuation_args *a = args;
+
+  xfree (a);
+}
+
+/* Wait for thread PTID to stop, or all threads if it is minus_one_ptid.
+   This is used in non-stop mode to implement "interrupt -a" without
+   complicating the infrun state machine more than it already is.  */
+
+static void
+wait_thread_stopped (ptid_t ptid)
+{
+  struct cleanup* cleanups = make_cleanup_restore_current_thread ();
+  int all_threads = ptid_equal (ptid, minus_one_ptid);
+
+  gdb_assert (non_stop);
+
+  while (all_threads
+	 ? any_running ()
+	 : is_running (ptid))
+    {
+      struct target_waitstatus last;
+      ptid_t last_ptid;
+
+      QUIT;
+      wait_for_inferior ();
+      get_last_target_status (&last_ptid, &last);
+      finish_thread_state (last_ptid);
+      /* Leave printing of the stop event of PTID to normal_stop.  */
+      if (!all_threads && !ptid_equal (last_ptid, ptid))
+	print_stop_event (&last);
+    }
+
+  do_cleanups (cleanups);
+
+  if (!all_threads && !target_can_async_p ())
+    normal_stop ();
+}
+
+/* Finish up waiting for thread WAIT_FOR_PTID to stop, or continue waiting
+   for all threads to stop if it is minus_one_ptid.
+   Note: This function is named the way it is as it's derived from
+   attach_command_post_wait.  */
+
+static void
+interrupt_command_post_wait (ptid_t wait_for_ptid)
+{
+  struct inferior *inferior;
+
+  inferior = current_inferior ();
+  inferior->control.stop_soon = NO_STOP_QUIETLY;
+
+  /* If this is all-stop we don't have to do anything special to wait for all
+     threads to stop.  */
+  if (non_stop)
+    wait_thread_stopped (wait_for_ptid);
+}
+
+/* continuation_ftype function for the interrupt command.  */
+
+static void
+interrupt_command_continuation (void *args, int err)
+{
+  struct interrupt_command_continuation_args *a = args;
+
+  if (err)
+    return;
+
+  interrupt_command_post_wait (a->wait_for_ptid);
+}
+
+/* Queue a continuation to complete the interrupt command.
+   WAIT_FOR_PTID is minus_one_ptid if waiting for all threads to stop.  */
+
+static void
+add_interrupt_continuation (ptid_t wait_for_ptid)
+{
+  struct interrupt_command_continuation_args *a;
+
+  a = xmalloc (sizeof (*a));
+  a->wait_for_ptid = wait_for_ptid;
+
+  /* We add the continuation to the inferior, even if we're only waiting for
+     a single thread, because we don't know which thread the event loop will
+     see first.  */
+  add_inferior_continuation (interrupt_command_continuation, a,
+			     interrupt_command_continuation_free_args);
+}
+
+/* interrupt [-a] [&]
+   Stop the execution of the target.
+   In all-stop, stop the whole process.  In non-stop mode, stop only
+   the current thread by default, or stop all threads if the "-a"
+   switch is used.  "-a" is ignored in all-stop mode.  */
 
 static void
 interrupt_command (char *args, int from_tty)
 {
-  if (target_can_async_p ())
+  int async_exec = 0;
+  int all_threads = 0;
+
+  /* Otherwise subsequent invocations will just throw an error.  */
+  dont_repeat ();
+
+  ERROR_NO_INFERIOR;
+  ensure_not_tfind_mode ();
+
+  if (args != NULL)
+    async_exec = strip_bg_char (&args);
+
+  if (args != NULL
+      && strncmp (args, "-a", sizeof ("-a") - 1) == 0)
+    all_threads = 1;
+
+  if (all_threads)
     {
-      int all_threads = 0;
+      if (!any_running ())
+	error (_("All threads are already stopped."));
+    }
+  else
+    {
+      if (!is_running (inferior_ptid))
+	error (_("Current thread is already stopped."));
+    }
 
-      dont_repeat ();		/* Not for the faint of heart.  */
+  /* If we're in synchronous all-stop mode, the above error checking for
+     whether threads are already stopped should have fired.  */
+  gdb_assert (non_stop || target_can_async_p ());
 
-      if (args != NULL
-	  && strncmp (args, "-a", sizeof ("-a") - 1) == 0)
-	all_threads = 1;
+  /* This will flag an error if & is given in non-async mode.
+     This isn't an execution command per se, but this performs what we
+     need.  */
+  prepare_execution_command (&current_target, async_exec);
 
-      if (!non_stop && all_threads)
-	error (_("-a is meaningless in all-stop mode."));
+  interrupt_target_1 (all_threads);
 
-      interrupt_target_1 (all_threads);
+  if (async_exec)
+    return;
+
+  /* Set the terminal to the inferior while we're waiting for it to stop.
+     One reason to do this is because if we don't in target-async mode,
+     after we return rl_linefunc will get set to NULL.  If we leave the
+     terminal as ours then if the user types a command while gdb is
+     waiting for the inferior, readline will get called to process the
+     command and will abort because rl_linefunc is NULL.  This is
+     fragile, but apparently exists as a workaround for a readline
+     issue.  Grep for "trick readline" in display_gdb_prompt.
+     This must be done after calling async_disable_stdin (which is called
+     by prepare_execution_command), or it will "early exit" without doing
+     anything.  */
+  target_terminal_inferior ();
+
+  if (target_can_async_p ())
+    add_interrupt_continuation (all_threads ? minus_one_ptid : inferior_ptid);
+  else
+    {
+      gdb_assert (non_stop);
+      wait_thread_stopped (all_threads ? minus_one_ptid : inferior_ptid);
     }
 }
 
diff --git a/gdb/testsuite/gdb.base/async-shell.exp b/gdb/testsuite/gdb.base/async-shell.exp
index 4890a59..d23eb4e 100644
--- a/gdb/testsuite/gdb.base/async-shell.exp
+++ b/gdb/testsuite/gdb.base/async-shell.exp
@@ -40,9 +40,9 @@  gdb_test "run &" "Starting program: \[^\r\n\]*(\r\n$gdbindex_warning_re)?"
 
 gdb_test "shell echo foo" "foo"
 
-set test "interrupt"
+set test "interrupt &"
 gdb_test_multiple $test $test {
-    -re "interrupt\r\n$gdb_prompt " {
+    -re "interrupt &\r\n$gdb_prompt " {
 	pass $test
     }
 }
diff --git a/gdb/testsuite/gdb.base/dprintf-non-stop.exp b/gdb/testsuite/gdb.base/dprintf-non-stop.exp
index fdaa5c1..5aa4463 100644
--- a/gdb/testsuite/gdb.base/dprintf-non-stop.exp
+++ b/gdb/testsuite/gdb.base/dprintf-non-stop.exp
@@ -52,9 +52,9 @@  gdb_expect {
 # Now test that we're still able to issue commands.  GDB used to
 # implement re-resuming from dprintfs with a synchronous "continue" in
 # the dprintf's command list, which stole the prompt from the user.
-set test "interrupt"
+set test "interrupt &"
 gdb_test_multiple $test $test {
-    -re "interrupt\r\n$gdb_prompt " {
+    -re "interrupt &\r\n$gdb_prompt " {
 	pass $test
     }
 }
diff --git a/gdb/testsuite/gdb.base/interrupt-noterm.exp b/gdb/testsuite/gdb.base/interrupt-noterm.exp
index a22acd2..323f5cc 100644
--- a/gdb/testsuite/gdb.base/interrupt-noterm.exp
+++ b/gdb/testsuite/gdb.base/interrupt-noterm.exp
@@ -59,9 +59,9 @@  if { $async_supported < 0 } {
 # With native debugging, and no terminal (emulated by interactive-mode
 # off, above), GDB had a bug where "interrupt" would send SIGINT to
 # its own process group, instead of the inferior's.
-set test "interrupt"
+set test "interrupt &"
 gdb_test_multiple $test $test {
-    -re "interrupt\r\n$gdb_prompt " {
+    -re "interrupt &\r\n$gdb_prompt " {
 	pass $test
     }
 }
diff --git a/gdb/testsuite/gdb.threads/interrupt-command.c b/gdb/testsuite/gdb.threads/interrupt-command.c
new file mode 100644
index 0000000..fdeea09
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/interrupt-command.c
@@ -0,0 +1,63 @@ 
+/* Test the "interrupt" command.
+
+   Copyright 2014 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   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 <pthread.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+#ifndef NR_THREADS
+#define NR_THREADS 4
+#endif
+
+pthread_t threads[NR_THREADS];
+
+static void *
+thread_entry (void *unused)
+{
+  while (1)
+    sleep (1);
+}
+
+static void
+all_threads_running (void)
+{
+}
+
+static void
+do_something (void)
+{
+}
+
+int
+main (int argc, char *argv[])
+{
+  int i;
+
+  alarm (60);
+
+  for (i = 0; i < NR_THREADS; ++i)
+    pthread_create (&threads[i], NULL, thread_entry, NULL);
+
+  all_threads_running ();
+
+  while (1)
+    do_something ();
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.threads/interrupt-command.exp b/gdb/testsuite/gdb.threads/interrupt-command.exp
new file mode 100644
index 0000000..c97a99c
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/interrupt-command.exp
@@ -0,0 +1,465 @@ 
+# Copyright (C) 2014 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 NR_THREADS 4
+
+standard_testfile
+
+if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable [list debug "additional_flags=-DNR_THREADS=$NR_THREADS"]] != "" } {
+    return -1
+}
+
+proc prepare_interrupt_test { target_async non_stop } {
+    global binfile
+    clean_restart ${binfile}
+
+    gdb_test_no_output "set target-async ${target_async}" "set async mode"
+    gdb_test_no_output "set non-stop ${non_stop}" "set non-stop mode"
+
+    if ![runto_main] {
+	untested "could not run to main"
+	return -1
+    }
+
+    gdb_breakpoint "all_threads_running"
+    gdb_continue_to_breakpoint "all_threads_running"
+}
+
+proc interrupt_all_stop { command } {
+    global gdb_prompt
+
+    set test_name $command
+    gdb_test_multiple $command "" {
+	-re "Program received signal SIGINT.*$gdb_prompt $" {
+	    pass $test_name
+	}
+	timeout {
+	    fail "$test_name (timeout)"
+	}
+    }
+}
+
+# Apparently the state of a thread isn't necessarily "running" immediately
+# after doing "c &".  This feels like a bug, but for robustness sake it's a
+# good thing to verify anyway.
+# THREAD_NR is either a thread number or "all".
+
+proc wait_thread_running { thread_nr } {
+    global gdb_prompt
+    set nr_attempts 4
+
+    set test_name "wait thread $thread_nr running"
+    for { set i 0 } { $i < $nr_attempts } { incr i } {
+	if { "$thread_nr" == "all" } {
+	    set running -1
+	} else {
+	    set running -1
+	    send_gdb "info thread $thread_nr\n"
+	    gdb_expect {
+		-re "\n\[ \]*$thread_nr +Thread \[^\r\]+\\(running\\)\[\r\n\]+$gdb_prompt $" {
+		    set running 1
+		}
+		-re "\n\[ \]*$thread_nr +Thread \[^\r\]+\[\r\n\]+$gdb_prompt $" {
+		    set running 0
+		}
+		-re "$gdb_prompt $" {
+		    verbose -log "unexpected output processing info thread output"
+		}
+		timeout {
+		    verbose -log "unexpected timeout processing info thread output"
+		}
+	    }
+	    if { $running < 0 } {
+		fail "$test_name (error processing info threads output)"
+		return
+	    }
+	    if { $running } {
+		pass $test_name
+		return
+	    }
+	}
+    }
+
+    fail "$test_name (thread still not running after $nr_attempts attempts)"
+}
+
+proc verify_current_thread { thread_nr } {
+    global gdb_prompt
+
+    set test_name "verify thread $thread_nr current"
+    set correct_current_thread 0
+    gdb_test_multiple "info threads" $test_name {
+	-re "\n\[*\]\[ \]+$thread_nr +Thread \[^\r\]+\r" {
+	    set correct_current_thread 1
+	    exp_continue
+	}
+	-re "\n\[*\]?\[ \]+\[\[:digit:\]\]+ +Thread \[^\r\]+\r" {
+	    exp_continue
+	}
+	-re "$gdb_prompt $" {
+	    if { $correct_current_thread } {
+		pass $test_name
+	    } else {
+		fail $test_name
+	    }
+	}
+	timeout {
+	    fail "$test_name (timeout)"
+	}
+    }
+}
+
+proc interrupt_all_threads_all_stop_background { command } {
+    global gdb_prompt
+
+    set command "$command &"
+    set test_name $command
+    # This has to watch for gdb_prompt separately from SIGINT because
+    # other cases added will catch gdb_prompt before we recognize SIGINT.
+    gdb_test_multiple $command $test_name {
+	-re "$gdb_prompt " {
+	    exp_continue
+	}
+	-re "\[\r\n\]+Program received signal SIGINT" {
+	    pass $test_name
+	}
+	timeout {
+	    fail "$test_name (timeout)"
+	}
+    }
+}
+
+proc interrupt_and_verify_one_thread_non_stop { ampersand } {
+    global gdb_prompt NR_THREADS
+
+    for { set i 0 } { $i < $NR_THREADS } { incr i } {
+	set thread_nr [expr 2 + $i]
+
+	gdb_test "thread $thread_nr" \
+	    "Switching to thread $thread_nr.*running.*"
+
+	set test_name [string trim "interrupt $ampersand"]
+	set test_name "$test_name, thread $thread_nr"
+	gdb_test_multiple "interrupt $ampersand" $test_name {
+	    -re "$gdb_prompt " {
+		if { "$ampersand" == "&" } {
+		    exp_continue
+		}
+		pass $test_name
+	    }
+	    -re "\n\[^\r\]+Thread \[^\r\]+ stopped\[^\r\]+\r" {
+		if { "$ampersand" != "&" } {
+		    exp_continue
+		}
+		pass $test_name
+	    }
+	    timeout {
+		fail "$test_name (timeout)"
+	    }
+	}
+
+	set test_name "thread $thread_nr stopped"
+	gdb_test_multiple "info thread $thread_nr" $test_name {
+	    -re "running.*$gdb_prompt $" {
+		fail $test_name
+	    }
+	    -re "$gdb_prompt $" {
+		pass $test_name
+	    }
+	    timeout {
+		fail "$test_name (timeout)"
+		set i $NR_THREADS
+	    }
+	}
+    }
+}
+
+proc interrupt_all_threads_non_stop { command ampersand nr_threads_running } {
+    global gdb_prompt NR_THREADS
+
+    set thread_stopped_count 0
+    set test_name [string trim "$command $ampersand"]
+
+    gdb_test_multiple "$command $ampersand" $test_name {
+	-re "\n\[^\r\]+Thread \[^\r\]+ stopped\[^\r\]+\r" {
+	    verbose -log "got stopped thread"
+	    incr thread_stopped_count
+	    if { "$ampersand" == "&" } {
+		if { $thread_stopped_count == $nr_threads_running } {
+		    pass $test_name
+		} else {
+		    exp_continue
+		}
+	    } else {
+		exp_continue
+	    }
+	}
+	-re "\nNo unwaited-for children left.\[\r\n\]+$gdb_prompt $" {
+	    # This appeared during development.  Ensure it doesn't come back.
+	    fail "$test_name (No unwaited-for children left)"
+	}
+	-re "$gdb_prompt " {
+	    if { "$ampersand" == "&" } {
+		exp_continue
+	    }
+	    if { $thread_stopped_count != $nr_threads_running } {
+		fail "$test_name (missing thread)"
+	    } else {
+		pass $test_name
+	    }
+	}
+	timeout {
+	    fail "$test_name (timeout)"
+	}
+    }
+}
+
+proc verify_all_threads_stopped { } {
+    global gdb_prompt NR_THREADS
+
+    set test_name "info threads"
+    set running_count 0
+    set thread_count 0
+
+    gdb_test_multiple "info threads" $test_name {
+	-re "\n\[ \]*\[\[:digit:\]\]+ +Thread \[^\r\]+\\(running\\)\r" {
+	    verbose -log "got running thread"
+	    incr running_count
+	    incr thread_count
+	    exp_continue
+	}
+	-re "\n\[*\]?\[ \]*\[\[:digit:\]\]+ +Thread \[^\r\]+\r" {
+	    verbose -log "got stopped thread"
+	    incr thread_count
+	    exp_continue
+	}
+	-re "$gdb_prompt $" {
+	    if { $thread_count != [expr $NR_THREADS + 1] } {
+		fail "$test_name (missing thread)"
+	    } elseif { $running_count != 0 } {
+		fail "$test_name (running thread)"
+	    } else {
+		pass $test_name
+	    }
+	}
+	timeout {
+	    fail "$test_name (timeout)"
+	}
+    }
+}
+
+proc interrupt_and_verify_all_threads_non_stop { ampersand } {
+    global gdb_prompt NR_THREADS
+
+    # These tests are repeated just for stress testing purposes.
+
+    for { set i 0 } { $i < $NR_THREADS } { incr i } {
+	set thread_nr [expr 2 + $i]
+	with_test_prefix "thread ${thread_nr}" {
+	    gdb_test "c -a &" "Continuing."
+	    gdb_test "thread $thread_nr" \
+		"Switching to thread $thread_nr.*running.*"
+	    interrupt_all_threads_non_stop "interrupt -a" $ampersand [expr $NR_THREADS + 1]
+	    verify_all_threads_stopped
+	}
+    }
+}
+
+proc test_sync_all_stop { } {
+    with_test_prefix "sync, all-stop" {
+	prepare_interrupt_test off off
+
+	gdb_test "interrupt" "Current thread is already stopped."
+	gdb_test "interrupt -a" "All threads are already stopped."
+    }
+}
+
+proc test_async_all_stop { } {
+    global NR_THREADS
+
+    with_test_prefix "async, all-stop" {
+	prepare_interrupt_test on off
+
+	# Do these tests several times because there was a bug during
+	# development when doing two in a row.
+
+	for { set i 0 } { $i < $NR_THREADS } { incr i } {
+	    set thread_nr [expr 2 + $i]
+	    with_test_prefix "interrupt, thread ${thread_nr}" {
+		gdb_test "c &" "Continuing."
+		wait_thread_running $thread_nr
+		gdb_test "thread ${thread_nr}" \
+		    "Switching to thread ${thread_nr}.*running.*"
+		interrupt_all_stop "interrupt"
+		verify_all_threads_stopped
+	    }
+	}
+
+	for { set i 0 } { $i < $NR_THREADS } { incr i } {
+	    set thread_nr [expr 2 + $i]
+	    with_test_prefix "interrupt -a, thread ${thread_nr}" {
+		gdb_test "c &" "Continuing."
+		wait_thread_running $thread_nr
+		gdb_test "thread ${thread_nr}" \
+		    "Switching to thread ${thread_nr}.*running.*"
+		interrupt_all_stop "interrupt -a"
+		verify_all_threads_stopped
+	    }
+	}
+    }
+
+    with_test_prefix "async, all-stop, &" {
+	prepare_interrupt_test on off
+
+	# These tests are repeated just for stress testing purposes.
+
+	for { set i 0 } { $i < $NR_THREADS } { incr i } {
+	    set thread_nr [expr 2 + $i]
+	    with_test_prefix "interrupt, thread ${thread_nr}" {
+		gdb_test "c &" "Continuing."
+		wait_thread_running $thread_nr
+		gdb_test "thread ${thread_nr}" \
+		    "Switching to thread ${thread_nr}.*running.*"
+		interrupt_all_threads_all_stop_background "interrupt"
+		verify_all_threads_stopped
+	    }
+	}
+
+	for { set i 0 } { $i < $NR_THREADS } { incr i } {
+	    set thread_nr [expr 2 + $i]
+	    with_test_prefix "interrupt -a, thread ${thread_nr}" {
+		gdb_test "c &" "Continuing."
+		wait_thread_running $thread_nr
+		gdb_test "thread ${thread_nr}" \
+		    "Switching to thread ${thread_nr}.*running.*"
+		interrupt_all_threads_all_stop_background "interrupt -a"
+		verify_all_threads_stopped
+	    }
+	}
+    }
+}
+
+proc test_sync_non_stop { } {
+    global NR_THREADS
+
+    with_test_prefix "sync, non-stop, one thread at a time" {
+	prepare_interrupt_test off on
+
+	interrupt_and_verify_one_thread_non_stop ""
+    }
+
+    with_test_prefix "sync, non-stop, all threads" {
+	prepare_interrupt_test off on
+
+	# IWBN to use interrupt_and_verify_all_threads_non_stop here,
+	# but it uses & to continually resume the threads, which we can't do.
+	# So we just test once, until we know the extra coverage is needed
+	# here.  One suggestions is to put a breakpoint on do_something.
+	interrupt_all_threads_non_stop "interrupt -a" "" $NR_THREADS
+	verify_all_threads_stopped
+    }
+}
+
+proc test_async_non_stop { } {
+    global NR_THREADS
+
+    with_test_prefix "async, non-stop, one thread at a time" {
+	prepare_interrupt_test on on
+
+	interrupt_and_verify_one_thread_non_stop ""
+    }
+
+    with_test_prefix "async, non-stop, one thread at a time, &" {
+	prepare_interrupt_test on on
+
+	interrupt_and_verify_one_thread_non_stop "&"
+    }
+
+    with_test_prefix "async, non-stop, all threads" {
+	prepare_interrupt_test on on
+
+	interrupt_and_verify_all_threads_non_stop ""
+    }
+
+    with_test_prefix "async, non-stop, all threads, &" {
+	prepare_interrupt_test on on
+
+	interrupt_and_verify_all_threads_non_stop "&"
+    }
+}
+
+test_sync_all_stop
+test_async_all_stop
+test_sync_non_stop
+test_async_non_stop
+
+# That's it for standard testing.
+# The rest of the file is specialized testing.
+
+# Verify "interrupt" waits for the right thread in non-stop.
+# This test is inheritantly racy, even if it succeeds we're not testing
+# the right things to verify success wasn't a fluke.  But it has been
+# quite useful in practice.
+
+proc test_right_thread_stopped { } {
+    global gdb_prompt
+
+    with_test_prefix "test_right_thread_stopped" {
+	prepare_interrupt_test on on
+	# At this point thread 1 is stopped, threads 2-5 are running.
+	gdb_test_multiple "define interrupt2" "define user command: interrupt2" {
+	    -re "Type commands for definition of \"interrupt2\".\r\nEnd with a line saying just \"end\".\r\n>$" {
+		gdb_test "thread 2\ninterrupt &\nthread 3\ninterrupt\nend" "" \
+		    "define user command: interrupt2"
+	    }
+	}
+	set test_name "interrupt2"
+	set prompt_seen 0
+	set thread2_stopped 0
+	set thread3_stopped 0
+	# Turn on command tracing, makes it easier to match output to the
+	# commands in the interrupt2 macro.
+	gdb_test_no_output "set trace-commands on"
+	gdb_test_multiple "interrupt2" $test_name {
+	    -re "\[\r\n\]+\[^\r\]+Thread \[^\r\]+ #3 stopped\[^\r\]+\[\r\n\]+" {
+		# Note: There may also be a notification that thread 2 stopped
+		# before we see the gdb prompt.  However, it can also occur
+		# after the prompt: be careful not to add a regexp for thread 2
+		# here lest it swallow the prompt!
+		verbose -log "saw thread 3"
+		set thread3_stopped 1
+		exp_continue
+	    }
+	    -re "$gdb_prompt " {
+		# Don't put a trailing $ on the regexp here.
+		# If expect buffers the thread 2 stopped output, we lose.
+		if { $thread3_stopped } {
+		    pass $test_name
+		} else {
+		    fail "$test_name (thread 3 not stopped before prompt)"
+		}
+	    }
+	    timeout {
+		fail "$test_name (timeout)"
+	    }
+	}
+	# We can't use gdb_test_no_output here because the command is traced.
+	gdb_test "set trace-commands off" ".*"
+	# One last thing, verify thread 3 is still the current thread.
+	verify_current_thread 3
+    }
+}
+
+test_right_thread_stopped