Continuing with my testing of VLAs in G++ (see bug 69516, bug 69496, and bug 69509), I discovered another problem. When compiled with GCC 4.9.3, the program below aborts with the following output: terminate called after throwing an instance of 'std::bad_array_length' what(): std::bad_array_length Aborted (core dumped) However, when compiled with 5.x or 6.0, it crashes with a SEGV: $ (cat t.c && ulimit -t 10 && ~/bin/gcc-5.1.0/bin/g++ -Wall -Wextra -Wpedantic -std=c++14 -xc++ t.c) && ./a.out int foo (int n) { int a[n] = { 1, 2, 3, 4, 5, 6 }; int z = 0; for (unsigned i = 0; i < 3; ++i) z += a[i]; return z; } int main () { int n = foo (3); __builtin_printf ("%d\n", n); } t.c: In function ‘int foo(int)’: t.c:3:13: warning: ISO C++ forbids variable length array ‘a’ [-Wvla] int a[n] = { 1, 2, 3, 4, 5, 6 }; ^ Segmentation fault (core dumped)
In 4.9.3 we implemented the proposal for arrays of runtime bound that was pulled out of C++14, and so it was pulled out of the compiler as well. So now excess initializers are undefined behavior. I don't think this is a bug.
Well, we shouldn't ICE here but reject the program instead?
Just to clarify: it's the program that crashes, not GCC (so removing the ice-on-invalid-code keyword). But I also think that rejecting or at least loudly diagnosing the code would be preferable to letting it run off the rails. We have tentatively agreed on this approach in a separate thread (https://gcc.gnu.org/ml/gcc-patches/2016-01/msg02167.html) so someone just needs to put together a patch. I'll see if I can find the time to do it if no one beats me to it. With that said, I wonder if restoring the exception that 4.9.3 would be feasible. It seems like the ideal solution, in line with the array new expression. Jason. were there problems with it that the exception throwing code had to be removed?
(In reply to Martin Sebor from comment #3) > Just to clarify: it's the program that crashes, not GCC (so removing the > ice-on-invalid-code keyword). > > But I also think that rejecting or at least loudly diagnosing the code would > be preferable to letting it run off the rails. We have tentatively agreed > on this approach in a separate thread > (https://gcc.gnu.org/ml/gcc-patches/2016-01/msg02167.html) so someone just > needs to put together a patch. I'll see if I can find the time to do it if > no one beats me to it. Thanks. > With that said, I wonder if restoring the exception that 4.9.3 would be > feasible. It seems like the ideal solution, in line with the array new > expression. Jason. were there problems with it that the exception throwing > code had to be removed? The only issue was that the exception class was removed from the working paper, so it isn't part of C++14. I'm open to restoring the throwing code, but we should probably use a different exception type, either bad_array_new_length or an extension.
By the way, it was removed in r219359.
Martin is planning some changes, thus confirmed.
I'll look into this.
Patch posted for review: https://gcc.gnu.org/ml/gcc-patches/2016-03/msg00441.html
Author: msebor Date: Wed Apr 13 23:26:41 2016 New Revision: 234966 URL: https://gcc.gnu.org/viewcvs?rev=234966&root=gcc&view=rev Log: PR c++/69517 - [5/6 regression] SEGV on a VLA with excess initializer elements PR c++/70019 - VLA size overflow not detected PR c++/70588 - SIGBUS on a VLA larger than SIZE_MAX / 2 gcc/testsuite/ChangeLog: 2016-04-13 Martin Sebor <msebor@redhat.com> PR c++/69517 PR c++/70019 PR c++/70588 * c-c++-common/ubsan/vla-1.c (main): Catch exceptions. * g++.dg/cpp1y/vla11.C: New test. * g++.dg/cpp1y/vla12.C: New test. * g++.dg/cpp1y/vla13.C: New test. * g++.dg/cpp1y/vla14.C: New test. * g++.dg/cpp1y/vla3.C: Restore deleted test. * gcc/testsuite/g++.dg/init/array24.C: Fully brace VLA initializer. * g++.dg/ubsan/vla-1.C: Disable exceptions. gcc/cp/ChangeLog: 2016-04-13 Martin Sebor <msebor@redhat.com> PR c++/69517 PR c++/70019 PR c++/70588 * cp-tree.h (throw_bad_array_length, build_vla_check): Declare new functions. * decl.c (check_initializer, cp_finish_decl): Call them. (reshape_init_r): Reject incompletely braced intializer-lists for VLAs. * init.c (throw_bad_array_length, build_vla_check) (build_vla_size_check, build_vla_init_check): Define new functions. * typeck2.c (split_nonconstant_init_1): Use variably_modified_type_p() to detect a VLA. (store_init_value): Same. gcc/doc/ChangeLog: 2016-04-13 Martin Sebor <msebor@redhat.com> PR c++/69517 PR c++/70019 PR c++/70588 * extend.texi (Variable Length): Document C++ specifics. libstdc++-v3/ChangeLog: 2016-04-13 Martin Sebor <msebor@redhat.com> PR c++/69517 * testsuite/25_algorithms/rotate/moveable2.cc: Make sure VLA upper bound is positive. Added: trunk/gcc/testsuite/g++.dg/cpp1y/vla11.C trunk/gcc/testsuite/g++.dg/cpp1y/vla12.C trunk/gcc/testsuite/g++.dg/cpp1y/vla13.C trunk/gcc/testsuite/g++.dg/cpp1y/vla14.C trunk/gcc/testsuite/g++.dg/cpp1y/vla3.C Modified: trunk/gcc/ChangeLog trunk/gcc/cp/ChangeLog trunk/gcc/cp/cp-tree.h trunk/gcc/cp/decl.c trunk/gcc/cp/init.c trunk/gcc/cp/typeck2.c trunk/gcc/doc/extend.texi trunk/gcc/testsuite/ChangeLog trunk/gcc/testsuite/c-c++-common/ubsan/vla-1.c trunk/gcc/testsuite/g++.dg/init/array24.C trunk/gcc/testsuite/g++.dg/ubsan/vla-1.C trunk/libstdc++-v3/ChangeLog trunk/libstdc++-v3/testsuite/25_algorithms/rotate/moveable2.cc
Fixed in 6.0 by r234966.
Restoring 6 regression since the fix was reverted due to bug 70652.
Retargetting at GCC 7.
Author: jakub Date: Fri Apr 15 12:29:32 2016 New Revision: 235021 URL: https://gcc.gnu.org/viewcvs?rev=235021&root=gcc&view=rev Log: PR c++/69517 PR c++/70019 PR c++/70588 * g++.dg/cpp1y/vla11.C: Revert for real. Removed: trunk/gcc/testsuite/g++.dg/cpp1y/vla11.C Modified: trunk/gcc/testsuite/ChangeLog
I don't see the segv anymore with current trunk.
The test case from comment #0 doesn't crash for me either but one that initializes the VLA with more than 6 elements, say like so, does: int a[n] = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16 }; With both test cases the tree dump shows that GCC writes past the end of the array.
I don't think this is a regression (it is not a regression compared to GCC versions that didn't implement the C++ feature that got removed from the C++ standard), nor a bug, there is no ICE, the program invokes undefined behavior, so anything including SIGSEGV happens.
(In reply to Jakub Jelinek from comment #16) The bug here is in G++ accepting a VLA initializer with more elements than there is room for in the VLA, and then trashing the stack at runtime with the extra elements. It is a regression with respect to GCC 4.9.3 which implements C++ VLAs as specified in n3639 (http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2013/n3639.html). This is documented in GCC 4.9 changes (https://gcc.gnu.org/gcc-4.9/changes.html) which highlights the feature using the following example: void f(int n) { int a[n] = { 1, 2, 3 }; // throws std::bad_array_length if n < 3 ... VLAs were subsequently removed from C++, and also partially (but not completely) removed from G++, which causes C++ programs developed and tested with G++ 4.9 to break when ported to a later version. C++ VLAs will be safer to use with the patch referenced in comment #9. It patch had to be reverted from GCC 6.0 because it caused problems in Java. Java has been removed and I plan/hope to resubmit the patch for GCC 8. (I wanted to do it for GCC 7 but didn't get to it.)
(In reply to Martin Sebor from comment #17) > (In reply to Jakub Jelinek from comment #16) > > The bug here is in G++ accepting a VLA initializer with more elements than > there is room for in the VLA, and then trashing the stack at runtime with > the extra elements. It is a regression with respect to GCC 4.9.3 which > implements C++ VLAs as specified in n3639 > (http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2013/n3639.html). This > is documented in GCC 4.9 changes (https://gcc.gnu.org/gcc-4.9/changes.html) > which highlights the feature using the following example: > > void f(int n) { > int a[n] = { 1, 2, 3 }; // throws std::bad_array_length if n < 3 > ... > > VLAs were subsequently removed from C++, and also partially (but not > completely) removed from G++, which causes C++ programs developed and tested > with G++ 4.9 to break when ported to a later version. > > C++ VLAs will be safer to use with the patch referenced in comment #9. It > patch had to be reverted from GCC 6.0 because it caused problems in Java. > Java has been removed and I plan/hope to resubmit the patch for GCC 8. (I > wanted to do it for GCC 7 but didn't get to it.) I don't see why it would be a bug. There is no standard covering VLAs in C++, it is all extensions, it is defined however we want. UB when storing something larger into something smaller is perfectly fine, users can put their own checks if they want to avoid it. You want to slow all the code down by doing the checks mandatory.
I'm no longer working on this.