Index: include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h =================================================================== --- include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h +++ include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h @@ -482,7 +482,6 @@ SymbolMapTy TheLiving; SymbolSetTy MetadataInUse; - SymbolSetTy TheDead; RegionSetTy RegionRoots; @@ -528,20 +527,6 @@ /// symbol marking has occurred, i.e. in the MarkLiveSymbols callback. void markInUse(SymbolRef sym); - /// \brief 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); - - typedef SymbolSetTy::const_iterator dead_iterator; - dead_iterator dead_begin() const { return TheDead.begin(); } - dead_iterator dead_end() const { return TheDead.end(); } - - bool hasDeadSymbols() const { - return !TheDead.empty(); - } - typedef RegionSetTy::const_iterator region_iterator; region_iterator region_begin() const { return RegionRoots.begin(); } region_iterator region_end() const { return RegionRoots.end(); } @@ -550,13 +535,13 @@ /// /// 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); + bool isDead(SymbolRef sym) { + return !isLive(sym); } - + void markLive(const MemRegion *region); void markElementIndicesLive(const MemRegion *region); - + /// \brief Set to the value of the symbolic store after /// StoreManager::removeDeadBindings has been called. void setReapedStore(StoreRef st) { reapedStore = st; } Index: lib/StaticAnalyzer/Checkers/CStringChecker.cpp =================================================================== --- lib/StaticAnalyzer/Checkers/CStringChecker.cpp +++ lib/StaticAnalyzer/Checkers/CStringChecker.cpp @@ -2184,9 +2184,6 @@ void CStringChecker::checkDeadSymbols(SymbolReaper &SR, CheckerContext &C) const { - if (!SR.hasDeadSymbols()) - return; - ProgramStateRef state = C.getState(); CStringLengthTy Entries = state->get(); if (Entries.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(); for (MostSpecializedTypeArgsMapTy::iterator I = TyArgMap.begin(), Index: lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp =================================================================== --- lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp +++ lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp @@ -32,7 +32,7 @@ typedef void (ExprInspectionChecker::*FnCheck)(const CallExpr *, CheckerContext &C) const; - void reportBug(llvm::StringRef Msg, CheckerContext &C) const; + ExplodedNode *reportBug(llvm::StringRef Msg, CheckerContext &C) const; public: bool evalCall(const CallExpr *CE, CheckerContext &C) const; @@ -98,16 +98,17 @@ } } -void ExprInspectionChecker::reportBug(llvm::StringRef Msg, - CheckerContext &C) const { +ExplodedNode *ExprInspectionChecker::reportBug(llvm::StringRef Msg, + CheckerContext &C) const { if (!BT) BT.reset(new BugType(this, "Checking analyzer assumptions", "debug")); ExplodedNode *N = C.generateNonFatalErrorNode(); if (!N) - return; + return NULL; C.emitReport(llvm::make_unique(*BT, Msg, N)); + return N; } void ExprInspectionChecker::analyzerEval(const CallExpr *CE, @@ -144,8 +145,10 @@ void ExprInspectionChecker::analyzerExplain(const CallExpr *CE, CheckerContext &C) const { - if (CE->getNumArgs() == 0) + if (CE->getNumArgs() == 0) { reportBug("Missing argument for explaining", C); + return; + } SVal V = C.getSVal(CE->getArg(0)); SValExplainer Ex(C.getASTContext()); @@ -154,12 +157,16 @@ void ExprInspectionChecker::analyzerGetExtent(const CallExpr *CE, CheckerContext &C) const { - if (CE->getNumArgs() == 0) + if (CE->getNumArgs() == 0) { reportBug("Missing region for obtaining extent", C); + return; + } auto MR = dyn_cast_or_null(C.getSVal(CE->getArg(0)).getAsRegion()); - if (!MR) + if (!MR) { reportBug("Obtaining extent of a non-region", C); + return; + } ProgramStateRef State = C.getState(); State = State->BindExpr(CE, C.getLocationContext(), @@ -185,15 +192,18 @@ CheckerContext &C) const { ProgramStateRef State = C.getState(); const MarkedSymbolsTy &Syms = State->get(); + ExplodedNode *N = C.getPredecessor(); for (auto I = Syms.begin(), E = Syms.end(); I != E; ++I) { SymbolRef Sym = *I; if (!SymReaper.isDead(Sym)) continue; - reportBug("SYMBOL DEAD", C); + // The non-fatal error node should be the same for all reports. + if (ExplodedNode *BugNode = reportBug("SYMBOL DEAD", C)) + N = BugNode; State = State->remove(Sym); } - C.addTransition(State); + C.addTransition(State, N); } void ExprInspectionChecker::analyzerCrash(const CallExpr *CE, Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp =================================================================== --- lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -2125,9 +2125,6 @@ void MallocChecker::checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const { - if (!SymReaper.hasDeadSymbols()) - return; - ProgramStateRef state = C.getState(); RegionStateTy RS = state->get(); RegionStateTy::Factory &F = state->get_context(); Index: lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp =================================================================== --- lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp +++ lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp @@ -431,9 +431,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(); for (NullabilityMapTy::iterator I = Nullabilities.begin(), Index: lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp =================================================================== --- lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp +++ lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp @@ -3922,20 +3922,20 @@ SmallVector 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)){ + for (auto I = B.begin(), E = B.end(); I != E; ++I) { + SymbolRef Sym = I->first; + const RefVal &T = I->second; + if (SymReaper.isDead(Sym)){ // Use the symbol as the tag. // FIXME: This might not be as unique as we would like. const ProgramPointTag *Tag = getDeadSymbolTag(Sym); - state = handleAutoreleaseCounts(state, Pred, Tag, C, Sym, *T); + state = handleAutoreleaseCounts(state, Pred, Tag, C, Sym, T); 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/StreamChecker.cpp =================================================================== --- lib/StaticAnalyzer/Checkers/StreamChecker.cpp +++ lib/StaticAnalyzer/Checkers/StreamChecker.cpp @@ -397,15 +397,12 @@ void StreamChecker::checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const { // 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(Sym); - if (!SS) - continue; - - if (SS->isOpened()) { + ProgramStateRef state = C.getState(); + const StreamMapTy &Map = state->get(); + for (auto I = Map.begin(), E = Map.end(); I != E; ++I) { + SymbolRef Sym = I->first; + const StreamState &SS = I->second; + if (SymReaper.isDead(Sym) && SS.isOpened()) { ExplodedNode *N = C.generateErrorNode(); if (N) { if (!BT_ResourceLeak) Index: lib/StaticAnalyzer/Core/Environment.cpp =================================================================== --- lib/StaticAnalyzer/Core/Environment.cpp +++ lib/StaticAnalyzer/Core/Environment.cpp @@ -174,10 +174,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/Core/ExprEngine.cpp =================================================================== --- lib/StaticAnalyzer/Core/ExprEngine.cpp +++ lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -380,45 +380,36 @@ // 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 (ExplodedNodeSet::const_iterator - I = CheckedSet.begin(), E = CheckedSet.end(); I != E; ++I) { - 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 (ExplodedNodeSet::const_iterator I = CheckedSet.begin(), + E = CheckedSet.end(); I != E; ++I) { + 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/RangeConstraintManager.cpp =================================================================== --- lib/StaticAnalyzer/Core/RangeConstraintManager.cpp +++ lib/StaticAnalyzer/Core/RangeConstraintManager.cpp @@ -405,7 +405,7 @@ for (ConstraintRangeTy::iterator I = CR.begin(), E = CR.end(); I != E; ++I) { SymbolRef sym = I.getKey(); - if (SymReaper.maybeDead(sym)) + if (!SymReaper.isLive(sym)) CR = CRFactory.remove(CR, sym); } Index: lib/StaticAnalyzer/Core/RegionStore.cpp =================================================================== --- lib/StaticAnalyzer/Core/RegionStore.cpp +++ lib/StaticAnalyzer/Core/RegionStore.cpp @@ -2437,19 +2437,6 @@ // Remove the dead entry. B = B.remove(Base); - - if (const SymbolicRegion *SymR = dyn_cast(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); - } } return StoreRef(B.asStore(), *this); Index: lib/StaticAnalyzer/Core/SymbolManager.cpp =================================================================== --- lib/StaticAnalyzer/Core/SymbolManager.cpp +++ lib/StaticAnalyzer/Core/SymbolManager.cpp @@ -385,7 +385,6 @@ void SymbolReaper::markLive(SymbolRef sym) { TheLiving[sym] = NotProcessed; - TheDead.erase(sym); markDependentsLive(sym); } @@ -410,14 +409,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: 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}} @@ -913,4 +913,3 @@ return 0; } - 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; // overwrittes the symbol with another value +} // expected-warning {{Opened file is never closed; potential resource leak}} Index: test/Analysis/unions.cpp =================================================================== --- test/Analysis/unions.cpp +++ test/Analysis/unions.cpp @@ -91,9 +91,8 @@ char str[] = "abc"; vv.s = str; - // FIXME: This is a leak of uu.s. uu = vv; - } + } // expected-warning{{leak}} void testIndirectInvalidation() { IntOrString uu;