public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc r12-3556] c++: Fix handling of decls with flexible array members initialized with side-effects [PR88578]
@ 2021-09-15 20:22 Jakub Jelinek
  0 siblings, 0 replies; only message in thread
From: Jakub Jelinek @ 2021-09-15 20:22 UTC (permalink / raw)
  To: gcc-cvs

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

commit r12-3556-ge5d1af8a07ae9fcc40ea5c781c3ad46d20ea12a6
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Wed Sep 15 22:21:17 2021 +0200

    c++: Fix handling of decls with flexible array members initialized with side-effects [PR88578]
    
    > > Note, if the flexible array member is initialized only with non-constant
    > > initializers, we have a worse bug that this patch doesn't solve, the
    > > splitting of initializers into constant and dynamic initialization removes
    > > the initializer and we don't have just wrong DECL_*SIZE, but nothing is
    > > emitted when emitting those vars into assembly either and so the dynamic
    > > initialization clobbers other vars that may overlap the variable.
    > > I think we need keep an empty CONSTRUCTOR elt in DECL_INITIAL for the
    > > flexible array member in that case.
    >
    > Makes sense.
    
    So, the following patch fixes that.
    
    The typeck2.c change makes sure we keep those CONSTRUCTORs around (although
    they should be empty because all their elts had side-effects/was
    non-constant if it was removed earlier), and the varasm.c change is to avoid
    ICEs on those as well as ICEs on other flex array members that had some
    initializers without side-effects, but not on the last array element.
    
    The code was already asserting that the (index of the last elt in the
    CONSTRUCTOR + 1) times elt size is equal to TYPE_SIZE_UNIT of the local->val
    type, which is true for C flex arrays or for C++ if they don't have any
    side-effects or the last elt doesn't have side-effects, this patch changes
    that to assertion that the TYPE_SIZE_UNIT is greater than equal to the
    offset of the end of last element in the CONSTRUCTOR and uses TYPE_SIZE_UNIT
    (int_size_in_bytes) in the code later on.
    
    2021-09-15  Jakub Jelinek  <jakub@redhat.com>
    
            PR c++/88578
            PR c++/102295
    gcc/
            * varasm.c (output_constructor_regular_field): Instead of assertion
            that array_size_for_constructor result is equal to size of
            TREE_TYPE (local->val) in bytes, assert that the type size is greater
            or equal to array_size_for_constructor result and use type size as
            fieldsize.
    gcc/cp/
            * typeck2.c (split_nonconstant_init_1): Don't throw away empty
            initializers of flexible array members if they have non-zero type
            size.
    gcc/testsuite/
            * g++.dg/ext/flexary39.C: New test.
            * g++.dg/ext/flexary40.C: New test.

Diff:
---
 gcc/cp/typeck2.c                     | 15 ++++++++-
 gcc/testsuite/g++.dg/ext/flexary39.C | 65 ++++++++++++++++++++++++++++++++++++
 gcc/testsuite/g++.dg/ext/flexary40.C | 50 +++++++++++++++++++++++++++
 gcc/varasm.c                         | 12 +++++--
 4 files changed, 138 insertions(+), 4 deletions(-)

diff --git a/gcc/cp/typeck2.c b/gcc/cp/typeck2.c
index f78dbf238af..abfd7dabd60 100644
--- a/gcc/cp/typeck2.c
+++ b/gcc/cp/typeck2.c
@@ -524,7 +524,20 @@ split_nonconstant_init_1 (tree dest, tree init, bool nested)
 		sub = build3 (COMPONENT_REF, inner_type, dest, field_index,
 			      NULL_TREE);
 
-	      if (!split_nonconstant_init_1 (sub, value, true))
+	      if (!split_nonconstant_init_1 (sub, value, true)
+		      /* For flexible array member with initializer we
+			 can't remove the initializer, because only the
+			 initializer determines how many elements the
+			 flexible array member has.  */
+		  || (!array_type_p
+		      && TREE_CODE (inner_type) == ARRAY_TYPE
+		      && TYPE_DOMAIN (inner_type) == NULL
+		      && TREE_CODE (TREE_TYPE (value)) == ARRAY_TYPE
+		      && COMPLETE_TYPE_P (TREE_TYPE (value))
+		      && !integer_zerop (TYPE_SIZE (TREE_TYPE (value)))
+		      && idx == CONSTRUCTOR_NELTS (init) - 1
+		      && TYPE_HAS_TRIVIAL_DESTRUCTOR
+				(strip_array_types (inner_type))))
 		complete_p = false;
 	      else
 		{
diff --git a/gcc/testsuite/g++.dg/ext/flexary39.C b/gcc/testsuite/g++.dg/ext/flexary39.C
new file mode 100644
index 00000000000..8eb81f26cf5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/flexary39.C
@@ -0,0 +1,65 @@
+// PR c++/88578
+// { dg-do run }
+// { dg-options -Wno-pedantic }
+
+#define STR(s) #s
+#define ASSERT(exp) \
+  ((exp) ? (void)0 : (void)(__builtin_printf ("%s:%i: assertion %s failed\n", \
+                     __FILE__, __LINE__, STR(exp)), \
+                      __builtin_abort ()))
+
+typedef int int32_t __attribute__((mode (__SI__)));
+
+struct Ax { int32_t n, a[]; };
+struct AAx { int32_t i; Ax ax; };
+
+int32_t i = 12345678;
+
+void
+test ()
+{
+  {
+    // OK.  Does not assign any elements to flexible array.
+    Ax s = { 0 };
+    ASSERT (s.n == 0);
+  }
+  {
+    // OK only for statically allocated objects, otherwise error.
+    static Ax s = { 0, { } };
+    ASSERT (s.n == 0);
+  }
+  {
+    static Ax s = { 1, { 2 } };
+    ASSERT (s.n == 1 && s.a [0] == 2);
+  }
+  {
+    static Ax s = { 2, { 3, 4 } };
+    ASSERT (s.n = 2 && s.a [0] == 3 && s.a [1] == 4);
+  }
+  {
+    static Ax s = { 123, i };
+    ASSERT (s.n == 123 && s.a [0] == i);
+  }
+  {
+    static Ax s = { 456, { i } };
+    ASSERT (s.n == 456 && s.a [0] == i);
+  }
+  {
+    int32_t j = i + 1, k = j + 1;
+    static Ax s = { 3, { i, j, k } };
+    ASSERT (s.n == 3 && s.a [0] == i && s.a [1] == j && s.a [2] == k);
+  }
+
+  {
+    // OK.  Does not assign any elements to flexible array.
+    AAx s = { 1, { 2 } };
+    ASSERT (s.i == 1 && s.ax.n == 2);
+  }
+}
+
+int
+main ()
+{
+  test ();
+  test ();
+}
diff --git a/gcc/testsuite/g++.dg/ext/flexary40.C b/gcc/testsuite/g++.dg/ext/flexary40.C
new file mode 100644
index 00000000000..ee824c29d3a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/flexary40.C
@@ -0,0 +1,50 @@
+// PR c++/102295
+// { dg-do run }
+// { dg-options "" }
+
+struct A { int a; int b[]; };
+struct B { B (); int k; };
+struct C { int l; B m[]; };
+
+int x[4];
+A c = { 42, { ++x[0], ++x[1], ++x[2], ++x[3] } };
+A d = { 43, { 0, ++x[0], ++x[1], ++x[2], ++x[3] } };
+A e = { 44, { ++x[0], ++x[1], ++x[2], 17 } };
+A f = { 45 };
+C n = { 50, { B (), B () } };
+C o = { 51, {} };
+
+int
+main ()
+{
+  static A g = { 46, { ++x[0], ++x[1], ++x[2], ++x[3] } };
+  static A h = { 47, { 0, ++x[0], ++x[1], ++x[2], ++x[3] } };
+  static A i = { 48, { ++x[0], ++x[1], ++x[2], 18 } };
+  static A j = { 49 };
+  if (c.a != 42 || c.b[0] != 1 || c.b[1] != 1 || c.b[2] != 1 || c.b[3] != 1)
+    __builtin_abort ();
+  if (d.a != 43 || d.b[0] != 0 || d.b[1] != 2 || d.b[2] != 2 || d.b[3] != 2 || d.b[4] != 2)
+    __builtin_abort ();
+  if (e.a != 44 || e.b[0] != 3 || e.b[1] != 3 || e.b[2] != 3 || e.b[3] != 17)
+    __builtin_abort ();
+  if (f.a != 45)
+    __builtin_abort ();
+  if (g.a != 46 || g.b[0] != 4 || g.b[1] != 4 || g.b[2] != 4 || g.b[3] != 3)
+    __builtin_abort ();
+  if (h.a != 47 || h.b[0] != 0 || h.b[1] != 5 || h.b[2] != 5 || h.b[3] != 5 || h.b[4] != 4)
+    __builtin_abort ();
+  if (i.a != 48 || i.b[0] != 6 || i.b[1] != 6 || i.b[2] != 6 || i.b[3] != 18)
+    __builtin_abort ();
+  if (j.a != 49)
+    __builtin_abort ();
+  if (n.l != 50 || n.m[0].k != 42 || n.m[1].k != 42)
+    __builtin_abort ();
+  if (o.l != 51)
+    __builtin_abort ();
+  if (x[0] != 6 || x[1] != 6 || x[2] != 6 || x[3] != 4)
+    __builtin_abort ();
+}
+
+B::B () : k (42)
+{
+}
diff --git a/gcc/varasm.c b/gcc/varasm.c
index 53cf6dea3f3..2d261b353bf 100644
--- a/gcc/varasm.c
+++ b/gcc/varasm.c
@@ -5531,14 +5531,20 @@ output_constructor_regular_field (oc_local_state *local)
 	  && (!TYPE_DOMAIN (TREE_TYPE (local->field))
 	      || !TYPE_MAX_VALUE (TYPE_DOMAIN (TREE_TYPE (local->field)))))
 	{
-	  fieldsize = array_size_for_constructor (local->val);
+	  unsigned HOST_WIDE_INT fldsize
+	    = array_size_for_constructor (local->val);
+	  fieldsize = int_size_in_bytes (TREE_TYPE (local->val));
+	  /* In most cases fieldsize == fldsize as the size of the initializer
+	     determines how many elements the flexible array member has.  For
+	     C++ fldsize can be smaller though, if the last or several last or
+	     all initializers of the flexible array member have side-effects
+	     and the FE splits them into dynamic initialization.  */
+	  gcc_checking_assert (fieldsize >= fldsize);
 	  /* Given a non-empty initialization, this field had better
 	     be last.  Given a flexible array member, the next field
 	     on the chain is a TYPE_DECL of the enclosing struct.  */
 	  const_tree next = DECL_CHAIN (local->field);
 	  gcc_assert (!fieldsize || !next || TREE_CODE (next) != FIELD_DECL);
-	  tree size = TYPE_SIZE_UNIT (TREE_TYPE (local->val));
-	  gcc_checking_assert (compare_tree_int (size, fieldsize) == 0);
 	}
       else
 	fieldsize = tree_to_uhwi (DECL_SIZE_UNIT (local->field));


^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2021-09-15 20:22 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-15 20:22 [gcc r12-3556] c++: Fix handling of decls with flexible array members initialized with side-effects [PR88578] 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).