From: Ted Kremenek Date: Wed, 11 Nov 2009 03:26:34 +0000 (+0000) Subject: Refactor DereferenceChecker to use only the new Checker API instead of X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=b4b817d704287836b52b34369009e682f208aa2b;p=clang Refactor DereferenceChecker to use only the new Checker API instead of the old builder API. This percolated a bunch of changes up to the Checker class (where CheckLocation has been renamed VisitLocation) and GRExprEngine. ProgramPoint now has the notion of a "LocationCheck" point (with PreLoad and PreStore respectively), and a bunch of the old ProgramPoints that are no longer used have been removed. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@86798 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Analysis/PathSensitive/Checker.h b/include/clang/Analysis/PathSensitive/Checker.h index 5e87e37993..5503a9099b 100644 --- a/include/clang/Analysis/PathSensitive/Checker.h +++ b/include/clang/Analysis/PathSensitive/Checker.h @@ -39,22 +39,19 @@ class CheckerContext { SaveAndRestore OldTag; SaveAndRestore OldPointKind; SaveOr OldHasGen; + const GRState *state; public: - CheckerContext(ExplodedNodeSet &dst, - GRStmtNodeBuilder &builder, - GRExprEngine &eng, - ExplodedNode *pred, - const void *tag, bool preVisit) + CheckerContext(ExplodedNodeSet &dst, GRStmtNodeBuilder &builder, + GRExprEngine &eng, ExplodedNode *pred, + const void *tag, ProgramPoint::Kind K, + const GRState *st = 0) : Dst(dst), B(builder), Eng(eng), Pred(pred), - OldSink(B.BuildSinks), OldTag(B.Tag), - OldPointKind(B.PointKind), OldHasGen(B.HasGeneratedNode) { - //assert(Dst.empty()); // This is a fake assertion. - // See GRExprEngine::CheckerVisit(), CurrSet is repeatedly used. - B.Tag = tag; - if (preVisit) - B.PointKind = ProgramPoint::PreStmtKind; - } + OldSink(B.BuildSinks), + OldTag(B.Tag, tag), + OldPointKind(B.PointKind, K), + OldHasGen(B.HasGeneratedNode), + state(st) {} ~CheckerContext() { if (!B.BuildSinks && !B.HasGeneratedNode) @@ -72,7 +69,7 @@ public: ExplodedNodeSet &getNodeSet() { return Dst; } GRStmtNodeBuilder &getNodeBuilder() { return B; } ExplodedNode *&getPredecessor() { return Pred; } - const GRState *getState() { return B.GetState(Pred); } + const GRState *getState() { return state ? state : B.GetState(Pred); } ASTContext &getASTContext() { return Eng.getContext(); @@ -113,43 +110,54 @@ class Checker { private: friend class GRExprEngine; + // FIXME: Remove the 'tag' option. void GR_Visit(ExplodedNodeSet &Dst, GRStmtNodeBuilder &Builder, GRExprEngine &Eng, - const Stmt *stmt, + const Stmt *S, ExplodedNode *Pred, void *tag, bool isPrevisit) { - CheckerContext C(Dst, Builder, Eng, Pred, tag, isPrevisit); + CheckerContext C(Dst, Builder, Eng, Pred, tag, + isPrevisit ? ProgramPoint::PreStmtKind : + ProgramPoint::PostStmtKind); assert(isPrevisit && "Only previsit supported for now."); - _PreVisit(C, stmt); + _PreVisit(C, S); } + // FIXME: Remove the 'tag' option. void GR_VisitBind(ExplodedNodeSet &Dst, GRStmtNodeBuilder &Builder, GRExprEngine &Eng, const Stmt *AssignE, const Stmt *StoreE, ExplodedNode *Pred, void *tag, SVal location, SVal val, bool isPrevisit) { - CheckerContext C(Dst, Builder, Eng, Pred, tag, isPrevisit); + CheckerContext C(Dst, Builder, Eng, Pred, tag, + isPrevisit ? ProgramPoint::PreStmtKind : + ProgramPoint::PostStmtKind); assert(isPrevisit && "Only previsit supported for now."); PreVisitBind(C, AssignE, StoreE, location, val); } + + // FIXME: Remove the 'tag' option. + void GR_VisitLocation(ExplodedNodeSet &Dst, + GRStmtNodeBuilder &Builder, + GRExprEngine &Eng, + const Stmt *S, + ExplodedNode *Pred, const GRState *state, + SVal location, + void *tag, bool isLoad) { + CheckerContext C(Dst, Builder, Eng, Pred, tag, + isLoad ? ProgramPoint::PreLoadKind : + ProgramPoint::PreStoreKind, state); + VisitLocation(C, S, location); + } public: virtual ~Checker() {} virtual void _PreVisit(CheckerContext &C, const Stmt *ST) {} - - // This is a previsit which takes a node returns a node. - virtual ExplodedNode *CheckLocation(const Stmt *S, ExplodedNode *Pred, - const GRState *state, SVal V, - GRExprEngine &Eng) { - return Pred; - } - - virtual void PreVisitBind(CheckerContext &C, - const Stmt *AssignE, const Stmt *StoreE, - SVal location, SVal val) {} + virtual void VisitLocation(CheckerContext &C, const Stmt *S, SVal location) {} + virtual void PreVisitBind(CheckerContext &C, const Stmt *AssignE, + const Stmt *StoreE, SVal location, SVal val) {} }; - } // end clang namespace #endif diff --git a/include/clang/Analysis/PathSensitive/Checkers/DereferenceChecker.h b/include/clang/Analysis/PathSensitive/Checkers/DereferenceChecker.h index 688cf64149..a84183e7f2 100644 --- a/include/clang/Analysis/PathSensitive/Checkers/DereferenceChecker.h +++ b/include/clang/Analysis/PathSensitive/Checkers/DereferenceChecker.h @@ -16,38 +16,16 @@ #ifndef LLVM_CLANG_DEREFCHECKER #define LLVM_CLANG_DEREFCHECKER -#include "clang/Analysis/PathSensitive/Checker.h" -#include "clang/Analysis/PathSensitive/BugType.h" +#include namespace clang { +class GRExprEngine; class ExplodedNode; -class NullDerefChecker : public Checker { - BuiltinBug *BT; - llvm::SmallVector ImplicitNullDerefNodes; - -public: - NullDerefChecker() : BT(0) {} - ExplodedNode *CheckLocation(const Stmt *S, ExplodedNode *Pred, - const GRState *state, SVal V,GRExprEngine &Eng); - - static void *getTag(); - typedef llvm::SmallVectorImpl::iterator iterator; - iterator implicit_nodes_begin() { return ImplicitNullDerefNodes.begin(); } - iterator implicit_nodes_end() { return ImplicitNullDerefNodes.end(); } -}; - -class UndefDerefChecker : public Checker { - BuiltinBug *BT; -public: - UndefDerefChecker() : BT(0) {} - - ExplodedNode *CheckLocation(const Stmt *S, ExplodedNode *Pred, - const GRState *state, SVal V, GRExprEngine &Eng); - - static void *getTag(); -}; +std::pair +GetImplicitNullDereferences(GRExprEngine &Eng); } // end clang namespace + #endif diff --git a/include/clang/Analysis/PathSensitive/GRExprEngine.h b/include/clang/Analysis/PathSensitive/GRExprEngine.h index db8720987c..0445896311 100644 --- a/include/clang/Analysis/PathSensitive/GRExprEngine.h +++ b/include/clang/Analysis/PathSensitive/GRExprEngine.h @@ -556,16 +556,21 @@ protected: bool atDeclInit = false); public: + // FIXME: 'tag' should be removed, and a LocationContext should be used + // instead. void EvalLoad(ExplodedNodeSet& Dst, Expr* Ex, ExplodedNode* Pred, const GRState* St, SVal location, const void *tag = 0); - ExplodedNode* EvalLocation(Stmt* Ex, ExplodedNode* Pred, + // FIXME: 'tag' should be removed, and a LocationContext should be used + // instead. + void EvalLocation(ExplodedNodeSet &Dst, Stmt *S, ExplodedNode* Pred, const GRState* St, SVal location, - const void *tag = 0); + const void *tag, bool isLoad); + // FIXME: 'tag' should be removed, and a LocationContext should be used + // instead. void EvalStore(ExplodedNodeSet& Dst, Expr* AssignE, Expr* StoreE, - ExplodedNode* Pred, - const GRState* St, SVal TargetLV, SVal Val, + ExplodedNode* Pred, const GRState* St, SVal TargetLV, SVal Val, const void *tag = 0); }; diff --git a/include/clang/Analysis/ProgramPoint.h b/include/clang/Analysis/ProgramPoint.h index 7aaae9c866..75ae83f0a6 100644 --- a/include/clang/Analysis/ProgramPoint.h +++ b/include/clang/Analysis/ProgramPoint.h @@ -33,13 +33,10 @@ public: BlockEntranceKind, BlockExitKind, PreStmtKind, - // Keep the following together and in this order. PostStmtKind, - PostLocationChecksSucceedKind, - PostOutOfBoundsCheckFailedKind, - PostNullCheckFailedKind, - PostUndefLocationCheckFailedKind, + PreLoadKind, PostLoadKind, + PreStoreKind, PostStoreKind, PostPurgeDeadSymbolsKind, PostStmtCustomKind, @@ -194,17 +191,6 @@ public: } }; -class PostLocationChecksSucceed : public PostStmt { -public: - PostLocationChecksSucceed(const Stmt* S, const LocationContext *L, - const void *tag = 0) - : PostStmt(S, PostLocationChecksSucceedKind, L, tag) {} - - static bool classof(const ProgramPoint* Location) { - return Location->getKind() == PostLocationChecksSucceedKind; - } -}; - class PostStmtCustom : public PostStmt { public: PostStmtCustom(const Stmt* S, @@ -226,36 +212,36 @@ public: } }; -class PostOutOfBoundsCheckFailed : public PostStmt { -public: - PostOutOfBoundsCheckFailed(const Stmt* S, const LocationContext *L, - const void *tag = 0) - : PostStmt(S, PostOutOfBoundsCheckFailedKind, L, tag) {} - - static bool classof(const ProgramPoint* Location) { - return Location->getKind() == PostOutOfBoundsCheckFailedKind; + +class LocationCheck : public StmtPoint { +protected: + LocationCheck(const Stmt *S, const LocationContext *L, + ProgramPoint::Kind K, const void *tag) + : StmtPoint(S, NULL, K, L, tag) {} + + static bool classof(const ProgramPoint *location) { + unsigned k = location->getKind(); + return k == PreLoadKind || PreStoreKind; } }; - -class PostUndefLocationCheckFailed : public PostStmt { + +class PreLoad : public LocationCheck { public: - PostUndefLocationCheckFailed(const Stmt* S, const LocationContext *L, - const void *tag = 0) - : PostStmt(S, PostUndefLocationCheckFailedKind, L, tag) {} - - static bool classof(const ProgramPoint* Location) { - return Location->getKind() == PostUndefLocationCheckFailedKind; + PreLoad(const Stmt *S, const LocationContext *L, const void *tag = 0) + : LocationCheck(S, L, PreLoadKind, tag) {} + + static bool classof(const ProgramPoint *location) { + return location->getKind() == PreLoadKind; } }; -class PostNullCheckFailed : public PostStmt { +class PreStore : public LocationCheck { public: - PostNullCheckFailed(const Stmt* S, const LocationContext *L, - const void *tag = 0) - : PostStmt(S, PostNullCheckFailedKind, L, tag) {} - - static bool classof(const ProgramPoint* Location) { - return Location->getKind() == PostNullCheckFailedKind; + PreStore(const Stmt *S, const LocationContext *L, const void *tag = 0) + : LocationCheck(S, L, PreStoreKind, tag) {} + + static bool classof(const ProgramPoint *location) { + return location->getKind() == PreStoreKind; } }; diff --git a/lib/Analysis/DereferenceChecker.cpp b/lib/Analysis/DereferenceChecker.cpp index b7233419ef..6fdad5d2a4 100644 --- a/lib/Analysis/DereferenceChecker.cpp +++ b/lib/Analysis/DereferenceChecker.cpp @@ -13,99 +13,103 @@ //===----------------------------------------------------------------------===// #include "clang/Analysis/PathSensitive/Checkers/DereferenceChecker.h" +#include "clang/Analysis/PathSensitive/Checker.h" #include "clang/Analysis/PathSensitive/GRExprEngine.h" #include "clang/Analysis/PathSensitive/BugReporter.h" +#include "GRExprEngineInternalChecks.h" using namespace clang; -void *NullDerefChecker::getTag() { - static int x = 0; - return &x; -} - -ExplodedNode *NullDerefChecker::CheckLocation(const Stmt *S, ExplodedNode *Pred, - const GRState *state, SVal V, - GRExprEngine &Eng) { - Loc *LV = dyn_cast(&V); - - // If the value is not a location, don't touch the node. - if (!LV) - return Pred; - - const GRState *NotNullState = state->Assume(*LV, true); - const GRState *NullState = state->Assume(*LV, false); +namespace { +class VISIBILITY_HIDDEN DereferenceChecker : public Checker { + BuiltinBug *BT_null; + BuiltinBug *BT_undef; + llvm::SmallVector ImplicitNullDerefNodes; +public: + DereferenceChecker() : BT_null(0), BT_undef(0) {} + static void *getTag() { static int tag = 0; return &tag; } + void VisitLocation(CheckerContext &C, const Stmt *S, SVal location); - GRStmtNodeBuilder &Builder = Eng.getBuilder(); - BugReporter &BR = Eng.getBugReporter(); - - // The explicit NULL case. - if (NullState) { - // Use the GDM to mark in the state what lval was null. - const SVal *PersistentLV = Eng.getBasicVals().getPersistentSVal(*LV); - NullState = NullState->set(PersistentLV); - - ExplodedNode *N = Builder.generateNode(S, NullState, Pred, - ProgramPoint::PostNullCheckFailedKind); - if (N) { - N->markAsSink(); - - if (!NotNullState) { // Explicit null case. - if (!BT) - BT = new BuiltinBug("Null dereference","Dereference of null pointer"); - - EnhancedBugReport *R = - new EnhancedBugReport(*BT, BT->getDescription().c_str(), N); - - R->addVisitorCreator(bugreporter::registerTrackNullOrUndefValue, - bugreporter::GetDerefExpr(N)); - - BR.EmitReport(R); - - return 0; - } else // Implicit null case. - ImplicitNullDerefNodes.push_back(N); - } + std::pair + getImplicitNodes() const { + return std::make_pair(ImplicitNullDerefNodes.data(), + ImplicitNullDerefNodes.data() + + ImplicitNullDerefNodes.size()); } - - if (!NotNullState) - return 0; +}; +} // end anonymous namespace - return Builder.generateNode(S, NotNullState, Pred, - ProgramPoint::PostLocationChecksSucceedKind); +void clang::RegisterDereferenceChecker(GRExprEngine &Eng) { + Eng.registerCheck(new DereferenceChecker()); } - -void *UndefDerefChecker::getTag() { - static int x = 0; - return &x; +std::pair +clang::GetImplicitNullDereferences(GRExprEngine &Eng) { + DereferenceChecker *checker = Eng.getChecker(); + if (!checker) + return std::make_pair((ExplodedNode * const *) 0, + (ExplodedNode * const *) 0); + return checker->getImplicitNodes(); } -ExplodedNode *UndefDerefChecker::CheckLocation(const Stmt *S, - ExplodedNode *Pred, - const GRState *state, SVal V, - GRExprEngine &Eng) { - GRStmtNodeBuilder &Builder = Eng.getBuilder(); - BugReporter &BR = Eng.getBugReporter(); - - if (V.isUndef()) { - ExplodedNode *N = Builder.generateNode(S, state, Pred, - ProgramPoint::PostUndefLocationCheckFailedKind); +void DereferenceChecker::VisitLocation(CheckerContext &C, const Stmt *S, + SVal l) { + // Check for dereference of an undefined value. + if (l.isUndef()) { + ExplodedNode *N = C.GenerateNode(S, true); if (N) { - N->markAsSink(); + if (!BT_undef) + BT_undef = new BuiltinBug("Dereference of undefined pointer value"); + + EnhancedBugReport *report = + new EnhancedBugReport(*BT_undef, BT_undef->getDescription().c_str(), N); + report->addVisitorCreator(bugreporter::registerTrackNullOrUndefValue, + bugreporter::GetDerefExpr(N)); + C.EmitReport(report); + } + return; + } + + DefinedOrUnknownSVal location = cast(l); + + // Check for null dereferences. + if (!isa(location)) + return; + + const GRState *state = C.getState(); + const GRState *notNullState, *nullState; + llvm::tie(notNullState, nullState) = state->Assume(location); + + // The explicit NULL case. + if (nullState) { + // Generate an error node. + ExplodedNode *N = C.GenerateNode(S, nullState, true); + if (N) { + if (!notNullState) { + // We know that 'location' cannot be non-null. This is what + // we call an "explicit" null dereference. + if (!BT_null) + BT_null = new BuiltinBug("Null pointer dereference", + "Dereference of null pointer"); - if (!BT) - BT = new BuiltinBug(0, "Undefined dereference", - "Dereference of undefined pointer value"); + EnhancedBugReport *report = + new EnhancedBugReport(*BT_null, BT_null->getDescription().c_str(), N); + report->addVisitorCreator(bugreporter::registerTrackNullOrUndefValue, + bugreporter::GetDerefExpr(N)); + + C.EmitReport(report); + return; + } - EnhancedBugReport *R = - new EnhancedBugReport(*BT, BT->getDescription().c_str(), N); - R->addVisitorCreator(bugreporter::registerTrackNullOrUndefValue, - bugreporter::GetDerefExpr(N)); - BR.EmitReport(R); + // Otherwise, we have the case where the location could either be + // null or not-null. Record the error node as an "implicit" null + // dereference. + ImplicitNullDerefNodes.push_back(N); } - return 0; } - - return Pred; + + // From this point forward, we know that the location is not null. + assert(notNullState); + C.addTransition(state != nullState ? C.GenerateNode(S, notNullState) : + C.getPredecessor()); } - diff --git a/lib/Analysis/GRCoreEngine.cpp b/lib/Analysis/GRCoreEngine.cpp index 87472472fd..b99ba4f257 100644 --- a/lib/Analysis/GRCoreEngine.cpp +++ b/lib/Analysis/GRCoreEngine.cpp @@ -418,51 +418,38 @@ void GRStmtNodeBuilder::GenerateAutoTransition(ExplodedNode* N) { Eng.WList->Enqueue(Succ, B, Idx+1); } -static inline PostStmt GetPostLoc(const Stmt* S, ProgramPoint::Kind K, - const LocationContext *L, const void *tag) { +static ProgramPoint GetProgramPoint(const Stmt *S, ProgramPoint::Kind K, + const LocationContext *LC, const void *tag){ switch (K) { default: - assert(false && "Invalid PostXXXKind."); - + assert(false && "Unhandled ProgramPoint kind"); + case ProgramPoint::PreStmtKind: + return PreStmt(S, LC, tag); case ProgramPoint::PostStmtKind: - return PostStmt(S, L, tag); - + return PostStmt(S, LC, tag); + case ProgramPoint::PreLoadKind: + return PreLoad(S, LC, tag); case ProgramPoint::PostLoadKind: - return PostLoad(S, L, tag); - - case ProgramPoint::PostUndefLocationCheckFailedKind: - return PostUndefLocationCheckFailed(S, L, tag); - - case ProgramPoint::PostLocationChecksSucceedKind: - return PostLocationChecksSucceed(S, L, tag); - - case ProgramPoint::PostOutOfBoundsCheckFailedKind: - return PostOutOfBoundsCheckFailed(S, L, tag); - - case ProgramPoint::PostNullCheckFailedKind: - return PostNullCheckFailed(S, L, tag); - + return PostLoad(S, LC, tag); + case ProgramPoint::PreStoreKind: + return PreStore(S, LC, tag); case ProgramPoint::PostStoreKind: - return PostStore(S, L, tag); - + return PostStore(S, LC, tag); case ProgramPoint::PostLValueKind: - return PostLValue(S, L, tag); - + return PostLValue(S, LC, tag); case ProgramPoint::PostPurgeDeadSymbolsKind: - return PostPurgeDeadSymbols(S, L, tag); + return PostPurgeDeadSymbols(S, LC, tag); } } ExplodedNode* -GRStmtNodeBuilder::generateNodeInternal(const Stmt* S, const GRState* State, +GRStmtNodeBuilder::generateNodeInternal(const Stmt* S, const GRState* state, ExplodedNode* Pred, ProgramPoint::Kind K, const void *tag) { - return K == ProgramPoint::PreStmtKind - ? generateNodeInternal(PreStmt(S, Pred->getLocationContext(),tag), - State, Pred) - : generateNodeInternal(GetPostLoc(S, K, Pred->getLocationContext(), tag), - State, Pred); + + const ProgramPoint &L = GetProgramPoint(S, K, Pred->getLocationContext(),tag); + return generateNodeInternal(L, state, Pred); } ExplodedNode* diff --git a/lib/Analysis/GRExprEngine.cpp b/lib/Analysis/GRExprEngine.cpp index 528a469286..c7b803679e 100644 --- a/lib/Analysis/GRExprEngine.cpp +++ b/lib/Analysis/GRExprEngine.cpp @@ -1190,68 +1190,87 @@ void GRExprEngine::EvalStore(ExplodedNodeSet& Dst, Expr *AssignE, assert(Builder && "GRStmtNodeBuilder must be defined."); // Evaluate the location (checks for bad dereferences). - Pred = EvalLocation(StoreE, Pred, state, location, tag); + ExplodedNodeSet Tmp; + EvalLocation(Tmp, StoreE, Pred, state, location, tag, false); - if (!Pred) + if (Tmp.empty()) return; - assert (!location.isUndef()); - state = GetState(Pred); + assert(!location.isUndef()); + SaveAndRestore OldSPointKind(Builder->PointKind, + ProgramPoint::PostStoreKind); + SaveAndRestore OldTag(Builder->Tag, tag); + // Proceed with the store. - SaveAndRestore OldSPointKind(Builder->PointKind); - SaveAndRestore OldTag(Builder->Tag); - Builder->PointKind = ProgramPoint::PostStoreKind; - Builder->Tag = tag; - EvalBind(Dst, AssignE, StoreE, Pred, state, location, Val); + for (ExplodedNodeSet::iterator NI=Tmp.begin(), NE=Tmp.end(); NI!=NE; ++NI) + EvalBind(Dst, AssignE, StoreE, *NI, GetState(*NI), location, Val); } -void GRExprEngine::EvalLoad(ExplodedNodeSet& Dst, Expr* Ex, ExplodedNode* Pred, +void GRExprEngine::EvalLoad(ExplodedNodeSet& Dst, Expr *Ex, ExplodedNode* Pred, const GRState* state, SVal location, const void *tag) { // Evaluate the location (checks for bad dereferences). - Pred = EvalLocation(Ex, Pred, state, location, tag); + ExplodedNodeSet Tmp; + EvalLocation(Tmp, Ex, Pred, state, location, tag, true); - if (!Pred) + if (Tmp.empty()) return; - - state = GetState(Pred); + + assert(!location.isUndef()); + + SaveAndRestore OldSPointKind(Builder->PointKind); + SaveAndRestore OldTag(Builder->Tag); // Proceed with the load. - ProgramPoint::Kind K = ProgramPoint::PostLoadKind; - - if (location.isUnknown()) { - // This is important. We must nuke the old binding. - MakeNode(Dst, Ex, Pred, state->BindExpr(Ex, UnknownVal()), - K, tag); - } - else { - SVal V = state->getSVal(cast(location), Ex->getType()); - MakeNode(Dst, Ex, Pred, state->BindExpr(Ex, V), K, tag); + for (ExplodedNodeSet::iterator NI=Tmp.begin(), NE=Tmp.end(); NI!=NE; ++NI) { + state = GetState(*NI); + if (location.isUnknown()) { + // This is important. We must nuke the old binding. + MakeNode(Dst, Ex, *NI, state->BindExpr(Ex, UnknownVal()), + ProgramPoint::PostLoadKind, tag); + } + else { + SVal V = state->getSVal(cast(location), Ex->getType()); + MakeNode(Dst, Ex, *NI, state->BindExpr(Ex, V), ProgramPoint::PostLoadKind, + tag); + } } } -ExplodedNode* GRExprEngine::EvalLocation(Stmt* Ex, ExplodedNode* Pred, - const GRState* state, SVal location, - const void *tag) { - - SaveAndRestore OldTag(Builder->Tag); - Builder->Tag = tag; - - if (location.isUnknown() || Checkers.empty()) - return Pred; +void GRExprEngine::EvalLocation(ExplodedNodeSet &Dst, Stmt *S, + ExplodedNode* Pred, + const GRState* state, SVal location, + const void *tag, bool isLoad) { + if (location.isUnknown() || Checkers.empty()) { + Dst.Add(Pred); + return; + } - for (CheckersOrdered::iterator I=Checkers.begin(), E=Checkers.end(); I!=E;++I) + ExplodedNodeSet Src, Tmp; + Src.Add(Pred); + ExplodedNodeSet *PrevSet = &Src; + + for (CheckersOrdered::iterator I=Checkers.begin(),E=Checkers.end(); I!=E; ++I) { - Pred = I->second->CheckLocation(Ex, Pred, state, location, *this); - if (!Pred) - break; + ExplodedNodeSet *CurrSet = (I+1 == E) ? &Dst + : (PrevSet == &Tmp) ? &Src : &Tmp; + + CurrSet->clear(); + void *tag = I->first; + Checker *checker = I->second; + + for (ExplodedNodeSet::iterator NI = PrevSet->begin(), NE = PrevSet->end(); + NI != NE; ++NI) + checker->GR_VisitLocation(*CurrSet, *Builder, *this, S, *NI, state, + location, tag, isLoad); + + // Update which NodeSet is the current one. + PrevSet = CurrSet; } - return Pred; - // FIXME: Temporarily disable out-of-bounds checking until we make // the logic reflect recent changes to CastRegion and friends. #if 0 @@ -1746,46 +1765,47 @@ void GRExprEngine::VisitObjCForCollectionStmtAux(ObjCForCollectionStmt* S, ExplodedNode* Pred, ExplodedNodeSet& Dst, SVal ElementV) { - - - // Get the current state. Use 'EvalLocation' to determine if it is a null - // pointer, etc. + // Check if the location we are writing back to is a null pointer. Stmt* elem = S->getElement(); - - Pred = EvalLocation(elem, Pred, GetState(Pred), ElementV); - if (!Pred) + ExplodedNodeSet Tmp; + EvalLocation(Tmp, elem, Pred, GetState(Pred), ElementV, NULL, false); + + if (Tmp.empty()) return; + + for (ExplodedNodeSet::iterator NI=Tmp.begin(), NE=Tmp.end(); NI!=NE; ++NI) { + Pred = *NI; + const GRState *state = GetState(Pred); + + // Handle the case where the container still has elements. + SVal TrueV = ValMgr.makeTruthVal(1); + const GRState *hasElems = state->BindExpr(S, TrueV); + + // Handle the case where the container has no elements. + SVal FalseV = ValMgr.makeTruthVal(0); + const GRState *noElems = state->BindExpr(S, FalseV); + + if (loc::MemRegionVal* MV = dyn_cast(&ElementV)) + if (const TypedRegion* R = dyn_cast(MV->getRegion())) { + // FIXME: The proper thing to do is to really iterate over the + // container. We will do this with dispatch logic to the store. + // For now, just 'conjure' up a symbolic value. + QualType T = R->getValueType(getContext()); + assert(Loc::IsLocType(T)); + unsigned Count = Builder->getCurrentBlockCount(); + SymbolRef Sym = SymMgr.getConjuredSymbol(elem, T, Count); + SVal V = ValMgr.makeLoc(Sym); + hasElems = hasElems->bindLoc(ElementV, V); + + // Bind the location to 'nil' on the false branch. + SVal nilV = ValMgr.makeIntVal(0, T); + noElems = noElems->bindLoc(ElementV, nilV); + } - const GRState *state = GetState(Pred); - - // Handle the case where the container still has elements. - SVal TrueV = ValMgr.makeTruthVal(1); - const GRState *hasElems = state->BindExpr(S, TrueV); - - // Handle the case where the container has no elements. - SVal FalseV = ValMgr.makeTruthVal(0); - const GRState *noElems = state->BindExpr(S, FalseV); - - if (loc::MemRegionVal* MV = dyn_cast(&ElementV)) - if (const TypedRegion* R = dyn_cast(MV->getRegion())) { - // FIXME: The proper thing to do is to really iterate over the - // container. We will do this with dispatch logic to the store. - // For now, just 'conjure' up a symbolic value. - QualType T = R->getValueType(getContext()); - assert (Loc::IsLocType(T)); - unsigned Count = Builder->getCurrentBlockCount(); - SymbolRef Sym = SymMgr.getConjuredSymbol(elem, T, Count); - SVal V = ValMgr.makeLoc(Sym); - hasElems = hasElems->bindLoc(ElementV, V); - - // Bind the location to 'nil' on the false branch. - SVal nilV = ValMgr.makeIntVal(0, T); - noElems = noElems->bindLoc(ElementV, nilV); - } - - // Create the new nodes. - MakeNode(Dst, S, Pred, hasElems); - MakeNode(Dst, S, Pred, noElems); + // Create the new nodes. + MakeNode(Dst, S, Pred, hasElems); + MakeNode(Dst, S, Pred, noElems); + } } //===----------------------------------------------------------------------===// @@ -2920,10 +2940,6 @@ struct VISIBILITY_HIDDEN DOTGraphTraits : Out << "\\lPostStore\\l"; else if (isa(Loc)) Out << "\\lPostLValue\\l"; - else if (isa(Loc)) - Out << "\\lPostLocationChecksSucceed\\l"; - else if (isa(Loc)) - Out << "\\lPostNullCheckFailed\\l"; if (GraphPrintCheckerState->isImplicitNullDeref(N)) Out << "\\|Implicit-Null Dereference.\\l"; diff --git a/lib/Analysis/GRExprEngineInternalChecks.cpp b/lib/Analysis/GRExprEngineInternalChecks.cpp index ee82969cd3..66e021095a 100644 --- a/lib/Analysis/GRExprEngineInternalChecks.cpp +++ b/lib/Analysis/GRExprEngineInternalChecks.cpp @@ -16,7 +16,6 @@ #include "clang/Analysis/PathSensitive/BugReporter.h" #include "clang/Analysis/PathSensitive/GRExprEngine.h" #include "clang/Analysis/PathSensitive/CheckerVisitor.h" -#include "clang/Analysis/PathSensitive/Checkers/DereferenceChecker.h" #include "clang/Analysis/PathSensitive/Checkers/BadCallChecker.h" #include "clang/Analysis/PathSensitive/Checkers/UndefinedArgChecker.h" #include "clang/Analysis/PathSensitive/Checkers/UndefinedAssignmentChecker.h" @@ -405,9 +404,8 @@ void GRExprEngine::RegisterInternalChecks() { registerCheck(new UndefinedArgChecker()); registerCheck(new UndefinedAssignmentChecker()); registerCheck(new BadCallChecker()); - registerCheck(new UndefDerefChecker()); - registerCheck(new NullDerefChecker()); + RegisterDereferenceChecker(*this); RegisterVLASizeChecker(*this); RegisterDivZeroChecker(*this); RegisterReturnStackAddressChecker(*this); diff --git a/lib/Analysis/GRExprEngineInternalChecks.h b/lib/Analysis/GRExprEngineInternalChecks.h index cfbf5d7e15..7ee1c2a1b5 100644 --- a/lib/Analysis/GRExprEngineInternalChecks.h +++ b/lib/Analysis/GRExprEngineInternalChecks.h @@ -19,6 +19,7 @@ namespace clang { class GRExprEngine; +void RegisterDereferenceChecker(GRExprEngine &Eng); void RegisterDivZeroChecker(GRExprEngine &Eng); void RegisterReturnPointerRangeChecker(GRExprEngine &Eng); void RegisterReturnStackAddressChecker(GRExprEngine &Eng); @@ -28,5 +29,6 @@ void RegisterPointerSubChecker(GRExprEngine &Eng); void RegisterPointerArithChecker(GRExprEngine &Eng); void RegisterFixedAddressChecker(GRExprEngine &Eng); void RegisterCastToStructChecker(GRExprEngine &Eng); + } // end clang namespace #endif diff --git a/lib/Analysis/NSErrorChecker.cpp b/lib/Analysis/NSErrorChecker.cpp index 307686ff57..93b617b115 100644 --- a/lib/Analysis/NSErrorChecker.cpp +++ b/lib/Analysis/NSErrorChecker.cpp @@ -209,15 +209,12 @@ void NSErrorChecker::CheckParamDeref(const VarDecl *Param, return; // Iterate over the implicit-null dereferences. - NullDerefChecker *Checker = Eng.getChecker(); - assert(Checker && "NullDerefChecker not exist."); - for (NullDerefChecker::iterator I = Checker->implicit_nodes_begin(), - E = Checker->implicit_nodes_end(); I != E; ++I) { - + ExplodedNode *const* I, *const* E; + llvm::tie(I, E) = GetImplicitNullDereferences(Eng); + for ( ; I != E; ++I) { const GRState *state = (*I)->getState(); - const SVal* X = state->get(); - - if (!X || X->getAsSymbol() != ParamSym) + SVal location = state->getSVal((*I)->getLocationAs()->getStmt()); + if (location.getAsSymbol() != ParamSym) continue; // Emit an error.