On 04/12/2016 09:43 AM, Jason Merrill wrote:
On 04/10/2016 07:14 PM, Martin Sebor wrote:
+ if (TREE_CODE (type) == ARRAY_TYPE
+ && variably_modified_type_p (type, NULL_TREE)
+ && !processing_template_decl)
+ {
+ /* Statically check for overflow in VLA bounds and build
+ an expression that checks at runtime whether the VLA
+ is erroneous due to invalid (runtime) bounds.
+ Another expression to check for excess initializers
+ is built in build_vec_init. */
Why do this both in check_initializer and then again in cp_finish_decl
right after the call to check_initializer?
I would have liked to make just one call but I couldn't find
a good way to do it.
The call to build_vla_check() in check_initializer() is to check
only explicitly initialized VLAs. The latter function may need
to complete the VLA element type and reshape the initializer so
the call cannot be made earlier. The function returns the runtime
initializer code so the call cannot be made after it returns.
The call to build_vla_check() in cp_finish_decl() is guarded with
!init and made to check VLAs that are not explicitly initialized.
This call could perhaps be moved into check_initializer() though
it doesn't seem like it logically belongs there (since there is
no initializer). If it were moved there, it seems to me that it
would require more extensive changes to the function than making
it in cp_finish_decl().
+ /* Also check to see if the final array size is zero (the size
+ is unsigned so the earlier overflow check detects negative
+ values as well. */
+ tree zerocheck = fold_build2 (EQ_EXPR, boolean_type_node,
+ vlasize, size_zero_node);
I'm not sure whether we want this, given that GCC allows zero-length
arrays in general. As I recall, with the C++14 stuff whether we checked
for zero was dependent on flag_iso, though I wasn't particularly happy
with that. If you want to check this unconditionally that's fine.
The (sizeof A / sizeof *A) expression is commonly used to compute
the number of elements in an array. When A is a two-dimensional
VLA with a runtime minor bound of zero, the division is undefined
and usually traps at runtime. (This is also true for ordinary
arrays but there GCC warns about it with -Wdiv-by-zero.) Since
we're already doing the runtime checking for VLAs it costs almost
nothing to detect and prevent it and results in arguably safer
code. So if it's okay with you, my preference is to keep it
without providing a way to disable it.
Martin
