diff mbox

[v4] Use cygwin spawn functions

Message ID 4CBA02C4.9050801@redhat.com
State New
Headers show

Commit Message

Richard Henderson Oct. 16, 2010, 7:53 p.m. UTC
Version 4.  The last attempt, while it fixed the hanging due
to pipe fds left open, closed some file descriptors too early.
This one has actually passed the entire testsuite.

Ok?


r~

Comments

DJ Delorie Oct. 19, 2010, 10:16 p.m. UTC | #1
I hope I'm reviewing the latest version... ;-)

-    calloc canonicalize_file_name clock \
+    calloc canonicalize_file_name clock dup3 \

dup3 would be on its own line.  They're grouped by starting letter.

-     strtoul strverscmp sysconf sysctl sysmp \
+     strtoul strverscmp sysconf sysctl sysmp spawnvpe \

Alphabetical please.

+  flags = fcntl (old_fd, F_GETFD);
+
+  /* If we could not retrieve the flags, then OLD_FD was not open.  */

Should we first check for old_fd < 0 ?

+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)

Note that DJGPP (pex-djgpp.c) also uses the spawn* functions, but
there are four - spawnv, spawnve, spawnvp, and spawnvpe.  Did you look
through pex-djgpp.c to see if there are other cases that it checks for
that this patch doesn't?

Look in setenv.c for how to deal with the "environ" variable.  You
can't just declare it.

Also, Microsoft deprecated spawn*() five years ago, replacing them
with _spawn*() instead.  Perhaps we should take the time to
future-proof this now, and prefer _spawn*() if present?

You call spawnve() but don't check for it in configure.  I've seen
implementations with some but not all exec*() calls, let's be paranoid
about spawn*() too.
Richard Henderson Oct. 19, 2010, 10:58 p.m. UTC | #2
On 10/19/2010 03:16 PM, DJ Delorie wrote:
> +  /* If we could not retrieve the flags, then OLD_FD was not open.  */
> 
> Should we first check for old_fd < 0 ?

It doesn't happen.  This function is called on the standard fds.

The "OLD_FD" was not open test is for when e.g. stderr is closed.
Which happened, somehow, in the testsuite.

> Note that DJGPP (pex-djgpp.c) also uses the spawn* functions, but
> there are four - spawnv, spawnve, spawnvp, and spawnvpe.  Did you look
> through pex-djgpp.c to see if there are other cases that it checks for
> that this patch doesn't?

No, they look fairly similar, except for the fact that DJGPP does
not have close-on-exec status to set or preserve around the spawn.

> Look in setenv.c for how to deal with the "environ" variable.  You
> can't just declare it.

I didn't add or change that.  It's pre-existing in that file.

> Also, Microsoft deprecated spawn*() five years ago, replacing them
> with _spawn*() instead.  Perhaps we should take the time to
> future-proof this now, and prefer _spawn*() if present?

*shrug* Today's Cygwin still does not have _spawn*.  Given that
this file is *not* used with mingw, and thus the Microsoft rtl,
it hardly seems worthwhile.

> You call spawnve() but don't check for it in configure.  I've seen
> implementations with some but not all exec*() calls, let's be paranoid
> about spawn*() too.

Ok.


r~
DJ Delorie Oct. 19, 2010, 11:04 p.m. UTC | #3
> Which happened, somehow, in the testsuite.

Ick?  ;-)

> > Look in setenv.c for how to deal with the "environ" variable.  You
> > can't just declare it.
> 
> I didn't add or change that.  It's pre-existing in that file.

Hmmm... so it was.

> *shrug* Today's Cygwin still does not have _spawn*.  Given that
> this file is *not* used with mingw, and thus the Microsoft rtl,
> it hardly seems worthwhile.

I suppose, until cygwin "catches up".  I suppose we can fix it then.
Joseph Myers Oct. 23, 2010, 5:04 p.m. UTC | #4
On Sat, 23 Oct 2010, Dave Korn wrote:

>   It might be suitable to address this in gcc by adding "-Wl,--stack,4194304"
> or simlar to the host build flags.  Or there could be some kind of bug or

Toplevel config/mh-{mingw,cygwin} already contain

LDFLAGS += -Wl,--stack,8388608

for this purpose.
Dave Korn Oct. 23, 2010, 5:21 p.m. UTC | #5
On 16/10/2010 20:53, Richard Henderson wrote:
> Version 4.  The last attempt, while it fixed the hanging due
> to pipe fds left open, closed some file descriptors too early.
> This one has actually passed the entire testsuite.
> 
> Ok?

  :-( This caused a number of regressions in a native testsuite run on
i686-pc-cygwin.

> FAIL: g++.dg/parse/stack1.C (test for excess errors)

and

> FAIL: gcc.c-torture/compile/limits-declparen.c (test for excess errors)
> FAIL: gcc.c-torture/compile/limits-declparen.c (test for excess errors)
> FAIL: gcc.c-torture/compile/limits-declparen.c (test for excess errors)
> FAIL: gcc.c-torture/compile/limits-exprparen.c (test for excess errors)
> FAIL: gcc.c-torture/compile/limits-exprparen.c (test for excess errors)
> FAIL: gcc.c-torture/compile/limits-exprparen.c (test for excess errors)
> FAIL: gcc.c-torture/compile/limits-exprparen.c (test for excess errors)
> FAIL: gcc.c-torture/compile/limits-exprparen.c (test for excess errors)
> FAIL: gcc.c-torture/compile/limits-exprparen.c (test for excess errors)
> FAIL: gcc.c-torture/compile/limits-exprparen.c (test for excess errors)
> FAIL: gcc.c-torture/compile/limits-exprparen.c (test for excess errors)
> FAIL: gcc.c-torture/compile/limits-pointer.c (test for excess errors)
> FAIL: gcc.c-torture/compile/limits-pointer.c (test for excess errors)
> FAIL: gcc.c-torture/compile/limits-pointer.c (test for excess errors)
> FAIL: gcc.c-torture/compile/limits-structnest.c (test for excess errors)
> FAIL: gcc.c-torture/compile/limits-structnest.c (test for excess errors)
> FAIL: gcc.c-torture/compile/limits-structnest.c (test for excess errors)
> FAIL: gcc.c-torture/compile/limits-structnest.c (test for excess errors)
> FAIL: gcc.c-torture/compile/limits-structnest.c (test for excess errors)
> FAIL: gcc.c-torture/compile/limits-structnest.c (test for excess errors)
> FAIL: gcc.c-torture/compile/limits-structnest.c (test for excess errors)
> FAIL: gcc.c-torture/compile/limits-structnest.c (test for excess errors)

are the most noticeable ones.  The common factor appears to be that these are
all tests that involve deeply recursive heavy stack usage.  The bug only
arises when the driver is invoked to launch cc1plus.exe, I couldn't reproduce
it by invoking the language compiler directly from the shell.

  Interestingly enough, even the clean cc1plus build failed the stack1.C test
when I ran it under GDB.  For some reason the stack ended up at a really low
address; the default reservation of stack space for w32 console apps is 2MB,
and although there was that much space, there wasn't any further room for it
to expand down into, which there would have been if the stack had been located
at a more normal address.

  It might be suitable to address this in gcc by adding "-Wl,--stack,4194304"
or simlar to the host build flags.  Or there could be some kind of bug or
anomaly in cygwin's spawn function.  I won't have time to investigate further
in the next few days, I'm rushing to get another stage 1 patch in.

    cheers,
      DaveK
Dave Korn Oct. 23, 2010, 5:45 p.m. UTC | #6
On 23/10/2010 18:04, Joseph S. Myers wrote:
> On Sat, 23 Oct 2010, Dave Korn wrote:
> 
>>   It might be suitable to address this in gcc by adding "-Wl,--stack,4194304"
>> or simlar to the host build flags.  Or there could be some kind of bug or
> 
> Toplevel config/mh-{mingw,cygwin} already contain
> 
> LDFLAGS += -Wl,--stack,8388608
> 
> for this purpose.
> 

  It's not working:

> admin@ubik /gnu/gcc/obj-lto
> $ objdump -p gcc/cc1.exe | grep Stack
> SizeOfStackReserve      00200000
> SizeOfStackCommit       00001000
> 
> admin@ubik /gnu/gcc/obj-lto
> $

  (I'm sure we've had this discussion before in a java-related context.)

    cheers,
      DaveK
Dave Korn Oct. 23, 2010, 5:54 p.m. UTC | #7
On 23/10/2010 18:45, Dave Korn wrote:
> On 23/10/2010 18:04, Joseph S. Myers wrote:
>> On Sat, 23 Oct 2010, Dave Korn wrote:
>>
>>>   It might be suitable to address this in gcc by adding "-Wl,--stack,4194304"
>>> or simlar to the host build flags.  Or there could be some kind of bug or
>> Toplevel config/mh-{mingw,cygwin} already contain
>>
>> LDFLAGS += -Wl,--stack,8388608
>>
>> for this purpose.
>>
> 
>   It's not working:
> 
>> admin@ubik /gnu/gcc/obj-lto
>> $ objdump -p gcc/cc1.exe | grep Stack
>> SizeOfStackReserve      00200000
>> SizeOfStackCommit       00001000
>>
>> admin@ubik /gnu/gcc/obj-lto
>> $
> 
>   (I'm sure we've had this discussion before in a java-related context.)

  Yep, found it(*).  The top-level config/mh-* flags are only used for stage1,
that's why.

    cheers,
      DaveK
Paolo Bonzini Oct. 23, 2010, 7:24 p.m. UTC | #8
On 10/23/2010 07:54 PM, Dave Korn wrote:
>>> LDFLAGS += -Wl,--stack,8388608
>
> Yep, found it(*).  The top-level config/mh-* flags are only used for
> stage1, that's why.

Just add BOOT_LDFLAGS too.

Paolo
diff mbox

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..ea828d3 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,191 @@  pex_child_error (struct pex_obj *obj, const char *executable,
 
 extern char **environ;
 
+#ifdef 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
+    {
+      new_fd = fcntl (old_fd, F_DUPFD_CLOEXEC, 3);
+      if (new_fd < 0)
+	return -1;
+      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)
+{
+  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 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)
+    {
+      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);
+    }
+
+  /* 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 +708,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.  */