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

[Siddhesh Poyarekar <siddhesh@gotplt.org>]

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;
> +}
>   
>   /* 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 *);
>