Re: [PATCH] avoid calling alloca(0)

On 12/2/16, Bernd Edlinger wrote: > On 12/01/16 19:39, Jeff Law wrote: >> On 11/30/2016 09:09 PM, Martin Sebor wrote: What I think this tells us is that we're not at a place where we're clean. But we can incrementally get there. The warning is only

Re: [PATCH] avoid calling alloca(0)

On 12/01/16 19:39, Jeff Law wrote: > On 11/30/2016 09:09 PM, Martin Sebor wrote: >>> What I think this tells us is that we're not at a place where we're >>> clean. But we can incrementally get there. The warning is only >>> catching a fairly small subset of the cases AFAICT. That's not unusual

Re: [PATCH] avoid calling alloca(0)

On 11/30/2016 09:09 PM, Martin Sebor wrote: What I think this tells us is that we're not at a place where we're clean. But we can incrementally get there. The warning is only catching a fairly small subset of the cases AFAICT. That's not unusual and analyzing why it didn't trigger on those

Re: [PATCH] avoid calling alloca(0)

On 11/30/2016 09:09 PM, Martin Sebor wrote: What I think this tells us is that we're not at a place where we're clean. But we can incrementally get there. The warning is only catching a fairly small subset of the cases AFAICT. That's not unusual and analyzing why it didn't trigger on those

Re: [PATCH] avoid calling alloca(0)

What I think this tells us is that we're not at a place where we're clean. But we can incrementally get there. The warning is only catching a fairly small subset of the cases AFAICT. That's not unusual and analyzing why it didn't trigger on those cases might be useful as well. The warning

Re: [PATCH] avoid calling alloca(0)

On 11/26/2016 05:52 PM, Martin Sebor wrote: On 11/25/2016 12:51 PM, Jeff Law wrote: On 11/23/2016 06:15 PM, Martin Sebor wrote: 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,

Re: [PATCH] avoid calling alloca(0)

On 11/25/2016 12:51 PM, Jeff Law wrote: On 11/23/2016 06:15 PM, Martin Sebor wrote: 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

Re: [PATCH] avoid calling alloca(0)

On 11/23/2016 06:15 PM, Martin Sebor wrote: 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. You

Re: [PATCH] avoid calling alloca(0)

On 11/24/2016 12:42 AM, Jakub Jelinek wrote: 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

Re: [PATCH] avoid calling alloca(0)

On 11/24/2016 12:47 AM, Jakub Jelinek wrote: On Wed, Nov 23, 2016 at 02:27:05PM -0700, Jeff Law wrote: I believe we should be warning on trying to allocation 0 bytes of memory via malloc, realloc or alloca, with the exception of a non-builtin alloca with no return value, but I think we've

Re: [PATCH] avoid calling alloca(0)

On Wed, Nov 23, 2016 at 02:27:05PM -0700, Jeff Law wrote: > I believe we should be warning on trying to allocation 0 bytes of memory via > malloc, realloc or alloca, with the exception of a non-builtin alloca with > no return value, but I think we've covered that elsewhere and Martin's code > will

Re: [PATCH] avoid calling alloca(0)

On Wed, Nov 23, 2016 at 06:15:11PM -0700, Martin Sebor wrote: > >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

Re: [PATCH] avoid calling alloca(0)

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

Re: [PATCH] avoid calling alloca(0)

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

Re: [PATCH] avoid calling alloca(0)

On 11/18/2016 10:25 AM, Jakub Jelinek wrote: On Fri, Nov 18, 2016 at 10:14:09AM -0700, Martin Sebor wrote: 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

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

Re: [PATCH] avoid calling alloca(0)

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)

Re: [PATCH] avoid calling alloca(0)

On 11/19/2016 11:18 PM, Jakub Jelinek wrote: On Sat, Nov 19, 2016 at 04:43:29PM -0700, Martin Sebor wrote: Thanks for calling out the realloc(0, p) case! Realloc(0, p) is ?? The DR you refer to deprecates realloc(p, 0), not realloc(0, p). We are discussing zero size allocation so I was

Re: [PATCH] avoid calling alloca(0)

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

Re: [PATCH] avoid calling alloca(0)

On Sat, Nov 19, 2016 at 04:43:29PM -0700, Martin Sebor wrote: > Thanks for calling out the realloc(0, p) case! Realloc(0, p) is ?? The DR you refer to deprecates realloc(p, 0), not realloc(0, p). The latter is used much more widely, e.g. by not special casing the first allocation. So you use

Re: [PATCH] avoid calling alloca(0)

So far I thought the warning is trying to make no differences between malloc, realloc and alloca. I would say that using realloc(x,0) is for sure always wrong. Nobody will object against a warning for that. Thanks for calling out the realloc(0, p) case! Realloc(0, p) is impossible to use

Re: [PATCH] avoid calling alloca(0)

On 11/19/16 00:52, Martin Sebor wrote: > If you or others are concerned about the rate of false positives > of this warning please point me at a code base that might be a good > test bed to to try it on. Besides GCC I've built Binutils and the > Linux kernel with just the 5 instances in GCC. >

Re: [PATCH] avoid calling alloca(0)

On 11/19/16 00:52, Martin Sebor wrote: > On 11/18/2016 03:51 PM, Bernd Edlinger wrote: >> > of the builtin (the function is not declared without attribute >> > alloc_size, at least in Glibc, but GCC still expands it inline). >> > This is as simple as enclosing alloca in parentheses: >> > >> >

Re: [PATCH] avoid calling alloca(0)

On 11/18/2016 03:51 PM, Bernd Edlinger wrote: > of the builtin (the function is not declared without attribute > alloc_size, at least in Glibc, but GCC still expands it inline). > This is as simple as enclosing alloca in parentheses: > > void *p = (alloca)(n); > > Ugly? Perhaps. One

Re: [PATCH] avoid calling alloca(0)

> of the builtin (the function is not declared without attribute > alloc_size, at least in Glibc, but GCC still expands it inline). > This is as simple as enclosing alloca in parentheses: > > void *p = (alloca)(n); > > Ugly? Perhaps. One might say that code that does tricky or No. I

Re: [PATCH] avoid calling alloca(0)

On 11/18/2016 10:25 AM, Jakub Jelinek wrote: On Fri, Nov 18, 2016 at 10:14:09AM -0700, Martin Sebor wrote: 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

Re: [PATCH] avoid calling alloca(0)

On 11/18/2016 11:46 AM, Jeff Law wrote: On 11/18/2016 12:58 AM, Jakub Jelinek wrote: On Thu, Nov 17, 2016 at 05:32:31PM -0700, Martin Sebor wrote: The point is warning on an alloca (0) may not be as clear cut as it might seem. It's probably a reasonable thing to do on the host, but on a

Re: [PATCH] avoid calling alloca(0)

On 11/18/2016 12:58 AM, Jakub Jelinek wrote: On Thu, Nov 17, 2016 at 05:32:31PM -0700, Martin Sebor wrote: The point is warning on an alloca (0) may not be as clear cut as it might seem. It's probably a reasonable thing to do on the host, but on a target, which might be embedded and explicitly

Re: [PATCH] avoid calling alloca(0)

On 11/17/2016 05:32 PM, Martin Sebor wrote: I confess I haven't heard of such an implementation before but after some searching I have managed to find a few examples of it online, such as in QNX or in the BlackBerry SDK, or in the GCC shipped by Texas Instruments. For instance: I have the

Re: [PATCH] avoid calling alloca(0)

On Fri, Nov 18, 2016 at 10:14:09AM -0700, Martin Sebor wrote: > 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

Re: [PATCH] avoid calling alloca(0)

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.

Re: [PATCH] avoid calling alloca(0)

On 11/18/16, 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

Re: [PATCH] avoid calling alloca(0)

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 >

Re: [PATCH] avoid calling alloca(0)

On 11/18/2016 12:58 AM, Jakub Jelinek wrote: On Thu, Nov 17, 2016 at 05:32:31PM -0700, Martin Sebor wrote: The point is warning on an alloca (0) may not be as clear cut as it might seem. It's probably a reasonable thing to do on the host, but on a target, which might be embedded and explicitly

Re: [PATCH] avoid calling alloca(0)

On Thu, Nov 17, 2016 at 05:32:31PM -0700, Martin Sebor wrote: > >The point is warning on an alloca (0) may not be as clear cut as it > >might seem. It's probably a reasonable thing to do on the host, but on > >a target, which might be embedded and explicitly overriding the builtin > >alloca,

Re: [PATCH] avoid calling alloca(0)

On 11/17/2016 05:32 PM, Martin Sebor wrote: On 11/17/2016 03:52 PM, Jeff Law wrote: On 11/17/2016 03:03 PM, Jakub Jelinek wrote: On Thu, Nov 17, 2016 at 01:56:03PM -0700, Jeff Law wrote: On 11/17/2016 11:24 AM, Jakub Jelinek wrote: On Thu, Nov 17, 2016 at 11:14:26AM -0700, Martin Sebor

Re: [PATCH] avoid calling alloca(0)

On 11/17/2016 03:52 PM, Jeff Law wrote: On 11/17/2016 03:03 PM, Jakub Jelinek wrote: On Thu, Nov 17, 2016 at 01:56:03PM -0700, Jeff Law wrote: On 11/17/2016 11:24 AM, Jakub Jelinek wrote: On Thu, Nov 17, 2016 at 11:14:26AM -0700, Martin Sebor wrote: --- a/gcc/fortran/interface.c +++

Re: [PATCH] avoid calling alloca(0)

On 11/17/2016 03:03 PM, Jakub Jelinek wrote: On Thu, Nov 17, 2016 at 01:56:03PM -0700, Jeff Law wrote: On 11/17/2016 11:24 AM, Jakub Jelinek wrote: On Thu, Nov 17, 2016 at 11:14:26AM -0700, Martin Sebor wrote: --- a/gcc/fortran/interface.c +++ b/gcc/fortran/interface.c @@ -2821,7 +2821,8 @@

Re: [PATCH] avoid calling alloca(0)

On Thu, Nov 17, 2016 at 01:56:03PM -0700, Jeff Law wrote: > On 11/17/2016 11:24 AM, Jakub Jelinek wrote: > >On Thu, Nov 17, 2016 at 11:14:26AM -0700, Martin Sebor wrote: > >>--- a/gcc/fortran/interface.c > >>+++ b/gcc/fortran/interface.c > >>@@ -2821,7 +2821,8 @@ compare_actual_formal

Re: [PATCH] avoid calling alloca(0)

On 11/17/2016 11:24 AM, Jakub Jelinek wrote: On Thu, Nov 17, 2016 at 11:14:26AM -0700, Martin Sebor wrote: --- a/gcc/fortran/interface.c +++ b/gcc/fortran/interface.c @@ -2821,7 +2821,8 @@ compare_actual_formal (gfc_actual_arglist **ap, gfc_formal_arglist *formal, for (f = formal; f; f =

Re: [PATCH] avoid calling alloca(0)

On Thu, Nov 17, 2016 at 11:14:26AM -0700, Martin Sebor wrote: > --- a/gcc/fortran/interface.c > +++ b/gcc/fortran/interface.c > @@ -2821,7 +2821,8 @@ compare_actual_formal (gfc_actual_arglist **ap, > gfc_formal_arglist *formal, >for (f = formal; f; f = f->next) > n++; > > - new_arg =