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

public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed

* [V6][PATCH 0/2] Handle component_ref to a structure/union field including FAM for builtin_object_size
@ 2023-03-28 15:49 Qing Zhao
  2023-03-28 15:49 ` [V6][PATCH 1/2] Handle component_ref to a structre/union field including flexible array member [PR101832] Qing Zhao
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Qing Zhao @ 2023-03-28 15:49 UTC (permalink / raw)
  To: jakub, joseph
  Cc: richard.guenther, keescook, siddhesh, gcc-patches, Qing Zhao

Hi, Joseph and Jakub,

this is the 6th version of the patch. 
compared to the 5th version, the major changes are:

1. Update the documentation Per Joseph's comments;
2. Change the name of the new warning option per Jakub's suggestions.
3. Update testing case per the above change.

these changes are all in the 2th patch (2/2 Update documentation to
clarify a GCC extension).

The first patch (1/2 Handle component_ref to a structre/union field
including  flexible array member [PR101832]) is not changed

For the first patch, As a record, Joseph has approved the C front-end change, 
I only need a review from Jakub for the Middle-end.

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                               |  20 +++
 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, 281 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] 13+ messages in thread

* [V6][PATCH 1/2] Handle component_ref to a structre/union field including flexible array member [PR101832]
  2023-03-28 15:49 [V6][PATCH 0/2] Handle component_ref to a structure/union field including FAM for builtin_object_size Qing Zhao
@ 2023-03-28 15:49 ` Qing Zhao
  2023-04-04 13:06   ` Fwd: " Qing Zhao
  2023-04-12 18:46   ` Kees Cook
  2023-03-28 15:49 ` [PATCH 2/2] Update documentation to clarify a GCC extension Qing Zhao
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 13+ messages in thread
From: Qing Zhao @ 2023-03-28 15:49 UTC (permalink / raw)
  To: jakub, joseph
  Cc: richard.guenther, keescook, siddhesh, gcc-patches, Qing Zhao

the C front-end has been approved by Joseph.

Jacub, could you please eview the middle end part of the changes of this patch?

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!

Qing

==========

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..83482537a6d 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..4a1d481015b 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 abcdb5638d4..3ab3a3a0cc7 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] 13+ messages in thread

* Fwd: [V6][PATCH 1/2] Handle component_ref to a structre/union field including flexible array member [PR101832]
  2023-03-28 15:49 ` [V6][PATCH 1/2] Handle component_ref to a structre/union field including flexible array member [PR101832] Qing Zhao
@ 2023-04-04 13:06   ` Qing Zhao
  2023-04-11 13:37     ` Qing Zhao
  2023-04-12 18:46   ` Kees Cook
  1 sibling, 1 reply; 13+ messages in thread
From: Qing Zhao @ 2023-04-04 13:06 UTC (permalink / raw)
  To: Jakub Jelinek
  Cc: Joseph Myers, Richard Biener, kees Cook, Siddhesh Poyarekar, gcc Patches

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

Ping…

Qing

Begin forwarded message:

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

the C front-end has been approved by Joseph.

Jacub, could you please eview the middle end part of the changes of this patch?

The major change is in tree-object-size.cc<http://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!

Qing

==========

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..83482537a6d 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..4a1d481015b 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 abcdb5638d4..3ab3a3a0cc7 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] 13+ messages in thread

* Fwd: [V6][PATCH 1/2] Handle component_ref to a structre/union field including flexible array member [PR101832]
  2023-04-04 13:06   ` Fwd: " Qing Zhao
@ 2023-04-11 13:37     ` Qing Zhao
  2023-04-20 14:10       ` Ping * 3: " Qing Zhao
  0 siblings, 1 reply; 13+ messages in thread
From: Qing Zhao @ 2023-04-11 13:37 UTC (permalink / raw)
  To: Jakub Jelinek
  Cc: Joseph Myers, Richard Biener, kees Cook, Siddhesh Poyarekar, gcc Patches

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

Hi,  Jakub,

This is the 2nd ping to the 6th version of the patches -:)

Please let me know if you have any further comments on this patch, and whether it’s Okay to commit it to trunk?

Thanks a lot for the help.

Qing

Begin forwarded message:

From: Qing Zhao via Gcc-patches <gcc-patches@gcc.gnu.org<mailto:gcc-patches@gcc.gnu.org>>
Subject: Fwd: [V6][PATCH 1/2] Handle component_ref to a structre/union field including flexible array member [PR101832]
Date: April 4, 2023 at 9:06:37 AM EDT
To: Jakub Jelinek <jakub@redhat.com<mailto:jakub@redhat.com>>
Cc: Joseph Myers <joseph@codesourcery.com<mailto:joseph@codesourcery.com>>, Richard Biener <richard.guenther@gmail.com<mailto:richard.guenther@gmail.com>>, kees Cook <keescook@chromium.org<mailto:keescook@chromium.org>>, Siddhesh Poyarekar <siddhesh@gotplt.org<mailto:siddhesh@gotplt.org>>, gcc Patches <gcc-patches@gcc.gnu.org<mailto:gcc-patches@gcc.gnu.org>>
Reply-To: Qing Zhao <qing.zhao@oracle.com<mailto:qing.zhao@oracle.com>>

Ping…

Qing

Begin forwarded message:

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

the C front-end has been approved by Joseph.

Jacub, could you please eview the middle end part of the changes of this patch?

The major change is in tree-object-size.cc<http://tree-object-size.cc><http://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!

Qing

==========

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><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><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><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><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><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><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><http://c-decl.cc>                               |  11 ++
gcc/lto/lto-common.cc<http://lto-common.cc><http://lto-common.cc>                         |   5 +-
gcc/print-tree.cc<http://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><http://tree-object-size.cc>                       |  23 ++-
gcc/tree-streamer-in.cc<http://tree-streamer-in.cc><http://tree-streamer-in.cc>                       |   5 +-
gcc/tree-streamer-out.cc<http://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><http://c-decl.cc> b/gcc/c/c-decl.cc<http://c-decl.cc><http://c-decl.cc>
index e537d33f398..14c54809b9d 100644
--- a/gcc/c/c-decl.cc<http://c-decl.cc><http://c-decl.cc>
+++ b/gcc/c/c-decl.cc<http://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><http://lto-common.cc> b/gcc/lto/lto-common.cc<http://lto-common.cc><http://lto-common.cc>
index 882dd8971a4..9dde7118266 100644
--- a/gcc/lto/lto-common.cc<http://lto-common.cc><http://lto-common.cc>
+++ b/gcc/lto/lto-common.cc<http://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><http://print-tree.cc> b/gcc/print-tree.cc<http://print-tree.cc><http://print-tree.cc>
index 1f3afcbbc86..efacdb7686f 100644
--- a/gcc/print-tree.cc<http://print-tree.cc><http://print-tree.cc>
+++ b/gcc/print-tree.cc<http://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..83482537a6d 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><http://tree-object-size.cc> b/gcc/tree-object-size.cc<http://tree-object-size.cc><http://tree-object-size.cc>
index 9a936a91983..1619d144ecd 100644
--- a/gcc/tree-object-size.cc<http://tree-object-size.cc><http://tree-object-size.cc>
+++ b/gcc/tree-object-size.cc<http://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><http://tree-streamer-in.cc> b/gcc/tree-streamer-in.cc<http://tree-streamer-in.cc><http://tree-streamer-in.cc>
index d4dc30f048f..56add06a70a 100644
--- a/gcc/tree-streamer-in.cc<http://tree-streamer-in.cc><http://tree-streamer-in.cc>
+++ b/gcc/tree-streamer-in.cc<http://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><http://tree-streamer-out.cc> b/gcc/tree-streamer-out.cc<http://tree-streamer-out.cc><http://tree-streamer-out.cc>
index d107229da5c..4a1d481015b 100644
--- a/gcc/tree-streamer-out.cc<http://tree-streamer-out.cc><http://tree-streamer-out.cc>
+++ b/gcc/tree-streamer-out.cc<http://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 abcdb5638d4..3ab3a3a0cc7 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] 13+ messages in thread

* Ping * 3:  [V6][PATCH 1/2] Handle component_ref to a structre/union field including flexible array member [PR101832]
  2023-04-11 13:37     ` Qing Zhao
@ 2023-04-20 14:10       ` Qing Zhao
  0 siblings, 0 replies; 13+ messages in thread
From: Qing Zhao @ 2023-04-20 14:10 UTC (permalink / raw)
  To: Jakub Jelinek, gcc Patches
  Cc: Joseph Myers, Richard Biener, Kees Cook, Siddhesh Poyarekar

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

Hi,

Is this patch ready for GCC14?

Thanks.

Qing

Begin forwarded message:

From: Qing Zhao via Gcc-patches <gcc-patches@gcc.gnu.org<mailto:gcc-patches@gcc.gnu.org>>
Subject: Fwd: [V6][PATCH 1/2] Handle component_ref to a structre/union field including flexible array member [PR101832]
Date: April 11, 2023 at 9:37:18 AM EDT
To: Jakub Jelinek <jakub@redhat.com<mailto:jakub@redhat.com>>
Cc: Joseph Myers <joseph@codesourcery.com<mailto:joseph@codesourcery.com>>, Richard Biener <richard.guenther@gmail.com<mailto:richard.guenther@gmail.com>>, kees Cook <keescook@chromium.org<mailto:keescook@chromium.org>>, Siddhesh Poyarekar <siddhesh@gotplt.org<mailto:siddhesh@gotplt.org>>, gcc Patches <gcc-patches@gcc.gnu.org<mailto:gcc-patches@gcc.gnu.org>>
Reply-To: Qing Zhao <qing.zhao@oracle.com<mailto:qing.zhao@oracle.com>>

Hi,  Jakub,

This is the 2nd ping to the 6th version of the patches -:)

Please let me know if you have any further comments on this patch, and whether it’s Okay to commit it to trunk?

Thanks a lot for the help.

Qing

Begin forwarded message:

From: Qing Zhao via Gcc-patches <gcc-patches@gcc.gnu.org<mailto:gcc-patches@gcc.gnu.org><mailto:gcc-patches@gcc.gnu.org>>
Subject: Fwd: [V6][PATCH 1/2] Handle component_ref to a structre/union field including flexible array member [PR101832]
Date: April 4, 2023 at 9:06:37 AM EDT
To: Jakub Jelinek <jakub@redhat.com<mailto:jakub@redhat.com><mailto:jakub@redhat.com>>
Cc: Joseph Myers <joseph@codesourcery.com<mailto:joseph@codesourcery.com><mailto:joseph@codesourcery.com>>, Richard Biener <richard.guenther@gmail.com<mailto:richard.guenther@gmail.com><mailto:richard.guenther@gmail.com>>, kees Cook <keescook@chromium.org<mailto:keescook@chromium.org><mailto:keescook@chromium.org>>, Siddhesh Poyarekar <siddhesh@gotplt.org<mailto:siddhesh@gotplt.org><mailto:siddhesh@gotplt.org>>, gcc Patches <gcc-patches@gcc.gnu.org<mailto:gcc-patches@gcc.gnu.org><mailto:gcc-patches@gcc.gnu.org>>
Reply-To: Qing Zhao <qing.zhao@oracle.com<mailto:qing.zhao@oracle.com><mailto:qing.zhao@oracle.com>>

Ping…

Qing

Begin forwarded message:

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

the C front-end has been approved by Joseph.

Jacub, could you please eview the middle end part of the changes of this patch?

The major change is in tree-object-size.cc<http://tree-object-size.cc><http://tree-object-size.cc><http://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!

Qing

==========

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><http://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><http://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><http://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><http://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><http://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><http://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><http://c-decl.cc><http://c-decl.cc>                               |  11 ++
gcc/lto/lto-common.cc<http://lto-common.cc><http://lto-common.cc><http://lto-common.cc>                         |   5 +-
gcc/print-tree.cc<http://print-tree.cc><http://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><http://tree-object-size.cc><http://tree-object-size.cc>                       |  23 ++-
gcc/tree-streamer-in.cc<http://tree-streamer-in.cc><http://tree-streamer-in.cc><http://tree-streamer-in.cc>                       |   5 +-
gcc/tree-streamer-out.cc<http://tree-streamer-out.cc><http://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><http://c-decl.cc><http://c-decl.cc> b/gcc/c/c-decl.cc<http://c-decl.cc><http://c-decl.cc><http://c-decl.cc>
index e537d33f398..14c54809b9d 100644
--- a/gcc/c/c-decl.cc<http://c-decl.cc><http://c-decl.cc><http://c-decl.cc>
+++ b/gcc/c/c-decl.cc<http://c-decl.cc><http://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><http://lto-common.cc><http://lto-common.cc> b/gcc/lto/lto-common.cc<http://lto-common.cc><http://lto-common.cc><http://lto-common.cc>
index 882dd8971a4..9dde7118266 100644
--- a/gcc/lto/lto-common.cc<http://lto-common.cc><http://lto-common.cc><http://lto-common.cc>
+++ b/gcc/lto/lto-common.cc<http://lto-common.cc><http://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><http://print-tree.cc><http://print-tree.cc> b/gcc/print-tree.cc<http://print-tree.cc><http://print-tree.cc><http://print-tree.cc>
index 1f3afcbbc86..efacdb7686f 100644
--- a/gcc/print-tree.cc<http://print-tree.cc><http://print-tree.cc><http://print-tree.cc>
+++ b/gcc/print-tree.cc<http://print-tree.cc><http://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..83482537a6d 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><http://tree-object-size.cc><http://tree-object-size.cc> b/gcc/tree-object-size.cc<http://tree-object-size.cc><http://tree-object-size.cc><http://tree-object-size.cc>
index 9a936a91983..1619d144ecd 100644
--- a/gcc/tree-object-size.cc<http://tree-object-size.cc><http://tree-object-size.cc><http://tree-object-size.cc>
+++ b/gcc/tree-object-size.cc<http://tree-object-size.cc><http://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><http://tree-streamer-in.cc><http://tree-streamer-in.cc> b/gcc/tree-streamer-in.cc<http://tree-streamer-in.cc><http://tree-streamer-in.cc><http://tree-streamer-in.cc>
index d4dc30f048f..56add06a70a 100644
--- a/gcc/tree-streamer-in.cc<http://tree-streamer-in.cc><http://tree-streamer-in.cc><http://tree-streamer-in.cc>
+++ b/gcc/tree-streamer-in.cc<http://tree-streamer-in.cc><http://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><http://tree-streamer-out.cc><http://tree-streamer-out.cc> b/gcc/tree-streamer-out.cc<http://tree-streamer-out.cc><http://tree-streamer-out.cc><http://tree-streamer-out.cc>
index d107229da5c..4a1d481015b 100644
--- a/gcc/tree-streamer-out.cc<http://tree-streamer-out.cc><http://tree-streamer-out.cc><http://tree-streamer-out.cc>
+++ b/gcc/tree-streamer-out.cc<http://tree-streamer-out.cc><http://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 abcdb5638d4..3ab3a3a0cc7 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] 13+ messages in thread

* Re: [V6][PATCH 1/2] Handle component_ref to a structre/union field including flexible array member [PR101832]
  2023-03-28 15:49 ` [V6][PATCH 1/2] Handle component_ref to a structre/union field including flexible array member [PR101832] Qing Zhao
  2023-04-04 13:06   ` Fwd: " Qing Zhao
@ 2023-04-12 18:46   ` Kees Cook
  2023-04-17 12:56     ` Qing Zhao
  1 sibling, 1 reply; 13+ messages in thread
From: Kees Cook @ 2023-04-12 18:46 UTC (permalink / raw)
  To: Qing Zhao; +Cc: jakub, joseph, richard.guenther, siddhesh, gcc-patches

On Tue, Mar 28, 2023 at 03:49:43PM +0000, Qing Zhao wrote:
> the C front-end has been approved by Joseph.
> 
> Jacub, could you please eview the middle end part of the changes of this patch?
> 
> 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!

Just to confirm, I've done build testing with the Linux kernel, and this
is behaving as I'd expect. This makes my life MUCH easier -- many fewer
false positives for our bounds checking. :)

-Kees

-- 
Kees Cook

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

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

Thanks a lot for the testing, Kees.

Jakub, do you have any more comment on the 6th version of the patches?
Is it ready for the GCC13 commit? Or we need to delay it to GCC14?

This patch has been there waiting for approving for a while, I think that it’s a nice improvement to GCC and also help to the
Kernel security. 

Thanks.

Qing

> On Apr 12, 2023, at 2:46 PM, Kees Cook <keescook@chromium.org> wrote:
> 
> On Tue, Mar 28, 2023 at 03:49:43PM +0000, Qing Zhao wrote:
>> the C front-end has been approved by Joseph.
>> 
>> Jacub, could you please eview the middle end part of the changes of this patch?
>> 
>> 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!
> 
> Just to confirm, I've done build testing with the Linux kernel, and this
> is behaving as I'd expect. This makes my life MUCH easier -- many fewer
> false positives for our bounds checking. :)
> 
> -Kees
> 
> -- 
> Kees Cook


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

* [PATCH 2/2] Update documentation to clarify a GCC extension
  2023-03-28 15:49 [V6][PATCH 0/2] Handle component_ref to a structure/union field including FAM for builtin_object_size Qing Zhao
  2023-03-28 15:49 ` [V6][PATCH 1/2] Handle component_ref to a structre/union field including flexible array member [PR101832] Qing Zhao
@ 2023-03-28 15:49 ` Qing Zhao
  2023-04-04 13:07   ` Fwd: [V6][PATCH " Qing Zhao
  2023-04-11 13:35 ` Fwd: [V6][PATCH 0/2] Handle component_ref to a structure/union field including FAM for builtin_object_size Qing Zhao
  2023-04-20 14:08 ` Ping * 3: " Qing Zhao
  3 siblings, 1 reply; 13+ messages in thread
From: Qing Zhao @ 2023-03-28 15:49 UTC (permalink / raw)
  To: jakub, joseph
  Cc: richard.guenther, keescook, siddhesh, 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 '-Wflex-array-member-not-at-end' to
     identify all such cases in the source code and modify them.  This
     warning will be on by default starting from GCC 14.
"

gcc/c-family/ChangeLog:

	* c.opt: New option -Wflex-array-member-not-at-end.

gcc/c/ChangeLog:

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

gcc/ChangeLog:

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

gcc/testsuite/ChangeLog:

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

diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 3333cddeece..c26d9801b63 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -737,6 +737,11 @@ Wformat-truncation=
 C ObjC C++ LTO ObjC++ Joined RejectNegative UInteger Var(warn_format_trunc) Warning LangEnabledBy(C ObjC C++ LTO ObjC++,Wformat=, warn_format >= 1, 0) IntegerRange(0, 2)
 Warn about calls to snprintf and similar functions that truncate output.
 
+Wflex-array-member-not-at-end
+C C++ Var(warn_flex_array_member_not_at_end) Warning
+Warn when a structure containing a C99 flexible array member as the last
+field is not at the end of another structure.
+
 Wif-not-aligned
 C ObjC C++ ObjC++ Var(warn_if_not_aligned) Init(1) Warning
 Warn when the field in a struct is not aligned.
diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
index 14c54809b9d..92304fd9c8f 100644
--- a/gcc/c/c-decl.cc
+++ b/gcc/c/c-decl.cc
@@ -9269,6 +9269,15 @@ 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_flex_array_member_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_Wflex_array_member_not_at_end,
+		    "structure containing a flexible array member"
+		    " is not at the end of another structure");
+
       if (DECL_NAME (x)
 	  || RECORD_OR_UNION_TYPE_P (TREE_TYPE (x)))
 	saw_named_field = true;
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 3adb67aa47a..ef46423339e 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{-Wflex-array-member-not-at-end} to
+identify all such cases in the source code and modify them.  This warning
+will be on by default staring from GCC 14.
 @end itemize
 
 Non-empty initialization of zero-length
diff --git a/gcc/testsuite/gcc.dg/variable-sized-type-flex-array.c b/gcc/testsuite/gcc.dg/variable-sized-type-flex-array.c
new file mode 100644
index 00000000000..3924937bad4
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/variable-sized-type-flex-array.c
@@ -0,0 +1,31 @@
+/* Test for -Wflex-array-member-not-at-end on structure/union with 
+   C99 flexible array members being embedded into another structure.  */
+/* { dg-do compile } */
+/* { dg-options "-Wflex-array-member-not-at-end" } */
+
+struct flex { int n; int data[]; };
+struct out_flex_end { int m; struct flex flex_data; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+struct out_flex_mid { struct flex flex_data; int m; };  /* { dg-warning "structure containing a flexible array member is not at the end of another structure" } */
+/* since the warning has been issued for out_flex_mid, no need to
+   issue warning again when it is included in another structure/union.  */
+struct outer_flex_mid { struct out_flex_mid out_flex_data; int p; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+union flex_union_mid { int a; struct outer_flex_mid b; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+
+
+struct flex0 { int n; int data[0]; };
+struct out_flex_end0 { int m; struct flex0 flex_data; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+struct out_flex_mid0 { struct flex0 flex_data; int m; };  /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+struct outer_flex_mid0 { struct out_flex_mid0 out_flex_data; int p; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+union flex_union_mid0 { int a; struct outer_flex_mid0 b; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+
+struct flex1 { int n; int data[1]; };
+struct out_flex_end1 { int m; struct flex1 flex_data; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+struct out_flex_mid1 { struct flex1 flex_data; int m; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */ 
+struct outer_flex_mid1 { struct out_flex_mid1 out_flex_data; int p; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+union flex_union_mid1 { int a; struct outer_flex_mid1 b; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+
+struct flexn { int n; int data[8]; }; 
+struct out_flex_endn { int m; struct flexn flex_data; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+struct out_flex_midn { struct flexn flex_data; int m; }; /* { dg-bogus"structure containing a flexible array member is not at the end of another structure" } */ 
+struct outer_flex_midn { struct out_flex_midn out_flex_data; int p; }; /* { dg-bogus"structure containing a flexible array member is not at the end of another structure" } */
+union flex_union_midn { int a; struct outer_flex_midn b; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
-- 
2.31.1


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

* Fwd: [V6][PATCH 2/2] Update documentation to clarify a GCC extension
  2023-03-28 15:49 ` [PATCH 2/2] Update documentation to clarify a GCC extension Qing Zhao
@ 2023-04-04 13:07   ` Qing Zhao
  2023-04-11 13:38     ` Qing Zhao
  0 siblings, 1 reply; 13+ messages in thread
From: Qing Zhao @ 2023-04-04 13:07 UTC (permalink / raw)
  To: Joseph Myers
  Cc: Jakub Jelinek, Richard Biener, Kees Cook, Siddhesh Poyarekar,
	gcc Patches

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

Ping….

Qing

Begin forwarded message:

From: Qing Zhao <qing.zhao@oracle.com<mailto:qing.zhao@oracle.com>>
Subject: [PATCH 2/2] Update documentation to clarify a GCC extension
Date: March 28, 2023 at 11:49:44 AM EDT
To: jakub@redhat.com<mailto:jakub@redhat.com>, joseph@codesourcery.com<mailto:joseph@codesourcery.com>
Cc: richard.guenther@gmail.com<mailto:richard.guenther@gmail.com>, keescook@chromium.org<mailto:keescook@chromium.org>, siddhesh@gotplt.org<mailto:siddhesh@gotplt.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 '-Wflex-array-member-not-at-end' to
    identify all such cases in the source code and modify them.  This
    warning will be on by default starting from GCC 14.
"

gcc/c-family/ChangeLog:

* c.opt: New option -Wflex-array-member-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>                               |  9 ++++
gcc/doc/extend.texi                           | 45 ++++++++++++++++++-
.../gcc.dg/variable-sized-type-flex-array.c   | 31 +++++++++++++
4 files changed, 89 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gcc.dg/variable-sized-type-flex-array.c

diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 3333cddeece..c26d9801b63 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -737,6 +737,11 @@ Wformat-truncation=
C ObjC C++ LTO ObjC++ Joined RejectNegative UInteger Var(warn_format_trunc) Warning LangEnabledBy(C ObjC C++ LTO ObjC++,Wformat=, warn_format >= 1, 0) IntegerRange(0, 2)
Warn about calls to snprintf and similar functions that truncate output.

+Wflex-array-member-not-at-end
+C C++ Var(warn_flex_array_member_not_at_end) Warning
+Warn when a structure containing a C99 flexible array member as the last
+field is not at the end of another structure.
+
Wif-not-aligned
C ObjC C++ ObjC++ Var(warn_if_not_aligned) Init(1) Warning
Warn when the field in a struct is not aligned.
diff --git a/gcc/c/c-decl.cc<http://c-decl.cc> b/gcc/c/c-decl.cc<http://c-decl.cc>
index 14c54809b9d..92304fd9c8f 100644
--- a/gcc/c/c-decl.cc<http://c-decl.cc>
+++ b/gcc/c/c-decl.cc<http://c-decl.cc>
@@ -9269,6 +9269,15 @@ 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_flex_array_member_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_Wflex_array_member_not_at_end,
+    "structure containing a flexible array member"
+    " is not at the end of another structure");
+
      if (DECL_NAME (x)
 || RECORD_OR_UNION_TYPE_P (TREE_TYPE (x)))
saw_named_field = true;
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 3adb67aa47a..ef46423339e 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{-Wflex-array-member-not-at-end} to
+identify all such cases in the source code and modify them.  This warning
+will be on by default staring from GCC 14.
@end itemize

Non-empty initialization of zero-length
diff --git a/gcc/testsuite/gcc.dg/variable-sized-type-flex-array.c b/gcc/testsuite/gcc.dg/variable-sized-type-flex-array.c
new file mode 100644
index 00000000000..3924937bad4
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/variable-sized-type-flex-array.c
@@ -0,0 +1,31 @@
+/* Test for -Wflex-array-member-not-at-end on structure/union with
+   C99 flexible array members being embedded into another structure.  */
+/* { dg-do compile } */
+/* { dg-options "-Wflex-array-member-not-at-end" } */
+
+struct flex { int n; int data[]; };
+struct out_flex_end { int m; struct flex flex_data; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+struct out_flex_mid { struct flex flex_data; int m; };  /* { dg-warning "structure containing a flexible array member is not at the end of another structure" } */
+/* since the warning has been issued for out_flex_mid, no need to
+   issue warning again when it is included in another structure/union.  */
+struct outer_flex_mid { struct out_flex_mid out_flex_data; int p; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+union flex_union_mid { int a; struct outer_flex_mid b; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+
+
+struct flex0 { int n; int data[0]; };
+struct out_flex_end0 { int m; struct flex0 flex_data; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+struct out_flex_mid0 { struct flex0 flex_data; int m; };  /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+struct outer_flex_mid0 { struct out_flex_mid0 out_flex_data; int p; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+union flex_union_mid0 { int a; struct outer_flex_mid0 b; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+
+struct flex1 { int n; int data[1]; };
+struct out_flex_end1 { int m; struct flex1 flex_data; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+struct out_flex_mid1 { struct flex1 flex_data; int m; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+struct outer_flex_mid1 { struct out_flex_mid1 out_flex_data; int p; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+union flex_union_mid1 { int a; struct outer_flex_mid1 b; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+
+struct flexn { int n; int data[8]; };
+struct out_flex_endn { int m; struct flexn flex_data; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+struct out_flex_midn { struct flexn flex_data; int m; }; /* { dg-bogus"structure containing a flexible array member is not at the end of another structure" } */
+struct outer_flex_midn { struct out_flex_midn out_flex_data; int p; }; /* { dg-bogus"structure containing a flexible array member is not at the end of another structure" } */
+union flex_union_midn { int a; struct outer_flex_midn b; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
--
2.31.1



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

* Fwd: [V6][PATCH 2/2] Update documentation to clarify a GCC extension
  2023-04-04 13:07   ` Fwd: [V6][PATCH " Qing Zhao
@ 2023-04-11 13:38     ` Qing Zhao
  2023-04-20 14:11       ` Ping * 3: " Qing Zhao
  0 siblings, 1 reply; 13+ messages in thread
From: Qing Zhao @ 2023-04-11 13:38 UTC (permalink / raw)
  To: Joseph Myers
  Cc: Jakub Jelinek, Richard Biener, kees Cook, Siddhesh Poyarekar,
	gcc Patches

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

Hi, Joseph,

This is the 2nd ping to the 6th version of the patch -:)

Please let me know if you have any further comments on the patch, and whether it’s Okay to commit it to trunk?

Thanks a lot for the help.

Qing

Begin forwarded message:

From: Qing Zhao via Gcc-patches <gcc-patches@gcc.gnu.org<mailto:gcc-patches@gcc.gnu.org>>
Subject: Fwd: [V6][PATCH 2/2] Update documentation to clarify a GCC extension
Date: April 4, 2023 at 9:07:55 AM EDT
To: Joseph Myers <joseph@codesourcery.com<mailto:joseph@codesourcery.com>>
Cc: Jakub Jelinek <jakub@redhat.com<mailto:jakub@redhat.com>>, Richard Biener <richard.guenther@gmail.com<mailto:richard.guenther@gmail.com>>, Kees Cook <keescook@chromium.org<mailto:keescook@chromium.org>>, Siddhesh Poyarekar <siddhesh@gotplt.org<mailto:siddhesh@gotplt.org>>, gcc Patches <gcc-patches@gcc.gnu.org<mailto:gcc-patches@gcc.gnu.org>>
Reply-To: Qing Zhao <qing.zhao@oracle.com<mailto:qing.zhao@oracle.com>>

Ping….

Qing

Begin forwarded message:

From: Qing Zhao <qing.zhao@oracle.com<mailto:qing.zhao@oracle.com><mailto:qing.zhao@oracle.com>>
Subject: [PATCH 2/2] Update documentation to clarify a GCC extension
Date: March 28, 2023 at 11:49:44 AM EDT
To: jakub@redhat.com<mailto:jakub@redhat.com><mailto:jakub@redhat.com>, joseph@codesourcery.com<mailto:joseph@codesourcery.com><mailto:joseph@codesourcery.com>
Cc: richard.guenther@gmail.com<mailto:richard.guenther@gmail.com><mailto:richard.guenther@gmail.com>, keescook@chromium.org<mailto:keescook@chromium.org><mailto:keescook@chromium.org>, siddhesh@gotplt.org<mailto:siddhesh@gotplt.org><mailto:siddhesh@gotplt.org>, gcc-patches@gcc.gnu.org<mailto:gcc-patches@gcc.gnu.org><mailto:gcc-patches@gcc.gnu.org>, Qing Zhao <qing.zhao@oracle.com<mailto: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 '-Wflex-array-member-not-at-end' to
   identify all such cases in the source code and modify them.  This
   warning will be on by default starting from GCC 14.
"

gcc/c-family/ChangeLog:

* c.opt: New option -Wflex-array-member-not-at-end.

gcc/c/ChangeLog:

* c-decl.cc<http://c-decl.cc/><http://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/><http://c-decl.cc<http://c-decl.cc/>>                               |  9 ++++
gcc/doc/extend.texi                           | 45 ++++++++++++++++++-
.../gcc.dg/variable-sized-type-flex-array.c   | 31 +++++++++++++
4 files changed, 89 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gcc.dg/variable-sized-type-flex-array.c

diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 3333cddeece..c26d9801b63 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -737,6 +737,11 @@ Wformat-truncation=
C ObjC C++ LTO ObjC++ Joined RejectNegative UInteger Var(warn_format_trunc) Warning LangEnabledBy(C ObjC C++ LTO ObjC++,Wformat=, warn_format >= 1, 0) IntegerRange(0, 2)
Warn about calls to snprintf and similar functions that truncate output.

+Wflex-array-member-not-at-end
+C C++ Var(warn_flex_array_member_not_at_end) Warning
+Warn when a structure containing a C99 flexible array member as the last
+field is not at the end of another structure.
+
Wif-not-aligned
C ObjC C++ ObjC++ Var(warn_if_not_aligned) Init(1) Warning
Warn when the field in a struct is not aligned.
diff --git a/gcc/c/c-decl.cc<http://c-decl.cc/><http://c-decl.cc<http://c-decl.cc/>> b/gcc/c/c-decl.cc<http://c-decl.cc/><http://c-decl.cc<http://c-decl.cc/>>
index 14c54809b9d..92304fd9c8f 100644
--- a/gcc/c/c-decl.cc<http://c-decl.cc/><http://c-decl.cc<http://c-decl.cc/>>
+++ b/gcc/c/c-decl.cc<http://c-decl.cc/><http://c-decl.cc<http://c-decl.cc/>>
@@ -9269,6 +9269,15 @@ 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_flex_array_member_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_Wflex_array_member_not_at_end,
+    "structure containing a flexible array member"
+    " is not at the end of another structure");
+
     if (DECL_NAME (x)
|| RECORD_OR_UNION_TYPE_P (TREE_TYPE (x)))
saw_named_field = true;
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 3adb67aa47a..ef46423339e 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{-Wflex-array-member-not-at-end} to
+identify all such cases in the source code and modify them.  This warning
+will be on by default staring from GCC 14.
@end itemize

Non-empty initialization of zero-length
diff --git a/gcc/testsuite/gcc.dg/variable-sized-type-flex-array.c b/gcc/testsuite/gcc.dg/variable-sized-type-flex-array.c
new file mode 100644
index 00000000000..3924937bad4
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/variable-sized-type-flex-array.c
@@ -0,0 +1,31 @@
+/* Test for -Wflex-array-member-not-at-end on structure/union with
+   C99 flexible array members being embedded into another structure.  */
+/* { dg-do compile } */
+/* { dg-options "-Wflex-array-member-not-at-end" } */
+
+struct flex { int n; int data[]; };
+struct out_flex_end { int m; struct flex flex_data; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+struct out_flex_mid { struct flex flex_data; int m; };  /* { dg-warning "structure containing a flexible array member is not at the end of another structure" } */
+/* since the warning has been issued for out_flex_mid, no need to
+   issue warning again when it is included in another structure/union.  */
+struct outer_flex_mid { struct out_flex_mid out_flex_data; int p; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+union flex_union_mid { int a; struct outer_flex_mid b; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+
+
+struct flex0 { int n; int data[0]; };
+struct out_flex_end0 { int m; struct flex0 flex_data; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+struct out_flex_mid0 { struct flex0 flex_data; int m; };  /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+struct outer_flex_mid0 { struct out_flex_mid0 out_flex_data; int p; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+union flex_union_mid0 { int a; struct outer_flex_mid0 b; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+
+struct flex1 { int n; int data[1]; };
+struct out_flex_end1 { int m; struct flex1 flex_data; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+struct out_flex_mid1 { struct flex1 flex_data; int m; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+struct outer_flex_mid1 { struct out_flex_mid1 out_flex_data; int p; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+union flex_union_mid1 { int a; struct outer_flex_mid1 b; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+
+struct flexn { int n; int data[8]; };
+struct out_flex_endn { int m; struct flexn flex_data; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+struct out_flex_midn { struct flexn flex_data; int m; }; /* { dg-bogus"structure containing a flexible array member is not at the end of another structure" } */
+struct outer_flex_midn { struct out_flex_midn out_flex_data; int p; }; /* { dg-bogus"structure containing a flexible array member is not at the end of another structure" } */
+union flex_union_midn { int a; struct outer_flex_midn b; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
--
2.31.1


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

* Ping * 3: [V6][PATCH 2/2] Update documentation to clarify a GCC extension
  2023-04-11 13:38     ` Qing Zhao
@ 2023-04-20 14:11       ` Qing Zhao
  0 siblings, 0 replies; 13+ messages in thread
From: Qing Zhao @ 2023-04-20 14:11 UTC (permalink / raw)
  To: Joseph Myers, gcc Patches
  Cc: Jakub Jelinek, Richard Biener, kees Cook, Siddhesh Poyarekar

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

Hi,

Is this patch ready for GCC14?

Thanks.

Qing

Begin forwarded message:

From: Qing Zhao via Gcc-patches <gcc-patches@gcc.gnu.org<mailto:gcc-patches@gcc.gnu.org>>
Subject: Fwd: [V6][PATCH 2/2] Update documentation to clarify a GCC extension
Date: April 11, 2023 at 9:38:29 AM EDT
To: Joseph Myers <joseph@codesourcery.com<mailto:joseph@codesourcery.com>>
Cc: Jakub Jelinek <jakub@redhat.com<mailto:jakub@redhat.com>>, Richard Biener <richard.guenther@gmail.com<mailto:richard.guenther@gmail.com>>, kees Cook <keescook@chromium.org<mailto:keescook@chromium.org>>, Siddhesh Poyarekar <siddhesh@gotplt.org<mailto:siddhesh@gotplt.org>>, gcc Patches <gcc-patches@gcc.gnu.org<mailto:gcc-patches@gcc.gnu.org>>
Reply-To: Qing Zhao <qing.zhao@oracle.com<mailto:qing.zhao@oracle.com>>

Hi, Joseph,

This is the 2nd ping to the 6th version of the patch -:)

Please let me know if you have any further comments on the patch, and whether it’s Okay to commit it to trunk?

Thanks a lot for the help.

Qing

Begin forwarded message:

From: Qing Zhao via Gcc-patches <gcc-patches@gcc.gnu.org<mailto:gcc-patches@gcc.gnu.org><mailto:gcc-patches@gcc.gnu.org>>
Subject: Fwd: [V6][PATCH 2/2] Update documentation to clarify a GCC extension
Date: April 4, 2023 at 9:07:55 AM EDT
To: Joseph Myers <joseph@codesourcery.com<mailto:joseph@codesourcery.com><mailto:joseph@codesourcery.com>>
Cc: Jakub Jelinek <jakub@redhat.com<mailto:jakub@redhat.com><mailto:jakub@redhat.com>>, Richard Biener <richard.guenther@gmail.com<mailto:richard.guenther@gmail.com><mailto:richard.guenther@gmail.com>>, Kees Cook <keescook@chromium.org<mailto:keescook@chromium.org><mailto:keescook@chromium.org>>, Siddhesh Poyarekar <siddhesh@gotplt.org<mailto:siddhesh@gotplt.org><mailto:siddhesh@gotplt.org>>, gcc Patches <gcc-patches@gcc.gnu.org<mailto:gcc-patches@gcc.gnu.org><mailto:gcc-patches@gcc.gnu.org>>
Reply-To: Qing Zhao <qing.zhao@oracle.com<mailto:qing.zhao@oracle.com><mailto:qing.zhao@oracle.com>>

Ping….

Qing

Begin forwarded message:

From: Qing Zhao <qing.zhao@oracle.com<mailto:qing.zhao@oracle.com><mailto:qing.zhao@oracle.com><mailto:qing.zhao@oracle.com>>
Subject: [PATCH 2/2] Update documentation to clarify a GCC extension
Date: March 28, 2023 at 11:49:44 AM EDT
To: jakub@redhat.com<mailto:jakub@redhat.com><mailto:jakub@redhat.com><mailto:jakub@redhat.com>, joseph@codesourcery.com<mailto:joseph@codesourcery.com><mailto:joseph@codesourcery.com><mailto:joseph@codesourcery.com>
Cc: richard.guenther@gmail.com<mailto:richard.guenther@gmail.com><mailto:richard.guenther@gmail.com><mailto:richard.guenther@gmail.com>, keescook@chromium.org<mailto:keescook@chromium.org><mailto:keescook@chromium.org><mailto:keescook@chromium.org>, siddhesh@gotplt.org<mailto:siddhesh@gotplt.org><mailto:siddhesh@gotplt.org><mailto:siddhesh@gotplt.org>, gcc-patches@gcc.gnu.org<mailto:gcc-patches@gcc.gnu.org><mailto:gcc-patches@gcc.gnu.org><mailto:gcc-patches@gcc.gnu.org>, Qing Zhao <qing.zhao@oracle.com<mailto:qing.zhao@oracle.com><mailto: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 '-Wflex-array-member-not-at-end' to
  identify all such cases in the source code and modify them.  This
  warning will be on by default starting from GCC 14.
"

gcc/c-family/ChangeLog:

* c.opt: New option -Wflex-array-member-not-at-end.

gcc/c/ChangeLog:

* c-decl.cc<http://c-decl.cc><http://c-decl.cc/><http://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><http://c-decl.cc/><http://c-decl.cc<http://c-decl.cc/>>                               |  9 ++++
gcc/doc/extend.texi                           | 45 ++++++++++++++++++-
.../gcc.dg/variable-sized-type-flex-array.c   | 31 +++++++++++++
4 files changed, 89 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gcc.dg/variable-sized-type-flex-array.c

diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 3333cddeece..c26d9801b63 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -737,6 +737,11 @@ Wformat-truncation=
C ObjC C++ LTO ObjC++ Joined RejectNegative UInteger Var(warn_format_trunc) Warning LangEnabledBy(C ObjC C++ LTO ObjC++,Wformat=, warn_format >= 1, 0) IntegerRange(0, 2)
Warn about calls to snprintf and similar functions that truncate output.

+Wflex-array-member-not-at-end
+C C++ Var(warn_flex_array_member_not_at_end) Warning
+Warn when a structure containing a C99 flexible array member as the last
+field is not at the end of another structure.
+
Wif-not-aligned
C ObjC C++ ObjC++ Var(warn_if_not_aligned) Init(1) Warning
Warn when the field in a struct is not aligned.
diff --git a/gcc/c/c-decl.cc<http://c-decl.cc><http://c-decl.cc/><http://c-decl.cc<http://c-decl.cc/>> b/gcc/c/c-decl.cc<http://c-decl.cc/><http://c-decl.cc<http://c-decl.cc/><http://c-decl.cc<http://c-decl.cc/>>%20b/gcc/c/c-decl.cc<http://c-decl.cc/><http://c-decl.cc<http://c-decl.cc/>>>
index 14c54809b9d..92304fd9c8f 100644
--- a/gcc/c/c-decl.cc<http://c-decl.cc><http://c-decl.cc/><http://c-decl.cc<http://c-decl.cc/>>
+++ b/gcc/c/c-decl.cc<http://c-decl.cc><http://c-decl.cc/><http://c-decl.cc<http://c-decl.cc/>>
@@ -9269,6 +9269,15 @@ 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_flex_array_member_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_Wflex_array_member_not_at_end,
+    "structure containing a flexible array member"
+    " is not at the end of another structure");
+
    if (DECL_NAME (x)
|| RECORD_OR_UNION_TYPE_P (TREE_TYPE (x)))
saw_named_field = true;
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 3adb67aa47a..ef46423339e 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{-Wflex-array-member-not-at-end} to
+identify all such cases in the source code and modify them.  This warning
+will be on by default staring from GCC 14.
@end itemize

Non-empty initialization of zero-length
diff --git a/gcc/testsuite/gcc.dg/variable-sized-type-flex-array.c b/gcc/testsuite/gcc.dg/variable-sized-type-flex-array.c
new file mode 100644
index 00000000000..3924937bad4
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/variable-sized-type-flex-array.c
@@ -0,0 +1,31 @@
+/* Test for -Wflex-array-member-not-at-end on structure/union with
+   C99 flexible array members being embedded into another structure.  */
+/* { dg-do compile } */
+/* { dg-options "-Wflex-array-member-not-at-end" } */
+
+struct flex { int n; int data[]; };
+struct out_flex_end { int m; struct flex flex_data; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+struct out_flex_mid { struct flex flex_data; int m; };  /* { dg-warning "structure containing a flexible array member is not at the end of another structure" } */
+/* since the warning has been issued for out_flex_mid, no need to
+   issue warning again when it is included in another structure/union.  */
+struct outer_flex_mid { struct out_flex_mid out_flex_data; int p; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+union flex_union_mid { int a; struct outer_flex_mid b; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+
+
+struct flex0 { int n; int data[0]; };
+struct out_flex_end0 { int m; struct flex0 flex_data; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+struct out_flex_mid0 { struct flex0 flex_data; int m; };  /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+struct outer_flex_mid0 { struct out_flex_mid0 out_flex_data; int p; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+union flex_union_mid0 { int a; struct outer_flex_mid0 b; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+
+struct flex1 { int n; int data[1]; };
+struct out_flex_end1 { int m; struct flex1 flex_data; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+struct out_flex_mid1 { struct flex1 flex_data; int m; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+struct outer_flex_mid1 { struct out_flex_mid1 out_flex_data; int p; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+union flex_union_mid1 { int a; struct outer_flex_mid1 b; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+
+struct flexn { int n; int data[8]; };
+struct out_flex_endn { int m; struct flexn flex_data; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+struct out_flex_midn { struct flexn flex_data; int m; }; /* { dg-bogus"structure containing a flexible array member is not at the end of another structure" } */
+struct outer_flex_midn { struct out_flex_midn out_flex_data; int p; }; /* { dg-bogus"structure containing a flexible array member is not at the end of another structure" } */
+union flex_union_midn { int a; struct outer_flex_midn b; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
--
2.31.1



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

* Fwd: [V6][PATCH 0/2] Handle component_ref to a structure/union field including FAM for builtin_object_size
  2023-03-28 15:49 [V6][PATCH 0/2] Handle component_ref to a structure/union field including FAM for builtin_object_size Qing Zhao
  2023-03-28 15:49 ` [V6][PATCH 1/2] Handle component_ref to a structre/union field including flexible array member [PR101832] Qing Zhao
  2023-03-28 15:49 ` [PATCH 2/2] Update documentation to clarify a GCC extension Qing Zhao
@ 2023-04-11 13:35 ` Qing Zhao
  2023-04-20 14:08 ` Ping * 3: " Qing Zhao
  3 siblings, 0 replies; 13+ messages in thread
From: Qing Zhao @ 2023-04-11 13:35 UTC (permalink / raw)
  To: Jakub Jelinek, Joseph Myers
  Cc: Richard Biener, kees Cook, Siddhesh Poyarekar, gcc Patches

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

Hi, Joseph and Jakub,

This is the 2nd ping to the 6th version of the patches -:)

Please let me know if you have any further comments on the patches, and whether it’s Okay to commit them to trunk?

Thanks a lot for the help.

Qing

Begin forwarded message:

From: Qing Zhao <qing.zhao@oracle.com<mailto:qing.zhao@oracle.com>>
Subject: [V6][PATCH 0/2] Handle component_ref to a structure/union field including FAM for builtin_object_size
Date: March 28, 2023 at 11:49:42 AM EDT
To: jakub@redhat.com<mailto:jakub@redhat.com>, joseph@codesourcery.com<mailto:joseph@codesourcery.com>
Cc: richard.guenther@gmail.com<mailto:richard.guenther@gmail.com>, keescook@chromium.org<mailto:keescook@chromium.org>, siddhesh@gotplt.org<mailto:siddhesh@gotplt.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 and Jakub,

this is the 6th version of the patch.
compared to the 5th version, the major changes are:

1. Update the documentation Per Joseph's comments;
2. Change the name of the new warning option per Jakub's suggestions.
3. Update testing case per the above change.

these changes are all in the 2th patch (2/2 Update documentation to
clarify a GCC extension).

The first patch (1/2 Handle component_ref to a structre/union field
including  flexible array member [PR101832]) is not changed

For the first patch, As a record, Joseph has approved the C front-end change,
I only need a review from Jakub for the Middle-end.

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>                               |  20 +++
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, 281 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] 13+ messages in thread

* Ping * 3: Fwd: [V6][PATCH 0/2] Handle component_ref to a structure/union field including FAM for builtin_object_size
  2023-03-28 15:49 [V6][PATCH 0/2] Handle component_ref to a structure/union field including FAM for builtin_object_size Qing Zhao
                   ` (2 preceding siblings ...)
  2023-04-11 13:35 ` Fwd: [V6][PATCH 0/2] Handle component_ref to a structure/union field including FAM for builtin_object_size Qing Zhao
@ 2023-04-20 14:08 ` Qing Zhao
  3 siblings, 0 replies; 13+ messages in thread
From: Qing Zhao @ 2023-04-20 14:08 UTC (permalink / raw)
  To: Jakub Jelinek, Joseph Myers, gcc Patches
  Cc: Richard Biener, Kees Cook, Siddhesh Poyarekar

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

This is the 3rd ping for the 6th version of the patches.

Now, GCC14 is open. Is it ready to commit these patches to GCC14?

Kees has tested this version of the patch with Linux kernel, and everything is good, and relsolved many false
positives for bounds checking.

Note for the review history of these patches (2 patches)
1.  The patch 1/2: Handle component_ref to a structre/union field including  flexible array member [PR101832]

    The C front-end part has been approved by Joseph.
    For the middle-end, most of the change has been reviewed by Richard (and modified based on his comments
     and suggestions), except the change in tree-object-size.cc<http://tree-object-size.cc>, which need Jakub’s review and approval.

    Jakub, could you review the middle end of change to see whether it’s ready for trunk?

3. The patch 2/2: Update documentation to clarify a GCC extension

    This is basically a C FE and documentation change, I have updated it based on previous comments and suggestions.
    Joseph, could you review it to see whether this version is ready to go?

Thanks a lot.

Qing

Begin forwarded message:

From: Qing Zhao <qing.zhao@oracle.com<mailto:qing.zhao@oracle.com>>
Subject: [V6][PATCH 0/2] Handle component_ref to a structure/union field including FAM for builtin_object_size
Date: March 28, 2023 at 11:49:42 AM EDT
To: jakub@redhat.com<mailto:jakub@redhat.com>, joseph@codesourcery.com<mailto:joseph@codesourcery.com>
Cc: richard.guenther@gmail.com<mailto:richard.guenther@gmail.com>, keescook@chromium.org<mailto:keescook@chromium.org>, siddhesh@gotplt.org<mailto:siddhesh@gotplt.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 and Jakub,

this is the 6th version of the patch.
compared to the 5th version, the major changes are:

1. Update the documentation Per Joseph's comments;
2. Change the name of the new warning option per Jakub's suggestions.
3. Update testing case per the above change.

these changes are all in the 2th patch (2/2 Update documentation to
clarify a GCC extension).

The first patch (1/2 Handle component_ref to a structre/union field
including  flexible array member [PR101832]) is not changed

For the first patch, As a record, Joseph has approved the C front-end change,
I only need a review from Jakub for the Middle-end.

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>                               |  20 +++
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, 281 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] 13+ messages in thread

end of thread, other threads:[~2023-04-20 14:12 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-28 15:49 [V6][PATCH 0/2] Handle component_ref to a structure/union field including FAM for builtin_object_size Qing Zhao
2023-03-28 15:49 ` [V6][PATCH 1/2] Handle component_ref to a structre/union field including flexible array member [PR101832] Qing Zhao
2023-04-04 13:06   ` Fwd: " Qing Zhao
2023-04-11 13:37     ` Qing Zhao
2023-04-20 14:10       ` Ping * 3: " Qing Zhao
2023-04-12 18:46   ` Kees Cook
2023-04-17 12:56     ` Qing Zhao
2023-03-28 15:49 ` [PATCH 2/2] Update documentation to clarify a GCC extension Qing Zhao
2023-04-04 13:07   ` Fwd: [V6][PATCH " Qing Zhao
2023-04-11 13:38     ` Qing Zhao
2023-04-20 14:11       ` Ping * 3: " Qing Zhao
2023-04-11 13:35 ` Fwd: [V6][PATCH 0/2] Handle component_ref to a structure/union field including FAM for builtin_object_size Qing Zhao
2023-04-20 14:08 ` Ping * 3: " 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).