NoQ updated this revision to Diff 172263.
NoQ added a comment.
Ha! Apart from Zombie symbols that are neither dead nor alive and therefore not
buried properly, there are also Schrödinger symbols that are dead and alive at
the same time. Moreover, asking whether a Schrödinger symbol is `isLive()` or
`maybeDead()` would immediately collapse it into an alive-only state. Moreover,
if a checker iterates through the dead set and asks whether a Schrödinger
symbol is alive, undefined behavior occurs. Ah, thin ice.
As far as i understand, this only happens to derived symbols under fairly
specific circumstances. Namely, if the derived symbol is marked as dead and
then its parent symbol is marked as live, the derived symbol is not
automatically removed from the "dead set". It is impossible to automatically
add derived symbols to the dead set because there may be infinitely many
symbols derived from the same parent symbol. Therefore there exists moment of
time when the derived symbol is still in the dead set, but asking whether it is
`isLive()` would yield true (because it'll check whether the parent symbol is
alive). Additionally, when `isLive()` notices that it's alive, it automatically
removes the symbol from the dead set in order to "memoize" the answer, so it
becomes purely alive. In particular, it invalidates dead set iterators, which
results in undefined behavior (which in practice boils down to past-end
iterator access, ultimately crashing Clang).
Schrödinger symbols cause false positive leaks, unlike zombies who only cause
false negative leaks. But those false positives are relatively rare because
most checkers don't track derived symbols; eg., MallocChecker only tracks pure
conjured symbols. RetainCountChecker is affected, as demonstrated by the newly
added test. Not sure if it's possible to make CStringChecker track a derived
symbol and forget a string length due to that effect. MacOSKeychainAPI checker
should also be affected, i think, but i didn't try. NullabilityChecker is
probably unaffected; its behavior on dead symbols is pretty weird, but it seems
safe; it also has a separate bug that was exposed but i'll address in a
separate patch.
https://reviews.llvm.org/D18860
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
lib/StaticAnalyzer/Checkers/CStringChecker.cpp
lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp
lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp
lib/StaticAnalyzer/Checkers/MallocChecker.cpp
lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
lib/StaticAnalyzer/Checkers/StreamChecker.cpp
lib/StaticAnalyzer/Core/Environment.cpp
lib/StaticAnalyzer/Core/ExprEngine.cpp
lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
lib/StaticAnalyzer/Core/RegionStore.cpp
lib/StaticAnalyzer/Core/SymbolManager.cpp
test/Analysis/keychainAPI.m
test/Analysis/pr22954.c
test/Analysis/retain-release-cpp-classes.cpp
test/Analysis/self-assign.cpp
test/Analysis/simple-stream-checks.c
test/Analysis/unions.cpp
Index: test/Analysis/unions.cpp
===================================================================
--- test/Analysis/unions.cpp
+++ test/Analysis/unions.cpp
@@ -90,9 +90,8 @@
char str[] = "abc";
vv.s = str;
- // FIXME: This is a leak of uu.s.
uu = vv;
- }
+ } // expected-warning{{leak}}
void testIndirectInvalidation() {
IntOrString uu;
Index: test/Analysis/simple-stream-checks.c
===================================================================
--- test/Analysis/simple-stream-checks.c
+++ test/Analysis/simple-stream-checks.c
@@ -89,3 +89,8 @@
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}}
Index: test/Analysis/self-assign.cpp
===================================================================
--- test/Analysis/self-assign.cpp
+++ test/Analysis/self-assign.cpp
@@ -32,13 +32,14 @@
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@-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 @@
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;
}
Index: test/Analysis/retain-release-cpp-classes.cpp
===================================================================
--- /dev/null
+++ test/Analysis/retain-release-cpp-classes.cpp
@@ -0,0 +1,35 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,osx -analyzer-output=text -verify %s
+
+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());
+ // expected-note@-1{{function call returns a Core Foundation object of type CFTypeRef with a +0 retain count}}
+ // expected-note@-2{{Reference count incremented. The object now has a +1 retain count}}
+
+ // Definitely no leak end-of-path note here. The retained pointer
+ // is still accessible through s1 and s2.
+ ((void) 0); // no-warning
+
+ invalidate(&s2);
+ // The per-function retain-release contract is violated: the programmer
+ // should release the symbol after it was retained, within the same function.
+ // We might be warning here accidentally, but we should ideally warn here.
+
+} // expected-warning{{Potential leak of an object}}
+ // expected-note@-1{{Object leaked: allocated object is not referenced later in this execution path and has a retain count of +1}}
Index: test/Analysis/pr22954.c
===================================================================
--- test/Analysis/pr22954.c
+++ test/Analysis/pr22954.c
@@ -585,7 +585,7 @@
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}}
Index: test/Analysis/keychainAPI.m
===================================================================
--- test/Analysis/keychainAPI.m
+++ test/Analysis/keychainAPI.m
@@ -212,7 +212,7 @@
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;
@@ -454,3 +454,15 @@
}
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;
+}
Index: lib/StaticAnalyzer/Core/SymbolManager.cpp
===================================================================
--- lib/StaticAnalyzer/Core/SymbolManager.cpp
+++ lib/StaticAnalyzer/Core/SymbolManager.cpp
@@ -401,7 +401,6 @@
void SymbolReaper::markLive(SymbolRef sym) {
TheLiving[sym] = NotProcessed;
- TheDead.erase(sym);
markDependentsLive(sym);
}
@@ -426,14 +425,6 @@
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;
Index: lib/StaticAnalyzer/Core/RegionStore.cpp
===================================================================
--- lib/StaticAnalyzer/Core/RegionStore.cpp
+++ lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -2571,24 +2571,9 @@
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);
Index: lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===================================================================
--- lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -399,7 +399,7 @@
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);
}
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -675,44 +675,35 @@
// 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);
}
}
Index: lib/StaticAnalyzer/Core/Environment.cpp
===================================================================
--- lib/StaticAnalyzer/Core/Environment.cpp
+++ lib/StaticAnalyzer/Core/Environment.cpp
@@ -189,11 +189,6 @@
// 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);
}
}
Index: lib/StaticAnalyzer/Checkers/StreamChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -383,26 +383,26 @@
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));
}
}
Index: lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
+++ lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
@@ -1337,20 +1337,18 @@
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);
}
}
Index: lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
+++ lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
@@ -446,9 +446,6 @@
/// 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(),
Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -2365,9 +2365,6 @@
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>();
Index: lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp
@@ -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 @@
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 @@
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 @@
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) {
Index: lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp
@@ -100,9 +100,6 @@
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())
Index: lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
+++ lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
@@ -123,11 +123,6 @@
}
}
- if (!SR.hasDeadSymbols()) {
- C.addTransition(State);
- return;
- }
-
MostSpecializedTypeArgsMapTy TyArgMap =
State->get<MostSpecializedTypeArgsMap>();
for (MostSpecializedTypeArgsMapTy::iterator I = TyArgMap.begin(),
Index: lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -2385,9 +2385,6 @@
void CStringChecker::checkDeadSymbols(SymbolReaper &SR,
CheckerContext &C) const {
- if (!SR.hasDeadSymbols())
- return;
-
ProgramStateRef state = C.getState();
CStringLengthTy Entries = state->get<CStringLength>();
if (Entries.isEmpty())
Index: include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
===================================================================
--- include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
@@ -558,7 +558,6 @@
SymbolMapTy TheLiving;
SymbolSetTy MetadataInUse;
- SymbolSetTy TheDead;
RegionSetTy RegionRoots;
@@ -603,32 +602,17 @@
/// 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(); }
region_iterator region_end() const { return RegionRoots.end(); }
/// 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);
Index: include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
===================================================================
--- include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
@@ -198,7 +198,7 @@
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);
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
