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

[Qing Zhao <qing.zhao@oracle.com>]

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

Ping * 2.

Hi, Joseph and Richard,

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

This is an important bug that need to be fixed for kernel security purpose.

thanks.

Qing

Begin forwarded message:

From: Qing Zhao <qing.zhao@oracle.com<mailto:qing.zhao@oracle.com>>
Subject: [v3][PATCH 1/2] Handle component_ref to a structre/union field including C99 FAM [PR101832]
Date: February 10, 2023 at 7:50:12 PM EST
To: joseph@codesourcery.com<mailto:joseph@codesourcery.com>, rguenther@suse.de<mailto:rguenther@suse.de>
Cc: siddhesh@gotplt.org<mailto:siddhesh@gotplt.org>, keescook@chromium.org<mailto:keescook@chromium.org>, gcc-patches@gcc.gnu.org<mailto:gcc-patches@gcc.gnu.org>, Qing Zhao <qing.zhao@oracle.com<mailto:qing.zhao@oracle.com>>

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

gcc/c/ChangeLog:

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

gcc/cp/ChangeLog:

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

gcc/ChangeLog:

PR tree-optimization/101832
* print-tree.cc<http://print-tree.cc> (print_node): Print new bit type_include_flexarray.
* tree-core.h (struct tree_type_common): New bit
type_include_flexarray.
* tree-object-size.cc<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 new bit type_include_flexarray.
* tree-streamer-out.cc<http://tree-streamer-out.cc> (pack_ts_type_common_value_fields): Stream
out new bit type_include_flexarray.
* tree.h (TYPE_INCLUDE_FLEXARRAY): New macro
TYPE_INCLUDE_FLEXARRAY.

gcc/testsuite/ChangeLog:

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

diff --git a/gcc/c/c-decl.cc<http://c-decl.cc> b/gcc/c/c-decl.cc<http://c-decl.cc>
index 20e7d1855bf..741a37560b0 100644
--- a/gcc/c/c-decl.cc<http://c-decl.cc>
+++ b/gcc/c/c-decl.cc<http://c-decl.cc>
@@ -9277,6 +9277,18 @@ finish_struct (location_t loc, tree t, tree fieldlist, tree attributes,
      /* Set DECL_NOT_FLEXARRAY flag for FIELD_DECL x.  */
      DECL_NOT_FLEXARRAY (x) = !is_flexible_array_member_p (is_last_field, x);

+      /* Set TYPE_INCLUDE_FLEXARRAY for the context of x, t
+       * when x is an array.  */
+      if (TREE_CODE (TREE_TYPE (x)) == ARRAY_TYPE)
+ TYPE_INCLUDE_FLEXARRAY (t) = flexible_array_member_type_p (TREE_TYPE (x)) ;
+      /* Recursively set TYPE_INCLUDE_FLEXARRAY for the context of x, t
+ when x is the last field.  */
+      else if ((TREE_CODE (TREE_TYPE (x)) == RECORD_TYPE
+ || TREE_CODE (TREE_TYPE (x)) == UNION_TYPE)
+       && TYPE_INCLUDE_FLEXARRAY (TREE_TYPE (x))
+       && is_last_field)
+ TYPE_INCLUDE_FLEXARRAY (t) = true;
+
      if (DECL_NAME (x)
 || RECORD_OR_UNION_TYPE_P (TREE_TYPE (x)))
saw_named_field = true;
diff --git a/gcc/cp/module.cc<http://module.cc> b/gcc/cp/module.cc<http://module.cc>
index ac2fe66b080..c750361b704 100644
--- a/gcc/cp/module.cc<http://module.cc>
+++ b/gcc/cp/module.cc<http://module.cc>
@@ -5371,6 +5371,7 @@ trees_out::core_bools (tree t)
      WB (t->type_common.lang_flag_5);
      WB (t->type_common.lang_flag_6);
      WB (t->type_common.typeless_storage);
+      WB (t->type_common.type_include_flexarray);
    }

  if (CODE_CONTAINS_STRUCT (code, TS_DECL_COMMON))
@@ -5551,6 +5552,7 @@ trees_in::core_bools (tree t)
      RB (t->type_common.lang_flag_5);
      RB (t->type_common.lang_flag_6);
      RB (t->type_common.typeless_storage);
+      RB (t->type_common.type_include_flexarray);
    }

  if (CODE_CONTAINS_STRUCT (code, TS_DECL_COMMON))
diff --git a/gcc/print-tree.cc<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 acd8deea34e..705d5702b9c 100644
--- a/gcc/tree-core.h
+++ b/gcc/tree-core.h
@@ -1718,7 +1718,9 @@ struct GTY(()) tree_type_common {
  unsigned empty_flag : 1;
  unsigned indivisible_p : 1;
  unsigned no_named_args_stdarg_p : 1;
-  unsigned spare : 15;
+  /* TYPE_INCLUDE_FLEXARRAY flag for RECORD_TYPE and UNION_TYPE.  */
+  unsigned int type_include_flexarray : 1;
+  unsigned spare : 14;

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

+/* True if this RECORD_TYPE or UNION_TYPE includes a flexible array member
+   at the last field recursively.  */
+#define TYPE_INCLUDE_FLEXARRAY(NODE) \
+  (TYPE_CHECK (NODE)->type_common.type_include_flexarray)
+
+
/* In an IDENTIFIER_NODE, this means that assemble_name was called with
   this string as an argument.  */
#define TREE_SYMBOL_REFERENCED(NODE) \
--
2.31.1