| Message ID | a969855a-b842-c7ad-7090-f678143f625c@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | reject invalid flexarrays even in anonymous structs (PR 93753) | expand |
On 2/18/20 12:06 AM, Martin Sebor wrote: > The improved checking of flexible array members committed in r231665 > (and later) deliberately excluded anonymous structs and unions > (unnamed members of types with no tags) to allow a harmless extension > that was accepted by G++ prior to the change. However, being overly > broad, the exemption prevents flexible array members that are invalid > for other reasons from being diagnosed in these contexts. That in > turn leads to a subset of such cases causing ICEs downstream. > > The attached trivial patch tightens up the conditions under which > this extension is accepted: it only considers such structs when they > are members, thus eliminating the ICE. The comment says, Ignore anonymous structs and unions whose members are considered to be members of the enclosing class and thus will be diagnosed when checking it. Certainly if there is no enclosing class this is not the case, so the patch is an improvement. Do we already properly diagnose an unnamed struct used to declare a data member? Jason
On 2/19/20 4:51 PM, Jason Merrill wrote: > On 2/18/20 12:06 AM, Martin Sebor wrote: >> The improved checking of flexible array members committed in r231665 >> (and later) deliberately excluded anonymous structs and unions >> (unnamed members of types with no tags) to allow a harmless extension >> that was accepted by G++ prior to the change. However, being overly >> broad, the exemption prevents flexible array members that are invalid >> for other reasons from being diagnosed in these contexts. That in >> turn leads to a subset of such cases causing ICEs downstream. >> >> The attached trivial patch tightens up the conditions under which >> this extension is accepted: it only considers such structs when they >> are members, thus eliminating the ICE. > > The comment says, > > Ignore anonymous structs and unions whose members are > considered to be members of the enclosing class and thus will > be diagnosed when checking it. > > Certainly if there is no enclosing class this is not the case, so the > patch is an improvement. Do we already properly diagnose an unnamed > struct used to declare a data member? Yes. A few cases verify this in the new test (in conjunction with different forms of invalid initialization), but there are a whole bunch in flexary19.C that don't also exercise initialization. I'm sure more could be added so if you think of something interesting that might be worth adding to the test suite I'm happy to do it. Martin
On 2/20/20 1:08 AM, Martin Sebor wrote: > On 2/19/20 4:51 PM, Jason Merrill wrote: >> On 2/18/20 12:06 AM, Martin Sebor wrote: >>> The improved checking of flexible array members committed in r231665 >>> (and later) deliberately excluded anonymous structs and unions >>> (unnamed members of types with no tags) to allow a harmless extension >>> that was accepted by G++ prior to the change. However, being overly >>> broad, the exemption prevents flexible array members that are invalid >>> for other reasons from being diagnosed in these contexts. That in >>> turn leads to a subset of such cases causing ICEs downstream. >>> >>> The attached trivial patch tightens up the conditions under which >>> this extension is accepted: it only considers such structs when they >>> are members, thus eliminating the ICE. >> >> The comment says, >> >> Ignore anonymous structs and unions whose members are >> considered to be members of the enclosing class and thus will >> be diagnosed when checking it. >> >> Certainly if there is no enclosing class this is not the case, so the >> patch is an improvement. Do we already properly diagnose an unnamed >> struct used to declare a data member? > > Yes. A few cases verify this in the new test (in conjunction with > different forms of invalid initialization), but there are a whole > bunch in flexary19.C that don't also exercise initialization. I'm > sure more could be added so if you think of something interesting > that might be worth adding to the test suite I'm happy to do it. Sounds good, the patch is OK.
PR c++/93753 - ICE on a flexible array followed by a member in an anonymous struct with an initializer gcc/cp/ChangeLog: PR gcov-profile/93753 * class.c (check_flexarrays): Tighten up a test for potential members of anonymous structs or unions. gcc/testsuite/ChangeLog: PR gcov-profile/93753 * g++.dg/ext/flexary36.C: New test. * g++.dg/lto/pr93166_0.C: Make struct with flexarray valid. diff --git a/gcc/cp/class.c b/gcc/cp/class.c index a1fd1aa91cc..772134df5fc 100644 --- a/gcc/cp/class.c +++ b/gcc/cp/class.c @@ -7142,6 +7142,8 @@ check_flexarrays (tree t, flexmems_t *fmem /* = NULL */, /* Is the type unnamed (and therefore a member of it potentially an anonymous struct or union)? */ bool maybe_anon_p = TYPE_UNNAMED_P (t); + if (tree ctx = maybe_anon_p ? TYPE_CONTEXT (t) : NULL_TREE) + maybe_anon_p = RECORD_OR_UNION_TYPE_P (ctx); /* Search the members of the current (possibly derived) class, skipping unnamed structs and unions since those could be anonymous. */ diff --git a/gcc/testsuite/g++.dg/ext/flexary36.C b/gcc/testsuite/g++.dg/ext/flexary36.C new file mode 100644 index 00000000000..220ccc0ccf8 --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/flexary36.C @@ -0,0 +1,123 @@ +/* PR c++/93753 - ICE on a flexible array followed by a member in + an anonymous struct with an initializer + { dg-do compile } + { dg-options "-Wall -Wno-unused-variable" } */ + +struct { + int a[]; // { dg-error "flexible array member '<unnamed struct>::a' not at end of 'struct<unnamed>'" } + int b; +} ax; + +struct { + int a[]; // { dg-error "flexible array member '<unnamed struct>::a' not at end of 'struct<unnamed>'" } + int b; +} bx = { }; + +struct { + int a[]; // { dg-error "flexible array member '<unnamed struct>::a' not at end of 'struct<unnamed>'" } + int b; +} cx = { 0 }; + +struct { + int a[]; // { dg-error "flexible array member '<unnamed struct>::a' not at end of 'struct<unnamed>'" } + int b; +} dx = { 1 }; + + +union { + int a[]; // { dg-error "flexible array member in union" } + int b; +} du = { 1 }; + + +struct A { + int a[]; // { dg-error "flexible array member 'A::a' not at end of 'struct A'" } + int b; +} a; + +struct B { + int a[]; // { dg-error "flexible array member 'B::a' not at end of 'struct B'" } + int b; +} b = { }; + +struct C { + int a[]; // { dg-error "flexible array member 'C::a' not at end of 'struct C'" } + int b; +} c = { 0 }; + +struct D { + int a[]; // { dg-error "flexible array member 'D::a' not at end of 'struct D'" } + int b; +} d = { 1 }; + + +struct E { + struct { + int a[]; // { dg-error " not at end " } + int b; + } e = { 1 }; // { dg-error "non-static initialization of a flexible array member" } +}; + +struct G { + struct { + int a[]; // { dg-error " not at end " } + int b; + }; +} g = { 1 }; // { dg-error "initialization of flexible array member in a nested context" } + +struct H { + int i; + struct { + int a[]; // { dg-error " not at end " } + int b; + }; +} h = { 1 }; + +namespace { + +struct { + int a[]; // { dg-error " not at end of " } + int b; +} ax; + +struct { + int a[]; // { dg-error " not at end " } + int b; +} bx = { }; + +struct { + int a[]; // { dg-error " not at end " } + int b; +} cx = { 0 }; + +struct { + int a[]; // { dg-error " not at end " } + int b; +} dx = { 1 }; + + +struct A { + int a[]; // { dg-error " not at end of 'struct {anonymous}::A'" } + int b; +} a; + +struct B { + int a[]; // { dg-error " not at end of 'struct {anonymous}::B'" } + int b; +} b = { }; + +struct C { + int a[]; // { dg-error " not at end of 'struct {anonymous}::C'" } + int b; +} c = { 0 }; + +struct D { + int a[]; // { dg-error " not at end of 'struct {anonymous}::D'" } + int b; +} d = { 1 }; + +} + +// { dg-prune-output "unnamed type with no linkage used to declare variable" } +// { dg-prune-output "non-static data member initializers" } +// { dg-prune-output "extended initializer lists" } diff --git a/gcc/testsuite/g++.dg/lto/pr93166_0.C b/gcc/testsuite/g++.dg/lto/pr93166_0.C index 52f7ddf4016..a83ba6e0400 100644 --- a/gcc/testsuite/g++.dg/lto/pr93166_0.C +++ b/gcc/testsuite/g++.dg/lto/pr93166_0.C @@ -109,7 +109,7 @@ public: QSignalMapper *m_sortSignalMapper; }; struct { - int data[]; + int n, data[]; } b; unsigned c[]{}; void TreeView::qt_static_metacall(QObject *p1, QMetaObject::Call, int,