From patchwork Mon Aug 20 17:38:30 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nathan Sidwell X-Patchwork-Id: 959849 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-483997-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=acm.org Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="W1YBbaSM"; dkim-atps=neutral Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 41vLdM4fWGz9s5c for ; Tue, 21 Aug 2018 03:38:47 +1000 (AEST) Received: (qmail 8519 invoked by alias); 20 Aug 2018 17:38:38 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 8411 invoked by uid 89); 20 Aug 2018 17:38:37 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-11.9 required=5.0 tests=BAYES_00, FREEMAIL_FROM, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mail-yw1-f46.google.com Received: from mail-yw1-f46.google.com (HELO mail-yw1-f46.google.com) (209.85.161.46) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 20 Aug 2018 17:38:35 +0000 Received: by mail-yw1-f46.google.com with SMTP id x67-v6so2045085ywg.0 for ; Mon, 20 Aug 2018 10:38:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:to:from:subject:message-id:date:user-agent:mime-version :content-language; bh=PXI/6eZZko5MLYuvoWyz2bxI+rvVP2J0YSD/z5YmbY4=; b=W1YBbaSMdm3lv1+YvBvvtbpAq6I/IQCMe9aGcYGPmztxLugeNNUAaor7dbvNc2p5o2 jJJb/SZrsybZf/8+hm9NkOenB+bIFCq5JQITNpd8su1NGLy2wgI9SsDLCEkh87xpTlr0 E0K2LzN0QcgjELkZd5GINfD9Q55UTilXCozYtycAPkCi+uRnSOSBbY55YSaOU0yqY9gu xcc5oAtrR63IJuIChxWpcrfbnEN5kUVADE9O+gns6X522ZPUtlE1eKpKZG78DCMfl9p0 RULx8yaSPQY7yoARfynWUAjtf2k7ijO1MGsBk76W30qV+7PHRcpZFS+XwKOFCe0rjszp dn9Q== Received: from ?IPv6:2620:10d:c0a3:20fb:7500:e7fb:4a6f:2254? ([2620:10d:c091:200::5c29]) by smtp.googlemail.com with ESMTPSA id m82-v6sm12437708ywm.19.2018.08.20.10.38.32 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 20 Aug 2018 10:38:32 -0700 (PDT) Sender: Nathan Sidwell To: GCC Patches , Ian Lance Taylor From: Nathan Sidwell Subject: [libiberty patch] PEX-unix forking Message-ID: <593c010d-0d12-cbf9-e1a9-e0f3108ffdf8@acm.org> Date: Mon, 20 Aug 2018 13:38:30 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 This is the first of a pair of patches I've had on the modules branch for a while. They improve the error behaviour in the case of child failure when vfork is the forking mechanism. This one commonizes the error paths in the parent and child to a pair of single blocks that print or return the error respectively. Currently each error case calls pex_child_error, or returns the error in the parent. The patch records the name of a failing system call, and uses that to print or return the error. It doesn't change functionality but will simplify the next patch that does. booted on x86_64-linux (which uses vfork), ok? nathan 2018-08-20 Nathan Sidwell * pex-unix.c (pex_child_error): Delete. (pex_unix_exec_child): Commonize error paths to single message & exit. Index: pex-unix.c =================================================================== --- pex-unix.c (revision 263667) +++ pex-unix.c (working copy) @@ -298,8 +298,6 @@ pex_wait (struct pex_obj *obj, pid_t pid #endif /* ! defined (HAVE_WAITPID) */ #endif /* ! defined (HAVE_WAIT4) */ -static void pex_child_error (struct pex_obj *, const char *, const char *, int) - ATTRIBUTE_NORETURN; static int pex_unix_open_read (struct pex_obj *, const char *, int); static int pex_unix_open_write (struct pex_obj *, const char *, int, int); static pid_t pex_unix_exec_child (struct pex_obj *, int, const char *, @@ -366,28 +364,6 @@ pex_unix_close (struct pex_obj *obj ATTR return close (fd); } -/* Report an error from a child process. We don't use stdio routines, - because we might be here due to a vfork call. */ - -static void -pex_child_error (struct pex_obj *obj, const char *executable, - const char *errmsg, int err) -{ - int retval = 0; -#define writeerr(s) retval |= (write (STDERR_FILE_NO, s, strlen (s)) < 0) - writeerr (obj->pname); - writeerr (": error trying to exec '"); - writeerr (executable); - writeerr ("': "); - writeerr (errmsg); - writeerr (": "); - writeerr (xstrerror (err)); - writeerr ("\n"); -#undef writeerr - /* Exit with -2 if the error output failed, too. */ - _exit (retval == 0 ? -1 : -2); -} - /* Execute a child. */ #if defined(HAVE_SPAWNVE) && defined(HAVE_SPAWNVPE) @@ -592,21 +568,22 @@ pex_unix_exec_child (struct pex_obj *obj int in, int out, int errdes, int toclose, const char **errmsg, int *err) { - pid_t pid; + pid_t pid = -1; /* We declare these to be volatile to avoid warnings from gcc about them being clobbered by vfork. */ - volatile int sleep_interval; + volatile int sleep_interval = 1; volatile int retries; /* We vfork and then set environ in the child before calling execvp. 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. */ + environment as a parameter, but that may have portability + issues. */ char **save_environ = environ; - sleep_interval = 1; - pid = -1; + const char *bad_fn = NULL; + for (retries = 0; retries < 4; ++retries) { pid = vfork (); @@ -625,57 +602,76 @@ pex_unix_exec_child (struct pex_obj *obj case 0: /* Child process. */ - if (in != STDIN_FILE_NO) + if (!bad_fn && in != STDIN_FILE_NO) { if (dup2 (in, STDIN_FILE_NO) < 0) - pex_child_error (obj, executable, "dup2", errno); - if (close (in) < 0) - pex_child_error (obj, executable, "close", errno); + bad_fn = "dup2"; + else if (close (in) < 0) + bad_fn = "close"; } - if (out != STDOUT_FILE_NO) + if (!bad_fn && out != STDOUT_FILE_NO) { if (dup2 (out, STDOUT_FILE_NO) < 0) - pex_child_error (obj, executable, "dup2", errno); - if (close (out) < 0) - pex_child_error (obj, executable, "close", errno); + bad_fn = "dup2"; + else if (close (out) < 0) + bad_fn = "close"; } - if (errdes != STDERR_FILE_NO) + if (!bad_fn && errdes != STDERR_FILE_NO) { if (dup2 (errdes, STDERR_FILE_NO) < 0) - pex_child_error (obj, executable, "dup2", errno); - if (close (errdes) < 0) - pex_child_error (obj, executable, "close", errno); + bad_fn = "dup2"; + else if (close (errdes) < 0) + bad_fn = "close"; } - if (toclose >= 0) + if (!bad_fn && toclose >= 0) { if (close (toclose) < 0) - pex_child_error (obj, executable, "close", errno); + bad_fn = "close"; } - if ((flags & PEX_STDERR_TO_STDOUT) != 0) + if (!bad_fn && (flags & PEX_STDERR_TO_STDOUT) != 0) { if (dup2 (STDOUT_FILE_NO, STDERR_FILE_NO) < 0) - pex_child_error (obj, executable, "dup2", errno); + bad_fn = "dup2"; } - - if (env) + if (!bad_fn) { - /* NOTE: In a standard vfork implementation this clobbers the - parent's copy of environ "too" (in reality there's only one copy). - This is ok as we restore it below. */ - environ = (char**) env; + if (env) + /* NOTE: In a standard vfork implementation this clobbers + the parent's copy of environ "too" (in reality there's + only one copy). This is ok as we restore it below. */ + environ = (char**) env; + if ((flags & PEX_SEARCH) != 0) + { + execvp (executable, to_ptr32 (argv)); + bad_fn = "execvp"; + } + else + { + execv (executable, to_ptr32 (argv)); + bad_fn = "execv"; + } } - if ((flags & PEX_SEARCH) != 0) - { - execvp (executable, to_ptr32 (argv)); - pex_child_error (obj, executable, "execvp", errno); - } - else - { - execv (executable, to_ptr32 (argv)); - pex_child_error (obj, executable, "execv", 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 err = errno; + +#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 (err)); + writeerr ("\n"); +#undef writeerr + /* Exit with -2 if the error output failed, too. */ + _exit (retval < 0 ? -2 : -1); + } /* NOTREACHED */ return (pid_t) -1; @@ -689,32 +685,18 @@ pex_unix_exec_child (struct pex_obj *obj the child's copy of environ. */ environ = save_environ; - if (in != STDIN_FILE_NO) - { - if (close (in) < 0) - { - *err = errno; - *errmsg = "close"; - return (pid_t) -1; - } - } - if (out != STDOUT_FILE_NO) - { - if (close (out) < 0) - { - *err = errno; - *errmsg = "close"; - return (pid_t) -1; - } - } - if (errdes != STDERR_FILE_NO) - { - if (close (errdes) < 0) - { - *err = errno; - *errmsg = "close"; - return (pid_t) -1; - } + if (!bad_fn && in != STDIN_FILE_NO && close (in) < 0) + bad_fn = "close"; + if (!bad_fn && out != STDOUT_FILE_NO && close (out) < 0) + bad_fn = "close"; + if (!bad_fn && errdes != STDERR_FILE_NO && close (errdes) < 0) + bad_fn = "close"; + + if (bad_fn) + { + *err = errno; + *errmsg = bad_fn; + return (pid_t) -1; } return pid;