[binutils-gdb] Always switch fork child to the main UI

[Pedro Alves <palves at sourceware dot org>]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=49940788ab38b9d58c663cf38855f29c0ebb1b55

commit 49940788ab38b9d58c663cf38855f29c0ebb1b55
Author: Pedro Alves <palves@redhat.com>
Date:   Tue Jun 21 01:11:57 2016 +0100

    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.

Diff:
---
 gdb/ChangeLog                        |   6 ++
 gdb/fork-child.c                     |  22 ++++-
 gdb/testsuite/ChangeLog              |   4 +
 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}
+	}
+    }
+}