[PATCH] avoid calling alloca(0)

[Jeff Law]

On 11/18/2016 10:14 AM, Martin Sebor wrote:
>>
>> Most apps know what malloc (0) means and treat it correctly, they know
>> they
>> shouldn't dereference the pointer, because it is either NULL or holds an
>> array with 0 elements.  I fail to see why you would want to warn.
>
> Because people make mistakes and warnings help us avoid them (isn't
> that obvious?)  Just because we get it right most of the time doesn't
> mean we get right all of the time.  The papers and exploits I pointed
> to should provide ample evidence that zero allocations are a problem
> that can have serious (and costly) consequences.  We (i.e., Red Hat)
> recognize this risk and have made it our goal to help stem some of
> these problems.
I suspect most applications don't ever do malloc (0) and that its 
appearance is more likely an indicator of a bug.  That's what I want to 
cater to -- finding bugs before they get out into the field.

For the oddball application that wants to malloc (0) and try to do 
something sensible, they can turn off the warning.

So I'm in agreement with Martin here.

>
>>> But malloc(0) has also been known to result from unsigned overflow
>>> which has led to vulnerabilities and exploits (famously by the JPG
>>> COM vulnerability exploit, but there are plenty of others, including
>>> for instance CVE-2016-2851).  Much academic research has been devoted
>>> to this problem and to solutions to detect and prevent it.
>>
>> How is it different from overflowing to 1 or 2 or 27?  What is special on
>> the value 0?
>
> It's a case that, unlike the others, can be readily detected.  It
> would be nice to detect the others as well but GCC can't do that
> yet.  That doesn't mean we shouldn't try to detect at least the
> small subset for now.  It doesn't have to be perfect for it to be
> useful.
Right.  And as I know I've mentioned to some folks, I'd really like us 
to be pondering a "may overflow" warning for expressions  that feed into 
an allocator.  There's a lot of value in that, but I suspect a lot of 
noise as well.


>
>>
>>> In the absence of a policy or guidelines it's a matter of opinion
>>> whether or not this warning belongs in either -Wall or -Wextra.
>>
>> It IMHO doesn't belong to either of these.
>
> You've made that quite clear.  I struggle to reconcile your
> position in this case with the one in debate about the
> -Wimplicit-fallthrough option where you insisted on the exact
> opposite despite the overwhelming number of false positives
> caused by it.  Why is it appropriate for that option to be in
> -Wextra and not this one?
I disagree with Jakub on this.  I think the warning would be fine for 
Wextra.  It's a lot less invasive than other stuff that's gone in there.

jeff