[v4,1/2] Handle component_ref to a structre/union field including C99 FAM [PR101832]
Checks
Commit Message
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 (finish_struct): Set TYPE_INCLUDE_FLEXARRAY for
struct/union type.
gcc/cp/ChangeLog:
PR tree-optimization/101832
* 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 (print_node): Print new bit type_include_flexarray.
* tree-core.h (struct tree_type_common): New bit
type_include_flexarray.
* 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 new bit type_include_flexarray.
* 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 | 12 ++
gcc/cp/module.cc | 2 +
gcc/print-tree.cc | 5 +
.../gcc.dg/builtin-object-size-pr101832.c | 134 ++++++++++++++++++
gcc/tree-core.h | 4 +-
gcc/tree-object-size.cc | 79 +++++++----
gcc/tree-streamer-in.cc | 1 +
gcc/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
Comments
Ping.
Qing
> On Feb 24, 2023, at 1:35 PM, Qing Zhao <qing.zhao@oracle.com> wrote:
>
> 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 (finish_struct): Set TYPE_INCLUDE_FLEXARRAY for
> struct/union type.
>
> gcc/cp/ChangeLog:
>
> PR tree-optimization/101832
> * 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 (print_node): Print new bit type_include_flexarray.
> * tree-core.h (struct tree_type_common): New bit
> type_include_flexarray.
> * 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 new bit type_include_flexarray.
> * 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 | 12 ++
> gcc/cp/module.cc | 2 +
> gcc/print-tree.cc | 5 +
> .../gcc.dg/builtin-object-size-pr101832.c | 134 ++++++++++++++++++
> gcc/tree-core.h | 4 +-
> gcc/tree-object-size.cc | 79 +++++++----
> gcc/tree-streamer-in.cc | 1 +
> gcc/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 b/gcc/c/c-decl.cc
> index 08078eadeb8..f589a2f5192 100644
> --- a/gcc/c/c-decl.cc
> +++ b/gcc/c/c-decl.cc
> @@ -9284,6 +9284,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 b/gcc/cp/module.cc
> index ac2fe66b080..c750361b704 100644
> --- a/gcc/cp/module.cc
> +++ b/gcc/cp/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 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 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 b/gcc/tree-object-size.cc
> index 9a936a91983..22b3c72ea6e 100644
> --- a/gcc/tree-object-size.cc
> +++ b/gcc/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 b/gcc/tree-streamer-in.cc
> index d4dc30f048f..c19ede0631d 100644
> --- a/gcc/tree-streamer-in.cc
> +++ b/gcc/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 b/gcc/tree-streamer-out.cc
> index d107229da5c..73e4b4e547c 100644
> --- a/gcc/tree-streamer-out.cc
> +++ b/gcc/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 92ac0e6a214..ab1cdc3dc85 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
>
On Fri, 24 Feb 2023, Qing Zhao wrote:
> 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 (finish_struct): Set TYPE_INCLUDE_FLEXARRAY for
> struct/union type.
I can't really comment on the correctness of this part but since
only the C frontend will ever set this and you are using it from
addr_object_size which is also used for other C family languages
(at least), I wonder if you can really test
+ if (!TYPE_INCLUDE_FLEXARRAY (TREE_TYPE (v)))
there.
Originally I was suggesting to set this flag in stor-layout.cc
which eventually all languages funnel their types through and
if there's language specific handling use a langhook (with the
default implementation preserving the status quo).
Some more comments below ...
> gcc/cp/ChangeLog:
>
> PR tree-optimization/101832
> * 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 (print_node): Print new bit type_include_flexarray.
> * tree-core.h (struct tree_type_common): New bit
> type_include_flexarray.
> * 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 new bit type_include_flexarray.
> * 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 | 12 ++
> gcc/cp/module.cc | 2 +
> gcc/print-tree.cc | 5 +
> .../gcc.dg/builtin-object-size-pr101832.c | 134 ++++++++++++++++++
> gcc/tree-core.h | 4 +-
> gcc/tree-object-size.cc | 79 +++++++----
> gcc/tree-streamer-in.cc | 1 +
> gcc/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 b/gcc/c/c-decl.cc
> index 08078eadeb8..f589a2f5192 100644
> --- a/gcc/c/c-decl.cc
> +++ b/gcc/c/c-decl.cc
> @@ -9284,6 +9284,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 b/gcc/cp/module.cc
> index ac2fe66b080..c750361b704 100644
> --- a/gcc/cp/module.cc
> +++ b/gcc/cp/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 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 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;
it looks like the bit could be eventually shared with
no_named_args_stdarg_p (but its accessor doesn't use
FUNC_OR_METHOD_CHECK)
> alias_set_type alias_set;
> tree pointer_to;
> diff --git a/gcc/tree-object-size.cc b/gcc/tree-object-size.cc
> index 9a936a91983..22b3c72ea6e 100644
> --- a/gcc/tree-object-size.cc
> +++ b/gcc/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 b/gcc/tree-streamer-in.cc
> index d4dc30f048f..c19ede0631d 100644
> --- a/gcc/tree-streamer-in.cc
> +++ b/gcc/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 b/gcc/tree-streamer-out.cc
> index d107229da5c..73e4b4e547c 100644
> --- a/gcc/tree-streamer-out.cc
> +++ b/gcc/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 92ac0e6a214..ab1cdc3dc85 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)
Please use RECORD_OR_UNION_CHECK. The comment "at the last field"
doesn't seem to match implementation? (at least not obviously)
Given this is a generic header expanding on what a "flexible array member"
is to the middle-end here would be good. Especially the guarantees
made when the bit is _not_ set (we've evaded that with DECL_NOT_FLEXARRAY
vs. DECL_FLEXARRAY).
Thanks,
Richard.
> On Mar 9, 2023, at 7:20 AM, Richard Biener <rguenther@suse.de> wrote:
>
> On Fri, 24 Feb 2023, Qing Zhao wrote:
>
>> 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 (finish_struct): Set TYPE_INCLUDE_FLEXARRAY for
>> struct/union type.
>
> I can't really comment on the correctness of this part but since
> only the C frontend will ever set this and you are using it from
> addr_object_size which is also used for other C family languages
> (at least), I wonder if you can really test
>
> + if (!TYPE_INCLUDE_FLEXARRAY (TREE_TYPE (v)))
>
> there.
You mean for C++ and also other C family languages (other than C), the above bit cannot be set?
Yes, that’s true. The bit is only set for C. So is the bit DECL_NOT_FLEXARRAY, which is only set for C too.
So, I am wondering whether the bit DECL_NOT_FLEXARRAY should be also set in middle end? Or we can set DECL_NOT_FLEXARRAY in C++ FE too? And then we can set TYPE_INCLUDE_FLEXARRAY also in C++ FE?
What’s your suggestion?
(I personally feel that DECL_NOT_FLEXARRAY and TYPE_INCLUDE_FLEXARRAY should be set in the same places).
>
> Originally I was suggesting to set this flag in stor-layout.cc
> which eventually all languages funnel their types through and
> if there's language specific handling use a langhook (with the
> default implementation preserving the status quo).
If we decide to set the bits in stor-layout.cc, where is the best place to do it? I checked the star-layout.cc code, looks like “layout_type” might be the place where we can set these bits for RECORD_TYPE, UNION_TYPE?
>
> Some more comments below ...
>
>> gcc/cp/ChangeLog:
>>
>> PR tree-optimization/101832
>> * 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 (print_node): Print new bit type_include_flexarray.
>> * tree-core.h (struct tree_type_common): New bit
>> type_include_flexarray.
>> * 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 new bit type_include_flexarray.
>> * 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 | 12 ++
>> gcc/cp/module.cc | 2 +
>> gcc/print-tree.cc | 5 +
>> .../gcc.dg/builtin-object-size-pr101832.c | 134 ++++++++++++++++++
>> gcc/tree-core.h | 4 +-
>> gcc/tree-object-size.cc | 79 +++++++----
>> gcc/tree-streamer-in.cc | 1 +
>> gcc/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 b/gcc/c/c-decl.cc
>> index 08078eadeb8..f589a2f5192 100644
>> --- a/gcc/c/c-decl.cc
>> +++ b/gcc/c/c-decl.cc
>> @@ -9284,6 +9284,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 b/gcc/cp/module.cc
>> index ac2fe66b080..c750361b704 100644
>> --- a/gcc/cp/module.cc
>> +++ b/gcc/cp/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 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 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;
>> RECORD_OR_UNION_CHECK
>
> it looks like the bit could be eventually shared with
> no_named_args_stdarg_p (but its accessor doesn't use
> FUNC_OR_METHOD_CHECK)
You mean to let “type_include_flexarray” share the same bit with “no_named_args_stdarg_p” in order to save 1-bit space in IR?
Then change the
/* True if this is a stdarg function with no named arguments (C2x
(...) 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)
/* 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)
To:
union {
unsigned no_named_args_stdarg_p : 1;
/* TYPE_INCLUDE_FLEXARRAY flag for RECORD_TYPE and UNION_TYPE. */
unsigned int type_include_flexarray : 1;
} shared_bit;
unsigned spare : 15;
/* True if this is a stdarg function with no named arguments (C2x
(...) 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) \
(FUNC_OR_METHOD_CHECK (NODE)->type_common.shared_bit.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.shared_bit.type_include_flexarray)
>
>> alias_set_type alias_set;
>> tree pointer_to;
>> diff --git a/gcc/tree-object-size.cc b/gcc/tree-object-size.cc
>> index 9a936a91983..22b3c72ea6e 100644
>> --- a/gcc/tree-object-size.cc
>> +++ b/gcc/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 b/gcc/tree-streamer-in.cc
>> index d4dc30f048f..c19ede0631d 100644
>> --- a/gcc/tree-streamer-in.cc
>> +++ b/gcc/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 b/gcc/tree-streamer-out.cc
>> index d107229da5c..73e4b4e547c 100644
>> --- a/gcc/tree-streamer-out.cc
>> +++ b/gcc/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 92ac0e6a214..ab1cdc3dc85 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)
>
> Please use RECORD_OR_UNION_CHECK.
Okay.
> The comment "at the last field"
> doesn't seem to match implementation? (at least not obviously)
The current implementation (in gcc/c/c-decl.cc) guarantees that it’s at the last field. (And recursively).
> Given this is a generic header expanding on what a "flexible array member"
> is to the middle-end here would be good. Especially the guarantees
> made when the bit is _not_ set (we've evaded that with DECL_NOT_FLEXARRAY
> vs. DECL_FLEXARRAY).
The bit TYPE_INCLUDE_FLEXARRAY default is FALSE, i.e, not including a flex array by default.
It’s only set to TRUE when the struct/union includes a flex array at the last field (and recursively). So, I think it should be correct.
Let me know if I missed anything here.
Thanks a lot for your comments.
Qing
>
> Thanks,
> Richard.
On Thu, 9 Mar 2023, Qing Zhao wrote:
>
>
> > On Mar 9, 2023, at 7:20 AM, Richard Biener <rguenther@suse.de> wrote:
> >
> > On Fri, 24 Feb 2023, Qing Zhao wrote:
> >
> >> 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 (finish_struct): Set TYPE_INCLUDE_FLEXARRAY for
> >> struct/union type.
> >
> > I can't really comment on the correctness of this part but since
> > only the C frontend will ever set this and you are using it from
> > addr_object_size which is also used for other C family languages
> > (at least), I wonder if you can really test
> >
> > + if (!TYPE_INCLUDE_FLEXARRAY (TREE_TYPE (v)))
> >
> > there.
>
> You mean for C++ and also other C family languages (other than C), the above bit cannot be set?
> Yes, that’s true. The bit is only set for C. So is the bit DECL_NOT_FLEXARRAY, which is only set for C too.
> So, I am wondering whether the bit DECL_NOT_FLEXARRAY should be also set in middle end? Or we can set DECL_NOT_FLEXARRAY in C++ FE too? And then we can set TYPE_INCLUDE_FLEXARRAY also in C++ FE?
> What’s your suggestion?
>
> (I personally feel that DECL_NOT_FLEXARRAY and TYPE_INCLUDE_FLEXARRAY should be set in the same places).
I was wondering if the above test errors on the conservative side
correctly - it will now, for all but C, cut off some thing where it
didn't before?
> >
> > Originally I was suggesting to set this flag in stor-layout.cc
> > which eventually all languages funnel their types through and
> > if there's language specific handling use a langhook (with the
> > default implementation preserving the status quo).
>
> If we decide to set the bits in stor-layout.cc, where is the best place to do it? I checked the star-layout.cc code, looks like “layout_type” might be the place where we can set these bits for RECORD_TYPE, UNION_TYPE?
Yes, it would be layout_type.
> >
> > Some more comments below ...
> >
> >> gcc/cp/ChangeLog:
> >>
> >> PR tree-optimization/101832
> >> * 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 (print_node): Print new bit type_include_flexarray.
> >> * tree-core.h (struct tree_type_common): New bit
> >> type_include_flexarray.
> >> * 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 new bit type_include_flexarray.
> >> * 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 | 12 ++
> >> gcc/cp/module.cc | 2 +
> >> gcc/print-tree.cc | 5 +
> >> .../gcc.dg/builtin-object-size-pr101832.c | 134 ++++++++++++++++++
> >> gcc/tree-core.h | 4 +-
> >> gcc/tree-object-size.cc | 79 +++++++----
> >> gcc/tree-streamer-in.cc | 1 +
> >> gcc/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 b/gcc/c/c-decl.cc
> >> index 08078eadeb8..f589a2f5192 100644
> >> --- a/gcc/c/c-decl.cc
> >> +++ b/gcc/c/c-decl.cc
> >> @@ -9284,6 +9284,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 b/gcc/cp/module.cc
> >> index ac2fe66b080..c750361b704 100644
> >> --- a/gcc/cp/module.cc
> >> +++ b/gcc/cp/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 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 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;
> >> RECORD_OR_UNION_CHECK
> >
> > it looks like the bit could be eventually shared with
> > no_named_args_stdarg_p (but its accessor doesn't use
> > FUNC_OR_METHOD_CHECK)
> You mean to let “type_include_flexarray” share the same bit with “no_named_args_stdarg_p” in order to save 1-bit space in IR?
> Then change the
>
> /* True if this is a stdarg function with no named arguments (C2x
> (...) 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)
>
> /* 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)
>
> To:
>
> union {
> unsigned no_named_args_stdarg_p : 1;
> /* TYPE_INCLUDE_FLEXARRAY flag for RECORD_TYPE and UNION_TYPE. */
> unsigned int type_include_flexarray : 1;
> } shared_bit;
> unsigned spare : 15;
>
> /* True if this is a stdarg function with no named arguments (C2x
> (...) 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) \
> (FUNC_OR_METHOD_CHECK (NODE)->type_common.shared_bit.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.shared_bit.type_include_flexarray)
No, we're just overloading bits by using the same name for different
purposes. I don't think such a union would pack nicely. We overload
by documenting the uses, in the above cases the uses are separated
by the kind of the tree node, RECORD/UNION_TYPE vs. FUNCTION/METHOD_TYPE
and the accessor macros should be updated to use the appropriate
tree check macros covering that so we don't try to inspect the
"wrong bit".
> >
> >> alias_set_type alias_set;
> >> tree pointer_to;
> >> diff --git a/gcc/tree-object-size.cc b/gcc/tree-object-size.cc
> >> index 9a936a91983..22b3c72ea6e 100644
> >> --- a/gcc/tree-object-size.cc
> >> +++ b/gcc/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 b/gcc/tree-streamer-in.cc
> >> index d4dc30f048f..c19ede0631d 100644
> >> --- a/gcc/tree-streamer-in.cc
> >> +++ b/gcc/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 b/gcc/tree-streamer-out.cc
> >> index d107229da5c..73e4b4e547c 100644
> >> --- a/gcc/tree-streamer-out.cc
> >> +++ b/gcc/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 92ac0e6a214..ab1cdc3dc85 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)
> >
> > Please use RECORD_OR_UNION_CHECK.
> Okay.
> > The comment "at the last field"
> > doesn't seem to match implementation? (at least not obviously)
> The current implementation (in gcc/c/c-decl.cc) guarantees that it’s at the last field. (And recursively).
> > Given this is a generic header expanding on what a "flexible array member"
> > is to the middle-end here would be good. Especially the guarantees
> > made when the bit is _not_ set (we've evaded that with DECL_NOT_FLEXARRAY
> > vs. DECL_FLEXARRAY).
>
> The bit TYPE_INCLUDE_FLEXARRAY default is FALSE, i.e, not including a flex array by default.
> It’s only set to TRUE when the struct/union includes a flex array at the last field (and recursively). So, I think it should be correct.
> Let me know if I missed anything here.
See above - I was looking at the use (but I'm not familiar with that
code), and wondered how the change affects uses from other frontends.
Richard.
> On Mar 10, 2023, at 2:54 AM, Richard Biener <rguenther@suse.de> wrote:
>
> On Thu, 9 Mar 2023, Qing Zhao wrote:
>
>>
>>
>>> On Mar 9, 2023, at 7:20 AM, Richard Biener <rguenther@suse.de> wrote:
>>>
>>> On Fri, 24 Feb 2023, Qing Zhao wrote:
>>>
>>>> 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 (finish_struct): Set TYPE_INCLUDE_FLEXARRAY for
>>>> struct/union type.
>>>
>>> I can't really comment on the correctness of this part but since
>>> only the C frontend will ever set this and you are using it from
>>> addr_object_size which is also used for other C family languages
>>> (at least), I wonder if you can really test
>>>
>>> + if (!TYPE_INCLUDE_FLEXARRAY (TREE_TYPE (v)))
>>>
>>> there.
>>
>> You mean for C++ and also other C family languages (other than C), the above bit cannot be set?
>> Yes, that’s true. The bit is only set for C. So is the bit DECL_NOT_FLEXARRAY, which is only set for C too.
>> So, I am wondering whether the bit DECL_NOT_FLEXARRAY should be also set in middle end? Or we can set DECL_NOT_FLEXARRAY in C++ FE too? And then we can set TYPE_INCLUDE_FLEXARRAY also in C++ FE?
>> What’s your suggestion?
>>
>> (I personally feel that DECL_NOT_FLEXARRAY and TYPE_INCLUDE_FLEXARRAY should be set in the same places).
>
> I was wondering if the above test errors on the conservative side
> correctly - it will now, for all but C, cut off some thing where it
> didn't before?
As long as the default value of TYPE_INCLUDE_FLEXARRAY reflects the correct conservative behavior, then the testing should be correct, right?
The default value of TYPE_INCLUDE_FLEXARRAY is 0, i.e, FALSE, means that the TYPE does not include a flex array by default, this is the correct behavior. Only when the TYPE does include a flexiarray, it will be set to TRUE. So, I think it’s correct.
This is a different situation for DECL_NOT_FLEXARRAY, by default, the compiler will treat ALL trailing arrays as FMA, so in order to keep the correct conservative behavior, we should keep the default value for DECL_NOT_FLEXARRAY as it’s a FMA, i.e, DECL_NOT_FLEXARRAY being FALSE, by default. Only when the array is NOT a FMA, we set it to true.
So, the default value for TYPE_INCLUDE_FLEXARRAY is correct.
>
>>>
>>> Originally I was suggesting to set this flag in stor-layout.cc
>>> which eventually all languages funnel their types through and
>>> if there's language specific handling use a langhook (with the
>>> default implementation preserving the status quo).
>>
>> If we decide to set the bits in stor-layout.cc, where is the best place to do it? I checked the star-layout.cc code, looks like “layout_type” might be the place where we can set these bits for RECORD_TYPE, UNION_TYPE?
>
> Yes, it would be layout_type.
>
>>>
>>> Some more comments below ...
>>>
>>>> gcc/cp/ChangeLog:
>>>>
>>>> PR tree-optimization/101832
>>>> * 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 (print_node): Print new bit type_include_flexarray.
>>>> * tree-core.h (struct tree_type_common): New bit
>>>> type_include_flexarray.
>>>> * 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 new bit type_include_flexarray.
>>>> * 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 | 12 ++
>>>> gcc/cp/module.cc | 2 +
>>>> gcc/print-tree.cc | 5 +
>>>> .../gcc.dg/builtin-object-size-pr101832.c | 134 ++++++++++++++++++
>>>> gcc/tree-core.h | 4 +-
>>>> gcc/tree-object-size.cc | 79 +++++++----
>>>> gcc/tree-streamer-in.cc | 1 +
>>>> gcc/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 b/gcc/c/c-decl.cc
>>>> index 08078eadeb8..f589a2f5192 100644
>>>> --- a/gcc/c/c-decl.cc
>>>> +++ b/gcc/c/c-decl.cc
>>>> @@ -9284,6 +9284,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 b/gcc/cp/module.cc
>>>> index ac2fe66b080..c750361b704 100644
>>>> --- a/gcc/cp/module.cc
>>>> +++ b/gcc/cp/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 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 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;
>>>> RECORD_OR_UNION_CHECK
>>>
>>> it looks like the bit could be eventually shared with
>>> no_named_args_stdarg_p (but its accessor doesn't use
>>> FUNC_OR_METHOD_CHECK)
>> You mean to let “type_include_flexarray” share the same bit with “no_named_args_stdarg_p” in order to save 1-bit space in IR?
>> Then change the
>>
>> /* True if this is a stdarg function with no named arguments (C2x
>> (...) 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)
>>
>> /* 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)
>>
>> To:
>>
>> union {
>> unsigned no_named_args_stdarg_p : 1;
>> /* TYPE_INCLUDE_FLEXARRAY flag for RECORD_TYPE and UNION_TYPE. */
>> unsigned int type_include_flexarray : 1;
>> } shared_bit;
>> unsigned spare : 15;
>>
>> /* True if this is a stdarg function with no named arguments (C2x
>> (...) 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) \
>> (FUNC_OR_METHOD_CHECK (NODE)->type_common.shared_bit.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.shared_bit.type_include_flexarray)
>
> No, we're just overloading bits by using the same name for different
> purposes. I don't think such a union would pack nicely. We overload
> by documenting the uses, in the above cases the uses are separated
> by the kind of the tree node, RECORD/UNION_TYPE vs. FUNCTION/METHOD_TYPE
> and the accessor macros should be updated to use the appropriate
> tree check macros covering that so we don't try to inspect the
> "wrong bit”.
Okay, I see. Then should I change the name of that bit to one that reflect two usages?
>
>>>
>>>> alias_set_type alias_set;
>>>> tree pointer_to;
>>>> diff --git a/gcc/tree-object-size.cc b/gcc/tree-object-size.cc
>>>> index 9a936a91983..22b3c72ea6e 100644
>>>> --- a/gcc/tree-object-size.cc
>>>> +++ b/gcc/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 b/gcc/tree-streamer-in.cc
>>>> index d4dc30f048f..c19ede0631d 100644
>>>> --- a/gcc/tree-streamer-in.cc
>>>> +++ b/gcc/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 b/gcc/tree-streamer-out.cc
>>>> index d107229da5c..73e4b4e547c 100644
>>>> --- a/gcc/tree-streamer-out.cc
>>>> +++ b/gcc/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 92ac0e6a214..ab1cdc3dc85 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)
>>>
>>> Please use RECORD_OR_UNION_CHECK.
>> Okay.
>>> The comment "at the last field"
>>> doesn't seem to match implementation? (at least not obviously)
>> The current implementation (in gcc/c/c-decl.cc) guarantees that it’s at the last field. (And recursively).
>>> Given this is a generic header expanding on what a "flexible array member"
>>> is to the middle-end here would be good. Especially the guarantees
>>> made when the bit is _not_ set (we've evaded that with DECL_NOT_FLEXARRAY
>>> vs. DECL_FLEXARRAY).
>>
>> The bit TYPE_INCLUDE_FLEXARRAY default is FALSE, i.e, not including a flex array by default.
>> It’s only set to TRUE when the struct/union includes a flex array at the last field (and recursively). So, I think it should be correct.
>> Let me know if I missed anything here.
>
> See above - I was looking at the use (but I'm not familiar with that
> code), and wondered how the change affects uses from other frontends.
Please see my explanation above, I think the default behavior from other FE should be kept correctly with this patch.
thanks.
Qing
>
> Richard.
Hi, Richard,
Do you have more comments on my responds to your previous questions?
thanks.
Qing
> On Mar 10, 2023, at 8:48 AM, Qing Zhao via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
>
>
>
>> On Mar 10, 2023, at 2:54 AM, Richard Biener <rguenther@suse.de> wrote:
>>
>> On Thu, 9 Mar 2023, Qing Zhao wrote:
>>
>>>
>>>
>>>> On Mar 9, 2023, at 7:20 AM, Richard Biener <rguenther@suse.de> wrote:
>>>>
>>>> On Fri, 24 Feb 2023, Qing Zhao wrote:
>>>>
>>>>> 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 (finish_struct): Set TYPE_INCLUDE_FLEXARRAY for
>>>>> struct/union type.
>>>>
>>>> I can't really comment on the correctness of this part but since
>>>> only the C frontend will ever set this and you are using it from
>>>> addr_object_size which is also used for other C family languages
>>>> (at least), I wonder if you can really test
>>>>
>>>> + if (!TYPE_INCLUDE_FLEXARRAY (TREE_TYPE (v)))
>>>>
>>>> there.
>>>
>>> You mean for C++ and also other C family languages (other than C), the above bit cannot be set?
>>> Yes, that’s true. The bit is only set for C. So is the bit DECL_NOT_FLEXARRAY, which is only set for C too.
>>> So, I am wondering whether the bit DECL_NOT_FLEXARRAY should be also set in middle end? Or we can set DECL_NOT_FLEXARRAY in C++ FE too? And then we can set TYPE_INCLUDE_FLEXARRAY also in C++ FE?
>>> What’s your suggestion?
>>>
>>> (I personally feel that DECL_NOT_FLEXARRAY and TYPE_INCLUDE_FLEXARRAY should be set in the same places).
>>
>> I was wondering if the above test errors on the conservative side
>> correctly - it will now, for all but C, cut off some thing where it
>> didn't before?
>
> As long as the default value of TYPE_INCLUDE_FLEXARRAY reflects the correct conservative behavior, then the testing should be correct, right?
>
> The default value of TYPE_INCLUDE_FLEXARRAY is 0, i.e, FALSE, means that the TYPE does not include a flex array by default, this is the correct behavior. Only when the TYPE does include a flexiarray, it will be set to TRUE. So, I think it’s correct.
>
> This is a different situation for DECL_NOT_FLEXARRAY, by default, the compiler will treat ALL trailing arrays as FMA, so in order to keep the correct conservative behavior, we should keep the default value for DECL_NOT_FLEXARRAY as it’s a FMA, i.e, DECL_NOT_FLEXARRAY being FALSE, by default. Only when the array is NOT a FMA, we set it to true.
>
> So, the default value for TYPE_INCLUDE_FLEXARRAY is correct.
>>
>>>>
>>>> Originally I was suggesting to set this flag in stor-layout.cc
>>>> which eventually all languages funnel their types through and
>>>> if there's language specific handling use a langhook (with the
>>>> default implementation preserving the status quo).
>>>
>>> If we decide to set the bits in stor-layout.cc, where is the best place to do it? I checked the star-layout.cc code, looks like “layout_type” might be the place where we can set these bits for RECORD_TYPE, UNION_TYPE?
>>
>> Yes, it would be layout_type.
>>
>>>>
>>>> Some more comments below ...
>>>>
>>>>> gcc/cp/ChangeLog:
>>>>>
>>>>> PR tree-optimization/101832
>>>>> * 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 (print_node): Print new bit type_include_flexarray.
>>>>> * tree-core.h (struct tree_type_common): New bit
>>>>> type_include_flexarray.
>>>>> * 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 new bit type_include_flexarray.
>>>>> * 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 | 12 ++
>>>>> gcc/cp/module.cc | 2 +
>>>>> gcc/print-tree.cc | 5 +
>>>>> .../gcc.dg/builtin-object-size-pr101832.c | 134 ++++++++++++++++++
>>>>> gcc/tree-core.h | 4 +-
>>>>> gcc/tree-object-size.cc | 79 +++++++----
>>>>> gcc/tree-streamer-in.cc | 1 +
>>>>> gcc/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 b/gcc/c/c-decl.cc
>>>>> index 08078eadeb8..f589a2f5192 100644
>>>>> --- a/gcc/c/c-decl.cc
>>>>> +++ b/gcc/c/c-decl.cc
>>>>> @@ -9284,6 +9284,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 b/gcc/cp/module.cc
>>>>> index ac2fe66b080..c750361b704 100644
>>>>> --- a/gcc/cp/module.cc
>>>>> +++ b/gcc/cp/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 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 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;
>>>>> RECORD_OR_UNION_CHECK
>>>>
>>>> it looks like the bit could be eventually shared with
>>>> no_named_args_stdarg_p (but its accessor doesn't use
>>>> FUNC_OR_METHOD_CHECK)
>>> You mean to let “type_include_flexarray” share the same bit with “no_named_args_stdarg_p” in order to save 1-bit space in IR?
>>> Then change the
>>>
>>> /* True if this is a stdarg function with no named arguments (C2x
>>> (...) 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)
>>>
>>> /* 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)
>>>
>>> To:
>>>
>>> union {
>>> unsigned no_named_args_stdarg_p : 1;
>>> /* TYPE_INCLUDE_FLEXARRAY flag for RECORD_TYPE and UNION_TYPE. */
>>> unsigned int type_include_flexarray : 1;
>>> } shared_bit;
>>> unsigned spare : 15;
>>>
>>> /* True if this is a stdarg function with no named arguments (C2x
>>> (...) 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) \
>>> (FUNC_OR_METHOD_CHECK (NODE)->type_common.shared_bit.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.shared_bit.type_include_flexarray)
>>
>> No, we're just overloading bits by using the same name for different
>> purposes. I don't think such a union would pack nicely. We overload
>> by documenting the uses, in the above cases the uses are separated
>> by the kind of the tree node, RECORD/UNION_TYPE vs. FUNCTION/METHOD_TYPE
>> and the accessor macros should be updated to use the appropriate
>> tree check macros covering that so we don't try to inspect the
>> "wrong bit”.
> Okay, I see. Then should I change the name of that bit to one that reflect two usages?
>>
>>>>
>>>>> alias_set_type alias_set;
>>>>> tree pointer_to;
>>>>> diff --git a/gcc/tree-object-size.cc b/gcc/tree-object-size.cc
>>>>> index 9a936a91983..22b3c72ea6e 100644
>>>>> --- a/gcc/tree-object-size.cc
>>>>> +++ b/gcc/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 b/gcc/tree-streamer-in.cc
>>>>> index d4dc30f048f..c19ede0631d 100644
>>>>> --- a/gcc/tree-streamer-in.cc
>>>>> +++ b/gcc/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 b/gcc/tree-streamer-out.cc
>>>>> index d107229da5c..73e4b4e547c 100644
>>>>> --- a/gcc/tree-streamer-out.cc
>>>>> +++ b/gcc/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 92ac0e6a214..ab1cdc3dc85 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)
>>>>
>>>> Please use RECORD_OR_UNION_CHECK.
>>> Okay.
>>>> The comment "at the last field"
>>>> doesn't seem to match implementation? (at least not obviously)
>>> The current implementation (in gcc/c/c-decl.cc) guarantees that it’s at the last field. (And recursively).
>>>> Given this is a generic header expanding on what a "flexible array member"
>>>> is to the middle-end here would be good. Especially the guarantees
>>>> made when the bit is _not_ set (we've evaded that with DECL_NOT_FLEXARRAY
>>>> vs. DECL_FLEXARRAY).
>>>
>>> The bit TYPE_INCLUDE_FLEXARRAY default is FALSE, i.e, not including a flex array by default.
>>> It’s only set to TRUE when the struct/union includes a flex array at the last field (and recursively). So, I think it should be correct.
>>> Let me know if I missed anything here.
>>
>> See above - I was looking at the use (but I'm not familiar with that
>> code), and wondered how the change affects uses from other frontends.
>
> Please see my explanation above, I think the default behavior from other FE should be kept correctly with this patch.
>
> thanks.
>
> Qing
>>
>> Richard.
On Mon, 13 Mar 2023, Qing Zhao wrote:
> Hi, Richard,
>
> Do you have more comments on my responds to your previous questions?
No, generally we're not good at naming the shared bits, so keeping the
old name is fine here.
Btw, I do not feel competent enough to approve the patch, instead that's
on the burden of the C frontend maintainers and the people understanding
the object-size code more.
Richard.
> thanks.
>
> Qing
>
> > On Mar 10, 2023, at 8:48 AM, Qing Zhao via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
> >
> >
> >
> >> On Mar 10, 2023, at 2:54 AM, Richard Biener <rguenther@suse.de> wrote:
> >>
> >> On Thu, 9 Mar 2023, Qing Zhao wrote:
> >>
> >>>
> >>>
> >>>> On Mar 9, 2023, at 7:20 AM, Richard Biener <rguenther@suse.de> wrote:
> >>>>
> >>>> On Fri, 24 Feb 2023, Qing Zhao wrote:
> >>>>
> >>>>> 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 (finish_struct): Set TYPE_INCLUDE_FLEXARRAY for
> >>>>> struct/union type.
> >>>>
> >>>> I can't really comment on the correctness of this part but since
> >>>> only the C frontend will ever set this and you are using it from
> >>>> addr_object_size which is also used for other C family languages
> >>>> (at least), I wonder if you can really test
> >>>>
> >>>> + if (!TYPE_INCLUDE_FLEXARRAY (TREE_TYPE (v)))
> >>>>
> >>>> there.
> >>>
> >>> You mean for C++ and also other C family languages (other than C), the above bit cannot be set?
> >>> Yes, that’s true. The bit is only set for C. So is the bit DECL_NOT_FLEXARRAY, which is only set for C too.
> >>> So, I am wondering whether the bit DECL_NOT_FLEXARRAY should be also set in middle end? Or we can set DECL_NOT_FLEXARRAY in C++ FE too? And then we can set TYPE_INCLUDE_FLEXARRAY also in C++ FE?
> >>> What’s your suggestion?
> >>>
> >>> (I personally feel that DECL_NOT_FLEXARRAY and TYPE_INCLUDE_FLEXARRAY should be set in the same places).
> >>
> >> I was wondering if the above test errors on the conservative side
> >> correctly - it will now, for all but C, cut off some thing where it
> >> didn't before?
> >
> > As long as the default value of TYPE_INCLUDE_FLEXARRAY reflects the correct conservative behavior, then the testing should be correct, right?
> >
> > The default value of TYPE_INCLUDE_FLEXARRAY is 0, i.e, FALSE, means that the TYPE does not include a flex array by default, this is the correct behavior. Only when the TYPE does include a flexiarray, it will be set to TRUE. So, I think it’s correct.
> >
> > This is a different situation for DECL_NOT_FLEXARRAY, by default, the compiler will treat ALL trailing arrays as FMA, so in order to keep the correct conservative behavior, we should keep the default value for DECL_NOT_FLEXARRAY as it’s a FMA, i.e, DECL_NOT_FLEXARRAY being FALSE, by default. Only when the array is NOT a FMA, we set it to true.
> >
> > So, the default value for TYPE_INCLUDE_FLEXARRAY is correct.
> >>
> >>>>
> >>>> Originally I was suggesting to set this flag in stor-layout.cc
> >>>> which eventually all languages funnel their types through and
> >>>> if there's language specific handling use a langhook (with the
> >>>> default implementation preserving the status quo).
> >>>
> >>> If we decide to set the bits in stor-layout.cc, where is the best place to do it? I checked the star-layout.cc code, looks like “layout_type” might be the place where we can set these bits for RECORD_TYPE, UNION_TYPE?
> >>
> >> Yes, it would be layout_type.
> >>
> >>>>
> >>>> Some more comments below ...
> >>>>
> >>>>> gcc/cp/ChangeLog:
> >>>>>
> >>>>> PR tree-optimization/101832
> >>>>> * 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 (print_node): Print new bit type_include_flexarray.
> >>>>> * tree-core.h (struct tree_type_common): New bit
> >>>>> type_include_flexarray.
> >>>>> * 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 new bit type_include_flexarray.
> >>>>> * 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 | 12 ++
> >>>>> gcc/cp/module.cc | 2 +
> >>>>> gcc/print-tree.cc | 5 +
> >>>>> .../gcc.dg/builtin-object-size-pr101832.c | 134 ++++++++++++++++++
> >>>>> gcc/tree-core.h | 4 +-
> >>>>> gcc/tree-object-size.cc | 79 +++++++----
> >>>>> gcc/tree-streamer-in.cc | 1 +
> >>>>> gcc/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 b/gcc/c/c-decl.cc
> >>>>> index 08078eadeb8..f589a2f5192 100644
> >>>>> --- a/gcc/c/c-decl.cc
> >>>>> +++ b/gcc/c/c-decl.cc
> >>>>> @@ -9284,6 +9284,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 b/gcc/cp/module.cc
> >>>>> index ac2fe66b080..c750361b704 100644
> >>>>> --- a/gcc/cp/module.cc
> >>>>> +++ b/gcc/cp/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 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 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;
> >>>>> RECORD_OR_UNION_CHECK
> >>>>
> >>>> it looks like the bit could be eventually shared with
> >>>> no_named_args_stdarg_p (but its accessor doesn't use
> >>>> FUNC_OR_METHOD_CHECK)
> >>> You mean to let “type_include_flexarray” share the same bit with “no_named_args_stdarg_p” in order to save 1-bit space in IR?
> >>> Then change the
> >>>
> >>> /* True if this is a stdarg function with no named arguments (C2x
> >>> (...) 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)
> >>>
> >>> /* 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)
> >>>
> >>> To:
> >>>
> >>> union {
> >>> unsigned no_named_args_stdarg_p : 1;
> >>> /* TYPE_INCLUDE_FLEXARRAY flag for RECORD_TYPE and UNION_TYPE. */
> >>> unsigned int type_include_flexarray : 1;
> >>> } shared_bit;
> >>> unsigned spare : 15;
> >>>
> >>> /* True if this is a stdarg function with no named arguments (C2x
> >>> (...) 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) \
> >>> (FUNC_OR_METHOD_CHECK (NODE)->type_common.shared_bit.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.shared_bit.type_include_flexarray)
> >>
> >> No, we're just overloading bits by using the same name for different
> >> purposes. I don't think such a union would pack nicely. We overload
> >> by documenting the uses, in the above cases the uses are separated
> >> by the kind of the tree node, RECORD/UNION_TYPE vs. FUNCTION/METHOD_TYPE
> >> and the accessor macros should be updated to use the appropriate
> >> tree check macros covering that so we don't try to inspect the
> >> "wrong bit”.
> > Okay, I see. Then should I change the name of that bit to one that reflect two usages?
> >>
> >>>>
> >>>>> alias_set_type alias_set;
> >>>>> tree pointer_to;
> >>>>> diff --git a/gcc/tree-object-size.cc b/gcc/tree-object-size.cc
> >>>>> index 9a936a91983..22b3c72ea6e 100644
> >>>>> --- a/gcc/tree-object-size.cc
> >>>>> +++ b/gcc/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 b/gcc/tree-streamer-in.cc
> >>>>> index d4dc30f048f..c19ede0631d 100644
> >>>>> --- a/gcc/tree-streamer-in.cc
> >>>>> +++ b/gcc/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 b/gcc/tree-streamer-out.cc
> >>>>> index d107229da5c..73e4b4e547c 100644
> >>>>> --- a/gcc/tree-streamer-out.cc
> >>>>> +++ b/gcc/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 92ac0e6a214..ab1cdc3dc85 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)
> >>>>
> >>>> Please use RECORD_OR_UNION_CHECK.
> >>> Okay.
> >>>> The comment "at the last field"
> >>>> doesn't seem to match implementation? (at least not obviously)
> >>> The current implementation (in gcc/c/c-decl.cc) guarantees that it’s at the last field. (And recursively).
> >>>> Given this is a generic header expanding on what a "flexible array member"
> >>>> is to the middle-end here would be good. Especially the guarantees
> >>>> made when the bit is _not_ set (we've evaded that with DECL_NOT_FLEXARRAY
> >>>> vs. DECL_FLEXARRAY).
> >>>
> >>> The bit TYPE_INCLUDE_FLEXARRAY default is FALSE, i.e, not including a flex array by default.
> >>> It’s only set to TRUE when the struct/union includes a flex array at the last field (and recursively). So, I think it should be correct.
> >>> Let me know if I missed anything here.
> >>
> >> See above - I was looking at the use (but I'm not familiar with that
> >> code), and wondered how the change affects uses from other frontends.
> >
> > Please see my explanation above, I think the default behavior from other FE should be kept correctly with this patch.
> >
> > thanks.
> >
> > Qing
> >>
> >> Richard.
>
>
> On Mar 14, 2023, at 5:04 AM, Richard Biener <rguenther@suse.de> wrote:
>
> On Mon, 13 Mar 2023, Qing Zhao wrote:
>
>> Hi, Richard,
>>
>> Do you have more comments on my responds to your previous questions?
>
> No, generally we're not good at naming the shared bits, so keeping the
> old name is fine here.
Okay. I will keep the old name for it.
>
> Btw, I do not feel competent enough to approve the patch, instead that's
> on the burden of the C frontend maintainers and the people understanding
> the object-size code more.
So, Joseph and Jakub should be the ones to approve these patches?
I will update the patches with your suggestions for the bit.
And send the 5th version .
Thanks.
Qing
>
> Richard.
>
>> thanks.
>>
>> Qing
>>
>>> On Mar 10, 2023, at 8:48 AM, Qing Zhao via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
>>>
>>>
>>>
>>>> On Mar 10, 2023, at 2:54 AM, Richard Biener <rguenther@suse.de> wrote:
>>>>
>>>> On Thu, 9 Mar 2023, Qing Zhao wrote:
>>>>
>>>>>
>>>>>
>>>>>> On Mar 9, 2023, at 7:20 AM, Richard Biener <rguenther@suse.de> wrote:
>>>>>>
>>>>>> On Fri, 24 Feb 2023, Qing Zhao wrote:
>>>>>>
>>>>>>> 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 (finish_struct): Set TYPE_INCLUDE_FLEXARRAY for
>>>>>>> struct/union type.
>>>>>>
>>>>>> I can't really comment on the correctness of this part but since
>>>>>> only the C frontend will ever set this and you are using it from
>>>>>> addr_object_size which is also used for other C family languages
>>>>>> (at least), I wonder if you can really test
>>>>>>
>>>>>> + if (!TYPE_INCLUDE_FLEXARRAY (TREE_TYPE (v)))
>>>>>>
>>>>>> there.
>>>>>
>>>>> You mean for C++ and also other C family languages (other than C), the above bit cannot be set?
>>>>> Yes, that’s true. The bit is only set for C. So is the bit DECL_NOT_FLEXARRAY, which is only set for C too.
>>>>> So, I am wondering whether the bit DECL_NOT_FLEXARRAY should be also set in middle end? Or we can set DECL_NOT_FLEXARRAY in C++ FE too? And then we can set TYPE_INCLUDE_FLEXARRAY also in C++ FE?
>>>>> What’s your suggestion?
>>>>>
>>>>> (I personally feel that DECL_NOT_FLEXARRAY and TYPE_INCLUDE_FLEXARRAY should be set in the same places).
>>>>
>>>> I was wondering if the above test errors on the conservative side
>>>> correctly - it will now, for all but C, cut off some thing where it
>>>> didn't before?
>>>
>>> As long as the default value of TYPE_INCLUDE_FLEXARRAY reflects the correct conservative behavior, then the testing should be correct, right?
>>>
>>> The default value of TYPE_INCLUDE_FLEXARRAY is 0, i.e, FALSE, means that the TYPE does not include a flex array by default, this is the correct behavior. Only when the TYPE does include a flexiarray, it will be set to TRUE. So, I think it’s correct.
>>>
>>> This is a different situation for DECL_NOT_FLEXARRAY, by default, the compiler will treat ALL trailing arrays as FMA, so in order to keep the correct conservative behavior, we should keep the default value for DECL_NOT_FLEXARRAY as it’s a FMA, i.e, DECL_NOT_FLEXARRAY being FALSE, by default. Only when the array is NOT a FMA, we set it to true.
>>>
>>> So, the default value for TYPE_INCLUDE_FLEXARRAY is correct.
>>>>
>>>>>>
>>>>>> Originally I was suggesting to set this flag in stor-layout.cc
>>>>>> which eventually all languages funnel their types through and
>>>>>> if there's language specific handling use a langhook (with the
>>>>>> default implementation preserving the status quo).
>>>>>
>>>>> If we decide to set the bits in stor-layout.cc, where is the best place to do it? I checked the star-layout.cc code, looks like “layout_type” might be the place where we can set these bits for RECORD_TYPE, UNION_TYPE?
>>>>
>>>> Yes, it would be layout_type.
>>>>
>>>>>>
>>>>>> Some more comments below ...
>>>>>>
>>>>>>> gcc/cp/ChangeLog:
>>>>>>>
>>>>>>> PR tree-optimization/101832
>>>>>>> * 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 (print_node): Print new bit type_include_flexarray.
>>>>>>> * tree-core.h (struct tree_type_common): New bit
>>>>>>> type_include_flexarray.
>>>>>>> * 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 new bit type_include_flexarray.
>>>>>>> * 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 | 12 ++
>>>>>>> gcc/cp/module.cc | 2 +
>>>>>>> gcc/print-tree.cc | 5 +
>>>>>>> .../gcc.dg/builtin-object-size-pr101832.c | 134 ++++++++++++++++++
>>>>>>> gcc/tree-core.h | 4 +-
>>>>>>> gcc/tree-object-size.cc | 79 +++++++----
>>>>>>> gcc/tree-streamer-in.cc | 1 +
>>>>>>> gcc/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 b/gcc/c/c-decl.cc
>>>>>>> index 08078eadeb8..f589a2f5192 100644
>>>>>>> --- a/gcc/c/c-decl.cc
>>>>>>> +++ b/gcc/c/c-decl.cc
>>>>>>> @@ -9284,6 +9284,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 b/gcc/cp/module.cc
>>>>>>> index ac2fe66b080..c750361b704 100644
>>>>>>> --- a/gcc/cp/module.cc
>>>>>>> +++ b/gcc/cp/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 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 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;
>>>>>>> RECORD_OR_UNION_CHECK
>>>>>>
>>>>>> it looks like the bit could be eventually shared with
>>>>>> no_named_args_stdarg_p (but its accessor doesn't use
>>>>>> FUNC_OR_METHOD_CHECK)
>>>>> You mean to let “type_include_flexarray” share the same bit with “no_named_args_stdarg_p” in order to save 1-bit space in IR?
>>>>> Then change the
>>>>>
>>>>> /* True if this is a stdarg function with no named arguments (C2x
>>>>> (...) 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)
>>>>>
>>>>> /* 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)
>>>>>
>>>>> To:
>>>>>
>>>>> union {
>>>>> unsigned no_named_args_stdarg_p : 1;
>>>>> /* TYPE_INCLUDE_FLEXARRAY flag for RECORD_TYPE and UNION_TYPE. */
>>>>> unsigned int type_include_flexarray : 1;
>>>>> } shared_bit;
>>>>> unsigned spare : 15;
>>>>>
>>>>> /* True if this is a stdarg function with no named arguments (C2x
>>>>> (...) 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) \
>>>>> (FUNC_OR_METHOD_CHECK (NODE)->type_common.shared_bit.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.shared_bit.type_include_flexarray)
>>>>
>>>> No, we're just overloading bits by using the same name for different
>>>> purposes. I don't think such a union would pack nicely. We overload
>>>> by documenting the uses, in the above cases the uses are separated
>>>> by the kind of the tree node, RECORD/UNION_TYPE vs. FUNCTION/METHOD_TYPE
>>>> and the accessor macros should be updated to use the appropriate
>>>> tree check macros covering that so we don't try to inspect the
>>>> "wrong bit”.
>>> Okay, I see. Then should I change the name of that bit to one that reflect two usages?
>>>>
>>>>>>
>>>>>>> alias_set_type alias_set;
>>>>>>> tree pointer_to;
>>>>>>> diff --git a/gcc/tree-object-size.cc b/gcc/tree-object-size.cc
>>>>>>> index 9a936a91983..22b3c72ea6e 100644
>>>>>>> --- a/gcc/tree-object-size.cc
>>>>>>> +++ b/gcc/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 b/gcc/tree-streamer-in.cc
>>>>>>> index d4dc30f048f..c19ede0631d 100644
>>>>>>> --- a/gcc/tree-streamer-in.cc
>>>>>>> +++ b/gcc/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 b/gcc/tree-streamer-out.cc
>>>>>>> index d107229da5c..73e4b4e547c 100644
>>>>>>> --- a/gcc/tree-streamer-out.cc
>>>>>>> +++ b/gcc/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 92ac0e6a214..ab1cdc3dc85 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)
>>>>>>
>>>>>> Please use RECORD_OR_UNION_CHECK.
>>>>> Okay.
>>>>>> The comment "at the last field"
>>>>>> doesn't seem to match implementation? (at least not obviously)
>>>>> The current implementation (in gcc/c/c-decl.cc) guarantees that it’s at the last field. (And recursively).
>>>>>> Given this is a generic header expanding on what a "flexible array member"
>>>>>> is to the middle-end here would be good. Especially the guarantees
>>>>>> made when the bit is _not_ set (we've evaded that with DECL_NOT_FLEXARRAY
>>>>>> vs. DECL_FLEXARRAY).
>>>>>
>>>>> The bit TYPE_INCLUDE_FLEXARRAY default is FALSE, i.e, not including a flex array by default.
>>>>> It’s only set to TRUE when the struct/union includes a flex array at the last field (and recursively). So, I think it should be correct.
>>>>> Let me know if I missed anything here.
>>>>
>>>> See above - I was looking at the use (but I'm not familiar with that
>>>> code), and wondered how the change affects uses from other frontends.
>>>
>>> Please see my explanation above, I think the default behavior from other FE should be kept correctly with this patch.
>>>
>>> thanks.
>>>
>>> Qing
>>>>
>>>> Richard.
>>
>>
>
> --
> Richard Biener <rguenther@suse.de>
> SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg,
> Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman;
> HRB 36809 (AG Nuernberg)
@@ -9284,6 +9284,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;
@@ -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))
@@ -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)
new file mode 100644
@@ -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;
+}
@@ -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;
@@ -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;
@@ -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);
@@ -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);
@@ -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) \