> 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." > > 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. ==== > >>> >>>> + >>>> +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++? > >>> >>>> >>>> /* 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? > > 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… > >> 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 > > /* 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. > > 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... > >> >>> >>>> + 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)