Re: protected alloca class for malloc fallback

[Jeff Law <law@redhat.com>]

On 08/04/2016 09:19 AM, Aldy Hernandez wrote:

>>>
>>> Particularly irritating and error prone is having to free malloc'd
>>> pointers
>>> on every function exit point.  We end up with a lot of:
>>>
>>> foo(size_t len)
>>> {
>>>    void *p, *m_p = NULL;
>>>    if (len < HUGE)
>>>      p = alloca(len);
>>>    else
>>>      p = m_p = malloc(len);
>>>    if (something)
>>>      goto out;
>>>    stuff();
>>> out:
>>>    free (m_p);
>>> }
>>>
>>> ...which nobody really likes.
Yup.  You'll see that all over glibc.  I don't think we use that idiom 
all that much for GCC.

>>>
>>> I've been thinking that for GCC we could have a protected_alloca
>>> class whose
>>> destructor frees any malloc'd memory:
>>>
>>> void foo()
>>> {
>>>    char *p;
>>>    protected_alloca chunk(50000);
>>>    p = (char *) chunk.pointer();
>>>    f(p);
>>> }
>>>
>>> This would generate:
>>>
>>> void foo() ()
>>> {
>>>    void * _3;
>>>
>>>    <bb 2>:
>>>    _3 = malloc (50000);
>>>    f (_3);
>>>
>>>    <bb 3>:
>>>    free (_3); [tail call]
>>>    return;
>>> }
>>>
>>> Now the problem with this is that the memory allocated by chunk is freed
>>> when it goes out of scope, which may not be what you want.  For example:
>>>
>>>       func()
>>>       {
>>>         char *str;
>>>         {
>>>           protected_alloca chunk (99999999);
>>>           // malloc'd pointer will be freed when chunk goes out of
>>> scope.
>>>           str = (char *) chunk.pointer ();
>>>         }
>>>         use (str);  // BAD!  Use after free.
>>>       }
>>
>> But how's that an issue if the chunk is created at the exact place
>> where there
>> previously was an alloca?
>
> The pointer can escape if you assign it to a variable outside the scope
> of chunk?  Take for instance the following snippet in tree.c:
Right.  The alloca model is (with the exception of vlas in loops) is the 
storage hangs around until function scope is left.  So releasing on exit 
of the scope in which the space was allocated is wrong unless allocation 
occurred at function scope.

That doesn't play as well with an RAII model where objects get destroyed 
as soon as they leave their lexical scope.



So
>
>     {
>     ...
>     ...
>       q = (char *) alloca (9 + 17 + len + 1);
>       memcpy (q, file, len + 1);
>
>       snprintf (q + len, 9 + 17 + 1, "_%08X_" HOST_WIDE_INT_PRINT_HEX,
>         crc32_string (0, name), get_random_seed (false));
>
>       p = q;
>     }
>
>     clean_symbol_name (q);
>
> If you define `protected_alloca chunk(9 + 17 + len + 1)' at the alloca()
> call, chunk will be destroyed at the "}", whereas `q' is still being
> used outside of that scope.
>
> What I am suggesting for this escaping case is to define
> "protected_alloca chunk()" at function scope, and then do chunk.alloc(N)
> in the spot where the alloca() call was previously at.
>
> Or am I missing something?
I think only thing you're missing is that we probably don't want to be 
using alloca in new code anyway.  And if we're going through the trouble 
of fixing old code, we ought to just remove alloca.  So building up this 
kind of class is probably taking folks in the wrong direction.

Now you might use that opportunity to convert some object to a RAII 
model (and I'm a huge fan of that model).

Jeff