| Message ID | 20230224183505.4112295-2-qing.zhao@oracle.com |
|---|---|
| State | New |
| Headers | show |
| Series | Handle component_ref to a structure/union field including FAM for builtin_object_size | expand |
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)
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) \