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 <rguent...@suse.de> wrote:
> >>
> >> On Thu, 9 Mar 2023, Qing Zhao wrote:
> >>
> >>>
> >>>
> >>>> On Mar 9, 2023, at 7:20 AM, Richard Biener <rguent...@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 <rguent...@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)
