https://gcc.gnu.org/g:fd62fdc5e1b3c4baf5218eedbc3c6d29861f027b

commit r15-5747-gfd62fdc5e1b3c4baf5218eedbc3c6d29861f027b
Author: Jakub Jelinek <ja...@redhat.com>
Date:   Thu Nov 28 11:30:32 2024 +0100

    c++: Small initial fixes for zeroing of padding bits [PR117256]
    
    https://eel.is/c++draft/dcl.init#general-6
    says that even padding bits are supposed to be zeroed during
    zero-initialization.
    The following patch on top of the
    https://gcc.gnu.org/pipermail/gcc-patches/2024-October/665565.html
    patch attempts to implement that, though only for the easy
    cases so far, in particular marks the CONSTRUCTOR created during
    zero-initialization (or zero-initialization done during the
    value-initialization) as having padding bits cleared and for
    constexpr evaluation attempts to preserve that bit on a new CONSTRUCTOR
    created for CONSTRUCTOR_ZERO_PADDING_BITS lhs.
    
    I think we need far more than that, but am not sure where exactly
    to implement that.
    In particular, I think __builtin_bitcast should take it into account
    during constant evaluation, if the padding bits in something are guaranteed
    to be zero, then I'd think std::bitcast out of it and testing those
    bits in there should be well defined.
    But if we do that, the flag needs to be maintained precisely, not just
    conservatively, so e.g. any place where some object is copied into another
    one (except bitcast?) which would be element-wise copy, the bit should
    be cleared (or preserved from the earlier state?  I'd hope
    element-wise copying invalidates even the padding bits, but then what
    about just stores into some members, do those invalidate the padding bits
    in the rest of the object?).  But if it is an elided copy, it shouldn't.
    And am not really sure what happens e.g. with non-automatic constexpr
    variables.  If it is constructed by something that doesn't guarantee
    the zeroing of the padding bits (so similarly constructed constexpr 
automatic
    variable would have undefined state of the padding bits), are those padding
    bits well defined because it isn't automatic variable?
    
    Anyway, I hope the following patch is at least a small step in the right
    direction.
    
    2024-11-28  Jakub Jelinek  <ja...@redhat.com>
    
            PR c++/78620
            PR c++/117256
            * init.cc (build_zero_init_1): Set CONSTRUCTOR_ZERO_PADDING_BITS.
            (build_value_init_noctor): Likewise.
            * constexpr.cc (cxx_eval_store_expression): Propagate
            CONSTRUCTOR_ZERO_PADDING_BITS flag.

Diff:
---
 gcc/cp/constexpr.cc                     | 10 ++++-
 gcc/cp/init.cc                          |  5 ++-
 gcc/testsuite/g++.dg/cpp0x/zero-init1.C | 70 +++++++++++++++++++++++++++++++++
 3 files changed, 83 insertions(+), 2 deletions(-)

diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index 55e44fcbafba..bc3dc3b2559e 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -6409,6 +6409,7 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree 
t,
 
   type = TREE_TYPE (object);
   bool no_zero_init = true;
+  bool zero_padding_bits = false;
 
   auto_vec<tree *> ctors;
   releasing_vec indexes;
@@ -6421,6 +6422,7 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree 
t,
        {
          *valp = build_constructor (type, NULL);
          CONSTRUCTOR_NO_CLEARING (*valp) = no_zero_init;
+         CONSTRUCTOR_ZERO_PADDING_BITS (*valp) = zero_padding_bits;
        }
       else if (STRIP_ANY_LOCATION_WRAPPER (*valp),
               TREE_CODE (*valp) == STRING_CST)
@@ -6480,8 +6482,10 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, 
tree t,
        }
 
       /* If the value of object is already zero-initialized, any new ctors for
-        subobjects will also be zero-initialized.  */
+        subobjects will also be zero-initialized.  Similarly with zeroing of
+        padding bits.  */
       no_zero_init = CONSTRUCTOR_NO_CLEARING (*valp);
+      zero_padding_bits = CONSTRUCTOR_ZERO_PADDING_BITS (*valp);
 
       if (code == RECORD_TYPE && is_empty_field (index))
        /* Don't build a sub-CONSTRUCTOR for an empty base or field, as they
@@ -6666,6 +6670,7 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree 
t,
        {
          *valp = build_constructor (type, NULL);
          CONSTRUCTOR_NO_CLEARING (*valp) = no_zero_init;
+         CONSTRUCTOR_ZERO_PADDING_BITS (*valp) = zero_padding_bits;
        }
       new_ctx.ctor = empty_base ? NULL_TREE : *valp;
       new_ctx.object = target;
@@ -6707,6 +6712,7 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree 
t,
          /* But do make sure we have something in *valp.  */
          *valp = build_constructor (type, nullptr);
          CONSTRUCTOR_NO_CLEARING (*valp) = no_zero_init;
+         CONSTRUCTOR_ZERO_PADDING_BITS (*valp) = zero_padding_bits;
        }
     }
   else if (*valp && TREE_CODE (*valp) == CONSTRUCTOR
@@ -6719,6 +6725,8 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree 
t,
       TREE_SIDE_EFFECTS (*valp) = TREE_SIDE_EFFECTS (init);
       CONSTRUCTOR_NO_CLEARING (*valp)
        = CONSTRUCTOR_NO_CLEARING (init);
+      CONSTRUCTOR_ZERO_PADDING_BITS (*valp)
+        = CONSTRUCTOR_ZERO_PADDING_BITS (init);
     }
   else
     *valp = init;
diff --git a/gcc/cp/init.cc b/gcc/cp/init.cc
index 8526d7581a78..ae516407c927 100644
--- a/gcc/cp/init.cc
+++ b/gcc/cp/init.cc
@@ -248,6 +248,7 @@ build_zero_init_1 (tree type, tree nelts, bool 
static_storage_p,
 
       /* Build a constructor to contain the initializations.  */
       init = build_constructor (type, v);
+      CONSTRUCTOR_ZERO_PADDING_BITS (init) = 1;
     }
   else if (TREE_CODE (type) == ARRAY_TYPE)
     {
@@ -466,7 +467,9 @@ build_value_init_noctor (tree type, tsubst_flags_t complain)
            }
 
          /* Build a constructor to contain the zero- initializations.  */
-         return build_constructor (type, v);
+         tree ret = build_constructor (type, v);
+         CONSTRUCTOR_ZERO_PADDING_BITS (ret) = 1;
+         return ret;
        }
     }
   else if (TREE_CODE (type) == ARRAY_TYPE)
diff --git a/gcc/testsuite/g++.dg/cpp0x/zero-init1.C 
b/gcc/testsuite/g++.dg/cpp0x/zero-init1.C
new file mode 100644
index 000000000000..d4b75c5fd918
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/zero-init1.C
@@ -0,0 +1,70 @@
+// PR c++/117256
+// { dg-do run { target c++11 } }
+// { dg-options "-O0" }
+
+void *operator new (decltype (sizeof 0), void *p) noexcept { return p; }
+
+struct A { char c; int i; };
+#if __cplusplus >= 201402L
+struct B { A a; constexpr B (char x, int y) : a () { a.c = x; a.i = y; } };
+#else
+struct B { A a; B (char x, int y) : a () { a.c = x; a.i = y; } };
+#endif
+
+[[gnu::noipa]] void
+foo ()
+{
+  unsigned char buf[sizeof (B)];
+  A a1 = A ();
+  __builtin_memcpy (buf, &a1, sizeof (buf));
+  if (buf[1])
+    __builtin_abort ();
+  unsigned char m1 alignas (A) [sizeof (A)];
+  __builtin_memset (m1, -1, sizeof (m1));
+  A *a2 = new (m1) A ();
+  __builtin_memcpy (buf, a2, sizeof (*a2));
+  if (buf[1])
+    __builtin_abort ();
+  B b1 (42, -42);
+  __builtin_memcpy (buf, &b1, sizeof (b1));
+  if (buf[1])
+    __builtin_abort ();
+  unsigned char m2 alignas (B) [sizeof (B)];
+  B *b2 = new (m2) B (1, 2);
+  __builtin_memcpy (buf, b2, sizeof (*b2));
+  if (buf[1])
+    __builtin_abort ();
+#if __cplusplus >= 201402L
+  constexpr B b3 (3, 4);
+  __builtin_memcpy (buf, &b3, sizeof (b3));
+  if (buf[1])
+    __builtin_abort ();
+#endif
+}
+
+[[gnu::noipa]] void
+bar (unsigned char *p)
+{
+  (void) p;
+}
+
+[[gnu::noipa]] void
+baz ()
+{
+  unsigned char buf[256];
+  __builtin_memset (buf, -1, sizeof (buf));
+  bar (buf);
+}
+
+int
+main ()
+{
+  if (__builtin_offsetof (A, c) == 0
+      && __builtin_offsetof (A, i) != 1
+      && __builtin_offsetof (B, a) == 0
+      && sizeof (A) == sizeof (B))
+    {
+      baz ();
+      foo ();
+    }
+}

Reply via email to