On 12/12/24 4:41 AM, Jakub Jelinek wrote:
Hi!

I believe we need to clear padding bits even in empty types when using
zero initialization,
https://eel.is/c++draft/dcl.init.general#6.2
doesn't have an exception for empty types.
I came to this when playing with an optimization for PR116416 to improve
tree-ssa/pr78687.C testcase back.

Initially I had in the patch also
--- gcc/cp/cp-gimplify.cc.jj    2024-12-11 12:46:32.958466985 +0100
+++ gcc/cp/cp-gimplify.cc       2024-12-11 16:23:11.598860505 +0100
@@ -674,7 +674,10 @@ cp_gimplify_expr (tree *expr_p, gimple_s
           TREE_OPERAND (*expr_p, 1) = build1 (VIEW_CONVERT_EXPR,
                                               TREE_TYPE (op0), op1);

-       else if (simple_empty_class_p (TREE_TYPE (op0), op1, code))
+       else if (simple_empty_class_p (TREE_TYPE (op0), op1, code)
+                && (TREE_CODE (*expr_p) != INIT_EXPR
+                    || TREE_CODE (op1) != CONSTRUCTOR
+                    || !CONSTRUCTOR_ZERO_PADDING_BITS (op1)))
           {
             while (TREE_CODE (op1) == TARGET_EXPR)
               /* We're disconnecting the initializer from its target,
hunk but that regressed the g++.dg/init/empty1.C testcase, where
the empty bases are overlaid with other data members and the test
wants to ensure that the non-static data members aren't overwritten
when initializing the base padding.
On the other side, with this patch and the cp-gimplify.cc hunk plus
the optimization I'm going to post we change
-      D.10177 = {};
+      D.10177._storage.D.9582.D.9163._tail.D.9221._tail.D.9280._head = {};
in the gimple dump (option_2 is zero initialized there), while with
just this patch and the optimization and no cp-gimplify.cc hunk
       D.10177 = {};
is simply removed altogether and no clearing done.
So, I'm not 100% sure if what the patch below does is 100% safe not to
overwrite the overlaid stuff, but at least testsuite doesn't reveal
anything further, and on the other side clears padding in everything it
should.

Earlier version of this patch (with the cp-gimplify.cc hunk and
without the TYPE_SIZE/integer_zerop subconditions) has been bootstrapped
and regtested on x86_64-linux and i686-linux, this version just tested
on the set of tests which regressed.

2024-12-12  Jakub Jelinek  <ja...@redhat.com>

        PR c++/118002
gcc/
        * gimplify.cc (gimplify_init_constructor, gimplify_modify_expr):
        Don't optimize away INIT_EXPRs of empty classes with rhs CONSTRUCTOR
        with CONSTRUCTOR_ZERO_PADDING_BITS.
gcc/testsuite/
        * g++.dg/cpp0x/zero-init2.C: New test.

--- gcc/gimplify.cc.jj  2024-12-07 11:35:49.475439705 +0100
+++ gcc/gimplify.cc     2024-12-12 09:38:03.865543272 +0100
@@ -6094,8 +6094,15 @@ gimplify_init_constructor (tree *expr_p,
       not emitted an assignment, do so now.   */
    if (*expr_p
        /* If the type is an empty type, we don't need to emit the
-        assignment. */
-      && !is_empty_type (TREE_TYPE (TREE_OPERAND (*expr_p, 0))))
+        assignment.  Except when rhs is a CONSTRUCTOR with
+        CONSTRUCTOR_ZERO_PADDING_BITS.  */
+      && (!is_empty_type (TREE_TYPE (TREE_OPERAND (*expr_p, 0)))
+         || (is_init_expr
+             && TREE_CODE (TREE_OPERAND (*expr_p, 1)) == CONSTRUCTOR
+             && CONSTRUCTOR_ZERO_PADDING_BITS (TREE_OPERAND (*expr_p, 1))
+             && TYPE_SIZE (TREE_TYPE (TREE_OPERAND (*expr_p, 0)))
+             && !integer_zerop (TYPE_SIZE (TREE_TYPE (TREE_OPERAND (*expr_p,
+                                                                    0)))))))
      {
        tree lhs = TREE_OPERAND (*expr_p, 0);
        tree rhs = TREE_OPERAND (*expr_p, 1);
@@ -6685,7 +6692,14 @@ gimplify_modify_expr (tree *expr_p, gimp
        /* Don't do this for calls that return addressable types, expand_call
         relies on those having a lhs.  */
        && !(TREE_ADDRESSABLE (TREE_TYPE (*from_p))
-          && TREE_CODE (*from_p) == CALL_EXPR))
+          && TREE_CODE (*from_p) == CALL_EXPR)
+      /* And similarly don't do that for rhs being CONSTRUCTOR with
+        CONSTRUCTOR_ZERO_PADDING_BITS set.  */
+      && !(TREE_CODE (*expr_p) == INIT_EXPR
+          && TREE_CODE (*to_p) == CONSTRUCTOR
+          && CONSTRUCTOR_ZERO_PADDING_BITS (*to_p)

Shouldn't these two *to_p be *from_p? Is this hunk actually doing anything as is?

+          && TYPE_SIZE (TREE_TYPE (*from_p))
+          && !integer_zerop (TYPE_SIZE (TREE_TYPE (*from_p)))))
      {
        gimplify_stmt (from_p, pre_p);
        gimplify_stmt (to_p, pre_p);
--- gcc/testsuite/g++.dg/cpp0x/zero-init2.C.jj  2024-12-11 16:50:26.513845473 
+0100
+++ gcc/testsuite/g++.dg/cpp0x/zero-init2.C     2024-12-11 16:50:45.879572789 
+0100
@@ -0,0 +1,37 @@
+// PR c++/118002
+// { dg-do run { target c++11 } }
+// { dg-options "-O0" }
+
+struct S {};
+struct T { S a, b, c, d, e, f, g, h; };
+struct U { T i, j, k, l, m, n, o, p; };
+
+[[gnu::noipa]] void
+foo (struct U *)
+{
+}
+
+[[gnu::noipa]] void
+bar ()
+{
+  U u[4];
+  __builtin_memset (&u, -1, sizeof (U) * 4);
+  foo (&u[0]);
+}
+
+[[gnu::noipa]] void
+baz ()
+{
+  U u = U ();
+  foo (&u);
+  for (int i = 0; i < sizeof (U); ++i)
+    if (((char *) &u)[i] != 0)
+      __builtin_abort ();
+}
+
+int
+main ()
+{
+  bar ();
+  baz ();
+}

        Jakub


Reply via email to