On 06/18/2016 07:55 PM, Martin Sebor wrote:
On 06/16/2016 02:32 AM, Aldy Hernandez wrote:
p.s. The pass currently warns on all uses of VLAs. I'm not completely
sold on this idea, so perhaps we could remove it, or gate it with a
flag.
I also think that VLA diagnostics would be better controlled
by a separate option, and emit a different diagnostic (one
that mentions VLA rather than alloca). Although again, and
for VLAs even more so than for alloca, providing an option
to have GCC use dynamic allocation, would be an even more
robust solution than issuing warnings. IIRC, this was the
early implementation of VLAs in GCC so there is a precedent
for it. (Though this seems complementary to the warnings.)
In addition, I'm of the opinion that potentially unbounded
VLA allocation should be checked at runtime and made trap on
size overflow in C and throw an exception in C++ (e.g., when
int a [A][B] when A * B * sizeof (int) exceeds SIZE_MAX / 2
or some runtime-configurable limit). My C++ patch for bug
69517 does just that (it needs to be resubmitted with the
runtime configuration limit added).
For a choice of VLA warning options, there already is -Wvla,
though it simply points out every VLA definition regardless
of its size. It also issues a diagnostic that's questionable
in C99 and later modes (that "ISO C90 forbids variable length
array" is only relevant in C90; what matters when -Wvla is
specified in C99 and C11 modes is that a VLA has been seen).
To keep things consistent with -Walloca, perhaps -Wvla could
be extended to take an optional argument to specify the VLA
threshold. (So that -Wvla=4096 would only diagnose VLA
definitions of 4k bytes or more, or unknown size.)
Hmmm...I kinda wanted a sane default for -Walloca, and keeping -Walloc
and -Wvla consistent would mean I don't get that.
Currently, -Walloca would mean "warn on unbounded uses of alloca where n
> DEFAULT", whereas -Walloca=0 would mean "warn on all uses of alloca".
The problem is that -Wvla means "warn on ALL uses".
One consistent option would be to change the definition to:
-Walloca: warn on all uses of alloca (strict mode).
-Walloca=N: warn on unbounded uses of alloca or bounded uses of n > N.
-Wvla: warn on all uses of VLAs (as currently implemented).
-Wvla=N: warn on unbounded uses of VLA or bounded uses of n > N.
This means we get no defaults, and the user must make the limit
explicit, but at least we're consistent with the current use of -Wvla.
How about we just use -Walloca (and no separate variants for -Wvla=??),
but we document that -Walloca will also flag VLAs (and explain why).
Also, we make sure the error messages say "variable-length array" not
"alloca" in the VLA case. This way we can have a default, and perhaps a
more consistent flag:
-Walloca: warn on all unbounded uses of alloca/vlas and bounded uses
where n > DEFAULT.
-Walloca=0: warn on all uses of alloca/vlas (strict mode).
-Walloca=N: warn on all unbounded uses of alloca/vlas and bounded uses
where n > N.
-Wla: untouched; as is currently implemented.
Would this be acceptable, or are y'all really against one -Walloca flag
to rule it all?