Index: lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h =================================================================== --- lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h +++ lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h @@ -252,7 +252,6 @@ check::PostStmt, check::PostStmt, check::PostCall, - check::PreStmt, check::RegionChanges, eval::Assume, eval::Call > { @@ -388,8 +387,7 @@ const LocationContext* LCtx, const CallEvent *Call) const; - void checkPreStmt(const ReturnStmt *S, CheckerContext &C) const; - void checkReturnWithRetEffect(const ReturnStmt *S, CheckerContext &C, + ExplodedNode* checkReturnWithRetEffect(const ReturnStmt *S, CheckerContext &C, ExplodedNode *Pred, RetEffect RE, RefVal X, SymbolRef Sym, ProgramStateRef state) const; @@ -416,12 +414,20 @@ ProgramStateRef handleAutoreleaseCounts(ProgramStateRef state, ExplodedNode *Pred, const ProgramPointTag *Tag, CheckerContext &Ctx, - SymbolRef Sym, RefVal V) const; + SymbolRef Sym, + RefVal V, + const ReturnStmt *S=nullptr) const; ExplodedNode *processLeaks(ProgramStateRef state, SmallVectorImpl &Leaked, CheckerContext &Ctx, ExplodedNode *Pred = nullptr) const; + +private: + /// Perform the necessary checks and state adjustments at the end of the + /// function. + /// \p S Return statement, may be null. + ExplodedNode * processReturn(const ReturnStmt *S, CheckerContext &C) const; }; //===----------------------------------------------------------------------===// Index: lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp =================================================================== --- lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp +++ lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp @@ -814,12 +814,9 @@ return true; } -//===----------------------------------------------------------------------===// -// Handle return statements. -//===----------------------------------------------------------------------===// - -void RetainCountChecker::checkPreStmt(const ReturnStmt *S, - CheckerContext &C) const { +ExplodedNode * RetainCountChecker::processReturn(const ReturnStmt *S, + CheckerContext &C) const { + ExplodedNode *Pred = C.getPredecessor(); // Only adjust the reference count if this is the top-level call frame, // and not the result of inlining. In the future, we should do @@ -827,22 +824,25 @@ // with their expected semantics (e.g., the method should return a retained // object, etc.). if (!C.inTopFrame()) - return; + return Pred; + + if (!S) + return Pred; const Expr *RetE = S->getRetValue(); if (!RetE) - return; + return Pred; ProgramStateRef state = C.getState(); SymbolRef Sym = state->getSValAsScalarOrLoc(RetE, C.getLocationContext()).getAsLocSymbol(); if (!Sym) - return; + return Pred; // Get the reference count binding (if any). const RefVal *T = getRefBinding(state, Sym); if (!T) - return; + return Pred; // Change the reference count. RefVal X = *T; @@ -861,20 +861,19 @@ if (cnt) { X.setCount(cnt - 1); X = X ^ RefVal::ReturnedOwned; - } - else { + } else { X = X ^ RefVal::ReturnedNotOwned; } break; } default: - return; + return Pred; } // Update the binding. state = setRefBinding(state, Sym, X); - ExplodedNode *Pred = C.addTransition(state); + Pred = C.addTransition(state); // At this point we have updated the state properly. // Everything after this is merely checking to see if the return value has @@ -882,15 +881,15 @@ // Did we cache out? if (!Pred) - return; + return nullptr; // Update the autorelease counts. static CheckerProgramPointTag AutoreleaseTag(this, "Autorelease"); - state = handleAutoreleaseCounts(state, Pred, &AutoreleaseTag, C, Sym, X); + state = handleAutoreleaseCounts(state, Pred, &AutoreleaseTag, C, Sym, X, S); - // Did we cache out? + // Have we generated a sink node? if (!state) - return; + return nullptr; // Get the updated binding. T = getRefBinding(state, Sym); @@ -913,10 +912,10 @@ } } - checkReturnWithRetEffect(S, C, Pred, RE, X, Sym, state); + return checkReturnWithRetEffect(S, C, Pred, RE, X, Sym, state); } -void RetainCountChecker::checkReturnWithRetEffect(const ReturnStmt *S, +ExplodedNode * RetainCountChecker::checkReturnWithRetEffect(const ReturnStmt *S, CheckerContext &C, ExplodedNode *Pred, RetEffect RE, RefVal X, @@ -929,20 +928,17 @@ // [self addSubview:_contentView]; // invalidates 'self' // [_contentView release]; if (X.getIvarAccessHistory() != RefVal::IvarAccessHistory::None) - return; + return Pred; // Any leaks or other errors? if (X.isReturnedOwned() && X.getCount() == 0) { if (RE.getKind() != RetEffect::NoRet) { - bool hasError = false; if (!RE.isOwned()) { + // The returning type is a CF, we expect the enclosing method should // return ownership. - hasError = true; X = X ^ RefVal::ErrorLeakReturned; - } - if (hasError) { // Generate an error node. state = setRefBinding(state, Sym, X); @@ -950,10 +946,12 @@ ExplodedNode *N = C.addTransition(state, Pred, &ReturnOwnLeakTag); if (N) { const LangOptions &LOpts = C.getASTContext().getLangOpts(); - C.emitReport(llvm::make_unique( + auto R = llvm::make_unique( *getLeakAtReturnBug(LOpts), LOpts, SummaryLog, N, Sym, C, - IncludeAllocationLine)); + IncludeAllocationLine); + C.emitReport(std::move(R)); } + return N; } } } else if (X.isReturnedNotOwned()) { @@ -977,13 +975,16 @@ if (!returnNotOwnedForOwned) returnNotOwnedForOwned.reset(new ReturnedNotOwnedForOwned(this)); - C.emitReport(llvm::make_unique( + auto R = llvm::make_unique( *returnNotOwnedForOwned, C.getASTContext().getLangOpts(), - SummaryLog, N, Sym)); + SummaryLog, N, Sym); + C.emitReport(std::move(R)); } + return N; } } } + return Pred; } //===----------------------------------------------------------------------===// @@ -1107,16 +1108,14 @@ return state; } -//===----------------------------------------------------------------------===// -// Handle dead symbols and end-of-path. -//===----------------------------------------------------------------------===// - ProgramStateRef RetainCountChecker::handleAutoreleaseCounts(ProgramStateRef state, ExplodedNode *Pred, const ProgramPointTag *Tag, CheckerContext &Ctx, - SymbolRef Sym, RefVal V) const { + SymbolRef Sym, + RefVal V, + const ReturnStmt *S) const { unsigned ACnt = V.getAutoreleaseCount(); // No autorelease counts? Nothing to be done. @@ -1141,10 +1140,11 @@ if (ACnt <= Cnt) { if (ACnt == Cnt) { V.clearCounts(); - if (V.getKind() == RefVal::ReturnedOwned) + if (V.getKind() == RefVal::ReturnedOwned) { V = V ^ RefVal::ReturnedNotOwned; - else + } else { V = V ^ RefVal::NotOwned; + } } else { V.setCount(V.getCount() - ACnt); V.setAutoreleaseCount(0); @@ -1181,8 +1181,9 @@ overAutorelease.reset(new OverAutorelease(this)); const LangOptions &LOpts = Ctx.getASTContext().getLangOpts(); - Ctx.emitReport(llvm::make_unique( - *overAutorelease, LOpts, SummaryLog, N, Sym, os.str())); + auto R = llvm::make_unique(*overAutorelease, LOpts, SummaryLog, + N, Sym, os.str()); + Ctx.emitReport(std::move(R)); } return nullptr; @@ -1281,9 +1282,15 @@ void RetainCountChecker::checkEndFunction(const ReturnStmt *RS, CheckerContext &Ctx) const { - ProgramStateRef state = Ctx.getState(); + ExplodedNode *Pred = processReturn(RS, Ctx); + + // Created state cached out. + if (!Pred) { + return; + } + + ProgramStateRef state = Pred->getState(); RefBindingsTy B = state->get(); - ExplodedNode *Pred = Ctx.getPredecessor(); // Don't process anything within synthesized bodies. const LocationContext *LCtx = Pred->getLocationContext(); Index: test/Analysis/Inputs/expected-plists/retain-release-path-notes.m.plist =================================================================== --- test/Analysis/Inputs/expected-plists/retain-release-path-notes.m.plist +++ test/Analysis/Inputs/expected-plists/retain-release-path-notes.m.plist @@ -2066,47 +2066,6 @@ file0 - - - line110 - col10 - file0 - - - line110 - col15 - file0 - - - - depth0 - extended_message - Object returned to caller with a +0 retain count - message - Object returned to caller with a +0 retain count - - - kindevent - location - - line110 - col3 - file0 - - ranges - - - - line110 - col3 - file0 - - - line110 - col15 - file0 - - depth0 extended_message @@ -2228,47 +2187,6 @@ file0 - - - line115 - col10 - file0 - - - line115 - col15 - file0 - - - - depth0 - extended_message - Object returned to caller as an owning reference (single retain count transferred to caller) - message - Object returned to caller as an owning reference (single retain count transferred to caller) - - - kindevent - location - - line115 - col3 - file0 - - ranges - - - - line115 - col3 - file0 - - - line115 - col15 - file0 - - depth0 extended_message @@ -2390,47 +2308,6 @@ file0 - - - line121 - col10 - file0 - - - line121 - col15 - file0 - - - - depth0 - extended_message - Object returned to caller with a +0 retain count - message - Object returned to caller with a +0 retain count - - - kindevent - location - - line121 - col3 - file0 - - ranges - - - - line121 - col3 - file0 - - - line121 - col15 - file0 - - depth0 extended_message @@ -2552,47 +2429,6 @@ file0 - - - line126 - col10 - file0 - - - line126 - col15 - file0 - - - - depth0 - extended_message - Object returned to caller with a +0 retain count - message - Object returned to caller with a +0 retain count - - - kindevent - location - - line126 - col3 - file0 - - ranges - - - - line126 - col3 - file0 - - - line126 - col15 - file0 - - depth0 extended_message @@ -2714,47 +2550,6 @@ file0 - - - line131 - col10 - file0 - - - line131 - col15 - file0 - - - - depth0 - extended_message - Object returned to caller with a +0 retain count - message - Object returned to caller with a +0 retain count - - - kindevent - location - - line131 - col3 - file0 - - ranges - - - - line131 - col3 - file0 - - - line131 - col15 - file0 - - depth0 extended_message @@ -2876,47 +2671,6 @@ file0 - - - line136 - col10 - file0 - - - line136 - col15 - file0 - - - - depth0 - extended_message - Object returned to caller as an owning reference (single retain count transferred to caller) - message - Object returned to caller as an owning reference (single retain count transferred to caller) - - - kindevent - location - - line136 - col3 - file0 - - ranges - - - - line136 - col3 - file0 - - - line136 - col15 - file0 - - depth0 extended_message @@ -5262,7 +5016,7 @@ files - /clang/test/Analysis/retain-release-path-notes.m + /test/Analysis/retain-release-path-notes.m Index: test/Analysis/retain-release-arc.m =================================================================== --- test/Analysis/retain-release-arc.m +++ test/Analysis/retain-release-arc.m @@ -95,8 +95,7 @@ return (__bridge NSDictionary *)testDict; #if HAS_ARC // expected-warning@-2 {{Potential leak of an object stored into 'testDict'}} - // expected-note@-3 {{Object returned to caller as an owning reference (single retain count transferred to caller)}} - // expected-note@-4 {{Object leaked: object allocated and stored into 'testDict' is returned from a method managed by Automatic Reference Counting}} + // expected-note@-3 {{Object leaked: object allocated and stored into 'testDict' is returned from a method managed by Automatic Reference Counting}} #endif } Index: test/Analysis/retain-release-path-notes.m =================================================================== --- test/Analysis/retain-release-path-notes.m +++ test/Analysis/retain-release-path-notes.m @@ -107,33 +107,33 @@ CFTypeRef CFCopyRuleViolation () { CFTypeRef object = CFGetSomething(); // expected-note{{Call to function 'CFGetSomething' returns a Core Foundation object of type CFTypeRef with a +0 retain count}} - return object; // 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 object; // 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}} } CFTypeRef CFGetRuleViolation () { CFTypeRef object = CFCreateSomething(); // expected-note{{Call to function 'CFCreateSomething' returns a Core Foundation object of type CFTypeRef with a +1 retain count}} - return object; // expected-warning{{leak}} expected-note{{Object returned to caller as an owning reference (single retain count transferred to caller)}} expected-note{{Object leaked: object allocated and stored into 'object' is returned from a function whose name ('CFGetRuleViolation') does not contain 'Copy' or 'Create'. This violates the naming convention rules given in the Memory Management Guide for Core Foundation}} + return object; // expected-warning{{leak}} expected-note{{Object leaked: object allocated and stored into 'object' is returned from a function whose name ('CFGetRuleViolation') does not contain 'Copy' or 'Create'. This violates the naming convention rules given in the Memory Management Guide for Core Foundation}} } @implementation Foo (FundamentalMemoryManagementRules) - (id)copyViolation { id result = self.propertyValue; // expected-note{{Property returns an Objective-C object with a +0 retain count}} - 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}} } - (id)copyViolationIndexedSubscript { id result = self[0]; // expected-note{{Subscript returns an Objective-C object with a +0 retain count}} - 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}} } - (id)copyViolationKeyedSubscript { id result = self[self]; // expected-note{{Subscript returns an Objective-C object with a +0 retain count}} - 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}} } - (id)getViolation { id result = [[Foo alloc] init]; // expected-note{{Method returns an instance of Foo with a +1 retain count}} - return result; // expected-warning{{leak}} expected-note{{Object returned to caller as an owning reference (single retain count transferred to caller)}} expected-note{{Object leaked: object allocated and stored into 'result' is returned from a method whose name ('getViolation') does not start with 'copy', 'mutableCopy', 'alloc' or 'new'. This violates the naming convention rules given in the Memory Management Guide for Cocoa}} + return result; // expected-warning{{leak}} expected-note{{Object leaked: object allocated and stored into 'result' is returned from a method whose name ('getViolation') does not start with 'copy', 'mutableCopy', 'alloc' or 'new'. This violates the naming convention rules given in the Memory Management Guide for Cocoa}} } - (id)copyAutorelease {