[cfe-commits] r90296 - in /cfe/trunk: include/clang/Analysis/PathSensitive/Checker.h include/clang/Analysis/PathSensitive/GRExprEngine.h include/clang/Analysis/PathSensitive/GRTransferFuncs.h lib/Analysis/CFRefCount.cpp lib/Analysis/CallAndMessageChecker.cpp lib/Analysis/GRExprEngine.cpp
Ted Kremenek
kremenek at apple.com
Wed Dec 2 00:41:29 PST 2009
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
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list