protected alloca class for malloc fallback

Richard Biener richard.guenther@gmail.com
Fri Aug 5 08:17:00 GMT 2016


On Thu, Aug 4, 2016 at 5:19 PM, Aldy Hernandez <aldyh@redhat.com> wrote:
> On 08/04/2016 08:57 AM, Richard Biener wrote:
>>
>> On Thu, Aug 4, 2016 at 1:30 PM, Aldy Hernandez <aldyh@redhat.com> wrote:
>>>
>>> Howdy!
>>>
>>> As part of my -Walloca-larger-than=life work, I've been running said pass
>>> over gcc, binutils, and gdb, and trying to fix things along the way.
>>>
>>> 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.
>>>
>>> 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:
>
>     {
>     ...
>     ...
>       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?

Ah, ok - alloca memory is only freed at the end of the function.  Only VLAs
are freed at scope boundary.

>>
>> Your class also will not work when internal_alloc is not inlined and
>> the alloca path
>> is taken like when using non-GCC host compilers.
>
>
> Does not work, or is not optimal?  Because defining _ALLOCA_INLINE_ to
> nothing and forcing no-inline with:
>
>         g++ -c b.cc -fno-inline -fdump-tree-all  -O1 -fno-exceptions
>
> I still see correct code.  It's just that it's inefficient, which we
> shouldn't care because bootstrap fixes the non-GCC inlining problem :).

As alloca() frees memory at function return code cannot be "correct".

>   protected_alloca chunk(123);
>   str = (char *) chunk.pointer();
>   use(str);
>
> becomes:
>
>   <bb 2>:
>   protected_alloca::protected_alloca (&chunk, 123);
>   str_3 = protected_alloca::pointer (&chunk);
>   use (str_3);
>   protected_alloca::~protected_alloca (&chunk);
>   return;
>
> What am I missing?

The memory is released after internal_alloc returns.  So if you do

  protected_alloca::protected_alloca (&chunk, 123);
  ...
  foo (); // function needing some stack space or GCC spilling
  ...

you'll corrupt memory.

>>
>>> In the attached patch implementing this class I have provided another
>>> idiom
>>> for avoiding this problem:
>>>
>>>       func()
>>>       {
>>>         void *ptr;
>>>         protected_alloca chunk;
>>>         {
>>>           chunk.alloc (9999999);
>>>           str = (char *) chunk.pointer ();
>>>         }
>>>         // OK, pointer will be freed on function exit.
>>>         use (str);
>>>       }
>>>
>>> So I guess it's between annoying gotos and keeping track of multiple exit
>>> points to a function previously calling alloca, or making sure the
>>> protected_alloca object always resides in the scope where the memory is
>>> going to be used.
>>>
>>> Is there a better blessed C++ way?  If not, is this OK?
>>
>>
>> It looks like you want to replace _all_ alloca uses?  What's the point
>> in doing this
>> at all?  Just to be able to enable the warning during bootstrap?
>
>
> Well, it did cross my mind to nix anything that had 0 bounds checks, but I
> was mostly interested in things like this:
>
> gcc.c:
>   temp = env.get ("COMPILER_PATH");
>   if (temp)
>     {
>       const char *startp, *endp;
>       char *nstore = (char *) alloca (strlen (temp) + 3);
>
> I was just providing a generic interface for dealing with these cases in the
> future, instead of gotoing my way out of it.
>
>>
>> Having the conditional malloc/alloca will also inhibit optimization like
>> eliding
>> the malloc or alloca calls completely.
>
>
> If we can elide the alloca, we can surely elide a conditional alloca /
> malloc pair, can't we?

We can't at the moment.  We don't for malloc (we only remove malloc/free
pairs when the memory is not used), we can elide alloca to use a local
decl instead.

Richard.

>
> Aldy



More information about the Gcc-patches mailing list