> On Aug 2, 2022, at 3:03 AM, Richard Biener <rguent...@suse.de> wrote: > > On Mon, 1 Aug 2022, Qing Zhao wrote: > >> >> >>> On Aug 1, 2022, at 3:38 AM, Richard Biener <rguent...@suse.de> wrote: >>> >>> On Fri, 29 Jul 2022, Qing Zhao wrote: >>> >>>> Hi, Richard, >>>> >>>> Thanks a lot for your comments and suggestions. (And sorry for my late >>>> reply). >>>> >>>>> On Jul 28, 2022, at 3:26 AM, Richard Biener <rguent...@suse.de> wrote: >>>>> >>>>> On Tue, 19 Jul 2022, Qing Zhao wrote: >>>>> >>>>>> From 3854004802b8e2f132ebf218fc35a632f5e80c6a Mon Sep 17 00:00:00 2001 >>>>>> From: Qing Zhao <qing.z...@oracle.com> >>>>>> Date: Mon, 18 Jul 2022 17:04:12 +0000 >>>>>> Subject: [PATCH 1/2] Add a new option -fstrict-flex-array[=n] and new >>>>>> attribute strict_flex_array >>>>>> >>>>>> Add the following new option -fstrict-flex-array[=n] and a corresponding >>>>>> attribute strict_flex_array to GCC: >>>>>> >>>>>> '-fstrict-flex-array' >>>>>> Treat the trailing array of a structure as a flexible array member >>>>>> in a stricter way. The positive form is equivalent to >>>>>> '-fstrict-flex-array=3', which is the strictest. A trailing array >>>>>> is treated as a flexible array member only when it is declared as a >>>>>> flexible array member per C99 standard onwards. The negative form >>>>>> is equivalent to '-fstrict-flex-array=0', which is the least >>>>>> strict. All trailing arrays of structures are treated as flexible >>>>>> array members. >>>>>> >>>>>> '-fstrict-flex-array=LEVEL' >>>>>> Treat the trailing array of a structure as a flexible array member >>>>>> in a stricter way. The value of LEVEL controls the level of >>>>>> strictness. >>>>>> >>>>>> The possible values of LEVEL are the same as for the >>>>>> 'strict_flex_array' attribute (*note Variable Attributes::). >>>>>> >>>>>> You can control this behavior for a specific trailing array field >>>>>> of a structure by using the variable attribute 'strict_flex_array' >>>>>> attribute (*note Variable Attributes::). >>>>>> >>>>>> 'strict_flex_array (LEVEL)' >>>>>> The 'strict_flex_array' attribute should be attached to the >>>>>> trailing array field of a structure. It specifies the level of >>>>>> strictness of treating the trailing array field of a structure as a >>>>>> flexible array member. LEVEL must be an integer betwen 0 to 3. >>>>>> >>>>>> LEVEL=0 is the least strict level, all trailing arrays of >>>>>> structures are treated as flexible array members. LEVEL=3 is the >>>>>> strictest level, only when the trailing array is declared as a >>>>>> flexible array member per C99 standard onwards ([]), it is treated >>>>>> as a flexible array member. >>>>>> >>>>>> There are two more levels in between 0 and 3, which are provided to >>>>>> support older codes that use GCC zero-length array extension ([0]) >>>>>> or one-size array as flexible array member ([1]): When LEVEL is 1, >>>>>> the trailing array is treated as a flexible array member when it is >>>>>> declared as either [], [0], or [1]; When LEVEL is 2, the trailing >>>>>> array is treated as a flexible array member when it is declared as >>>>>> either [], or [0]. >>>>>> >>>>>> This attribute can be used with or without '-fstrict-flex-array'. >>>>>> When both the attribute and the option present at the same time, >>>>>> the level of the strictness for the specific trailing array field >>>>>> is determined by the attribute. >>>>>> >>>>>> gcc/c-family/ChangeLog: >>>>>> >>>>>> * c-attribs.cc (handle_strict_flex_array_attribute): New function. >>>>>> (c_common_attribute_table): New item for strict_flex_array. >>>>>> * c.opt (fstrict-flex-array): New option. >>>>>> (fstrict-flex-array=): New option. >>>>>> >>>>>> gcc/c/ChangeLog: >>>>>> >>>>>> * c-decl.cc (add_flexible_array_elts_to_size): Call new utility >>>>>> routine flexible_array_member_p. >>>>>> (is_flexible_array_member_p): New function. >>>>>> (finish_struct): Set the new DECL_NOT_FLEXARRAY flag. >>>>>> >>>>>> gcc/ChangeLog: >>>>>> >>>>>> * doc/extend.texi: Document strict_flex_array attribute. >>>>>> * doc/invoke.texi: Document -fstrict-flex-array[=n] option. >>>>>> * tree-core.h (struct tree_decl_common): New bit field >>>>>> decl_not_flexarray. >>>>>> * tree.cc (component_ref_size): Reorg by using new utility functions. >>>>>> (flexible_array_member_p): New function. >>>>>> (zero_length_array_p): Likewise. >>>>>> (one_element_array_p): Likewise. >>>>>> (flexible_array_type_p): Likewise. >>>>>> * tree.h (DECL_NOT_FLEXARRAY): New flag. >>>>>> (zero_length_array_p): New function prototype. >>>>>> (one_element_array_p): Likewise. >>>>>> (flexible_array_member_p): Likewise. >>>>>> >>>>>> gcc/testsuite/ChangeLog: >>>>>> >>>>>> * gcc.dg/strict-flex-array-1.c: New test. >>>>>> --- >>>>>> gcc/c-family/c-attribs.cc | 47 ++++++++ >>>>>> gcc/c-family/c.opt | 7 ++ >>>>>> gcc/c/c-decl.cc | 91 +++++++++++++-- >>>>>> gcc/doc/extend.texi | 25 ++++ >>>>>> gcc/doc/invoke.texi | 27 ++++- >>>>>> gcc/testsuite/gcc.dg/strict-flex-array-1.c | 31 +++++ >>>>>> gcc/tree-core.h | 5 +- >>>>>> gcc/tree.cc | 130 ++++++++++++++------- >>>>>> gcc/tree.h | 16 ++- >>>>>> 9 files changed, 322 insertions(+), 57 deletions(-) >>>>>> create mode 100644 gcc/testsuite/gcc.dg/strict-flex-array-1.c >>>>>> >>>>>> diff --git a/gcc/c-family/c-attribs.cc b/gcc/c-family/c-attribs.cc >>>>>> index c8d96723f4c..10d16532f0d 100644 >>>>>> --- a/gcc/c-family/c-attribs.cc >>>>>> +++ b/gcc/c-family/c-attribs.cc >>>>>> @@ -101,6 +101,8 @@ static tree handle_special_var_sec_attribute (tree >>>>>> *, tree, tree, int, bool *); >>>>>> static tree handle_aligned_attribute (tree *, tree, tree, int, bool *); >>>>>> static tree handle_warn_if_not_aligned_attribute (tree *, tree, tree, >>>>>> int, bool *); >>>>>> +static tree handle_strict_flex_array_attribute (tree *, tree, tree, >>>>>> + int, bool *); >>>>>> static tree handle_weak_attribute (tree *, tree, tree, int, bool *) ; >>>>>> static tree handle_noplt_attribute (tree *, tree, tree, int, bool *) ; >>>>>> static tree handle_alias_ifunc_attribute (bool, tree *, tree, tree, bool >>>>>> *); >>>>>> @@ -367,6 +369,8 @@ const struct attribute_spec >>>>>> c_common_attribute_table[] = >>>>>> attr_aligned_exclusions }, >>>>>> { "warn_if_not_aligned", 0, 1, false, false, false, false, >>>>>> handle_warn_if_not_aligned_attribute, NULL }, >>>>>> + { "strict_flex_array", 1, 1, false, false, false, false, >>>>>> + handle_strict_flex_array_attribute, NULL >>>>>> }, >>>>>> { "weak", 0, 0, true, false, false, false, >>>>>> handle_weak_attribute, NULL }, >>>>>> { "noplt", 0, 0, true, false, false, false, >>>>>> @@ -2498,6 +2502,49 @@ handle_warn_if_not_aligned_attribute (tree *node, >>>>>> tree name, >>>>>> no_add_attrs, true); >>>>>> } >>>>>> >>>>>> +/* Handle a "strict_flex_array" attribute; arguments as in >>>>>> + struct attribute_spec.handler. */ >>>>>> + >>>>>> +static tree >>>>>> +handle_strict_flex_array_attribute (tree *node, tree name, >>>>>> + tree args, int ARG_UNUSED (flags), >>>>>> + bool *no_add_attrs) >>>>>> +{ >>>>>> + tree decl = *node; >>>>>> + tree argval = TREE_VALUE (args); >>>>>> + >>>>>> + /* This attribute only applies to field decls of a structure. */ >>>>>> + if (TREE_CODE (decl) != FIELD_DECL) >>>>>> + { >>>>>> + error_at (DECL_SOURCE_LOCATION (decl), >>>>>> + "%qE attribute may not be specified for %q+D", name, >>>>>> decl); >>>>>> + *no_add_attrs = true; >>>>>> + } >>>>>> + /* This attribute only applies to field with array type. */ >>>>>> + else if (TREE_CODE (TREE_TYPE (decl)) != ARRAY_TYPE) >>>>>> + { >>>>>> + error_at (DECL_SOURCE_LOCATION (decl), >>>>>> + "%qE attribute may not be specified for a non array >>>>>> field", >>>>>> + name); >>>>>> + *no_add_attrs = true; >>>>>> + } >>>>>> + else if (TREE_CODE (argval) != INTEGER_CST) >>>>>> + { >>>>>> + error_at (DECL_SOURCE_LOCATION (decl), >>>>>> + "%qE attribute argument not an integer", name); >>>>>> + *no_add_attrs = true; >>>>>> + } >>>>>> + else if (!tree_fits_uhwi_p (argval) || tree_to_uhwi (argval) > 3) >>>>>> + { >>>>>> + error_at (DECL_SOURCE_LOCATION (decl), >>>>>> + "%qE attribute argument %qE is not an integer constant" >>>>>> + " between 0 and 3", name, argval); >>>>>> + *no_add_attrs = true; >>>>>> + } >>>>>> + >>>>>> + return NULL_TREE; >>>>>> +} >>>>>> + >>>>>> /* Handle a "weak" attribute; arguments as in >>>>>> struct attribute_spec.handler. */ >>>>>> >>>>>> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt >>>>>> index 44e1a60ce24..864cd8df1d3 100644 >>>>>> --- a/gcc/c-family/c.opt >>>>>> +++ b/gcc/c-family/c.opt >>>>>> @@ -2060,6 +2060,13 @@ fsized-deallocation >>>>>> C++ ObjC++ Var(flag_sized_deallocation) Init(-1) >>>>>> Enable C++14 sized deallocation support. >>>>>> >>>>>> +fstrict-flex-array >>>>>> +C C++ Common Alias(fstrict-flex-array=,3,0) >>>>>> + >>>>>> +fstrict-flex-array= >>>>>> +C C++ Common Joined RejectNegative UInteger Var(flag_strict_flex_array) >>>>>> Init(0) IntegerRange(0,3) >>>>>> +-fstrict-flex-array=<level> Treat the trailing array of a structure >>>>>> as a flexible array in a stricter way. The default is treating all >>>>>> trailing arrays of structures as flexible arrays. >>>>>> + >>>>>> fsquangle >>>>>> C++ ObjC++ WarnRemoved >>>>>> >>>>>> diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc >>>>>> index ae8990c138f..14defae9584 100644 >>>>>> --- a/gcc/c/c-decl.cc >>>>>> +++ b/gcc/c/c-decl.cc >>>>>> @@ -5013,10 +5013,7 @@ add_flexible_array_elts_to_size (tree decl, tree >>>>>> init) >>>>>> >>>>>> elt = CONSTRUCTOR_ELTS (init)->last ().value; >>>>>> type = TREE_TYPE (elt); >>>>>> - if (TREE_CODE (type) == ARRAY_TYPE >>>>>> - && TYPE_SIZE (type) == NULL_TREE >>>>>> - && TYPE_DOMAIN (type) != NULL_TREE >>>>>> - && TYPE_MAX_VALUE (TYPE_DOMAIN (type)) == NULL_TREE) >>>>>> + if (flexible_array_member_p (type)) >>>>>> { >>>>>> complete_array_type (&type, elt, false); >>>>>> DECL_SIZE (decl) >>>>>> @@ -8720,6 +8717,80 @@ finish_incomplete_vars (tree incomplete_vars, >>>>>> bool toplevel) >>>>>> } >>>>>> } >>>>>> >>>>>> +/* Determine whether the FIELD_DECL X is a flexible array member >>>>>> according to >>>>>> + the following info: >>>>>> + A. whether the FIELD_DECL X is the last field of the DECL_CONTEXT; >>>>>> + B. whether the FIELD_DECL is an array that is declared as "[]", "[0]", >>>>>> + or "[1]"; >>>>>> + C. flag_strict_flex_array; >>>>>> + D. the attribute strict_flex_array that is attached to the field >>>>>> + if presenting. >>>>>> + Return TRUE when it's a flexible array member, FALSE otherwise. */ >>>>>> + >>>>>> +static bool >>>>>> +is_flexible_array_member_p (bool is_last_field, >>>>>> + tree x) >>>>>> +{ >>>>>> + /* if not the last field, return false. */ >>>>>> + if (!is_last_field) >>>>>> + return false; >>>>>> + >>>>>> + /* if not an array field, return false. */ >>>>>> + if (TREE_CODE (TREE_TYPE (x)) != ARRAY_TYPE) >>>>>> + return false; >>>>>> + >>>>>> + bool is_zero_length_array = zero_length_array_p (TREE_TYPE (x)); >>>>>> + bool is_one_element_array = one_element_array_p (TREE_TYPE (x)); >>>>>> + bool is_flexible_array = flexible_array_member_p (TREE_TYPE (x)); >>>>>> + >>>>>> + unsigned int strict_flex_array_level = flag_strict_flex_array; >>>>>> + >>>>>> + tree attr_strict_flex_array = lookup_attribute ("strict_flex_array", >>>>>> + DECL_ATTRIBUTES (x)); >>>>>> + /* if there is a strict_flex_array attribute attached to the field, >>>>>> + override the flag_strict_flex_array. */ >>>>>> + if (attr_strict_flex_array) >>>>>> + { >>>>>> + /* get the value of the level first from the attribute. */ >>>>>> + unsigned HOST_WIDE_INT attr_strict_flex_array_level = 0; >>>>>> + gcc_assert (TREE_VALUE (attr_strict_flex_array) != NULL_TREE); >>>>>> + attr_strict_flex_array = TREE_VALUE (attr_strict_flex_array); >>>>>> + gcc_assert (TREE_VALUE (attr_strict_flex_array) != NULL_TREE); >>>>>> + attr_strict_flex_array = TREE_VALUE (attr_strict_flex_array); >>>>>> + gcc_assert (tree_fits_uhwi_p (attr_strict_flex_array)); >>>>>> + attr_strict_flex_array_level = tree_to_uhwi >>>>>> (attr_strict_flex_array); >>>>>> + >>>>>> + /* the attribute has higher priority than flag_struct_flex_array. >>>>>> */ >>>>>> + strict_flex_array_level = attr_strict_flex_array_level; >>>>>> + } >>>>>> + >>>>>> + switch (strict_flex_array_level) >>>>>> + { >>>>>> + case 0: >>>>>> + /* default, all trailing arrays are flexiable array members. */ >>>>>> + return true; >>>>>> + case 1: >>>>>> + /* Level 1: all "[1]", "[0]", and "[]" are flexiable array >>>>>> members. */ >>>>>> + if (is_one_element_array) >>>>>> + return true; >>>>>> + /* FALLTHROUGH. */ >>>>>> + case 2: >>>>>> + /* Level 2: all "[0]", and "[]" are flexiable array members. */ >>>>>> + if (is_zero_length_array) >>>>>> + return true; >>>>>> + /* FALLTHROUGH. */ >>>>>> + case 3: >>>>>> + /* Level 3: Only "[]" are flexible array members. */ >>>>>> + if (is_flexible_array) >>>>>> + return true; >>>>>> + break; >>>>>> + default: >>>>>> + gcc_unreachable (); >>>>>> + } >>>>>> + return false; >>>>>> +} >>>>>> + >>>>>> + >>>>>> /* Fill in the fields of a RECORD_TYPE or UNION_TYPE node, T. >>>>>> LOC is the location of the RECORD_TYPE or UNION_TYPE's definition. >>>>>> FIELDLIST is a chain of FIELD_DECL nodes for the fields. >>>>>> @@ -8781,6 +8852,8 @@ finish_struct (location_t loc, tree t, tree >>>>>> fieldlist, tree attributes, >>>>>> bool saw_named_field = false; >>>>>> for (x = fieldlist; x; x = DECL_CHAIN (x)) >>>>>> { >>>>>> + bool is_last_field = (DECL_CHAIN (x) == NULL_TREE); >>>>>> + >>>>>> if (TREE_TYPE (x) == error_mark_node) >>>>>> continue; >>>>>> >>>>>> @@ -8819,10 +8892,7 @@ finish_struct (location_t loc, tree t, tree >>>>>> fieldlist, tree attributes, >>>>>> DECL_PACKED (x) = 1; >>>>>> >>>>>> /* Detect flexible array member in an invalid context. */ >>>>>> - if (TREE_CODE (TREE_TYPE (x)) == ARRAY_TYPE >>>>>> - && TYPE_SIZE (TREE_TYPE (x)) == NULL_TREE >>>>>> - && TYPE_DOMAIN (TREE_TYPE (x)) != NULL_TREE >>>>>> - && TYPE_MAX_VALUE (TYPE_DOMAIN (TREE_TYPE (x))) == NULL_TREE) >>>>>> + if (flexible_array_member_p (TREE_TYPE (x))) >>>>>> { >>>>>> if (TREE_CODE (t) == UNION_TYPE) >>>>>> { >>>>>> @@ -8830,7 +8900,7 @@ finish_struct (location_t loc, tree t, tree >>>>>> fieldlist, tree attributes, >>>>>> "flexible array member in union"); >>>>>> TREE_TYPE (x) = error_mark_node; >>>>>> } >>>>>> - else if (DECL_CHAIN (x) != NULL_TREE) >>>>>> + else if (!is_last_field) >>>>>> { >>>>>> error_at (DECL_SOURCE_LOCATION (x), >>>>>> "flexible array member not at end of struct"); >>>>>> @@ -8850,6 +8920,9 @@ finish_struct (location_t loc, tree t, tree >>>>>> fieldlist, tree attributes, >>>>>> pedwarn (DECL_SOURCE_LOCATION (x), OPT_Wpedantic, >>>>>> "invalid use of structure with flexible array member"); >>>>>> >>>>>> + /* Set DECL_NOT_FLEXARRAY flag for FIELD_DECL x. */ >>>>>> + DECL_NOT_FLEXARRAY (x) = !is_flexible_array_member_p >>>>>> (is_last_field, x); >>>>>> + >>>>>> if (DECL_NAME (x) >>>>>> || RECORD_OR_UNION_TYPE_P (TREE_TYPE (x))) >>>>>> saw_named_field = true; >>>>>> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi >>>>>> index dfbe33ac652..7451410a011 100644 >>>>>> --- a/gcc/doc/extend.texi >>>>>> +++ b/gcc/doc/extend.texi >>>>>> @@ -7436,6 +7436,31 @@ This warning can be disabled by >>>>>> @option{-Wno-if-not-aligned}. >>>>>> The @code{warn_if_not_aligned} attribute can also be used for types >>>>>> (@pxref{Common Type Attributes}.) >>>>>> >>>>>> +@cindex @code{strict_flex_array} variable attribute >>>>>> +@item strict_flex_array (@var{level}) >>>>>> +The @code{strict_flex_array} attribute should be attached to the >>>>>> trailing >>>>>> +array field of a structure. It specifies the level of strictness of >>>>>> +treating the trailing array field of a structure as a flexible array >>>>>> +member. @var{level} must be an integer betwen 0 to 3. >>>>>> + >>>>>> +@var{level}=0 is the least strict level, all trailing arrays of >>>>>> structures >>>>>> +are treated as flexible array members. @var{level}=3 is the strictest >>>>>> level, >>>>>> +only when the trailing array is declared as a flexible array member per >>>>>> C99 >>>>>> +standard onwards ([]), it is treated as a flexible array member. >>>>> >>>>> How is level 3 (thus -fstrict-flex-array) interpreted when you specify >>>>> -std=c89? How for -std=gnu89? >>>> >>>> 1. what’s the major difference between -std=c89 and -std=gnu89 on flexible >>>> array? (Checked online, cannot find a concrete answer on this). >>>> ** my understanding is: -std=c89 will not support any flexible array >>>> (neither [], [0], [1]), but -std=gnu89 will support [0] and [1], but not >>>> []. >>>> Is this correct? >>> >>> Yes. >>> >>>> If my answer to the first question is correct, then: >>>> >>>> 2. When -fstrict-flex-array=n and -std=c89 present at the same time, which >>>> one has the higher priority? >>>> ** I think that -std=c89 should be honored over >>>> -fstrict-flex-array, therefore we should disable -fstrict-flex-array=n >>>> when n > 0 and issue warnings to the user. >>>> >>>> >>>> 3. how about -fstrict-flex-array=n and -std=gnu89 present at the same >>>> time? >>>> ** When -std=gnu89 present, [] is not supported. So, we need to >>>> issue an warning to disable -fstrict-flex-array=3; but level 1 and level 2 >>>> is Okay. >>>> >>>> We also need to document the above. >>>> >>>> Let me know if you have any more comment and suggestions here. >>> >>> In my opinion -fstrict-flex-array only makes sense with C99 and above. >>> The extra sub-levels allowing [0] or [1] are of questionable benefit. >> The multiple-levels control for -fstrict-flex-arrays was an agreement on the >> discussion of PR101836: >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836 >> >> Please see comment#17 till #25 for the details. >> >> A summary of the discussion is: (Please comment on the following paragraph >> if you see any issue with it) >> >> ===== >> Flexible Array Member (FAM) is a feature introduced in the C99 standard of >> the C language in May 2000. It's used as the last element of a structure >> which is really a header for a variable-length object. >> This feature was officially standardized in C99. However, compilers (GCC, >> Micorsoft compiler, etc.) supported this feature well before C99, with the >> following variants: >> A. trailing zero-length array of a structure was treated as FAM. >> Before C99, a valid GCC extension, zero-length array was used to support >> flexible array member. See more details at: >> https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html. >> B. trailing one-element array of a structure was treated as FAM. >> In the absence of the zero-length array extension, in ISO C90, >> one-element arrays at the end of structures were used to represent FAMs as a >> convertion. >> C. any trailing array of a structure was treated as FAM. >> Even earlier time, in the absence of both zero-length array GCC >> extension and one-element array convention, all trailing arrays of >> structures could be used as a FAM. >> As a result, older codes (before C99) might include any of the above >> variants (A, B, and C) to represent FAMs. >> Another complication is C++, standard C++ doesn't allow either C99 flexible >> array members or zero-length array. A C++ programmer has to use the GCC >> zero-length array extension or one-element array convention for the purpose >> of FAM. >> ===== >> >> As mentioned by Jakub in comment #25: >> >> "differentiating between only allowing [] as flex, or [] and [0], >> or [], [0] and [1], or any trailing array is useful." > > OK. > >>> >>> I think that with -fstrict-flex-array we're missing a -Wstrict-flex-array >>> that would diagnose accesses that are no longer treated as flex array >>> access but would be without -fstrict-flex-array? The goal of >>> -fstrict-flex-array + -Wstrict-flex-array is to make code standard >>> conformant, no? Not to enable extra optimization for [1] arrays when >>> we know [0] is used as flexarray? >> >> Yes, I agree, and this will make the feature more complete and more >> useful to help users to get rid of the non-standard-comforming styles. >> >> Please see comments #23, #35 >> >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836#c23 >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836#c35 >> >> A summary is: >> >> ==== >> I think that -Wstrict-flex-arrays will need to be cooperated with >> -fstrict-flex-arrays=N, i.e, only when -fstrict-flex-arrays=N is presenting, >> -Wstrict-flex-arrays will be valid and report the warnings when see a [0], >> or [1], or any trailing array based on N: >> >> when -fstrict-flex-arrays >> =0, -Wstrict-flex-arrays will NOT issue any warnings; >> =1, -Wstrict-flex-arrays will issue warnings when an array ref's index is >> larger than the upper bounds of a trailing array that is a regular trailing >> array; >> =2, -Wstrict-flex-arrays will issue warnings when an array ref's index is >> larger than the upper bounds of a trailing array that is a regular trailing >> array or [1]; >> =3, -Wstrict-flex-arrays will issue warnings when an array ref's index is >> larger than the upper bounds of a trailing array that is a regular trailing >> array or [1], or [0]. >> >> let me know if you have any comment and suggestion on this. >> >> ==== > > Ah, I see - good that this has been discussed. -Wstrict-flex-array > shouldn't be a requirement for -fstrict-flex-array to go in, but it > would be nice to followup with an implementation for the above > (it might be tricky to find a good place for it, but eventually it > sounds like -Warray-bounds diagnosing places could eventually be > adjusted) Yes, this is on my to-do list, I will finish this part in GCC13. The following is my plan: =====
1. Add a new IR flag “DECL_NOT_FLEXARRAY” to FIELD_DECL; 2. In C/C++ FE, set the new flag “DECL_NOT_FLEXARRAY” for a FIELD_DECL based on [0], [1], [ ] and the option -fstrict-flex-arrays, the attribute strict_flex_arrays and whether it’s the last field of the DECL_CONTEXT. 3. In Middle end, update all places that treat any trailing array as flexible array member with the new flag DECL_NOT_FLEXARRAY + array_at_struct_end_p to control the behavior with multiple levels consistently. 3.1. update tree-object-size.cc to fix PR101836; 3.2. update other places to make GCC consistently handle flexible array member; 4. In Middle end, add -Wstrict-flex-array and update the warnings emitted by -Warray-bounds based on multiple levels; 5. Add a new -Wzero-length-array warning; (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94428). ===== Let me know if you have any comment or suggestion here. > >> >>> >>>>> >>>>>> + >>>>>> +There are two more levels in between 0 and 3, which are provided to >>>>>> support >>>>>> +older codes that use GCC zero-length array extension ([0]) or one-size >>>>>> array >>>>>> +as flexible array member ([1]): >>>>>> +When @var{level} is 1, the trailing array is treated as a flexible >>>>>> array member >>>>>> +when it is declared as either "[]", "[0]", or "[1]"; >>>>>> +When @var{level} is 2, the trailing array is treated as a flexible >>>>>> array member >>>>>> +when it is declared as either "[]", or "[0]". >>>>> >>>>> Given the above does adding level 2 make sense given that [0] is a GNU >>>>> extension? >>>> I think Kees already answered this question. >>>>> >>>>>> +This attribute can be used with or without >>>>>> @option{-fstrict-flex-array}. When >>>>>> +both the attribute and the option present at the same time, the level >>>>>> of the >>>>>> +strictness for the specific trailing array field is determined by the >>>>>> attribute. >>>>>> + >>>>>> + >>>>>> @item alloc_size (@var{position}) >>>>>> @itemx alloc_size (@var{position-1}, @var{position-2}) >>>>>> @cindex @code{alloc_size} variable attribute >>>>>> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi >>>>>> index 94fe57aa4e2..9befe601817 100644 >>>>>> --- a/gcc/doc/invoke.texi >>>>>> +++ b/gcc/doc/invoke.texi >>>>>> @@ -207,7 +207,8 @@ in the following sections. >>>>>> -fopenmp -fopenmp-simd @gol >>>>>> -fpermitted-flt-eval-methods=@var{standard} @gol >>>>>> -fplan9-extensions -fsigned-bitfields -funsigned-bitfields @gol >>>>>> --fsigned-char -funsigned-char -fsso-struct=@var{endianness}} >>>>>> +-fsigned-char -funsigned-char -fstrict-flex-array[=@var{n}] @gol >>>>>> +-fsso-struct=@var{endianness}} >>>>>> >>>>>> @item C++ Language Options >>>>>> @xref{C++ Dialect Options,,Options Controlling C++ Dialect}. >>>>>> @@ -2825,6 +2826,30 @@ The type @code{char} is always a distinct type >>>>>> from each of >>>>>> @code{signed char} or @code{unsigned char}, even though its behavior >>>>>> is always just like one of those two. >>>>>> >>>>>> +@item -fstrict-flex-array >>>>>> +@opindex fstrict-flex-array >>>>>> +@opindex fno-strict-flex-array >>>>>> +Treat the trailing array of a structure as a flexible array member in a >>>>>> +stricter way. >>>>>> +The positive form is equivalent to @option{-fstrict-flex-array=3}, >>>>>> which is the >>>>>> +strictest. A trailing array is treated as a flexible array member only >>>>>> when it >>>>>> +is declared as a flexible array member per C99 standard onwards. >>>>>> +The negative form is equivalent to @option{-fstrict-flex-array=0}, >>>>>> which is the >>>>>> +least strict. All trailing arrays of structures are treated as >>>>>> flexible array >>>>>> +members. >>>>>> + >>>>>> +@item -fstrict-flex-array=@var{level} >>>>>> +@opindex fstrict-flex-array=@var{level} >>>>>> +Treat the trailing array of a structure as a flexible array member in a >>>>>> +stricter way. The value of @var{level} controls the level of >>>>>> strictness. >>>>>> + >>>>>> +The possible values of @var{level} are the same as for the >>>>>> +@code{strict_flex_array} attribute (@pxref{Variable Attributes}). >>>>>> + >>>>>> +You can control this behavior for a specific trailing array field of a >>>>>> +structure by using the variable attribute @code{strict_flex_array} >>>>>> attribute >>>>>> +(@pxref{Variable Attributes}). >>>>>> + >>>>>> @item -fsso-struct=@var{endianness} >>>>>> @opindex fsso-struct >>>>>> Set the default scalar storage order of structures and unions to the >>>>>> diff --git a/gcc/testsuite/gcc.dg/strict-flex-array-1.c >>>>>> b/gcc/testsuite/gcc.dg/strict-flex-array-1.c >>>>>> new file mode 100644 >>>>>> index 00000000000..ec886c99b25 >>>>>> --- /dev/null >>>>>> +++ b/gcc/testsuite/gcc.dg/strict-flex-array-1.c >>>>>> @@ -0,0 +1,31 @@ >>>>>> +/* testing the correct usage of attribute strict_flex_array. */ >>>>>> +/* { dg-do compile } */ >>>>>> +/* { dg-options "-O2" } */ >>>>>> + >>>>>> + >>>>>> +int x __attribute__ ((strict_flex_array (1))); /* { dg-error >>>>>> "'strict_flex_array' attribute may not be specified for 'x'" } */ >>>>>> + >>>>>> +struct trailing { >>>>>> + int a; >>>>>> + int c __attribute ((strict_flex_array)); /* { dg-error "wrong >>>>>> number of arguments specified for 'strict_flex_array' attribute" } */ >>>>>> +}; >>>>>> + >>>>>> +struct trailing_1 { >>>>>> + int a; >>>>>> + int b; >>>>>> + int c __attribute ((strict_flex_array (2))); /* { dg-error >>>>>> "'strict_flex_array' attribute may not be specified for a non array >>>>>> field" } */ >>>>>> +}; >>>>>> + >>>>>> +extern int d; >>>>>> + >>>>>> +struct trailing_array_2 { >>>>>> + int a; >>>>>> + int b; >>>>>> + int c[1] __attribute ((strict_flex_array (d))); /* { dg-error >>>>>> "'strict_flex_array' attribute argument not an integer" } */ >>>>>> +}; >>>>>> + >>>>>> +struct trailing_array_3 { >>>>>> + int a; >>>>>> + int b; >>>>>> + int c[0] __attribute ((strict_flex_array (5))); /* { dg-error >>>>>> "'strict_flex_array' attribute argument '5' is not an integer constant >>>>>> between 0 and 3" } */ >>>>>> +}; >>>>>> diff --git a/gcc/tree-core.h b/gcc/tree-core.h >>>>>> index ea9f281f1cc..458c6e6ceea 100644 >>>>>> --- a/gcc/tree-core.h >>>>>> +++ b/gcc/tree-core.h >>>>>> @@ -1813,7 +1813,10 @@ struct GTY(()) tree_decl_common { >>>>>> TYPE_WARN_IF_NOT_ALIGN. */ >>>>>> unsigned int warn_if_not_align : 6; >>>>>> >>>>>> - /* 14 bits unused. */ >>>>>> + /* In FIELD_DECL, this is DECL_NOT_FLEXARRAY. */ >>>>>> + unsigned int decl_not_flexarray : 1; >>>>>> + >>>>>> + /* 13 bits unused. */ >>>>> >>>>> I've not seen it so you are probably missing it - the bit has to be >>>>> streamed in tree-streamer-{in,out}.cc to be usable from LTO. >>>> >>>> You mean add it to the routine “unpack_ts_decl_common_value_fields” of >>>> tree-streamer-in.cc >>>> And “pack_ts_decl_common_value_fields” of tree-streamer-out.cc? >>> >>> Yes. >>> >>>> >>>>> C++ module streaming also needs to handle it. >>>> >>>> Which file is for this C++ module streaming? >>> >>> Not sure, maybe cp/module.cc >> >> Just checked this file, looks like it’s “Experimental”: >> /* C++ modules. Experimental! >> >> Is it used right now? To whom I can consult with to get more detail on C++? > > There's testsuite in g++.dg/modules/, Nathan Sidwell should know since > he implemented it. Thanks, will check on this and confirm with Nathan. > >>> >>>>> >>>>>> >>>>>> /* UID for points-to sets, stable over copying from inlining. */ >>>>>> unsigned int pt_uid; >>>>>> diff --git a/gcc/tree.cc b/gcc/tree.cc >>>>>> index 84000dd8b69..02e274699fb 100644 >>>>>> --- a/gcc/tree.cc >>>>>> +++ b/gcc/tree.cc >>>>>> @@ -12862,7 +12862,7 @@ get_initializer_for (tree init, tree decl) >>>>>> /* Determines the size of the member referenced by the COMPONENT_REF >>>>>> REF, using its initializer expression if necessary in order to >>>>>> determine the size of an initialized flexible array member. >>>>>> - If non-null, set *ARK when REF refers to an interior zero-length >>>>>> + If non-null, set *SAM when REF refers to an interior zero-length >>>>>> array or a trailing one-element array. >>>>>> Returns the size as sizetype (which might be zero for an object >>>>>> with an uninitialized flexible array member) or null if the size >>>>>> @@ -12878,16 +12878,32 @@ component_ref_size (tree ref, >>>>>> special_array_member *sam /* = NULL */) >>>>>> sam = &sambuf; >>>>>> *sam = special_array_member::none; >>>>>> >>>>>> + /* Whether this ref is an array at the end of a structure. */ >>>>>> + bool trailing = array_at_struct_end_p (ref); >>>>>> + >>>>> >>>>> actually array_at_struct_end_p returns whether ref possibly refers to >>>>> a trailing array. In particular it may return false for arrays at >>>>> struct end with a known length as in a.b[i] for the reference to global >>>>> 'a': >>>>> >>>>> struct { int b[1]; } a; >>>> >>>> Yes, I noticed this behavior during my recent debugging of this routine. I >>>> thought it’s a bug and planned to file another bug against it. >>>> Looks like that this is an expected behavior. >>>> >>>> Then I really feel the name and the comments of the routine is very >>>> confusing… >>>> Shall we change the name of this routine to a more descriptive one? For >>>> example, “flexible_array_member_p”? >>> >>> Note the routine is about classifying a concrete reference, so a better >>> name could be flex_array_ref_p (). The name change should be done >>> separately (if at all). >> >> Okay. >> >>> >>>>> >>>>> so in the end it should be array_at_struct_end_p also honoring >>>>> DECL_NOT_FLEXARRAY. >>>> >>>> Then it’s make more sense to check DECL_NOT_FLEXARRAY inside this utility >>>> routine? >>> >>> Yes. >>> >>>>> >>>>>> /* The object/argument referenced by the COMPONENT_REF and its type. */ >>>>>> tree arg = TREE_OPERAND (ref, 0); >>>>>> tree argtype = TREE_TYPE (arg); >>>>>> - /* The referenced member. */ >>>>>> - tree member = TREE_OPERAND (ref, 1); >>>>>> >>>>>> + /* The referenced field member. */ >>>>>> + tree member = TREE_OPERAND (ref, 1); >>>>>> + tree memtype = TREE_TYPE (member); >>>>>> tree memsize = DECL_SIZE_UNIT (member); >>>>>> + >>>>>> + bool is_zero_length_array_ref = zero_length_array_p (memtype); >>>>>> + bool is_constant_length_array_ref = false; >>>>>> + bool is_one_element_array_ref >>>>>> + = one_element_array_p (memtype, &is_constant_length_array_ref); >>>>>> + >>>>>> + /* Determine the type of the special array member. */ >>>>>> + if (is_zero_length_array_ref) >>>>>> + *sam = trailing ? special_array_member::trail_0 >>>>>> + : special_array_member::int_0; >>>>>> + else if (is_one_element_array_ref && trailing) >>>>>> + *sam = special_array_member::trail_1; >>>>>> + >>>>>> if (memsize) >>>>>> { >>>>>> - tree memtype = TREE_TYPE (member); >>>>>> if (TREE_CODE (memtype) != ARRAY_TYPE) >>>>>> /* DECL_SIZE may be less than TYPE_SIZE in C++ when referring >>>>>> to the type of a class with a virtual base which doesn't >>>>>> @@ -12897,50 +12913,30 @@ component_ref_size (tree ref, >>>>>> special_array_member *sam /* = NULL */) >>>>>> return (tree_int_cst_equal (memsize, TYPE_SIZE_UNIT (memtype)) >>>>>> ? memsize : NULL_TREE); >>>>>> >>>>>> - bool trailing = array_at_struct_end_p (ref); >>>>>> - bool zero_length = integer_zerop (memsize); >>>>>> - if (!trailing && !zero_length) >>>>>> + if (!trailing && !is_zero_length_array_ref) >>>>>> /* MEMBER is either an interior array or is an array with >>>>>> more than one element. */ >>>>>> return memsize; >>>>>> >>>>>> - if (zero_length) >>>>>> - { >>>>>> - if (trailing) >>>>>> - *sam = special_array_member::trail_0; >>>>>> - else >>>>>> - { >>>>>> - *sam = special_array_member::int_0; >>>>>> - memsize = NULL_TREE; >>>>>> - } >>>>>> - } >>>>>> + if (*sam != special_array_member::trail_1 >>>>>> + && is_constant_length_array_ref) >>>>>> + /* MEMBER is a constant length array which is not a one-element >>>>>> + trailing array. */ >>>>>> + return memsize; >>>>>> >>>>>> - if (!zero_length) >>>>>> - if (tree dom = TYPE_DOMAIN (memtype)) >>>>>> - if (tree min = TYPE_MIN_VALUE (dom)) >>>>>> - if (tree max = TYPE_MAX_VALUE (dom)) >>>>>> - if (TREE_CODE (min) == INTEGER_CST >>>>>> - && TREE_CODE (max) == INTEGER_CST) >>>>>> - { >>>>>> - offset_int minidx = wi::to_offset (min); >>>>>> - offset_int maxidx = wi::to_offset (max); >>>>>> - offset_int neltsm1 = maxidx - minidx; >>>>>> - if (neltsm1 > 0) >>>>>> - /* MEMBER is an array with more than one element. >>>>>> */ >>>>>> - return memsize; >>>>>> - >>>>>> - if (neltsm1 == 0) >>>>>> - *sam = special_array_member::trail_1; >>>>>> - } >>>>>> + if (*sam == special_array_member::int_0) >>>>>> + memsize = NULL_TREE; >>>>>> >>>>>> - /* For a reference to a zero- or one-element array member of a >>>>>> union >>>>>> - use the size of the union instead of the size of the member. >>>>>> */ >>>>>> + /* For a reference to a flexible array member, an interior zero >>>>>> length >>>>>> + array, or an array with variable length of a union, use the >>>>>> size of >>>>>> + the union instead of the size of the member. */ >>>>>> if (TREE_CODE (argtype) == UNION_TYPE) >>>>>> memsize = TYPE_SIZE_UNIT (argtype); >>>>>> } >>>>>> >>>>>> - /* MEMBER is either a bona fide flexible array member, or a >>>>>> zero-length >>>>>> - array member, or an array of length one treated as such. */ >>>>>> + /* MEMBER now is a flexible array member, an interior zero length >>>>>> array, or >>>>>> + an array with variable length. We need to decide its size from its >>>>>> + initializer. */ >>>>>> >>>>>> /* If the reference is to a declared object and the member a true >>>>>> flexible array, try to determine its size from its initializer. */ >>>>>> @@ -14351,6 +14347,59 @@ default_is_empty_record (const_tree type) >>>>>> return is_empty_type (TYPE_MAIN_VARIANT (type)); >>>>>> } >>>>>> >>>>>> +/* Determine whether TYPE is a ISO flexible array memeber type "[]". */ >>>>> >>>>> ISO C99 >>>> >>>> Okay, will add this. >>>>> >>>>>> +bool >>>>>> +flexible_array_member_p (const_tree type) >>>>> >>>>> since you pass in a type a better name would be >>>>> flexible_array_type_p? >>>> Okay, how about “flexible_array_member_type_p”? (Since “flexible array >>>> member” is a complete concept). >>> >>> works for me >>> >>>>> >>>>>> +{ >>>>>> + if (TREE_CODE (type) == ARRAY_TYPE >>>>>> + && TYPE_SIZE (type) == NULL_TREE >>>>>> + && TYPE_DOMAIN (type) != NULL_TREE >>>>> >>>>> why require a specified TYPE_DOMAIN? >>>> >>>> There are multiple places in the current GCC used the following sequence: >>>> >>>> - if (TREE_CODE (type)) == ARRAY_TYPE >>>> - && TYPE_SIZE (type) == NULL_TREE >>>> - && TYPE_DOMAIN (type) != NULL_TREE >>>> - && TYPE_MAX_VALUE (TYPE_DOMAIN (type)) == NULL_TREE) >>>> >>>> To check whether the type is a flexible_array_member type. >>>> (For example, the routine “add_flexible_array_elts_to_size” in c/c-decl.cc >>>> the routine “finish_struct” in c/c-decl.cc >>>> the routine “flexible_array_type_p” in tree.cc) >>>> >>>> That’s the reason I come up with this common routine to replace all these >>>> sequences. >>>> >>>> >>>>>> + && TYPE_MAX_VALUE (TYPE_DOMAIN (type)) == NULL_TREE) >>>>> >>>>> and why a NULL TYPE_MAX_VALUE? Isn't an unknown TYPE_SIZE enough? >>>> >>>> Does the current FE generate such IR for a []? An array type, without >>>> TYPE_SIZE, with a TYPE_DOMAIN, but the MAX_VALUE of the TYPE_DOMAIN is >>>> NULL? >>>> >>>>> >>>>> That said, I'm not sure providing this abstraction is a good idea >>>>> given the use I see is in frontend code. >>>> >>>> This one is also used in middle-end, for example “flexible_array_type_p” >>>> in tree.cc. >>> >>> That was originally in c-decl.c, seems some target wanted to use it. The >>> issue with this is that it tests the C frontend way of declaring a flex >>> array (and it is used by the frontend), but there are many other possible >>> representations the middle-end should classify as flex array. >>> >>> The Objective C frontend also has a function named this way and it seems >>> to be a 1:1 copy :/ >>> >>> The nios2 use of the function doesn't make much sense to me - externals >>> should not end up in _any_ section!? >> >> What’s you mean by “nios2” ? Not quite understand this sentence. -:) could >> you explain a little bit? > > nios2 is one of GCCs target architectures, it was a use in the nios2 > backend this function was moved to generic code for but the actual use > doesn't really make sense to me ... (it's not your fault of course) Okay, I see. Thanks for the detailed explanation. And now I understand your point, GCC has multiple FEs, so the FE specific concept (for example, this flexible array member concept is a C/C++ specific concept) should not be put into middle end. I will keep this in mind in the next version. > >>> >>> So I'd rather move this function back to where it came from, ideally >>> to c-family/, removing the copy in objective C ... >> >> Okay, I will try this in the next version. >>> >>>>> >>>>>> + return true; >>>>>> + >>>>>> + return false; >>>>>> +} >>>>>> + >>>>>> +/* Determine whether TYPE is a zero-length array type "[0]". */ >>>>>> +bool >>>>>> +zero_length_array_p (const_tree type) >>>>>> +{ >>>>>> + if (TREE_CODE (type) == ARRAY_TYPE) >>>>>> + if (tree type_size = TYPE_SIZE_UNIT (type)) >>>>>> + if ((integer_zerop (type_size)) >>>>>> + && TYPE_DOMAIN (type) != NULL_TREE >>>>>> + && TYPE_MAX_VALUE (TYPE_DOMAIN (type)) == NULL_TREE) >>>>> >>>>> that again seems very C(?) frontend specific, please drop it. >>>> Currently, the middle-end utility routine “component_ref_size” need to >>>> check zero_length_array, one_element_array, etc, >>>> So, this is not only a FE specific routine. >>> >>> Sigh, it's nother strange function that was introduced just for >>> diagnostics (so it's not "correct"), it shouldn't have ended up in >>> tree.cc IMHO. >> >> The codes in GCC to handle flexible array are really messy, I hope that we >> can make it clean and consistent through this work. >> Of course gradually… > > As said, it's unfortunate to have many APIs doing similar things and > I'd rather avoid introducing new APIs (with to me, questionable > implementation or general usefulness) when possible. Okay, will keep the APIs as simple as possible. > >>> >>>> And I think that for making the -fstrict-flex-array work clear, we might >>>> want to emphasize and distinguish the concepts of >>>> >>>> [] flexible_array_member_type_p >>>> [0] zero_length_array_type_p >>>> [1] one_element_array_type_p >>>> >>>> Across GCC. >>> >>> Why? >> >> Because the whole confusion of flexible array handlings in our compiler >> started from the confusion of these concepts. >> During my study of this whole problem, I found that these concepts were >> really confusion, and after I was finally clear on >> These concepts, I started to know how to resolve the whole issue. >> >> In our GCC doc, https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html >> >> We already have such concepts, but not very clear. >> I think making these concepts clear is very important to resolve such messy >> situation in GCC. >> >>> The only important thing to the middle-end is whether the >>> size of the array is known (aka array_at_struct_end_p). This >>> routine classifies it with >> >> But whether the size of the array is known depends on whether the array is a >> >> [0], >> [1], >> [] >> >> + -fstrict-flex-arrays=n > > But only array_at_struct_end_p should care and for it we will have > DECL_NOT_FLEXARRAY. And frontends may have different ideas how they > represent [0], [1] or [] (and in fact they do). The rest of the > middle-end really should _not_ care. Agreed now. -:) > >> >>> >>> /* The array now is at struct end. Treat flexible arrays as >>> always subject to extend, even into just padding constrained by >>> an underlying decl. */ >>> if (! TYPE_SIZE (atype) >>> || ! TYPE_DOMAIN (atype) >>> || ! TYPE_MAX_VALUE (TYPE_DOMAIN (atype))) >>> return true; >> >> This sequence is only for identifying []. Right? >> >> The [0] and [1] is not handled by the above. > > Yes. But also int a[1:] would be identified this way, other frontends > like Ada or Fortran have means to more elaborately specify array > dimensions. Okay. I see. > >>> >>> any flexible_array_member_type_p that covers a different subset of >>> flex-array types would be hugely confusing. >> >> Don’t quite understand this sentence... > > I mean if array_at_struct_end_p decides sth is a flex array reference > but flexible_array_member_type_p on the type decides it is not, that > would be confusing. Thus if adding such new API then it should be > possible to use it from array_at_struct_end_p, removing then some > of the redundant checks there. Okay, I see now. Thanks. Will work on the next version. Qing > >>> >>>> >>>>> >>>>>> + return true; >>>>>> + return false; >>>>>> +} >>>>>> + >>>>>> +/* Determine whether TYPE is a one-element array type "[1]". >>>>>> + Set IS_CONSTANT_LENGTH to true if the length is constant, >>>>>> + otherwise, IS_CONSTANT_LENGTH is set to false. */ >>>>>> +bool >>>>>> +one_element_array_p (const_tree type, bool *is_constant_length /* = >>>>>> NULL */) >>>>>> +{ >>>>>> + if (is_constant_length) >>>>>> + *is_constant_length = false; >>>>>> + >>>>>> + if (TREE_CODE (type) == ARRAY_TYPE) >>>>>> + if (tree dom = TYPE_DOMAIN (type)) >>>>>> + if (tree min = TYPE_MIN_VALUE (dom)) >>>>>> + if (tree max = TYPE_MAX_VALUE (dom)) >>>>>> + if (TREE_CODE (min) == INTEGER_CST >>>>>> + && TREE_CODE (max) == INTEGER_CST) >>>>>> + { >>>>>> + offset_int minidx = wi::to_offset (min); >>>>>> + offset_int maxidx = wi::to_offset (max); >>>>>> + offset_int neltsm1 = maxidx - minidx; >>>>>> + if (is_constant_length) >>>>>> + *is_constant_length = true; >>>>>> + if (neltsm1 == 0) >>>>>> + return true; >>>>>> + } >>>>> >>>>> I'd say likewise. Maybe move them to c-family/c-common.{cc,h} instead >>>>> in a more specialized way for the single use you have? >>>> >>>> It’s not single use, it’s also used in “component_ref_size” routine. >>> >>> Don't copy from a routine that's not designed to be "correct”. >> >> Okay…. >> >> >>> In >>> fact the middle-end already has array_type_nelts, so one_element_array_p >>> can be written as integer_zerop (array_type_nelts (type)). It also >>> has the comment >>> >>> /* TYPE_MAX_VALUE may not be set if the array has unknown length. */ >>> if (!max) >>> { >>> /* zero sized arrays are represented from C FE as complete types >>> with >>> NULL TYPE_MAX_VALUE and zero TYPE_SIZE, while C++ FE represents >>> them as min 0, max -1. */ >>> if (COMPLETE_TYPE_P (type) >>> && integer_zerop (TYPE_SIZE (type)) >>> && integer_zerop (min)) >>> return build_int_cst (TREE_TYPE (min), -1); >> >> Thanks for the info, I will check on the above, and rewrite >> “one_element_array_p” with “array_type_nelts”. >> >> Qing >>> >>> Richard. >>> >>>> Thanks a lot for your comments. >>>> >>>> Qing >>>>> >>>>>> + return false; >>>>>> +} >>>>>> + >>>>>> /* Determine whether TYPE is a structure with a flexible array member, >>>>>> or a union containing such a structure (possibly recursively). */ >>>>>> >>>>>> @@ -14367,10 +14416,7 @@ flexible_array_type_p (const_tree type) >>>>>> last = x; >>>>>> if (last == NULL_TREE) >>>>>> return false; >>>>>> - if (TREE_CODE (TREE_TYPE (last)) == ARRAY_TYPE >>>>>> - && TYPE_SIZE (TREE_TYPE (last)) == NULL_TREE >>>>>> - && TYPE_DOMAIN (TREE_TYPE (last)) != NULL_TREE >>>>>> - && TYPE_MAX_VALUE (TYPE_DOMAIN (TREE_TYPE (last))) == >>>>>> NULL_TREE) >>>>>> + if (flexible_array_member_p (TREE_TYPE (last))) >>>>>> return true; >>>>>> return false; >>>>>> case UNION_TYPE: >>>>>> diff --git a/gcc/tree.h b/gcc/tree.h >>>>>> index e6564aaccb7..3107de5b499 100644 >>>>>> --- a/gcc/tree.h >>>>>> +++ b/gcc/tree.h >>>>>> @@ -2993,6 +2993,11 @@ extern void decl_value_expr_insert (tree, tree); >>>>>> #define DECL_PADDING_P(NODE) \ >>>>>> (FIELD_DECL_CHECK (NODE)->decl_common.decl_flag_3) >>>>>> >>>>>> +/* Used in a FIELD_DECL to indicate whether this field is not a flexible >>>>>> + array member. */ >>>>>> +#define DECL_NOT_FLEXARRAY(NODE) \ >>>>>> + (FIELD_DECL_CHECK (NODE)->decl_common.decl_not_flexarray) >>>>>> + >>>>>> /* A numeric unique identifier for a LABEL_DECL. The UID allocation is >>>>>> dense, unique within any one function, and may be used to index arrays. >>>>>> If the value is -1, then no UID has been assigned. */ >>>>>> @@ -5531,10 +5536,10 @@ extern tree component_ref_field_offset (tree); >>>>>> returns null. */ >>>>>> enum struct special_array_member >>>>>> { >>>>>> - none, /* Not a special array member. */ >>>>>> - int_0, /* Interior array member with size zero. */ >>>>>> - trail_0, /* Trailing array member with size zero. */ >>>>>> - trail_1 /* Trailing array member with one element. */ >>>>>> + none, /* Not a special array member. */ >>>>>> + int_0, /* Interior array member with size zero. */ >>>>>> + trail_0, /* Trailing array member with size zero. */ >>>>>> + trail_1 /* Trailing array member with one element. */ >>>>>> }; >>>>>> >>>>>> /* Return the size of the member referenced by the COMPONENT_REF, using >>>>>> @@ -6489,6 +6494,9 @@ extern void gt_pch_nx (tree &, >>>>>> gt_pointer_operator, void *); >>>>>> extern bool nonnull_arg_p (const_tree); >>>>>> extern bool is_empty_type (const_tree); >>>>>> extern bool default_is_empty_record (const_tree); >>>>>> +extern bool zero_length_array_p (const_tree); >>>>>> +extern bool one_element_array_p (const_tree, bool * = NULL); >>>>>> +extern bool flexible_array_member_p (const_tree); >>>>>> extern bool flexible_array_type_p (const_tree); >>>>>> extern HOST_WIDE_INT arg_int_size_in_bytes (const_tree); >>>>>> extern tree arg_size_in_bytes (const_tree); >>>>>> >>>>> >>>>> -- >>>>> 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) >>>> >>>> >>> >>> -- >>> 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) >> >> > > -- > 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)