New attribute: returns_nonnull

[Jeff Law]

On 10/07/13 08:17, Marc Glisse wrote:
> Hello,
>
> this patch adds an attribute to let the compiler know that a function
> never returns NULL. I saw some ECF_* flags, but the attribute seems
> sufficient. I considered using nonnull(0), but then it would have been
> confusing that the version of nonnull without arguments applies only to
> parameters and not the return value.
>
> 2013-10-08  Marc Glisse <marc.glisse@inria.fr>
>
>      PR tree-optimization/20318
> gcc/c-family/
>      * c-common.c (handle_returns_nonnull_attribute): New function.
>      (c_common_attribute_table): Add returns_nonnull.
>
> gcc/
>      * doc/extend.texi (returns_nonnull): New function attribute.
>      * fold-const.c (tree_expr_nonzero_warnv_p): Look for returns_nonnull
>      attribute.
>      * tree-vrp.c (gimple_stmt_nonzero_warnv_p): Likewise.
>      (stmt_interesting_for_vrp): Accept all GIMPLE_CALL.
>
> gcc/testsuite/
>      * c-c++-common/pr20318.c: New file.
>      * gcc.dg/tree-ssa/pr20318.c: New file.
>
> --
> Marc Glisse
>
> p12
>
>
> Index: c-family/c-common.c
> ===================================================================
> --- c-family/c-common.c	(revision 203241)
> +++ c-family/c-common.c	(working copy)
> @@ -740,20 +741,22 @@ const struct attribute_spec c_common_att
>     { "*tm regparm",            0, 0, false, true, true,
>   			      ignore_attribute, false },
>     { "no_split_stack",	      0, 0, true,  false, false,
>   			      handle_no_split_stack_attribute, false },
>     /* For internal use (marking of builtins and runtime functions) only.
>        The name contains space to prevent its usage in source code.  */
>     { "fn spec",	 	      1, 1, false, true, true,
>   			      handle_fnspec_attribute, false },
>     { "warn_unused",            0, 0, false, false, false,
>   			      handle_warn_unused_attribute, false },
> +  { "returns_nonnull",        0, 0, false, true, true,
> +			      handle_returns_nonnull_attribute, false },
>     { NULL,                     0, 0, false, false, false, NULL, false }
>   };
I'm going to assume this is correct -- it looks sane, but I've never 
really done much with the attribute tables.

> +
> +/* Handle a "returns_nonnull" attribute; arguments as in
> +   struct attribute_spec.handler.  */
> +
> +static tree
> +handle_returns_nonnull_attribute (tree *node, tree, tree, int,
> +				  bool *no_add_attrs)
> +{
> +  // Even without a prototype we still have a return type we can check.
> +  if (TREE_CODE (TREE_TYPE (*node)) != POINTER_TYPE)
> +    {
> +      error ("returns_nonnull attribute on a function not returning a pointer");
> +      *no_add_attrs = true;
> +    }
> +  return NULL_TREE;
> +}
Glad to see you checked this and have a test for it.

Not required for approval, but an "extra credit" -- a warning if a NULL 
value flows into a return statement in a function with this marking.

Similarly, not required for approval, but it'd be real cool if we could 
back-propagate the non-null return value attribute.  ie, any value 
flowing into the return statement of one of these functions can be 
assumed to be non-zero, which may help eliminate more null pointer 
checks in the decorated function.  I guess ultimately we'd have to see 
if noting this actually helps any real code.

Also not required for approval, but adding returns_nonnull markups to 
appropriate functions in gcc itself.



> Index: testsuite/gcc.dg/tree-ssa/pr20318.c
> ===================================================================
> --- testsuite/gcc.dg/tree-ssa/pr20318.c	(revision 0)
> +++ testsuite/gcc.dg/tree-ssa/pr20318.c	(working copy)
> @@ -0,0 +1,19 @@
> +/* { dg-do compile { target { ! keeps_null_pointer_checks } } } */
> +/* { dg-options "-O2 -fdump-tree-original -fdump-tree-vrp1" } */
> +
> +extern int* f(int) __attribute__((returns_nonnull));
> +extern void eliminate ();
> +void g () {
> +  if (f (2) == 0)
> +    eliminate ();
> +}
> +void h () {
> +  int *p = f (2);
> +  if (p == 0)
> +    eliminate ();
> +}
> +
> +/* { dg-final { scan-tree-dump-times "== 0" 1 "original" } } */
> +/* { dg-final { scan-tree-dump-times "Folding predicate\[^\\n\]*to 0" 1 "vrp1" } } */
> +/* { dg-final { cleanup-tree-dump "original" } } */
> +/* { dg-final { cleanup-tree-dump "vrp1" } } */
Presumably g() is testing the fold-const.c and h() tests the tree-vrp 
changes, right?

This is OK for the trunk, please install.

Jeff