reject invalid flexarrays even in anonymous structs (PR 93753)

Message ID a969855a-b842-c7ad-7090-f678143f625c@gmail.com
State New
Headers show
Series
  • reject invalid flexarrays even in anonymous structs (PR 93753)
Related show

Commit Message

Martin Sebor Feb. 17, 2020, 11:06 p.m.
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.

Tested on x86_64-linux.

Martin

Comments

Jason Merrill Feb. 19, 2020, 11:51 p.m. | #1
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
Martin Sebor Feb. 20, 2020, 12:08 a.m. | #2
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
Jason Merrill Feb. 20, 2020, 12:11 a.m. | #3
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.

Patch

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,