Re: [PATCH] c++: Fix zero initialization of flexible array members [PR99033]

2021-02-11 Thread Jason Merrill via Gcc-patches

On 2/11/21 3:50 AM, Jakub Jelinek wrote:

Hi!

array_type_nelts returns error_mark_node for type of flexible array members
and build_zero_init_1 was placing an error_mark_node into the CONSTRUCTOR,
on which e.g. varasm ICEs.  I think there is nothing erroneous on zero
initialization of flexible array members though, such arrays should simply
get no elements, like they do if such classes are constructed (everything
except when some larger initializer comes from an explicit initializer).

So, this patch handles [] arrays in zero initialization like [0] arrays
and fixes handling of the [0] arrays - the
tree_int_cst_equal (max_index, integer_minus_one_node) check
didn't do what it thought it would do, max_index is typically unsigned
integer (sizetype) and so it is never equal to a -1.


I wonder about using build_minus_one_cst and integer_minus_onep instead. 
OK either way.



What the patch doesn't do and maybe would be desirable is if it returns
error_mark_node for other reasons let the recursive callers not stick that
into CONSTRUCTOR but return error_mark_node instead.  But I don't have a
testcase where that would be needed right now.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2021-02-11  Jakub Jelinek  

PR c++/99033
* init.c (build_zero_init_1): Handle zero initialiation of
flexible array members like initialization of [0] arrays.
Use integer_all_onesp instead of comparison to integer_minus_one_node
and integer_zerop instead of comparison against size_zero_node.
Formatting fixes.

* g++.dg/ext/flexary38.C: New test.

--- gcc/cp/init.c.jj2021-01-27 10:05:35.273942499 +0100
+++ gcc/cp/init.c   2021-02-10 15:03:02.668042115 +0100
@@ -247,9 +247,12 @@ build_zero_init_1 (tree type, tree nelts
  
/* Iterate over the array elements, building initializations.  */

if (nelts)
-   max_index = fold_build2_loc (input_location,
-MINUS_EXPR, TREE_TYPE (nelts),
-nelts, integer_one_node);
+   max_index = fold_build2_loc (input_location, MINUS_EXPR,
+TREE_TYPE (nelts), nelts,
+build_one_cst (TREE_TYPE (nelts)));
+  /* Treat flexible array members like [0] arrays.  */
+  else if (TYPE_DOMAIN (type) == NULL_TREE)
+   max_index = build_all_ones_cst (sizetype);
else
max_index = array_type_nelts (type);
  
@@ -261,20 +264,19 @@ build_zero_init_1 (tree type, tree nelts
  
/* A zero-sized array, which is accepted as an extension, will

 have an upper bound of -1.  */
-  if (!tree_int_cst_equal (max_index, integer_minus_one_node))
+  if (!integer_all_onesp (max_index))
{
  constructor_elt ce;
  
  	  /* If this is a one element array, we just use a regular init.  */

- if (tree_int_cst_equal (size_zero_node, max_index))
+ if (integer_zerop (max_index))
ce.index = size_zero_node;
  else
ce.index = build2 (RANGE_EXPR, sizetype, size_zero_node,
-   max_index);
+  max_index);
  
-	  ce.value = build_zero_init_1 (TREE_TYPE (type),

-/*nelts=*/NULL_TREE,
-static_storage_p, NULL_TREE);
+ ce.value = build_zero_init_1 (TREE_TYPE (type), /*nelts=*/NULL_TREE,
+   static_storage_p, NULL_TREE);
  if (ce.value)
{
  vec_alloc (v, 1);
--- gcc/testsuite/g++.dg/ext/flexary38.C.jj 2021-02-10 15:16:40.700102841 
+0100
+++ gcc/testsuite/g++.dg/ext/flexary38.C2021-02-10 15:16:22.243304533 
+0100
@@ -0,0 +1,18 @@
+// PR c++/99033
+// { dg-do compile }
+// { dg-options "" }
+
+struct T { int t; };
+struct S { char c; int T::*b[]; } a;
+struct U { char c; int T::*b[0]; } b;
+struct V { char c; int T::*b[1]; } c;
+struct W { char c; int T::*b[2]; } d;
+
+void
+foo ()
+{
+  a.c = 1;
+  b.c = 2;
+  c.c = 3;
+  d.c = 4;
+}

Jakub





[PATCH] c++: Fix zero initialization of flexible array members [PR99033]

2021-02-11 Thread Jakub Jelinek via Gcc-patches
Hi!

array_type_nelts returns error_mark_node for type of flexible array members
and build_zero_init_1 was placing an error_mark_node into the CONSTRUCTOR,
on which e.g. varasm ICEs.  I think there is nothing erroneous on zero
initialization of flexible array members though, such arrays should simply
get no elements, like they do if such classes are constructed (everything
except when some larger initializer comes from an explicit initializer).

So, this patch handles [] arrays in zero initialization like [0] arrays
and fixes handling of the [0] arrays - the
tree_int_cst_equal (max_index, integer_minus_one_node) check
didn't do what it thought it would do, max_index is typically unsigned
integer (sizetype) and so it is never equal to a -1.

What the patch doesn't do and maybe would be desirable is if it returns
error_mark_node for other reasons let the recursive callers not stick that
into CONSTRUCTOR but return error_mark_node instead.  But I don't have a
testcase where that would be needed right now.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2021-02-11  Jakub Jelinek  

PR c++/99033
* init.c (build_zero_init_1): Handle zero initialiation of
flexible array members like initialization of [0] arrays.
Use integer_all_onesp instead of comparison to integer_minus_one_node
and integer_zerop instead of comparison against size_zero_node.
Formatting fixes.

* g++.dg/ext/flexary38.C: New test.

--- gcc/cp/init.c.jj2021-01-27 10:05:35.273942499 +0100
+++ gcc/cp/init.c   2021-02-10 15:03:02.668042115 +0100
@@ -247,9 +247,12 @@ build_zero_init_1 (tree type, tree nelts
 
   /* Iterate over the array elements, building initializations.  */
   if (nelts)
-   max_index = fold_build2_loc (input_location,
-MINUS_EXPR, TREE_TYPE (nelts),
-nelts, integer_one_node);
+   max_index = fold_build2_loc (input_location, MINUS_EXPR,
+TREE_TYPE (nelts), nelts,
+build_one_cst (TREE_TYPE (nelts)));
+  /* Treat flexible array members like [0] arrays.  */
+  else if (TYPE_DOMAIN (type) == NULL_TREE)
+   max_index = build_all_ones_cst (sizetype);
   else
max_index = array_type_nelts (type);
 
@@ -261,20 +264,19 @@ build_zero_init_1 (tree type, tree nelts
 
   /* A zero-sized array, which is accepted as an extension, will
 have an upper bound of -1.  */
-  if (!tree_int_cst_equal (max_index, integer_minus_one_node))
+  if (!integer_all_onesp (max_index))
{
  constructor_elt ce;
 
  /* If this is a one element array, we just use a regular init.  */
- if (tree_int_cst_equal (size_zero_node, max_index))
+ if (integer_zerop (max_index))
ce.index = size_zero_node;
  else
ce.index = build2 (RANGE_EXPR, sizetype, size_zero_node,
-   max_index);
+  max_index);
 
- ce.value = build_zero_init_1 (TREE_TYPE (type),
-/*nelts=*/NULL_TREE,
-static_storage_p, NULL_TREE);
+ ce.value = build_zero_init_1 (TREE_TYPE (type), /*nelts=*/NULL_TREE,
+   static_storage_p, NULL_TREE);
  if (ce.value)
{
  vec_alloc (v, 1);
--- gcc/testsuite/g++.dg/ext/flexary38.C.jj 2021-02-10 15:16:40.700102841 
+0100
+++ gcc/testsuite/g++.dg/ext/flexary38.C2021-02-10 15:16:22.243304533 
+0100
@@ -0,0 +1,18 @@
+// PR c++/99033
+// { dg-do compile }
+// { dg-options "" }
+
+struct T { int t; };
+struct S { char c; int T::*b[]; } a;
+struct U { char c; int T::*b[0]; } b;
+struct V { char c; int T::*b[1]; } c;
+struct W { char c; int T::*b[2]; } d;
+
+void
+foo ()
+{
+  a.c = 1;
+  b.c = 2;
+  c.c = 3;
+  d.c = 4;
+}

Jakub