public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [V9][PATCH 0/2] Accept and Handle the case when a structure including a FAM nested in another structure
@ 2023-05-30 18:30 Qing Zhao
  2023-05-30 18:30 ` [V9][PATCH 1/2] Handle component_ref to a structre/union field including flexible array member [PR101832] Qing Zhao
  2023-05-30 18:30 ` [V9][PATCH 2/2] Update documentation to clarify a GCC extension [PR77650] Qing Zhao
  0 siblings, 2 replies; 6+ messages in thread
From: Qing Zhao @ 2023-05-30 18:30 UTC (permalink / raw)
  To: joseph, richard.guenther, jakub, gcc-patches
  Cc: keescook, siddhesh, uecker, Qing Zhao

Hi,

This is the 8th version of the patch, which rebased on the latest trunk.
This is an important patch needed by Linux Kernel security project. 

compared to the 8th version, the Only change is in PATCH 2/2 (per
Joseph's comment):


diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 17ef80e75cc..e8a8be93ff1 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -1792,8 +1792,8 @@ consistently.  Any code relying on this case should be modified to ensure
that flexible array members only end up at the ends of structures.

Please use the warning option @option{-Wflex-array-member-not-at-end} to
-identify all such cases in the source code and modify them.  This warning
-will be on by default starting from GCC 15.
+identify all such cases in the source code and modify them.  This extension
+is now deprecated.
@end itemize 

all others keep the same as 8th version.

bootstrapped and regresson tested on aarch64 and x86.

Okay for commit?

thanks a lot.

Qing


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [V9][PATCH 1/2] Handle component_ref to a structre/union field including flexible array member [PR101832]
  2023-05-30 18:30 [V9][PATCH 0/2] Accept and Handle the case when a structure including a FAM nested in another structure Qing Zhao
@ 2023-05-30 18:30 ` Qing Zhao
  2023-06-05 15:12   ` Ping: Fwd: " Qing Zhao
  2023-05-30 18:30 ` [V9][PATCH 2/2] Update documentation to clarify a GCC extension [PR77650] Qing Zhao
  1 sibling, 1 reply; 6+ messages in thread
From: Qing Zhao @ 2023-05-30 18:30 UTC (permalink / raw)
  To: joseph, richard.guenther, jakub, gcc-patches
  Cc: keescook, siddhesh, uecker, Qing Zhao

Richard or Jakub, 

could you please review this patch and see whether it's Okay to commit?

thanks a lot.

Qing

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

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

gcc/c/ChangeLog:

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

gcc/lto/ChangeLog:

	PR tree-optimization/101832
	* lto-common.cc (compare_tree_sccs_1): Compare bit
	TYPE_NO_NAMED_ARGS_STDARG_P or TYPE_INCLUDES_FLEXARRAY properly
	for its corresponding type.

gcc/ChangeLog:

	PR tree-optimization/101832
	* print-tree.cc (print_node): Print new bit type_includes_flexarray.
	* tree-core.h (struct tree_type_common): Use bit no_named_args_stdarg_p
	as type_includes_flexarray for RECORD_TYPE or UNION_TYPE.
	* 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 bit no_named_args_stdarg_p properly for its corresponding type.
	* tree-streamer-out.cc (pack_ts_type_common_value_fields): Stream
	out bit no_named_args_stdarg_p properly for its corresponding type.
	* tree.h (TYPE_INCLUDES_FLEXARRAY): New macro TYPE_INCLUDES_FLEXARRAY.

gcc/testsuite/ChangeLog:

	PR tree-optimization/101832
	* gcc.dg/builtin-object-size-pr101832.c: New test.

change TYPE_INCLUDES_FLEXARRAY to TYPE_INCLUDES_FLEXARRAY
---
 gcc/c/c-decl.cc                               |  11 ++
 gcc/lto/lto-common.cc                         |   5 +-
 gcc/print-tree.cc                             |   5 +
 .../gcc.dg/builtin-object-size-pr101832.c     | 134 ++++++++++++++++++
 gcc/tree-core.h                               |   2 +
 gcc/tree-object-size.cc                       |  23 ++-
 gcc/tree-streamer-in.cc                       |   5 +-
 gcc/tree-streamer-out.cc                      |   5 +-
 gcc/tree.h                                    |   7 +-
 9 files changed, 192 insertions(+), 5 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 b5b491cf2da..0c718151f6d 100644
--- a/gcc/c/c-decl.cc
+++ b/gcc/c/c-decl.cc
@@ -9282,6 +9282,17 @@ 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_INCLUDES_FLEXARRAY for the context of x, t.
+	 when x is an array and is the last field.  */
+      if (TREE_CODE (TREE_TYPE (x)) == ARRAY_TYPE)
+	TYPE_INCLUDES_FLEXARRAY (t)
+	  = is_last_field && flexible_array_member_type_p (TREE_TYPE (x));
+      /* Recursively set TYPE_INCLUDES_FLEXARRAY for the context of x, t
+	 when x is an union or record and is the last field.  */
+      else if (RECORD_OR_UNION_TYPE_P (TREE_TYPE (x)))
+	TYPE_INCLUDES_FLEXARRAY (t)
+	  = is_last_field && TYPE_INCLUDES_FLEXARRAY (TREE_TYPE (x));
+
       if (DECL_NAME (x)
 	  || RECORD_OR_UNION_TYPE_P (TREE_TYPE (x)))
 	saw_named_field = true;
diff --git a/gcc/lto/lto-common.cc b/gcc/lto/lto-common.cc
index 537570204b3..f6b85bbc6f7 100644
--- a/gcc/lto/lto-common.cc
+++ b/gcc/lto/lto-common.cc
@@ -1275,7 +1275,10 @@ compare_tree_sccs_1 (tree t1, tree t2, tree **map)
       if (AGGREGATE_TYPE_P (t1))
 	compare_values (TYPE_TYPELESS_STORAGE);
       compare_values (TYPE_EMPTY_P);
-      compare_values (TYPE_NO_NAMED_ARGS_STDARG_P);
+      if (FUNC_OR_METHOD_TYPE_P (t1))
+	compare_values (TYPE_NO_NAMED_ARGS_STDARG_P);
+      if (RECORD_OR_UNION_TYPE_P (t1))
+	compare_values (TYPE_INCLUDES_FLEXARRAY);
       compare_values (TYPE_PACKED);
       compare_values (TYPE_RESTRICT);
       compare_values (TYPE_USER_ALIGN);
diff --git a/gcc/print-tree.cc b/gcc/print-tree.cc
index ccecd3dc6a7..62451b6cf4e 100644
--- a/gcc/print-tree.cc
+++ b/gcc/print-tree.cc
@@ -632,6 +632,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_INCLUDES_FLEXARRAY (node))
+	fputs (" includes-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 9d44c04bf03..b2f619c5efc 100644
--- a/gcc/tree-core.h
+++ b/gcc/tree-core.h
@@ -1712,6 +1712,8 @@ struct GTY(()) tree_type_common {
   unsigned typeless_storage : 1;
   unsigned empty_flag : 1;
   unsigned indivisible_p : 1;
+  /* TYPE_NO_NAMED_ARGS_STDARG_P for a stdarg function.
+     Or TYPE_INCLUDES_FLEXARRAY for RECORD_TYPE and UNION_TYPE.  */
   unsigned no_named_args_stdarg_p : 1;
   unsigned spare : 1;
 
diff --git a/gcc/tree-object-size.cc b/gcc/tree-object-size.cc
index 9a936a91983..a62af050056 100644
--- a/gcc/tree-object-size.cc
+++ b/gcc/tree-object-size.cc
@@ -633,11 +633,32 @@ 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 aggregate type, i.e, an array,
+		       a record or a union, it will not have flexible size,
+		       compute the object size directly.  */
+		    if (!AGGREGATE_TYPE_P (TREE_TYPE (v)))
 		      {
 			v = NULL_TREE;
 			break;
 		      }
+		    /* if the ref is to a record or union type, but the type
+		       does not include a flexible array recursively, compute
+		       the object size directly.  */
+		    if (RECORD_OR_UNION_TYPE_P (TREE_TYPE (v)))
+		      {
+			if (!TYPE_INCLUDES_FLEXARRAY (TREE_TYPE (v)))
+			  {
+			    v = NULL_TREE;
+			    break;
+			  }
+			else
+			  {
+			    v = TREE_OPERAND (v, 0);
+			    break;
+			  }
+		      }
+		    /* Now the ref is to an array type.  */
+		    gcc_assert (TREE_CODE (TREE_TYPE (v)) == 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)))
diff --git a/gcc/tree-streamer-in.cc b/gcc/tree-streamer-in.cc
index c803800862c..be2bdbb7699 100644
--- a/gcc/tree-streamer-in.cc
+++ b/gcc/tree-streamer-in.cc
@@ -386,7 +386,10 @@ unpack_ts_type_common_value_fields (struct bitpack_d *bp, tree expr)
   if (AGGREGATE_TYPE_P (expr))
     TYPE_TYPELESS_STORAGE (expr) = (unsigned) bp_unpack_value (bp, 1);
   TYPE_EMPTY_P (expr) = (unsigned) bp_unpack_value (bp, 1);
-  TYPE_NO_NAMED_ARGS_STDARG_P (expr) = (unsigned) bp_unpack_value (bp, 1);
+  if (FUNC_OR_METHOD_TYPE_P (expr))
+    TYPE_NO_NAMED_ARGS_STDARG_P (expr) = (unsigned) bp_unpack_value (bp, 1);
+  if (RECORD_OR_UNION_TYPE_P (expr))
+    TYPE_INCLUDES_FLEXARRAY (expr) = (unsigned) bp_unpack_value (bp, 1);
   TYPE_PRECISION (expr) = bp_unpack_var_len_unsigned (bp);
   SET_TYPE_ALIGN (expr, bp_unpack_var_len_unsigned (bp));
 #ifdef ACCEL_COMPILER
diff --git a/gcc/tree-streamer-out.cc b/gcc/tree-streamer-out.cc
index 5751f77273b..6d4a9d90da6 100644
--- a/gcc/tree-streamer-out.cc
+++ b/gcc/tree-streamer-out.cc
@@ -355,7 +355,10 @@ pack_ts_type_common_value_fields (struct bitpack_d *bp, tree expr)
   if (AGGREGATE_TYPE_P (expr))
     bp_pack_value (bp, TYPE_TYPELESS_STORAGE (expr), 1);
   bp_pack_value (bp, TYPE_EMPTY_P (expr), 1);
-  bp_pack_value (bp, TYPE_NO_NAMED_ARGS_STDARG_P (expr), 1);
+  if (FUNC_OR_METHOD_TYPE_P (expr))
+    bp_pack_value (bp, TYPE_NO_NAMED_ARGS_STDARG_P (expr), 1);
+  if (RECORD_OR_UNION_TYPE_P (expr))
+    bp_pack_value (bp, TYPE_INCLUDES_FLEXARRAY (expr), 1);
   bp_pack_var_len_unsigned (bp, TYPE_PRECISION (expr));
   bp_pack_var_len_unsigned (bp, TYPE_ALIGN (expr));
 }
diff --git a/gcc/tree.h b/gcc/tree.h
index 0b72663e6a1..d4755317a4c 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -786,7 +786,12 @@ extern void omp_clause_range_check_failed (const_tree, const char *, int,
    (...) 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)
+  (FUNC_OR_METHOD_CHECK (NODE)->type_common.no_named_args_stdarg_p)
+
+/* True if this RECORD_TYPE or UNION_TYPE includes a flexible array member
+   as the last field recursively.  */
+#define TYPE_INCLUDES_FLEXARRAY(NODE) \
+  (RECORD_OR_UNION_CHECK (NODE)->type_common.no_named_args_stdarg_p)
 
 /* In an IDENTIFIER_NODE, this means that assemble_name was called with
    this string as an argument.  */
-- 
2.31.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [V9][PATCH 2/2] Update documentation to clarify a GCC extension [PR77650]
  2023-05-30 18:30 [V9][PATCH 0/2] Accept and Handle the case when a structure including a FAM nested in another structure Qing Zhao
  2023-05-30 18:30 ` [V9][PATCH 1/2] Handle component_ref to a structre/union field including flexible array member [PR101832] Qing Zhao
@ 2023-05-30 18:30 ` Qing Zhao
  2023-05-30 23:25   ` Joseph Myers
  1 sibling, 1 reply; 6+ messages in thread
From: Qing Zhao @ 2023-05-30 18:30 UTC (permalink / raw)
  To: joseph, richard.guenther, jakub, gcc-patches
  Cc: keescook, siddhesh, uecker, Qing Zhao

Joseph,

could you please review this patch and see whether it's Okay for commit
now?

thanks a lot for all your comments and suggestions for this patch.

Qing.

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

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

"The 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:

   * A structure containing a C99 flexible array member, or a union
     containing such a structure, 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.

   * A structure containing a C99 flexible array member, or a union
     containing such a structure, is not the last 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, accessing a member of the array 'mid_flex.flex_data.data[]'
     might have undefined behavior.  Compilers do not handle such a case
     consistently, Any code relying on this case should be modified to ensure
     that flexible array members only end up at the ends of structures.

     Please use the warning option '-Wflex-array-member-not-at-end' to
     identify all such cases in the source code and modify them.  This extension
     is now deprecated.
"

gcc/c-family/ChangeLog:

	* c.opt: New option -Wflex-array-member-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                               |  9 ++++
 gcc/doc/extend.texi                           | 44 ++++++++++++++++++-
 .../gcc.dg/variable-sized-type-flex-array.c   | 31 +++++++++++++
 4 files changed, 88 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..c26d9801b63 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.
 
+Wflex-array-member-not-at-end
+C C++ Var(warn_flex_array_member_not_at_end) Warning
+Warn when a structure containing a C99 flexible array member as the last
+field is not at the end of another 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 0c718151f6d..a1a8e9bd341 100644
--- a/gcc/c/c-decl.cc
+++ b/gcc/c/c-decl.cc
@@ -9293,6 +9293,15 @@ finish_struct (location_t loc, tree t, tree fieldlist, tree attributes,
 	TYPE_INCLUDES_FLEXARRAY (t)
 	  = is_last_field && TYPE_INCLUDES_FLEXARRAY (TREE_TYPE (x));
 
+      if (warn_flex_array_member_not_at_end
+	  && !is_last_field
+	  && RECORD_OR_UNION_TYPE_P (TREE_TYPE (x))
+	  && TYPE_INCLUDES_FLEXARRAY (TREE_TYPE (x)))
+	warning_at (DECL_SOURCE_LOCATION (x),
+		    OPT_Wflex_array_member_not_at_end,
+		    "structure containing a flexible array member"
+		    " is not at the end of another structure");
+
       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 ed8b9c8a87b..aa1ecb6968e 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -1751,7 +1751,49 @@ 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
+
+The 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
+A structure containing a C99 flexible array member, or a union containing
+such a structure, 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
+A structure containing a C99 flexible array member, or a union containing
+such a structure, is not the last 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, accessing a member of the array @code{mid_flex.flex_data.data[]}
+might have undefined behavior.  Compilers do not handle such a case
+consistently.  Any code relying on this case should be modified to ensure
+that flexible array members only end up at the ends of structures.
+
+Please use the warning option @option{-Wflex-array-member-not-at-end} to
+identify all such cases in the source code and modify them.  This extension
+is now deprecated.
 @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..3924937bad4
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/variable-sized-type-flex-array.c
@@ -0,0 +1,31 @@
+/* Test for -Wflex-array-member-not-at-end on structure/union with 
+   C99 flexible array members being embedded into another structure.  */
+/* { dg-do compile } */
+/* { dg-options "-Wflex-array-member-not-at-end" } */
+
+struct flex { int n; int data[]; };
+struct out_flex_end { int m; struct flex flex_data; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+struct out_flex_mid { struct flex flex_data; int m; };  /* { dg-warning "structure containing a flexible array member is not at the end of another structure" } */
+/* 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 "structure containing a flexible array member is not at the end of another structure" } */
+union flex_union_mid { int a; struct outer_flex_mid b; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+
+
+struct flex0 { int n; int data[0]; };
+struct out_flex_end0 { int m; struct flex0 flex_data; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+struct out_flex_mid0 { struct flex0 flex_data; int m; };  /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+struct outer_flex_mid0 { struct out_flex_mid0 out_flex_data; int p; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+union flex_union_mid0 { int a; struct outer_flex_mid0 b; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+
+struct flex1 { int n; int data[1]; };
+struct out_flex_end1 { int m; struct flex1 flex_data; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+struct out_flex_mid1 { struct flex1 flex_data; int m; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */ 
+struct outer_flex_mid1 { struct out_flex_mid1 out_flex_data; int p; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+union flex_union_mid1 { int a; struct outer_flex_mid1 b; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+
+struct flexn { int n; int data[8]; }; 
+struct out_flex_endn { int m; struct flexn flex_data; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+struct out_flex_midn { struct flexn flex_data; int m; }; /* { dg-bogus"structure containing a flexible array member is not at the end of another structure" } */ 
+struct outer_flex_midn { struct out_flex_midn out_flex_data; int p; }; /* { dg-bogus"structure containing a flexible array member is not at the end of another structure" } */
+union flex_union_midn { int a; struct outer_flex_midn b; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
-- 
2.31.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [V9][PATCH 2/2] Update documentation to clarify a GCC extension [PR77650]
  2023-05-30 18:30 ` [V9][PATCH 2/2] Update documentation to clarify a GCC extension [PR77650] Qing Zhao
@ 2023-05-30 23:25   ` Joseph Myers
  0 siblings, 0 replies; 6+ messages in thread
From: Joseph Myers @ 2023-05-30 23:25 UTC (permalink / raw)
  To: Qing Zhao
  Cc: richard.guenther, jakub, gcc-patches, keescook, siddhesh, uecker

On Tue, 30 May 2023, Qing Zhao via Gcc-patches wrote:

> Joseph,
> 
> could you please review this patch and see whether it's Okay for commit
> now?

This version is OK.

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Ping: Fwd: [V9][PATCH 1/2] Handle component_ref to a structre/union field including flexible array member [PR101832]
  2023-05-30 18:30 ` [V9][PATCH 1/2] Handle component_ref to a structre/union field including flexible array member [PR101832] Qing Zhao
@ 2023-06-05 15:12   ` Qing Zhao
  2023-06-13 13:52     ` Ping * 2 : " Qing Zhao
  0 siblings, 1 reply; 6+ messages in thread
From: Qing Zhao @ 2023-06-05 15:12 UTC (permalink / raw)
  To: Jakub Jelinek, Richard Biener
  Cc: Joseph Myers, Kees Cook, Siddhesh Poyarekar, Martin Uecker,
	Qing Zhao via Gcc-patches

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

Ping on this patch.

The C FE and Doc changes has been approved.
Please help to review and approve the Middle-end change.

Or provide guide on how to move this patch forward.

Thanks a lot for the help.

Qing

Begin forwarded message:

From: Qing Zhao <qing.zhao@oracle.com<mailto:qing.zhao@oracle.com>>
Subject: [V9][PATCH 1/2] Handle component_ref to a structre/union field including flexible array member [PR101832]
Date: May 30, 2023 at 2:30:28 PM EDT
To: joseph@codesourcery.com<mailto:joseph@codesourcery.com>, richard.guenther@gmail.com<mailto:richard.guenther@gmail.com>, jakub@redhat.com<mailto:jakub@redhat.com>, gcc-patches@gcc.gnu.org<mailto:gcc-patches@gcc.gnu.org>
Cc: keescook@chromium.org<mailto:keescook@chromium.org>, siddhesh@gotplt.org<mailto:siddhesh@gotplt.org>, uecker@tugraz.at<mailto:uecker@tugraz.at>, Qing Zhao <qing.zhao@oracle.com<mailto:qing.zhao@oracle.com>>

Richard or Jakub,

could you please review this patch and see whether it's Okay to commit?

thanks a lot.

Qing

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

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

gcc/c/ChangeLog:

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

gcc/lto/ChangeLog:

PR tree-optimization/101832
* lto-common.cc<http://lto-common.cc> (compare_tree_sccs_1): Compare bit
TYPE_NO_NAMED_ARGS_STDARG_P or TYPE_INCLUDES_FLEXARRAY properly
for its corresponding type.

gcc/ChangeLog:

PR tree-optimization/101832
* print-tree.cc<http://print-tree.cc> (print_node): Print new bit type_includes_flexarray.
* tree-core.h (struct tree_type_common): Use bit no_named_args_stdarg_p
as type_includes_flexarray for RECORD_TYPE or UNION_TYPE.
* tree-object-size.cc<http://tree-object-size.cc> (addr_object_size): Handle structure/union type
when it has flexible size.
* tree-streamer-in.cc<http://tree-streamer-in.cc> (unpack_ts_type_common_value_fields): Stream
in bit no_named_args_stdarg_p properly for its corresponding type.
* tree-streamer-out.cc<http://tree-streamer-out.cc> (pack_ts_type_common_value_fields): Stream
out bit no_named_args_stdarg_p properly for its corresponding type.
* tree.h (TYPE_INCLUDES_FLEXARRAY): New macro TYPE_INCLUDES_FLEXARRAY.

gcc/testsuite/ChangeLog:

PR tree-optimization/101832
* gcc.dg/builtin-object-size-pr101832.c: New test.

change TYPE_INCLUDES_FLEXARRAY to TYPE_INCLUDES_FLEXARRAY
---
gcc/c/c-decl.cc<http://c-decl.cc>                               |  11 ++
gcc/lto/lto-common.cc<http://lto-common.cc>                         |   5 +-
gcc/print-tree.cc<http://print-tree.cc>                             |   5 +
.../gcc.dg/builtin-object-size-pr101832.c     | 134 ++++++++++++++++++
gcc/tree-core.h                               |   2 +
gcc/tree-object-size.cc<http://tree-object-size.cc>                       |  23 ++-
gcc/tree-streamer-in.cc<http://tree-streamer-in.cc>                       |   5 +-
gcc/tree-streamer-out.cc<http://tree-streamer-out.cc>                      |   5 +-
gcc/tree.h                                    |   7 +-
9 files changed, 192 insertions(+), 5 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c

diff --git a/gcc/c/c-decl.cc<http://c-decl.cc> b/gcc/c/c-decl.cc<http://c-decl.cc>
index b5b491cf2da..0c718151f6d 100644
--- a/gcc/c/c-decl.cc<http://c-decl.cc>
+++ b/gcc/c/c-decl.cc<http://c-decl.cc>
@@ -9282,6 +9282,17 @@ 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_INCLUDES_FLEXARRAY for the context of x, t.
+ when x is an array and is the last field.  */
+      if (TREE_CODE (TREE_TYPE (x)) == ARRAY_TYPE)
+ TYPE_INCLUDES_FLEXARRAY (t)
+  = is_last_field && flexible_array_member_type_p (TREE_TYPE (x));
+      /* Recursively set TYPE_INCLUDES_FLEXARRAY for the context of x, t
+ when x is an union or record and is the last field.  */
+      else if (RECORD_OR_UNION_TYPE_P (TREE_TYPE (x)))
+ TYPE_INCLUDES_FLEXARRAY (t)
+  = is_last_field && TYPE_INCLUDES_FLEXARRAY (TREE_TYPE (x));
+
      if (DECL_NAME (x)
 || RECORD_OR_UNION_TYPE_P (TREE_TYPE (x)))
saw_named_field = true;
diff --git a/gcc/lto/lto-common.cc<http://lto-common.cc> b/gcc/lto/lto-common.cc<http://lto-common.cc>
index 537570204b3..f6b85bbc6f7 100644
--- a/gcc/lto/lto-common.cc<http://lto-common.cc>
+++ b/gcc/lto/lto-common.cc<http://lto-common.cc>
@@ -1275,7 +1275,10 @@ compare_tree_sccs_1 (tree t1, tree t2, tree **map)
      if (AGGREGATE_TYPE_P (t1))
compare_values (TYPE_TYPELESS_STORAGE);
      compare_values (TYPE_EMPTY_P);
-      compare_values (TYPE_NO_NAMED_ARGS_STDARG_P);
+      if (FUNC_OR_METHOD_TYPE_P (t1))
+ compare_values (TYPE_NO_NAMED_ARGS_STDARG_P);
+      if (RECORD_OR_UNION_TYPE_P (t1))
+ compare_values (TYPE_INCLUDES_FLEXARRAY);
      compare_values (TYPE_PACKED);
      compare_values (TYPE_RESTRICT);
      compare_values (TYPE_USER_ALIGN);
diff --git a/gcc/print-tree.cc<http://print-tree.cc> b/gcc/print-tree.cc<http://print-tree.cc>
index ccecd3dc6a7..62451b6cf4e 100644
--- a/gcc/print-tree.cc<http://print-tree.cc>
+++ b/gcc/print-tree.cc<http://print-tree.cc>
@@ -632,6 +632,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_INCLUDES_FLEXARRAY (node))
+ fputs (" includes-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 9d44c04bf03..b2f619c5efc 100644
--- a/gcc/tree-core.h
+++ b/gcc/tree-core.h
@@ -1712,6 +1712,8 @@ struct GTY(()) tree_type_common {
  unsigned typeless_storage : 1;
  unsigned empty_flag : 1;
  unsigned indivisible_p : 1;
+  /* TYPE_NO_NAMED_ARGS_STDARG_P for a stdarg function.
+     Or TYPE_INCLUDES_FLEXARRAY for RECORD_TYPE and UNION_TYPE.  */
  unsigned no_named_args_stdarg_p : 1;
  unsigned spare : 1;

diff --git a/gcc/tree-object-size.cc<http://tree-object-size.cc> b/gcc/tree-object-size.cc<http://tree-object-size.cc>
index 9a936a91983..a62af050056 100644
--- a/gcc/tree-object-size.cc<http://tree-object-size.cc>
+++ b/gcc/tree-object-size.cc<http://tree-object-size.cc>
@@ -633,11 +633,32 @@ 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 aggregate type, i.e, an array,
+       a record or a union, it will not have flexible size,
+       compute the object size directly.  */
+    if (!AGGREGATE_TYPE_P (TREE_TYPE (v)))
     {
v = NULL_TREE;
break;
     }
+    /* if the ref is to a record or union type, but the type
+       does not include a flexible array recursively, compute
+       the object size directly.  */
+    if (RECORD_OR_UNION_TYPE_P (TREE_TYPE (v)))
+      {
+ if (!TYPE_INCLUDES_FLEXARRAY (TREE_TYPE (v)))
+  {
+    v = NULL_TREE;
+    break;
+  }
+ else
+  {
+    v = TREE_OPERAND (v, 0);
+    break;
+  }
+      }
+    /* Now the ref is to an array type.  */
+    gcc_assert (TREE_CODE (TREE_TYPE (v)) == 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)))
diff --git a/gcc/tree-streamer-in.cc<http://tree-streamer-in.cc> b/gcc/tree-streamer-in.cc<http://tree-streamer-in.cc>
index c803800862c..be2bdbb7699 100644
--- a/gcc/tree-streamer-in.cc<http://tree-streamer-in.cc>
+++ b/gcc/tree-streamer-in.cc<http://tree-streamer-in.cc>
@@ -386,7 +386,10 @@ unpack_ts_type_common_value_fields (struct bitpack_d *bp, tree expr)
  if (AGGREGATE_TYPE_P (expr))
    TYPE_TYPELESS_STORAGE (expr) = (unsigned) bp_unpack_value (bp, 1);
  TYPE_EMPTY_P (expr) = (unsigned) bp_unpack_value (bp, 1);
-  TYPE_NO_NAMED_ARGS_STDARG_P (expr) = (unsigned) bp_unpack_value (bp, 1);
+  if (FUNC_OR_METHOD_TYPE_P (expr))
+    TYPE_NO_NAMED_ARGS_STDARG_P (expr) = (unsigned) bp_unpack_value (bp, 1);
+  if (RECORD_OR_UNION_TYPE_P (expr))
+    TYPE_INCLUDES_FLEXARRAY (expr) = (unsigned) bp_unpack_value (bp, 1);
  TYPE_PRECISION (expr) = bp_unpack_var_len_unsigned (bp);
  SET_TYPE_ALIGN (expr, bp_unpack_var_len_unsigned (bp));
#ifdef ACCEL_COMPILER
diff --git a/gcc/tree-streamer-out.cc<http://tree-streamer-out.cc> b/gcc/tree-streamer-out.cc<http://tree-streamer-out.cc>
index 5751f77273b..6d4a9d90da6 100644
--- a/gcc/tree-streamer-out.cc<http://tree-streamer-out.cc>
+++ b/gcc/tree-streamer-out.cc<http://tree-streamer-out.cc>
@@ -355,7 +355,10 @@ pack_ts_type_common_value_fields (struct bitpack_d *bp, tree expr)
  if (AGGREGATE_TYPE_P (expr))
    bp_pack_value (bp, TYPE_TYPELESS_STORAGE (expr), 1);
  bp_pack_value (bp, TYPE_EMPTY_P (expr), 1);
-  bp_pack_value (bp, TYPE_NO_NAMED_ARGS_STDARG_P (expr), 1);
+  if (FUNC_OR_METHOD_TYPE_P (expr))
+    bp_pack_value (bp, TYPE_NO_NAMED_ARGS_STDARG_P (expr), 1);
+  if (RECORD_OR_UNION_TYPE_P (expr))
+    bp_pack_value (bp, TYPE_INCLUDES_FLEXARRAY (expr), 1);
  bp_pack_var_len_unsigned (bp, TYPE_PRECISION (expr));
  bp_pack_var_len_unsigned (bp, TYPE_ALIGN (expr));
}
diff --git a/gcc/tree.h b/gcc/tree.h
index 0b72663e6a1..d4755317a4c 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -786,7 +786,12 @@ extern void omp_clause_range_check_failed (const_tree, const char *, int,
   (...) 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)
+  (FUNC_OR_METHOD_CHECK (NODE)->type_common.no_named_args_stdarg_p)
+
+/* True if this RECORD_TYPE or UNION_TYPE includes a flexible array member
+   as the last field recursively.  */
+#define TYPE_INCLUDES_FLEXARRAY(NODE) \
+  (RECORD_OR_UNION_CHECK (NODE)->type_common.no_named_args_stdarg_p)

/* In an IDENTIFIER_NODE, this means that assemble_name was called with
   this string as an argument.  */
--
2.31.1



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Ping * 2 : Fwd: [V9][PATCH 1/2] Handle component_ref to a structre/union field including flexible array member [PR101832]
  2023-06-05 15:12   ` Ping: Fwd: " Qing Zhao
@ 2023-06-13 13:52     ` Qing Zhao
  0 siblings, 0 replies; 6+ messages in thread
From: Qing Zhao @ 2023-06-13 13:52 UTC (permalink / raw)
  To: Jakub Jelinek, Richard Biener, Qing Zhao via Gcc-patches
  Cc: Joseph Myers, Kees Cook, Siddhesh Poyarekar, Martin Uecker

Hi,

I’d like to ping this patch again for the Middle-end approval (on gcc/tree-object-size.cc change).
This is an important patch to Linux Kernel security. 

The patch has addressed all the comments and suggestions raised during the review process. 
The C FE, Doc changes has been approved.
Most of the Middle-end changes been reviewed by Richard Biener and have been updated based on his suggestions.

The only change that has not been reviewed is the simple change in gcc/tree-object-size.cc.  which is simple and straightforward.
Please review this change and let me know whether it’s okay for commit to trunk?

Thanks a lot!

Qing

> On Jun 5, 2023, at 11:12 AM, Qing Zhao via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
> 
> Ping on this patch.
> 
> The C FE and Doc changes has been approved.
> Please help to review and approve the Middle-end change.
> 
> Or provide guide on how to move this patch forward.
> 
> Thanks a lot for the help.
> 
> Qing
> 
> Begin forwarded message:
> 
> From: Qing Zhao <qing.zhao@oracle.com<mailto:qing.zhao@oracle.com>>
> Subject: [V9][PATCH 1/2] Handle component_ref to a structre/union field including flexible array member [PR101832]
> Date: May 30, 2023 at 2:30:28 PM EDT
> To: joseph@codesourcery.com<mailto:joseph@codesourcery.com>, richard.guenther@gmail.com<mailto:richard.guenther@gmail.com>, jakub@redhat.com<mailto:jakub@redhat.com>, gcc-patches@gcc.gnu.org<mailto:gcc-patches@gcc.gnu.org>
> Cc: keescook@chromium.org<mailto:keescook@chromium.org>, siddhesh@gotplt.org<mailto:siddhesh@gotplt.org>, uecker@tugraz.at<mailto:uecker@tugraz.at>, Qing Zhao <qing.zhao@oracle.com<mailto:qing.zhao@oracle.com>>
> 
> Richard or Jakub,
> 
> could you please review this patch and see whether it's Okay to commit?
> 
> thanks a lot.
> 
> Qing
> 
> ===================================
> 
> GCC extension accepts the case when a struct with a C99 flexible array member
> is embedded into another struct or union (possibly recursively) as the last
> field.
> __builtin_object_size should treat such struct as flexible size.
> 
> gcc/c/ChangeLog:
> 
> PR tree-optimization/101832
> * c-decl.cc<http://c-decl.cc> (finish_struct): Set TYPE_INCLUDES_FLEXARRAY for
> struct/union type.
> 
> gcc/lto/ChangeLog:
> 
> PR tree-optimization/101832
> * lto-common.cc<http://lto-common.cc> (compare_tree_sccs_1): Compare bit
> TYPE_NO_NAMED_ARGS_STDARG_P or TYPE_INCLUDES_FLEXARRAY properly
> for its corresponding type.
> 
> gcc/ChangeLog:
> 
> PR tree-optimization/101832
> * print-tree.cc<http://print-tree.cc> (print_node): Print new bit type_includes_flexarray.
> * tree-core.h (struct tree_type_common): Use bit no_named_args_stdarg_p
> as type_includes_flexarray for RECORD_TYPE or UNION_TYPE.
> * tree-object-size.cc<http://tree-object-size.cc> (addr_object_size): Handle structure/union type
> when it has flexible size.
> * tree-streamer-in.cc<http://tree-streamer-in.cc> (unpack_ts_type_common_value_fields): Stream
> in bit no_named_args_stdarg_p properly for its corresponding type.
> * tree-streamer-out.cc<http://tree-streamer-out.cc> (pack_ts_type_common_value_fields): Stream
> out bit no_named_args_stdarg_p properly for its corresponding type.
> * tree.h (TYPE_INCLUDES_FLEXARRAY): New macro TYPE_INCLUDES_FLEXARRAY.
> 
> gcc/testsuite/ChangeLog:
> 
> PR tree-optimization/101832
> * gcc.dg/builtin-object-size-pr101832.c: New test.
> 
> change TYPE_INCLUDES_FLEXARRAY to TYPE_INCLUDES_FLEXARRAY
> ---
> gcc/c/c-decl.cc<http://c-decl.cc>                               |  11 ++
> gcc/lto/lto-common.cc<http://lto-common.cc>                         |   5 +-
> gcc/print-tree.cc<http://print-tree.cc>                             |   5 +
> .../gcc.dg/builtin-object-size-pr101832.c     | 134 ++++++++++++++++++
> gcc/tree-core.h                               |   2 +
> gcc/tree-object-size.cc<http://tree-object-size.cc>                       |  23 ++-
> gcc/tree-streamer-in.cc<http://tree-streamer-in.cc>                       |   5 +-
> gcc/tree-streamer-out.cc<http://tree-streamer-out.cc>                      |   5 +-
> gcc/tree.h                                    |   7 +-
> 9 files changed, 192 insertions(+), 5 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c
> 
> diff --git a/gcc/c/c-decl.cc<http://c-decl.cc> b/gcc/c/c-decl.cc<http://c-decl.cc>
> index b5b491cf2da..0c718151f6d 100644
> --- a/gcc/c/c-decl.cc<http://c-decl.cc>
> +++ b/gcc/c/c-decl.cc<http://c-decl.cc>
> @@ -9282,6 +9282,17 @@ 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_INCLUDES_FLEXARRAY for the context of x, t.
> + when x is an array and is the last field.  */
> +      if (TREE_CODE (TREE_TYPE (x)) == ARRAY_TYPE)
> + TYPE_INCLUDES_FLEXARRAY (t)
> +  = is_last_field && flexible_array_member_type_p (TREE_TYPE (x));
> +      /* Recursively set TYPE_INCLUDES_FLEXARRAY for the context of x, t
> + when x is an union or record and is the last field.  */
> +      else if (RECORD_OR_UNION_TYPE_P (TREE_TYPE (x)))
> + TYPE_INCLUDES_FLEXARRAY (t)
> +  = is_last_field && TYPE_INCLUDES_FLEXARRAY (TREE_TYPE (x));
> +
>      if (DECL_NAME (x)
> || RECORD_OR_UNION_TYPE_P (TREE_TYPE (x)))
> saw_named_field = true;
> diff --git a/gcc/lto/lto-common.cc<http://lto-common.cc> b/gcc/lto/lto-common.cc<http://lto-common.cc>
> index 537570204b3..f6b85bbc6f7 100644
> --- a/gcc/lto/lto-common.cc<http://lto-common.cc>
> +++ b/gcc/lto/lto-common.cc<http://lto-common.cc>
> @@ -1275,7 +1275,10 @@ compare_tree_sccs_1 (tree t1, tree t2, tree **map)
>      if (AGGREGATE_TYPE_P (t1))
> compare_values (TYPE_TYPELESS_STORAGE);
>      compare_values (TYPE_EMPTY_P);
> -      compare_values (TYPE_NO_NAMED_ARGS_STDARG_P);
> +      if (FUNC_OR_METHOD_TYPE_P (t1))
> + compare_values (TYPE_NO_NAMED_ARGS_STDARG_P);
> +      if (RECORD_OR_UNION_TYPE_P (t1))
> + compare_values (TYPE_INCLUDES_FLEXARRAY);
>      compare_values (TYPE_PACKED);
>      compare_values (TYPE_RESTRICT);
>      compare_values (TYPE_USER_ALIGN);
> diff --git a/gcc/print-tree.cc<http://print-tree.cc> b/gcc/print-tree.cc<http://print-tree.cc>
> index ccecd3dc6a7..62451b6cf4e 100644
> --- a/gcc/print-tree.cc<http://print-tree.cc>
> +++ b/gcc/print-tree.cc<http://print-tree.cc>
> @@ -632,6 +632,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_INCLUDES_FLEXARRAY (node))
> + fputs (" includes-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 9d44c04bf03..b2f619c5efc 100644
> --- a/gcc/tree-core.h
> +++ b/gcc/tree-core.h
> @@ -1712,6 +1712,8 @@ struct GTY(()) tree_type_common {
>  unsigned typeless_storage : 1;
>  unsigned empty_flag : 1;
>  unsigned indivisible_p : 1;
> +  /* TYPE_NO_NAMED_ARGS_STDARG_P for a stdarg function.
> +     Or TYPE_INCLUDES_FLEXARRAY for RECORD_TYPE and UNION_TYPE.  */
>  unsigned no_named_args_stdarg_p : 1;
>  unsigned spare : 1;
> 
> diff --git a/gcc/tree-object-size.cc<http://tree-object-size.cc> b/gcc/tree-object-size.cc<http://tree-object-size.cc>
> index 9a936a91983..a62af050056 100644
> --- a/gcc/tree-object-size.cc<http://tree-object-size.cc>
> +++ b/gcc/tree-object-size.cc<http://tree-object-size.cc>
> @@ -633,11 +633,32 @@ 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 aggregate type, i.e, an array,
> +       a record or a union, it will not have flexible size,
> +       compute the object size directly.  */
> +    if (!AGGREGATE_TYPE_P (TREE_TYPE (v)))
>     {
> v = NULL_TREE;
> break;
>     }
> +    /* if the ref is to a record or union type, but the type
> +       does not include a flexible array recursively, compute
> +       the object size directly.  */
> +    if (RECORD_OR_UNION_TYPE_P (TREE_TYPE (v)))
> +      {
> + if (!TYPE_INCLUDES_FLEXARRAY (TREE_TYPE (v)))
> +  {
> +    v = NULL_TREE;
> +    break;
> +  }
> + else
> +  {
> +    v = TREE_OPERAND (v, 0);
> +    break;
> +  }
> +      }
> +    /* Now the ref is to an array type.  */
> +    gcc_assert (TREE_CODE (TREE_TYPE (v)) == 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)))
> diff --git a/gcc/tree-streamer-in.cc<http://tree-streamer-in.cc> b/gcc/tree-streamer-in.cc<http://tree-streamer-in.cc>
> index c803800862c..be2bdbb7699 100644
> --- a/gcc/tree-streamer-in.cc<http://tree-streamer-in.cc>
> +++ b/gcc/tree-streamer-in.cc<http://tree-streamer-in.cc>
> @@ -386,7 +386,10 @@ unpack_ts_type_common_value_fields (struct bitpack_d *bp, tree expr)
>  if (AGGREGATE_TYPE_P (expr))
>    TYPE_TYPELESS_STORAGE (expr) = (unsigned) bp_unpack_value (bp, 1);
>  TYPE_EMPTY_P (expr) = (unsigned) bp_unpack_value (bp, 1);
> -  TYPE_NO_NAMED_ARGS_STDARG_P (expr) = (unsigned) bp_unpack_value (bp, 1);
> +  if (FUNC_OR_METHOD_TYPE_P (expr))
> +    TYPE_NO_NAMED_ARGS_STDARG_P (expr) = (unsigned) bp_unpack_value (bp, 1);
> +  if (RECORD_OR_UNION_TYPE_P (expr))
> +    TYPE_INCLUDES_FLEXARRAY (expr) = (unsigned) bp_unpack_value (bp, 1);
>  TYPE_PRECISION (expr) = bp_unpack_var_len_unsigned (bp);
>  SET_TYPE_ALIGN (expr, bp_unpack_var_len_unsigned (bp));
> #ifdef ACCEL_COMPILER
> diff --git a/gcc/tree-streamer-out.cc<http://tree-streamer-out.cc> b/gcc/tree-streamer-out.cc<http://tree-streamer-out.cc>
> index 5751f77273b..6d4a9d90da6 100644
> --- a/gcc/tree-streamer-out.cc<http://tree-streamer-out.cc>
> +++ b/gcc/tree-streamer-out.cc<http://tree-streamer-out.cc>
> @@ -355,7 +355,10 @@ pack_ts_type_common_value_fields (struct bitpack_d *bp, tree expr)
>  if (AGGREGATE_TYPE_P (expr))
>    bp_pack_value (bp, TYPE_TYPELESS_STORAGE (expr), 1);
>  bp_pack_value (bp, TYPE_EMPTY_P (expr), 1);
> -  bp_pack_value (bp, TYPE_NO_NAMED_ARGS_STDARG_P (expr), 1);
> +  if (FUNC_OR_METHOD_TYPE_P (expr))
> +    bp_pack_value (bp, TYPE_NO_NAMED_ARGS_STDARG_P (expr), 1);
> +  if (RECORD_OR_UNION_TYPE_P (expr))
> +    bp_pack_value (bp, TYPE_INCLUDES_FLEXARRAY (expr), 1);
>  bp_pack_var_len_unsigned (bp, TYPE_PRECISION (expr));
>  bp_pack_var_len_unsigned (bp, TYPE_ALIGN (expr));
> }
> diff --git a/gcc/tree.h b/gcc/tree.h
> index 0b72663e6a1..d4755317a4c 100644
> --- a/gcc/tree.h
> +++ b/gcc/tree.h
> @@ -786,7 +786,12 @@ extern void omp_clause_range_check_failed (const_tree, const char *, int,
>   (...) 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)
> +  (FUNC_OR_METHOD_CHECK (NODE)->type_common.no_named_args_stdarg_p)
> +
> +/* True if this RECORD_TYPE or UNION_TYPE includes a flexible array member
> +   as the last field recursively.  */
> +#define TYPE_INCLUDES_FLEXARRAY(NODE) \
> +  (RECORD_OR_UNION_CHECK (NODE)->type_common.no_named_args_stdarg_p)
> 
> /* In an IDENTIFIER_NODE, this means that assemble_name was called with
>   this string as an argument.  */
> --
> 2.31.1
> 
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-06-13 13:52 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-30 18:30 [V9][PATCH 0/2] Accept and Handle the case when a structure including a FAM nested in another structure Qing Zhao
2023-05-30 18:30 ` [V9][PATCH 1/2] Handle component_ref to a structre/union field including flexible array member [PR101832] Qing Zhao
2023-06-05 15:12   ` Ping: Fwd: " Qing Zhao
2023-06-13 13:52     ` Ping * 2 : " Qing Zhao
2023-05-30 18:30 ` [V9][PATCH 2/2] Update documentation to clarify a GCC extension [PR77650] Qing Zhao
2023-05-30 23:25   ` Joseph Myers

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).