Re: [PATCH 2/6] Add returns_zero_on_success/failure attributes

[David Malcolm <dmalcolm@redhat.com>]

On Mon, 2021-11-15 at 15:45 +0100, Peter Zijlstra wrote:
> On Mon, Nov 15, 2021 at 12:33:16PM +0530, Prathamesh Kulkarni wrote:
> > On Sun, 14 Nov 2021 at 02:07, David Malcolm via Gcc-patches
> 
> > > +/* Handle "returns_zero_on_failure" and "returns_zero_on_success"
> > > attributes;
> > > +   arguments as in struct attribute_spec.handler.  */
> > > +
> > > +static tree
> > > +handle_returns_zero_on_attributes (tree *node, tree name, tree,
> > > int,
> > > +                                  bool *no_add_attrs)
> > > +{
> > > +  if (!INTEGRAL_TYPE_P (TREE_TYPE (*node)))
> > > +    {
> > > +      error ("%qE attribute on a function not returning an
> > > integral type",
> > > +            name);
> > > +      *no_add_attrs = true;
> > > +    }
> > > +  return NULL_TREE;
> > Hi David,
> > Just curious if a warning should be emitted if the function is marked
> > with the attribute but it's return value isn't actually 0 ?
> > 
> > There are other constants like -1 or 1 that are often used to
> > indicate
> > error, so maybe tweak the attribute to
> > take the integer as an argument ?
> > Sth like returns_int_on_success(cst) / returns_int_on_failure(cst) ?
> > 
> > Also, would it make sense to extend it for pointers too for returning
> > NULL on success / failure ?
> 
> Please also consider that in Linux we use the 'last' page for error
> code
> returns. That is, a function returning a pointer could return '(void
> *)-EFAULT' also see linux/err.h
> 

Thanks.

Am I right in thinking that such functions return non-NULL, giving
something like:

  __attribute__((returns_ptr_in_range_on_success (0x1, NULL - 4096)))
  __attribute__((returns_ptr_in_range_on_failure (NULL - 4096, NULL - 1)))
  __attribute__((returns_non_null))

as attributes?  (I have no idea if the above will parse, and I admit
these look ugly as-is, though I suppose they could be hidden behind a
macro).

Looking at include/linux/err.h I see functions:

static inline bool __must_check IS_ERR(__force const void *ptr)
{
	return IS_ERR_VALUE((unsigned long)ptr);
}

static inline bool __must_check IS_ERR_OR_NULL(__force const void *ptr)
{
	return unlikely(!ptr) || IS_ERR_VALUE((unsigned long)ptr);
}

so maybe attribute could refer to predicate functions, something like
this:

  __attribute__((return_value_success_predicate(FUNCTION_DECL)))
  __attribute__((return_value_failure_predicate(FUNCTION_DECL)))

where this case could use something like:

  __attribute__((return_value_failure_predicate(IS_ERR)))

to express the idea "this function can succeed or fail, and the given
function decl expresses whether a given return value is a failure" - or
somesuch.  The predicate function would probably have to be pure.

Obviously I'm just brainstorming here; as noted in my reply to
Prathamesh, all I need for the initial implementation of the trust
boundary work is just being able to express that zero vs non-zero
return is the success vs failure condition for a function.

Dave