On Thu, Aug 4, 2016 at 1:30 PM, Aldy Hernandez <[email protected]> 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?
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.
> 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?
Having the conditional malloc/alloca will also inhibit optimization like eliding
the malloc or alloca calls completely.
Thanks,
Richard.
> Included is the conversion of tree.c. More to follow once we agree on a
> solution.
>
> Tested on x86-64 Linux.
>
> Aldy
