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