Re: [V3][PATCH 1/3] Provide counted_by attribute to flexible array member field (PR108896)

[Siddhesh Poyarekar <siddhesh@gotplt.org>]

On 2023-10-05 14:51, Siddhesh Poyarekar wrote:
> On 2023-08-25 11:24, Qing Zhao wrote:
>> Provide a new counted_by attribute to flexible array member field.
> 
> The obligatory "I can't ack the patch but here's a review" disclaimer :)
> 
>>
>> '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;
>> +    }
> 
> Applies only to struct fields.  OK.
> 
>> +  /* 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;
>> +    }
> 
> The struct field should also be an array.  OK.
> 
>> +  /* 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;
>> +    }
> 
> Additionally, the field should be a *flex* array.  OK.  Could this be 
> reworded to:
> 
>    %qE attribute only applies to C99 flexible array members
> 
> That would make it clear that the GNU extension flex arrays (i.e. any 
> last array member of a struct) don't support this attribute.
> 
>> +  /* 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;
>> +    }
> 
> Argument should be an identifier, and the check for validity of the 
> identifier comes later in finish_struct.  OK.
> 
>> +
>> +  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;
>> +}
>> +
> 
> Hoist flexible_array_member_type_p to use more widely.  OK.
> 
>>   /* 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);
> 
> Simple refactoring.  OK.
> 
>> @@ -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
> 
> Verify *that* the argument...
> 
>> +   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
> 
> s/attrbute/attribute/
> 
>> +     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)));
> 
> Probably someone with English as a first language would make a better 
> suggestion, but how about:
> 
>    Argument specified in %qE attribute is not a field declaration in the
>    same structure, ignoring it.
> 
>> +
>> +      DECL_ATTRIBUTES (field_decl)
>> +    = remove_attribute ("counted_by", DECL_ATTRIBUTES (field_decl));
>> +    }
>> +  else
>> +  /* Error when the field is not with an integer type.  */
> 
> Suggest: Flag an error when the field is not of 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)));
> 
> Suggest:
> 
>    Argument specified in %qE attribute is not of an integer type,
>    ignoring it.
> 
>> +
>> +      DECL_ATTRIBUTES (field_decl)
>> +        = remove_attribute ("counted_by", DECL_ATTRIBUTES (field_decl));
>> +    }
>> +    }
>> +
>> +  return;

I forgot to mention the redundant return here.

>> +}
>>   /* 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}.
> 
> Maybe specify somehow that this only applies to C99 flexible arrays?  Like:
> 
>    The @code{counted_by} attribute may be attached to the C99 flexible
>    array member of a structure...
> 
>> +
>> +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.
> 
> Suggest:
> 
>    The field that represents the number of the elements must 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.  The user is responsible to ensure that this
>    relationship is consistently maintained during the lifetime of the
>    object. Failure to do so may result in results of the array bound
>    sanitizer and the @code{__builtin_dynamic_object_size} being
>    undefined.
> 
>> +
>> +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
>> +
>> +
> 
> I'm unsure if we should be demonstrating example code with undefined 
> behaviour.  Maybe the documentation is better off without it.
> 
>>   @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" } */
>> +};
> 
> Tests look OK in principle, but may need updating for the error message.
> 
>> 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;
>> +}
> 
> Descending recursively into the field list of a struct to find an 
> identifier with a matching name.  OK.
> 
>> +
>> +
>>   /* 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 *);
>