From: Ted Kremenek Date: Thu, 5 Nov 2009 00:42:23 +0000 (+0000) Subject: Modify GRExprEngine::EvalBind() to take both a "store expression" and X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=50ecd1536a2b70327e9eb2c2c2a652cde3dae365;p=clang Modify GRExprEngine::EvalBind() to take both a "store expression" and an "assign expression", representing the expressions where the value binding occurs and the assignment takes place respectively. These are largely syntactic clues for better error reporting. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@86084 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Analysis/PathSensitive/Checker.h b/include/clang/Analysis/PathSensitive/Checker.h index 3bef08d754..4fc0a617ec 100644 --- a/include/clang/Analysis/PathSensitive/Checker.h +++ b/include/clang/Analysis/PathSensitive/Checker.h @@ -116,12 +116,13 @@ private: void GR_VisitBind(ExplodedNodeSet &Dst, GRStmtNodeBuilder &Builder, GRExprEngine &Eng, - const Stmt *stmt, ExplodedNode *Pred, void *tag, + const Stmt *AssignE, + const Stmt *StoreE, ExplodedNode *Pred, void *tag, SVal location, SVal val, bool isPrevisit) { CheckerContext C(Dst, Builder, Eng, Pred, tag, isPrevisit); assert(isPrevisit && "Only previsit supported for now."); - PreVisitBind(C, stmt, location, val); + PreVisitBind(C, AssignE, StoreE, location, val); } public: @@ -135,7 +136,8 @@ public: return Pred; } - virtual void PreVisitBind(CheckerContext &C, const Stmt *ST, + virtual void PreVisitBind(CheckerContext &C, + const Stmt *AssignE, const Stmt *StoreE, SVal location, SVal val) {} virtual ExplodedNode *CheckType(QualType T, ExplodedNode *Pred, diff --git a/include/clang/Analysis/PathSensitive/Checkers/UndefinedAssignmentChecker.h b/include/clang/Analysis/PathSensitive/Checkers/UndefinedAssignmentChecker.h index 7fee5011c4..13437eb2ac 100644 --- a/include/clang/Analysis/PathSensitive/Checkers/UndefinedAssignmentChecker.h +++ b/include/clang/Analysis/PathSensitive/Checkers/UndefinedAssignmentChecker.h @@ -24,7 +24,8 @@ class UndefinedAssignmentChecker public: UndefinedAssignmentChecker() : BT(0) {} static void *getTag(); - virtual void PreVisitBind(CheckerContext &C, const Stmt *S, SVal location, + virtual void PreVisitBind(CheckerContext &C, const Stmt *AssignE, + const Stmt *StoreE, SVal location, SVal val); }; } diff --git a/include/clang/Analysis/PathSensitive/GRExprEngine.h b/include/clang/Analysis/PathSensitive/GRExprEngine.h index 2f2a11a83c..25e47038d7 100644 --- a/include/clang/Analysis/PathSensitive/GRExprEngine.h +++ b/include/clang/Analysis/PathSensitive/GRExprEngine.h @@ -412,7 +412,8 @@ protected: void CheckerVisit(Stmt *S, ExplodedNodeSet &Dst, ExplodedNodeSet &Src, bool isPrevisit); - void CheckerVisitBind(Stmt *S, ExplodedNodeSet &Dst, ExplodedNodeSet &Src, + void CheckerVisitBind(const Stmt *AssignE, const Stmt *StoreE, + ExplodedNodeSet &Dst, ExplodedNodeSet &Src, SVal location, SVal val, bool isPrevisit); @@ -566,7 +567,8 @@ protected: /// EvalBind - Handle the semantics of binding a value to a specific location. /// This method is used by EvalStore, VisitDeclStmt, and others. - void EvalBind(ExplodedNodeSet& Dst, Stmt* Ex, ExplodedNode* Pred, + void EvalBind(ExplodedNodeSet& Dst, Stmt *AssignE, + Stmt* StoreE, ExplodedNode* Pred, const GRState* St, SVal location, SVal Val, bool atDeclInit = false); @@ -578,14 +580,10 @@ public: const GRState* St, SVal location, const void *tag = 0); - - void EvalStore(ExplodedNodeSet& Dst, Expr* E, ExplodedNode* Pred, const GRState* St, - SVal TargetLV, SVal Val, const void *tag = 0); - - void EvalStore(ExplodedNodeSet& Dst, Expr* E, Expr* StoreE, ExplodedNode* Pred, + void EvalStore(ExplodedNodeSet& Dst, Expr* AssignE, Expr* StoreE, + ExplodedNode* Pred, const GRState* St, SVal TargetLV, SVal Val, const void *tag = 0); - }; } // end clang namespace diff --git a/lib/Analysis/GRExprEngine.cpp b/lib/Analysis/GRExprEngine.cpp index c71882e4d8..212fea3a6b 100644 --- a/lib/Analysis/GRExprEngine.cpp +++ b/lib/Analysis/GRExprEngine.cpp @@ -141,7 +141,8 @@ void GRExprEngine::CheckerVisit(Stmt *S, ExplodedNodeSet &Dst, // FIXME: This is largely copy-paste from CheckerVisit(). Need to // unify. -void GRExprEngine::CheckerVisitBind(Stmt *S, ExplodedNodeSet &Dst, +void GRExprEngine::CheckerVisitBind(const Stmt *AssignE, const Stmt *StoreE, + ExplodedNodeSet &Dst, ExplodedNodeSet &Src, SVal location, SVal val, bool isPrevisit) { @@ -164,8 +165,8 @@ void GRExprEngine::CheckerVisitBind(Stmt *S, ExplodedNodeSet &Dst, for (ExplodedNodeSet::iterator NI = PrevSet->begin(), NE = PrevSet->end(); NI != NE; ++NI) - checker->GR_VisitBind(*CurrSet, *Builder, *this, S, *NI, tag, location, - val, isPrevisit); + checker->GR_VisitBind(*CurrSet, *Builder, *this, AssignE, StoreE, + *NI, tag, location, val, isPrevisit); // Update which NodeSet is the current one. PrevSet = CurrSet; @@ -1125,7 +1126,8 @@ void GRExprEngine::VisitMemberExpr(MemberExpr* M, ExplodedNode* Pred, /// EvalBind - Handle the semantics of binding a value to a specific location. /// This method is used by EvalStore and (soon) VisitDeclStmt, and others. -void GRExprEngine::EvalBind(ExplodedNodeSet& Dst, Stmt* Ex, ExplodedNode* Pred, +void GRExprEngine::EvalBind(ExplodedNodeSet& Dst, Stmt *AssignE, + Stmt* StoreE, ExplodedNode* Pred, const GRState* state, SVal location, SVal Val, bool atDeclInit) { @@ -1133,7 +1135,7 @@ void GRExprEngine::EvalBind(ExplodedNodeSet& Dst, Stmt* Ex, ExplodedNode* Pred, // Do a previsit of the bind. ExplodedNodeSet CheckedSet, Src; Src.Add(Pred); - CheckerVisitBind(Ex, CheckedSet, Src, location, Val, true); + CheckerVisitBind(AssignE, StoreE, CheckedSet, Src, location, Val, true); for (ExplodedNodeSet::iterator I = CheckedSet.begin(), E = CheckedSet.end(); I!=E; ++I) { @@ -1166,7 +1168,7 @@ void GRExprEngine::EvalBind(ExplodedNodeSet& Dst, Stmt* Ex, ExplodedNode* Pred, // The next thing to do is check if the GRTransferFuncs object wants to // update the state based on the new binding. If the GRTransferFunc object // doesn't do anything, just auto-propagate the current state. - GRStmtNodeBuilderRef BuilderRef(Dst, *Builder, *this, *I, newState, Ex, + GRStmtNodeBuilderRef BuilderRef(Dst, *Builder, *this, *I, newState, StoreE, newState != state); getTF().EvalBind(BuilderRef, location, Val); @@ -1179,14 +1181,16 @@ void GRExprEngine::EvalBind(ExplodedNodeSet& Dst, Stmt* Ex, ExplodedNode* Pred, /// @param state The current simulation state /// @param location The location to store the value /// @param Val The value to be stored -void GRExprEngine::EvalStore(ExplodedNodeSet& Dst, Expr* Ex, ExplodedNode* Pred, +void GRExprEngine::EvalStore(ExplodedNodeSet& Dst, Expr *AssignE, + Expr* StoreE, + ExplodedNode* Pred, const GRState* state, SVal location, SVal Val, const void *tag) { - assert (Builder && "GRStmtNodeBuilder must be defined."); + assert(Builder && "GRStmtNodeBuilder must be defined."); // Evaluate the location (checks for bad dereferences). - Pred = EvalLocation(Ex, Pred, state, location, tag); + Pred = EvalLocation(StoreE, Pred, state, location, tag); if (!Pred) return; @@ -1199,7 +1203,7 @@ void GRExprEngine::EvalStore(ExplodedNodeSet& Dst, Expr* Ex, ExplodedNode* Pred, SaveAndRestore OldTag(Builder->Tag); Builder->PointKind = ProgramPoint::PostStoreKind; Builder->Tag = tag; - EvalBind(Dst, Ex, Pred, state, location, Val); + EvalBind(Dst, AssignE, StoreE, Pred, state, location, Val); } void GRExprEngine::EvalLoad(ExplodedNodeSet& Dst, Expr* Ex, ExplodedNode* Pred, @@ -1231,17 +1235,6 @@ void GRExprEngine::EvalLoad(ExplodedNodeSet& Dst, Expr* Ex, ExplodedNode* Pred, } } -void GRExprEngine::EvalStore(ExplodedNodeSet& Dst, Expr* Ex, Expr* StoreE, - ExplodedNode* Pred, const GRState* state, - SVal location, SVal Val, const void *tag) { - - ExplodedNodeSet TmpDst; - EvalStore(TmpDst, StoreE, Pred, state, location, Val, tag); - - for (ExplodedNodeSet::iterator I=TmpDst.begin(), E=TmpDst.end(); I!=E; ++I) - MakeNode(Dst, Ex, *I, (*I)->getState(), ProgramPoint::PostStmtKind, tag); -} - ExplodedNode* GRExprEngine::EvalLocation(Stmt* Ex, ExplodedNode* Pred, const GRState* state, SVal location, const void *tag) { @@ -1402,7 +1395,7 @@ static bool EvalOSAtomicCompareAndSwap(ExplodedNodeSet& Dst, newValueExpr->getType()); } - Engine.EvalStore(TmpStore, theValueExpr, N, stateEqual, location, + Engine.EvalStore(TmpStore, NULL, theValueExpr, N, stateEqual, location, val, OSAtomicStoreTag); // Now bind the result of the comparison. @@ -2147,8 +2140,8 @@ void GRExprEngine::VisitDeclStmt(DeclStmt *DS, ExplodedNode *Pred, Builder->getCurrentBlockCount()); } - EvalBind(Dst, DS, *I, state, loc::MemRegionVal(state->getRegion(VD, LC)), - InitVal, true); + EvalBind(Dst, DS, DS, *I, state, + loc::MemRegionVal(state->getRegion(VD, LC)), InitVal, true); } else { state = state->bindDeclWithNoInit(state->getRegion(VD, LC)); @@ -2570,7 +2563,7 @@ void GRExprEngine::VisitUnaryOperator(UnaryOperator* U, ExplodedNode* Pred, state = state->BindExpr(U, U->isPostfix() ? V2 : Result); // Perform the store. - EvalStore(Dst, U, *I2, state, V1, Result); + EvalStore(Dst, NULL, U, *I2, state, V1, Result); } } } diff --git a/lib/Analysis/UndefinedAssignmentChecker.cpp b/lib/Analysis/UndefinedAssignmentChecker.cpp index c5b2401f47..2e3ac34913 100644 --- a/lib/Analysis/UndefinedAssignmentChecker.cpp +++ b/lib/Analysis/UndefinedAssignmentChecker.cpp @@ -22,14 +22,15 @@ void *UndefinedAssignmentChecker::getTag() { return &x; } -void UndefinedAssignmentChecker::PreVisitBind(CheckerContext &C, - const Stmt *S, +void UndefinedAssignmentChecker::PreVisitBind(CheckerContext &C, + const Stmt *AssignE, + const Stmt *StoreE, SVal location, SVal val) { if (!val.isUndef()) return; - ExplodedNode *N = C.GenerateNode(S, true); + ExplodedNode *N = C.GenerateNode(StoreE, true); if (!N) return; @@ -40,20 +41,20 @@ void UndefinedAssignmentChecker::PreVisitBind(CheckerContext &C, // Generate a report for this bug. EnhancedBugReport *R = new EnhancedBugReport(*BT, BT->getName().c_str(), N); - const Expr *ex = 0; - - // FIXME: This check needs to be done on the expression doing the - // assignment, not the "store" expression. - if (const BinaryOperator *B = dyn_cast(S)) - ex = B->getRHS(); - else if (const DeclStmt *DS = dyn_cast(S)) { - const VarDecl* VD = dyn_cast(DS->getSingleDecl()); - ex = VD->getInit(); - } - if (ex) { - R->addRange(ex->getSourceRange()); - R->addVisitorCreator(bugreporter::registerTrackNullOrUndefValue, ex); + if (AssignE) { + const Expr *ex = 0; + + if (const BinaryOperator *B = dyn_cast(AssignE)) + ex = B->getRHS(); + else if (const DeclStmt *DS = dyn_cast(AssignE)) { + const VarDecl* VD = dyn_cast(DS->getSingleDecl()); + ex = VD->getInit(); + } + if (ex) { + R->addRange(ex->getSourceRange()); + R->addVisitorCreator(bugreporter::registerTrackNullOrUndefValue, ex); + } } C.EmitReport(R);