From 04553e530f971eff94d2be7111855af3781b41f4 Mon Sep 17 00:00:00 2001 From: George Karpenkov Date: Fri, 21 Sep 2018 20:37:20 +0000 Subject: [analyzer] Process state in checkEndFunction in RetainCountChecker Modify the RetainCountChecker to perform state "adjustments" in checkEndFunction, as performing work in PreStmt does not work with destructors. The previous version made an implicit assumption that no code runs after the return statement is executed. rdar://43945028 Differential Revision: https://reviews.llvm.org/D52338 llvm-svn: 342770 --- .../RetainCountChecker/RetainCountChecker.cpp | 87 ++++++++++++---------- .../RetainCountChecker/RetainCountChecker.h | 14 +++- 2 files changed, 57 insertions(+), 44 deletions(-) (limited to 'clang/lib/StaticAnalyzer') diff --git a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp index de113bbd057..e5d27f577d1 100644 --- a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp @@ -814,12 +814,9 @@ bool RetainCountChecker::evalCall(const CallExpr *CE, CheckerContext &C) const { 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 @@ void RetainCountChecker::checkPreStmt(const ReturnStmt *S, // 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 @@ void RetainCountChecker::checkPreStmt(const ReturnStmt *S, 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 @@ void RetainCountChecker::checkPreStmt(const ReturnStmt *S, // 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 @@ void RetainCountChecker::checkPreStmt(const ReturnStmt *S, } } - 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 @@ void RetainCountChecker::checkReturnWithRetEffect(const ReturnStmt *S, // [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 @@ void RetainCountChecker::checkReturnWithRetEffect(const ReturnStmt *S, 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 @@ void RetainCountChecker::checkReturnWithRetEffect(const ReturnStmt *S, 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 @@ RetainCountChecker::checkRegionChanges(ProgramStateRef state, 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 @@ RetainCountChecker::handleAutoreleaseCounts(ProgramStateRef state, 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 @@ RetainCountChecker::handleAutoreleaseCounts(ProgramStateRef state, 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::checkBeginFunction(CheckerContext &Ctx) const { 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(); diff --git a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h index 8683b23dd96..e8d9136ffd2 100644 --- a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h +++ b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h @@ -252,7 +252,6 @@ class RetainCountChecker check::PostStmt, check::PostStmt, check::PostCall, - check::PreStmt, check::RegionChanges, eval::Assume, eval::Call > { @@ -388,8 +387,7 @@ public: 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 @@ public: 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; }; //===----------------------------------------------------------------------===// -- cgit v1.2.3