Message ID | alpine.DEB.2.02.1310071600430.22663@stedding.saclay.inria.fr |
---|---|
State | New |
Headers | show |
On Mon, 2013-10-07 at 16:17 +0200, 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. I can't comment on the patch itself, but could there also be an attribute "returns_null", for functions that *always* return NULL? This may sound weird, but I know of at least one API that exposes such functions: CPython's exception-handling API: see e.g. http://docs.python.org/2/c-api/exceptions.html#PyErr_NoMemory and various other functions that have "Return value: Always NULL." This allows the user to write one-liners like: return PyErr_NoMemory(); Hope this is helpful Dave
On Mon, 7 Oct 2013, David Malcolm wrote: > On Mon, 2013-10-07 at 16:17 +0200, 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. > > I can't comment on the patch itself, but could there also be an > attribute "returns_null", for functions that *always* return NULL? > This may sound weird, but I know of at least one API that exposes such > functions: CPython's exception-handling API: see e.g. > http://docs.python.org/2/c-api/exceptions.html#PyErr_NoMemory > and various other functions that have "Return value: Always NULL." > This allows the user to write one-liners like: > > return PyErr_NoMemory(); I didn't think about it very long, so I probably missed the best reasons, but it doesn't sound like such a good idea. If PyErr_NoMemory always returns NULL, why not make that clear in the code? It could be an inline function, or even a macro that expands to (PyErr_SetNone(PyExc_MemoryError),NULL). To me, attributes are there for when the language is insufficient, kind of a last resort. Could you explain why you think it would be the best option here?
On Mon, 2013-10-07 at 19:51 +0200, Marc Glisse wrote: > On Mon, 7 Oct 2013, David Malcolm wrote: > > > On Mon, 2013-10-07 at 16:17 +0200, 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. > > > > I can't comment on the patch itself, but could there also be an > > attribute "returns_null", for functions that *always* return NULL? > > This may sound weird, but I know of at least one API that exposes such > > functions: CPython's exception-handling API: see e.g. > > http://docs.python.org/2/c-api/exceptions.html#PyErr_NoMemory > > and various other functions that have "Return value: Always NULL." > > This allows the user to write one-liners like: > > > > return PyErr_NoMemory(); > > I didn't think about it very long, so I probably missed the best reasons, > but it doesn't sound like such a good idea. If PyErr_NoMemory always > returns NULL, why not make that clear in the code? It could be an inline > function, or even a macro that expands to > (PyErr_SetNone(PyExc_MemoryError),NULL). > > To me, attributes are there for when the language is insufficient, kind of > a last resort. Could you explain why you think it would be the best option > here? No, I can't :) (that API predates my involvement in that project).
On 10/07/13 12:37, David Malcolm wrote: > On Mon, 2013-10-07 at 19:51 +0200, Marc Glisse wrote: >> On Mon, 7 Oct 2013, David Malcolm wrote: >> >>> On Mon, 2013-10-07 at 16:17 +0200, 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. >>> >>> I can't comment on the patch itself, but could there also be an >>> attribute "returns_null", for functions that *always* return NULL? >>> This may sound weird, but I know of at least one API that exposes such >>> functions: CPython's exception-handling API: see e.g. >>> http://docs.python.org/2/c-api/exceptions.html#PyErr_NoMemory >>> and various other functions that have "Return value: Always NULL." >>> This allows the user to write one-liners like: >>> >>> return PyErr_NoMemory(); >> >> I didn't think about it very long, so I probably missed the best reasons, >> but it doesn't sound like such a good idea. If PyErr_NoMemory always >> returns NULL, why not make that clear in the code? It could be an inline >> function, or even a macro that expands to >> (PyErr_SetNone(PyExc_MemoryError),NULL). >> >> To me, attributes are there for when the language is insufficient, kind of >> a last resort. Could you explain why you think it would be the best option >> here? > No, I can't :) (that API predates my involvement in that project). This seems to have dubious value to me -- ie, I don't think there's many functions that always return NULL in practice and of those, I doubt there's a lot of opportunity to optimize with that knowledge. In contrast, knowing a function can not return null has a ton of value, both for optimization purposes and for improving warnings. jeff
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
On Tue, 8 Oct 2013, Jeff Law wrote: > 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. I looked at nonnull and noreturn, and the second one says that it is wrong and should be like nonnull, so I mostly copied from nonnull. I can't say I really understand what it is doing, but I was happy that everything worked so nicely. >> + >> +/* 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. I am slowly starting to understand how reviewers think ;-) > 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. I completely agree that returns_nonnull has more uses. The warning is the first one (not sure where to add such a check though), and maybe we should have a sanitizer option to check the nonnull and returns_nonnull attributes, although I don't know if that should be in the caller, in the callee, or both. From an optimization POV, I guess there is also the inlining that could be improved so it doesn't lose the nonnull property. The main request I am aware of is PR21856, but I am not familiar enough with java to handle it. Do you have suggestions of functions which should get the attribute? memcpy could, but that's really a special case since it returns its argument, which is itself non-null. I'll open a new PR about all these. I mostly implemented returns_nonnull because I'd just done the same for operator new and thus I knew exactly where the optimization was, I can't promise I'll manage much of the rest. >> 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? Yes. > This is OK for the trunk, please install. Thank you.
On 10/08/13 13:41, Marc Glisse wrote: >> Glad to see you checked this and have a test for it. > > I am slowly starting to understand how reviewers think ;-) That's a huge part of the submission process. Once you know what folks are looking for the path to approval gets appropriately short :-) > >> 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. > > I completely agree that returns_nonnull has more uses. The warning is > the first one (not sure where to add such a check though), and maybe we > should have a sanitizer option to check the nonnull and returns_nonnull > attributes, although I don't know if that should be in the caller, in > the callee, or both. Well, it seems to me there's both compile-time and runtime (sanitizer) possibilities. I'm much more familiar with the compile-time analysis and can comment on that much more substantially. In a function with the attribute, what we want to know is if any NULL values reach the return statement. Obviously if CCP/VRP/DOM manage to propagate a NULL into a return statement, then we issue a "returns NULL" warning, much like we do for "is used uninitialized". The more interesting case is if a value is defined by a PHI. If a NULL is found in the PHI argument list, then we'd want to use a "may return NULL warning" much like we do for "may be used uninitialized". Note this is true anytime we have a NULL value in a PHI arg and the result of the PHI can ultimately reach a return statement. If set of all values flowing to the return are SSA_NAMEs and range information indicates some might be NULL, then the question is do we issue a warning or not. My inclination would be yes, but we'd want it to be separate from the earlier two cases due to the high noise ratio. Note there is significant overlap with a queued enhancement of mine to warn about potential null pointer dereferences -- which needs to work in precisely the same manner and has the same problems with signal to noise ratios, particularly in the 3rd case. Given that, I'd be happy to pull that patch out of my stash and let you play with it if you're interested. It hasn't been hacked on in a couple years, so it'd need some updating. In terms of exploiting the non-nullness, lots of fun here too. For example, if CCP/VRP/DOM propagate a NULL into a return statement, a program executing that code should be declared as having undefined behaviour. Assuming we do that, then we change the return 0 into a trap. If there is a PHI argument with a NULL value that then feeds a return statement, we should isolate that path by block duplication. Once the path is isolated, it turns into the former case and we apply the same transformation. As it turns out I'm working right now on cleaning up a change to do precisely that for cases where a NULL value feeds into a pointer dereference :-) Finally, backpropagation. Any SSA_NAME that directly or indirectly reaches a return statement in one of these decorated functions has a known non-zero value. It shouldn't be too hard to teach that to VRP which will then use that information to do more aggressive NULL pointer elimination. > > From an optimization POV, I guess there is also the inlining that could > be improved so it doesn't lose the nonnull property. > > The main request I am aware of is PR21856, but I am not familiar enough > with java to handle it. Do you have suggestions of functions which > should get the attribute? memcpy could, but that's really a special case > since it returns its argument, which is itself non-null. The most obvious are things like xmalloc and wrappers around it, probably the GC allocation routines and wrappers around those. Maybe some of the str* and mem* functions as well, I'd have to look at the ISO specs to be sure though. You end up wanting to do a transitive closure on whatever initial set of non-null functions you come up with and any functions which call them and return those results unconditionally. You could even build a warning around that -- ie, "is missing non-null return attribute" kind of warning when you can prove that all values feeding the return statement are non-null by whatever means are available to you. With regard to 21856, the caveat I would mention for that is java as a GCC front-end is becoming less and less important over time. So I'd suggest focusing on how this can be used for the more important languages and if getting the optimization for Java happens in the process, great, but I wouldn't jump through any hoops specifically for java. > > I'll open a new PR about all these. I mostly implemented returns_nonnull > because I'd just done the same for operator new and thus I knew exactly > where the optimization was, I can't promise I'll manage much of the rest. Understood completely. Fine with me. jeff
Marc> + if (flag_delete_null_pointer_checks Marc> + && lookup_attribute ("returns_nonnull", Marc> + TYPE_ATTRIBUTES (TREE_TYPE (fndecl)))) Marc> + return true; I wired all this up to gcj, but it tripped over the flag_delete_null_pointer_checks test, because java/lang.c sets it: /* Java catches NULL pointer exceptions, thus we can not necessarily rely on a pointer having a non-NULL value after a dereference. */ opts->x_flag_delete_null_pointer_checks = 0; svn claims that Jeff wrote this line. Hi Jeff, I'm sure you remember this patch from two years ago in great detail :-). (I did look up the original mail thread but there wasn't really anything there.) My question is, is this really needed? The docs for -fdelete-null-pointer-checks say: In addition, other optimization passes in GCC use this flag to control global dataflow analyses that eliminate useless checks for null pointers; these assume that if a pointer is checked after it has already been dereferenced, it cannot be null. I think the key situation is one where the code has a dereference that is caught, followed by a null pointer check, like: try { x.toString(); } catch (NullPointerException _) { } if (x == null) System.out.println("ok"); I changed java/lang.c to set x_flag_delete_null_pointer_checks and I couldn't make this test case (or a few others) fail. However, I'm not sure if that is because GCC understands that -fnon-call-exceptions means that the dereference can throw, and thus does the right thing; or whether I'm just getting lucky. Tom
On Thu, Oct 10, 2013 at 4:17 PM, Tom Tromey <tromey@redhat.com> wrote: > Marc> + if (flag_delete_null_pointer_checks > Marc> + && lookup_attribute ("returns_nonnull", > Marc> + TYPE_ATTRIBUTES (TREE_TYPE (fndecl)))) > Marc> + return true; > > I wired all this up to gcj, but it tripped over the > flag_delete_null_pointer_checks test, because java/lang.c sets it: > > /* Java catches NULL pointer exceptions, thus we can not necessarily > rely on a pointer having a non-NULL value after a dereference. */ > opts->x_flag_delete_null_pointer_checks = 0; > > svn claims that Jeff wrote this line. Hi Jeff, I'm sure you remember > this patch from two years ago in great detail :-). (I did look up the > original mail thread but there wasn't really anything there.) > > My question is, is this really needed? The docs for > -fdelete-null-pointer-checks say: > > In > addition, other optimization passes in GCC use this flag to > control global dataflow analyses that eliminate useless checks for > null pointers; these assume that if a pointer is checked after it > has already been dereferenced, it cannot be null. > > I think the key situation is one where the code has a dereference that > is caught, followed by a null pointer check, like: > > try { > x.toString(); > } catch (NullPointerException _) { > } > > if (x == null) > System.out.println("ok"); > > I changed java/lang.c to set x_flag_delete_null_pointer_checks and I > couldn't make this test case (or a few others) fail. > > However, I'm not sure if that is because GCC understands that > -fnon-call-exceptions means that the dereference can throw, and thus > does the right thing; or whether I'm just getting lucky. In this case it will notice that if x.toString did not throw x will be not null. But as you catch the exception before the NULL test this information doesn't prevail until the test. Richard. > Tom
On 10/10/13 08:17, Tom Tromey wrote: > In > addition, other optimization passes in GCC use this flag to > control global dataflow analyses that eliminate useless checks for > null pointers; these assume that if a pointer is checked after it > has already been dereferenced, it cannot be null. > > I think the key situation is one where the code has a dereference that > is caught, followed by a null pointer check, like: Right. That's my recollection. > > try { > x.toString(); > } catch (NullPointerException _) { > } > > if (x == null) > System.out.println("ok"); That's my recollection. http://gcc.gnu.org/ml/gcc-patches/2011-11/msg00936.html Starts the thread where it was decided to set that flag for java > > I changed java/lang.c to set x_flag_delete_null_pointer_checks and I > couldn't make this test case (or a few others) fail. > > However, I'm not sure if that is because GCC understands that > -fnon-call-exceptions means that the dereference can throw, and thus > does the right thing; or whether I'm just getting lucky. I think it was done out of an abundance of caution -- I don't think we have ever tripped over this problem in the wild. jeff
Index: c-family/c-common.c =================================================================== --- c-family/c-common.c (revision 203241) +++ c-family/c-common.c (working copy) @@ -364,20 +364,21 @@ static tree handle_warn_unused_result_at bool *); static tree handle_sentinel_attribute (tree *, tree, tree, int, bool *); static tree handle_type_generic_attribute (tree *, tree, tree, int, bool *); static tree handle_alloc_size_attribute (tree *, tree, tree, int, bool *); static tree handle_target_attribute (tree *, tree, tree, int, bool *); static tree handle_optimize_attribute (tree *, tree, tree, int, bool *); static tree ignore_attribute (tree *, tree, tree, int, bool *); static tree handle_no_split_stack_attribute (tree *, tree, tree, int, bool *); static tree handle_fnspec_attribute (tree *, tree, tree, int, bool *); static tree handle_warn_unused_attribute (tree *, tree, tree, int, bool *); +static tree handle_returns_nonnull_attribute (tree *, tree, tree, int, bool *); static void check_function_nonnull (tree, int, tree *); static void check_nonnull_arg (void *, tree, unsigned HOST_WIDE_INT); static bool nonnull_check_p (tree, unsigned HOST_WIDE_INT); static bool get_nonnull_operand (tree, unsigned HOST_WIDE_INT *); static int resort_field_decl_cmp (const void *, const void *); /* Reserved words. The third field is a mask: keywords are disabled if they match the mask. @@ -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 } }; /* Give the specifications for the format attributes, used by C and all descendants. */ const struct attribute_spec c_common_format_attribute_table[] = { /* { name, min_len, max_len, decl_req, type_req, fn_type_req, handler, affects_type_identity } */ @@ -9041,20 +9044,37 @@ handle_no_split_stack_attribute (tree *n } else if (DECL_INITIAL (decl)) { error_at (DECL_SOURCE_LOCATION (decl), "can%'t set %qE attribute after definition", name); *no_add_attrs = true; } return NULL_TREE; } + +/* 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; +} + /* Check for valid arguments being passed to a function with FNTYPE. There are NARGS arguments in the array ARGARRAY. */ void check_function_arguments (const_tree fntype, int nargs, tree *argarray) { /* Check for null being passed in a pointer argument that must be non-null. We also need to do this if format checking is enabled. */ if (warn_nonnull) Index: doc/extend.texi =================================================================== --- doc/extend.texi (revision 203241) +++ doc/extend.texi (working copy) @@ -2126,21 +2126,22 @@ attributes when making a declaration. T attribute specification inside double parentheses. The following attributes are currently defined for functions on all targets: @code{aligned}, @code{alloc_size}, @code{noreturn}, @code{returns_twice}, @code{noinline}, @code{noclone}, @code{always_inline}, @code{flatten}, @code{pure}, @code{const}, @code{nothrow}, @code{sentinel}, @code{format}, @code{format_arg}, @code{no_instrument_function}, @code{no_split_stack}, @code{section}, @code{constructor}, @code{destructor}, @code{used}, @code{unused}, @code{deprecated}, @code{weak}, @code{malloc}, @code{alias}, @code{ifunc}, -@code{warn_unused_result}, @code{nonnull}, @code{gnu_inline}, +@code{warn_unused_result}, @code{nonnull}, +@code{returns_nonnull}, @code{gnu_inline}, @code{externally_visible}, @code{hot}, @code{cold}, @code{artificial}, @code{no_sanitize_address}, @code{no_address_safety_analysis}, @code{no_sanitize_undefined}, @code{error} and @code{warning}. Several other attributes are defined for functions on particular target systems. Other attributes, including @code{section} are supported for variables declarations (@pxref{Variable Attributes}) and for types (@pxref{Type Attributes}). GCC plugins may provide their own attributes. @@ -3302,20 +3303,34 @@ on the knowledge that certain function a If no argument index list is given to the @code{nonnull} attribute, all pointer arguments are marked as non-null. To illustrate, the following declaration is equivalent to the previous example: @smallexample extern void * my_memcpy (void *dest, const void *src, size_t len) __attribute__((nonnull)); @end smallexample +@item returns_nonnull (@var{arg-index}, @dots{}) +@cindex @code{returns_nonnull} function attribute +The @code{returns_nonnull} attribute specifies that the function +return value should be a non-null pointer. For instance, the declaration: + +@smallexample +extern void * +mymalloc (size_t len) __attribute__((returns_nonnull)); +@end smallexample + +@noindent +lets the compiler optimize callers based on the knowledge +that the return value will never be null. + @item noreturn @cindex @code{noreturn} function attribute A few standard library functions, such as @code{abort} and @code{exit}, cannot return. GCC knows this automatically. Some programs define their own functions that never return. You can declare them @code{noreturn} to tell the compiler this fact. For example, @smallexample @group void fatal () __attribute__ ((noreturn)); Index: fold-const.c =================================================================== --- fold-const.c (revision 203241) +++ fold-const.c (working copy) @@ -16222,20 +16222,24 @@ tree_expr_nonzero_warnv_p (tree t, bool strict_overflow_p); case CALL_EXPR: { tree fndecl = get_callee_fndecl (t); if (!fndecl) return false; if (flag_delete_null_pointer_checks && !flag_check_new && DECL_IS_OPERATOR_NEW (fndecl) && !TREE_NOTHROW (fndecl)) return true; + if (flag_delete_null_pointer_checks + && lookup_attribute ("returns_nonnull", + TYPE_ATTRIBUTES (TREE_TYPE (fndecl)))) + return true; return alloca_call_p (t); } default: break; } return false; } /* Return true when T is an address and is known to be nonzero. Index: testsuite/c-c++-common/pr20318.c =================================================================== --- testsuite/c-c++-common/pr20318.c (revision 0) +++ testsuite/c-c++-common/pr20318.c (working copy) @@ -0,0 +1,3 @@ +/* { dg-do compile } */ + +extern int f() __attribute__((returns_nonnull)); /* { dg-error "not returning a pointer" } */ Property changes on: testsuite/c-c++-common/pr20318.c ___________________________________________________________________ Added: svn:eol-style ## -0,0 +1 ## +native \ No newline at end of property Added: svn:keywords ## -0,0 +1 ## +Author Date Id Revision URL \ No newline at end of property 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" } } */ Property changes on: testsuite/gcc.dg/tree-ssa/pr20318.c ___________________________________________________________________ Added: svn:keywords ## -0,0 +1 ## +Author Date Id Revision URL \ No newline at end of property Added: svn:eol-style ## -0,0 +1 ## +native \ No newline at end of property Index: tree-vrp.c =================================================================== --- tree-vrp.c (revision 203241) +++ tree-vrp.c (working copy) @@ -1031,40 +1031,44 @@ gimple_assign_nonzero_warnv_p (gimple st case GIMPLE_SINGLE_RHS: return tree_single_nonzero_warnv_p (gimple_assign_rhs1 (stmt), strict_overflow_p); case GIMPLE_INVALID_RHS: gcc_unreachable (); default: gcc_unreachable (); } } -/* Return true if STMT is know to to compute a non-zero value. +/* Return true if STMT is known to compute a non-zero value. If the return value is based on the assumption that signed overflow is undefined, set *STRICT_OVERFLOW_P to true; otherwise, don't change *STRICT_OVERFLOW_P.*/ static bool gimple_stmt_nonzero_warnv_p (gimple stmt, bool *strict_overflow_p) { switch (gimple_code (stmt)) { case GIMPLE_ASSIGN: return gimple_assign_nonzero_warnv_p (stmt, strict_overflow_p); case GIMPLE_CALL: { tree fndecl = gimple_call_fndecl (stmt); if (!fndecl) return false; if (flag_delete_null_pointer_checks && !flag_check_new && DECL_IS_OPERATOR_NEW (fndecl) && !TREE_NOTHROW (fndecl)) return true; + if (flag_delete_null_pointer_checks && + lookup_attribute ("returns_nonnull", + TYPE_ATTRIBUTES (gimple_call_fntype (stmt)))) + return true; return gimple_alloca_call_p (stmt); } default: gcc_unreachable (); } } /* Like tree_expr_nonzero_warnv_p, but this function uses value ranges obtained so far. */ @@ -6489,24 +6493,21 @@ stmt_interesting_for_vrp (gimple stmt) else if (is_gimple_assign (stmt) || is_gimple_call (stmt)) { tree lhs = gimple_get_lhs (stmt); /* In general, assignments with virtual operands are not useful for deriving ranges, with the obvious exception of calls to builtin functions. */ if (lhs && TREE_CODE (lhs) == SSA_NAME && (INTEGRAL_TYPE_P (TREE_TYPE (lhs)) || POINTER_TYPE_P (TREE_TYPE (lhs))) - && ((is_gimple_call (stmt) - && gimple_call_fndecl (stmt) != NULL_TREE - && (DECL_BUILT_IN (gimple_call_fndecl (stmt)) - || DECL_IS_OPERATOR_NEW (gimple_call_fndecl (stmt)))) + && (is_gimple_call (stmt) || !gimple_vuse (stmt))) return true; } else if (gimple_code (stmt) == GIMPLE_COND || gimple_code (stmt) == GIMPLE_SWITCH) return true; return false; }