Hi Zhongxing, I think this is a great improvement over my previous hack! (thanks for removing it).
On Dec 1, 2009, at 9:49 PM, Zhongxing Xu wrote: > Author: zhongxingxu > Date: Tue Dec 1 23:49:12 2009 > New Revision: 90296 > > URL: http://llvm.org/viewvc/llvm-project?rev=90296&view=rev > Log: > Hard bifurcate the state into nil receiver and non-nil receiver, so that > we don't need to use the DoneEvaluation hack when check for > ObjCMessageExpr. > > PreVisitObjCMessageExpr() only checks for undefined receiver or arguments. > > Add checker interface EvalNilReceiver(). This is a 'once-and-done' interface. > > > Modified: > cfe/trunk/include/clang/Analysis/PathSensitive/Checker.h > cfe/trunk/include/clang/Analysis/PathSensitive/GRExprEngine.h > cfe/trunk/include/clang/Analysis/PathSensitive/GRTransferFuncs.h > cfe/trunk/lib/Analysis/CFRefCount.cpp > cfe/trunk/lib/Analysis/CallAndMessageChecker.cpp > cfe/trunk/lib/Analysis/GRExprEngine.cpp > > Modified: cfe/trunk/include/clang/Analysis/PathSensitive/Checker.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/PathSensitive/Checker.h?rev=90296&r1=90295&r2=90296&view=diff > > ============================================================================== > --- cfe/trunk/include/clang/Analysis/PathSensitive/Checker.h (original) > +++ cfe/trunk/include/clang/Analysis/PathSensitive/Checker.h Tue Dec 1 > 23:49:12 2009 > @@ -53,22 +53,10 @@ > OldTag(B.Tag, tag), > OldPointKind(B.PointKind, K), > OldHasGen(B.HasGeneratedNode), > - state(st), statement(stmt), size(Dst.size()), > - DoneEvaluating(false) {} > + state(st), statement(stmt), size(Dst.size()) {} > > ~CheckerContext(); > > - // FIXME: This were added to support CallAndMessageChecker to indicating > - // to GRExprEngine to "stop evaluating" a message expression under certain > - // cases. This is *not* meant to be a permanent API change, and was added > - // to aid in the transition of removing logic for checks from > GRExprEngine. > - void setDoneEvaluating() { > - DoneEvaluating = true; > - } > - bool isDoneEvaluating() const { > - return DoneEvaluating; > - } > - > ConstraintManager &getConstraintManager() { > return Eng.getConstraintManager(); > } > @@ -165,7 +153,7 @@ > friend class GRExprEngine; > > // FIXME: Remove the 'tag' option. > - bool GR_Visit(ExplodedNodeSet &Dst, > + void GR_Visit(ExplodedNodeSet &Dst, > GRStmtNodeBuilder &Builder, > GRExprEngine &Eng, > const Stmt *S, > @@ -177,7 +165,14 @@ > _PreVisit(C, S); > else > _PostVisit(C, S); > - return C.isDoneEvaluating(); > + } > + > + bool GR_EvalNilReceiver(ExplodedNodeSet &Dst, GRStmtNodeBuilder &Builder, > + GRExprEngine &Eng, const ObjCMessageExpr *ME, > + ExplodedNode *Pred, const GRState *state, void > *tag) { > + CheckerContext C(Dst, Builder, Eng, Pred, tag, > ProgramPoint::PostStmtKind, > + ME, state); > + return EvalNilReceiver(C, ME); > } > > // FIXME: Remove the 'tag' option. > @@ -231,6 +226,10 @@ > virtual void VisitBranchCondition(GRBranchNodeBuilder &Builder, > GRExprEngine &Eng, > Stmt *Condition, void *tag) {} > + > + virtual bool EvalNilReceiver(CheckerContext &C, const ObjCMessageExpr *ME) > { > + return false; > + } > }; > } // end clang namespace > > > Modified: cfe/trunk/include/clang/Analysis/PathSensitive/GRExprEngine.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/PathSensitive/GRExprEngine.h?rev=90296&r1=90295&r2=90296&view=diff > > ============================================================================== > --- cfe/trunk/include/clang/Analysis/PathSensitive/GRExprEngine.h (original) > +++ cfe/trunk/include/clang/Analysis/PathSensitive/GRExprEngine.h Tue Dec 1 > 23:49:12 2009 > @@ -209,8 +209,13 @@ > protected: > /// CheckerVisit - Dispatcher for performing checker-specific logic > /// at specific statements. > - bool CheckerVisit(Stmt *S, ExplodedNodeSet &Dst, ExplodedNodeSet &Src, > + void CheckerVisit(Stmt *S, ExplodedNodeSet &Dst, ExplodedNodeSet &Src, > bool isPrevisit); > + > + void CheckerEvalNilReceiver(const ObjCMessageExpr *ME, > + ExplodedNodeSet &Dst, > + const GRState *state, > + ExplodedNode *Pred); > > void CheckerVisitBind(const Stmt *AssignE, const Stmt *StoreE, > ExplodedNodeSet &Dst, ExplodedNodeSet &Src, > @@ -358,9 +363,10 @@ > } > > protected: > - void EvalObjCMessageExpr(ExplodedNodeSet& Dst, ObjCMessageExpr* ME, > ExplodedNode* Pred) { > + void EvalObjCMessageExpr(ExplodedNodeSet& Dst, ObjCMessageExpr* ME, > + ExplodedNode* Pred, const GRState *state) { > assert (Builder && "GRStmtNodeBuilder must be defined."); > - getTF().EvalObjCMessageExpr(Dst, *this, *Builder, ME, Pred); > + getTF().EvalObjCMessageExpr(Dst, *this, *Builder, ME, Pred, state); > } > > const GRState* MarkBranch(const GRState* St, Stmt* Terminator, > > Modified: cfe/trunk/include/clang/Analysis/PathSensitive/GRTransferFuncs.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/PathSensitive/GRTransferFuncs.h?rev=90296&r1=90295&r2=90296&view=diff > > ============================================================================== > --- cfe/trunk/include/clang/Analysis/PathSensitive/GRTransferFuncs.h > (original) > +++ cfe/trunk/include/clang/Analysis/PathSensitive/GRTransferFuncs.h Tue Dec > 1 23:49:12 2009 > @@ -47,7 +47,8 @@ > GRExprEngine& Engine, > GRStmtNodeBuilder& Builder, > ObjCMessageExpr* ME, > - ExplodedNode* Pred) {} > + ExplodedNode* Pred, > + const GRState *state) {} > > // Stores. > > > Modified: cfe/trunk/lib/Analysis/CFRefCount.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CFRefCount.cpp?rev=90296&r1=90295&r2=90296&view=diff > > ============================================================================== > --- cfe/trunk/lib/Analysis/CFRefCount.cpp (original) > +++ cfe/trunk/lib/Analysis/CFRefCount.cpp Tue Dec 1 23:49:12 2009 > @@ -1985,7 +1985,7 @@ > Expr* Receiver, > const RetainSummary& Summ, > ExprIterator arg_beg, ExprIterator arg_end, > - ExplodedNode* Pred); > + ExplodedNode* Pred, const GRState *state); > > virtual void EvalCall(ExplodedNodeSet& Dst, > GRExprEngine& Eng, > @@ -1998,7 +1998,8 @@ > GRExprEngine& Engine, > GRStmtNodeBuilder& Builder, > ObjCMessageExpr* ME, > - ExplodedNode* Pred); > + ExplodedNode* Pred, > + const GRState *state); > > bool EvalObjCMessageExprAux(ExplodedNodeSet& Dst, > GRExprEngine& Engine, > @@ -2777,10 +2778,7 @@ > Expr* Receiver, > const RetainSummary& Summ, > ExprIterator arg_beg, ExprIterator arg_end, > - ExplodedNode* Pred) { > - > - // Get the state. > - const GRState *state = Builder.GetState(Pred); > + ExplodedNode* Pred, const GRState *state) { > > // Evaluate the effect of the arguments. > RefVal::Kind hasErr = (RefVal::Kind) 0; > @@ -3013,34 +3011,23 @@ > > assert(Summ); > EvalSummary(Dst, Eng, Builder, CE, 0, *Summ, > - CE->arg_begin(), CE->arg_end(), Pred); > + CE->arg_begin(), CE->arg_end(), Pred, Builder.GetState(Pred)); > } > > void CFRefCount::EvalObjCMessageExpr(ExplodedNodeSet& Dst, > GRExprEngine& Eng, > GRStmtNodeBuilder& Builder, > ObjCMessageExpr* ME, > - ExplodedNode* Pred) { > - // FIXME: Since we moved the nil check into a checker, we could get nil > - // receiver here. Need a better way to check such case. > - if (Expr* Receiver = ME->getReceiver()) { > - const GRState *state = Pred->getState(); > - DefinedOrUnknownSVal > L=cast<DefinedOrUnknownSVal>(state->getSVal(Receiver)); > - if (!state->Assume(L, true)) { > - Dst.Add(Pred); > - return; > - } > - } > - > + ExplodedNode* Pred, > + const GRState *state) { > RetainSummary *Summ = > ME->getReceiver() > - ? Summaries.getInstanceMethodSummary(ME, Builder.GetState(Pred), > - Pred->getLocationContext()) > + ? Summaries.getInstanceMethodSummary(ME, > state,Pred->getLocationContext()) > : Summaries.getClassMethodSummary(ME); > > assert(Summ && "RetainSummary is null"); > EvalSummary(Dst, Eng, Builder, ME, ME->getReceiver(), *Summ, > - ME->arg_begin(), ME->arg_end(), Pred); > + ME->arg_begin(), ME->arg_end(), Pred, state); > } > > namespace { > > Modified: cfe/trunk/lib/Analysis/CallAndMessageChecker.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CallAndMessageChecker.cpp?rev=90296&r1=90295&r2=90296&view=diff > > ============================================================================== > --- cfe/trunk/lib/Analysis/CallAndMessageChecker.cpp (original) > +++ cfe/trunk/lib/Analysis/CallAndMessageChecker.cpp Tue Dec 1 23:49:12 2009 > @@ -41,6 +41,7 @@ > > void PreVisitCallExpr(CheckerContext &C, const CallExpr *CE); > void PreVisitObjCMessageExpr(CheckerContext &C, const ObjCMessageExpr *ME); > + bool EvalNilReceiver(CheckerContext &C, const ObjCMessageExpr *ME); > > private: > void EmitBadCall(BugType *BT, CheckerContext &C, const CallExpr *CE); > @@ -148,28 +149,12 @@ > } > } > } > +} > > - // Check if the receiver was nil and then returns a value that may > - // be garbage. > - if (const Expr *Receiver = ME->getReceiver()) { > - DefinedOrUnknownSVal receiverVal = > - cast<DefinedOrUnknownSVal>(state->getSVal(Receiver)); > - > - const GRState *notNullState, *nullState; > - llvm::tie(notNullState, nullState) = state->Assume(receiverVal); > - > - if (nullState && !notNullState) { > - HandleNilReceiver(C, nullState, ME); > - C.setDoneEvaluating(); // FIXME: eventually remove. > - return; > - } > - > - assert(notNullState); > - state = notNullState; > - } > - > - // Add a state transition if the state has changed. > - C.addTransition(state); > +bool CallAndMessageChecker::EvalNilReceiver(CheckerContext &C, > + const ObjCMessageExpr *ME) { > + HandleNilReceiver(C, C.getState(), ME); > + return true; // Nil receiver is not handled elsewhere. > } > > void CallAndMessageChecker::EmitNilReceiverBug(CheckerContext &C, > > Modified: cfe/trunk/lib/Analysis/GRExprEngine.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/GRExprEngine.cpp?rev=90296&r1=90295&r2=90296&view=diff > > ============================================================================== > --- cfe/trunk/lib/Analysis/GRExprEngine.cpp (original) > +++ cfe/trunk/lib/Analysis/GRExprEngine.cpp Tue Dec 1 23:49:12 2009 > @@ -116,20 +116,18 @@ > // Checker worklist routines. > //===----------------------------------------------------------------------===// > > -bool GRExprEngine::CheckerVisit(Stmt *S, ExplodedNodeSet &Dst, > +void GRExprEngine::CheckerVisit(Stmt *S, ExplodedNodeSet &Dst, > ExplodedNodeSet &Src, bool isPrevisit) { > > if (Checkers.empty()) { > - Dst.insert(Src); > - return false; > + Dst = Src; > + return; > } > > ExplodedNodeSet Tmp; > ExplodedNodeSet *PrevSet = &Src; > - bool stopProcessingAfterCurrentChecker = false; > > - for (CheckersOrdered::iterator I=Checkers.begin(),E=Checkers.end(); I!=E; > ++I) > - { > + for (CheckersOrdered::iterator I=Checkers.begin(),E=Checkers.end(); > I!=E;++I){ > ExplodedNodeSet *CurrSet = (I+1 == E) ? &Dst > : (PrevSet == &Tmp) ? &Src : &Tmp; > > @@ -138,31 +136,26 @@ > Checker *checker = I->second; > > for (ExplodedNodeSet::iterator NI = PrevSet->begin(), NE = PrevSet->end(); > - NI != NE; ++NI) { > - // FIXME: Halting evaluation of the checkers is something we may > - // not support later. The design is still evolving. > - if (checker->GR_Visit(*CurrSet, *Builder, *this, S, *NI, > - tag, isPrevisit)) { > - if (CurrSet != &Dst) > - Dst.insert(*CurrSet); > - > - stopProcessingAfterCurrentChecker = true; > - continue; > - } > - assert(stopProcessingAfterCurrentChecker == false && > - "Inconsistent setting of 'stopProcessingAfterCurrentChecker'"); > - } > - > - if (stopProcessingAfterCurrentChecker) > - return true; > - > - // Continue on to the next checker. Update the current NodeSet. > + NI != NE; ++NI) > + checker->GR_Visit(*CurrSet, *Builder, *this, S, *NI, tag, isPrevisit); > PrevSet = CurrSet; > } > > // Don't autotransition. The CheckerContext objects should do this > // automatically. > - return false; > +} > + > +void GRExprEngine::CheckerEvalNilReceiver(const ObjCMessageExpr *ME, > + ExplodedNodeSet &Dst, > + const GRState *state, > + ExplodedNode *Pred) { > + for (CheckersOrdered::iterator > I=Checkers.begin(),E=Checkers.end();I!=E;++I) { > + void *tag = I->first; > + Checker *checker = I->second; > + > + if (checker->GR_EvalNilReceiver(Dst, *Builder, *this, ME, Pred, state, > tag)) > + break; > + } > } > > // FIXME: This is largely copy-paste from CheckerVisit(). Need to > @@ -1922,10 +1915,7 @@ > ExplodedNodeSet Src, DstTmp; > Src.Add(Pred); > > - if (CheckerVisit(ME, DstTmp, Src, true)) { > - Dst.insert(DstTmp); > - return; > - } > + CheckerVisit(ME, DstTmp, Src, true); > > unsigned size = Dst.size(); > > @@ -1934,10 +1924,38 @@ > Pred = *DI; > bool RaisesException = false; > > - if (ME->getReceiver()) { > + if (const Expr *Receiver = ME->getReceiver()) { > + const GRState *state = Pred->getState(); > + > + // Bifurcate the state into nil and non-nil ones. > + DefinedOrUnknownSVal receiverVal = > + cast<DefinedOrUnknownSVal>(state->getSVal(Receiver)); > + > + const GRState *notNilState, *nilState; > + llvm::tie(notNilState, nilState) = state->Assume(receiverVal); > + > + // There are three cases: can be nil or non-nil, must be nil, must be > + // non-nil. We handle must be nil, and merge the rest two into non-nil. > + if (nilState && !notNilState) { > + CheckerEvalNilReceiver(ME, Dst, nilState, Pred); > + return; > + } > + > + assert(notNilState); > + > // Check if the "raise" message was sent. > if (ME->getSelector() == RaiseSel) > RaisesException = true; > + > + // Check if we raise an exception. For now treat these as sinks. > + // Eventually we will want to handle exceptions properly. > + SaveAndRestore<bool> OldSink(Builder->BuildSinks); > + if (RaisesException) > + Builder->BuildSinks = true; > + > + // Dispatch to plug-in transfer function. > + SaveOr OldHasGen(Builder->HasGeneratedNode); > + EvalObjCMessageExpr(Dst, ME, Pred, notNilState); > } > else { > > @@ -1984,17 +2002,17 @@ > RaisesException = true; break; > } > } > - } > > - // Check if we raise an exception. For now treat these as sinks. > Eventually > - // we will want to handle exceptions properly. > - SaveAndRestore<bool> OldSink(Builder->BuildSinks); > - if (RaisesException) > - Builder->BuildSinks = true; > + // Check if we raise an exception. For now treat these as sinks. > + // Eventually we will want to handle exceptions properly. > + SaveAndRestore<bool> OldSink(Builder->BuildSinks); > + if (RaisesException) > + Builder->BuildSinks = true; > > - // Dispatch to plug-in transfer function. > - SaveOr OldHasGen(Builder->HasGeneratedNode); > - EvalObjCMessageExpr(Dst, ME, Pred); > + // Dispatch to plug-in transfer function. > + SaveOr OldHasGen(Builder->HasGeneratedNode); > + EvalObjCMessageExpr(Dst, ME, Pred, Builder->GetState(Pred)); > + } > } > > // Handle the case where no nodes where generated. Auto-generate that > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
