From: Pedro Alves <palves@redhat.com>
To: gdb-patches@sourceware.org
Subject: [PATCH v3 34/34] Always switch fork child to the main UI
Date: Fri, 06 May 2016 12:45:00 -0000 [thread overview]
Message-ID: <1462538104-19109-35-git-send-email-palves@redhat.com> (raw)
In-Reply-To: <1462538104-19109-1-git-send-email-palves@redhat.com>
The following scenario:
- gdb started in normal cli mode.
- separate MI channel created with MI
- 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:
yyyy-mm-dd 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:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* gdb.mi/mi-exec-run.exp: New file.
---
gdb/fork-child.c | 22 ++++-
gdb/testsuite/gdb.mi/mi-exec-run.exp | 158 +++++++++++++++++++++++++++++++++++
2 files changed, 178 insertions(+), 2 deletions(-)
create mode 100644 gdb/testsuite/gdb.mi/mi-exec-run.exp
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/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}
+ }
+ }
+}
--
2.5.5
next prev parent reply other threads:[~2016-05-06 12:45 UTC|newest]
Thread overview: 71+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-05-06 12:35 [PATCH v3 00/34] Towards great frontend GDB consoles Pedro Alves
2016-05-06 12:35 ` [PATCH v3 24/34] Push thread->control.command_interp to the struct thread_fsm Pedro Alves
2016-07-01 11:02 ` Thomas Preudhomme
[not found] ` <20144b4c-11ee-fc84-e3ad-b9992f14ce15@redhat.com>
2016-07-01 15:24 ` [PATCH] Build gdb.opt/inline-*.exp tests at -O0, rely on __attribute__((always_inline)) (was: Re: [PATCH v3 24/34] Push thread->control.command_interp to the struct thread_fsm) Thomas Preudhomme
2016-07-15 12:05 ` Thomas Preudhomme
2016-07-19 17:02 ` [PATCH] Build gdb.opt/inline-*.exp tests at -O0, rely on __attribute__((always_inline)) Pedro Alves
2016-07-20 16:35 ` Thomas Preudhomme
2016-05-06 12:35 ` [PATCH v3 29/34] Add new command to create extra console/mi UI channels Pedro Alves
2016-05-26 18:34 ` Pedro Alves
2016-05-06 12:35 ` [PATCH v3 03/34] Introduce "struct ui" Pedro Alves
2016-05-06 12:35 ` [PATCH v3 16/34] Make target_terminal_inferior/ours almost nops on non-main UIs Pedro Alves
2016-05-06 12:35 ` [PATCH v3 15/34] Always process target events in the main UI Pedro Alves
2016-05-06 12:35 ` [PATCH v3 01/34] Prepare gdb.python/mi-py-events.exp for Python/MI in separate channels Pedro Alves
2016-05-06 12:35 ` [PATCH v3 14/34] Make command line editing (use of readline) be per UI Pedro Alves
2016-05-06 12:35 ` [PATCH v3 21/34] Replace the sync_execution global with a new enum prompt_state tristate Pedro Alves
2016-05-06 12:35 ` [PATCH v3 20/34] Make gdb_in_secondary_prompt_p() be per UI Pedro Alves
2016-05-06 12:35 ` [PATCH v3 33/34] Make mi-break.exp always expect breakpoint commands output on the main UI Pedro Alves
2016-05-06 12:35 ` [PATCH v3 02/34] [Ada catchpoints] Fix "warning: failed to get exception name: No definition of \"e.full_name\" in current context" Pedro Alves
2016-05-06 12:36 ` [PATCH v3 31/34] Add testing infrastruture bits for running with MI on a separate UI Pedro Alves
2016-06-28 20:19 ` Simon Marchi
2016-06-29 10:50 ` Pedro Alves
2016-06-30 11:12 ` [pushed] Fix gdbserver/MI testing regression (was: Re: [PATCH v3 31/34] Add testing infrastruture bits for running with MI on a separate UI) Pedro Alves
2016-06-30 12:10 ` gdbserver/ada testing broken (was: Re: [pushed] Fix gdbserver/MI testing regression) Pedro Alves
2016-07-04 20:40 ` gdbserver/ada testing broken Simon Marchi
2016-07-05 15:28 ` Joel Brobecker
2016-07-05 15:47 ` Joel Brobecker
2016-07-05 16:36 ` gdbserver/ada testing broken (was: Re: [pushed] Fix gdbserver/MI testing regression) Joel Brobecker
2016-07-05 17:19 ` gdbserver/ada testing broken Simon Marchi
2016-07-06 13:23 ` Joel Brobecker
2016-07-06 14:28 ` Simon Marchi
2016-07-19 17:11 ` Pedro Alves
2016-07-04 17:22 ` [pushed] Fix gdbserver/MI testing regression Simon Marchi
2016-05-06 12:40 ` [PATCH v3 11/34] Make out and error streams be per UI Pedro Alves
2016-05-06 12:40 ` [PATCH v3 13/34] Make current_ui_out " Pedro Alves
2016-05-06 12:40 ` [PATCH v3 23/34] New function should_print_stop_to_console Pedro Alves
2016-05-06 12:41 ` [PATCH v3 06/34] Introduce interpreter factories Pedro Alves
2016-05-18 19:18 ` Simon Marchi
2016-05-26 18:11 ` Pedro Alves
2016-05-18 19:20 ` Simon Marchi
2016-05-26 18:08 ` Pedro Alves
2016-05-06 12:42 ` [PATCH v3 30/34] [DOC] Document support for running interpreters on separate UI channels Pedro Alves
2016-05-06 13:04 ` Eli Zaretskii
2016-05-26 11:11 ` Pedro Alves
2016-06-17 17:24 ` Pedro Alves
2016-06-17 20:02 ` Eli Zaretskii
2016-05-06 12:43 ` [PATCH v3 12/34] Delete def_uiout Pedro Alves
2016-05-06 12:43 ` [PATCH v3 04/34] Make gdb_stdout&co be per UI Pedro Alves
2016-05-06 12:43 ` [PATCH v3 17/34] Introduce display_mi_prompt Pedro Alves
2016-05-06 12:43 ` [PATCH v3 05/34] Make the interpreters be per UI Pedro Alves
2016-05-18 17:51 ` Simon Marchi
2016-05-26 18:08 ` Pedro Alves
2016-05-06 12:43 ` [PATCH v3 10/34] Make input_fd " Pedro Alves
2016-05-06 12:43 ` [PATCH v3 07/34] Make the intepreters output to all UIs Pedro Alves
2016-05-19 15:16 ` Simon Marchi
2016-05-26 18:12 ` Pedro Alves
2016-05-06 12:43 ` [PATCH v3 25/34] Only send sync execution command output to the UI that ran the command Pedro Alves
2016-05-06 12:43 ` [PATCH v3 08/34] Always run async signal handlers in the main UI Pedro Alves
2016-05-19 19:28 ` Simon Marchi
2016-05-26 18:13 ` Pedro Alves
2016-05-26 18:15 ` Simon Marchi
2016-05-06 12:43 ` [PATCH v3 28/34] Make stdin be per UI Pedro Alves
2016-05-06 12:45 ` Pedro Alves [this message]
2016-05-06 12:45 ` [PATCH v3 32/34] Send deleted watchpoint-scope output to all UIs Pedro Alves
2016-05-06 12:45 ` [PATCH v3 22/34] Fix for spurious prompts in secondary UIs Pedro Alves
2016-05-06 12:45 ` [PATCH v3 27/34] Handle UI's terminal closing Pedro Alves
2016-05-06 12:45 ` [PATCH v3 26/34] Make main_ui be heap allocated Pedro Alves
2016-05-06 12:52 ` [PATCH v3 18/34] Make raw_stdout be per MI instance Pedro Alves
2016-05-06 12:53 ` [PATCH v3 09/34] Make instream be per UI Pedro Alves
2016-05-06 12:53 ` [PATCH v3 19/34] Simplify starting the command event loop Pedro Alves
2016-05-26 18:37 ` [PATCH v3 35/34] Add "new-ui console" tests Pedro Alves
2016-06-21 0:23 ` [pushed] Re: [PATCH v3 00/34] Towards great frontend GDB consoles Pedro Alves
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1462538104-19109-35-git-send-email-palves@redhat.com \
--to=palves@redhat.com \
--cc=gdb-patches@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox