* [PATCH] c++: Don't try to initialize zero width bitfields in zero initialization [PR109868]
@ 2023-05-16 19:34 Jakub Jelinek
2023-05-16 19:37 ` Jason Merrill
0 siblings, 1 reply; 2+ messages in thread
From: Jakub Jelinek @ 2023-05-16 19:34 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches
Hi!
My GCC 12 change to avoid removing zero-sized bitfields as they are
important for ABI and are needed for layout compatibility traits
apparently causes zero sized bitfields to be initialized in the IL,
which at least in 13+ results in ICEs in the ranger which is upset
about zero precision types.
I think we could even avoid initializing other unnamed bitfields, but
unfortunately !CONSTRUCTOR_NO_CLEARING doesn't mean in the middle-end
clearing of padding bits and until we have some new flag that represents
the request to clear padding bits, I think it is better to keep zeroing
non-zero sized unnamed bitfields.
In addition to skipping those fields, I have changed the logic how
UNION_TYPEs are handled, the current code was a little bit weird in that
e.g. if first non-static data member had error_mark_node type, we'd happily
zero initialize the second non-static data member, etc.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk/13,
perhaps even 12?
2023-05-16 Jakub Jelinek <jakub@redhat.com>
PR c++/109868
* init.cc (build_zero_init_1): Don't initialize zero-width bitfields.
For unions only initialize the first FIELD_DECL.
* g++.dg/init/pr109868.C: New test.
--- gcc/cp/init.cc.jj 2023-05-01 23:07:05.147417750 +0200
+++ gcc/cp/init.cc 2023-05-16 10:01:14.512489727 +0200
@@ -189,15 +189,21 @@ build_zero_init_1 (tree type, tree nelts
init = build_zero_cst (type);
else if (RECORD_OR_UNION_CODE_P (TREE_CODE (type)))
{
- tree field;
+ tree field, next;
vec<constructor_elt, va_gc> *v = NULL;
/* Iterate over the fields, building initializations. */
- for (field = TYPE_FIELDS (type); field; field = DECL_CHAIN (field))
+ for (field = TYPE_FIELDS (type); field; field = next)
{
+ next = DECL_CHAIN (field);
+
if (TREE_CODE (field) != FIELD_DECL)
continue;
+ /* For unions, only the first field is initialized. */
+ if (TREE_CODE (type) == UNION_TYPE)
+ next = NULL_TREE;
+
if (TREE_TYPE (field) == error_mark_node)
continue;
@@ -212,6 +218,11 @@ build_zero_init_1 (tree type, tree nelts
continue;
}
+ /* Don't add zero width bitfields. */
+ if (DECL_C_BIT_FIELD (field)
+ && integer_zerop (DECL_SIZE (field)))
+ continue;
+
/* Note that for class types there will be FIELD_DECLs
corresponding to base classes as well. Thus, iterating
over TYPE_FIELDs will result in correct initialization of
@@ -230,10 +241,6 @@ build_zero_init_1 (tree type, tree nelts
if (value)
CONSTRUCTOR_APPEND_ELT(v, field, value);
}
-
- /* For unions, only the first field is initialized. */
- if (TREE_CODE (type) == UNION_TYPE)
- break;
}
/* Build a constructor to contain the initializations. */
--- gcc/testsuite/g++.dg/init/pr109868.C.jj 2023-05-16 09:43:54.706278293 +0200
+++ gcc/testsuite/g++.dg/init/pr109868.C 2023-05-16 09:44:16.581966894 +0200
@@ -0,0 +1,13 @@
+// PR c++/109868
+// { dg-do compile }
+// { dg-options "-O2" }
+
+struct A { virtual void foo (); };
+struct B { long b; int : 0; };
+struct C : A { B c; };
+
+void
+bar (C *p)
+{
+ *p = C ();
+}
Jakub
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] c++: Don't try to initialize zero width bitfields in zero initialization [PR109868]
2023-05-16 19:34 [PATCH] c++: Don't try to initialize zero width bitfields in zero initialization [PR109868] Jakub Jelinek
@ 2023-05-16 19:37 ` Jason Merrill
0 siblings, 0 replies; 2+ messages in thread
From: Jason Merrill @ 2023-05-16 19:37 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
On 5/16/23 15:34, Jakub Jelinek wrote:
> Hi!
>
> My GCC 12 change to avoid removing zero-sized bitfields as they are
> important for ABI and are needed for layout compatibility traits
> apparently causes zero sized bitfields to be initialized in the IL,
> which at least in 13+ results in ICEs in the ranger which is upset
> about zero precision types.
>
> I think we could even avoid initializing other unnamed bitfields, but
> unfortunately !CONSTRUCTOR_NO_CLEARING doesn't mean in the middle-end
> clearing of padding bits and until we have some new flag that represents
> the request to clear padding bits, I think it is better to keep zeroing
> non-zero sized unnamed bitfields.
>
> In addition to skipping those fields, I have changed the logic how
> UNION_TYPEs are handled, the current code was a little bit weird in that
> e.g. if first non-static data member had error_mark_node type, we'd happily
> zero initialize the second non-static data member, etc.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk/13,
> perhaps even 12?
OK back to 12, I think.
> 2023-05-16 Jakub Jelinek <jakub@redhat.com>
>
> PR c++/109868
> * init.cc (build_zero_init_1): Don't initialize zero-width bitfields.
> For unions only initialize the first FIELD_DECL.
>
> * g++.dg/init/pr109868.C: New test.
>
> --- gcc/cp/init.cc.jj 2023-05-01 23:07:05.147417750 +0200
> +++ gcc/cp/init.cc 2023-05-16 10:01:14.512489727 +0200
> @@ -189,15 +189,21 @@ build_zero_init_1 (tree type, tree nelts
> init = build_zero_cst (type);
> else if (RECORD_OR_UNION_CODE_P (TREE_CODE (type)))
> {
> - tree field;
> + tree field, next;
> vec<constructor_elt, va_gc> *v = NULL;
>
> /* Iterate over the fields, building initializations. */
> - for (field = TYPE_FIELDS (type); field; field = DECL_CHAIN (field))
> + for (field = TYPE_FIELDS (type); field; field = next)
> {
> + next = DECL_CHAIN (field);
> +
> if (TREE_CODE (field) != FIELD_DECL)
> continue;
>
> + /* For unions, only the first field is initialized. */
> + if (TREE_CODE (type) == UNION_TYPE)
> + next = NULL_TREE;
> +
> if (TREE_TYPE (field) == error_mark_node)
> continue;
>
> @@ -212,6 +218,11 @@ build_zero_init_1 (tree type, tree nelts
> continue;
> }
>
> + /* Don't add zero width bitfields. */
> + if (DECL_C_BIT_FIELD (field)
> + && integer_zerop (DECL_SIZE (field)))
> + continue;
> +
> /* Note that for class types there will be FIELD_DECLs
> corresponding to base classes as well. Thus, iterating
> over TYPE_FIELDs will result in correct initialization of
> @@ -230,10 +241,6 @@ build_zero_init_1 (tree type, tree nelts
> if (value)
> CONSTRUCTOR_APPEND_ELT(v, field, value);
> }
> -
> - /* For unions, only the first field is initialized. */
> - if (TREE_CODE (type) == UNION_TYPE)
> - break;
> }
>
> /* Build a constructor to contain the initializations. */
> --- gcc/testsuite/g++.dg/init/pr109868.C.jj 2023-05-16 09:43:54.706278293 +0200
> +++ gcc/testsuite/g++.dg/init/pr109868.C 2023-05-16 09:44:16.581966894 +0200
> @@ -0,0 +1,13 @@
> +// PR c++/109868
> +// { dg-do compile }
> +// { dg-options "-O2" }
> +
> +struct A { virtual void foo (); };
> +struct B { long b; int : 0; };
> +struct C : A { B c; };
> +
> +void
> +bar (C *p)
> +{
> + *p = C ();
> +}
>
> Jakub
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2023-05-16 19:37 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-16 19:34 [PATCH] c++: Don't try to initialize zero width bitfields in zero initialization [PR109868] Jakub Jelinek
2023-05-16 19:37 ` 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).