Hi,
I’d like to ping this patch set one more time.
Thanks
Qing
> On Aug 25, 2023, at 11:24 AM, Qing Zhao <qing.z...@oracle.com> wrote:
>
> Provide a new counted_by attribute to flexible array member field.
>
> 'counted_by (COUNT)'
> The 'counted_by' attribute may be attached to the flexible array
> member of a structure. It indicates that the number of the
> elements of the array is given by the field named "COUNT" in the
> same structure as the flexible array member. GCC uses this
> information to improve the results of the array bound sanitizer and
> the '__builtin_dynamic_object_size'.
>
> For instance, the following code:
>
> struct P {
> size_t count;
> char other;
> char array[] __attribute__ ((counted_by (count)));
> } *p;
>
> specifies that the 'array' is a flexible array member whose number
> of elements is given by the field 'count' in the same structure.
>
> The field that represents the number of the elements should have an
> integer type. An explicit 'counted_by' annotation defines a
> relationship between two objects, 'p->array' and 'p->count', that
> 'p->array' has _at least_ 'p->count' number of elements available.
> This relationship must hold even after any of these related objects
> are updated. It's the user's responsibility to make sure this
> relationship to be kept all the time. Otherwise the results of the
> array bound sanitizer and the '__builtin_dynamic_object_size' might
> be incorrect.
>
> For instance, in the following example, the allocated array has
> less elements than what's specified by the 'sbuf->count', this is
> an user error. As a result, out-of-bounds access to the array
> might not be detected.
>
> #define SIZE_BUMP 10
> struct P *sbuf;
> void alloc_buf (size_t nelems)
> {
> sbuf = (struct P *) malloc (MAX (sizeof (struct P),
> (offsetof (struct P, array[0])
> + nelems * sizeof (char))));
> sbuf->count = nelems + SIZE_BUMP;
> /* This is invalid when the sbuf->array has less than sbuf->count
> elements. */
> }
>
> In the following example, the 2nd update to the field 'sbuf->count'
> of the above structure will permit out-of-bounds access to the
> array 'sbuf>array' as well.
>
> #define SIZE_BUMP 10
> struct P *sbuf;
> void alloc_buf (size_t nelems)
> {
> sbuf = (struct P *) malloc (MAX (sizeof (struct P),
> (offsetof (struct P, array[0])
> + (nelems + SIZE_BUMP) * sizeof
> (char))));
> sbuf->count = nelems;
> /* This is valid when the sbuf->array has at least sbuf->count
> elements. */
> }
> void use_buf (int index)
> {
> sbuf->count = sbuf->count + SIZE_BUMP + 1;
> /* Now the value of sbuf->count is larger than the number
> of elements of sbuf->array. */
> sbuf->array[index] = 0;
> /* then the out-of-bound access to this array
> might not be detected. */
> }
>
> gcc/c-family/ChangeLog:
>
> PR C/108896
> * c-attribs.cc (handle_counted_by_attribute): New function.
> (attribute_takes_identifier_p): Add counted_by attribute to the list.
> * c-common.cc (c_flexible_array_member_type_p): ...To this.
> * c-common.h (c_flexible_array_member_type_p): New prototype.
>
> gcc/c/ChangeLog:
>
> PR C/108896
> * c-decl.cc (flexible_array_member_type_p): Renamed and moved to...
> (add_flexible_array_elts_to_size): Use renamed function.
> (is_flexible_array_member_p): Use renamed function.
> (verify_counted_by_attribute): New function.
> (finish_struct): Use renamed function and verify counted_by
> attribute.
>
> gcc/ChangeLog:
>
> PR C/108896
> * doc/extend.texi: Document attribute counted_by.
> * tree.cc (get_named_field): New function.
> * tree.h (get_named_field): New prototype.
>
> gcc/testsuite/ChangeLog:
>
> PR C/108896
> * gcc.dg/flex-array-counted-by.c: New test.
> ---
> gcc/c-family/c-attribs.cc | 54 ++++++++++++-
> gcc/c-family/c-common.cc | 13 ++++
> gcc/c-family/c-common.h | 1 +
> gcc/c/c-decl.cc | 79 +++++++++++++++-----
> gcc/doc/extend.texi | 77 +++++++++++++++++++
> gcc/testsuite/gcc.dg/flex-array-counted-by.c | 40 ++++++++++
> gcc/tree.cc | 40 ++++++++++
> gcc/tree.h | 5 ++
> 8 files changed, 291 insertions(+), 18 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/flex-array-counted-by.c
>
> diff --git a/gcc/c-family/c-attribs.cc b/gcc/c-family/c-attribs.cc
> index e2792ca6898b..65e4f6639109 100644
> --- a/gcc/c-family/c-attribs.cc
> +++ b/gcc/c-family/c-attribs.cc
> @@ -103,6 +103,8 @@ 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_counted_by_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 *);
> @@ -373,6 +375,8 @@ const struct attribute_spec c_common_attribute_table[] =
> handle_warn_if_not_aligned_attribute, NULL },
> { "strict_flex_array", 1, 1, true, false, false, false,
> handle_strict_flex_array_attribute, NULL },
> + { "counted_by", 1, 1, true, false, false, false,
> + handle_counted_by_attribute, NULL },
> { "weak", 0, 0, true, false, false, false,
> handle_weak_attribute, NULL },
> { "noplt", 0, 0, true, false, false, false,
> @@ -601,7 +605,8 @@ attribute_takes_identifier_p (const_tree attr_id)
> else if (!strcmp ("mode", spec->name)
> || !strcmp ("format", spec->name)
> || !strcmp ("cleanup", spec->name)
> - || !strcmp ("access", spec->name))
> + || !strcmp ("access", spec->name)
> + || !strcmp ("counted_by", spec->name))
> return true;
> else
> return targetm.attribute_takes_identifier_p (attr_id);
> @@ -2555,6 +2560,53 @@ handle_strict_flex_array_attribute (tree *node, tree
> name,
> return NULL_TREE;
> }
>
> +/* Handle a "counted_by" attribute; arguments as in
> + struct attribute_spec.handler. */
> +
> +static tree
> +handle_counted_by_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 non-field"
> + " declaration %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;
> + }
> + /* This attribute only applies to a C99 flexible array member type. */
> + else if (! c_flexible_array_member_type_p (TREE_TYPE (decl)))
> + {
> + error_at (DECL_SOURCE_LOCATION (decl),
> + "%qE attribute may not be specified for a non"
> + " flexible array member field",
> + name);
> + *no_add_attrs = true;
> + }
> + /* The argument should be an identifier. */
> + else if (TREE_CODE (argval) != IDENTIFIER_NODE)
> + {
> + error_at (DECL_SOURCE_LOCATION (decl),
> + "%<counted_by%> argument not an identifier");
> + *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-common.cc b/gcc/c-family/c-common.cc
> index 9fbaeb437a12..a18937245c2a 100644
> --- a/gcc/c-family/c-common.cc
> +++ b/gcc/c-family/c-common.cc
> @@ -9521,6 +9521,19 @@ c_common_finalize_early_debug (void)
> (*debug_hooks->early_global_decl) (cnode->decl);
> }
>
> +/* Determine whether TYPE is a ISO C99 flexible array memeber type "[]". */
> +bool
> +c_flexible_array_member_type_p (const_tree type)
> +{
> + if (TREE_CODE (type) == ARRAY_TYPE
> + && TYPE_SIZE (type) == NULL_TREE
> + && TYPE_DOMAIN (type) != NULL_TREE
> + && TYPE_MAX_VALUE (TYPE_DOMAIN (type)) == NULL_TREE)
> + return true;
> +
> + return false;
> +}
> +
> /* Get the LEVEL of the strict_flex_array for the ARRAY_FIELD based on the
> values of attribute strict_flex_array and the flag_strict_flex_arrays. */
> unsigned int
> diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
> index 78fc5248ba68..c29bb429062b 100644
> --- a/gcc/c-family/c-common.h
> +++ b/gcc/c-family/c-common.h
> @@ -909,6 +909,7 @@ extern tree fold_for_warn (tree);
> extern tree c_common_get_narrower (tree, int *);
> extern bool get_attribute_operand (tree, unsigned HOST_WIDE_INT *);
> extern void c_common_finalize_early_debug (void);
> +extern bool c_flexible_array_member_type_p (const_tree);
> extern unsigned int c_strict_flex_array_level_of (tree);
> extern bool c_option_is_from_cpp_diagnostics (int);
>
> diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
> index 1f9eb44dbaa2..e943b49b5230 100644
> --- a/gcc/c/c-decl.cc
> +++ b/gcc/c/c-decl.cc
> @@ -5173,19 +5173,6 @@ set_array_declarator_inner (struct c_declarator *decl,
> return decl;
> }
>
> -/* Determine whether TYPE is a ISO C99 flexible array memeber type "[]". */
> -static bool
> -flexible_array_member_type_p (const_tree type)
> -{
> - if (TREE_CODE (type) == ARRAY_TYPE
> - && TYPE_SIZE (type) == NULL_TREE
> - && TYPE_DOMAIN (type) != NULL_TREE
> - && TYPE_MAX_VALUE (TYPE_DOMAIN (type)) == NULL_TREE)
> - return true;
> -
> - return false;
> -}
> -
> /* Determine whether TYPE is a one-element array type "[1]". */
> static bool
> one_element_array_type_p (const_tree type)
> @@ -5222,7 +5209,7 @@ add_flexible_array_elts_to_size (tree decl, tree init)
>
> elt = CONSTRUCTOR_ELTS (init)->last ().value;
> type = TREE_TYPE (elt);
> - if (flexible_array_member_type_p (type))
> + if (c_flexible_array_member_type_p (type))
> {
> complete_array_type (&type, elt, false);
> DECL_SIZE (decl)
> @@ -9094,7 +9081,7 @@ is_flexible_array_member_p (bool is_last_field,
>
> bool is_zero_length_array = zero_length_array_type_p (TREE_TYPE (x));
> bool is_one_element_array = one_element_array_type_p (TREE_TYPE (x));
> - bool is_flexible_array = flexible_array_member_type_p (TREE_TYPE (x));
> + bool is_flexible_array = c_flexible_array_member_type_p (TREE_TYPE (x));
>
> unsigned int strict_flex_array_level = c_strict_flex_array_level_of (x);
>
> @@ -9124,6 +9111,61 @@ is_flexible_array_member_p (bool is_last_field,
> return false;
> }
>
> +/* Verify the argument of the counted_by attribute of the flexible array
> + member FIELD_DECL is a valid field of the containing structure's
> fieldlist,
> + FIELDLIST, Report error and remove this attribute when it's not. */
> +static void
> +verify_counted_by_attribute (tree fieldlist, tree field_decl)
> +{
> + tree attr_counted_by = lookup_attribute ("counted_by",
> + DECL_ATTRIBUTES (field_decl));
> +
> + if (!attr_counted_by)
> + return;
> +
> + /* If there is an counted_by attribute attached to the field,
> + verify it. */
> +
> + const char *fieldname
> + = IDENTIFIER_POINTER (TREE_VALUE (TREE_VALUE (attr_counted_by)));
> +
> + /* Verify the argument of the attrbute is a valid field of the
> + containing structure. */
> +
> + tree counted_by_field = get_named_field (fieldlist, fieldname);
> +
> + /* Error when the field is not found in the containing structure. */
> + if (!counted_by_field)
> + {
> + error_at (DECL_SOURCE_LOCATION (field_decl),
> + "%qE attribute argument not a field declaration"
> + " in the same structure, ignore it",
> + (get_attribute_name (attr_counted_by)));
> +
> + DECL_ATTRIBUTES (field_decl)
> + = remove_attribute ("counted_by", DECL_ATTRIBUTES (field_decl));
> + }
> + else
> + /* Error when the field is not with an integer type. */
> + {
> + while (TREE_CHAIN (counted_by_field))
> + counted_by_field = TREE_CHAIN (counted_by_field);
> + tree real_field = TREE_VALUE (counted_by_field);
> +
> + if (TREE_CODE (TREE_TYPE (real_field)) != INTEGER_TYPE)
> + {
> + error_at (DECL_SOURCE_LOCATION (field_decl),
> + "%qE attribute argument not a field declaration"
> + " with integer type, ignore it",
> + (get_attribute_name (attr_counted_by)));
> +
> + DECL_ATTRIBUTES (field_decl)
> + = remove_attribute ("counted_by", DECL_ATTRIBUTES (field_decl));
> + }
> + }
> +
> + return;
> +}
>
> /* 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.
> @@ -9244,7 +9286,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 (flexible_array_member_type_p (TREE_TYPE (x)))
> + if (c_flexible_array_member_type_p (TREE_TYPE (x)))
> {
> if (TREE_CODE (t) == UNION_TYPE)
> {
> @@ -9265,6 +9307,9 @@ finish_struct (location_t loc, tree t, tree fieldlist,
> tree attributes,
> "members");
> TREE_TYPE (x) = error_mark_node;
> }
> + /* if there is a counted_by attribute attached to this field,
> + verify it. */
> + verify_counted_by_attribute (fieldlist, x);
> }
>
> if (pedantic && TREE_CODE (t) == RECORD_TYPE
> @@ -9279,7 +9324,7 @@ finish_struct (location_t loc, tree t, tree fieldlist,
> tree attributes,
> when x is an array and is the last field. */
> if (TREE_CODE (TREE_TYPE (x)) == ARRAY_TYPE)
> TYPE_INCLUDES_FLEXARRAY (t)
> - = is_last_field && flexible_array_member_type_p (TREE_TYPE (x));
> + = is_last_field && c_flexible_array_member_type_p (TREE_TYPE (x));
> /* Recursively set TYPE_INCLUDES_FLEXARRAY for the context of x, t
> when x is an union or record and is the last field. */
> else if (RECORD_OR_UNION_TYPE_P (TREE_TYPE (x)))
> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> index 97eaacf8a7ec..ea6240646936 100644
> --- a/gcc/doc/extend.texi
> +++ b/gcc/doc/extend.texi
> @@ -7617,6 +7617,83 @@ 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.
>
> +@cindex @code{counted_by} variable attribute
> +@item counted_by (@var{count})
> +The @code{counted_by} attribute may be attached to the flexible array
> +member of a structure. It indicates that the number of the elements of the
> +array is given by the field named "@var{count}" in the same structure as the
> +flexible array member. GCC uses this information to improve the results of
> +the array bound sanitizer and the @code{__builtin_dynamic_object_size}.
> +
> +For instance, the following code:
> +
> +@smallexample
> +struct P @{
> + size_t count;
> + char other;
> + char array[] __attribute__ ((counted_by (count)));
> +@} *p;
> +@end smallexample
> +
> +@noindent
> +specifies that the @code{array} is a flexible array member whose number of
> +elements is given by the field @code{count} in the same structure.
> +
> +The field that represents the number of the elements should have an integer
> +type. An explicit @code{counted_by} annotation defines a relationship
> between
> +two objects, @code{p->array} and @code{p->count}, that @code{p->array} has
> +@emph{at least} @code{p->count} number of elements available. This
> relationship
> +must hold even after any of these related objects are updated. It's the
> user's
> +responsibility to make sure this relationship to be kept all the time.
> +Otherwise the results of the array bound sanitizer and the
> +@code{__builtin_dynamic_object_size} might be incorrect.
> +
> +For instance, in the following example, the allocated array has less elements
> +than what's specified by the @code{sbuf->count}, this is an user error. As a
> +result, out-of-bounds access to the array might not be detected.
> +
> +@smallexample
> +#define SIZE_BUMP 10
> +struct P *sbuf;
> +void alloc_buf (size_t nelems)
> +@{
> + sbuf = (struct P *) malloc (MAX (sizeof (struct P),
> + (offsetof (struct P, array[0])
> + + nelems * sizeof (char))));
> + sbuf->count = nelems + SIZE_BUMP;
> + /* This is invalid when the sbuf->array has less than sbuf->count
> + elements. */
> +@}
> +@end smallexample
> +
> +In the following example, the 2nd update to the field @code{sbuf->count} of
> +the above structure will permit out-of-bounds access to the array
> +@code{sbuf>array} as well.
> +
> +@smallexample
> +#define SIZE_BUMP 10
> +struct P *sbuf;
> +void alloc_buf (size_t nelems)
> +@{
> + sbuf = (struct P *) malloc (MAX (sizeof (struct P),
> + (offsetof (struct P, array[0])
> + + (nelems + SIZE_BUMP) * sizeof (char))));
> + sbuf->count = nelems;
> + /* This is valid when the sbuf->array has at least sbuf->count
> + elements. */
> +@}
> +void use_buf (int index)
> +@{
> + sbuf->count = sbuf->count + SIZE_BUMP + 1;
> + /* Now the value of sbuf->count is larger than the number
> + of elements of sbuf->array. */
> + sbuf->array[index] = 0;
> + /* then the out-of-bound access to this array
> + might not be detected. */
> +@}
> +@end smallexample
> +
> +
> @cindex @code{alloc_size} variable attribute
> @item alloc_size (@var{position})
> @itemx alloc_size (@var{position-1}, @var{position-2})
> diff --git a/gcc/testsuite/gcc.dg/flex-array-counted-by.c
> b/gcc/testsuite/gcc.dg/flex-array-counted-by.c
> new file mode 100644
> index 000000000000..f8ce9776bf86
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/flex-array-counted-by.c
> @@ -0,0 +1,40 @@
> +/* testing the correct usage of attribute counted_by. */
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +
> +#include <wchar.h>
> +
> +int size;
> +int x __attribute ((counted_by (size))); /* { dg-error "attribute may not be
> specified for non-field declaration" } */
> +
> +struct trailing {
> + int count;
> + int field __attribute ((counted_by (count))); /* { dg-error "attribute may
> not be specified for a non-array field" } */
> +};
> +
> +struct trailing_1 {
> + int count;
> + int array_1[0] __attribute ((counted_by (count))); /* { dg-error
> "attribute may not be specified for a non flexible array member field" } */
> +};
> +
> +int count;
> +struct trailing_array_2 {
> + int count;
> + int array_2[] __attribute ((counted_by ("count"))); /* { dg-error
> "argument not an identifier" } */
> +};
> +
> +struct trailing_array_3 {
> + int other;
> + int array_3[] __attribute ((counted_by (L"count"))); /* { dg-error
> "argument not an identifier" } */
> +};
> +
> +struct trailing_array_4 {
> + int other;
> + int array_4[] __attribute ((counted_by (count))); /* { dg-error "attribute
> argument not a field declaration in the same structure, ignore it" } */
> +};
> +
> +int count;
> +struct trailing_array_5 {
> + float count;
> + int array_5[] __attribute ((counted_by (count))); /* { dg-error "attribute
> argument not a field declaration with integer type, ignore it" } */
> +};
> diff --git a/gcc/tree.cc b/gcc/tree.cc
> index 420857b110c4..fcd36ae0cd74 100644
> --- a/gcc/tree.cc
> +++ b/gcc/tree.cc
> @@ -12745,6 +12745,46 @@ array_ref_element_size (tree exp)
> return SUBSTITUTE_PLACEHOLDER_IN_EXPR (TYPE_SIZE_UNIT (elmt_type), exp);
> }
>
> +/* Given a field list, FIELDLIST, of a structure/union, return a TREE_LIST,
> + with each TREE_VALUE a FIELD_DECL stepping down the chain to the FIELD
> + whose name is FIELDNAME, which is the last TREE_VALUE of the list.
> + return NULL_TREE if such field is not found. Normally this list is of
> + length one, but if the field is embedded with (nested) anonymous
> structures
> + or unions, this list steps down the chain to the field. */
> +tree
> +get_named_field (tree fieldlist, const char *fieldname)
> +{
> + tree named_field = NULL_TREE;
> + for (tree field = fieldlist; field; field = DECL_CHAIN (field))
> + {
> + if (TREE_CODE (field) != FIELD_DECL)
> + continue;
> + if (DECL_NAME (field) != NULL)
> + if (strcmp (IDENTIFIER_POINTER (DECL_NAME (field)), fieldname) == 0)
> + {
> + named_field = tree_cons (NULL_TREE, field, named_field);
> + break;
> + }
> + else
> + continue;
> + /* if the field is an anonymous struct/union, we will check the nested
> + fields inside it recursively. */
> + else if (RECORD_OR_UNION_TYPE_P (TREE_TYPE (field)))
> + if ((named_field = get_named_field (TYPE_FIELDS (TREE_TYPE (field)),
> + fieldname)) != NULL_TREE)
> + {
> + named_field = tree_cons (NULL_TREE, field, named_field);
> + break;
> + }
> + else
> + continue;
> + else
> + continue;
> + }
> + return named_field;
> +}
> +
> +
> /* Return a tree representing the lower bound of the array mentioned in
> EXP, an ARRAY_REF or an ARRAY_RANGE_REF. */
>
> diff --git a/gcc/tree.h b/gcc/tree.h
> index 4c04245e2b1b..4859becaa1e7 100644
> --- a/gcc/tree.h
> +++ b/gcc/tree.h
> @@ -5619,6 +5619,11 @@ extern tree get_base_address (tree t);
> of EXP, an ARRAY_REF or an ARRAY_RANGE_REF. */
> extern tree array_ref_element_size (tree);
>
> +/* Given a field list, FIELDLIST, of a structure/union, return the FIELD
> whose
> + name is FIELDNAME, return NULL_TREE if such field is not found.
> + searching nested anonymous structure/union recursively. */
> +extern tree get_named_field (tree, const char *);
> +
> /* Return a typenode for the "standard" C type with a given name. */
> extern tree get_typenode_from_name (const char *);
>
> --
> 2.31.1
>
