This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[PATCH] accept flexible arrays in struct in unions (c++/71912 - [6/7 regression])
- From: Martin Sebor <msebor at gmail dot com>
- To: Gcc Patch List <gcc-patches at gcc dot gnu dot org>, Jason Merrill <jason at redhat dot com>
- Date: Fri, 22 Jul 2016 13:14:01 -0600
- Subject: [PATCH] accept flexible arrays in struct in unions (c++/71912 - [6/7 regression])
- Authentication-results: sourceware.org; auth=none
r231665 committed in the 6.0 cycle tightened up the checking of
flexible array members in C++ with the goal of rejecting code
that could lead to hard to find bugs, while at the same time
accepting (with a warning) benign extensions also accepted in
C mode.
c++/71912 is a complaint that G++ rejects one such extension:
defining structs with flexible array members in a union. When
reviewing the code that causes it to be rejected I realized
that it was, in fact, meant to be accepted and is rejected due
to a bug.
The attached patch fixes that bug and implements more robust
checking for this class of problems. As in C mode, it accepts
"benign" instances of this idiom (with a pedantic warning) while
still rejecting the erroneous cases.
There are outstanding problems with flexible array members in
C++, such as c++/68489 - arrays of flexible array members are
silently accepted. In a follow-on patch I will address at
least a part of the problem. I submit this one separately
since it's considered a regression and as such might be
a candidate for backporting to the 6.x branch.
Is this patch okay for trunk? For 6.x?
Thanks
Martin
PR c++/71912 - [6/7 regression] flexible array in struct in union rejected
gcc/cp/ChangeLog:
2016-07-22 Martin Sebor <msebor@redhat.com>
PR c++/71912
* class.c (struct flexmems_t): Add members.
(find_flexarrays): Add arguments. Correct handling of anonymous
structs.
(diagnose_flexarrays): Adjust to issue warnings in addition to errors.
(check_flexarrays): Add argument.
gcc/testsuite/ChangeLog:
2016-07-22 Martin Sebor <msebor@redhat.com>
PR c++/71912
* g++.dg/ext/flexary18.C: New test.
* g++.dg/ext/flexary4.C: Correct the handling of anonymous structs.
diff --git a/gcc/cp/class.c b/gcc/cp/class.c
index b2db7f8..65b373f 100644
--- a/gcc/cp/class.c
+++ b/gcc/cp/class.c
@@ -149,9 +149,9 @@ static bool accessible_nvdtor_p (tree);
/* Used by find_flexarrays and related. */
struct flexmems_t;
-static void find_flexarrays (tree, flexmems_t *);
+static void find_flexarrays (tree, flexmems_t *, bool = false, tree = NULL_TREE);
static void diagnose_flexarrays (tree, const flexmems_t *);
-static void check_flexarrays (tree, flexmems_t * = NULL);
+static void check_flexarrays (tree, bool = false, flexmems_t * = NULL);
static void check_bases (tree, int *, int *);
static void check_bases_and_members (tree);
static tree create_vtable_ptr (tree, tree *);
@@ -6688,25 +6688,45 @@ field_nonempty_p (const_tree fld)
return false;
}
-/* Used by find_flexarrays and related. */
-struct flexmems_t {
+/* Used by find_flexarrays and related members. */
+
+struct flexmems_t
+{
/* The first flexible array member or non-zero array member found
- in order of layout. */
+ in the order of layout. */
tree array;
/* First non-static non-empty data member in the class or its bases. */
tree first;
- /* First non-static non-empty data member following either the flexible
- array member, if found, or the zero-length array member. */
- tree after;
+ /* A pair of the first non-static non-empty data members following
+ either the flexible array member, if found, or the zero-length
+ array member otherwise. AFTER[1] refers to the first such data
+ member of a union that the struct containing the flexible array
+ member or zero-length array is a member, or NULL when no such
+ union exists. AFTER[0] refers to the first such data member
+ that is not a member of such a union. */
+ tree after[2];
+
+ /* The type in which an anonymous struct or union containing ARRAY
+ is defined.. */
+ tree anonctx;
};
/* Find either the first flexible array member or the first zero-length
array, in that order or preference, among members of class T (but not
- its base classes), and set members of FMEM accordingly. */
+ its base classes), and set members of FMEM accordingly.
+ BASE_P is true if T is a base class of another class.
+ PUN is set to the outermost union of which T is a member if one such
+ union exists, otherwsie to NULL. */
static void
-find_flexarrays (tree t, flexmems_t *fmem)
+find_flexarrays (tree t, flexmems_t *fmem, bool base_p,
+ tree pun /* = NULL_TREE */)
{
+ /* Set the "pointer" to the outsermost enclosing union if not set
+ yet and maintain it for the remainder of the recursion. */
+ if (!pun && TREE_CODE (t) == UNION_TYPE)
+ pun = t;
+
for (tree fld = TYPE_FIELDS (t), next; fld; fld = next)
{
/* Find the next non-static data member if it exists. */
@@ -6714,17 +6734,48 @@ find_flexarrays (tree t, flexmems_t *fmem)
(next = DECL_CHAIN (next))
&& TREE_CODE (next) != FIELD_DECL; );
+ /* Type of the member. */
tree fldtype = TREE_TYPE (fld);
+
if (TREE_CODE (fld) != TYPE_DECL
&& RECORD_OR_UNION_TYPE_P (fldtype)
- && TYPE_ANONYMOUS_P (fldtype))
+ && (FIELD_DECL != TREE_CODE (fld) || !DECL_FIELD_IS_BASE (fld)))
{
- /* Members of anonymous structs and unions are treated as if
- they were members of the containing class. Descend into
- the anonymous struct or union and find a flexible array
- member or zero-length array among its fields. */
- find_flexarrays (fldtype, fmem);
- continue;
+ /* Descend into the member struct or union and try to find
+ a flexible array member or zero-length array among its
+ members. */
+
+ tree first = fmem->first;
+ tree array = fmem->array;
+
+ find_flexarrays (fldtype, fmem, false, pun);
+
+ if (fmem->array != array)
+ {
+ /* If the member struct contains the first flexible array
+ member, store the enclosing struct if it is anonymous.
+ If it isn't anonymous and a prior member has been seen
+ in one of the enclosing structs, clear the FIRST member
+ since it doesn't contribute to the flexible array struct's
+ members. */
+
+ if (TYPE_ANONYMOUS_P (fldtype))
+ {
+ if (!DECL_NAME (fld) || anon_aggrname_p (DECL_NAME (fld)))
+ fmem->anonctx = t;
+ else if (fmem->first == first)
+ fmem->first = NULL_TREE;
+ }
+
+ continue;
+ }
+
+ /* If the member struct contains the first flexible array
+ member, or if this member is a base class, continue to
+ the next member and avoid setting the FMEM->NEXT pointer
+ to point to it. */
+ if (base_p)
+ continue;
}
/* Skip anything that's not a (non-static) data member. */
@@ -6744,8 +6795,8 @@ find_flexarrays (tree t, flexmems_t *fmem)
/* Remember the first non-static data member after the flexible
array member, if one has been found, or the zero-length array
if it has been found. */
- if (!fmem->after && fmem->array)
- fmem->after = fld;
+ if (fmem->array && !fmem->after[bool (pun)])
+ fmem->after[bool (pun)] = fld;
}
/* Skip non-arrays. */
@@ -6761,8 +6812,8 @@ find_flexarrays (tree t, flexmems_t *fmem)
such field or a flexible array member has been seen to
handle the pathological and unlikely case of multiple
such members. */
- if (!fmem->after)
- fmem->after = fld;
+ if (!fmem->after[bool (pun)])
+ fmem->after[bool (pun)] = fld;
}
else if (integer_all_onesp (TYPE_MAX_VALUE (TYPE_DOMAIN (fldtype))))
/* Remember the first zero-length array unless a flexible array
@@ -6778,8 +6829,8 @@ find_flexarrays (tree t, flexmems_t *fmem)
reset the after pointer. */
if (TYPE_DOMAIN (TREE_TYPE (fmem->array)))
{
+ fmem->after[bool (pun)] = NULL_TREE;
fmem->array = fld;
- fmem->after = NULL_TREE;
}
}
else
@@ -6797,47 +6848,94 @@ diagnose_flexarrays (tree t, const flexmems_t *fmem)
{
/* Members of anonymous structs and unions are considered to be members
of the containing struct or union. */
- if (TYPE_ANONYMOUS_P (t) || !fmem->array)
+ if (!fmem->array)
return;
- const char *msg = 0;
+ /* Issue errors first, and when no errors are found, then warnings
+ for flexible array members of structs in unions. */
+ for (int in_union = false; in_union != 2; ++in_union) {
- if (TYPE_DOMAIN (TREE_TYPE (fmem->array)))
- {
- if (fmem->after)
- msg = G_("zero-size array member %qD not at end of %q#T");
- else if (!fmem->first)
- msg = G_("zero-size array member %qD in an otherwise empty %q#T");
+ /* If the The declaration context */
+ tree fmemctx = fmem->anonctx ? fmem->anonctx : t;
- if (msg && pedwarn (DECL_SOURCE_LOCATION (fmem->array),
- OPT_Wpedantic, msg, fmem->array, t))
+ const char *msg = 0;
- inform (location_of (t), "in the definition of %q#T", t);
- }
- else
- {
- if (fmem->after)
- msg = G_("flexible array member %qD not at end of %q#T");
- else if (!fmem->first)
- msg = G_("flexible array member %qD in an otherwise empty %q#T");
+ if (TYPE_DOMAIN (TREE_TYPE (fmem->array)))
+ {
+ if (fmem->after[in_union])
+ msg = (in_union
+ ? G_("zero-size array member %qD belonging to %q#T")
+ : G_("zero-size array member %qD not at end of %q#T"));
+ else if (!fmem->first)
+ msg = G_("zero-size array member %qD in an otherwise empty %q#T");
+
+ if (msg && pedwarn (DECL_SOURCE_LOCATION (fmem->array),
+ OPT_Wpedantic, msg, fmem->array, fmemctx))
+ {
+ inform (location_of (t), "in the definition of %q#T", fmemctx);
+
+ /* Prevent the same flexible array member from being diagnosed
+ more than once if it happens to be nested in more than one
+ union and overlap with another member. This avoids multiple
+ warnings for perverse cases like the the following where
+ U::U1::X::a1 would otherwise be diagnosed first followed by
+ S::U1::X::a1:
+ struct S {
+ union U {
+ union U1 { struct X { int n1, a1[]; } x1; } u1;
+ union U2 { struct X { int n2, a2[]; } x1; } u2;
+ } u;
+ } s;
+ */
+ TREE_NO_WARNING (fmem->array) = 1;
+ }
+ }
+ else
+ {
+ if (fmem->after[in_union])
+ msg = (in_union
+ ? G_("flexible array member %qD belonging to %q#T")
+ : G_("flexible array member %qD not at end of %q#T"));
+ else if (!fmem->first)
+ msg = G_("flexible array member %qD in an otherwise empty %q#T");
+
+ if (msg)
+ {
+ location_t loc = DECL_SOURCE_LOCATION (fmem->array);
- if (msg)
- {
- error_at (DECL_SOURCE_LOCATION (fmem->array), msg,
- fmem->array, t);
-
- /* In the unlikely event that the member following the flexible
- array member is declared in a different class, point to it.
- Otherwise it should be obvious. */
- if (fmem->after
- && (DECL_CONTEXT (fmem->after) != DECL_CONTEXT (fmem->array)))
- inform (DECL_SOURCE_LOCATION (fmem->after),
- "next member %q#D declared here",
- fmem->after);
-
- inform (location_of (t), "in the definition of %q#T", t);
- }
- }
+ /* A union containing a struct with a flexible array member,
+ followed by another member (of the union) is diagnosed
+ with a warning for compatibility with GCC (C mode), even
+ though it's not valid accoding to C11. */
+ if (in_union)
+ {
+ if (!TREE_NO_WARNING (fmem->array))
+ pedwarn (loc, OPT_Wpedantic, msg, fmem->array, fmemctx);
+ }
+ else
+ error_at (loc, msg, fmem->array, fmemctx);
+
+ TREE_NO_WARNING (fmem->array) = 1;
+
+ /* In the unlikely event that the member following the flexible
+ array member is declared in a different class, point to it.
+ Otherwise it should be obvious. */
+ if (fmem->after[in_union]
+ && (DECL_CONTEXT (fmem->after[in_union])
+ != DECL_CONTEXT (fmem->array)))
+ inform (DECL_SOURCE_LOCATION (fmem->after[in_union]),
+ (in_union ? "overlaps member %q#D declared here"
+ : "next member %q#D declared here"),
+ fmem->after[in_union]);
+
+ inform (location_of (t), "in the definition of %q#T", t);
+
+ /* Avoid issuing further duiagnostics after the error above. */
+ if (!in_union)
+ break;
+ }
+ }
+ }
}
@@ -6850,7 +6948,8 @@ diagnose_flexarrays (tree t, const flexmems_t *fmem)
that fails the checks. */
static void
-check_flexarrays (tree t, flexmems_t *fmem /* = NULL */)
+check_flexarrays (tree t, bool base_p /* = false */,
+ flexmems_t *fmem /* = NULL */)
{
/* Initialize the result of a search for flexible array and zero-length
array members. Avoid doing any work if the most interesting FMEM data
@@ -6858,14 +6957,17 @@ check_flexarrays (tree t, flexmems_t *fmem /* = NULL */)
flexmems_t flexmems = flexmems_t ();
if (!fmem)
fmem = &flexmems;
- else if (fmem->array && fmem->first && fmem->after)
+ else if (fmem->array && fmem->first
+ && fmem->after[false] && fmem->after[true])
return;
+ tree fam = fmem->array;
+
/* Recursively check the primary base class first. */
if (CLASSTYPE_HAS_PRIMARY_BASE_P (t))
{
tree basetype = BINFO_TYPE (CLASSTYPE_PRIMARY_BINFO (t));
- check_flexarrays (basetype, fmem);
+ check_flexarrays (basetype, true, fmem);
}
/* Recursively check the base classes. */
@@ -6883,7 +6985,7 @@ check_flexarrays (tree t, flexmems_t *fmem /* = NULL */)
continue;
/* Check the base class. */
- check_flexarrays (BINFO_TYPE (base_binfo), fmem);
+ check_flexarrays (BINFO_TYPE (base_binfo), true, fmem);
}
if (fmem == &flexmems)
@@ -6900,17 +7002,21 @@ check_flexarrays (tree t, flexmems_t *fmem /* = NULL */)
/* Check the virtual base class. */
tree basetype = TREE_TYPE (base_binfo);
- check_flexarrays (basetype, fmem);
+ check_flexarrays (basetype, true, fmem);
}
}
- /* Search the members of the current (derived) class. */
- find_flexarrays (t, fmem);
+ /* Search the members of the current (possibly derived) class. */
+ find_flexarrays (t, fmem, base_p || fam != fmem->array);
- if (fmem == &flexmems)
+ if (fmem == &flexmems
+ && !TYPE_ANONYMOUS_P (t) && !anon_aggrname_p (TYPE_IDENTIFIER (t)))
{
- /* Issue diagnostics for invalid flexible and zero-length array members
- found in base classes or among the members of the current class. */
+ /* Issue diagnostics for invalid flexible and zero-length array
+ members found in base classes or among the members of the current
+ class. Ignore anonymous structs and unions whose members are
+ considered to be members of the enclosing class and thus will
+ be diagnosed when checking it. */
diagnose_flexarrays (t, fmem);
}
}
diff --git a/gcc/testsuite/g++.dg/ext/flexary18.C b/gcc/testsuite/g++.dg/ext/flexary18.C
new file mode 100644
index 0000000..ddf55a8
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/flexary18.C
@@ -0,0 +1,217 @@
+// PR c++/71912 - [6/7 regression] flexible array in struct in union rejected
+// { dg-do compile }
+// { dg-additional-options "-Wpedantic -Wno-error=pedantic" }
+
+namespace pr71912 {
+
+struct foo {
+ int a;
+ char s[]; // { dg-warning "flexible array member" }
+};
+
+struct bar {
+ double d; // { dg-message "overlaps member" }
+ char t[];
+};
+
+struct baz { // { dg-message "in the definition" }
+ union {
+ struct foo f;
+ struct bar b;
+ } u;
+};
+
+struct xyyzy { // { dg-message "in the definition" }
+ union {
+ struct {
+ int a;
+ char s[]; // { dg-warning "flexible array member" }
+ } f;
+ struct {
+ double d; // { dg-message "overlaps member" }
+ char t[];
+ } b;
+ } u;
+};
+
+struct baz b;
+struct xyyzy x;
+
+}
+
+// The following definitions aren't strictly valid but, like those above,
+// are accepted for compatibility with GCC (in C mode). They are benign
+// in that the flexible array member is at the highest offset within
+// the outermost type and doesn't overlap with other members except for
+// those of the union.
+union UnionStruct1 {
+ struct { int n1, a[]; } s; // { dg-warning "flexible array member" }
+ int n2; // { dg-message "overlaps" }
+};
+
+union UnionStruct2 {
+ struct { int n1, a1[]; } s1; // { dg-warning "flexible array member" }
+ struct { int n2, a2[]; } s2; // { dg-message "overlaps" }
+ int n3;
+};
+
+union UnionStruct3 {
+ struct { int n1, a1[]; } s1; // { dg-warning "flexible array member" }
+ struct { double n2, a2[]; } s2; // { dg-message "overlaps" }
+ char n3;
+};
+
+union UnionStruct4 {
+ struct { int n1, a1[]; } s1; // { dg-warning "flexible array member" }
+ struct {
+ struct { double n2, a2[]; } s2; // { dg-message "overlaps" }
+ } s3;
+ char n3;
+};
+
+union UnionStruct5 {
+ struct { struct { int n1, a1[]; } s1; } s2; // { dg-warning "flexible array" }
+ struct { double n2, a2[]; } s3; // { dg-message "overlaps" }
+ char n3;
+};
+
+union UnionStruct6 {
+ struct { struct { int n1, a1[]; } s1; } s2; // { dg-warning "flexible array" }
+ struct {
+ struct { double n2, a2[]; } s3; // { dg-message "overlaps" }
+ } s4;
+ char n3;
+};
+
+union UnionStruct7 {
+ struct { int n1, a1[]; } s1; // { dg-warning "flexible array" }
+ struct { double n2, a2[]; } s2; // { dg-message "overlaps" }
+ struct { struct { int n3, a3[]; } s3; } s4;
+};
+
+union UnionStruct8 {
+ struct { int n1, a1[]; } s1; // { dg-warning "flexible array" }
+ struct {
+ struct { double n2, a2[]; } s2; // { dg-message "overlaps" }
+ } s3;
+ struct { struct { int n3, a3[]; } s4; } s5;
+};
+
+union UnionStruct9 {
+ struct { struct { int n1, a1[]; } s1; } s2;// { dg-warning "flexible array" }
+ struct {
+ struct { double n2, a2[]; } s3; // { dg-message "overlaps" }
+ } s4;
+ struct { struct { int n3, a3[]; } s5; } s6;
+};
+
+struct StructUnion1 {
+ union {
+ struct { int n1, a1[]; } s1; // { dg-warning "flexible array" }
+ struct { double n2, a2[]; } s2; // { dg-message "overlaps" }
+ char n3;
+ } u;
+};
+
+// The following are invalid and rejected.
+struct StructUnion2 {
+ union {
+ struct { int n1, a1[]; } s1; // { dg-error "not at end" }
+ } u;
+ char n3; // { dg-message "next member" }
+};
+
+struct StructUnion3 {
+ union {
+ struct { int n1, a1[]; } s1; // { dg-error "not at end" }
+ struct { double n2, a2[]; } s2;
+ } u;
+ char n3; // { dg-message "next member" }
+};
+
+struct StructUnion4 {
+ union {
+ struct { int n1, a1[]; } s1; // { dg-error "not at end" }
+ } u1;
+ union {
+ struct { double n2, a2[]; } s2;
+ } u2; // { dg-message "next member" }
+};
+
+struct StructUnion5 {
+ union {
+ union {
+ struct { int n1, a1[]; } s1; // { dg-warning "flexible array" }
+ } u1;
+ union {
+ struct { double n2, a2[]; } s2; // { dg-message "overlaps" }
+ } u2;
+ } u;
+};
+
+struct StructUnion6 {
+ union {
+ struct { int n1, a1[]; } s1; // { dg-warning "flexible array" }
+ union {
+ struct { double n2, a2[]; } s2; // { dg-message "overlaps" }
+ } u2;
+ } u;
+};
+
+struct StructUnion7 {
+ union {
+ union {
+ struct { double n2, a2[]; } s2; // { dg-warning "flexible array" }
+ } u2;
+ struct { int n1, a1[]; } s1; // { dg-message "overlaps" }
+ } u;
+};
+
+struct StructUnion8 {
+ struct {
+ union {
+ union {
+ struct { int n1, a1[]; } s1; // { dg-error "not at end" }
+ } u1;
+ union {
+ struct { double n2, a2[]; } s2;
+ } u2;
+ } u;
+ } s1;
+
+ struct {
+ union {
+ union {
+ struct { int n1, a1[]; } s1;
+ } u1;
+ union {
+ struct { double n2, a2[]; } s2;
+ } u2;
+ } u; // { dg-message "next member" }
+ } s2;
+};
+
+struct StructUnion9 {
+ struct A1 {
+ union B1 {
+ union C1 {
+ struct Sx1 { int n1, a1[]; } sx1; // { dg-error "not at end" }
+ // { dg-warning "flexible array" "" { target *-*-*-* } 198 }
+ } c1;
+ union D1 {
+ struct Sx2 { double n2, a2[]; } sx2;
+ } d1;
+ } b1;
+ } a1;
+
+ struct A2 {
+ union B2 {
+ union C2 {
+ struct Sx3 { int n3, a3[]; } sx3; // { dg-warning "flexible array" }
+ } c2;
+ union D2 {
+ struct Sx3 { double n4, a4[]; } sx4; // { dg-message "overlaps member" }
+ } d2;
+ } b2;
+ } a2;
+};
diff --git a/gcc/testsuite/g++.dg/ext/flexary4.C b/gcc/testsuite/g++.dg/ext/flexary4.C
index 97ec625..29d6bdd 100644
--- a/gcc/testsuite/g++.dg/ext/flexary4.C
+++ b/gcc/testsuite/g++.dg/ext/flexary4.C
@@ -102,31 +102,28 @@ struct Sx17 {
int a_0 [0];
};
-// Empty structs are a GCC extension that (in C++ only) is treated
-// as if it had a single member of type char. Therefore, a struct
+// An empty struct is treated as if it had a single member of type
+// char but the member cannot be accessed. Therefore, a struct
// containing a flexible array member followed by an empty struct
// is diagnosed to prevent the former subobject from sharing space
// with the latter.
struct Sx18 {
int a_x []; // { dg-error "flexible array member" }
- struct S { };
+ struct { /* empty */ } s;
};
-// Anonymous structs and unions are another GCC extension. Since
-// they cannot be named and thus used to store the size of a flexible
-// array member, a struct containing both is diagnosed as if
-// the flexible array member appeared alone.
+// Anonymous structs are a G++ extension. Members of anonymous structs
+// are treated as if they were declared in the enclosing class.
struct Sx19 {
- struct S { };
- union U { };
- int a_x []; // { dg-error "in an otherwise empty" }
+ struct { int i; }; // anonymous struct
+ int a_x [];
};
-// Unlike in the case above, a named member of an anonymous struct
-// prevents a subsequent flexible array member from being diagnosed.
+// Unlike in the case above, a named struct is not anonymous and
+// so doesn't contribute its member to that of the enclosing struct.
struct Sx20 {
- struct S { } s;
- int a_x [];
+ struct S { int i; };
+ int a_x []; // { dg-error "in an otherwise empty" }
};
struct Sx21 {
@@ -298,6 +295,15 @@ struct Anon1 {
ASSERT_AT_END (Anon1, good);
+struct NotAnon1 {
+ int n;
+ // The following is not an anonymous struct -- the type is unnamed
+ // but the object has a name.
+ struct {
+ int bad[]; // { dg-error "otherwise empty" }
+ } name;
+};
+
struct Anon2 {
struct {
int n;
@@ -352,7 +358,6 @@ struct Anon7 {
int n;
};
-
struct Six {
int i;
int a[];