On Oct 1, 2013, at 9:40 , Jordan Rose <[email protected]> wrote:
> [+cfe-commits, which I didn't really mean to remove] > > On Sep 27, 2013, at 1:01 , Richard <[email protected]> wrote: > >> >> On 25 Sep 2013, at 18:44, Jordan Rose <[email protected]> wrote: >> >>> >>> On Sep 23, 2013, at 4:38 , Richard <[email protected]> wrote: >>>> >>>> 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: >>>> >>>> { >>>> ... >>>> if (StNull && !StNonNull) { >>>> if (!BT_call_null) >>>> BT_call_null.reset( >>>> new BuiltinBug("Called function pointer is null (null >>>> dereference)")); >>>> emitBadCall(BT_call_null.get(), C, Callee); >>>> } else if (StNull) { >>>> // Called functions are assumed to be non-null, so this is >>>> // an implicit null dereference. >>>> if (ExplodedNode *N = C.generateSink(StNull)) { >>>> ImplicitNullDerefEvent Event = { L, false, N, &C.getBugReporter() }; >>>> dispatchEvent(Event); >>>> } >>>> } >>>> >>>> C.addTransition(StNonNull); >>>> } >>>> >>>> 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. >>>> >>>> Otherwise the functionality works well, and using C.getPredecessor() >>>> instead of generating a sink for the Event makes everything work fine. >>>> Richard. >>> >>> Well, you did show me another bug: there's a missing return after >>> emitBadCall. But that's not in the new code... >>> >>> 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? >>> >>> if (!State || (State == Pred->getState() && !Tag && !MarkAsSink)) >>> return Pred; >>> >>> I'm not sure we can simply remove the optimization, but it'll help narrow >>> down the problem. >>> >>> Jordan >>> >>> >> >> 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: >> >> C.addTransition(StNonNull, this); >> >> 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. >> >> 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? > > Hm, I wonder if this is from the missing return at the end of the null check > clause in checkPreStmt. Transitioning to StNonNull when we know the value is > null is just wrong. I'll commit that back soon, but meanwhile can you try it > locally? > > (That probably also means you don't have to add the ", this" to the final > addTransition there.) Committed in r191805. Jordan
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
