Done. -Walloca and -Wvla warn on any use of alloca and VLAs
accordingly, with or without optimization. I sorry() on the bounded cases.
I think it's an improvement though I suspect we each have a slightly
different understanding of what the sorry message is meant to be used
for. It's documented in the Diagnostics Conventions section of the
GCC Coding Conventions as:
sorry is for correct user input programs but unimplemented
functionalities.
I take that to mean that it should be used for what is considered
valid user input that cannot be processed because the functionality
is not yet implemented (but eventually will be). So unless this
case falls into this category I would expect GCC to issue a warning
saying that the options have no effect (or limited effect, whatever
the case may be) without optimization. But maybe I'm not reading
the coding conventions text right.
2) When passed an argument of a signed type, GCC prints
warning: cast from signed type in alloca
even though there is no explicit cast in the code. It may not
be obvious why the conversion is a problem in this context. I
would suggest to rephrase the warning along the lines of
-Wsign-conversion which prints:
conversion to ‘long unsigned int’ from ‘int’ may change the sign of
the result
and add why it's a potential problem. Perhaps something like:
argument to alloca may be too large due to conversion from
'int to 'long unsigned int'
Fixed:
Cool.
FWIW, by coincidence I was just educated about the subtle nuances
of quoting in GCC messages in a discussion with David and Manu.
Types, functions, variables, and literals that appear in the source
code should be referenced in diagnostics by using the "%qT", "%qD",
and "%qE" directives so that GCC can add the right quotes and
highlighting. Enclosing "'%T'" in quotes will not use the same
kind of quotes as "%qT" and won't highlight the type name.
https://gcc.gnu.org/wiki/DiagnosticsGuidelines
There is a "documented" reason for this: :)
// Do not warn on VLAs occurring in a loop, since VLAs are
// guaranteed to be cleaned up when they go out of scope.
// That is, there is a corresponding __builtin_stack_restore
// at the end of the scope in which the VLA occurs.
Yes, I understand that VLAs in loops are treated differently than
alloca. But I don't think this is quite how the logic should work.
I.e., an excessively large VLA should be diagnosed regardless of
whether it's in a loop or outside. Consider the following case
where with the patch as is, the warning is issued only for one
of the two functions, even though they both allocate a VLA in
excess of the threshold.
#define FOO(n) if (1) { \
char a [n]; \
f (a); \
} else (void)0
#define BAR(n) do { \
char a [n]; \
f (a); \
} while (0)
void f (void*);
void foo (void)
{
int n = 8000;
FOO (n); // warning with -Wla-larger-than=4000
}
void bar (void)
{
int n = 8000;
BAR (n); // no warning
}
5) The -Wvla=N logic only seems to take into consideration the number
of elements but not the size of the element type. For example, I wasn't
able to get it to warn on the following with -Wvla=255 or greater:
void f0 (void*);
void f1 (unsigned char a)
{
int x [a]; // or even char a [n][__INT_MAX__];
f0 (x);
}
That was a huge oversight (or should I say over-engineering) on my part.
Fixed.
Looks good.
I did notice one minor glitch, though not one caused by your patch.
GCC apparently transforms simple VLAs that are 256 bytes or less
in size into ordinary arrays (i.e., it doesn't call
__builtin_alloca_with_align). Because of that, specifying
-Wvla-larger-than=N with N less than 256 may not give a warning,
as in the example below. I suspect there may not be anything
the Walloca pass can do about this so perhaps just mentioning
it in the manual might be enough to avoid bug reports about false
negatives.
void f0 (void*);
unsigned f1 (void) { return 256; }
void f2 (void)
{
unsigned n = f1 ();
char a [n];
f0 (a);
}
GCC doesn't do the same transformation for alloca calls so the
-Walloca-larger-than warning doesn't have this quirk.
This is a problem with the generic machinery so I would prefer someone
file a bugzilla report :), as this affects other options.
I raised bug 71905 for this.
Ughhh...you're making me write user friendly stuff. The reason I got
into compilers was so I wouldn't have to deal with the user :).
if (n < 2000)
{
p = __builtin_alloca (n);
f (p);
}
./cc1 a.c -fdump-tree-all-vops-alias-details-graph -quiet -I/tmp
-Walloca-larger-than=100 -O2
a.c: In function ‘g2’:
a.c:9:7: warning: argument to alloca may be too large
[-Walloca-larger-than=]
p = __builtin_alloca (n);
~~^~~~~~~~~~~~~~~~~~~~~~
a.c:9:7: note: limit is '100' bytes, but argument may be '1999'
Happy? :-)
I along with untold numbers of users thank you! :)
(As another quoting nit, unlike functions, variables, and constants
that appear in the source code, ordinary numbers apparently aren't
supposed to be quoted in GCC messages. (I'm guilty of making this
mistake in at least two of my own patches and will be correcting
it.)
Martin
