[analyzer] Preliminary version of retain count checking for OSObjects

Has quite a lot of false positives, disabled behind the flag.

Differential Revision: https://reviews.llvm.org/D50880

llvm-svn: 340502

clang/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h

@@ -154,7 +154,10 @@ public:
     /// Indicates that the tracked object could be a CF or Objective-C object.
     AnyObj,
     /// Indicates that the tracked object is a generalized object.
-    Generalized
+    Generalized,
+
+    /// A descendant of OSObject.
+    OS
   };
 
 private:
@@ -318,14 +321,21 @@ class RetainSummary {
   ///  this is the effect applied to the state of the receiver.
   ArgEffect Receiver;
 
+  /// Effect on "this" pointer - applicable only to C++ method calls.
+  ArgEffect This;
+
   /// Ret - The effect on the return value.  Used to indicate if the
   ///  function/method call returns a new tracked symbol.
   RetEffect Ret;
 
 public:
-  RetainSummary(ArgEffects A, RetEffect R, ArgEffect defaultEff,
-                ArgEffect ReceiverEff)
-    : Args(A), DefaultArgEffect(defaultEff), Receiver(ReceiverEff), Ret(R) {}
+  RetainSummary(ArgEffects A,
+                RetEffect R,
+                ArgEffect defaultEff,
+                ArgEffect ReceiverEff,
+                ArgEffect ThisEff)
+    : Args(A), DefaultArgEffect(defaultEff), Receiver(ReceiverEff),
+      This(ThisEff), Ret(R) {}
 
   /// getArg - Return the argument effect on the argument specified by
   ///  idx (starting from 0).
@@ -359,12 +369,20 @@ public:
   ///  This is only meaningful if the summary applies to an ObjCMessageExpr*.
   ArgEffect getReceiverEffect() const { return Receiver; }
 
+  ArgEffect getThisEffect() const { return This; }
+
+  bool isNoop() const {
+    return Ret == RetEffect::MakeNoRet() && Receiver == DoNothing
+      && DefaultArgEffect == MayEscape && This == DoNothing
+      && Args.isEmpty();
+  }
+
   /// Test if two retain summaries are identical. Note that merely equivalent
   /// summaries are not necessarily identical (for example, if an explicit
   /// argument effect matches the default effect).
   bool operator==(const RetainSummary &Other) const {
     return Args == Other.Args && DefaultArgEffect == Other.DefaultArgEffect &&
-           Receiver == Other.Receiver && Ret == Other.Ret;
+           Receiver == Other.Receiver && This == Other.This && Ret == Other.Ret;
   }
 
   /// Profile this summary for inclusion in a FoldingSet.
@@ -372,6 +390,7 @@ public:
     ID.Add(Args);
     ID.Add(DefaultArgEffect);
     ID.Add(Receiver);
+    ID.Add(This);
     ID.Add(Ret);
   }
 
@@ -460,6 +479,9 @@ class RetainSummaryManager {
   /// Records whether or not the analyzed code runs in ARC mode.
   const bool ARCEnabled;
 
+  /// Track sublcasses of OSObject
+  const bool TrackOSObjects;
+
   /// FuncSummaries - A map from FunctionDecls to summaries.
   FuncSummariesTy FuncSummaries;
 
@@ -496,6 +518,19 @@ class RetainSummaryManager {
   ///  data in ScratchArgs.
   ArgEffects getArgEffects();
 
+  /// Create an OS object at +1.
+  const RetainSummary *getOSSummaryCreateRule(const FunctionDecl *FD);
+
+  /// Get an OS object at +0.
+  const RetainSummary *getOSSummaryGetRule(const FunctionDecl *FD);
+
+  /// Increment the reference count on OS object.
+  const RetainSummary *getOSSummaryRetainRule(const FunctionDecl *FD);
+
+  /// Decrement the reference count on OS object.
+  const RetainSummary *getOSSummaryReleaseRule(const FunctionDecl *FD);
+
+
   enum UnaryFuncKind { cfretain, cfrelease, cfautorelease, cfmakecollectable };
 
   const RetainSummary *getUnarySummary(const FunctionType* FT,
@@ -509,8 +544,10 @@ class RetainSummaryManager {
 
   const RetainSummary *getPersistentSummary(RetEffect RetEff,
                                             ArgEffect ReceiverEff = DoNothing,
-                                            ArgEffect DefaultEff = MayEscape) {
-    RetainSummary Summ(getArgEffects(), RetEff, DefaultEff, ReceiverEff);
+                                            ArgEffect DefaultEff = MayEscape,
+                                            ArgEffect ThisEff = DoNothing) {
+    RetainSummary Summ(getArgEffects(), RetEff, DefaultEff, ReceiverEff,
+                       ThisEff);
     return getPersistentSummary(Summ);
   }
 
@@ -584,9 +621,12 @@ class RetainSummaryManager {
                                         bool &AllowAnnotations);
 
 public:
-  RetainSummaryManager(ASTContext &ctx, bool usesARC)
+  RetainSummaryManager(ASTContext &ctx,
+                       bool usesARC,
+                       bool trackOSObject)
    : Ctx(ctx),
      ARCEnabled(usesARC),
+     TrackOSObjects(trackOSObject),
      AF(BPAlloc), ScratchArgs(AF.getEmptyMap()),
      ObjCAllocRetE(usesARC ? RetEffect::MakeNotOwned(RetEffect::ObjC)
                                : RetEffect::MakeOwned(RetEffect::ObjC)),

clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp

@@ -418,13 +418,18 @@ void RetainCountChecker::processSummaryOfInlined(const RetainSummary &Summ,
   }
 
   // Consult the summary for the return value.
+  SymbolRef Sym = CallOrMsg.getReturnValue().getAsSymbol();
   RetEffect RE = Summ.getRetEffect();
-  if (RE.getKind() == RetEffect::NoRetHard) {
-    SymbolRef Sym = CallOrMsg.getReturnValue().getAsSymbol();
-    if (Sym)
-      state = removeRefBinding(state, Sym);
+  if (const auto *MCall = dyn_cast<CXXMemberCall>(&CallOrMsg)) {
+    if (Optional<RefVal> updatedRefVal =
+            refValFromRetEffect(RE, MCall->getResultType())) {
+      state = setRefBinding(state, Sym, *updatedRefVal);
+    }
   }
 
+  if (RE.getKind() == RetEffect::NoRetHard && Sym)
+    state = removeRefBinding(state, Sym);
+
   C.addTransition(state);
 }
 
@@ -490,11 +495,10 @@ void RetainCountChecker::checkSummary(const RetainSummary &Summ,
     }
   }
 
-  // Evaluate the effect on the message receiver.
+  // Evaluate the effect on the message receiver / `this` argument.
   bool ReceiverIsTracked = false;
   if (!hasErr) {
-    const ObjCMethodCall *MsgInvocation = dyn_cast<ObjCMethodCall>(&CallOrMsg);
-    if (MsgInvocation) {
+    if (const auto *MsgInvocation = dyn_cast<ObjCMethodCall>(&CallOrMsg)) {
       if (SymbolRef Sym = MsgInvocation->getReceiverSVal().getAsLocSymbol()) {
         if (const RefVal *T = getRefBinding(state, Sym)) {
           ReceiverIsTracked = true;
@@ -506,6 +510,17 @@ void RetainCountChecker::checkSummary(const RetainSummary &Summ,
           }
         }
       }
+    } else if (const auto *MCall = dyn_cast<CXXMemberCall>(&CallOrMsg)) {
+      if (SymbolRef Sym = MCall->getCXXThisVal().getAsLocSymbol()) {
+        if (const RefVal *T = getRefBinding(state, Sym)) {
+          state = updateSymbol(state, Sym, *T, Summ.getThisEffect(),
+                               hasErr, C);
+          if (hasErr) {
+            ErrorRange = MCall->getOriginExpr()->getSourceRange();
+            ErrorSym = Sym;
+          }
+        }
+      }
     }
   }
 

clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h

@@ -98,7 +98,7 @@ private:
   /// The kind of object being tracked (CF or ObjC), if known.
   ///
   /// See the RetEffect::ObjKind enum for possible values.
-  unsigned RawObjectKind : 2;
+  unsigned RawObjectKind : 3;
 
   /// True if the current state and/or retain count may turn out to not be the
   /// best possible approximation of the reference counting state.
@@ -268,6 +268,8 @@ class RetainCountChecker
 
   mutable std::unique_ptr<RetainSummaryManager> Summaries;
   mutable SummaryLogTy SummaryLog;
+
+  AnalyzerOptions &Options;
   mutable bool ShouldResetSummaryLog;
 
   /// Optional setting to indicate if leak reports should include
@@ -275,12 +277,17 @@ class RetainCountChecker
   mutable bool IncludeAllocationLine;
 
 public:
-  RetainCountChecker(AnalyzerOptions &AO)
-    : ShouldResetSummaryLog(false),
-      IncludeAllocationLine(shouldIncludeAllocationSiteInLeakDiagnostics(AO)) {}
+  RetainCountChecker(AnalyzerOptions &Options)
+      : Options(Options), ShouldResetSummaryLog(false),
+        IncludeAllocationLine(
+            shouldIncludeAllocationSiteInLeakDiagnostics(Options)) {}
 
   ~RetainCountChecker() override { DeleteContainerSeconds(DeadSymbolTags); }
 
+  bool shouldCheckOSObjectRetainCount() const {
+    return Options.getBooleanOption("CheckOSObject", false, this);
+  }
+
   void checkEndAnalysis(ExplodedGraph &G, BugReporter &BR,
                         ExprEngine &Eng) const {
     // FIXME: This is a hack to make sure the summary log gets cleared between
@@ -333,10 +340,12 @@ public:
     // FIXME: We don't support ARC being turned on and off during one analysis.
     // (nor, for that matter, do we support changing ASTContexts)
     bool ARCEnabled = (bool)Ctx.getLangOpts().ObjCAutoRefCount;
-    if (!Summaries)
-      Summaries.reset(new RetainSummaryManager(Ctx, ARCEnabled));
-    else
+    if (!Summaries) {
+      Summaries.reset(new RetainSummaryManager(
+          Ctx, ARCEnabled, shouldCheckOSObjectRetainCount()));
+    } else {
       assert(Summaries->isARCEnabled() == ARCEnabled);
+    }
     return *Summaries;
   }
 

clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp

@@ -120,6 +120,9 @@ CFRefReportVisitor::VisitNode(const ExplodedNode *N, const ExplodedNode *PrevN,
       if (CurrV.getObjKind() == RetEffect::CF) {
         os << " returns a Core Foundation object of type "
            << Sym->getType().getAsString() << " with a ";
+      } else if (CurrV.getObjKind() == RetEffect::OS) {
+        os << " returns an OSObject of type "
+           << Sym->getType().getAsString() << " with a ";
       } else if (CurrV.getObjKind() == RetEffect::Generalized) {
         os << " returns an object of type " << Sym->getType().getAsString()
            << " with a ";

clang/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp

@@ -53,6 +53,31 @@ RetainSummaryManager::getPersistentSummary(const RetainSummary &OldSumm) {
   return Summ;
 }
 
+static bool isOSObjectSubclass(QualType T);
+
+static bool isOSObjectSubclass(const CXXRecordDecl *RD) {
+  if (RD->getDeclName().getAsString() == "OSObject")
+    return true;
+
+  const CXXRecordDecl *RDD = RD->getDefinition();
+  if (!RDD)
+    return false;
+
+  for (const CXXBaseSpecifier Spec : RDD->bases()) {
+    if (isOSObjectSubclass(Spec.getType()))
+      return true;
+  }
+  return false;
+}
+
+/// \return Whether type represents an OSObject successor.
+static bool isOSObjectSubclass(QualType T) {
+  if (const auto *RD = T->getAsCXXRecordDecl()) {
+    return isOSObjectSubclass(RD);
+  }
+  return false;
+}
+
 static bool hasRCAnnotation(const Decl *D, StringRef rcAnnotation) {
   for (const auto *Ann : D->specific_attrs<AnnotateAttr>()) {
     if (Ann->getAnnotation() == rcAnnotation)
@@ -196,6 +221,17 @@ RetainSummaryManager::generateSummary(const FunctionDecl *FD,
   }
 
   if (RetTy->isPointerType()) {
+    if (TrackOSObjects && isOSObjectSubclass(RetTy->getPointeeType())) {
+      if (const IdentifierInfo *II = FD->getIdentifier()) {
+        StringRef FuncName = II->getName();
+        if (FuncName.contains_lower("with")
+            || FuncName.contains_lower("create")
+            || FuncName.contains_lower("copy"))
+          return getOSSummaryCreateRule(FD);
+      }
+      return getOSSummaryGetRule(FD);
+    }
+
     // For CoreFoundation ('CF') types.
     if (cocoa::isRefType(RetTy, "CF", FName)) {
       if (isRetain(FD, FName)) {
@@ -241,6 +277,17 @@ RetainSummaryManager::generateSummary(const FunctionDecl *FD,
     }
   }
 
+  if (const auto *MD = dyn_cast<CXXMethodDecl>(FD)) {
+    const CXXRecordDecl *Parent = MD->getParent();
+    if (TrackOSObjects && isOSObjectSubclass(Parent)) {
+      if (isRelease(FD, FName))
+        return getOSSummaryReleaseRule(FD);
+
+      if (isRetain(FD, FName))
+        return getOSSummaryRetainRule(FD);
+    }
+  }
+
   // Check for release functions, the only kind of functions that we care
   // about that don't return a pointer type.
   if (FName.size() >= 2 && FName[0] == 'C' &&
@@ -279,6 +326,14 @@ RetainSummaryManager::generateSummary(const FunctionDecl *FD,
     }
   }
 
+  if (const auto *MD = dyn_cast<CXXMethodDecl>(FD)) {
+
+    // Stop tracking arguments passed to C++ methods, as those might be
+    // wrapping smart pointers.
+    return getPersistentSummary(RetEffect::MakeNoRet(), DoNothing, StopTracking,
+                                DoNothing);
+  }
+
   return getDefaultSummary();
 }
 
@@ -411,6 +466,8 @@ RetainSummaryManager::getSummary(const CallEvent &Call,
     Summ = getFunctionSummary(cast<SimpleFunctionCall>(Call).getDecl());
     break;
   case CE_CXXMember:
+    Summ = getFunctionSummary(cast<CXXMemberCall>(Call).getDecl());
+    break;
   case CE_CXXMemberOperator:
   case CE_Block:
   case CE_CXXConstructor:
@@ -513,6 +570,32 @@ RetainSummaryManager::getUnarySummary(const FunctionType* FT,
   return getPersistentSummary(RetEffect::MakeNoRet(), DoNothing, DoNothing);
 }
 
+const RetainSummary *
+RetainSummaryManager::getOSSummaryRetainRule(const FunctionDecl *FD) {
+  return getPersistentSummary(RetEffect::MakeNoRet(),
+                              /*ReceiverEff=*/DoNothing,
+                              /*DefaultEff=*/DoNothing,
+                              /*ThisEff=*/IncRef);
+}
+
+const RetainSummary *
+RetainSummaryManager::getOSSummaryReleaseRule(const FunctionDecl *FD) {
+  return getPersistentSummary(RetEffect::MakeNoRet(),
+                              /*ReceiverEff=*/DoNothing,
+                              /*DefaultEff=*/DoNothing,
+                              /*ThisEff=*/DecRef);
+}
+
+const RetainSummary *
+RetainSummaryManager::getOSSummaryCreateRule(const FunctionDecl *FD) {
+  return getPersistentSummary(RetEffect::MakeOwned(RetEffect::OS));
+}
+
+const RetainSummary *
+RetainSummaryManager::getOSSummaryGetRule(const FunctionDecl *FD) {
+  return getPersistentSummary(RetEffect::MakeNotOwned(RetEffect::OS));
+}
+
 const RetainSummary *
 RetainSummaryManager::getCFSummaryCreateRule(const FunctionDecl *FD) {
   assert (ScratchArgs.isEmpty());
@@ -877,7 +960,7 @@ void RetainSummaryManager::InitializeMethodSummaries() {
 CallEffects CallEffects::getEffect(const ObjCMethodDecl *MD) {
   ASTContext &Ctx = MD->getASTContext();
   LangOptions L = Ctx.getLangOpts();
-  RetainSummaryManager M(Ctx, L.ObjCAutoRefCount);
+  RetainSummaryManager M(Ctx, L.ObjCAutoRefCount, /*TrackOSObjects=*/false);
   const RetainSummary *S = M.getMethodSummary(MD);
   CallEffects CE(S->getRetEffect());
   CE.Receiver = S->getReceiverEffect();
@@ -891,7 +974,7 @@ CallEffects CallEffects::getEffect(const ObjCMethodDecl *MD) {
 CallEffects CallEffects::getEffect(const FunctionDecl *FD) {
   ASTContext &Ctx = FD->getASTContext();
   LangOptions L = Ctx.getLangOpts();
-  RetainSummaryManager M(Ctx, L.ObjCAutoRefCount);
+  RetainSummaryManager M(Ctx, L.ObjCAutoRefCount, /*TrackOSObjects=*/false);
   const RetainSummary *S = M.getFunctionSummary(FD);
   CallEffects CE(S->getRetEffect());
   unsigned N = FD->param_size();

clang/test/Analysis/osobject-retain-release.cpp

@@ -0,0 +1,105 @@
+// RUN: %clang_analyze_cc1 -analyze -analyzer-checker=core,osx.cocoa.RetainCount -analyzer-config osx.cocoa.RetainCount:CheckOSObject=true -analyzer-output=text -verify %s
+
+struct OSObject {
+  virtual void retain();
+  virtual void release();
+
+  virtual ~OSObject(){}
+};
+
+struct OSArray : public OSObject {
+  unsigned int getCount();
+
+  static OSArray *withCapacity(unsigned int capacity);
+};
+
+void use_after_release() {
+  OSArray *arr = OSArray::withCapacity(10); // expected-note{{Call to function 'withCapacity' returns an OSObject of type struct OSArray * with a +1 retain count}}
+  arr->release(); // expected-note{{Object released}}
+  arr->getCount(); // expected-warning{{Reference-counted object is used after it is released}}
+                   // expected-note@-1{{Reference-counted object is used after it is released}}
+}
+
+void potential_leak() {
+  OSArray *arr = OSArray::withCapacity(10); // expected-note{{Call to function 'withCapacity' returns an OSObject of type struct OSArray * with a +1 retain count}}
+  arr->retain(); // expected-note{{Reference count incremented. The object now has a +2 retain count}}
+  arr->release(); // expected-note{{Reference count decremented. The object now has a +1 retain count}}
+  arr->getCount();
+} // expected-warning{{Potential leak of an object stored into 'arr'}}
+  // expected-note@-1{{Object leaked: object allocated and stored into 'arr' is not referenced later in this execution path and has a retain count of +1}}
+
+void proper_cleanup() {
+  OSArray *arr = OSArray::withCapacity(10); // +1
+  arr->retain(); // +2
+  arr->release(); // +1
+  arr->getCount();
+  arr->release(); // 0
+}
+
+struct ArrayOwner {
+  OSArray *arr;
+
+  OSArray *getArray() {
+    return arr;
+  }
+
+  OSArray *createArray() {
+    return OSArray::withCapacity(10);
+  }
+
+  OSArray *createArraySourceUnknown();
+
+  OSArray *getArraySourceUnknown();
+};
+
+//unsigned int leak_on_create_no_release(ArrayOwner *owner) {
+  //OSArray *myArray = 
+
+//}
+
+unsigned int no_warning_on_getter(ArrayOwner *owner) {
+  OSArray *arr = owner->getArray();
+  return arr->getCount();
+}
+
+unsigned int warn_on_overrelease(ArrayOwner *owner) {
+  OSArray *arr = owner->getArray(); // expected-note{{function call returns an OSObject of type struct OSArray * with a +0 retain count}}
+  arr->release(); // expected-warning{{Incorrect decrement of the reference count of an object that is not owned at this point by the caller}}
+                  // expected-note@-1{{Incorrect decrement of the reference count of an object that is not owned at this point by the caller}}
+  return arr->getCount();
+}
+
+unsigned int nowarn_on_release_of_created(ArrayOwner *owner) {
+  OSArray *arr = owner->createArray();
+  unsigned int out = arr->getCount();
+  arr->release();
+  return out;
+}
+
+unsigned int nowarn_on_release_of_created_source_unknown(ArrayOwner *owner) {
+  OSArray *arr = owner->createArraySourceUnknown();
+  unsigned int out = arr->getCount();
+  arr->release();
+  return out;
+}
+
+unsigned int no_warn_ok_release(ArrayOwner *owner) {
+  OSArray *arr = owner->getArray(); // +0
+  arr->retain(); // +1
+  arr->release(); // +0
+  return arr->getCount(); // no-warning
+}
+
+unsigned int warn_on_overrelease_with_unknown_source(ArrayOwner *owner) {
+  OSArray *arr = owner->getArraySourceUnknown(); // expected-note{{function call returns an OSObject of type struct OSArray * with a +0 retain count}}
+  arr->release(); // expected-warning{{Incorrect decrement of the reference count of an object that is not owned at this point by the caller}}
+                  // expected-note@-1{{Incorrect decrement of the reference count of an object that is not owned at this point by the caller}}
+  return arr->getCount();
+}
+
+unsigned int ok_release_with_unknown_source(ArrayOwner *owner) {
+  OSArray *arr = owner->getArraySourceUnknown(); // +0
+  arr->retain(); // +1
+  arr->release(); // +0
+  return arr->getCount();
+}