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/lib/Analysis/DereferenceChecker.cpp b/lib/Analysis/DereferenceChecker.cpp
index b723341..6fdad5d 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;
+namespace {
+class VISIBILITY_HIDDEN DereferenceChecker : public Checker {
+ BuiltinBug *BT_null;
+ BuiltinBug *BT_undef;
+ llvm::SmallVector<ExplodedNode*, 2> 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);
+
+ std::pair<ExplodedNode * const*, ExplodedNode * const*>
+ getImplicitNodes() const {
+ return std::make_pair(ImplicitNullDerefNodes.data(),
+ ImplicitNullDerefNodes.data() +
+ ImplicitNullDerefNodes.size());
+ }
+};
+} // end anonymous namespace
+
+void clang::RegisterDereferenceChecker(GRExprEngine &Eng) {
+ Eng.registerCheck(new DereferenceChecker());
}
-ExplodedNode *NullDerefChecker::CheckLocation(const Stmt *S, ExplodedNode *Pred,
- const GRState *state, SVal V,
- GRExprEngine &Eng) {
- Loc *LV = dyn_cast<Loc>(&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);
-
- 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<GRState::NullDerefTag>(PersistentLV);
-
- ExplodedNode *N = Builder.generateNode(S, NullState, Pred,
- ProgramPoint::PostNullCheckFailedKind);
+std::pair<ExplodedNode * const *, ExplodedNode * const *>
+clang::GetImplicitNullDereferences(GRExprEngine &Eng) {
+ DereferenceChecker *checker = Eng.getChecker<DereferenceChecker>();
+ if (!checker)
+ return std::make_pair((ExplodedNode * const *) 0,
+ (ExplodedNode * const *) 0);
+ return checker->getImplicitNodes();
+}
+
+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");
- if (!NotNullState) { // Explicit null case.
- if (!BT)
- BT = new BuiltinBug("Null dereference","Dereference of null pointer");
+ 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<DefinedOrUnknownSVal>(l);
+
+ // Check for null dereferences.
+ if (!isa<Loc>(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");
- EnhancedBugReport *R =
- new EnhancedBugReport(*BT, BT->getDescription().c_str(), N);
+ EnhancedBugReport *report =
+ new EnhancedBugReport(*BT_null, BT_null->getDescription().c_str(), N);
+ report->addVisitorCreator(bugreporter::registerTrackNullOrUndefValue,
+ bugreporter::GetDerefExpr(N));
+
+ C.EmitReport(report);
+ return;
+ }
- R->addVisitorCreator(bugreporter::registerTrackNullOrUndefValue,
- bugreporter::GetDerefExpr(N));
-
- BR.EmitReport(R);
-
- return 0;
- } else // Implicit null case.
- ImplicitNullDerefNodes.push_back(N);
+ // 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);
}
}
- if (!NotNullState)
- return 0;
-
- return Builder.generateNode(S, NotNullState, Pred,
- ProgramPoint::PostLocationChecksSucceedKind);
+ // From this point forward, we know that the location is not null.
+ assert(notNullState);
+ C.addTransition(state != nullState ? C.GenerateNode(S, notNullState) :
+ C.getPredecessor());
}
-
-
-void *UndefDerefChecker::getTag() {
- static int x = 0;
- return &x;
-}
-
-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);
- if (N) {
- N->markAsSink();
-
- if (!BT)
- BT = new BuiltinBug(0, "Undefined dereference",
- "Dereference of undefined pointer value");
-
- EnhancedBugReport *R =
- new EnhancedBugReport(*BT, BT->getDescription().c_str(), N);
- R->addVisitorCreator(bugreporter::registerTrackNullOrUndefValue,
- bugreporter::GetDerefExpr(N));
- BR.EmitReport(R);
- }
- return 0;
- }
-
- return Pred;
-}
-
diff --git a/lib/Analysis/GRCoreEngine.cpp b/lib/Analysis/GRCoreEngine.cpp
index 8747247..b99ba4f 100644
--- a/lib/Analysis/GRCoreEngine.cpp
+++ b/lib/Analysis/GRCoreEngine.cpp
@@ -418,51 +418,38 @@
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 528a469..c7b8036 100644
--- a/lib/Analysis/GRExprEngine.cpp
+++ b/lib/Analysis/GRExprEngine.cpp
@@ -1190,68 +1190,87 @@
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<ProgramPoint::Kind> OldSPointKind(Builder->PointKind,
+ ProgramPoint::PostStoreKind);
+ SaveAndRestore<const void*> OldTag(Builder->Tag, tag);
+
// Proceed with the store.
- SaveAndRestore<ProgramPoint::Kind> OldSPointKind(Builder->PointKind);
- SaveAndRestore<const void*> 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<ProgramPoint::Kind> OldSPointKind(Builder->PointKind);
+ SaveAndRestore<const void*> 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<Loc>(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<Loc>(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) {
+void GRExprEngine::EvalLocation(ExplodedNodeSet &Dst, Stmt *S,
+ ExplodedNode* Pred,
+ const GRState* state, SVal location,
+ const void *tag, bool isLoad) {
- SaveAndRestore<const void*> OldTag(Builder->Tag);
- Builder->Tag = tag;
-
- if (location.isUnknown() || Checkers.empty())
- return Pred;
-
-
- for (CheckersOrdered::iterator I=Checkers.begin(), E=Checkers.end(); I!=E;++I)
- {
- Pred = I->second->CheckLocation(Ex, Pred, state, location, *this);
- if (!Pred)
- break;
+ if (location.isUnknown() || Checkers.empty()) {
+ Dst.Add(Pred);
+ return;
}
- return Pred;
-
+ ExplodedNodeSet Src, Tmp;
+ Src.Add(Pred);
+ ExplodedNodeSet *PrevSet = &Src;
+
+ for (CheckersOrdered::iterator I=Checkers.begin(),E=Checkers.end(); I!=E; ++I)
+ {
+ 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;
+ }
+
// 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 @@
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);
- 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 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);
- // 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<loc::MemRegionVal>(&ElementV))
+ if (const TypedRegion* R = dyn_cast<TypedRegion>(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);
- if (loc::MemRegionVal* MV = dyn_cast<loc::MemRegionVal>(&ElementV))
- if (const TypedRegion* R = dyn_cast<TypedRegion>(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);
+ }
- // 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 @@
Out << "\\lPostStore\\l";
else if (isa<PostLValue>(Loc))
Out << "\\lPostLValue\\l";
- else if (isa<PostLocationChecksSucceed>(Loc))
- Out << "\\lPostLocationChecksSucceed\\l";
- else if (isa<PostNullCheckFailed>(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 ee82969..66e0210 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 @@
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 cfbf5d7..7ee1c2a 100644
--- a/lib/Analysis/GRExprEngineInternalChecks.h
+++ b/lib/Analysis/GRExprEngineInternalChecks.h
@@ -19,6 +19,7 @@
class GRExprEngine;
+void RegisterDereferenceChecker(GRExprEngine &Eng);
void RegisterDivZeroChecker(GRExprEngine &Eng);
void RegisterReturnPointerRangeChecker(GRExprEngine &Eng);
void RegisterReturnStackAddressChecker(GRExprEngine &Eng);
@@ -28,5 +29,6 @@
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 307686f..93b617b 100644
--- a/lib/Analysis/NSErrorChecker.cpp
+++ b/lib/Analysis/NSErrorChecker.cpp
@@ -209,15 +209,12 @@
return;
// Iterate over the implicit-null dereferences.
- NullDerefChecker *Checker = Eng.getChecker<NullDerefChecker>();
- 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<GRState::NullDerefTag>();
-
- if (!X || X->getAsSymbol() != ParamSym)
+ SVal location = state->getSVal((*I)->getLocationAs<StmtPoint>()->getStmt());
+ if (location.getAsSymbol() != ParamSym)
continue;
// Emit an error.