On Wed, 2013-07-03 at 12:44 +0200, Petr Machata wrote:
We tend to use stack allocation a fair deal in elfutils (61
alloca's
vs. 112 [cm]alloc's; not counting VLA's, as they are hard to grep for).
I'm afraid more of those places might blow up in face of malformed, or
simply huge data.
In glibc, there are macros for handling malloc/alloca splits (alloca if
small, malloc otherwise). In this case, we don't know the line count in
advance, and would have to use LINEENDP-LINEP as a hint. But maybe it
would be worth it to port those over, as other places might benefit from
this more directly. I'll look into it.
Most other locations (I found less than you, so maybe I missed some)
seem to be somewhat constrained and used for smaller data structures.
But there are a couple of other places (going over the dirlist, parsing
everything in an expression block, etc.) that could potentially contain
many items and surprise us. If there is a way to automatically keep
track of the stack depth and switch to other allocation techniques when
detecting a large amount of stack allocations that would be nice.
For now, your patch makes
sense, except:
> + if (unlikely (add_new_line (ll, end_seq))) \
> goto invalid_data; \
Wouldn't this leak the malloc'd blocks?
Indeed it would, as would some other failure paths. Thanks for spotting
that. I tweaked the patch a little to initialize everything slightly
earlier and do the deallocation at the very end (all success and failure
paths end up at out:). New patch attached. Also tested with valgrind to
make sure there are no leaks or erroneous frees.
Cheers,
Mark