[6/8] tree-dynamic-object-size: Handle function parameters

  Handle either static sizes in function parameters or hints provided by
__attribute__ ((access (...))) to compute sizes for objects.

gcc/ChangeLog:

	* tree-dynamic-object-size.c: Include tree-dfa.h.
	(emit_size_stmts): New argument osi.  Handle GIMPLE_NOP.
	(eval_size_expr, gimplify_size_exprs): Adjust.
	(parm_object_size): New function.
	(collect_object_sizes_for): Handle GIMPLE_NOP.

gcc/testsuite/ChangeLog:

	* gcc.dg/builtin-dynamic-object-size-0.c (test_parmsz): New
	test.
	(main): Call it.

Signed-off-by: Siddhesh Poyarekar <siddhesh@gotplt.org>
---
 .../gcc.dg/builtin-dynamic-object-size-0.c    | 22 +++++
 gcc/tree-dynamic-object-size.c                | 98 +++++++++++++++++--
 2 files changed, 112 insertions(+), 8 deletions(-)
  

On 10/7/21 4:14 PM, Siddhesh Poyarekar wrote:
> Handle either static sizes in function parameters or hints provided by
> __attribute__ ((access (...))) to compute sizes for objects.

It's been my hope to eventually teach __builtin_object_size about
attribute access but implementing it in the new built-in might be
preferable.  Glad to see you noticed it and took advantage of it!

Does this include handling "VLA function parameters" as in

   void f (int n, char d[n]);   // (or char d[static n])

I don't see tests for it in this patch but since internally, GCC
describes VLA (and array) function arguments using attribute access
hanndling it should automatically give us VLA (and array) support
as well unless we disable it, either intentionally or by accident.
Either way, I would recommend adding tests for VLA parameters.

Martin

> 
> gcc/ChangeLog:
> 
> 	* tree-dynamic-object-size.c: Include tree-dfa.h.
> 	(emit_size_stmts): New argument osi.  Handle GIMPLE_NOP.
> 	(eval_size_expr, gimplify_size_exprs): Adjust.
> 	(parm_object_size): New function.
> 	(collect_object_sizes_for): Handle GIMPLE_NOP.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.dg/builtin-dynamic-object-size-0.c (test_parmsz): New
> 	test.
> 	(main): Call it.
> 
> Signed-off-by: Siddhesh Poyarekar <siddhesh@gotplt.org>
> ---
>   .../gcc.dg/builtin-dynamic-object-size-0.c    | 22 +++++
>   gcc/tree-dynamic-object-size.c                | 98 +++++++++++++++++--
>   2 files changed, 112 insertions(+), 8 deletions(-)
> 
> diff --git a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c
> index 3c2c4c84264..c72fa0508db 100644
> --- a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c
> +++ b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c
> @@ -255,6 +255,15 @@ test_substring_ptrplus (size_t sz, size_t off)
>     return __builtin_dynamic_object_size (str + off, 0);
>   }
>   
> +size_t
> +__attribute__ ((noinline))

I think attribute noipa might be a better choice if the goal
is to keep GCC from sneaking in data from the caller and so
defeating the purpose of the test.

Martin

> +__attribute__ ((access (__read_write__, 1, 2)))
> +test_parmsz (void *obj, size_t sz, size_t off)
> +{
> +  return __builtin_dynamic_object_size (obj + off, 0);
> +}
> +
> +
>   int
>   main (int argc, char **argv)
>   {
> @@ -338,6 +347,19 @@ main (int argc, char **argv)
>     if (test_deploop (128, 129) != 32)
>       FAIL ();
>   
> +  if (test_parmsz (argv[0], __builtin_strlen (argv[0]) + 1, -1)!= 0)
> +    FAIL ();
> +
> +  if (test_parmsz (argv[0], __builtin_strlen (argv[0]) + 1, 0)
> +      != __builtin_strlen (argv[0]) + 1)
> +    FAIL ();
> +  if (test_parmsz (argv[0], __builtin_strlen (argv[0]) + 1,
> +		   __builtin_strlen (argv[0]))!= 1)
> +    FAIL ();
> +  if (test_parmsz (argv[0], __builtin_strlen (argv[0]) + 1,
> +		   __builtin_strlen (argv[0]) + 2)!= 0)
> +    FAIL ();
> +
>     if (nfails > 0)
>       __builtin_abort ();
>   
> diff --git a/gcc/tree-dynamic-object-size.c b/gcc/tree-dynamic-object-size.c
> index f143a64777c..8d7283623dc 100644
> --- a/gcc/tree-dynamic-object-size.c
> +++ b/gcc/tree-dynamic-object-size.c
> @@ -58,6 +58,7 @@ along with GCC; see the file COPYING3.  If not see
>   #include "gimple-fold.h"
>   #include "gimple-iterator.h"
>   #include "tree-cfg.h"
> +#include "tree-dfa.h"
>   #include "stringpool.h"
>   #include "attribs.h"
>   #include "builtins.h"
> @@ -456,7 +457,7 @@ pass_through_call (const gcall *call)
>   
>   
>   static void
> -emit_size_stmts (gimple *stmt, tree wholesize_ssa,
> +emit_size_stmts (object_size_info *osi, gimple *stmt, tree wholesize_ssa,
>   		 tree wholesize_expr, tree size_ssa, tree size_expr)
>   {
>     gimple_seq seq = NULL;
> @@ -481,7 +482,14 @@ emit_size_stmts (gimple *stmt, tree wholesize_ssa,
>        statements involved in evaluation of the object size expression precede
>        the definition statement.  For parameters, we don't have a definition
>        statement, so insert into the first code basic block.  */
> -  gimple_stmt_iterator i = gsi_for_stmt (stmt);
> +  gimple_stmt_iterator i;
> +  if (gimple_code (stmt) == GIMPLE_NOP)
> +    {
> +      basic_block first_bb = single_succ (ENTRY_BLOCK_PTR_FOR_FN (osi->fun));
> +      i = gsi_start_bb (first_bb);
> +    }
> +  else
> +    i = gsi_for_stmt (stmt);
>     gsi_insert_seq_before (&i, seq, GSI_CONTINUE_LINKING);
>   }
>   
> @@ -542,8 +550,8 @@ size_bound_expr (tree sz)
>   }
>   
>   static void
> -eval_size_expr (tree var, tree wholesize, tree *wholesize_expr,
> -		tree size, tree *size_expr)
> +eval_size_expr (struct object_size_info *osi, tree var, tree wholesize,
> +		tree *wholesize_expr, tree size, tree *size_expr)
>   {
>     if (size_expr != NULL)
>       {
> @@ -560,7 +568,7 @@ eval_size_expr (tree var, tree wholesize, tree *wholesize_expr,
>   	}
>         else
>   	{
> -	  emit_size_stmts (stmt, wholesize, *wholesize_expr, size,
> +	  emit_size_stmts (osi, stmt, wholesize, *wholesize_expr, size,
>   			   size_bound_expr (*size_expr));
>   	  delete wholesize_expr;
>   	  delete size_expr;
> @@ -611,7 +619,7 @@ gimplify_size_exprs (object_size_info *osi, tree ptr)
>     for (int object_size_type = 0; object_size_type <= 3; object_size_type++)
>       EXECUTE_IF_SET_IN_BITMAP (computed[object_size_type], 0, i, bi)
>         {
> -	eval_size_expr (ssa_name (i),
> +	eval_size_expr (osi, ssa_name (i),
>   			object_whole_sizes[object_size_type][i],
>   			object_whole_size_exprs[object_size_type][i],
>   			object_sizes[object_size_type][i],
> @@ -939,6 +947,71 @@ call_object_size (struct object_size_info *osi, tree ptr, gcall *call)
>     cache_object_size_copy (osi, SSA_NAME_VERSION (ptr), sz, sz);
>   }
>   
> +/* Find size of an object passed as a parameter to the function.  */
> +
> +static void
> +parm_object_size (struct object_size_info *osi, tree var)
> +{
> +  tree parm = SSA_NAME_VAR (var);
> +  unsigned varno = SSA_NAME_VERSION (var);
> +  tree sz = NULL_TREE, wholesz = NULL_TREE;
> +
> +  if (dump_file)
> +  {
> +	  fprintf (dump_file, "Object is a parameter: ");
> +	  print_generic_expr (dump_file, parm, dump_flags);
> +	  fprintf (dump_file, " which is %s a pointer type\n",
> +			  POINTER_TYPE_P (TREE_TYPE (parm)) ? "" : "not");
> +  }
> +
> +  if (POINTER_TYPE_P (TREE_TYPE (parm)))
> +    {
> +      /* Look for access attribute.  */
> +      rdwr_map rdwr_idx;
> +
> +      tree fndecl = osi->fun->decl;
> +      const attr_access *access = get_parm_access (rdwr_idx, parm, fndecl);
> +      tree typesize = TYPE_SIZE_UNIT (TREE_TYPE (TREE_TYPE (parm)));
> +
> +      if (access && access->sizarg != UINT_MAX)
> +	{
> +	  tree fnargs = DECL_ARGUMENTS (fndecl);
> +	  tree arg = NULL_TREE;
> +	  unsigned argpos = 0;
> +
> +	  /* Walk through the parameters to pick the size parameter and safely
> +	     scale it by the type size.  */
> +	  for (arg = fnargs; argpos != access->sizarg && arg;
> +	       arg = TREE_CHAIN (arg), ++argpos);
> +
> +	  if (arg != NULL_TREE && INTEGRAL_TYPE_P (TREE_TYPE (arg)))
> +	    {
> +	      tree arrsz = get_or_create_ssa_default_def (osi->fun, arg);
> +	      if (arrsz != NULL_TREE)
> +		{
> +		  arrsz = fold_convert (sizetype, arrsz);
> +		  if (typesize)
> +		    {
> +		      tree res = fold_build2 (MULT_EXPR, sizetype, arrsz,
> +					      typesize);
> +		      tree check = fold_build2 (EXACT_DIV_EXPR, sizetype,
> +						TYPE_MAX_VALUE (sizetype),
> +						typesize);
> +		      check = fold_build2 (LT_EXPR, sizetype, arrsz, check);
> +		      arrsz = fold_build3 (COND_EXPR, sizetype, check, res,
> +					   size_zero_node);
> +		    }
> +		}
> +	      sz = wholesz = arrsz;
> +	    }
> +	}
> +    }
> +  else
> +      expr_object_size (osi, parm, &sz, &wholesz);
> +
> +  cache_object_size_copy (osi, varno, sz, wholesz);
> +}
> +
>   /* Compute object sizes for VAR.
>   
>      - For allocation GIMPLE_CALL like malloc or calloc object size is the size
> @@ -948,7 +1021,9 @@ call_object_size (struct object_size_info *osi, tree ptr, gcall *call)
>      - For GIMPLE_PHI, compute a PHI node with sizes of all branches in the PHI
>        node of the object.
>      - For GIMPLE_ASSIGN, return the size of the object the SSA name points
> -     to.  */
> +     to.
> +   - For GIMPLE_NOP, if the variable is a function parameter, compute the size
> +     of the parameter.  */
>   
>   static void
>   collect_object_sizes_for (struct object_size_info *osi, tree var)
> @@ -1076,8 +1151,15 @@ collect_object_sizes_for (struct object_size_info *osi, tree var)
>   	break;
>         }
>   
> -    /* Bail out for all other cases.  */
>       case GIMPLE_NOP:
> +      if (SSA_NAME_VAR (var) && TREE_CODE (SSA_NAME_VAR (var)) == PARM_DECL)
> +	parm_object_size (osi, var);
> +      else
> +	/* Uninitialized SSA names point nowhere.  */
> +	cache_object_size_copy (osi, varno, NULL_TREE, NULL_TREE);
> +      break;
> +
> +    /* Bail out for all other cases.  */
>       case GIMPLE_ASM:
>         cache_object_size_copy (osi, varno, NULL_TREE, NULL_TREE);
>         break;
>
  

On 10/20/21 22:26, Martin Sebor wrote:
> On 10/7/21 4:14 PM, Siddhesh Poyarekar wrote:
>> Handle either static sizes in function parameters or hints provided by
>> __attribute__ ((access (...))) to compute sizes for objects.
> 
> It's been my hope to eventually teach __builtin_object_size about
> attribute access but implementing it in the new built-in might be
> preferable.  Glad to see you noticed it and took advantage of it!
> 
> Does this include handling "VLA function parameters" as in
> 
>    void f (int n, char d[n]);   // (or char d[static n])
> 
> I don't see tests for it in this patch but since internally, GCC
> describes VLA (and array) function arguments using attribute access
> hanndling it should automatically give us VLA (and array) support
> as well unless we disable it, either intentionally or by accident.
> Either way, I would recommend adding tests for VLA parameters.

Not in this patchset, but I did notice it and put it in my list of 
things to add on once this makes it in.

Thanks,
Siddhesh