Index: clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h =================================================================== --- clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h +++ clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h @@ -251,10 +251,14 @@ check::RegionChanges, eval::Assume, eval::Call > { - mutable std::unique_ptr useAfterRelease, releaseNotOwned; - mutable std::unique_ptr deallocNotOwned; - mutable std::unique_ptr overAutorelease, returnNotOwnedForOwned; - mutable std::unique_ptr leakWithinFunction, leakAtReturn; + + RefCountBug useAfterRelease{this, RefCountBug::UseAfterRelease}; + RefCountBug releaseNotOwned{this, RefCountBug::ReleaseNotOwned}; + RefCountBug deallocNotOwned{this, RefCountBug::DeallocNotOwned}; + RefCountBug overAutorelease{this, RefCountBug::OverAutorelease}; + RefCountBug returnNotOwnedForOwned{this, RefCountBug::ReturnNotOwnedForOwned}; + RefCountBug leakWithinFunction{this, RefCountBug::LeakWithinFunction}; + RefCountBug leakAtReturn{this, RefCountBug::LeakAtReturn}; mutable std::unique_ptr Summaries; public: @@ -266,11 +270,7 @@ /// Track sublcasses of OSObject. bool TrackOSObjects = false; - RetainCountChecker() {} - - RefCountBug *getLeakWithinFunctionBug(const LangOptions &LOpts) const; - - RefCountBug *getLeakAtReturnBug(const LangOptions &LOpts) const; + RetainCountChecker() {}; RetainSummaryManager &getSummaryManager(ASTContext &Ctx) const { // FIXME: We don't support ARC being turned on and off during one analysis. @@ -336,6 +336,9 @@ RefVal V, ArgEffect E, RefVal::Kind &hasErr, CheckerContext &C) const; + + const RefCountBug & errorKindToBugKind(RefVal::Kind ErrorKind) const; + void processNonLeakError(ProgramStateRef St, SourceRange ErrorRange, RefVal::Kind ErrorKind, SymbolRef Sym, CheckerContext &C) const; Index: clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp +++ clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp @@ -39,69 +39,6 @@ return State->remove(Sym); } -class UseAfterRelease : public RefCountBug { -public: - UseAfterRelease(const CheckerBase *checker) - : RefCountBug(checker, "Use-after-release") {} - - const char *getDescription() const override { - return "Reference-counted object is used after it is released"; - } -}; - -class BadRelease : public RefCountBug { -public: - BadRelease(const CheckerBase *checker) : RefCountBug(checker, "Bad release") {} - - const char *getDescription() const override { - return "Incorrect decrement of the reference count of an object that is " - "not owned at this point by the caller"; - } -}; - -class DeallocNotOwned : public RefCountBug { -public: - DeallocNotOwned(const CheckerBase *checker) - : RefCountBug(checker, "-dealloc sent to non-exclusively owned object") {} - - const char *getDescription() const override { - return "-dealloc sent to object that may be referenced elsewhere"; - } -}; - -class OverAutorelease : public RefCountBug { -public: - OverAutorelease(const CheckerBase *checker) - : RefCountBug(checker, "Object autoreleased too many times") {} - - const char *getDescription() const override { - return "Object autoreleased too many times"; - } -}; - -class ReturnedNotOwnedForOwned : public RefCountBug { -public: - ReturnedNotOwnedForOwned(const CheckerBase *checker) - : RefCountBug(checker, "Method should return an owned object") {} - - const char *getDescription() const override { - return "Object with a +0 retain count returned to caller where a +1 " - "(owning) retain count is expected"; - } -}; - -class Leak : public RefCountBug { -public: - Leak(const CheckerBase *checker, StringRef name) : RefCountBug(checker, name) { - // Leaks should not be reported if they are post-dominated by a sink. - setSuppressOnSink(true); - } - - const char *getDescription() const override { return ""; } - - bool isLeak() const override { return true; } -}; - } // end namespace retaincountchecker } // end namespace ento } // end namespace clang @@ -414,20 +351,6 @@ checkSummary(*Summ, Call, C); } -RefCountBug * -RetainCountChecker::getLeakWithinFunctionBug(const LangOptions &LOpts) const { - if (!leakWithinFunction) - leakWithinFunction.reset(new Leak(this, "Leak")); - return leakWithinFunction.get(); -} - -RefCountBug * -RetainCountChecker::getLeakAtReturnBug(const LangOptions &LOpts) const { - if (!leakAtReturn) - leakAtReturn.reset(new Leak(this, "Leak of returned object")); - return leakAtReturn.get(); -} - /// GetReturnType - Used to get the return type of a message expression or /// function call with the intention of affixing that type to a tracked symbol. /// While the return type can be queried directly from RetEx, when @@ -879,6 +802,20 @@ return setRefBinding(state, sym, V); } +const RefCountBug & +RetainCountChecker::errorKindToBugKind(RefVal::Kind ErrorKind) const { + switch (ErrorKind) { + case RefVal::ErrorUseAfterRelease: + return useAfterRelease; + case RefVal::ErrorReleaseNotOwned: + return releaseNotOwned; + case RefVal::ErrorDeallocNotOwned: + return deallocNotOwned; + default: + llvm_unreachable("Unhandled error."); + } +} + void RetainCountChecker::processNonLeakError(ProgramStateRef St, SourceRange ErrorRange, RefVal::Kind ErrorKind, @@ -898,30 +835,8 @@ if (!N) return; - RefCountBug *BT; - switch (ErrorKind) { - default: - llvm_unreachable("Unhandled error."); - case RefVal::ErrorUseAfterRelease: - if (!useAfterRelease) - useAfterRelease.reset(new UseAfterRelease(this)); - BT = useAfterRelease.get(); - break; - case RefVal::ErrorReleaseNotOwned: - if (!releaseNotOwned) - releaseNotOwned.reset(new BadRelease(this)); - BT = releaseNotOwned.get(); - break; - case RefVal::ErrorDeallocNotOwned: - if (!deallocNotOwned) - deallocNotOwned.reset(new DeallocNotOwned(this)); - BT = deallocNotOwned.get(); - break; - } - - assert(BT); auto report = llvm::make_unique( - *BT, C.getASTContext().getLangOpts(), N, Sym); + errorKindToBugKind(ErrorKind), C.getASTContext().getLangOpts(), N, Sym); report->addRange(ErrorRange); C.emitReport(std::move(report)); } @@ -1124,8 +1039,8 @@ ExplodedNode *N = C.addTransition(state, Pred, &ReturnOwnLeakTag); if (N) { const LangOptions &LOpts = C.getASTContext().getLangOpts(); - auto R = llvm::make_unique( - *getLeakAtReturnBug(LOpts), LOpts, N, Sym, C); + auto R = + llvm::make_unique(leakAtReturn, LOpts, N, Sym, C); C.emitReport(std::move(R)); } return N; @@ -1149,11 +1064,8 @@ ExplodedNode *N = C.addTransition(state, Pred, &ReturnNotOwnedTag); if (N) { - if (!returnNotOwnedForOwned) - returnNotOwnedForOwned.reset(new ReturnedNotOwnedForOwned(this)); - auto R = llvm::make_unique( - *returnNotOwnedForOwned, C.getASTContext().getLangOpts(), N, Sym); + returnNotOwnedForOwned, C.getASTContext().getLangOpts(), N, Sym); C.emitReport(std::move(R)); } return N; @@ -1305,12 +1217,9 @@ os << "but "; os << "has a +" << V.getCount() << " retain count"; - if (!overAutorelease) - overAutorelease.reset(new OverAutorelease(this)); - const LangOptions &LOpts = Ctx.getASTContext().getLangOpts(); - auto R = llvm::make_unique(*overAutorelease, LOpts, N, Sym, - os.str()); + auto R = llvm::make_unique(overAutorelease, LOpts, N, Sym, + os.str()); Ctx.emitReport(std::move(R)); } @@ -1356,11 +1265,8 @@ if (N) { for (SymbolRef L : Leaked) { - RefCountBug *BT = Pred ? getLeakWithinFunctionBug(LOpts) - : getLeakAtReturnBug(LOpts); - assert(BT && "BugType not initialized."); - - Ctx.emitReport(llvm::make_unique(*BT, LOpts, N, L, Ctx)); + const RefCountBug &BT = Pred ? leakWithinFunction : leakAtReturn; + Ctx.emitReport(llvm::make_unique(BT, LOpts, N, L, Ctx)); } } Index: clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h =================================================================== --- clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h +++ clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h @@ -25,32 +25,43 @@ namespace retaincountchecker { class RefCountBug : public BugType { -protected: - RefCountBug(const CheckerBase *checker, StringRef name) - : BugType(checker, name, categories::MemoryRefCount) {} - public: - virtual const char *getDescription() const = 0; + enum RefCountBugType { + UseAfterRelease, + ReleaseNotOwned, + DeallocNotOwned, + OverAutorelease, + ReturnNotOwnedForOwned, + LeakWithinFunction, + LeakAtReturn, + }; + RefCountBug(const CheckerBase *checker, RefCountBugType BT); + StringRef getDescription() const; + RefCountBugType getBugType() const { + return BT; + } - virtual bool isLeak() const { return false; } +private: + RefCountBugType BT; + static StringRef bugTypeToName(RefCountBugType BT); }; class RefCountReport : public BugReport { protected: SymbolRef Sym; + bool isLeak; public: - RefCountReport(RefCountBug &D, const LangOptions &LOpts, + RefCountReport(const RefCountBug &D, const LangOptions &LOpts, ExplodedNode *n, SymbolRef sym, - bool registerVisitor = true); + bool isLeak=false); - RefCountReport(RefCountBug &D, const LangOptions &LOpts, + RefCountReport(const RefCountBug &D, const LangOptions &LOpts, ExplodedNode *n, SymbolRef sym, StringRef endText); llvm::iterator_range getRanges() override { - const RefCountBug& BugTy = static_cast(getBugType()); - if (!BugTy.isLeak()) + if (!isLeak) return BugReport::getRanges(); return llvm::make_range(ranges_iterator(), ranges_iterator()); } @@ -69,7 +80,7 @@ void createDescription(CheckerContext &Ctx); public: - RefLeakReport(RefCountBug &D, const LangOptions &LOpts, ExplodedNode *n, + RefLeakReport(const RefCountBug &D, const LangOptions &LOpts, ExplodedNode *n, SymbolRef sym, CheckerContext &Ctx); PathDiagnosticLocation getLocation(const SourceManager &SM) const override { Index: clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp +++ clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp @@ -19,6 +19,50 @@ using namespace ento; using namespace retaincountchecker; +StringRef RefCountBug::bugTypeToName(RefCountBug::RefCountBugType BT) { + switch (BT) { + case UseAfterRelease: + return "Use-after-release"; + case ReleaseNotOwned: + return "Bad release"; + case DeallocNotOwned: + return "-dealloc sent to non-exclusively owned object"; + case OverAutorelease: + return "Object autoreleased too many times"; + case ReturnNotOwnedForOwned: + return "Method should return an owned object"; + case LeakWithinFunction: + return "Leak"; + case LeakAtReturn: + return "Leak of returned object"; + } +} + +StringRef RefCountBug::getDescription() const { + switch (BT) { + case UseAfterRelease: + return "Reference-counted object is used after it is released"; + case ReleaseNotOwned: + return "Incorrect decrement of the reference count of an object that is " + "not owned at this point by the caller"; + case DeallocNotOwned: + return "-dealloc sent to object that may be referenced elsewhere"; + case OverAutorelease: + return "Object autoreleased too many times"; + case ReturnNotOwnedForOwned: + return "Object with a +0 retain count returned to caller where a +1 " + "(owning) retain count is expected"; + case LeakWithinFunction: + case LeakAtReturn: + return ""; + } +} + +RefCountBug::RefCountBug(const CheckerBase *Checker, RefCountBugType BT) + : BugType(Checker, bugTypeToName(BT), categories::MemoryRefCount, + /*SupressOnSink=*/BT == LeakWithinFunction || BT == LeakAtReturn), + BT(BT) {} + static bool isNumericLiteralExpression(const Expr *E) { // FIXME: This set of cases was copied from SemaExprObjC. return isa(E) || @@ -711,15 +755,15 @@ return std::make_shared(L, os.str()); } -RefCountReport::RefCountReport(RefCountBug &D, const LangOptions &LOpts, +RefCountReport::RefCountReport(const RefCountBug &D, const LangOptions &LOpts, ExplodedNode *n, SymbolRef sym, - bool registerVisitor) - : BugReport(D, D.getDescription(), n), Sym(sym) { - if (registerVisitor) + bool isLeak) + : BugReport(D, D.getDescription(), n), Sym(sym), isLeak(isLeak) { + if (!isLeak) addVisitor(llvm::make_unique(sym)); } -RefCountReport::RefCountReport(RefCountBug &D, const LangOptions &LOpts, +RefCountReport::RefCountReport(const RefCountBug &D, const LangOptions &LOpts, ExplodedNode *n, SymbolRef sym, StringRef endText) : BugReport(D, D.getDescription(), endText, n) { @@ -805,10 +849,10 @@ } } -RefLeakReport::RefLeakReport(RefCountBug &D, const LangOptions &LOpts, +RefLeakReport::RefLeakReport(const RefCountBug &D, const LangOptions &LOpts, ExplodedNode *n, SymbolRef sym, CheckerContext &Ctx) - : RefCountReport(D, LOpts, n, sym, false) { + : RefCountReport(D, LOpts, n, sym, /*isLeak=*/true) { deriveAllocLocation(Ctx, sym); if (!AllocBinding)