diff options
author | Pedro Alves <palves@redhat.com> | 2014-07-09 17:52:58 +0100 |
---|---|---|
committer | Pedro Alves <palves@redhat.com> | 2014-07-09 17:52:58 +0100 |
commit | 1fe2833b6dd03602ba86aa334e81466ea9abe66a (patch) | |
tree | 8520c25af7784886891634b1b86c9e93d47edc77 | |
parent | 0318afefeff53becc1e3e738793ecdb6653629f4 (diff) | |
download | gdb-upstream/gdb-7.8-branch.zip gdb-upstream/gdb-7.8-branch.tar.gz gdb-upstream/gdb-7.8-branch.tar.bz2 |
Fix "attach" command vs user input raceupstream/gdb-7.8-branch
On async targets, a synchronous attach is done like this:
#1 - target_attach is called (PTRACE_ATTACH is issued)
#2 - a continuation is installed
#3 - we go back to the event loop
#4 - target reports stop (SIGSTOP), event loop wakes up, and
attach continuation is called
#5 - among other things, the continuation calls
target_terminal_inferior, which removes stdin from the event
loop
Note that in #3, GDB is still processing user input. If the user is
fast enough, e.g., with something like:
echo -e "attach PID\nset xxx=1" | gdb
... then the "set" command is processed before the attach completes.
We get worse behavior even, if input is a tty and therefore
readline/editing is enabled, with e.g.,:
(gdb) attach PID\nset xxx=1
we then crash readline/gdb, with:
Attaching to program: attach-wait-input, process 14537
readline: readline_callback_read_char() called with no handler!
Aborted
$
Fix this by calling target_terminal_inferior before #3 above.
The test covers both scenarios by running with editing/readline forced
to both on and off.
gdb/
2014-07-09 Pedro Alves <palves@redhat.com>
* infcmd.c (attach_command_post_wait): Don't call
target_terminal_inferior here.
(attach_command): Call it here instead.
gdb/testsuite/
2014-07-09 Pedro Alves <palves@redhat.com>
* gdb.base/attach-wait-input.exp: New file.
* gdb.base/attach-wait-input.c: New file.
-rw-r--r-- | gdb/ChangeLog | 6 | ||||
-rw-r--r-- | gdb/infcmd.c | 19 | ||||
-rw-r--r-- | gdb/testsuite/ChangeLog | 5 | ||||
-rw-r--r-- | gdb/testsuite/gdb.base/attach-wait-input.c | 40 | ||||
-rw-r--r-- | gdb/testsuite/gdb.base/attach-wait-input.exp | 119 |
5 files changed, 186 insertions, 3 deletions
diff --git a/gdb/ChangeLog b/gdb/ChangeLog index d4304e6..1bd19fc 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,9 @@ +2014-07-09 Pedro Alves <palves@redhat.com> + + * infcmd.c (attach_command_post_wait): Don't call + target_terminal_inferior here. + (attach_command): Call it here instead. + 2014-07-07 Pedro Alves <palves@redhat.com> PR gdb/17096 diff --git a/gdb/infcmd.c b/gdb/infcmd.c index df4fd40..a712b6f 100644 --- a/gdb/infcmd.c +++ b/gdb/infcmd.c @@ -2381,9 +2381,6 @@ attach_command_post_wait (char *args, int from_tty, int async_exec) post_create_inferior (¤t_target, from_tty); - /* Install inferior's terminal modes. */ - target_terminal_inferior (); - if (async_exec) { /* The user requested an `attach&', so be sure to leave threads @@ -2499,6 +2496,22 @@ attach_command (char *args, int from_tty) based on what modes we are starting it with. */ target_terminal_init (); + /* Install inferior's terminal modes. This may look like a no-op, + as we've just saved them above, however, this does more than + restore terminal settings: + + - installs a SIGINT handler that forwards SIGINT to the inferior. + Otherwise a Ctrl-C pressed just while waiting for the initial + stop would end up as a spurious Quit. + + - removes stdin from the event loop, which we need if attaching + in the foreground, otherwise on targets that report an initial + stop on attach (which are most) we'd process input/commands + while we're in the event loop waiting for that stop. That is, + before the attach continuation runs and the command is really + finished. */ + target_terminal_inferior (); + /* Set up execution context to know that we should return from wait_for_inferior as soon as the target reports a stop. */ init_wait_for_inferior (); diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 7667969..66dcc95 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2014-07-09 Pedro Alves <palves@redhat.com> + + * gdb.base/attach-wait-input.exp: New file. + * gdb.base/attach-wait-input.c: New file. + 2014-06-23 Siva Chandra Reddy <sivachandra@google.com> * gdb.python/py-xmethods.exp: Use "progspace" instead of the diff --git a/gdb/testsuite/gdb.base/attach-wait-input.c b/gdb/testsuite/gdb.base/attach-wait-input.c new file mode 100644 index 0000000..bddf2e1 --- /dev/null +++ b/gdb/testsuite/gdb.base/attach-wait-input.c @@ -0,0 +1,40 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 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/>. */ + +#include <sys/types.h> +#include <unistd.h> + +volatile int should_exit = 0; +int mypid; + +static void +setup_done (void) +{ +} + +int +main (void) +{ + unsigned int counter = 1; + + mypid = getpid (); + setup_done (); + + for (counter = 0; !should_exit && counter < 100; counter++) + sleep (1); + return 0; +} diff --git a/gdb/testsuite/gdb.base/attach-wait-input.exp b/gdb/testsuite/gdb.base/attach-wait-input.exp new file mode 100644 index 0000000..d6d572e --- /dev/null +++ b/gdb/testsuite/gdb.base/attach-wait-input.exp @@ -0,0 +1,119 @@ +# Copyright 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/>. */ + +# Verify that GDB waits for the "attach" command to finish before +# processing the following command. +# +# GDB used to have a race where on async targets, in the small window +# between the attach request and the initial stop for the attach, GDB +# was still processing user input. +# +# The issue was originally detected with: +# +# echo -e "attach PID\nset xxx=1" | gdb +# +# In that scenario, stdin is not a tty, which disables readline. +# Explicitly turning off editing exercises the same code path, and is +# simpler to do, so we test with both editing on and off. + +# The test uses the "attach" command. +if [target_info exists use_gdb_stub] { + return +} + +standard_testfile + +if {[build_executable "failed to build" $testfile $srcfile debug]} { + return -1 +} + +# Start the program running, and return its PID, ready for attaching. + +proc start_program {binfile} { + global gdb_prompt + global decimal + + clean_restart $binfile + + if ![runto setup_done] then { + fail "Can't run to setup_done" + return 0 + } + + # Get the PID of the test process. + set testpid "" + set test "get inferior process ID" + gdb_test_multiple "p mypid" $test { + -re " = ($decimal)\r\n$gdb_prompt $" { + set testpid $expect_out(1,string) + pass $test + } + } + + gdb_test "detach" "Detaching from program: .*" + + if {$testpid == ""} { + return + } + + return $testpid +} + +# Do test proper. EDITING indicates whether "set editing" is on or +# off. + +proc test { editing } { + global gdb_prompt + global binfile + global decimal + + with_test_prefix "editing $editing" { + + set testpid [start_program $binfile] + if {$testpid == ""} { + return + } + + # Enable/disable readline. + gdb_test_no_output "set editing $editing" + + # Send both commands at once. + send_gdb "attach $testpid\nprint should_exit = 1\n" + + # Use gdb_expect directly instead of gdb_test_multiple to + # avoid races with the double prompt. + set test "attach and print" + gdb_expect { + -re "Attaching to program.*process $testpid\r\n.*$gdb_prompt.*$decimal = 1\r\n$gdb_prompt $" { + pass "$test" + } + timeout { + fail "$test (timeout)" + } + } + + # As we've used attach, on quit, we'll detach from the + # program. Explicitly kill it in case we failed above. + gdb_test "kill" \ + "" \ + "after attach, exit" \ + "Kill the program being debugged.*y or n. $" \ + "y" + } +} + +foreach editing {"on" "off"} { + test $editing +} |