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

[Adhemerval Zanella <adhemerval dot zanella at linaro dot org>]

On 09-02-2016 11:30, Szabolcs Nagy wrote:
> On 09/02/16 11:35, Richard Henderson wrote:
>> 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?
>>
> 
> i think it is a hard to maintain, nasty hack
> 
> is execl stack usage the bottleneck somewhere?
> 
> to improve the stack usage in glibc, the extreme
> wasteful cases should be fixed first.
> (crypt uses >100k, strtod uses ???, etc)
> 
> e.g. musl guarantees hard limits on libc stack usage
> and execl is one of the (two) exceptions where it just
> seems useless to do so (you cannot do it portably anyway).
> 


I agree with Szabolcs and I also see these kind of asm specific
implementation also adds platform different behaviour which I also
think we should avoid to add within GLIBC.

The only doubt with my patch I have now is if it is worth to add
the -fstack-check or not. Florian has raised some questioning about
its efficacy and I am not sure how well it is supported on all 
architectures.