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 wrote: > > > > > > > >> On Mar 10, 2023, at 2:54 AM, Richard Biener wrote: > >> > >> On Thu, 9 Mar 2023, Qing Zhao wrote: > >> > >>> > >>> > >>>> On Mar 9, 2023, at 7:20 AM, Richard Biener 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 SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg, Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman; HRB 36809 (AG Nuernberg)