Author: george.karpenkov Date: Thu Jan 17 19:13:53 2019 New Revision: 351514
URL: http://llvm.org/viewvc/llvm-project?rev=351514&view=rev Log: [analyzer] Introduce proper diagnostic for freeing unowned object Insert a note when the object becomes not (exclusively) owned. Differential Revision: https://reviews.llvm.org/D56891 Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h cfe/trunk/test/Analysis/osobject-retain-release.cpp Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp?rev=351514&r1=351513&r2=351514&view=diff ============================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp Thu Jan 17 19:13:53 2019 @@ -803,13 +803,16 @@ ProgramStateRef RetainCountChecker::upda } const RefCountBug & -RetainCountChecker::errorKindToBugKind(RefVal::Kind ErrorKind) const { +RetainCountChecker::errorKindToBugKind(RefVal::Kind ErrorKind, + SymbolRef Sym) const { switch (ErrorKind) { case RefVal::ErrorUseAfterRelease: return useAfterRelease; case RefVal::ErrorReleaseNotOwned: return releaseNotOwned; case RefVal::ErrorDeallocNotOwned: + if (Sym->getType()->getPointeeCXXRecordDecl()) + return freeNotOwned; return deallocNotOwned; default: llvm_unreachable("Unhandled error."); @@ -836,7 +839,8 @@ void RetainCountChecker::processNonLeakE return; auto report = llvm::make_unique<RefCountReport>( - errorKindToBugKind(ErrorKind), C.getASTContext().getLangOpts(), N, Sym); + errorKindToBugKind(ErrorKind, Sym), + C.getASTContext().getLangOpts(), N, Sym); report->addRange(ErrorRange); C.emitReport(std::move(report)); } Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h?rev=351514&r1=351513&r2=351514&view=diff ============================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h (original) +++ cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h Thu Jan 17 19:13:53 2019 @@ -255,6 +255,7 @@ class RetainCountChecker RefCountBug useAfterRelease{this, RefCountBug::UseAfterRelease}; RefCountBug releaseNotOwned{this, RefCountBug::ReleaseNotOwned}; RefCountBug deallocNotOwned{this, RefCountBug::DeallocNotOwned}; + RefCountBug freeNotOwned{this, RefCountBug::FreeNotOwned}; RefCountBug overAutorelease{this, RefCountBug::OverAutorelease}; RefCountBug returnNotOwnedForOwned{this, RefCountBug::ReturnNotOwnedForOwned}; RefCountBug leakWithinFunction{this, RefCountBug::LeakWithinFunction}; @@ -336,8 +337,8 @@ public: RefVal V, ArgEffect E, RefVal::Kind &hasErr, CheckerContext &C) const; - - const RefCountBug &errorKindToBugKind(RefVal::Kind ErrorKind) const; + const RefCountBug &errorKindToBugKind(RefVal::Kind ErrorKind, + SymbolRef Sym) const; void processNonLeakError(ProgramStateRef St, SourceRange ErrorRange, RefVal::Kind ErrorKind, SymbolRef Sym, Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp?rev=351514&r1=351513&r2=351514&view=diff ============================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp Thu Jan 17 19:13:53 2019 @@ -27,6 +27,8 @@ StringRef RefCountBug::bugTypeToName(Ref return "Bad release"; case DeallocNotOwned: return "-dealloc sent to non-exclusively owned object"; + case FreeNotOwned: + return "freeing non-exclusively owned object"; case OverAutorelease: return "Object autoreleased too many times"; case ReturnNotOwnedForOwned: @@ -47,6 +49,8 @@ StringRef RefCountBug::getDescription() "not owned at this point by the caller"; case DeallocNotOwned: return "-dealloc sent to object that may be referenced elsewhere"; + case FreeNotOwned: + return "'free' called on an object that may be referenced elsewhere"; case OverAutorelease: return "Object autoreleased too many times"; case ReturnNotOwnedForOwned: @@ -86,7 +90,8 @@ static std::string getPrettyTypeName(Qua /// Write information about the type state change to {@code os}, /// return whether the note should be generated. static bool shouldGenerateNote(llvm::raw_string_ostream &os, - const RefVal *PrevT, const RefVal &CurrV, + const RefVal *PrevT, + const RefVal &CurrV, bool DeallocSent) { // Get the previous type state. RefVal PrevV = *PrevT; @@ -416,6 +421,11 @@ std::shared_ptr<PathDiagnosticPiece> RefCountReportVisitor::VisitNode(const ExplodedNode *N, BugReporterContext &BRC, BugReport &BR) { + const auto &BT = static_cast<const RefCountBug&>(BR.getBugType()); + + bool IsFreeUnowned = BT.getBugType() == RefCountBug::FreeNotOwned || + BT.getBugType() == RefCountBug::DeallocNotOwned; + const SourceManager &SM = BRC.getSourceManager(); CallEventManager &CEMgr = BRC.getStateManager().getCallEventManager(); if (auto CE = N->getLocationAs<CallExitBegin>()) @@ -434,7 +444,8 @@ RefCountReportVisitor::VisitNode(const E const LocationContext *LCtx = N->getLocationContext(); const RefVal* CurrT = getRefBinding(CurrSt, Sym); - if (!CurrT) return nullptr; + if (!CurrT) + return nullptr; const RefVal &CurrV = *CurrT; const RefVal *PrevT = getRefBinding(PrevSt, Sym); @@ -444,6 +455,12 @@ RefCountReportVisitor::VisitNode(const E std::string sbuf; llvm::raw_string_ostream os(sbuf); + if (PrevT && IsFreeUnowned && CurrV.isNotOwned() && PrevT->isOwned()) { + os << "Object is now not exclusively owned"; + auto Pos = PathDiagnosticLocation::create(N->getLocation(), SM); + return std::make_shared<PathDiagnosticEventPiece>(Pos, os.str()); + } + // This is the allocation site since the previous node had no bindings // for this symbol. if (!PrevT) { @@ -490,9 +507,9 @@ RefCountReportVisitor::VisitNode(const E // program point bool DeallocSent = false; - if (N->getLocation().getTag() && - N->getLocation().getTag()->getTagDescription().contains( - RetainCountChecker::DeallocTagDescription)) { + const ProgramPointTag *Tag = N->getLocation().getTag(); + if (Tag && Tag->getTagDescription().contains( + RetainCountChecker::DeallocTagDescription)) { // We only have summaries attached to nodes after evaluating CallExpr and // ObjCMessageExprs. const Stmt *S = N->getLocation().castAs<StmtPoint>().getStmt(); Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h?rev=351514&r1=351513&r2=351514&view=diff ============================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h (original) +++ cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h Thu Jan 17 19:13:53 2019 @@ -30,6 +30,7 @@ public: UseAfterRelease, ReleaseNotOwned, DeallocNotOwned, + FreeNotOwned, OverAutorelease, ReturnNotOwnedForOwned, LeakWithinFunction, Modified: cfe/trunk/test/Analysis/osobject-retain-release.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/osobject-retain-release.cpp?rev=351514&r1=351513&r2=351514&view=diff ============================================================================== --- cfe/trunk/test/Analysis/osobject-retain-release.cpp (original) +++ cfe/trunk/test/Analysis/osobject-retain-release.cpp Thu Jan 17 19:13:53 2019 @@ -634,3 +634,13 @@ void test_ostypealloc_correct_diagnostic arr->release(); // expected-note{{Reference count decremented. The object now has a +1 retain count}} } // expected-note{{Object leaked: object allocated and stored into 'arr' is not referenced later in this execution path and has a retain count of +1}} // expected-warning@-1{{Potential leak of an object stored into 'arr'}} + +void escape_elsewhere(OSObject *obj); + +void test_free_on_escaped_object_diagnostics() { + OSObject *obj = new OSObject; // expected-note{{Operator 'new' returns an OSObject of type 'OSObject' with a +1 retain count}} + escape_elsewhere(obj); // expected-note{{Object is now not exclusively owned}} + obj->free(); // expected-note{{'free' called on an object that may be referenced elsewhere}} + // expected-warning@-1{{'free' called on an object that may be referenced elsewhere}} +} + _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits