This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] avoid calling alloca(0)
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? 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.
ed 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.
Correct.
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.
Also correct.
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.)
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?
jeff