diff mbox

[v4] Use cygwin spawn functions

Message ID 4CCB871A.5060801@redhat.com
State New
Headers show

Commit Message

Richard Henderson Oct. 30, 2010, 2:46 a.m. UTC
v4 -> v5
  * I believe I've addressed all of DJ's comments

  * Test for F_DUPFD_CLOEXEC, not present in cygwin 1.5, which the last version that
    works on Win98 and related systems.


Ok?


r~

Comments

Dave Korn Oct. 30, 2010, 11:04 a.m. UTC | #1
On 30/10/2010 03:46, Richard Henderson wrote:
> v4 -> v5
>   * I believe I've addressed all of DJ's comments
> 
>   * Test for F_DUPFD_CLOEXEC, not present in cygwin 1.5, which the last version that
>     works on Win98 and related systems.
> 
> 
> Ok?

  I still had a bunch of regressions in the limits-exprparen.c compile
tests(*), even after turning up the default stack size to 8MB, sigh.  For some
reason using spawn rather than fork appears to result in the stack getting
allocated down at the lowest end of memory where it doesn't have as much room
to grow beyond the default allocation as it otherwise might if it were located
higher up.

  Maybe, given that this is potentially troublesome, we should have a
configure option to en/disable it?  DJ, would you in principle accept a
follow-on patch making the use of spawn configurable by a --enable option?
(And if so, do you have a preferred name you'd like used for it?)

    cheers,
      DaveK
Richard Henderson Oct. 30, 2010, 6:08 p.m. UTC | #2
On 10/30/2010 04:04 AM, Dave Korn wrote:
> On 30/10/2010 03:46, Richard Henderson wrote:
>> v4 -> v5
>>   * I believe I've addressed all of DJ's comments
>>
>>   * Test for F_DUPFD_CLOEXEC, not present in cygwin 1.5, which the last version that
>>     works on Win98 and related systems.
>>
>>
>> Ok?
> 
>   I still had a bunch of regressions in the limits-exprparen.c compile
> tests(*), even after turning up the default stack size to 8MB, sigh. 

That test fails on linux as well:

  http://gcc.gnu.org/ml/gcc-testresults/2010-10/msg02408.html


r~
Dave Korn Oct. 30, 2010, 6:42 p.m. UTC | #3
On 30/10/2010 19:08, Richard Henderson wrote:
> On 10/30/2010 04:04 AM, Dave Korn wrote:
>> On 30/10/2010 03:46, Richard Henderson wrote:
>>> v4 -> v5
>>>   * I believe I've addressed all of DJ's comments
>>>
>>>   * Test for F_DUPFD_CLOEXEC, not present in cygwin 1.5, which the last version that
>>>     works on Win98 and related systems.
>>>
>>>
>>> Ok?
>>   I still had a bunch of regressions in the limits-exprparen.c compile
>> tests(*), even after turning up the default stack size to 8MB, sigh. 
> 
> That test fails on linux as well:
> 
>   http://gcc.gnu.org/ml/gcc-testresults/2010-10/msg02408.html
> 
> 
> r~

  Oh, I didn't think to check that!  It started failing for me at just the
same moment as I applied the patch, along with a bunch of the other limits-*
tests; I assumed they were all caused by the same change.

    cheers,
      DaveK
DJ Delorie Nov. 4, 2010, 8:43 p.m. UTC | #4
I'm OK with this one if you've appeased Dave's concerns about the
might-not-be-regressions.
Dave Korn Nov. 4, 2010, 11:45 p.m. UTC | #5
On 04/11/2010 20:43, DJ Delorie wrote:
> I'm OK with this one if you've appeased Dave's concerns about the
> might-not-be-regressions.

  Yeh, let it go in.  If I think there's a problem, I'll send a patch to make
it configure-time selectable.

    cheers,
      DaveK
diff mbox

Patch

diff --git a/libiberty/config.in b/libiberty/config.in
index 02d93da..9b6dbd2 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,12 @@ 
 /* Define to 1 if you have the `snprintf' function. */
 #undef HAVE_SNPRINTF
 
+/* Define to 1 if you have the `spawnve' function. */
+#undef HAVE_SPAWNVE
+
+/* 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 142c81c..4b3426c 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"
@@ -5282,9 +5282,10 @@  funcs="$funcs setproctitle"
 
 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="__fsetlocking canonicalize_file_name dup3 getrusage getsysinfo \
+ gettimeofday on_exit psignal pstat_getdynamic pstat_getstatic realpath \
+ sbrk spawnve spawnvpe strerror strsignal sysconf sysctl sysmp table \
+ times wait3 wait4"
 
 # These are neither executed nor required, but they help keep
 # autoheader happy without adding a bunch of text to acconfig.h.
@@ -5292,6 +5293,7 @@  if test "x" = "y"; then
   for ac_func in asprintf atexit \
     basename bcmp bcopy bsearch bzero \
     calloc canonicalize_file_name clock \
+    dup3 \
     ffs __fsetlocking \
     getcwd getpagesize getrusage getsysinfo gettimeofday \
     index insque \
@@ -5299,8 +5301,8 @@  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 spawnve spawnvpe \
+     stpcpy stpncpy strcasecmp strchr strdup \
      strerror strncasecmp strndup strrchr strsignal strstr strtod strtol \
      strtoul strverscmp sysconf sysctl sysmp \
     table times tmpnam \
diff --git a/libiberty/configure.ac b/libiberty/configure.ac
index 73ea6c9..df19b3c 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
 
@@ -357,9 +357,10 @@  funcs="$funcs setproctitle"
 
 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="__fsetlocking canonicalize_file_name dup3 getrusage getsysinfo \
+ gettimeofday on_exit psignal pstat_getdynamic pstat_getstatic realpath \
+ sbrk spawnve spawnvpe strerror strsignal sysconf sysctl sysmp table \
+ times wait3 wait4"
 
 # These are neither executed nor required, but they help keep
 # autoheader happy without adding a bunch of text to acconfig.h.
@@ -367,6 +368,7 @@  if test "x" = "y"; then
   AC_CHECK_FUNCS(asprintf atexit \
     basename bcmp bcopy bsearch bzero \
     calloc canonicalize_file_name clock \
+    dup3 \
     ffs __fsetlocking \
     getcwd getpagesize getrusage getsysinfo gettimeofday \
     index insque \
@@ -374,8 +376,8 @@  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 spawnve spawnvpe \
+     stpcpy stpncpy strcasecmp strchr strdup \
      strerror strncasecmp strndup strrchr strsignal strstr strtod strtol \
      strtoul strverscmp sysconf sysctl sysmp \
     table times tmpnam \
diff --git a/libiberty/pex-unix.c b/libiberty/pex-unix.c
index 85733a6..c4a69ad 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,202 @@  pex_child_error (struct pex_obj *obj, const char *executable,
 
 extern char **environ;
 
+#if defined(HAVE_SPAWNVE) && defined(HAVE_SPAWNVPE)
+/* Implementation of pex->exec_child using the Cygwin spawn operation.  */
+
+/* Subroutine of pex_unix_exec_child.  Move OLD_FD to a new file descriptor
+   to be stored in *PNEW_FD, save the flags in *PFLAGS, and arrange for the
+   saved copy to be close-on-exec.  Move CHILD_FD into OLD_FD.  If CHILD_FD
+   is -1, OLD_FD is to be closed.  Return -1 on error.  */
+
+static int
+save_and_install_fd(int *pnew_fd, int *pflags, int old_fd, int child_fd)
+{
+  int new_fd, flags;
+
+  flags = fcntl (old_fd, F_GETFD);
+
+  /* If we could not retrieve the flags, then OLD_FD was not open.  */
+  if (flags < 0)
+    {
+      new_fd = -1, flags = 0;
+      if (child_fd >= 0 && dup2 (child_fd, old_fd) < 0)
+	return -1;
+    }
+  /* If we wish to close OLD_FD, just mark it CLOEXEC.  */
+  else if (child_fd == -1)
+    {
+      new_fd = old_fd;
+      if ((flags & FD_CLOEXEC) == 0 && fcntl (old_fd, F_SETFD, FD_CLOEXEC) < 0)
+	return -1;
+    }
+  /* Otherwise we need to save a copy of OLD_FD before installing CHILD_FD.  */
+  else
+    {
+#ifdef F_DUPFD_CLOEXEC
+      new_fd = fcntl (old_fd, F_DUPFD_CLOEXEC, 3);
+      if (new_fd < 0)
+	return -1;
+#else
+      /* Prefer F_DUPFD over dup in order to avoid getting a new fd
+	 in the range 0-2, right where a new stderr fd might get put.  */
+      new_fd = fcntl (old_fd, F_DUPFD, 3);
+      if (new_fd < 0)
+	return -1;
+      if (fcntl (new_fd, F_SETFD, FD_CLOEXEC) < 0)
+	return -1;
+#endif
+      if (dup2 (child_fd, old_fd) < 0)
+	return -1;
+    }
+
+  *pflags = flags;
+  if (pnew_fd)
+    *pnew_fd = new_fd;
+  else if (new_fd != old_fd)
+    abort ();
+
+  return 0;
+}
+
+/* Subroutine of pex_unix_exec_child.  Move SAVE_FD back to OLD_FD
+   restoring FLAGS.  If SAVE_FD < 0, OLD_FD is to be closed.  */
+
+static int
+restore_fd(int old_fd, int save_fd, int flags)
+{
+  /* For SAVE_FD < 0, all we have to do is restore the
+     "closed-ness" of the original.  */
+  if (save_fd < 0)
+    return close (old_fd);
+
+  /* For SAVE_FD == OLD_FD, all we have to do is restore the
+     original setting of the CLOEXEC flag.  */
+  if (save_fd == old_fd)
+    {
+      if (flags & FD_CLOEXEC)
+	return 0;
+      return fcntl (old_fd, F_SETFD, flags);
+    }
+
+  /* Otherwise we have to move the descriptor back, restore the flags,
+     and close the saved copy.  */
+#ifdef HAVE_DUP3
+  if (flags == FD_CLOEXEC)
+    {
+      if (dup3 (save_fd, old_fd, O_CLOEXEC) < 0)
+	return -1;
+    }
+  else
+#endif
+    {
+      if (dup2 (save_fd, old_fd) < 0)
+	return -1;
+      if (flags != 0 && fcntl (old_fd, F_SETFD, flags) < 0)
+	return -1;
+    }
+  return close (save_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)
+{
+  int fl_in = 0, fl_out = 0, fl_err = 0, fl_tc = 0;
+  int save_in = -1, save_out = -1, save_err = -1;
+  int max, retries;
+  pid_t pid;
+
+  if (flags & PEX_STDERR_TO_STDOUT)
+    errdes = out;
+
+  /* We need the three standard file descriptors to be set up as for
+     the child before we perform the spawn.  The file descriptors for
+     the parent need to be moved and marked for close-on-exec.  */
+  if (in != STDIN_FILE_NO
+      && save_and_install_fd (&save_in, &fl_in, STDIN_FILE_NO, in) < 0)
+    goto error_dup2;
+  if (out != STDOUT_FILE_NO
+      && save_and_install_fd (&save_out, &fl_out, STDOUT_FILE_NO, out) < 0)
+    goto error_dup2;
+  if (errdes != STDERR_FILE_NO
+      && save_and_install_fd (&save_err, &fl_err, STDERR_FILE_NO, errdes) < 0)
+    goto error_dup2;
+  if (toclose >= 0
+      && save_and_install_fd (NULL, &fl_tc, toclose, -1) < 0)
+    goto error_dup2;
+
+  /* Now that we've moved the file descriptors for the child into place,
+     close the originals.  Be careful not to close any of the standard
+     file descriptors that we just set up.  */
+  max = -1;
+  if (errdes >= 0)
+    max = STDERR_FILE_NO;
+  else if (out >= 0)
+    max = STDOUT_FILE_NO;
+  else if (in >= 0)
+    max = STDIN_FILE_NO;
+  if (in > max)
+    close (in);
+  if (out > max)
+    close (out);
+  if (errdes > max && errdes != out)
+    close (errdes);
+
+  /* If we were not given an environment, use the global environment.  */
+  if (env == NULL)
+    env = environ;
+
+  /* Launch the program.  If we get EAGAIN (normally out of pid's), try
+     again a few times with increasing backoff times.  */
+  retries = 0;
+  while (1)
+    {
+      typedef const char * const *cc_cp;
+
+      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 (pid_t) -1;
+      sleep (1 << retries);
+    }
+
+  /* Success.  Restore the parent's file descriptors that we saved above.  */
+  if (toclose >= 0
+      && restore_fd (toclose, toclose, fl_tc) < 0)
+    goto error_dup2;
+  if (in != STDIN_FILE_NO
+      && restore_fd (STDIN_FILE_NO, save_in, fl_in) < 0)
+    goto error_dup2;
+  if (out != STDOUT_FILE_NO
+      && restore_fd (STDOUT_FILE_NO, save_out, fl_out) < 0)
+    goto error_dup2;
+  if (errdes != STDERR_FILE_NO
+      && restore_fd (STDERR_FILE_NO, save_err, fl_err) < 0)
+    goto error_dup2;
+
+  return pid;
+
+ error_dup2:
+  *err = errno;
+  *errmsg = "dup2";
+  return (pid_t) -1;
+}
+
+#else
+/* Implementation of pex->exec_child using standard vfork + exec.  */
+
 static pid_t
 pex_unix_exec_child (struct pex_obj *obj, int flags, const char *executable,
 		     char * const * argv, char * const * env,
@@ -521,6 +719,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.  */