[V4][PATCH 0/2] Handle component_ref to a structure/union field including FAM for builtin_object_size

[V4][PATCH 0/2] Handle component_ref to a structure/union field including FAM for builtin_object_size

[Qing Zhao]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 2460 bytes --]

Hi, Joseph and Richard,

Could you please review this patch and let me know whether it’s ready
 for committing into GCC13?

The fix to Bug PR101832 is an important patch for kernel security
purpose. it's better to be put into GCC13.

=============================

These are the 4th version of the patches for PR101832, to fix
builtin_object_size to correctly handle component_ref to a
structure/union field that includes a flexible array member.

also includes a documentation update for the GCC extension on embedding
a structure/union with flexible array member into another structure.
which includes a fix to PR77650.

compared to the 3rd version of the patch, the major changes are:

1. update the documentation part per Joseph's comments.

compared to the 2nd version of the patch, the major changes are:

1. only include C99 flexible array member to this extension, trailing [0], [1]
  and [4] are all excluded.
2. for the new bit type_include_flexarray in tree_type_common, print it
  and also stream in/out it. 
3. update testing cases.
4. more clarification on the documentation. warnings for deprecating the 
  case when the structure with C99 FAM is embedded in the middle of
  another structure. 
5. add a new warning option -Wgnu-variable-sized-type-not-at-end for
  identifing all such cases.

bootstrapped and regression tested on aarch64 and x86.

Okay for commit?

thanks.

Qing

Qing Zhao (2):
  Handle component_ref to a structre/union field including C99 FAM
    [PR101832]
  Update documentation to clarify a GCC extension

 gcc/c-family/c.opt                            |   5 +
 gcc/c/c-decl.cc                               |  19 +++
 gcc/cp/module.cc                              |   2 +
 gcc/doc/extend.texi                           |  48 ++++++-
 gcc/print-tree.cc                             |   5 +
 .../gcc.dg/builtin-object-size-pr101832.c     | 134 ++++++++++++++++++
 .../gcc.dg/variable-sized-type-flex-array.c   |  31 ++++
 gcc/tree-core.h                               |   4 +-
 gcc/tree-object-size.cc                       |  79 +++++++----
 gcc/tree-streamer-in.cc                       |   1 +
 gcc/tree-streamer-out.cc                      |   1 +
 gcc/tree.h                                    |   6 +
 12 files changed, 305 insertions(+), 30 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c
 create mode 100644 gcc/testsuite/gcc.dg/variable-sized-type-flex-array.c

-- 
2.31.1

[v4][PATCH 1/2] Handle component_ref to a structre/union field including C99 FAM [PR101832]

[Qing Zhao]

GCC extension accepts the case when a struct with a C99 flexible array member
is embedded into another struct or union (possibly recursively).
__builtin_object_size should treat such struct as flexible size.

gcc/c/ChangeLog:

	PR tree-optimization/101832
	* c-decl.cc (finish_struct): Set TYPE_INCLUDE_FLEXARRAY for
	struct/union type.

gcc/cp/ChangeLog:

	PR tree-optimization/101832
	* module.cc (trees_out::core_bools): Stream out new bit
	type_include_flexarray.
	(trees_in::core_bools): Stream in new bit type_include_flexarray.

gcc/ChangeLog:

	PR tree-optimization/101832
	* print-tree.cc (print_node): Print new bit type_include_flexarray.
	* tree-core.h (struct tree_type_common): New bit
	type_include_flexarray.
	* tree-object-size.cc (addr_object_size): Handle structure/union type
	when it has flexible size.
	* tree-streamer-in.cc (unpack_ts_type_common_value_fields): Stream
	in new bit type_include_flexarray.
	* tree-streamer-out.cc (pack_ts_type_common_value_fields): Stream
	out new bit type_include_flexarray.
	* tree.h (TYPE_INCLUDE_FLEXARRAY): New macro
	TYPE_INCLUDE_FLEXARRAY.

gcc/testsuite/ChangeLog:

	PR tree-optimization/101832
	* gcc.dg/builtin-object-size-pr101832.c: New test.
---
 gcc/c/c-decl.cc                               |  12 ++
 gcc/cp/module.cc                              |   2 +
 gcc/print-tree.cc                             |   5 +
 .../gcc.dg/builtin-object-size-pr101832.c     | 134 ++++++++++++++++++
 gcc/tree-core.h                               |   4 +-
 gcc/tree-object-size.cc                       |  79 +++++++----
 gcc/tree-streamer-in.cc                       |   1 +
 gcc/tree-streamer-out.cc                      |   1 +
 gcc/tree.h                                    |   6 +
 9 files changed, 215 insertions(+), 29 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c

diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
index 08078eadeb8..f589a2f5192 100644
--- a/gcc/c/c-decl.cc
+++ b/gcc/c/c-decl.cc
@@ -9284,6 +9284,18 @@ finish_struct (location_t loc, tree t, tree fieldlist, tree attributes,
       /* Set DECL_NOT_FLEXARRAY flag for FIELD_DECL x.  */
       DECL_NOT_FLEXARRAY (x) = !is_flexible_array_member_p (is_last_field, x);
 
+      /* Set TYPE_INCLUDE_FLEXARRAY for the context of x, t
+       * when x is an array.  */
+      if (TREE_CODE (TREE_TYPE (x)) == ARRAY_TYPE)
+	TYPE_INCLUDE_FLEXARRAY (t) = flexible_array_member_type_p (TREE_TYPE (x)) ;
+      /* Recursively set TYPE_INCLUDE_FLEXARRAY for the context of x, t
+	 when x is the last field.  */
+      else if ((TREE_CODE (TREE_TYPE (x)) == RECORD_TYPE
+		|| TREE_CODE (TREE_TYPE (x)) == UNION_TYPE)
+	       && TYPE_INCLUDE_FLEXARRAY (TREE_TYPE (x))
+	       && is_last_field)
+	TYPE_INCLUDE_FLEXARRAY (t) = true;
+
       if (DECL_NAME (x)
 	  || RECORD_OR_UNION_TYPE_P (TREE_TYPE (x)))
 	saw_named_field = true;
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index ac2fe66b080..c750361b704 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -5371,6 +5371,7 @@ trees_out::core_bools (tree t)
       WB (t->type_common.lang_flag_5);
       WB (t->type_common.lang_flag_6);
       WB (t->type_common.typeless_storage);
+      WB (t->type_common.type_include_flexarray);
     }
 
   if (CODE_CONTAINS_STRUCT (code, TS_DECL_COMMON))
@@ -5551,6 +5552,7 @@ trees_in::core_bools (tree t)
       RB (t->type_common.lang_flag_5);
       RB (t->type_common.lang_flag_6);
       RB (t->type_common.typeless_storage);
+      RB (t->type_common.type_include_flexarray);
     }
 
   if (CODE_CONTAINS_STRUCT (code, TS_DECL_COMMON))
diff --git a/gcc/print-tree.cc b/gcc/print-tree.cc
index 1f3afcbbc86..efacdb7686f 100644
--- a/gcc/print-tree.cc
+++ b/gcc/print-tree.cc
@@ -631,6 +631,11 @@ print_node (FILE *file, const char *prefix, tree node, int indent,
 	  && TYPE_CXX_ODR_P (node))
 	fputs (" cxx-odr-p", file);
 
+      if ((code == RECORD_TYPE
+	   || code == UNION_TYPE)
+	  && TYPE_INCLUDE_FLEXARRAY (node))
+	fputs (" include-flexarray", file);
+
       /* The transparent-union flag is used for different things in
 	 different nodes.  */
       if ((code == UNION_TYPE || code == RECORD_TYPE)
diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c b/gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c
new file mode 100644
index 00000000000..60078e11634
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c
@@ -0,0 +1,134 @@
+/* PR 101832: 
+   GCC extension accepts the case when a struct with a C99 flexible array
+   member is embedded into another struct (possibly recursively).
+   __builtin_object_size will treat such struct as flexible size.
+   However, when a structure with non-C99 flexible array member, i.e, trailing
+   [0], [1], or [4], is embedded into anther struct, the stucture will not
+   be treated as flexible size.  */ 
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+
+#include "builtin-object-size-common.h"
+
+#define expect(p, _v) do { \
+  size_t v = _v; \
+  if (p == v) \
+    __builtin_printf ("ok:  %s == %zd\n", #p, p); \
+  else {\
+    __builtin_printf ("WAT: %s == %zd (expected %zd)\n", #p, p, v); \
+    FAIL (); \
+  } \
+} while (0);
+
+
+struct A {
+  int n;
+  char data[];
+};
+
+struct B {
+  int m;
+  struct A a;
+};
+
+struct C {
+  int q;
+  struct B b;
+};
+
+struct A0 {
+  int n;
+  char data[0];
+};
+
+struct B0 {
+  int m;
+  struct A0 a;
+};
+
+struct C0 {
+  int q;
+  struct B0 b;
+};
+
+struct A1 {
+  int n;
+  char data[1];
+};
+
+struct B1 {
+  int m;
+  struct A1 a;
+};
+
+struct C1 {
+  int q;
+  struct B1 b;
+};
+
+struct An {
+  int n;
+  char data[8];
+};
+
+struct Bn {
+  int m;
+  struct An a;
+};
+
+struct Cn {
+  int q;
+  struct Bn b;
+};
+
+volatile void *magic1, *magic2;
+
+int main (int argc, char *argv[])
+{
+  struct B *outer;
+  struct C *outest;
+
+  /* Make sure optimization can't find some other object size. */
+  outer = (void *)magic1;
+  outest = (void *)magic2;
+
+  expect (__builtin_object_size (&outer->a, 1), -1);
+  expect (__builtin_object_size (&outest->b, 1), -1);
+  expect (__builtin_object_size (&outest->b.a, 1), -1);
+
+  struct B0 *outer0;
+  struct C0 *outest0;
+
+  /* Make sure optimization can't find some other object size. */
+  outer0 = (void *)magic1;
+  outest0 = (void *)magic2;
+
+  expect (__builtin_object_size (&outer0->a, 1), sizeof (outer0->a));
+  expect (__builtin_object_size (&outest0->b, 1), sizeof (outest0->b));
+  expect (__builtin_object_size (&outest0->b.a, 1), sizeof (outest0->b.a));
+
+  struct B1 *outer1;
+  struct C1 *outest1;
+
+  /* Make sure optimization can't find some other object size. */
+  outer1 = (void *)magic1;
+  outest1 = (void *)magic2;
+
+  expect (__builtin_object_size (&outer1->a, 1), sizeof (outer1->a));
+  expect (__builtin_object_size (&outest1->b, 1), sizeof (outest1->b));
+  expect (__builtin_object_size (&outest1->b.a, 1), sizeof (outest1->b.a));
+
+  struct Bn *outern;
+  struct Cn *outestn;
+
+  /* Make sure optimization can't find some other object size. */
+  outern = (void *)magic1;
+  outestn = (void *)magic2;
+
+  expect (__builtin_object_size (&outern->a, 1), sizeof (outern->a));
+  expect (__builtin_object_size (&outestn->b, 1), sizeof (outestn->b));
+  expect (__builtin_object_size (&outestn->b.a, 1), sizeof (outestn->b.a));
+
+  DONE ();
+  return 0;
+}
diff --git a/gcc/tree-core.h b/gcc/tree-core.h
index acd8deea34e..705d5702b9c 100644
--- a/gcc/tree-core.h
+++ b/gcc/tree-core.h
@@ -1718,7 +1718,9 @@ struct GTY(()) tree_type_common {
   unsigned empty_flag : 1;
   unsigned indivisible_p : 1;
   unsigned no_named_args_stdarg_p : 1;
-  unsigned spare : 15;
+  /* TYPE_INCLUDE_FLEXARRAY flag for RECORD_TYPE and UNION_TYPE.  */
+  unsigned int type_include_flexarray : 1;
+  unsigned spare : 14;
 
   alias_set_type alias_set;
   tree pointer_to;
diff --git a/gcc/tree-object-size.cc b/gcc/tree-object-size.cc
index 9a936a91983..22b3c72ea6e 100644
--- a/gcc/tree-object-size.cc
+++ b/gcc/tree-object-size.cc
@@ -633,45 +633,68 @@ addr_object_size (struct object_size_info *osi, const_tree ptr,
 		    v = NULL_TREE;
 		    break;
 		  case COMPONENT_REF:
-		    if (TREE_CODE (TREE_TYPE (v)) != ARRAY_TYPE)
+		    /* When the ref is not to an array, a record or a union, it
+		       will not have flexible size, compute the object size
+		       directly.  */
+		    if ((TREE_CODE (TREE_TYPE (v)) != ARRAY_TYPE)
+			&& (TREE_CODE (TREE_TYPE (v)) != RECORD_TYPE)
+			&& (TREE_CODE (TREE_TYPE (v)) != UNION_TYPE))
 		      {
 			v = NULL_TREE;
 			break;
 		      }
-		    is_flexible_array_mem_ref = array_ref_flexible_size_p (v);
-		    while (v != pt_var && TREE_CODE (v) == COMPONENT_REF)
-		      if (TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
-			  != UNION_TYPE
-			  && TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
-			  != QUAL_UNION_TYPE)
-			break;
-		      else
-			v = TREE_OPERAND (v, 0);
-		    if (TREE_CODE (v) == COMPONENT_REF
-			&& TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
-			   == RECORD_TYPE)
+		    if (TREE_CODE (TREE_TYPE (v)) == RECORD_TYPE
+			|| TREE_CODE (TREE_TYPE (v)) == UNION_TYPE)
+		    /* if the record or union does not include a flexible array
+		       recursively, compute the object size directly.  */
 		      {
-			/* compute object size only if v is not a
-			   flexible array member.  */
-			if (!is_flexible_array_mem_ref)
+			if (!TYPE_INCLUDE_FLEXARRAY (TREE_TYPE (v)))
 			  {
 			    v = NULL_TREE;
 			    break;
 			  }
-			v = TREE_OPERAND (v, 0);
+			else
+			  v = TREE_OPERAND (v, 0);
 		      }
-		    while (v != pt_var && TREE_CODE (v) == COMPONENT_REF)
-		      if (TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
-			  != UNION_TYPE
-			  && TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
-			  != QUAL_UNION_TYPE)
-			break;
-		      else
-			v = TREE_OPERAND (v, 0);
-		    if (v != pt_var)
-		      v = NULL_TREE;
 		    else
-		      v = pt_var;
+		      {
+			/* Now the ref is to an array type.  */
+			is_flexible_array_mem_ref
+			  = array_ref_flexible_size_p (v);
+			while (v != pt_var && TREE_CODE (v) == COMPONENT_REF)
+			if (TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
+			      != UNION_TYPE
+			    && TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
+				 != QUAL_UNION_TYPE)
+			  break;
+			else
+			  v = TREE_OPERAND (v, 0);
+			if (TREE_CODE (v) == COMPONENT_REF
+			    && TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
+				 == RECORD_TYPE)
+			  {
+			    /* compute object size only if v is not a
+			       flexible array member.  */
+			    if (!is_flexible_array_mem_ref)
+			      {
+				v = NULL_TREE;
+				break;
+			      }
+			    v = TREE_OPERAND (v, 0);
+			  }
+			while (v != pt_var && TREE_CODE (v) == COMPONENT_REF)
+			  if (TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
+				!= UNION_TYPE
+			      && TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
+				   != QUAL_UNION_TYPE)
+			    break;
+			  else
+			    v = TREE_OPERAND (v, 0);
+			if (v != pt_var)
+			  v = NULL_TREE;
+			else
+			  v = pt_var;
+		      }
 		    break;
 		  default:
 		    v = pt_var;
diff --git a/gcc/tree-streamer-in.cc b/gcc/tree-streamer-in.cc
index d4dc30f048f..c19ede0631d 100644
--- a/gcc/tree-streamer-in.cc
+++ b/gcc/tree-streamer-in.cc
@@ -390,6 +390,7 @@ unpack_ts_type_common_value_fields (struct bitpack_d *bp, tree expr)
       TYPE_TRANSPARENT_AGGR (expr) = (unsigned) bp_unpack_value (bp, 1);
       TYPE_FINAL_P (expr) = (unsigned) bp_unpack_value (bp, 1);
       TYPE_CXX_ODR_P (expr) = (unsigned) bp_unpack_value (bp, 1);
+      TYPE_INCLUDE_FLEXARRAY (expr) = (unsigned) bp_unpack_value (bp, 1);
     }
   else if (TREE_CODE (expr) == ARRAY_TYPE)
     TYPE_NONALIASED_COMPONENT (expr) = (unsigned) bp_unpack_value (bp, 1);
diff --git a/gcc/tree-streamer-out.cc b/gcc/tree-streamer-out.cc
index d107229da5c..73e4b4e547c 100644
--- a/gcc/tree-streamer-out.cc
+++ b/gcc/tree-streamer-out.cc
@@ -357,6 +357,7 @@ pack_ts_type_common_value_fields (struct bitpack_d *bp, tree expr)
       bp_pack_value (bp, flag_wpa && TYPE_CANONICAL (expr)
 			 ? TYPE_CXX_ODR_P (TYPE_CANONICAL (expr))
 			 : TYPE_CXX_ODR_P (expr), 1);
+      bp_pack_value (bp, TYPE_INCLUDE_FLEXARRAY (expr), 1);
     }
   else if (TREE_CODE (expr) == ARRAY_TYPE)
     bp_pack_value (bp, TYPE_NONALIASED_COMPONENT (expr), 1);
diff --git a/gcc/tree.h b/gcc/tree.h
index 92ac0e6a214..ab1cdc3dc85 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -778,6 +778,12 @@ extern void omp_clause_range_check_failed (const_tree, const char *, int,
 #define TYPE_NO_NAMED_ARGS_STDARG_P(NODE) \
   (TYPE_CHECK (NODE)->type_common.no_named_args_stdarg_p)
 
+/* True if this RECORD_TYPE or UNION_TYPE includes a flexible array member
+   at the last field recursively.  */
+#define TYPE_INCLUDE_FLEXARRAY(NODE) \
+  (TYPE_CHECK (NODE)->type_common.type_include_flexarray)
+
+
 /* In an IDENTIFIER_NODE, this means that assemble_name was called with
    this string as an argument.  */
 #define TREE_SYMBOL_REFERENCED(NODE) \
-- 
2.31.1

Re: [v4][PATCH 1/2] Handle component_ref to a structre/union field including C99 FAM [PR101832]

[Qing Zhao]

Ping.

Qing

> On Feb 24, 2023, at 1:35 PM, Qing Zhao <qing.zhao@oracle.com> wrote:
> 
> GCC extension accepts the case when a struct with a C99 flexible array member
> is embedded into another struct or union (possibly recursively).
> __builtin_object_size should treat such struct as flexible size.
> 
> gcc/c/ChangeLog:
> 
> 	PR tree-optimization/101832
> 	* c-decl.cc (finish_struct): Set TYPE_INCLUDE_FLEXARRAY for
> 	struct/union type.
> 
> gcc/cp/ChangeLog:
> 
> 	PR tree-optimization/101832
> 	* module.cc (trees_out::core_bools): Stream out new bit
> 	type_include_flexarray.
> 	(trees_in::core_bools): Stream in new bit type_include_flexarray.
> 
> gcc/ChangeLog:
> 
> 	PR tree-optimization/101832
> 	* print-tree.cc (print_node): Print new bit type_include_flexarray.
> 	* tree-core.h (struct tree_type_common): New bit
> 	type_include_flexarray.
> 	* tree-object-size.cc (addr_object_size): Handle structure/union type
> 	when it has flexible size.
> 	* tree-streamer-in.cc (unpack_ts_type_common_value_fields): Stream
> 	in new bit type_include_flexarray.
> 	* tree-streamer-out.cc (pack_ts_type_common_value_fields): Stream
> 	out new bit type_include_flexarray.
> 	* tree.h (TYPE_INCLUDE_FLEXARRAY): New macro
> 	TYPE_INCLUDE_FLEXARRAY.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR tree-optimization/101832
> 	* gcc.dg/builtin-object-size-pr101832.c: New test.
> ---
> gcc/c/c-decl.cc                               |  12 ++
> gcc/cp/module.cc                              |   2 +
> gcc/print-tree.cc                             |   5 +
> .../gcc.dg/builtin-object-size-pr101832.c     | 134 ++++++++++++++++++
> gcc/tree-core.h                               |   4 +-
> gcc/tree-object-size.cc                       |  79 +++++++----
> gcc/tree-streamer-in.cc                       |   1 +
> gcc/tree-streamer-out.cc                      |   1 +
> gcc/tree.h                                    |   6 +
> 9 files changed, 215 insertions(+), 29 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c
> 
> diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
> index 08078eadeb8..f589a2f5192 100644
> --- a/gcc/c/c-decl.cc
> +++ b/gcc/c/c-decl.cc
> @@ -9284,6 +9284,18 @@ finish_struct (location_t loc, tree t, tree fieldlist, tree attributes,
>       /* Set DECL_NOT_FLEXARRAY flag for FIELD_DECL x.  */
>       DECL_NOT_FLEXARRAY (x) = !is_flexible_array_member_p (is_last_field, x);
> 
> +      /* Set TYPE_INCLUDE_FLEXARRAY for the context of x, t
> +       * when x is an array.  */
> +      if (TREE_CODE (TREE_TYPE (x)) == ARRAY_TYPE)
> +	TYPE_INCLUDE_FLEXARRAY (t) = flexible_array_member_type_p (TREE_TYPE (x)) ;
> +      /* Recursively set TYPE_INCLUDE_FLEXARRAY for the context of x, t
> +	 when x is the last field.  */
> +      else if ((TREE_CODE (TREE_TYPE (x)) == RECORD_TYPE
> +		|| TREE_CODE (TREE_TYPE (x)) == UNION_TYPE)
> +	       && TYPE_INCLUDE_FLEXARRAY (TREE_TYPE (x))
> +	       && is_last_field)
> +	TYPE_INCLUDE_FLEXARRAY (t) = true;
> +
>       if (DECL_NAME (x)
> 	  || RECORD_OR_UNION_TYPE_P (TREE_TYPE (x)))
> 	saw_named_field = true;
> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> index ac2fe66b080..c750361b704 100644
> --- a/gcc/cp/module.cc
> +++ b/gcc/cp/module.cc
> @@ -5371,6 +5371,7 @@ trees_out::core_bools (tree t)
>       WB (t->type_common.lang_flag_5);
>       WB (t->type_common.lang_flag_6);
>       WB (t->type_common.typeless_storage);
> +      WB (t->type_common.type_include_flexarray);
>     }
> 
>   if (CODE_CONTAINS_STRUCT (code, TS_DECL_COMMON))
> @@ -5551,6 +5552,7 @@ trees_in::core_bools (tree t)
>       RB (t->type_common.lang_flag_5);
>       RB (t->type_common.lang_flag_6);
>       RB (t->type_common.typeless_storage);
> +      RB (t->type_common.type_include_flexarray);
>     }
> 
>   if (CODE_CONTAINS_STRUCT (code, TS_DECL_COMMON))
> diff --git a/gcc/print-tree.cc b/gcc/print-tree.cc
> index 1f3afcbbc86..efacdb7686f 100644
> --- a/gcc/print-tree.cc
> +++ b/gcc/print-tree.cc
> @@ -631,6 +631,11 @@ print_node (FILE *file, const char *prefix, tree node, int indent,
> 	  && TYPE_CXX_ODR_P (node))
> 	fputs (" cxx-odr-p", file);
> 
> +      if ((code == RECORD_TYPE
> +	   || code == UNION_TYPE)
> +	  && TYPE_INCLUDE_FLEXARRAY (node))
> +	fputs (" include-flexarray", file);
> +
>       /* The transparent-union flag is used for different things in
> 	 different nodes.  */
>       if ((code == UNION_TYPE || code == RECORD_TYPE)
> diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c b/gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c
> new file mode 100644
> index 00000000000..60078e11634
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c
> @@ -0,0 +1,134 @@
> +/* PR 101832: 
> +   GCC extension accepts the case when a struct with a C99 flexible array
> +   member is embedded into another struct (possibly recursively).
> +   __builtin_object_size will treat such struct as flexible size.
> +   However, when a structure with non-C99 flexible array member, i.e, trailing
> +   [0], [1], or [4], is embedded into anther struct, the stucture will not
> +   be treated as flexible size.  */ 
> +/* { dg-do run } */
> +/* { dg-options "-O2" } */
> +
> +#include "builtin-object-size-common.h"
> +
> +#define expect(p, _v) do { \
> +  size_t v = _v; \
> +  if (p == v) \
> +    __builtin_printf ("ok:  %s == %zd\n", #p, p); \
> +  else {\
> +    __builtin_printf ("WAT: %s == %zd (expected %zd)\n", #p, p, v); \
> +    FAIL (); \
> +  } \
> +} while (0);
> +
> +
> +struct A {
> +  int n;
> +  char data[];
> +};
> +
> +struct B {
> +  int m;
> +  struct A a;
> +};
> +
> +struct C {
> +  int q;
> +  struct B b;
> +};
> +
> +struct A0 {
> +  int n;
> +  char data[0];
> +};
> +
> +struct B0 {
> +  int m;
> +  struct A0 a;
> +};
> +
> +struct C0 {
> +  int q;
> +  struct B0 b;
> +};
> +
> +struct A1 {
> +  int n;
> +  char data[1];
> +};
> +
> +struct B1 {
> +  int m;
> +  struct A1 a;
> +};
> +
> +struct C1 {
> +  int q;
> +  struct B1 b;
> +};
> +
> +struct An {
> +  int n;
> +  char data[8];
> +};
> +
> +struct Bn {
> +  int m;
> +  struct An a;
> +};
> +
> +struct Cn {
> +  int q;
> +  struct Bn b;
> +};
> +
> +volatile void *magic1, *magic2;
> +
> +int main (int argc, char *argv[])
> +{
> +  struct B *outer;
> +  struct C *outest;
> +
> +  /* Make sure optimization can't find some other object size. */
> +  outer = (void *)magic1;
> +  outest = (void *)magic2;
> +
> +  expect (__builtin_object_size (&outer->a, 1), -1);
> +  expect (__builtin_object_size (&outest->b, 1), -1);
> +  expect (__builtin_object_size (&outest->b.a, 1), -1);
> +
> +  struct B0 *outer0;
> +  struct C0 *outest0;
> +
> +  /* Make sure optimization can't find some other object size. */
> +  outer0 = (void *)magic1;
> +  outest0 = (void *)magic2;
> +
> +  expect (__builtin_object_size (&outer0->a, 1), sizeof (outer0->a));
> +  expect (__builtin_object_size (&outest0->b, 1), sizeof (outest0->b));
> +  expect (__builtin_object_size (&outest0->b.a, 1), sizeof (outest0->b.a));
> +
> +  struct B1 *outer1;
> +  struct C1 *outest1;
> +
> +  /* Make sure optimization can't find some other object size. */
> +  outer1 = (void *)magic1;
> +  outest1 = (void *)magic2;
> +
> +  expect (__builtin_object_size (&outer1->a, 1), sizeof (outer1->a));
> +  expect (__builtin_object_size (&outest1->b, 1), sizeof (outest1->b));
> +  expect (__builtin_object_size (&outest1->b.a, 1), sizeof (outest1->b.a));
> +
> +  struct Bn *outern;
> +  struct Cn *outestn;
> +
> +  /* Make sure optimization can't find some other object size. */
> +  outern = (void *)magic1;
> +  outestn = (void *)magic2;
> +
> +  expect (__builtin_object_size (&outern->a, 1), sizeof (outern->a));
> +  expect (__builtin_object_size (&outestn->b, 1), sizeof (outestn->b));
> +  expect (__builtin_object_size (&outestn->b.a, 1), sizeof (outestn->b.a));
> +
> +  DONE ();
> +  return 0;
> +}
> diff --git a/gcc/tree-core.h b/gcc/tree-core.h
> index acd8deea34e..705d5702b9c 100644
> --- a/gcc/tree-core.h
> +++ b/gcc/tree-core.h
> @@ -1718,7 +1718,9 @@ struct GTY(()) tree_type_common {
>   unsigned empty_flag : 1;
>   unsigned indivisible_p : 1;
>   unsigned no_named_args_stdarg_p : 1;
> -  unsigned spare : 15;
> +  /* TYPE_INCLUDE_FLEXARRAY flag for RECORD_TYPE and UNION_TYPE.  */
> +  unsigned int type_include_flexarray : 1;
> +  unsigned spare : 14;
> 
>   alias_set_type alias_set;
>   tree pointer_to;
> diff --git a/gcc/tree-object-size.cc b/gcc/tree-object-size.cc
> index 9a936a91983..22b3c72ea6e 100644
> --- a/gcc/tree-object-size.cc
> +++ b/gcc/tree-object-size.cc
> @@ -633,45 +633,68 @@ addr_object_size (struct object_size_info *osi, const_tree ptr,
> 		    v = NULL_TREE;
> 		    break;
> 		  case COMPONENT_REF:
> -		    if (TREE_CODE (TREE_TYPE (v)) != ARRAY_TYPE)
> +		    /* When the ref is not to an array, a record or a union, it
> +		       will not have flexible size, compute the object size
> +		       directly.  */
> +		    if ((TREE_CODE (TREE_TYPE (v)) != ARRAY_TYPE)
> +			&& (TREE_CODE (TREE_TYPE (v)) != RECORD_TYPE)
> +			&& (TREE_CODE (TREE_TYPE (v)) != UNION_TYPE))
> 		      {
> 			v = NULL_TREE;
> 			break;
> 		      }
> -		    is_flexible_array_mem_ref = array_ref_flexible_size_p (v);
> -		    while (v != pt_var && TREE_CODE (v) == COMPONENT_REF)
> -		      if (TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
> -			  != UNION_TYPE
> -			  && TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
> -			  != QUAL_UNION_TYPE)
> -			break;
> -		      else
> -			v = TREE_OPERAND (v, 0);
> -		    if (TREE_CODE (v) == COMPONENT_REF
> -			&& TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
> -			   == RECORD_TYPE)
> +		    if (TREE_CODE (TREE_TYPE (v)) == RECORD_TYPE
> +			|| TREE_CODE (TREE_TYPE (v)) == UNION_TYPE)
> +		    /* if the record or union does not include a flexible array
> +		       recursively, compute the object size directly.  */
> 		      {
> -			/* compute object size only if v is not a
> -			   flexible array member.  */
> -			if (!is_flexible_array_mem_ref)
> +			if (!TYPE_INCLUDE_FLEXARRAY (TREE_TYPE (v)))
> 			  {
> 			    v = NULL_TREE;
> 			    break;
> 			  }
> -			v = TREE_OPERAND (v, 0);
> +			else
> +			  v = TREE_OPERAND (v, 0);
> 		      }
> -		    while (v != pt_var && TREE_CODE (v) == COMPONENT_REF)
> -		      if (TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
> -			  != UNION_TYPE
> -			  && TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
> -			  != QUAL_UNION_TYPE)
> -			break;
> -		      else
> -			v = TREE_OPERAND (v, 0);
> -		    if (v != pt_var)
> -		      v = NULL_TREE;
> 		    else
> -		      v = pt_var;
> +		      {
> +			/* Now the ref is to an array type.  */
> +			is_flexible_array_mem_ref
> +			  = array_ref_flexible_size_p (v);
> +			while (v != pt_var && TREE_CODE (v) == COMPONENT_REF)
> +			if (TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
> +			      != UNION_TYPE
> +			    && TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
> +				 != QUAL_UNION_TYPE)
> +			  break;
> +			else
> +			  v = TREE_OPERAND (v, 0);
> +			if (TREE_CODE (v) == COMPONENT_REF
> +			    && TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
> +				 == RECORD_TYPE)
> +			  {
> +			    /* compute object size only if v is not a
> +			       flexible array member.  */
> +			    if (!is_flexible_array_mem_ref)
> +			      {
> +				v = NULL_TREE;
> +				break;
> +			      }
> +			    v = TREE_OPERAND (v, 0);
> +			  }
> +			while (v != pt_var && TREE_CODE (v) == COMPONENT_REF)
> +			  if (TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
> +				!= UNION_TYPE
> +			      && TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
> +				   != QUAL_UNION_TYPE)
> +			    break;
> +			  else
> +			    v = TREE_OPERAND (v, 0);
> +			if (v != pt_var)
> +			  v = NULL_TREE;
> +			else
> +			  v = pt_var;
> +		      }
> 		    break;
> 		  default:
> 		    v = pt_var;
> diff --git a/gcc/tree-streamer-in.cc b/gcc/tree-streamer-in.cc
> index d4dc30f048f..c19ede0631d 100644
> --- a/gcc/tree-streamer-in.cc
> +++ b/gcc/tree-streamer-in.cc
> @@ -390,6 +390,7 @@ unpack_ts_type_common_value_fields (struct bitpack_d *bp, tree expr)
>       TYPE_TRANSPARENT_AGGR (expr) = (unsigned) bp_unpack_value (bp, 1);
>       TYPE_FINAL_P (expr) = (unsigned) bp_unpack_value (bp, 1);
>       TYPE_CXX_ODR_P (expr) = (unsigned) bp_unpack_value (bp, 1);
> +      TYPE_INCLUDE_FLEXARRAY (expr) = (unsigned) bp_unpack_value (bp, 1);
>     }
>   else if (TREE_CODE (expr) == ARRAY_TYPE)
>     TYPE_NONALIASED_COMPONENT (expr) = (unsigned) bp_unpack_value (bp, 1);
> diff --git a/gcc/tree-streamer-out.cc b/gcc/tree-streamer-out.cc
> index d107229da5c..73e4b4e547c 100644
> --- a/gcc/tree-streamer-out.cc
> +++ b/gcc/tree-streamer-out.cc
> @@ -357,6 +357,7 @@ pack_ts_type_common_value_fields (struct bitpack_d *bp, tree expr)
>       bp_pack_value (bp, flag_wpa && TYPE_CANONICAL (expr)
> 			 ? TYPE_CXX_ODR_P (TYPE_CANONICAL (expr))
> 			 : TYPE_CXX_ODR_P (expr), 1);
> +      bp_pack_value (bp, TYPE_INCLUDE_FLEXARRAY (expr), 1);
>     }
>   else if (TREE_CODE (expr) == ARRAY_TYPE)
>     bp_pack_value (bp, TYPE_NONALIASED_COMPONENT (expr), 1);
> diff --git a/gcc/tree.h b/gcc/tree.h
> index 92ac0e6a214..ab1cdc3dc85 100644
> --- a/gcc/tree.h
> +++ b/gcc/tree.h
> @@ -778,6 +778,12 @@ extern void omp_clause_range_check_failed (const_tree, const char *, int,
> #define TYPE_NO_NAMED_ARGS_STDARG_P(NODE) \
>   (TYPE_CHECK (NODE)->type_common.no_named_args_stdarg_p)
> 
> +/* True if this RECORD_TYPE or UNION_TYPE includes a flexible array member
> +   at the last field recursively.  */
> +#define TYPE_INCLUDE_FLEXARRAY(NODE) \
> +  (TYPE_CHECK (NODE)->type_common.type_include_flexarray)
> +
> +
> /* In an IDENTIFIER_NODE, this means that assemble_name was called with
>    this string as an argument.  */
> #define TREE_SYMBOL_REFERENCED(NODE) \
> -- 
> 2.31.1
> 

Re: [v4][PATCH 1/2] Handle component_ref to a structre/union field including C99 FAM [PR101832]

[Richard Biener]

On Fri, 24 Feb 2023, Qing Zhao wrote:

> GCC extension accepts the case when a struct with a C99 flexible array member
> is embedded into another struct or union (possibly recursively).
> __builtin_object_size should treat such struct as flexible size.
> 
> gcc/c/ChangeLog:
> 
> 	PR tree-optimization/101832
> 	* c-decl.cc (finish_struct): Set TYPE_INCLUDE_FLEXARRAY for
> 	struct/union type.

I can't really comment on the correctness of this part but since
only the C frontend will ever set this and you are using it from
addr_object_size which is also used for other C family languages
(at least), I wonder if you can really test

+                       if (!TYPE_INCLUDE_FLEXARRAY (TREE_TYPE (v)))

there.

Originally I was suggesting to set this flag in stor-layout.cc
which eventually all languages funnel their types through and
if there's language specific handling use a langhook (with the
default implementation preserving the status quo).

Some more comments below ...

> gcc/cp/ChangeLog:
> 
> 	PR tree-optimization/101832
> 	* module.cc (trees_out::core_bools): Stream out new bit
> 	type_include_flexarray.
> 	(trees_in::core_bools): Stream in new bit type_include_flexarray.
> 
> gcc/ChangeLog:
> 
> 	PR tree-optimization/101832
> 	* print-tree.cc (print_node): Print new bit type_include_flexarray.
> 	* tree-core.h (struct tree_type_common): New bit
> 	type_include_flexarray.
> 	* tree-object-size.cc (addr_object_size): Handle structure/union type
> 	when it has flexible size.
> 	* tree-streamer-in.cc (unpack_ts_type_common_value_fields): Stream
> 	in new bit type_include_flexarray.
> 	* tree-streamer-out.cc (pack_ts_type_common_value_fields): Stream
> 	out new bit type_include_flexarray.
> 	* tree.h (TYPE_INCLUDE_FLEXARRAY): New macro
> 	TYPE_INCLUDE_FLEXARRAY.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR tree-optimization/101832
> 	* gcc.dg/builtin-object-size-pr101832.c: New test.
> ---
>  gcc/c/c-decl.cc                               |  12 ++
>  gcc/cp/module.cc                              |   2 +
>  gcc/print-tree.cc                             |   5 +
>  .../gcc.dg/builtin-object-size-pr101832.c     | 134 ++++++++++++++++++
>  gcc/tree-core.h                               |   4 +-
>  gcc/tree-object-size.cc                       |  79 +++++++----
>  gcc/tree-streamer-in.cc                       |   1 +
>  gcc/tree-streamer-out.cc                      |   1 +
>  gcc/tree.h                                    |   6 +
>  9 files changed, 215 insertions(+), 29 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c
> 
> diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
> index 08078eadeb8..f589a2f5192 100644
> --- a/gcc/c/c-decl.cc
> +++ b/gcc/c/c-decl.cc
> @@ -9284,6 +9284,18 @@ finish_struct (location_t loc, tree t, tree fieldlist, tree attributes,
>        /* Set DECL_NOT_FLEXARRAY flag for FIELD_DECL x.  */
>        DECL_NOT_FLEXARRAY (x) = !is_flexible_array_member_p (is_last_field, x);
>  
> +      /* Set TYPE_INCLUDE_FLEXARRAY for the context of x, t
> +       * when x is an array.  */
> +      if (TREE_CODE (TREE_TYPE (x)) == ARRAY_TYPE)
> +	TYPE_INCLUDE_FLEXARRAY (t) = flexible_array_member_type_p (TREE_TYPE (x)) ;
> +      /* Recursively set TYPE_INCLUDE_FLEXARRAY for the context of x, t
> +	 when x is the last field.  */
> +      else if ((TREE_CODE (TREE_TYPE (x)) == RECORD_TYPE
> +		|| TREE_CODE (TREE_TYPE (x)) == UNION_TYPE)
> +	       && TYPE_INCLUDE_FLEXARRAY (TREE_TYPE (x))
> +	       && is_last_field)
> +	TYPE_INCLUDE_FLEXARRAY (t) = true;
> +
>        if (DECL_NAME (x)
>  	  || RECORD_OR_UNION_TYPE_P (TREE_TYPE (x)))
>  	saw_named_field = true;
> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> index ac2fe66b080..c750361b704 100644
> --- a/gcc/cp/module.cc
> +++ b/gcc/cp/module.cc
> @@ -5371,6 +5371,7 @@ trees_out::core_bools (tree t)
>        WB (t->type_common.lang_flag_5);
>        WB (t->type_common.lang_flag_6);
>        WB (t->type_common.typeless_storage);
> +      WB (t->type_common.type_include_flexarray);
>      }
>  
>    if (CODE_CONTAINS_STRUCT (code, TS_DECL_COMMON))
> @@ -5551,6 +5552,7 @@ trees_in::core_bools (tree t)
>        RB (t->type_common.lang_flag_5);
>        RB (t->type_common.lang_flag_6);
>        RB (t->type_common.typeless_storage);
> +      RB (t->type_common.type_include_flexarray);
>      }
>  
>    if (CODE_CONTAINS_STRUCT (code, TS_DECL_COMMON))
> diff --git a/gcc/print-tree.cc b/gcc/print-tree.cc
> index 1f3afcbbc86..efacdb7686f 100644
> --- a/gcc/print-tree.cc
> +++ b/gcc/print-tree.cc
> @@ -631,6 +631,11 @@ print_node (FILE *file, const char *prefix, tree node, int indent,
>  	  && TYPE_CXX_ODR_P (node))
>  	fputs (" cxx-odr-p", file);
>  
> +      if ((code == RECORD_TYPE
> +	   || code == UNION_TYPE)
> +	  && TYPE_INCLUDE_FLEXARRAY (node))
> +	fputs (" include-flexarray", file);
> +
>        /* The transparent-union flag is used for different things in
>  	 different nodes.  */
>        if ((code == UNION_TYPE || code == RECORD_TYPE)
> diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c b/gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c
> new file mode 100644
> index 00000000000..60078e11634
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c
> @@ -0,0 +1,134 @@
> +/* PR 101832: 
> +   GCC extension accepts the case when a struct with a C99 flexible array
> +   member is embedded into another struct (possibly recursively).
> +   __builtin_object_size will treat such struct as flexible size.
> +   However, when a structure with non-C99 flexible array member, i.e, trailing
> +   [0], [1], or [4], is embedded into anther struct, the stucture will not
> +   be treated as flexible size.  */ 
> +/* { dg-do run } */
> +/* { dg-options "-O2" } */
> +
> +#include "builtin-object-size-common.h"
> +
> +#define expect(p, _v) do { \
> +  size_t v = _v; \
> +  if (p == v) \
> +    __builtin_printf ("ok:  %s == %zd\n", #p, p); \
> +  else {\
> +    __builtin_printf ("WAT: %s == %zd (expected %zd)\n", #p, p, v); \
> +    FAIL (); \
> +  } \
> +} while (0);
> +
> +
> +struct A {
> +  int n;
> +  char data[];
> +};
> +
> +struct B {
> +  int m;
> +  struct A a;
> +};
> +
> +struct C {
> +  int q;
> +  struct B b;
> +};
> +
> +struct A0 {
> +  int n;
> +  char data[0];
> +};
> +
> +struct B0 {
> +  int m;
> +  struct A0 a;
> +};
> +
> +struct C0 {
> +  int q;
> +  struct B0 b;
> +};
> +
> +struct A1 {
> +  int n;
> +  char data[1];
> +};
> +
> +struct B1 {
> +  int m;
> +  struct A1 a;
> +};
> +
> +struct C1 {
> +  int q;
> +  struct B1 b;
> +};
> +
> +struct An {
> +  int n;
> +  char data[8];
> +};
> +
> +struct Bn {
> +  int m;
> +  struct An a;
> +};
> +
> +struct Cn {
> +  int q;
> +  struct Bn b;
> +};
> +
> +volatile void *magic1, *magic2;
> +
> +int main (int argc, char *argv[])
> +{
> +  struct B *outer;
> +  struct C *outest;
> +
> +  /* Make sure optimization can't find some other object size. */
> +  outer = (void *)magic1;
> +  outest = (void *)magic2;
> +
> +  expect (__builtin_object_size (&outer->a, 1), -1);
> +  expect (__builtin_object_size (&outest->b, 1), -1);
> +  expect (__builtin_object_size (&outest->b.a, 1), -1);
> +
> +  struct B0 *outer0;
> +  struct C0 *outest0;
> +
> +  /* Make sure optimization can't find some other object size. */
> +  outer0 = (void *)magic1;
> +  outest0 = (void *)magic2;
> +
> +  expect (__builtin_object_size (&outer0->a, 1), sizeof (outer0->a));
> +  expect (__builtin_object_size (&outest0->b, 1), sizeof (outest0->b));
> +  expect (__builtin_object_size (&outest0->b.a, 1), sizeof (outest0->b.a));
> +
> +  struct B1 *outer1;
> +  struct C1 *outest1;
> +
> +  /* Make sure optimization can't find some other object size. */
> +  outer1 = (void *)magic1;
> +  outest1 = (void *)magic2;
> +
> +  expect (__builtin_object_size (&outer1->a, 1), sizeof (outer1->a));
> +  expect (__builtin_object_size (&outest1->b, 1), sizeof (outest1->b));
> +  expect (__builtin_object_size (&outest1->b.a, 1), sizeof (outest1->b.a));
> +
> +  struct Bn *outern;
> +  struct Cn *outestn;
> +
> +  /* Make sure optimization can't find some other object size. */
> +  outern = (void *)magic1;
> +  outestn = (void *)magic2;
> +
> +  expect (__builtin_object_size (&outern->a, 1), sizeof (outern->a));
> +  expect (__builtin_object_size (&outestn->b, 1), sizeof (outestn->b));
> +  expect (__builtin_object_size (&outestn->b.a, 1), sizeof (outestn->b.a));
> +
> +  DONE ();
> +  return 0;
> +}
> diff --git a/gcc/tree-core.h b/gcc/tree-core.h
> index acd8deea34e..705d5702b9c 100644
> --- a/gcc/tree-core.h
> +++ b/gcc/tree-core.h
> @@ -1718,7 +1718,9 @@ struct GTY(()) tree_type_common {
>    unsigned empty_flag : 1;
>    unsigned indivisible_p : 1;
>    unsigned no_named_args_stdarg_p : 1;
> -  unsigned spare : 15;
> +  /* TYPE_INCLUDE_FLEXARRAY flag for RECORD_TYPE and UNION_TYPE.  */
> +  unsigned int type_include_flexarray : 1;
> +  unsigned spare : 14;

it looks like the bit could be eventually shared with
no_named_args_stdarg_p (but its accessor doesn't use
FUNC_OR_METHOD_CHECK)

>    alias_set_type alias_set;
>    tree pointer_to;
> diff --git a/gcc/tree-object-size.cc b/gcc/tree-object-size.cc
> index 9a936a91983..22b3c72ea6e 100644
> --- a/gcc/tree-object-size.cc
> +++ b/gcc/tree-object-size.cc
> @@ -633,45 +633,68 @@ addr_object_size (struct object_size_info *osi, const_tree ptr,
>  		    v = NULL_TREE;
>  		    break;
>  		  case COMPONENT_REF:
> -		    if (TREE_CODE (TREE_TYPE (v)) != ARRAY_TYPE)
> +		    /* When the ref is not to an array, a record or a union, it
> +		       will not have flexible size, compute the object size
> +		       directly.  */
> +		    if ((TREE_CODE (TREE_TYPE (v)) != ARRAY_TYPE)
> +			&& (TREE_CODE (TREE_TYPE (v)) != RECORD_TYPE)
> +			&& (TREE_CODE (TREE_TYPE (v)) != UNION_TYPE))
>  		      {
>  			v = NULL_TREE;
>  			break;
>  		      }
> -		    is_flexible_array_mem_ref = array_ref_flexible_size_p (v);
> -		    while (v != pt_var && TREE_CODE (v) == COMPONENT_REF)
> -		      if (TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
> -			  != UNION_TYPE
> -			  && TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
> -			  != QUAL_UNION_TYPE)
> -			break;
> -		      else
> -			v = TREE_OPERAND (v, 0);
> -		    if (TREE_CODE (v) == COMPONENT_REF
> -			&& TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
> -			   == RECORD_TYPE)
> +		    if (TREE_CODE (TREE_TYPE (v)) == RECORD_TYPE
> +			|| TREE_CODE (TREE_TYPE (v)) == UNION_TYPE)
> +		    /* if the record or union does not include a flexible array
> +		       recursively, compute the object size directly.  */
>  		      {
> -			/* compute object size only if v is not a
> -			   flexible array member.  */
> -			if (!is_flexible_array_mem_ref)
> +			if (!TYPE_INCLUDE_FLEXARRAY (TREE_TYPE (v)))
>  			  {
>  			    v = NULL_TREE;
>  			    break;
>  			  }
> -			v = TREE_OPERAND (v, 0);
> +			else
> +			  v = TREE_OPERAND (v, 0);
>  		      }
> -		    while (v != pt_var && TREE_CODE (v) == COMPONENT_REF)
> -		      if (TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
> -			  != UNION_TYPE
> -			  && TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
> -			  != QUAL_UNION_TYPE)
> -			break;
> -		      else
> -			v = TREE_OPERAND (v, 0);
> -		    if (v != pt_var)
> -		      v = NULL_TREE;
>  		    else
> -		      v = pt_var;
> +		      {
> +			/* Now the ref is to an array type.  */
> +			is_flexible_array_mem_ref
> +			  = array_ref_flexible_size_p (v);
> +			while (v != pt_var && TREE_CODE (v) == COMPONENT_REF)
> +			if (TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
> +			      != UNION_TYPE
> +			    && TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
> +				 != QUAL_UNION_TYPE)
> +			  break;
> +			else
> +			  v = TREE_OPERAND (v, 0);
> +			if (TREE_CODE (v) == COMPONENT_REF
> +			    && TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
> +				 == RECORD_TYPE)
> +			  {
> +			    /* compute object size only if v is not a
> +			       flexible array member.  */
> +			    if (!is_flexible_array_mem_ref)
> +			      {
> +				v = NULL_TREE;
> +				break;
> +			      }
> +			    v = TREE_OPERAND (v, 0);
> +			  }
> +			while (v != pt_var && TREE_CODE (v) == COMPONENT_REF)
> +			  if (TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
> +				!= UNION_TYPE
> +			      && TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
> +				   != QUAL_UNION_TYPE)
> +			    break;
> +			  else
> +			    v = TREE_OPERAND (v, 0);
> +			if (v != pt_var)
> +			  v = NULL_TREE;
> +			else
> +			  v = pt_var;
> +		      }
>  		    break;
>  		  default:
>  		    v = pt_var;
> diff --git a/gcc/tree-streamer-in.cc b/gcc/tree-streamer-in.cc
> index d4dc30f048f..c19ede0631d 100644
> --- a/gcc/tree-streamer-in.cc
> +++ b/gcc/tree-streamer-in.cc
> @@ -390,6 +390,7 @@ unpack_ts_type_common_value_fields (struct bitpack_d *bp, tree expr)
>        TYPE_TRANSPARENT_AGGR (expr) = (unsigned) bp_unpack_value (bp, 1);
>        TYPE_FINAL_P (expr) = (unsigned) bp_unpack_value (bp, 1);
>        TYPE_CXX_ODR_P (expr) = (unsigned) bp_unpack_value (bp, 1);
> +      TYPE_INCLUDE_FLEXARRAY (expr) = (unsigned) bp_unpack_value (bp, 1);
>      }
>    else if (TREE_CODE (expr) == ARRAY_TYPE)
>      TYPE_NONALIASED_COMPONENT (expr) = (unsigned) bp_unpack_value (bp, 1);
> diff --git a/gcc/tree-streamer-out.cc b/gcc/tree-streamer-out.cc
> index d107229da5c..73e4b4e547c 100644
> --- a/gcc/tree-streamer-out.cc
> +++ b/gcc/tree-streamer-out.cc
> @@ -357,6 +357,7 @@ pack_ts_type_common_value_fields (struct bitpack_d *bp, tree expr)
>        bp_pack_value (bp, flag_wpa && TYPE_CANONICAL (expr)
>  			 ? TYPE_CXX_ODR_P (TYPE_CANONICAL (expr))
>  			 : TYPE_CXX_ODR_P (expr), 1);
> +      bp_pack_value (bp, TYPE_INCLUDE_FLEXARRAY (expr), 1);
>      }
>    else if (TREE_CODE (expr) == ARRAY_TYPE)
>      bp_pack_value (bp, TYPE_NONALIASED_COMPONENT (expr), 1);
> diff --git a/gcc/tree.h b/gcc/tree.h
> index 92ac0e6a214..ab1cdc3dc85 100644
> --- a/gcc/tree.h
> +++ b/gcc/tree.h
> @@ -778,6 +778,12 @@ extern void omp_clause_range_check_failed (const_tree, const char *, int,
>  #define TYPE_NO_NAMED_ARGS_STDARG_P(NODE) \
>    (TYPE_CHECK (NODE)->type_common.no_named_args_stdarg_p)
>  
> +/* True if this RECORD_TYPE or UNION_TYPE includes a flexible array member
> +   at the last field recursively.  */
> +#define TYPE_INCLUDE_FLEXARRAY(NODE) \
> +  (TYPE_CHECK (NODE)->type_common.type_include_flexarray)

Please use RECORD_OR_UNION_CHECK.  The comment "at the last field"
doesn't seem to match implementation? (at least not obviously)
Given this is a generic header expanding on what a "flexible array member"
is to the middle-end here would be good.  Especially the guarantees
made when the bit is _not_ set (we've evaded that with DECL_NOT_FLEXARRAY
vs. DECL_FLEXARRAY).

Thanks,
Richard.

Re: [v4][PATCH 1/2] Handle component_ref to a structre/union field including C99 FAM [PR101832]

[Qing Zhao]

> On Mar 9, 2023, at 7:20 AM, Richard Biener <rguenther@suse.de> wrote:
> 
> On Fri, 24 Feb 2023, Qing Zhao wrote:
> 
>> GCC extension accepts the case when a struct with a C99 flexible array member
>> is embedded into another struct or union (possibly recursively).
>> __builtin_object_size should treat such struct as flexible size.
>> 
>> gcc/c/ChangeLog:
>> 
>> 	PR tree-optimization/101832
>> 	* c-decl.cc (finish_struct): Set TYPE_INCLUDE_FLEXARRAY for
>> 	struct/union type.
> 
> I can't really comment on the correctness of this part but since
> only the C frontend will ever set this and you are using it from
> addr_object_size which is also used for other C family languages
> (at least), I wonder if you can really test
> 
> +                       if (!TYPE_INCLUDE_FLEXARRAY (TREE_TYPE (v)))
> 
> there.

You mean for C++ and also other C family languages (other than C), the above bit cannot be set?
Yes, that’s true. The bit is only set for C. So is the bit DECL_NOT_FLEXARRAY, which is only set for C too. 
So, I am wondering whether the bit DECL_NOT_FLEXARRAY should be also set in middle end? Or we can set DECL_NOT_FLEXARRAY in C++ FE too? And then we can set TYPE_INCLUDE_FLEXARRAY also in C++ FE?
What’s your suggestion?

(I personally feel that DECL_NOT_FLEXARRAY and TYPE_INCLUDE_FLEXARRAY should be set in the same places).

> 
> Originally I was suggesting to set this flag in stor-layout.cc
> which eventually all languages funnel their types through and
> if there's language specific handling use a langhook (with the
> default implementation preserving the status quo).

If we decide to set the bits in stor-layout.cc, where is the best place to do it? I checked the star-layout.cc code, looks like “layout_type” might be the place where we can set these bits for RECORD_TYPE, UNION_TYPE? 
> 
> Some more comments below ...
> 
>> gcc/cp/ChangeLog:
>> 
>> 	PR tree-optimization/101832
>> 	* module.cc (trees_out::core_bools): Stream out new bit
>> 	type_include_flexarray.
>> 	(trees_in::core_bools): Stream in new bit type_include_flexarray.
>> 
>> gcc/ChangeLog:
>> 
>> 	PR tree-optimization/101832
>> 	* print-tree.cc (print_node): Print new bit type_include_flexarray.
>> 	* tree-core.h (struct tree_type_common): New bit
>> 	type_include_flexarray.
>> 	* tree-object-size.cc (addr_object_size): Handle structure/union type
>> 	when it has flexible size.
>> 	* tree-streamer-in.cc (unpack_ts_type_common_value_fields): Stream
>> 	in new bit type_include_flexarray.
>> 	* tree-streamer-out.cc (pack_ts_type_common_value_fields): Stream
>> 	out new bit type_include_flexarray.
>> 	* tree.h (TYPE_INCLUDE_FLEXARRAY): New macro
>> 	TYPE_INCLUDE_FLEXARRAY.
>> 
>> gcc/testsuite/ChangeLog:
>> 
>> 	PR tree-optimization/101832
>> 	* gcc.dg/builtin-object-size-pr101832.c: New test.
>> ---
>> gcc/c/c-decl.cc                               |  12 ++
>> gcc/cp/module.cc                              |   2 +
>> gcc/print-tree.cc                             |   5 +
>> .../gcc.dg/builtin-object-size-pr101832.c     | 134 ++++++++++++++++++
>> gcc/tree-core.h                               |   4 +-
>> gcc/tree-object-size.cc                       |  79 +++++++----
>> gcc/tree-streamer-in.cc                       |   1 +
>> gcc/tree-streamer-out.cc                      |   1 +
>> gcc/tree.h                                    |   6 +
>> 9 files changed, 215 insertions(+), 29 deletions(-)
>> create mode 100644 gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c
>> 
>> diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
>> index 08078eadeb8..f589a2f5192 100644
>> --- a/gcc/c/c-decl.cc
>> +++ b/gcc/c/c-decl.cc
>> @@ -9284,6 +9284,18 @@ finish_struct (location_t loc, tree t, tree fieldlist, tree attributes,
>>       /* Set DECL_NOT_FLEXARRAY flag for FIELD_DECL x.  */
>>       DECL_NOT_FLEXARRAY (x) = !is_flexible_array_member_p (is_last_field, x);
>> 
>> +      /* Set TYPE_INCLUDE_FLEXARRAY for the context of x, t
>> +       * when x is an array.  */
>> +      if (TREE_CODE (TREE_TYPE (x)) == ARRAY_TYPE)
>> +	TYPE_INCLUDE_FLEXARRAY (t) = flexible_array_member_type_p (TREE_TYPE (x)) ;
>> +      /* Recursively set TYPE_INCLUDE_FLEXARRAY for the context of x, t
>> +	 when x is the last field.  */
>> +      else if ((TREE_CODE (TREE_TYPE (x)) == RECORD_TYPE
>> +		|| TREE_CODE (TREE_TYPE (x)) == UNION_TYPE)
>> +	       && TYPE_INCLUDE_FLEXARRAY (TREE_TYPE (x))
>> +	       && is_last_field)
>> +	TYPE_INCLUDE_FLEXARRAY (t) = true;
>> +
>>       if (DECL_NAME (x)
>> 	  || RECORD_OR_UNION_TYPE_P (TREE_TYPE (x)))
>> 	saw_named_field = true;
>> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
>> index ac2fe66b080..c750361b704 100644
>> --- a/gcc/cp/module.cc
>> +++ b/gcc/cp/module.cc
>> @@ -5371,6 +5371,7 @@ trees_out::core_bools (tree t)
>>       WB (t->type_common.lang_flag_5);
>>       WB (t->type_common.lang_flag_6);
>>       WB (t->type_common.typeless_storage);
>> +      WB (t->type_common.type_include_flexarray);
>>     }
>> 
>>   if (CODE_CONTAINS_STRUCT (code, TS_DECL_COMMON))
>> @@ -5551,6 +5552,7 @@ trees_in::core_bools (tree t)
>>       RB (t->type_common.lang_flag_5);
>>       RB (t->type_common.lang_flag_6);
>>       RB (t->type_common.typeless_storage);
>> +      RB (t->type_common.type_include_flexarray);
>>     }
>> 
>>   if (CODE_CONTAINS_STRUCT (code, TS_DECL_COMMON))
>> diff --git a/gcc/print-tree.cc b/gcc/print-tree.cc
>> index 1f3afcbbc86..efacdb7686f 100644
>> --- a/gcc/print-tree.cc
>> +++ b/gcc/print-tree.cc
>> @@ -631,6 +631,11 @@ print_node (FILE *file, const char *prefix, tree node, int indent,
>> 	  && TYPE_CXX_ODR_P (node))
>> 	fputs (" cxx-odr-p", file);
>> 
>> +      if ((code == RECORD_TYPE
>> +	   || code == UNION_TYPE)
>> +	  && TYPE_INCLUDE_FLEXARRAY (node))
>> +	fputs (" include-flexarray", file);
>> +
>>       /* The transparent-union flag is used for different things in
>> 	 different nodes.  */
>>       if ((code == UNION_TYPE || code == RECORD_TYPE)
>> diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c b/gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c
>> new file mode 100644
>> index 00000000000..60078e11634
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c
>> @@ -0,0 +1,134 @@
>> +/* PR 101832: 
>> +   GCC extension accepts the case when a struct with a C99 flexible array
>> +   member is embedded into another struct (possibly recursively).
>> +   __builtin_object_size will treat such struct as flexible size.
>> +   However, when a structure with non-C99 flexible array member, i.e, trailing
>> +   [0], [1], or [4], is embedded into anther struct, the stucture will not
>> +   be treated as flexible size.  */ 
>> +/* { dg-do run } */
>> +/* { dg-options "-O2" } */
>> +
>> +#include "builtin-object-size-common.h"
>> +
>> +#define expect(p, _v) do { \
>> +  size_t v = _v; \
>> +  if (p == v) \
>> +    __builtin_printf ("ok:  %s == %zd\n", #p, p); \
>> +  else {\
>> +    __builtin_printf ("WAT: %s == %zd (expected %zd)\n", #p, p, v); \
>> +    FAIL (); \
>> +  } \
>> +} while (0);
>> +
>> +
>> +struct A {
>> +  int n;
>> +  char data[];
>> +};
>> +
>> +struct B {
>> +  int m;
>> +  struct A a;
>> +};
>> +
>> +struct C {
>> +  int q;
>> +  struct B b;
>> +};
>> +
>> +struct A0 {
>> +  int n;
>> +  char data[0];
>> +};
>> +
>> +struct B0 {
>> +  int m;
>> +  struct A0 a;
>> +};
>> +
>> +struct C0 {
>> +  int q;
>> +  struct B0 b;
>> +};
>> +
>> +struct A1 {
>> +  int n;
>> +  char data[1];
>> +};
>> +
>> +struct B1 {
>> +  int m;
>> +  struct A1 a;
>> +};
>> +
>> +struct C1 {
>> +  int q;
>> +  struct B1 b;
>> +};
>> +
>> +struct An {
>> +  int n;
>> +  char data[8];
>> +};
>> +
>> +struct Bn {
>> +  int m;
>> +  struct An a;
>> +};
>> +
>> +struct Cn {
>> +  int q;
>> +  struct Bn b;
>> +};
>> +
>> +volatile void *magic1, *magic2;
>> +
>> +int main (int argc, char *argv[])
>> +{
>> +  struct B *outer;
>> +  struct C *outest;
>> +
>> +  /* Make sure optimization can't find some other object size. */
>> +  outer = (void *)magic1;
>> +  outest = (void *)magic2;
>> +
>> +  expect (__builtin_object_size (&outer->a, 1), -1);
>> +  expect (__builtin_object_size (&outest->b, 1), -1);
>> +  expect (__builtin_object_size (&outest->b.a, 1), -1);
>> +
>> +  struct B0 *outer0;
>> +  struct C0 *outest0;
>> +
>> +  /* Make sure optimization can't find some other object size. */
>> +  outer0 = (void *)magic1;
>> +  outest0 = (void *)magic2;
>> +
>> +  expect (__builtin_object_size (&outer0->a, 1), sizeof (outer0->a));
>> +  expect (__builtin_object_size (&outest0->b, 1), sizeof (outest0->b));
>> +  expect (__builtin_object_size (&outest0->b.a, 1), sizeof (outest0->b.a));
>> +
>> +  struct B1 *outer1;
>> +  struct C1 *outest1;
>> +
>> +  /* Make sure optimization can't find some other object size. */
>> +  outer1 = (void *)magic1;
>> +  outest1 = (void *)magic2;
>> +
>> +  expect (__builtin_object_size (&outer1->a, 1), sizeof (outer1->a));
>> +  expect (__builtin_object_size (&outest1->b, 1), sizeof (outest1->b));
>> +  expect (__builtin_object_size (&outest1->b.a, 1), sizeof (outest1->b.a));
>> +
>> +  struct Bn *outern;
>> +  struct Cn *outestn;
>> +
>> +  /* Make sure optimization can't find some other object size. */
>> +  outern = (void *)magic1;
>> +  outestn = (void *)magic2;
>> +
>> +  expect (__builtin_object_size (&outern->a, 1), sizeof (outern->a));
>> +  expect (__builtin_object_size (&outestn->b, 1), sizeof (outestn->b));
>> +  expect (__builtin_object_size (&outestn->b.a, 1), sizeof (outestn->b.a));
>> +
>> +  DONE ();
>> +  return 0;
>> +}
>> diff --git a/gcc/tree-core.h b/gcc/tree-core.h
>> index acd8deea34e..705d5702b9c 100644
>> --- a/gcc/tree-core.h
>> +++ b/gcc/tree-core.h
>> @@ -1718,7 +1718,9 @@ struct GTY(()) tree_type_common {
>>   unsigned empty_flag : 1;
>>   unsigned indivisible_p : 1;
>>  RECORD_OR_UNION_CHECK
> 
> it looks like the bit could be eventually shared with
> no_named_args_stdarg_p (but its accessor doesn't use
> FUNC_OR_METHOD_CHECK)
You mean to let “type_include_flexarray” share the same bit with “no_named_args_stdarg_p” in order to save 1-bit space in IR?
Then change the 

/* True if this is a stdarg function with no named arguments (C2x
   (...) prototype, where arguments can be accessed with va_start and
   va_arg), as opposed to an unprototyped function.  */
#define TYPE_NO_NAMED_ARGS_STDARG_P(NODE) \
  (TYPE_CHECK (NODE)->type_common.no_named_args_stdarg_p)

/* True if this RECORD_TYPE or UNION_TYPE includes a flexible array member
   at the last field recursively.  */
#define TYPE_INCLUDE_FLEXARRAY(NODE) \
  (TYPE_CHECK (NODE)->type_common.type_include_flexarray)

To:

union {
    unsigned no_named_args_stdarg_p : 1;
    /* TYPE_INCLUDE_FLEXARRAY flag for RECORD_TYPE and UNION_TYPE.  */
    unsigned int type_include_flexarray : 1;
  } shared_bit;
  unsigned spare : 15;

/* True if this is a stdarg function with no named arguments (C2x
   (...) prototype, where arguments can be accessed with va_start and
   va_arg), as opposed to an unprototyped function.  */
#define TYPE_NO_NAMED_ARGS_STDARG_P(NODE) \
  (FUNC_OR_METHOD_CHECK (NODE)->type_common.shared_bit.no_named_args_stdarg_p)

/* True if this RECORD_TYPE or UNION_TYPE includes a flexible array member
   at the last field recursively.  */
#define TYPE_INCLUDE_FLEXARRAY(NODE) \
  (RECORD_OR_UNION_CHECK(NODE)->type_common.shared_bit.type_include_flexarray)

> 
>>   alias_set_type alias_set;
>>   tree pointer_to;
>> diff --git a/gcc/tree-object-size.cc b/gcc/tree-object-size.cc
>> index 9a936a91983..22b3c72ea6e 100644
>> --- a/gcc/tree-object-size.cc
>> +++ b/gcc/tree-object-size.cc
>> @@ -633,45 +633,68 @@ addr_object_size (struct object_size_info *osi, const_tree ptr,
>> 		    v = NULL_TREE;
>> 		    break;
>> 		  case COMPONENT_REF:
>> -		    if (TREE_CODE (TREE_TYPE (v)) != ARRAY_TYPE)
>> +		    /* When the ref is not to an array, a record or a union, it
>> +		       will not have flexible size, compute the object size
>> +		       directly.  */
>> +		    if ((TREE_CODE (TREE_TYPE (v)) != ARRAY_TYPE)
>> +			&& (TREE_CODE (TREE_TYPE (v)) != RECORD_TYPE)
>> +			&& (TREE_CODE (TREE_TYPE (v)) != UNION_TYPE))
>> 		      {
>> 			v = NULL_TREE;
>> 			break;
>> 		      }
>> -		    is_flexible_array_mem_ref = array_ref_flexible_size_p (v);
>> -		    while (v != pt_var && TREE_CODE (v) == COMPONENT_REF)
>> -		      if (TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
>> -			  != UNION_TYPE
>> -			  && TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
>> -			  != QUAL_UNION_TYPE)
>> -			break;
>> -		      else
>> -			v = TREE_OPERAND (v, 0);
>> -		    if (TREE_CODE (v) == COMPONENT_REF
>> -			&& TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
>> -			   == RECORD_TYPE)
>> +		    if (TREE_CODE (TREE_TYPE (v)) == RECORD_TYPE
>> +			|| TREE_CODE (TREE_TYPE (v)) == UNION_TYPE)
>> +		    /* if the record or union does not include a flexible array
>> +		       recursively, compute the object size directly.  */
>> 		      {
>> -			/* compute object size only if v is not a
>> -			   flexible array member.  */
>> -			if (!is_flexible_array_mem_ref)
>> +			if (!TYPE_INCLUDE_FLEXARRAY (TREE_TYPE (v)))
>> 			  {
>> 			    v = NULL_TREE;
>> 			    break;
>> 			  }
>> -			v = TREE_OPERAND (v, 0);
>> +			else
>> +			  v = TREE_OPERAND (v, 0);
>> 		      }
>> -		    while (v != pt_var && TREE_CODE (v) == COMPONENT_REF)
>> -		      if (TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
>> -			  != UNION_TYPE
>> -			  && TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
>> -			  != QUAL_UNION_TYPE)
>> -			break;
>> -		      else
>> -			v = TREE_OPERAND (v, 0);
>> -		    if (v != pt_var)
>> -		      v = NULL_TREE;
>> 		    else
>> -		      v = pt_var;
>> +		      {
>> +			/* Now the ref is to an array type.  */
>> +			is_flexible_array_mem_ref
>> +			  = array_ref_flexible_size_p (v);
>> +			while (v != pt_var && TREE_CODE (v) == COMPONENT_REF)
>> +			if (TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
>> +			      != UNION_TYPE
>> +			    && TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
>> +				 != QUAL_UNION_TYPE)
>> +			  break;
>> +			else
>> +			  v = TREE_OPERAND (v, 0);
>> +			if (TREE_CODE (v) == COMPONENT_REF
>> +			    && TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
>> +				 == RECORD_TYPE)
>> +			  {
>> +			    /* compute object size only if v is not a
>> +			       flexible array member.  */
>> +			    if (!is_flexible_array_mem_ref)
>> +			      {
>> +				v = NULL_TREE;
>> +				break;
>> +			      }
>> +			    v = TREE_OPERAND (v, 0);
>> +			  }
>> +			while (v != pt_var && TREE_CODE (v) == COMPONENT_REF)
>> +			  if (TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
>> +				!= UNION_TYPE
>> +			      && TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
>> +				   != QUAL_UNION_TYPE)
>> +			    break;
>> +			  else
>> +			    v = TREE_OPERAND (v, 0);
>> +			if (v != pt_var)
>> +			  v = NULL_TREE;
>> +			else
>> +			  v = pt_var;
>> +		      }
>> 		    break;
>> 		  default:
>> 		    v = pt_var;
>> diff --git a/gcc/tree-streamer-in.cc b/gcc/tree-streamer-in.cc
>> index d4dc30f048f..c19ede0631d 100644
>> --- a/gcc/tree-streamer-in.cc
>> +++ b/gcc/tree-streamer-in.cc
>> @@ -390,6 +390,7 @@ unpack_ts_type_common_value_fields (struct bitpack_d *bp, tree expr)
>>       TYPE_TRANSPARENT_AGGR (expr) = (unsigned) bp_unpack_value (bp, 1);
>>       TYPE_FINAL_P (expr) = (unsigned) bp_unpack_value (bp, 1);
>>       TYPE_CXX_ODR_P (expr) = (unsigned) bp_unpack_value (bp, 1);
>> +      TYPE_INCLUDE_FLEXARRAY (expr) = (unsigned) bp_unpack_value (bp, 1);
>>     }
>>   else if (TREE_CODE (expr) == ARRAY_TYPE)
>>     TYPE_NONALIASED_COMPONENT (expr) = (unsigned) bp_unpack_value (bp, 1);
>> diff --git a/gcc/tree-streamer-out.cc b/gcc/tree-streamer-out.cc
>> index d107229da5c..73e4b4e547c 100644
>> --- a/gcc/tree-streamer-out.cc
>> +++ b/gcc/tree-streamer-out.cc
>> @@ -357,6 +357,7 @@ pack_ts_type_common_value_fields (struct bitpack_d *bp, tree expr)
>>       bp_pack_value (bp, flag_wpa && TYPE_CANONICAL (expr)
>> 			 ? TYPE_CXX_ODR_P (TYPE_CANONICAL (expr))
>> 			 : TYPE_CXX_ODR_P (expr), 1);
>> +      bp_pack_value (bp, TYPE_INCLUDE_FLEXARRAY (expr), 1);
>>     }
>>   else if (TREE_CODE (expr) == ARRAY_TYPE)
>>     bp_pack_value (bp, TYPE_NONALIASED_COMPONENT (expr), 1);
>> diff --git a/gcc/tree.h b/gcc/tree.h
>> index 92ac0e6a214..ab1cdc3dc85 100644
>> --- a/gcc/tree.h
>> +++ b/gcc/tree.h
>> @@ -778,6 +778,12 @@ extern void omp_clause_range_check_failed (const_tree, const char *, int,
>> #define TYPE_NO_NAMED_ARGS_STDARG_P(NODE) \
>>   (TYPE_CHECK (NODE)->type_common.no_named_args_stdarg_p)
>> 
>> +/* True if this RECORD_TYPE or UNION_TYPE includes a flexible array member
>> +   at the last field recursively.  */
>> +#define TYPE_INCLUDE_FLEXARRAY(NODE) \
>> +  (TYPE_CHECK (NODE)->type_common.type_include_flexarray)
> 
> Please use RECORD_OR_UNION_CHECK.  
Okay. 
> The comment "at the last field"
> doesn't seem to match implementation? (at least not obviously)
The current implementation (in gcc/c/c-decl.cc) guarantees that it’s at the last field. (And recursively). 
> Given this is a generic header expanding on what a "flexible array member"
> is to the middle-end here would be good.  Especially the guarantees
> made when the bit is _not_ set (we've evaded that with DECL_NOT_FLEXARRAY
> vs. DECL_FLEXARRAY).

The bit TYPE_INCLUDE_FLEXARRAY default is FALSE, i.e, not including a flex array by default.
It’s only set to TRUE when the struct/union includes a flex array at the last field (and recursively). So, I think it should be correct. 
Let me know if I missed anything here.

Thanks a lot for your comments.

Qing
> 
> Thanks,
> Richard.

Re: [v4][PATCH 1/2] Handle component_ref to a structre/union field including C99 FAM [PR101832]

[Richard Biener]

[-- Attachment #1: Type: text/plain, Size: 19308 bytes --]

On Thu, 9 Mar 2023, Qing Zhao wrote:

> 
> 
> > On Mar 9, 2023, at 7:20 AM, Richard Biener <rguenther@suse.de> wrote:
> > 
> > On Fri, 24 Feb 2023, Qing Zhao wrote:
> > 
> >> GCC extension accepts the case when a struct with a C99 flexible array member
> >> is embedded into another struct or union (possibly recursively).
> >> __builtin_object_size should treat such struct as flexible size.
> >> 
> >> gcc/c/ChangeLog:
> >> 
> >> 	PR tree-optimization/101832
> >> 	* c-decl.cc (finish_struct): Set TYPE_INCLUDE_FLEXARRAY for
> >> 	struct/union type.
> > 
> > I can't really comment on the correctness of this part but since
> > only the C frontend will ever set this and you are using it from
> > addr_object_size which is also used for other C family languages
> > (at least), I wonder if you can really test
> > 
> > +                       if (!TYPE_INCLUDE_FLEXARRAY (TREE_TYPE (v)))
> > 
> > there.
> 
> You mean for C++ and also other C family languages (other than C), the above bit cannot be set?
> Yes, that’s true. The bit is only set for C. So is the bit DECL_NOT_FLEXARRAY, which is only set for C too. 
> So, I am wondering whether the bit DECL_NOT_FLEXARRAY should be also set in middle end? Or we can set DECL_NOT_FLEXARRAY in C++ FE too? And then we can set TYPE_INCLUDE_FLEXARRAY also in C++ FE?
> What’s your suggestion?
> 
> (I personally feel that DECL_NOT_FLEXARRAY and TYPE_INCLUDE_FLEXARRAY should be set in the same places).

I was wondering if the above test errors on the conservative side
correctly - it will now, for all but C, cut off some thing where it
didn't before?

> > 
> > Originally I was suggesting to set this flag in stor-layout.cc
> > which eventually all languages funnel their types through and
> > if there's language specific handling use a langhook (with the
> > default implementation preserving the status quo).
> 
> If we decide to set the bits in stor-layout.cc, where is the best place to do it? I checked the star-layout.cc code, looks like “layout_type” might be the place where we can set these bits for RECORD_TYPE, UNION_TYPE? 

Yes, it would be layout_type.

> > 
> > Some more comments below ...
> > 
> >> gcc/cp/ChangeLog:
> >> 
> >> 	PR tree-optimization/101832
> >> 	* module.cc (trees_out::core_bools): Stream out new bit
> >> 	type_include_flexarray.
> >> 	(trees_in::core_bools): Stream in new bit type_include_flexarray.
> >> 
> >> gcc/ChangeLog:
> >> 
> >> 	PR tree-optimization/101832
> >> 	* print-tree.cc (print_node): Print new bit type_include_flexarray.
> >> 	* tree-core.h (struct tree_type_common): New bit
> >> 	type_include_flexarray.
> >> 	* tree-object-size.cc (addr_object_size): Handle structure/union type
> >> 	when it has flexible size.
> >> 	* tree-streamer-in.cc (unpack_ts_type_common_value_fields): Stream
> >> 	in new bit type_include_flexarray.
> >> 	* tree-streamer-out.cc (pack_ts_type_common_value_fields): Stream
> >> 	out new bit type_include_flexarray.
> >> 	* tree.h (TYPE_INCLUDE_FLEXARRAY): New macro
> >> 	TYPE_INCLUDE_FLEXARRAY.
> >> 
> >> gcc/testsuite/ChangeLog:
> >> 
> >> 	PR tree-optimization/101832
> >> 	* gcc.dg/builtin-object-size-pr101832.c: New test.
> >> ---
> >> gcc/c/c-decl.cc                               |  12 ++
> >> gcc/cp/module.cc                              |   2 +
> >> gcc/print-tree.cc                             |   5 +
> >> .../gcc.dg/builtin-object-size-pr101832.c     | 134 ++++++++++++++++++
> >> gcc/tree-core.h                               |   4 +-
> >> gcc/tree-object-size.cc                       |  79 +++++++----
> >> gcc/tree-streamer-in.cc                       |   1 +
> >> gcc/tree-streamer-out.cc                      |   1 +
> >> gcc/tree.h                                    |   6 +
> >> 9 files changed, 215 insertions(+), 29 deletions(-)
> >> create mode 100644 gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c
> >> 
> >> diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
> >> index 08078eadeb8..f589a2f5192 100644
> >> --- a/gcc/c/c-decl.cc
> >> +++ b/gcc/c/c-decl.cc
> >> @@ -9284,6 +9284,18 @@ finish_struct (location_t loc, tree t, tree fieldlist, tree attributes,
> >>       /* Set DECL_NOT_FLEXARRAY flag for FIELD_DECL x.  */
> >>       DECL_NOT_FLEXARRAY (x) = !is_flexible_array_member_p (is_last_field, x);
> >> 
> >> +      /* Set TYPE_INCLUDE_FLEXARRAY for the context of x, t
> >> +       * when x is an array.  */
> >> +      if (TREE_CODE (TREE_TYPE (x)) == ARRAY_TYPE)
> >> +	TYPE_INCLUDE_FLEXARRAY (t) = flexible_array_member_type_p (TREE_TYPE (x)) ;
> >> +      /* Recursively set TYPE_INCLUDE_FLEXARRAY for the context of x, t
> >> +	 when x is the last field.  */
> >> +      else if ((TREE_CODE (TREE_TYPE (x)) == RECORD_TYPE
> >> +		|| TREE_CODE (TREE_TYPE (x)) == UNION_TYPE)
> >> +	       && TYPE_INCLUDE_FLEXARRAY (TREE_TYPE (x))
> >> +	       && is_last_field)
> >> +	TYPE_INCLUDE_FLEXARRAY (t) = true;
> >> +
> >>       if (DECL_NAME (x)
> >> 	  || RECORD_OR_UNION_TYPE_P (TREE_TYPE (x)))
> >> 	saw_named_field = true;
> >> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> >> index ac2fe66b080..c750361b704 100644
> >> --- a/gcc/cp/module.cc
> >> +++ b/gcc/cp/module.cc
> >> @@ -5371,6 +5371,7 @@ trees_out::core_bools (tree t)
> >>       WB (t->type_common.lang_flag_5);
> >>       WB (t->type_common.lang_flag_6);
> >>       WB (t->type_common.typeless_storage);
> >> +      WB (t->type_common.type_include_flexarray);
> >>     }
> >> 
> >>   if (CODE_CONTAINS_STRUCT (code, TS_DECL_COMMON))
> >> @@ -5551,6 +5552,7 @@ trees_in::core_bools (tree t)
> >>       RB (t->type_common.lang_flag_5);
> >>       RB (t->type_common.lang_flag_6);
> >>       RB (t->type_common.typeless_storage);
> >> +      RB (t->type_common.type_include_flexarray);
> >>     }
> >> 
> >>   if (CODE_CONTAINS_STRUCT (code, TS_DECL_COMMON))
> >> diff --git a/gcc/print-tree.cc b/gcc/print-tree.cc
> >> index 1f3afcbbc86..efacdb7686f 100644
> >> --- a/gcc/print-tree.cc
> >> +++ b/gcc/print-tree.cc
> >> @@ -631,6 +631,11 @@ print_node (FILE *file, const char *prefix, tree node, int indent,
> >> 	  && TYPE_CXX_ODR_P (node))
> >> 	fputs (" cxx-odr-p", file);
> >> 
> >> +      if ((code == RECORD_TYPE
> >> +	   || code == UNION_TYPE)
> >> +	  && TYPE_INCLUDE_FLEXARRAY (node))
> >> +	fputs (" include-flexarray", file);
> >> +
> >>       /* The transparent-union flag is used for different things in
> >> 	 different nodes.  */
> >>       if ((code == UNION_TYPE || code == RECORD_TYPE)
> >> diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c b/gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c
> >> new file mode 100644
> >> index 00000000000..60078e11634
> >> --- /dev/null
> >> +++ b/gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c
> >> @@ -0,0 +1,134 @@
> >> +/* PR 101832: 
> >> +   GCC extension accepts the case when a struct with a C99 flexible array
> >> +   member is embedded into another struct (possibly recursively).
> >> +   __builtin_object_size will treat such struct as flexible size.
> >> +   However, when a structure with non-C99 flexible array member, i.e, trailing
> >> +   [0], [1], or [4], is embedded into anther struct, the stucture will not
> >> +   be treated as flexible size.  */ 
> >> +/* { dg-do run } */
> >> +/* { dg-options "-O2" } */
> >> +
> >> +#include "builtin-object-size-common.h"
> >> +
> >> +#define expect(p, _v) do { \
> >> +  size_t v = _v; \
> >> +  if (p == v) \
> >> +    __builtin_printf ("ok:  %s == %zd\n", #p, p); \
> >> +  else {\
> >> +    __builtin_printf ("WAT: %s == %zd (expected %zd)\n", #p, p, v); \
> >> +    FAIL (); \
> >> +  } \
> >> +} while (0);
> >> +
> >> +
> >> +struct A {
> >> +  int n;
> >> +  char data[];
> >> +};
> >> +
> >> +struct B {
> >> +  int m;
> >> +  struct A a;
> >> +};
> >> +
> >> +struct C {
> >> +  int q;
> >> +  struct B b;
> >> +};
> >> +
> >> +struct A0 {
> >> +  int n;
> >> +  char data[0];
> >> +};
> >> +
> >> +struct B0 {
> >> +  int m;
> >> +  struct A0 a;
> >> +};
> >> +
> >> +struct C0 {
> >> +  int q;
> >> +  struct B0 b;
> >> +};
> >> +
> >> +struct A1 {
> >> +  int n;
> >> +  char data[1];
> >> +};
> >> +
> >> +struct B1 {
> >> +  int m;
> >> +  struct A1 a;
> >> +};
> >> +
> >> +struct C1 {
> >> +  int q;
> >> +  struct B1 b;
> >> +};
> >> +
> >> +struct An {
> >> +  int n;
> >> +  char data[8];
> >> +};
> >> +
> >> +struct Bn {
> >> +  int m;
> >> +  struct An a;
> >> +};
> >> +
> >> +struct Cn {
> >> +  int q;
> >> +  struct Bn b;
> >> +};
> >> +
> >> +volatile void *magic1, *magic2;
> >> +
> >> +int main (int argc, char *argv[])
> >> +{
> >> +  struct B *outer;
> >> +  struct C *outest;
> >> +
> >> +  /* Make sure optimization can't find some other object size. */
> >> +  outer = (void *)magic1;
> >> +  outest = (void *)magic2;
> >> +
> >> +  expect (__builtin_object_size (&outer->a, 1), -1);
> >> +  expect (__builtin_object_size (&outest->b, 1), -1);
> >> +  expect (__builtin_object_size (&outest->b.a, 1), -1);
> >> +
> >> +  struct B0 *outer0;
> >> +  struct C0 *outest0;
> >> +
> >> +  /* Make sure optimization can't find some other object size. */
> >> +  outer0 = (void *)magic1;
> >> +  outest0 = (void *)magic2;
> >> +
> >> +  expect (__builtin_object_size (&outer0->a, 1), sizeof (outer0->a));
> >> +  expect (__builtin_object_size (&outest0->b, 1), sizeof (outest0->b));
> >> +  expect (__builtin_object_size (&outest0->b.a, 1), sizeof (outest0->b.a));
> >> +
> >> +  struct B1 *outer1;
> >> +  struct C1 *outest1;
> >> +
> >> +  /* Make sure optimization can't find some other object size. */
> >> +  outer1 = (void *)magic1;
> >> +  outest1 = (void *)magic2;
> >> +
> >> +  expect (__builtin_object_size (&outer1->a, 1), sizeof (outer1->a));
> >> +  expect (__builtin_object_size (&outest1->b, 1), sizeof (outest1->b));
> >> +  expect (__builtin_object_size (&outest1->b.a, 1), sizeof (outest1->b.a));
> >> +
> >> +  struct Bn *outern;
> >> +  struct Cn *outestn;
> >> +
> >> +  /* Make sure optimization can't find some other object size. */
> >> +  outern = (void *)magic1;
> >> +  outestn = (void *)magic2;
> >> +
> >> +  expect (__builtin_object_size (&outern->a, 1), sizeof (outern->a));
> >> +  expect (__builtin_object_size (&outestn->b, 1), sizeof (outestn->b));
> >> +  expect (__builtin_object_size (&outestn->b.a, 1), sizeof (outestn->b.a));
> >> +
> >> +  DONE ();
> >> +  return 0;
> >> +}
> >> diff --git a/gcc/tree-core.h b/gcc/tree-core.h
> >> index acd8deea34e..705d5702b9c 100644
> >> --- a/gcc/tree-core.h
> >> +++ b/gcc/tree-core.h
> >> @@ -1718,7 +1718,9 @@ struct GTY(()) tree_type_common {
> >>   unsigned empty_flag : 1;
> >>   unsigned indivisible_p : 1;
> >>  RECORD_OR_UNION_CHECK
> > 
> > it looks like the bit could be eventually shared with
> > no_named_args_stdarg_p (but its accessor doesn't use
> > FUNC_OR_METHOD_CHECK)
> You mean to let “type_include_flexarray” share the same bit with “no_named_args_stdarg_p” in order to save 1-bit space in IR?
> Then change the 
> 
> /* True if this is a stdarg function with no named arguments (C2x
>    (...) prototype, where arguments can be accessed with va_start and
>    va_arg), as opposed to an unprototyped function.  */
> #define TYPE_NO_NAMED_ARGS_STDARG_P(NODE) \
>   (TYPE_CHECK (NODE)->type_common.no_named_args_stdarg_p)
> 
> /* True if this RECORD_TYPE or UNION_TYPE includes a flexible array member
>    at the last field recursively.  */
> #define TYPE_INCLUDE_FLEXARRAY(NODE) \
>   (TYPE_CHECK (NODE)->type_common.type_include_flexarray)
> 
> To:
> 
> union {
>     unsigned no_named_args_stdarg_p : 1;
>     /* TYPE_INCLUDE_FLEXARRAY flag for RECORD_TYPE and UNION_TYPE.  */
>     unsigned int type_include_flexarray : 1;
>   } shared_bit;
>   unsigned spare : 15;
> 
> /* True if this is a stdarg function with no named arguments (C2x
>    (...) prototype, where arguments can be accessed with va_start and
>    va_arg), as opposed to an unprototyped function.  */
> #define TYPE_NO_NAMED_ARGS_STDARG_P(NODE) \
>   (FUNC_OR_METHOD_CHECK (NODE)->type_common.shared_bit.no_named_args_stdarg_p)
> 
> /* True if this RECORD_TYPE or UNION_TYPE includes a flexible array member
>    at the last field recursively.  */
> #define TYPE_INCLUDE_FLEXARRAY(NODE) \
>   (RECORD_OR_UNION_CHECK(NODE)->type_common.shared_bit.type_include_flexarray)

No, we're just overloading bits by using the same name for different
purposes.  I don't think such a union would pack nicely.  We overload
by documenting the uses, in the above cases the uses are separated
by the kind of the tree node, RECORD/UNION_TYPE vs. FUNCTION/METHOD_TYPE
and the accessor macros should be updated to use the appropriate
tree check macros covering that so we don't try to inspect the
"wrong bit".

> > 
> >>   alias_set_type alias_set;
> >>   tree pointer_to;
> >> diff --git a/gcc/tree-object-size.cc b/gcc/tree-object-size.cc
> >> index 9a936a91983..22b3c72ea6e 100644
> >> --- a/gcc/tree-object-size.cc
> >> +++ b/gcc/tree-object-size.cc
> >> @@ -633,45 +633,68 @@ addr_object_size (struct object_size_info *osi, const_tree ptr,
> >> 		    v = NULL_TREE;
> >> 		    break;
> >> 		  case COMPONENT_REF:
> >> -		    if (TREE_CODE (TREE_TYPE (v)) != ARRAY_TYPE)
> >> +		    /* When the ref is not to an array, a record or a union, it
> >> +		       will not have flexible size, compute the object size
> >> +		       directly.  */
> >> +		    if ((TREE_CODE (TREE_TYPE (v)) != ARRAY_TYPE)
> >> +			&& (TREE_CODE (TREE_TYPE (v)) != RECORD_TYPE)
> >> +			&& (TREE_CODE (TREE_TYPE (v)) != UNION_TYPE))
> >> 		      {
> >> 			v = NULL_TREE;
> >> 			break;
> >> 		      }
> >> -		    is_flexible_array_mem_ref = array_ref_flexible_size_p (v);
> >> -		    while (v != pt_var && TREE_CODE (v) == COMPONENT_REF)
> >> -		      if (TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
> >> -			  != UNION_TYPE
> >> -			  && TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
> >> -			  != QUAL_UNION_TYPE)
> >> -			break;
> >> -		      else
> >> -			v = TREE_OPERAND (v, 0);
> >> -		    if (TREE_CODE (v) == COMPONENT_REF
> >> -			&& TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
> >> -			   == RECORD_TYPE)
> >> +		    if (TREE_CODE (TREE_TYPE (v)) == RECORD_TYPE
> >> +			|| TREE_CODE (TREE_TYPE (v)) == UNION_TYPE)
> >> +		    /* if the record or union does not include a flexible array
> >> +		       recursively, compute the object size directly.  */
> >> 		      {
> >> -			/* compute object size only if v is not a
> >> -			   flexible array member.  */
> >> -			if (!is_flexible_array_mem_ref)
> >> +			if (!TYPE_INCLUDE_FLEXARRAY (TREE_TYPE (v)))
> >> 			  {
> >> 			    v = NULL_TREE;
> >> 			    break;
> >> 			  }
> >> -			v = TREE_OPERAND (v, 0);
> >> +			else
> >> +			  v = TREE_OPERAND (v, 0);
> >> 		      }
> >> -		    while (v != pt_var && TREE_CODE (v) == COMPONENT_REF)
> >> -		      if (TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
> >> -			  != UNION_TYPE
> >> -			  && TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
> >> -			  != QUAL_UNION_TYPE)
> >> -			break;
> >> -		      else
> >> -			v = TREE_OPERAND (v, 0);
> >> -		    if (v != pt_var)
> >> -		      v = NULL_TREE;
> >> 		    else
> >> -		      v = pt_var;
> >> +		      {
> >> +			/* Now the ref is to an array type.  */
> >> +			is_flexible_array_mem_ref
> >> +			  = array_ref_flexible_size_p (v);
> >> +			while (v != pt_var && TREE_CODE (v) == COMPONENT_REF)
> >> +			if (TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
> >> +			      != UNION_TYPE
> >> +			    && TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
> >> +				 != QUAL_UNION_TYPE)
> >> +			  break;
> >> +			else
> >> +			  v = TREE_OPERAND (v, 0);
> >> +			if (TREE_CODE (v) == COMPONENT_REF
> >> +			    && TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
> >> +				 == RECORD_TYPE)
> >> +			  {
> >> +			    /* compute object size only if v is not a
> >> +			       flexible array member.  */
> >> +			    if (!is_flexible_array_mem_ref)
> >> +			      {
> >> +				v = NULL_TREE;
> >> +				break;
> >> +			      }
> >> +			    v = TREE_OPERAND (v, 0);
> >> +			  }
> >> +			while (v != pt_var && TREE_CODE (v) == COMPONENT_REF)
> >> +			  if (TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
> >> +				!= UNION_TYPE
> >> +			      && TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
> >> +				   != QUAL_UNION_TYPE)
> >> +			    break;
> >> +			  else
> >> +			    v = TREE_OPERAND (v, 0);
> >> +			if (v != pt_var)
> >> +			  v = NULL_TREE;
> >> +			else
> >> +			  v = pt_var;
> >> +		      }
> >> 		    break;
> >> 		  default:
> >> 		    v = pt_var;
> >> diff --git a/gcc/tree-streamer-in.cc b/gcc/tree-streamer-in.cc
> >> index d4dc30f048f..c19ede0631d 100644
> >> --- a/gcc/tree-streamer-in.cc
> >> +++ b/gcc/tree-streamer-in.cc
> >> @@ -390,6 +390,7 @@ unpack_ts_type_common_value_fields (struct bitpack_d *bp, tree expr)
> >>       TYPE_TRANSPARENT_AGGR (expr) = (unsigned) bp_unpack_value (bp, 1);
> >>       TYPE_FINAL_P (expr) = (unsigned) bp_unpack_value (bp, 1);
> >>       TYPE_CXX_ODR_P (expr) = (unsigned) bp_unpack_value (bp, 1);
> >> +      TYPE_INCLUDE_FLEXARRAY (expr) = (unsigned) bp_unpack_value (bp, 1);
> >>     }
> >>   else if (TREE_CODE (expr) == ARRAY_TYPE)
> >>     TYPE_NONALIASED_COMPONENT (expr) = (unsigned) bp_unpack_value (bp, 1);
> >> diff --git a/gcc/tree-streamer-out.cc b/gcc/tree-streamer-out.cc
> >> index d107229da5c..73e4b4e547c 100644
> >> --- a/gcc/tree-streamer-out.cc
> >> +++ b/gcc/tree-streamer-out.cc
> >> @@ -357,6 +357,7 @@ pack_ts_type_common_value_fields (struct bitpack_d *bp, tree expr)
> >>       bp_pack_value (bp, flag_wpa && TYPE_CANONICAL (expr)
> >> 			 ? TYPE_CXX_ODR_P (TYPE_CANONICAL (expr))
> >> 			 : TYPE_CXX_ODR_P (expr), 1);
> >> +      bp_pack_value (bp, TYPE_INCLUDE_FLEXARRAY (expr), 1);
> >>     }
> >>   else if (TREE_CODE (expr) == ARRAY_TYPE)
> >>     bp_pack_value (bp, TYPE_NONALIASED_COMPONENT (expr), 1);
> >> diff --git a/gcc/tree.h b/gcc/tree.h
> >> index 92ac0e6a214..ab1cdc3dc85 100644
> >> --- a/gcc/tree.h
> >> +++ b/gcc/tree.h
> >> @@ -778,6 +778,12 @@ extern void omp_clause_range_check_failed (const_tree, const char *, int,
> >> #define TYPE_NO_NAMED_ARGS_STDARG_P(NODE) \
> >>   (TYPE_CHECK (NODE)->type_common.no_named_args_stdarg_p)
> >> 
> >> +/* True if this RECORD_TYPE or UNION_TYPE includes a flexible array member
> >> +   at the last field recursively.  */
> >> +#define TYPE_INCLUDE_FLEXARRAY(NODE) \
> >> +  (TYPE_CHECK (NODE)->type_common.type_include_flexarray)
> > 
> > Please use RECORD_OR_UNION_CHECK.  
> Okay. 
> > The comment "at the last field"
> > doesn't seem to match implementation? (at least not obviously)
> The current implementation (in gcc/c/c-decl.cc) guarantees that it’s at the last field. (And recursively). 
> > Given this is a generic header expanding on what a "flexible array member"
> > is to the middle-end here would be good.  Especially the guarantees
> > made when the bit is _not_ set (we've evaded that with DECL_NOT_FLEXARRAY
> > vs. DECL_FLEXARRAY).
> 
> The bit TYPE_INCLUDE_FLEXARRAY default is FALSE, i.e, not including a flex array by default.
> It’s only set to TRUE when the struct/union includes a flex array at the last field (and recursively). So, I think it should be correct. 
> Let me know if I missed anything here.

See above - I was looking at the use (but I'm not familiar with that 
code), and wondered how the change affects uses from other frontends.

Richard.

Re: [v4][PATCH 1/2] Handle component_ref to a structre/union field including C99 FAM [PR101832]

[Qing Zhao]

> On Mar 10, 2023, at 2:54 AM, Richard Biener <rguenther@suse.de> wrote:
> 
> On Thu, 9 Mar 2023, Qing Zhao wrote:
> 
>> 
>> 
>>> On Mar 9, 2023, at 7:20 AM, Richard Biener <rguenther@suse.de> wrote:
>>> 
>>> On Fri, 24 Feb 2023, Qing Zhao wrote:
>>> 
>>>> GCC extension accepts the case when a struct with a C99 flexible array member
>>>> is embedded into another struct or union (possibly recursively).
>>>> __builtin_object_size should treat such struct as flexible size.
>>>> 
>>>> gcc/c/ChangeLog:
>>>> 
>>>> 	PR tree-optimization/101832
>>>> 	* c-decl.cc (finish_struct): Set TYPE_INCLUDE_FLEXARRAY for
>>>> 	struct/union type.
>>> 
>>> I can't really comment on the correctness of this part but since
>>> only the C frontend will ever set this and you are using it from
>>> addr_object_size which is also used for other C family languages
>>> (at least), I wonder if you can really test
>>> 
>>> +                       if (!TYPE_INCLUDE_FLEXARRAY (TREE_TYPE (v)))
>>> 
>>> there.
>> 
>> You mean for C++ and also other C family languages (other than C), the above bit cannot be set?
>> Yes, that’s true. The bit is only set for C. So is the bit DECL_NOT_FLEXARRAY, which is only set for C too. 
>> So, I am wondering whether the bit DECL_NOT_FLEXARRAY should be also set in middle end? Or we can set DECL_NOT_FLEXARRAY in C++ FE too? And then we can set TYPE_INCLUDE_FLEXARRAY also in C++ FE?
>> What’s your suggestion?
>> 
>> (I personally feel that DECL_NOT_FLEXARRAY and TYPE_INCLUDE_FLEXARRAY should be set in the same places).
> 
> I was wondering if the above test errors on the conservative side
> correctly - it will now, for all but C, cut off some thing where it
> didn't before?

As long as the default value of TYPE_INCLUDE_FLEXARRAY reflects the correct conservative  behavior, then the testing should be correct, right?

The default value of TYPE_INCLUDE_FLEXARRAY is 0, i.e, FALSE, means that the TYPE does not include a flex array by default, this is the correct behavior. Only when the TYPE does include a flexiarray, it will be set to TRUE. So, I think it’s correct.

This is a different situation for DECL_NOT_FLEXARRAY, by default, the compiler will treat ALL trailing arrays as FMA, so in order to keep the correct conservative behavior, we should keep the default value for DECL_NOT_FLEXARRAY as it’s a FMA, i.e, DECL_NOT_FLEXARRAY being FALSE, by default. Only when the array is NOT a FMA, we set it to true. 

So, the default value for TYPE_INCLUDE_FLEXARRAY is correct. 
> 
>>> 
>>> Originally I was suggesting to set this flag in stor-layout.cc
>>> which eventually all languages funnel their types through and
>>> if there's language specific handling use a langhook (with the
>>> default implementation preserving the status quo).
>> 
>> If we decide to set the bits in stor-layout.cc, where is the best place to do it? I checked the star-layout.cc code, looks like “layout_type” might be the place where we can set these bits for RECORD_TYPE, UNION_TYPE? 
> 
> Yes, it would be layout_type.
> 
>>> 
>>> Some more comments below ...
>>> 
>>>> gcc/cp/ChangeLog:
>>>> 
>>>> 	PR tree-optimization/101832
>>>> 	* module.cc (trees_out::core_bools): Stream out new bit
>>>> 	type_include_flexarray.
>>>> 	(trees_in::core_bools): Stream in new bit type_include_flexarray.
>>>> 
>>>> gcc/ChangeLog:
>>>> 
>>>> 	PR tree-optimization/101832
>>>> 	* print-tree.cc (print_node): Print new bit type_include_flexarray.
>>>> 	* tree-core.h (struct tree_type_common): New bit
>>>> 	type_include_flexarray.
>>>> 	* tree-object-size.cc (addr_object_size): Handle structure/union type
>>>> 	when it has flexible size.
>>>> 	* tree-streamer-in.cc (unpack_ts_type_common_value_fields): Stream
>>>> 	in new bit type_include_flexarray.
>>>> 	* tree-streamer-out.cc (pack_ts_type_common_value_fields): Stream
>>>> 	out new bit type_include_flexarray.
>>>> 	* tree.h (TYPE_INCLUDE_FLEXARRAY): New macro
>>>> 	TYPE_INCLUDE_FLEXARRAY.
>>>> 
>>>> gcc/testsuite/ChangeLog:
>>>> 
>>>> 	PR tree-optimization/101832
>>>> 	* gcc.dg/builtin-object-size-pr101832.c: New test.
>>>> ---
>>>> gcc/c/c-decl.cc                               |  12 ++
>>>> gcc/cp/module.cc                              |   2 +
>>>> gcc/print-tree.cc                             |   5 +
>>>> .../gcc.dg/builtin-object-size-pr101832.c     | 134 ++++++++++++++++++
>>>> gcc/tree-core.h                               |   4 +-
>>>> gcc/tree-object-size.cc                       |  79 +++++++----
>>>> gcc/tree-streamer-in.cc                       |   1 +
>>>> gcc/tree-streamer-out.cc                      |   1 +
>>>> gcc/tree.h                                    |   6 +
>>>> 9 files changed, 215 insertions(+), 29 deletions(-)
>>>> create mode 100644 gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c
>>>> 
>>>> diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
>>>> index 08078eadeb8..f589a2f5192 100644
>>>> --- a/gcc/c/c-decl.cc
>>>> +++ b/gcc/c/c-decl.cc
>>>> @@ -9284,6 +9284,18 @@ finish_struct (location_t loc, tree t, tree fieldlist, tree attributes,
>>>>      /* Set DECL_NOT_FLEXARRAY flag for FIELD_DECL x.  */
>>>>      DECL_NOT_FLEXARRAY (x) = !is_flexible_array_member_p (is_last_field, x);
>>>> 
>>>> +      /* Set TYPE_INCLUDE_FLEXARRAY for the context of x, t
>>>> +       * when x is an array.  */
>>>> +      if (TREE_CODE (TREE_TYPE (x)) == ARRAY_TYPE)
>>>> +	TYPE_INCLUDE_FLEXARRAY (t) = flexible_array_member_type_p (TREE_TYPE (x)) ;
>>>> +      /* Recursively set TYPE_INCLUDE_FLEXARRAY for the context of x, t
>>>> +	 when x is the last field.  */
>>>> +      else if ((TREE_CODE (TREE_TYPE (x)) == RECORD_TYPE
>>>> +		|| TREE_CODE (TREE_TYPE (x)) == UNION_TYPE)
>>>> +	       && TYPE_INCLUDE_FLEXARRAY (TREE_TYPE (x))
>>>> +	       && is_last_field)
>>>> +	TYPE_INCLUDE_FLEXARRAY (t) = true;
>>>> +
>>>>      if (DECL_NAME (x)
>>>> 	  || RECORD_OR_UNION_TYPE_P (TREE_TYPE (x)))
>>>> 	saw_named_field = true;
>>>> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
>>>> index ac2fe66b080..c750361b704 100644
>>>> --- a/gcc/cp/module.cc
>>>> +++ b/gcc/cp/module.cc
>>>> @@ -5371,6 +5371,7 @@ trees_out::core_bools (tree t)
>>>>      WB (t->type_common.lang_flag_5);
>>>>      WB (t->type_common.lang_flag_6);
>>>>      WB (t->type_common.typeless_storage);
>>>> +      WB (t->type_common.type_include_flexarray);
>>>>    }
>>>> 
>>>>  if (CODE_CONTAINS_STRUCT (code, TS_DECL_COMMON))
>>>> @@ -5551,6 +5552,7 @@ trees_in::core_bools (tree t)
>>>>      RB (t->type_common.lang_flag_5);
>>>>      RB (t->type_common.lang_flag_6);
>>>>      RB (t->type_common.typeless_storage);
>>>> +      RB (t->type_common.type_include_flexarray);
>>>>    }
>>>> 
>>>>  if (CODE_CONTAINS_STRUCT (code, TS_DECL_COMMON))
>>>> diff --git a/gcc/print-tree.cc b/gcc/print-tree.cc
>>>> index 1f3afcbbc86..efacdb7686f 100644
>>>> --- a/gcc/print-tree.cc
>>>> +++ b/gcc/print-tree.cc
>>>> @@ -631,6 +631,11 @@ print_node (FILE *file, const char *prefix, tree node, int indent,
>>>> 	  && TYPE_CXX_ODR_P (node))
>>>> 	fputs (" cxx-odr-p", file);
>>>> 
>>>> +      if ((code == RECORD_TYPE
>>>> +	   || code == UNION_TYPE)
>>>> +	  && TYPE_INCLUDE_FLEXARRAY (node))
>>>> +	fputs (" include-flexarray", file);
>>>> +
>>>>      /* The transparent-union flag is used for different things in
>>>> 	 different nodes.  */
>>>>      if ((code == UNION_TYPE || code == RECORD_TYPE)
>>>> diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c b/gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c
>>>> new file mode 100644
>>>> index 00000000000..60078e11634
>>>> --- /dev/null
>>>> +++ b/gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c
>>>> @@ -0,0 +1,134 @@
>>>> +/* PR 101832: 
>>>> +   GCC extension accepts the case when a struct with a C99 flexible array
>>>> +   member is embedded into another struct (possibly recursively).
>>>> +   __builtin_object_size will treat such struct as flexible size.
>>>> +   However, when a structure with non-C99 flexible array member, i.e, trailing
>>>> +   [0], [1], or [4], is embedded into anther struct, the stucture will not
>>>> +   be treated as flexible size.  */ 
>>>> +/* { dg-do run } */
>>>> +/* { dg-options "-O2" } */
>>>> +
>>>> +#include "builtin-object-size-common.h"
>>>> +
>>>> +#define expect(p, _v) do { \
>>>> +  size_t v = _v; \
>>>> +  if (p == v) \
>>>> +    __builtin_printf ("ok:  %s == %zd\n", #p, p); \
>>>> +  else {\
>>>> +    __builtin_printf ("WAT: %s == %zd (expected %zd)\n", #p, p, v); \
>>>> +    FAIL (); \
>>>> +  } \
>>>> +} while (0);
>>>> +
>>>> +
>>>> +struct A {
>>>> +  int n;
>>>> +  char data[];
>>>> +};
>>>> +
>>>> +struct B {
>>>> +  int m;
>>>> +  struct A a;
>>>> +};
>>>> +
>>>> +struct C {
>>>> +  int q;
>>>> +  struct B b;
>>>> +};
>>>> +
>>>> +struct A0 {
>>>> +  int n;
>>>> +  char data[0];
>>>> +};
>>>> +
>>>> +struct B0 {
>>>> +  int m;
>>>> +  struct A0 a;
>>>> +};
>>>> +
>>>> +struct C0 {
>>>> +  int q;
>>>> +  struct B0 b;
>>>> +};
>>>> +
>>>> +struct A1 {
>>>> +  int n;
>>>> +  char data[1];
>>>> +};
>>>> +
>>>> +struct B1 {
>>>> +  int m;
>>>> +  struct A1 a;
>>>> +};
>>>> +
>>>> +struct C1 {
>>>> +  int q;
>>>> +  struct B1 b;
>>>> +};
>>>> +
>>>> +struct An {
>>>> +  int n;
>>>> +  char data[8];
>>>> +};
>>>> +
>>>> +struct Bn {
>>>> +  int m;
>>>> +  struct An a;
>>>> +};
>>>> +
>>>> +struct Cn {
>>>> +  int q;
>>>> +  struct Bn b;
>>>> +};
>>>> +
>>>> +volatile void *magic1, *magic2;
>>>> +
>>>> +int main (int argc, char *argv[])
>>>> +{
>>>> +  struct B *outer;
>>>> +  struct C *outest;
>>>> +
>>>> +  /* Make sure optimization can't find some other object size. */
>>>> +  outer = (void *)magic1;
>>>> +  outest = (void *)magic2;
>>>> +
>>>> +  expect (__builtin_object_size (&outer->a, 1), -1);
>>>> +  expect (__builtin_object_size (&outest->b, 1), -1);
>>>> +  expect (__builtin_object_size (&outest->b.a, 1), -1);
>>>> +
>>>> +  struct B0 *outer0;
>>>> +  struct C0 *outest0;
>>>> +
>>>> +  /* Make sure optimization can't find some other object size. */
>>>> +  outer0 = (void *)magic1;
>>>> +  outest0 = (void *)magic2;
>>>> +
>>>> +  expect (__builtin_object_size (&outer0->a, 1), sizeof (outer0->a));
>>>> +  expect (__builtin_object_size (&outest0->b, 1), sizeof (outest0->b));
>>>> +  expect (__builtin_object_size (&outest0->b.a, 1), sizeof (outest0->b.a));
>>>> +
>>>> +  struct B1 *outer1;
>>>> +  struct C1 *outest1;
>>>> +
>>>> +  /* Make sure optimization can't find some other object size. */
>>>> +  outer1 = (void *)magic1;
>>>> +  outest1 = (void *)magic2;
>>>> +
>>>> +  expect (__builtin_object_size (&outer1->a, 1), sizeof (outer1->a));
>>>> +  expect (__builtin_object_size (&outest1->b, 1), sizeof (outest1->b));
>>>> +  expect (__builtin_object_size (&outest1->b.a, 1), sizeof (outest1->b.a));
>>>> +
>>>> +  struct Bn *outern;
>>>> +  struct Cn *outestn;
>>>> +
>>>> +  /* Make sure optimization can't find some other object size. */
>>>> +  outern = (void *)magic1;
>>>> +  outestn = (void *)magic2;
>>>> +
>>>> +  expect (__builtin_object_size (&outern->a, 1), sizeof (outern->a));
>>>> +  expect (__builtin_object_size (&outestn->b, 1), sizeof (outestn->b));
>>>> +  expect (__builtin_object_size (&outestn->b.a, 1), sizeof (outestn->b.a));
>>>> +
>>>> +  DONE ();
>>>> +  return 0;
>>>> +}
>>>> diff --git a/gcc/tree-core.h b/gcc/tree-core.h
>>>> index acd8deea34e..705d5702b9c 100644
>>>> --- a/gcc/tree-core.h
>>>> +++ b/gcc/tree-core.h
>>>> @@ -1718,7 +1718,9 @@ struct GTY(()) tree_type_common {
>>>>  unsigned empty_flag : 1;
>>>>  unsigned indivisible_p : 1;
>>>> RECORD_OR_UNION_CHECK
>>> 
>>> it looks like the bit could be eventually shared with
>>> no_named_args_stdarg_p (but its accessor doesn't use
>>> FUNC_OR_METHOD_CHECK)
>> You mean to let “type_include_flexarray” share the same bit with “no_named_args_stdarg_p” in order to save 1-bit space in IR?
>> Then change the 
>> 
>> /* True if this is a stdarg function with no named arguments (C2x
>>   (...) prototype, where arguments can be accessed with va_start and
>>   va_arg), as opposed to an unprototyped function.  */
>> #define TYPE_NO_NAMED_ARGS_STDARG_P(NODE) \
>>  (TYPE_CHECK (NODE)->type_common.no_named_args_stdarg_p)
>> 
>> /* True if this RECORD_TYPE or UNION_TYPE includes a flexible array member
>>   at the last field recursively.  */
>> #define TYPE_INCLUDE_FLEXARRAY(NODE) \
>>  (TYPE_CHECK (NODE)->type_common.type_include_flexarray)
>> 
>> To:
>> 
>> union {
>>    unsigned no_named_args_stdarg_p : 1;
>>    /* TYPE_INCLUDE_FLEXARRAY flag for RECORD_TYPE and UNION_TYPE.  */
>>    unsigned int type_include_flexarray : 1;
>>  } shared_bit;
>>  unsigned spare : 15;
>> 
>> /* True if this is a stdarg function with no named arguments (C2x
>>   (...) prototype, where arguments can be accessed with va_start and
>>   va_arg), as opposed to an unprototyped function.  */
>> #define TYPE_NO_NAMED_ARGS_STDARG_P(NODE) \
>>  (FUNC_OR_METHOD_CHECK (NODE)->type_common.shared_bit.no_named_args_stdarg_p)
>> 
>> /* True if this RECORD_TYPE or UNION_TYPE includes a flexible array member
>>   at the last field recursively.  */
>> #define TYPE_INCLUDE_FLEXARRAY(NODE) \
>>  (RECORD_OR_UNION_CHECK(NODE)->type_common.shared_bit.type_include_flexarray)
> 
> No, we're just overloading bits by using the same name for different
> purposes.  I don't think such a union would pack nicely.  We overload
> by documenting the uses, in the above cases the uses are separated
> by the kind of the tree node, RECORD/UNION_TYPE vs. FUNCTION/METHOD_TYPE
> and the accessor macros should be updated to use the appropriate
> tree check macros covering that so we don't try to inspect the
> "wrong bit”.
Okay, I see. Then should I change the name of that bit to one that reflect two usages?
> 
>>> 
>>>>  alias_set_type alias_set;
>>>>  tree pointer_to;
>>>> diff --git a/gcc/tree-object-size.cc b/gcc/tree-object-size.cc
>>>> index 9a936a91983..22b3c72ea6e 100644
>>>> --- a/gcc/tree-object-size.cc
>>>> +++ b/gcc/tree-object-size.cc
>>>> @@ -633,45 +633,68 @@ addr_object_size (struct object_size_info *osi, const_tree ptr,
>>>> 		    v = NULL_TREE;
>>>> 		    break;
>>>> 		  case COMPONENT_REF:
>>>> -		    if (TREE_CODE (TREE_TYPE (v)) != ARRAY_TYPE)
>>>> +		    /* When the ref is not to an array, a record or a union, it
>>>> +		       will not have flexible size, compute the object size
>>>> +		       directly.  */
>>>> +		    if ((TREE_CODE (TREE_TYPE (v)) != ARRAY_TYPE)
>>>> +			&& (TREE_CODE (TREE_TYPE (v)) != RECORD_TYPE)
>>>> +			&& (TREE_CODE (TREE_TYPE (v)) != UNION_TYPE))
>>>> 		      {
>>>> 			v = NULL_TREE;
>>>> 			break;
>>>> 		      }
>>>> -		    is_flexible_array_mem_ref = array_ref_flexible_size_p (v);
>>>> -		    while (v != pt_var && TREE_CODE (v) == COMPONENT_REF)
>>>> -		      if (TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
>>>> -			  != UNION_TYPE
>>>> -			  && TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
>>>> -			  != QUAL_UNION_TYPE)
>>>> -			break;
>>>> -		      else
>>>> -			v = TREE_OPERAND (v, 0);
>>>> -		    if (TREE_CODE (v) == COMPONENT_REF
>>>> -			&& TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
>>>> -			   == RECORD_TYPE)
>>>> +		    if (TREE_CODE (TREE_TYPE (v)) == RECORD_TYPE
>>>> +			|| TREE_CODE (TREE_TYPE (v)) == UNION_TYPE)
>>>> +		    /* if the record or union does not include a flexible array
>>>> +		       recursively, compute the object size directly.  */
>>>> 		      {
>>>> -			/* compute object size only if v is not a
>>>> -			   flexible array member.  */
>>>> -			if (!is_flexible_array_mem_ref)
>>>> +			if (!TYPE_INCLUDE_FLEXARRAY (TREE_TYPE (v)))
>>>> 			  {
>>>> 			    v = NULL_TREE;
>>>> 			    break;
>>>> 			  }
>>>> -			v = TREE_OPERAND (v, 0);
>>>> +			else
>>>> +			  v = TREE_OPERAND (v, 0);
>>>> 		      }
>>>> -		    while (v != pt_var && TREE_CODE (v) == COMPONENT_REF)
>>>> -		      if (TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
>>>> -			  != UNION_TYPE
>>>> -			  && TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
>>>> -			  != QUAL_UNION_TYPE)
>>>> -			break;
>>>> -		      else
>>>> -			v = TREE_OPERAND (v, 0);
>>>> -		    if (v != pt_var)
>>>> -		      v = NULL_TREE;
>>>> 		    else
>>>> -		      v = pt_var;
>>>> +		      {
>>>> +			/* Now the ref is to an array type.  */
>>>> +			is_flexible_array_mem_ref
>>>> +			  = array_ref_flexible_size_p (v);
>>>> +			while (v != pt_var && TREE_CODE (v) == COMPONENT_REF)
>>>> +			if (TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
>>>> +			      != UNION_TYPE
>>>> +			    && TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
>>>> +				 != QUAL_UNION_TYPE)
>>>> +			  break;
>>>> +			else
>>>> +			  v = TREE_OPERAND (v, 0);
>>>> +			if (TREE_CODE (v) == COMPONENT_REF
>>>> +			    && TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
>>>> +				 == RECORD_TYPE)
>>>> +			  {
>>>> +			    /* compute object size only if v is not a
>>>> +			       flexible array member.  */
>>>> +			    if (!is_flexible_array_mem_ref)
>>>> +			      {
>>>> +				v = NULL_TREE;
>>>> +				break;
>>>> +			      }
>>>> +			    v = TREE_OPERAND (v, 0);
>>>> +			  }
>>>> +			while (v != pt_var && TREE_CODE (v) == COMPONENT_REF)
>>>> +			  if (TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
>>>> +				!= UNION_TYPE
>>>> +			      && TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
>>>> +				   != QUAL_UNION_TYPE)
>>>> +			    break;
>>>> +			  else
>>>> +			    v = TREE_OPERAND (v, 0);
>>>> +			if (v != pt_var)
>>>> +			  v = NULL_TREE;
>>>> +			else
>>>> +			  v = pt_var;
>>>> +		      }
>>>> 		    break;
>>>> 		  default:
>>>> 		    v = pt_var;
>>>> diff --git a/gcc/tree-streamer-in.cc b/gcc/tree-streamer-in.cc
>>>> index d4dc30f048f..c19ede0631d 100644
>>>> --- a/gcc/tree-streamer-in.cc
>>>> +++ b/gcc/tree-streamer-in.cc
>>>> @@ -390,6 +390,7 @@ unpack_ts_type_common_value_fields (struct bitpack_d *bp, tree expr)
>>>>      TYPE_TRANSPARENT_AGGR (expr) = (unsigned) bp_unpack_value (bp, 1);
>>>>      TYPE_FINAL_P (expr) = (unsigned) bp_unpack_value (bp, 1);
>>>>      TYPE_CXX_ODR_P (expr) = (unsigned) bp_unpack_value (bp, 1);
>>>> +      TYPE_INCLUDE_FLEXARRAY (expr) = (unsigned) bp_unpack_value (bp, 1);
>>>>    }
>>>>  else if (TREE_CODE (expr) == ARRAY_TYPE)
>>>>    TYPE_NONALIASED_COMPONENT (expr) = (unsigned) bp_unpack_value (bp, 1);
>>>> diff --git a/gcc/tree-streamer-out.cc b/gcc/tree-streamer-out.cc
>>>> index d107229da5c..73e4b4e547c 100644
>>>> --- a/gcc/tree-streamer-out.cc
>>>> +++ b/gcc/tree-streamer-out.cc
>>>> @@ -357,6 +357,7 @@ pack_ts_type_common_value_fields (struct bitpack_d *bp, tree expr)
>>>>      bp_pack_value (bp, flag_wpa && TYPE_CANONICAL (expr)
>>>> 			 ? TYPE_CXX_ODR_P (TYPE_CANONICAL (expr))
>>>> 			 : TYPE_CXX_ODR_P (expr), 1);
>>>> +      bp_pack_value (bp, TYPE_INCLUDE_FLEXARRAY (expr), 1);
>>>>    }
>>>>  else if (TREE_CODE (expr) == ARRAY_TYPE)
>>>>    bp_pack_value (bp, TYPE_NONALIASED_COMPONENT (expr), 1);
>>>> diff --git a/gcc/tree.h b/gcc/tree.h
>>>> index 92ac0e6a214..ab1cdc3dc85 100644
>>>> --- a/gcc/tree.h
>>>> +++ b/gcc/tree.h
>>>> @@ -778,6 +778,12 @@ extern void omp_clause_range_check_failed (const_tree, const char *, int,
>>>> #define TYPE_NO_NAMED_ARGS_STDARG_P(NODE) \
>>>>  (TYPE_CHECK (NODE)->type_common.no_named_args_stdarg_p)
>>>> 
>>>> +/* True if this RECORD_TYPE or UNION_TYPE includes a flexible array member
>>>> +   at the last field recursively.  */
>>>> +#define TYPE_INCLUDE_FLEXARRAY(NODE) \
>>>> +  (TYPE_CHECK (NODE)->type_common.type_include_flexarray)
>>> 
>>> Please use RECORD_OR_UNION_CHECK.  
>> Okay. 
>>> The comment "at the last field"
>>> doesn't seem to match implementation? (at least not obviously)
>> The current implementation (in gcc/c/c-decl.cc) guarantees that it’s at the last field. (And recursively). 
>>> Given this is a generic header expanding on what a "flexible array member"
>>> is to the middle-end here would be good.  Especially the guarantees
>>> made when the bit is _not_ set (we've evaded that with DECL_NOT_FLEXARRAY
>>> vs. DECL_FLEXARRAY).
>> 
>> The bit TYPE_INCLUDE_FLEXARRAY default is FALSE, i.e, not including a flex array by default.
>> It’s only set to TRUE when the struct/union includes a flex array at the last field (and recursively). So, I think it should be correct. 
>> Let me know if I missed anything here.
> 
> See above - I was looking at the use (but I'm not familiar with that 
> code), and wondered how the change affects uses from other frontends.

Please see my explanation above, I think the default behavior from other FE should be kept correctly with this patch.

thanks.

Qing
> 
> Richard.

Re: [v4][PATCH 1/2] Handle component_ref to a structre/union field including C99 FAM [PR101832]

[Qing Zhao]

Hi, Richard,

Do you have more comments on my responds to your previous questions?

thanks.

Qing

> On Mar 10, 2023, at 8:48 AM, Qing Zhao via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
> 
> 
> 
>> On Mar 10, 2023, at 2:54 AM, Richard Biener <rguenther@suse.de> wrote:
>> 
>> On Thu, 9 Mar 2023, Qing Zhao wrote:
>> 
>>> 
>>> 
>>>> On Mar 9, 2023, at 7:20 AM, Richard Biener <rguenther@suse.de> wrote:
>>>> 
>>>> On Fri, 24 Feb 2023, Qing Zhao wrote:
>>>> 
>>>>> GCC extension accepts the case when a struct with a C99 flexible array member
>>>>> is embedded into another struct or union (possibly recursively).
>>>>> __builtin_object_size should treat such struct as flexible size.
>>>>> 
>>>>> gcc/c/ChangeLog:
>>>>> 
>>>>> 	PR tree-optimization/101832
>>>>> 	* c-decl.cc (finish_struct): Set TYPE_INCLUDE_FLEXARRAY for
>>>>> 	struct/union type.
>>>> 
>>>> I can't really comment on the correctness of this part but since
>>>> only the C frontend will ever set this and you are using it from
>>>> addr_object_size which is also used for other C family languages
>>>> (at least), I wonder if you can really test
>>>> 
>>>> +                       if (!TYPE_INCLUDE_FLEXARRAY (TREE_TYPE (v)))
>>>> 
>>>> there.
>>> 
>>> You mean for C++ and also other C family languages (other than C), the above bit cannot be set?
>>> Yes, that’s true. The bit is only set for C. So is the bit DECL_NOT_FLEXARRAY, which is only set for C too. 
>>> So, I am wondering whether the bit DECL_NOT_FLEXARRAY should be also set in middle end? Or we can set DECL_NOT_FLEXARRAY in C++ FE too? And then we can set TYPE_INCLUDE_FLEXARRAY also in C++ FE?
>>> What’s your suggestion?
>>> 
>>> (I personally feel that DECL_NOT_FLEXARRAY and TYPE_INCLUDE_FLEXARRAY should be set in the same places).
>> 
>> I was wondering if the above test errors on the conservative side
>> correctly - it will now, for all but C, cut off some thing where it
>> didn't before?
> 
> As long as the default value of TYPE_INCLUDE_FLEXARRAY reflects the correct conservative  behavior, then the testing should be correct, right?
> 
> The default value of TYPE_INCLUDE_FLEXARRAY is 0, i.e, FALSE, means that the TYPE does not include a flex array by default, this is the correct behavior. Only when the TYPE does include a flexiarray, it will be set to TRUE. So, I think it’s correct.
> 
> This is a different situation for DECL_NOT_FLEXARRAY, by default, the compiler will treat ALL trailing arrays as FMA, so in order to keep the correct conservative behavior, we should keep the default value for DECL_NOT_FLEXARRAY as it’s a FMA, i.e, DECL_NOT_FLEXARRAY being FALSE, by default. Only when the array is NOT a FMA, we set it to true. 
> 
> So, the default value for TYPE_INCLUDE_FLEXARRAY is correct. 
>> 
>>>> 
>>>> Originally I was suggesting to set this flag in stor-layout.cc
>>>> which eventually all languages funnel their types through and
>>>> if there's language specific handling use a langhook (with the
>>>> default implementation preserving the status quo).
>>> 
>>> If we decide to set the bits in stor-layout.cc, where is the best place to do it? I checked the star-layout.cc code, looks like “layout_type” might be the place where we can set these bits for RECORD_TYPE, UNION_TYPE? 
>> 
>> Yes, it would be layout_type.
>> 
>>>> 
>>>> Some more comments below ...
>>>> 
>>>>> gcc/cp/ChangeLog:
>>>>> 
>>>>> 	PR tree-optimization/101832
>>>>> 	* module.cc (trees_out::core_bools): Stream out new bit
>>>>> 	type_include_flexarray.
>>>>> 	(trees_in::core_bools): Stream in new bit type_include_flexarray.
>>>>> 
>>>>> gcc/ChangeLog:
>>>>> 
>>>>> 	PR tree-optimization/101832
>>>>> 	* print-tree.cc (print_node): Print new bit type_include_flexarray.
>>>>> 	* tree-core.h (struct tree_type_common): New bit
>>>>> 	type_include_flexarray.
>>>>> 	* tree-object-size.cc (addr_object_size): Handle structure/union type
>>>>> 	when it has flexible size.
>>>>> 	* tree-streamer-in.cc (unpack_ts_type_common_value_fields): Stream
>>>>> 	in new bit type_include_flexarray.
>>>>> 	* tree-streamer-out.cc (pack_ts_type_common_value_fields): Stream
>>>>> 	out new bit type_include_flexarray.
>>>>> 	* tree.h (TYPE_INCLUDE_FLEXARRAY): New macro
>>>>> 	TYPE_INCLUDE_FLEXARRAY.
>>>>> 
>>>>> gcc/testsuite/ChangeLog:
>>>>> 
>>>>> 	PR tree-optimization/101832
>>>>> 	* gcc.dg/builtin-object-size-pr101832.c: New test.
>>>>> ---
>>>>> gcc/c/c-decl.cc                               |  12 ++
>>>>> gcc/cp/module.cc                              |   2 +
>>>>> gcc/print-tree.cc                             |   5 +
>>>>> .../gcc.dg/builtin-object-size-pr101832.c     | 134 ++++++++++++++++++
>>>>> gcc/tree-core.h                               |   4 +-
>>>>> gcc/tree-object-size.cc                       |  79 +++++++----
>>>>> gcc/tree-streamer-in.cc                       |   1 +
>>>>> gcc/tree-streamer-out.cc                      |   1 +
>>>>> gcc/tree.h                                    |   6 +
>>>>> 9 files changed, 215 insertions(+), 29 deletions(-)
>>>>> create mode 100644 gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c
>>>>> 
>>>>> diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
>>>>> index 08078eadeb8..f589a2f5192 100644
>>>>> --- a/gcc/c/c-decl.cc
>>>>> +++ b/gcc/c/c-decl.cc
>>>>> @@ -9284,6 +9284,18 @@ finish_struct (location_t loc, tree t, tree fieldlist, tree attributes,
>>>>>     /* Set DECL_NOT_FLEXARRAY flag for FIELD_DECL x.  */
>>>>>     DECL_NOT_FLEXARRAY (x) = !is_flexible_array_member_p (is_last_field, x);
>>>>> 
>>>>> +      /* Set TYPE_INCLUDE_FLEXARRAY for the context of x, t
>>>>> +       * when x is an array.  */
>>>>> +      if (TREE_CODE (TREE_TYPE (x)) == ARRAY_TYPE)
>>>>> +	TYPE_INCLUDE_FLEXARRAY (t) = flexible_array_member_type_p (TREE_TYPE (x)) ;
>>>>> +      /* Recursively set TYPE_INCLUDE_FLEXARRAY for the context of x, t
>>>>> +	 when x is the last field.  */
>>>>> +      else if ((TREE_CODE (TREE_TYPE (x)) == RECORD_TYPE
>>>>> +		|| TREE_CODE (TREE_TYPE (x)) == UNION_TYPE)
>>>>> +	       && TYPE_INCLUDE_FLEXARRAY (TREE_TYPE (x))
>>>>> +	       && is_last_field)
>>>>> +	TYPE_INCLUDE_FLEXARRAY (t) = true;
>>>>> +
>>>>>     if (DECL_NAME (x)
>>>>> 	  || RECORD_OR_UNION_TYPE_P (TREE_TYPE (x)))
>>>>> 	saw_named_field = true;
>>>>> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
>>>>> index ac2fe66b080..c750361b704 100644
>>>>> --- a/gcc/cp/module.cc
>>>>> +++ b/gcc/cp/module.cc
>>>>> @@ -5371,6 +5371,7 @@ trees_out::core_bools (tree t)
>>>>>     WB (t->type_common.lang_flag_5);
>>>>>     WB (t->type_common.lang_flag_6);
>>>>>     WB (t->type_common.typeless_storage);
>>>>> +      WB (t->type_common.type_include_flexarray);
>>>>>   }
>>>>> 
>>>>> if (CODE_CONTAINS_STRUCT (code, TS_DECL_COMMON))
>>>>> @@ -5551,6 +5552,7 @@ trees_in::core_bools (tree t)
>>>>>     RB (t->type_common.lang_flag_5);
>>>>>     RB (t->type_common.lang_flag_6);
>>>>>     RB (t->type_common.typeless_storage);
>>>>> +      RB (t->type_common.type_include_flexarray);
>>>>>   }
>>>>> 
>>>>> if (CODE_CONTAINS_STRUCT (code, TS_DECL_COMMON))
>>>>> diff --git a/gcc/print-tree.cc b/gcc/print-tree.cc
>>>>> index 1f3afcbbc86..efacdb7686f 100644
>>>>> --- a/gcc/print-tree.cc
>>>>> +++ b/gcc/print-tree.cc
>>>>> @@ -631,6 +631,11 @@ print_node (FILE *file, const char *prefix, tree node, int indent,
>>>>> 	  && TYPE_CXX_ODR_P (node))
>>>>> 	fputs (" cxx-odr-p", file);
>>>>> 
>>>>> +      if ((code == RECORD_TYPE
>>>>> +	   || code == UNION_TYPE)
>>>>> +	  && TYPE_INCLUDE_FLEXARRAY (node))
>>>>> +	fputs (" include-flexarray", file);
>>>>> +
>>>>>     /* The transparent-union flag is used for different things in
>>>>> 	 different nodes.  */
>>>>>     if ((code == UNION_TYPE || code == RECORD_TYPE)
>>>>> diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c b/gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c
>>>>> new file mode 100644
>>>>> index 00000000000..60078e11634
>>>>> --- /dev/null
>>>>> +++ b/gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c
>>>>> @@ -0,0 +1,134 @@
>>>>> +/* PR 101832: 
>>>>> +   GCC extension accepts the case when a struct with a C99 flexible array
>>>>> +   member is embedded into another struct (possibly recursively).
>>>>> +   __builtin_object_size will treat such struct as flexible size.
>>>>> +   However, when a structure with non-C99 flexible array member, i.e, trailing
>>>>> +   [0], [1], or [4], is embedded into anther struct, the stucture will not
>>>>> +   be treated as flexible size.  */ 
>>>>> +/* { dg-do run } */
>>>>> +/* { dg-options "-O2" } */
>>>>> +
>>>>> +#include "builtin-object-size-common.h"
>>>>> +
>>>>> +#define expect(p, _v) do { \
>>>>> +  size_t v = _v; \
>>>>> +  if (p == v) \
>>>>> +    __builtin_printf ("ok:  %s == %zd\n", #p, p); \
>>>>> +  else {\
>>>>> +    __builtin_printf ("WAT: %s == %zd (expected %zd)\n", #p, p, v); \
>>>>> +    FAIL (); \
>>>>> +  } \
>>>>> +} while (0);
>>>>> +
>>>>> +
>>>>> +struct A {
>>>>> +  int n;
>>>>> +  char data[];
>>>>> +};
>>>>> +
>>>>> +struct B {
>>>>> +  int m;
>>>>> +  struct A a;
>>>>> +};
>>>>> +
>>>>> +struct C {
>>>>> +  int q;
>>>>> +  struct B b;
>>>>> +};
>>>>> +
>>>>> +struct A0 {
>>>>> +  int n;
>>>>> +  char data[0];
>>>>> +};
>>>>> +
>>>>> +struct B0 {
>>>>> +  int m;
>>>>> +  struct A0 a;
>>>>> +};
>>>>> +
>>>>> +struct C0 {
>>>>> +  int q;
>>>>> +  struct B0 b;
>>>>> +};
>>>>> +
>>>>> +struct A1 {
>>>>> +  int n;
>>>>> +  char data[1];
>>>>> +};
>>>>> +
>>>>> +struct B1 {
>>>>> +  int m;
>>>>> +  struct A1 a;
>>>>> +};
>>>>> +
>>>>> +struct C1 {
>>>>> +  int q;
>>>>> +  struct B1 b;
>>>>> +};
>>>>> +
>>>>> +struct An {
>>>>> +  int n;
>>>>> +  char data[8];
>>>>> +};
>>>>> +
>>>>> +struct Bn {
>>>>> +  int m;
>>>>> +  struct An a;
>>>>> +};
>>>>> +
>>>>> +struct Cn {
>>>>> +  int q;
>>>>> +  struct Bn b;
>>>>> +};
>>>>> +
>>>>> +volatile void *magic1, *magic2;
>>>>> +
>>>>> +int main (int argc, char *argv[])
>>>>> +{
>>>>> +  struct B *outer;
>>>>> +  struct C *outest;
>>>>> +
>>>>> +  /* Make sure optimization can't find some other object size. */
>>>>> +  outer = (void *)magic1;
>>>>> +  outest = (void *)magic2;
>>>>> +
>>>>> +  expect (__builtin_object_size (&outer->a, 1), -1);
>>>>> +  expect (__builtin_object_size (&outest->b, 1), -1);
>>>>> +  expect (__builtin_object_size (&outest->b.a, 1), -1);
>>>>> +
>>>>> +  struct B0 *outer0;
>>>>> +  struct C0 *outest0;
>>>>> +
>>>>> +  /* Make sure optimization can't find some other object size. */
>>>>> +  outer0 = (void *)magic1;
>>>>> +  outest0 = (void *)magic2;
>>>>> +
>>>>> +  expect (__builtin_object_size (&outer0->a, 1), sizeof (outer0->a));
>>>>> +  expect (__builtin_object_size (&outest0->b, 1), sizeof (outest0->b));
>>>>> +  expect (__builtin_object_size (&outest0->b.a, 1), sizeof (outest0->b.a));
>>>>> +
>>>>> +  struct B1 *outer1;
>>>>> +  struct C1 *outest1;
>>>>> +
>>>>> +  /* Make sure optimization can't find some other object size. */
>>>>> +  outer1 = (void *)magic1;
>>>>> +  outest1 = (void *)magic2;
>>>>> +
>>>>> +  expect (__builtin_object_size (&outer1->a, 1), sizeof (outer1->a));
>>>>> +  expect (__builtin_object_size (&outest1->b, 1), sizeof (outest1->b));
>>>>> +  expect (__builtin_object_size (&outest1->b.a, 1), sizeof (outest1->b.a));
>>>>> +
>>>>> +  struct Bn *outern;
>>>>> +  struct Cn *outestn;
>>>>> +
>>>>> +  /* Make sure optimization can't find some other object size. */
>>>>> +  outern = (void *)magic1;
>>>>> +  outestn = (void *)magic2;
>>>>> +
>>>>> +  expect (__builtin_object_size (&outern->a, 1), sizeof (outern->a));
>>>>> +  expect (__builtin_object_size (&outestn->b, 1), sizeof (outestn->b));
>>>>> +  expect (__builtin_object_size (&outestn->b.a, 1), sizeof (outestn->b.a));
>>>>> +
>>>>> +  DONE ();
>>>>> +  return 0;
>>>>> +}
>>>>> diff --git a/gcc/tree-core.h b/gcc/tree-core.h
>>>>> index acd8deea34e..705d5702b9c 100644
>>>>> --- a/gcc/tree-core.h
>>>>> +++ b/gcc/tree-core.h
>>>>> @@ -1718,7 +1718,9 @@ struct GTY(()) tree_type_common {
>>>>> unsigned empty_flag : 1;
>>>>> unsigned indivisible_p : 1;
>>>>> RECORD_OR_UNION_CHECK
>>>> 
>>>> it looks like the bit could be eventually shared with
>>>> no_named_args_stdarg_p (but its accessor doesn't use
>>>> FUNC_OR_METHOD_CHECK)
>>> You mean to let “type_include_flexarray” share the same bit with “no_named_args_stdarg_p” in order to save 1-bit space in IR?
>>> Then change the 
>>> 
>>> /* True if this is a stdarg function with no named arguments (C2x
>>>  (...) prototype, where arguments can be accessed with va_start and
>>>  va_arg), as opposed to an unprototyped function.  */
>>> #define TYPE_NO_NAMED_ARGS_STDARG_P(NODE) \
>>> (TYPE_CHECK (NODE)->type_common.no_named_args_stdarg_p)
>>> 
>>> /* True if this RECORD_TYPE or UNION_TYPE includes a flexible array member
>>>  at the last field recursively.  */
>>> #define TYPE_INCLUDE_FLEXARRAY(NODE) \
>>> (TYPE_CHECK (NODE)->type_common.type_include_flexarray)
>>> 
>>> To:
>>> 
>>> union {
>>>   unsigned no_named_args_stdarg_p : 1;
>>>   /* TYPE_INCLUDE_FLEXARRAY flag for RECORD_TYPE and UNION_TYPE.  */
>>>   unsigned int type_include_flexarray : 1;
>>> } shared_bit;
>>> unsigned spare : 15;
>>> 
>>> /* True if this is a stdarg function with no named arguments (C2x
>>>  (...) prototype, where arguments can be accessed with va_start and
>>>  va_arg), as opposed to an unprototyped function.  */
>>> #define TYPE_NO_NAMED_ARGS_STDARG_P(NODE) \
>>> (FUNC_OR_METHOD_CHECK (NODE)->type_common.shared_bit.no_named_args_stdarg_p)
>>> 
>>> /* True if this RECORD_TYPE or UNION_TYPE includes a flexible array member
>>>  at the last field recursively.  */
>>> #define TYPE_INCLUDE_FLEXARRAY(NODE) \
>>> (RECORD_OR_UNION_CHECK(NODE)->type_common.shared_bit.type_include_flexarray)
>> 
>> No, we're just overloading bits by using the same name for different
>> purposes.  I don't think such a union would pack nicely.  We overload
>> by documenting the uses, in the above cases the uses are separated
>> by the kind of the tree node, RECORD/UNION_TYPE vs. FUNCTION/METHOD_TYPE
>> and the accessor macros should be updated to use the appropriate
>> tree check macros covering that so we don't try to inspect the
>> "wrong bit”.
> Okay, I see. Then should I change the name of that bit to one that reflect two usages?
>> 
>>>> 
>>>>> alias_set_type alias_set;
>>>>> tree pointer_to;
>>>>> diff --git a/gcc/tree-object-size.cc b/gcc/tree-object-size.cc
>>>>> index 9a936a91983..22b3c72ea6e 100644
>>>>> --- a/gcc/tree-object-size.cc
>>>>> +++ b/gcc/tree-object-size.cc
>>>>> @@ -633,45 +633,68 @@ addr_object_size (struct object_size_info *osi, const_tree ptr,
>>>>> 		    v = NULL_TREE;
>>>>> 		    break;
>>>>> 		  case COMPONENT_REF:
>>>>> -		    if (TREE_CODE (TREE_TYPE (v)) != ARRAY_TYPE)
>>>>> +		    /* When the ref is not to an array, a record or a union, it
>>>>> +		       will not have flexible size, compute the object size
>>>>> +		       directly.  */
>>>>> +		    if ((TREE_CODE (TREE_TYPE (v)) != ARRAY_TYPE)
>>>>> +			&& (TREE_CODE (TREE_TYPE (v)) != RECORD_TYPE)
>>>>> +			&& (TREE_CODE (TREE_TYPE (v)) != UNION_TYPE))
>>>>> 		      {
>>>>> 			v = NULL_TREE;
>>>>> 			break;
>>>>> 		      }
>>>>> -		    is_flexible_array_mem_ref = array_ref_flexible_size_p (v);
>>>>> -		    while (v != pt_var && TREE_CODE (v) == COMPONENT_REF)
>>>>> -		      if (TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
>>>>> -			  != UNION_TYPE
>>>>> -			  && TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
>>>>> -			  != QUAL_UNION_TYPE)
>>>>> -			break;
>>>>> -		      else
>>>>> -			v = TREE_OPERAND (v, 0);
>>>>> -		    if (TREE_CODE (v) == COMPONENT_REF
>>>>> -			&& TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
>>>>> -			   == RECORD_TYPE)
>>>>> +		    if (TREE_CODE (TREE_TYPE (v)) == RECORD_TYPE
>>>>> +			|| TREE_CODE (TREE_TYPE (v)) == UNION_TYPE)
>>>>> +		    /* if the record or union does not include a flexible array
>>>>> +		       recursively, compute the object size directly.  */
>>>>> 		      {
>>>>> -			/* compute object size only if v is not a
>>>>> -			   flexible array member.  */
>>>>> -			if (!is_flexible_array_mem_ref)
>>>>> +			if (!TYPE_INCLUDE_FLEXARRAY (TREE_TYPE (v)))
>>>>> 			  {
>>>>> 			    v = NULL_TREE;
>>>>> 			    break;
>>>>> 			  }
>>>>> -			v = TREE_OPERAND (v, 0);
>>>>> +			else
>>>>> +			  v = TREE_OPERAND (v, 0);
>>>>> 		      }
>>>>> -		    while (v != pt_var && TREE_CODE (v) == COMPONENT_REF)
>>>>> -		      if (TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
>>>>> -			  != UNION_TYPE
>>>>> -			  && TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
>>>>> -			  != QUAL_UNION_TYPE)
>>>>> -			break;
>>>>> -		      else
>>>>> -			v = TREE_OPERAND (v, 0);
>>>>> -		    if (v != pt_var)
>>>>> -		      v = NULL_TREE;
>>>>> 		    else
>>>>> -		      v = pt_var;
>>>>> +		      {
>>>>> +			/* Now the ref is to an array type.  */
>>>>> +			is_flexible_array_mem_ref
>>>>> +			  = array_ref_flexible_size_p (v);
>>>>> +			while (v != pt_var && TREE_CODE (v) == COMPONENT_REF)
>>>>> +			if (TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
>>>>> +			      != UNION_TYPE
>>>>> +			    && TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
>>>>> +				 != QUAL_UNION_TYPE)
>>>>> +			  break;
>>>>> +			else
>>>>> +			  v = TREE_OPERAND (v, 0);
>>>>> +			if (TREE_CODE (v) == COMPONENT_REF
>>>>> +			    && TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
>>>>> +				 == RECORD_TYPE)
>>>>> +			  {
>>>>> +			    /* compute object size only if v is not a
>>>>> +			       flexible array member.  */
>>>>> +			    if (!is_flexible_array_mem_ref)
>>>>> +			      {
>>>>> +				v = NULL_TREE;
>>>>> +				break;
>>>>> +			      }
>>>>> +			    v = TREE_OPERAND (v, 0);
>>>>> +			  }
>>>>> +			while (v != pt_var && TREE_CODE (v) == COMPONENT_REF)
>>>>> +			  if (TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
>>>>> +				!= UNION_TYPE
>>>>> +			      && TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
>>>>> +				   != QUAL_UNION_TYPE)
>>>>> +			    break;
>>>>> +			  else
>>>>> +			    v = TREE_OPERAND (v, 0);
>>>>> +			if (v != pt_var)
>>>>> +			  v = NULL_TREE;
>>>>> +			else
>>>>> +			  v = pt_var;
>>>>> +		      }
>>>>> 		    break;
>>>>> 		  default:
>>>>> 		    v = pt_var;
>>>>> diff --git a/gcc/tree-streamer-in.cc b/gcc/tree-streamer-in.cc
>>>>> index d4dc30f048f..c19ede0631d 100644
>>>>> --- a/gcc/tree-streamer-in.cc
>>>>> +++ b/gcc/tree-streamer-in.cc
>>>>> @@ -390,6 +390,7 @@ unpack_ts_type_common_value_fields (struct bitpack_d *bp, tree expr)
>>>>>     TYPE_TRANSPARENT_AGGR (expr) = (unsigned) bp_unpack_value (bp, 1);
>>>>>     TYPE_FINAL_P (expr) = (unsigned) bp_unpack_value (bp, 1);
>>>>>     TYPE_CXX_ODR_P (expr) = (unsigned) bp_unpack_value (bp, 1);
>>>>> +      TYPE_INCLUDE_FLEXARRAY (expr) = (unsigned) bp_unpack_value (bp, 1);
>>>>>   }
>>>>> else if (TREE_CODE (expr) == ARRAY_TYPE)
>>>>>   TYPE_NONALIASED_COMPONENT (expr) = (unsigned) bp_unpack_value (bp, 1);
>>>>> diff --git a/gcc/tree-streamer-out.cc b/gcc/tree-streamer-out.cc
>>>>> index d107229da5c..73e4b4e547c 100644
>>>>> --- a/gcc/tree-streamer-out.cc
>>>>> +++ b/gcc/tree-streamer-out.cc
>>>>> @@ -357,6 +357,7 @@ pack_ts_type_common_value_fields (struct bitpack_d *bp, tree expr)
>>>>>     bp_pack_value (bp, flag_wpa && TYPE_CANONICAL (expr)
>>>>> 			 ? TYPE_CXX_ODR_P (TYPE_CANONICAL (expr))
>>>>> 			 : TYPE_CXX_ODR_P (expr), 1);
>>>>> +      bp_pack_value (bp, TYPE_INCLUDE_FLEXARRAY (expr), 1);
>>>>>   }
>>>>> else if (TREE_CODE (expr) == ARRAY_TYPE)
>>>>>   bp_pack_value (bp, TYPE_NONALIASED_COMPONENT (expr), 1);
>>>>> diff --git a/gcc/tree.h b/gcc/tree.h
>>>>> index 92ac0e6a214..ab1cdc3dc85 100644
>>>>> --- a/gcc/tree.h
>>>>> +++ b/gcc/tree.h
>>>>> @@ -778,6 +778,12 @@ extern void omp_clause_range_check_failed (const_tree, const char *, int,
>>>>> #define TYPE_NO_NAMED_ARGS_STDARG_P(NODE) \
>>>>> (TYPE_CHECK (NODE)->type_common.no_named_args_stdarg_p)
>>>>> 
>>>>> +/* True if this RECORD_TYPE or UNION_TYPE includes a flexible array member
>>>>> +   at the last field recursively.  */
>>>>> +#define TYPE_INCLUDE_FLEXARRAY(NODE) \
>>>>> +  (TYPE_CHECK (NODE)->type_common.type_include_flexarray)
>>>> 
>>>> Please use RECORD_OR_UNION_CHECK.  
>>> Okay. 
>>>> The comment "at the last field"
>>>> doesn't seem to match implementation? (at least not obviously)
>>> The current implementation (in gcc/c/c-decl.cc) guarantees that it’s at the last field. (And recursively). 
>>>> Given this is a generic header expanding on what a "flexible array member"
>>>> is to the middle-end here would be good.  Especially the guarantees
>>>> made when the bit is _not_ set (we've evaded that with DECL_NOT_FLEXARRAY
>>>> vs. DECL_FLEXARRAY).
>>> 
>>> The bit TYPE_INCLUDE_FLEXARRAY default is FALSE, i.e, not including a flex array by default.
>>> It’s only set to TRUE when the struct/union includes a flex array at the last field (and recursively). So, I think it should be correct. 
>>> Let me know if I missed anything here.
>> 
>> See above - I was looking at the use (but I'm not familiar with that 
>> code), and wondered how the change affects uses from other frontends.
> 
> Please see my explanation above, I think the default behavior from other FE should be kept correctly with this patch.
> 
> thanks.
> 
> Qing
>> 
>> Richard.

Re: [v4][PATCH 1/2] Handle component_ref to a structre/union field including C99 FAM [PR101832]

[Richard Biener]

[-- Attachment #1: Type: text/plain, Size: 22752 bytes --]

On Mon, 13 Mar 2023, Qing Zhao wrote:

> Hi, Richard,
> 
> Do you have more comments on my responds to your previous questions?

No, generally we're not good at naming the shared bits, so keeping the
old name is fine here.

Btw, I do not feel competent enough to approve the patch, instead that's
on the burden of the C frontend maintainers and the people understanding
the object-size code more.

Richard.

> thanks.
> 
> Qing
> 
> > On Mar 10, 2023, at 8:48 AM, Qing Zhao via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
> > 
> > 
> > 
> >> On Mar 10, 2023, at 2:54 AM, Richard Biener <rguenther@suse.de> wrote:
> >> 
> >> On Thu, 9 Mar 2023, Qing Zhao wrote:
> >> 
> >>> 
> >>> 
> >>>> On Mar 9, 2023, at 7:20 AM, Richard Biener <rguenther@suse.de> wrote:
> >>>> 
> >>>> On Fri, 24 Feb 2023, Qing Zhao wrote:
> >>>> 
> >>>>> GCC extension accepts the case when a struct with a C99 flexible array member
> >>>>> is embedded into another struct or union (possibly recursively).
> >>>>> __builtin_object_size should treat such struct as flexible size.
> >>>>> 
> >>>>> gcc/c/ChangeLog:
> >>>>> 
> >>>>> 	PR tree-optimization/101832
> >>>>> 	* c-decl.cc (finish_struct): Set TYPE_INCLUDE_FLEXARRAY for
> >>>>> 	struct/union type.
> >>>> 
> >>>> I can't really comment on the correctness of this part but since
> >>>> only the C frontend will ever set this and you are using it from
> >>>> addr_object_size which is also used for other C family languages
> >>>> (at least), I wonder if you can really test
> >>>> 
> >>>> +                       if (!TYPE_INCLUDE_FLEXARRAY (TREE_TYPE (v)))
> >>>> 
> >>>> there.
> >>> 
> >>> You mean for C++ and also other C family languages (other than C), the above bit cannot be set?
> >>> Yes, that’s true. The bit is only set for C. So is the bit DECL_NOT_FLEXARRAY, which is only set for C too. 
> >>> So, I am wondering whether the bit DECL_NOT_FLEXARRAY should be also set in middle end? Or we can set DECL_NOT_FLEXARRAY in C++ FE too? And then we can set TYPE_INCLUDE_FLEXARRAY also in C++ FE?
> >>> What’s your suggestion?
> >>> 
> >>> (I personally feel that DECL_NOT_FLEXARRAY and TYPE_INCLUDE_FLEXARRAY should be set in the same places).
> >> 
> >> I was wondering if the above test errors on the conservative side
> >> correctly - it will now, for all but C, cut off some thing where it
> >> didn't before?
> > 
> > As long as the default value of TYPE_INCLUDE_FLEXARRAY reflects the correct conservative  behavior, then the testing should be correct, right?
> > 
> > The default value of TYPE_INCLUDE_FLEXARRAY is 0, i.e, FALSE, means that the TYPE does not include a flex array by default, this is the correct behavior. Only when the TYPE does include a flexiarray, it will be set to TRUE. So, I think it’s correct.
> > 
> > This is a different situation for DECL_NOT_FLEXARRAY, by default, the compiler will treat ALL trailing arrays as FMA, so in order to keep the correct conservative behavior, we should keep the default value for DECL_NOT_FLEXARRAY as it’s a FMA, i.e, DECL_NOT_FLEXARRAY being FALSE, by default. Only when the array is NOT a FMA, we set it to true. 
> > 
> > So, the default value for TYPE_INCLUDE_FLEXARRAY is correct. 
> >> 
> >>>> 
> >>>> Originally I was suggesting to set this flag in stor-layout.cc
> >>>> which eventually all languages funnel their types through and
> >>>> if there's language specific handling use a langhook (with the
> >>>> default implementation preserving the status quo).
> >>> 
> >>> If we decide to set the bits in stor-layout.cc, where is the best place to do it? I checked the star-layout.cc code, looks like “layout_type” might be the place where we can set these bits for RECORD_TYPE, UNION_TYPE? 
> >> 
> >> Yes, it would be layout_type.
> >> 
> >>>> 
> >>>> Some more comments below ...
> >>>> 
> >>>>> gcc/cp/ChangeLog:
> >>>>> 
> >>>>> 	PR tree-optimization/101832
> >>>>> 	* module.cc (trees_out::core_bools): Stream out new bit
> >>>>> 	type_include_flexarray.
> >>>>> 	(trees_in::core_bools): Stream in new bit type_include_flexarray.
> >>>>> 
> >>>>> gcc/ChangeLog:
> >>>>> 
> >>>>> 	PR tree-optimization/101832
> >>>>> 	* print-tree.cc (print_node): Print new bit type_include_flexarray.
> >>>>> 	* tree-core.h (struct tree_type_common): New bit
> >>>>> 	type_include_flexarray.
> >>>>> 	* tree-object-size.cc (addr_object_size): Handle structure/union type
> >>>>> 	when it has flexible size.
> >>>>> 	* tree-streamer-in.cc (unpack_ts_type_common_value_fields): Stream
> >>>>> 	in new bit type_include_flexarray.
> >>>>> 	* tree-streamer-out.cc (pack_ts_type_common_value_fields): Stream
> >>>>> 	out new bit type_include_flexarray.
> >>>>> 	* tree.h (TYPE_INCLUDE_FLEXARRAY): New macro
> >>>>> 	TYPE_INCLUDE_FLEXARRAY.
> >>>>> 
> >>>>> gcc/testsuite/ChangeLog:
> >>>>> 
> >>>>> 	PR tree-optimization/101832
> >>>>> 	* gcc.dg/builtin-object-size-pr101832.c: New test.
> >>>>> ---
> >>>>> gcc/c/c-decl.cc                               |  12 ++
> >>>>> gcc/cp/module.cc                              |   2 +
> >>>>> gcc/print-tree.cc                             |   5 +
> >>>>> .../gcc.dg/builtin-object-size-pr101832.c     | 134 ++++++++++++++++++
> >>>>> gcc/tree-core.h                               |   4 +-
> >>>>> gcc/tree-object-size.cc                       |  79 +++++++----
> >>>>> gcc/tree-streamer-in.cc                       |   1 +
> >>>>> gcc/tree-streamer-out.cc                      |   1 +
> >>>>> gcc/tree.h                                    |   6 +
> >>>>> 9 files changed, 215 insertions(+), 29 deletions(-)
> >>>>> create mode 100644 gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c
> >>>>> 
> >>>>> diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
> >>>>> index 08078eadeb8..f589a2f5192 100644
> >>>>> --- a/gcc/c/c-decl.cc
> >>>>> +++ b/gcc/c/c-decl.cc
> >>>>> @@ -9284,6 +9284,18 @@ finish_struct (location_t loc, tree t, tree fieldlist, tree attributes,
> >>>>>     /* Set DECL_NOT_FLEXARRAY flag for FIELD_DECL x.  */
> >>>>>     DECL_NOT_FLEXARRAY (x) = !is_flexible_array_member_p (is_last_field, x);
> >>>>> 
> >>>>> +      /* Set TYPE_INCLUDE_FLEXARRAY for the context of x, t
> >>>>> +       * when x is an array.  */
> >>>>> +      if (TREE_CODE (TREE_TYPE (x)) == ARRAY_TYPE)
> >>>>> +	TYPE_INCLUDE_FLEXARRAY (t) = flexible_array_member_type_p (TREE_TYPE (x)) ;
> >>>>> +      /* Recursively set TYPE_INCLUDE_FLEXARRAY for the context of x, t
> >>>>> +	 when x is the last field.  */
> >>>>> +      else if ((TREE_CODE (TREE_TYPE (x)) == RECORD_TYPE
> >>>>> +		|| TREE_CODE (TREE_TYPE (x)) == UNION_TYPE)
> >>>>> +	       && TYPE_INCLUDE_FLEXARRAY (TREE_TYPE (x))
> >>>>> +	       && is_last_field)
> >>>>> +	TYPE_INCLUDE_FLEXARRAY (t) = true;
> >>>>> +
> >>>>>     if (DECL_NAME (x)
> >>>>> 	  || RECORD_OR_UNION_TYPE_P (TREE_TYPE (x)))
> >>>>> 	saw_named_field = true;
> >>>>> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> >>>>> index ac2fe66b080..c750361b704 100644
> >>>>> --- a/gcc/cp/module.cc
> >>>>> +++ b/gcc/cp/module.cc
> >>>>> @@ -5371,6 +5371,7 @@ trees_out::core_bools (tree t)
> >>>>>     WB (t->type_common.lang_flag_5);
> >>>>>     WB (t->type_common.lang_flag_6);
> >>>>>     WB (t->type_common.typeless_storage);
> >>>>> +      WB (t->type_common.type_include_flexarray);
> >>>>>   }
> >>>>> 
> >>>>> if (CODE_CONTAINS_STRUCT (code, TS_DECL_COMMON))
> >>>>> @@ -5551,6 +5552,7 @@ trees_in::core_bools (tree t)
> >>>>>     RB (t->type_common.lang_flag_5);
> >>>>>     RB (t->type_common.lang_flag_6);
> >>>>>     RB (t->type_common.typeless_storage);
> >>>>> +      RB (t->type_common.type_include_flexarray);
> >>>>>   }
> >>>>> 
> >>>>> if (CODE_CONTAINS_STRUCT (code, TS_DECL_COMMON))
> >>>>> diff --git a/gcc/print-tree.cc b/gcc/print-tree.cc
> >>>>> index 1f3afcbbc86..efacdb7686f 100644
> >>>>> --- a/gcc/print-tree.cc
> >>>>> +++ b/gcc/print-tree.cc
> >>>>> @@ -631,6 +631,11 @@ print_node (FILE *file, const char *prefix, tree node, int indent,
> >>>>> 	  && TYPE_CXX_ODR_P (node))
> >>>>> 	fputs (" cxx-odr-p", file);
> >>>>> 
> >>>>> +      if ((code == RECORD_TYPE
> >>>>> +	   || code == UNION_TYPE)
> >>>>> +	  && TYPE_INCLUDE_FLEXARRAY (node))
> >>>>> +	fputs (" include-flexarray", file);
> >>>>> +
> >>>>>     /* The transparent-union flag is used for different things in
> >>>>> 	 different nodes.  */
> >>>>>     if ((code == UNION_TYPE || code == RECORD_TYPE)
> >>>>> diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c b/gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c
> >>>>> new file mode 100644
> >>>>> index 00000000000..60078e11634
> >>>>> --- /dev/null
> >>>>> +++ b/gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c
> >>>>> @@ -0,0 +1,134 @@
> >>>>> +/* PR 101832: 
> >>>>> +   GCC extension accepts the case when a struct with a C99 flexible array
> >>>>> +   member is embedded into another struct (possibly recursively).
> >>>>> +   __builtin_object_size will treat such struct as flexible size.
> >>>>> +   However, when a structure with non-C99 flexible array member, i.e, trailing
> >>>>> +   [0], [1], or [4], is embedded into anther struct, the stucture will not
> >>>>> +   be treated as flexible size.  */ 
> >>>>> +/* { dg-do run } */
> >>>>> +/* { dg-options "-O2" } */
> >>>>> +
> >>>>> +#include "builtin-object-size-common.h"
> >>>>> +
> >>>>> +#define expect(p, _v) do { \
> >>>>> +  size_t v = _v; \
> >>>>> +  if (p == v) \
> >>>>> +    __builtin_printf ("ok:  %s == %zd\n", #p, p); \
> >>>>> +  else {\
> >>>>> +    __builtin_printf ("WAT: %s == %zd (expected %zd)\n", #p, p, v); \
> >>>>> +    FAIL (); \
> >>>>> +  } \
> >>>>> +} while (0);
> >>>>> +
> >>>>> +
> >>>>> +struct A {
> >>>>> +  int n;
> >>>>> +  char data[];
> >>>>> +};
> >>>>> +
> >>>>> +struct B {
> >>>>> +  int m;
> >>>>> +  struct A a;
> >>>>> +};
> >>>>> +
> >>>>> +struct C {
> >>>>> +  int q;
> >>>>> +  struct B b;
> >>>>> +};
> >>>>> +
> >>>>> +struct A0 {
> >>>>> +  int n;
> >>>>> +  char data[0];
> >>>>> +};
> >>>>> +
> >>>>> +struct B0 {
> >>>>> +  int m;
> >>>>> +  struct A0 a;
> >>>>> +};
> >>>>> +
> >>>>> +struct C0 {
> >>>>> +  int q;
> >>>>> +  struct B0 b;
> >>>>> +};
> >>>>> +
> >>>>> +struct A1 {
> >>>>> +  int n;
> >>>>> +  char data[1];
> >>>>> +};
> >>>>> +
> >>>>> +struct B1 {
> >>>>> +  int m;
> >>>>> +  struct A1 a;
> >>>>> +};
> >>>>> +
> >>>>> +struct C1 {
> >>>>> +  int q;
> >>>>> +  struct B1 b;
> >>>>> +};
> >>>>> +
> >>>>> +struct An {
> >>>>> +  int n;
> >>>>> +  char data[8];
> >>>>> +};
> >>>>> +
> >>>>> +struct Bn {
> >>>>> +  int m;
> >>>>> +  struct An a;
> >>>>> +};
> >>>>> +
> >>>>> +struct Cn {
> >>>>> +  int q;
> >>>>> +  struct Bn b;
> >>>>> +};
> >>>>> +
> >>>>> +volatile void *magic1, *magic2;
> >>>>> +
> >>>>> +int main (int argc, char *argv[])
> >>>>> +{
> >>>>> +  struct B *outer;
> >>>>> +  struct C *outest;
> >>>>> +
> >>>>> +  /* Make sure optimization can't find some other object size. */
> >>>>> +  outer = (void *)magic1;
> >>>>> +  outest = (void *)magic2;
> >>>>> +
> >>>>> +  expect (__builtin_object_size (&outer->a, 1), -1);
> >>>>> +  expect (__builtin_object_size (&outest->b, 1), -1);
> >>>>> +  expect (__builtin_object_size (&outest->b.a, 1), -1);
> >>>>> +
> >>>>> +  struct B0 *outer0;
> >>>>> +  struct C0 *outest0;
> >>>>> +
> >>>>> +  /* Make sure optimization can't find some other object size. */
> >>>>> +  outer0 = (void *)magic1;
> >>>>> +  outest0 = (void *)magic2;
> >>>>> +
> >>>>> +  expect (__builtin_object_size (&outer0->a, 1), sizeof (outer0->a));
> >>>>> +  expect (__builtin_object_size (&outest0->b, 1), sizeof (outest0->b));
> >>>>> +  expect (__builtin_object_size (&outest0->b.a, 1), sizeof (outest0->b.a));
> >>>>> +
> >>>>> +  struct B1 *outer1;
> >>>>> +  struct C1 *outest1;
> >>>>> +
> >>>>> +  /* Make sure optimization can't find some other object size. */
> >>>>> +  outer1 = (void *)magic1;
> >>>>> +  outest1 = (void *)magic2;
> >>>>> +
> >>>>> +  expect (__builtin_object_size (&outer1->a, 1), sizeof (outer1->a));
> >>>>> +  expect (__builtin_object_size (&outest1->b, 1), sizeof (outest1->b));
> >>>>> +  expect (__builtin_object_size (&outest1->b.a, 1), sizeof (outest1->b.a));
> >>>>> +
> >>>>> +  struct Bn *outern;
> >>>>> +  struct Cn *outestn;
> >>>>> +
> >>>>> +  /* Make sure optimization can't find some other object size. */
> >>>>> +  outern = (void *)magic1;
> >>>>> +  outestn = (void *)magic2;
> >>>>> +
> >>>>> +  expect (__builtin_object_size (&outern->a, 1), sizeof (outern->a));
> >>>>> +  expect (__builtin_object_size (&outestn->b, 1), sizeof (outestn->b));
> >>>>> +  expect (__builtin_object_size (&outestn->b.a, 1), sizeof (outestn->b.a));
> >>>>> +
> >>>>> +  DONE ();
> >>>>> +  return 0;
> >>>>> +}
> >>>>> diff --git a/gcc/tree-core.h b/gcc/tree-core.h
> >>>>> index acd8deea34e..705d5702b9c 100644
> >>>>> --- a/gcc/tree-core.h
> >>>>> +++ b/gcc/tree-core.h
> >>>>> @@ -1718,7 +1718,9 @@ struct GTY(()) tree_type_common {
> >>>>> unsigned empty_flag : 1;
> >>>>> unsigned indivisible_p : 1;
> >>>>> RECORD_OR_UNION_CHECK
> >>>> 
> >>>> it looks like the bit could be eventually shared with
> >>>> no_named_args_stdarg_p (but its accessor doesn't use
> >>>> FUNC_OR_METHOD_CHECK)
> >>> You mean to let “type_include_flexarray” share the same bit with “no_named_args_stdarg_p” in order to save 1-bit space in IR?
> >>> Then change the 
> >>> 
> >>> /* True if this is a stdarg function with no named arguments (C2x
> >>>  (...) prototype, where arguments can be accessed with va_start and
> >>>  va_arg), as opposed to an unprototyped function.  */
> >>> #define TYPE_NO_NAMED_ARGS_STDARG_P(NODE) \
> >>> (TYPE_CHECK (NODE)->type_common.no_named_args_stdarg_p)
> >>> 
> >>> /* True if this RECORD_TYPE or UNION_TYPE includes a flexible array member
> >>>  at the last field recursively.  */
> >>> #define TYPE_INCLUDE_FLEXARRAY(NODE) \
> >>> (TYPE_CHECK (NODE)->type_common.type_include_flexarray)
> >>> 
> >>> To:
> >>> 
> >>> union {
> >>>   unsigned no_named_args_stdarg_p : 1;
> >>>   /* TYPE_INCLUDE_FLEXARRAY flag for RECORD_TYPE and UNION_TYPE.  */
> >>>   unsigned int type_include_flexarray : 1;
> >>> } shared_bit;
> >>> unsigned spare : 15;
> >>> 
> >>> /* True if this is a stdarg function with no named arguments (C2x
> >>>  (...) prototype, where arguments can be accessed with va_start and
> >>>  va_arg), as opposed to an unprototyped function.  */
> >>> #define TYPE_NO_NAMED_ARGS_STDARG_P(NODE) \
> >>> (FUNC_OR_METHOD_CHECK (NODE)->type_common.shared_bit.no_named_args_stdarg_p)
> >>> 
> >>> /* True if this RECORD_TYPE or UNION_TYPE includes a flexible array member
> >>>  at the last field recursively.  */
> >>> #define TYPE_INCLUDE_FLEXARRAY(NODE) \
> >>> (RECORD_OR_UNION_CHECK(NODE)->type_common.shared_bit.type_include_flexarray)
> >> 
> >> No, we're just overloading bits by using the same name for different
> >> purposes.  I don't think such a union would pack nicely.  We overload
> >> by documenting the uses, in the above cases the uses are separated
> >> by the kind of the tree node, RECORD/UNION_TYPE vs. FUNCTION/METHOD_TYPE
> >> and the accessor macros should be updated to use the appropriate
> >> tree check macros covering that so we don't try to inspect the
> >> "wrong bit”.
> > Okay, I see. Then should I change the name of that bit to one that reflect two usages?
> >> 
> >>>> 
> >>>>> alias_set_type alias_set;
> >>>>> tree pointer_to;
> >>>>> diff --git a/gcc/tree-object-size.cc b/gcc/tree-object-size.cc
> >>>>> index 9a936a91983..22b3c72ea6e 100644
> >>>>> --- a/gcc/tree-object-size.cc
> >>>>> +++ b/gcc/tree-object-size.cc
> >>>>> @@ -633,45 +633,68 @@ addr_object_size (struct object_size_info *osi, const_tree ptr,
> >>>>> 		    v = NULL_TREE;
> >>>>> 		    break;
> >>>>> 		  case COMPONENT_REF:
> >>>>> -		    if (TREE_CODE (TREE_TYPE (v)) != ARRAY_TYPE)
> >>>>> +		    /* When the ref is not to an array, a record or a union, it
> >>>>> +		       will not have flexible size, compute the object size
> >>>>> +		       directly.  */
> >>>>> +		    if ((TREE_CODE (TREE_TYPE (v)) != ARRAY_TYPE)
> >>>>> +			&& (TREE_CODE (TREE_TYPE (v)) != RECORD_TYPE)
> >>>>> +			&& (TREE_CODE (TREE_TYPE (v)) != UNION_TYPE))
> >>>>> 		      {
> >>>>> 			v = NULL_TREE;
> >>>>> 			break;
> >>>>> 		      }
> >>>>> -		    is_flexible_array_mem_ref = array_ref_flexible_size_p (v);
> >>>>> -		    while (v != pt_var && TREE_CODE (v) == COMPONENT_REF)
> >>>>> -		      if (TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
> >>>>> -			  != UNION_TYPE
> >>>>> -			  && TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
> >>>>> -			  != QUAL_UNION_TYPE)
> >>>>> -			break;
> >>>>> -		      else
> >>>>> -			v = TREE_OPERAND (v, 0);
> >>>>> -		    if (TREE_CODE (v) == COMPONENT_REF
> >>>>> -			&& TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
> >>>>> -			   == RECORD_TYPE)
> >>>>> +		    if (TREE_CODE (TREE_TYPE (v)) == RECORD_TYPE
> >>>>> +			|| TREE_CODE (TREE_TYPE (v)) == UNION_TYPE)
> >>>>> +		    /* if the record or union does not include a flexible array
> >>>>> +		       recursively, compute the object size directly.  */
> >>>>> 		      {
> >>>>> -			/* compute object size only if v is not a
> >>>>> -			   flexible array member.  */
> >>>>> -			if (!is_flexible_array_mem_ref)
> >>>>> +			if (!TYPE_INCLUDE_FLEXARRAY (TREE_TYPE (v)))
> >>>>> 			  {
> >>>>> 			    v = NULL_TREE;
> >>>>> 			    break;
> >>>>> 			  }
> >>>>> -			v = TREE_OPERAND (v, 0);
> >>>>> +			else
> >>>>> +			  v = TREE_OPERAND (v, 0);
> >>>>> 		      }
> >>>>> -		    while (v != pt_var && TREE_CODE (v) == COMPONENT_REF)
> >>>>> -		      if (TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
> >>>>> -			  != UNION_TYPE
> >>>>> -			  && TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
> >>>>> -			  != QUAL_UNION_TYPE)
> >>>>> -			break;
> >>>>> -		      else
> >>>>> -			v = TREE_OPERAND (v, 0);
> >>>>> -		    if (v != pt_var)
> >>>>> -		      v = NULL_TREE;
> >>>>> 		    else
> >>>>> -		      v = pt_var;
> >>>>> +		      {
> >>>>> +			/* Now the ref is to an array type.  */
> >>>>> +			is_flexible_array_mem_ref
> >>>>> +			  = array_ref_flexible_size_p (v);
> >>>>> +			while (v != pt_var && TREE_CODE (v) == COMPONENT_REF)
> >>>>> +			if (TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
> >>>>> +			      != UNION_TYPE
> >>>>> +			    && TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
> >>>>> +				 != QUAL_UNION_TYPE)
> >>>>> +			  break;
> >>>>> +			else
> >>>>> +			  v = TREE_OPERAND (v, 0);
> >>>>> +			if (TREE_CODE (v) == COMPONENT_REF
> >>>>> +			    && TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
> >>>>> +				 == RECORD_TYPE)
> >>>>> +			  {
> >>>>> +			    /* compute object size only if v is not a
> >>>>> +			       flexible array member.  */
> >>>>> +			    if (!is_flexible_array_mem_ref)
> >>>>> +			      {
> >>>>> +				v = NULL_TREE;
> >>>>> +				break;
> >>>>> +			      }
> >>>>> +			    v = TREE_OPERAND (v, 0);
> >>>>> +			  }
> >>>>> +			while (v != pt_var && TREE_CODE (v) == COMPONENT_REF)
> >>>>> +			  if (TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
> >>>>> +				!= UNION_TYPE
> >>>>> +			      && TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
> >>>>> +				   != QUAL_UNION_TYPE)
> >>>>> +			    break;
> >>>>> +			  else
> >>>>> +			    v = TREE_OPERAND (v, 0);
> >>>>> +			if (v != pt_var)
> >>>>> +			  v = NULL_TREE;
> >>>>> +			else
> >>>>> +			  v = pt_var;
> >>>>> +		      }
> >>>>> 		    break;
> >>>>> 		  default:
> >>>>> 		    v = pt_var;
> >>>>> diff --git a/gcc/tree-streamer-in.cc b/gcc/tree-streamer-in.cc
> >>>>> index d4dc30f048f..c19ede0631d 100644
> >>>>> --- a/gcc/tree-streamer-in.cc
> >>>>> +++ b/gcc/tree-streamer-in.cc
> >>>>> @@ -390,6 +390,7 @@ unpack_ts_type_common_value_fields (struct bitpack_d *bp, tree expr)
> >>>>>     TYPE_TRANSPARENT_AGGR (expr) = (unsigned) bp_unpack_value (bp, 1);
> >>>>>     TYPE_FINAL_P (expr) = (unsigned) bp_unpack_value (bp, 1);
> >>>>>     TYPE_CXX_ODR_P (expr) = (unsigned) bp_unpack_value (bp, 1);
> >>>>> +      TYPE_INCLUDE_FLEXARRAY (expr) = (unsigned) bp_unpack_value (bp, 1);
> >>>>>   }
> >>>>> else if (TREE_CODE (expr) == ARRAY_TYPE)
> >>>>>   TYPE_NONALIASED_COMPONENT (expr) = (unsigned) bp_unpack_value (bp, 1);
> >>>>> diff --git a/gcc/tree-streamer-out.cc b/gcc/tree-streamer-out.cc
> >>>>> index d107229da5c..73e4b4e547c 100644
> >>>>> --- a/gcc/tree-streamer-out.cc
> >>>>> +++ b/gcc/tree-streamer-out.cc
> >>>>> @@ -357,6 +357,7 @@ pack_ts_type_common_value_fields (struct bitpack_d *bp, tree expr)
> >>>>>     bp_pack_value (bp, flag_wpa && TYPE_CANONICAL (expr)
> >>>>> 			 ? TYPE_CXX_ODR_P (TYPE_CANONICAL (expr))
> >>>>> 			 : TYPE_CXX_ODR_P (expr), 1);
> >>>>> +      bp_pack_value (bp, TYPE_INCLUDE_FLEXARRAY (expr), 1);
> >>>>>   }
> >>>>> else if (TREE_CODE (expr) == ARRAY_TYPE)
> >>>>>   bp_pack_value (bp, TYPE_NONALIASED_COMPONENT (expr), 1);
> >>>>> diff --git a/gcc/tree.h b/gcc/tree.h
> >>>>> index 92ac0e6a214..ab1cdc3dc85 100644
> >>>>> --- a/gcc/tree.h
> >>>>> +++ b/gcc/tree.h
> >>>>> @@ -778,6 +778,12 @@ extern void omp_clause_range_check_failed (const_tree, const char *, int,
> >>>>> #define TYPE_NO_NAMED_ARGS_STDARG_P(NODE) \
> >>>>> (TYPE_CHECK (NODE)->type_common.no_named_args_stdarg_p)
> >>>>> 
> >>>>> +/* True if this RECORD_TYPE or UNION_TYPE includes a flexible array member
> >>>>> +   at the last field recursively.  */
> >>>>> +#define TYPE_INCLUDE_FLEXARRAY(NODE) \
> >>>>> +  (TYPE_CHECK (NODE)->type_common.type_include_flexarray)
> >>>> 
> >>>> Please use RECORD_OR_UNION_CHECK.  
> >>> Okay. 
> >>>> The comment "at the last field"
> >>>> doesn't seem to match implementation? (at least not obviously)
> >>> The current implementation (in gcc/c/c-decl.cc) guarantees that it’s at the last field. (And recursively). 
> >>>> Given this is a generic header expanding on what a "flexible array member"
> >>>> is to the middle-end here would be good.  Especially the guarantees
> >>>> made when the bit is _not_ set (we've evaded that with DECL_NOT_FLEXARRAY
> >>>> vs. DECL_FLEXARRAY).
> >>> 
> >>> The bit TYPE_INCLUDE_FLEXARRAY default is FALSE, i.e, not including a flex array by default.
> >>> It’s only set to TRUE when the struct/union includes a flex array at the last field (and recursively). So, I think it should be correct. 
> >>> Let me know if I missed anything here.
> >> 
> >> See above - I was looking at the use (but I'm not familiar with that 
> >> code), and wondered how the change affects uses from other frontends.
> > 
> > Please see my explanation above, I think the default behavior from other FE should be kept correctly with this patch.
> > 
> > thanks.
> > 
> > Qing
> >> 
> >> Richard.
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg,
Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman;
HRB 36809 (AG Nuernberg)

Re: [v4][PATCH 1/2] Handle component_ref to a structre/union field including C99 FAM [PR101832]

[Qing Zhao]

> On Mar 14, 2023, at 5:04 AM, Richard Biener <rguenther@suse.de> wrote:
> 
> On Mon, 13 Mar 2023, Qing Zhao wrote:
> 
>> Hi, Richard,
>> 
>> Do you have more comments on my responds to your previous questions?
> 
> No, generally we're not good at naming the shared bits, so keeping the
> old name is fine here.
Okay. I will keep the old name for it. 

> 
> Btw, I do not feel competent enough to approve the patch, instead that's
> on the burden of the C frontend maintainers and the people understanding
> the object-size code more.

So, Joseph and Jakub should be the ones to approve these patches?

I will update the patches with your suggestions for the bit.
And send the 5th version .

Thanks.

Qing

> 
> Richard.
> 
>> thanks.
>> 
>> Qing
>> 
>>> On Mar 10, 2023, at 8:48 AM, Qing Zhao via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
>>> 
>>> 
>>> 
>>>> On Mar 10, 2023, at 2:54 AM, Richard Biener <rguenther@suse.de> wrote:
>>>> 
>>>> On Thu, 9 Mar 2023, Qing Zhao wrote:
>>>> 
>>>>> 
>>>>> 
>>>>>> On Mar 9, 2023, at 7:20 AM, Richard Biener <rguenther@suse.de> wrote:
>>>>>> 
>>>>>> On Fri, 24 Feb 2023, Qing Zhao wrote:
>>>>>> 
>>>>>>> GCC extension accepts the case when a struct with a C99 flexible array member
>>>>>>> is embedded into another struct or union (possibly recursively).
>>>>>>> __builtin_object_size should treat such struct as flexible size.
>>>>>>> 
>>>>>>> gcc/c/ChangeLog:
>>>>>>> 
>>>>>>> 	PR tree-optimization/101832
>>>>>>> 	* c-decl.cc (finish_struct): Set TYPE_INCLUDE_FLEXARRAY for
>>>>>>> 	struct/union type.
>>>>>> 
>>>>>> I can't really comment on the correctness of this part but since
>>>>>> only the C frontend will ever set this and you are using it from
>>>>>> addr_object_size which is also used for other C family languages
>>>>>> (at least), I wonder if you can really test
>>>>>> 
>>>>>> +                       if (!TYPE_INCLUDE_FLEXARRAY (TREE_TYPE (v)))
>>>>>> 
>>>>>> there.
>>>>> 
>>>>> You mean for C++ and also other C family languages (other than C), the above bit cannot be set?
>>>>> Yes, that’s true. The bit is only set for C. So is the bit DECL_NOT_FLEXARRAY, which is only set for C too. 
>>>>> So, I am wondering whether the bit DECL_NOT_FLEXARRAY should be also set in middle end? Or we can set DECL_NOT_FLEXARRAY in C++ FE too? And then we can set TYPE_INCLUDE_FLEXARRAY also in C++ FE?
>>>>> What’s your suggestion?
>>>>> 
>>>>> (I personally feel that DECL_NOT_FLEXARRAY and TYPE_INCLUDE_FLEXARRAY should be set in the same places).
>>>> 
>>>> I was wondering if the above test errors on the conservative side
>>>> correctly - it will now, for all but C, cut off some thing where it
>>>> didn't before?
>>> 
>>> As long as the default value of TYPE_INCLUDE_FLEXARRAY reflects the correct conservative  behavior, then the testing should be correct, right?
>>> 
>>> The default value of TYPE_INCLUDE_FLEXARRAY is 0, i.e, FALSE, means that the TYPE does not include a flex array by default, this is the correct behavior. Only when the TYPE does include a flexiarray, it will be set to TRUE. So, I think it’s correct.
>>> 
>>> This is a different situation for DECL_NOT_FLEXARRAY, by default, the compiler will treat ALL trailing arrays as FMA, so in order to keep the correct conservative behavior, we should keep the default value for DECL_NOT_FLEXARRAY as it’s a FMA, i.e, DECL_NOT_FLEXARRAY being FALSE, by default. Only when the array is NOT a FMA, we set it to true. 
>>> 
>>> So, the default value for TYPE_INCLUDE_FLEXARRAY is correct. 
>>>> 
>>>>>> 
>>>>>> Originally I was suggesting to set this flag in stor-layout.cc
>>>>>> which eventually all languages funnel their types through and
>>>>>> if there's language specific handling use a langhook (with the
>>>>>> default implementation preserving the status quo).
>>>>> 
>>>>> If we decide to set the bits in stor-layout.cc, where is the best place to do it? I checked the star-layout.cc code, looks like “layout_type” might be the place where we can set these bits for RECORD_TYPE, UNION_TYPE? 
>>>> 
>>>> Yes, it would be layout_type.
>>>> 
>>>>>> 
>>>>>> Some more comments below ...
>>>>>> 
>>>>>>> gcc/cp/ChangeLog:
>>>>>>> 
>>>>>>> 	PR tree-optimization/101832
>>>>>>> 	* module.cc (trees_out::core_bools): Stream out new bit
>>>>>>> 	type_include_flexarray.
>>>>>>> 	(trees_in::core_bools): Stream in new bit type_include_flexarray.
>>>>>>> 
>>>>>>> gcc/ChangeLog:
>>>>>>> 
>>>>>>> 	PR tree-optimization/101832
>>>>>>> 	* print-tree.cc (print_node): Print new bit type_include_flexarray.
>>>>>>> 	* tree-core.h (struct tree_type_common): New bit
>>>>>>> 	type_include_flexarray.
>>>>>>> 	* tree-object-size.cc (addr_object_size): Handle structure/union type
>>>>>>> 	when it has flexible size.
>>>>>>> 	* tree-streamer-in.cc (unpack_ts_type_common_value_fields): Stream
>>>>>>> 	in new bit type_include_flexarray.
>>>>>>> 	* tree-streamer-out.cc (pack_ts_type_common_value_fields): Stream
>>>>>>> 	out new bit type_include_flexarray.
>>>>>>> 	* tree.h (TYPE_INCLUDE_FLEXARRAY): New macro
>>>>>>> 	TYPE_INCLUDE_FLEXARRAY.
>>>>>>> 
>>>>>>> gcc/testsuite/ChangeLog:
>>>>>>> 
>>>>>>> 	PR tree-optimization/101832
>>>>>>> 	* gcc.dg/builtin-object-size-pr101832.c: New test.
>>>>>>> ---
>>>>>>> gcc/c/c-decl.cc                               |  12 ++
>>>>>>> gcc/cp/module.cc                              |   2 +
>>>>>>> gcc/print-tree.cc                             |   5 +
>>>>>>> .../gcc.dg/builtin-object-size-pr101832.c     | 134 ++++++++++++++++++
>>>>>>> gcc/tree-core.h                               |   4 +-
>>>>>>> gcc/tree-object-size.cc                       |  79 +++++++----
>>>>>>> gcc/tree-streamer-in.cc                       |   1 +
>>>>>>> gcc/tree-streamer-out.cc                      |   1 +
>>>>>>> gcc/tree.h                                    |   6 +
>>>>>>> 9 files changed, 215 insertions(+), 29 deletions(-)
>>>>>>> create mode 100644 gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c
>>>>>>> 
>>>>>>> diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
>>>>>>> index 08078eadeb8..f589a2f5192 100644
>>>>>>> --- a/gcc/c/c-decl.cc
>>>>>>> +++ b/gcc/c/c-decl.cc
>>>>>>> @@ -9284,6 +9284,18 @@ finish_struct (location_t loc, tree t, tree fieldlist, tree attributes,
>>>>>>>    /* Set DECL_NOT_FLEXARRAY flag for FIELD_DECL x.  */
>>>>>>>    DECL_NOT_FLEXARRAY (x) = !is_flexible_array_member_p (is_last_field, x);
>>>>>>> 
>>>>>>> +      /* Set TYPE_INCLUDE_FLEXARRAY for the context of x, t
>>>>>>> +       * when x is an array.  */
>>>>>>> +      if (TREE_CODE (TREE_TYPE (x)) == ARRAY_TYPE)
>>>>>>> +	TYPE_INCLUDE_FLEXARRAY (t) = flexible_array_member_type_p (TREE_TYPE (x)) ;
>>>>>>> +      /* Recursively set TYPE_INCLUDE_FLEXARRAY for the context of x, t
>>>>>>> +	 when x is the last field.  */
>>>>>>> +      else if ((TREE_CODE (TREE_TYPE (x)) == RECORD_TYPE
>>>>>>> +		|| TREE_CODE (TREE_TYPE (x)) == UNION_TYPE)
>>>>>>> +	       && TYPE_INCLUDE_FLEXARRAY (TREE_TYPE (x))
>>>>>>> +	       && is_last_field)
>>>>>>> +	TYPE_INCLUDE_FLEXARRAY (t) = true;
>>>>>>> +
>>>>>>>    if (DECL_NAME (x)
>>>>>>> 	  || RECORD_OR_UNION_TYPE_P (TREE_TYPE (x)))
>>>>>>> 	saw_named_field = true;
>>>>>>> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
>>>>>>> index ac2fe66b080..c750361b704 100644
>>>>>>> --- a/gcc/cp/module.cc
>>>>>>> +++ b/gcc/cp/module.cc
>>>>>>> @@ -5371,6 +5371,7 @@ trees_out::core_bools (tree t)
>>>>>>>    WB (t->type_common.lang_flag_5);
>>>>>>>    WB (t->type_common.lang_flag_6);
>>>>>>>    WB (t->type_common.typeless_storage);
>>>>>>> +      WB (t->type_common.type_include_flexarray);
>>>>>>>  }
>>>>>>> 
>>>>>>> if (CODE_CONTAINS_STRUCT (code, TS_DECL_COMMON))
>>>>>>> @@ -5551,6 +5552,7 @@ trees_in::core_bools (tree t)
>>>>>>>    RB (t->type_common.lang_flag_5);
>>>>>>>    RB (t->type_common.lang_flag_6);
>>>>>>>    RB (t->type_common.typeless_storage);
>>>>>>> +      RB (t->type_common.type_include_flexarray);
>>>>>>>  }
>>>>>>> 
>>>>>>> if (CODE_CONTAINS_STRUCT (code, TS_DECL_COMMON))
>>>>>>> diff --git a/gcc/print-tree.cc b/gcc/print-tree.cc
>>>>>>> index 1f3afcbbc86..efacdb7686f 100644
>>>>>>> --- a/gcc/print-tree.cc
>>>>>>> +++ b/gcc/print-tree.cc
>>>>>>> @@ -631,6 +631,11 @@ print_node (FILE *file, const char *prefix, tree node, int indent,
>>>>>>> 	  && TYPE_CXX_ODR_P (node))
>>>>>>> 	fputs (" cxx-odr-p", file);
>>>>>>> 
>>>>>>> +      if ((code == RECORD_TYPE
>>>>>>> +	   || code == UNION_TYPE)
>>>>>>> +	  && TYPE_INCLUDE_FLEXARRAY (node))
>>>>>>> +	fputs (" include-flexarray", file);
>>>>>>> +
>>>>>>>    /* The transparent-union flag is used for different things in
>>>>>>> 	 different nodes.  */
>>>>>>>    if ((code == UNION_TYPE || code == RECORD_TYPE)
>>>>>>> diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c b/gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c
>>>>>>> new file mode 100644
>>>>>>> index 00000000000..60078e11634
>>>>>>> --- /dev/null
>>>>>>> +++ b/gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c
>>>>>>> @@ -0,0 +1,134 @@
>>>>>>> +/* PR 101832: 
>>>>>>> +   GCC extension accepts the case when a struct with a C99 flexible array
>>>>>>> +   member is embedded into another struct (possibly recursively).
>>>>>>> +   __builtin_object_size will treat such struct as flexible size.
>>>>>>> +   However, when a structure with non-C99 flexible array member, i.e, trailing
>>>>>>> +   [0], [1], or [4], is embedded into anther struct, the stucture will not
>>>>>>> +   be treated as flexible size.  */ 
>>>>>>> +/* { dg-do run } */
>>>>>>> +/* { dg-options "-O2" } */
>>>>>>> +
>>>>>>> +#include "builtin-object-size-common.h"
>>>>>>> +
>>>>>>> +#define expect(p, _v) do { \
>>>>>>> +  size_t v = _v; \
>>>>>>> +  if (p == v) \
>>>>>>> +    __builtin_printf ("ok:  %s == %zd\n", #p, p); \
>>>>>>> +  else {\
>>>>>>> +    __builtin_printf ("WAT: %s == %zd (expected %zd)\n", #p, p, v); \
>>>>>>> +    FAIL (); \
>>>>>>> +  } \
>>>>>>> +} while (0);
>>>>>>> +
>>>>>>> +
>>>>>>> +struct A {
>>>>>>> +  int n;
>>>>>>> +  char data[];
>>>>>>> +};
>>>>>>> +
>>>>>>> +struct B {
>>>>>>> +  int m;
>>>>>>> +  struct A a;
>>>>>>> +};
>>>>>>> +
>>>>>>> +struct C {
>>>>>>> +  int q;
>>>>>>> +  struct B b;
>>>>>>> +};
>>>>>>> +
>>>>>>> +struct A0 {
>>>>>>> +  int n;
>>>>>>> +  char data[0];
>>>>>>> +};
>>>>>>> +
>>>>>>> +struct B0 {
>>>>>>> +  int m;
>>>>>>> +  struct A0 a;
>>>>>>> +};
>>>>>>> +
>>>>>>> +struct C0 {
>>>>>>> +  int q;
>>>>>>> +  struct B0 b;
>>>>>>> +};
>>>>>>> +
>>>>>>> +struct A1 {
>>>>>>> +  int n;
>>>>>>> +  char data[1];
>>>>>>> +};
>>>>>>> +
>>>>>>> +struct B1 {
>>>>>>> +  int m;
>>>>>>> +  struct A1 a;
>>>>>>> +};
>>>>>>> +
>>>>>>> +struct C1 {
>>>>>>> +  int q;
>>>>>>> +  struct B1 b;
>>>>>>> +};
>>>>>>> +
>>>>>>> +struct An {
>>>>>>> +  int n;
>>>>>>> +  char data[8];
>>>>>>> +};
>>>>>>> +
>>>>>>> +struct Bn {
>>>>>>> +  int m;
>>>>>>> +  struct An a;
>>>>>>> +};
>>>>>>> +
>>>>>>> +struct Cn {
>>>>>>> +  int q;
>>>>>>> +  struct Bn b;
>>>>>>> +};
>>>>>>> +
>>>>>>> +volatile void *magic1, *magic2;
>>>>>>> +
>>>>>>> +int main (int argc, char *argv[])
>>>>>>> +{
>>>>>>> +  struct B *outer;
>>>>>>> +  struct C *outest;
>>>>>>> +
>>>>>>> +  /* Make sure optimization can't find some other object size. */
>>>>>>> +  outer = (void *)magic1;
>>>>>>> +  outest = (void *)magic2;
>>>>>>> +
>>>>>>> +  expect (__builtin_object_size (&outer->a, 1), -1);
>>>>>>> +  expect (__builtin_object_size (&outest->b, 1), -1);
>>>>>>> +  expect (__builtin_object_size (&outest->b.a, 1), -1);
>>>>>>> +
>>>>>>> +  struct B0 *outer0;
>>>>>>> +  struct C0 *outest0;
>>>>>>> +
>>>>>>> +  /* Make sure optimization can't find some other object size. */
>>>>>>> +  outer0 = (void *)magic1;
>>>>>>> +  outest0 = (void *)magic2;
>>>>>>> +
>>>>>>> +  expect (__builtin_object_size (&outer0->a, 1), sizeof (outer0->a));
>>>>>>> +  expect (__builtin_object_size (&outest0->b, 1), sizeof (outest0->b));
>>>>>>> +  expect (__builtin_object_size (&outest0->b.a, 1), sizeof (outest0->b.a));
>>>>>>> +
>>>>>>> +  struct B1 *outer1;
>>>>>>> +  struct C1 *outest1;
>>>>>>> +
>>>>>>> +  /* Make sure optimization can't find some other object size. */
>>>>>>> +  outer1 = (void *)magic1;
>>>>>>> +  outest1 = (void *)magic2;
>>>>>>> +
>>>>>>> +  expect (__builtin_object_size (&outer1->a, 1), sizeof (outer1->a));
>>>>>>> +  expect (__builtin_object_size (&outest1->b, 1), sizeof (outest1->b));
>>>>>>> +  expect (__builtin_object_size (&outest1->b.a, 1), sizeof (outest1->b.a));
>>>>>>> +
>>>>>>> +  struct Bn *outern;
>>>>>>> +  struct Cn *outestn;
>>>>>>> +
>>>>>>> +  /* Make sure optimization can't find some other object size. */
>>>>>>> +  outern = (void *)magic1;
>>>>>>> +  outestn = (void *)magic2;
>>>>>>> +
>>>>>>> +  expect (__builtin_object_size (&outern->a, 1), sizeof (outern->a));
>>>>>>> +  expect (__builtin_object_size (&outestn->b, 1), sizeof (outestn->b));
>>>>>>> +  expect (__builtin_object_size (&outestn->b.a, 1), sizeof (outestn->b.a));
>>>>>>> +
>>>>>>> +  DONE ();
>>>>>>> +  return 0;
>>>>>>> +}
>>>>>>> diff --git a/gcc/tree-core.h b/gcc/tree-core.h
>>>>>>> index acd8deea34e..705d5702b9c 100644
>>>>>>> --- a/gcc/tree-core.h
>>>>>>> +++ b/gcc/tree-core.h
>>>>>>> @@ -1718,7 +1718,9 @@ struct GTY(()) tree_type_common {
>>>>>>> unsigned empty_flag : 1;
>>>>>>> unsigned indivisible_p : 1;
>>>>>>> RECORD_OR_UNION_CHECK
>>>>>> 
>>>>>> it looks like the bit could be eventually shared with
>>>>>> no_named_args_stdarg_p (but its accessor doesn't use
>>>>>> FUNC_OR_METHOD_CHECK)
>>>>> You mean to let “type_include_flexarray” share the same bit with “no_named_args_stdarg_p” in order to save 1-bit space in IR?
>>>>> Then change the 
>>>>> 
>>>>> /* True if this is a stdarg function with no named arguments (C2x
>>>>> (...) prototype, where arguments can be accessed with va_start and
>>>>> va_arg), as opposed to an unprototyped function.  */
>>>>> #define TYPE_NO_NAMED_ARGS_STDARG_P(NODE) \
>>>>> (TYPE_CHECK (NODE)->type_common.no_named_args_stdarg_p)
>>>>> 
>>>>> /* True if this RECORD_TYPE or UNION_TYPE includes a flexible array member
>>>>> at the last field recursively.  */
>>>>> #define TYPE_INCLUDE_FLEXARRAY(NODE) \
>>>>> (TYPE_CHECK (NODE)->type_common.type_include_flexarray)
>>>>> 
>>>>> To:
>>>>> 
>>>>> union {
>>>>>  unsigned no_named_args_stdarg_p : 1;
>>>>>  /* TYPE_INCLUDE_FLEXARRAY flag for RECORD_TYPE and UNION_TYPE.  */
>>>>>  unsigned int type_include_flexarray : 1;
>>>>> } shared_bit;
>>>>> unsigned spare : 15;
>>>>> 
>>>>> /* True if this is a stdarg function with no named arguments (C2x
>>>>> (...) prototype, where arguments can be accessed with va_start and
>>>>> va_arg), as opposed to an unprototyped function.  */
>>>>> #define TYPE_NO_NAMED_ARGS_STDARG_P(NODE) \
>>>>> (FUNC_OR_METHOD_CHECK (NODE)->type_common.shared_bit.no_named_args_stdarg_p)
>>>>> 
>>>>> /* True if this RECORD_TYPE or UNION_TYPE includes a flexible array member
>>>>> at the last field recursively.  */
>>>>> #define TYPE_INCLUDE_FLEXARRAY(NODE) \
>>>>> (RECORD_OR_UNION_CHECK(NODE)->type_common.shared_bit.type_include_flexarray)
>>>> 
>>>> No, we're just overloading bits by using the same name for different
>>>> purposes.  I don't think such a union would pack nicely.  We overload
>>>> by documenting the uses, in the above cases the uses are separated
>>>> by the kind of the tree node, RECORD/UNION_TYPE vs. FUNCTION/METHOD_TYPE
>>>> and the accessor macros should be updated to use the appropriate
>>>> tree check macros covering that so we don't try to inspect the
>>>> "wrong bit”.
>>> Okay, I see. Then should I change the name of that bit to one that reflect two usages?
>>>> 
>>>>>> 
>>>>>>> alias_set_type alias_set;
>>>>>>> tree pointer_to;
>>>>>>> diff --git a/gcc/tree-object-size.cc b/gcc/tree-object-size.cc
>>>>>>> index 9a936a91983..22b3c72ea6e 100644
>>>>>>> --- a/gcc/tree-object-size.cc
>>>>>>> +++ b/gcc/tree-object-size.cc
>>>>>>> @@ -633,45 +633,68 @@ addr_object_size (struct object_size_info *osi, const_tree ptr,
>>>>>>> 		    v = NULL_TREE;
>>>>>>> 		    break;
>>>>>>> 		  case COMPONENT_REF:
>>>>>>> -		    if (TREE_CODE (TREE_TYPE (v)) != ARRAY_TYPE)
>>>>>>> +		    /* When the ref is not to an array, a record or a union, it
>>>>>>> +		       will not have flexible size, compute the object size
>>>>>>> +		       directly.  */
>>>>>>> +		    if ((TREE_CODE (TREE_TYPE (v)) != ARRAY_TYPE)
>>>>>>> +			&& (TREE_CODE (TREE_TYPE (v)) != RECORD_TYPE)
>>>>>>> +			&& (TREE_CODE (TREE_TYPE (v)) != UNION_TYPE))
>>>>>>> 		      {
>>>>>>> 			v = NULL_TREE;
>>>>>>> 			break;
>>>>>>> 		      }
>>>>>>> -		    is_flexible_array_mem_ref = array_ref_flexible_size_p (v);
>>>>>>> -		    while (v != pt_var && TREE_CODE (v) == COMPONENT_REF)
>>>>>>> -		      if (TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
>>>>>>> -			  != UNION_TYPE
>>>>>>> -			  && TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
>>>>>>> -			  != QUAL_UNION_TYPE)
>>>>>>> -			break;
>>>>>>> -		      else
>>>>>>> -			v = TREE_OPERAND (v, 0);
>>>>>>> -		    if (TREE_CODE (v) == COMPONENT_REF
>>>>>>> -			&& TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
>>>>>>> -			   == RECORD_TYPE)
>>>>>>> +		    if (TREE_CODE (TREE_TYPE (v)) == RECORD_TYPE
>>>>>>> +			|| TREE_CODE (TREE_TYPE (v)) == UNION_TYPE)
>>>>>>> +		    /* if the record or union does not include a flexible array
>>>>>>> +		       recursively, compute the object size directly.  */
>>>>>>> 		      {
>>>>>>> -			/* compute object size only if v is not a
>>>>>>> -			   flexible array member.  */
>>>>>>> -			if (!is_flexible_array_mem_ref)
>>>>>>> +			if (!TYPE_INCLUDE_FLEXARRAY (TREE_TYPE (v)))
>>>>>>> 			  {
>>>>>>> 			    v = NULL_TREE;
>>>>>>> 			    break;
>>>>>>> 			  }
>>>>>>> -			v = TREE_OPERAND (v, 0);
>>>>>>> +			else
>>>>>>> +			  v = TREE_OPERAND (v, 0);
>>>>>>> 		      }
>>>>>>> -		    while (v != pt_var && TREE_CODE (v) == COMPONENT_REF)
>>>>>>> -		      if (TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
>>>>>>> -			  != UNION_TYPE
>>>>>>> -			  && TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
>>>>>>> -			  != QUAL_UNION_TYPE)
>>>>>>> -			break;
>>>>>>> -		      else
>>>>>>> -			v = TREE_OPERAND (v, 0);
>>>>>>> -		    if (v != pt_var)
>>>>>>> -		      v = NULL_TREE;
>>>>>>> 		    else
>>>>>>> -		      v = pt_var;
>>>>>>> +		      {
>>>>>>> +			/* Now the ref is to an array type.  */
>>>>>>> +			is_flexible_array_mem_ref
>>>>>>> +			  = array_ref_flexible_size_p (v);
>>>>>>> +			while (v != pt_var && TREE_CODE (v) == COMPONENT_REF)
>>>>>>> +			if (TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
>>>>>>> +			      != UNION_TYPE
>>>>>>> +			    && TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
>>>>>>> +				 != QUAL_UNION_TYPE)
>>>>>>> +			  break;
>>>>>>> +			else
>>>>>>> +			  v = TREE_OPERAND (v, 0);
>>>>>>> +			if (TREE_CODE (v) == COMPONENT_REF
>>>>>>> +			    && TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
>>>>>>> +				 == RECORD_TYPE)
>>>>>>> +			  {
>>>>>>> +			    /* compute object size only if v is not a
>>>>>>> +			       flexible array member.  */
>>>>>>> +			    if (!is_flexible_array_mem_ref)
>>>>>>> +			      {
>>>>>>> +				v = NULL_TREE;
>>>>>>> +				break;
>>>>>>> +			      }
>>>>>>> +			    v = TREE_OPERAND (v, 0);
>>>>>>> +			  }
>>>>>>> +			while (v != pt_var && TREE_CODE (v) == COMPONENT_REF)
>>>>>>> +			  if (TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
>>>>>>> +				!= UNION_TYPE
>>>>>>> +			      && TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
>>>>>>> +				   != QUAL_UNION_TYPE)
>>>>>>> +			    break;
>>>>>>> +			  else
>>>>>>> +			    v = TREE_OPERAND (v, 0);
>>>>>>> +			if (v != pt_var)
>>>>>>> +			  v = NULL_TREE;
>>>>>>> +			else
>>>>>>> +			  v = pt_var;
>>>>>>> +		      }
>>>>>>> 		    break;
>>>>>>> 		  default:
>>>>>>> 		    v = pt_var;
>>>>>>> diff --git a/gcc/tree-streamer-in.cc b/gcc/tree-streamer-in.cc
>>>>>>> index d4dc30f048f..c19ede0631d 100644
>>>>>>> --- a/gcc/tree-streamer-in.cc
>>>>>>> +++ b/gcc/tree-streamer-in.cc
>>>>>>> @@ -390,6 +390,7 @@ unpack_ts_type_common_value_fields (struct bitpack_d *bp, tree expr)
>>>>>>>    TYPE_TRANSPARENT_AGGR (expr) = (unsigned) bp_unpack_value (bp, 1);
>>>>>>>    TYPE_FINAL_P (expr) = (unsigned) bp_unpack_value (bp, 1);
>>>>>>>    TYPE_CXX_ODR_P (expr) = (unsigned) bp_unpack_value (bp, 1);
>>>>>>> +      TYPE_INCLUDE_FLEXARRAY (expr) = (unsigned) bp_unpack_value (bp, 1);
>>>>>>>  }
>>>>>>> else if (TREE_CODE (expr) == ARRAY_TYPE)
>>>>>>>  TYPE_NONALIASED_COMPONENT (expr) = (unsigned) bp_unpack_value (bp, 1);
>>>>>>> diff --git a/gcc/tree-streamer-out.cc b/gcc/tree-streamer-out.cc
>>>>>>> index d107229da5c..73e4b4e547c 100644
>>>>>>> --- a/gcc/tree-streamer-out.cc
>>>>>>> +++ b/gcc/tree-streamer-out.cc
>>>>>>> @@ -357,6 +357,7 @@ pack_ts_type_common_value_fields (struct bitpack_d *bp, tree expr)
>>>>>>>    bp_pack_value (bp, flag_wpa && TYPE_CANONICAL (expr)
>>>>>>> 			 ? TYPE_CXX_ODR_P (TYPE_CANONICAL (expr))
>>>>>>> 			 : TYPE_CXX_ODR_P (expr), 1);
>>>>>>> +      bp_pack_value (bp, TYPE_INCLUDE_FLEXARRAY (expr), 1);
>>>>>>>  }
>>>>>>> else if (TREE_CODE (expr) == ARRAY_TYPE)
>>>>>>>  bp_pack_value (bp, TYPE_NONALIASED_COMPONENT (expr), 1);
>>>>>>> diff --git a/gcc/tree.h b/gcc/tree.h
>>>>>>> index 92ac0e6a214..ab1cdc3dc85 100644
>>>>>>> --- a/gcc/tree.h
>>>>>>> +++ b/gcc/tree.h
>>>>>>> @@ -778,6 +778,12 @@ extern void omp_clause_range_check_failed (const_tree, const char *, int,
>>>>>>> #define TYPE_NO_NAMED_ARGS_STDARG_P(NODE) \
>>>>>>> (TYPE_CHECK (NODE)->type_common.no_named_args_stdarg_p)
>>>>>>> 
>>>>>>> +/* True if this RECORD_TYPE or UNION_TYPE includes a flexible array member
>>>>>>> +   at the last field recursively.  */
>>>>>>> +#define TYPE_INCLUDE_FLEXARRAY(NODE) \
>>>>>>> +  (TYPE_CHECK (NODE)->type_common.type_include_flexarray)
>>>>>> 
>>>>>> Please use RECORD_OR_UNION_CHECK.  
>>>>> Okay. 
>>>>>> The comment "at the last field"
>>>>>> doesn't seem to match implementation? (at least not obviously)
>>>>> The current implementation (in gcc/c/c-decl.cc) guarantees that it’s at the last field. (And recursively). 
>>>>>> Given this is a generic header expanding on what a "flexible array member"
>>>>>> is to the middle-end here would be good.  Especially the guarantees
>>>>>> made when the bit is _not_ set (we've evaded that with DECL_NOT_FLEXARRAY
>>>>>> vs. DECL_FLEXARRAY).
>>>>> 
>>>>> The bit TYPE_INCLUDE_FLEXARRAY default is FALSE, i.e, not including a flex array by default.
>>>>> It’s only set to TRUE when the struct/union includes a flex array at the last field (and recursively). So, I think it should be correct. 
>>>>> Let me know if I missed anything here.
>>>> 
>>>> See above - I was looking at the use (but I'm not familiar with that 
>>>> code), and wondered how the change affects uses from other frontends.
>>> 
>>> Please see my explanation above, I think the default behavior from other FE should be kept correctly with this patch.
>>> 
>>> thanks.
>>> 
>>> Qing
>>>> 
>>>> Richard.
>> 
>> 
> 
> -- 
> Richard Biener <rguenther@suse.de>
> SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg,
> Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman;
> HRB 36809 (AG Nuernberg)

[V4][PATCH 2/2] Update documentation to clarify a GCC extension

[Qing Zhao]

on a structure with a C99 flexible array member being nested in
another structure.

"GCC extension accepts a structure containing an ISO C99 "flexible array
member", or a union containing such a structure (possibly recursively)
to be a member of a structure.

 There are two situations:

   * The structure with a C99 flexible array member is the last field of
     another structure, for example:

          struct flex  { int length; char data[]; };
          union union_flex { int others; struct flex f; };

          struct out_flex_struct { int m; struct flex flex_data; };
          struct out_flex_union { int n; union union_flex flex_data; };

     In the above, both 'out_flex_struct.flex_data.data[]' and
     'out_flex_union.flex_data.f.data[]' are considered as flexible
     arrays too.

   * The structure with a C99 flexible array member is the middle field
     of another structure, for example:

          struct flex  { int length; char data[]; };

          struct mid_flex { int m; struct flex flex_data; int n; };

     In the above, 'mid_flex.flex_data.data[]' is allowed to be extended
     flexibly to the padding.  E.g, up to 4 elements.

     However, relying on space in struct padding is a bad programming
     practice, compilers do not handle such extension consistently, Any
     code relying on this behavior should be modified to ensure that
     flexible array members only end up at the ends of structures.

     Please use warning option '-Wgnu-variable-sized-type-not-at-end' to
     identify all such cases in the source code and modify them.  This
     extension will be deprecated from gcc in the next release.
"

gcc/c-family/ChangeLog:

	* c.opt: New option -Wgnu-variable-sized-type-not-at-end.

gcc/c/ChangeLog:

	* c-decl.cc (finish_struct): Issue warnings for new option.

gcc/ChangeLog:

	* doc/extend.texi: Document GCC extension on a structure containing
	a flexible array member to be a member of another structure.

gcc/testsuite/ChangeLog:

	* gcc.dg/variable-sized-type-flex-array.c: New test.
---
 gcc/c-family/c.opt                            |  5 ++
 gcc/c/c-decl.cc                               |  7 +++
 gcc/doc/extend.texi                           | 48 ++++++++++++++++++-
 .../gcc.dg/variable-sized-type-flex-array.c   | 31 ++++++++++++
 4 files changed, 90 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.dg/variable-sized-type-flex-array.c

diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 3333cddeece..660ac07f3d4 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -737,6 +737,11 @@ Wformat-truncation=
 C ObjC C++ LTO ObjC++ Joined RejectNegative UInteger Var(warn_format_trunc) Warning LangEnabledBy(C ObjC C++ LTO ObjC++,Wformat=, warn_format >= 1, 0) IntegerRange(0, 2)
 Warn about calls to snprintf and similar functions that truncate output.
 
+Wgnu-variable-sized-type-not-at-end
+C C++ Var(warn_variable_sized_type_not_at_end) Warning
+Warn about structures or unions with C99 flexible array members are not
+at the end of a structure.
+
 Wif-not-aligned
 C ObjC C++ ObjC++ Var(warn_if_not_aligned) Init(1) Warning
 Warn when the field in a struct is not aligned.
diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
index f589a2f5192..c5b54f07965 100644
--- a/gcc/c/c-decl.cc
+++ b/gcc/c/c-decl.cc
@@ -9296,6 +9296,13 @@ finish_struct (location_t loc, tree t, tree fieldlist, tree attributes,
 	       && is_last_field)
 	TYPE_INCLUDE_FLEXARRAY (t) = true;
 
+      if (warn_variable_sized_type_not_at_end
+	  && !is_last_field
+	  && TYPE_INCLUDE_FLEXARRAY (TREE_TYPE (x)))
+	warning_at (DECL_SOURCE_LOCATION (x),
+		    OPT_Wgnu_variable_sized_type_not_at_end,
+		    "variable sized type not at the end of a struct");
+
       if (DECL_NAME (x)
 	  || RECORD_OR_UNION_TYPE_P (TREE_TYPE (x)))
 	saw_named_field = true;
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index c1122916255..e278148c332 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -1748,7 +1748,53 @@ Flexible array members may only appear as the last member of a
 A structure containing a flexible array member, or a union containing
 such a structure (possibly recursively), may not be a member of a
 structure or an element of an array.  (However, these uses are
-permitted by GCC as extensions.)
+permitted by GCC as extensions, see details below.)
+@end itemize
+
+GCC extension accepts a structure containing an ISO C99 @dfn{flexible array
+member}, or a union containing such a structure (possibly recursively)
+to be a member of a structure.
+
+There are two situations:
+
+@itemize @bullet
+@item
+The structure with a C99 flexible array member is the last field of another
+structure, for example:
+
+@smallexample
+struct flex  @{ int length; char data[]; @};
+union union_flex @{ int others; struct flex f; @};
+
+struct out_flex_struct @{ int m; struct flex flex_data; @};
+struct out_flex_union @{ int n; union union_flex flex_data; @};
+@end smallexample
+
+In the above, both @code{out_flex_struct.flex_data.data[]} and
+@code{out_flex_union.flex_data.f.data[]} are considered as flexible arrays too.
+
+
+@item
+The structure with a C99 flexible array member is the middle field of another
+structure, for example:
+
+@smallexample
+struct flex  @{ int length; char data[]; @};
+
+struct mid_flex @{ int m; struct flex flex_data; int n; @};
+@end smallexample
+
+In the above, @code{mid_flex.flex_data.data[]} is allowed to be extended
+flexibly to the padding.  E.g, up to 4 elements.
+
+However, relying on space in struct padding is a bad programming practice,
+compilers do not handle such extension consistently, Any code relying on
+this behavior should be modified to ensure that flexible array members
+only end up at the ends of structures.
+
+Please use warning option  @option{-Wgnu-variable-sized-type-not-at-end} to
+identify all such cases in the source code and modify them.  This extension
+will be deprecated from gcc in the next release.
 @end itemize
 
 Non-empty initialization of zero-length
diff --git a/gcc/testsuite/gcc.dg/variable-sized-type-flex-array.c b/gcc/testsuite/gcc.dg/variable-sized-type-flex-array.c
new file mode 100644
index 00000000000..e3f65c5ed07
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/variable-sized-type-flex-array.c
@@ -0,0 +1,31 @@
+/* Test for -Wgnu-variable-sized-type-not-at-end on structure/union with 
+   C99 flexible array members being embedded into another structure.  */
+/* { dg-do compile } */
+/* { dg-options "-Wgnu-variable-sized-type-not-at-end" } */
+
+struct flex { int n; int data[]; };
+struct out_flex_end { int m; struct flex flex_data; }; /* { dg-bogus "variable sized type not at the end of a struct" } */
+struct out_flex_mid { struct flex flex_data; int m; };  /* { dg-warning "variable sized type not at the end of a struct" } */
+/* since the warning has been issued for out_flex_mid, no need to
+   issue warning again when it is included in another structure/union.  */
+struct outer_flex_mid { struct out_flex_mid out_flex_data; int p; }; /* { dg-bogus "variable sized type not at the end of a struct" } */
+union flex_union_mid { int a; struct outer_flex_mid b; }; /* { dg-bogus "variable sized type not at the end of a struct" } */
+
+
+struct flex0 { int n; int data[0]; };
+struct out_flex_end0 { int m; struct flex0 flex_data; }; /* { dg-bogus "variable sized type not at the end of a struct" } */
+struct out_flex_mid0 { struct flex0 flex_data; int m; };  /* { dg-bogus "variable sized type not at the end of a struct" } */
+struct outer_flex_mid0 { struct out_flex_mid0 out_flex_data; int p; }; /* { dg-bogus "variable sized type not at the end of a struct" } */
+union flex_union_mid0 { int a; struct outer_flex_mid0 b; }; /* { dg-bogus "variable sized type not at the end of a struct" } */
+
+struct flex1 { int n; int data[1]; };
+struct out_flex_end1 { int m; struct flex1 flex_data; }; /* { dg-bogus "variable sized type not at the end of a struct" } */
+struct out_flex_mid1 { struct flex1 flex_data; int m; }; /* { dg-bogus "variable sized type not at the end of a struct" } */ 
+struct outer_flex_mid1 { struct out_flex_mid1 out_flex_data; int p; }; /* { dg-bogus "variable sized type not at the end of a struct" } */
+union flex_union_mid1 { int a; struct outer_flex_mid1 b; }; /* { dg-bogus "variable sized type not at the end of a struct" } */
+
+struct flexn { int n; int data[8]; }; 
+struct out_flex_endn { int m; struct flexn flex_data; }; /* { dg-bogus "variable sized type not at the end of a struct" } */
+struct out_flex_midn { struct flexn flex_data; int m; }; /* { dg-bogus"variable sized type not at the end of a struct" } */ 
+struct outer_flex_midn { struct out_flex_midn out_flex_data; int p; }; /* { dg-bogus"variable sized type not at the end of a struct" } */
+union flex_union_midn { int a; struct outer_flex_midn b; }; /* { dg-bogus "variable sized type not at the end of a struct" } */
-- 
2.31.1

Re: [V4][PATCH 2/2] Update documentation to clarify a GCC extension

[Qing Zhao]

Ping.

Qing

> On Feb 24, 2023, at 1:35 PM, Qing Zhao <qing.zhao@oracle.com> wrote:
> 
> on a structure with a C99 flexible array member being nested in
> another structure.
> 
> "GCC extension accepts a structure containing an ISO C99 "flexible array
> member", or a union containing such a structure (possibly recursively)
> to be a member of a structure.
> 
> There are two situations:
> 
>   * The structure with a C99 flexible array member is the last field of
>     another structure, for example:
> 
>          struct flex  { int length; char data[]; };
>          union union_flex { int others; struct flex f; };
> 
>          struct out_flex_struct { int m; struct flex flex_data; };
>          struct out_flex_union { int n; union union_flex flex_data; };
> 
>     In the above, both 'out_flex_struct.flex_data.data[]' and
>     'out_flex_union.flex_data.f.data[]' are considered as flexible
>     arrays too.
> 
>   * The structure with a C99 flexible array member is the middle field
>     of another structure, for example:
> 
>          struct flex  { int length; char data[]; };
> 
>          struct mid_flex { int m; struct flex flex_data; int n; };
> 
>     In the above, 'mid_flex.flex_data.data[]' is allowed to be extended
>     flexibly to the padding.  E.g, up to 4 elements.
> 
>     However, relying on space in struct padding is a bad programming
>     practice, compilers do not handle such extension consistently, Any
>     code relying on this behavior should be modified to ensure that
>     flexible array members only end up at the ends of structures.
> 
>     Please use warning option '-Wgnu-variable-sized-type-not-at-end' to
>     identify all such cases in the source code and modify them.  This
>     extension will be deprecated from gcc in the next release.
> "
> 
> gcc/c-family/ChangeLog:
> 
> 	* c.opt: New option -Wgnu-variable-sized-type-not-at-end.
> 
> gcc/c/ChangeLog:
> 
> 	* c-decl.cc (finish_struct): Issue warnings for new option.
> 
> gcc/ChangeLog:
> 
> 	* doc/extend.texi: Document GCC extension on a structure containing
> 	a flexible array member to be a member of another structure.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.dg/variable-sized-type-flex-array.c: New test.
> ---
> gcc/c-family/c.opt                            |  5 ++
> gcc/c/c-decl.cc                               |  7 +++
> gcc/doc/extend.texi                           | 48 ++++++++++++++++++-
> .../gcc.dg/variable-sized-type-flex-array.c   | 31 ++++++++++++
> 4 files changed, 90 insertions(+), 1 deletion(-)
> create mode 100644 gcc/testsuite/gcc.dg/variable-sized-type-flex-array.c
> 
> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
> index 3333cddeece..660ac07f3d4 100644
> --- a/gcc/c-family/c.opt
> +++ b/gcc/c-family/c.opt
> @@ -737,6 +737,11 @@ Wformat-truncation=
> C ObjC C++ LTO ObjC++ Joined RejectNegative UInteger Var(warn_format_trunc) Warning LangEnabledBy(C ObjC C++ LTO ObjC++,Wformat=, warn_format >= 1, 0) IntegerRange(0, 2)
> Warn about calls to snprintf and similar functions that truncate output.
> 
> +Wgnu-variable-sized-type-not-at-end
> +C C++ Var(warn_variable_sized_type_not_at_end) Warning
> +Warn about structures or unions with C99 flexible array members are not
> +at the end of a structure.
> +
> Wif-not-aligned
> C ObjC C++ ObjC++ Var(warn_if_not_aligned) Init(1) Warning
> Warn when the field in a struct is not aligned.
> diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
> index f589a2f5192..c5b54f07965 100644
> --- a/gcc/c/c-decl.cc
> +++ b/gcc/c/c-decl.cc
> @@ -9296,6 +9296,13 @@ finish_struct (location_t loc, tree t, tree fieldlist, tree attributes,
> 	       && is_last_field)
> 	TYPE_INCLUDE_FLEXARRAY (t) = true;
> 
> +      if (warn_variable_sized_type_not_at_end
> +	  && !is_last_field
> +	  && TYPE_INCLUDE_FLEXARRAY (TREE_TYPE (x)))
> +	warning_at (DECL_SOURCE_LOCATION (x),
> +		    OPT_Wgnu_variable_sized_type_not_at_end,
> +		    "variable sized type not at the end of a struct");
> +
>       if (DECL_NAME (x)
> 	  || RECORD_OR_UNION_TYPE_P (TREE_TYPE (x)))
> 	saw_named_field = true;
> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> index c1122916255..e278148c332 100644
> --- a/gcc/doc/extend.texi
> +++ b/gcc/doc/extend.texi
> @@ -1748,7 +1748,53 @@ Flexible array members may only appear as the last member of a
> A structure containing a flexible array member, or a union containing
> such a structure (possibly recursively), may not be a member of a
> structure or an element of an array.  (However, these uses are
> -permitted by GCC as extensions.)
> +permitted by GCC as extensions, see details below.)
> +@end itemize
> +
> +GCC extension accepts a structure containing an ISO C99 @dfn{flexible array
> +member}, or a union containing such a structure (possibly recursively)
> +to be a member of a structure.
> +
> +There are two situations:
> +
> +@itemize @bullet
> +@item
> +The structure with a C99 flexible array member is the last field of another
> +structure, for example:
> +
> +@smallexample
> +struct flex  @{ int length; char data[]; @};
> +union union_flex @{ int others; struct flex f; @};
> +
> +struct out_flex_struct @{ int m; struct flex flex_data; @};
> +struct out_flex_union @{ int n; union union_flex flex_data; @};
> +@end smallexample
> +
> +In the above, both @code{out_flex_struct.flex_data.data[]} and
> +@code{out_flex_union.flex_data.f.data[]} are considered as flexible arrays too.
> +
> +
> +@item
> +The structure with a C99 flexible array member is the middle field of another
> +structure, for example:
> +
> +@smallexample
> +struct flex  @{ int length; char data[]; @};
> +
> +struct mid_flex @{ int m; struct flex flex_data; int n; @};
> +@end smallexample
> +
> +In the above, @code{mid_flex.flex_data.data[]} is allowed to be extended
> +flexibly to the padding.  E.g, up to 4 elements.
> +
> +However, relying on space in struct padding is a bad programming practice,
> +compilers do not handle such extension consistently, Any code relying on
> +this behavior should be modified to ensure that flexible array members
> +only end up at the ends of structures.
> +
> +Please use warning option  @option{-Wgnu-variable-sized-type-not-at-end} to
> +identify all such cases in the source code and modify them.  This extension
> +will be deprecated from gcc in the next release.
> @end itemize
> 
> Non-empty initialization of zero-length
> diff --git a/gcc/testsuite/gcc.dg/variable-sized-type-flex-array.c b/gcc/testsuite/gcc.dg/variable-sized-type-flex-array.c
> new file mode 100644
> index 00000000000..e3f65c5ed07
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/variable-sized-type-flex-array.c
> @@ -0,0 +1,31 @@
> +/* Test for -Wgnu-variable-sized-type-not-at-end on structure/union with 
> +   C99 flexible array members being embedded into another structure.  */
> +/* { dg-do compile } */
> +/* { dg-options "-Wgnu-variable-sized-type-not-at-end" } */
> +
> +struct flex { int n; int data[]; };
> +struct out_flex_end { int m; struct flex flex_data; }; /* { dg-bogus "variable sized type not at the end of a struct" } */
> +struct out_flex_mid { struct flex flex_data; int m; };  /* { dg-warning "variable sized type not at the end of a struct" } */
> +/* since the warning has been issued for out_flex_mid, no need to
> +   issue warning again when it is included in another structure/union.  */
> +struct outer_flex_mid { struct out_flex_mid out_flex_data; int p; }; /* { dg-bogus "variable sized type not at the end of a struct" } */
> +union flex_union_mid { int a; struct outer_flex_mid b; }; /* { dg-bogus "variable sized type not at the end of a struct" } */
> +
> +
> +struct flex0 { int n; int data[0]; };
> +struct out_flex_end0 { int m; struct flex0 flex_data; }; /* { dg-bogus "variable sized type not at the end of a struct" } */
> +struct out_flex_mid0 { struct flex0 flex_data; int m; };  /* { dg-bogus "variable sized type not at the end of a struct" } */
> +struct outer_flex_mid0 { struct out_flex_mid0 out_flex_data; int p; }; /* { dg-bogus "variable sized type not at the end of a struct" } */
> +union flex_union_mid0 { int a; struct outer_flex_mid0 b; }; /* { dg-bogus "variable sized type not at the end of a struct" } */
> +
> +struct flex1 { int n; int data[1]; };
> +struct out_flex_end1 { int m; struct flex1 flex_data; }; /* { dg-bogus "variable sized type not at the end of a struct" } */
> +struct out_flex_mid1 { struct flex1 flex_data; int m; }; /* { dg-bogus "variable sized type not at the end of a struct" } */ 
> +struct outer_flex_mid1 { struct out_flex_mid1 out_flex_data; int p; }; /* { dg-bogus "variable sized type not at the end of a struct" } */
> +union flex_union_mid1 { int a; struct outer_flex_mid1 b; }; /* { dg-bogus "variable sized type not at the end of a struct" } */
> +
> +struct flexn { int n; int data[8]; }; 
> +struct out_flex_endn { int m; struct flexn flex_data; }; /* { dg-bogus "variable sized type not at the end of a struct" } */
> +struct out_flex_midn { struct flexn flex_data; int m; }; /* { dg-bogus"variable sized type not at the end of a struct" } */ 
> +struct outer_flex_midn { struct out_flex_midn out_flex_data; int p; }; /* { dg-bogus"variable sized type not at the end of a struct" } */
> +union flex_union_midn { int a; struct outer_flex_midn b; }; /* { dg-bogus "variable sized type not at the end of a struct" } */
> -- 
> 2.31.1
> 

Re: [V4][PATCH 2/2] Update documentation to clarify a GCC extension

[Sandra Loosemore]

On 3/2/23 17:03, Qing Zhao via Gcc-patches wrote:
> Ping.

It looks to me like there is an associated code patch (for PR101832) 
that is still under technical discussion?  Or is this documentation 
patch independent of that change?

-Sandra

Re: [V4][PATCH 2/2] Update documentation to clarify a GCC extension

[Qing Zhao]

> On Mar 12, 2023, at 7:14 PM, Sandra Loosemore <sandra@codesourcery.com> wrote:
> 
> On 3/2/23 17:03, Qing Zhao via Gcc-patches wrote:
>> Ping.
> 
> It looks to me like there is an associated code patch (for PR101832) that is still under technical discussion?  

Yes, the 1st patch in this serie is the patch for PR101832.  Which is under review.
This 2nd patch is the associated doc change.
> Or is this documentation patch independent of that change?

These two patches are better to be associated together. But I think the patch for PR101832 might be
 put in first into GCC13 if people think the doc change is risky at this moment. 

Qing
> 
> -Sandra

Re: [V4][PATCH 2/2] Update documentation to clarify a GCC extension

[Sandra Loosemore]

On 2/24/23 11:35, Qing Zhao via Gcc-patches wrote:

> gcc/c-family/ChangeLog:
> 
> 	* c.opt: New option -Wgnu-variable-sized-type-not-at-end.
> 
> gcc/c/ChangeLog:
> 
> 	* c-decl.cc (finish_struct): Issue warnings for new option.
> 
> gcc/ChangeLog:
> 
> 	* doc/extend.texi: Document GCC extension on a structure containing
> 	a flexible array member to be a member of another structure.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.dg/variable-sized-type-flex-array.c: New test.

I'm only a documentation (and nios2) maintainer so I cannot approve 
adding a new option or warning.  I was going to review the documentation 
parts, at least, but I think this proposal is technically flawed because 
it is trying to document something that is undefined behavior in ways 
that it doesn't actually behave on all targets.

> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> index c1122916255..e278148c332 100644
> --- a/gcc/doc/extend.texi
> +++ b/gcc/doc/extend.texi
> @@ -1748,7 +1748,53 @@ Flexible array members may only appear as the last member of a
>   A structure containing a flexible array member, or a union containing
>   such a structure (possibly recursively), may not be a member of a
>   structure or an element of an array.  (However, these uses are
> -permitted by GCC as extensions.)
> +permitted by GCC as extensions, see details below.)
> +@end itemize
> +
> +GCC extension accepts a structure containing an ISO C99 @dfn{flexible array
> +member}, or a union containing such a structure (possibly recursively)
> +to be a member of a structure.
> +
> +There are two situations:
> +
> +@itemize @bullet
> +@item
> +The structure with a C99 flexible array member is the last field of another
> +structure, for example:
> +
> +@smallexample
> +struct flex  @{ int length; char data[]; @};
> +union union_flex @{ int others; struct flex f; @};
> +
> +struct out_flex_struct @{ int m; struct flex flex_data; @};
> +struct out_flex_union @{ int n; union union_flex flex_data; @};
> +@end smallexample
> +
> +In the above, both @code{out_flex_struct.flex_data.data[]} and
> +@code{out_flex_union.flex_data.f.data[]} are considered as flexible arrays too.
> +
> +
> +@item
> +The structure with a C99 flexible array member is the middle field of another
> +structure, for example:
> +
> +@smallexample
> +struct flex  @{ int length; char data[]; @};
> +
> +struct mid_flex @{ int m; struct flex flex_data; int n; @};
> +@end smallexample
> +
> +In the above, @code{mid_flex.flex_data.data[]} is allowed to be extended
> +flexibly to the padding.  E.g, up to 4 elements.

I think this paragraph is incorrect; how GCC lays out this structure 
depends on the target and ABI.  Looking at output from a GCC 12 
nios2-elf build I have handy, I see it is in fact laying out mid_flex so 
that member n is at the same address at offset 8 as flex_data.data[0], 
which is not useful at all.

> +However, relying on space in struct padding is a bad programming practice,
> +compilers do not handle such extension consistently, Any code relying on
> +this behavior should be modified to ensure that flexible array members
> +only end up at the ends of structures.
> +
> +Please use warning option  @option{-Wgnu-variable-sized-type-not-at-end} to
> +identify all such cases in the source code and modify them.  This extension
> +will be deprecated from gcc in the next release.

My suggestion would be to make this a hard error instead of a warning, 
unless there is some real body of code out there that depends on this 
feature on a target that actually does insert padding.  If it's a 
warning, it ought to be enabled by default.  And, rather than trying to 
document the behavior, the manual should just say it's undefined.

-Sandra

Re: [V4][PATCH 2/2] Update documentation to clarify a GCC extension

[Qing Zhao]

Hi, Sandra,

Thanks a lot for your review and comment.

Yes, the issue you raised in below was  a really tough one that I didn’t feel very comfortable to handle it well…

This documentation change is mainly to fix: PR77650 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77650).

The real user case for it was the old glibc headers. (As Joshph mentioned in the comment #1:
 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77650#c1)

And from my understanding, the code in glibc has already been fixed. 
https://gcc.gnu.org/pipermail/gcc-patches/2023-February/611220.html

But I am not sure in addition to Glibc, whether there are other use cases out there currently.

That’s the reason we cannot simply reject this case as an error at this moment. And the new warning -Wgnu-variable-sized-type-not-at-end
Is added to catch such case in user case to encourage user to update them. And then later we can completely delete this case from GCC support.

Right now, GCC’s implementation cannot handle such case consistently. So, how to document this case is really hard.

> On Mar 14, 2023, at 11:26 PM, Sandra Loosemore <sandra@codesourcery.com> wrote:
> 
> On 2/24/23 11:35, Qing Zhao via Gcc-patches wrote:
> 
>> gcc/c-family/ChangeLog:
>> 	* c.opt: New option -Wgnu-variable-sized-type-not-at-end.
>> gcc/c/ChangeLog:
>> 	* c-decl.cc (finish_struct): Issue warnings for new option.
>> gcc/ChangeLog:
>> 	* doc/extend.texi: Document GCC extension on a structure containing
>> 	a flexible array member to be a member of another structure.
>> gcc/testsuite/ChangeLog:
>> 	* gcc.dg/variable-sized-type-flex-array.c: New test.
> 
> I'm only a documentation (and nios2) maintainer so I cannot approve adding a new option or warning.  I was going to review the documentation parts, at least, but I think this proposal is technically flawed because it is trying to document something that is undefined behavior in ways that it doesn't actually behave on all targets.

So, should we mention it in the documentation at all?
Of mention it but specify the possible undefined behavior, the potential risk to use this case, and then warn the user to get rid of this usage with the new warning?

> 
>> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
>> index c1122916255..e278148c332 100644
>> --- a/gcc/doc/extend.texi
>> +++ b/gcc/doc/extend.texi
>> @@ -1748,7 +1748,53 @@ Flexible array members may only appear as the last member of a
>>  A structure containing a flexible array member, or a union containing
>>  such a structure (possibly recursively), may not be a member of a
>>  structure or an element of an array.  (However, these uses are
>> -permitted by GCC as extensions.)
>> +permitted by GCC as extensions, see details below.)
>> +@end itemize
>> +
>> +GCC extension accepts a structure containing an ISO C99 @dfn{flexible array
>> +member}, or a union containing such a structure (possibly recursively)
>> +to be a member of a structure.
>> +
>> +There are two situations:
>> +
>> +@itemize @bullet
>> +@item
>> +The structure with a C99 flexible array member is the last field of another
>> +structure, for example:
>> +
>> +@smallexample
>> +struct flex  @{ int length; char data[]; @};
>> +union union_flex @{ int others; struct flex f; @};
>> +
>> +struct out_flex_struct @{ int m; struct flex flex_data; @};
>> +struct out_flex_union @{ int n; union union_flex flex_data; @};
>> +@end smallexample
>> +
>> +In the above, both @code{out_flex_struct.flex_data.data[]} and
>> +@code{out_flex_union.flex_data.f.data[]} are considered as flexible arrays too.
>> +
>> +
>> +@item
>> +The structure with a C99 flexible array member is the middle field of another
>> +structure, for example:
>> +
>> +@smallexample
>> +struct flex  @{ int length; char data[]; @};
>> +
>> +struct mid_flex @{ int m; struct flex flex_data; int n; @};
>> +@end smallexample
>> +
>> +In the above, @code{mid_flex.flex_data.data[]} is allowed to be extended
>> +flexibly to the padding.  E.g, up to 4 elements.
> 
> I think this paragraph is incorrect; how GCC lays out this structure depends on the target and ABI.  Looking at output from a GCC 12 nios2-elf build I have handy, I see it is in fact laying out mid_flex so that member n is at the same address at offset 8 as flex_data.data[0], which is not useful at all.

I think that this behavior you mentioned is consistent with what’s the expected. See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77650#c5. 
But not sure whether we should document it or not?

>> +However, relying on space in struct padding is a bad programming practice,
>> +compilers do not handle such extension consistently, Any code relying on
>> +this behavior should be modified to ensure that flexible array members
>> +only end up at the ends of structures.
>> +
>> +Please use warning option  @option{-Wgnu-variable-sized-type-not-at-end} to
>> +identify all such cases in the source code and modify them.  This extension
>> +will be deprecated from gcc in the next release.
> 
> My suggestion would be to make this a hard error instead of a warning, unless there is some real body of code out there that depends on this feature on a target that actually does insert padding.  If it's a warning, it ought to be enabled by default.  And, rather than trying to document the behavior, the manual should just say it's undefined.

As I mentioned above, the warning was added mainly for unknown user cases, to warn them about this case and encourage them to update the code to prepare GCC deprecating this misfeature. 

Yes, I think “rather than trying to document the behavior, just say it’s undefined” might be more reasonable, I will update the doc based on this suggestion.

Thanks a lot.

Qing
> 
> -Sandra

Re: [V4][PATCH 0/2] Handle component_ref to a structure/union field including FAM for builtin_object_size

[Qing Zhao]

Ping

Qing

> On Feb 24, 2023, at 1:35 PM, Qing Zhao <qing.zhao@oracle.com> wrote:
> 
> Hi, Joseph and Richard,
> 
> Could you please review this patch and let me know whether it’s ready
> for committing into GCC13?
> 
> The fix to Bug PR101832 is an important patch for kernel security
> purpose. it's better to be put into GCC13.
> 
> =============================
> 
> These are the 4th version of the patches for PR101832, to fix
> builtin_object_size to correctly handle component_ref to a
> structure/union field that includes a flexible array member.
> 
> also includes a documentation update for the GCC extension on embedding
> a structure/union with flexible array member into another structure.
> which includes a fix to PR77650.
> 
> compared to the 3rd version of the patch, the major changes are:
> 
> 1. update the documentation part per Joseph's comments.
> 
> compared to the 2nd version of the patch, the major changes are:
> 
> 1. only include C99 flexible array member to this extension, trailing [0], [1]
>  and [4] are all excluded.
> 2. for the new bit type_include_flexarray in tree_type_common, print it
>  and also stream in/out it. 
> 3. update testing cases.
> 4. more clarification on the documentation. warnings for deprecating the 
>  case when the structure with C99 FAM is embedded in the middle of
>  another structure. 
> 5. add a new warning option -Wgnu-variable-sized-type-not-at-end for
>  identifing all such cases.
> 
> bootstrapped and regression tested on aarch64 and x86.
> 
> Okay for commit?
> 
> thanks.
> 
> Qing
> 
> Qing Zhao (2):
>  Handle component_ref to a structre/union field including C99 FAM
>    [PR101832]
>  Update documentation to clarify a GCC extension
> 
> gcc/c-family/c.opt                            |   5 +
> gcc/c/c-decl.cc                               |  19 +++
> gcc/cp/module.cc                              |   2 +
> gcc/doc/extend.texi                           |  48 ++++++-
> gcc/print-tree.cc                             |   5 +
> .../gcc.dg/builtin-object-size-pr101832.c     | 134 ++++++++++++++++++
> .../gcc.dg/variable-sized-type-flex-array.c   |  31 ++++
> gcc/tree-core.h                               |   4 +-
> gcc/tree-object-size.cc                       |  79 +++++++----
> gcc/tree-streamer-in.cc                       |   1 +
> gcc/tree-streamer-out.cc                      |   1 +
> gcc/tree.h                                    |   6 +
> 12 files changed, 305 insertions(+), 30 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c
> create mode 100644 gcc/testsuite/gcc.dg/variable-sized-type-flex-array.c
> 
> -- 
> 2.31.1
> 

Re: [V4][PATCH 0/2] Handle component_ref to a structure/union field including FAM for builtin_object_size

[Kees Cook]

On Fri, Feb 24, 2023 at 06:35:03PM +0000, Qing Zhao wrote:
> Hi, Joseph and Richard,
> 
> Could you please review this patch and let me know whether it’s ready
>  for committing into GCC13?
> 
> The fix to Bug PR101832 is an important patch for kernel security
> purpose. it's better to be put into GCC13.

Hi!

This series tests well for me. Getting this landed would be very nice
for the Linux kernel. :)

Are there any additional review notes for it, or is it ready to land?

Thanks!

-Kees

-- 
Kees Cook