This revision was automatically updated to reflect the committed changes.
Closed by commit rC351514: [analyzer] Introduce proper diagnostic for freeing
unowned object (authored by george.karpenkov, committed by ).
Herald added a subscriber: cfe-commits.
Changed prior to commit:
https://reviews.llvm.org/D56891?vs=182431&id=182445#toc
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56891/new/
https://reviews.llvm.org/D56891
Files:
lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h
lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h
test/Analysis/osobject-retain-release.cpp
Index: lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h
===================================================================
--- lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h
+++ lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h
@@ -30,6 +30,7 @@
UseAfterRelease,
ReleaseNotOwned,
DeallocNotOwned,
+ FreeNotOwned,
OverAutorelease,
ReturnNotOwnedForOwned,
LeakWithinFunction,
Index: lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h
===================================================================
--- lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h
+++ lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h
@@ -255,6 +255,7 @@
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 @@
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,
Index: lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
+++ lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
@@ -27,6 +27,8 @@
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 @@
"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 @@
/// 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 @@
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 @@
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 @@
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 @@
// 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();
Index: lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
+++ lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
@@ -803,13 +803,16 @@
}
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 @@
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));
}
Index: test/Analysis/osobject-retain-release.cpp
===================================================================
--- test/Analysis/osobject-retain-release.cpp
+++ test/Analysis/osobject-retain-release.cpp
@@ -634,3 +634,13 @@
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
