<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, May 30, 2017 at 6:47 AM, Artem Dergachev <span dir="ltr"><<a href="mailto:noqnoqneo@gmail.com" target="_blank">noqnoqneo@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">+Devin.<br>
<br>
Yep, this behavior can be improved quite a bit. It's indeed overly conservative to believe that a single escaping pointer argument would cause other const pointer arguments' pointed-to regions invalidated.<br>
<br>
argumentsMayEscape() should still be relied upon, it seems, because it contains a nice blacklist of weird functions, eg. <a href="https://stackoverflow.com/questions/7726329/what-is-the-purpose-of-const-pointer-to-void/7726359" rel="noreferrer" target="_blank">https://stackoverflow.com/ques<wbr>tions/7726329/what-is-the-purp<wbr>ose-of-const-pointer-to-void/<wbr>7726359</a><br>
<br>
However, making argumentsMayEscape() more fine-grained, eg. letting it provide a list of arguments that may escape, should be quite an improvement.<br>
<br>
Additionally, when structures are passed into functions, we could consider const-qualifiers on pointer fields in these structures and avoid invalidating through pointers stored in const-pointer fields - might be an improvement as well. Direct parameters are not special.<br>
<br>
Possible aliasing between arguments should still be taken into account, eg. tests with testMixedConstNonConstCalls() added in <a href="https://reviews.llvm.org/D19057" rel="noreferrer" target="_blank">https://reviews.llvm.org/D1905<wbr>7</a> should remain "UNKNOWN". So i'd imagine a two-pass procedure, where on the first pass we mark base regions of all const pointers passed into the function with TK_PreserveContents, and on the second pass we remove TK_PreserveContents from base regions of non-const pointers.<br>
<br>
Finally, i agree that argumentsMayEscape() makes sense for plain C system library functions only. It should ideally check that the function is within the standard library, and is a plain-C function, and avoid jumping to conclusions if this is not the case.</blockquote><div> </div><div>We do rely on argumentsMayEscape() to support other languages, specifically, there is ObjC specific code. I suspect, we would benefit from reasoning about C++ as well, but currently, C++ method decls overloads are not implemented. I suspect we need to do more investigation on libc++.<br></div><div><br></div><div>As Alexey pointed out, argumentsMayEscape() is used/called mainly after we know that the function comes from the system header. I do not recall if there are cases where it is needed for non-system calls, but we could envision having heuristics and treating some non-system calls with specific names the same way we treat system calls. </div><div><br></div><div>Note, that the implementation itself does include generic heuristics, applicable to any functions, not just system functions. For example, it call:</div><div> /// \brief Returns true if any of the arguments appear to represent callbacks.</div><div>  bool hasNonZeroCallbackArg() const;</div><div><br></div><div>Further investigation would be great here as that code is somewhat convoluted!<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div class="gmail-HOEnZb"><div class="gmail-h5">
<br>
25/05/2017 8:07 PM, Aleksei Sidorin wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">
Hi all,<br>
<br>
I have some questions about CSA invalidation behaviour for the case where some arguments can escape after call.<br>
<br>
1. There is a condition in CallEvent::invalidateRegions()<wbr>:<br>
<br>
  if (!argumentsMayEscape())<br>
    findPtrToConstParams(PreserveA<wbr>rgs, *this);<br>
<br>
The contents of PreserveArgs changed by findPtrToConstParams() is used later for setting a special invalidation trait for its items: TK_PreserveContents. But, as I understand, if some pointer passed to function can escape, all the pointers passed to function get invalidated independently on can they escape or not. Why we don't just filter the escaping regions and invalidate them but invalidate all the pointers instead?<br>
<br>
2. For AnyFunctionCall, we think that void* arguments of can escape:<br>
<br>
  if (CallEvent::argumentsMayEscape<wbr>() || hasVoidPointerToNonConstArg())<br>
    return true;<br>
<br>
But because of (1), this means that all other pointers passed to such function (including pointers to const) are invalidated. Checkers that use argumentsMayEscape() method explicitly check that the call is located in system header. So, should we move the check for system header into argumentsMayEscape()? It looks like the commit that introduced this behaviour was targeting system header functions only. And should we avoid the invalidation of pointers to constant memory if some pointer argument can escape?<br>
<br>
</blockquote>
<br>
</div></div></blockquote></div><br></div></div>