diff options
author | Pedro Alves <palves@redhat.com> | 2016-06-21 01:11:57 +0100 |
---|---|---|
committer | Pedro Alves <palves@redhat.com> | 2016-06-21 01:11:57 +0100 |
commit | 49940788ab38b9d58c663cf38855f29c0ebb1b55 (patch) | |
tree | ef322e16fd2feb6219aacba550aac37b9b38772f | |
parent | ef274d26b57336b3baa5bb0ae93b49178bc45631 (diff) | |
download | gdb-49940788ab38b9d58c663cf38855f29c0ebb1b55.zip gdb-49940788ab38b9d58c663cf38855f29c0ebb1b55.tar.gz gdb-49940788ab38b9d58c663cf38855f29c0ebb1b55.tar.bz2 |
Always switch fork child to the main UI
The following scenario:
- gdb started in normal CLI mode.
- separate MI channel created with new-ui
- inferior output redirected with the "set inferior-tty" command.
- use -exec-run in the MI channel to run the inferior
is presently mishandled.
When we create the inferior, in fork-child.c, right after vfork, we'll
close all the file descriptors in the vfork child, and then dup the
tty to file descriptors 0/1/2, create a session, etc. Note that when
we close all descriptors, we close the file descriptors behind
gdb_stdin/gdb_stdout/gdb_stderr of all secondary UIs... So if
anything goes wrong in the child and it calls warning/error, it'll end
up writting to the current UI's stdout/stderr streams, which are
backed by file descriptors that have since been closed. Because this
happens in a vfork region, the corresponding stdin/stdout/stderr in
the parent/gdb end up corrupted.
The fix is to switch to the main UI right after the vfork, so that
gdb_stdin/gdb_stdout/gdb_stderr are correctly mapped to
stdin/stdout/stderr (and thus to file descriptors 0/1/2), so this code
works as it has always worked.
(Technically, we're doing a lot of stuff we shouldn't be doing after a
vfork, while we should only be calling async-signal-safe functions.)
gdb/ChangeLog:
2016-06-21 Pedro Alves <palves@redhat.com>
* fork-child.c (fork_inferior): Switch the child to the main UI
right after vfork. Save/restore the current UI in the parent.
Flush outputs of the main UI instead of the current UI.
gdb/testsuite/ChangeLog:
2016-06-21 Pedro Alves <palves@redhat.com>
* gdb.mi/mi-exec-run.exp: New file.
-rw-r--r-- | gdb/ChangeLog | 6 | ||||
-rw-r--r-- | gdb/fork-child.c | 22 | ||||
-rw-r--r-- | gdb/testsuite/ChangeLog | 4 | ||||
-rw-r--r-- | gdb/testsuite/gdb.mi/mi-exec-run.exp | 158 |
4 files changed, 188 insertions, 2 deletions
diff --git a/gdb/ChangeLog b/gdb/ChangeLog index d494dd5..af8f925 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,5 +1,11 @@ 2016-06-21 Pedro Alves <palves@redhat.com> + * fork-child.c (fork_inferior): Switch the child to the main UI + right after vfork. Save/restore the current UI in the parent. + Flush outputs of the main UI instead of the current UI. + +2016-06-21 Pedro Alves <palves@redhat.com> + * breakpoint.c (watchpoint_check): Send watchpoint-deleted output to all UIs. diff --git a/gdb/fork-child.c b/gdb/fork-child.c index 204b7cf..8ac3bef 100644 --- a/gdb/fork-child.c +++ b/gdb/fork-child.c @@ -31,6 +31,7 @@ #include "gdbcmd.h" #include "solib.h" #include "filestuff.h" +#include "top.h" #include <signal.h> @@ -141,6 +142,7 @@ fork_inferior (char *exec_file_arg, char *allargs, char **env, struct inferior *inf; int i; int save_errno; + struct ui *save_ui; /* If no exec file handed to us, get it from the exec-file command -- with a good, common error message if none is specified. */ @@ -275,6 +277,9 @@ fork_inferior (char *exec_file_arg, char *allargs, char **env, restore it. */ save_our_env = environ; + /* Likewise the current UI. */ + save_ui = current_ui; + /* Tell the terminal handling subsystem what tty we plan to run on; it will just record the information for later. */ new_tty_prefork (inferior_io_terminal); @@ -282,8 +287,8 @@ fork_inferior (char *exec_file_arg, char *allargs, char **env, /* It is generally good practice to flush any possible pending stdio output prior to doing a fork, to avoid the possibility of both the parent and child flushing the same data after the fork. */ - gdb_flush (gdb_stdout); - gdb_flush (gdb_stderr); + gdb_flush (main_ui->m_gdb_stdout); + gdb_flush (main_ui->m_gdb_stderr); /* If there's any initialization of the target layers that must happen to prepare to handle the child we're about fork, do it @@ -312,6 +317,16 @@ fork_inferior (char *exec_file_arg, char *allargs, char **env, if (pid == 0) { + /* Switch to the main UI, so that gdb_std{in/out/err} in the + child are mapped to std{in/out/err}. This makes it possible + to use fprintf_unfiltered/warning/error/etc. in the child + from here on. */ + current_ui = main_ui; + + /* Close all file descriptors except those that gdb inherited + (usually 0/1/2), so they don't leak to the inferior. Note + that this closes the file descriptors of all secondary + UIs. */ close_most_fds (); if (debug_fork) @@ -378,6 +393,9 @@ fork_inferior (char *exec_file_arg, char *allargs, char **env, /* Restore our environment in case a vforked child clob'd it. */ environ = save_our_env; + /* Likewise the current UI. */ + current_ui = save_ui; + if (!have_inferiors ()) init_thread_list (); diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index ee27566..6790c45 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,5 +1,9 @@ 2016-06-21 Pedro Alves <palves@redhat.com> + * gdb.mi/mi-exec-run.exp: New file. + +2016-06-21 Pedro Alves <palves@redhat.com> + * gdb.mi/mi-break.exp (test_breakpoint_commands): Always expect breakpoint command's output on the main UI. (test_break): New procedure, factored out from calls in the top diff --git a/gdb/testsuite/gdb.mi/mi-exec-run.exp b/gdb/testsuite/gdb.mi/mi-exec-run.exp new file mode 100644 index 0000000..a550a7f --- /dev/null +++ b/gdb/testsuite/gdb.mi/mi-exec-run.exp @@ -0,0 +1,158 @@ +# Copyright 2016 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 that -exec-run works as expected. Exercises various testing +# axes: +# +# - MI running on main UI vs separate UI. +# +# - inferior tty set to main tty vs separate tty. +# +# - forking the child failing and sending output to the right inferior +# terminal, vs the child not failing to start. + +load_lib mi-support.exp +set MIFLAGS "-i=mi" + +# The purpose of this testcase is to test the -exec-run command. If we +# cannot use it, then there is no point in running this testcase. +if [target_info exists use_gdb_stub] { + untested "cannot use -exec-run command" + return -1 +} + +standard_testfile mi-start.c + +if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } { + untested "could not build mi-exec-run" + return -1 +} + +# The test proper. INFTTY_MODE determines whether "set inferior-tty" +# is in effect. MI_MODE determines whether MI is run on the main UI, +# or as a separate UI. FORCE_FAIL is true when we want -exec-run to +# fail and cause inferior output be sent to the inferior tty. + +proc test {inftty_mode mi_mode force_fail} { + global srcdir subdir binfile srcfile + global gdb_spawn_id gdb_main_spawn_id mi_spawn_id inferior_spawn_id + global decimal + + mi_gdb_exit + + set start_ops {} + if {$inftty_mode == "separate"} { + lappend start_ops "separate-inferior-tty" + } + if {$mi_mode == "separate"} { + lappend start_ops "separate-mi-tty" + } + + if [eval mi_gdb_start $start_ops] { + return + } + + if {$force_fail} { + # Disable the shell so that its the first exec that fails, + # instead of the shell starting and then failing with some + # unspecified output. + mi_gdb_test "-gdb-set startup-with-shell off" ".*" + set bin $binfile.nox + } else { + set bin $binfile + } + + mi_delete_breakpoints + mi_gdb_reinitialize_dir $srcdir/$subdir + mi_gdb_reinitialize_dir $srcdir/$subdir + mi_gdb_load ${bin} + + # Useful for debugging: + verbose -log "Channels:" + verbose -log " inferior_spawn_id=$inferior_spawn_id" + verbose -log " gdb_spawn_id=$gdb_spawn_id" + verbose -log " gdb_main_spawn_id=$gdb_main_spawn_id" + verbose -log " mi_spawn_id=$mi_spawn_id" + + if {$force_fail} { + set saw_perm_error 0 + set saw_mi_error 0 + set test "run failure detected" + send_gdb "-exec-run --start\n" + + while {1} { + gdb_expect { + -i "$inferior_spawn_id" + -re ".*Cannot exec.*Permission denied" { + set saw_perm_error 1 + verbose -log "saw mi error" + } + -i "$gdb_spawn_id" + -re "\\^error,msg=\"During startup program exited with code 127" { + set saw_mi_error 1 + verbose -log "saw mi error" + } + timeout { + fail "$test (timeout)" + break + } + -i "$gdb_main_spawn_id" + eof { + fail "$test (eof)" + break + } + } + + if {$saw_perm_error && $saw_mi_error} { + pass $test + break + } + } + } else { + mi_run_cmd "--start" + mi_expect_stop "breakpoint-hit" "main" "" ".*$srcfile" "$decimal" \ + { "" "disp=\"del\"" } "breakpoint hit reported on mi" + + if {$mi_mode == "separate"} { + # Check that the breakpoint hit is reported on the main + # UI/CLI. Note no prompt is expected. + switch_gdb_spawn_id $gdb_main_spawn_id + + set test "breakpoint hit reported on console" + gdb_test_multiple "" $test { + -re "Temporary breakpoint .*, main \\(\\) at .*$srcfile:$decimal.*return 0;" { + pass $test + } + } + + # Switch back to the MI UI. + global mi_spawn_id + switch_gdb_spawn_id $mi_spawn_id + } + } +} + +# Create a not-executable copy of the program, in order to exercise +# vfork->exec failing. +gdb_remote_download host $binfile $binfile.nox +remote_spawn target "chmod \"a-x\" $binfile.nox" + +foreach_with_prefix inferior-tty {"main" "separate"} { + foreach_with_prefix mi {"main" "separate"} { + foreach_with_prefix force-fail {0 1} { + test ${inferior-tty} ${mi} ${force-fail} + } + } +} |