This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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 portably and has been deprecated in C11 in
response to defect report 400:
http://www.open-std.org/jtc1/sc22/wg14/www/docs/summary.htm#dr_400
This change will be published in the 2017 technical corrigendum to
the C11 standard.

And yes, malloc(0) is unportable, but if the result is not used and
directly fed into free that should be no problem, but a warning would
be also helpful because it is unportable code.  But I hope that it is
not the same as with the false positive array bounds warnings where
the compiler first transforms the source code into something completely
different and then starts to warn about it.

I'm not aware of any such transformation or false positives due to
it but if you have more details or a test case I'll verify that it
doesn't trip it up.

Regarding alloca, there are probably three different forms.

First a function that is not a builtin but has the name "alloca"
like special_function_p understands it.  It has no attributes
unless the header mentions them.  Calling this function with
0 should not be warned about, right?

Right.  (I assume this means -fno-builtin-alloca or its equivalent
such as -ffreestanding; otherwise GCC mostly treats "alloca" the
same as "__builtin_alloca", with the exception of special_function_p).


Then a function that has the name "alloca" and matches the signature
of the builtin "alloca" function.

Right.  (This means that -fbuiltin-alloca is set (i.e., without
an explicit -fno-builtin-alloca, -fno-builtin, or -ffreestanding).


And last a function that has the name "__builtin_alloca".

Right.  (Either a direct call to it or as a result of macro
expansion like in Glibc <alloca.h>.)


You can distinguish between these, and possibly only warn for
__builtin_alloca?  Note, that will depend on the way the glibc
header works.

Yes, it's possible to distinguish these cases.  The last patch
I posted doesn't warn on case (1) when -fno-builtin-alloca is
set because then the function code isn't BUILT_IN_ALLOCA.  It
does warn on case (2) because it only looks at the function
code.  I offered to make it not warn on case (2) to help avoid
changing calls to it from alloca(n) to alloca(n + !n) when n is
determined to be zero by constant propagation.

Everything would be more easy if the glibc header would not do
that, and just use the alloca and no macro.  Then it would be
more natural to warn about alloca and not about __builtin_alloca,
because that is always implemented in a sensible way.

But even if the __builtin_alloca is called with 0, what do we
do with the result?  I mean any use of the value would be wrong.
So why is there a warning, when the value is not used?

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.

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.


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.

Martin


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]