From patchwork Tue Feb 9 11:35:54 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Henderson X-Patchwork-Id: 10770 Received: (qmail 52145 invoked by alias); 9 Feb 2016 11:36:07 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 52026 invoked by uid 89); 9 Feb 2016 11:36:06 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=3.5 required=5.0 tests=BAYES_99, BAYES_999, FREEMAIL_ENVFROM_END_DIGIT, FREEMAIL_FROM, KAM_LOTSOFHASH, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=no version=3.3.2 spammy=$40, likelihood, 165, exits X-HELO: mail-pf0-f194.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:sender:subject:to:references:from:message-id :date:user-agent:mime-version:in-reply-to:content-type; bh=Tr7pl3zrGJcGcsjkeT6gpxUL9wdrBGmCulaz/2s0iic=; b=L95fWBjrX4iAyG14ys1H1CJCdIEI6Fef8Gfc8sOtrWM3Y0lKs+W/YAo/uyWYlwmCaT 5wfdoAKcK9+RBCF6cKJg2gMEUSTR2CVwUszwUIZ9FaY/7gB/kPIEUgorfT7RUaMZ0ACp O6z6j4LvBeMY9arZEv/xpFy9Z283TWKGTQy5O+DMNyooBF+b3k1SAOurlk8xj3ZGIHct l48uJTvP8K5oWJYZFGMalqvvi9Zc3hZ5WErTOlVATzMPZa4kHn0IoSK8JvLLokPepesd Mtg2BaTP+y90VdDCdfx+F2owDAWTnwgumyeGcNWHiPgr64/u2wqP1znnAZL2tezE0nRh aZvg== X-Gm-Message-State: AG10YORVDKbRNaCW7ffKIy3r4bIgfqygocB7pq1DHDb8c+juWTSfRdL2QczHZnEm5hRg9A== X-Received: by 10.98.86.8 with SMTP id k8mr48766994pfb.28.1455017761029; Tue, 09 Feb 2016 03:36:01 -0800 (PST) Subject: Re: [PATCH v2 1/3] posix: Remove dynamic memory allocation from execl{e,p} To: Rasmus Villemoes , libc-alpha@sourceware.org References: <1454343665-1706-1-git-send-email-adhemerval.zanella@linaro.org> <1454343665-1706-2-git-send-email-adhemerval.zanella@linaro.org> <20160202163335.GJ9349@brightrain.aerifal.cx> <878u2wfbwv.fsf@rasmusvillemoes.dk> From: Richard Henderson Message-ID: <56B9CF1A.6020807@twiddle.net> Date: Tue, 9 Feb 2016 22:35:54 +1100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.0 MIME-Version: 1.0 In-Reply-To: <878u2wfbwv.fsf@rasmusvillemoes.dk> On 02/08/2016 08:28 AM, Rasmus Villemoes wrote: > On Tue, Feb 02 2016, Rich Felker wrote: > >> On Mon, Feb 01, 2016 at 04:52:15PM +0000, Joseph Myers wrote: >>> On Mon, 1 Feb 2016, Adhemerval Zanella wrote: >>> >>>> + char *argv[argc+1]; >>>> + va_start (ap, arg); >>>> + argv[0] = (char*) arg; >>>> + for (i = 1; i < argc; i++) >>>> + argv[i] = va_arg (ap, char *); >>>> + argv[i] = NULL; >>> >>> I don't see how you're ensuring this stack allocation is safe (i.e. if >>> it's too big, it doesn't corrupt memory that's in use by other threads). >> >> There's no obligation to. If you pass something like a million >> arguments to a variadic function, the compiler will generate code in >> the caller that overflows the stack before the callee is even reached. >> The size of the vla used in execl is exactly the same size as the >> argument block on the stack used for passing arguments to execl from >> its caller, and it's nobody's fault but the programmer's if this is >> way too big. It's not a runtime variable. > > This is true, and maybe it's not worth the extra complication, but if > we're willing to make arch-specific versions of execl and execle we can > avoid the double stack use and the time spent copying the argv > array. That won't remove the possible stack overflow, of course, but > then it'll in all likelihood happen in the user code and not glibc. I like the idea. It's a quality of implementation issue, wherein by re-using the data that's (mostly) on the stack already we don't need twice again the amount of stack space for any given call. I think that Adhemerval's patch should go in as a default implementation, and various targets can implement the assembly as desired. I've tested the following on x86_64 and i686. I've compile-tested for x32 (but need a more complete x32 installation for testing), and alpha (testing is just slow). Thoughts? r~ >From 5b78856069a21550d4b67b4c0a269915f37fce0f Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Tue, 9 Feb 2016 13:43:08 +1100 Subject: [PATCH 5/5] alpha: Implement execl{,e,p} without double stack allocation --- sysdeps/unix/sysv/linux/alpha/execl.S | 52 ++++++++++++++++++++++++++++++ sysdeps/unix/sysv/linux/alpha/execle.S | 58 ++++++++++++++++++++++++++++++++++ sysdeps/unix/sysv/linux/alpha/execlp.S | 53 +++++++++++++++++++++++++++++++ 3 files changed, 163 insertions(+) create mode 100644 sysdeps/unix/sysv/linux/alpha/execl.S create mode 100644 sysdeps/unix/sysv/linux/alpha/execle.S create mode 100644 sysdeps/unix/sysv/linux/alpha/execlp.S diff --git a/sysdeps/unix/sysv/linux/alpha/execl.S b/sysdeps/unix/sysv/linux/alpha/execl.S new file mode 100644 index 0000000..11f4307 --- /dev/null +++ b/sysdeps/unix/sysv/linux/alpha/execl.S @@ -0,0 +1,52 @@ +/* Copyright (C) 2016 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +#include + +ENTRY(execl) + cfi_startproc + ldgp gp, 0(pv) + lda sp, -48(sp) + cfi_adjust_cfa_offset(48) + .frame sp, 48, ra + .prologue 1 + + /* Save the portions of the argv argument list in registers. */ + stq a5, 40(sp) + stq a4, 32(sp) + stq a3, 24(sp) + stq a2, 16(sp) + stq a1, 8(sp) + + /* Load the argv and envp arguments; path is already in place. */ + lda a1, 8(sp) + ldq a2, __environ + + lda v0, SYS_ify(execve) + call_pal PAL_callsys + + /* Discard the stack frame now. */ + lda sp, 48(sp) + cfi_adjust_cfa_offset(-48) + + /* All returns are errors. */ + br SYSCALL_ERROR_LABEL + +PSEUDO_END (execle) + cfi_endproc + +libc_hidden_def (execl) diff --git a/sysdeps/unix/sysv/linux/alpha/execle.S b/sysdeps/unix/sysv/linux/alpha/execle.S new file mode 100644 index 0000000..ce75ce3 --- /dev/null +++ b/sysdeps/unix/sysv/linux/alpha/execle.S @@ -0,0 +1,58 @@ +/* Copyright (C) 2016 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +#include + +ENTRY(execle) + cfi_startproc + lda sp, -48(sp) + cfi_adjust_cfa_offset(48) + .frame sp, 48, ra + .prologue 0 + + /* Save the portions of the argv argument list in registers. */ + stq a5, 40(sp) + stq a4, 32(sp) + stq a3, 24(sp) + stq a2, 16(sp) + stq a1, 8(sp) + + /* Find the env argument. It is the array element after the argv + NULL terminator, which cannot be located before argv[1]. */ + lda t0, 16(sp) +1: ldq t1, 0(t0) + addq t0, 8, t0 + bne t1, 1b + + /* Load the argv and envp arguments; path is already in place. */ + lda a1, 8(sp) + ldq a2, 0(t0) + + lda v0, SYS_ify(execve) + call_pal PAL_callsys + + /* Discard the stack frame now. */ + lda sp, 48(sp) + cfi_adjust_cfa_offset(-48) + + /* All returns are errors. */ + br SYSCALL_ERROR_LABEL + +PSEUDO_END (execle) + cfi_endproc + +libc_hidden_def (execle) diff --git a/sysdeps/unix/sysv/linux/alpha/execlp.S b/sysdeps/unix/sysv/linux/alpha/execlp.S new file mode 100644 index 0000000..b0ef76d --- /dev/null +++ b/sysdeps/unix/sysv/linux/alpha/execlp.S @@ -0,0 +1,53 @@ +/* Copyright (C) 2016 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +#include + +ENTRY(execlp) + cfi_startproc + ldgp gp, 0(pv) + lda sp, -48(sp) + cfi_adjust_cfa_offset(48) + stq ra, 0(sp) + cfi_rel_offset(ra, 0) + .prologue 1 + + /* Save the portions of the argv argument list in registers. */ + stq a5, 40(sp) + stq a4, 32(sp) + stq a3, 24(sp) + stq a2, 16(sp) + stq a1, 8(sp) + + /* Load the argv and envp arguments; path is already in place. */ + lda a1, 8(sp) + ldq a2, __environ +#ifdef PIC + bsr ra, __execvpe !samegp +#else + jsr ra, __execvpe + ldgp gp, 0(ra) +#endif + + lda sp, 48(sp) + cfi_adjust_cfa_offset(-48) + ret + +END (execlp) + cfi_endproc + +libc_hidden_def (execlp) -- 2.5.0