From: Jordan Rose Date: Thu, 6 Dec 2012 18:58:18 +0000 (+0000) Subject: [analyzer] Simplify RetainCountChecker's handling of dead symbols. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=4ee1c557c3ebddb8a9be8f6fb66605b971793820;p=clang [analyzer] Simplify RetainCountChecker's handling of dead symbols. Previously we made three passes over the set of dead symbols, and removed them from the state /twice/. Now we combine the autorelease pass and the symbol death pass, and only have to remove the bindings for the symbols that leaked. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@169527 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp b/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp index 4cf47999c2..5ab9499a61 100644 --- a/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp @@ -2545,7 +2545,7 @@ public: SymbolRef sid, RefVal V, SmallVectorImpl &Leaked) const; - std::pair + ProgramStateRef handleAutoreleaseCounts(ProgramStateRef state, ExplodedNode *Pred, const ProgramPointTag *Tag, CheckerContext &Ctx, SymbolRef Sym, RefVal V) const; @@ -3260,11 +3260,10 @@ void RetainCountChecker::checkPreStmt(const ReturnStmt *S, // Update the autorelease counts. static SimpleProgramPointTag AutoreleaseTag("RetainCountChecker : Autorelease"); - llvm::tie(Pred, state) = handleAutoreleaseCounts(state, Pred, &AutoreleaseTag, - C, Sym, X); + state = handleAutoreleaseCounts(state, Pred, &AutoreleaseTag, C, Sym, X); // Did we cache out? - if (!Pred) + if (!state) return; // Get the updated binding. @@ -3473,8 +3472,8 @@ RetainCountChecker::checkRegionChanges(ProgramStateRef state, // Handle dead symbols and end-of-path. //===----------------------------------------------------------------------===// -std::pair -RetainCountChecker::handleAutoreleaseCounts(ProgramStateRef state, +ProgramStateRef +RetainCountChecker::handleAutoreleaseCounts(ProgramStateRef state, ExplodedNode *Pred, const ProgramPointTag *Tag, CheckerContext &Ctx, @@ -3483,7 +3482,7 @@ RetainCountChecker::handleAutoreleaseCounts(ProgramStateRef state, // No autorelease counts? Nothing to be done. if (!ACnt) - return std::make_pair(Pred, state); + return state; assert(!Ctx.isObjCGCEnabled() && "Autorelease counts in GC mode?"); unsigned Cnt = V.getCount(); @@ -3504,11 +3503,7 @@ RetainCountChecker::handleAutoreleaseCounts(ProgramStateRef state, V.setCount(Cnt - ACnt); V.setAutoreleaseCount(0); } - state = setRefBinding(state, Sym, V); - ExplodedNode *N = Ctx.addTransition(state, Pred, Tag); - if (N == 0) - state = 0; - return std::make_pair(N, state); + return setRefBinding(state, Sym, V); } // Woah! More autorelease counts then retain counts left. @@ -3535,7 +3530,7 @@ RetainCountChecker::handleAutoreleaseCounts(ProgramStateRef state, Ctx.emitReport(report); } - return std::make_pair((ExplodedNode *)0, (ProgramStateRef )0); + return 0; } ProgramStateRef @@ -3560,9 +3555,6 @@ RetainCountChecker::processLeaks(ProgramStateRef state, SmallVectorImpl &Leaked, CheckerContext &Ctx, ExplodedNode *Pred) const { - if (Leaked.empty()) - return Pred; - // Generate an intermediate node representing the leak point. ExplodedNode *N = Ctx.addTransition(state, Pred); @@ -3591,8 +3583,8 @@ void RetainCountChecker::checkEndPath(CheckerContext &Ctx) const { ExplodedNode *Pred = Ctx.getPredecessor(); for (RefBindingsTy::iterator I = B.begin(), E = B.end(); I != E; ++I) { - llvm::tie(Pred, state) = handleAutoreleaseCounts(state, Pred, /*Tag=*/0, - Ctx, I->first, I->second); + state = handleAutoreleaseCounts(state, Pred, /*Tag=*/0, Ctx, + I->first, I->second); if (!state) return; } @@ -3632,6 +3624,7 @@ void RetainCountChecker::checkDeadSymbols(SymbolReaper &SymReaper, ProgramStateRef state = C.getState(); RefBindingsTy B = state->get(); + SmallVector Leaked; // Update counts from autorelease pools for (SymbolReaper::dead_iterator I = SymReaper.dead_begin(), @@ -3641,20 +3634,19 @@ void RetainCountChecker::checkDeadSymbols(SymbolReaper &SymReaper, // Use the symbol as the tag. // FIXME: This might not be as unique as we would like. const ProgramPointTag *Tag = getDeadSymbolTag(Sym); - llvm::tie(Pred, 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); } } - B = state->get(); - SmallVector Leaked; - - for (SymbolReaper::dead_iterator I = SymReaper.dead_begin(), - E = SymReaper.dead_end(); I != E; ++I) { - if (const RefVal *T = B.lookup(*I)) - state = handleSymbolDeath(state, *I, *T, Leaked); + if (Leaked.empty()) { + C.addTransition(state); + return; } Pred = processLeaks(state, Leaked, C, Pred); @@ -3664,10 +3656,13 @@ void RetainCountChecker::checkDeadSymbols(SymbolReaper &SymReaper, return; // Now generate a new node that nukes the old bindings. + // The only bindings left at this point are the leaked symbols. RefBindingsTy::Factory &F = state->get_context(); + B = state->get(); - for (SymbolReaper::dead_iterator I = SymReaper.dead_begin(), - E = SymReaper.dead_end(); I != E; ++I) + for (SmallVectorImpl::iterator I = Leaked.begin(), + E = Leaked.end(); + I != E; ++I) B = F.remove(B, *I); state = state->set(B); diff --git a/test/Analysis/retain-release-path-notes.m b/test/Analysis/retain-release-path-notes.m index 0daeecb39c..1d70a4a0db 100644 --- a/test/Analysis/retain-release-path-notes.m +++ b/test/Analysis/retain-release-path-notes.m @@ -138,7 +138,7 @@ CFTypeRef CFGetRuleViolation () { - (id)copyAutorelease { id result = [[Foo alloc] init]; // expected-note{{Method returns an Objective-C object with a +1 retain count}} [result autorelease]; // expected-note{{Object sent -autorelease message}} - return result; // expected-warning{{Object with a +0 retain count returned to caller where a +1 (owning) retain count is expected}} expected-note{{Object returned to caller with a +0 retain count}} expected-note{{Object with a +0 retain count returned to caller where a +1 (owning) retain count is expected}} + return result; // expected-warning{{Object with a +0 retain count returned to caller where a +1 (owning) retain count is expected}} expected-note{{Object with a +0 retain count returned to caller where a +1 (owning) retain count is expected}} } @end @@ -3824,47 +3824,6 @@ void testDictionary(id key, id value) { // CHECK-NEXT: file0 // CHECK-NEXT: // CHECK-NEXT: -// CHECK-NEXT: -// CHECK-NEXT: -// CHECK-NEXT: line141 -// CHECK-NEXT: col10 -// CHECK-NEXT: file0 -// CHECK-NEXT: -// CHECK-NEXT: -// CHECK-NEXT: line141 -// CHECK-NEXT: col15 -// CHECK-NEXT: file0 -// CHECK-NEXT: -// CHECK-NEXT: -// CHECK-NEXT: -// CHECK-NEXT: depth0 -// CHECK-NEXT: extended_message -// CHECK-NEXT: Object returned to caller with a +0 retain count -// CHECK-NEXT: message -// CHECK-NEXT: Object returned to caller with a +0 retain count -// CHECK-NEXT: -// CHECK-NEXT: -// CHECK-NEXT: kindevent -// CHECK-NEXT: location -// CHECK-NEXT: -// CHECK-NEXT: line141 -// CHECK-NEXT: col3 -// CHECK-NEXT: file0 -// CHECK-NEXT: -// CHECK-NEXT: ranges -// CHECK-NEXT: -// CHECK-NEXT: -// CHECK-NEXT: -// CHECK-NEXT: line141 -// CHECK-NEXT: col3 -// CHECK-NEXT: file0 -// CHECK-NEXT: -// CHECK-NEXT: -// CHECK-NEXT: line141 -// CHECK-NEXT: col15 -// CHECK-NEXT: file0 -// CHECK-NEXT: -// CHECK-NEXT: // CHECK-NEXT: // CHECK-NEXT: depth0 // CHECK-NEXT: extended_message