r351514 - [analyzer] Introduce proper diagnostic for freeing unowned object
George Karpenkov via cfe-commits
cfe-commits at lists.llvm.org
Thu Jan 17 19:13:53 PST 2019
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 at -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 at -1{{'free' called on an object that may be referenced elsewhere}}
+}
+
More information about the cfe-commits
mailing list