Rework ExprEngine::evalLoad and clients (e.g. VisitBinaryOperator) so that when we generate a new ExplodedNode
we use the same Expr* as the one being currently visited. This is preparation for transitioning to having
ProgramPoints refer to CFGStmts.
This required a bit of trickery. We wish to keep the old Expr* bindings in the Environment intact,
as plenty of logic relies on it and there is no reason to change it, but we sometimes want the Stmt* for
the ProgramPoint to be different than the Expr* being used for bindings. This requires adding an extra
argument for some functions (e.g., evalLocation). This looks a bit strange for some clients, but
it will look a lot cleaner when were start using CFGStmt* in the appropriate places.
As some fallout, the diagnostics arrows are a bit difference, since some of the node locations have changed.
I have audited these, and they look reasonable.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@154214 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/lib/StaticAnalyzer/Checkers/OSAtomicChecker.cpp b/lib/StaticAnalyzer/Checkers/OSAtomicChecker.cpp
index dc806c5..7b724d2 100644
--- a/lib/StaticAnalyzer/Checkers/OSAtomicChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/OSAtomicChecker.cpp
@@ -120,8 +120,8 @@
dyn_cast_or_null<TypedValueRegion>(location.getAsRegion())) {
LoadTy = TR->getValueType();
}
- Eng.evalLoad(Tmp, theValueExpr, Pred,
- state, location, &OSAtomicLoadTag, LoadTy);
+ Eng.evalLoad(Tmp, CE, theValueExpr, Pred,
+ state, location, &OSAtomicLoadTag, LoadTy);
if (Tmp.empty()) {
// If no nodes were generated, other checkers must have generated sinks.
@@ -172,8 +172,8 @@
val = svalBuilder.evalCast(val,R->getValueType(), newValueExpr->getType());
}
- Eng.evalStore(TmpStore, NULL, theValueExpr, N,
- stateEqual, location, val, &OSAtomicStoreTag);
+ Eng.evalStore(TmpStore, CE, theValueExpr, N,
+ stateEqual, location, val, &OSAtomicStoreTag);
if (TmpStore.empty()) {
// If no nodes were generated, other checkers must have generated sinks.
diff --git a/lib/StaticAnalyzer/Core/CheckerManager.cpp b/lib/StaticAnalyzer/Core/CheckerManager.cpp
index e8de329..0bcc343 100644
--- a/lib/StaticAnalyzer/Core/CheckerManager.cpp
+++ b/lib/StaticAnalyzer/Core/CheckerManager.cpp
@@ -222,25 +222,30 @@
const CheckersTy &Checkers;
SVal Loc;
bool IsLoad;
- const Stmt *S;
+ const Stmt *NodeEx; /* Will become a CFGStmt */
+ const Stmt *BoundEx;
ExprEngine &Eng;
CheckersTy::const_iterator checkers_begin() { return Checkers.begin(); }
CheckersTy::const_iterator checkers_end() { return Checkers.end(); }
CheckLocationContext(const CheckersTy &checkers,
- SVal loc, bool isLoad, const Stmt *s, ExprEngine &eng)
- : Checkers(checkers), Loc(loc), IsLoad(isLoad), S(s), Eng(eng) { }
+ SVal loc, bool isLoad, const Stmt *NodeEx,
+ const Stmt *BoundEx,
+ ExprEngine &eng)
+ : Checkers(checkers), Loc(loc), IsLoad(isLoad), NodeEx(NodeEx),
+ BoundEx(BoundEx), Eng(eng) {}
void runChecker(CheckerManager::CheckLocationFunc checkFn,
NodeBuilder &Bldr, ExplodedNode *Pred) {
ProgramPoint::Kind K = IsLoad ? ProgramPoint::PreLoadKind :
ProgramPoint::PreStoreKind;
- const ProgramPoint &L = ProgramPoint::getProgramPoint(S, K,
- Pred->getLocationContext(), checkFn.Checker);
+ const ProgramPoint &L =
+ ProgramPoint::getProgramPoint(NodeEx, K,
+ Pred->getLocationContext(),
+ checkFn.Checker);
CheckerContext C(Bldr, Eng, Pred, L);
-
- checkFn(Loc, IsLoad, S, C);
+ checkFn(Loc, IsLoad, BoundEx, C);
}
};
}
@@ -250,8 +255,11 @@
void CheckerManager::runCheckersForLocation(ExplodedNodeSet &Dst,
const ExplodedNodeSet &Src,
SVal location, bool isLoad,
- const Stmt *S, ExprEngine &Eng) {
- CheckLocationContext C(LocationCheckers, location, isLoad, S, Eng);
+ const Stmt *NodeEx,
+ const Stmt *BoundEx,
+ ExprEngine &Eng) {
+ CheckLocationContext C(LocationCheckers, location, isLoad, NodeEx,
+ BoundEx, Eng);
expandGraphWithCheckers(C, Dst, Src);
}
diff --git a/lib/StaticAnalyzer/Core/ExprEngine.cpp b/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 70921c5..30a511d 100644
--- a/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1542,7 +1542,7 @@
ProgramPoint::PostLValueKind);
else {
Bldr.takeNodes(Pred);
- evalLoad(Dst, M, Pred, state, L);
+ evalLoad(Dst, M, M, Pred, state, L);
Bldr.addNodes(Dst);
}
}
@@ -1611,7 +1611,7 @@
// Evaluate the location (checks for bad dereferences).
ExplodedNodeSet Tmp;
- evalLocation(Tmp, LocationE, Pred, state, location, tag, false);
+ evalLocation(Tmp, AssignE, LocationE, Pred, state, location, tag, false);
if (Tmp.empty())
return;
@@ -1623,15 +1623,17 @@
evalBind(Dst, StoreE, *NI, location, Val, false);
}
-void ExprEngine::evalLoad(ExplodedNodeSet &Dst, const Expr *Ex,
- ExplodedNode *Pred,
- ProgramStateRef state, SVal location,
- const ProgramPointTag *tag, QualType LoadTy) {
+void ExprEngine::evalLoad(ExplodedNodeSet &Dst,
+ const Expr *NodeEx,
+ const Expr *BoundEx,
+ ExplodedNode *Pred,
+ ProgramStateRef state,
+ SVal location,
+ const ProgramPointTag *tag,
+ QualType LoadTy)
+{
assert(!isa<NonLoc>(location) && "location cannot be a NonLoc.");
-
- if (isa<loc::ObjCPropRef>(location)) {
- assert(false);
- }
+ assert(!isa<loc::ObjCPropRef>(location));
// Are we loading from a region? This actually results in two loads; one
// to fetch the address of the referenced value and one to fetch the
@@ -1644,30 +1646,36 @@
static SimpleProgramPointTag
loadReferenceTag("ExprEngine : Load Reference");
ExplodedNodeSet Tmp;
- evalLoadCommon(Tmp, Ex, Pred, state, location, &loadReferenceTag,
+ evalLoadCommon(Tmp, NodeEx, BoundEx, Pred, state,
+ location, &loadReferenceTag,
getContext().getPointerType(RT->getPointeeType()));
// Perform the load from the referenced value.
for (ExplodedNodeSet::iterator I=Tmp.begin(), E=Tmp.end() ; I!=E; ++I) {
state = (*I)->getState();
- location = state->getSVal(Ex, (*I)->getLocationContext());
- evalLoadCommon(Dst, Ex, *I, state, location, tag, LoadTy);
+ location = state->getSVal(BoundEx, (*I)->getLocationContext());
+ evalLoadCommon(Dst, NodeEx, BoundEx, *I, state, location, tag, LoadTy);
}
return;
}
}
- evalLoadCommon(Dst, Ex, Pred, state, location, tag, LoadTy);
+ evalLoadCommon(Dst, NodeEx, BoundEx, Pred, state, location, tag, LoadTy);
}
-void ExprEngine::evalLoadCommon(ExplodedNodeSet &Dst, const Expr *Ex,
- ExplodedNode *Pred,
- ProgramStateRef state, SVal location,
- const ProgramPointTag *tag, QualType LoadTy) {
-
+void ExprEngine::evalLoadCommon(ExplodedNodeSet &Dst,
+ const Expr *NodeEx,
+ const Expr *BoundEx,
+ ExplodedNode *Pred,
+ ProgramStateRef state,
+ SVal location,
+ const ProgramPointTag *tag,
+ QualType LoadTy) {
+ assert(NodeEx);
+ assert(BoundEx);
// Evaluate the location (checks for bad dereferences).
ExplodedNodeSet Tmp;
- evalLocation(Tmp, Ex, Pred, state, location, tag, true);
+ evalLocation(Tmp, NodeEx, BoundEx, Pred, state, location, tag, true);
if (Tmp.empty())
return;
@@ -1682,24 +1690,30 @@
if (location.isUnknown()) {
// This is important. We must nuke the old binding.
- Bldr.generateNode(Ex, *NI, state->BindExpr(Ex, LCtx, UnknownVal()),
- false, tag, ProgramPoint::PostLoadKind);
+ Bldr.generateNode(NodeEx, *NI,
+ state->BindExpr(BoundEx, LCtx, UnknownVal()),
+ false, tag,
+ ProgramPoint::PostLoadKind);
}
else {
if (LoadTy.isNull())
- LoadTy = Ex->getType();
+ LoadTy = BoundEx->getType();
SVal V = state->getSVal(cast<Loc>(location), LoadTy);
- Bldr.generateNode(Ex, *NI, state->bindExprAndLocation(Ex, LCtx,
- location, V),
+ Bldr.generateNode(NodeEx, *NI,
+ state->bindExprAndLocation(BoundEx, LCtx, location, V),
false, tag, ProgramPoint::PostLoadKind);
}
}
}
-void ExprEngine::evalLocation(ExplodedNodeSet &Dst, const Stmt *S,
- ExplodedNode *Pred,
- ProgramStateRef state, SVal location,
- const ProgramPointTag *tag, bool isLoad) {
+void ExprEngine::evalLocation(ExplodedNodeSet &Dst,
+ const Stmt *NodeEx,
+ const Stmt *BoundEx,
+ ExplodedNode *Pred,
+ ProgramStateRef state,
+ SVal location,
+ const ProgramPointTag *tag,
+ bool isLoad) {
StmtNodeBuilder BldrTop(Pred, Dst, *currentBuilderContext);
// Early checks for performance reason.
if (location.isUnknown()) {
@@ -1721,12 +1735,11 @@
// FIXME: why is 'tag' not used instead of etag?
static SimpleProgramPointTag etag("ExprEngine: Location");
-
- Bldr.generateNode(S, Pred, state, false, &etag);
+ Bldr.generateNode(NodeEx, Pred, state, false, &etag);
}
ExplodedNodeSet Tmp;
- getCheckerManager().runCheckersForLocation(Tmp, Src, location, isLoad, S,
- *this);
+ getCheckerManager().runCheckersForLocation(Tmp, Src, location, isLoad,
+ NodeEx, BoundEx, *this);
BldrTop.addNodes(Tmp);
}
diff --git a/lib/StaticAnalyzer/Core/ExprEngineC.cpp b/lib/StaticAnalyzer/Core/ExprEngineC.cpp
index ee2d052..5ec3bc7 100644
--- a/lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -92,7 +92,7 @@
// null dereferences, and so on.
ExplodedNodeSet Tmp;
SVal location = LeftV;
- evalLoad(Tmp, LHS, *it, state, location);
+ evalLoad(Tmp, B, LHS, *it, state, location);
for (ExplodedNodeSet::iterator I = Tmp.begin(), E = Tmp.end(); I != E;
++I) {
@@ -189,7 +189,7 @@
ExplodedNode *subExprNode = *I;
ProgramStateRef state = subExprNode->getState();
const LocationContext *LCtx = subExprNode->getLocationContext();
- evalLoad(Dst, CastE, subExprNode, state, state->getSVal(Ex, LCtx));
+ evalLoad(Dst, CastE, CastE, subExprNode, state, state->getSVal(Ex, LCtx));
}
return;
}
@@ -693,7 +693,7 @@
// Perform a load.
ExplodedNodeSet Tmp;
- evalLoad(Tmp, Ex, Pred, state, loc);
+ evalLoad(Tmp, U, Ex, Pred, state, loc);
ExplodedNodeSet Dst2;
StmtNodeBuilder Bldr(Tmp, Dst2, *currentBuilderContext);
@@ -762,7 +762,7 @@
// Perform the store.
Bldr.takeNodes(*I);
ExplodedNodeSet Dst3;
- evalStore(Dst3, NULL, U, *I, state, loc, Result);
+ evalStore(Dst3, U, U, *I, state, loc, Result);
Bldr.addNodes(Dst3);
}
Dst.insert(Dst2);
diff --git a/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp b/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp
index 72d03a1..c8ad70a 100644
--- a/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp
@@ -87,7 +87,7 @@
ExplodedNodeSet dstLocation;
Bldr.takeNodes(Pred);
- evalLocation(dstLocation, elem, Pred, state, elementV, NULL, false);
+ evalLocation(dstLocation, S, elem, Pred, state, elementV, NULL, false);
Bldr.addNodes(dstLocation);
for (ExplodedNodeSet::iterator NI = dstLocation.begin(),