public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [V5][PATCH 0/2] Handle component_ref to a structure/union field including FAM for builtin_object_size
@ 2023-03-16 21:47 Qing Zhao
  2023-03-16 21:47 ` [V5][PATCH 1/2] Handle component_ref to a structre/union field including flexible array member [PR101832] Qing Zhao
                   ` (2 more replies)
  0 siblings, 3 replies; 20+ messages in thread
From: Qing Zhao @ 2023-03-16 21:47 UTC (permalink / raw)
  To: joseph, jakub, sandra
  Cc: rguenther, siddhesh, keescook, gcc-patches, Qing Zhao

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

Hi, Joseph, Jakub and Sandra,

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 5th 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 4th version of the patch, the major changes are:

1. Update the documentation per Sandra's comments and
suggestion.
2. per Richard's suggestion, let the new bit TYPE_INCLUDE_FLEXARRAY to
share the same bit with no_named_args_stdarg_p to save space in the IR.
and corresponding changes to support such sharing.
3. I also changed the code inside tree-object-size.cc to make it cleaner
and easier to be understood.

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  flexible
    array member [PR101832]
  Update documentation to clarify a GCC extension

 gcc/c-family/c.opt                            |   5 +
 gcc/c/c-decl.cc                               |  19 +++
 gcc/doc/extend.texi                           |  45 +++++-
 gcc/lto/lto-common.cc                         |   5 +-
 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                               |   2 +
 gcc/tree-object-size.cc                       |  23 ++-
 gcc/tree-streamer-in.cc                       |   5 +-
 gcc/tree-streamer-out.cc                      |   5 +-
 gcc/tree.h                                    |   7 +-
 12 files changed, 280 insertions(+), 6 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


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

* [V5][PATCH 1/2] Handle component_ref to a structre/union field including  flexible array member [PR101832]
  2023-03-16 21:47 [V5][PATCH 0/2] Handle component_ref to a structure/union field including FAM for builtin_object_size Qing Zhao
@ 2023-03-16 21:47 ` Qing Zhao
  2023-03-23 13:03   ` Fwd: " Qing Zhao
  2023-03-16 21:47 ` [V5][PATCH 2/2] Update documentation to clarify a GCC extension Qing Zhao
  2023-03-23 13:02 ` Fwd: [V5][PATCH 0/2] Handle component_ref to a structure/union field including FAM for builtin_object_size Qing Zhao
  2 siblings, 1 reply; 20+ messages in thread
From: Qing Zhao @ 2023-03-16 21:47 UTC (permalink / raw)
  To: joseph, jakub, sandra
  Cc: rguenther, siddhesh, keescook, gcc-patches, Qing Zhao

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

gcc/c/ChangeLog:

	PR tree-optimization/101832
	* c-decl.cc (finish_struct): Set TYPE_INCLUDE_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_INCLUDE_FLEXARRAY properly
	for its corresponding type.

gcc/ChangeLog:

	PR tree-optimization/101832
	* print-tree.cc (print_node): Print new bit type_include_flexarray.
	* tree-core.h (struct tree_type_common): Use bit no_named_args_stdarg_p
	as type_include_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_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                               |  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 e537d33f398..14c54809b9d 100644
--- a/gcc/c/c-decl.cc
+++ b/gcc/c/c-decl.cc
@@ -9258,6 +9258,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_INCLUDE_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_INCLUDE_FLEXARRAY (t)
+	  = is_last_field && flexible_array_member_type_p (TREE_TYPE (x));
+      /* Recursively set TYPE_INCLUDE_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_INCLUDE_FLEXARRAY (t)
+	  = is_last_field && TYPE_INCLUDE_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 882dd8971a4..9dde7118266 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_INCLUDE_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 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 fd2be57b78c..1d94c9a8ce2 100644
--- a/gcc/tree-core.h
+++ b/gcc/tree-core.h
@@ -1717,6 +1717,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_INCLUDE_FLEXARRAY
+     for RECORD_TYPE and UNION_TYPE.  */
   unsigned no_named_args_stdarg_p : 1;
   unsigned spare : 15;
 
diff --git a/gcc/tree-object-size.cc b/gcc/tree-object-size.cc
index 9a936a91983..1619d144ecd 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_INCLUDE_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 d4dc30f048f..56add06a70a 100644
--- a/gcc/tree-streamer-in.cc
+++ b/gcc/tree-streamer-in.cc
@@ -398,7 +398,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_INCLUDE_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 d107229da5c..64f6c8bbabb 100644
--- a/gcc/tree-streamer-out.cc
+++ b/gcc/tree-streamer-out.cc
@@ -365,7 +365,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_INCLUDE_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 91375f9652f..18c9f416758 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
+   at the last field recursively.  */
+#define TYPE_INCLUDE_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] 20+ messages in thread

* [V5][PATCH 2/2] Update documentation to clarify a GCC extension
  2023-03-16 21:47 [V5][PATCH 0/2] Handle component_ref to a structure/union field including FAM for builtin_object_size Qing Zhao
  2023-03-16 21:47 ` [V5][PATCH 1/2] Handle component_ref to a structre/union field including flexible array member [PR101832] Qing Zhao
@ 2023-03-16 21:47 ` Qing Zhao
  2023-03-23 13:05   ` Fwd: " Qing Zhao
  2023-03-23 13:02 ` Fwd: [V5][PATCH 0/2] Handle component_ref to a structure/union field including FAM for builtin_object_size Qing Zhao
  2 siblings, 1 reply; 20+ messages in thread
From: Qing Zhao @ 2023-03-16 21:47 UTC (permalink / raw)
  To: joseph, jakub, sandra
  Cc: rguenther, siddhesh, keescook, gcc-patches, Qing Zhao

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

"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 or a union 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.

   * A structure or a union 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[]' has undefined behavior.
     Compilers do not handle such case consistently, Any code relying on
     such case 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                               |  8 ++++
 gcc/doc/extend.texi                           | 45 ++++++++++++++++++-
 .../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..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 14c54809b9d..1632043a8ff 100644
--- a/gcc/c/c-decl.cc
+++ b/gcc/c/c-decl.cc
@@ -9269,6 +9269,14 @@ finish_struct (location_t loc, tree t, tree fieldlist, tree attributes,
 	TYPE_INCLUDE_FLEXARRAY (t)
 	  = is_last_field && TYPE_INCLUDE_FLEXARRAY (TREE_TYPE (x));
 
+      if (warn_variable_sized_type_not_at_end
+	  && !is_last_field
+	  && RECORD_OR_UNION_TYPE_P (TREE_TYPE (x))
+	  && 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 fd3745c5608..0928b962a60 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -1748,7 +1748,50 @@ 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
+A structure or a union 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
+A structure or a union 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[]} has undefined behavior.
+Compilers do not handle such case consistently, Any code relying on
+such case 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


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

* Fwd: [V5][PATCH 0/2] Handle component_ref to a structure/union field including FAM for builtin_object_size
  2023-03-16 21:47 [V5][PATCH 0/2] Handle component_ref to a structure/union field including FAM for builtin_object_size Qing Zhao
  2023-03-16 21:47 ` [V5][PATCH 1/2] Handle component_ref to a structre/union field including flexible array member [PR101832] Qing Zhao
  2023-03-16 21:47 ` [V5][PATCH 2/2] Update documentation to clarify a GCC extension Qing Zhao
@ 2023-03-23 13:02 ` Qing Zhao
  2 siblings, 0 replies; 20+ messages in thread
From: Qing Zhao @ 2023-03-23 13:02 UTC (permalink / raw)
  To: Joseph Myers, Jakub Jelinek, sandra
  Cc: Siddhesh Poyarekar, Richard Biener, Kees Cook, gcc Patches

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

Ping…

Please let me know if you have any further comments on the patch.

thanks.

Qing

Begin forwarded message:

From: Qing Zhao <qing.zhao@oracle.com<mailto:qing.zhao@oracle.com>>
Subject: [V5][PATCH 0/2] Handle component_ref to a structure/union field including FAM for builtin_object_size
Date: March 16, 2023 at 5:47:13 PM EDT
To: joseph@codesourcery.com<mailto:joseph@codesourcery.com>, jakub@redhat.com<mailto:jakub@redhat.com>, sandra@codesourcery.com<mailto:sandra@codesourcery.com>
Cc: rguenther@suse.de<mailto:rguenther@suse.de>, siddhesh@gotplt.org<mailto:siddhesh@gotplt.org>, keescook@chromium.org<mailto:keescook@chromium.org>, gcc-patches@gcc.gnu.org<mailto:gcc-patches@gcc.gnu.org>, Qing Zhao <qing.zhao@oracle.com<mailto:qing.zhao@oracle.com>>

Hi, Joseph, Jakub and Sandra,

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 5th 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 4th version of the patch, the major changes are:

1. Update the documentation per Sandra's comments and
suggestion.
2. per Richard's suggestion, let the new bit TYPE_INCLUDE_FLEXARRAY to
share the same bit with no_named_args_stdarg_p to save space in the IR.
and corresponding changes to support such sharing.
3. I also changed the code inside tree-object-size.cc<http://tree-object-size.cc> to make it cleaner
and easier to be understood.

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  flexible
   array member [PR101832]
 Update documentation to clarify a GCC extension

gcc/c-family/c.opt                            |   5 +
gcc/c/c-decl.cc<http://c-decl.cc>                               |  19 +++
gcc/doc/extend.texi                           |  45 +++++-
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.dg/variable-sized-type-flex-array.c   |  31 ++++
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 +-
12 files changed, 280 insertions(+), 6 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



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

* Fwd: [V5][PATCH 1/2] Handle component_ref to a structre/union field including  flexible array member [PR101832]
  2023-03-16 21:47 ` [V5][PATCH 1/2] Handle component_ref to a structre/union field including flexible array member [PR101832] Qing Zhao
@ 2023-03-23 13:03   ` Qing Zhao
  2023-03-23 18:55     ` Joseph Myers
  2023-03-27 14:22     ` Qing Zhao
  0 siblings, 2 replies; 20+ messages in thread
From: Qing Zhao @ 2023-03-23 13:03 UTC (permalink / raw)
  To: Joseph Myers, Jakub Jelinek, Sandra
  Cc: Siddhesh Poyarekar, Richard Biener, Kees Cook, gcc Patches

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

Ping…

Please let me know if you have any further comments on the patch.

thanks.

Qing


Begin forwarded message:

From: Qing Zhao <qing.zhao@oracle.com<mailto:qing.zhao@oracle.com>>
Subject: [V5][PATCH 1/2] Handle component_ref to a structre/union field including flexible array member [PR101832]
Date: March 16, 2023 at 5:47:14 PM EDT
To: joseph@codesourcery.com<mailto:joseph@codesourcery.com>, jakub@redhat.com<mailto:jakub@redhat.com>, sandra@codesourcery.com<mailto:sandra@codesourcery.com>
Cc: rguenther@suse.de<mailto:rguenther@suse.de>, siddhesh@gotplt.org<mailto:siddhesh@gotplt.org>, keescook@chromium.org<mailto:keescook@chromium.org>, gcc-patches@gcc.gnu.org<mailto:gcc-patches@gcc.gnu.org>, Qing Zhao <qing.zhao@oracle.com<mailto:qing.zhao@oracle.com>>

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

gcc/c/ChangeLog:

PR tree-optimization/101832
* c-decl.cc<http://c-decl.cc> (finish_struct): Set TYPE_INCLUDE_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_INCLUDE_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_include_flexarray.
* tree-core.h (struct tree_type_common): Use bit no_named_args_stdarg_p
as type_include_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_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<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 e537d33f398..14c54809b9d 100644
--- a/gcc/c/c-decl.cc<http://c-decl.cc>
+++ b/gcc/c/c-decl.cc<http://c-decl.cc>
@@ -9258,6 +9258,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_INCLUDE_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_INCLUDE_FLEXARRAY (t)
+  = is_last_field && flexible_array_member_type_p (TREE_TYPE (x));
+      /* Recursively set TYPE_INCLUDE_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_INCLUDE_FLEXARRAY (t)
+  = is_last_field && TYPE_INCLUDE_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 882dd8971a4..9dde7118266 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_INCLUDE_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 1f3afcbbc86..efacdb7686f 100644
--- a/gcc/print-tree.cc<http://print-tree.cc>
+++ b/gcc/print-tree.cc<http://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 fd2be57b78c..1d94c9a8ce2 100644
--- a/gcc/tree-core.h
+++ b/gcc/tree-core.h
@@ -1717,6 +1717,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_INCLUDE_FLEXARRAY
+     for RECORD_TYPE and UNION_TYPE.  */
  unsigned no_named_args_stdarg_p : 1;
  unsigned spare : 15;

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..1619d144ecd 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_INCLUDE_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 d4dc30f048f..56add06a70a 100644
--- a/gcc/tree-streamer-in.cc<http://tree-streamer-in.cc>
+++ b/gcc/tree-streamer-in.cc<http://tree-streamer-in.cc>
@@ -398,7 +398,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_INCLUDE_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 d107229da5c..64f6c8bbabb 100644
--- a/gcc/tree-streamer-out.cc<http://tree-streamer-out.cc>
+++ b/gcc/tree-streamer-out.cc<http://tree-streamer-out.cc>
@@ -365,7 +365,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_INCLUDE_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 91375f9652f..18c9f416758 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
+   at the last field recursively.  */
+#define TYPE_INCLUDE_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] 20+ messages in thread

* Fwd: [V5][PATCH 2/2] Update documentation to clarify a GCC extension
  2023-03-16 21:47 ` [V5][PATCH 2/2] Update documentation to clarify a GCC extension Qing Zhao
@ 2023-03-23 13:05   ` Qing Zhao
  2023-03-23 20:14     ` Joseph Myers
  0 siblings, 1 reply; 20+ messages in thread
From: Qing Zhao @ 2023-03-23 13:05 UTC (permalink / raw)
  To: Joseph Myers, Jakub Jelinek, sandra
  Cc: Siddhesh Poyarekar, Kees Cook, Richard Biener, gcc Patches

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

Ping…

Please let me know if you have any further comments on the patch.

thanks.

Qing

Begin forwarded message:

From: Qing Zhao <qing.zhao@oracle.com<mailto:qing.zhao@oracle.com>>
Subject: [V5][PATCH 2/2] Update documentation to clarify a GCC extension
Date: March 16, 2023 at 5:47:15 PM EDT
To: joseph@codesourcery.com<mailto:joseph@codesourcery.com>, jakub@redhat.com<mailto:jakub@redhat.com>, sandra@codesourcery.com<mailto:sandra@codesourcery.com>
Cc: rguenther@suse.de<mailto:rguenther@suse.de>, siddhesh@gotplt.org<mailto:siddhesh@gotplt.org>, keescook@chromium.org<mailto:keescook@chromium.org>, gcc-patches@gcc.gnu.org<mailto:gcc-patches@gcc.gnu.org>, Qing Zhao <qing.zhao@oracle.com<mailto:qing.zhao@oracle.com>>

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

"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 or a union 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.

  * A structure or a union 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[]' has undefined behavior.
    Compilers do not handle such case consistently, Any code relying on
    such case 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<http://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<http://c-decl.cc>                               |  8 ++++
gcc/doc/extend.texi                           | 45 ++++++++++++++++++-
.../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..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<http://c-decl.cc> b/gcc/c/c-decl.cc<http://c-decl.cc>
index 14c54809b9d..1632043a8ff 100644
--- a/gcc/c/c-decl.cc<http://c-decl.cc>
+++ b/gcc/c/c-decl.cc<http://c-decl.cc>
@@ -9269,6 +9269,14 @@ finish_struct (location_t loc, tree t, tree fieldlist, tree attributes,
TYPE_INCLUDE_FLEXARRAY (t)
 = is_last_field && TYPE_INCLUDE_FLEXARRAY (TREE_TYPE (x));

+      if (warn_variable_sized_type_not_at_end
+  && !is_last_field
+  && RECORD_OR_UNION_TYPE_P (TREE_TYPE (x))
+  && 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 fd3745c5608..0928b962a60 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -1748,7 +1748,50 @@ 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
+A structure or a union 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
+A structure or a union 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[]} has undefined behavior.
+Compilers do not handle such case consistently, Any code relying on
+such case 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



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

* Re: Fwd: [V5][PATCH 1/2] Handle component_ref to a structre/union field including  flexible array member [PR101832]
  2023-03-23 13:03   ` Fwd: " Qing Zhao
@ 2023-03-23 18:55     ` Joseph Myers
  2023-03-27 13:31       ` Qing Zhao
  2023-03-27 14:22     ` Qing Zhao
  1 sibling, 1 reply; 20+ messages in thread
From: Joseph Myers @ 2023-03-23 18:55 UTC (permalink / raw)
  To: Qing Zhao
  Cc: Jakub Jelinek, Sandra, Siddhesh Poyarekar, Richard Biener,
	Kees Cook, gcc Patches

On Thu, 23 Mar 2023, Qing Zhao via Gcc-patches wrote:

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

The C front-end changes are OK (supposing the original patch has correct 
whitespace, since it seems to be messed up here).

-- 
Joseph S. Myers
joseph@codesourcery.com

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

* Re: Fwd: [V5][PATCH 2/2] Update documentation to clarify a GCC extension
  2023-03-23 13:05   ` Fwd: " Qing Zhao
@ 2023-03-23 20:14     ` Joseph Myers
  2023-03-27 13:38       ` Qing Zhao
  0 siblings, 1 reply; 20+ messages in thread
From: Joseph Myers @ 2023-03-23 20:14 UTC (permalink / raw)
  To: Qing Zhao
  Cc: Jakub Jelinek, sandra, Siddhesh Poyarekar, Kees Cook,
	Richard Biener, gcc Patches

On Thu, 23 Mar 2023, Qing Zhao via Gcc-patches wrote:

> +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.

I think there's at least one word missing here, e.g. "that" before "are".

> +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.

We don't generally say "in the next release" in the manual (or "deprecated 
from gcc").  Maybe it *is* deprecated, maybe it will be *removed*, or will 
*start to warn by default*, in some specified version number (giving a 
version number seems better than "next release"), but "will be deprecated" 
is odd.

-- 
Joseph S. Myers
joseph@codesourcery.com

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

* Re: [V5][PATCH 1/2] Handle component_ref to a structre/union field including  flexible array member [PR101832]
  2023-03-23 18:55     ` Joseph Myers
@ 2023-03-27 13:31       ` Qing Zhao
  0 siblings, 0 replies; 20+ messages in thread
From: Qing Zhao @ 2023-03-27 13:31 UTC (permalink / raw)
  To: Joseph Myers
  Cc: Jakub Jelinek, Sandra, Siddhesh Poyarekar, Richard Biener,
	Kees Cook, gcc Patches



> On Mar 23, 2023, at 2:55 PM, Joseph Myers <joseph@codesourcery.com> wrote:
> 
> On Thu, 23 Mar 2023, Qing Zhao via Gcc-patches wrote:
> 
>> gcc/c/ChangeLog:
>> 
>> PR tree-optimization/101832
>> * c-decl.cc<http://c-decl.cc> (finish_struct): Set TYPE_INCLUDE_FLEXARRAY for
>> struct/union type.
> 
> The C front-end changes are OK (supposing the original patch has correct 
> whitespace, since it seems to be messed up here).

Thanks for your review.

I just double checked the change in gcc/c/c-decl.cc, looks like the whitespaces are good:

diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
index e537d33f398..14c54809b9d 100644
--- a/gcc/c/c-decl.cc
+++ b/gcc/c/c-decl.cc
@@ -9258,6 +9258,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_INCLUDE_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_INCLUDE_FLEXARRAY (t)
+	  = is_last_field && flexible_array_member_type_p (TREE_TYPE (x));
+      /* Recursively set TYPE_INCLUDE_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_INCLUDE_FLEXARRAY (t)
+	  = is_last_field && TYPE_INCLUDE_FLEXARRAY (TREE_TYPE (x));
+
       if (DECL_NAME (x)
 	  || RECORD_OR_UNION_TYPE_P (TREE_TYPE (x)))
 	saw_named_field = true;

I guess that the git send-mail might mess up them. -:).

Qing

> 
> -- 
> Joseph S. Myers
> joseph@codesourcery.com


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

* Re: [V5][PATCH 2/2] Update documentation to clarify a GCC extension
  2023-03-23 20:14     ` Joseph Myers
@ 2023-03-27 13:38       ` Qing Zhao
  2023-03-27 14:34         ` Xi Ruoyao
  2023-03-27 15:43         ` Jakub Jelinek
  0 siblings, 2 replies; 20+ messages in thread
From: Qing Zhao @ 2023-03-27 13:38 UTC (permalink / raw)
  To: Joseph Myers
  Cc: Jakub Jelinek, sandra, Siddhesh Poyarekar, Kees Cook,
	Richard Biener, gcc Patches



> On Mar 23, 2023, at 4:14 PM, Joseph Myers <joseph@codesourcery.com> wrote:
> 
> On Thu, 23 Mar 2023, Qing Zhao via Gcc-patches wrote:
> 
>> +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.
> 
> I think there's at least one word missing here, e.g. "that" before "are".

Will fix it.
> 
>> +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.
> 
> We don't generally say "in the next release" in the manual (or "deprecated 
> from gcc").  Maybe it *is* deprecated, maybe it will be *removed*, or will 
> *start to warn by default*, in some specified version number (giving a 
> version number seems better than "next release"), but "will be deprecated" 
> is odd.
How about the following:

+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 warning will be 
+ on by default starting from GCC14.

Thanks.

Qing

> 
> -- 
> Joseph S. Myers
> joseph@codesourcery.com


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

* Re: [V5][PATCH 1/2] Handle component_ref to a structre/union field including  flexible array member [PR101832]
  2023-03-23 13:03   ` Fwd: " Qing Zhao
  2023-03-23 18:55     ` Joseph Myers
@ 2023-03-27 14:22     ` Qing Zhao
  1 sibling, 0 replies; 20+ messages in thread
From: Qing Zhao @ 2023-03-27 14:22 UTC (permalink / raw)
  To: Jakub Jelinek
  Cc: Joseph Myers, Sandra, Siddhesh Poyarekar, Richard Biener,
	Kees Cook, gcc Patches

Hi, Jakub,

Could you please review the middle end part of the changes of this patch? (The C FE part changes were Okayed by Joseph already).

The major change is in tree-object-size.cc (addr_object_size). (To use the new TYPE_INCLUDE_FLEXARRAY info). 

This patch is to fix PR101832(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101832), and is needed for Linux Kernel security.  It’s better to be put into GCC13.

Thanks a lot!

https://gcc.gnu.org/pipermail/gcc-patches/2023-March/614101.html
https://gcc.gnu.org/pipermail/gcc-patches/2023-March/614511.html

> On Mar 23, 2023, at 9:03 AM, Qing Zhao via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
> 
> Ping…
> 
> Please let me know if you have any further comments on the patch.
> 
> thanks.
> 
> Qing
> 
> 
> Begin forwarded message:
> 
> From: Qing Zhao <qing.zhao@oracle.com<mailto:qing.zhao@oracle.com>>
> Subject: [V5][PATCH 1/2] Handle component_ref to a structre/union field including flexible array member [PR101832]
> Date: March 16, 2023 at 5:47:14 PM EDT
> To: joseph@codesourcery.com<mailto:joseph@codesourcery.com>, jakub@redhat.com<mailto:jakub@redhat.com>, sandra@codesourcery.com<mailto:sandra@codesourcery.com>
> Cc: rguenther@suse.de<mailto:rguenther@suse.de>, siddhesh@gotplt.org<mailto:siddhesh@gotplt.org>, keescook@chromium.org<mailto:keescook@chromium.org>, gcc-patches@gcc.gnu.org<mailto:gcc-patches@gcc.gnu.org>, Qing Zhao <qing.zhao@oracle.com<mailto:qing.zhao@oracle.com>>
> 
> GCC extension accepts the case when a struct with a flexible array member
> is embedded into another struct or union (possibly recursively).
> __builtin_object_size should treat such struct as flexible size per
> -fstrict-flex-arrays.
> 
> gcc/c/ChangeLog:
> 
> PR tree-optimization/101832
> * c-decl.cc<http://c-decl.cc> (finish_struct): Set TYPE_INCLUDE_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_INCLUDE_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_include_flexarray.
> * tree-core.h (struct tree_type_common): Use bit no_named_args_stdarg_p
> as type_include_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_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<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 e537d33f398..14c54809b9d 100644
> --- a/gcc/c/c-decl.cc<http://c-decl.cc>
> +++ b/gcc/c/c-decl.cc<http://c-decl.cc>
> @@ -9258,6 +9258,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_INCLUDE_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_INCLUDE_FLEXARRAY (t)
> +  = is_last_field && flexible_array_member_type_p (TREE_TYPE (x));
> +      /* Recursively set TYPE_INCLUDE_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_INCLUDE_FLEXARRAY (t)
> +  = is_last_field && TYPE_INCLUDE_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 882dd8971a4..9dde7118266 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_INCLUDE_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 1f3afcbbc86..efacdb7686f 100644
> --- a/gcc/print-tree.cc<http://print-tree.cc>
> +++ b/gcc/print-tree.cc<http://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 fd2be57b78c..1d94c9a8ce2 100644
> --- a/gcc/tree-core.h
> +++ b/gcc/tree-core.h
> @@ -1717,6 +1717,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_INCLUDE_FLEXARRAY
> +     for RECORD_TYPE and UNION_TYPE.  */
>  unsigned no_named_args_stdarg_p : 1;
>  unsigned spare : 15;
> 
> 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..1619d144ecd 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_INCLUDE_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 d4dc30f048f..56add06a70a 100644
> --- a/gcc/tree-streamer-in.cc<http://tree-streamer-in.cc>
> +++ b/gcc/tree-streamer-in.cc<http://tree-streamer-in.cc>
> @@ -398,7 +398,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_INCLUDE_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 d107229da5c..64f6c8bbabb 100644
> --- a/gcc/tree-streamer-out.cc<http://tree-streamer-out.cc>
> +++ b/gcc/tree-streamer-out.cc<http://tree-streamer-out.cc>
> @@ -365,7 +365,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_INCLUDE_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 91375f9652f..18c9f416758 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
> +   at the last field recursively.  */
> +#define TYPE_INCLUDE_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] 20+ messages in thread

* Re: [V5][PATCH 2/2] Update documentation to clarify a GCC extension
  2023-03-27 13:38       ` Qing Zhao
@ 2023-03-27 14:34         ` Xi Ruoyao
  2023-03-27 15:37           ` Qing Zhao
  2023-03-27 15:43         ` Jakub Jelinek
  1 sibling, 1 reply; 20+ messages in thread
From: Xi Ruoyao @ 2023-03-27 14:34 UTC (permalink / raw)
  To: Qing Zhao, Joseph Myers
  Cc: Jakub Jelinek, sandra, Siddhesh Poyarekar, Kees Cook,
	Richard Biener, gcc Patches

On Mon, 2023-03-27 at 13:38 +0000, Qing Zhao via Gcc-patches wrote:
> 
> 
> > On Mar 23, 2023, at 4:14 PM, Joseph Myers <joseph@codesourcery.com> wrote:
> > 
> > On Thu, 23 Mar 2023, Qing Zhao via Gcc-patches wrote:
> > 
> > > +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.
> > 
> > I think there's at least one word missing here, e.g. "that" before "are".
> 
> Will fix it.
> > 
> > > +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.
> > 
> > We don't generally say "in the next release" in the manual (or "deprecated 
> > from gcc").  Maybe it *is* deprecated, maybe it will be *removed*, or will 
> > *start to warn by default*, in some specified version number (giving a 
> > version number seems better than "next release"), but "will be deprecated" 
> > is odd.
> How about the following:
> 
> +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 warning will be 
> + on by default starting from GCC14.

I'm wondering why it *was" not on by default... 


-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

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

* Re: [V5][PATCH 2/2] Update documentation to clarify a GCC extension
  2023-03-27 14:34         ` Xi Ruoyao
@ 2023-03-27 15:37           ` Qing Zhao
  0 siblings, 0 replies; 20+ messages in thread
From: Qing Zhao @ 2023-03-27 15:37 UTC (permalink / raw)
  To: Xi Ruoyao
  Cc: Joseph Myers, Jakub Jelinek, sandra, Siddhesh Poyarekar,
	Kees Cook, Richard Biener, gcc Patches



> On Mar 27, 2023, at 10:34 AM, Xi Ruoyao <xry111@xry111.site> wrote:
> 
> On Mon, 2023-03-27 at 13:38 +0000, Qing Zhao via Gcc-patches wrote:
>> 
>> 
>>> On Mar 23, 2023, at 4:14 PM, Joseph Myers <joseph@codesourcery.com> wrote:
>>> 
>>> On Thu, 23 Mar 2023, Qing Zhao via Gcc-patches wrote:
>>> 
>>>> +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.
>>> 
>>> I think there's at least one word missing here, e.g. "that" before "are".
>> 
>> Will fix it.
>>> 
>>>> +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.
>>> 
>>> We don't generally say "in the next release" in the manual (or "deprecated 
>>> from gcc").  Maybe it *is* deprecated, maybe it will be *removed*, or will 
>>> *start to warn by default*, in some specified version number (giving a 
>>> version number seems better than "next release"), but "will be deprecated" 
>>> is odd.
>> How about the following:
>> 
>> +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 warning will be 
>> + on by default starting from GCC14.
> 
> I'm wondering why it *was" not on by default... 

This is a new warning that will be added to gcc13, since it’s in a very late stage before gcc13 release,
So I am not feeling comfortable to turn it on by default now. 
I think it might be safer to turn it on by default in the beginning of gcc14.

Qing
> 
> 
> -- 
> Xi Ruoyao <xry111@xry111.site>
> School of Aerospace Science and Technology, Xidian University


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

* Re: [V5][PATCH 2/2] Update documentation to clarify a GCC extension
  2023-03-27 13:38       ` Qing Zhao
  2023-03-27 14:34         ` Xi Ruoyao
@ 2023-03-27 15:43         ` Jakub Jelinek
  2023-03-27 15:57           ` Qing Zhao
  1 sibling, 1 reply; 20+ messages in thread
From: Jakub Jelinek @ 2023-03-27 15:43 UTC (permalink / raw)
  To: Qing Zhao
  Cc: Joseph Myers, sandra, Siddhesh Poyarekar, Kees Cook,
	Richard Biener, gcc Patches

On Mon, Mar 27, 2023 at 01:38:34PM +0000, Qing Zhao wrote:
> 
> 
> > On Mar 23, 2023, at 4:14 PM, Joseph Myers <joseph@codesourcery.com> wrote:
> > 
> > On Thu, 23 Mar 2023, Qing Zhao via Gcc-patches wrote:
> > 
> >> +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.
> > 
> > I think there's at least one word missing here, e.g. "that" before "are".
> 
> Will fix it.
> > 
> >> +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.
> > 
> > We don't generally say "in the next release" in the manual (or "deprecated 
> > from gcc").  Maybe it *is* deprecated, maybe it will be *removed*, or will 
> > *start to warn by default*, in some specified version number (giving a 
> > version number seems better than "next release"), but "will be deprecated" 
> > is odd.
> How about the following:
> 
> +Please use warning option  @option{-Wgnu-variable-sized-type-not-at-end} to

This is certainly misnamed.  GNU variable sized type not at the end of a
struct is something like
void bar (void *);
void foo (int n) {
  struct S { int a; int b[n]; int c; } s;
  s.a = 1;
  __builtin_memset (s.b, 0, sizeof (s.b));
  s.c = 3;
  bar (&s);
}
Certainly not flexible array members in the middle of structure.

> +identify all such cases in the source code and modify them.  This warning will be 
> + on by default starting from GCC14.

	Jakub


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

* Re: [V5][PATCH 2/2] Update documentation to clarify a GCC extension
  2023-03-27 15:43         ` Jakub Jelinek
@ 2023-03-27 15:57           ` Qing Zhao
  2023-03-27 16:06             ` Jakub Jelinek
  0 siblings, 1 reply; 20+ messages in thread
From: Qing Zhao @ 2023-03-27 15:57 UTC (permalink / raw)
  To: Jakub Jelinek
  Cc: Joseph Myers, sandra, Siddhesh Poyarekar, Kees Cook,
	Richard Biener, gcc Patches



> On Mar 27, 2023, at 11:43 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> 
> On Mon, Mar 27, 2023 at 01:38:34PM +0000, Qing Zhao wrote:
>> 
>> 
>>> On Mar 23, 2023, at 4:14 PM, Joseph Myers <joseph@codesourcery.com> wrote:
>>> 
>>> On Thu, 23 Mar 2023, Qing Zhao via Gcc-patches wrote:
>>> 
>>>> +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.
>>> 
>>> I think there's at least one word missing here, e.g. "that" before "are".
>> 
>> Will fix it.
>>> 
>>>> +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.
>>> 
>>> We don't generally say "in the next release" in the manual (or "deprecated 
>>> from gcc").  Maybe it *is* deprecated, maybe it will be *removed*, or will 
>>> *start to warn by default*, in some specified version number (giving a 
>>> version number seems better than "next release"), but "will be deprecated" 
>>> is odd.
>> How about the following:
>> 
>> +Please use warning option  @option{-Wgnu-variable-sized-type-not-at-end} to
> This is certainly misnamed.

The name “-Wgnu-variable-sized-type-not-at-end” was just used the warning name from CLANG. -:)

Shall we use the same name as CLANG? Or we invent a new name?

>  GNU variable sized type not at the end of a
> struct is something like
> void bar (void *);
> void foo (int n) {
>  struct S { int a; int b[n]; int c; } s;
>  s.a = 1;
>  __builtin_memset (s.b, 0, sizeof (s.b));
>  s.c = 3;
>  bar (&s);
> }
> Certainly not flexible array members in the middle of structure.

Right now, with -Wpedantic, we have the following warning for the above small case:

t2.c:3:24: warning: a member of a structure or union cannot have a variably modified type [-Wpedantic]
    3 |  struct S { int a; int b[n]; int c; } s;
      |                        ^


Do we have a definition for “GNU variable sized type” now?
Shall we include "flexible array members” and" the structure/union with a flexible array members at the end" into “GNU variable sized type”?

thanks.

Qing
> 
>> +identify all such cases in the source code and modify them.  This warning will be 
>> + on by default starting from GCC14.
> 
> 	Jakub


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

* Re: [V5][PATCH 2/2] Update documentation to clarify a GCC extension
  2023-03-27 15:57           ` Qing Zhao
@ 2023-03-27 16:06             ` Jakub Jelinek
  2023-03-27 16:22               ` Qing Zhao
  0 siblings, 1 reply; 20+ messages in thread
From: Jakub Jelinek @ 2023-03-27 16:06 UTC (permalink / raw)
  To: Qing Zhao
  Cc: Joseph Myers, sandra, Siddhesh Poyarekar, Kees Cook,
	Richard Biener, gcc Patches

On Mon, Mar 27, 2023 at 03:57:58PM +0000, Qing Zhao wrote:
> >> +Please use warning option  @option{-Wgnu-variable-sized-type-not-at-end} to
> > This is certainly misnamed.
> 
> The name “-Wgnu-variable-sized-type-not-at-end” was just used the warning name from CLANG. -:)
> 
> Shall we use the same name as CLANG? Or we invent a new name?

The latter IMHO.  Having a warning with completely nonsensical name will
just confuse users.

> >  GNU variable sized type not at the end of a
> > struct is something like
> > void bar (void *);
> > void foo (int n) {
> >  struct S { int a; int b[n]; int c; } s;
> >  s.a = 1;
> >  __builtin_memset (s.b, 0, sizeof (s.b));
> >  s.c = 3;
> >  bar (&s);
> > }
> > Certainly not flexible array members in the middle of structure.
> 
> Right now, with -Wpedantic, we have the following warning for the above small case:
> 
> t2.c:3:24: warning: a member of a structure or union cannot have a variably modified type [-Wpedantic]
>     3 |  struct S { int a; int b[n]; int c; } s;
>       |                        ^

Sure, it is a GNU C extension (not allowed in C++ BTW).
It is documented in https://gcc.gnu.org/onlinedocs/gcc/Variable-Length.html
though just very briefly:
As an extension, GCC accepts variable-length arrays as a member of a structure or a union. For example: 
void
foo (int n)
{
  struct S { int x[n]; };
}

> Do we have a definition for “GNU variable sized type” now?

Naturally, variable sized type should have non-constant sizeof, because
otherwise it is constant sized type.  That is not
the case for flexible array members, there is nothing variable sized on
them, especially if they are in the middle of a structure.

	Jakub


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

* Re: [V5][PATCH 2/2] Update documentation to clarify a GCC extension
  2023-03-27 16:06             ` Jakub Jelinek
@ 2023-03-27 16:22               ` Qing Zhao
  2023-03-27 16:31                 ` Jakub Jelinek
  0 siblings, 1 reply; 20+ messages in thread
From: Qing Zhao @ 2023-03-27 16:22 UTC (permalink / raw)
  To: Jakub Jelinek
  Cc: Joseph Myers, sandra, Siddhesh Poyarekar, Kees Cook,
	Richard Biener, gcc Patches



> On Mar 27, 2023, at 12:06 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> 
> On Mon, Mar 27, 2023 at 03:57:58PM +0000, Qing Zhao wrote:
>>>> +Please use warning option  @option{-Wgnu-variable-sized-type-not-at-end} to
>>> This is certainly misnamed.
>> 
>> The name “-Wgnu-variable-sized-type-not-at-end” was just used the warning name from CLANG. -:)
>> 
>> Shall we use the same name as CLANG? Or we invent a new name?
> 
> The latter IMHO.  Having a warning with completely nonsensical name will
> just confuse users.

Okay. -:)
How about "-Wstruct-with-fam-not-at-end”?  Or do you have any suggestion on the name?
> 
>>> GNU variable sized type not at the end of a
>>> struct is something like
>>> void bar (void *);
>>> void foo (int n) {
>>> struct S { int a; int b[n]; int c; } s;
>>> s.a = 1;
>>> __builtin_memset (s.b, 0, sizeof (s.b));
>>> s.c = 3;
>>> bar (&s);
>>> }
>>> Certainly not flexible array members in the middle of structure.
>> 
>> Right now, with -Wpedantic, we have the following warning for the above small case:
>> 
>> t2.c:3:24: warning: a member of a structure or union cannot have a variably modified type [-Wpedantic]
>>    3 |  struct S { int a; int b[n]; int c; } s;
>>      |                        ^
> 
> Sure, it is a GNU C extension (not allowed in C++ BTW).
> It is documented in https://gcc.gnu.org/onlinedocs/gcc/Variable-Length.html
> though just very briefly:
> As an extension, GCC accepts variable-length arrays as a member of a structure or a union. For example: 
> void
> foo (int n)
> {
>  struct S { int x[n]; };
> }

Okay, I see. 
> 
>> Do we have a definition for “GNU variable sized type” now?
> 
> Naturally, variable sized type should have non-constant sizeof, because
> otherwise it is constant sized type.

Oh, for flexible array members, we cannot take sizeof it, So they are considered as incomplete type, right?

thanks.

Qing
>  That is not
> the case for flexible array members, there is nothing variable sized on
> them, especially if they are in the middle of a structure.
> 
> 	Jakub
> 


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

* Re: [V5][PATCH 2/2] Update documentation to clarify a GCC extension
  2023-03-27 16:22               ` Qing Zhao
@ 2023-03-27 16:31                 ` Jakub Jelinek
  2023-03-27 16:48                   ` Qing Zhao
  0 siblings, 1 reply; 20+ messages in thread
From: Jakub Jelinek @ 2023-03-27 16:31 UTC (permalink / raw)
  To: Qing Zhao
  Cc: Joseph Myers, sandra, Siddhesh Poyarekar, Kees Cook,
	Richard Biener, gcc Patches

On Mon, Mar 27, 2023 at 04:22:25PM +0000, Qing Zhao via Gcc-patches wrote:
> > The latter IMHO.  Having a warning with completely nonsensical name will
> > just confuse users.
> 
> Okay. -:)
> How about "-Wstruct-with-fam-not-at-end”?  Or do you have any suggestion on the name?

Nobody will know what fam is.
-Wflex-array-member-not-at-end ?

	Jakub


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

* Re: [V5][PATCH 2/2] Update documentation to clarify a GCC extension
  2023-03-27 16:31                 ` Jakub Jelinek
@ 2023-03-27 16:48                   ` Qing Zhao
  2023-03-27 17:29                     ` Qing Zhao
  0 siblings, 1 reply; 20+ messages in thread
From: Qing Zhao @ 2023-03-27 16:48 UTC (permalink / raw)
  To: Jakub Jelinek
  Cc: Joseph Myers, sandra, Siddhesh Poyarekar, Kees Cook,
	Richard Biener, gcc Patches



> On Mar 27, 2023, at 12:31 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> 
> On Mon, Mar 27, 2023 at 04:22:25PM +0000, Qing Zhao via Gcc-patches wrote:
>>> The latter IMHO.  Having a warning with completely nonsensical name will
>>> just confuse users.
>> 
>> Okay. -:)
>> How about "-Wstruct-with-fam-not-at-end”?  Or do you have any suggestion on the name?
> 
> Nobody will know what fam is.

Yes, I agree -:)

> -Wflex-array-member-not-at-end ?

However, Will this name include “a structure with flexible array member is not at end”?  
Qing

> 
> 	Jakub
> 


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

* Re: [V5][PATCH 2/2] Update documentation to clarify a GCC extension
  2023-03-27 16:48                   ` Qing Zhao
@ 2023-03-27 17:29                     ` Qing Zhao
  0 siblings, 0 replies; 20+ messages in thread
From: Qing Zhao @ 2023-03-27 17:29 UTC (permalink / raw)
  To: jakub Jelinek
  Cc: Joseph Myers, sandra, Siddhesh Poyarekar, Kees Cook,
	Richard Biener, gcc Patches



> On Mar 27, 2023, at 12:48 PM, Qing Zhao via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
> 
> 
> 
>> On Mar 27, 2023, at 12:31 PM, Jakub Jelinek <jakub@redhat.com> wrote:
>> 
>> On Mon, Mar 27, 2023 at 04:22:25PM +0000, Qing Zhao via Gcc-patches wrote:
>>>> The latter IMHO.  Having a warning with completely nonsensical name will
>>>> just confuse users.
>>> 
>>> Okay. -:)
>>> How about "-Wstruct-with-fam-not-at-end”?  Or do you have any suggestion on the name?
>> 
>> Nobody will know what fam is.
> 
> Yes, I agree -:)
> 
>> -Wflex-array-member-not-at-end ?
> 
> However, Will this name include “a structure with flexible array member is not at end”?  

Looks like no better name than “-Wflex-array-member-not-at-end” as I can think of..

I will use this one.

Let me know if you have any further comments on the documentation part.

thanks.

Qing
> Qing
> 
>> 
>> 	Jakub
>> 
> 


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

end of thread, other threads:[~2023-03-27 17:29 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-16 21:47 [V5][PATCH 0/2] Handle component_ref to a structure/union field including FAM for builtin_object_size Qing Zhao
2023-03-16 21:47 ` [V5][PATCH 1/2] Handle component_ref to a structre/union field including flexible array member [PR101832] Qing Zhao
2023-03-23 13:03   ` Fwd: " Qing Zhao
2023-03-23 18:55     ` Joseph Myers
2023-03-27 13:31       ` Qing Zhao
2023-03-27 14:22     ` Qing Zhao
2023-03-16 21:47 ` [V5][PATCH 2/2] Update documentation to clarify a GCC extension Qing Zhao
2023-03-23 13:05   ` Fwd: " Qing Zhao
2023-03-23 20:14     ` Joseph Myers
2023-03-27 13:38       ` Qing Zhao
2023-03-27 14:34         ` Xi Ruoyao
2023-03-27 15:37           ` Qing Zhao
2023-03-27 15:43         ` Jakub Jelinek
2023-03-27 15:57           ` Qing Zhao
2023-03-27 16:06             ` Jakub Jelinek
2023-03-27 16:22               ` Qing Zhao
2023-03-27 16:31                 ` Jakub Jelinek
2023-03-27 16:48                   ` Qing Zhao
2023-03-27 17:29                     ` Qing Zhao
2023-03-23 13:02 ` Fwd: [V5][PATCH 0/2] Handle component_ref to a structure/union field including FAM for builtin_object_size Qing Zhao

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).