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