r345099 - [analyzer] Trust summaries for OSObject::retain and OSObject::release

[George Karpenkov via cfe-commits]

Author: george.karpenkov
Date: Tue Oct 23 16:11:30 2018
New Revision: 345099

URL: http://llvm.org/viewvc/llvm-project?rev=345099&view=rev
Log:
[analyzer] Trust summaries for OSObject::retain and OSObject::release

Refactor the way in which summaries are consumed for safeMetaCast

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

Modified:
    cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp
    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=345099&r1=345098&r2=345099&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp Tue Oct 23 16:11:30 2018
@@ -776,31 +776,27 @@ bool RetainCountChecker::evalCall(const
 
   const LocationContext *LCtx = C.getLocationContext();
 
-  // Process OSDynamicCast: should just return the first argument.
-  // For now, tresting the cast as a no-op, and disregarding the case where
-  // the output becomes null due to the type mismatch.
-  if (FD->getNameAsString() == "safeMetaCast") {
-    state = state->BindExpr(CE, LCtx, 
-                            state->getSVal(CE->getArg(0), LCtx));
-    C.addTransition(state);
-    return true;
-  }
-
   // See if it's one of the specific functions we know how to eval.
   if (!SmrMgr.canEval(CE, FD, hasTrustedImplementationAnnotation))
     return false;
 
   // Bind the return value.
-  SVal RetVal = state->getSVal(CE->getArg(0), LCtx);
-  if (RetVal.isUnknown() ||
-      (hasTrustedImplementationAnnotation && !ResultTy.isNull())) {
+  // For now, all the functions which we can evaluate and which take
+  // at least one argument are identities.
+  if (CE->getNumArgs() >= 1) {
+    SVal RetVal = state->getSVal(CE->getArg(0), LCtx);
+
     // If the receiver is unknown or the function has
     // 'rc_ownership_trusted_implementation' annotate attribute, conjure a
     // return value.
-    SValBuilder &SVB = C.getSValBuilder();
-    RetVal = SVB.conjureSymbolVal(nullptr, CE, LCtx, ResultTy, C.blockCount());
+    if (RetVal.isUnknown() ||
+        (hasTrustedImplementationAnnotation && !ResultTy.isNull())) {
+      SValBuilder &SVB = C.getSValBuilder();
+      RetVal =
+          SVB.conjureSymbolVal(nullptr, CE, LCtx, ResultTy, C.blockCount());
+    }
+    state = state->BindExpr(CE, LCtx, RetVal, false);
   }
-  state = state->BindExpr(CE, LCtx, RetVal, false);
 
   C.addTransition(state);
   return true;

Modified: cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp?rev=345099&r1=345098&r2=345099&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp Tue Oct 23 16:11:30 2018
@@ -102,9 +102,6 @@ RetainSummaryManager::generateSummary(co
     return getPersistentStopSummary();
   }
 
-  // [PR 3337] Use 'getAs<FunctionType>' to strip away any typedefs on the
-  // function's type.
-  const FunctionType *FT = FD->getType()->getAs<FunctionType>();
   const IdentifierInfo *II = FD->getIdentifier();
   if (!II)
     return getDefaultSummary();
@@ -115,7 +112,8 @@ RetainSummaryManager::generateSummary(co
   // down below.
   FName = FName.substr(FName.find_first_not_of('_'));
 
-  // Inspect the result type.
+  // Inspect the result type. Strip away any typedefs.
+  const auto *FT = FD->getType()->getAs<FunctionType>();
   QualType RetTy = FT->getReturnType();
   std::string RetTyName = RetTy.getAsString();
 
@@ -506,12 +504,6 @@ bool RetainSummaryManager::isTrustedRefe
 bool RetainSummaryManager::canEval(const CallExpr *CE,
                                    const FunctionDecl *FD,
                                    bool &hasTrustedImplementationAnnotation) {
-  // For now, we're only handling the functions that return aliases of their
-  // arguments: CFRetain (and its families).
-  // Eventually we should add other functions we can model entirely,
-  // such as CFRelease, which don't invalidate their arguments or globals.
-  if (CE->getNumArgs() != 1)
-    return false;
 
   IdentifierInfo *II = FD->getIdentifier();
   if (!II)
@@ -533,6 +525,13 @@ bool RetainSummaryManager::canEval(const
       return isRetain(FD, FName) || isAutorelease(FD, FName) ||
              isMakeCollectable(FName);
 
+    // Process OSDynamicCast: should just return the first argument.
+    // For now, treating the cast as a no-op, and disregarding the case where
+    // the output becomes null due to the type mismatch.
+    if (TrackOSObjects && FName == "safeMetaCast") {
+      return true;
+    }
+
     const FunctionDecl* FDD = FD->getDefinition();
     if (FDD && isTrustedReferenceCountImplementation(FDD)) {
       hasTrustedImplementationAnnotation = true;
@@ -540,6 +539,12 @@ bool RetainSummaryManager::canEval(const
     }
   }
 
+  if (const auto *MD = dyn_cast<CXXMethodDecl>(FD)) {
+    const CXXRecordDecl *Parent = MD->getParent();
+    if (TrackOSObjects && Parent && isOSObjectSubclass(Parent))
+      return FName == "release" || FName == "retain";
+  }
+
   return false;
 
 }

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=345099&r1=345098&r2=345099&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/osobject-retain-release.cpp (original)
+++ cfe/trunk/test/Analysis/osobject-retain-release.cpp Tue Oct 23 16:11:30 2018
@@ -9,7 +9,7 @@ struct OSMetaClass;
 
 struct OSObject {
   virtual void retain();
-  virtual void release();
+  virtual void release() {};
   virtual ~OSObject(){}
 
   static OSObject *generateObject(int);