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

Tue Jul 11 14:38:00 PDT 2017

Hum... I am confused.

Note that you didn't put any attribute on the return value, which should
forbid any use / analysis.

Maybe we should look into what the reference counter count.

After some reflexion, I came to the conclusion that it should never count
to more than 1 as it should not know that obj_copy returns it's argument.
And that is fine!

On Jul 11, 2017 9:36 PM, "Devin Coughlin via cfe-dev" <
cfe-dev at lists.llvm.org> wrote:

>
> On Jul 11, 2017, at 1:12 PM, Malhar Thakkar via cfe-dev <
> cfe-dev at lists.llvm.org> wrote:
>
>
>
> On Tue, Jul 11, 2017 at 10:49 PM, Devin Coughlin <devin.coughlin at gmail.com
> > wrote:
>
>>
>> On Jul 11, 2017, at 5:02 AM, Malhar Thakkar via cfe-dev <
>> cfe-dev at lists.llvm.org> wrote:
>>
>>
>>
>> On Tue, Jul 11, 2017 at 5:03 PM, Sven Verdoolaege <skimo-cfe at kotnet.org>
>> wrote:
>>
>>> On Tue, Jul 11, 2017 at 10:33:06AM +0530, Malhar Thakkar wrote:
>>> > Hence, although evalCall() works perfectly for ISL, we may not be able
>>> to
>>> > generalize it for other C codebases.
>>>
>>> I think it's reasonable to assume that frameworks that shield off
>>> free for reference counting, would also shield off malloc
>>> in order to initialize the reference counting.
>>> Of course, this may just be a lack of imagination on my part.
>>> Do you have any examples of frameworks that could use your
>>> annotations where this is not the case?
>>>
>> Well, I haven't come across/thought of any such codebase which doesn't
>> shield off malloc which is why I created a hypothetical test case. Now that
>> you mention it, it does seem reasonable to assume that frameworks would
>> shield off malloc as well.
>>
>>
>> There are some frameworks/idioms that allow a transfer of ownership from
>> a raw malloc’d pointer to a referenced-counted opaque pointer container
>> (this is common in C++ where you use new to allocate and initialize an
>> object). However, I think the ‘trusted implementation’ annotation could be
>> extended to handle this as well. It would require the function that
>> transfers ownership to the reference-counted implementation to be
>> annotated, but I don’t think that is a high burden. That said, I wouldn’t
>> worry about raw malloc’d pointer case for now.
>>
>>
>> Also, keeping MallocChecker aside, there are a lot of other checkers
>> which may create some issues if there are additional side-effects (as Dr.
>> Artem mentioned) in such annotated functions. However, I guess it may be
>> safe to assume that there are no such additional side-effects in such
>> "trusted" functions.
>>
>>
>> I suspect that many of these side effects would have high-level
>> invariants that the analyzer on its own wouldn’t be able to discover (for
>> example, that a set created with a single item in it is not empty) — so
>> custom modeling would be needed in any event, even if the calls were
>> inlined.
>>
>> Devin
>>
>>
>>
>>> skimo
>>>
>>
>> Now, I was experimenting a bit more with evalCall-ing based on
>> annotations and although this works like a charm for functions of the type
>> obj_free() and obj_cow(), it is unable to avoid the problems created by
>> obj_copy(). This is probably because of the lack of a core-foundation
>> annotation which is analogous to isl_keep.
>> Consider the following example.
>>
>>
>> #define __isl_give __attribute__((cf_returns_retained))
>> #define __isl_take __attribute__((cf_consumed))
>> #define __isl_null
>> #define __isl_keep
>> #define NULL 0
>>
>> typedef struct
>> {
>>   int ref;
>> } isl_basic_map;
>>
>> __isl_give isl_basic_map *isl_basic_map_cow(__isl_take isl_basic_map
>> *bmap);
>> __isl_null isl_basic_map *isl_basic_map_free(__isl_take isl_basic_map
>> *bmap);
>>
>> __attribute__((annotate("rc_ownership_trusted_implementation")))
>> __isl_give isl_basic_map *isl_basic_map_copy(__isl_keep isl_basic_map *bmap)
>> {
>>   if (!bmap)
>>     return NULL;
>>
>>   bmap->ref++;
>>   return bmap;
>> }
>>
>> void test_use_after_release_with_trusted_implementation_annotate_attribute(__isl_take
>> isl_basic_map *bmap) {
>>   bmap = isl_basic_map_cow(bmap);
>>   isl_basic_map *temp = isl_basic_map_cow(isl_basic_map_copy(bmap)); *//
>> Here, the analyzer states "Object released".*
>>   isl_basic_map *temp2 = isl_basic_map_cow(bmap); *// Use-after-release
>> for 'bmap' raised here.*
>>   isl_basic_map_free(temp2);
>>   isl_basic_map_free(temp);
>> }
>>
>>
>> I don’t understand what is going on in the above. Can you please simplify
>> the example and explain what is going on and what you expected the analyzer
>> to do? What call is the analyzer complaining about? Is it inlining the
>> call? Please present the simplest example that illustrates the problem.
>>
>
>
> #define __isl_keep
> isl_basic_map *foo(__isl_keep isl_basic_map *bmap);
>
> Consider the above code snippet. Now, I feel that when a parameter has no
> annotation associated to it in some function (foo, in this case), the
> analyzer assumes that the object returned by that function (foo) points to
> the same memory location as that pointed to by the argument passed to it
> (bmap). Please correct me if I'm wrong.
>
>
> The analyzer shouldn’t be doing this when the foo is not inlined. If foo
> is inlined it should only do this if foo() returns its single parameter.
> Is foo() being inlined, or not? If so, what is its body? Is this when
> evalCall is used, or not?
>
> This is probably the reason why, in the example specified in my previous
> email, the analyzer decrements the reference count for 'bmap' when the
> object returned from 'isl_basic_map_copy' is passed as __isl_take
> (cf_consumed) to 'isl_basic_map_cow'. This in turn results in a
> 'use-after-release' warning on the next line.
>
>
> It would be good to determine whether this is actually the reason why. You
> will probably need to do some debugging here. You might find it helpful to
> visualize the exploded node graph. There are docs on how to do this at <
> https://clang-analyzer.llvm.org/checker_dev_manual.html#commands>.
>
> Devin
>
>
> _______________________________________________
> 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/20170711/7b0874e8/attachment.html>