public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Qing Zhao <qing.zhao@oracle.com>
To: Martin Sebor <msebor@gmail.com>
Cc: "gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>,
	Richard Biener <rguenther@suse.de>,
	"jakub@redhat.com" <jakub@redhat.com>,
	"keescook@chromium.org" <keescook@chromium.org>,
	"joseph@codesourcery.com" <joseph@codesourcery.com>
Subject: Re: [GCC13][Patch][V4][PATCH 1/2] Add a new option -fstrict-flex-arrays[=n] and new attribute strict_flex_arrays
Date: Wed, 28 Sep 2022 19:17:14 +0000	[thread overview]
Message-ID: <90234BB4-BAFA-4528-AF95-BD1809B2D37B@oracle.com> (raw)
In-Reply-To: <e5c08729-1c9c-c746-9d15-e6d306c9ad5e@gmail.com>

Hi, Martin,

Thanks for the comments. And sorry for my late reply till now (I just came back home from LPC, GNU Cauldron and then a one-week vacation after that…)

> On Sep 12, 2022, at 12:42 PM, Martin Sebor <msebor@gmail.com> wrote:
> 
> On 9/6/22 18:28, Qing Zhao wrote:
>> Add the following new option -fstrict-flex-arrays[=n] and a corresponding
>> attribute strict_flex_arrays to GCC:
>> '-fstrict-flex-arrays'
>>      Treat the trailing array of a structure as a flexible array member
>>      in a stricter way.
> 
> A minor problem with this phrasing was pointed out in the review
> of the Clang option: https://reviews.llvm.org/D126864#inline-1282716
> It would be good to avoid it here.
Yes, I agree.

>  (I think qualifying the sentence
> by adding "for the purposes of accessing the elements of such arrays"
> might be one way to do it).

How about:

'-fstrict-flex-arrays’
     Treat the trailing array of a structure as a variable-length array
      in a stricter way.
?

> 
>> The positive form is equivalent to
>>      '-fstrict-flex-arrays=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-arrays=0', which is the least
>>      strict.  All trailing arrays of structures are treated as flexible
>>      array members.
>> '-fstrict-flex-arrays=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_arrays' 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_arrays'
>>      attribute (*note Variable Attributes::).
>> 'strict_flex_arrays (LEVEL)'
>>      The 'strict_flex_arrays' 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.
> 
> 
> Since the attribute applies to just a single array declaration it
> seems that its name should be strict_flex_array (i.e., singular,
> without the trailing 's').

Will make the change as you suggested.

Thanks.

Qing
> 
> Martin
> 
>>      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 the
>>      '-fstrict-flex-arrays'.  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_arrays_attribute): New function.
>> 	(c_common_attribute_table): New item for strict_flex_arrays.
>> 	* c.opt: (fstrict-flex-arrays): New option.
>> 	(fstrict-flex-arrays=): New option.
>> gcc/c/ChangeLog:
>> 	* c-decl.cc (flexible_array_member_type_p): New function.
>> 	(one_element_array_type_p): Likewise.
>> 	(zero_length_array_type_p): Likewise.
>> 	(add_flexible_array_elts_to_size): Call new utility
>> 	routine flexible_array_member_type_p.
>> 	(is_flexible_array_member_p): New function.
>> 	(finish_struct): Set the new DECL_NOT_FLEXARRAY flag.
>> gcc/cp/ChangeLog:
>> 	* module.cc (trees_out::core_bools): Stream out new bit
>> 	decl_not_flexarray.
>> 	(trees_in::core_bools): Stream in new bit decl_not_flexarray.
>> gcc/ChangeLog:
>> 	* doc/extend.texi: Document strict_flex_arrays attribute.
>> 	* doc/invoke.texi: Document -fstrict-flex-arrays[=n] option.
>> 	* print-tree.cc (print_node): Print new bit decl_not_flexarray.
>> 	* tree-core.h (struct tree_decl_common): New bit field
>> 	decl_not_flexarray.
>> 	* tree-streamer-in.cc (unpack_ts_decl_common_value_fields): Stream
>> 	in new bit decl_not_flexarray.
>> 	* tree-streamer-out.cc (pack_ts_decl_common_value_fields): Stream
>> 	out new bit decl_not_flexarray.
>> 	* tree.cc (array_at_struct_end_p): Update it with the new bit field
>> 	decl_not_flexarray.
>> 	* tree.h (DECL_NOT_FLEXARRAY): New flag.
>> gcc/testsuite/ChangeLog:
>> 	* g++.dg/strict-flex-array-1.C: New test.
>> 	* 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                            | 130 +++++++++++++++++++--
>>  gcc/cp/module.cc                           |   2 +
>>  gcc/doc/extend.texi                        |  25 ++++
>>  gcc/doc/invoke.texi                        |  27 ++++-
>>  gcc/print-tree.cc                          |   8 +-
>>  gcc/testsuite/g++.dg/strict-flex-array-1.C |  31 +++++
>>  gcc/testsuite/gcc.dg/strict-flex-array-1.c |  31 +++++
>>  gcc/tree-core.h                            |   5 +-
>>  gcc/tree-streamer-in.cc                    |   1 +
>>  gcc/tree-streamer-out.cc                   |   1 +
>>  gcc/tree.cc                                |  45 +++++--
>>  gcc/tree.h                                 |  14 ++-
>>  14 files changed, 346 insertions(+), 28 deletions(-)
>>  create mode 100644 gcc/testsuite/g++.dg/strict-flex-array-1.C
>>  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 8bb80e251dc2..0becd4bbe8d2 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_arrays_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 *);
>> @@ -368,6 +370,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_arrays",      1, 1, false, false, false, false,
>> +			      handle_strict_flex_arrays_attribute, NULL },
>>    { "weak",                   0, 0, true,  false, false, false,
>>  			      handle_weak_attribute, NULL },
>>    { "noplt",                   0, 0, true,  false, false, false,
>> @@ -2505,6 +2509,49 @@ handle_warn_if_not_aligned_attribute (tree *node, tree name,
>>  					  no_add_attrs, true);
>>  }
>>  +/* Handle a "strict_flex_arrays" attribute; arguments as in
>> +   struct attribute_spec.handler.  */
>> +
>> +static tree
>> +handle_strict_flex_arrays_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 4515664aa590..e053bd939559 100644
>> --- a/gcc/c-family/c.opt
>> +++ b/gcc/c-family/c.opt
>> @@ -2072,6 +2072,13 @@ fsized-deallocation
>>  C++ ObjC++ Var(flag_sized_deallocation) Init(-1)
>>  Enable C++14 sized deallocation support.
>>  +fstrict-flex-arrays
>> +C C++ Common Alias(fstrict-flex-arrays=,3,0)
>> +
>> +fstrict-flex-arrays=
>> +C C++ Common Joined RejectNegative UInteger Var(flag_strict_flex_arrays) Init(0) IntegerRange(0,3)
>> +-fstrict-flex-arrays=<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 34f8feda897f..ee90808bcb0e 100644
>> --- a/gcc/c/c-decl.cc
>> +++ b/gcc/c/c-decl.cc
>> @@ -5005,6 +5005,41 @@ set_array_declarator_inner (struct c_declarator *decl,
>>    return decl;
>>  }
>>  +/* Determine whether TYPE is a ISO C99 flexible array memeber type "[]".  */
>> +static bool
>> +flexible_array_member_type_p (const_tree type)
>> +{
>> +  if (TREE_CODE (type) == ARRAY_TYPE
>> +      && TYPE_SIZE (type) == NULL_TREE
>> +      && TYPE_DOMAIN (type) != NULL_TREE
>> +      && TYPE_MAX_VALUE (TYPE_DOMAIN (type)) == NULL_TREE)
>> +    return true;
>> +
>> +  return false;
>> +}
>> +
>> +/* Determine whether TYPE is a one-element array type "[1]".  */
>> +static bool
>> +one_element_array_type_p (const_tree type)
>> +{
>> +  if (TREE_CODE (type) != ARRAY_TYPE)
>> +    return false;
>> +  return integer_zerop (array_type_nelts (type));
>> +}
>> +
>> +/* Determine whether TYPE is a zero-length array type "[0]".  */
>> +static bool
>> +zero_length_array_type_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)
>> +	return true;
>> +  return false;
>> +}
>> +
>>  /* INIT is a constructor that forms DECL's initializer.  If the final
>>     element initializes a flexible array field, add the size of that
>>     initializer to DECL's size.  */
>> @@ -5019,10 +5054,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_type_p (type))
>>      {
>>        complete_array_type (&type, elt, false);
>>        DECL_SIZE (decl)
>> @@ -8710,6 +8742,81 @@ 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_arrays;
>> +  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_type_p (TREE_TYPE (x));
>> +  bool is_one_element_array = one_element_array_type_p (TREE_TYPE (x));
>> +  bool is_flexible_array = flexible_array_member_type_p (TREE_TYPE (x));
>> +
>> +  unsigned int strict_flex_array_level = flag_strict_flex_arrays;
>> +
>> +  tree attr_strict_flex_array = lookup_attribute ("strict_flex_arrays",
>> +						  DECL_ATTRIBUTES (x));
>> +  /* if there is a strict_flex_array attribute attached to the field,
>> +     override the flag_strict_flex_arrays.  */
>> +  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.
>> @@ -8771,6 +8878,11 @@ finish_struct (location_t loc, tree t, tree fieldlist, tree attributes,
>>    bool saw_named_field = false;
>>    for (x = fieldlist; x; x = DECL_CHAIN (x))
>>      {
>> +      /* whether this field is the last field of the structure or union.
>> +	 for UNION, any field is the last field of it.  */
>> +      bool is_last_field = (DECL_CHAIN (x) == NULL_TREE)
>> +			    || (TREE_CODE (t) == UNION_TYPE);
>> +
>>        if (TREE_TYPE (x) == error_mark_node)
>>  	continue;
>>  @@ -8809,10 +8921,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_type_p (TREE_TYPE (x)))
>>  	{
>>  	  if (TREE_CODE (t) == UNION_TYPE)
>>  	    {
>> @@ -8820,7 +8929,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");
>> @@ -8840,6 +8949,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/cp/module.cc b/gcc/cp/module.cc
>> index f27f4d091e5e..75ee2514f66b 100644
>> --- a/gcc/cp/module.cc
>> +++ b/gcc/cp/module.cc
>> @@ -5414,6 +5414,7 @@ trees_out::core_bools (tree t)
>>        WB (t->decl_common.decl_by_reference_flag);
>>        WB (t->decl_common.decl_read_flag);
>>        WB (t->decl_common.decl_nonshareable_flag);
>> +      WB (t->decl_common.decl_not_flexarray);
>>      }
>>      if (CODE_CONTAINS_STRUCT (code, TS_DECL_WITH_VIS))
>> @@ -5558,6 +5559,7 @@ trees_in::core_bools (tree t)
>>        RB (t->decl_common.decl_by_reference_flag);
>>        RB (t->decl_common.decl_read_flag);
>>        RB (t->decl_common.decl_nonshareable_flag);
>> +      RB (t->decl_common.decl_not_flexarray);
>>      }
>>      if (CODE_CONTAINS_STRUCT (code, TS_DECL_WITH_VIS))
>> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
>> index 0fedab96610b..62328d77c78c 100644
>> --- a/gcc/doc/extend.texi
>> +++ b/gcc/doc/extend.texi
>> @@ -7459,6 +7459,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_arrays} variable attribute
>> +@item strict_flex_arrays (@var{level})
>> +The @code{strict_flex_arrays} 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 (@samp{[]}), 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 (@samp{[0]}) or one-size
>> +array as flexible array member (@samp{[1]}):
>> +When @var{level} is 1, the trailing array is treated as a flexible array member
>> +when it is declared as either @samp{[]}, @samp{[0]}, or @samp{[1]};
>> +When @var{level} is 2, the trailing array is treated as a flexible array member
>> +when it is declared as either @samp{[]}, or @samp{[0]}.
>> +
>> +This attribute can be used with or without the @option{-fstrict-flex-arrays}.
>> +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 9d662e353163..db1a68cd545c 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-arrays[=@var{n}] @gol
>> +-fsso-struct=@var{endianness}}
>>    @item C++ Language Options
>>  @xref{C++ Dialect Options,,Options Controlling C++ Dialect}.
>> @@ -2826,6 +2827,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-arrays
>> +@opindex fstrict-flex-arrays
>> +@opindex fno-strict-flex-arrays
>> +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-arrays=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-arrays=0}, which is the
>> +least strict.  All trailing arrays of structures are treated as flexible array
>> +members.
>> +
>> +@item -fstrict-flex-arrays=@var{level}
>> +@opindex fstrict-flex-arrays=@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_arrays} 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_arrays} 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/print-tree.cc b/gcc/print-tree.cc
>> index 6d45a4a59669..58a98250cc4f 100644
>> --- a/gcc/print-tree.cc
>> +++ b/gcc/print-tree.cc
>> @@ -517,8 +517,12 @@ print_node (FILE *file, const char *prefix, tree node, int indent,
>>  	  fprintf (file, " align:%d warn_if_not_align:%d",
>>  		   DECL_ALIGN (node), DECL_WARN_IF_NOT_ALIGN (node));
>>  	  if (code == FIELD_DECL)
>> -	    fprintf (file, " offset_align " HOST_WIDE_INT_PRINT_UNSIGNED,
>> -		     DECL_OFFSET_ALIGN (node));
>> +	    {
>> +	      fprintf (file, " offset_align " HOST_WIDE_INT_PRINT_UNSIGNED,
>> +		       DECL_OFFSET_ALIGN (node));
>> +	      fprintf (file, " decl_not_flexarray: %d",
>> +		       DECL_NOT_FLEXARRAY (node));
>> +	    }
>>    	  if (code == FUNCTION_DECL && fndecl_built_in_p (node))
>>  	    {
>> diff --git a/gcc/testsuite/g++.dg/strict-flex-array-1.C b/gcc/testsuite/g++.dg/strict-flex-array-1.C
>> new file mode 100644
>> index 000000000000..cd6a65d2b7e6
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.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_arrays (1))); /* { dg-error "'strict_flex_arrays' attribute may not be specified for 'x'" } */
>> +
>> +struct trailing {
>> +    int a;
>> +    int c __attribute ((strict_flex_arrays)); /* { dg-error "wrong number of arguments specified for 'strict_flex_arrays' attribute" } */
>> +};
>> +
>> +struct trailing_1 {
>> +    int a;
>> +    int b;
>> +    int c __attribute ((strict_flex_arrays (2))); /* { dg-error "'strict_flex_arrays' 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_arrays (d))); /* { dg-error "'strict_flex_arrays' attribute argument not an integer" } */
>> +};
>> +
>> +struct trailing_array_3 {
>> +    int a;
>> +    int b;
>> +    int c[0] __attribute ((strict_flex_arrays (5))); /* { dg-error "'strict_flex_arrays' attribute argument '5' is not an integer constant between 0 and 3" } */
>> +};
>> 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 000000000000..cd6a65d2b7e6
>> --- /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_arrays (1))); /* { dg-error "'strict_flex_arrays' attribute may not be specified for 'x'" } */
>> +
>> +struct trailing {
>> +    int a;
>> +    int c __attribute ((strict_flex_arrays)); /* { dg-error "wrong number of arguments specified for 'strict_flex_arrays' attribute" } */
>> +};
>> +
>> +struct trailing_1 {
>> +    int a;
>> +    int b;
>> +    int c __attribute ((strict_flex_arrays (2))); /* { dg-error "'strict_flex_arrays' 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_arrays (d))); /* { dg-error "'strict_flex_arrays' attribute argument not an integer" } */
>> +};
>> +
>> +struct trailing_array_3 {
>> +    int a;
>> +    int b;
>> +    int c[0] __attribute ((strict_flex_arrays (5))); /* { dg-error "'strict_flex_arrays' 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 80c2bcb333dd..18bd6d82a32d 100644
>> --- a/gcc/tree-core.h
>> +++ b/gcc/tree-core.h
>> @@ -1823,7 +1823,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.  */
>>      /* UID for points-to sets, stable over copying from inlining.  */
>>    unsigned int pt_uid;
>> diff --git a/gcc/tree-streamer-in.cc b/gcc/tree-streamer-in.cc
>> index 219cf5e7ef45..57923da3741e 100644
>> --- a/gcc/tree-streamer-in.cc
>> +++ b/gcc/tree-streamer-in.cc
>> @@ -261,6 +261,7 @@ unpack_ts_decl_common_value_fields (struct bitpack_d *bp, tree expr)
>>        else
>>  	SET_DECL_FIELD_ABI_IGNORED (expr, val);
>>        expr->decl_common.off_align = bp_unpack_value (bp, 8);
>> +      DECL_NOT_FLEXARRAY (expr) = (unsigned) bp_unpack_value (bp, 1);
>>      }
>>      else if (VAR_P (expr))
>> diff --git a/gcc/tree-streamer-out.cc b/gcc/tree-streamer-out.cc
>> index 9b114dc05bb1..68a2818a9f93 100644
>> --- a/gcc/tree-streamer-out.cc
>> +++ b/gcc/tree-streamer-out.cc
>> @@ -229,6 +229,7 @@ pack_ts_decl_common_value_fields (struct bitpack_d *bp, tree expr)
>>        else
>>  	bp_pack_value (bp, DECL_FIELD_ABI_IGNORED (expr), 1);
>>        bp_pack_value (bp, expr->decl_common.off_align, 8);
>> +      bp_pack_value (bp, DECL_NOT_FLEXARRAY (expr), 1);
>>      }
>>      else if (VAR_P (expr))
>> diff --git a/gcc/tree.cc b/gcc/tree.cc
>> index 2f488e4467c4..e4664eab54ef 100644
>> --- a/gcc/tree.cc
>> +++ b/gcc/tree.cc
>> @@ -12688,14 +12688,30 @@ array_ref_up_bound (tree exp)
>>  }
>>    /* Returns true if REF is an array reference, component reference,
>> -   or memory reference to an array at the end of a structure.
>> -   If this is the case, the array may be allocated larger
>> -   than its upper bound implies.  */
>> +   or memory reference to an array whose actual size might be larger
>> +   than its upper bound implies, there are multiple cases:
>> +   A. a ref to a flexible array member at the end of a structure;
>> +   B. a ref to an array with a different type against the original decl;
>> +      for example:
>>  +   short a[16] = { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16 };
>> +   (*((char(*)[16])&a[0]))[i+8]
>> +
>> +   C. a ref to an array that was passed as a parameter;
>> +      for example:
>> +
>> +   int test (uint8_t *p, uint32_t t[1][1], int n) {
>> +   for (int i = 0; i < 4; i++, p++)
>> +     t[i][0] = ...;
>> +
>> +   FIXME, the name of this routine need to be changed to be more accurate.  */
>>  bool
>>  array_at_struct_end_p (tree ref)
>>  {
>> -  tree atype;
>> +  /* the TYPE for this array referece.  */
>> +  tree atype = NULL_TREE;
>> +  /* the FIELD_DECL for the array field in the containing structure.  */
>> +  tree afield_decl = NULL_TREE;
>>      if (TREE_CODE (ref) == ARRAY_REF
>>        || TREE_CODE (ref) == ARRAY_RANGE_REF)
>> @@ -12705,7 +12721,10 @@ array_at_struct_end_p (tree ref)
>>      }
>>    else if (TREE_CODE (ref) == COMPONENT_REF
>>  	   && TREE_CODE (TREE_TYPE (TREE_OPERAND (ref, 1))) == ARRAY_TYPE)
>> -    atype = TREE_TYPE (TREE_OPERAND (ref, 1));
>> +    {
>> +      atype = TREE_TYPE (TREE_OPERAND (ref, 1));
>> +      afield_decl = TREE_OPERAND (ref, 1);
>> +    }
>>    else if (TREE_CODE (ref) == MEM_REF)
>>      {
>>        tree arg = TREE_OPERAND (ref, 0);
>> @@ -12717,6 +12736,7 @@ array_at_struct_end_p (tree ref)
>>  	  if (tree fld = last_field (argtype))
>>  	    {
>>  	      atype = TREE_TYPE (fld);
>> +	      afield_decl = fld;
>>  	      if (TREE_CODE (atype) != ARRAY_TYPE)
>>  		return false;
>>  	      if (VAR_P (arg) && DECL_SIZE (fld))
>> @@ -12770,13 +12790,16 @@ array_at_struct_end_p (tree ref)
>>        ref = TREE_OPERAND (ref, 0);
>>      }
>>  -  /* The array now is at struct end.  Treat flexible arrays as
>> +  gcc_assert (!afield_decl
>> +	      || (afield_decl && TREE_CODE (afield_decl) == FIELD_DECL));
>> +
>> +  /* The array now is at struct end.  Treat flexible array member 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;
>> +    return afield_decl ? !DECL_NOT_FLEXARRAY (afield_decl) : true;
>>      /* If the reference is based on a declared entity, the size of the array
>>       is constrained by its given domain.  (Do not trust commons PR/69368).  */
>> @@ -12798,9 +12821,9 @@ array_at_struct_end_p (tree ref)
>>        if (TREE_CODE (TYPE_SIZE_UNIT (TREE_TYPE (atype))) != INTEGER_CST
>>  	  || TREE_CODE (TYPE_MAX_VALUE (TYPE_DOMAIN (atype))) != INTEGER_CST
>>            || TREE_CODE (TYPE_MIN_VALUE (TYPE_DOMAIN (atype))) != INTEGER_CST)
>> -	return true;
>> +	return afield_decl ? !DECL_NOT_FLEXARRAY (afield_decl) : true;
>>        if (! get_addr_base_and_unit_offset (ref_to_array, &offset))
>> -	return true;
>> +	return afield_decl ? !DECL_NOT_FLEXARRAY (afield_decl) : true;
>>          /* If at least one extra element fits it is a flexarray.  */
>>        if (known_le ((wi::to_offset (TYPE_MAX_VALUE (TYPE_DOMAIN (atype)))
>> @@ -12808,12 +12831,12 @@ array_at_struct_end_p (tree ref)
>>  		     + 2)
>>  		    * wi::to_offset (TYPE_SIZE_UNIT (TREE_TYPE (atype))),
>>  		    wi::to_offset (DECL_SIZE_UNIT (ref)) - offset))
>> -	return true;
>> +	return afield_decl ? !DECL_NOT_FLEXARRAY (afield_decl) : true;
>>          return false;
>>      }
>>  -  return true;
>> +  return afield_decl ? !DECL_NOT_FLEXARRAY (afield_decl) : true;
>>  }
>>    /* Return a tree representing the offset, in bytes, of the field referenced
>> diff --git a/gcc/tree.h b/gcc/tree.h
>> index 266e24a05633..81076b045003 100644
>> --- a/gcc/tree.h
>> +++ b/gcc/tree.h
>> @@ -3005,6 +3005,12 @@ 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. This is only valid for the last array type field of a
>> +   structure.  */
>> +#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.  */
>> @@ -5543,10 +5549,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


  reply	other threads:[~2022-09-28 19:17 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-07  0:28 [GCC13][Patch][V4][PATCH 0/2] Add a new option -fstrict-flex-array[=n] and attribute strict_flex_array(n) and use it in PR101836 Qing Zhao
2022-09-07  0:28 ` [GCC13][Patch][V4][PATCH 1/2] Add a new option -fstrict-flex-arrays[=n] and new attribute strict_flex_arrays Qing Zhao
2022-09-12 16:42   ` Martin Sebor
2022-09-28 19:17     ` Qing Zhao [this message]
2022-09-30 17:39       ` Martin Sebor
2022-10-03 15:20         ` Qing Zhao
2022-09-07  0:28 ` [GCC13][Patch][V4][PATCH 2/2] Use array_at_struct_end_p in __builtin_object_size [PR101836] Qing Zhao

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=90234BB4-BAFA-4528-AF95-BD1809B2D37B@oracle.com \
    --to=qing.zhao@oracle.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jakub@redhat.com \
    --cc=joseph@codesourcery.com \
    --cc=keescook@chromium.org \
    --cc=msebor@gmail.com \
    --cc=rguenther@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).