[cfe-dev] RFC: static analysis, malloc annontation for return via pointer argument

[Anna Zaks]

It feels like attributes are not strong enough to represent the complex relationships in the general case. Even if we managed to make it work, the annotations would look non-intuitive. It would be great to have a more general system to summarize the effects of the functions/represent pre and post conditions.

On Dec 18, 2012, at 12:13 PM, Scott Parlane wrote:

> Hi Ted,
> 
> I believe it should take 3 parameters.
> - which argument is being used to return
> - whether the ret value and pointer are associated
> (and how, invalid if < 0, != 0 , 0)
> - what the size is (this is complicated)
> 
> lets take the case of asprintf()
> return argument is 1, ret == -1 is invalid,
> size is (ret+1) * sizeof(pointeeetype)
> 
> now the api from our codebase we are trying to analyze
> int avl_create (struct avl_tree **avl_tree, int max_nodes, int (*compare_function) (void *data1, void *data2))
> return argument is 1, ret != 0 is invalid,
> size is fixed at sizeof(struct avl_tree)
> also, it will leak if not free'd with the correct function, however we
> can deal with that issue separately.
> 

I think specifying a custom free function is very important (even more than the size) since this is what the malloc checker is supposed to check for. This could be solved by specifying the corresponding free function's name in the attribute. 

> I think if I looked I would find more examples.
> 
> I guess the annotation should take 3 parameters, how would I represent
> the ret value to validness binding ?

You could probably use a separate annotation to represent the fact that the function "succeeded" or "failed". This is not as generic since it would not be related to the specific malloc attribute, but could work well in practice.

> and similarly for the size ? 
> 
> Can an annotation have an expression in it ?

> If so, something like (for the asprintf case)
> ownership_returns_byref(malloc, 1, ==-1, (ret+1) * sizeof)
> and (for the avl_create case)
> ownership_returns_byref(malloc, 1, !=0, sizeof)
> 

As for the name, maybe it could be possible to reuse "ownership_returns" attribute and rely on the argument to differentiate between returns as the return value from the function vs returns by ptr/reference through one of the arguments?

> Regards,
> Scott
> 
> 
> On Tue, 2012-12-18 at 11:33 -0800, Ted Kremenek wrote:
>> Hi Scott,
>> 
>> I've thought about this patch some more, and I have some questions.  What are the intended semantics of this attribute?  With the other attributes its clearer.  If a function is annotated as returning an owned pointer, it's clear that if it is non-NULL then it is an owned pointer.  But what's the case here?  Can a function decide not to return a value at all (e.g., on failure)?  How would that be captured by the attribute?
>> 
>> Thanks,
>> Ted
>> 
>> On Dec 17, 2012, at 1:55 PM, Scott Parlane <scott at scottnz.com> wrote:
>> 
>>> Hi All,
>>> 
>>> This patch allows for APIs were memory is allocated and placed in a
>>> pointer given to them. (like asprintf, but without the realloc feature)
>>> 
>>> Currently, when using a pointer to a stack variable as the input to the
>>> function, it declares the memory leak on the next source line after the
>>> stack variable is used. I think it should be declaring the leak on the
>>> last line of the current scope. Which is correct ?
>>> 
>>> Regards,
>>> Scott
>>> <0001-Make-clang-static-analysis-support-allocation-into-a.patch>_______________________________________________
>>> cfe-dev mailing list
>>> cfe-dev at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>> 
> 
> 
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev