[cfe-dev] Prevent RetainCountChecker from Analyzing Function Bodies when They Have Certain Annotate Attribute

[Malhar Thakkar via cfe-dev]

I've been thinking a lot lately and I feel evalCall-ing is not the right
way to proceed (even though it works as desired) as I feel I won't be able
to generalize it to work for different kinds of codebases written in C. For
example, it won't work for the following case (as you pointed out before).

*// evalCall-ing based on annotation*

#include <stdlib.h>
typedef struct
{
int ref_count;
} rc_struct;
void free(void *);

__attribute__((annotate("rc_ownership_trusted_implementation"))) rc_struct
*bar(rc_struct *r) {
  if (!r)
    return NULL;

  if (--r->ref_count > 0)
    return NULL;

  free(r);
  return NULL;
}

void foo() {
rc_struct *r = (rc_struct *)malloc(sizeof(rc_struct));
bar(r);
} *// Leak warning raised for 'r'*


Is it possible to check if the RetainCountChecker entered a function
containing a certain annotation by walking through the diagnostic path just
before emitting a bug report?


On Fri, Jul 7, 2017 at 6:33 PM, Malhar Thakkar <cs13b1031 at iith.ac.in> wrote:

>
>
> On Fri, Jul 7, 2017 at 3:37 PM, Artem Dergachev <noqnoqneo at gmail.com>
> wrote:
>
>> For now RetainCountChecker avoids inlining CoreFoundation's CFRetain() by
>> looking at its annotation and performing evalCall().
>
> Oh, I see.
>
>> evalCall is the checker callback that allows the checker to completely
>> model the function's effects; if the function is evalCall-ed by the
>> checker, the analyzer core doesn't try to inline it. I think this is
>> exactly what you need to do: the function you're modeling is *the* release,
>> and the analyzer doesn't need to know anything about how it is implemented.
>>
> Yes, I think, this is exactly what I need but I just read that only one
> checker can evaluate a call at a given instance and that, as you mentioned
> with your example containing MallocChecker, can cause some problems. I just
> tried evalCall-ing functions which contain a certain annotation and it
> seems to be working on the test-cases which I wrote. But, would that
> solution be acceptable to the community(i.e., commit-worthy)? If not, I'll
> have to think of something else.
>
>>
>> However, by evalCall-ing based on annotation, you essentially say that
>> all functions that wear this annotation are to be modeled similarly. If
>> some of them have additional side effects, you may fail to model them. For
>> example, if the analyzer has seen the malloc() that corresponds to the
>> free() in isl_basic_map_free(), it'd make MallocChecker unhappy - unlike
>> the case when you neither evalCall nor inline but evaluate conservatively,
>> causing the malloc-ed pointer to "escape" into this function.
>
> What exactly do you mean by evaluating conservatively?

> If this becomes a problem, you may need to look into our body farms - the
>> mechanism that synthesizes ASTs of various functions for analysis purposes.
>> I imagine farming a function that only does releases, and then farming more
>> functions that call this function and do additional side effects.
>
> How would farming a function work exactly? Also, the ISL codebase contains
a lot of different functions (one for each ISL data type) which perform
releases. So, farming functions for each of them doesn't seem like a good
option.

>
>>
>> On 7/7/17 10:23 AM, Malhar Thakkar via cfe-dev wrote:
>>
>>> Dear all,
>>>
>>> I wish to prevent the RetainCountChecker from analyzing function bodies
>>> if these functions have certain annotate attributes. Consider the following
>>> example to get a better idea of why I wish to do that.
>>>
>>> Below is a small snippet from the Integer Set Library (ISL).
>>>
>>> typedef struct {
>>>   int ref;
>>> } isl_basic_map;
>>>
>>> __attribute__((cf_returns_retained))
>>> isl_basic_map *isl_basic_map_copy(isl_basic_map *bmap);
>>>
>>> __attribute__((cf_returns_retained))
>>> isl_basic_map *isl_basic_map_cow
>>> (__attribute__((cf_consumed)) isl_basic_map *bmap);
>>>
>>> void free(void *);
>>>
>>> __attribute__((annotate("rc_ownership_trusted_implementation")))
>>> isl_basic_map *isl_basic_map_free
>>> (__attribute__((cf_consumed)) isl_basic_map *bmap) {
>>> if (!bmap)
>>> return NULL;
>>>
>>> if (--bmap->ref > 0)
>>> return NULL;
>>>
>>>   free(bmap);
>>> return NULL;
>>> }
>>>
>>> __attribute__((cf_returns_retained))
>>> isl_basic_map *foo
>>> (__attribute__((cf_consumed)) isl_basic_map *bmap) {
>>> // *After this call, 'temp' has a +1 reference count.*
>>>   isl_basic_map *temp = isl_basic_map_copy(bmap);
>>> // *After this call, 'bmap' has a +1 reference count.*
>>>   bmap = isl_basic_map_cow(bmap);
>>> // *After this call, assuming the predicate of the second if branch to
>>> be true, 'bmap' has a +1 reference count.*
>>> isl_basic_map_free(bmap);
>>> return temp; *// Object leaked: 'bmap'*
>>> }
>>>
>>> While running the RetainCountChecker on the above example, it raises a
>>> leak warning for 'bmap' in function 'foo'. This warning is a true positive
>>> from the checker's perspective in the sense that the reference count of
>>> 'bmap' obtained from 'isl_basic_map_cow' is not decremented(from the
>>> checker's perspective) in 'isl_basic_map_free' even though it takes the
>>> argument 'bmap' as '__attribute__((cf_consumed))'.
>>>
>>> Actually, '--bmap->ref' does decrement the reference count (from ISL's
>>> perspective). Hence, to prevent such false positives (from ISL's
>>> perspective) to be raised, I wish to prevent the RetainCountChecker to
>>> analyze the bodies of the functions having 'rc_ownership_trusted_implementation'
>>> annotate attribute. I want the checker to just look at the declaration of
>>> such functions (and not go inside their bodies) to get the necessary
>>> information about reference counting.
>>>
>>> Could someone suggest me a way to achieve my objective?
>>>
>>>
>>> Thank you.
>>>
>>>
>>> Regards,
>>> Malhar Thakkar
>>> ᐧ
>>>
>>>
>>> _______________________________________________
>>> cfe-dev mailing list
>>> cfe-dev at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>>>
>>
>>
> ᐧ
>

ᐧ
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20170710/964efc66/attachment.html>