* [PATCH, C++] Fix PR c++/88261 @ 2018-12-15 8:36 Bernd Edlinger 2018-12-15 9:33 ` Jakub Jelinek 2018-12-19 3:35 ` Jason Merrill 0 siblings, 2 replies; 16+ messages in thread From: Bernd Edlinger @ 2018-12-15 8:36 UTC (permalink / raw) To: gcc-patches, Jeff Law, Jason Merrill, Nathan Sidwell [-- Attachment #1: Type: text/plain, Size: 531 bytes --] Hi, this patch implements an error message, for non-static initialization of a flexible array member. This duplicates the existing error message from the C-FE, to avoid ICE and wrong code generation issues, as pointed out in the PR. It is a bit funny that a non-functional feature like that has already rather much test coverage. The most easy adjustment seems to change the existing test cases to use static declarations. Bootstrapped and reg-tested on x86_64-pc-linux-gnu. Is it OK for trunk? Thanks Bernd. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: patch-pr88261.diff --] [-- Type: text/x-patch; name="patch-pr88261.diff", Size: 19653 bytes --] gcc/cp: 2018-12-15 Bernd Edlinger <bernd.edlinger@hotmail.de> PR c++/88261 * typeck2.c (digest_init_r): Add a decl parameter. Raise an error for non-static initialization of a flexible array member. (process_init_constructor, digest_init_flags, massage_init_elt, process_init_constructor_array, process_init_constructor_record, process_init_constructor_union, process_init_constructor): Add a decl parameter and pass it thru. (digest_nsdmi_init): Pass decl parameter to digest_init_flags. (digest_init): Pass NULL as decl parameter to digest_init_r. * semantics.c (finish_compound_literal): Likewise. * cp-tree.h (digest_init_flags): Adjust prototype. gcc/testsuite: 2018-12-15 Bernd Edlinger <bernd.edlinger@hotmail.de> PR c++/88261 * gcc.dg/array-6.c: Move to ... * c-c++-common/array-6.c: ... here. * g++.dg/ext/flexary3.C: Adjust test. * g++.dg/ext/flexary12.C: Likewise. * g++.dg/ext/flexary13.C: Likewise. * g++.dg/ext/flexary15.C: Likewise. * g++.dg/warn/Wplacement-new-size-1.C: Likewise. * g++.dg/warn/Wplacement-new-size-2.C: Likewise. * g++.dg/warn/Wplacement-new-size-6.C: Likewise. Index: gcc/cp/cp-tree.h =================================================================== --- gcc/cp/cp-tree.h (revision 266971) +++ gcc/cp/cp-tree.h (working copy) @@ -7483,7 +7483,8 @@ extern tree split_nonconstant_init (tree, tree); extern bool check_narrowing (tree, tree, tsubst_flags_t, bool = false); extern tree digest_init (tree, tree, tsubst_flags_t); -extern tree digest_init_flags (tree, tree, int, tsubst_flags_t); +extern tree digest_init_flags (tree, tree, int, + tsubst_flags_t, tree); extern tree digest_nsdmi_init (tree, tree, tsubst_flags_t); extern tree build_scoped_ref (tree, tree, tree *); extern tree build_x_arrow (location_t, tree, Index: gcc/cp/semantics.c =================================================================== --- gcc/cp/semantics.c (revision 266971) +++ gcc/cp/semantics.c (working copy) @@ -2828,7 +2828,7 @@ finish_compound_literal (tree type, tree compound_ return error_mark_node; } compound_literal = digest_init_flags (type, compound_literal, LOOKUP_NORMAL, - complain); + complain, NULL_TREE); if (TREE_CODE (compound_literal) == CONSTRUCTOR) { TREE_HAS_CONSTRUCTOR (compound_literal) = true; Index: gcc/cp/typeck2.c =================================================================== --- gcc/cp/typeck2.c (revision 266971) +++ gcc/cp/typeck2.c (working copy) @@ -36,7 +36,7 @@ along with GCC; see the file COPYING3. If not see static tree process_init_constructor (tree type, tree init, int nested, - tsubst_flags_t complain); + tsubst_flags_t complain, tree decl); /* Print an error message stemming from an attempt to use @@ -813,7 +813,7 @@ store_init_value (tree decl, tree init, vec<tree, value = init; else /* Digest the specified initializer into an expression. */ - value = digest_init_flags (type, init, flags, tf_warning_or_error); + value = digest_init_flags (type, init, flags, tf_warning_or_error, decl); if (TREE_CODE (type) == ARRAY_TYPE && TYPE_STRING_FLAG (TREE_TYPE (type)) @@ -1025,11 +1025,14 @@ check_narrowing (tree type, tree init, tsubst_flag initializer will have the right shape (brace elision has been undone). NESTED is non-zero iff we are being called for an element of a CONSTRUCTOR, - 2 iff the element of a CONSTRUCTOR is inside another CONSTRUCTOR. */ + 2 iff the element of a CONSTRUCTOR is inside another CONSTRUCTOR. + DECL points to the decl which is being initialized. It may be null if + the decl is unknown. In that case, assume it will be non-static. */ + static tree digest_init_r (tree type, tree init, int nested, int flags, - tsubst_flags_t complain) + tsubst_flags_t complain, tree decl) { enum tree_code code = TREE_CODE (type); @@ -1059,8 +1062,18 @@ digest_init_r (tree type, tree init, int nested, i { if (nested && !TYPE_DOMAIN (type)) /* C++ flexible array members have a null domain. */ - pedwarn (loc, OPT_Wpedantic, - "initialization of a flexible array member"); + { + if (decl && TREE_STATIC (decl)) + pedwarn (loc, OPT_Wpedantic, + "initialization of a flexible array member"); + else + { + if (complain & tf_error) + error_at (loc, "non-static initialization of" + " a flexible array member"); + return error_mark_node; + } + } tree typ1 = TYPE_MAIN_VARIANT (TREE_TYPE (type)); if (char_type_p (typ1) @@ -1175,7 +1188,7 @@ digest_init_r (tree type, tree init, int nested, i if (BRACE_ENCLOSED_INITIALIZER_P (init) && !TYPE_NON_AGGREGATE_CLASS (type)) - return process_init_constructor (type, init, nested, complain); + return process_init_constructor (type, init, nested, complain, decl); else { if (COMPOUND_LITERAL_P (init) && code == ARRAY_TYPE) @@ -1214,13 +1227,14 @@ digest_init_r (tree type, tree init, int nested, i tree digest_init (tree type, tree init, tsubst_flags_t complain) { - return digest_init_r (type, init, 0, LOOKUP_IMPLICIT, complain); + return digest_init_r (type, init, 0, LOOKUP_IMPLICIT, complain, NULL_TREE); } tree -digest_init_flags (tree type, tree init, int flags, tsubst_flags_t complain) +digest_init_flags (tree type, tree init, int flags, tsubst_flags_t complain, + tree decl) { - return digest_init_r (type, init, 0, flags, complain); + return digest_init_r (type, init, 0, flags, complain, decl); } /* Process the initializer INIT for an NSDMI DECL (a FIELD_DECL). */ @@ -1236,7 +1250,7 @@ digest_nsdmi_init (tree decl, tree init, tsubst_fl if (BRACE_ENCLOSED_INITIALIZER_P (init) && CP_AGGREGATE_TYPE_P (type)) init = reshape_init (type, init, complain); - init = digest_init_flags (type, init, flags, complain); + init = digest_init_flags (type, init, flags, complain, decl); if (TREE_CODE (init) == TARGET_EXPR) /* This represents the whole initialization. */ TARGET_EXPR_DIRECT_INIT_P (init) = true; @@ -1273,9 +1287,11 @@ picflag_from_initializer (tree init) /* Adjust INIT for going into a CONSTRUCTOR. */ static tree -massage_init_elt (tree type, tree init, int nested, tsubst_flags_t complain) +massage_init_elt (tree type, tree init, int nested, tsubst_flags_t complain, + tree decl) { - init = digest_init_r (type, init, nested ? 2 : 1, LOOKUP_IMPLICIT, complain); + init = digest_init_r (type, init, nested ? 2 : 1, LOOKUP_IMPLICIT, complain, + decl); /* Strip a simple TARGET_EXPR when we know this is an initializer. */ if (SIMPLE_TARGET_EXPR_P (init)) init = TARGET_EXPR_INITIAL (init); @@ -1294,7 +1310,7 @@ static tree static int process_init_constructor_array (tree type, tree init, int nested, - tsubst_flags_t complain) + tsubst_flags_t complain, tree decl) { unsigned HOST_WIDE_INT i, len = 0; int flags = 0; @@ -1347,7 +1363,8 @@ process_init_constructor_array (tree type, tree in ce->index = error_mark_node; gcc_assert (ce->value); ce->value - = massage_init_elt (TREE_TYPE (type), ce->value, nested, complain); + = massage_init_elt (TREE_TYPE (type), ce->value, nested, complain, + decl); gcc_checking_assert (ce->value == error_mark_node @@ -1371,7 +1388,8 @@ process_init_constructor_array (tree type, tree in we can't rely on the back end to do it for us, so make the initialization explicit by list-initializing from T{}. */ next = build_constructor (init_list_type_node, NULL); - next = massage_init_elt (TREE_TYPE (type), next, nested, complain); + next = massage_init_elt (TREE_TYPE (type), next, nested, complain, + decl); if (initializer_zerop (next)) /* The default zero-initialization is fine for us; don't add anything to the CONSTRUCTOR. */ @@ -1417,7 +1435,7 @@ process_init_constructor_array (tree type, tree in static int process_init_constructor_record (tree type, tree init, int nested, - tsubst_flags_t complain) + tsubst_flags_t complain, tree decl) { vec<constructor_elt, va_gc> *v = NULL; tree field; @@ -1499,7 +1517,7 @@ process_init_constructor_record (tree type, tree i if (ce) { gcc_assert (ce->value); - next = massage_init_elt (type, next, nested, complain); + next = massage_init_elt (type, next, nested, complain, decl); ++idx; } } @@ -1528,7 +1546,8 @@ process_init_constructor_record (tree type, tree i for us, so build up TARGET_EXPRs. If the type in question is a class, just build one up; if it's an array, recurse. */ next = build_constructor (init_list_type_node, NULL); - next = massage_init_elt (TREE_TYPE (field), next, nested, complain); + next = massage_init_elt (TREE_TYPE (field), next, nested, complain, + decl); /* Warn when some struct elements are implicitly initialized. */ if ((complain & tf_warning) @@ -1644,7 +1663,7 @@ process_init_constructor_record (tree type, tree i static int process_init_constructor_union (tree type, tree init, int nested, - tsubst_flags_t complain) + tsubst_flags_t complain, tree decl) { constructor_elt *ce; int len; @@ -1731,7 +1750,7 @@ process_init_constructor_union (tree type, tree in if (ce->value && ce->value != error_mark_node) ce->value = massage_init_elt (TREE_TYPE (ce->index), ce->value, nested, - complain); + complain, decl); return picflag_from_initializer (ce->value); } @@ -1752,7 +1771,7 @@ process_init_constructor_union (tree type, tree in static tree process_init_constructor (tree type, tree init, int nested, - tsubst_flags_t complain) + tsubst_flags_t complain, tree decl) { int flags; @@ -1759,11 +1778,11 @@ process_init_constructor (tree type, tree init, in gcc_assert (BRACE_ENCLOSED_INITIALIZER_P (init)); if (TREE_CODE (type) == ARRAY_TYPE || VECTOR_TYPE_P (type)) - flags = process_init_constructor_array (type, init, nested, complain); + flags = process_init_constructor_array (type, init, nested, complain, decl); else if (TREE_CODE (type) == RECORD_TYPE) - flags = process_init_constructor_record (type, init, nested, complain); + flags = process_init_constructor_record (type, init, nested, complain, decl); else if (TREE_CODE (type) == UNION_TYPE) - flags = process_init_constructor_union (type, init, nested, complain); + flags = process_init_constructor_union (type, init, nested, complain, decl); else gcc_unreachable (); Index: gcc/testsuite/g++.dg/ext/flexary12.C =================================================================== --- gcc/testsuite/g++.dg/ext/flexary12.C (revision 266971) +++ gcc/testsuite/g++.dg/ext/flexary12.C (working copy) @@ -12,7 +12,7 @@ struct A { void f1 () { // This is the meat of the test from c++/69290: - struct A a + static struct A a = { "c" }; // { dg-error "invalid conversion from .const char\\*. to .int." } (void)&a; @@ -27,13 +27,13 @@ struct B { void f2 () { - struct B b1 + static struct B b1 = { 0, "c" }; // { dg-error "invalid conversion from .const char\\*. to .int." } (void)&b1; const char s[] = "c"; - struct B b2 + static struct B b2 = { 0, s }; // { dg-error "invalid conversion from .const char\\*. to .int." } (void)&b2; @@ -57,7 +57,7 @@ struct C { void f3 () { - struct C<double> cd + static struct C<double> cd = { "c" }; // { dg-error "cannot convert .const char\\*. to .double." } (void)&cd; Index: gcc/testsuite/g++.dg/ext/flexary13.C =================================================================== --- gcc/testsuite/g++.dg/ext/flexary13.C (revision 266971) +++ gcc/testsuite/g++.dg/ext/flexary13.C (working copy) @@ -19,33 +19,33 @@ int main () ASSERT (s.n == 0); } { - Ax s = + static Ax s = { 0, { } }; // dg-warning "initialization of a flexible array member" } ASSERT (s.n == 0); } { - Ax s = + static Ax s = { 1, { 2 } }; // dg-warning "initialization of a flexible array member" } ASSERT (s.n == 1 && s.a [0] == 2); } { - Ax s = + static Ax s = { 2, { 3, 4 } }; // dg-warning "initialization of a flexible array member" } ASSERT (s.n = 2 && s.a [0] == 3 && s.a [1] == 4); } { - Ax s = + static Ax s = { 123, i }; // dg-warning "initialization of a flexible array member" } ASSERT (s.n == 123 && s.a [0] == i); } { - Ax s = + static Ax s = { 456, { i } }; // dg-warning "initialization of a flexible array member" } ASSERT (s.n == 456 && s.a [0] == i); } { int j = i + 1, k = j + 1; - Ax s = + static Ax s = { 3, { i, j, k } }; // dg-warning "initialization of a flexible array member" } ASSERT (s.n == 3 && s.a [0] == i && s.a [1] == j && s.a [2] == k); } Index: gcc/testsuite/g++.dg/ext/flexary15.C =================================================================== --- gcc/testsuite/g++.dg/ext/flexary15.C (revision 266971) +++ gcc/testsuite/g++.dg/ext/flexary15.C (working copy) @@ -10,5 +10,5 @@ struct S { void foo (const char *a) { - const S s = { 1, { a, "b" } }; // { dg-warning "invalid conversion" } + static const S s = { 1, { a, "b" } }; // { dg-warning "invalid conversion" } } Index: gcc/testsuite/g++.dg/ext/flexary3.C =================================================================== --- gcc/testsuite/g++.dg/ext/flexary3.C (revision 266971) +++ gcc/testsuite/g++.dg/ext/flexary3.C (working copy) @@ -17,5 +17,6 @@ struct s { int main() { struct s s = { .c = 0 }; // { dg-error "initializer" } + // { dg-error "non-static initialization of a flexible array member" "" { target *-*-* } .-1 } return 0; } Index: gcc/testsuite/g++.dg/warn/Wplacement-new-size-1.C =================================================================== --- gcc/testsuite/g++.dg/warn/Wplacement-new-size-1.C (revision 266971) +++ gcc/testsuite/g++.dg/warn/Wplacement-new-size-1.C (working copy) @@ -28,7 +28,7 @@ void fAx (Ax *px, Ax &rx) void fAx2 () { - Ax ax2 = { 1, { 2, 3 } }; + static Ax ax2 = { 1, { 2, 3 } }; new (ax2.a) Int16; new (ax2.a) Int32; // { dg-warning "placement" } @@ -82,7 +82,7 @@ void fBx (BAx *pbx, BAx &rbx) void fBx1 () { - BAx bax1 = { 1, /* Ax = */ { 2, /* a[] = */ {} } }; + static BAx bax1 = { 1, /* Ax = */ { 2, /* a[] = */ {} } }; new (bax1.ax.a) char; // { dg-warning "placement" } new (bax1.ax.a) char[2]; // { dg-warning "placement" } Index: gcc/testsuite/g++.dg/warn/Wplacement-new-size-2.C =================================================================== --- gcc/testsuite/g++.dg/warn/Wplacement-new-size-2.C (revision 266971) +++ gcc/testsuite/g++.dg/warn/Wplacement-new-size-2.C (working copy) @@ -36,10 +36,10 @@ void fAx2 () // isn't allowed in C and should perhaps be disallowed in C++ as // well to avoid c++/69696 - incorrect initialization of block-scope // flexible array members. - Ax ax2 = { 1, { 2, 3 } }; + Ax ax2 = { 1, { 2, 3 } }; // { dg-error "non-static initialization of a flexible array member" } - new (ax2.a) Int16; - new (ax2.a) Int16[1]; + new (ax2.a) Int16; // { dg-warning "placement" } + new (ax2.a) Int16[1]; // { dg-warning "placement" } new (ax2.a) Int16[2]; // { dg-warning "placement" } new (ax2.a) Int32; // { dg-warning "placement" } new (ax2.a) Int32[2]; // { dg-warning "placement" } @@ -140,7 +140,7 @@ void fBx (BAx *pbx, BAx &rbx) void fBx1 () { - BAx bax1 = { 1, /* Ax = */ { 2, /* a[] = */ {} } }; + static BAx bax1 = { 1, /* Ax = */ { 2, /* a[] = */ {} } }; new (bax1.ax.a) char; // { dg-warning "placement" } new (bax1.ax.a) char[2]; // { dg-warning "placement" } Index: gcc/testsuite/g++.dg/warn/Wplacement-new-size-6.C =================================================================== --- gcc/testsuite/g++.dg/warn/Wplacement-new-size-6.C (revision 266971) +++ gcc/testsuite/g++.dg/warn/Wplacement-new-size-6.C (working copy) @@ -15,7 +15,7 @@ struct BAx { int i; Ax ax; }; void fBx1 () { - BAx bax1 = { 1, /* Ax = */ { 2, /* a[] = */ { 3 } } }; // { dg-error "initialization of flexible array member in a nested context" } + static BAx bax1 = { 1, /* Ax = */ { 2, /* a[] = */ { 3 } } }; // { dg-error "initialization of flexible array member in a nested context" } new (bax1.ax.a) char; // { dg-warning "placement" } new (bax1.ax.a) char[2]; // { dg-warning "placement" } @@ -25,7 +25,7 @@ void fBx1 () void fBx2 () { - BAx bax2 = { 1, /* Ax = */ { 2, /* a[] = */ { 3, 4 } } }; // { dg-error "initialization of flexible array member in a nested context" } + static BAx bax2 = { 1, /* Ax = */ { 2, /* a[] = */ { 3, 4 } } }; // { dg-error "initialization of flexible array member in a nested context" } new (bax2.ax.a) char; // { dg-warning "placement" } new (bax2.ax.a) char[2]; // { dg-warning "placement" } @@ -37,7 +37,7 @@ void fBx2 () void fBx3 () { - BAx bax2 = { 1, /* Ax = */ { 3, /* a[] = */ { 4, 5, 6 } } }; // { dg-error "initialization of flexible array member in a nested context" } + static BAx bax2 = { 1, /* Ax = */ { 3, /* a[] = */ { 4, 5, 6 } } }; // { dg-error "initialization of flexible array member in a nested context" } new (bax2.ax.a) char; // { dg-warning "placement" } new (bax2.ax.a) char[2]; // { dg-warning "placement" } Index: gcc/testsuite/gcc.dg/array-6.c =================================================================== --- gcc/testsuite/gcc.dg/array-6.c (revision 266971) +++ gcc/testsuite/gcc.dg/array-6.c (working copy) @@ -1,18 +0,0 @@ -/* PR c/5597 */ -/* { dg-do compile } */ -/* { dg-options "" } */ - -/* Verify that GCC forbids non-static initialization of - flexible array members. */ - -struct str { int len; char s[]; }; - -struct str a = { 2, "a" }; - -void foo() -{ - static struct str b = { 2, "b" }; - struct str c = { 2, "c" }; /* { dg-error "(non-static)|(near initialization)" } */ - struct str d = (struct str) { 2, "d" }; /* { dg-error "(non-static)|(near initialization)" } */ - struct str e = (struct str) { d.len, "e" }; /* { dg-error "(non-static)|(initialization)" } */ -} Index: gcc/testsuite/c-c++-common/array-6.c =================================================================== --- gcc/testsuite/c-c++-common/array-6.c (revision 0) +++ gcc/testsuite/c-c++-common/array-6.c (working copy) @@ -0,0 +1,18 @@ +/* PR c/5597 */ +/* { dg-do compile } */ +/* { dg-options "" } */ + +/* Verify that GCC forbids non-static initialization of + flexible array members. */ + +struct str { int len; char s[]; }; + +struct str a = { 2, "a" }; + +void foo() +{ + static struct str b = { 2, "b" }; + struct str c = { 2, "c" }; /* { dg-error "(non-static)|(near initialization)" } */ + struct str d = (struct str) { 2, "d" }; /* { dg-error "(non-static)|(near initialization)" } */ + struct str e = (struct str) { d.len, "e" }; /* { dg-error "(non-static)|(initialization)" } */ +} ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH, C++] Fix PR c++/88261 2018-12-15 8:36 [PATCH, C++] Fix PR c++/88261 Bernd Edlinger @ 2018-12-15 9:33 ` Jakub Jelinek 2018-12-15 10:36 ` Bernd Edlinger 2018-12-19 3:35 ` Jason Merrill 1 sibling, 1 reply; 16+ messages in thread From: Jakub Jelinek @ 2018-12-15 9:33 UTC (permalink / raw) To: Bernd Edlinger; +Cc: gcc-patches, Jeff Law, Jason Merrill, Nathan Sidwell On Sat, Dec 15, 2018 at 08:36:25AM +0000, Bernd Edlinger wrote: > this patch implements an error message, for non-static initialization of a flexible array member. > This duplicates the existing error message from the C-FE, to avoid ICE and wrong code generation > issues, as pointed out in the PR. > > It is a bit funny that a non-functional feature like that has already rather much test coverage. > The most easy adjustment seems to change the existing test cases to use static declarations. > > > Bootstrapped and reg-tested on x86_64-pc-linux-gnu. > Is it OK for trunk? Will defer to Jason or others, just a few nits. Conceptually I'm all for making C and C++ behave the same here and it seems the C behavior has been the same for years (3.2 behaves the same as current trunk). > gcc/cp: > 2018-12-15 Bernd Edlinger <bernd.edlinger@hotmail.de> > > PR c++/88261 > * typeck2.c (digest_init_r): Add a decl parameter. Raise an error > for non-static initialization of a flexible array member. > (process_init_constructor, digest_init_flags, > massage_init_elt, process_init_constructor_array, > process_init_constructor_record, process_init_constructor_union, > process_init_constructor): Add a decl parameter and pass it thru. > (digest_nsdmi_init): Pass decl parameter to digest_init_flags. > (digest_init): Pass NULL as decl parameter to digest_init_r. > * semantics.c (finish_compound_literal): Likewise. > * cp-tree.h (digest_init_flags): Adjust prototype. 8 spaces instead of tab. > gcc/testsuite: > 2018-12-15 Bernd Edlinger <bernd.edlinger@hotmail.de> > > PR c++/88261 > * gcc.dg/array-6.c: Move to ... > * c-c++-common/array-6.c: ... here. Would be better to have more complete test coverage. In particular, the C FE passes on the following extension of array-6.c, note "str" or {} is rejected, but if there is no initializer at all, it is accepted. And, if it isn't already covered in the testsuite, would be good to verify also how it behaves in new, initializer list and any other kind of C++ style initializer, etc. (check for the errors you've added). So void bar() { str x { 2, "a" }; } etc. /* PR c/5597 */ /* { dg-do compile } */ /* { dg-options "" } */ /* Verify that GCC forbids non-static initialization of flexible array members. */ struct str { int len; char s[]; }; struct str a = { 2, "a" }; void foo() { static struct str b = { 2, "b" }; struct str c = { 2, "c" }; /* { dg-error "(non-static)|(near initialization)" } */ struct str d = (struct str) { 2, "d" }; /* { dg-error "(non-static)|(near initialization)" } */ struct str e = (struct str) { d.len, "e" }; /* { dg-error "(non-static)|(initialization)" } */ } struct str f = { 0, {} }; void bar() { static struct str g = { 0, {} }; struct str h = { 0, {} }; /* { dg-error "(non-static)|(near initialization)" } */ struct str i = (struct str) { 0, {} }; /* { dg-error "(non-static)|(near initialization)" } */ struct str j = (struct str) { i.len, {} }; /* { dg-error "(non-static)|(initialization)" } */ } struct str k = { 0 }; void baz() { static struct str l = { 0 }; struct str m = { 0 }; struct str n = (struct str) { 0 }; struct str o = (struct str) { n.len }; } struct str p = {}; void qux() { static struct str q = {}; struct str r = {}; struct str s = (struct str) {}; } Jakub ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH, C++] Fix PR c++/88261 2018-12-15 9:33 ` Jakub Jelinek @ 2018-12-15 10:36 ` Bernd Edlinger 0 siblings, 0 replies; 16+ messages in thread From: Bernd Edlinger @ 2018-12-15 10:36 UTC (permalink / raw) To: Jakub Jelinek; +Cc: gcc-patches, Jeff Law, Jason Merrill, Nathan Sidwell [-- Attachment #1: Type: text/plain, Size: 3782 bytes --] Thanks Jakub, for the additional test cases, they work just fine, so I added them as-is. Bernd. On 12/15/18 10:33 AM, Jakub Jelinek wrote: > On Sat, Dec 15, 2018 at 08:36:25AM +0000, Bernd Edlinger wrote: >> this patch implements an error message, for non-static initialization of a flexible array member. >> This duplicates the existing error message from the C-FE, to avoid ICE and wrong code generation >> issues, as pointed out in the PR. >> >> It is a bit funny that a non-functional feature like that has already rather much test coverage. >> The most easy adjustment seems to change the existing test cases to use static declarations. >> >> >> Bootstrapped and reg-tested on x86_64-pc-linux-gnu. >> Is it OK for trunk? > > Will defer to Jason or others, just a few nits. Conceptually I'm all for > making C and C++ behave the same here and it seems the C behavior has been > the same for years (3.2 behaves the same as current trunk). > >> gcc/cp: >> 2018-12-15 Bernd Edlinger <bernd.edlinger@hotmail.de> >> >> PR c++/88261 >> * typeck2.c (digest_init_r): Add a decl parameter. Raise an error >> for non-static initialization of a flexible array member. >> (process_init_constructor, digest_init_flags, >> massage_init_elt, process_init_constructor_array, >> process_init_constructor_record, process_init_constructor_union, >> process_init_constructor): Add a decl parameter and pass it thru. >> (digest_nsdmi_init): Pass decl parameter to digest_init_flags. >> (digest_init): Pass NULL as decl parameter to digest_init_r. >> * semantics.c (finish_compound_literal): Likewise. >> * cp-tree.h (digest_init_flags): Adjust prototype. > > 8 spaces instead of tab. > >> gcc/testsuite: >> 2018-12-15 Bernd Edlinger <bernd.edlinger@hotmail.de> >> >> PR c++/88261 >> * gcc.dg/array-6.c: Move to ... >> * c-c++-common/array-6.c: ... here. > > Would be better to have more complete test coverage. > > In particular, the C FE passes on the following extension of array-6.c, > note "str" or {} is rejected, but if there is no initializer at all, > it is accepted. And, if it isn't already covered in the testsuite, would be > good to verify also how it behaves in new, initializer list and any other > kind of C++ style initializer, etc. (check for the errors you've added). > So void bar() { str x { 2, "a" }; } etc. > > /* PR c/5597 */ > /* { dg-do compile } */ > /* { dg-options "" } */ > > /* Verify that GCC forbids non-static initialization of > flexible array members. */ > > struct str { int len; char s[]; }; > > struct str a = { 2, "a" }; > > void foo() > { > static struct str b = { 2, "b" }; > struct str c = { 2, "c" }; /* { dg-error "(non-static)|(near initialization)" } */ > struct str d = (struct str) { 2, "d" }; /* { dg-error "(non-static)|(near initialization)" } */ > struct str e = (struct str) { d.len, "e" }; /* { dg-error "(non-static)|(initialization)" } */ > } > > struct str f = { 0, {} }; > > void bar() > { > static struct str g = { 0, {} }; > struct str h = { 0, {} }; /* { dg-error "(non-static)|(near initialization)" } */ > struct str i = (struct str) { 0, {} }; /* { dg-error "(non-static)|(near initialization)" } */ > struct str j = (struct str) { i.len, {} }; /* { dg-error "(non-static)|(initialization)" } */ > } > > struct str k = { 0 }; > > void baz() > { > static struct str l = { 0 }; > struct str m = { 0 }; > struct str n = (struct str) { 0 }; > struct str o = (struct str) { n.len }; > } > > struct str p = {}; > > void qux() > { > static struct str q = {}; > struct str r = {}; > struct str s = (struct str) {}; > } > > Jakub > [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: patch-pr88261.diff --] [-- Type: text/x-patch; name="patch-pr88261.diff", Size: 20815 bytes --] gcc/cp: 2018-12-15 Bernd Edlinger <bernd.edlinger@hotmail.de> PR c++/88261 * typeck2.c (digest_init_r): Add a decl parameter. Raise an error for non-static initialization of a flexible array member. (process_init_constructor, digest_init_flags, massage_init_elt, process_init_constructor_array, process_init_constructor_record, process_init_constructor_union, process_init_constructor): Add a decl parameter and pass it thru. (digest_nsdmi_init): Pass decl parameter to digest_init_flags. (digest_init): Pass NULL as decl parameter to digest_init_r. * semantics.c (finish_compound_literal): Likewise. * cp-tree.h (digest_init_flags): Adjust prototype. gcc/testsuite: 2018-12-15 Bernd Edlinger <bernd.edlinger@hotmail.de> PR c++/88261 * gcc.dg/array-6.c: Move to ... * c-c++-common/array-6.c: ... here. * g++.dg/ext/flexary3.C: Adjust test. * g++.dg/ext/flexary12.C: Likewise. * g++.dg/ext/flexary13.C: Likewise. * g++.dg/ext/flexary15.C: Likewise. * g++.dg/warn/Wplacement-new-size-1.C: Likewise. * g++.dg/warn/Wplacement-new-size-2.C: Likewise. * g++.dg/warn/Wplacement-new-size-6.C: Likewise. Index: gcc/cp/cp-tree.h =================================================================== --- gcc/cp/cp-tree.h (revision 266971) +++ gcc/cp/cp-tree.h (working copy) @@ -7483,7 +7483,8 @@ extern tree split_nonconstant_init (tree, tree); extern bool check_narrowing (tree, tree, tsubst_flags_t, bool = false); extern tree digest_init (tree, tree, tsubst_flags_t); -extern tree digest_init_flags (tree, tree, int, tsubst_flags_t); +extern tree digest_init_flags (tree, tree, int, + tsubst_flags_t, tree); extern tree digest_nsdmi_init (tree, tree, tsubst_flags_t); extern tree build_scoped_ref (tree, tree, tree *); extern tree build_x_arrow (location_t, tree, Index: gcc/cp/semantics.c =================================================================== --- gcc/cp/semantics.c (revision 266971) +++ gcc/cp/semantics.c (working copy) @@ -2828,7 +2828,7 @@ finish_compound_literal (tree type, tree compound_ return error_mark_node; } compound_literal = digest_init_flags (type, compound_literal, LOOKUP_NORMAL, - complain); + complain, NULL_TREE); if (TREE_CODE (compound_literal) == CONSTRUCTOR) { TREE_HAS_CONSTRUCTOR (compound_literal) = true; Index: gcc/cp/typeck2.c =================================================================== --- gcc/cp/typeck2.c (revision 266971) +++ gcc/cp/typeck2.c (working copy) @@ -36,7 +36,7 @@ along with GCC; see the file COPYING3. If not see static tree process_init_constructor (tree type, tree init, int nested, - tsubst_flags_t complain); + tsubst_flags_t complain, tree decl); /* Print an error message stemming from an attempt to use @@ -813,7 +813,7 @@ store_init_value (tree decl, tree init, vec<tree, value = init; else /* Digest the specified initializer into an expression. */ - value = digest_init_flags (type, init, flags, tf_warning_or_error); + value = digest_init_flags (type, init, flags, tf_warning_or_error, decl); if (TREE_CODE (type) == ARRAY_TYPE && TYPE_STRING_FLAG (TREE_TYPE (type)) @@ -1025,11 +1025,14 @@ check_narrowing (tree type, tree init, tsubst_flag initializer will have the right shape (brace elision has been undone). NESTED is non-zero iff we are being called for an element of a CONSTRUCTOR, - 2 iff the element of a CONSTRUCTOR is inside another CONSTRUCTOR. */ + 2 iff the element of a CONSTRUCTOR is inside another CONSTRUCTOR. + DECL points to the decl which is being initialized. It may be null if + the decl is unknown. In that case, assume it will be non-static. */ + static tree digest_init_r (tree type, tree init, int nested, int flags, - tsubst_flags_t complain) + tsubst_flags_t complain, tree decl) { enum tree_code code = TREE_CODE (type); @@ -1059,8 +1062,18 @@ digest_init_r (tree type, tree init, int nested, i { if (nested && !TYPE_DOMAIN (type)) /* C++ flexible array members have a null domain. */ - pedwarn (loc, OPT_Wpedantic, - "initialization of a flexible array member"); + { + if (decl && TREE_STATIC (decl)) + pedwarn (loc, OPT_Wpedantic, + "initialization of a flexible array member"); + else + { + if (complain & tf_error) + error_at (loc, "non-static initialization of" + " a flexible array member"); + return error_mark_node; + } + } tree typ1 = TYPE_MAIN_VARIANT (TREE_TYPE (type)); if (char_type_p (typ1) @@ -1175,7 +1188,7 @@ digest_init_r (tree type, tree init, int nested, i if (BRACE_ENCLOSED_INITIALIZER_P (init) && !TYPE_NON_AGGREGATE_CLASS (type)) - return process_init_constructor (type, init, nested, complain); + return process_init_constructor (type, init, nested, complain, decl); else { if (COMPOUND_LITERAL_P (init) && code == ARRAY_TYPE) @@ -1214,13 +1227,14 @@ digest_init_r (tree type, tree init, int nested, i tree digest_init (tree type, tree init, tsubst_flags_t complain) { - return digest_init_r (type, init, 0, LOOKUP_IMPLICIT, complain); + return digest_init_r (type, init, 0, LOOKUP_IMPLICIT, complain, NULL_TREE); } tree -digest_init_flags (tree type, tree init, int flags, tsubst_flags_t complain) +digest_init_flags (tree type, tree init, int flags, tsubst_flags_t complain, + tree decl) { - return digest_init_r (type, init, 0, flags, complain); + return digest_init_r (type, init, 0, flags, complain, decl); } /* Process the initializer INIT for an NSDMI DECL (a FIELD_DECL). */ @@ -1236,7 +1250,7 @@ digest_nsdmi_init (tree decl, tree init, tsubst_fl if (BRACE_ENCLOSED_INITIALIZER_P (init) && CP_AGGREGATE_TYPE_P (type)) init = reshape_init (type, init, complain); - init = digest_init_flags (type, init, flags, complain); + init = digest_init_flags (type, init, flags, complain, decl); if (TREE_CODE (init) == TARGET_EXPR) /* This represents the whole initialization. */ TARGET_EXPR_DIRECT_INIT_P (init) = true; @@ -1273,9 +1287,11 @@ picflag_from_initializer (tree init) /* Adjust INIT for going into a CONSTRUCTOR. */ static tree -massage_init_elt (tree type, tree init, int nested, tsubst_flags_t complain) +massage_init_elt (tree type, tree init, int nested, tsubst_flags_t complain, + tree decl) { - init = digest_init_r (type, init, nested ? 2 : 1, LOOKUP_IMPLICIT, complain); + init = digest_init_r (type, init, nested ? 2 : 1, LOOKUP_IMPLICIT, complain, + decl); /* Strip a simple TARGET_EXPR when we know this is an initializer. */ if (SIMPLE_TARGET_EXPR_P (init)) init = TARGET_EXPR_INITIAL (init); @@ -1294,7 +1310,7 @@ static tree static int process_init_constructor_array (tree type, tree init, int nested, - tsubst_flags_t complain) + tsubst_flags_t complain, tree decl) { unsigned HOST_WIDE_INT i, len = 0; int flags = 0; @@ -1347,7 +1363,8 @@ process_init_constructor_array (tree type, tree in ce->index = error_mark_node; gcc_assert (ce->value); ce->value - = massage_init_elt (TREE_TYPE (type), ce->value, nested, complain); + = massage_init_elt (TREE_TYPE (type), ce->value, nested, complain, + decl); gcc_checking_assert (ce->value == error_mark_node @@ -1371,7 +1388,8 @@ process_init_constructor_array (tree type, tree in we can't rely on the back end to do it for us, so make the initialization explicit by list-initializing from T{}. */ next = build_constructor (init_list_type_node, NULL); - next = massage_init_elt (TREE_TYPE (type), next, nested, complain); + next = massage_init_elt (TREE_TYPE (type), next, nested, complain, + decl); if (initializer_zerop (next)) /* The default zero-initialization is fine for us; don't add anything to the CONSTRUCTOR. */ @@ -1417,7 +1435,7 @@ process_init_constructor_array (tree type, tree in static int process_init_constructor_record (tree type, tree init, int nested, - tsubst_flags_t complain) + tsubst_flags_t complain, tree decl) { vec<constructor_elt, va_gc> *v = NULL; tree field; @@ -1499,7 +1517,7 @@ process_init_constructor_record (tree type, tree i if (ce) { gcc_assert (ce->value); - next = massage_init_elt (type, next, nested, complain); + next = massage_init_elt (type, next, nested, complain, decl); ++idx; } } @@ -1528,7 +1546,8 @@ process_init_constructor_record (tree type, tree i for us, so build up TARGET_EXPRs. If the type in question is a class, just build one up; if it's an array, recurse. */ next = build_constructor (init_list_type_node, NULL); - next = massage_init_elt (TREE_TYPE (field), next, nested, complain); + next = massage_init_elt (TREE_TYPE (field), next, nested, complain, + decl); /* Warn when some struct elements are implicitly initialized. */ if ((complain & tf_warning) @@ -1644,7 +1663,7 @@ process_init_constructor_record (tree type, tree i static int process_init_constructor_union (tree type, tree init, int nested, - tsubst_flags_t complain) + tsubst_flags_t complain, tree decl) { constructor_elt *ce; int len; @@ -1731,7 +1750,7 @@ process_init_constructor_union (tree type, tree in if (ce->value && ce->value != error_mark_node) ce->value = massage_init_elt (TREE_TYPE (ce->index), ce->value, nested, - complain); + complain, decl); return picflag_from_initializer (ce->value); } @@ -1752,7 +1771,7 @@ process_init_constructor_union (tree type, tree in static tree process_init_constructor (tree type, tree init, int nested, - tsubst_flags_t complain) + tsubst_flags_t complain, tree decl) { int flags; @@ -1759,11 +1778,11 @@ process_init_constructor (tree type, tree init, in gcc_assert (BRACE_ENCLOSED_INITIALIZER_P (init)); if (TREE_CODE (type) == ARRAY_TYPE || VECTOR_TYPE_P (type)) - flags = process_init_constructor_array (type, init, nested, complain); + flags = process_init_constructor_array (type, init, nested, complain, decl); else if (TREE_CODE (type) == RECORD_TYPE) - flags = process_init_constructor_record (type, init, nested, complain); + flags = process_init_constructor_record (type, init, nested, complain, decl); else if (TREE_CODE (type) == UNION_TYPE) - flags = process_init_constructor_union (type, init, nested, complain); + flags = process_init_constructor_union (type, init, nested, complain, decl); else gcc_unreachable (); Index: gcc/testsuite/g++.dg/ext/flexary12.C =================================================================== --- gcc/testsuite/g++.dg/ext/flexary12.C (revision 266971) +++ gcc/testsuite/g++.dg/ext/flexary12.C (working copy) @@ -12,7 +12,7 @@ struct A { void f1 () { // This is the meat of the test from c++/69290: - struct A a + static struct A a = { "c" }; // { dg-error "invalid conversion from .const char\\*. to .int." } (void)&a; @@ -27,13 +27,13 @@ struct B { void f2 () { - struct B b1 + static struct B b1 = { 0, "c" }; // { dg-error "invalid conversion from .const char\\*. to .int." } (void)&b1; const char s[] = "c"; - struct B b2 + static struct B b2 = { 0, s }; // { dg-error "invalid conversion from .const char\\*. to .int." } (void)&b2; @@ -57,7 +57,7 @@ struct C { void f3 () { - struct C<double> cd + static struct C<double> cd = { "c" }; // { dg-error "cannot convert .const char\\*. to .double." } (void)&cd; Index: gcc/testsuite/g++.dg/ext/flexary13.C =================================================================== --- gcc/testsuite/g++.dg/ext/flexary13.C (revision 266971) +++ gcc/testsuite/g++.dg/ext/flexary13.C (working copy) @@ -19,33 +19,33 @@ int main () ASSERT (s.n == 0); } { - Ax s = + static Ax s = { 0, { } }; // dg-warning "initialization of a flexible array member" } ASSERT (s.n == 0); } { - Ax s = + static Ax s = { 1, { 2 } }; // dg-warning "initialization of a flexible array member" } ASSERT (s.n == 1 && s.a [0] == 2); } { - Ax s = + static Ax s = { 2, { 3, 4 } }; // dg-warning "initialization of a flexible array member" } ASSERT (s.n = 2 && s.a [0] == 3 && s.a [1] == 4); } { - Ax s = + static Ax s = { 123, i }; // dg-warning "initialization of a flexible array member" } ASSERT (s.n == 123 && s.a [0] == i); } { - Ax s = + static Ax s = { 456, { i } }; // dg-warning "initialization of a flexible array member" } ASSERT (s.n == 456 && s.a [0] == i); } { int j = i + 1, k = j + 1; - Ax s = + static Ax s = { 3, { i, j, k } }; // dg-warning "initialization of a flexible array member" } ASSERT (s.n == 3 && s.a [0] == i && s.a [1] == j && s.a [2] == k); } Index: gcc/testsuite/g++.dg/ext/flexary15.C =================================================================== --- gcc/testsuite/g++.dg/ext/flexary15.C (revision 266971) +++ gcc/testsuite/g++.dg/ext/flexary15.C (working copy) @@ -10,5 +10,5 @@ struct S { void foo (const char *a) { - const S s = { 1, { a, "b" } }; // { dg-warning "invalid conversion" } + static const S s = { 1, { a, "b" } }; // { dg-warning "invalid conversion" } } Index: gcc/testsuite/g++.dg/ext/flexary3.C =================================================================== --- gcc/testsuite/g++.dg/ext/flexary3.C (revision 266971) +++ gcc/testsuite/g++.dg/ext/flexary3.C (working copy) @@ -17,5 +17,6 @@ struct s { int main() { struct s s = { .c = 0 }; // { dg-error "initializer" } + // { dg-error "non-static initialization of a flexible array member" "" { target *-*-* } .-1 } return 0; } Index: gcc/testsuite/g++.dg/warn/Wplacement-new-size-1.C =================================================================== --- gcc/testsuite/g++.dg/warn/Wplacement-new-size-1.C (revision 266971) +++ gcc/testsuite/g++.dg/warn/Wplacement-new-size-1.C (working copy) @@ -28,7 +28,7 @@ void fAx (Ax *px, Ax &rx) void fAx2 () { - Ax ax2 = { 1, { 2, 3 } }; + static Ax ax2 = { 1, { 2, 3 } }; new (ax2.a) Int16; new (ax2.a) Int32; // { dg-warning "placement" } @@ -82,7 +82,7 @@ void fBx (BAx *pbx, BAx &rbx) void fBx1 () { - BAx bax1 = { 1, /* Ax = */ { 2, /* a[] = */ {} } }; + static BAx bax1 = { 1, /* Ax = */ { 2, /* a[] = */ {} } }; new (bax1.ax.a) char; // { dg-warning "placement" } new (bax1.ax.a) char[2]; // { dg-warning "placement" } Index: gcc/testsuite/g++.dg/warn/Wplacement-new-size-2.C =================================================================== --- gcc/testsuite/g++.dg/warn/Wplacement-new-size-2.C (revision 266971) +++ gcc/testsuite/g++.dg/warn/Wplacement-new-size-2.C (working copy) @@ -36,10 +36,10 @@ void fAx2 () // isn't allowed in C and should perhaps be disallowed in C++ as // well to avoid c++/69696 - incorrect initialization of block-scope // flexible array members. - Ax ax2 = { 1, { 2, 3 } }; + Ax ax2 = { 1, { 2, 3 } }; // { dg-error "non-static initialization of a flexible array member" } - new (ax2.a) Int16; - new (ax2.a) Int16[1]; + new (ax2.a) Int16; // { dg-warning "placement" } + new (ax2.a) Int16[1]; // { dg-warning "placement" } new (ax2.a) Int16[2]; // { dg-warning "placement" } new (ax2.a) Int32; // { dg-warning "placement" } new (ax2.a) Int32[2]; // { dg-warning "placement" } @@ -140,7 +140,7 @@ void fBx (BAx *pbx, BAx &rbx) void fBx1 () { - BAx bax1 = { 1, /* Ax = */ { 2, /* a[] = */ {} } }; + static BAx bax1 = { 1, /* Ax = */ { 2, /* a[] = */ {} } }; new (bax1.ax.a) char; // { dg-warning "placement" } new (bax1.ax.a) char[2]; // { dg-warning "placement" } Index: gcc/testsuite/g++.dg/warn/Wplacement-new-size-6.C =================================================================== --- gcc/testsuite/g++.dg/warn/Wplacement-new-size-6.C (revision 266971) +++ gcc/testsuite/g++.dg/warn/Wplacement-new-size-6.C (working copy) @@ -15,7 +15,7 @@ struct BAx { int i; Ax ax; }; void fBx1 () { - BAx bax1 = { 1, /* Ax = */ { 2, /* a[] = */ { 3 } } }; // { dg-error "initialization of flexible array member in a nested context" } + static BAx bax1 = { 1, /* Ax = */ { 2, /* a[] = */ { 3 } } }; // { dg-error "initialization of flexible array member in a nested context" } new (bax1.ax.a) char; // { dg-warning "placement" } new (bax1.ax.a) char[2]; // { dg-warning "placement" } @@ -25,7 +25,7 @@ void fBx1 () void fBx2 () { - BAx bax2 = { 1, /* Ax = */ { 2, /* a[] = */ { 3, 4 } } }; // { dg-error "initialization of flexible array member in a nested context" } + static BAx bax2 = { 1, /* Ax = */ { 2, /* a[] = */ { 3, 4 } } }; // { dg-error "initialization of flexible array member in a nested context" } new (bax2.ax.a) char; // { dg-warning "placement" } new (bax2.ax.a) char[2]; // { dg-warning "placement" } @@ -37,7 +37,7 @@ void fBx2 () void fBx3 () { - BAx bax2 = { 1, /* Ax = */ { 3, /* a[] = */ { 4, 5, 6 } } }; // { dg-error "initialization of flexible array member in a nested context" } + static BAx bax2 = { 1, /* Ax = */ { 3, /* a[] = */ { 4, 5, 6 } } }; // { dg-error "initialization of flexible array member in a nested context" } new (bax2.ax.a) char; // { dg-warning "placement" } new (bax2.ax.a) char[2]; // { dg-warning "placement" } Index: gcc/testsuite/gcc.dg/array-6.c =================================================================== --- gcc/testsuite/gcc.dg/array-6.c (revision 266971) +++ gcc/testsuite/gcc.dg/array-6.c (working copy) @@ -1,18 +0,0 @@ -/* PR c/5597 */ -/* { dg-do compile } */ -/* { dg-options "" } */ - -/* Verify that GCC forbids non-static initialization of - flexible array members. */ - -struct str { int len; char s[]; }; - -struct str a = { 2, "a" }; - -void foo() -{ - static struct str b = { 2, "b" }; - struct str c = { 2, "c" }; /* { dg-error "(non-static)|(near initialization)" } */ - struct str d = (struct str) { 2, "d" }; /* { dg-error "(non-static)|(near initialization)" } */ - struct str e = (struct str) { d.len, "e" }; /* { dg-error "(non-static)|(initialization)" } */ -} Index: gcc/testsuite/c-c++-common/array-6.c =================================================================== --- gcc/testsuite/c-c++-common/array-6.c (revision 0) +++ gcc/testsuite/c-c++-common/array-6.c (working copy) @@ -0,0 +1,18 @@ +/* PR c/5597 */ +/* { dg-do compile } */ +/* { dg-options "" } */ + +/* Verify that GCC forbids non-static initialization of + flexible array members. */ + +struct str { int len; char s[]; }; + +struct str a = { 2, "a" }; + +void foo() +{ + static struct str b = { 2, "b" }; + struct str c = { 2, "c" }; /* { dg-error "(non-static)|(near initialization)" } */ + struct str d = (struct str) { 2, "d" }; /* { dg-error "(non-static)|(near initialization)" } */ + struct str e = (struct str) { d.len, "e" }; /* { dg-error "(non-static)|(initialization)" } */ +} Index: gcc/testsuite/c-c++-common/array-6.c =================================================================== --- gcc/testsuite/c-c++-common/array-6.c (revision 266971) +++ gcc/testsuite/c-c++-common/array-6.c (working copy) @@ -16,3 +16,32 @@ void foo() struct str d = (struct str) { 2, "d" }; /* { dg-error "(non-static)|(near initialization)" } */ struct str e = (struct str) { d.len, "e" }; /* { dg-error "(non-static)|(initialization)" } */ } + +struct str f = { 0, {} }; + +void bar() +{ + static struct str g = { 0, {} }; + struct str h = { 0, {} }; /* { dg-error "(non-static)|(near initialization)" } */ + struct str i = (struct str) { 0, {} }; /* { dg-error "(non-static)|(near initialization)" } */ + struct str j = (struct str) { i.len, {} }; /* { dg-error "(non-static)|(initialization)" } */ +} + +struct str k = { 0 }; + +void baz() +{ + static struct str l = { 0 }; + struct str m = { 0 }; + struct str n = (struct str) { 0 }; + struct str o = (struct str) { n.len }; +} + +struct str p = {}; + +void qux() +{ + static struct str q = {}; + struct str r = {}; + struct str s = (struct str) {}; +} ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH, C++] Fix PR c++/88261 2018-12-15 8:36 [PATCH, C++] Fix PR c++/88261 Bernd Edlinger 2018-12-15 9:33 ` Jakub Jelinek @ 2018-12-19 3:35 ` Jason Merrill 2018-12-20 17:50 ` Martin Sebor 1 sibling, 1 reply; 16+ messages in thread From: Jason Merrill @ 2018-12-19 3:35 UTC (permalink / raw) To: Bernd Edlinger, gcc-patches, Jeff Law, Nathan Sidwell, Martin Sebor On 12/15/18 3:36 AM, Bernd Edlinger wrote: > this patch implements an error message, for non-static initialization of a flexible array member. > This duplicates the existing error message from the C-FE, to avoid ICE and wrong code generation > issues, as pointed out in the PR. > > It is a bit funny that a non-functional feature like that has already rather much test coverage. > The most easy adjustment seems to change the existing test cases to use static declarations. Martin, thoughts? Jason ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH, C++] Fix PR c++/88261 2018-12-19 3:35 ` Jason Merrill @ 2018-12-20 17:50 ` Martin Sebor 2018-12-20 18:11 ` Martin Sebor 0 siblings, 1 reply; 16+ messages in thread From: Martin Sebor @ 2018-12-20 17:50 UTC (permalink / raw) To: Jason Merrill, Bernd Edlinger, gcc-patches, Jeff Law, Nathan Sidwell On 12/17/18 7:58 AM, Jason Merrill wrote: > On 12/15/18 3:36 AM, Bernd Edlinger wrote: >> this patch implements an error message, for non-static initialization >> of a flexible array member. >> This duplicates the existing error message from the C-FE, to avoid ICE >> and wrong code generation >> issues, as pointed out in the PR. >> >> It is a bit funny that a non-functional feature like that has already >> rather much test coverage. >> The most easy adjustment seems to change the existing test cases to >> use static declarations. > > Martin, thoughts? Our high-level goal when tightening up how flexible array members are handled in C++ was to accept what's accepted in standard C mode and reject (or, at a minimum, warn for) C++ extensions that could be relied on in existing code. The flexarray tests I added back then were for features that looked like intentional extensions and that seemed to work for at least some use cases as far as I could tell. What I noticed didn't work I created bugs for: 69338, 69696, and 69338 look related, but there are others. I think all these bugs should all be reviewed and a decision made about what's intended to work and what continues to be accepted as an accident and should be rejected. After that, we can adjust the existing tests. Martin ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH, C++] Fix PR c++/88261 2018-12-20 17:50 ` Martin Sebor @ 2018-12-20 18:11 ` Martin Sebor 2018-12-20 21:08 ` Bernd Edlinger 0 siblings, 1 reply; 16+ messages in thread From: Martin Sebor @ 2018-12-20 18:11 UTC (permalink / raw) To: Martin Sebor, Jason Merrill, Bernd Edlinger, gcc-patches, Jeff Law, Nathan Sidwell On 12/20/18 10:46 AM, Martin Sebor wrote: > On 12/17/18 7:58 AM, Jason Merrill wrote: >> On 12/15/18 3:36 AM, Bernd Edlinger wrote: >>> this patch implements an error message, for non-static initialization >>> of a flexible array member. >>> This duplicates the existing error message from the C-FE, to avoid >>> ICE and wrong code generation >>> issues, as pointed out in the PR. >>> >>> It is a bit funny that a non-functional feature like that has already >>> rather much test coverage. >>> The most easy adjustment seems to change the existing test cases to >>> use static declarations. >> >> Martin, thoughts? > > Our high-level goal when tightening up how flexible array members > are handled in C++ was to accept what's accepted in standard C mode > and reject (or, at a minimum, warn for) C++ extensions that could > be relied on in existing code. I meant "reject what couldn't be relied on" and "warn for that could be." (Sorry for the delay, by the way. I've been migrating to a new machine this week and things aren't yet working quite like I'm used to.) > > The flexarray tests I added back then were for features that looked > like intentional extensions and that seemed to work for at least > some use cases as far as I could tell. What I noticed didn't work > I created bugs for: 69338, 69696, and 69338 look related, but there > are others. > > I think all these bugs should all be reviewed and a decision made > about what's intended to work and what continues to be accepted as > an accident and should be rejected. After that, we can adjust > the existing tests. > > Martin ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH, C++] Fix PR c++/88261 2018-12-20 18:11 ` Martin Sebor @ 2018-12-20 21:08 ` Bernd Edlinger 2018-12-21 1:04 ` Martin Sebor 0 siblings, 1 reply; 16+ messages in thread From: Bernd Edlinger @ 2018-12-20 21:08 UTC (permalink / raw) To: Martin Sebor, Martin Sebor, Jason Merrill, gcc-patches, Jeff Law, Nathan Sidwell On 12/20/18 6:50 PM, Martin Sebor wrote: > On 12/20/18 10:46 AM, Martin Sebor wrote: >> On 12/17/18 7:58 AM, Jason Merrill wrote: >>> On 12/15/18 3:36 AM, Bernd Edlinger wrote: >>>> this patch implements an error message, for non-static initialization of a flexible array member. >>>> This duplicates the existing error message from the C-FE, to avoid ICE and wrong code generation >>>> issues, as pointed out in the PR. >>>> >>>> It is a bit funny that a non-functional feature like that has already rather much test coverage. >>>> The most easy adjustment seems to change the existing test cases to use static declarations. >>> >>> Martin, thoughts? >> >> Our high-level goal when tightening up how flexible array members >> are handled in C++ was to accept what's accepted in standard C mode >> and reject (or, at a minimum, warn for) C++ extensions that could >> be relied on in existing code. > > I meant "reject what couldn't be relied on" and "warn for that could > be." > I believe the problem here is effectively that initializing non-static flexible array is not supported by the middle-end. All examples where flexible array members are initialized on automatic variable work only as long as they are simple enough that they are optimized away so that they do not survive until expansion. Take as example gcc/testsuite/g++.dg/ext/flexary13.C, it compiles and runs successfully, but the assertions start to fail if Ax is declared volatile, and at the same time, we know that the automatic variables are allocated in a way that they can overlap and crash at any time. My impression is that the existing C error made the middle-end kind of rely on this behavior. So I think the right thing to do is duplicate the existing C error in the C++ FE. I do not see any automatic variable with initialized flexible data members where it would be safe to only warn about them. > (Sorry for the delay, by the way. I've been migrating to a new machine > this week and things aren't yet working quite like I'm used to.) > >> >> The flexarray tests I added back then were for features that looked >> like intentional extensions and that seemed to work for at least >> some use cases as far as I could tell. What I noticed didn't work >> I created bugs for: 69338, 69696, and 69338 look related, but there >> are others. >> >> I think all these bugs should all be reviewed and a decision made >> about what's intended to work and what continues to be accepted as >> an accident and should be rejected. After that, we can adjust >> the existing tests. >> I would not rule out the possibility that there can be more bugs. But I think the existing tests need to avoid the case which evokes the new error. The question is, if changing from automatic to static objects prevents those tests to test what they were originally written for. I believe this is not the case, but I do probably not know all the background here. Thanks Bernd. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH, C++] Fix PR c++/88261 2018-12-20 21:08 ` Bernd Edlinger @ 2018-12-21 1:04 ` Martin Sebor 2018-12-22 19:38 ` Bernd Edlinger 0 siblings, 1 reply; 16+ messages in thread From: Martin Sebor @ 2018-12-21 1:04 UTC (permalink / raw) To: Bernd Edlinger, Martin Sebor, Jason Merrill, gcc-patches, Jeff Law, Nathan Sidwell On 12/20/18 2:07 PM, Bernd Edlinger wrote: > On 12/20/18 6:50 PM, Martin Sebor wrote: >> On 12/20/18 10:46 AM, Martin Sebor wrote: >>> On 12/17/18 7:58 AM, Jason Merrill wrote: >>>> On 12/15/18 3:36 AM, Bernd Edlinger wrote: >>>>> this patch implements an error message, for non-static initialization of a flexible array member. >>>>> This duplicates the existing error message from the C-FE, to avoid ICE and wrong code generation >>>>> issues, as pointed out in the PR. >>>>> >>>>> It is a bit funny that a non-functional feature like that has already rather much test coverage. >>>>> The most easy adjustment seems to change the existing test cases to use static declarations. >>>> >>>> Martin, thoughts? >>> >>> Our high-level goal when tightening up how flexible array members >>> are handled in C++ was to accept what's accepted in standard C mode >>> and reject (or, at a minimum, warn for) C++ extensions that could >>> be relied on in existing code. >> >> I meant "reject what couldn't be relied on" and "warn for that could >> be." >> > > I believe the problem here is effectively that initializing non-static > flexible array is not supported by the middle-end. All examples > where flexible array members are initialized on automatic variable > work only as long as they are simple enough that they are optimized > away so that they do not survive until expansion. > > Take as example gcc/testsuite/g++.dg/ext/flexary13.C, > it compiles and runs successfully, but the assertions start to > fail if Ax is declared volatile, and at the same time, we know > that the automatic variables are allocated in a way that they > can overlap and crash at any time. > > My impression is that the existing C error made the middle-end kind of rely > on this behavior. > > So I think the right thing to do is duplicate the existing C error in > the C++ FE. I do not see any automatic variable with initialized flexible > data members where it would be safe to only warn about them. If there are no reasonable use cases that code out there could be relying on because none of them works correctly then rejecting the initialization makes sense to me. >> (Sorry for the delay, by the way. I've been migrating to a new machine >> this week and things aren't yet working quite like I'm used to.) >> >>> >>> The flexarray tests I added back then were for features that looked >>> like intentional extensions and that seemed to work for at least >>> some use cases as far as I could tell. What I noticed didn't work >>> I created bugs for: 69338, 69696, and 69338 look related, but there >>> are others. >>> >>> I think all these bugs should all be reviewed and a decision made >>> about what's intended to work and what continues to be accepted as >>> an accident and should be rejected. After that, we can adjust >>> the existing tests. >>> > > I would not rule out the possibility that there can be more bugs. > But I think the existing tests need to avoid the case which evokes > the new error. The question is, if changing from automatic to static > objects prevents those tests to test what they were originally written for. > I believe this is not the case, but I do probably not know all the > background here. IIRC, most of the tests I added were meant to exercise just the front-end, not any later stages (if that's what you meant). Otherwise, if you're worried about the changes from auto to static no longer exercising downstream front-end code, whether that matters depends on the intent of each test. flexary13.C was most likely meant to also verify codegen (hence the assertions) so I would suggest to make it do that (i.e., verify the assertions are optimized out if in fact they are, or make the test run so they must pass). The changes to the rest of the flexary*.C tests seem okay, though a new test should be added to explicitly exercise this change (bug 88261), even if the error happens to be tested by one of the changed tests. In changes to the Wplacement-new-size*.C tests I would suggest to follow the same approach of using statics instead of testing for errors so the code that exercises warnings doesn't depend on erroneous constructs. The comment in Wplacement-new-size-2.C just above the code your patch changes that reads: // Initialization of non-static objects with flexible array members // isn't allowed in C and should perhaps be disallowed in C++ as // well to avoid c++/69696 - incorrect initialization of block-scope // flexible array members. Ax ax2 = { 1, { 2, 3 } }; should be updated and the referenced bug and any others that this change prevents should be resolved. Martin ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH, C++] Fix PR c++/88261 2018-12-21 1:04 ` Martin Sebor @ 2018-12-22 19:38 ` Bernd Edlinger 2019-01-04 15:31 ` [PATCH, C++,rebased] " Bernd Edlinger 0 siblings, 1 reply; 16+ messages in thread From: Bernd Edlinger @ 2018-12-22 19:38 UTC (permalink / raw) To: Martin Sebor, Martin Sebor, Jason Merrill, gcc-patches, Jeff Law, Nathan Sidwell [-- Attachment #1: Type: text/plain, Size: 5702 bytes --] On 12/21/18 2:03 AM, Martin Sebor wrote: > On 12/20/18 2:07 PM, Bernd Edlinger wrote: >> On 12/20/18 6:50 PM, Martin Sebor wrote: >>> On 12/20/18 10:46 AM, Martin Sebor wrote: >>>> On 12/17/18 7:58 AM, Jason Merrill wrote: >>>>> On 12/15/18 3:36 AM, Bernd Edlinger wrote: >>>>>> this patch implements an error message, for non-static initialization of a flexible array member. >>>>>> This duplicates the existing error message from the C-FE, to avoid ICE and wrong code generation >>>>>> issues, as pointed out in the PR. >>>>>> >>>>>> It is a bit funny that a non-functional feature like that has already rather much test coverage. >>>>>> The most easy adjustment seems to change the existing test cases to use static declarations. >>>>> >>>>> Martin, thoughts? >>>> >>>> Our high-level goal when tightening up how flexible array members >>>> are handled in C++ was to accept what's accepted in standard C mode >>>> and reject (or, at a minimum, warn for) C++ extensions that could >>>> be relied on in existing code. >>> >>> I meant "reject what couldn't be relied on" and "warn for that could >>> be." >>> >> >> I believe the problem here is effectively that initializing non-static >> flexible array is not supported by the middle-end. All examples >> where flexible array members are initialized on automatic variable >> work only as long as they are simple enough that they are optimized >> away so that they do not survive until expansion. >> >> Take as example gcc/testsuite/g++.dg/ext/flexary13.C, >> it compiles and runs successfully, but the assertions start to >> fail if Ax is declared volatile, and at the same time, we know >> that the automatic variables are allocated in a way that they >> can overlap and crash at any time. >> >> My impression is that the existing C error made the middle-end kind of rely >> on this behavior. >> >> So I think the right thing to do is duplicate the existing C error in >> the C++ FE. I do not see any automatic variable with initialized flexible >> data members where it would be safe to only warn about them. > > If there are no reasonable use cases that code out there could > be relying on because none of them works correctly then rejecting > the initialization makes sense to me. > >>> (Sorry for the delay, by the way. I've been migrating to a new machine >>> this week and things aren't yet working quite like I'm used to.) >>> >>>> >>>> The flexarray tests I added back then were for features that looked >>>> like intentional extensions and that seemed to work for at least >>>> some use cases as far as I could tell. What I noticed didn't work >>>> I created bugs for: 69338, 69696, and 69338 look related, but there >>>> are others. >>>> >>>> I think all these bugs should all be reviewed and a decision made >>>> about what's intended to work and what continues to be accepted as >>>> an accident and should be rejected. After that, we can adjust >>>> the existing tests. >>>> >> >> I would not rule out the possibility that there can be more bugs. >> But I think the existing tests need to avoid the case which evokes >> the new error. The question is, if changing from automatic to static >> objects prevents those tests to test what they were originally written for. >> I believe this is not the case, but I do probably not know all the >> background here. > > IIRC, most of the tests I added were meant to exercise just > the front-end, not any later stages (if that's what you meant). > Otherwise, if you're worried about the changes from auto to > static no longer exercising downstream front-end code, whether > that matters depends on the intent of each test. > > flexary13.C was most likely meant to also verify codegen (hence > the assertions) so I would suggest to make it do that (i.e., > verify the assertions are optimized out if in fact they are, > or make the test run so they must pass). > Oh well, unfortunately the modified test case with static objects fails one assertion when executed at -O0, I missed that before, because I used -O2 or higher. I filed that as PR 88578, so in the moment I would like to leave the test case as compile only, and change that to run once PR 88578 is resolved. > The changes to the rest of the flexary*.C tests seem okay, > though a new test should be added to explicitly exercise this > change (bug 88261), even if the error happens to be tested by > one of the changed tests. > That is the case, because the array-6.c test case was moved to c-c++-common. That is the reproducer for the ICE from the PR. > In changes to the Wplacement-new-size*.C tests I would suggest > to follow the same approach of using statics instead of testing > for errors so the code that exercises warnings doesn't depend > on erroneous constructs. > > The comment in Wplacement-new-size-2.C just above the code your > patch changes that reads: > > // Initialization of non-static objects with flexible array members > // isn't allowed in C and should perhaps be disallowed in C++ as > // well to avoid c++/69696 - incorrect initialization of block-scope > // flexible array members. > Ax ax2 = { 1, { 2, 3 } }; > > should be updated and the referenced bug and any others that this > change prevents should be resolved. > Done. I also added PR c++/69696 to the changelog as this should definitely be fixed by this patch as well. So despite the newly discovered problem with the non-constant initializers which appears to be a separate problem, I would still like to get an OK for this patch in the current form. Thanks Bernd. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: patch-pr88261.diff --] [-- Type: text/x-patch; name="patch-pr88261.diff", Size: 20228 bytes --] gcc/cp: 2018-12-15 Bernd Edlinger <bernd.edlinger@hotmail.de> PR c++/88261 PR c++/69696 * typeck2.c (digest_init_r): Add a decl parameter. Raise an error for non-static initialization of a flexible array member. (process_init_constructor, digest_init_flags, massage_init_elt, process_init_constructor_array, process_init_constructor_record, process_init_constructor_union, process_init_constructor): Add a decl parameter and pass it thru. (digest_nsdmi_init): Pass decl parameter to digest_init_flags. (digest_init): Pass NULL as decl parameter to digest_init_r. * semantics.c (finish_compound_literal): Likewise. * cp-tree.h (digest_init_flags): Adjust prototype. gcc/testsuite: 2018-12-15 Bernd Edlinger <bernd.edlinger@hotmail.de> PR c++/88261 PR c++/69696 * gcc.dg/array-6.c: Move from here ... * c-c++-common/array-6.c: ... to here and add some more test coverage. * g++.dg/ext/flexary3.C: Adjust test. * g++.dg/ext/flexary12.C: Likewise. * g++.dg/ext/flexary13.C: Likewise. * g++.dg/ext/flexary15.C: Likewise. * g++.dg/warn/Wplacement-new-size-1.C: Likewise. * g++.dg/warn/Wplacement-new-size-2.C: Likewise. * g++.dg/warn/Wplacement-new-size-6.C: Likewise. Index: gcc/cp/cp-tree.h =================================================================== --- gcc/cp/cp-tree.h (revision 267233) +++ gcc/cp/cp-tree.h (working copy) @@ -7483,7 +7483,8 @@ extern tree split_nonconstant_init (tree, tree); extern bool check_narrowing (tree, tree, tsubst_flags_t, bool = false); extern tree digest_init (tree, tree, tsubst_flags_t); -extern tree digest_init_flags (tree, tree, int, tsubst_flags_t); +extern tree digest_init_flags (tree, tree, int, + tsubst_flags_t, tree); extern tree digest_nsdmi_init (tree, tree, tsubst_flags_t); extern tree build_scoped_ref (tree, tree, tree *); extern tree build_x_arrow (location_t, tree, Index: gcc/cp/semantics.c =================================================================== --- gcc/cp/semantics.c (revision 267233) +++ gcc/cp/semantics.c (working copy) @@ -2828,7 +2828,7 @@ finish_compound_literal (tree type, tree compound_ return error_mark_node; } compound_literal = digest_init_flags (type, compound_literal, LOOKUP_NORMAL, - complain); + complain, NULL_TREE); if (TREE_CODE (compound_literal) == CONSTRUCTOR) { TREE_HAS_CONSTRUCTOR (compound_literal) = true; Index: gcc/cp/typeck2.c =================================================================== --- gcc/cp/typeck2.c (revision 267233) +++ gcc/cp/typeck2.c (working copy) @@ -36,7 +36,7 @@ along with GCC; see the file COPYING3. If not see static tree process_init_constructor (tree type, tree init, int nested, - tsubst_flags_t complain); + tsubst_flags_t complain, tree decl); /* Print an error message stemming from an attempt to use @@ -813,7 +813,7 @@ store_init_value (tree decl, tree init, vec<tree, value = init; else /* Digest the specified initializer into an expression. */ - value = digest_init_flags (type, init, flags, tf_warning_or_error); + value = digest_init_flags (type, init, flags, tf_warning_or_error, decl); if (TREE_CODE (type) == ARRAY_TYPE && TYPE_STRING_FLAG (TREE_TYPE (type)) @@ -1025,11 +1025,14 @@ check_narrowing (tree type, tree init, tsubst_flag initializer will have the right shape (brace elision has been undone). NESTED is non-zero iff we are being called for an element of a CONSTRUCTOR, - 2 iff the element of a CONSTRUCTOR is inside another CONSTRUCTOR. */ + 2 iff the element of a CONSTRUCTOR is inside another CONSTRUCTOR. + DECL points to the decl which is being initialized. It may be null if + the decl is unknown. In that case, assume it will be non-static. */ + static tree digest_init_r (tree type, tree init, int nested, int flags, - tsubst_flags_t complain) + tsubst_flags_t complain, tree decl) { enum tree_code code = TREE_CODE (type); @@ -1059,8 +1062,18 @@ digest_init_r (tree type, tree init, int nested, i { if (nested && !TYPE_DOMAIN (type)) /* C++ flexible array members have a null domain. */ - pedwarn (loc, OPT_Wpedantic, - "initialization of a flexible array member"); + { + if (decl && TREE_STATIC (decl)) + pedwarn (loc, OPT_Wpedantic, + "initialization of a flexible array member"); + else + { + if (complain & tf_error) + error_at (loc, "non-static initialization of" + " a flexible array member"); + return error_mark_node; + } + } tree typ1 = TYPE_MAIN_VARIANT (TREE_TYPE (type)); if (char_type_p (typ1) @@ -1175,7 +1188,7 @@ digest_init_r (tree type, tree init, int nested, i if (BRACE_ENCLOSED_INITIALIZER_P (init) && !TYPE_NON_AGGREGATE_CLASS (type)) - return process_init_constructor (type, init, nested, complain); + return process_init_constructor (type, init, nested, complain, decl); else { if (COMPOUND_LITERAL_P (init) && code == ARRAY_TYPE) @@ -1214,13 +1227,14 @@ digest_init_r (tree type, tree init, int nested, i tree digest_init (tree type, tree init, tsubst_flags_t complain) { - return digest_init_r (type, init, 0, LOOKUP_IMPLICIT, complain); + return digest_init_r (type, init, 0, LOOKUP_IMPLICIT, complain, NULL_TREE); } tree -digest_init_flags (tree type, tree init, int flags, tsubst_flags_t complain) +digest_init_flags (tree type, tree init, int flags, tsubst_flags_t complain, + tree decl) { - return digest_init_r (type, init, 0, flags, complain); + return digest_init_r (type, init, 0, flags, complain, decl); } /* Process the initializer INIT for an NSDMI DECL (a FIELD_DECL). */ @@ -1236,7 +1250,7 @@ digest_nsdmi_init (tree decl, tree init, tsubst_fl if (BRACE_ENCLOSED_INITIALIZER_P (init) && CP_AGGREGATE_TYPE_P (type)) init = reshape_init (type, init, complain); - init = digest_init_flags (type, init, flags, complain); + init = digest_init_flags (type, init, flags, complain, decl); if (TREE_CODE (init) == TARGET_EXPR) /* This represents the whole initialization. */ TARGET_EXPR_DIRECT_INIT_P (init) = true; @@ -1273,9 +1287,11 @@ picflag_from_initializer (tree init) /* Adjust INIT for going into a CONSTRUCTOR. */ static tree -massage_init_elt (tree type, tree init, int nested, tsubst_flags_t complain) +massage_init_elt (tree type, tree init, int nested, tsubst_flags_t complain, + tree decl) { - init = digest_init_r (type, init, nested ? 2 : 1, LOOKUP_IMPLICIT, complain); + init = digest_init_r (type, init, nested ? 2 : 1, LOOKUP_IMPLICIT, complain, + decl); /* Strip a simple TARGET_EXPR when we know this is an initializer. */ if (SIMPLE_TARGET_EXPR_P (init)) init = TARGET_EXPR_INITIAL (init); @@ -1294,7 +1310,7 @@ static tree static int process_init_constructor_array (tree type, tree init, int nested, - tsubst_flags_t complain) + tsubst_flags_t complain, tree decl) { unsigned HOST_WIDE_INT i, len = 0; int flags = 0; @@ -1347,7 +1363,8 @@ process_init_constructor_array (tree type, tree in ce->index = error_mark_node; gcc_assert (ce->value); ce->value - = massage_init_elt (TREE_TYPE (type), ce->value, nested, complain); + = massage_init_elt (TREE_TYPE (type), ce->value, nested, complain, + decl); gcc_checking_assert (ce->value == error_mark_node @@ -1371,7 +1388,8 @@ process_init_constructor_array (tree type, tree in we can't rely on the back end to do it for us, so make the initialization explicit by list-initializing from T{}. */ next = build_constructor (init_list_type_node, NULL); - next = massage_init_elt (TREE_TYPE (type), next, nested, complain); + next = massage_init_elt (TREE_TYPE (type), next, nested, complain, + decl); if (initializer_zerop (next)) /* The default zero-initialization is fine for us; don't add anything to the CONSTRUCTOR. */ @@ -1417,7 +1435,7 @@ process_init_constructor_array (tree type, tree in static int process_init_constructor_record (tree type, tree init, int nested, - tsubst_flags_t complain) + tsubst_flags_t complain, tree decl) { vec<constructor_elt, va_gc> *v = NULL; tree field; @@ -1499,7 +1517,7 @@ process_init_constructor_record (tree type, tree i if (ce) { gcc_assert (ce->value); - next = massage_init_elt (type, next, nested, complain); + next = massage_init_elt (type, next, nested, complain, decl); ++idx; } } @@ -1528,7 +1546,8 @@ process_init_constructor_record (tree type, tree i for us, so build up TARGET_EXPRs. If the type in question is a class, just build one up; if it's an array, recurse. */ next = build_constructor (init_list_type_node, NULL); - next = massage_init_elt (TREE_TYPE (field), next, nested, complain); + next = massage_init_elt (TREE_TYPE (field), next, nested, complain, + decl); /* Warn when some struct elements are implicitly initialized. */ if ((complain & tf_warning) @@ -1644,7 +1663,7 @@ process_init_constructor_record (tree type, tree i static int process_init_constructor_union (tree type, tree init, int nested, - tsubst_flags_t complain) + tsubst_flags_t complain, tree decl) { constructor_elt *ce; int len; @@ -1731,7 +1750,7 @@ process_init_constructor_union (tree type, tree in if (ce->value && ce->value != error_mark_node) ce->value = massage_init_elt (TREE_TYPE (ce->index), ce->value, nested, - complain); + complain, decl); return picflag_from_initializer (ce->value); } @@ -1752,7 +1771,7 @@ process_init_constructor_union (tree type, tree in static tree process_init_constructor (tree type, tree init, int nested, - tsubst_flags_t complain) + tsubst_flags_t complain, tree decl) { int flags; @@ -1759,11 +1778,11 @@ process_init_constructor (tree type, tree init, in gcc_assert (BRACE_ENCLOSED_INITIALIZER_P (init)); if (TREE_CODE (type) == ARRAY_TYPE || VECTOR_TYPE_P (type)) - flags = process_init_constructor_array (type, init, nested, complain); + flags = process_init_constructor_array (type, init, nested, complain, decl); else if (TREE_CODE (type) == RECORD_TYPE) - flags = process_init_constructor_record (type, init, nested, complain); + flags = process_init_constructor_record (type, init, nested, complain, decl); else if (TREE_CODE (type) == UNION_TYPE) - flags = process_init_constructor_union (type, init, nested, complain); + flags = process_init_constructor_union (type, init, nested, complain, decl); else gcc_unreachable (); Index: gcc/testsuite/c-c++-common/array-6.c =================================================================== --- gcc/testsuite/c-c++-common/array-6.c (revision 267233) +++ gcc/testsuite/c-c++-common/array-6.c (working copy) @@ -16,3 +16,32 @@ void foo() struct str d = (struct str) { 2, "d" }; /* { dg-error "(non-static)|(near initialization)" } */ struct str e = (struct str) { d.len, "e" }; /* { dg-error "(non-static)|(initialization)" } */ } + +struct str f = { 0, {} }; + +void bar() +{ + static struct str g = { 0, {} }; + struct str h = { 0, {} }; /* { dg-error "(non-static)|(near initialization)" } */ + struct str i = (struct str) { 0, {} }; /* { dg-error "(non-static)|(near initialization)" } */ + struct str j = (struct str) { i.len, {} }; /* { dg-error "(non-static)|(initialization)" } */ +} + +struct str k = { 0 }; + +void baz() +{ + static struct str l = { 0 }; + struct str m = { 0 }; + struct str n = (struct str) { 0 }; + struct str o = (struct str) { n.len }; +} + +struct str p = {}; + +void qux() +{ + static struct str q = {}; + struct str r = {}; + struct str s = (struct str) {}; +} Index: gcc/testsuite/g++.dg/ext/flexary12.C =================================================================== --- gcc/testsuite/g++.dg/ext/flexary12.C (revision 267233) +++ gcc/testsuite/g++.dg/ext/flexary12.C (working copy) @@ -12,7 +12,7 @@ struct A { void f1 () { // This is the meat of the test from c++/69290: - struct A a + static struct A a = { "c" }; // { dg-error "invalid conversion from .const char\\*. to .int." } (void)&a; @@ -27,13 +27,13 @@ struct B { void f2 () { - struct B b1 + static struct B b1 = { 0, "c" }; // { dg-error "invalid conversion from .const char\\*. to .int." } (void)&b1; const char s[] = "c"; - struct B b2 + static struct B b2 = { 0, s }; // { dg-error "invalid conversion from .const char\\*. to .int." } (void)&b2; @@ -57,7 +57,7 @@ struct C { void f3 () { - struct C<double> cd + static struct C<double> cd = { "c" }; // { dg-error "cannot convert .const char\\*. to .double." } (void)&cd; Index: gcc/testsuite/g++.dg/ext/flexary13.C =================================================================== --- gcc/testsuite/g++.dg/ext/flexary13.C (revision 267233) +++ gcc/testsuite/g++.dg/ext/flexary13.C (working copy) @@ -19,33 +19,33 @@ int main () ASSERT (s.n == 0); } { - Ax s = + static Ax s = { 0, { } }; // dg-warning "initialization of a flexible array member" } ASSERT (s.n == 0); } { - Ax s = + static Ax s = { 1, { 2 } }; // dg-warning "initialization of a flexible array member" } ASSERT (s.n == 1 && s.a [0] == 2); } { - Ax s = + static Ax s = { 2, { 3, 4 } }; // dg-warning "initialization of a flexible array member" } ASSERT (s.n = 2 && s.a [0] == 3 && s.a [1] == 4); } { - Ax s = + static Ax s = { 123, i }; // dg-warning "initialization of a flexible array member" } ASSERT (s.n == 123 && s.a [0] == i); } { - Ax s = + static Ax s = { 456, { i } }; // dg-warning "initialization of a flexible array member" } ASSERT (s.n == 456 && s.a [0] == i); } { int j = i + 1, k = j + 1; - Ax s = + static Ax s = { 3, { i, j, k } }; // dg-warning "initialization of a flexible array member" } ASSERT (s.n == 3 && s.a [0] == i && s.a [1] == j && s.a [2] == k); } Index: gcc/testsuite/g++.dg/ext/flexary15.C =================================================================== --- gcc/testsuite/g++.dg/ext/flexary15.C (revision 267233) +++ gcc/testsuite/g++.dg/ext/flexary15.C (working copy) @@ -10,5 +10,5 @@ struct S { void foo (const char *a) { - const S s = { 1, { a, "b" } }; // { dg-warning "invalid conversion" } + static const S s = { 1, { a, "b" } }; // { dg-warning "invalid conversion" } } Index: gcc/testsuite/g++.dg/ext/flexary3.C =================================================================== --- gcc/testsuite/g++.dg/ext/flexary3.C (revision 267233) +++ gcc/testsuite/g++.dg/ext/flexary3.C (working copy) @@ -17,5 +17,6 @@ struct s { int main() { struct s s = { .c = 0 }; // { dg-error "initializer" } + // { dg-error "non-static initialization of a flexible array member" "" { target *-*-* } .-1 } return 0; } Index: gcc/testsuite/g++.dg/warn/Wplacement-new-size-1.C =================================================================== --- gcc/testsuite/g++.dg/warn/Wplacement-new-size-1.C (revision 267233) +++ gcc/testsuite/g++.dg/warn/Wplacement-new-size-1.C (working copy) @@ -28,7 +28,7 @@ void fAx (Ax *px, Ax &rx) void fAx2 () { - Ax ax2 = { 1, { 2, 3 } }; + static Ax ax2 = { 1, { 2, 3 } }; new (ax2.a) Int16; new (ax2.a) Int32; // { dg-warning "placement" } @@ -82,7 +82,7 @@ void fBx (BAx *pbx, BAx &rbx) void fBx1 () { - BAx bax1 = { 1, /* Ax = */ { 2, /* a[] = */ {} } }; + static BAx bax1 = { 1, /* Ax = */ { 2, /* a[] = */ {} } }; new (bax1.ax.a) char; // { dg-warning "placement" } new (bax1.ax.a) char[2]; // { dg-warning "placement" } Index: gcc/testsuite/g++.dg/warn/Wplacement-new-size-2.C =================================================================== --- gcc/testsuite/g++.dg/warn/Wplacement-new-size-2.C (revision 267233) +++ gcc/testsuite/g++.dg/warn/Wplacement-new-size-2.C (working copy) @@ -33,13 +33,13 @@ void fAx (Ax *px, Ax &rx) void fAx2 () { // Initialization of non-static objects with flexible array members - // isn't allowed in C and should perhaps be disallowed in C++ as + // isn't allowed in C and had to be be disallowed in C++ as // well to avoid c++/69696 - incorrect initialization of block-scope // flexible array members. - Ax ax2 = { 1, { 2, 3 } }; + Ax ax2 = { 1, { 2, 3 } }; // { dg-error "non-static initialization of a flexible array member" } - new (ax2.a) Int16; - new (ax2.a) Int16[1]; + new (ax2.a) Int16; // { dg-warning "placement" } + new (ax2.a) Int16[1]; // { dg-warning "placement" } new (ax2.a) Int16[2]; // { dg-warning "placement" } new (ax2.a) Int32; // { dg-warning "placement" } new (ax2.a) Int32[2]; // { dg-warning "placement" } @@ -140,7 +140,7 @@ void fBx (BAx *pbx, BAx &rbx) void fBx1 () { - BAx bax1 = { 1, /* Ax = */ { 2, /* a[] = */ {} } }; + static BAx bax1 = { 1, /* Ax = */ { 2, /* a[] = */ {} } }; new (bax1.ax.a) char; // { dg-warning "placement" } new (bax1.ax.a) char[2]; // { dg-warning "placement" } Index: gcc/testsuite/g++.dg/warn/Wplacement-new-size-6.C =================================================================== --- gcc/testsuite/g++.dg/warn/Wplacement-new-size-6.C (revision 267233) +++ gcc/testsuite/g++.dg/warn/Wplacement-new-size-6.C (working copy) @@ -15,7 +15,7 @@ struct BAx { int i; Ax ax; }; void fBx1 () { - BAx bax1 = { 1, /* Ax = */ { 2, /* a[] = */ { 3 } } }; // { dg-error "initialization of flexible array member in a nested context" } + static BAx bax1 = { 1, /* Ax = */ { 2, /* a[] = */ { 3 } } }; // { dg-error "initialization of flexible array member in a nested context" } new (bax1.ax.a) char; // { dg-warning "placement" } new (bax1.ax.a) char[2]; // { dg-warning "placement" } @@ -25,7 +25,7 @@ void fBx1 () void fBx2 () { - BAx bax2 = { 1, /* Ax = */ { 2, /* a[] = */ { 3, 4 } } }; // { dg-error "initialization of flexible array member in a nested context" } + static BAx bax2 = { 1, /* Ax = */ { 2, /* a[] = */ { 3, 4 } } }; // { dg-error "initialization of flexible array member in a nested context" } new (bax2.ax.a) char; // { dg-warning "placement" } new (bax2.ax.a) char[2]; // { dg-warning "placement" } @@ -37,7 +37,7 @@ void fBx2 () void fBx3 () { - BAx bax2 = { 1, /* Ax = */ { 3, /* a[] = */ { 4, 5, 6 } } }; // { dg-error "initialization of flexible array member in a nested context" } + static BAx bax2 = { 1, /* Ax = */ { 3, /* a[] = */ { 4, 5, 6 } } }; // { dg-error "initialization of flexible array member in a nested context" } new (bax2.ax.a) char; // { dg-warning "placement" } new (bax2.ax.a) char[2]; // { dg-warning "placement" } Index: gcc/testsuite/gcc.dg/array-6.c =================================================================== --- gcc/testsuite/gcc.dg/array-6.c (revision 267233) +++ gcc/testsuite/gcc.dg/array-6.c (working copy) @@ -1,18 +0,0 @@ -/* PR c/5597 */ -/* { dg-do compile } */ -/* { dg-options "" } */ - -/* Verify that GCC forbids non-static initialization of - flexible array members. */ - -struct str { int len; char s[]; }; - -struct str a = { 2, "a" }; - -void foo() -{ - static struct str b = { 2, "b" }; - struct str c = { 2, "c" }; /* { dg-error "(non-static)|(near initialization)" } */ - struct str d = (struct str) { 2, "d" }; /* { dg-error "(non-static)|(near initialization)" } */ - struct str e = (struct str) { d.len, "e" }; /* { dg-error "(non-static)|(initialization)" } */ -} ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH, C++,rebased] Fix PR c++/88261 2018-12-22 19:38 ` Bernd Edlinger @ 2019-01-04 15:31 ` Bernd Edlinger 2019-01-04 21:23 ` Jason Merrill 0 siblings, 1 reply; 16+ messages in thread From: Bernd Edlinger @ 2019-01-04 15:31 UTC (permalink / raw) To: Martin Sebor, Martin Sebor, Jason Merrill, gcc-patches, Jeff Law, Nathan Sidwell [-- Attachment #1: Type: text/plain, Size: 6148 bytes --] On 12/22/18 7:53 PM, Bernd Edlinger wrote: > On 12/21/18 2:03 AM, Martin Sebor wrote: >> On 12/20/18 2:07 PM, Bernd Edlinger wrote: >>> On 12/20/18 6:50 PM, Martin Sebor wrote: >>>> On 12/20/18 10:46 AM, Martin Sebor wrote: >>>>> On 12/17/18 7:58 AM, Jason Merrill wrote: >>>>>> On 12/15/18 3:36 AM, Bernd Edlinger wrote: >>>>>>> this patch implements an error message, for non-static initialization of a flexible array member. >>>>>>> This duplicates the existing error message from the C-FE, to avoid ICE and wrong code generation >>>>>>> issues, as pointed out in the PR. >>>>>>> >>>>>>> It is a bit funny that a non-functional feature like that has already rather much test coverage. >>>>>>> The most easy adjustment seems to change the existing test cases to use static declarations. >>>>>> >>>>>> Martin, thoughts? >>>>> >>>>> Our high-level goal when tightening up how flexible array members >>>>> are handled in C++ was to accept what's accepted in standard C mode >>>>> and reject (or, at a minimum, warn for) C++ extensions that could >>>>> be relied on in existing code. >>>> >>>> I meant "reject what couldn't be relied on" and "warn for that could >>>> be." >>>> >>> >>> I believe the problem here is effectively that initializing non-static >>> flexible array is not supported by the middle-end. All examples >>> where flexible array members are initialized on automatic variable >>> work only as long as they are simple enough that they are optimized >>> away so that they do not survive until expansion. >>> >>> Take as example gcc/testsuite/g++.dg/ext/flexary13.C, >>> it compiles and runs successfully, but the assertions start to >>> fail if Ax is declared volatile, and at the same time, we know >>> that the automatic variables are allocated in a way that they >>> can overlap and crash at any time. >>> >>> My impression is that the existing C error made the middle-end kind of rely >>> on this behavior. >>> >>> So I think the right thing to do is duplicate the existing C error in >>> the C++ FE. I do not see any automatic variable with initialized flexible >>> data members where it would be safe to only warn about them. >> >> If there are no reasonable use cases that code out there could >> be relying on because none of them works correctly then rejecting >> the initialization makes sense to me. >> >>>> (Sorry for the delay, by the way. I've been migrating to a new machine >>>> this week and things aren't yet working quite like I'm used to.) >>>> >>>>> >>>>> The flexarray tests I added back then were for features that looked >>>>> like intentional extensions and that seemed to work for at least >>>>> some use cases as far as I could tell. What I noticed didn't work >>>>> I created bugs for: 69338, 69696, and 69338 look related, but there >>>>> are others. >>>>> >>>>> I think all these bugs should all be reviewed and a decision made >>>>> about what's intended to work and what continues to be accepted as >>>>> an accident and should be rejected. After that, we can adjust >>>>> the existing tests. >>>>> >>> >>> I would not rule out the possibility that there can be more bugs. >>> But I think the existing tests need to avoid the case which evokes >>> the new error. The question is, if changing from automatic to static >>> objects prevents those tests to test what they were originally written for. >>> I believe this is not the case, but I do probably not know all the >>> background here. >> >> IIRC, most of the tests I added were meant to exercise just >> the front-end, not any later stages (if that's what you meant). >> Otherwise, if you're worried about the changes from auto to >> static no longer exercising downstream front-end code, whether >> that matters depends on the intent of each test. >> >> flexary13.C was most likely meant to also verify codegen (hence >> the assertions) so I would suggest to make it do that (i.e., >> verify the assertions are optimized out if in fact they are, >> or make the test run so they must pass). >> > > Oh well, unfortunately the modified test case with static objects > fails one assertion when executed at -O0, I missed that before, > because I used -O2 or higher. I filed that as PR 88578, so in the > moment I would like to leave the test case as compile only, > and change that to run once PR 88578 is resolved. > >> The changes to the rest of the flexary*.C tests seem okay, >> though a new test should be added to explicitly exercise this >> change (bug 88261), even if the error happens to be tested by >> one of the changed tests. >> > > That is the case, because the array-6.c test case was moved > to c-c++-common. That is the reproducer for the ICE from the PR. > >> In changes to the Wplacement-new-size*.C tests I would suggest >> to follow the same approach of using statics instead of testing >> for errors so the code that exercises warnings doesn't depend >> on erroneous constructs. >> >> The comment in Wplacement-new-size-2.C just above the code your >> patch changes that reads: >> >> // Initialization of non-static objects with flexible array members >> // isn't allowed in C and should perhaps be disallowed in C++ as >> // well to avoid c++/69696 - incorrect initialization of block-scope >> // flexible array members. >> Ax ax2 = { 1, { 2, 3 } }; >> >> should be updated and the referenced bug and any others that this >> change prevents should be resolved. >> > > Done. > > I also added PR c++/69696 to the changelog as this should definitely > be fixed by this patch as well. > > > So despite the newly discovered problem with the non-constant > initializers which appears to be a separate problem, I would still like > to get an OK for this patch in the current form. > > > Thanks > Bernd. The patch itself is unchanged, the new version fixes a merge conflict due to the recently added parameter stripped_init of process_init_constructor. Bootstrapped and reg-tested on x86_64-pc-linux-gnu. Is it OK for trunk? Thanks Bernd. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: patch-pr88261.diff --] [-- Type: text/x-patch; name="patch-pr88261.diff", Size: 20275 bytes --] gcc/cp: 2018-12-15 Bernd Edlinger <bernd.edlinger@hotmail.de> PR c++/88261 PR c++/69696 * typeck2.c (digest_init_r): Add a decl parameter. Raise an error for non-static initialization of a flexible array member. (process_init_constructor, digest_init_flags, massage_init_elt, process_init_constructor_array, process_init_constructor_record, process_init_constructor_union, process_init_constructor): Add a decl parameter and pass it thru. (digest_nsdmi_init): Pass decl parameter to digest_init_flags. (digest_init): Pass NULL as decl parameter to digest_init_r. * semantics.c (finish_compound_literal): Likewise. * cp-tree.h (digest_init_flags): Adjust prototype. gcc/testsuite: 2018-12-15 Bernd Edlinger <bernd.edlinger@hotmail.de> PR c++/88261 PR c++/69696 * gcc.dg/array-6.c: Move from here ... * c-c++-common/array-6.c: ... to here and add some more test coverage. * g++.dg/ext/flexary3.C: Adjust test. * g++.dg/ext/flexary12.C: Likewise. * g++.dg/ext/flexary13.C: Likewise. * g++.dg/ext/flexary15.C: Likewise. * g++.dg/warn/Wplacement-new-size-1.C: Likewise. * g++.dg/warn/Wplacement-new-size-2.C: Likewise. * g++.dg/warn/Wplacement-new-size-6.C: Likewise. Index: gcc/cp/cp-tree.h =================================================================== --- gcc/cp/cp-tree.h (revision 267569) +++ gcc/cp/cp-tree.h (working copy) @@ -7485,7 +7485,8 @@ extern tree split_nonconstant_init (tree, tree); extern bool check_narrowing (tree, tree, tsubst_flags_t, bool = false); extern tree digest_init (tree, tree, tsubst_flags_t); -extern tree digest_init_flags (tree, tree, int, tsubst_flags_t); +extern tree digest_init_flags (tree, tree, int, + tsubst_flags_t, tree); extern tree digest_nsdmi_init (tree, tree, tsubst_flags_t); extern tree build_scoped_ref (tree, tree, tree *); extern tree build_x_arrow (location_t, tree, Index: gcc/cp/semantics.c =================================================================== --- gcc/cp/semantics.c (revision 267569) +++ gcc/cp/semantics.c (working copy) @@ -2835,7 +2835,7 @@ finish_compound_literal (tree type, tree compound_ return error_mark_node; } compound_literal = digest_init_flags (type, compound_literal, LOOKUP_NORMAL, - complain); + complain, NULL_TREE); if (TREE_CODE (compound_literal) == CONSTRUCTOR) { TREE_HAS_CONSTRUCTOR (compound_literal) = true; Index: gcc/cp/typeck2.c =================================================================== --- gcc/cp/typeck2.c (revision 267569) +++ gcc/cp/typeck2.c (working copy) @@ -36,7 +36,7 @@ along with GCC; see the file COPYING3. If not see static tree process_init_constructor (tree type, tree init, int nested, - tsubst_flags_t complain); + tsubst_flags_t complain, tree decl); /* Print an error message stemming from an attempt to use @@ -818,7 +818,7 @@ store_init_value (tree decl, tree init, vec<tree, value = init; else /* Digest the specified initializer into an expression. */ - value = digest_init_flags (type, init, flags, tf_warning_or_error); + value = digest_init_flags (type, init, flags, tf_warning_or_error, decl); if (TREE_CODE (type) == ARRAY_TYPE && TYPE_STRING_FLAG (TREE_TYPE (type)) @@ -1030,11 +1030,14 @@ check_narrowing (tree type, tree init, tsubst_flag initializer will have the right shape (brace elision has been undone). NESTED is non-zero iff we are being called for an element of a CONSTRUCTOR, - 2 iff the element of a CONSTRUCTOR is inside another CONSTRUCTOR. */ + 2 iff the element of a CONSTRUCTOR is inside another CONSTRUCTOR. + DECL points to the decl which is being initialized. It may be null if + the decl is unknown. In that case, assume it will be non-static. */ + static tree digest_init_r (tree type, tree init, int nested, int flags, - tsubst_flags_t complain) + tsubst_flags_t complain, tree decl) { enum tree_code code = TREE_CODE (type); @@ -1068,8 +1071,18 @@ digest_init_r (tree type, tree init, int nested, i { if (nested && !TYPE_DOMAIN (type)) /* C++ flexible array members have a null domain. */ - pedwarn (loc, OPT_Wpedantic, - "initialization of a flexible array member"); + { + if (decl && TREE_STATIC (decl)) + pedwarn (loc, OPT_Wpedantic, + "initialization of a flexible array member"); + else + { + if (complain & tf_error) + error_at (loc, "non-static initialization of" + " a flexible array member"); + return error_mark_node; + } + } tree typ1 = TYPE_MAIN_VARIANT (TREE_TYPE (type)); if (char_type_p (typ1) @@ -1193,7 +1206,8 @@ digest_init_r (tree type, tree init, int nested, i if (BRACE_ENCLOSED_INITIALIZER_P (stripped_init) && !TYPE_NON_AGGREGATE_CLASS (type)) - return process_init_constructor (type, stripped_init, nested, complain); + return process_init_constructor (type, stripped_init, nested, complain, + decl); else { if (COMPOUND_LITERAL_P (stripped_init) && code == ARRAY_TYPE) @@ -1232,13 +1246,14 @@ digest_init_r (tree type, tree init, int nested, i tree digest_init (tree type, tree init, tsubst_flags_t complain) { - return digest_init_r (type, init, 0, LOOKUP_IMPLICIT, complain); + return digest_init_r (type, init, 0, LOOKUP_IMPLICIT, complain, NULL_TREE); } tree -digest_init_flags (tree type, tree init, int flags, tsubst_flags_t complain) +digest_init_flags (tree type, tree init, int flags, tsubst_flags_t complain, + tree decl) { - return digest_init_r (type, init, 0, flags, complain); + return digest_init_r (type, init, 0, flags, complain, decl); } /* Process the initializer INIT for an NSDMI DECL (a FIELD_DECL). */ @@ -1254,7 +1269,7 @@ digest_nsdmi_init (tree decl, tree init, tsubst_fl if (BRACE_ENCLOSED_INITIALIZER_P (init) && CP_AGGREGATE_TYPE_P (type)) init = reshape_init (type, init, complain); - init = digest_init_flags (type, init, flags, complain); + init = digest_init_flags (type, init, flags, complain, decl); if (TREE_CODE (init) == TARGET_EXPR) /* This represents the whole initialization. */ TARGET_EXPR_DIRECT_INIT_P (init) = true; @@ -1291,9 +1306,11 @@ picflag_from_initializer (tree init) /* Adjust INIT for going into a CONSTRUCTOR. */ static tree -massage_init_elt (tree type, tree init, int nested, tsubst_flags_t complain) +massage_init_elt (tree type, tree init, int nested, tsubst_flags_t complain, + tree decl) { - init = digest_init_r (type, init, nested ? 2 : 1, LOOKUP_IMPLICIT, complain); + init = digest_init_r (type, init, nested ? 2 : 1, LOOKUP_IMPLICIT, complain, + decl); /* Strip a simple TARGET_EXPR when we know this is an initializer. */ if (SIMPLE_TARGET_EXPR_P (init)) init = TARGET_EXPR_INITIAL (init); @@ -1312,7 +1329,7 @@ static tree static int process_init_constructor_array (tree type, tree init, int nested, - tsubst_flags_t complain) + tsubst_flags_t complain, tree decl) { unsigned HOST_WIDE_INT i, len = 0; int flags = 0; @@ -1365,7 +1382,8 @@ process_init_constructor_array (tree type, tree in ce->index = error_mark_node; gcc_assert (ce->value); ce->value - = massage_init_elt (TREE_TYPE (type), ce->value, nested, complain); + = massage_init_elt (TREE_TYPE (type), ce->value, nested, complain, + decl); gcc_checking_assert (ce->value == error_mark_node @@ -1389,7 +1407,8 @@ process_init_constructor_array (tree type, tree in we can't rely on the back end to do it for us, so make the initialization explicit by list-initializing from T{}. */ next = build_constructor (init_list_type_node, NULL); - next = massage_init_elt (TREE_TYPE (type), next, nested, complain); + next = massage_init_elt (TREE_TYPE (type), next, nested, complain, + decl); if (initializer_zerop (next)) /* The default zero-initialization is fine for us; don't add anything to the CONSTRUCTOR. */ @@ -1435,7 +1454,7 @@ process_init_constructor_array (tree type, tree in static int process_init_constructor_record (tree type, tree init, int nested, - tsubst_flags_t complain) + tsubst_flags_t complain, tree decl) { vec<constructor_elt, va_gc> *v = NULL; tree field; @@ -1517,7 +1536,7 @@ process_init_constructor_record (tree type, tree i if (ce) { gcc_assert (ce->value); - next = massage_init_elt (type, next, nested, complain); + next = massage_init_elt (type, next, nested, complain, decl); ++idx; } } @@ -1546,7 +1565,8 @@ process_init_constructor_record (tree type, tree i for us, so build up TARGET_EXPRs. If the type in question is a class, just build one up; if it's an array, recurse. */ next = build_constructor (init_list_type_node, NULL); - next = massage_init_elt (TREE_TYPE (field), next, nested, complain); + next = massage_init_elt (TREE_TYPE (field), next, nested, complain, + decl); /* Warn when some struct elements are implicitly initialized. */ if ((complain & tf_warning) @@ -1662,7 +1682,7 @@ process_init_constructor_record (tree type, tree i static int process_init_constructor_union (tree type, tree init, int nested, - tsubst_flags_t complain) + tsubst_flags_t complain, tree decl) { constructor_elt *ce; int len; @@ -1749,7 +1769,7 @@ process_init_constructor_union (tree type, tree in if (ce->value && ce->value != error_mark_node) ce->value = massage_init_elt (TREE_TYPE (ce->index), ce->value, nested, - complain); + complain, decl); return picflag_from_initializer (ce->value); } @@ -1770,7 +1790,7 @@ process_init_constructor_union (tree type, tree in static tree process_init_constructor (tree type, tree init, int nested, - tsubst_flags_t complain) + tsubst_flags_t complain, tree decl) { int flags; @@ -1777,11 +1797,11 @@ process_init_constructor (tree type, tree init, in gcc_assert (BRACE_ENCLOSED_INITIALIZER_P (init)); if (TREE_CODE (type) == ARRAY_TYPE || VECTOR_TYPE_P (type)) - flags = process_init_constructor_array (type, init, nested, complain); + flags = process_init_constructor_array (type, init, nested, complain, decl); else if (TREE_CODE (type) == RECORD_TYPE) - flags = process_init_constructor_record (type, init, nested, complain); + flags = process_init_constructor_record (type, init, nested, complain, decl); else if (TREE_CODE (type) == UNION_TYPE) - flags = process_init_constructor_union (type, init, nested, complain); + flags = process_init_constructor_union (type, init, nested, complain, decl); else gcc_unreachable (); Index: gcc/testsuite/c-c++-common/array-6.c =================================================================== --- gcc/testsuite/c-c++-common/array-6.c (revision 267569) +++ gcc/testsuite/c-c++-common/array-6.c (working copy) @@ -16,3 +16,32 @@ void foo() struct str d = (struct str) { 2, "d" }; /* { dg-error "(non-static)|(near initialization)" } */ struct str e = (struct str) { d.len, "e" }; /* { dg-error "(non-static)|(initialization)" } */ } + +struct str f = { 0, {} }; + +void bar() +{ + static struct str g = { 0, {} }; + struct str h = { 0, {} }; /* { dg-error "(non-static)|(near initialization)" } */ + struct str i = (struct str) { 0, {} }; /* { dg-error "(non-static)|(near initialization)" } */ + struct str j = (struct str) { i.len, {} }; /* { dg-error "(non-static)|(initialization)" } */ +} + +struct str k = { 0 }; + +void baz() +{ + static struct str l = { 0 }; + struct str m = { 0 }; + struct str n = (struct str) { 0 }; + struct str o = (struct str) { n.len }; +} + +struct str p = {}; + +void qux() +{ + static struct str q = {}; + struct str r = {}; + struct str s = (struct str) {}; +} Index: gcc/testsuite/g++.dg/ext/flexary12.C =================================================================== --- gcc/testsuite/g++.dg/ext/flexary12.C (revision 267569) +++ gcc/testsuite/g++.dg/ext/flexary12.C (working copy) @@ -12,7 +12,7 @@ struct A { void f1 () { // This is the meat of the test from c++/69290: - struct A a + static struct A a = { "c" }; // { dg-error "invalid conversion from .const char\\*. to .int." } (void)&a; @@ -27,13 +27,13 @@ struct B { void f2 () { - struct B b1 + static struct B b1 = { 0, "c" }; // { dg-error "invalid conversion from .const char\\*. to .int." } (void)&b1; const char s[] = "c"; - struct B b2 + static struct B b2 = { 0, s }; // { dg-error "invalid conversion from .const char\\*. to .int." } (void)&b2; @@ -57,7 +57,7 @@ struct C { void f3 () { - struct C<double> cd + static struct C<double> cd = { "c" }; // { dg-error "cannot convert .const char\\*. to .double." } (void)&cd; Index: gcc/testsuite/g++.dg/ext/flexary13.C =================================================================== --- gcc/testsuite/g++.dg/ext/flexary13.C (revision 267569) +++ gcc/testsuite/g++.dg/ext/flexary13.C (working copy) @@ -19,33 +19,33 @@ int main () ASSERT (s.n == 0); } { - Ax s = + static Ax s = { 0, { } }; // dg-warning "initialization of a flexible array member" } ASSERT (s.n == 0); } { - Ax s = + static Ax s = { 1, { 2 } }; // dg-warning "initialization of a flexible array member" } ASSERT (s.n == 1 && s.a [0] == 2); } { - Ax s = + static Ax s = { 2, { 3, 4 } }; // dg-warning "initialization of a flexible array member" } ASSERT (s.n = 2 && s.a [0] == 3 && s.a [1] == 4); } { - Ax s = + static Ax s = { 123, i }; // dg-warning "initialization of a flexible array member" } ASSERT (s.n == 123 && s.a [0] == i); } { - Ax s = + static Ax s = { 456, { i } }; // dg-warning "initialization of a flexible array member" } ASSERT (s.n == 456 && s.a [0] == i); } { int j = i + 1, k = j + 1; - Ax s = + static Ax s = { 3, { i, j, k } }; // dg-warning "initialization of a flexible array member" } ASSERT (s.n == 3 && s.a [0] == i && s.a [1] == j && s.a [2] == k); } Index: gcc/testsuite/g++.dg/ext/flexary15.C =================================================================== --- gcc/testsuite/g++.dg/ext/flexary15.C (revision 267569) +++ gcc/testsuite/g++.dg/ext/flexary15.C (working copy) @@ -10,5 +10,5 @@ struct S { void foo (const char *a) { - const S s = { 1, { a, "b" } }; // { dg-warning "invalid conversion" } + static const S s = { 1, { a, "b" } }; // { dg-warning "invalid conversion" } } Index: gcc/testsuite/g++.dg/ext/flexary3.C =================================================================== --- gcc/testsuite/g++.dg/ext/flexary3.C (revision 267569) +++ gcc/testsuite/g++.dg/ext/flexary3.C (working copy) @@ -17,5 +17,6 @@ struct s { int main() { struct s s = { .c = 0 }; // { dg-error "initializer" } + // { dg-error "non-static initialization of a flexible array member" "" { target *-*-* } .-1 } return 0; } Index: gcc/testsuite/g++.dg/warn/Wplacement-new-size-1.C =================================================================== --- gcc/testsuite/g++.dg/warn/Wplacement-new-size-1.C (revision 267569) +++ gcc/testsuite/g++.dg/warn/Wplacement-new-size-1.C (working copy) @@ -28,7 +28,7 @@ void fAx (Ax *px, Ax &rx) void fAx2 () { - Ax ax2 = { 1, { 2, 3 } }; + static Ax ax2 = { 1, { 2, 3 } }; new (ax2.a) Int16; new (ax2.a) Int32; // { dg-warning "placement" } @@ -82,7 +82,7 @@ void fBx (BAx *pbx, BAx &rbx) void fBx1 () { - BAx bax1 = { 1, /* Ax = */ { 2, /* a[] = */ {} } }; + static BAx bax1 = { 1, /* Ax = */ { 2, /* a[] = */ {} } }; new (bax1.ax.a) char; // { dg-warning "placement" } new (bax1.ax.a) char[2]; // { dg-warning "placement" } Index: gcc/testsuite/g++.dg/warn/Wplacement-new-size-2.C =================================================================== --- gcc/testsuite/g++.dg/warn/Wplacement-new-size-2.C (revision 267569) +++ gcc/testsuite/g++.dg/warn/Wplacement-new-size-2.C (working copy) @@ -33,13 +33,13 @@ void fAx (Ax *px, Ax &rx) void fAx2 () { // Initialization of non-static objects with flexible array members - // isn't allowed in C and should perhaps be disallowed in C++ as + // isn't allowed in C and had to be be disallowed in C++ as // well to avoid c++/69696 - incorrect initialization of block-scope // flexible array members. - Ax ax2 = { 1, { 2, 3 } }; + Ax ax2 = { 1, { 2, 3 } }; // { dg-error "non-static initialization of a flexible array member" } - new (ax2.a) Int16; - new (ax2.a) Int16[1]; + new (ax2.a) Int16; // { dg-warning "placement" } + new (ax2.a) Int16[1]; // { dg-warning "placement" } new (ax2.a) Int16[2]; // { dg-warning "placement" } new (ax2.a) Int32; // { dg-warning "placement" } new (ax2.a) Int32[2]; // { dg-warning "placement" } @@ -140,7 +140,7 @@ void fBx (BAx *pbx, BAx &rbx) void fBx1 () { - BAx bax1 = { 1, /* Ax = */ { 2, /* a[] = */ {} } }; + static BAx bax1 = { 1, /* Ax = */ { 2, /* a[] = */ {} } }; new (bax1.ax.a) char; // { dg-warning "placement" } new (bax1.ax.a) char[2]; // { dg-warning "placement" } Index: gcc/testsuite/g++.dg/warn/Wplacement-new-size-6.C =================================================================== --- gcc/testsuite/g++.dg/warn/Wplacement-new-size-6.C (revision 267569) +++ gcc/testsuite/g++.dg/warn/Wplacement-new-size-6.C (working copy) @@ -15,7 +15,7 @@ struct BAx { int i; Ax ax; }; void fBx1 () { - BAx bax1 = { 1, /* Ax = */ { 2, /* a[] = */ { 3 } } }; // { dg-error "initialization of flexible array member in a nested context" } + static BAx bax1 = { 1, /* Ax = */ { 2, /* a[] = */ { 3 } } }; // { dg-error "initialization of flexible array member in a nested context" } new (bax1.ax.a) char; // { dg-warning "placement" } new (bax1.ax.a) char[2]; // { dg-warning "placement" } @@ -25,7 +25,7 @@ void fBx1 () void fBx2 () { - BAx bax2 = { 1, /* Ax = */ { 2, /* a[] = */ { 3, 4 } } }; // { dg-error "initialization of flexible array member in a nested context" } + static BAx bax2 = { 1, /* Ax = */ { 2, /* a[] = */ { 3, 4 } } }; // { dg-error "initialization of flexible array member in a nested context" } new (bax2.ax.a) char; // { dg-warning "placement" } new (bax2.ax.a) char[2]; // { dg-warning "placement" } @@ -37,7 +37,7 @@ void fBx2 () void fBx3 () { - BAx bax2 = { 1, /* Ax = */ { 3, /* a[] = */ { 4, 5, 6 } } }; // { dg-error "initialization of flexible array member in a nested context" } + static BAx bax2 = { 1, /* Ax = */ { 3, /* a[] = */ { 4, 5, 6 } } }; // { dg-error "initialization of flexible array member in a nested context" } new (bax2.ax.a) char; // { dg-warning "placement" } new (bax2.ax.a) char[2]; // { dg-warning "placement" } Index: gcc/testsuite/gcc.dg/array-6.c =================================================================== --- gcc/testsuite/gcc.dg/array-6.c (revision 267569) +++ gcc/testsuite/gcc.dg/array-6.c (working copy) @@ -1,18 +0,0 @@ -/* PR c/5597 */ -/* { dg-do compile } */ -/* { dg-options "" } */ - -/* Verify that GCC forbids non-static initialization of - flexible array members. */ - -struct str { int len; char s[]; }; - -struct str a = { 2, "a" }; - -void foo() -{ - static struct str b = { 2, "b" }; - struct str c = { 2, "c" }; /* { dg-error "(non-static)|(near initialization)" } */ - struct str d = (struct str) { 2, "d" }; /* { dg-error "(non-static)|(near initialization)" } */ - struct str e = (struct str) { d.len, "e" }; /* { dg-error "(non-static)|(initialization)" } */ -} ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH, C++,rebased] Fix PR c++/88261 2019-01-04 15:31 ` [PATCH, C++,rebased] " Bernd Edlinger @ 2019-01-04 21:23 ` Jason Merrill 2019-01-04 22:23 ` Bernd Edlinger 2019-01-05 16:05 ` Bernd Edlinger 0 siblings, 2 replies; 16+ messages in thread From: Jason Merrill @ 2019-01-04 21:23 UTC (permalink / raw) To: Bernd Edlinger, Martin Sebor, Martin Sebor, gcc-patches, Jeff Law, Nathan Sidwell On 1/4/19 10:30 AM, Bernd Edlinger wrote: > On 12/22/18 7:53 PM, Bernd Edlinger wrote: >> On 12/21/18 2:03 AM, Martin Sebor wrote: >>> On 12/20/18 2:07 PM, Bernd Edlinger wrote: >>>> On 12/20/18 6:50 PM, Martin Sebor wrote: >>>>> On 12/20/18 10:46 AM, Martin Sebor wrote: >>>>>> On 12/17/18 7:58 AM, Jason Merrill wrote: >>>>>>> On 12/15/18 3:36 AM, Bernd Edlinger wrote: >>>>>>>> this patch implements an error message, for non-static initialization of a flexible array member. >>>>>>>> This duplicates the existing error message from the C-FE, to avoid ICE and wrong code generation >>>>>>>> issues, as pointed out in the PR. >>>>>>>> >>>>>>>> It is a bit funny that a non-functional feature like that has already rather much test coverage. >>>>>>>> The most easy adjustment seems to change the existing test cases to use static declarations. >>>>>>> >>>>>>> Martin, thoughts? >>>>>> >>>>>> Our high-level goal when tightening up how flexible array members >>>>>> are handled in C++ was to accept what's accepted in standard C mode >>>>>> and reject (or, at a minimum, warn for) C++ extensions that could >>>>>> be relied on in existing code. >>>>> >>>>> I meant "reject what couldn't be relied on" and "warn for that could >>>>> be." >>>>> >>>> >>>> I believe the problem here is effectively that initializing non-static >>>> flexible array is not supported by the middle-end. All examples >>>> where flexible array members are initialized on automatic variable >>>> work only as long as they are simple enough that they are optimized >>>> away so that they do not survive until expansion. >>>> >>>> Take as example gcc/testsuite/g++.dg/ext/flexary13.C, >>>> it compiles and runs successfully, but the assertions start to >>>> fail if Ax is declared volatile, and at the same time, we know >>>> that the automatic variables are allocated in a way that they >>>> can overlap and crash at any time. >>>> >>>> My impression is that the existing C error made the middle-end kind of rely >>>> on this behavior. >>>> >>>> So I think the right thing to do is duplicate the existing C error in >>>> the C++ FE. I do not see any automatic variable with initialized flexible >>>> data members where it would be safe to only warn about them. >>> >>> If there are no reasonable use cases that code out there could >>> be relying on because none of them works correctly then rejecting >>> the initialization makes sense to me. >>> >>>>> (Sorry for the delay, by the way. I've been migrating to a new machine >>>>> this week and things aren't yet working quite like I'm used to.) >>>>> >>>>>> >>>>>> The flexarray tests I added back then were for features that looked >>>>>> like intentional extensions and that seemed to work for at least >>>>>> some use cases as far as I could tell. What I noticed didn't work >>>>>> I created bugs for: 69338, 69696, and 69338 look related, but there >>>>>> are others. >>>>>> >>>>>> I think all these bugs should all be reviewed and a decision made >>>>>> about what's intended to work and what continues to be accepted as >>>>>> an accident and should be rejected. After that, we can adjust >>>>>> the existing tests. >>>>>> >>>> >>>> I would not rule out the possibility that there can be more bugs. >>>> But I think the existing tests need to avoid the case which evokes >>>> the new error. The question is, if changing from automatic to static >>>> objects prevents those tests to test what they were originally written for. >>>> I believe this is not the case, but I do probably not know all the >>>> background here. >>> >>> IIRC, most of the tests I added were meant to exercise just >>> the front-end, not any later stages (if that's what you meant). >>> Otherwise, if you're worried about the changes from auto to >>> static no longer exercising downstream front-end code, whether >>> that matters depends on the intent of each test. >>> >>> flexary13.C was most likely meant to also verify codegen (hence >>> the assertions) so I would suggest to make it do that (i.e., >>> verify the assertions are optimized out if in fact they are, >>> or make the test run so they must pass). >>> >> >> Oh well, unfortunately the modified test case with static objects >> fails one assertion when executed at -O0, I missed that before, >> because I used -O2 or higher. I filed that as PR 88578, so in the >> moment I would like to leave the test case as compile only, >> and change that to run once PR 88578 is resolved. >> >>> The changes to the rest of the flexary*.C tests seem okay, >>> though a new test should be added to explicitly exercise this >>> change (bug 88261), even if the error happens to be tested by >>> one of the changed tests. >>> >> >> That is the case, because the array-6.c test case was moved >> to c-c++-common. That is the reproducer for the ICE from the PR. >> >>> In changes to the Wplacement-new-size*.C tests I would suggest >>> to follow the same approach of using statics instead of testing >>> for errors so the code that exercises warnings doesn't depend >>> on erroneous constructs. >>> >>> The comment in Wplacement-new-size-2.C just above the code your >>> patch changes that reads: >>> >>>   // Initialization of non-static objects with flexible array members >>>   // isn't allowed in C and should perhaps be disallowed in C++ as >>>   // well to avoid c++/69696 - incorrect initialization of block-scope >>>   // flexible array members. >>>   Ax ax2 = { 1, { 2, 3 } }; >>> >>> should be updated and the referenced bug and any others that this >>> change prevents should be resolved. >>> >> >> Done. >> >> I also added PR c++/69696 to the changelog as this should definitely >> be fixed by this patch as well. >> >> >> So despite the newly discovered problem with the non-constant >> initializers which appears to be a separate problem, I would still like >> to get an OK for this patch in the current form. Hmm, I'm uncomfortable with starting to pass in the decl just for the sake of deciding whether this diagnostic should be a pedwarn or error. In general, because of copy elision, we can't know at this point what we're initializing, so I'd rather not pretend we can. Instead, maybe add a LOOKUP_ALLOW_FLEXARY_INIT flag that you can add to the flags argument in the call from store_init_value? Jason ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH, C++,rebased] Fix PR c++/88261 2019-01-04 21:23 ` Jason Merrill @ 2019-01-04 22:23 ` Bernd Edlinger 2019-01-05 16:05 ` Bernd Edlinger 1 sibling, 0 replies; 16+ messages in thread From: Bernd Edlinger @ 2019-01-04 22:23 UTC (permalink / raw) To: Jason Merrill, Martin Sebor, Martin Sebor, gcc-patches, Jeff Law, Nathan Sidwell On 1/4/19 10:22 PM, Jason Merrill wrote: > On 1/4/19 10:30 AM, Bernd Edlinger wrote: >> On 12/22/18 7:53 PM, Bernd Edlinger wrote: >>> On 12/21/18 2:03 AM, Martin Sebor wrote: >>>> On 12/20/18 2:07 PM, Bernd Edlinger wrote: >>>>> On 12/20/18 6:50 PM, Martin Sebor wrote: >>>>>> On 12/20/18 10:46 AM, Martin Sebor wrote: >>>>>>> On 12/17/18 7:58 AM, Jason Merrill wrote: >>>>>>>> On 12/15/18 3:36 AM, Bernd Edlinger wrote: >>>>>>>>> this patch implements an error message, for non-static initialization of a flexible array member. >>>>>>>>> This duplicates the existing error message from the C-FE, to avoid ICE and wrong code generation >>>>>>>>> issues, as pointed out in the PR. >>>>>>>>> >>>>>>>>> It is a bit funny that a non-functional feature like that has already rather much test coverage. >>>>>>>>> The most easy adjustment seems to change the existing test cases to use static declarations. >>>>>>>> >>>>>>>> Martin, thoughts? >>>>>>> >>>>>>> Our high-level goal when tightening up how flexible array members >>>>>>> are handled in C++ was to accept what's accepted in standard C mode >>>>>>> and reject (or, at a minimum, warn for) C++ extensions that could >>>>>>> be relied on in existing code. >>>>>> >>>>>> I meant "reject what couldn't be relied on" and "warn for that could >>>>>> be." >>>>>> >>>>> >>>>> I believe the problem here is effectively that initializing non-static >>>>> flexible array is not supported by the middle-end. All examples >>>>> where flexible array members are initialized on automatic variable >>>>> work only as long as they are simple enough that they are optimized >>>>> away so that they do not survive until expansion. >>>>> >>>>> Take as example gcc/testsuite/g++.dg/ext/flexary13.C, >>>>> it compiles and runs successfully, but the assertions start to >>>>> fail if Ax is declared volatile, and at the same time, we know >>>>> that the automatic variables are allocated in a way that they >>>>> can overlap and crash at any time. >>>>> >>>>> My impression is that the existing C error made the middle-end kind of rely >>>>> on this behavior. >>>>> >>>>> So I think the right thing to do is duplicate the existing C error in >>>>> the C++ FE. I do not see any automatic variable with initialized flexible >>>>> data members where it would be safe to only warn about them. >>>> >>>> If there are no reasonable use cases that code out there could >>>> be relying on because none of them works correctly then rejecting >>>> the initialization makes sense to me. >>>> >>>>>> (Sorry for the delay, by the way. I've been migrating to a new machine >>>>>> this week and things aren't yet working quite like I'm used to.) >>>>>> >>>>>>> >>>>>>> The flexarray tests I added back then were for features that looked >>>>>>> like intentional extensions and that seemed to work for at least >>>>>>> some use cases as far as I could tell. What I noticed didn't work >>>>>>> I created bugs for: 69338, 69696, and 69338 look related, but there >>>>>>> are others. >>>>>>> >>>>>>> I think all these bugs should all be reviewed and a decision made >>>>>>> about what's intended to work and what continues to be accepted as >>>>>>> an accident and should be rejected. After that, we can adjust >>>>>>> the existing tests. >>>>>>> >>>>> >>>>> I would not rule out the possibility that there can be more bugs. >>>>> But I think the existing tests need to avoid the case which evokes >>>>> the new error. The question is, if changing from automatic to static >>>>> objects prevents those tests to test what they were originally written for. >>>>> I believe this is not the case, but I do probably not know all the >>>>> background here. >>>> >>>> IIRC, most of the tests I added were meant to exercise just >>>> the front-end, not any later stages (if that's what you meant). >>>> Otherwise, if you're worried about the changes from auto to >>>> static no longer exercising downstream front-end code, whether >>>> that matters depends on the intent of each test. >>>> >>>> flexary13.C was most likely meant to also verify codegen (hence >>>> the assertions) so I would suggest to make it do that (i.e., >>>> verify the assertions are optimized out if in fact they are, >>>> or make the test run so they must pass). >>>> >>> >>> Oh well, unfortunately the modified test case with static objects >>> fails one assertion when executed at -O0, I missed that before, >>> because I used -O2 or higher. I filed that as PR 88578, so in the >>> moment I would like to leave the test case as compile only, >>> and change that to run once PR 88578 is resolved. >>> >>>> The changes to the rest of the flexary*.C tests seem okay, >>>> though a new test should be added to explicitly exercise this >>>> change (bug 88261), even if the error happens to be tested by >>>> one of the changed tests. >>>> >>> >>> That is the case, because the array-6.c test case was moved >>> to c-c++-common. That is the reproducer for the ICE from the PR. >>> >>>> In changes to the Wplacement-new-size*.C tests I would suggest >>>> to follow the same approach of using statics instead of testing >>>> for errors so the code that exercises warnings doesn't depend >>>> on erroneous constructs. >>>> >>>> The comment in Wplacement-new-size-2.C just above the code your >>>> patch changes that reads: >>>> >>>> // Initialization of non-static objects with flexible array members >>>> // isn't allowed in C and should perhaps be disallowed in C++ as >>>> // well to avoid c++/69696 - incorrect initialization of block-scope >>>> // flexible array members. >>>> Ax ax2 = { 1, { 2, 3 } }; >>>> >>>> should be updated and the referenced bug and any others that this >>>> change prevents should be resolved. >>>> >>> >>> Done. >>> >>> I also added PR c++/69696 to the changelog as this should definitely >>> be fixed by this patch as well. >>> >>> >>> So despite the newly discovered problem with the non-constant >>> initializers which appears to be a separate problem, I would still like >>> to get an OK for this patch in the current form. > > Hmm, I'm uncomfortable with starting to pass in the decl just for the sake of deciding whether this diagnostic should be a pedwarn or error. In general, because of copy elision, we can't know at this point what we're initializing, so I'd rather not pretend we can. Instead, maybe add a LOOKUP_ALLOW_FLEXARY_INIT flag that you can add to the flags argument in the call from store_init_value? > Yes that sounds reasonable, just I think the error or warning is always emitted from digest_nsdmi_init. I have never seen that happening from store_init_value. I'll give it a try and come back with an updated patch. Bernd. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH, C++,rebased] Fix PR c++/88261 2019-01-04 21:23 ` Jason Merrill 2019-01-04 22:23 ` Bernd Edlinger @ 2019-01-05 16:05 ` Bernd Edlinger 2019-01-07 0:09 ` Martin Sebor 1 sibling, 1 reply; 16+ messages in thread From: Bernd Edlinger @ 2019-01-05 16:05 UTC (permalink / raw) To: Jason Merrill, Martin Sebor, Martin Sebor, gcc-patches, Jeff Law, Nathan Sidwell [-- Attachment #1: Type: text/plain, Size: 1173 bytes --] On 1/4/19 10:22 PM, Jason Merrill wrote: > Hmm, I'm uncomfortable with starting to pass in the decl just for the sake of deciding whether this diagnostic should be a pedwarn or error. In general, because of copy elision, we can't know at this point what we're initializing, so I'd rather not pretend we can. Instead, maybe add a LOOKUP_ALLOW_FLEXARY_INIT flag that you can add to the flags argument in the call from store_init_value? > Okay, I reworked the patch, to pass a bit in the flags, it was a bit more complicated than anticipated, because it is necessary to pass the flag thru process_init_constructor and friends to the recursive invocation of digest_init_r. It turned out that digest_nsdmi_init did not need to change, since it is always wrong to use flexarray init there. I added a new test case (flexary32.C) to exercises a few cases where non static direct member intializers are allowed to use flexarrays (in static members) and where that would be wrong (in automatic members). So that seems to work. New version of the patch is attached. Bootstrapped and reg-tested on x86_64-pc-linux-gnu. Is it OK for trunk? Thanks Bernd. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: patch-pr88261v2.diff --] [-- Type: text/x-patch; name="patch-pr88261v2.diff", Size: 21212 bytes --] gcc/cp: 2019-01-05 Bernd Edlinger <bernd.edlinger@hotmail.de> PR c++/88261 PR c++/69696 * cp-tree.h (LOOKUP_ALLOW_FLEXARRAY_INIT): New flag value. * typeck2.c (digest_init_r): Raise an error for non-static initialization of a flexible array member. (process_init_constructor, massage_init_elt, process_init_constructor_array, process_init_constructor_record, process_init_constructor_union, process_init_constructor): Add the flags parameter and pass it thru. (store_init_value): Pass LOOKUP_ALLOW_FLEXARRAY_INIT parameter to digest_init_flags for static decls. gcc/testsuite: 2019-01-05 Bernd Edlinger <bernd.edlinger@hotmail.de> PR c++/88261 PR c++/69696 * gcc.dg/array-6.c: Move from here ... * c-c++-common/array-6.c: ... to here and add some more test coverage. * g++.dg/ext/flexary32.C: New test. * g++.dg/ext/flexary3.C: Adjust test. * g++.dg/ext/flexary12.C: Likewise. * g++.dg/ext/flexary13.C: Likewise. * g++.dg/ext/flexary15.C: Likewise. * g++.dg/warn/Wplacement-new-size-1.C: Likewise. * g++.dg/warn/Wplacement-new-size-2.C: Likewise. * g++.dg/warn/Wplacement-new-size-6.C: Likewise. Index: gcc/cp/cp-tree.h Index: gcc/cp/cp-tree.h =================================================================== --- gcc/cp/cp-tree.h (revision 267569) +++ gcc/cp/cp-tree.h (working copy) @@ -5454,6 +5454,8 @@ enum overload_flags { NO_SPECIAL = 0, DTOR_FLAG, T #define LOOKUP_NO_NON_INTEGRAL (LOOKUP_NO_RVAL_BIND << 1) /* Used for delegating constructors in order to diagnose self-delegation. */ #define LOOKUP_DELEGATING_CONS (LOOKUP_NO_NON_INTEGRAL << 1) +/* Allow initialization of a flexible array members. */ +#define LOOKUP_ALLOW_FLEXARRAY_INIT (LOOKUP_DELEGATING_CONS << 1) #define LOOKUP_NAMESPACES_ONLY(F) \ (((F) & LOOKUP_PREFER_NAMESPACES) && !((F) & LOOKUP_PREFER_TYPES)) Index: gcc/cp/typeck2.c =================================================================== --- gcc/cp/typeck2.c (revision 267569) +++ gcc/cp/typeck2.c (working copy) @@ -35,7 +35,7 @@ along with GCC; see the file COPYING3. If not see #include "gcc-rich-location.h" static tree -process_init_constructor (tree type, tree init, int nested, +process_init_constructor (tree type, tree init, int nested, int flags, tsubst_flags_t complain); @@ -817,8 +817,12 @@ store_init_value (tree decl, tree init, vec<tree, if (flags & LOOKUP_ALREADY_DIGESTED) value = init; else - /* Digest the specified initializer into an expression. */ - value = digest_init_flags (type, init, flags, tf_warning_or_error); + { + if (TREE_STATIC (decl)) + flags |= LOOKUP_ALLOW_FLEXARRAY_INIT; + /* Digest the specified initializer into an expression. */ + value = digest_init_flags (type, init, flags, tf_warning_or_error); + } if (TREE_CODE (type) == ARRAY_TYPE && TYPE_STRING_FLAG (TREE_TYPE (type)) @@ -1068,8 +1072,18 @@ digest_init_r (tree type, tree init, int nested, i { if (nested && !TYPE_DOMAIN (type)) /* C++ flexible array members have a null domain. */ - pedwarn (loc, OPT_Wpedantic, - "initialization of a flexible array member"); + { + if (flags & LOOKUP_ALLOW_FLEXARRAY_INIT) + pedwarn (loc, OPT_Wpedantic, + "initialization of a flexible array member"); + else + { + if (complain & tf_error) + error_at (loc, "non-static initialization of" + " a flexible array member"); + return error_mark_node; + } + } tree typ1 = TYPE_MAIN_VARIANT (TREE_TYPE (type)); if (char_type_p (typ1) @@ -1193,7 +1207,8 @@ digest_init_r (tree type, tree init, int nested, i if (BRACE_ENCLOSED_INITIALIZER_P (stripped_init) && !TYPE_NON_AGGREGATE_CLASS (type)) - return process_init_constructor (type, stripped_init, nested, complain); + return process_init_constructor (type, stripped_init, nested, flags, + complain); else { if (COMPOUND_LITERAL_P (stripped_init) && code == ARRAY_TYPE) @@ -1291,9 +1306,12 @@ picflag_from_initializer (tree init) /* Adjust INIT for going into a CONSTRUCTOR. */ static tree -massage_init_elt (tree type, tree init, int nested, tsubst_flags_t complain) +massage_init_elt (tree type, tree init, int nested, int flags, + tsubst_flags_t complain) { - init = digest_init_r (type, init, nested ? 2 : 1, LOOKUP_IMPLICIT, complain); + flags &= LOOKUP_ALLOW_FLEXARRAY_INIT; + flags |= LOOKUP_IMPLICIT; + init = digest_init_r (type, init, nested ? 2 : 1, flags, complain); /* Strip a simple TARGET_EXPR when we know this is an initializer. */ if (SIMPLE_TARGET_EXPR_P (init)) init = TARGET_EXPR_INITIAL (init); @@ -1311,11 +1329,11 @@ static tree which describe the initializers. */ static int -process_init_constructor_array (tree type, tree init, int nested, +process_init_constructor_array (tree type, tree init, int nested, int flags, tsubst_flags_t complain) { unsigned HOST_WIDE_INT i, len = 0; - int flags = 0; + int picflags = 0; bool unbounded = false; constructor_elt *ce; vec<constructor_elt, va_gc> *v = CONSTRUCTOR_ELTS (init); @@ -1365,7 +1383,8 @@ static int ce->index = error_mark_node; gcc_assert (ce->value); ce->value - = massage_init_elt (TREE_TYPE (type), ce->value, nested, complain); + = massage_init_elt (TREE_TYPE (type), ce->value, nested, flags, + complain); gcc_checking_assert (ce->value == error_mark_node @@ -1373,7 +1392,7 @@ static int (strip_array_types (TREE_TYPE (type)), strip_array_types (TREE_TYPE (ce->value))))); - flags |= picflag_from_initializer (ce->value); + picflags |= picflag_from_initializer (ce->value); } /* No more initializers. If the array is unbounded, we are done. Otherwise, @@ -1389,7 +1408,8 @@ static int we can't rely on the back end to do it for us, so make the initialization explicit by list-initializing from T{}. */ next = build_constructor (init_list_type_node, NULL); - next = massage_init_elt (TREE_TYPE (type), next, nested, complain); + next = massage_init_elt (TREE_TYPE (type), next, nested, flags, + complain); if (initializer_zerop (next)) /* The default zero-initialization is fine for us; don't add anything to the CONSTRUCTOR. */ @@ -1406,7 +1426,7 @@ static int if (next) { - flags |= picflag_from_initializer (next); + picflags |= picflag_from_initializer (next); if (len > i+1 && (initializer_constant_valid_p (next, TREE_TYPE (next)) == null_pointer_node)) @@ -1426,7 +1446,7 @@ static int } CONSTRUCTOR_ELTS (init) = v; - return flags; + return picflags; } /* Subroutine of process_init_constructor, which will process an initializer @@ -1434,7 +1454,7 @@ static int the initializers. */ static int -process_init_constructor_record (tree type, tree init, int nested, +process_init_constructor_record (tree type, tree init, int nested, int flags, tsubst_flags_t complain) { vec<constructor_elt, va_gc> *v = NULL; @@ -1449,7 +1469,7 @@ static int gcc_assert (!TYPE_POLYMORPHIC_P (type)); restart: - int flags = 0; + int picflags = 0; unsigned HOST_WIDE_INT idx = 0; int designator_skip = -1; /* Generally, we will always have an index for each initializer (which is @@ -1517,7 +1537,7 @@ static int if (ce) { gcc_assert (ce->value); - next = massage_init_elt (type, next, nested, complain); + next = massage_init_elt (type, next, nested, flags, complain); ++idx; } } @@ -1546,7 +1566,8 @@ static int for us, so build up TARGET_EXPRs. If the type in question is a class, just build one up; if it's an array, recurse. */ next = build_constructor (init_list_type_node, NULL); - next = massage_init_elt (TREE_TYPE (field), next, nested, complain); + next = massage_init_elt (TREE_TYPE (field), next, nested, flags, + complain); /* Warn when some struct elements are implicitly initialized. */ if ((complain & tf_warning) @@ -1597,7 +1618,7 @@ static int /* If this is a bitfield, now convert to the lowered type. */ if (type != TREE_TYPE (field)) next = cp_convert_and_check (TREE_TYPE (field), next, complain); - flags |= picflag_from_initializer (next); + picflags |= picflag_from_initializer (next); CONSTRUCTOR_APPEND_ELT (v, field, next); } @@ -1653,7 +1674,7 @@ static int } CONSTRUCTOR_ELTS (init) = v; - return flags; + return picflags; } /* Subroutine of process_init_constructor, which will process a single @@ -1661,7 +1682,7 @@ static int which describe the initializer. */ static int -process_init_constructor_union (tree type, tree init, int nested, +process_init_constructor_union (tree type, tree init, int nested, int flags, tsubst_flags_t complain) { constructor_elt *ce; @@ -1749,7 +1770,7 @@ static int if (ce->value && ce->value != error_mark_node) ce->value = massage_init_elt (TREE_TYPE (ce->index), ce->value, nested, - complain); + flags, complain); return picflag_from_initializer (ce->value); } @@ -1769,40 +1790,43 @@ static int of error. */ static tree -process_init_constructor (tree type, tree init, int nested, +process_init_constructor (tree type, tree init, int nested, int flags, tsubst_flags_t complain) { - int flags; + int picflags; gcc_assert (BRACE_ENCLOSED_INITIALIZER_P (init)); if (TREE_CODE (type) == ARRAY_TYPE || VECTOR_TYPE_P (type)) - flags = process_init_constructor_array (type, init, nested, complain); + picflags = process_init_constructor_array (type, init, nested, flags, + complain); else if (TREE_CODE (type) == RECORD_TYPE) - flags = process_init_constructor_record (type, init, nested, complain); + picflags = process_init_constructor_record (type, init, nested, flags, + complain); else if (TREE_CODE (type) == UNION_TYPE) - flags = process_init_constructor_union (type, init, nested, complain); + picflags = process_init_constructor_union (type, init, nested, flags, + complain); else gcc_unreachable (); - if (flags & PICFLAG_ERRONEOUS) + if (picflags & PICFLAG_ERRONEOUS) return error_mark_node; TREE_TYPE (init) = type; if (TREE_CODE (type) == ARRAY_TYPE && TYPE_DOMAIN (type) == NULL_TREE) cp_complete_array_type (&TREE_TYPE (init), init, /*do_default=*/0); - if (flags & PICFLAG_SIDE_EFFECTS) + if (picflags & PICFLAG_SIDE_EFFECTS) { TREE_CONSTANT (init) = false; TREE_SIDE_EFFECTS (init) = true; } - else if (flags & PICFLAG_NOT_ALL_CONSTANT) + else if (picflags & PICFLAG_NOT_ALL_CONSTANT) /* Make sure TREE_CONSTANT isn't set from build_constructor. */ TREE_CONSTANT (init) = false; else { TREE_CONSTANT (init) = 1; - if (!(flags & PICFLAG_NOT_ALL_SIMPLE)) + if (!(picflags & PICFLAG_NOT_ALL_SIMPLE)) TREE_STATIC (init) = 1; } return init; Index: gcc/testsuite/c-c++-common/array-6.c =================================================================== --- gcc/testsuite/c-c++-common/array-6.c (revision 267569) +++ gcc/testsuite/c-c++-common/array-6.c (working copy) @@ -16,3 +16,32 @@ void foo() struct str d = (struct str) { 2, "d" }; /* { dg-error "(non-static)|(near initialization)" } */ struct str e = (struct str) { d.len, "e" }; /* { dg-error "(non-static)|(initialization)" } */ } + +struct str f = { 0, {} }; + +void bar() +{ + static struct str g = { 0, {} }; + struct str h = { 0, {} }; /* { dg-error "(non-static)|(near initialization)" } */ + struct str i = (struct str) { 0, {} }; /* { dg-error "(non-static)|(near initialization)" } */ + struct str j = (struct str) { i.len, {} }; /* { dg-error "(non-static)|(initialization)" } */ +} + +struct str k = { 0 }; + +void baz() +{ + static struct str l = { 0 }; + struct str m = { 0 }; + struct str n = (struct str) { 0 }; + struct str o = (struct str) { n.len }; +} + +struct str p = {}; + +void qux() +{ + static struct str q = {}; + struct str r = {}; + struct str s = (struct str) {}; +} Index: gcc/testsuite/g++.dg/ext/flexary12.C =================================================================== --- gcc/testsuite/g++.dg/ext/flexary12.C (revision 267569) +++ gcc/testsuite/g++.dg/ext/flexary12.C (working copy) @@ -12,7 +12,7 @@ struct A { void f1 () { // This is the meat of the test from c++/69290: - struct A a + static struct A a = { "c" }; // { dg-error "invalid conversion from .const char\\*. to .int." } (void)&a; @@ -27,13 +27,13 @@ struct B { void f2 () { - struct B b1 + static struct B b1 = { 0, "c" }; // { dg-error "invalid conversion from .const char\\*. to .int." } (void)&b1; const char s[] = "c"; - struct B b2 + static struct B b2 = { 0, s }; // { dg-error "invalid conversion from .const char\\*. to .int." } (void)&b2; @@ -57,7 +57,7 @@ struct C { void f3 () { - struct C<double> cd + static struct C<double> cd = { "c" }; // { dg-error "cannot convert .const char\\*. to .double." } (void)&cd; Index: gcc/testsuite/g++.dg/ext/flexary13.C =================================================================== --- gcc/testsuite/g++.dg/ext/flexary13.C (revision 267569) +++ gcc/testsuite/g++.dg/ext/flexary13.C (working copy) @@ -19,33 +19,33 @@ int main () ASSERT (s.n == 0); } { - Ax s = + static Ax s = { 0, { } }; // dg-warning "initialization of a flexible array member" } ASSERT (s.n == 0); } { - Ax s = + static Ax s = { 1, { 2 } }; // dg-warning "initialization of a flexible array member" } ASSERT (s.n == 1 && s.a [0] == 2); } { - Ax s = + static Ax s = { 2, { 3, 4 } }; // dg-warning "initialization of a flexible array member" } ASSERT (s.n = 2 && s.a [0] == 3 && s.a [1] == 4); } { - Ax s = + static Ax s = { 123, i }; // dg-warning "initialization of a flexible array member" } ASSERT (s.n == 123 && s.a [0] == i); } { - Ax s = + static Ax s = { 456, { i } }; // dg-warning "initialization of a flexible array member" } ASSERT (s.n == 456 && s.a [0] == i); } { int j = i + 1, k = j + 1; - Ax s = + static Ax s = { 3, { i, j, k } }; // dg-warning "initialization of a flexible array member" } ASSERT (s.n == 3 && s.a [0] == i && s.a [1] == j && s.a [2] == k); } Index: gcc/testsuite/g++.dg/ext/flexary15.C =================================================================== --- gcc/testsuite/g++.dg/ext/flexary15.C (revision 267569) +++ gcc/testsuite/g++.dg/ext/flexary15.C (working copy) @@ -10,5 +10,5 @@ struct S { void foo (const char *a) { - const S s = { 1, { a, "b" } }; // { dg-warning "invalid conversion" } + static const S s = { 1, { a, "b" } }; // { dg-warning "invalid conversion" } } Index: gcc/testsuite/g++.dg/ext/flexary3.C =================================================================== --- gcc/testsuite/g++.dg/ext/flexary3.C (revision 267569) +++ gcc/testsuite/g++.dg/ext/flexary3.C (working copy) @@ -17,5 +17,6 @@ struct s { int main() { struct s s = { .c = 0 }; // { dg-error "initializer" } + // { dg-error "non-static initialization of a flexible array member" "" { target *-*-* } .-1 } return 0; } Index: gcc/testsuite/g++.dg/ext/flexary32.C =================================================================== --- gcc/testsuite/g++.dg/ext/flexary32.C (revision 0) +++ gcc/testsuite/g++.dg/ext/flexary32.C (working copy) @@ -0,0 +1,19 @@ +/* { dg-do compile { target c++11 } } */ +/* { dg-options -Wno-pedantic } */ + +struct str { int len; char s[]; }; + +struct foo { + str x = {3, {1,2,3}}; /* { dg-error "(non-static)|(initialization)" } */ + foo() {} +}; + +struct bar { + static constexpr str x = {3, {1,2,3}}; + bar() {} +}; + +struct baz { + str x = {3}; + baz() {} +}; Index: gcc/testsuite/g++.dg/warn/Wplacement-new-size-1.C =================================================================== --- gcc/testsuite/g++.dg/warn/Wplacement-new-size-1.C (revision 267569) +++ gcc/testsuite/g++.dg/warn/Wplacement-new-size-1.C (working copy) @@ -28,7 +28,7 @@ void fAx (Ax *px, Ax &rx) void fAx2 () { - Ax ax2 = { 1, { 2, 3 } }; + static Ax ax2 = { 1, { 2, 3 } }; new (ax2.a) Int16; new (ax2.a) Int32; // { dg-warning "placement" } @@ -82,7 +82,7 @@ void fBx (BAx *pbx, BAx &rbx) void fBx1 () { - BAx bax1 = { 1, /* Ax = */ { 2, /* a[] = */ {} } }; + static BAx bax1 = { 1, /* Ax = */ { 2, /* a[] = */ {} } }; new (bax1.ax.a) char; // { dg-warning "placement" } new (bax1.ax.a) char[2]; // { dg-warning "placement" } Index: gcc/testsuite/g++.dg/warn/Wplacement-new-size-2.C =================================================================== --- gcc/testsuite/g++.dg/warn/Wplacement-new-size-2.C (revision 267569) +++ gcc/testsuite/g++.dg/warn/Wplacement-new-size-2.C (working copy) @@ -33,13 +33,13 @@ void fAx (Ax *px, Ax &rx) void fAx2 () { // Initialization of non-static objects with flexible array members - // isn't allowed in C and should perhaps be disallowed in C++ as + // isn't allowed in C and had to be be disallowed in C++ as // well to avoid c++/69696 - incorrect initialization of block-scope // flexible array members. - Ax ax2 = { 1, { 2, 3 } }; + Ax ax2 = { 1, { 2, 3 } }; // { dg-error "non-static initialization of a flexible array member" } - new (ax2.a) Int16; - new (ax2.a) Int16[1]; + new (ax2.a) Int16; // { dg-warning "placement" } + new (ax2.a) Int16[1]; // { dg-warning "placement" } new (ax2.a) Int16[2]; // { dg-warning "placement" } new (ax2.a) Int32; // { dg-warning "placement" } new (ax2.a) Int32[2]; // { dg-warning "placement" } @@ -140,7 +140,7 @@ void fBx (BAx *pbx, BAx &rbx) void fBx1 () { - BAx bax1 = { 1, /* Ax = */ { 2, /* a[] = */ {} } }; + static BAx bax1 = { 1, /* Ax = */ { 2, /* a[] = */ {} } }; new (bax1.ax.a) char; // { dg-warning "placement" } new (bax1.ax.a) char[2]; // { dg-warning "placement" } Index: gcc/testsuite/g++.dg/warn/Wplacement-new-size-6.C =================================================================== --- gcc/testsuite/g++.dg/warn/Wplacement-new-size-6.C (revision 267569) +++ gcc/testsuite/g++.dg/warn/Wplacement-new-size-6.C (working copy) @@ -15,7 +15,7 @@ struct BAx { int i; Ax ax; }; void fBx1 () { - BAx bax1 = { 1, /* Ax = */ { 2, /* a[] = */ { 3 } } }; // { dg-error "initialization of flexible array member in a nested context" } + static BAx bax1 = { 1, /* Ax = */ { 2, /* a[] = */ { 3 } } }; // { dg-error "initialization of flexible array member in a nested context" } new (bax1.ax.a) char; // { dg-warning "placement" } new (bax1.ax.a) char[2]; // { dg-warning "placement" } @@ -25,7 +25,7 @@ void fBx1 () void fBx2 () { - BAx bax2 = { 1, /* Ax = */ { 2, /* a[] = */ { 3, 4 } } }; // { dg-error "initialization of flexible array member in a nested context" } + static BAx bax2 = { 1, /* Ax = */ { 2, /* a[] = */ { 3, 4 } } }; // { dg-error "initialization of flexible array member in a nested context" } new (bax2.ax.a) char; // { dg-warning "placement" } new (bax2.ax.a) char[2]; // { dg-warning "placement" } @@ -37,7 +37,7 @@ void fBx2 () void fBx3 () { - BAx bax2 = { 1, /* Ax = */ { 3, /* a[] = */ { 4, 5, 6 } } }; // { dg-error "initialization of flexible array member in a nested context" } + static BAx bax2 = { 1, /* Ax = */ { 3, /* a[] = */ { 4, 5, 6 } } }; // { dg-error "initialization of flexible array member in a nested context" } new (bax2.ax.a) char; // { dg-warning "placement" } new (bax2.ax.a) char[2]; // { dg-warning "placement" } Index: gcc/testsuite/gcc.dg/array-6.c =================================================================== --- gcc/testsuite/gcc.dg/array-6.c (revision 267569) +++ gcc/testsuite/gcc.dg/array-6.c (working copy) @@ -1,18 +0,0 @@ -/* PR c/5597 */ -/* { dg-do compile } */ -/* { dg-options "" } */ - -/* Verify that GCC forbids non-static initialization of - flexible array members. */ - -struct str { int len; char s[]; }; - -struct str a = { 2, "a" }; - -void foo() -{ - static struct str b = { 2, "b" }; - struct str c = { 2, "c" }; /* { dg-error "(non-static)|(near initialization)" } */ - struct str d = (struct str) { 2, "d" }; /* { dg-error "(non-static)|(near initialization)" } */ - struct str e = (struct str) { d.len, "e" }; /* { dg-error "(non-static)|(initialization)" } */ -} ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH, C++,rebased] Fix PR c++/88261 2019-01-05 16:05 ` Bernd Edlinger @ 2019-01-07 0:09 ` Martin Sebor 2019-01-07 15:38 ` Bernd Edlinger 0 siblings, 1 reply; 16+ messages in thread From: Martin Sebor @ 2019-01-07 0:09 UTC (permalink / raw) To: Bernd Edlinger, Jason Merrill, Martin Sebor, gcc-patches, Jeff Law, Nathan Sidwell On 1/5/19 9:04 AM, Bernd Edlinger wrote: > On 1/4/19 10:22 PM, Jason Merrill wrote: >> Hmm, I'm uncomfortable with starting to pass in the decl just for the sake of deciding whether this diagnostic should be a pedwarn or error. In general, because of copy elision, we can't know at this point what we're initializing, so I'd rather not pretend we can. Instead, maybe add a LOOKUP_ALLOW_FLEXARY_INIT flag that you can add to the flags argument in the call from store_init_value? >> > > Okay, I reworked the patch, to pass a bit in the flags, it was a bit more complicated > than anticipated, because it is necessary to pass the flag thru process_init_constructor > and friends to the recursive invocation of digest_init_r. It turned out that > digest_nsdmi_init did not need to change, since it is always wrong to use flexarray init > there. I added a new test case (flexary32.C) to exercises a few cases where non static > direct member intializers are allowed to use flexarrays (in static members) and where that > would be wrong (in automatic members). So that seems to work. If that resolves pr69338 can you please also reference the bug in the test and in the ChangeLog? (Ditto for pr69697.) Martin > > New version of the patch is attached. > > Bootstrapped and reg-tested on x86_64-pc-linux-gnu. > Is it OK for trunk? > > > Thanks > Bernd. > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH, C++,rebased] Fix PR c++/88261 2019-01-07 0:09 ` Martin Sebor @ 2019-01-07 15:38 ` Bernd Edlinger 2019-01-07 16:58 ` Jason Merrill 0 siblings, 1 reply; 16+ messages in thread From: Bernd Edlinger @ 2019-01-07 15:38 UTC (permalink / raw) To: Martin Sebor, Jason Merrill, Martin Sebor, gcc-patches, Jeff Law, Nathan Sidwell [-- Attachment #1: Type: text/plain, Size: 1511 bytes --] On 1/7/19 1:08 AM, Martin Sebor wrote: > On 1/5/19 9:04 AM, Bernd Edlinger wrote: >> On 1/4/19 10:22 PM, Jason Merrill wrote: >>> Hmm, I'm uncomfortable with starting to pass in the decl just for the sake of deciding whether this diagnostic should be a pedwarn or error. In general, because of copy elision, we can't know at this point what we're initializing, so I'd rather not pretend we can. Instead, maybe add a LOOKUP_ALLOW_FLEXARY_INIT flag that you can add to the flags argument in the call from store_init_value? >>> >> >> Okay, I reworked the patch, to pass a bit in the flags, it was a bit more complicated >> than anticipated, because it is necessary to pass the flag thru process_init_constructor >> and friends to the recursive invocation of digest_init_r. It turned out that >> digest_nsdmi_init did not need to change, since it is always wrong to use flexarray init >> there. I added a new test case (flexary32.C) to exercises a few cases where non static >> direct member intializers are allowed to use flexarrays (in static members) and where that >> would be wrong (in automatic members). So that seems to work. > > If that resolves pr69338 can you please also reference the bug in > the test and in the ChangeLog? (Ditto for pr69697.) > Yes, those appear to be fixed as well. Added pr69338 + pr69697 to the ChangeLog, and also added test cases from both PRs. Attached the otherwise unchanged v3 of my patch. Is to OK for trunk? Thanks Bernd. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: patch-pr88261v3.diff --] [-- Type: text/x-patch; name="patch-pr88261v3.diff", Size: 22774 bytes --] gcc/cp: 2019-01-05 Bernd Edlinger <bernd.edlinger@hotmail.de> PR c++/88261 PR c++/69338 PR c++/69696 PR c++/69697 * cp-tree.h (LOOKUP_ALLOW_FLEXARRAY_INIT): New flag value. * typeck2.c (digest_init_r): Raise an error for non-static initialization of a flexible array member. (process_init_constructor, massage_init_elt, process_init_constructor_array, process_init_constructor_record, process_init_constructor_union, process_init_constructor): Add the flags parameter and pass it thru. (store_init_value): Pass LOOKUP_ALLOW_FLEXARRAY_INIT parameter to digest_init_flags for static decls. gcc/testsuite: 2019-01-05 Bernd Edlinger <bernd.edlinger@hotmail.de> PR c++/88261 PR c++/69338 PR c++/69696 PR c++/69697 * gcc.dg/array-6.c: Move from here ... * c-c++-common/array-6.c: ... to here and add some more test coverage. * g++.dg/pr69338.C: New test. * g++.dg/pr69697.C: Likewise. * g++.dg/ext/flexary32.C: Likewise. * g++.dg/ext/flexary3.C: Adjust test. * g++.dg/ext/flexary12.C: Likewise. * g++.dg/ext/flexary13.C: Likewise. * g++.dg/ext/flexary15.C: Likewise. * g++.dg/warn/Wplacement-new-size-1.C: Likewise. * g++.dg/warn/Wplacement-new-size-2.C: Likewise. * g++.dg/warn/Wplacement-new-size-6.C: Likewise. Index: gcc/cp/cp-tree.h =================================================================== --- gcc/cp/cp-tree.h (revision 267569) +++ gcc/cp/cp-tree.h (working copy) @@ -5454,6 +5454,8 @@ enum overload_flags { NO_SPECIAL = 0, DTOR_FLAG, T #define LOOKUP_NO_NON_INTEGRAL (LOOKUP_NO_RVAL_BIND << 1) /* Used for delegating constructors in order to diagnose self-delegation. */ #define LOOKUP_DELEGATING_CONS (LOOKUP_NO_NON_INTEGRAL << 1) +/* Allow initialization of a flexible array members. */ +#define LOOKUP_ALLOW_FLEXARRAY_INIT (LOOKUP_DELEGATING_CONS << 1) #define LOOKUP_NAMESPACES_ONLY(F) \ (((F) & LOOKUP_PREFER_NAMESPACES) && !((F) & LOOKUP_PREFER_TYPES)) Index: gcc/cp/typeck2.c =================================================================== --- gcc/cp/typeck2.c (revision 267569) +++ gcc/cp/typeck2.c (working copy) @@ -35,7 +35,7 @@ along with GCC; see the file COPYING3. If not see #include "gcc-rich-location.h" static tree -process_init_constructor (tree type, tree init, int nested, +process_init_constructor (tree type, tree init, int nested, int flags, tsubst_flags_t complain); @@ -817,8 +817,12 @@ store_init_value (tree decl, tree init, vec<tree, if (flags & LOOKUP_ALREADY_DIGESTED) value = init; else - /* Digest the specified initializer into an expression. */ - value = digest_init_flags (type, init, flags, tf_warning_or_error); + { + if (TREE_STATIC (decl)) + flags |= LOOKUP_ALLOW_FLEXARRAY_INIT; + /* Digest the specified initializer into an expression. */ + value = digest_init_flags (type, init, flags, tf_warning_or_error); + } if (TREE_CODE (type) == ARRAY_TYPE && TYPE_STRING_FLAG (TREE_TYPE (type)) @@ -1068,8 +1072,18 @@ digest_init_r (tree type, tree init, int nested, i { if (nested && !TYPE_DOMAIN (type)) /* C++ flexible array members have a null domain. */ - pedwarn (loc, OPT_Wpedantic, - "initialization of a flexible array member"); + { + if (flags & LOOKUP_ALLOW_FLEXARRAY_INIT) + pedwarn (loc, OPT_Wpedantic, + "initialization of a flexible array member"); + else + { + if (complain & tf_error) + error_at (loc, "non-static initialization of" + " a flexible array member"); + return error_mark_node; + } + } tree typ1 = TYPE_MAIN_VARIANT (TREE_TYPE (type)); if (char_type_p (typ1) @@ -1193,7 +1207,8 @@ digest_init_r (tree type, tree init, int nested, i if (BRACE_ENCLOSED_INITIALIZER_P (stripped_init) && !TYPE_NON_AGGREGATE_CLASS (type)) - return process_init_constructor (type, stripped_init, nested, complain); + return process_init_constructor (type, stripped_init, nested, flags, + complain); else { if (COMPOUND_LITERAL_P (stripped_init) && code == ARRAY_TYPE) @@ -1291,9 +1306,12 @@ picflag_from_initializer (tree init) /* Adjust INIT for going into a CONSTRUCTOR. */ static tree -massage_init_elt (tree type, tree init, int nested, tsubst_flags_t complain) +massage_init_elt (tree type, tree init, int nested, int flags, + tsubst_flags_t complain) { - init = digest_init_r (type, init, nested ? 2 : 1, LOOKUP_IMPLICIT, complain); + flags &= LOOKUP_ALLOW_FLEXARRAY_INIT; + flags |= LOOKUP_IMPLICIT; + init = digest_init_r (type, init, nested ? 2 : 1, flags, complain); /* Strip a simple TARGET_EXPR when we know this is an initializer. */ if (SIMPLE_TARGET_EXPR_P (init)) init = TARGET_EXPR_INITIAL (init); @@ -1311,11 +1329,11 @@ static tree which describe the initializers. */ static int -process_init_constructor_array (tree type, tree init, int nested, +process_init_constructor_array (tree type, tree init, int nested, int flags, tsubst_flags_t complain) { unsigned HOST_WIDE_INT i, len = 0; - int flags = 0; + int picflags = 0; bool unbounded = false; constructor_elt *ce; vec<constructor_elt, va_gc> *v = CONSTRUCTOR_ELTS (init); @@ -1365,7 +1383,8 @@ static int ce->index = error_mark_node; gcc_assert (ce->value); ce->value - = massage_init_elt (TREE_TYPE (type), ce->value, nested, complain); + = massage_init_elt (TREE_TYPE (type), ce->value, nested, flags, + complain); gcc_checking_assert (ce->value == error_mark_node @@ -1373,7 +1392,7 @@ static int (strip_array_types (TREE_TYPE (type)), strip_array_types (TREE_TYPE (ce->value))))); - flags |= picflag_from_initializer (ce->value); + picflags |= picflag_from_initializer (ce->value); } /* No more initializers. If the array is unbounded, we are done. Otherwise, @@ -1389,7 +1408,8 @@ static int we can't rely on the back end to do it for us, so make the initialization explicit by list-initializing from T{}. */ next = build_constructor (init_list_type_node, NULL); - next = massage_init_elt (TREE_TYPE (type), next, nested, complain); + next = massage_init_elt (TREE_TYPE (type), next, nested, flags, + complain); if (initializer_zerop (next)) /* The default zero-initialization is fine for us; don't add anything to the CONSTRUCTOR. */ @@ -1406,7 +1426,7 @@ static int if (next) { - flags |= picflag_from_initializer (next); + picflags |= picflag_from_initializer (next); if (len > i+1 && (initializer_constant_valid_p (next, TREE_TYPE (next)) == null_pointer_node)) @@ -1426,7 +1446,7 @@ static int } CONSTRUCTOR_ELTS (init) = v; - return flags; + return picflags; } /* Subroutine of process_init_constructor, which will process an initializer @@ -1434,7 +1454,7 @@ static int the initializers. */ static int -process_init_constructor_record (tree type, tree init, int nested, +process_init_constructor_record (tree type, tree init, int nested, int flags, tsubst_flags_t complain) { vec<constructor_elt, va_gc> *v = NULL; @@ -1449,7 +1469,7 @@ static int gcc_assert (!TYPE_POLYMORPHIC_P (type)); restart: - int flags = 0; + int picflags = 0; unsigned HOST_WIDE_INT idx = 0; int designator_skip = -1; /* Generally, we will always have an index for each initializer (which is @@ -1517,7 +1537,7 @@ static int if (ce) { gcc_assert (ce->value); - next = massage_init_elt (type, next, nested, complain); + next = massage_init_elt (type, next, nested, flags, complain); ++idx; } } @@ -1546,7 +1566,8 @@ static int for us, so build up TARGET_EXPRs. If the type in question is a class, just build one up; if it's an array, recurse. */ next = build_constructor (init_list_type_node, NULL); - next = massage_init_elt (TREE_TYPE (field), next, nested, complain); + next = massage_init_elt (TREE_TYPE (field), next, nested, flags, + complain); /* Warn when some struct elements are implicitly initialized. */ if ((complain & tf_warning) @@ -1597,7 +1618,7 @@ static int /* If this is a bitfield, now convert to the lowered type. */ if (type != TREE_TYPE (field)) next = cp_convert_and_check (TREE_TYPE (field), next, complain); - flags |= picflag_from_initializer (next); + picflags |= picflag_from_initializer (next); CONSTRUCTOR_APPEND_ELT (v, field, next); } @@ -1653,7 +1674,7 @@ static int } CONSTRUCTOR_ELTS (init) = v; - return flags; + return picflags; } /* Subroutine of process_init_constructor, which will process a single @@ -1661,7 +1682,7 @@ static int which describe the initializer. */ static int -process_init_constructor_union (tree type, tree init, int nested, +process_init_constructor_union (tree type, tree init, int nested, int flags, tsubst_flags_t complain) { constructor_elt *ce; @@ -1749,7 +1770,7 @@ static int if (ce->value && ce->value != error_mark_node) ce->value = massage_init_elt (TREE_TYPE (ce->index), ce->value, nested, - complain); + flags, complain); return picflag_from_initializer (ce->value); } @@ -1769,40 +1790,43 @@ static int of error. */ static tree -process_init_constructor (tree type, tree init, int nested, +process_init_constructor (tree type, tree init, int nested, int flags, tsubst_flags_t complain) { - int flags; + int picflags; gcc_assert (BRACE_ENCLOSED_INITIALIZER_P (init)); if (TREE_CODE (type) == ARRAY_TYPE || VECTOR_TYPE_P (type)) - flags = process_init_constructor_array (type, init, nested, complain); + picflags = process_init_constructor_array (type, init, nested, flags, + complain); else if (TREE_CODE (type) == RECORD_TYPE) - flags = process_init_constructor_record (type, init, nested, complain); + picflags = process_init_constructor_record (type, init, nested, flags, + complain); else if (TREE_CODE (type) == UNION_TYPE) - flags = process_init_constructor_union (type, init, nested, complain); + picflags = process_init_constructor_union (type, init, nested, flags, + complain); else gcc_unreachable (); - if (flags & PICFLAG_ERRONEOUS) + if (picflags & PICFLAG_ERRONEOUS) return error_mark_node; TREE_TYPE (init) = type; if (TREE_CODE (type) == ARRAY_TYPE && TYPE_DOMAIN (type) == NULL_TREE) cp_complete_array_type (&TREE_TYPE (init), init, /*do_default=*/0); - if (flags & PICFLAG_SIDE_EFFECTS) + if (picflags & PICFLAG_SIDE_EFFECTS) { TREE_CONSTANT (init) = false; TREE_SIDE_EFFECTS (init) = true; } - else if (flags & PICFLAG_NOT_ALL_CONSTANT) + else if (picflags & PICFLAG_NOT_ALL_CONSTANT) /* Make sure TREE_CONSTANT isn't set from build_constructor. */ TREE_CONSTANT (init) = false; else { TREE_CONSTANT (init) = 1; - if (!(flags & PICFLAG_NOT_ALL_SIMPLE)) + if (!(picflags & PICFLAG_NOT_ALL_SIMPLE)) TREE_STATIC (init) = 1; } return init; Index: gcc/testsuite/c-c++-common/array-6.c =================================================================== --- gcc/testsuite/c-c++-common/array-6.c (revision 267569) +++ gcc/testsuite/c-c++-common/array-6.c (working copy) @@ -16,3 +16,32 @@ void foo() struct str d = (struct str) { 2, "d" }; /* { dg-error "(non-static)|(near initialization)" } */ struct str e = (struct str) { d.len, "e" }; /* { dg-error "(non-static)|(initialization)" } */ } + +struct str f = { 0, {} }; + +void bar() +{ + static struct str g = { 0, {} }; + struct str h = { 0, {} }; /* { dg-error "(non-static)|(near initialization)" } */ + struct str i = (struct str) { 0, {} }; /* { dg-error "(non-static)|(near initialization)" } */ + struct str j = (struct str) { i.len, {} }; /* { dg-error "(non-static)|(initialization)" } */ +} + +struct str k = { 0 }; + +void baz() +{ + static struct str l = { 0 }; + struct str m = { 0 }; + struct str n = (struct str) { 0 }; + struct str o = (struct str) { n.len }; +} + +struct str p = {}; + +void qux() +{ + static struct str q = {}; + struct str r = {}; + struct str s = (struct str) {}; +} Index: gcc/testsuite/g++.dg/ext/flexary12.C =================================================================== --- gcc/testsuite/g++.dg/ext/flexary12.C (revision 267569) +++ gcc/testsuite/g++.dg/ext/flexary12.C (working copy) @@ -12,7 +12,7 @@ struct A { void f1 () { // This is the meat of the test from c++/69290: - struct A a + static struct A a = { "c" }; // { dg-error "invalid conversion from .const char\\*. to .int." } (void)&a; @@ -27,13 +27,13 @@ struct B { void f2 () { - struct B b1 + static struct B b1 = { 0, "c" }; // { dg-error "invalid conversion from .const char\\*. to .int." } (void)&b1; const char s[] = "c"; - struct B b2 + static struct B b2 = { 0, s }; // { dg-error "invalid conversion from .const char\\*. to .int." } (void)&b2; @@ -57,7 +57,7 @@ struct C { void f3 () { - struct C<double> cd + static struct C<double> cd = { "c" }; // { dg-error "cannot convert .const char\\*. to .double." } (void)&cd; Index: gcc/testsuite/g++.dg/ext/flexary13.C =================================================================== --- gcc/testsuite/g++.dg/ext/flexary13.C (revision 267569) +++ gcc/testsuite/g++.dg/ext/flexary13.C (working copy) @@ -19,33 +19,33 @@ int main () ASSERT (s.n == 0); } { - Ax s = + static Ax s = { 0, { } }; // dg-warning "initialization of a flexible array member" } ASSERT (s.n == 0); } { - Ax s = + static Ax s = { 1, { 2 } }; // dg-warning "initialization of a flexible array member" } ASSERT (s.n == 1 && s.a [0] == 2); } { - Ax s = + static Ax s = { 2, { 3, 4 } }; // dg-warning "initialization of a flexible array member" } ASSERT (s.n = 2 && s.a [0] == 3 && s.a [1] == 4); } { - Ax s = + static Ax s = { 123, i }; // dg-warning "initialization of a flexible array member" } ASSERT (s.n == 123 && s.a [0] == i); } { - Ax s = + static Ax s = { 456, { i } }; // dg-warning "initialization of a flexible array member" } ASSERT (s.n == 456 && s.a [0] == i); } { int j = i + 1, k = j + 1; - Ax s = + static Ax s = { 3, { i, j, k } }; // dg-warning "initialization of a flexible array member" } ASSERT (s.n == 3 && s.a [0] == i && s.a [1] == j && s.a [2] == k); } Index: gcc/testsuite/g++.dg/ext/flexary15.C =================================================================== --- gcc/testsuite/g++.dg/ext/flexary15.C (revision 267569) +++ gcc/testsuite/g++.dg/ext/flexary15.C (working copy) @@ -10,5 +10,5 @@ struct S { void foo (const char *a) { - const S s = { 1, { a, "b" } }; // { dg-warning "invalid conversion" } + static const S s = { 1, { a, "b" } }; // { dg-warning "invalid conversion" } } Index: gcc/testsuite/g++.dg/ext/flexary3.C =================================================================== --- gcc/testsuite/g++.dg/ext/flexary3.C (revision 267569) +++ gcc/testsuite/g++.dg/ext/flexary3.C (working copy) @@ -17,5 +17,6 @@ struct s { int main() { struct s s = { .c = 0 }; // { dg-error "initializer" } + // { dg-error "non-static initialization of a flexible array member" "" { target *-*-* } .-1 } return 0; } Index: gcc/testsuite/g++.dg/ext/flexary32.C =================================================================== --- gcc/testsuite/g++.dg/ext/flexary32.C (revision 0) +++ gcc/testsuite/g++.dg/ext/flexary32.C (working copy) @@ -0,0 +1,19 @@ +/* { dg-do compile { target c++11 } } */ +/* { dg-options -Wno-pedantic } */ + +struct str { int len; char s[]; }; + +struct foo { + str x = {3, {1,2,3}}; /* { dg-error "(non-static)|(initialization)" } */ + foo() {} +}; + +struct bar { + static constexpr str x = {3, {1,2,3}}; + bar() {} +}; + +struct baz { + str x = {3}; + baz() {} +}; Index: gcc/testsuite/g++.dg/pr69338.C =================================================================== --- gcc/testsuite/g++.dg/pr69338.C (revision 0) +++ gcc/testsuite/g++.dg/pr69338.C (working copy) @@ -0,0 +1,23 @@ +/* { dg-do compile { target c++11 } } */ +/* { dg-additional-options "-Wno-pedantic" } */ + +struct A { char i, a[]; }; + +void foo() +{ + struct A a0 = { 3, "AB" }; /* { dg-error "(non-static)|(initialization)" } */ +} + +struct A a1 = { 3, "AB" }; /* { dg-bogus "(non-static)|(initialization)" } */ + +struct A a2 = (struct A){ 3, "AB" }; /* { dg-error "(non-static)|(initialization)" } */ + +struct B1 { + A a3; + B1 (): a3 { 3, "AB" } { } /* { dg-error "(non-static)|(initialization)" } */ +} b1; + +struct B2 { + A a4; + B2 (): a4 ((struct A){ 3, "AB" }) { } /* { dg-error "(non-static)|(initialization)" } */ +} b2; Index: gcc/testsuite/g++.dg/pr69697.C =================================================================== --- gcc/testsuite/g++.dg/pr69697.C (revision 0) +++ gcc/testsuite/g++.dg/pr69697.C (working copy) @@ -0,0 +1,7 @@ +/* { dg-do compile { target c++11 } } */ +/* { dg-additional-options "-Wno-pedantic" } */ + +int i; +struct A { int n, a[]; } + a = i ? A({ 1, { 2 } }) /* { dg-error "(non-static)|(initialization)" } */ + : A({ 2, { 3, 4 } }); /* { dg-error "(non-static)|(initialization)" } */ Index: gcc/testsuite/g++.dg/warn/Wplacement-new-size-1.C =================================================================== --- gcc/testsuite/g++.dg/warn/Wplacement-new-size-1.C (revision 267569) +++ gcc/testsuite/g++.dg/warn/Wplacement-new-size-1.C (working copy) @@ -28,7 +28,7 @@ void fAx (Ax *px, Ax &rx) void fAx2 () { - Ax ax2 = { 1, { 2, 3 } }; + static Ax ax2 = { 1, { 2, 3 } }; new (ax2.a) Int16; new (ax2.a) Int32; // { dg-warning "placement" } @@ -82,7 +82,7 @@ void fBx (BAx *pbx, BAx &rbx) void fBx1 () { - BAx bax1 = { 1, /* Ax = */ { 2, /* a[] = */ {} } }; + static BAx bax1 = { 1, /* Ax = */ { 2, /* a[] = */ {} } }; new (bax1.ax.a) char; // { dg-warning "placement" } new (bax1.ax.a) char[2]; // { dg-warning "placement" } Index: gcc/testsuite/g++.dg/warn/Wplacement-new-size-2.C =================================================================== --- gcc/testsuite/g++.dg/warn/Wplacement-new-size-2.C (revision 267569) +++ gcc/testsuite/g++.dg/warn/Wplacement-new-size-2.C (working copy) @@ -33,13 +33,13 @@ void fAx (Ax *px, Ax &rx) void fAx2 () { // Initialization of non-static objects with flexible array members - // isn't allowed in C and should perhaps be disallowed in C++ as + // isn't allowed in C and had to be be disallowed in C++ as // well to avoid c++/69696 - incorrect initialization of block-scope // flexible array members. - Ax ax2 = { 1, { 2, 3 } }; + Ax ax2 = { 1, { 2, 3 } }; // { dg-error "non-static initialization of a flexible array member" } - new (ax2.a) Int16; - new (ax2.a) Int16[1]; + new (ax2.a) Int16; // { dg-warning "placement" } + new (ax2.a) Int16[1]; // { dg-warning "placement" } new (ax2.a) Int16[2]; // { dg-warning "placement" } new (ax2.a) Int32; // { dg-warning "placement" } new (ax2.a) Int32[2]; // { dg-warning "placement" } @@ -140,7 +140,7 @@ void fBx (BAx *pbx, BAx &rbx) void fBx1 () { - BAx bax1 = { 1, /* Ax = */ { 2, /* a[] = */ {} } }; + static BAx bax1 = { 1, /* Ax = */ { 2, /* a[] = */ {} } }; new (bax1.ax.a) char; // { dg-warning "placement" } new (bax1.ax.a) char[2]; // { dg-warning "placement" } Index: gcc/testsuite/g++.dg/warn/Wplacement-new-size-6.C =================================================================== --- gcc/testsuite/g++.dg/warn/Wplacement-new-size-6.C (revision 267569) +++ gcc/testsuite/g++.dg/warn/Wplacement-new-size-6.C (working copy) @@ -15,7 +15,7 @@ struct BAx { int i; Ax ax; }; void fBx1 () { - BAx bax1 = { 1, /* Ax = */ { 2, /* a[] = */ { 3 } } }; // { dg-error "initialization of flexible array member in a nested context" } + static BAx bax1 = { 1, /* Ax = */ { 2, /* a[] = */ { 3 } } }; // { dg-error "initialization of flexible array member in a nested context" } new (bax1.ax.a) char; // { dg-warning "placement" } new (bax1.ax.a) char[2]; // { dg-warning "placement" } @@ -25,7 +25,7 @@ void fBx1 () void fBx2 () { - BAx bax2 = { 1, /* Ax = */ { 2, /* a[] = */ { 3, 4 } } }; // { dg-error "initialization of flexible array member in a nested context" } + static BAx bax2 = { 1, /* Ax = */ { 2, /* a[] = */ { 3, 4 } } }; // { dg-error "initialization of flexible array member in a nested context" } new (bax2.ax.a) char; // { dg-warning "placement" } new (bax2.ax.a) char[2]; // { dg-warning "placement" } @@ -37,7 +37,7 @@ void fBx2 () void fBx3 () { - BAx bax2 = { 1, /* Ax = */ { 3, /* a[] = */ { 4, 5, 6 } } }; // { dg-error "initialization of flexible array member in a nested context" } + static BAx bax2 = { 1, /* Ax = */ { 3, /* a[] = */ { 4, 5, 6 } } }; // { dg-error "initialization of flexible array member in a nested context" } new (bax2.ax.a) char; // { dg-warning "placement" } new (bax2.ax.a) char[2]; // { dg-warning "placement" } Index: gcc/testsuite/gcc.dg/array-6.c =================================================================== --- gcc/testsuite/gcc.dg/array-6.c (revision 267569) +++ gcc/testsuite/gcc.dg/array-6.c (working copy) @@ -1,18 +0,0 @@ -/* PR c/5597 */ -/* { dg-do compile } */ -/* { dg-options "" } */ - -/* Verify that GCC forbids non-static initialization of - flexible array members. */ - -struct str { int len; char s[]; }; - -struct str a = { 2, "a" }; - -void foo() -{ - static struct str b = { 2, "b" }; - struct str c = { 2, "c" }; /* { dg-error "(non-static)|(near initialization)" } */ - struct str d = (struct str) { 2, "d" }; /* { dg-error "(non-static)|(near initialization)" } */ - struct str e = (struct str) { d.len, "e" }; /* { dg-error "(non-static)|(initialization)" } */ -} ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH, C++,rebased] Fix PR c++/88261 2019-01-07 15:38 ` Bernd Edlinger @ 2019-01-07 16:58 ` Jason Merrill 0 siblings, 0 replies; 16+ messages in thread From: Jason Merrill @ 2019-01-07 16:58 UTC (permalink / raw) To: Bernd Edlinger, Martin Sebor, Martin Sebor, gcc-patches, Jeff Law, Nathan Sidwell On 1/7/19 10:38 AM, Bernd Edlinger wrote: > On 1/7/19 1:08 AM, Martin Sebor wrote: >> On 1/5/19 9:04 AM, Bernd Edlinger wrote: >>> On 1/4/19 10:22 PM, Jason Merrill wrote: >>>> Hmm, I'm uncomfortable with starting to pass in the decl just for the sake of deciding whether this diagnostic should be a pedwarn or error. In general, because of copy elision, we can't know at this point what we're initializing, so I'd rather not pretend we can. Instead, maybe add a LOOKUP_ALLOW_FLEXARY_INIT flag that you can add to the flags argument in the call from store_init_value? >>>> >>> >>> Okay, I reworked the patch, to pass a bit in the flags, it was a bit more complicated >>> than anticipated, because it is necessary to pass the flag thru process_init_constructor >>> and friends to the recursive invocation of digest_init_r. It turned out that >>> digest_nsdmi_init did not need to change, since it is always wrong to use flexarray init >>> there. I added a new test case (flexary32.C) to exercises a few cases where non static >>> direct member intializers are allowed to use flexarrays (in static members) and where that >>> would be wrong (in automatic members). So that seems to work. >> >> If that resolves pr69338 can you please also reference the bug in >> the test and in the ChangeLog? (Ditto for pr69697.) >> > > Yes, those appear to be fixed as well. > Added pr69338 + pr69697 to the ChangeLog, > and also added test cases from both PRs. > > > Attached the otherwise unchanged v3 of my patch. > > Is to OK for trunk? OK, thanks. Jason ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2019-01-07 16:58 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-12-15 8:36 [PATCH, C++] Fix PR c++/88261 Bernd Edlinger 2018-12-15 9:33 ` Jakub Jelinek 2018-12-15 10:36 ` Bernd Edlinger 2018-12-19 3:35 ` Jason Merrill 2018-12-20 17:50 ` Martin Sebor 2018-12-20 18:11 ` Martin Sebor 2018-12-20 21:08 ` Bernd Edlinger 2018-12-21 1:04 ` Martin Sebor 2018-12-22 19:38 ` Bernd Edlinger 2019-01-04 15:31 ` [PATCH, C++,rebased] " Bernd Edlinger 2019-01-04 21:23 ` Jason Merrill 2019-01-04 22:23 ` Bernd Edlinger 2019-01-05 16:05 ` Bernd Edlinger 2019-01-07 0:09 ` Martin Sebor 2019-01-07 15:38 ` Bernd Edlinger 2019-01-07 16:58 ` Jason Merrill
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).