NoQ updated this revision to Diff 173487.
NoQ added a comment.
Add an interesting test for the `MisusedMovedObject` checker that demonstrates
one more potential source of false positives caused by the zombie symbol
problem. In this test there are, well, //no symbols//. Therefore, there are no
dead symbols or zombie symbols. Therefore `SymReaper.hasDeadSymbols()` is
always `false`. Therefore `checkDeadSymbols()` is never called at all. However,
`MisusedMovedObject` checker is not interested in symbols; it is only
interested in regions, including concrete regions that aren't based on symbols.
So it was missing the `checkDeadSymbols()` callback that would have unmarked
the region for variable `e` (in inlined function or not in inlined function -
doesn't matter). And next time it sees variable `e` in that function within the
same stack frame, it thinks it's the same variable that has just been moved.
This problem was already discussed in D24246?id=82469#inline-249803
<https://reviews.llvm.org/D24246?id=82469#inline-249803>.
Add tests in `loop-block-counts.c` that demonstrate the other source of the
problem in `MisusedMovedObject`: in fact, variable `e` should not be the same
variable on different iterations of the loop. In case of the inlined function,
the problem is caused by how our `StackFrameContext` doesn't contain "block
count" for the entrance - which is a hack to discriminate between different
iterations of the loop that is used for, eg., conjured symbols, but,
unfortunately, not for addresses of variables / temporaries. In case of
non-inlined functions, the problem is deeper: we simply don't have a
`LocationContext` for a single loop iteration, so there's no way we can
discriminate between loop locals on different loop iterations by their memory
spaces.
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/MisusedMovedObject.cpp
test/Analysis/keychainAPI.m
test/Analysis/loop-block-counts.c
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/loop-block-counts.c
===================================================================
--- /dev/null
+++ test/Analysis/loop-block-counts.c
@@ -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}}
+}
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: test/Analysis/MisusedMovedObject.cpp
===================================================================
--- test/Analysis/MisusedMovedObject.cpp
+++ test/Analysis/MisusedMovedObject.cpp
@@ -688,3 +688,25 @@
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
+ }
+}
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
