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.
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_all_threads_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.
@@ -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
@@ -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
@@ -2700,29 +2700,182 @@ 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
+{
+ int all_threads;
+};
+
+/* 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 all threads to stopped.
+ 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_all_threads_stopped (void)
+{
+ struct cleanup* cleanups = make_cleanup_restore_current_thread ();
+
+ gdb_assert (non_stop);
+
+ while (any_running ())
+ {
+ struct target_waitstatus last;
+ ptid_t last_ptid;
+
+ QUIT;
+ wait_for_inferior ();
+ get_last_target_status (&last_ptid, &last);
+ finish_thread_state (last_ptid);
+ print_stop_event (&last);
+ }
+
+ do_cleanups (cleanups);
+}
+
+/* Finish up waiting for the current thread to stop, or continue waiting
+ for all threads to stop ALL_THREADS is non-zero.
+ Note: This function is named the way it is as it's derived from
+ attach_command_post_wait. */
+
+static void
+interrupt_command_post_wait (int all_threads)
+{
+ 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 (all_threads && non_stop)
+ wait_all_threads_stopped ();
+}
+
+/* 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->all_threads);
+}
+
+/* Queue a continuation to run when a thread has stopped.
+ This is used by the interrupt command in target-async mode where we need to
+ leave waiting for threads to stop to the main loop. */
+
+static void
+add_interrupt_continuation (int all_threads)
+{
+ struct interrupt_command_continuation_args *a;
+
+ a = xmalloc (sizeof (*a));
+ a->all_threads = all_threads;
+
+ if (all_threads)
+ {
+ add_inferior_continuation (interrupt_command_continuation, a,
+ interrupt_command_continuation_free_args);
+ }
+ else
+ {
+ struct thread_info *tp = inferior_thread ();
+
+ add_continuation (tp, 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;
+
+ /* 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 (¤t_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);
+ else
+ {
+ gdb_assert (non_stop);
+ if (all_threads)
+ wait_all_threads_stopped ();
+ else
+ {
+ wait_for_inferior ();
+ normal_stop ();
+ }
}
}
@@ -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
}
}
@@ -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
}
}
@@ -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
}
}
new file mode 100644
@@ -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 (10);
+}
+
+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;
+}
new file mode 100644
@@ -0,0 +1,332 @@
+# 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)"
+ }
+ }
+}
+
+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" "" {
+ -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" "" {
+ -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."
+ 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."
+ 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."
+ 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."
+ 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