[v3,4/4] Update the C FE routine "add_flexible_array_elts_to_size" C++ FE routine "layout_var_decl" to handle the cases when the DECL is union.

  Add testing cases to test the _bos for flexible array members in unions
or alone in structures.

gcc/c/ChangeLog:

	* c-decl.cc (add_flexible_array_elts_to_size): Handle the cases
	when the DECL is union.

gcc/cp/ChangeLog:

	* decl.cc (layout_var_decl): Handle the cases when the DECL is
	union with a flexible array member initializer.

gcc/testsuite/ChangeLog:

	* c-c++-common/fam-in-union-alone-in-struct-bos-1.c: New test.
	* c-c++-common/fam-in-union-alone-in-struct-bos.c: New test.
---
 gcc/c/c-decl.cc                               | 32 +++++++--
 gcc/cp/decl.cc                                | 33 ++++++++--
 .../fam-in-union-alone-in-struct-bos-1.c      | 66 +++++++++++++++++++
 .../fam-in-union-alone-in-struct-bos.c        | 45 +++++++++++++
 4 files changed, 162 insertions(+), 14 deletions(-)
 create mode 100644 gcc/testsuite/c-c++-common/fam-in-union-alone-in-struct-bos-1.c
 create mode 100644 gcc/testsuite/c-c++-common/fam-in-union-alone-in-struct-bos.c
  

On 4/30/24 07:58, Qing Zhao wrote:
> Add testing cases to test the _bos for flexible array members in unions
> or alone in structures.
> 
> gcc/c/ChangeLog:
> 
> 	* c-decl.cc (add_flexible_array_elts_to_size): Handle the cases
> 	when the DECL is union.
> 
> gcc/cp/ChangeLog:
> 
> 	* decl.cc (layout_var_decl): Handle the cases when the DECL is
> 	union with a flexible array member initializer.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* c-c++-common/fam-in-union-alone-in-struct-bos-1.c: New test.
> 	* c-c++-common/fam-in-union-alone-in-struct-bos.c: New test.
> ---
>   gcc/c/c-decl.cc                               | 32 +++++++--
>   gcc/cp/decl.cc                                | 33 ++++++++--
>   .../fam-in-union-alone-in-struct-bos-1.c      | 66 +++++++++++++++++++
>   .../fam-in-union-alone-in-struct-bos.c        | 45 +++++++++++++
>   4 files changed, 162 insertions(+), 14 deletions(-)
>   create mode 100644 gcc/testsuite/c-c++-common/fam-in-union-alone-in-struct-bos-1.c
>   create mode 100644 gcc/testsuite/c-c++-common/fam-in-union-alone-in-struct-bos.c
> 
> diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
> index 947f3cd589eb..59302c5cfb1f 100644
> --- a/gcc/c/c-decl.cc
> +++ b/gcc/c/c-decl.cc
> @@ -5337,9 +5337,9 @@ zero_length_array_type_p (const_tree type)
>   }
>   
>   /* INIT is a constructor that forms DECL's initializer.  If the final
> -   element initializes a flexible array field, add the size of that
> -   initializer to DECL's size.  */
> -
> +   element initializes a flexible array field, adjust the size of the
> +   DECL with the initializer based on whether the DECL is a union or
> +   a structure.  */
>   static void
>   add_flexible_array_elts_to_size (tree decl, tree init)
>   {
> @@ -5348,15 +5348,33 @@ add_flexible_array_elts_to_size (tree decl, tree init)
>     if (vec_safe_is_empty (CONSTRUCTOR_ELTS (init)))
>       return;
>   
> +  bool is_decl_type_union = (TREE_CODE (TREE_TYPE (decl)) == UNION_TYPE);

In both C and C++ front-ends, I don't see any reason for this variable 
to be declared outside the if, since it's only used inside it.

It's not clear to me that it's needed at all since it's only used once, 
but if you prefer to have it, please move it inside the if.

> +
>     elt = CONSTRUCTOR_ELTS (init)->last ().value;
>     type = TREE_TYPE (elt);
>     if (flexible_array_member_type_p (type))
>       {
>         complete_array_type (&type, elt, false);
> -      DECL_SIZE (decl)
> -	= size_binop (PLUS_EXPR, DECL_SIZE (decl), TYPE_SIZE (type));
> -      DECL_SIZE_UNIT (decl)
> -	= size_binop (PLUS_EXPR, DECL_SIZE_UNIT (decl), TYPE_SIZE_UNIT (type));
> +      /* For a structure, add the size of the initializer to the DECL's
> +	 size.  */
> +      if (!is_decl_type_union)
> +	{
> +	  DECL_SIZE (decl)
> +	    = size_binop (PLUS_EXPR, DECL_SIZE (decl), TYPE_SIZE (type));
> +	  DECL_SIZE_UNIT (decl)
> +	    = size_binop (PLUS_EXPR, DECL_SIZE_UNIT (decl),
> +			  TYPE_SIZE_UNIT (type));
> +	}
> +      /* For a union, the DECL's size is the maximum of the current size
> +	 and the size of the initializer.  */
> +      else
> +	{
> +	  DECL_SIZE (decl)
> +	    = size_binop (MAX_EXPR, DECL_SIZE (decl), TYPE_SIZE (type));
> +	  DECL_SIZE_UNIT (decl)
> +	    = size_binop (MAX_EXPR, DECL_SIZE_UNIT (decl),
> +			  TYPE_SIZE_UNIT (type));
> +	}
>       }
>   }
>   
> diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
> index 9a91c6f80da1..0ea3ef165b66 100644
> --- a/gcc/cp/decl.cc
> +++ b/gcc/cp/decl.cc
> @@ -6555,8 +6555,9 @@ layout_var_decl (tree decl)
>   	}
>       }
>   
> -  /* If the final element initializes a flexible array field, add the size of
> -     that initializer to DECL's size.  */
> +  /* If the final element initializes a flexible array field, adjust
> +     the size of the DECL with the initializer based on whether the
> +     DECL is a union or a structure.  */
>     if (type != error_mark_node
>         && DECL_INITIAL (decl)
>         && TREE_CODE (DECL_INITIAL (decl)) == CONSTRUCTOR
> @@ -6568,6 +6569,7 @@ layout_var_decl (tree decl)
>         && tree_int_cst_equal (DECL_SIZE (decl), TYPE_SIZE (type)))
>       {
>         constructor_elt &elt = CONSTRUCTOR_ELTS (DECL_INITIAL (decl))->last ();
> +      bool is_decl_type_union = (TREE_CODE (TREE_TYPE (decl)) == UNION_TYPE);
>         if (elt.index)
>   	{
>   	  tree itype = TREE_TYPE (elt.index);
> @@ -6577,11 +6579,28 @@ layout_var_decl (tree decl)
>   	      && TREE_CODE (vtype) == ARRAY_TYPE
>   	      && COMPLETE_TYPE_P (vtype))
>   	    {
> -	      DECL_SIZE (decl)
> -		= size_binop (PLUS_EXPR, DECL_SIZE (decl), TYPE_SIZE (vtype));
> -	      DECL_SIZE_UNIT (decl)
> -		= size_binop (PLUS_EXPR, DECL_SIZE_UNIT (decl),
> -			      TYPE_SIZE_UNIT (vtype));
> +	      /* For a structure, add the size of the initializer to the DECL's
> +		 size.  */
> +	      if (!is_decl_type_union)
> +		{
> +		  DECL_SIZE (decl)
> +		    = size_binop (PLUS_EXPR, DECL_SIZE (decl),
> +				  TYPE_SIZE (vtype));
> +		  DECL_SIZE_UNIT (decl)
> +		    = size_binop (PLUS_EXPR, DECL_SIZE_UNIT (decl),
> +				  TYPE_SIZE_UNIT (vtype));
> +		}
> +	      /* a union, the DECL's size is the maximum of the current size
> +		 and the size of the initializer.  */
> +	      else
> +		{
> +		  DECL_SIZE (decl)
> +		    = size_binop (MAX_EXPR, DECL_SIZE (decl),
> +				  TYPE_SIZE (vtype));
> +		  DECL_SIZE_UNIT (decl)
> +		    = size_binop (MAX_EXPR, DECL_SIZE_UNIT (decl),
> +				  TYPE_SIZE_UNIT (vtype));
> +		}
>   	    }
>   	}
>       }
> diff --git a/gcc/testsuite/c-c++-common/fam-in-union-alone-in-struct-bos-1.c b/gcc/testsuite/c-c++-common/fam-in-union-alone-in-struct-bos-1.c
> new file mode 100644
> index 000000000000..aae9cf39c8c7
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/fam-in-union-alone-in-struct-bos-1.c
> @@ -0,0 +1,66 @@
> +/* testing flexible array members in unions and alone in structures:
> +   __bos/__bdos  */
> +/* { dg-do run } */
> +/* { dg-options "-O2" } */
> +
> +union with_fam_1 {
> +  char a;
> +  int b[];
> +} *with_fam_1_v;
> +
> +union with_fam_2 {
> +  int a;
> +  char b[];
> +} *with_fam_2_v;
> +
> +union with_fam_3 {
> +  char a[];
> +  int b[];
> +} *with_fam_3_v;
> +
> +struct only_fam {
> +  int b[];
> +} *only_fam_v;
> +
> +struct only_fam_2 {
> +  unsigned int : 2;
> +  unsigned int : 3;
> +  int b[];
> +} *only_fam_2_v;
> +
> +void __attribute__((__noinline__))
> +setup (int n1, int n2, int n3, int n4, int n5)
> +{
> +  with_fam_1_v = (union with_fam_1 *) __builtin_malloc (n1 * sizeof (int));
> +  with_fam_2_v = (union with_fam_2 *) __builtin_malloc (n2 * sizeof (char));
> +  with_fam_3_v = (union with_fam_3 *) __builtin_malloc (n3 * sizeof (int));
> +  only_fam_v = (struct only_fam *) __builtin_malloc (n4 * sizeof (int));
> +  only_fam_2_v = (struct only_fam_2 *) __builtin_malloc (n5 * sizeof (int));
> +  return;
> +}
> +
> +void __attribute__((__noinline__)) stuff(
> +    union with_fam_1 *with_fam_1_v,
> +    union with_fam_2 *with_fam_2_v,
> +    union with_fam_3 *with_fam_3_v,
> +    struct only_fam *only_fam_v,
> +    struct only_fam_2 *only_fam_2_v)
> +{
> +  if (__builtin_object_size(with_fam_1_v->b, 1) != -1)
> +    __builtin_abort ();
> +  if (__builtin_object_size(with_fam_2_v->b, 1) != -1)
> +    __builtin_abort ();
> +  if (__builtin_object_size(with_fam_3_v->b, 1) != -1)
> +    __builtin_abort ();
> +  if (__builtin_object_size(only_fam_v->b, 1) != -1)
> +    __builtin_abort ();
> +  if (__builtin_object_size(only_fam_2_v->b, 1) != -1)
> +    __builtin_abort ();
> +}
> +
> +int main (int argc, char *argv[])
> +{
> +  setup (2, 3, 4, 5, 6);
> +  stuff (with_fam_1_v, with_fam_2_v, with_fam_3_v, only_fam_v, only_fam_2_v);
> +  return 0;
> +}
> diff --git a/gcc/testsuite/c-c++-common/fam-in-union-alone-in-struct-bos.c b/gcc/testsuite/c-c++-common/fam-in-union-alone-in-struct-bos.c
> new file mode 100644
> index 000000000000..21badc57982b
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/fam-in-union-alone-in-struct-bos.c
> @@ -0,0 +1,45 @@
> +/* testing flexible array members in unions and alone in structures:
> +   __bos/__bdos  */
> +/* { dg-do run } */
> +/* { dg-options "-O2" } */
> +
> +union with_fam_1 {
> +  char a;
> +  int b[];
> +} with_fam_1_v = {.b = {1, 2, 3, 4, 5}};
> +
> +union with_fam_2 {
> +  int a;
> +  char b[];
> +} with_fam_2_v = {.a = 0x1f2f3f4f};
> +
> +union with_fam_3 {
> +  char a[];
> +  int b[];
> +} with_fam_3_v = {.b = {0x1f2f3f4f, 0x5f6f7f7f}};
> +
> +struct only_fam {
> +  int b[];
> +} only_fam_v = {{7, 11}};
> +
> +struct only_fam_2 {
> +  unsigned int : 2;
> +  unsigned int : 3;
> +  int b[];
> +} only_fam_2_v = {{7, 11}};
> +
> +int main ()
> +{
> +  if (__builtin_object_size(with_fam_1_v.b, 1) != 20)
> +    __builtin_abort ();
> +  if (__builtin_object_size(with_fam_2_v.b, 1) != 4)
> +    __builtin_abort ();
> +  if (__builtin_object_size(with_fam_3_v.b, 1) != 8)
> +    __builtin_abort ();
> +  if (__builtin_object_size(only_fam_v.b, 1) != 8)
> +    __builtin_abort ();
> +  if (__builtin_object_size(only_fam_2_v.b, 1) != 8)
> +    __builtin_abort ();
> +
> +  return 0;
> +}
  

On Apr 30, 2024, at 15:29, Jason Merrill <jason@redhat.com> wrote:

On 4/30/24 07:58, Qing Zhao wrote:
Add testing cases to test the _bos for flexible array members in unions
or alone in structures.
gcc/c/ChangeLog:
* c-decl.cc (add_flexible_array_elts_to_size): Handle the cases
when the DECL is union.
gcc/cp/ChangeLog:
* decl.cc (layout_var_decl): Handle the cases when the DECL is
union with a flexible array member initializer.
gcc/testsuite/ChangeLog:
* c-c++-common/fam-in-union-alone-in-struct-bos-1.c: New test.
* c-c++-common/fam-in-union-alone-in-struct-bos.c: New test.
---
 gcc/c/c-decl.cc                               | 32 +++++++--
 gcc/cp/decl.cc                                | 33 ++++++++--
 .../fam-in-union-alone-in-struct-bos-1.c      | 66 +++++++++++++++++++
 .../fam-in-union-alone-in-struct-bos.c        | 45 +++++++++++++
 4 files changed, 162 insertions(+), 14 deletions(-)
 create mode 100644 gcc/testsuite/c-c++-common/fam-in-union-alone-in-struct-bos-1.c
 create mode 100644 gcc/testsuite/c-c++-common/fam-in-union-alone-in-struct-bos.c
diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
index 947f3cd589eb..59302c5cfb1f 100644
--- a/gcc/c/c-decl.cc
+++ b/gcc/c/c-decl.cc
@@ -5337,9 +5337,9 @@ zero_length_array_type_p (const_tree type)
 }
   /* INIT is a constructor that forms DECL's initializer.  If the final
-   element initializes a flexible array field, add the size of that
-   initializer to DECL's size.  */
-
+   element initializes a flexible array field, adjust the size of the
+   DECL with the initializer based on whether the DECL is a union or
+   a structure.  */
 static void
 add_flexible_array_elts_to_size (tree decl, tree init)
 {
@@ -5348,15 +5348,33 @@ add_flexible_array_elts_to_size (tree decl, tree init)
   if (vec_safe_is_empty (CONSTRUCTOR_ELTS (init)))
     return;
 +  bool is_decl_type_union = (TREE_CODE (TREE_TYPE (decl)) == UNION_TYPE);

In both C and C++ front-ends, I don't see any reason for this variable to be declared outside the if, since it's only used inside it.

It's not clear to me that it's needed at all since it's only used once, but if you prefer to have it, please move it inside the if.

Okay, will do that.

thanks.

Qing

+
   elt = CONSTRUCTOR_ELTS (init)->last ().value;
   type = TREE_TYPE (elt);
   if (flexible_array_member_type_p (type))
     {
       complete_array_type (&type, elt, false);
-      DECL_SIZE (decl)
- = size_binop (PLUS_EXPR, DECL_SIZE (decl), TYPE_SIZE (type));
-      DECL_SIZE_UNIT (decl)
- = size_binop (PLUS_EXPR, DECL_SIZE_UNIT (decl), TYPE_SIZE_UNIT (type));
+      /* For a structure, add the size of the initializer to the DECL's
+  size.  */
+      if (!is_decl_type_union)
+ {
+   DECL_SIZE (decl)
+     = size_binop (PLUS_EXPR, DECL_SIZE (decl), TYPE_SIZE (type));
+   DECL_SIZE_UNIT (decl)
+     = size_binop (PLUS_EXPR, DECL_SIZE_UNIT (decl),
+   TYPE_SIZE_UNIT (type));
+ }
+      /* For a union, the DECL's size is the maximum of the current size
+  and the size of the initializer.  */
+      else
+ {
+   DECL_SIZE (decl)
+     = size_binop (MAX_EXPR, DECL_SIZE (decl), TYPE_SIZE (type));
+   DECL_SIZE_UNIT (decl)
+     = size_binop (MAX_EXPR, DECL_SIZE_UNIT (decl),
+   TYPE_SIZE_UNIT (type));
+ }
     }
 }

diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index 9a91c6f80da1..0ea3ef165b66 100644
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -6555,8 +6555,9 @@ layout_var_decl (tree decl)
  }
     }
 -  /* If the final element initializes a flexible array field, add the size of
-     that initializer to DECL's size.  */
+  /* If the final element initializes a flexible array field, adjust
+     the size of the DECL with the initializer based on whether the
+     DECL is a union or a structure.  */
   if (type != error_mark_node
       && DECL_INITIAL (decl)
       && TREE_CODE (DECL_INITIAL (decl)) == CONSTRUCTOR
@@ -6568,6 +6569,7 @@ layout_var_decl (tree decl)
       && tree_int_cst_equal (DECL_SIZE (decl), TYPE_SIZE (type)))
     {
       constructor_elt &elt = CONSTRUCTOR_ELTS (DECL_INITIAL (decl))->last ();
+      bool is_decl_type_union = (TREE_CODE (TREE_TYPE (decl)) == UNION_TYPE);
       if (elt.index)
  {
    tree itype = TREE_TYPE (elt.index);
@@ -6577,11 +6579,28 @@ layout_var_decl (tree decl)
        && TREE_CODE (vtype) == ARRAY_TYPE
        && COMPLETE_TYPE_P (vtype))
      {
-       DECL_SIZE (decl)
- = size_binop (PLUS_EXPR, DECL_SIZE (decl), TYPE_SIZE (vtype));
-       DECL_SIZE_UNIT (decl)
- = size_binop (PLUS_EXPR, DECL_SIZE_UNIT (decl),
-       TYPE_SIZE_UNIT (vtype));
+       /* For a structure, add the size of the initializer to the DECL's
+  size.  */
+       if (!is_decl_type_union)
+ {
+   DECL_SIZE (decl)
+     = size_binop (PLUS_EXPR, DECL_SIZE (decl),
+   TYPE_SIZE (vtype));
+   DECL_SIZE_UNIT (decl)
+     = size_binop (PLUS_EXPR, DECL_SIZE_UNIT (decl),
+   TYPE_SIZE_UNIT (vtype));
+ }
+       /* a union, the DECL's size is the maximum of the current size
+  and the size of the initializer.  */
+       else
+ {
+   DECL_SIZE (decl)
+     = size_binop (MAX_EXPR, DECL_SIZE (decl),
+   TYPE_SIZE (vtype));
+   DECL_SIZE_UNIT (decl)
+     = size_binop (MAX_EXPR, DECL_SIZE_UNIT (decl),
+   TYPE_SIZE_UNIT (vtype));
+ }
      }
  }
     }
diff --git a/gcc/testsuite/c-c++-common/fam-in-union-alone-in-struct-bos-1.c b/gcc/testsuite/c-c++-common/fam-in-union-alone-in-struct-bos-1.c
new file mode 100644
index 000000000000..aae9cf39c8c7
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/fam-in-union-alone-in-struct-bos-1.c
@@ -0,0 +1,66 @@
+/* testing flexible array members in unions and alone in structures:
+   __bos/__bdos  */
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+
+union with_fam_1 {
+  char a;
+  int b[];
+} *with_fam_1_v;
+
+union with_fam_2 {
+  int a;
+  char b[];
+} *with_fam_2_v;
+
+union with_fam_3 {
+  char a[];
+  int b[];
+} *with_fam_3_v;
+
+struct only_fam {
+  int b[];
+} *only_fam_v;
+
+struct only_fam_2 {
+  unsigned int : 2;
+  unsigned int : 3;
+  int b[];
+} *only_fam_2_v;
+
+void __attribute__((__noinline__))
+setup (int n1, int n2, int n3, int n4, int n5)
+{
+  with_fam_1_v = (union with_fam_1 *) __builtin_malloc (n1 * sizeof (int));
+  with_fam_2_v = (union with_fam_2 *) __builtin_malloc (n2 * sizeof (char));
+  with_fam_3_v = (union with_fam_3 *) __builtin_malloc (n3 * sizeof (int));
+  only_fam_v = (struct only_fam *) __builtin_malloc (n4 * sizeof (int));
+  only_fam_2_v = (struct only_fam_2 *) __builtin_malloc (n5 * sizeof (int));
+  return;
+}
+
+void __attribute__((__noinline__)) stuff(
+    union with_fam_1 *with_fam_1_v,
+    union with_fam_2 *with_fam_2_v,
+    union with_fam_3 *with_fam_3_v,
+    struct only_fam *only_fam_v,
+    struct only_fam_2 *only_fam_2_v)
+{
+  if (__builtin_object_size(with_fam_1_v->b, 1) != -1)
+    __builtin_abort ();
+  if (__builtin_object_size(with_fam_2_v->b, 1) != -1)
+    __builtin_abort ();
+  if (__builtin_object_size(with_fam_3_v->b, 1) != -1)
+    __builtin_abort ();
+  if (__builtin_object_size(only_fam_v->b, 1) != -1)
+    __builtin_abort ();
+  if (__builtin_object_size(only_fam_2_v->b, 1) != -1)
+    __builtin_abort ();
+}
+
+int main (int argc, char *argv[])
+{
+  setup (2, 3, 4, 5, 6);
+  stuff (with_fam_1_v, with_fam_2_v, with_fam_3_v, only_fam_v, only_fam_2_v);
+  return 0;
+}
diff --git a/gcc/testsuite/c-c++-common/fam-in-union-alone-in-struct-bos.c b/gcc/testsuite/c-c++-common/fam-in-union-alone-in-struct-bos.c
new file mode 100644
index 000000000000..21badc57982b
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/fam-in-union-alone-in-struct-bos.c
@@ -0,0 +1,45 @@
+/* testing flexible array members in unions and alone in structures:
+   __bos/__bdos  */
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+
+union with_fam_1 {
+  char a;
+  int b[];
+} with_fam_1_v = {.b = {1, 2, 3, 4, 5}};
+
+union with_fam_2 {
+  int a;
+  char b[];
+} with_fam_2_v = {.a = 0x1f2f3f4f};
+
+union with_fam_3 {
+  char a[];
+  int b[];
+} with_fam_3_v = {.b = {0x1f2f3f4f, 0x5f6f7f7f}};
+
+struct only_fam {
+  int b[];
+} only_fam_v = {{7, 11}};
+
+struct only_fam_2 {
+  unsigned int : 2;
+  unsigned int : 3;
+  int b[];
+} only_fam_2_v = {{7, 11}};
+
+int main ()
+{
+  if (__builtin_object_size(with_fam_1_v.b, 1) != 20)
+    __builtin_abort ();
+  if (__builtin_object_size(with_fam_2_v.b, 1) != 4)
+    __builtin_abort ();
+  if (__builtin_object_size(with_fam_3_v.b, 1) != 8)
+    __builtin_abort ();
+  if (__builtin_object_size(only_fam_v.b, 1) != 8)
+    __builtin_abort ();
+  if (__builtin_object_size(only_fam_2_v.b, 1) != 8)
+    __builtin_abort ();
+
+  return 0;
+}