public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Qing Zhao <qing.zhao@oracle.com>
To: Jakub Jelinek <jakub@redhat.com>
Cc: Joseph Myers <joseph@codesourcery.com>,
	Richard Biener <richard.guenther@gmail.com>,
	kees Cook <keescook@chromium.org>,
	Siddhesh Poyarekar <siddhesh@gotplt.org>,
	gcc Patches <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: Tue, 11 Apr 2023 13:37:18 +0000	[thread overview]
Message-ID: <9FB1E5C7-4229-49A8-851E-8AC3B38ABC82@oracle.com> (raw)
In-Reply-To: <4C199CEE-3796-41A3-AB1E-E4CC847888D7@oracle.com>

[-- 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




  reply	other threads:[~2023-04-11 13:37 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9FB1E5C7-4229-49A8-851E-8AC3B38ABC82@oracle.com \
    --to=qing.zhao@oracle.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jakub@redhat.com \
    --cc=joseph@codesourcery.com \
    --cc=keescook@chromium.org \
    --cc=richard.guenther@gmail.com \
    --cc=siddhesh@gotplt.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).