Patchwork Use cygwin spawn functions

login
register
mail settings
Submitter Richard Henderson
Date Oct. 14, 2010, 11:26 p.m.
Message ID <4CB791AE.9080303@redhat.com>
Download mbox | patch
Permalink /patch/67872/
State New
Headers show

Comments

Richard Henderson - Oct. 14, 2010, 11:26 p.m.
On 10/11/2010 05:41 PM, Richard Henderson wrote:
> Emulating fork(2) on cygwin is a bit of a challenge; the hoop jumping is
> a regular circus act.  Worse, fork has a habit of failing on Windows 7 64-bit,
> as seen in the many "Bad address" and "Resource temporarily unavailable"
> errors during testing
> (e.g. http://gcc.gnu.org/ml/gcc-testresults/2010-10/msg00624.html).
> 
> Cygwin's own faq (sec 5.5) says that it's better to avoid using fork+exec
> when possible and use the spawn family of calls instead.

The previous attempt failed to close all of the descriptors within
the parent, which lead to collect2 holding an open input end of a
pipe, which of course meant that it would wait forever for end-of-data.

I'm currently running a full test cycle on i686-cygwin, but this has
passed a few LTO tests run by hand, which had been the path that lead
to the failure case described above.

Ok?


r~

Patch

diff --git a/libiberty/config.in b/libiberty/config.in
index 02d93da..8500860 100644
--- a/libiberty/config.in
+++ b/libiberty/config.in
@@ -91,6 +91,9 @@ 
    don't. */
 #undef HAVE_DECL_VSNPRINTF
 
+/* Define to 1 if you have the `dup3' function. */
+#undef HAVE_DUP3
+
 /* Define to 1 if you have the <fcntl.h> header file. */
 #undef HAVE_FCNTL_H
 
@@ -169,6 +172,9 @@ 
 /* Define if you have prctl PR_SET_NAME */
 #undef HAVE_PRCTL_SET_NAME
 
+/* Define to 1 if you have the <process.h> header file. */
+#undef HAVE_PROCESS_H
+
 /* Define to 1 if you have the `psignal' function. */
 #undef HAVE_PSIGNAL
 
@@ -208,6 +214,9 @@ 
 /* Define to 1 if you have the `snprintf' function. */
 #undef HAVE_SNPRINTF
 
+/* Define to 1 if you have the `spawnvpe' function. */
+#undef HAVE_SPAWNVPE
+
 /* Define to 1 if you have the <stdint.h> header file. */
 #undef HAVE_STDINT_H
 
diff --git a/libiberty/configure b/libiberty/configure
index 7579000..1aad26f 100755
--- a/libiberty/configure
+++ b/libiberty/configure
@@ -4895,7 +4895,7 @@  host_makefile_frag=${frag}
 # It's OK to check for header files.  Although the compiler may not be
 # able to link anything, it had better be able to at least compile
 # something.
-for ac_header in sys/file.h sys/param.h limits.h stdlib.h malloc.h string.h unistd.h strings.h sys/time.h time.h sys/resource.h sys/stat.h sys/mman.h fcntl.h alloca.h sys/pstat.h sys/sysmp.h sys/sysinfo.h machine/hal_sysinfo.h sys/table.h sys/sysctl.h sys/systemcfg.h stdint.h stdio_ext.h
+for ac_header in sys/file.h sys/param.h limits.h stdlib.h malloc.h string.h unistd.h strings.h sys/time.h time.h sys/resource.h sys/stat.h sys/mman.h fcntl.h alloca.h sys/pstat.h sys/sysmp.h sys/sysinfo.h machine/hal_sysinfo.h sys/table.h sys/sysctl.h sys/systemcfg.h stdint.h stdio_ext.h process.h
 do :
   as_ac_Header=`$as_echo "ac_cv_header_$ac_header" | $as_tr_sh`
 ac_fn_c_check_header_preproc "$LINENO" "$ac_header" "$as_ac_Header"
@@ -5284,14 +5284,14 @@  vars="sys_errlist sys_nerr sys_siglist"
 
 checkfuncs="getrusage on_exit psignal strerror strsignal sysconf times sbrk gettimeofday"
 checkfuncs="$checkfuncs realpath canonicalize_file_name pstat_getstatic pstat_getdynamic sysmp"
-checkfuncs="$checkfuncs getsysinfo table sysctl wait3 wait4 __fsetlocking"
+checkfuncs="$checkfuncs getsysinfo table sysctl wait3 wait4 __fsetlocking dup3 spawnvpe"
 
 # These are neither executed nor required, but they help keep
 # autoheader happy without adding a bunch of text to acconfig.h.
 if test "x" = "y"; then
   for ac_func in asprintf atexit \
     basename bcmp bcopy bsearch bzero \
-    calloc canonicalize_file_name clock \
+    calloc canonicalize_file_name clock dup3 \
     ffs __fsetlocking \
     getcwd getpagesize getrusage getsysinfo gettimeofday \
     index insque \
@@ -5299,10 +5299,10 @@  if test "x" = "y"; then
     on_exit \
     psignal pstat_getdynamic pstat_getstatic putenv \
     random realpath rename rindex \
-    sbrk setenv setproctitle sigsetmask snprintf stpcpy stpncpy strcasecmp strchr \
-    strdup \
+    sbrk setenv setproctitle sigsetmask snprintf stpcpy stpncpy \
+     strcasecmp strchr strdup \
      strerror strncasecmp strndup strrchr strsignal strstr strtod strtol \
-     strtoul strverscmp sysconf sysctl sysmp \
+     strtoul strverscmp sysconf sysctl sysmp spawnvpe \
     table times tmpnam \
     vasprintf vfprintf vprintf vsprintf \
     wait3 wait4 waitpid
diff --git a/libiberty/configure.ac b/libiberty/configure.ac
index 73ea6c9..34a1a7e 100644
--- a/libiberty/configure.ac
+++ b/libiberty/configure.ac
@@ -246,7 +246,7 @@  AC_SUBST_FILE(host_makefile_frag)
 # It's OK to check for header files.  Although the compiler may not be
 # able to link anything, it had better be able to at least compile
 # something.
-AC_CHECK_HEADERS(sys/file.h sys/param.h limits.h stdlib.h malloc.h string.h unistd.h strings.h sys/time.h time.h sys/resource.h sys/stat.h sys/mman.h fcntl.h alloca.h sys/pstat.h sys/sysmp.h sys/sysinfo.h machine/hal_sysinfo.h sys/table.h sys/sysctl.h sys/systemcfg.h stdint.h stdio_ext.h)
+AC_CHECK_HEADERS(sys/file.h sys/param.h limits.h stdlib.h malloc.h string.h unistd.h strings.h sys/time.h time.h sys/resource.h sys/stat.h sys/mman.h fcntl.h alloca.h sys/pstat.h sys/sysmp.h sys/sysinfo.h machine/hal_sysinfo.h sys/table.h sys/sysctl.h sys/systemcfg.h stdint.h stdio_ext.h process.h)
 AC_HEADER_SYS_WAIT
 AC_HEADER_TIME
 
@@ -359,14 +359,14 @@  vars="sys_errlist sys_nerr sys_siglist"
 
 checkfuncs="getrusage on_exit psignal strerror strsignal sysconf times sbrk gettimeofday"
 checkfuncs="$checkfuncs realpath canonicalize_file_name pstat_getstatic pstat_getdynamic sysmp"
-checkfuncs="$checkfuncs getsysinfo table sysctl wait3 wait4 __fsetlocking"
+checkfuncs="$checkfuncs getsysinfo table sysctl wait3 wait4 __fsetlocking dup3 spawnvpe"
 
 # These are neither executed nor required, but they help keep
 # autoheader happy without adding a bunch of text to acconfig.h.
 if test "x" = "y"; then
   AC_CHECK_FUNCS(asprintf atexit \
     basename bcmp bcopy bsearch bzero \
-    calloc canonicalize_file_name clock \
+    calloc canonicalize_file_name clock dup3 \
     ffs __fsetlocking \
     getcwd getpagesize getrusage getsysinfo gettimeofday \
     index insque \
@@ -374,10 +374,10 @@  if test "x" = "y"; then
     on_exit \
     psignal pstat_getdynamic pstat_getstatic putenv \
     random realpath rename rindex \
-    sbrk setenv setproctitle sigsetmask snprintf stpcpy stpncpy strcasecmp strchr \
-    strdup \
+    sbrk setenv setproctitle sigsetmask snprintf stpcpy stpncpy \
+     strcasecmp strchr strdup \
      strerror strncasecmp strndup strrchr strsignal strstr strtod strtol \
-     strtoul strverscmp sysconf sysctl sysmp \
+     strtoul strverscmp sysconf sysctl sysmp spawnvpe \
     table times tmpnam \
     vasprintf vfprintf vprintf vsprintf \
     wait3 wait4 waitpid)
diff --git a/libiberty/pex-unix.c b/libiberty/pex-unix.c
index 85733a6..0727051 100644
--- a/libiberty/pex-unix.c
+++ b/libiberty/pex-unix.c
@@ -55,7 +55,9 @@  extern int errno;
 #ifdef HAVE_SYS_STAT_H
 #include <sys/stat.h>
 #endif
-
+#ifdef HAVE_PROCESS_H
+#include <process.h>
+#endif
 
 #ifdef vfork /* Autoconf may define this to fork for us. */
 # define VFORK_STRING "fork"
@@ -387,6 +389,149 @@  pex_child_error (struct pex_obj *obj, const char *executable,
 
 extern char **environ;
 
+#ifdef HAVE_SPAWNVPE
+/* Helpers for saving file descriptors around spawning a child.  */
+
+static int
+save_and_install_fd(int *pflags, int old_fd, int child_fd)
+{
+  int new_fd, flags;
+
+  flags = fcntl (old_fd, F_GETFD);
+  if (child_fd == -1)
+    {
+      new_fd = old_fd;
+      if ((flags & FD_CLOEXEC) == 0
+	  && fcntl (old_fd, F_SETFD, FD_CLOEXEC) < 0)
+	return -1;
+    }
+  else
+    {
+      new_fd = fcntl (old_fd, F_DUPFD_CLOEXEC, 3);
+      if (new_fd < 0)
+	return -1;
+    }
+
+  *pflags = flags;
+  return new_fd;
+}
+
+static void
+restore_fd(int old_fd, int new_fd, int flags)
+{
+  if (new_fd == old_fd)
+    {
+      if ((flags & FD_CLOEXEC) == 0)
+	fcntl (old_fd, F_SETFD, flags);
+    }
+  else
+    {
+#ifdef HAVE_DUP3
+      if (flags == FD_CLOEXEC)
+	dup3 (new_fd, old_fd, O_CLOEXEC);
+      else
+#endif
+	{
+	  dup2 (new_fd, old_fd);
+	  if (flags != 0)
+	    fcntl (old_fd, F_SETFD, flags);
+	}
+      close (new_fd);
+    }
+}
+
+static pid_t
+pex_unix_exec_child (struct pex_obj *obj ATTRIBUTE_UNUSED,
+		     int flags, const char *executable,
+		     char * const * argv, char * const * env,
+                     int in, int out, int errdes, int toclose,
+		     const char **errmsg, int *err)
+{
+  typedef const char * const *cc_cp;
+  int fl_in = 0, fl_out = 0, fl_err = 0, fl_tc = 0;
+  int save_in = -1, save_out = -1, save_err = -1;
+  int retries;
+  pid_t pid;
+
+  if (flags & PEX_STDERR_TO_STDOUT)
+    errdes = out;
+
+  if (in != STDIN_FILE_NO)
+    {
+      save_in = save_and_install_fd(&fl_in, STDIN_FILE_NO, in);
+      if (save_in < 0)
+	goto error_dup2;
+    }
+  if (out != STDOUT_FILE_NO)
+    {
+      save_out = save_and_install_fd(&fl_out, STDOUT_FILE_NO, out);
+      if (save_out < 0)
+	goto error_dup2;
+    }
+  if (errdes != STDERR_FILE_NO)
+    {
+      save_err = save_and_install_fd(&fl_err, STDIN_FILE_NO, errdes);
+      if (save_err < 0)
+	goto error_dup2;
+    }
+  if (toclose >= 0)
+    save_and_install_fd(&fl_tc, toclose, -1);
+
+  if (env == NULL)
+    env = environ;
+
+  retries = 0;
+  while (1)
+    {
+      if (flags & PEX_SEARCH)
+	pid = spawnvpe (_P_NOWAITO, executable, (cc_cp)argv, (cc_cp)env);
+      else
+	pid = spawnve (_P_NOWAITO, executable, (cc_cp)argv, (cc_cp)env);
+
+      if (pid > 0)
+	break;
+
+      *err = errno;
+      *errmsg = "spawn";
+      if (errno != EAGAIN || ++retries == 4)
+	return -1;
+      sleep (1 << retries);
+    }
+
+  if (toclose >= 0)
+    restore_fd (toclose, toclose, fl_tc);
+  if (in != STDIN_FILE_NO)
+    {
+      restore_fd (STDIN_FILE_NO, save_in, fl_in);
+      if (close (in) < 0)
+	goto error_close;
+    }
+  if (out != STDOUT_FILE_NO)
+    {
+      restore_fd (STDOUT_FILE_NO, save_out, fl_out);
+      if (close (out) < 0)
+	goto error_close;
+    }
+  if (errdes != STDERR_FILE_NO)
+    {
+      restore_fd (STDERR_FILE_NO, save_err, fl_err);
+      if (errdes != out && close (errdes) < 0)
+	goto error_close;
+    }
+
+  return pid;
+
+ error_dup2:
+  *err = errno;
+  *errmsg = "dup2";
+  return (pid_t) -1;
+
+ error_close:
+  *err = errno;
+  *errmsg = "close";
+  return (pid_t) -1;
+}
+#else
 static pid_t
 pex_unix_exec_child (struct pex_obj *obj, int flags, const char *executable,
 		     char * const * argv, char * const * env,
@@ -521,6 +666,7 @@  pex_unix_exec_child (struct pex_obj *obj, int flags, const char *executable,
       return pid;
     }
 }
+#endif /* SPAWN */
 
 /* Wait for a child process to complete.  */