From df1346b423f1c8849b6090b47023ee29f6dddf7a Mon Sep 17 00:00:00 2001 From: Nathan Sidwell Date: Mon, 1 Oct 2018 18:46:51 +0000 Subject: [PATCH] [libiberty] Use pipe inside pex_run https://gcc.gnu.org/ml/gcc-patches/2018-10/msg00039.html * configure.ac (checkfuncs): Add pipe2. * config.in, configure: Rebuilt. * pex-unix.c (pex_unix_exec_child): Comminicate errors from child to parent with a pipe, when possible. From-SVN: r264769 --- libiberty/ChangeLog | 7 +++ libiberty/config.in | 3 + libiberty/configure | 4 +- libiberty/configure.ac | 4 +- libiberty/pex-unix.c | 136 ++++++++++++++++++++++++++++------------- 5 files changed, 109 insertions(+), 45 deletions(-) diff --git a/libiberty/ChangeLog b/libiberty/ChangeLog index 294bcb6b261..b2e833cf7ea 100644 --- a/libiberty/ChangeLog +++ b/libiberty/ChangeLog @@ -1,3 +1,10 @@ +2018-10-01 Nathan Sidwell + + * configure.ac (checkfuncs): Add pipe2. + * config.in, configure: Rebuilt. + * pex-unix.c (pex_unix_exec_child): Comminicate errors from child + to parent with a pipe, when possible. + 2018-08-23 Nathan Sidwell Martin Liska diff --git a/libiberty/config.in b/libiberty/config.in index 66c78e81684..c7b472543cf 100644 --- a/libiberty/config.in +++ b/libiberty/config.in @@ -195,6 +195,9 @@ /* Define to 1 if you have the `on_exit' function. */ #undef HAVE_ON_EXIT +/* Define to 1 if you have the `pipe2' function. */ +#undef HAVE_PIPE2 + /* Define to 1 if you have the header file. */ #undef HAVE_PROCESS_H diff --git a/libiberty/configure b/libiberty/configure index ccadfaa8224..03ff3949115 100755 --- a/libiberty/configure +++ b/libiberty/configure @@ -5727,7 +5727,7 @@ funcs="$funcs setproctitle" vars="sys_errlist sys_nerr sys_siglist" checkfuncs="__fsetlocking canonicalize_file_name dup3 getrlimit getrusage \ - getsysinfo gettimeofday on_exit psignal pstat_getdynamic pstat_getstatic \ + getsysinfo gettimeofday on_exit pipe2 psignal pstat_getdynamic pstat_getstatic \ realpath setrlimit sbrk spawnve spawnvpe strerror strsignal sysconf sysctl \ sysmp table times wait3 wait4" @@ -5743,7 +5743,7 @@ if test "x" = "y"; then index insque \ memchr memcmp memcpy memmem memmove memset mkstemps \ on_exit \ - psignal pstat_getdynamic pstat_getstatic putenv \ + pipe2 psignal pstat_getdynamic pstat_getstatic putenv \ random realpath rename rindex \ sbrk setenv setproctitle setrlimit sigsetmask snprintf spawnve spawnvpe \ stpcpy stpncpy strcasecmp strchr strdup \ diff --git a/libiberty/configure.ac b/libiberty/configure.ac index 6917cfaf4fb..59c45c973ab 100644 --- a/libiberty/configure.ac +++ b/libiberty/configure.ac @@ -391,7 +391,7 @@ funcs="$funcs setproctitle" vars="sys_errlist sys_nerr sys_siglist" checkfuncs="__fsetlocking canonicalize_file_name dup3 getrlimit getrusage \ - getsysinfo gettimeofday on_exit psignal pstat_getdynamic pstat_getstatic \ + getsysinfo gettimeofday on_exit pipe2 psignal pstat_getdynamic pstat_getstatic \ realpath setrlimit sbrk spawnve spawnvpe strerror strsignal sysconf sysctl \ sysmp table times wait3 wait4" @@ -407,7 +407,7 @@ if test "x" = "y"; then index insque \ memchr memcmp memcpy memmem memmove memset mkstemps \ on_exit \ - psignal pstat_getdynamic pstat_getstatic putenv \ + pipe2 psignal pstat_getdynamic pstat_getstatic putenv \ random realpath rename rindex \ sbrk setenv setproctitle setrlimit sigsetmask snprintf spawnve spawnvpe \ stpcpy stpncpy strcasecmp strchr strdup \ diff --git a/libiberty/pex-unix.c b/libiberty/pex-unix.c index 703010bdeae..0f283e6b037 100644 --- a/libiberty/pex-unix.c +++ b/libiberty/pex-unix.c @@ -569,6 +569,38 @@ pex_unix_exec_child (struct pex_obj *obj, int flags, const char *executable, int toclose, const char **errmsg, int *err) { pid_t pid = -1; + /* Tuple to communicate error from child to parent. We can safely + transfer string literal pointers as both run with identical + address mappings. */ + struct fn_err + { + const char *fn; + int err; + }; + volatile int do_pipe = 0; + volatile int pipes[2]; /* [0]:reader,[1]:writer. */ +#ifdef O_CLOEXEC + do_pipe = 1; +#endif + if (do_pipe) + { +#ifdef HAVE_PIPE2 + if (pipe2 ((int *)pipes, O_CLOEXEC)) + do_pipe = 0; +#else + if (pipe ((int *)pipes)) + do_pipe = 0; + else + { + if (fcntl (pipes[1], F_SETFD, FD_CLOEXEC) == -1) + { + close (pipes[0]); + close (pipes[1]); + do_pipe = 0; + } + } +#endif + } /* We declare these to be volatile to avoid warnings from gcc about them being clobbered by vfork. */ @@ -579,8 +611,9 @@ pex_unix_exec_child (struct pex_obj *obj, int flags, const char *executable, This clobbers the parent's environ so we need to restore it. It would be nice to use one of the exec* functions that takes an environment as a parameter, but that may have portability - issues. */ - char **save_environ = environ; + issues. It is marked volatile so the child doesn't consider it a + dead variable and therefore clobber where ever it is stored. */ + char **volatile save_environ = environ; for (retries = 0; retries < 4; ++retries) { @@ -594,6 +627,11 @@ pex_unix_exec_child (struct pex_obj *obj, int flags, const char *executable, switch (pid) { case -1: + if (do_pipe) + { + close (pipes[0]); + close (pipes[1]); + } *err = errno; *errmsg = VFORK_STRING; return (pid_t) -1; @@ -601,40 +639,43 @@ pex_unix_exec_child (struct pex_obj *obj, int flags, const char *executable, case 0: /* Child process. */ { - const char *bad_fn = NULL; + struct fn_err failed; + failed.fn = NULL; - if (!bad_fn && in != STDIN_FILE_NO) + if (do_pipe) + close (pipes[0]); + if (!failed.fn && in != STDIN_FILE_NO) { if (dup2 (in, STDIN_FILE_NO) < 0) - bad_fn = "dup2"; + failed.fn = "dup2", failed.err = errno; else if (close (in) < 0) - bad_fn = "close"; + failed.fn = "close", failed.err = errno; } - if (!bad_fn && out != STDOUT_FILE_NO) + if (!failed.fn && out != STDOUT_FILE_NO) { if (dup2 (out, STDOUT_FILE_NO) < 0) - bad_fn = "dup2"; + failed.fn = "dup2", failed.err = errno; else if (close (out) < 0) - bad_fn = "close"; + failed.fn = "close", failed.err = errno; } - if (!bad_fn && errdes != STDERR_FILE_NO) + if (!failed.fn && errdes != STDERR_FILE_NO) { if (dup2 (errdes, STDERR_FILE_NO) < 0) - bad_fn = "dup2"; + failed.fn = "dup2", failed.err = errno; else if (close (errdes) < 0) - bad_fn = "close"; + failed.fn = "close", failed.err = errno; } - if (!bad_fn && toclose >= 0) + if (!failed.fn && toclose >= 0) { if (close (toclose) < 0) - bad_fn = "close"; + failed.fn = "close", failed.err = errno; } - if (!bad_fn && (flags & PEX_STDERR_TO_STDOUT) != 0) + if (!failed.fn && (flags & PEX_STDERR_TO_STDOUT) != 0) { if (dup2 (STDOUT_FILE_NO, STDERR_FILE_NO) < 0) - bad_fn = "dup2"; + failed.fn = "dup2", failed.err = errno; } - if (!bad_fn) + if (!failed.fn) { if (env) /* NOTE: In a standard vfork implementation this clobbers @@ -644,30 +685,35 @@ pex_unix_exec_child (struct pex_obj *obj, int flags, const char *executable, if ((flags & PEX_SEARCH) != 0) { execvp (executable, to_ptr32 (argv)); - bad_fn = "execvp"; + failed.fn = "execvp", failed.err = errno; } else { execv (executable, to_ptr32 (argv)); - bad_fn = "execv"; + failed.fn = "execv", failed.err = errno; } } /* Something failed, report an error. We don't use stdio routines, because we might be here due to a vfork call. */ ssize_t retval = 0; - int eno = errno; - + + if (!do_pipe + || write (pipes[1], &failed, sizeof (failed)) != sizeof (failed)) + { + /* The parent will not see our scream above, so write to + stdout. */ #define writeerr(s) (retval |= write (STDERR_FILE_NO, s, strlen (s))) - writeerr (obj->pname); - writeerr (": error trying to exec '"); - writeerr (executable); - writeerr ("': "); - writeerr (bad_fn); - writeerr (": "); - writeerr (xstrerror (eno)); - writeerr ("\n"); + writeerr (obj->pname); + writeerr (": error trying to exec '"); + writeerr (executable); + writeerr ("': "); + writeerr (failed.fn); + writeerr (": "); + writeerr (xstrerror (failed.err)); + writeerr ("\n"); #undef writeerr + } /* Exit with -2 if the error output failed, too. */ _exit (retval < 0 ? -2 : -1); @@ -678,8 +724,6 @@ pex_unix_exec_child (struct pex_obj *obj, int flags, const char *executable, default: /* Parent process. */ { - const char *bad_fn = NULL; - /* Restore environ. Note that the parent either doesn't run until the child execs/exits (standard vfork behaviour), or if it does run then vfork is behaving more like fork. In @@ -687,24 +731,34 @@ pex_unix_exec_child (struct pex_obj *obj, int flags, const char *executable, copy of environ. */ environ = save_environ; - if (!bad_fn && in != STDIN_FILE_NO) + struct fn_err failed; + failed.fn = NULL; + if (do_pipe) + { + close (pipes[1]); + ssize_t len = read (pipes[0], &failed, sizeof (failed)); + if (len < 0) + failed.fn = NULL; + close (pipes[0]); + } + + if (!failed.fn && in != STDIN_FILE_NO) if (close (in) < 0) - bad_fn = "close"; - if (!bad_fn && out != STDOUT_FILE_NO) + failed.fn = "close", failed.err = errno; + if (!failed.fn && out != STDOUT_FILE_NO) if (close (out) < 0) - bad_fn = "close"; - if (!bad_fn && errdes != STDERR_FILE_NO) + failed.fn = "close", failed.err = errno; + if (!failed.fn && errdes != STDERR_FILE_NO) if (close (errdes) < 0) - bad_fn = "close"; + failed.fn = "close", failed.err = errno; - if (bad_fn) + if (failed.fn) { - *err = errno; - *errmsg = bad_fn; + *err = failed.err; + *errmsg = failed.fn; return (pid_t) -1; } } - return pid; } } -- 2.30.2