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?
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.
>
>> +
>> +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?
> C++ module streaming also needs to handle it.
Which file is for this C++ module streaming?
>
>>
>> /* 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”?
>
> 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?
>
>> /* 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).
>
>> +{
>> + 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.
>
>> + 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.
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.
>
>> + 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.
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)
