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.
you should also include glibc and busybox, to be sure.
Thanks for the suggestion. I've done that and found no instances
of any of these warnings in either Busybox 1.25.1 or Glibc trunk.
Good. I see many alloca calls all over, but mostly in the runtime
libraries, that don't use -Werror.
Have you done a bootstrap with all languages?
Were there warnings in the target libraries (note they don't use
-Werror, so you have to grep)?
Yes, I bootstrapped all languages. When testing my latest patch
yesterday I found a couple more places in GCC (one in Ada and
another in the middle end I think) that trigger the warning that
I had missed in the first patch. I'll post an updated patch soon.
I am a bit worried about that suggestion in libiberty/alloca.c:
"It is a good idea to use alloca(0) in
your main control loop, etc. to force garbage collection."
Because:
#include <stdio.h>
#include <alloca.h>
int main()
{
void* p;
int i;
for (i=0 ; i<100 ; i++)
{
p = alloca(0);
printf("%p\n", p);
}
}
... is fine, and allocates no memory.
But if I change that to alloca(1) the stack will blow up.
The suggestion to call alloca(0) in the main loop is to trigger
garbage collection when alloca is implemented using malloc (as
is apparently the case in some embedded environments). In that
case, alloca is not a built-in but a library function (i.e.,
-fno-builtin-alloca must be set) and the warning doesn't trigger.
This call to alloca(0) would also likely be made without using
the return value. When __builtin_alloca is called without using
the return value, the call is eliminated and the warning doesn't
trigger either.
Maybe it would be better to have a different warning option
for malloc/realloc with zero and for alloca with zero?
There is a -Walloca-larger-than that Aldy added earlier this
year. The option also diagnoses alloca(0), and it also warns
on calls to alloca in loops, but it's disabled by default.
I feel fairly strongly that all zero-length allocations
should be diagnosed at least with -Wextra but if that's what
it takes to move forward I'm willing to break things up this
way. If that's preferred then I would also expect to enable
-Walloca-larger-than=SIZE_MAX/2 (or smaller) by default, in
addition to having -Walloca-zero in -Wextra, analogously to
the corresponding warnings for the heap allocation functions.
(Ditto for -Wvla-larger-than=SIZE_MAX/2 and -Wvla-zero which
should probably also be added for consistency.)
Martin
