<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Jul 11, 2017, at 1:12 PM, Malhar Thakkar via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org" class="">cfe-dev@lists.llvm.org</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class=""><br class=""><div class="gmail_extra"><br class=""><div class="gmail_quote">On Tue, Jul 11, 2017 at 10:49 PM, Devin Coughlin <span dir="ltr" class=""><<a href="mailto:devin.coughlin@gmail.com" target="_blank" class="">devin.coughlin@gmail.com</a>></span> wrote:<br class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class=""><br class=""><div class=""><span class=""><blockquote type="cite" class=""><div class="">On Jul 11, 2017, at 5:02 AM, Malhar Thakkar via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org" target="_blank" class="">cfe-dev@lists.llvm.org</a>> wrote:</div><br class="m_4204122371872823108Apple-interchange-newline"><div class=""><div dir="ltr" class=""><br class=""><div class="gmail_extra"><br class=""><div class="gmail_quote">On Tue, Jul 11, 2017 at 5:03 PM, Sven Verdoolaege <span dir="ltr" class=""><<a href="mailto:skimo-cfe@kotnet.org" target="_blank" class="">skimo-cfe@kotnet.org</a>></span> wrote:<br class=""><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="m_4204122371872823108gmail-">On Tue, Jul 11, 2017 at 10:33:06AM +0530, Malhar Thakkar wrote:<br class="">
> Hence, although evalCall() works perfectly for ISL, we may not be able to<br class="">
> generalize it for other C codebases.<br class="">
<br class="">
</span>I think it's reasonable to assume that frameworks that shield off<br class="">
free for reference counting, would also shield off malloc<br class="">
in order to initialize the reference counting.<br class="">
Of course, this may just be a lack of imagination on my part.<br class="">
Do you have any examples of frameworks that could use your<br class="">
annotations where this is not the case?<br class=""></blockquote><div class="">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. </div></div></div></div></div></blockquote><div class=""><br class=""></div></span><div class="">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.</div><span class=""><div class=""><br class=""></div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_extra"><div class="gmail_quote"><div class="">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.</div></div></div></div></div></blockquote><div class=""><br class=""></div></span><div class="">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.</div><div class=""><br class=""></div><div class="">Devin</div><span class=""><div class=""><br class=""></div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br class="">
skimo<br class=""></blockquote><div class=""><br class=""></div><div class="">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.<br class=""></div><div class="">Consider the following example.</div><div class=""><br class=""></div><div class=""><div class=""></div><br class=""><div class=""><div class="m_4204122371872823108gmail-hljs m_4204122371872823108gmail-cpp" style="display:block;overflow-x:auto;padding:0.5em;background:rgb(248,248,248)"><div class=""><font color="#1f7199" face="monospace" class="">#define __isl_give __attribute__((cf_returns_<wbr class="">retained))</font></div><div class=""><font color="#1f7199" face="monospace" class="">#define __isl_take __attribute__((cf_consumed))</font></div><div class=""><font color="#1f7199" face="monospace" class="">#define __isl_null</font></div><div class=""><font color="#1f7199" face="monospace" class="">#define __isl_keep</font></div><div class=""><font color="#1f7199" face="monospace" class="">#define NULL 0</font></div><div class=""><font color="#1f7199" face="monospace" class=""><br class=""></font></div><div class=""><font color="#1f7199" face="monospace" class="">typedef struct</font></div><div class=""><font color="#1f7199" face="monospace" class="">{</font></div><div class=""><font color="#1f7199" face="monospace" class=""> int ref;</font></div><div class=""><font color="#1f7199" face="monospace" class="">} isl_basic_map;</font></div><div class=""><font color="#1f7199" face="monospace" class=""><br class=""></font></div><div class=""><font color="#1f7199" face="monospace" class="">__isl_give isl_basic_map *isl_basic_map_cow(__isl_take isl_basic_map *bmap);</font></div><div class=""><font color="#1f7199" face="monospace" class="">__isl_null isl_basic_map *isl_basic_map_free(__isl_take isl_basic_map *bmap);</font></div><div class=""><font color="#1f7199" face="monospace" class=""><br class=""></font></div><div class=""><font color="#1f7199" face="monospace" class="">__attribute__((annotate("rc_<wbr class="">ownership_trusted_<wbr class="">implementation"))) __isl_give isl_basic_map *isl_basic_map_copy(__isl_keep isl_basic_map *bmap)</font></div><div class=""><font color="#1f7199" face="monospace" class="">{</font></div><div class=""><font color="#1f7199" face="monospace" class=""> if (!bmap)</font></div><div class=""><font color="#1f7199" face="monospace" class=""> return NULL;</font></div><div class=""><font color="#1f7199" face="monospace" class=""><br class=""></font></div><div class=""><font color="#1f7199" face="monospace" class=""> bmap->ref++;</font></div><div class=""><font color="#1f7199" face="monospace" class=""> return bmap;</font></div><div class=""><font color="#1f7199" face="monospace" class="">}</font></div><div class=""><font color="#1f7199" face="monospace" class=""><br class=""></font></div><div class=""><font color="#1f7199" face="monospace" class="">void test_use_after_release_with_<wbr class="">trusted_implementation_<wbr class="">annotate_attribute(__isl_take isl_basic_map *bmap) {</font></div><div class=""><font color="#1f7199" face="monospace" class=""> bmap = isl_basic_map_cow(bmap);</font></div><div class=""><font color="#1f7199" face="monospace" class=""> isl_basic_map *temp = isl_basic_map_cow(isl_basic_<wbr class="">map_copy(bmap)); <b class="">// Here, the analyzer states "Object released".</b></font></div><div class=""><font color="#1f7199" face="monospace" class=""> isl_basic_map *temp2 = isl_basic_map_cow(bmap); <b class="">// Use-after-release for 'bmap' raised here.</b></font></div><div class=""><font color="#1f7199" face="monospace" class=""> isl_basic_map_free(temp2);</font></div><div class=""><font color="#1f7199" face="monospace" class=""> isl_basic_map_free(temp);</font></div><div class=""><font color="#1f7199" face="monospace" class="">}</font></div></div></div></div></div></div></div></div></blockquote><div class=""><br class=""></div></span><div class="">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.</div></div></div></blockquote><div class=""><br class=""></div><div class=""></div><br class=""><div class=""><div class="cpp hljs" style="display:block;overflow-x:auto;padding:0.5em;color:rgb(51,51,51);background:rgb(248,248,248);font-family:monospace"><div class=""><span class="hljs-meta" style="color:rgb(31,113,153)">#<span class="hljs-meta-keyword" style="font-weight:bold">define</span> __isl_keep</span></div><div class=""><span class="hljs-meta" style="color:rgb(31,113,153)">isl_basic_map *foo(__isl_keep isl_basic_map *bmap);</span></div></div></div><div class="gmail_quote"><br class=""></div>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.</div></div></div></div></blockquote><div><br class=""></div><div class="">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?</div><div class=""><br class=""></div><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_extra"><div class="gmail_quote">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.<br class=""></div></div></div></div></blockquote><div><br class=""></div><div>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 <<a href="https://clang-analyzer.llvm.org/checker_dev_manual.html#commands" class="">https://clang-analyzer.llvm.org/checker_dev_manual.html#commands</a>>.</div><div><br class=""></div><div>Devin</div><div><br class=""></div></div></body></html>