[analyzer] Run remove dead on end of path.
This will simplify checkers that need to register for leaks. Currently,
they have to register for both: check dead and check end of path.
I've modified the SymbolReaper to consider everything on the stack dead
if the input StackLocationContext is 0.
(This is a bit disruptive, so I'd like to flash out all the issues
asap.)
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@167352 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
index 6ac69c1..78b2542 100644
--- a/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ b/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -167,8 +167,12 @@
/// are usually reported here).
/// \param K - In some cases it is possible to use PreStmt kind. (Do
/// not use it unless you know what you are doing.)
+ /// If the ReferenceStmt is NULL, everything is this and parent contexts is
+ /// considered live.
+ /// If the stack frame context is NULL, everything on stack is considered
+ /// dead.
void removeDead(ExplodedNode *Node, ExplodedNodeSet &Out,
- const Stmt *ReferenceStmt, const LocationContext *LC,
+ const Stmt *ReferenceStmt, const StackFrameContext *LC,
const Stmt *DiagnosticStmt,
ProgramPoint::Kind K = ProgramPoint::PreStmtPurgeDeadSymbolsKind);
@@ -219,6 +223,11 @@
void processEndOfFunction(NodeBuilderContext& BC,
ExplodedNode *Pred);
+ /// Remove dead bindings/symbols before exiting a function.
+ void removeDeadOnEndOfFunction(NodeBuilderContext& BC,
+ ExplodedNode *Pred,
+ ExplodedNodeSet &Dst);
+
/// Generate the entry node of the callee.
void processCallEnter(CallEnter CE, ExplodedNode *Pred);
diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h b/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
index 0780716..873f773 100644
--- a/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
+++ b/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
@@ -580,9 +580,11 @@
///
/// If the statement is NULL, everything is this and parent contexts is
/// considered live.
- SymbolReaper(const LocationContext *ctx, const Stmt *s, SymbolManager& symmgr,
+ /// If the stack frame context is NULL, everything on stack is considered
+ /// dead.
+ SymbolReaper(const StackFrameContext *Ctx, const Stmt *s, SymbolManager& symmgr,
StoreManager &storeMgr)
- : LCtx(ctx->getCurrentStackFrame()), Loc(s), SymMgr(symmgr),
+ : LCtx(Ctx), Loc(s), SymMgr(symmgr),
reapedStore(0, storeMgr) {}
~SymbolReaper() {}
diff --git a/lib/StaticAnalyzer/Core/ExprEngine.cpp b/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 664d4d2..045591c 100644
--- a/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -268,12 +268,12 @@
void ExprEngine::removeDead(ExplodedNode *Pred, ExplodedNodeSet &Out,
const Stmt *ReferenceStmt,
- const LocationContext *LC,
+ const StackFrameContext *LC,
const Stmt *DiagnosticStmt,
ProgramPoint::Kind K) {
assert((K == ProgramPoint::PreStmtPurgeDeadSymbolsKind ||
- ReferenceStmt == 0) && "PreStmt is not generally supported by "
- "the SymbolReaper yet");
+ ReferenceStmt == 0)
+ && "PostStmt is not generally supported by the SymbolReaper yet");
NumRemoveDeadBindings++;
CleanedState = Pred->getState();
SymbolReaper SymReaper(LC, ReferenceStmt, SymMgr, getStoreManager());
@@ -346,7 +346,7 @@
ExplodedNodeSet CleanedStates;
if (shouldRemoveDeadBindings(AMgr, S, Pred, EntryNode->getLocationContext())){
removeDead(EntryNode, CleanedStates, currStmt,
- Pred->getLocationContext(), currStmt);
+ Pred->getStackFrame(), currStmt);
} else
CleanedStates.Add(EntryNode);
@@ -1315,8 +1315,22 @@
void ExprEngine::processEndOfFunction(NodeBuilderContext& BC,
ExplodedNode *Pred) {
StateMgr.EndPath(Pred->getState());
+
ExplodedNodeSet Dst;
- getCheckerManager().runCheckersForEndPath(BC, Dst, Pred, *this);
+ if (Pred->getLocationContext()->inTopFrame()) {
+ // Remove dead symbols.
+ ExplodedNodeSet AfterRemovedDead;
+ removeDeadOnEndOfFunction(BC, Pred, AfterRemovedDead);
+
+ // Notify checkers.
+ for (ExplodedNodeSet::iterator I = AfterRemovedDead.begin(),
+ E = AfterRemovedDead.end(); I != E; ++I) {
+ getCheckerManager().runCheckersForEndPath(BC, Dst, *I, *this);
+ }
+ } else {
+ getCheckerManager().runCheckersForEndPath(BC, Dst, Pred, *this);
+ }
+
Engine.enqueueEndOfFunction(Dst);
}
diff --git a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
index 438fbdf..cd67a03 100644
--- a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -98,13 +98,16 @@
break;
}
+ if (Node->pred_empty())
+ return std::pair<const Stmt*, const CFGBlock*>(0, 0);
+
Node = *Node->pred_begin();
}
const CFGBlock *Blk = 0;
if (S) {
// Now, get the enclosing basic block.
- while (Node && Node->pred_size() >=1 ) {
+ while (Node) {
const ProgramPoint &PP = Node->getLocation();
if (isa<BlockEdge>(PP) &&
(PP.getLocationContext()->getCurrentStackFrame() == SF)) {
@@ -112,6 +115,9 @@
Blk = EPP.getDst();
break;
}
+ if (Node->pred_empty())
+ return std::pair<const Stmt*, const CFGBlock*>(S, 0);
+
Node = *Node->pred_begin();
}
}
@@ -157,6 +163,32 @@
return UnknownVal();
}
+void ExprEngine::removeDeadOnEndOfFunction(NodeBuilderContext& BC,
+ ExplodedNode *Pred,
+ ExplodedNodeSet &Dst) {
+ NodeBuilder Bldr(Pred, Dst, BC);
+
+ // Find the last statement in the function and the corresponding basic block.
+ const Stmt *LastSt = 0;
+ const CFGBlock *Blk = 0;
+ llvm::tie(LastSt, Blk) = getLastStmt(Pred);
+ if (!Blk || !LastSt) {
+ return;
+ }
+
+ // If the last statement is return, everything it references should stay live.
+ if (isa<ReturnStmt>(LastSt))
+ return;
+
+ // Here, we call the Symbol Reaper with 0 stack context telling it to clean up
+ // everything on the stack. We use LastStmt as a diagnostic statement, with
+ // which the PreStmtPurgeDead point will be associated.
+ currBldrCtx = &BC;
+ removeDead(Pred, Dst, 0, 0, LastSt,
+ ProgramPoint::PostStmtPurgeDeadSymbolsKind);
+ currBldrCtx = 0;
+}
+
/// The call exit is simulated with a sequence of nodes, which occur between
/// CallExitBegin and CallExitEnd. The following operations occur between the
/// two program points:
diff --git a/lib/StaticAnalyzer/Core/RegionStore.cpp b/lib/StaticAnalyzer/Core/RegionStore.cpp
index 4c2f087..5b8f65d 100644
--- a/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ b/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -1805,7 +1805,8 @@
const StackArgumentsSpaceRegion *StackReg =
cast<StackArgumentsSpaceRegion>(TR->getSuperRegion());
const StackFrameContext *RegCtx = StackReg->getStackFrame();
- if (RegCtx == CurrentLCtx || RegCtx->isParentOf(CurrentLCtx))
+ if (CurrentLCtx &&
+ (RegCtx == CurrentLCtx || RegCtx->isParentOf(CurrentLCtx)))
AddToWorkList(TR, &C);
}
}
diff --git a/lib/StaticAnalyzer/Core/SymbolManager.cpp b/lib/StaticAnalyzer/Core/SymbolManager.cpp
index 9a11945..0c5098b 100644
--- a/lib/StaticAnalyzer/Core/SymbolManager.cpp
+++ b/lib/StaticAnalyzer/Core/SymbolManager.cpp
@@ -510,6 +510,9 @@
bool
SymbolReaper::isLive(const Stmt *ExprVal, const LocationContext *ELCtx) const {
+ if (LCtx == 0)
+ return false;
+
if (LCtx != ELCtx) {
// If the reaper's location context is a parent of the expression's
// location context, then the expression value is now "out of scope".
@@ -517,6 +520,7 @@
return false;
return true;
}
+
// If no statement is provided, everything is this and parent contexts is live.
if (!Loc)
return true;
@@ -526,6 +530,12 @@
bool SymbolReaper::isLive(const VarRegion *VR, bool includeStoreBindings) const{
const StackFrameContext *VarContext = VR->getStackFrame();
+
+ if (!VarContext)
+ return true;
+
+ if (!LCtx)
+ return false;
const StackFrameContext *CurrentContext = LCtx->getCurrentStackFrame();
if (VarContext == CurrentContext) {
@@ -557,7 +567,7 @@
return false;
}
- return !VarContext || VarContext->isParentOf(CurrentContext);
+ return VarContext->isParentOf(CurrentContext);
}
SymbolVisitor::~SymbolVisitor() {}
diff --git a/test/Analysis/simple-stream-checks.c b/test/Analysis/simple-stream-checks.c
index fca2edc..7a2718e 100644
--- a/test/Analysis/simple-stream-checks.c
+++ b/test/Analysis/simple-stream-checks.c
@@ -49,3 +49,17 @@
fclose(F);
int x = 0; // no warning
}
+
+void leakOnEnfOfPath1(int *Data) {
+ FILE *F = fopen("myfile.txt", "w");// expected-warning {{Opened file is never closed; potential resource leak}}
+}
+
+void leakOnEnfOfPath2(int *Data) {
+ FILE *F = fopen("myfile.txt", "w");
+ return; // expected-warning {{Opened file is never closed; potential resource leak}}
+}
+
+FILE *leakOnEnfOfPath3(int *Data) {
+ FILE *F = fopen("myfile.txt", "w");
+ return F;
+}
