<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Oct 1, 2013, at 9:40 , Jordan Rose <<a href="mailto:jordan_rose@apple.com">jordan_rose@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><meta http-equiv="Content-Type" content="text/html charset=windows-1252"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div>[+cfe-commits, which I didn't really mean to remove]</div><br><div><div>On Sep 27, 2013, at 1:01 , Richard <<a href="mailto:tarka.t.otter@gmail.com">tarka.t.otter@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><meta http-equiv="Content-Type" content="text/html charset=windows-1252"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On 25 Sep 2013, at 18:44, Jordan Rose <<a href="mailto:jordan_rose@apple.com">jordan_rose@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><meta http-equiv="Content-Type" content="text/html charset=windows-1252"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Sep 23, 2013, at 4:38 , Richard <<a href="mailto:tarka.t.otter@gmail.com">tarka.t.otter@gmail.com</a>> wrote:</div><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><br></div>This sounds exactly what is required. I have this mostly working, except for a bug that I cannot seem to track down. I have implemented the event dispatch in the CallAndMessageChecker as:<div><div style="margin: 0px; font-size: 14px; font-family: Monaco; min-height: 19px; "><br></div><div style="margin: 0px; font-size: 14px; font-family: Monaco; min-height: 19px; ">{<span class="Apple-tab-span" style="white-space:pre"> </span></div><div style="margin: 0px; font-size: 14px; font-family: Monaco; min-height: 19px; "> ...</div><div style="margin: 0px; font-size: 14px; font-family: Monaco; "> <span style="color: #bb2ca2">if</span> (StNull && !StNonNull) {</div><div style="margin: 0px; font-size: 14px; font-family: Monaco; "> <span style="color: #bb2ca2">if</span> (!BT_call_null)</div><div style="margin: 0px; font-size: 14px; font-family: Monaco; "> BT_call_null.reset(</div><div style="margin: 0px; font-size: 14px; font-family: Monaco; color: rgb(209, 47, 27); "><span style=""> </span><span style="color: #bb2ca2">new</span><span style=""> BuiltinBug(</span>"Called function pointer is null (null dereference)"<span style="">));</span></div><div style="margin: 0px; font-size: 14px; font-family: Monaco; "> emitBadCall(BT_call_null.get(), C, Callee);</div><div style="margin: 0px; font-size: 14px; font-family: Monaco; "> } <span style="color: #bb2ca2">else</span> <span style="color: #bb2ca2">if</span> (StNull) {</div><div style="margin: 0px; font-size: 14px; font-family: Monaco; color: rgb(0, 132, 0); "><span style=""> </span>// Called functions are assumed to be non-null, so this is</div><div style="margin: 0px; font-size: 14px; font-family: Monaco; color: rgb(0, 132, 0); "><span style=""> </span>// an implicit null dereference.</div><div style="margin: 0px; font-size: 14px; font-family: Monaco; "> <span style="color: #bb2ca2">if</span> (ExplodedNode *N = C.generateSink(StNull)) {</div><div style="margin: 0px; font-size: 14px; font-family: Monaco; "> ImplicitNullDerefEvent Event = { L, <span style="color: #bb2ca2">false</span>, N, &C.getBugReporter() };</div><div style="margin: 0px; font-size: 14px; font-family: Monaco; "> dispatchEvent(Event);</div><div style="margin: 0px; font-size: 14px; font-family: Monaco; "> }</div><div style="margin: 0px; font-size: 14px; font-family: Monaco; "> }</div><div style="margin: 0px; font-size: 14px; font-family: Monaco; min-height: 19px; "><br></div><div style="margin: 0px; font-size: 14px; font-family: Monaco; "> C.addTransition(StNonNull);</div></div><div style="margin: 0px; font-size: 14px; font-family: Monaco; ">}</div><div><br></div><div>This makes the unavailable method checker work fine, but causes 2 failures in some C++ tests, dtor.cpp:431 and method-call-path-notes.cpp:37. I am having a hard time working out why, any suggestions? In both cases the sink seems to abort the analysis early, checkPreCall is never called in the CallAndMessageChecker for the c++ methods that should be generating warnings. I am not sure where I should be looking here.</div><div><br></div><div>Otherwise the functionality works well, and using C.getPredecessor() instead of generating a sink for the Event makes everything work fine.</div><div>Richard.<br></div></div></blockquote><br></div><div>Well, you did show me another bug: there's a missing return after emitBadCall. But that's not in the new code...</div><div><br></div><div>Aha: it's possible the value is UnknownVal, and thus StNull, StNonNull, and the original state are all the same. That means that the regular addTransition gets ignored, but the generateSink does not. That seems like a bug in CheckerContext::addTransitionImpl...if you remove this optimization, does it start working?</div><div><br></div><div><div style="margin: 0px; font-size: 11px; font-family: Menlo; "> <span style="color: #bb2ca2">if</span> (!State || (State == <span style="color: #4f8187">Pred</span>-><span style="color: #31595d">getState</span>() && !Tag && !MarkAsSink))</div><div><div style="margin: 0px; font-size: 11px; font-family: Menlo; "> <span style="color: #bb2ca2">return</span> <span style="color: #4f8187">Pred</span>;</div></div><div><br></div><div>I'm not sure we can simply remove the optimization, but it'll help narrow down the problem.</div><div><br></div><div>Jordan</div></div><div><br></div><br></div></blockquote></div><br><div>Yes, the SVal was an UnknownVal, that was it. I have fixed the dtor.cpp test by forcing a transition for StNonNull at the end of checkPreStmt with:</div><div><br></div><div><div style="margin: 0px; font-size: 14px; font-family: Monaco; "> C.addTransition(StNonNull, <span style="color: #bb2ca2">this</span>);</div></div><div><br></div><div>Unfortunately the other test failure remains, method-call-path-notes.cpp. This one is a bit trickier, checkPreCall is now being called and the null state is triggering a bug report. The problem is the generateSink() method in emitBadCall() is returning null, so the bug is never actually reported.</div><div><br></div><div>I guess the previously generated sink is causing the new sink to not be generated, but I do not know how to work around this. The generated graph is also a bit odd, I have attached it in case it is any help. Any suggestions here?</div></div></blockquote><div><br></div><div>Hm, I wonder if <i>this</i> is from the missing return at the end of the null check clause in checkPreStmt. Transitioning to StNonNull when we <i>know</i> the value is null is just wrong. I'll commit that back soon, but meanwhile can you try it locally?</div><div><br></div><div>(That probably also means you don't have to add the ", this" to the final addTransition there.)</div></div></div></blockquote><br></div><div>Committed in r191805.</div><div><br></div><div>Jordan</div><br></body></html>