diff mbox series

[v10,2/5] Convert references with "counted_by" attributes to/from .ACCESS_WITH_SIZE.

Message ID 20240530122700.1516243-3-qing.zhao@oracle.com
State New
Headers show
Series New attribute "counted_by" to annotate bounds for C99 FAM(PR108896) | expand

Commit Message

Qing Zhao May 30, 2024, 12:26 p.m. UTC
Including the following changes:
* The definition of the new internal function .ACCESS_WITH_SIZE
  in internal-fn.def.
* C FE converts every reference to a FAM with a "counted_by" attribute
  to a call to the internal function .ACCESS_WITH_SIZE.
  (build_component_ref in c_typeck.cc)

  This includes the case when the object is statically allocated and
  initialized.
  In order to make this working, the routine digest_init in c-typeck.cc
  is updated to fold calls to .ACCESS_WITH_SIZE to its first argument
  when require_constant is TRUE.

  However, for the reference inside "offsetof", the "counted_by" attribute is
  ignored since it's not useful at all.
  (c_parser_postfix_expression in c/c-parser.cc)

  In addtion to "offsetof", for the reference inside operator "typeof" and
  "alignof", we ignore counted_by attribute too.

  When building ADDR_EXPR for the .ACCESS_WITH_SIZE in C FE,
  replace the call with its first argument.

* Convert every call to .ACCESS_WITH_SIZE to its first argument.
  (expand_ACCESS_WITH_SIZE in internal-fn.cc)
* Provide the utility routines to check the call is .ACCESS_WITH_SIZE and
  get the reference from the call to .ACCESS_WITH_SIZE.
  (is_access_with_size_p and get_ref_from_access_with_size in tree.cc)

gcc/c/ChangeLog:

	* c-parser.cc (c_parser_postfix_expression): Ignore the counted-by
	attribute when build_component_ref inside offsetof operator.
	* c-tree.h (build_component_ref): Add one more parameter.
	* c-typeck.cc (build_counted_by_ref): New function.
	(build_access_with_size_for_counted_by): New function.
	(build_component_ref): Check the counted-by attribute and build
	call to .ACCESS_WITH_SIZE.
	(build_unary_op): When building ADDR_EXPR for
        .ACCESS_WITH_SIZE, use its first argument.
        (lvalue_p): Accept call to .ACCESS_WITH_SIZE.
	(digest_init): Fold call to .ACCESS_WITH_SIZE to its first
	argument when require_constant is TRUE.

gcc/ChangeLog:

	* internal-fn.cc (expand_ACCESS_WITH_SIZE): New function.
	* internal-fn.def (ACCESS_WITH_SIZE): New internal function.
	* tree.cc (is_access_with_size_p): New function.
	(get_ref_from_access_with_size): New function.
	* tree.h (is_access_with_size_p): New prototype.
	(get_ref_from_access_with_size): New prototype.

gcc/testsuite/ChangeLog:

	* gcc.dg/flex-array-counted-by-2.c: New test.
---
 gcc/c/c-parser.cc                             |  10 +-
 gcc/c/c-tree.h                                |   2 +-
 gcc/c/c-typeck.cc                             | 142 +++++++++++++++++-
 gcc/internal-fn.cc                            |  34 +++++
 gcc/internal-fn.def                           |   5 +
 .../gcc.dg/flex-array-counted-by-2.c          | 112 ++++++++++++++
 gcc/tree.cc                                   |  22 +++
 gcc/tree.h                                    |   8 +
 8 files changed, 328 insertions(+), 7 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/flex-array-counted-by-2.c

Comments

Joseph Myers May 30, 2024, 7:43 p.m. UTC | #1
On Thu, 30 May 2024, Qing Zhao wrote:

>   In order to make this working, the routine digest_init in c-typeck.cc
>   is updated to fold calls to .ACCESS_WITH_SIZE to its first argument
>   when require_constant is TRUE.

The new changes here are OK.
Qing Zhao May 30, 2024, 8:03 p.m. UTC | #2
> On May 30, 2024, at 15:43, Joseph Myers <josmyers@redhat.com> wrote:
> 
> On Thu, 30 May 2024, Qing Zhao wrote:
> 
>>  In order to make this working, the routine digest_init in c-typeck.cc
>>  is updated to fold calls to .ACCESS_WITH_SIZE to its first argument
>>  when require_constant is TRUE.
> 
> The new changes here are OK.
Thanks.

Qing
> 
> -- 
> Joseph S. Myers
> josmyers@redhat.com
>
Richard Biener May 31, 2024, 12:58 p.m. UTC | #3
On Thu, 30 May 2024, Qing Zhao wrote:

> Including the following changes:
> * The definition of the new internal function .ACCESS_WITH_SIZE
>   in internal-fn.def.
> * C FE converts every reference to a FAM with a "counted_by" attribute
>   to a call to the internal function .ACCESS_WITH_SIZE.
>   (build_component_ref in c_typeck.cc)
> 
>   This includes the case when the object is statically allocated and
>   initialized.
>   In order to make this working, the routine digest_init in c-typeck.cc
>   is updated to fold calls to .ACCESS_WITH_SIZE to its first argument
>   when require_constant is TRUE.
> 
>   However, for the reference inside "offsetof", the "counted_by" attribute is
>   ignored since it's not useful at all.
>   (c_parser_postfix_expression in c/c-parser.cc)
> 
>   In addtion to "offsetof", for the reference inside operator "typeof" and
>   "alignof", we ignore counted_by attribute too.
> 
>   When building ADDR_EXPR for the .ACCESS_WITH_SIZE in C FE,
>   replace the call with its first argument.
> 
> * Convert every call to .ACCESS_WITH_SIZE to its first argument.
>   (expand_ACCESS_WITH_SIZE in internal-fn.cc)
> * Provide the utility routines to check the call is .ACCESS_WITH_SIZE and
>   get the reference from the call to .ACCESS_WITH_SIZE.
>   (is_access_with_size_p and get_ref_from_access_with_size in tree.cc)

The middle-end parts of this revised patch are OK.

Thanks,
Richard.

> gcc/c/ChangeLog:
> 
> 	* c-parser.cc (c_parser_postfix_expression): Ignore the counted-by
> 	attribute when build_component_ref inside offsetof operator.
> 	* c-tree.h (build_component_ref): Add one more parameter.
> 	* c-typeck.cc (build_counted_by_ref): New function.
> 	(build_access_with_size_for_counted_by): New function.
> 	(build_component_ref): Check the counted-by attribute and build
> 	call to .ACCESS_WITH_SIZE.
> 	(build_unary_op): When building ADDR_EXPR for
>         .ACCESS_WITH_SIZE, use its first argument.
>         (lvalue_p): Accept call to .ACCESS_WITH_SIZE.
> 	(digest_init): Fold call to .ACCESS_WITH_SIZE to its first
> 	argument when require_constant is TRUE.
> 
> gcc/ChangeLog:
> 
> 	* internal-fn.cc (expand_ACCESS_WITH_SIZE): New function.
> 	* internal-fn.def (ACCESS_WITH_SIZE): New internal function.
> 	* tree.cc (is_access_with_size_p): New function.
> 	(get_ref_from_access_with_size): New function.
> 	* tree.h (is_access_with_size_p): New prototype.
> 	(get_ref_from_access_with_size): New prototype.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.dg/flex-array-counted-by-2.c: New test.
> ---
>  gcc/c/c-parser.cc                             |  10 +-
>  gcc/c/c-tree.h                                |   2 +-
>  gcc/c/c-typeck.cc                             | 142 +++++++++++++++++-
>  gcc/internal-fn.cc                            |  34 +++++
>  gcc/internal-fn.def                           |   5 +
>  .../gcc.dg/flex-array-counted-by-2.c          | 112 ++++++++++++++
>  gcc/tree.cc                                   |  22 +++
>  gcc/tree.h                                    |   8 +
>  8 files changed, 328 insertions(+), 7 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/flex-array-counted-by-2.c
> 
> diff --git a/gcc/c/c-parser.cc b/gcc/c/c-parser.cc
> index 00f8bf4376e5..2d9e9c0969f0 100644
> --- a/gcc/c/c-parser.cc
> +++ b/gcc/c/c-parser.cc
> @@ -10848,9 +10848,12 @@ c_parser_postfix_expression (c_parser *parser)
>  	    if (c_parser_next_token_is (parser, CPP_NAME))
>  	      {
>  		c_token *comp_tok = c_parser_peek_token (parser);
> +		/* Ignore the counted_by attribute for reference inside
> +		   offsetof since the information is not useful at all.  */
>  		offsetof_ref
>  		  = build_component_ref (loc, offsetof_ref, comp_tok->value,
> -					 comp_tok->location, UNKNOWN_LOCATION);
> +					 comp_tok->location, UNKNOWN_LOCATION,
> +					 false);
>  		c_parser_consume_token (parser);
>  		while (c_parser_next_token_is (parser, CPP_DOT)
>  		       || c_parser_next_token_is (parser,
> @@ -10877,11 +10880,14 @@ c_parser_postfix_expression (c_parser *parser)
>  			    break;
>  			  }
>  			c_token *comp_tok = c_parser_peek_token (parser);
> +			/* Ignore the counted_by attribute for reference inside
> +			   offsetof since the information is not useful.  */
>  			offsetof_ref
>  			  = build_component_ref (loc, offsetof_ref,
>  						 comp_tok->value,
>  						 comp_tok->location,
> -						 UNKNOWN_LOCATION);
> +						 UNKNOWN_LOCATION,
> +						 false);
>  			c_parser_consume_token (parser);
>  		      }
>  		    else
> diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h
> index 531a7e8742e3..56a33b8156c6 100644
> --- a/gcc/c/c-tree.h
> +++ b/gcc/c/c-tree.h
> @@ -779,7 +779,7 @@ extern void mark_exp_read (tree);
>  extern tree composite_type (tree, tree);
>  extern tree lookup_field (const_tree, tree);
>  extern tree build_component_ref (location_t, tree, tree, location_t,
> -				 location_t);
> +				 location_t, bool = true);
>  extern tree build_array_ref (location_t, tree, tree);
>  extern tree build_omp_array_section (location_t, tree, tree, tree);
>  extern tree build_external_ref (location_t, tree, bool, tree *);
> diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
> index 2375953fdb62..0d9c7a34a0df 100644
> --- a/gcc/c/c-typeck.cc
> +++ b/gcc/c/c-typeck.cc
> @@ -2584,15 +2584,116 @@ should_suggest_deref_p (tree datum_type)
>      return false;
>  }
>  
> +/* For a SUBDATUM field of a structure or union DATUM, generate a REF to
> +   the object that represents its counted_by per the attribute counted_by
> +   attached to this field if it's a flexible array member field, otherwise
> +   return NULL_TREE.
> +   Set COUNTED_BY_TYPE to the TYPE of the counted_by field.
> +   For example, if:
> +
> +    struct P {
> +      int k;
> +      int x[] __attribute__ ((counted_by (k)));
> +    } *p;
> +
> +    for:
> +    p->x
> +
> +    the ref to the object that represents its element count will be:
> +
> +    &(p->k)
> +
> +*/
> +static tree
> +build_counted_by_ref (tree datum, tree subdatum, tree *counted_by_type)
> +{
> +  tree type = TREE_TYPE (datum);
> +  if (!c_flexible_array_member_type_p (TREE_TYPE (subdatum)))
> +    return NULL_TREE;
> +
> +  tree attr_counted_by = lookup_attribute ("counted_by",
> +					   DECL_ATTRIBUTES (subdatum));
> +  tree counted_by_ref = NULL_TREE;
> +  *counted_by_type = NULL_TREE;
> +  if (attr_counted_by)
> +    {
> +      tree field_id = TREE_VALUE (TREE_VALUE (attr_counted_by));
> +      counted_by_ref
> +	= build_component_ref (UNKNOWN_LOCATION,
> +			       datum, field_id,
> +			       UNKNOWN_LOCATION, UNKNOWN_LOCATION);
> +      counted_by_ref = build_fold_addr_expr (counted_by_ref);
> +
> +      /* Get the TYPE of the counted_by field.  */
> +      tree counted_by_field = lookup_field (type, field_id);
> +      gcc_assert (counted_by_field);
> +
> +      do
> +	{
> +	  *counted_by_type = TREE_TYPE (TREE_VALUE (counted_by_field));
> +	  counted_by_field = TREE_CHAIN (counted_by_field);
> +	}
> +      while (counted_by_field);
> +    }
> +  return counted_by_ref;
> +}
> +
> +/* Given a COMPONENT_REF REF with the location LOC, the corresponding
> +   COUNTED_BY_REF, and the COUNTED_BY_TYPE, generate an INDIRECT_REF
> +   to a call to the internal function .ACCESS_WITH_SIZE.
> +
> +   REF
> +
> +   to:
> +
> +   (*.ACCESS_WITH_SIZE (REF, COUNTED_BY_REF, 1, (TYPE_OF_SIZE)0, -1))
> +
> +   NOTE: The return type of this function is the POINTER type pointing
> +   to the original flexible array type.
> +   Then the type of the INDIRECT_REF is the original flexible array type.
> +
> +   The type of the first argument of this function is a POINTER type
> +   to the original flexible array type.
> +
> +   The 4th argument of the call is a constant 0 with the TYPE of the
> +   object pointed by COUNTED_BY_REF.
> +
> +  */
> +static tree
> +build_access_with_size_for_counted_by (location_t loc, tree ref,
> +				       tree counted_by_ref,
> +				       tree counted_by_type)
> +{
> +  gcc_assert (c_flexible_array_member_type_p (TREE_TYPE (ref)));
> +  /* The result type of the call is a pointer to the flexible array type.  */
> +  tree result_type = build_pointer_type (TREE_TYPE (ref));
> +
> +  tree call
> +    = build_call_expr_internal_loc (loc, IFN_ACCESS_WITH_SIZE,
> +				    result_type, 5,
> +				    array_to_pointer_conversion (loc, ref),
> +				    counted_by_ref,
> +				    build_int_cst (integer_type_node, 1),
> +				    build_int_cst (counted_by_type, 0),
> +				    build_int_cst (integer_type_node, -1));
> +  /* Wrap the call with an INDIRECT_REF with the flexible array type.  */
> +  call = build1 (INDIRECT_REF, TREE_TYPE (ref), call);
> +  SET_EXPR_LOCATION (call, loc);
> +  return call;
> +}
> +
>  /* Make an expression to refer to the COMPONENT field of structure or
>     union value DATUM.  COMPONENT is an IDENTIFIER_NODE.  LOC is the
>     location of the COMPONENT_REF.  COMPONENT_LOC is the location
>     of COMPONENT.  ARROW_LOC is the location of the first -> operand if
> -   it is from -> operator.  */
> +   it is from -> operator.
> +   If HANDLE_COUNTED_BY is true, check the counted_by attribute and generate
> +   a call to .ACCESS_WITH_SIZE.  Otherwise, ignore the attribute.  */
>  
>  tree
>  build_component_ref (location_t loc, tree datum, tree component,
> -		     location_t component_loc, location_t arrow_loc)
> +		     location_t component_loc, location_t arrow_loc,
> +		     bool handle_counted_by)
>  {
>    tree type = TREE_TYPE (datum);
>    enum tree_code code = TREE_CODE (type);
> @@ -2664,7 +2765,13 @@ build_component_ref (location_t loc, tree datum, tree component,
>  	  int quals;
>  	  tree subtype;
>  	  bool use_datum_quals;
> -
> +	  tree counted_by_type = NULL_TREE;
> +	  /* Do not handle counted_by when in typeof and alignof operator.  */
> +	  handle_counted_by = handle_counted_by && !in_typeof && !in_alignof;
> +	  tree counted_by_ref = handle_counted_by
> +				? build_counted_by_ref (datum, subdatum,
> +							&counted_by_type)
> +				: NULL_TREE;
>  	  if (TREE_TYPE (subdatum) == error_mark_node)
>  	    return error_mark_node;
>  
> @@ -2683,6 +2790,12 @@ build_component_ref (location_t loc, tree datum, tree component,
>  	  ref = build3 (COMPONENT_REF, subtype, datum, subdatum,
>  			NULL_TREE);
>  	  SET_EXPR_LOCATION (ref, loc);
> +
> +	  if (counted_by_ref)
> +	    ref = build_access_with_size_for_counted_by (loc, ref,
> +							 counted_by_ref,
> +							 counted_by_type);
> +
>  	  if (TREE_READONLY (subdatum)
>  	      || (use_datum_quals && TREE_READONLY (datum)))
>  	    TREE_READONLY (ref) = 1;
> @@ -5087,7 +5200,11 @@ build_unary_op (location_t location, enum tree_code code, tree xarg,
>  	  goto return_build_unary_op;
>  	}
>  
> -      /* Ordinary case; arg is a COMPONENT_REF or a decl.  */
> +      /* Ordinary case; arg is a COMPONENT_REF or a decl, or a call to
> +	 .ACCESS_WITH_SIZE.  */
> +      if (is_access_with_size_p (arg))
> +	arg = TREE_OPERAND (TREE_OPERAND (CALL_EXPR_ARG (arg, 0), 0), 0);
> +
>        argtype = TREE_TYPE (arg);
>  
>        /* If the lvalue is const or volatile, merge that into the type
> @@ -5238,6 +5355,9 @@ lvalue_p (const_tree ref)
>      case BIND_EXPR:
>        return TREE_CODE (TREE_TYPE (ref)) == ARRAY_TYPE;
>  
> +    case CALL_EXPR:
> +      return is_access_with_size_p (ref);
> +
>      default:
>        return false;
>      }
> @@ -8525,6 +8645,20 @@ digest_init (location_t init_loc, tree type, tree init, tree origtype,
>  
>    STRIP_TYPE_NOPS (inside_init);
>  
> +  /* If require_constant is TRUE,  when the initializer is a call to
> +     .ACCESS_WITH_SIZE, use the first argument as the initializer.
> +     For example:
> +     y = (char *) .ACCESS_WITH_SIZE ((char *) &static_annotated.c,...)
> +     will be converted to
> +     y = &static_annotated.c.  */
> +
> +  if (require_constant
> +      && TREE_CODE (inside_init) == NOP_EXPR
> +      && TREE_CODE (TREE_OPERAND (inside_init, 0)) == CALL_EXPR
> +      && is_access_with_size_p (TREE_OPERAND (inside_init, 0)))
> +    inside_init
> +      = get_ref_from_access_with_size (TREE_OPERAND (inside_init, 0));
> +
>    if (!c_in_omp_for)
>      {
>        if (TREE_CODE (inside_init) == EXCESS_PRECISION_EXPR)
> diff --git a/gcc/internal-fn.cc b/gcc/internal-fn.cc
> index 9c09026793fa..eb2c4cd59048 100644
> --- a/gcc/internal-fn.cc
> +++ b/gcc/internal-fn.cc
> @@ -3438,6 +3438,40 @@ expand_DEFERRED_INIT (internal_fn, gcall *stmt)
>      }
>  }
>  
> +/* Expand the IFN_ACCESS_WITH_SIZE function:
> +   ACCESS_WITH_SIZE (REF_TO_OBJ, REF_TO_SIZE, CLASS_OF_SIZE,
> +		     TYPE_OF_SIZE, ACCESS_MODE)
> +   which returns the REF_TO_OBJ same as the 1st argument;
> +
> +   1st argument REF_TO_OBJ: The reference to the object;
> +   2nd argument REF_TO_SIZE: The reference to the size of the object,
> +   3rd argument CLASS_OF_SIZE: The size referenced by the REF_TO_SIZE represents
> +     0: the number of bytes.
> +     1: the number of the elements of the object type;
> +   4th argument TYPE_OF_SIZE: A constant 0 with its TYPE being the same as the TYPE
> +    of the object referenced by REF_TO_SIZE
> +   5th argument ACCESS_MODE:
> +    -1: Unknown access semantics
> +     0: none
> +     1: read_only
> +     2: write_only
> +     3: read_write
> +
> +   Both the return type and the type of the first argument of this
> +   function have been converted from the incomplete array type to
> +   the corresponding pointer type.
> +
> +   For each call to a .ACCESS_WITH_SIZE, replace it with its 1st argument.  */
> +
> +static void
> +expand_ACCESS_WITH_SIZE (internal_fn, gcall *stmt)
> +{
> +  tree lhs = gimple_call_lhs (stmt);
> +  tree ref_to_obj = gimple_call_arg (stmt, 0);
> +  if (lhs)
> +    expand_assignment (lhs, ref_to_obj, false);
> +}
> +
>  /* The size of an OpenACC compute dimension.  */
>  
>  static void
> diff --git a/gcc/internal-fn.def b/gcc/internal-fn.def
> index 25badbb86e56..8de1fa882e95 100644
> --- a/gcc/internal-fn.def
> +++ b/gcc/internal-fn.def
> @@ -512,6 +512,11 @@ DEF_INTERNAL_FN (PHI, 0, NULL)
>     automatic variable.  */
>  DEF_INTERNAL_FN (DEFERRED_INIT, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL)
>  
> +/* A function to associate the access size and access mode information
> +   with the corresponding reference to an object.  It only reads from the
> +   2nd argument.  */
> +DEF_INTERNAL_FN (ACCESS_WITH_SIZE, ECF_PURE | ECF_LEAF | ECF_NOTHROW, NULL)
> +
>  /* DIM_SIZE and DIM_POS return the size of a particular compute
>     dimension and the executing thread's position within that
>     dimension.  DIM_POS is pure (and not const) so that it isn't
> diff --git a/gcc/testsuite/gcc.dg/flex-array-counted-by-2.c b/gcc/testsuite/gcc.dg/flex-array-counted-by-2.c
> new file mode 100644
> index 000000000000..d4899a63af3c
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/flex-array-counted-by-2.c
> @@ -0,0 +1,112 @@
> +/* Test the code generation for the new attribute counted_by.
> +   And also the offsetof operator on such array.  */
> +/* { dg-do run } */
> +/* { dg-options "-O2 -fdump-tree-original" } */
> +
> +#include <stdlib.h>
> +
> +struct annotated {
> +  int b;
> +  char c[] __attribute__ ((counted_by (b)));
> +} *array_annotated;
> +
> +static struct annotated static_annotated = { sizeof "hello", "hello" };
> +static char *y = static_annotated.c;
> +
> +struct flex {
> +  int b;
> +  char c[]; 
> +}; 
> +
> +struct nested_annotated {
> +  struct {
> +    union {
> +      int b;
> +      float f;	
> +    };
> +    int n;
> +  };
> +  char c[] __attribute__ ((counted_by (b)));
> +} *array_nested_annotated;
> +
> +static struct nested_annotated nested_static_annotated
> +				 = { sizeof "hello1", 0, "hello1" };
> +static char *nested_y = nested_static_annotated.c;
> +
> +struct nested_flex {
> +  struct {
> +    union {
> +      int b;
> +      float f;	
> +    };
> +    int n;
> +  };
> +  char c[];
> +};
> +
> +void __attribute__((__noinline__)) setup (int normal_count, int attr_count)
> +{
> +  array_annotated
> +    = (struct annotated *)malloc (sizeof (struct annotated)
> +				  + attr_count *  sizeof (char));
> +  array_annotated->b = attr_count;
> +
> +  array_nested_annotated
> +    = (struct nested_annotated *)malloc (sizeof (struct nested_annotated)
> +					 + attr_count *  sizeof (char));
> +  array_nested_annotated->b = attr_count;
> +
> +  return;
> +}
> +
> +void __attribute__((__noinline__)) test (char a, char b)
> +{
> +  if (__builtin_offsetof (struct annotated, c[0])
> +      != __builtin_offsetof (struct flex, c[0]))
> +    abort ();
> +  if (__builtin_offsetof (struct annotated, c[1])
> +      != __builtin_offsetof (struct flex, c[1]))
> +    abort ();
> +  if (__builtin_offsetof (struct nested_annotated, c[0]) 
> +      != __builtin_offsetof (struct nested_flex, c[0])) 
> +    abort ();
> +  if (__builtin_offsetof (struct nested_annotated, c[1]) 
> +      != __builtin_offsetof (struct nested_flex, c[1])) 
> +    abort ();
> +
> +  if (__builtin_types_compatible_p (typeof (array_annotated->c),
> +				    typeof (&(array_annotated->c)[0])))
> +    abort ();
> +  if (__builtin_types_compatible_p (typeof (array_nested_annotated->c),
> +				    typeof (&(array_nested_annotated->c)[0])))
> +    abort ();
> +
> +  if (__alignof (array_annotated->c) != __alignof (char))
> +    abort ();
> +  if (__alignof (array_nested_annotated->c) != __alignof (char))
> +    abort ();
> +
> +  if ((unsigned long) array_annotated->c != (unsigned long) &array_annotated->c)
> +    abort ();
> +  if ((unsigned long) array_nested_annotated->c
> +       != (unsigned long) &array_nested_annotated->c)
> +    abort ();
> +
> +  array_annotated->c[2] = a;
> +  array_nested_annotated->c[3] = b;
> +
> +  if (y[2] != 'l') abort ();
> +  if (nested_y[4] !='o') abort ();
> +
> +}
> +
> +int main(int argc, char *argv[])
> +{
> +  setup (10,10);   
> +  test ('A', 'B');
> +  if (array_annotated->c[2] != 'A') abort ();
> +  if (array_nested_annotated->c[3] != 'B') abort ();
> +  return 0;
> +}
> +
> +/* { dg-final { scan-tree-dump-times "ACCESS_WITH_SIZE" 8 "original" } } */
> diff --git a/gcc/tree.cc b/gcc/tree.cc
> index 6564b002dc1a..01572fe70f72 100644
> --- a/gcc/tree.cc
> +++ b/gcc/tree.cc
> @@ -13405,6 +13405,28 @@ component_ref_size (tree ref, special_array_member *sam /* = NULL */)
>  	  ? NULL_TREE : size_zero_node);
>  }
>  
> +/* Return true if the given node CALL is a call to a .ACCESS_WITH_SIZE
> +   function.  */
> +bool
> +is_access_with_size_p (const_tree call)
> +{
> +  if (TREE_CODE (call) != CALL_EXPR)
> +    return false;
> +  if (CALL_EXPR_IFN (call) == IFN_ACCESS_WITH_SIZE)
> +    return true;
> +  return false;
> +}
> +
> +/* Get the corresponding reference from the call to a .ACCESS_WITH_SIZE.
> + * i.e the first argument of this call.  Return NULL_TREE otherwise.  */
> +tree
> +get_ref_from_access_with_size (tree call)
> +{
> +  if (is_access_with_size_p (call))
> +    return  CALL_EXPR_ARG (call, 0);
> +  return NULL_TREE;
> +}
> +
>  /* Return the machine mode of T.  For vectors, returns the mode of the
>     inner type.  The main use case is to feed the result to HONOR_NANS,
>     avoiding the BLKmode that a direct TYPE_MODE (T) might return.  */
> diff --git a/gcc/tree.h b/gcc/tree.h
> index ee2aae332a41..604885641184 100644
> --- a/gcc/tree.h
> +++ b/gcc/tree.h
> @@ -5772,6 +5772,14 @@ extern special_array_member component_ref_sam_type (tree);
>     cannot be determined.  */
>  extern tree component_ref_size (tree, special_array_member * = NULL);
>  
> +/* Return true if the given node is a call to a .ACCESS_WITH_SIZE
> +   function.  */
> +extern bool is_access_with_size_p (const_tree);
> +
> +/* Get the corresponding reference from the call to a .ACCESS_WITH_SIZE,
> + * i.e. the first argument of this call.  Return NULL_TREE otherwise.  */
> +extern tree get_ref_from_access_with_size (tree);
> +
>  extern int tree_map_base_eq (const void *, const void *);
>  extern unsigned int tree_map_base_hash (const void *);
>  extern bool tree_map_base_marked_p (const void *);
>
Qing Zhao May 31, 2024, 1:11 p.m. UTC | #4
> On May 31, 2024, at 08:58, Richard Biener <rguenther@suse.de> wrote:
> 
> On Thu, 30 May 2024, Qing Zhao wrote:
> 
>> Including the following changes:
>> * The definition of the new internal function .ACCESS_WITH_SIZE
>>  in internal-fn.def.
>> * C FE converts every reference to a FAM with a "counted_by" attribute
>>  to a call to the internal function .ACCESS_WITH_SIZE.
>>  (build_component_ref in c_typeck.cc)
>> 
>>  This includes the case when the object is statically allocated and
>>  initialized.
>>  In order to make this working, the routine digest_init in c-typeck.cc
>>  is updated to fold calls to .ACCESS_WITH_SIZE to its first argument
>>  when require_constant is TRUE.
>> 
>>  However, for the reference inside "offsetof", the "counted_by" attribute is
>>  ignored since it's not useful at all.
>>  (c_parser_postfix_expression in c/c-parser.cc)
>> 
>>  In addtion to "offsetof", for the reference inside operator "typeof" and
>>  "alignof", we ignore counted_by attribute too.
>> 
>>  When building ADDR_EXPR for the .ACCESS_WITH_SIZE in C FE,
>>  replace the call with its first argument.
>> 
>> * Convert every call to .ACCESS_WITH_SIZE to its first argument.
>>  (expand_ACCESS_WITH_SIZE in internal-fn.cc)
>> * Provide the utility routines to check the call is .ACCESS_WITH_SIZE and
>>  get the reference from the call to .ACCESS_WITH_SIZE.
>>  (is_access_with_size_p and get_ref_from_access_with_size in tree.cc)
> 
> The middle-end parts of this revised patch are OK.

Thanks a lot for the review.
Will commit the patch set soon.

Qing
> 
> Thanks,
> Richard.
> 
>> gcc/c/ChangeLog:
>> 
>> * c-parser.cc (c_parser_postfix_expression): Ignore the counted-by
>> attribute when build_component_ref inside offsetof operator.
>> * c-tree.h (build_component_ref): Add one more parameter.
>> * c-typeck.cc (build_counted_by_ref): New function.
>> (build_access_with_size_for_counted_by): New function.
>> (build_component_ref): Check the counted-by attribute and build
>> call to .ACCESS_WITH_SIZE.
>> (build_unary_op): When building ADDR_EXPR for
>>        .ACCESS_WITH_SIZE, use its first argument.
>>        (lvalue_p): Accept call to .ACCESS_WITH_SIZE.
>> (digest_init): Fold call to .ACCESS_WITH_SIZE to its first
>> argument when require_constant is TRUE.
>> 
>> gcc/ChangeLog:
>> 
>> * internal-fn.cc (expand_ACCESS_WITH_SIZE): New function.
>> * internal-fn.def (ACCESS_WITH_SIZE): New internal function.
>> * tree.cc (is_access_with_size_p): New function.
>> (get_ref_from_access_with_size): New function.
>> * tree.h (is_access_with_size_p): New prototype.
>> (get_ref_from_access_with_size): New prototype.
>> 
>> gcc/testsuite/ChangeLog:
>> 
>> * gcc.dg/flex-array-counted-by-2.c: New test.
>> ---
>> gcc/c/c-parser.cc                             |  10 +-
>> gcc/c/c-tree.h                                |   2 +-
>> gcc/c/c-typeck.cc                             | 142 +++++++++++++++++-
>> gcc/internal-fn.cc                            |  34 +++++
>> gcc/internal-fn.def                           |   5 +
>> .../gcc.dg/flex-array-counted-by-2.c          | 112 ++++++++++++++
>> gcc/tree.cc                                   |  22 +++
>> gcc/tree.h                                    |   8 +
>> 8 files changed, 328 insertions(+), 7 deletions(-)
>> create mode 100644 gcc/testsuite/gcc.dg/flex-array-counted-by-2.c
>> 
>> diff --git a/gcc/c/c-parser.cc b/gcc/c/c-parser.cc
>> index 00f8bf4376e5..2d9e9c0969f0 100644
>> --- a/gcc/c/c-parser.cc
>> +++ b/gcc/c/c-parser.cc
>> @@ -10848,9 +10848,12 @@ c_parser_postfix_expression (c_parser *parser)
>>     if (c_parser_next_token_is (parser, CPP_NAME))
>>       {
>> c_token *comp_tok = c_parser_peek_token (parser);
>> + /* Ignore the counted_by attribute for reference inside
>> +    offsetof since the information is not useful at all.  */
>> offsetof_ref
>>   = build_component_ref (loc, offsetof_ref, comp_tok->value,
>> -  comp_tok->location, UNKNOWN_LOCATION);
>> +  comp_tok->location, UNKNOWN_LOCATION,
>> +  false);
>> c_parser_consume_token (parser);
>> while (c_parser_next_token_is (parser, CPP_DOT)
>>        || c_parser_next_token_is (parser,
>> @@ -10877,11 +10880,14 @@ c_parser_postfix_expression (c_parser *parser)
>>     break;
>>   }
>> c_token *comp_tok = c_parser_peek_token (parser);
>> + /* Ignore the counted_by attribute for reference inside
>> +    offsetof since the information is not useful.  */
>> offsetof_ref
>>   = build_component_ref (loc, offsetof_ref,
>>  comp_tok->value,
>>  comp_tok->location,
>> -  UNKNOWN_LOCATION);
>> +  UNKNOWN_LOCATION,
>> +  false);
>> c_parser_consume_token (parser);
>>       }
>>     else
>> diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h
>> index 531a7e8742e3..56a33b8156c6 100644
>> --- a/gcc/c/c-tree.h
>> +++ b/gcc/c/c-tree.h
>> @@ -779,7 +779,7 @@ extern void mark_exp_read (tree);
>> extern tree composite_type (tree, tree);
>> extern tree lookup_field (const_tree, tree);
>> extern tree build_component_ref (location_t, tree, tree, location_t,
>> -  location_t);
>> +  location_t, bool = true);
>> extern tree build_array_ref (location_t, tree, tree);
>> extern tree build_omp_array_section (location_t, tree, tree, tree);
>> extern tree build_external_ref (location_t, tree, bool, tree *);
>> diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
>> index 2375953fdb62..0d9c7a34a0df 100644
>> --- a/gcc/c/c-typeck.cc
>> +++ b/gcc/c/c-typeck.cc
>> @@ -2584,15 +2584,116 @@ should_suggest_deref_p (tree datum_type)
>>     return false;
>> }
>> 
>> +/* For a SUBDATUM field of a structure or union DATUM, generate a REF to
>> +   the object that represents its counted_by per the attribute counted_by
>> +   attached to this field if it's a flexible array member field, otherwise
>> +   return NULL_TREE.
>> +   Set COUNTED_BY_TYPE to the TYPE of the counted_by field.
>> +   For example, if:
>> +
>> +    struct P {
>> +      int k;
>> +      int x[] __attribute__ ((counted_by (k)));
>> +    } *p;
>> +
>> +    for:
>> +    p->x
>> +
>> +    the ref to the object that represents its element count will be:
>> +
>> +    &(p->k)
>> +
>> +*/
>> +static tree
>> +build_counted_by_ref (tree datum, tree subdatum, tree *counted_by_type)
>> +{
>> +  tree type = TREE_TYPE (datum);
>> +  if (!c_flexible_array_member_type_p (TREE_TYPE (subdatum)))
>> +    return NULL_TREE;
>> +
>> +  tree attr_counted_by = lookup_attribute ("counted_by",
>> +    DECL_ATTRIBUTES (subdatum));
>> +  tree counted_by_ref = NULL_TREE;
>> +  *counted_by_type = NULL_TREE;
>> +  if (attr_counted_by)
>> +    {
>> +      tree field_id = TREE_VALUE (TREE_VALUE (attr_counted_by));
>> +      counted_by_ref
>> + = build_component_ref (UNKNOWN_LOCATION,
>> +        datum, field_id,
>> +        UNKNOWN_LOCATION, UNKNOWN_LOCATION);
>> +      counted_by_ref = build_fold_addr_expr (counted_by_ref);
>> +
>> +      /* Get the TYPE of the counted_by field.  */
>> +      tree counted_by_field = lookup_field (type, field_id);
>> +      gcc_assert (counted_by_field);
>> +
>> +      do
>> + {
>> +   *counted_by_type = TREE_TYPE (TREE_VALUE (counted_by_field));
>> +   counted_by_field = TREE_CHAIN (counted_by_field);
>> + }
>> +      while (counted_by_field);
>> +    }
>> +  return counted_by_ref;
>> +}
>> +
>> +/* Given a COMPONENT_REF REF with the location LOC, the corresponding
>> +   COUNTED_BY_REF, and the COUNTED_BY_TYPE, generate an INDIRECT_REF
>> +   to a call to the internal function .ACCESS_WITH_SIZE.
>> +
>> +   REF
>> +
>> +   to:
>> +
>> +   (*.ACCESS_WITH_SIZE (REF, COUNTED_BY_REF, 1, (TYPE_OF_SIZE)0, -1))
>> +
>> +   NOTE: The return type of this function is the POINTER type pointing
>> +   to the original flexible array type.
>> +   Then the type of the INDIRECT_REF is the original flexible array type.
>> +
>> +   The type of the first argument of this function is a POINTER type
>> +   to the original flexible array type.
>> +
>> +   The 4th argument of the call is a constant 0 with the TYPE of the
>> +   object pointed by COUNTED_BY_REF.
>> +
>> +  */
>> +static tree
>> +build_access_with_size_for_counted_by (location_t loc, tree ref,
>> +        tree counted_by_ref,
>> +        tree counted_by_type)
>> +{
>> +  gcc_assert (c_flexible_array_member_type_p (TREE_TYPE (ref)));
>> +  /* The result type of the call is a pointer to the flexible array type.  */
>> +  tree result_type = build_pointer_type (TREE_TYPE (ref));
>> +
>> +  tree call
>> +    = build_call_expr_internal_loc (loc, IFN_ACCESS_WITH_SIZE,
>> +     result_type, 5,
>> +     array_to_pointer_conversion (loc, ref),
>> +     counted_by_ref,
>> +     build_int_cst (integer_type_node, 1),
>> +     build_int_cst (counted_by_type, 0),
>> +     build_int_cst (integer_type_node, -1));
>> +  /* Wrap the call with an INDIRECT_REF with the flexible array type.  */
>> +  call = build1 (INDIRECT_REF, TREE_TYPE (ref), call);
>> +  SET_EXPR_LOCATION (call, loc);
>> +  return call;
>> +}
>> +
>> /* Make an expression to refer to the COMPONENT field of structure or
>>    union value DATUM.  COMPONENT is an IDENTIFIER_NODE.  LOC is the
>>    location of the COMPONENT_REF.  COMPONENT_LOC is the location
>>    of COMPONENT.  ARROW_LOC is the location of the first -> operand if
>> -   it is from -> operator.  */
>> +   it is from -> operator.
>> +   If HANDLE_COUNTED_BY is true, check the counted_by attribute and generate
>> +   a call to .ACCESS_WITH_SIZE.  Otherwise, ignore the attribute.  */
>> 
>> tree
>> build_component_ref (location_t loc, tree datum, tree component,
>> -      location_t component_loc, location_t arrow_loc)
>> +      location_t component_loc, location_t arrow_loc,
>> +      bool handle_counted_by)
>> {
>>   tree type = TREE_TYPE (datum);
>>   enum tree_code code = TREE_CODE (type);
>> @@ -2664,7 +2765,13 @@ build_component_ref (location_t loc, tree datum, tree component,
>>   int quals;
>>   tree subtype;
>>   bool use_datum_quals;
>> -
>> +   tree counted_by_type = NULL_TREE;
>> +   /* Do not handle counted_by when in typeof and alignof operator.  */
>> +   handle_counted_by = handle_counted_by && !in_typeof && !in_alignof;
>> +   tree counted_by_ref = handle_counted_by
>> + ? build_counted_by_ref (datum, subdatum,
>> + &counted_by_type)
>> + : NULL_TREE;
>>   if (TREE_TYPE (subdatum) == error_mark_node)
>>     return error_mark_node;
>> 
>> @@ -2683,6 +2790,12 @@ build_component_ref (location_t loc, tree datum, tree component,
>>   ref = build3 (COMPONENT_REF, subtype, datum, subdatum,
>> NULL_TREE);
>>   SET_EXPR_LOCATION (ref, loc);
>> +
>> +   if (counted_by_ref)
>> +     ref = build_access_with_size_for_counted_by (loc, ref,
>> +  counted_by_ref,
>> +  counted_by_type);
>> +
>>   if (TREE_READONLY (subdatum)
>>       || (use_datum_quals && TREE_READONLY (datum)))
>>     TREE_READONLY (ref) = 1;
>> @@ -5087,7 +5200,11 @@ build_unary_op (location_t location, enum tree_code code, tree xarg,
>>   goto return_build_unary_op;
>> }
>> 
>> -      /* Ordinary case; arg is a COMPONENT_REF or a decl.  */
>> +      /* Ordinary case; arg is a COMPONENT_REF or a decl, or a call to
>> +  .ACCESS_WITH_SIZE.  */
>> +      if (is_access_with_size_p (arg))
>> + arg = TREE_OPERAND (TREE_OPERAND (CALL_EXPR_ARG (arg, 0), 0), 0);
>> +
>>       argtype = TREE_TYPE (arg);
>> 
>>       /* If the lvalue is const or volatile, merge that into the type
>> @@ -5238,6 +5355,9 @@ lvalue_p (const_tree ref)
>>     case BIND_EXPR:
>>       return TREE_CODE (TREE_TYPE (ref)) == ARRAY_TYPE;
>> 
>> +    case CALL_EXPR:
>> +      return is_access_with_size_p (ref);
>> +
>>     default:
>>       return false;
>>     }
>> @@ -8525,6 +8645,20 @@ digest_init (location_t init_loc, tree type, tree init, tree origtype,
>> 
>>   STRIP_TYPE_NOPS (inside_init);
>> 
>> +  /* If require_constant is TRUE,  when the initializer is a call to
>> +     .ACCESS_WITH_SIZE, use the first argument as the initializer.
>> +     For example:
>> +     y = (char *) .ACCESS_WITH_SIZE ((char *) &static_annotated.c,...)
>> +     will be converted to
>> +     y = &static_annotated.c.  */
>> +
>> +  if (require_constant
>> +      && TREE_CODE (inside_init) == NOP_EXPR
>> +      && TREE_CODE (TREE_OPERAND (inside_init, 0)) == CALL_EXPR
>> +      && is_access_with_size_p (TREE_OPERAND (inside_init, 0)))
>> +    inside_init
>> +      = get_ref_from_access_with_size (TREE_OPERAND (inside_init, 0));
>> +
>>   if (!c_in_omp_for)
>>     {
>>       if (TREE_CODE (inside_init) == EXCESS_PRECISION_EXPR)
>> diff --git a/gcc/internal-fn.cc b/gcc/internal-fn.cc
>> index 9c09026793fa..eb2c4cd59048 100644
>> --- a/gcc/internal-fn.cc
>> +++ b/gcc/internal-fn.cc
>> @@ -3438,6 +3438,40 @@ expand_DEFERRED_INIT (internal_fn, gcall *stmt)
>>     }
>> }
>> 
>> +/* Expand the IFN_ACCESS_WITH_SIZE function:
>> +   ACCESS_WITH_SIZE (REF_TO_OBJ, REF_TO_SIZE, CLASS_OF_SIZE,
>> +      TYPE_OF_SIZE, ACCESS_MODE)
>> +   which returns the REF_TO_OBJ same as the 1st argument;
>> +
>> +   1st argument REF_TO_OBJ: The reference to the object;
>> +   2nd argument REF_TO_SIZE: The reference to the size of the object,
>> +   3rd argument CLASS_OF_SIZE: The size referenced by the REF_TO_SIZE represents
>> +     0: the number of bytes.
>> +     1: the number of the elements of the object type;
>> +   4th argument TYPE_OF_SIZE: A constant 0 with its TYPE being the same as the TYPE
>> +    of the object referenced by REF_TO_SIZE
>> +   5th argument ACCESS_MODE:
>> +    -1: Unknown access semantics
>> +     0: none
>> +     1: read_only
>> +     2: write_only
>> +     3: read_write
>> +
>> +   Both the return type and the type of the first argument of this
>> +   function have been converted from the incomplete array type to
>> +   the corresponding pointer type.
>> +
>> +   For each call to a .ACCESS_WITH_SIZE, replace it with its 1st argument.  */
>> +
>> +static void
>> +expand_ACCESS_WITH_SIZE (internal_fn, gcall *stmt)
>> +{
>> +  tree lhs = gimple_call_lhs (stmt);
>> +  tree ref_to_obj = gimple_call_arg (stmt, 0);
>> +  if (lhs)
>> +    expand_assignment (lhs, ref_to_obj, false);
>> +}
>> +
>> /* The size of an OpenACC compute dimension.  */
>> 
>> static void
>> diff --git a/gcc/internal-fn.def b/gcc/internal-fn.def
>> index 25badbb86e56..8de1fa882e95 100644
>> --- a/gcc/internal-fn.def
>> +++ b/gcc/internal-fn.def
>> @@ -512,6 +512,11 @@ DEF_INTERNAL_FN (PHI, 0, NULL)
>>    automatic variable.  */
>> DEF_INTERNAL_FN (DEFERRED_INIT, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL)
>> 
>> +/* A function to associate the access size and access mode information
>> +   with the corresponding reference to an object.  It only reads from the
>> +   2nd argument.  */
>> +DEF_INTERNAL_FN (ACCESS_WITH_SIZE, ECF_PURE | ECF_LEAF | ECF_NOTHROW, NULL)
>> +
>> /* DIM_SIZE and DIM_POS return the size of a particular compute
>>    dimension and the executing thread's position within that
>>    dimension.  DIM_POS is pure (and not const) so that it isn't
>> diff --git a/gcc/testsuite/gcc.dg/flex-array-counted-by-2.c b/gcc/testsuite/gcc.dg/flex-array-counted-by-2.c
>> new file mode 100644
>> index 000000000000..d4899a63af3c
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/flex-array-counted-by-2.c
>> @@ -0,0 +1,112 @@
>> +/* Test the code generation for the new attribute counted_by.
>> +   And also the offsetof operator on such array.  */
>> +/* { dg-do run } */
>> +/* { dg-options "-O2 -fdump-tree-original" } */
>> +
>> +#include <stdlib.h>
>> +
>> +struct annotated {
>> +  int b;
>> +  char c[] __attribute__ ((counted_by (b)));
>> +} *array_annotated;
>> +
>> +static struct annotated static_annotated = { sizeof "hello", "hello" };
>> +static char *y = static_annotated.c;
>> +
>> +struct flex {
>> +  int b;
>> +  char c[]; 
>> +}; 
>> +
>> +struct nested_annotated {
>> +  struct {
>> +    union {
>> +      int b;
>> +      float f; 
>> +    };
>> +    int n;
>> +  };
>> +  char c[] __attribute__ ((counted_by (b)));
>> +} *array_nested_annotated;
>> +
>> +static struct nested_annotated nested_static_annotated
>> +  = { sizeof "hello1", 0, "hello1" };
>> +static char *nested_y = nested_static_annotated.c;
>> +
>> +struct nested_flex {
>> +  struct {
>> +    union {
>> +      int b;
>> +      float f; 
>> +    };
>> +    int n;
>> +  };
>> +  char c[];
>> +};
>> +
>> +void __attribute__((__noinline__)) setup (int normal_count, int attr_count)
>> +{
>> +  array_annotated
>> +    = (struct annotated *)malloc (sizeof (struct annotated)
>> +   + attr_count *  sizeof (char));
>> +  array_annotated->b = attr_count;
>> +
>> +  array_nested_annotated
>> +    = (struct nested_annotated *)malloc (sizeof (struct nested_annotated)
>> +  + attr_count *  sizeof (char));
>> +  array_nested_annotated->b = attr_count;
>> +
>> +  return;
>> +}
>> +
>> +void __attribute__((__noinline__)) test (char a, char b)
>> +{
>> +  if (__builtin_offsetof (struct annotated, c[0])
>> +      != __builtin_offsetof (struct flex, c[0]))
>> +    abort ();
>> +  if (__builtin_offsetof (struct annotated, c[1])
>> +      != __builtin_offsetof (struct flex, c[1]))
>> +    abort ();
>> +  if (__builtin_offsetof (struct nested_annotated, c[0]) 
>> +      != __builtin_offsetof (struct nested_flex, c[0])) 
>> +    abort ();
>> +  if (__builtin_offsetof (struct nested_annotated, c[1]) 
>> +      != __builtin_offsetof (struct nested_flex, c[1])) 
>> +    abort ();
>> +
>> +  if (__builtin_types_compatible_p (typeof (array_annotated->c),
>> +     typeof (&(array_annotated->c)[0])))
>> +    abort ();
>> +  if (__builtin_types_compatible_p (typeof (array_nested_annotated->c),
>> +     typeof (&(array_nested_annotated->c)[0])))
>> +    abort ();
>> +
>> +  if (__alignof (array_annotated->c) != __alignof (char))
>> +    abort ();
>> +  if (__alignof (array_nested_annotated->c) != __alignof (char))
>> +    abort ();
>> +
>> +  if ((unsigned long) array_annotated->c != (unsigned long) &array_annotated->c)
>> +    abort ();
>> +  if ((unsigned long) array_nested_annotated->c
>> +       != (unsigned long) &array_nested_annotated->c)
>> +    abort ();
>> +
>> +  array_annotated->c[2] = a;
>> +  array_nested_annotated->c[3] = b;
>> +
>> +  if (y[2] != 'l') abort ();
>> +  if (nested_y[4] !='o') abort ();
>> +
>> +}
>> +
>> +int main(int argc, char *argv[])
>> +{
>> +  setup (10,10);   
>> +  test ('A', 'B');
>> +  if (array_annotated->c[2] != 'A') abort ();
>> +  if (array_nested_annotated->c[3] != 'B') abort ();
>> +  return 0;
>> +}
>> +
>> +/* { dg-final { scan-tree-dump-times "ACCESS_WITH_SIZE" 8 "original" } } */
>> diff --git a/gcc/tree.cc b/gcc/tree.cc
>> index 6564b002dc1a..01572fe70f72 100644
>> --- a/gcc/tree.cc
>> +++ b/gcc/tree.cc
>> @@ -13405,6 +13405,28 @@ component_ref_size (tree ref, special_array_member *sam /* = NULL */)
>>   ? NULL_TREE : size_zero_node);
>> }
>> 
>> +/* Return true if the given node CALL is a call to a .ACCESS_WITH_SIZE
>> +   function.  */
>> +bool
>> +is_access_with_size_p (const_tree call)
>> +{
>> +  if (TREE_CODE (call) != CALL_EXPR)
>> +    return false;
>> +  if (CALL_EXPR_IFN (call) == IFN_ACCESS_WITH_SIZE)
>> +    return true;
>> +  return false;
>> +}
>> +
>> +/* Get the corresponding reference from the call to a .ACCESS_WITH_SIZE.
>> + * i.e the first argument of this call.  Return NULL_TREE otherwise.  */
>> +tree
>> +get_ref_from_access_with_size (tree call)
>> +{
>> +  if (is_access_with_size_p (call))
>> +    return  CALL_EXPR_ARG (call, 0);
>> +  return NULL_TREE;
>> +}
>> +
>> /* Return the machine mode of T.  For vectors, returns the mode of the
>>    inner type.  The main use case is to feed the result to HONOR_NANS,
>>    avoiding the BLKmode that a direct TYPE_MODE (T) might return.  */
>> diff --git a/gcc/tree.h b/gcc/tree.h
>> index ee2aae332a41..604885641184 100644
>> --- a/gcc/tree.h
>> +++ b/gcc/tree.h
>> @@ -5772,6 +5772,14 @@ extern special_array_member component_ref_sam_type (tree);
>>    cannot be determined.  */
>> extern tree component_ref_size (tree, special_array_member * = NULL);
>> 
>> +/* Return true if the given node is a call to a .ACCESS_WITH_SIZE
>> +   function.  */
>> +extern bool is_access_with_size_p (const_tree);
>> +
>> +/* Get the corresponding reference from the call to a .ACCESS_WITH_SIZE,
>> + * i.e. the first argument of this call.  Return NULL_TREE otherwise.  */
>> +extern tree get_ref_from_access_with_size (tree);
>> +
>> extern int tree_map_base_eq (const void *, const void *);
>> extern unsigned int tree_map_base_hash (const void *);
>> extern bool tree_map_base_marked_p (const void *);
>> 
> 
> -- 
> Richard Biener <rguenther@suse.de>
> SUSE Software Solutions Germany GmbH,
> Frankenstrasse 146, 90461 Nuernberg, Germany;
> GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
David Malcolm June 4, 2024, 9:55 p.m. UTC | #5
On Fri, 2024-05-31 at 13:11 +0000, Qing Zhao wrote:
> 
> 
> > On May 31, 2024, at 08:58, Richard Biener <rguenther@suse.de>
> > wrote:
> > 
> > On Thu, 30 May 2024, Qing Zhao wrote:
> > 
> > > Including the following changes:
> > > * The definition of the new internal function .ACCESS_WITH_SIZE
> > >  in internal-fn.def.
> > > * C FE converts every reference to a FAM with a "counted_by"
> > > attribute
> > >  to a call to the internal function .ACCESS_WITH_SIZE.
> > >  (build_component_ref in c_typeck.cc)
> > > 
> > >  This includes the case when the object is statically allocated
> > > and
> > >  initialized.
> > >  In order to make this working, the routine digest_init in c-
> > > typeck.cc
> > >  is updated to fold calls to .ACCESS_WITH_SIZE to its first
> > > argument
> > >  when require_constant is TRUE.
> > > 
> > >  However, for the reference inside "offsetof", the "counted_by"
> > > attribute is
> > >  ignored since it's not useful at all.
> > >  (c_parser_postfix_expression in c/c-parser.cc)
> > > 
> > >  In addtion to "offsetof", for the reference inside operator
> > > "typeof" and
> > >  "alignof", we ignore counted_by attribute too.
> > > 
> > >  When building ADDR_EXPR for the .ACCESS_WITH_SIZE in C FE,
> > >  replace the call with its first argument.
> > > 
> > > * Convert every call to .ACCESS_WITH_SIZE to its first argument.
> > >  (expand_ACCESS_WITH_SIZE in internal-fn.cc)
> > > * Provide the utility routines to check the call is
> > > .ACCESS_WITH_SIZE and
> > >  get the reference from the call to .ACCESS_WITH_SIZE.
> > >  (is_access_with_size_p and get_ref_from_access_with_size in
> > > tree.cc)
> > 
> > The middle-end parts of this revised patch are OK.
> 
> Thanks a lot for the review.
> Will commit the patch set soon.

[...snip...]

Congratulations on getting this merged.

FWIW I've started investigating adding support for the new attribute to
-fanalyzer (and am tracked this as PR analyzer/111567
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111567 ).

The docs for the attribute speak of the implied relationship between
the count field and size of the flex array, and say that: "It's the
user's responsibility to make sure the above requirements to be kept
all the time.  Otherwise the compiler *reports warnings*, at the same
time, the results of the array bound sanitizer and the
'__builtin_dynamic_object_size' is undefined." (my emphasis).

What are these warnings that are reported?  I looked through 
r15-944-gf824acd0e80754 through r15-948-g4c5bea7def1361 and I didn't
see any new warnings or test coverage for warnings (beyond misuing the
attribute).  Sorry if I'm missing something obvious here.

Does anyone have examples of cases that -fanalyzer ought to warn for?
Presumably it would be helpful for the analyzer to report about code
paths in which the requirements are violated (but it may be that the
analyzer runs too late to do this...)

Thanks
Dave
Qing Zhao June 4, 2024, 10:09 p.m. UTC | #6
> On Jun 4, 2024, at 17:55, David Malcolm <dmalcolm@redhat.com> wrote:
> 
> On Fri, 2024-05-31 at 13:11 +0000, Qing Zhao wrote:
>> 
>> 
>>> On May 31, 2024, at 08:58, Richard Biener <rguenther@suse.de>
>>> wrote:
>>> 
>>> On Thu, 30 May 2024, Qing Zhao wrote:
>>> 
>>>> Including the following changes:
>>>> * The definition of the new internal function .ACCESS_WITH_SIZE
>>>>  in internal-fn.def.
>>>> * C FE converts every reference to a FAM with a "counted_by"
>>>> attribute
>>>>  to a call to the internal function .ACCESS_WITH_SIZE.
>>>>  (build_component_ref in c_typeck.cc)
>>>> 
>>>>  This includes the case when the object is statically allocated
>>>> and
>>>>  initialized.
>>>>  In order to make this working, the routine digest_init in c-
>>>> typeck.cc
>>>>  is updated to fold calls to .ACCESS_WITH_SIZE to its first
>>>> argument
>>>>  when require_constant is TRUE.
>>>> 
>>>>  However, for the reference inside "offsetof", the "counted_by"
>>>> attribute is
>>>>  ignored since it's not useful at all.
>>>>  (c_parser_postfix_expression in c/c-parser.cc)
>>>> 
>>>>  In addtion to "offsetof", for the reference inside operator
>>>> "typeof" and
>>>>  "alignof", we ignore counted_by attribute too.
>>>> 
>>>>  When building ADDR_EXPR for the .ACCESS_WITH_SIZE in C FE,
>>>>  replace the call with its first argument.
>>>> 
>>>> * Convert every call to .ACCESS_WITH_SIZE to its first argument.
>>>>  (expand_ACCESS_WITH_SIZE in internal-fn.cc)
>>>> * Provide the utility routines to check the call is
>>>> .ACCESS_WITH_SIZE and
>>>>  get the reference from the call to .ACCESS_WITH_SIZE.
>>>>  (is_access_with_size_p and get_ref_from_access_with_size in
>>>> tree.cc)
>>> 
>>> The middle-end parts of this revised patch are OK.
>> 
>> Thanks a lot for the review.
>> Will commit the patch set soon.
> 
> [...snip...]
> 
> Congratulations on getting this merged.
> 
> FWIW I've started investigating adding support for the new attribute to
> -fanalyzer (and am tracked this as PR analyzer/111567
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111567 ).

Thank you for starting looking at this.
> 
> The docs for the attribute speak of the implied relationship between
> the count field and size of the flex array, and say that: "It's the
> user's responsibility to make sure the above requirements to be kept
> all the time.  Otherwise the compiler *reports warnings*, at the same
> time, the results of the array bound sanitizer and the
> '__builtin_dynamic_object_size' is undefined." (my emphasis).
> 
> What are these warnings that are reported?  I looked through 
> r15-944-gf824acd0e80754 through r15-948-g4c5bea7def1361 and I didn't
> see any new warnings or test coverage for warnings (beyond misuing the
> attribute).  Sorry if I'm missing something obvious here.

These warnings will be in the remaining work (I listed the remaining work in all versions except the last one):

>>>> ******Remaining works: 
>>>> 
>>>> 6  Improve __bdos to use the counted_by info in whole-object size for the structure with FAM.
>>>> 7  Emit warnings when the user breaks the requirments for the new counted_by attribute
>>>> compilation time: -Wcounted-by
>>>> run time: -fsanitizer=counted-by
>>>>    * The initialization to the size field should be done before the first reference to the FAM field.
>>>>    * the array has at least # of elements specified by the size field all the time during the program.

With the current patches that have been committed, the warnings are not emitted. 
I believe that more analysis and more information are needed for these warnings to be effective, it might not
be a trivial patch.  More discussion is needed for emitting such warnings.

> 
> Does anyone have examples of cases that -fanalyzer ought to warn for?

At this moment, I don’t have concrete testing cases for this yet, but I can come up with several small examples and share with you in a later email.

Qing
> Presumably it would be helpful for the analyzer to report about code
> paths in which the requirements are violated (but it may be that the
> analyzer runs too late to do this...)
> 
> Thanks
> Dave
>
David Malcolm June 5, 2024, 1:49 p.m. UTC | #7
On Tue, 2024-06-04 at 22:09 +0000, Qing Zhao wrote:
> 
> 
> > On Jun 4, 2024, at 17:55, David Malcolm <dmalcolm@redhat.com>
> > wrote:
> > 
> > On Fri, 2024-05-31 at 13:11 +0000, Qing Zhao wrote:
> > > 
> > > 

[...]

> > > 
> > > 
> > > Thanks a lot for the review.
> > > Will commit the patch set soon.
> > 
> > [...snip...]
> > 
> > Congratulations on getting this merged.
> > 
> > FWIW I've started investigating adding support for the new
> > attribute to
> > -fanalyzer (and am tracked this as PR analyzer/111567
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111567 ).
> 
> Thank you for starting looking at this.
> > 
> > The docs for the attribute speak of the implied relationship
> > between
> > the count field and size of the flex array, and say that: "It's the
> > user's responsibility to make sure the above requirements to be
> > kept
> > all the time.  Otherwise the compiler *reports warnings*, at the
> > same
> > time, the results of the array bound sanitizer and the
> > '__builtin_dynamic_object_size' is undefined." (my emphasis).
> > 
> > What are these warnings that are reported?  I looked through 
> > r15-944-gf824acd0e80754 through r15-948-g4c5bea7def1361 and I
> > didn't
> > see any new warnings or test coverage for warnings (beyond misuing
> > the
> > attribute).  Sorry if I'm missing something obvious here.
> 
> These warnings will be in the remaining work (I listed the remaining
> work in all versions except the last one):
> 
> > > > > ******Remaining works: 
> > > > > 
> > > > > 6  Improve __bdos to use the counted_by info in whole-object
> > > > > size for the structure with FAM.
> > > > > 7  Emit warnings when the user breaks the requirments for the
> > > > > new counted_by attribute
> > > > > compilation time: -Wcounted-by
> > > > > run time: -fsanitizer=counted-by
> > > > >    * The initialization to the size field should be done
> > > > > before the first reference to the FAM field.
> > > > >    * the array has at least # of elements specified by the
> > > > > size field all the time during the program.

Aha - thanks.  Sorry for missing this, I confess I haven't been paying
close attention to this thread.

> 
> With the current patches that have been committed, the warnings are
> not emitted. 
> I believe that more analysis and more information are needed for
> these warnings to be effective, it might not
> be a trivial patch.  More discussion is needed for emitting such
> warnings.
> 
> > 
> > Does anyone have examples of cases that -fanalyzer ought to warn
> > for?
> 
> At this moment, I don’t have concrete testing cases for this yet, but
> I can come up with several small examples and share with you in a
> later email.

FWIW I did some brainstorming and put together the following .c file,
am posting it inline here for the sake of discussion; does this look
like the kind of thing to test for (in terms of how users are expected
to use the attribute, and the kinds of mistake they'd want warnings
about) ?

/* TODO:
   Some ideas for dimensions of test matrix:
   (a) concrete value vs symbolic value for "count"
   (b) concrete value vs symbolic value for size of array
   (c) dynamic vs static allocation of buffer (and malloc vs alloca)
   (d) relative size of array and of count
       - same size (not an issue)
       - array is too small compared to "count"
         - off by one
	 - off by more than one
	 - size is zero (but not negative)
         - negative size (which the docs say is OK)
       - array is too large compared to "count" (not an issue)
   (e) type of flex array:
       - char
       - non-char
       - type requiring padding
   (f) type/size/signedness of count field; what about overflow
       in (count * sizeof (type of array element)) ?
   ... etc: ideas?

    Other ideas for test coverage:
    - realloc
      - growing object
      - shrinking object
      - symbolic sizes where could be growth or shrinkage
      - failing realloc
    - ...etc: ideas?  */

#include <stddef.h>
#include <stdlib.h>
#include <stdint.h>

/* Example from the docs.  */

struct P {
  size_t count;
  char other;
  char array[] __attribute__ ((counted_by (count)));
} *p;

struct P *
test_malloc_with_correct_symbolic (size_t n)
{
  struct P *p = malloc (sizeof (struct P) + n);
  if (!p)
    return NULL;
  p->count = n; // don't warn here
  return p;  
}

struct P *
test_malloc_with_correct_count_concrete (void)
{
  struct P *p = malloc (sizeof (struct P) + 42);
  if (!p)
    return NULL;
  p->count = 42; // don't warn here
  return p;  
}

struct P *
test_malloc_with_array_smaller_than_count_concrete (void)
{
  struct P *p = malloc (sizeof (struct P) + 42);
  if (!p)
    return NULL;
  p->count = 80; // TODO: warn here
  return p;
}

struct P *
test_malloc_with_array_larger_than_count_concrete (void)
{
  struct P *p = malloc (sizeof (struct P) + 80);
  if (!p)
    return NULL;
  p->count = 42; // don't warn here
  return p;  
}

struct P *
test_malloc_with_array_access_before_count_init_concrete_1 (void)
{
  struct P *p = malloc (sizeof (struct P) + 42);
  if (!p)
    return NULL;
  /* Forgetting to set count altogether.  */
  __builtin_memset (p->array, 0, 42); // TODO: should warn here
  return p;  
}

struct P *
test_malloc_with_array_access_before_count_init_concrete_2 (void)
{
  struct P *p = malloc (sizeof (struct P) + 42);
  if (!p)
    return NULL;
  /* Erroneously touching array before setting count.  */
  __builtin_memset (p->array, 0, 42); // TODO: should warn here
  p->count = 42;
  return p;  
}

struct P *
test_malloc_with_array_access_before_count_init_symbolic_1 (size_t n)
{
  struct P *p = malloc (sizeof (struct P) + n);
  if (!p)
    return NULL;
  /* Forgetting to set count altogether.  */
  __builtin_memset (p->array, 0, n); // TODO: should warn here
  return p;  
}

struct P *
test_malloc_with_array_access_before_count_init_symbolic_2 (size_t n)
{
  struct P *p = malloc (sizeof (struct P) + n);
  if (!p)
    return NULL;
  /* Erroneously touching array before setting count.  */
  __builtin_memset (p->array, 0, n); // TODO: should warn here
  p->count = n;
  return p;  
}

/* Example where sizeof array element != 1.  */

struct Q
{
  size_t count;
  int32_t array[] __attribute__ ((counted_by (count)));
};
  
struct Q *
test_malloc_of_non_char_array_valid_symbolic (size_t n)
{
  size_t alloc_sz = sizeof (struct Q) + (sizeof (int32_t) * n);
  struct Q *q = malloc (alloc_sz);
  if (!q)
    return NULL;
  // Don't warn for this:
  q->count = n;
  __builtin_memset (q->array, 0,  sizeof (int32_t) * n);
  return q;
}
  
struct Q *
test_malloc_of_non_char_array_bad_size_symbolic (size_t n)
{
  /* Allocation size is too small: forgetting to multiply
     count by sizeof (array element).  */
  size_t alloc_sz = sizeof (struct Q) + n;
  struct Q *q = malloc (alloc_sz);
  if (!q)
    return NULL;

  /* TODO: should we warn here?
     Allocated size of flex array is too small relative
     to implicit size of accesses.  */
  q->count = n;

  /* TODO: should we warn here?
     This initializes the buffer we allocated,
     but only the first quarter of the flex array.  */
  __builtin_memset (q->array, 0,  n);
  
  /* TODO: should we warn here?
     This initializes the full flex array as specified by
     "count", but is out-of-bounds relative to our heap
     allocation.  */
  __builtin_memset (q->array, 0,  sizeof (int32_t) * n);

  return q;
}
Qing Zhao June 5, 2024, 7:54 p.m. UTC | #8
> On Jun 5, 2024, at 09:49, David Malcolm <dmalcolm@redhat.com> wrote:
> 
> On Tue, 2024-06-04 at 22:09 +0000, Qing Zhao wrote:
>> 
>> 
>>> On Jun 4, 2024, at 17:55, David Malcolm <dmalcolm@redhat.com>
>>> wrote:
>>> 
>>> On Fri, 2024-05-31 at 13:11 +0000, Qing Zhao wrote:
>>>> 
>>>> 
> 
> [...]
> 
>>>> 
>>>> 
>>>> Thanks a lot for the review.
>>>> Will commit the patch set soon.
>>> 
>>> [...snip...]
>>> 
>>> Congratulations on getting this merged.
>>> 
>>> FWIW I've started investigating adding support for the new
>>> attribute to
>>> -fanalyzer (and am tracked this as PR analyzer/111567
>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111567 ).
>> 
>> Thank you for starting looking at this.
>>> 
>>> The docs for the attribute speak of the implied relationship
>>> between
>>> the count field and size of the flex array, and say that: "It's the
>>> user's responsibility to make sure the above requirements to be
>>> kept
>>> all the time.  Otherwise the compiler *reports warnings*, at the
>>> same
>>> time, the results of the array bound sanitizer and the
>>> '__builtin_dynamic_object_size' is undefined." (my emphasis).
>>> 
>>> What are these warnings that are reported?  I looked through 
>>> r15-944-gf824acd0e80754 through r15-948-g4c5bea7def1361 and I
>>> didn't
>>> see any new warnings or test coverage for warnings (beyond misuing
>>> the
>>> attribute).  Sorry if I'm missing something obvious here.
>> 
>> These warnings will be in the remaining work (I listed the remaining
>> work in all versions except the last one):
>> 
>>>>>> ******Remaining works: 
>>>>>> 
>>>>>> 6  Improve __bdos to use the counted_by info in whole-object
>>>>>> size for the structure with FAM.
>>>>>> 7  Emit warnings when the user breaks the requirments for the
>>>>>> new counted_by attribute
>>>>>> compilation time: -Wcounted-by
>>>>>> run time: -fsanitizer=counted-by
>>>>>>    * The initialization to the size field should be done
>>>>>> before the first reference to the FAM field.
>>>>>>    * the array has at least # of elements specified by the
>>>>>> size field all the time during the program.
> 
> Aha - thanks.  Sorry for missing this, I confess I haven't been paying
> close attention to this thread.
> 
>> 
>> With the current patches that have been committed, the warnings are
>> not emitted. 
>> I believe that more analysis and more information are needed for
>> these warnings to be effective, it might not
>> be a trivial patch.  More discussion is needed for emitting such
>> warnings.
>> 
>>> 
>>> Does anyone have examples of cases that -fanalyzer ought to warn
>>> for?
>> 
>> At this moment, I don’t have concrete testing cases for this yet, but
>> I can come up with several small examples and share with you in a
>> later email.
> 
> FWIW I did some brainstorming and put together the following .c file,
> am posting it inline here for the sake of discussion; does this look
> like the kind of thing to test for (in terms of how users are expected
> to use the attribute, and the kinds of mistake they'd want warnings
> about) ?

From my understanding, there are two parts of work to support “counted-by” in -fanalyzer:

1. Use this new attribute to improve out-of-bound, buffer overflow detection in -fanalyzer (maybe -Wanalyzer-out-of-bounds can be improved with this new attribute?)
2. Report user errors that breaks the following 2 requirements for the new counted-by attribute:
    -fsanitizer=counted-by
     * The initialization to the size field should be done before the first reference to the FAM field.
     * the array has at least # of elements specified by the size field all the time during the program.    

So, the testing cases might include the above 1 and 2. 
From my understanding of the below testings, mostly belong to 2. 
Some more comments inlined below:

> 
> /* TODO:
>   Some ideas for dimensions of test matrix:
>   (a) concrete value vs symbolic value for "count"
>   (b) concrete value vs symbolic value for size of array
>   (c) dynamic vs static allocation of buffer (and malloc vs alloca)
>   (d) relative size of array and of count
>       - same size (not an issue)
>       - array is too small compared to "count"
>         - off by one
> - off by more than one
> - size is zero (but not negative)
>         - negative size (which the docs say is OK)
>       - array is too large compared to "count" (not an issue)
>   (e) type of flex array:
>       - char
>       - non-char
>       - type requiring padding
>   (f) type/size/signedness of count field; what about overflow
>       in (count * sizeof (type of array element)) ?
>   ... etc: ideas?
> 
>    Other ideas for test coverage:
>    - realloc
>      - growing object
>      - shrinking object
>      - symbolic sizes where could be growth or shrinkage
>      - failing realloc
>    - ...etc: ideas?  */
> 
> #include <stddef.h>
> #include <stdlib.h>
> #include <stdint.h>
> 
> /* Example from the docs.  */
> 
> struct P {
>  size_t count;
>  char other;
>  char array[] __attribute__ ((counted_by (count)));
> } *p;
> 
> struct P *
> test_malloc_with_correct_symbolic (size_t n)
> {
>  struct P *p = malloc (sizeof (struct P) + n);

The size of the malloc might not be computed very accurately here, you might want:

malloc (MAX (sizeof (struct P), (offsetof (struct P, array[0]) + n))

>  if (!p)
>    return NULL;
>  p->count = n; // don't warn here
>  return p;  
> }
> 
> struct P *
> test_malloc_with_correct_count_concrete (void)
> {
>  struct P *p = malloc (sizeof (struct P) + 42);
>  if (!p)
>    return NULL;
>  p->count = 42; // don't warn here
>  return p;  
> }
> 
> struct P *
> test_malloc_with_array_smaller_than_count_concrete (void)
> {
>  struct P *p = malloc (sizeof (struct P) + 42);
>  if (!p)
>    return NULL;
>  p->count = 80; // TODO: warn here
>  return p;
> }
> 
> struct P *
> test_malloc_with_array_larger_than_count_concrete (void)
> {
>  struct P *p = malloc (sizeof (struct P) + 80);
>  if (!p)
>    return NULL;
>  p->count = 42; // don't warn here
>  return p;  
> }

Okay (except the malloc size). You can take a look at: 
gcc/testsuite/gcc.dg/flex-array-counted-by-4.c

alloc_buf_more and alloc_buf_less

> struct P *
> test_malloc_with_array_access_before_count_init_concrete_1 (void)
> {
>  struct P *p = malloc (sizeof (struct P) + 42);
>  if (!p)
>    return NULL;
>  /* Forgetting to set count altogether.  */
>  __builtin_memset (p->array, 0, 42); // TODO: should warn here
>  return p;  
> }
> 

> struct P *
> test_malloc_with_array_access_before_count_init_concrete_2 (void)
> {
>  struct P *p = malloc (sizeof (struct P) + 42);
>  if (!p)
>    return NULL;
>  /* Erroneously touching array before setting count.  */
>  __builtin_memset (p->array, 0, 42); // TODO: should warn here
>  p->count = 42;
>  return p;  
> }
> 
> struct P *
> test_malloc_with_array_access_before_count_init_symbolic_1 (size_t n)
> {
>  struct P *p = malloc (sizeof (struct P) + n);
>  if (!p)
>    return NULL;
>  /* Forgetting to set count altogether.  */
>  __builtin_memset (p->array, 0, n); // TODO: should warn here
>  return p;  
> }
> 
> struct P *
> test_malloc_with_array_access_before_count_init_symbolic_2 (size_t n)
> {
>  struct P *p = malloc (sizeof (struct P) + n);
>  if (!p)
>    return NULL;
>  /* Erroneously touching array before setting count.  */
>  __builtin_memset (p->array, 0, n); // TODO: should warn here
>  p->count = n;
>  return p;  
> }

Yes, the above are good for: The initialization to the size field should be done before the first reference to the FAM field.

/* Example where sizeof array element != 1.  */
> 
> 
> struct Q
> {
>  size_t count;
>  int32_t array[] __attribute__ ((counted_by (count)));
> };
> 
> struct Q *
> test_malloc_of_non_char_array_valid_symbolic (size_t n)
> {
>  size_t alloc_sz = sizeof (struct Q) + (sizeof (int32_t) * n);
>  struct Q *q = malloc (alloc_sz);
>  if (!q)
>    return NULL;
>  // Don't warn for this:
>  q->count = n;
>  __builtin_memset (q->array, 0,  sizeof (int32_t) * n);
>  return q;
> }
> 
> struct Q *
> test_malloc_of_non_char_array_bad_size_symbolic (size_t n)
> {
>  /* Allocation size is too small: forgetting to multiply
>     count by sizeof (array element).  */
>  size_t alloc_sz = sizeof (struct Q) + n;
>  struct Q *q = malloc (alloc_sz);
>  if (!q)
>    return NULL;
> 
>  /* TODO: should we warn here?
>     Allocated size of flex array is too small relative
>     to implicit size of accesses.  */
>  q->count = n;


If the real # of elements of the array "q->array”  is smaller than that is specified by “q->count”, we should issue warning here. 
> 
>  /* TODO: should we warn here?
>     This initializes the buffer we allocated,
>     but only the first quarter of the flex array.  */
>  __builtin_memset (q->array, 0,  n);
I think that no warning is needed for -fanalyzer=counted-by here, since “q->count” has been initialized before the reference to “q->array”.  This is correct.

 (The error of the initialization is bigger than the real array has been issued in the above warning already). 
> 
>  /* TODO: should we warn here?
>     This initializes the full flex array as specified by
>     "count", but is out-of-bounds relative to our heap
>     allocation.  */
>  __builtin_memset (q->array, 0,  sizeof (int32_t) * n);

I think that no warning is needed for -fanalyzer=counted-by here. 
But whether there are warnings for -Wanalyzer-out-of-bounds is another question, I think when the user use the “counted-by” attribute incorrectly in their source code, the behavior of out-of-bounds detection is undefined as we mentioned in the documentation:

"It's the user's responsibility to make sure the above requirements to
be kept all the time.  Otherwise the compiler reports warnings,
at the same time, the results of the array bound sanitizer and the
@code{__builtin_dynamic_object_size} is undefined”

Qing


>  return q;
> }
> 
> 
> 
> 
> 
> 
> 
>
diff mbox series

Patch

diff --git a/gcc/c/c-parser.cc b/gcc/c/c-parser.cc
index 00f8bf4376e5..2d9e9c0969f0 100644
--- a/gcc/c/c-parser.cc
+++ b/gcc/c/c-parser.cc
@@ -10848,9 +10848,12 @@  c_parser_postfix_expression (c_parser *parser)
 	    if (c_parser_next_token_is (parser, CPP_NAME))
 	      {
 		c_token *comp_tok = c_parser_peek_token (parser);
+		/* Ignore the counted_by attribute for reference inside
+		   offsetof since the information is not useful at all.  */
 		offsetof_ref
 		  = build_component_ref (loc, offsetof_ref, comp_tok->value,
-					 comp_tok->location, UNKNOWN_LOCATION);
+					 comp_tok->location, UNKNOWN_LOCATION,
+					 false);
 		c_parser_consume_token (parser);
 		while (c_parser_next_token_is (parser, CPP_DOT)
 		       || c_parser_next_token_is (parser,
@@ -10877,11 +10880,14 @@  c_parser_postfix_expression (c_parser *parser)
 			    break;
 			  }
 			c_token *comp_tok = c_parser_peek_token (parser);
+			/* Ignore the counted_by attribute for reference inside
+			   offsetof since the information is not useful.  */
 			offsetof_ref
 			  = build_component_ref (loc, offsetof_ref,
 						 comp_tok->value,
 						 comp_tok->location,
-						 UNKNOWN_LOCATION);
+						 UNKNOWN_LOCATION,
+						 false);
 			c_parser_consume_token (parser);
 		      }
 		    else
diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h
index 531a7e8742e3..56a33b8156c6 100644
--- a/gcc/c/c-tree.h
+++ b/gcc/c/c-tree.h
@@ -779,7 +779,7 @@  extern void mark_exp_read (tree);
 extern tree composite_type (tree, tree);
 extern tree lookup_field (const_tree, tree);
 extern tree build_component_ref (location_t, tree, tree, location_t,
-				 location_t);
+				 location_t, bool = true);
 extern tree build_array_ref (location_t, tree, tree);
 extern tree build_omp_array_section (location_t, tree, tree, tree);
 extern tree build_external_ref (location_t, tree, bool, tree *);
diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
index 2375953fdb62..0d9c7a34a0df 100644
--- a/gcc/c/c-typeck.cc
+++ b/gcc/c/c-typeck.cc
@@ -2584,15 +2584,116 @@  should_suggest_deref_p (tree datum_type)
     return false;
 }
 
+/* For a SUBDATUM field of a structure or union DATUM, generate a REF to
+   the object that represents its counted_by per the attribute counted_by
+   attached to this field if it's a flexible array member field, otherwise
+   return NULL_TREE.
+   Set COUNTED_BY_TYPE to the TYPE of the counted_by field.
+   For example, if:
+
+    struct P {
+      int k;
+      int x[] __attribute__ ((counted_by (k)));
+    } *p;
+
+    for:
+    p->x
+
+    the ref to the object that represents its element count will be:
+
+    &(p->k)
+
+*/
+static tree
+build_counted_by_ref (tree datum, tree subdatum, tree *counted_by_type)
+{
+  tree type = TREE_TYPE (datum);
+  if (!c_flexible_array_member_type_p (TREE_TYPE (subdatum)))
+    return NULL_TREE;
+
+  tree attr_counted_by = lookup_attribute ("counted_by",
+					   DECL_ATTRIBUTES (subdatum));
+  tree counted_by_ref = NULL_TREE;
+  *counted_by_type = NULL_TREE;
+  if (attr_counted_by)
+    {
+      tree field_id = TREE_VALUE (TREE_VALUE (attr_counted_by));
+      counted_by_ref
+	= build_component_ref (UNKNOWN_LOCATION,
+			       datum, field_id,
+			       UNKNOWN_LOCATION, UNKNOWN_LOCATION);
+      counted_by_ref = build_fold_addr_expr (counted_by_ref);
+
+      /* Get the TYPE of the counted_by field.  */
+      tree counted_by_field = lookup_field (type, field_id);
+      gcc_assert (counted_by_field);
+
+      do
+	{
+	  *counted_by_type = TREE_TYPE (TREE_VALUE (counted_by_field));
+	  counted_by_field = TREE_CHAIN (counted_by_field);
+	}
+      while (counted_by_field);
+    }
+  return counted_by_ref;
+}
+
+/* Given a COMPONENT_REF REF with the location LOC, the corresponding
+   COUNTED_BY_REF, and the COUNTED_BY_TYPE, generate an INDIRECT_REF
+   to a call to the internal function .ACCESS_WITH_SIZE.
+
+   REF
+
+   to:
+
+   (*.ACCESS_WITH_SIZE (REF, COUNTED_BY_REF, 1, (TYPE_OF_SIZE)0, -1))
+
+   NOTE: The return type of this function is the POINTER type pointing
+   to the original flexible array type.
+   Then the type of the INDIRECT_REF is the original flexible array type.
+
+   The type of the first argument of this function is a POINTER type
+   to the original flexible array type.
+
+   The 4th argument of the call is a constant 0 with the TYPE of the
+   object pointed by COUNTED_BY_REF.
+
+  */
+static tree
+build_access_with_size_for_counted_by (location_t loc, tree ref,
+				       tree counted_by_ref,
+				       tree counted_by_type)
+{
+  gcc_assert (c_flexible_array_member_type_p (TREE_TYPE (ref)));
+  /* The result type of the call is a pointer to the flexible array type.  */
+  tree result_type = build_pointer_type (TREE_TYPE (ref));
+
+  tree call
+    = build_call_expr_internal_loc (loc, IFN_ACCESS_WITH_SIZE,
+				    result_type, 5,
+				    array_to_pointer_conversion (loc, ref),
+				    counted_by_ref,
+				    build_int_cst (integer_type_node, 1),
+				    build_int_cst (counted_by_type, 0),
+				    build_int_cst (integer_type_node, -1));
+  /* Wrap the call with an INDIRECT_REF with the flexible array type.  */
+  call = build1 (INDIRECT_REF, TREE_TYPE (ref), call);
+  SET_EXPR_LOCATION (call, loc);
+  return call;
+}
+
 /* Make an expression to refer to the COMPONENT field of structure or
    union value DATUM.  COMPONENT is an IDENTIFIER_NODE.  LOC is the
    location of the COMPONENT_REF.  COMPONENT_LOC is the location
    of COMPONENT.  ARROW_LOC is the location of the first -> operand if
-   it is from -> operator.  */
+   it is from -> operator.
+   If HANDLE_COUNTED_BY is true, check the counted_by attribute and generate
+   a call to .ACCESS_WITH_SIZE.  Otherwise, ignore the attribute.  */
 
 tree
 build_component_ref (location_t loc, tree datum, tree component,
-		     location_t component_loc, location_t arrow_loc)
+		     location_t component_loc, location_t arrow_loc,
+		     bool handle_counted_by)
 {
   tree type = TREE_TYPE (datum);
   enum tree_code code = TREE_CODE (type);
@@ -2664,7 +2765,13 @@  build_component_ref (location_t loc, tree datum, tree component,
 	  int quals;
 	  tree subtype;
 	  bool use_datum_quals;
-
+	  tree counted_by_type = NULL_TREE;
+	  /* Do not handle counted_by when in typeof and alignof operator.  */
+	  handle_counted_by = handle_counted_by && !in_typeof && !in_alignof;
+	  tree counted_by_ref = handle_counted_by
+				? build_counted_by_ref (datum, subdatum,
+							&counted_by_type)
+				: NULL_TREE;
 	  if (TREE_TYPE (subdatum) == error_mark_node)
 	    return error_mark_node;
 
@@ -2683,6 +2790,12 @@  build_component_ref (location_t loc, tree datum, tree component,
 	  ref = build3 (COMPONENT_REF, subtype, datum, subdatum,
 			NULL_TREE);
 	  SET_EXPR_LOCATION (ref, loc);
+
+	  if (counted_by_ref)
+	    ref = build_access_with_size_for_counted_by (loc, ref,
+							 counted_by_ref,
+							 counted_by_type);
+
 	  if (TREE_READONLY (subdatum)
 	      || (use_datum_quals && TREE_READONLY (datum)))
 	    TREE_READONLY (ref) = 1;
@@ -5087,7 +5200,11 @@  build_unary_op (location_t location, enum tree_code code, tree xarg,
 	  goto return_build_unary_op;
 	}
 
-      /* Ordinary case; arg is a COMPONENT_REF or a decl.  */
+      /* Ordinary case; arg is a COMPONENT_REF or a decl, or a call to
+	 .ACCESS_WITH_SIZE.  */
+      if (is_access_with_size_p (arg))
+	arg = TREE_OPERAND (TREE_OPERAND (CALL_EXPR_ARG (arg, 0), 0), 0);
+
       argtype = TREE_TYPE (arg);
 
       /* If the lvalue is const or volatile, merge that into the type
@@ -5238,6 +5355,9 @@  lvalue_p (const_tree ref)
     case BIND_EXPR:
       return TREE_CODE (TREE_TYPE (ref)) == ARRAY_TYPE;
 
+    case CALL_EXPR:
+      return is_access_with_size_p (ref);
+
     default:
       return false;
     }
@@ -8525,6 +8645,20 @@  digest_init (location_t init_loc, tree type, tree init, tree origtype,
 
   STRIP_TYPE_NOPS (inside_init);
 
+  /* If require_constant is TRUE,  when the initializer is a call to
+     .ACCESS_WITH_SIZE, use the first argument as the initializer.
+     For example:
+     y = (char *) .ACCESS_WITH_SIZE ((char *) &static_annotated.c,...)
+     will be converted to
+     y = &static_annotated.c.  */
+
+  if (require_constant
+      && TREE_CODE (inside_init) == NOP_EXPR
+      && TREE_CODE (TREE_OPERAND (inside_init, 0)) == CALL_EXPR
+      && is_access_with_size_p (TREE_OPERAND (inside_init, 0)))
+    inside_init
+      = get_ref_from_access_with_size (TREE_OPERAND (inside_init, 0));
+
   if (!c_in_omp_for)
     {
       if (TREE_CODE (inside_init) == EXCESS_PRECISION_EXPR)
diff --git a/gcc/internal-fn.cc b/gcc/internal-fn.cc
index 9c09026793fa..eb2c4cd59048 100644
--- a/gcc/internal-fn.cc
+++ b/gcc/internal-fn.cc
@@ -3438,6 +3438,40 @@  expand_DEFERRED_INIT (internal_fn, gcall *stmt)
     }
 }
 
+/* Expand the IFN_ACCESS_WITH_SIZE function:
+   ACCESS_WITH_SIZE (REF_TO_OBJ, REF_TO_SIZE, CLASS_OF_SIZE,
+		     TYPE_OF_SIZE, ACCESS_MODE)
+   which returns the REF_TO_OBJ same as the 1st argument;
+
+   1st argument REF_TO_OBJ: The reference to the object;
+   2nd argument REF_TO_SIZE: The reference to the size of the object,
+   3rd argument CLASS_OF_SIZE: The size referenced by the REF_TO_SIZE represents
+     0: the number of bytes.
+     1: the number of the elements of the object type;
+   4th argument TYPE_OF_SIZE: A constant 0 with its TYPE being the same as the TYPE
+    of the object referenced by REF_TO_SIZE
+   5th argument ACCESS_MODE:
+    -1: Unknown access semantics
+     0: none
+     1: read_only
+     2: write_only
+     3: read_write
+
+   Both the return type and the type of the first argument of this
+   function have been converted from the incomplete array type to
+   the corresponding pointer type.
+
+   For each call to a .ACCESS_WITH_SIZE, replace it with its 1st argument.  */
+
+static void
+expand_ACCESS_WITH_SIZE (internal_fn, gcall *stmt)
+{
+  tree lhs = gimple_call_lhs (stmt);
+  tree ref_to_obj = gimple_call_arg (stmt, 0);
+  if (lhs)
+    expand_assignment (lhs, ref_to_obj, false);
+}
+
 /* The size of an OpenACC compute dimension.  */
 
 static void
diff --git a/gcc/internal-fn.def b/gcc/internal-fn.def
index 25badbb86e56..8de1fa882e95 100644
--- a/gcc/internal-fn.def
+++ b/gcc/internal-fn.def
@@ -512,6 +512,11 @@  DEF_INTERNAL_FN (PHI, 0, NULL)
    automatic variable.  */
 DEF_INTERNAL_FN (DEFERRED_INIT, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL)
 
+/* A function to associate the access size and access mode information
+   with the corresponding reference to an object.  It only reads from the
+   2nd argument.  */
+DEF_INTERNAL_FN (ACCESS_WITH_SIZE, ECF_PURE | ECF_LEAF | ECF_NOTHROW, NULL)
+
 /* DIM_SIZE and DIM_POS return the size of a particular compute
    dimension and the executing thread's position within that
    dimension.  DIM_POS is pure (and not const) so that it isn't
diff --git a/gcc/testsuite/gcc.dg/flex-array-counted-by-2.c b/gcc/testsuite/gcc.dg/flex-array-counted-by-2.c
new file mode 100644
index 000000000000..d4899a63af3c
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/flex-array-counted-by-2.c
@@ -0,0 +1,112 @@ 
+/* Test the code generation for the new attribute counted_by.
+   And also the offsetof operator on such array.  */
+/* { dg-do run } */
+/* { dg-options "-O2 -fdump-tree-original" } */
+
+#include <stdlib.h>
+
+struct annotated {
+  int b;
+  char c[] __attribute__ ((counted_by (b)));
+} *array_annotated;
+
+static struct annotated static_annotated = { sizeof "hello", "hello" };
+static char *y = static_annotated.c;
+
+struct flex {
+  int b;
+  char c[]; 
+}; 
+
+struct nested_annotated {
+  struct {
+    union {
+      int b;
+      float f;	
+    };
+    int n;
+  };
+  char c[] __attribute__ ((counted_by (b)));
+} *array_nested_annotated;
+
+static struct nested_annotated nested_static_annotated
+				 = { sizeof "hello1", 0, "hello1" };
+static char *nested_y = nested_static_annotated.c;
+
+struct nested_flex {
+  struct {
+    union {
+      int b;
+      float f;	
+    };
+    int n;
+  };
+  char c[];
+};
+
+void __attribute__((__noinline__)) setup (int normal_count, int attr_count)
+{
+  array_annotated
+    = (struct annotated *)malloc (sizeof (struct annotated)
+				  + attr_count *  sizeof (char));
+  array_annotated->b = attr_count;
+
+  array_nested_annotated
+    = (struct nested_annotated *)malloc (sizeof (struct nested_annotated)
+					 + attr_count *  sizeof (char));
+  array_nested_annotated->b = attr_count;
+
+  return;
+}
+
+void __attribute__((__noinline__)) test (char a, char b)
+{
+  if (__builtin_offsetof (struct annotated, c[0])
+      != __builtin_offsetof (struct flex, c[0]))
+    abort ();
+  if (__builtin_offsetof (struct annotated, c[1])
+      != __builtin_offsetof (struct flex, c[1]))
+    abort ();
+  if (__builtin_offsetof (struct nested_annotated, c[0]) 
+      != __builtin_offsetof (struct nested_flex, c[0])) 
+    abort ();
+  if (__builtin_offsetof (struct nested_annotated, c[1]) 
+      != __builtin_offsetof (struct nested_flex, c[1])) 
+    abort ();
+
+  if (__builtin_types_compatible_p (typeof (array_annotated->c),
+				    typeof (&(array_annotated->c)[0])))
+    abort ();
+  if (__builtin_types_compatible_p (typeof (array_nested_annotated->c),
+				    typeof (&(array_nested_annotated->c)[0])))
+    abort ();
+
+  if (__alignof (array_annotated->c) != __alignof (char))
+    abort ();
+  if (__alignof (array_nested_annotated->c) != __alignof (char))
+    abort ();
+
+  if ((unsigned long) array_annotated->c != (unsigned long) &array_annotated->c)
+    abort ();
+  if ((unsigned long) array_nested_annotated->c
+       != (unsigned long) &array_nested_annotated->c)
+    abort ();
+
+  array_annotated->c[2] = a;
+  array_nested_annotated->c[3] = b;
+
+  if (y[2] != 'l') abort ();
+  if (nested_y[4] !='o') abort ();
+
+}
+
+int main(int argc, char *argv[])
+{
+  setup (10,10);   
+  test ('A', 'B');
+  if (array_annotated->c[2] != 'A') abort ();
+  if (array_nested_annotated->c[3] != 'B') abort ();
+  return 0;
+}
+
+/* { dg-final { scan-tree-dump-times "ACCESS_WITH_SIZE" 8 "original" } } */
diff --git a/gcc/tree.cc b/gcc/tree.cc
index 6564b002dc1a..01572fe70f72 100644
--- a/gcc/tree.cc
+++ b/gcc/tree.cc
@@ -13405,6 +13405,28 @@  component_ref_size (tree ref, special_array_member *sam /* = NULL */)
 	  ? NULL_TREE : size_zero_node);
 }
 
+/* Return true if the given node CALL is a call to a .ACCESS_WITH_SIZE
+   function.  */
+bool
+is_access_with_size_p (const_tree call)
+{
+  if (TREE_CODE (call) != CALL_EXPR)
+    return false;
+  if (CALL_EXPR_IFN (call) == IFN_ACCESS_WITH_SIZE)
+    return true;
+  return false;
+}
+
+/* Get the corresponding reference from the call to a .ACCESS_WITH_SIZE.
+ * i.e the first argument of this call.  Return NULL_TREE otherwise.  */
+tree
+get_ref_from_access_with_size (tree call)
+{
+  if (is_access_with_size_p (call))
+    return  CALL_EXPR_ARG (call, 0);
+  return NULL_TREE;
+}
+
 /* Return the machine mode of T.  For vectors, returns the mode of the
    inner type.  The main use case is to feed the result to HONOR_NANS,
    avoiding the BLKmode that a direct TYPE_MODE (T) might return.  */
diff --git a/gcc/tree.h b/gcc/tree.h
index ee2aae332a41..604885641184 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -5772,6 +5772,14 @@  extern special_array_member component_ref_sam_type (tree);
    cannot be determined.  */
 extern tree component_ref_size (tree, special_array_member * = NULL);
 
+/* Return true if the given node is a call to a .ACCESS_WITH_SIZE
+   function.  */
+extern bool is_access_with_size_p (const_tree);
+
+/* Get the corresponding reference from the call to a .ACCESS_WITH_SIZE,
+ * i.e. the first argument of this call.  Return NULL_TREE otherwise.  */
+extern tree get_ref_from_access_with_size (tree);
+
 extern int tree_map_base_eq (const void *, const void *);
 extern unsigned int tree_map_base_hash (const void *);
 extern bool tree_map_base_marked_p (const void *);