On 11/18/2016 09:29 AM, Jakub Jelinek wrote:
On Fri, Nov 18, 2016 at 09:21:38AM -0700, Martin Sebor wrote:
In the libiberty/alloca.c, I don't see anything special about alloca (0).
Yes, alloca (0) reclaims garbage, but so does alloca (1) and alloca (4035).
alloca (0) just returns NULL after it. The diffutils link is the same
alloca.c as in libiberty. Returning NULL or returning a non-unique pointer
for alloca (0) is just fine, it is the same thing as returning NULL or
returning a non-unique pointer from malloc (0). We definitely don't want
to warn for malloc (0), and IMNSHO don't want to warn for alloca (0),
because it behaves the same.
I disagree. At a minimum, the return value of malloc(0) is
implementation-defined and so relying on it being either null
or non-null is non-portable and can be a source of subtle bugs.
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.
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.
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?
Martin
