[PATCH] avoid calling alloca(0)

[Martin Sebor]

On 11/23/2016 01:57 PM, Jeff Law wrote:
> On 11/20/2016 04:06 PM, Martin Sebor wrote:
>> On 11/20/2016 01:03 AM, Bernd Edlinger wrote:
>>> On 11/20/16 00:43, Martin Sebor wrote:
>>>> As best I can tell the result isn't actually used (the code that
>>>> uses the result gets branched over).  GCC just doesn't see it.
>>>> I verified this by changing the XALLOCAVEC macro to
>>>>
>>>>   #define XALLOCAVEC(T, N)  (N ? alloca (sizeof (T) * N) : 0)
>>>>
>>>> and bootstrapping and running the regression test suite with
>>>> no apparent regressions.
>>>>
>>>
>>> I would like this solution with braces around N better than
>>> allocating one element when actually zero were requested.
>>>
>>> The disadvantage of N+1 or N+!N is that it will hide true
>>> programming errors from the sanitizer.
>>
>> I agree.  Let me post an updated patch with the above fix and
>> leave it to others to choose which approach to go with.
> Just so I'm clear, this part of the discussions is talking about
> mitigation strategies within GCC itself, right?

That's right.

> Can't we just
> gcc_assert (x != 0) before the problematical calls?  That avoids
> unnecessary over-allocation and gets us a clean ICE if we were to try
> and call alloca with a problematical value.

gcc_assert works only in some instances (e.g., in c-ada-spec.c:191)
but not in others because some actually do make the alloca(0) call
at runtime: at a minimum, lto.c:3285, reg-stack.c:2008, and
tree-ssa-threadedge.c:344 assert during bootstrap.

After reviewing a few more of the XALLOCAVEC calls in the affected
files I suspect that those to alloca(0) pointed out by the warning
may be just a subset that GCC happens to see thanks to constant
propagation.  If that's so then changing this subset to
alloca(N + !N) or some such is probably not the right approach
because it will miss all the other calls where GCC doesn't see that
N is zero.  I think the most robust solution is to do as Bernd
suggests: change XALLOCAVEC as shown above.  That will let us
prevent any and all unsafe assumptions about the result of
alloca(0) being either non-null or distinct.  The other approach
would be to change XALLOCAVEC to add 1 to the byte count.  That
would be in line with what XMALLOC does.

> I'm not sure we need to break things up.  I haven't seen a good argument
> for that yet.
>
> Is there an updated patch to post?  Or has it already been posted?

Given the above I suggest going with one of the attached two patches.

Martin


-------------- next part --------------
A non-text attachment was scrubbed...
Name: gcc-libiberty-xallocavec-null.diff
Type: text/x-patch
Size: 710 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20161124/07e1723c/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: gcc-libiberty-xallocavec-nonzero.diff
Type: text/x-patch
Size: 696 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20161124/07e1723c/attachment-0001.bin>