r347953 - [analyzer] Fix the "Zombie Symbols" bug.
Artem Dergachev via cfe-commits
cfe-commits at lists.llvm.org
Thu Nov 29 19:27:50 PST 2018
Author: dergachev
Date: Thu Nov 29 19:27:50 2018
New Revision: 347953
URL: http://llvm.org/viewvc/llvm-project?rev=347953&view=rev
Log:
[analyzer] Fix the "Zombie Symbols" bug.
It's an old bug that consists in stale references to symbols remaining in the
GDM if they disappear from other program state sections as a result of any
operation that isn't the actual dead symbol collection. The most common example
here is:
FILE *fp = fopen("myfile.txt", "w");
fp = 0; // leak of file descriptor
In this example the leak were not detected previously because the symbol
disappears from the public part of the program state due to evaluating
the assignment. For that reason the checker never receives a notification
that the symbol is dead, and never reports a leak.
This patch not only causes leak false negatives, but also a number of other
problems, including false positives on some checkers.
What's worse, even though the program state contains a finite number of symbols,
the set of symbols that dies is potentially infinite. This means that is
impossible to compute the set of all dead symbols to pass off to the checkers
for cleaning up their part of the GDM.
No longer compute the dead set at all. Disallow iterating over dead symbols.
Disallow querying if any symbols are dead. Remove the API for marking symbols
as dead, as it is no longer necessary. Update checkers accordingly.
Differential Revision: https://reviews.llvm.org/D18860
Added:
cfe/trunk/test/Analysis/loop-block-counts.c
cfe/trunk/test/Analysis/retain-release-cpp-classes.cpp
Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Core/Environment.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
cfe/trunk/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
cfe/trunk/lib/StaticAnalyzer/Core/SymbolManager.cpp
cfe/trunk/test/Analysis/MisusedMovedObject.cpp
cfe/trunk/test/Analysis/keychainAPI.m
cfe/trunk/test/Analysis/pr22954.c
cfe/trunk/test/Analysis/self-assign.cpp
cfe/trunk/test/Analysis/simple-stream-checks.c
cfe/trunk/test/Analysis/unions.cpp
Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h?rev=347953&r1=347952&r2=347953&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h Thu Nov 29 19:27:50 2018
@@ -198,7 +198,7 @@ public:
auto &CZFactory = State->get_context<ConstraintSMT>();
for (auto I = CZ.begin(), E = CZ.end(); I != E; ++I) {
- if (SymReaper.maybeDead(I->first))
+ if (SymReaper.isDead(I->first))
CZ = CZFactory.remove(CZ, *I);
}
Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h?rev=347953&r1=347952&r2=347953&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h Thu Nov 29 19:27:50 2018
@@ -558,7 +558,6 @@ class SymbolReaper {
SymbolMapTy TheLiving;
SymbolSetTy MetadataInUse;
- SymbolSetTy TheDead;
RegionSetTy RegionRoots;
@@ -603,21 +602,6 @@ public:
/// symbol marking has occurred, i.e. in the MarkLiveSymbols callback.
void markInUse(SymbolRef sym);
- /// If a symbol is known to be live, marks the symbol as live.
- ///
- /// Otherwise, if the symbol cannot be proven live, it is marked as dead.
- /// Returns true if the symbol is dead, false if live.
- bool maybeDead(SymbolRef sym);
-
- using dead_iterator = SymbolSetTy::const_iterator;
-
- dead_iterator dead_begin() const { return TheDead.begin(); }
- dead_iterator dead_end() const { return TheDead.end(); }
-
- bool hasDeadSymbols() const {
- return !TheDead.empty();
- }
-
using region_iterator = RegionSetTy::const_iterator;
region_iterator region_begin() const { return RegionRoots.begin(); }
@@ -626,9 +610,9 @@ public:
/// Returns whether or not a symbol has been confirmed dead.
///
/// This should only be called once all marking of dead symbols has completed.
- /// (For checkers, this means only in the evalDeadSymbols callback.)
- bool isDead(SymbolRef sym) const {
- return TheDead.count(sym);
+ /// (For checkers, this means only in the checkDeadSymbols callback.)
+ bool isDead(SymbolRef sym) {
+ return !isLive(sym);
}
void markLive(const MemRegion *region);
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp?rev=347953&r1=347952&r2=347953&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp Thu Nov 29 19:27:50 2018
@@ -2385,9 +2385,6 @@ void CStringChecker::checkLiveSymbols(Pr
void CStringChecker::checkDeadSymbols(SymbolReaper &SR,
CheckerContext &C) const {
- if (!SR.hasDeadSymbols())
- return;
-
ProgramStateRef state = C.getState();
CStringLengthTy Entries = state->get<CStringLength>();
if (Entries.isEmpty())
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp?rev=347953&r1=347952&r2=347953&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp Thu Nov 29 19:27:50 2018
@@ -123,11 +123,6 @@ void DynamicTypePropagation::checkDeadSy
}
}
- if (!SR.hasDeadSymbols()) {
- C.addTransition(State);
- return;
- }
-
MostSpecializedTypeArgsMapTy TyArgMap =
State->get<MostSpecializedTypeArgsMap>();
for (MostSpecializedTypeArgsMapTy::iterator I = TyArgMap.begin(),
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp?rev=347953&r1=347952&r2=347953&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp Thu Nov 29 19:27:50 2018
@@ -100,9 +100,6 @@ void MPIChecker::checkUnmatchedWaits(con
void MPIChecker::checkMissingWaits(SymbolReaper &SymReaper,
CheckerContext &Ctx) const {
- if (!SymReaper.hasDeadSymbols())
- return;
-
ProgramStateRef State = Ctx.getState();
const auto &Requests = State->get<RequestMap>();
if (Requests.isEmpty())
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp?rev=347953&r1=347952&r2=347953&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp Thu Nov 29 19:27:50 2018
@@ -16,6 +16,7 @@
#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
#include "clang/StaticAnalyzer/Core/Checker.h"
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
@@ -29,6 +30,7 @@ namespace {
class MacOSKeychainAPIChecker : public Checker<check::PreStmt<CallExpr>,
check::PostStmt<CallExpr>,
check::DeadSymbols,
+ check::PointerEscape,
eval::Assume> {
mutable std::unique_ptr<BugType> BT;
@@ -58,6 +60,10 @@ public:
void checkPreStmt(const CallExpr *S, CheckerContext &C) const;
void checkPostStmt(const CallExpr *S, CheckerContext &C) const;
void checkDeadSymbols(SymbolReaper &SR, CheckerContext &C) const;
+ ProgramStateRef checkPointerEscape(ProgramStateRef State,
+ const InvalidatedSymbols &Escaped,
+ const CallEvent *Call,
+ PointerEscapeKind Kind) const;
ProgramStateRef evalAssume(ProgramStateRef state, SVal Cond,
bool Assumption) const;
void printState(raw_ostream &Out, ProgramStateRef State,
@@ -570,6 +576,44 @@ void MacOSKeychainAPIChecker::checkDeadS
C.addTransition(State, N);
}
+ProgramStateRef MacOSKeychainAPIChecker::checkPointerEscape(
+ ProgramStateRef State, const InvalidatedSymbols &Escaped,
+ const CallEvent *Call, PointerEscapeKind Kind) const {
+ // FIXME: This branch doesn't make any sense at all, but it is an overfitted
+ // replacement for a previous overfitted code that was making even less sense.
+ if (!Call || Call->getDecl())
+ return State;
+
+ for (auto I : State->get<AllocatedData>()) {
+ SymbolRef Sym = I.first;
+ if (Escaped.count(Sym))
+ State = State->remove<AllocatedData>(Sym);
+
+ // This checker is special. Most checkers in fact only track symbols of
+ // SymbolConjured type, eg. symbols returned from functions such as
+ // malloc(). This checker tracks symbols returned as out-parameters.
+ //
+ // When a function is evaluated conservatively, the out-parameter's pointee
+ // base region gets invalidated with a SymbolConjured. If the base region is
+ // larger than the region we're interested in, the value we're interested in
+ // would be SymbolDerived based on that SymbolConjured. However, such
+ // SymbolDerived will never be listed in the Escaped set when the base
+ // region is invalidated because ExprEngine doesn't know which symbols
+ // were derived from a given symbol, while there can be infinitely many
+ // valid symbols derived from any given symbol.
+ //
+ // Hence the extra boilerplate: remove the derived symbol when its parent
+ // symbol escapes.
+ //
+ if (const auto *SD = dyn_cast<SymbolDerived>(Sym)) {
+ SymbolRef ParentSym = SD->getParentSymbol();
+ if (Escaped.count(ParentSym))
+ State = State->remove<AllocatedData>(Sym);
+ }
+ }
+ return State;
+}
+
std::shared_ptr<PathDiagnosticPiece>
MacOSKeychainAPIChecker::SecKeychainBugVisitor::VisitNode(
const ExplodedNode *N, BugReporterContext &BRC, BugReport &BR) {
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp?rev=347953&r1=347952&r2=347953&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp Thu Nov 29 19:27:50 2018
@@ -2345,9 +2345,6 @@ void MallocChecker::reportLeak(SymbolRef
void MallocChecker::checkDeadSymbols(SymbolReaper &SymReaper,
CheckerContext &C) const
{
- if (!SymReaper.hasDeadSymbols())
- return;
-
ProgramStateRef state = C.getState();
RegionStateTy RS = state->get<RegionState>();
RegionStateTy::Factory &F = state->get_context<RegionState>();
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp?rev=347953&r1=347952&r2=347953&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp Thu Nov 29 19:27:50 2018
@@ -446,9 +446,6 @@ void NullabilityChecker::reportBugIfInva
/// Cleaning up the program state.
void NullabilityChecker::checkDeadSymbols(SymbolReaper &SR,
CheckerContext &C) const {
- if (!SR.hasDeadSymbols())
- return;
-
ProgramStateRef State = C.getState();
NullabilityMapTy Nullabilities = State->get<NullabilityMap>();
for (NullabilityMapTy::iterator I = Nullabilities.begin(),
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp?rev=347953&r1=347952&r2=347953&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp Thu Nov 29 19:27:50 2018
@@ -1442,20 +1442,18 @@ void RetainCountChecker::checkDeadSymbol
SmallVector<SymbolRef, 10> Leaked;
// Update counts from autorelease pools
- for (SymbolReaper::dead_iterator I = SymReaper.dead_begin(),
- E = SymReaper.dead_end(); I != E; ++I) {
- SymbolRef Sym = *I;
- if (const RefVal *T = B.lookup(Sym)){
- // Use the symbol as the tag.
- // FIXME: This might not be as unique as we would like.
+ for (const auto &I: state->get<RefBindings>()) {
+ SymbolRef Sym = I.first;
+ if (SymReaper.isDead(Sym)) {
static CheckerProgramPointTag Tag(this, "DeadSymbolAutorelease");
- state = handleAutoreleaseCounts(state, Pred, &Tag, C, Sym, *T);
+ const RefVal &V = I.second;
+ state = handleAutoreleaseCounts(state, Pred, &Tag, C, Sym, V);
if (!state)
return;
// Fetch the new reference count from the state, and use it to handle
// this symbol.
- state = handleSymbolDeath(state, *I, *getRefBinding(state, Sym), Leaked);
+ state = handleSymbolDeath(state, Sym, *getRefBinding(state, Sym), Leaked);
}
}
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/StreamChecker.cpp?rev=347953&r1=347952&r2=347953&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/StreamChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/StreamChecker.cpp Thu Nov 29 19:27:50 2018
@@ -383,26 +383,26 @@ ProgramStateRef StreamChecker::CheckDoub
void StreamChecker::checkDeadSymbols(SymbolReaper &SymReaper,
CheckerContext &C) const {
+ ProgramStateRef state = C.getState();
+
// TODO: Clean up the state.
- for (SymbolReaper::dead_iterator I = SymReaper.dead_begin(),
- E = SymReaper.dead_end(); I != E; ++I) {
- SymbolRef Sym = *I;
- ProgramStateRef state = C.getState();
- const StreamState *SS = state->get<StreamMap>(Sym);
- if (!SS)
+ const StreamMapTy &Map = state->get<StreamMap>();
+ for (const auto &I: Map) {
+ SymbolRef Sym = I.first;
+ const StreamState &SS = I.second;
+ if (!SymReaper.isDead(Sym) || !SS.isOpened())
continue;
- if (SS->isOpened()) {
- ExplodedNode *N = C.generateErrorNode();
- if (N) {
- if (!BT_ResourceLeak)
- BT_ResourceLeak.reset(new BuiltinBug(
- this, "Resource Leak",
- "Opened File never closed. Potential Resource leak."));
- C.emitReport(llvm::make_unique<BugReport>(
- *BT_ResourceLeak, BT_ResourceLeak->getDescription(), N));
- }
- }
+ ExplodedNode *N = C.generateErrorNode();
+ if (!N)
+ return;
+
+ if (!BT_ResourceLeak)
+ BT_ResourceLeak.reset(
+ new BuiltinBug(this, "Resource Leak",
+ "Opened File never closed. Potential Resource leak."));
+ C.emitReport(llvm::make_unique<BugReport>(
+ *BT_ResourceLeak, BT_ResourceLeak->getDescription(), N));
}
}
Modified: cfe/trunk/lib/StaticAnalyzer/Core/Environment.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/Environment.cpp?rev=347953&r1=347952&r2=347953&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/Environment.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/Environment.cpp Thu Nov 29 19:27:50 2018
@@ -193,11 +193,6 @@ EnvironmentManager::removeDeadBindings(E
// Mark all symbols in the block expr's value live.
RSScaner.scan(X);
- continue;
- } else {
- SymExpr::symbol_iterator SI = X.symbol_begin(), SE = X.symbol_end();
- for (; SI != SE; ++SI)
- SymReaper.maybeDead(*SI);
}
}
Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp?rev=347953&r1=347952&r2=347953&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp Thu Nov 29 19:27:50 2018
@@ -675,44 +675,35 @@ void ExprEngine::removeDead(ExplodedNode
// Process any special transfer function for dead symbols.
// A tag to track convenience transitions, which can be removed at cleanup.
static SimpleProgramPointTag cleanupTag(TagProviderName, "Clean Node");
- if (!SymReaper.hasDeadSymbols()) {
- // Generate a CleanedNode that has the environment and store cleaned
- // up. Since no symbols are dead, we can optimize and not clean out
- // the constraint manager.
- StmtNodeBuilder Bldr(Pred, Out, *currBldrCtx);
- Bldr.generateNode(DiagnosticStmt, Pred, CleanedState, &cleanupTag, K);
-
- } else {
- // Call checkers with the non-cleaned state so that they could query the
- // values of the soon to be dead symbols.
- ExplodedNodeSet CheckedSet;
- getCheckerManager().runCheckersForDeadSymbols(CheckedSet, Pred, SymReaper,
- DiagnosticStmt, *this, K);
-
- // For each node in CheckedSet, generate CleanedNodes that have the
- // environment, the store, and the constraints cleaned up but have the
- // user-supplied states as the predecessors.
- StmtNodeBuilder Bldr(CheckedSet, Out, *currBldrCtx);
- for (const auto I : CheckedSet) {
- ProgramStateRef CheckerState = I->getState();
-
- // The constraint manager has not been cleaned up yet, so clean up now.
- CheckerState = getConstraintManager().removeDeadBindings(CheckerState,
- SymReaper);
-
- assert(StateMgr.haveEqualEnvironments(CheckerState, Pred->getState()) &&
- "Checkers are not allowed to modify the Environment as a part of "
- "checkDeadSymbols processing.");
- assert(StateMgr.haveEqualStores(CheckerState, Pred->getState()) &&
- "Checkers are not allowed to modify the Store as a part of "
- "checkDeadSymbols processing.");
-
- // Create a state based on CleanedState with CheckerState GDM and
- // generate a transition to that state.
- ProgramStateRef CleanedCheckerSt =
+ // Call checkers with the non-cleaned state so that they could query the
+ // values of the soon to be dead symbols.
+ ExplodedNodeSet CheckedSet;
+ getCheckerManager().runCheckersForDeadSymbols(CheckedSet, Pred, SymReaper,
+ DiagnosticStmt, *this, K);
+
+ // For each node in CheckedSet, generate CleanedNodes that have the
+ // environment, the store, and the constraints cleaned up but have the
+ // user-supplied states as the predecessors.
+ StmtNodeBuilder Bldr(CheckedSet, Out, *currBldrCtx);
+ for (const auto I : CheckedSet) {
+ ProgramStateRef CheckerState = I->getState();
+
+ // The constraint manager has not been cleaned up yet, so clean up now.
+ CheckerState =
+ getConstraintManager().removeDeadBindings(CheckerState, SymReaper);
+
+ assert(StateMgr.haveEqualEnvironments(CheckerState, Pred->getState()) &&
+ "Checkers are not allowed to modify the Environment as a part of "
+ "checkDeadSymbols processing.");
+ assert(StateMgr.haveEqualStores(CheckerState, Pred->getState()) &&
+ "Checkers are not allowed to modify the Store as a part of "
+ "checkDeadSymbols processing.");
+
+ // Create a state based on CleanedState with CheckerState GDM and
+ // generate a transition to that state.
+ ProgramStateRef CleanedCheckerSt =
StateMgr.getPersistentStateWithGDM(CleanedState, CheckerState);
- Bldr.generateNode(DiagnosticStmt, I, CleanedCheckerSt, &cleanupTag, K);
- }
+ Bldr.generateNode(DiagnosticStmt, I, CleanedCheckerSt, &cleanupTag, K);
}
}
Modified: cfe/trunk/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp?rev=347953&r1=347952&r2=347953&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp Thu Nov 29 19:27:50 2018
@@ -399,7 +399,7 @@ RangeConstraintManager::removeDeadBindin
for (ConstraintRangeTy::iterator I = CR.begin(), E = CR.end(); I != E; ++I) {
SymbolRef Sym = I.getKey();
- if (SymReaper.maybeDead(Sym)) {
+ if (SymReaper.isDead(Sym)) {
Changed = true;
CR = CRFactory.remove(CR, Sym);
}
Modified: cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp?rev=347953&r1=347952&r2=347953&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp Thu Nov 29 19:27:50 2018
@@ -2571,24 +2571,9 @@ StoreRef RegionStoreManager::removeDeadB
const MemRegion *Base = I.getKey();
// If the cluster has been visited, we know the region has been marked.
- if (W.isVisited(Base))
- continue;
-
- // Remove the dead entry.
- B = B.remove(Base);
-
- if (const SymbolicRegion *SymR = dyn_cast<SymbolicRegion>(Base))
- SymReaper.maybeDead(SymR->getSymbol());
-
- // Mark all non-live symbols that this binding references as dead.
- const ClusterBindings &Cluster = I.getData();
- for (ClusterBindings::iterator CI = Cluster.begin(), CE = Cluster.end();
- CI != CE; ++CI) {
- SVal X = CI.getData();
- SymExpr::symbol_iterator SI = X.symbol_begin(), SE = X.symbol_end();
- for (; SI != SE; ++SI)
- SymReaper.maybeDead(*SI);
- }
+ // Otherwise, remove the dead entry.
+ if (!W.isVisited(Base))
+ B = B.remove(Base);
}
return StoreRef(B.asStore(), *this);
Modified: cfe/trunk/lib/StaticAnalyzer/Core/SymbolManager.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/SymbolManager.cpp?rev=347953&r1=347952&r2=347953&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/SymbolManager.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/SymbolManager.cpp Thu Nov 29 19:27:50 2018
@@ -401,7 +401,6 @@ void SymbolReaper::markDependentsLive(Sy
void SymbolReaper::markLive(SymbolRef sym) {
TheLiving[sym] = NotProcessed;
- TheDead.erase(sym);
markDependentsLive(sym);
}
@@ -426,14 +425,6 @@ void SymbolReaper::markInUse(SymbolRef s
MetadataInUse.insert(sym);
}
-bool SymbolReaper::maybeDead(SymbolRef sym) {
- if (isLive(sym))
- return false;
-
- TheDead.insert(sym);
- return true;
-}
-
bool SymbolReaper::isLiveRegion(const MemRegion *MR) {
if (RegionRoots.count(MR))
return true;
Modified: cfe/trunk/test/Analysis/MisusedMovedObject.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/MisusedMovedObject.cpp?rev=347953&r1=347952&r2=347953&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/MisusedMovedObject.cpp (original)
+++ cfe/trunk/test/Analysis/MisusedMovedObject.cpp Thu Nov 29 19:27:50 2018
@@ -688,3 +688,25 @@ void reportSuperClass() {
c.foo(); // expected-warning {{Method call on a 'moved-from' object 'c'}} expected-note {{Method call on a 'moved-from' object 'c'}}
C c2 = c; // no-warning
}
+
+struct Empty {};
+
+Empty inlinedCall() {
+ // Used to warn because region 'e' failed to be cleaned up because no symbols
+ // have ever died during the analysis and the checkDeadSymbols callback
+ // was skipped entirely.
+ Empty e{};
+ return e; // no-warning
+}
+
+void checkInlinedCallZombies() {
+ while (true)
+ inlinedCall();
+}
+
+void checkLoopZombies() {
+ while (true) {
+ Empty e{};
+ Empty f = std::move(e); // no-warning
+ }
+}
Modified: cfe/trunk/test/Analysis/keychainAPI.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/keychainAPI.m?rev=347953&r1=347952&r2=347953&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/keychainAPI.m (original)
+++ cfe/trunk/test/Analysis/keychainAPI.m Thu Nov 29 19:27:50 2018
@@ -212,7 +212,7 @@ int foo(CFTypeRef keychainOrArray, SecPr
if (st == noErr)
SecKeychainItemFreeContent(ptr, outData[3]);
}
- if (length) { // TODO: We do not report a warning here since the symbol is no longer live, but it's not marked as dead.
+ if (length) { // expected-warning{{Allocated data is not released: missing a call to 'SecKeychainItemFreeContent'}}
length++;
}
return 0;
@@ -450,6 +450,18 @@ int radar_19196494() {
cb.SetContextVal(&login_password);
if (err == noErr) {
SecKeychainItemFreeContent(0, login_password.data);
+ }
+ }
+ return 0;
+}
+int radar_19196494_v2() {
+ @autoreleasepool {
+ AuthorizationValue login_password = {};
+ OSStatus err = SecKeychainFindGenericPassword(0, 0, "", 0, "", (UInt32 *)&login_password.length, (void**)&login_password.data, 0);
+ if (!login_password.data) return 0;
+ cb.SetContextVal(&login_password);
+ if (err == noErr) {
+ SecKeychainItemFreeContent(0, login_password.data);
}
}
return 0;
Added: cfe/trunk/test/Analysis/loop-block-counts.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/loop-block-counts.c?rev=347953&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/loop-block-counts.c (added)
+++ cfe/trunk/test/Analysis/loop-block-counts.c Thu Nov 29 19:27:50 2018
@@ -0,0 +1,26 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify %s
+
+void clang_analyzer_eval(int);
+
+void callee(void **p) {
+ int x;
+ *p = &x;
+}
+
+void loop() {
+ void *arr[2];
+ for (int i = 0; i < 2; ++i)
+ callee(&arr[i]);
+ // FIXME: Should be UNKNOWN.
+ clang_analyzer_eval(arr[0] == arr[1]); // expected-warning{{TRUE}}
+}
+
+void loopWithCall() {
+ void *arr[2];
+ for (int i = 0; i < 2; ++i) {
+ int x;
+ arr[i] = &x;
+ }
+ // FIXME: Should be UNKNOWN.
+ clang_analyzer_eval(arr[0] == arr[1]); // expected-warning{{TRUE}}
+}
Modified: cfe/trunk/test/Analysis/pr22954.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/pr22954.c?rev=347953&r1=347952&r2=347953&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/pr22954.c (original)
+++ cfe/trunk/test/Analysis/pr22954.c Thu Nov 29 19:27:50 2018
@@ -585,7 +585,7 @@ int f28(int i, int j, int k, int l) {
m28[j].s3[k] = 1;
struct ll * l28 = (struct ll*)(&m28[1]);
l28->s1[l] = 2;
- char input[] = {'a', 'b', 'c', 'd'};
+ char input[] = {'a', 'b', 'c', 'd'}; // expected-warning{{Potential leak of memory pointed to by field 's4'}}
memcpy(l28->s1, input, 4);
clang_analyzer_eval(m28[0].s3[0] == 1); // expected-warning{{UNKNOWN}}
clang_analyzer_eval(m28[0].s3[1] == 1); // expected-warning{{UNKNOWN}}
Added: cfe/trunk/test/Analysis/retain-release-cpp-classes.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/retain-release-cpp-classes.cpp?rev=347953&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/retain-release-cpp-classes.cpp (added)
+++ cfe/trunk/test/Analysis/retain-release-cpp-classes.cpp Thu Nov 29 19:27:50 2018
@@ -0,0 +1,33 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,osx -analyzer-output=text -verify %s
+
+// expected-no-diagnostics
+
+typedef void *CFTypeRef;
+typedef struct _CFURLCacheRef *CFURLCacheRef;
+
+CFTypeRef CustomCFRetain(CFTypeRef);
+void invalidate(void *);
+struct S1 {
+ CFTypeRef s;
+ CFTypeRef returnFieldAtPlus0() {
+ return s;
+ }
+};
+struct S2 {
+ S1 *s1;
+};
+void foo(S1 *s1) {
+ invalidate(s1);
+ S2 s2;
+ s2.s1 = s1;
+ CustomCFRetain(s1->returnFieldAtPlus0());
+
+ // Definitely no leak end-of-path note here. The retained pointer
+ // is still accessible through s1 and s2.
+ ((void) 0); // no-warning
+
+ // FIXME: Ideally we need to warn after this invalidate(). The per-function
+ // retain-release contract is violated: the programmer should release
+ // the symbol after it was retained, within the same function.
+ invalidate(&s2);
+}
Modified: cfe/trunk/test/Analysis/self-assign.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/self-assign.cpp?rev=347953&r1=347952&r2=347953&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/self-assign.cpp (original)
+++ cfe/trunk/test/Analysis/self-assign.cpp Thu Nov 29 19:27:50 2018
@@ -32,13 +32,14 @@ StringUsed& StringUsed::operator=(const
clang_analyzer_eval(*this == rhs); // expected-warning{{TRUE}} expected-warning{{UNKNOWN}} expected-note{{TRUE}} expected-note{{UNKNOWN}}
free(str); // expected-note{{Memory is released}}
str = strdup(rhs.str); // expected-warning{{Use of memory after it is freed}} expected-note{{Use of memory after it is freed}}
+// expected-note at -1{{Memory is allocated}}
return *this;
}
StringUsed& StringUsed::operator=(StringUsed &&rhs) { // expected-note{{Assuming rhs == *this}} expected-note{{Assuming rhs != *this}}
clang_analyzer_eval(*this == rhs); // expected-warning{{TRUE}} expected-warning{{UNKNOWN}} expected-note{{TRUE}} expected-note{{UNKNOWN}}
str = rhs.str;
- rhs.str = nullptr; // FIXME: An improved leak checker should warn here
+ rhs.str = nullptr; // expected-warning{{Potential memory leak}} expected-note{{Potential memory leak}}
return *this;
}
@@ -83,7 +84,7 @@ StringUnused::operator const char*() con
int main() {
StringUsed s1 ("test"), s2;
- s2 = s1;
- s2 = std::move(s1);
+ s2 = s1; // expected-note{{Calling copy assignment operator for 'StringUsed'}} // expected-note{{Returned allocated memory}}
+ s2 = std::move(s1); // expected-note{{Calling move assignment operator for 'StringUsed'}}
return 0;
}
Modified: cfe/trunk/test/Analysis/simple-stream-checks.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/simple-stream-checks.c?rev=347953&r1=347952&r2=347953&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/simple-stream-checks.c (original)
+++ cfe/trunk/test/Analysis/simple-stream-checks.c Thu Nov 29 19:27:50 2018
@@ -89,3 +89,8 @@ void testPassToSystemHeaderFunctionIndir
fs.p = fopen("myfile.txt", "w");
fakeSystemHeaderCall(&fs); // invalidates fs, making fs.p unreachable
} // no-warning
+
+void testOverwrite() {
+ FILE *fp = fopen("myfile.txt", "w");
+ fp = 0;
+} // expected-warning {{Opened file is never closed; potential resource leak}}
Modified: cfe/trunk/test/Analysis/unions.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/unions.cpp?rev=347953&r1=347952&r2=347953&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/unions.cpp (original)
+++ cfe/trunk/test/Analysis/unions.cpp Thu Nov 29 19:27:50 2018
@@ -90,9 +90,8 @@ namespace PR17596 {
char str[] = "abc";
vv.s = str;
- // FIXME: This is a leak of uu.s.
uu = vv;
- }
+ } // expected-warning{{leak}}
void testIndirectInvalidation() {
IntOrString uu;
More information about the cfe-commits
mailing list