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,9 @@ 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, releaseNotOwned, deallocNotOwned, + overAutorelease, returnNotOwnedForOwned, leakWithinFunction, leakAtReturn; mutable std::unique_ptr Summaries; public: @@ -266,11 +265,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 +331,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 @@ -202,6 +139,30 @@ }; } // end anonymous namespace +RetainCountChecker::RetainCountChecker() : + useAfterRelease( + this, "Use-after-release", + "Reference-counted object is used after it is released"), + releaseNotOwned( + this, "Bad release", + "Incorrect decrement of the reference count of an object that is " + "not owned at this point by the caller"), + deallocNotOwned( + this, "-dealloc sent to non-exclusively owned object", + "-dealloc sent to object that may be referenced elsewhere"), + overAutorelease( + this, "Object autoreleased too many times", + "Object autoreleased too many times" + ), + returnNotOwnedForOwned( + this, "Method should return an owned object", + "Object with a +0 retain count returned to caller where a +1 " + "(owning) retain count is expected" + ), + leakWithinFunction(this, "Leak", "", /*SuppressOnSink=*/true), + leakAtReturn(this, "Leak of returned object", "", + /*SuppressOnSink=*/true) {} + //===----------------------------------------------------------------------===// // Handle statements that may have an effect on refcounts. //===----------------------------------------------------------------------===// @@ -414,20 +375,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 +826,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 +859,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 +1063,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 +1088,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 +1241,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 +1289,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,34 @@ namespace retaincountchecker { class RefCountBug : public BugType { -protected: - RefCountBug(const CheckerBase *checker, StringRef name) - : BugType(checker, name, categories::MemoryRefCount) {} - + std::string Description; public: - virtual const char *getDescription() const = 0; + RefCountBug(const CheckerBase *checker, StringRef name, StringRef Description, + bool SuppressOnSink = false) + : BugType(checker, name, categories::MemoryRefCount, SuppressOnSink), + Description(std::move(Description)) {} - virtual bool isLeak() const { return false; } + StringRef getDescription() const { + return Description; + } }; 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 +71,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 @@ -711,15 +711,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 +805,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)