* [PATCH] middle-end: Reject flexible array members in __builtin_clear_padding [PR97943]
@ 2020-11-24 10:33 Jakub Jelinek
2020-11-24 10:56 ` Richard Biener
0 siblings, 1 reply; 3+ messages in thread
From: Jakub Jelinek @ 2020-11-24 10:33 UTC (permalink / raw)
To: Richard Biener, Jason Merrill, Jonathan Wakely; +Cc: gcc-patches
Hi!
As mentioned in the PR, we currently ICE on flexible array members in
structs and unions during __builtin_clear_padding processing.
Jason said in the PR he'd prefer an error in these cases over forcefully
handling it as [0] arrays (everything is padding then) or consider the
arrays to have as many whole elements as would fit into the tail padding.
So, this patch implements that.
Ok for trunk if it passes bootstrap/regtest? So far has been tested just
on the dg.exp=builtin-clear-padding* dg-torture.exp=builtin-clear-padding*
check-gcc check-c++-all testsuite, but the builtin is currently not used
in anything else.
2020-11-24 Jakub Jelinek <jakub@redhat.com>
PR middle-end/97943
* gimple-fold.c (clear_padding_union, clear_padding_type): Error on and
ignore flexible array member fields. Ignore fields with
error_mark_node type.
* c-c++-common/builtin-clear-padding-2.c: New test.
* c-c++-common/builtin-clear-padding-3.c: New test.
* g++.dg/ext/builtin-clear-padding-1.C: New test.
* gcc.dg/builtin-clear-padding-2.c: New test.
--- gcc/gimple-fold.c.jj 2020-11-23 17:01:48.255054823 +0100
+++ gcc/gimple-fold.c 2020-11-24 09:53:17.724943252 +0100
@@ -4255,6 +4255,17 @@ clear_padding_union (clear_padding_struc
for (tree field = TYPE_FIELDS (type); field; field = DECL_CHAIN (field))
if (TREE_CODE (field) == FIELD_DECL)
{
+ if (DECL_SIZE_UNIT (field) == NULL_TREE)
+ {
+ if (TREE_TYPE (field) == error_mark_node)
+ continue;
+ gcc_assert (TREE_CODE (TREE_TYPE (field)) == ARRAY_TYPE
+ && !COMPLETE_TYPE_P (TREE_TYPE (field)));
+ error_at (buf->loc, "flexible array member %qD does not have "
+ "well defined padding bits for %qs",
+ field, "__builtin_clear_padding");
+ continue;
+ }
HOST_WIDE_INT fldsz = tree_to_shwi (DECL_SIZE_UNIT (field));
gcc_assert (union_buf->size == 0);
union_buf->off = start_off;
@@ -4372,11 +4383,12 @@ clear_padding_type (clear_padding_struct
for (tree field = TYPE_FIELDS (type); field; field = DECL_CHAIN (field))
if (TREE_CODE (field) == FIELD_DECL)
{
+ tree ftype = TREE_TYPE (field);
if (DECL_BIT_FIELD (field))
{
if (DECL_NAME (field) == NULL_TREE)
continue;
- HOST_WIDE_INT fldsz = TYPE_PRECISION (TREE_TYPE (field));
+ HOST_WIDE_INT fldsz = TYPE_PRECISION (ftype);
if (fldsz == 0)
continue;
HOST_WIDE_INT pos = int_byte_position (field);
@@ -4439,6 +4451,16 @@ clear_padding_type (clear_padding_struct
}
}
}
+ else if (DECL_SIZE_UNIT (field) == NULL_TREE)
+ {
+ if (ftype == error_mark_node)
+ continue;
+ gcc_assert (TREE_CODE (ftype) == ARRAY_TYPE
+ && !COMPLETE_TYPE_P (ftype));
+ error_at (buf->loc, "flexible array member %qD does not have "
+ "well defined padding bits for %qs",
+ field, "__builtin_clear_padding");
+ }
else
{
HOST_WIDE_INT pos = int_byte_position (field);
--- gcc/testsuite/c-c++-common/builtin-clear-padding-2.c.jj 2020-11-24 09:54:36.331055770 +0100
+++ gcc/testsuite/c-c++-common/builtin-clear-padding-2.c 2020-11-24 10:16:30.468221320 +0100
@@ -0,0 +1,17 @@
+/* PR middle-end/97943 */
+/* { dg-do compile } */
+/* { dg-options "" } */
+
+struct S { int a; char b[] __attribute__((aligned (2 * sizeof (int)))); };
+struct T { int a; struct S b; };
+union U { int a; struct S b; };
+struct V { int a; union U b; };
+
+void
+foo (struct S *s, struct T *t, union U *u, struct V *v)
+{
+ __builtin_clear_padding (s); /* { dg-error "flexible array member '(S::)?b' does not have well defined padding bits for '__builtin_clear_padding'" } */
+ __builtin_clear_padding (t); /* { dg-error "flexible array member '(S::)?b' does not have well defined padding bits for '__builtin_clear_padding'" } */
+ __builtin_clear_padding (u); /* { dg-error "flexible array member '(S::)?b' does not have well defined padding bits for '__builtin_clear_padding'" } */
+ __builtin_clear_padding (v); /* { dg-error "flexible array member '(S::)?b' does not have well defined padding bits for '__builtin_clear_padding'" } */
+}
--- gcc/testsuite/c-c++-common/builtin-clear-padding-3.c.jj 2020-11-24 10:09:34.735913452 +0100
+++ gcc/testsuite/c-c++-common/builtin-clear-padding-3.c 2020-11-24 10:16:41.732094196 +0100
@@ -0,0 +1,15 @@
+/* PR middle-end/97943 */
+/* { dg-do compile } */
+/* { dg-options "" } */
+
+union U { int a; char b[] __attribute__((aligned (2 * sizeof (int)))); }; /* { dg-error "flexible array member in union" } */
+struct V { int a; union U b; };
+struct W { int a; union U b; int c; };
+
+void
+foo (union U *u, struct V *v, struct W *w)
+{
+ __builtin_clear_padding (u);
+ __builtin_clear_padding (v);
+ __builtin_clear_padding (w);
+}
--- gcc/testsuite/g++.dg/ext/builtin-clear-padding-1.C.jj 2020-11-24 10:10:49.315071724 +0100
+++ gcc/testsuite/g++.dg/ext/builtin-clear-padding-1.C 2020-11-24 10:17:22.461634498 +0100
@@ -0,0 +1,15 @@
+// PR middle-end/97943
+// { dg-do compile }
+// { dg-options "" }
+
+struct S { int a; char b[] __attribute__((aligned (2 * sizeof (int)))); }; // { dg-error "flexible array member 'S::b' not at end of 'struct \[TV]'" }
+struct T { int a; struct S b; int c; }; // { dg-message "next member 'int T::c' declared here|in the definition of 'struct T'" }
+union U { int a; struct S b; };
+struct V { int a; union U b; int : 15; int c; }; // { dg-message "next member 'int V::c' declared here|in the definition of 'struct V'" }
+
+void
+foo (struct T *t, struct V *v)
+{
+ __builtin_clear_padding (t); // { dg-error "flexible array member 'S::b' does not have well defined padding bits for '__builtin_clear_padding'" }
+ __builtin_clear_padding (v); // { dg-error "flexible array member 'S::b' does not have well defined padding bits for '__builtin_clear_padding'" }
+}
--- gcc/testsuite/gcc.dg/builtin-clear-padding-2.c.jj 2020-11-24 10:08:47.543446090 +0100
+++ gcc/testsuite/gcc.dg/builtin-clear-padding-2.c 2020-11-24 10:17:03.031853796 +0100
@@ -0,0 +1,15 @@
+/* PR middle-end/97943 */
+/* { dg-do compile } */
+/* { dg-options "" } */
+
+struct S { int a; char b[] __attribute__((aligned (2 * sizeof (int)))); };
+struct T { int a; struct S b; int c; };
+union U { int a; struct S b; };
+struct V { int a; union U b; int : 15; int c; };
+
+void
+foo (struct T *t, struct V *v)
+{
+ __builtin_clear_padding (t); /* { dg-error "flexible array member 'b' does not have well defined padding bits for '__builtin_clear_padding'" } */
+ __builtin_clear_padding (v); /* { dg-error "flexible array member 'b' does not have well defined padding bits for '__builtin_clear_padding'" } */
+}
Jakub
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] middle-end: Reject flexible array members in __builtin_clear_padding [PR97943]
2020-11-24 10:33 [PATCH] middle-end: Reject flexible array members in __builtin_clear_padding [PR97943] Jakub Jelinek
@ 2020-11-24 10:56 ` Richard Biener
2020-11-24 11:03 ` Jakub Jelinek
0 siblings, 1 reply; 3+ messages in thread
From: Richard Biener @ 2020-11-24 10:56 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Jason Merrill, Jonathan Wakely, gcc-patches
On Tue, 24 Nov 2020, Jakub Jelinek wrote:
> Hi!
>
> As mentioned in the PR, we currently ICE on flexible array members in
> structs and unions during __builtin_clear_padding processing.
>
> Jason said in the PR he'd prefer an error in these cases over forcefully
> handling it as [0] arrays (everything is padding then) or consider the
> arrays to have as many whole elements as would fit into the tail padding.
>
> So, this patch implements that.
>
> Ok for trunk if it passes bootstrap/regtest? So far has been tested just
> on the dg.exp=builtin-clear-padding* dg-torture.exp=builtin-clear-padding*
> check-gcc check-c++-all testsuite, but the builtin is currently not used
> in anything else.
So code-gen wise this will still elide the builtin but error, right?
OK then.
Note it still leaves struct { int n; char c[1]; } to be handled
which I think is the actual way we'll face "flexarray" members.
IMHO we need to treat them as data but maybe we should emit a warning?
Richard.
> 2020-11-24 Jakub Jelinek <jakub@redhat.com>
>
> PR middle-end/97943
> * gimple-fold.c (clear_padding_union, clear_padding_type): Error on and
> ignore flexible array member fields. Ignore fields with
> error_mark_node type.
>
> * c-c++-common/builtin-clear-padding-2.c: New test.
> * c-c++-common/builtin-clear-padding-3.c: New test.
> * g++.dg/ext/builtin-clear-padding-1.C: New test.
> * gcc.dg/builtin-clear-padding-2.c: New test.
>
> --- gcc/gimple-fold.c.jj 2020-11-23 17:01:48.255054823 +0100
> +++ gcc/gimple-fold.c 2020-11-24 09:53:17.724943252 +0100
> @@ -4255,6 +4255,17 @@ clear_padding_union (clear_padding_struc
> for (tree field = TYPE_FIELDS (type); field; field = DECL_CHAIN (field))
> if (TREE_CODE (field) == FIELD_DECL)
> {
> + if (DECL_SIZE_UNIT (field) == NULL_TREE)
> + {
> + if (TREE_TYPE (field) == error_mark_node)
> + continue;
> + gcc_assert (TREE_CODE (TREE_TYPE (field)) == ARRAY_TYPE
> + && !COMPLETE_TYPE_P (TREE_TYPE (field)));
> + error_at (buf->loc, "flexible array member %qD does not have "
> + "well defined padding bits for %qs",
> + field, "__builtin_clear_padding");
> + continue;
> + }
> HOST_WIDE_INT fldsz = tree_to_shwi (DECL_SIZE_UNIT (field));
> gcc_assert (union_buf->size == 0);
> union_buf->off = start_off;
> @@ -4372,11 +4383,12 @@ clear_padding_type (clear_padding_struct
> for (tree field = TYPE_FIELDS (type); field; field = DECL_CHAIN (field))
> if (TREE_CODE (field) == FIELD_DECL)
> {
> + tree ftype = TREE_TYPE (field);
> if (DECL_BIT_FIELD (field))
> {
> if (DECL_NAME (field) == NULL_TREE)
> continue;
> - HOST_WIDE_INT fldsz = TYPE_PRECISION (TREE_TYPE (field));
> + HOST_WIDE_INT fldsz = TYPE_PRECISION (ftype);
> if (fldsz == 0)
> continue;
> HOST_WIDE_INT pos = int_byte_position (field);
> @@ -4439,6 +4451,16 @@ clear_padding_type (clear_padding_struct
> }
> }
> }
> + else if (DECL_SIZE_UNIT (field) == NULL_TREE)
> + {
> + if (ftype == error_mark_node)
> + continue;
> + gcc_assert (TREE_CODE (ftype) == ARRAY_TYPE
> + && !COMPLETE_TYPE_P (ftype));
> + error_at (buf->loc, "flexible array member %qD does not have "
> + "well defined padding bits for %qs",
> + field, "__builtin_clear_padding");
> + }
> else
> {
> HOST_WIDE_INT pos = int_byte_position (field);
> --- gcc/testsuite/c-c++-common/builtin-clear-padding-2.c.jj 2020-11-24 09:54:36.331055770 +0100
> +++ gcc/testsuite/c-c++-common/builtin-clear-padding-2.c 2020-11-24 10:16:30.468221320 +0100
> @@ -0,0 +1,17 @@
> +/* PR middle-end/97943 */
> +/* { dg-do compile } */
> +/* { dg-options "" } */
> +
> +struct S { int a; char b[] __attribute__((aligned (2 * sizeof (int)))); };
> +struct T { int a; struct S b; };
> +union U { int a; struct S b; };
> +struct V { int a; union U b; };
> +
> +void
> +foo (struct S *s, struct T *t, union U *u, struct V *v)
> +{
> + __builtin_clear_padding (s); /* { dg-error "flexible array member '(S::)?b' does not have well defined padding bits for '__builtin_clear_padding'" } */
> + __builtin_clear_padding (t); /* { dg-error "flexible array member '(S::)?b' does not have well defined padding bits for '__builtin_clear_padding'" } */
> + __builtin_clear_padding (u); /* { dg-error "flexible array member '(S::)?b' does not have well defined padding bits for '__builtin_clear_padding'" } */
> + __builtin_clear_padding (v); /* { dg-error "flexible array member '(S::)?b' does not have well defined padding bits for '__builtin_clear_padding'" } */
> +}
> --- gcc/testsuite/c-c++-common/builtin-clear-padding-3.c.jj 2020-11-24 10:09:34.735913452 +0100
> +++ gcc/testsuite/c-c++-common/builtin-clear-padding-3.c 2020-11-24 10:16:41.732094196 +0100
> @@ -0,0 +1,15 @@
> +/* PR middle-end/97943 */
> +/* { dg-do compile } */
> +/* { dg-options "" } */
> +
> +union U { int a; char b[] __attribute__((aligned (2 * sizeof (int)))); }; /* { dg-error "flexible array member in union" } */
> +struct V { int a; union U b; };
> +struct W { int a; union U b; int c; };
> +
> +void
> +foo (union U *u, struct V *v, struct W *w)
> +{
> + __builtin_clear_padding (u);
> + __builtin_clear_padding (v);
> + __builtin_clear_padding (w);
> +}
> --- gcc/testsuite/g++.dg/ext/builtin-clear-padding-1.C.jj 2020-11-24 10:10:49.315071724 +0100
> +++ gcc/testsuite/g++.dg/ext/builtin-clear-padding-1.C 2020-11-24 10:17:22.461634498 +0100
> @@ -0,0 +1,15 @@
> +// PR middle-end/97943
> +// { dg-do compile }
> +// { dg-options "" }
> +
> +struct S { int a; char b[] __attribute__((aligned (2 * sizeof (int)))); }; // { dg-error "flexible array member 'S::b' not at end of 'struct \[TV]'" }
> +struct T { int a; struct S b; int c; }; // { dg-message "next member 'int T::c' declared here|in the definition of 'struct T'" }
> +union U { int a; struct S b; };
> +struct V { int a; union U b; int : 15; int c; }; // { dg-message "next member 'int V::c' declared here|in the definition of 'struct V'" }
> +
> +void
> +foo (struct T *t, struct V *v)
> +{
> + __builtin_clear_padding (t); // { dg-error "flexible array member 'S::b' does not have well defined padding bits for '__builtin_clear_padding'" }
> + __builtin_clear_padding (v); // { dg-error "flexible array member 'S::b' does not have well defined padding bits for '__builtin_clear_padding'" }
> +}
> --- gcc/testsuite/gcc.dg/builtin-clear-padding-2.c.jj 2020-11-24 10:08:47.543446090 +0100
> +++ gcc/testsuite/gcc.dg/builtin-clear-padding-2.c 2020-11-24 10:17:03.031853796 +0100
> @@ -0,0 +1,15 @@
> +/* PR middle-end/97943 */
> +/* { dg-do compile } */
> +/* { dg-options "" } */
> +
> +struct S { int a; char b[] __attribute__((aligned (2 * sizeof (int)))); };
> +struct T { int a; struct S b; int c; };
> +union U { int a; struct S b; };
> +struct V { int a; union U b; int : 15; int c; };
> +
> +void
> +foo (struct T *t, struct V *v)
> +{
> + __builtin_clear_padding (t); /* { dg-error "flexible array member 'b' does not have well defined padding bits for '__builtin_clear_padding'" } */
> + __builtin_clear_padding (v); /* { dg-error "flexible array member 'b' does not have well defined padding bits for '__builtin_clear_padding'" } */
> +}
>
> Jakub
>
>
--
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imend
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] middle-end: Reject flexible array members in __builtin_clear_padding [PR97943]
2020-11-24 10:56 ` Richard Biener
@ 2020-11-24 11:03 ` Jakub Jelinek
0 siblings, 0 replies; 3+ messages in thread
From: Jakub Jelinek @ 2020-11-24 11:03 UTC (permalink / raw)
To: Richard Biener; +Cc: Jason Merrill, Jonathan Wakely, gcc-patches
On Tue, Nov 24, 2020 at 10:56:46AM +0000, Richard Biener wrote:
> On Tue, 24 Nov 2020, Jakub Jelinek wrote:
> > As mentioned in the PR, we currently ICE on flexible array members in
> > structs and unions during __builtin_clear_padding processing.
> >
> > Jason said in the PR he'd prefer an error in these cases over forcefully
> > handling it as [0] arrays (everything is padding then) or consider the
> > arrays to have as many whole elements as would fit into the tail padding.
> >
> > So, this patch implements that.
> >
> > Ok for trunk if it passes bootstrap/regtest? So far has been tested just
> > on the dg.exp=builtin-clear-padding* dg-torture.exp=builtin-clear-padding*
> > check-gcc check-c++-all testsuite, but the builtin is currently not used
> > in anything else.
>
> So code-gen wise this will still elide the builtin but error, right?
Yes.
> OK then.
>
> Note it still leaves struct { int n; char c[1]; } to be handled
> which I think is the actual way we'll face "flexarray" members.
>
> IMHO we need to treat them as data but maybe we should emit a warning?
This is only about std::atomic used with such type, for the above, one
can safely use on common ABIs only at most 4 elements of the array, more
just wouldn't work anyway. And for the warning I'd think people would be
annoyed by that then they treat it like an array (i.e. the only thing the
standard allows, because they don't do anything wrong and it would be hard
to avoid the warning (only add pragmas somewhere, but where exactly, on all
the method calls of the atomic and constructors?), rather than poor man's
flexible array member.
Jakub
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-11-24 11:03 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-24 10:33 [PATCH] middle-end: Reject flexible array members in __builtin_clear_padding [PR97943] Jakub Jelinek
2020-11-24 10:56 ` Richard Biener
2020-11-24 11:03 ` Jakub Jelinek
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).