[PATCH v2 1/3] posix: Remove dynamic memory allocation from execl{e,p}

[Richard Henderson]

On 02/08/2016 08:28 AM, Rasmus Villemoes wrote:
> On Tue, Feb 02 2016, Rich Felker <dalias@libc.org> 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~
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Move-posix-execl-files-to-sysdeps.patch
Type: text/x-patch
Size: 978 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/libc-alpha/attachments/20160209/1cddde7a/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-x86_64-Implement-execl-e-p-without-double-stack-allo.patch
Type: text/x-patch
Size: 7219 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/libc-alpha/attachments/20160209/1cddde7a/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-i386-Implement-execl-e-p-without-double-stack-alloca.patch
Type: text/x-patch
Size: 4114 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/libc-alpha/attachments/20160209/1cddde7a/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-x32-Implement-execl-e-p-without-double-stack-allocat.patch
Type: text/x-patch
Size: 8125 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/libc-alpha/attachments/20160209/1cddde7a/attachment-0003.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-alpha-Implement-execl-e-p-without-double-stack-alloc.patch
Type: text/x-patch
Size: 5899 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/libc-alpha/attachments/20160209/1cddde7a/attachment-0004.bin>