RetainCountChecker: for now, do not trust the summaries of inlined code

[george.karpenkov]

Summary

Trusting summaries of inlined code would require a more thorough work,
as the current approach was causing too many false positives, as the new example in test.
The culprit lies in the fact that we currently escape all variables written into a field (but not passed off to unknown functions!), which can result in inconsistent behavior.

rdar://45655344

[NoQ]

Oh well. I guess it was worth a try :)

lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp

if (SymbolRef Sym = MsgInvocation->getReceiverSVal().getAsLocSymbol()) {
       }
     }
   }
 
   // Consult the summary for the return value.
   RetEffect RE = Summ.getRetEffect();
 
   if (SymbolRef Sym = CallOrMsg.getReturnValue().getAsSymbol()) {
-    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)
       state = removeRefBinding(state, Sym);
   }
 
   C.addTransition(state);
 }
 
 static ProgramStateRef updateOutParameter(ProgramStateRef State,
RetainCountChecker::checkRegionChanges(ProgramStateRef state,
 
   llvm::SmallPtrSet<SymbolRef, 8> WhitelistedSymbols;
   for (ArrayRef<const MemRegion *>::iterator I = ExplicitRegions.begin(),
        E = ExplicitRegions.end(); I != E; ++I) {
     if (const SymbolicRegion *SR = (*I)->StripCasts()->getAs<SymbolicRegion>())
       WhitelistedSymbols.insert(SR->getSymbol());
   }
 
-  for (InvalidatedSymbols::const_iterator I=invalidated->begin(),
-       E = invalidated->end(); I!=E; ++I) {
-    SymbolRef sym = *I;
+  for (SymbolRef sym :
+       llvm::make_range(invalidated->begin(), invalidated->end())) {
     if (WhitelistedSymbols.count(sym))
       continue;
     // Remove any existing reference-count binding.
     state = removeRefBinding(state, sym);
   }
   return state;
 }
 

lib/StaticAnalyzer/Core/RetainSummaryManager.cpp

 
 /// A function is OSObject related if it is declared on a subclass
 /// of OSObject, or any of the parameters is a subclass of an OSObject.
 static bool isOSObjectRelated(const CXXMethodDecl *MD) {
   if (isOSObjectSubclass(MD->getParent()))
     return true;
 
   for (ParmVarDecl *Param : MD->parameters()) {
-    QualType PT = Param->getType();
-    if (CXXRecordDecl *RD = PT->getPointeeType()->getAsCXXRecordDecl())
+    QualType PT = Param->getType()->getPointeeType();
+    if (!PT.isNull())
+      if (CXXRecordDecl *RD = PT->getAsCXXRecordDecl())
         if (isOSObjectSubclass(RD))
           return true;
   }
 
   return false;
 }
 
 const RetainSummary *
 RetainSummaryManager::generateSummary(const FunctionDecl *FD,
                                       bool &AllowAnnotations) {

test/Analysis/osobject-retain-release.cpp

 // RUN: %clang_analyze_cc1 -analyze -analyzer-checker=core,osx.cocoa.RetainCount -analyzer-output=text -verify %s
 
 struct OSMetaClass;
 
-#define TRUSTED __attribute__((annotate("rc_ownership_trusted_implementation")))
-#define OS_CONSUME TRUSTED __attribute__((annotate("rc_ownership_consumed")))
-#define OS_RETURNS_RETAINED TRUSTED __attribute__((annotate("rc_ownership_returns_retained")))
-#define OS_RETURNS_NOT_RETAINED TRUSTED __attribute__((annotate("rc_ownership_returns_not_retained")))
+#define OS_CONSUME __attribute__((annotate("rc_ownership_consumed")))
+#define OS_RETURNS_RETAINED __attribute__((annotate("rc_ownership_returns_retained")))
+#define OS_RETURNS_NOT_RETAINED __attribute__((annotate("rc_ownership_returns_not_retained")))
 
 #define OSTypeID(type)   (type::metaClass)
 
 #define OSDynamicCast(type, inst)   \
     ((type *) OSMetaClassBase::safeMetaCast((inst), OSTypeID(type)))
 
 struct OSObject {
   virtual void retain();
void check_no_invalidation() {
   // expected-note@-1{{Object leaked}}
 
 void check_no_invalidation_other_struct() {
   OSArray *arr = OSArray::withCapacity(10); // expected-note{{Call to function 'withCapacity' returns an OSObject of type struct OSArray * with a +1 retain count}}
   OtherStruct other(arr); // expected-warning{{Potential leak}}
                           // expected-note@-1{{Object leaked}}
 }
 
+struct ArrayOwner : public OSObject {
+  OSArray *arr;
+  ArrayOwner(OSArray *arr) : arr(arr) {}
+
+  static ArrayOwner* create(OSArray *arr) {
+    return new ArrayOwner(arr);
+  }
+
+  OSArray *getArray() {
+    return arr;
+  }
+
+  OSArray *createArray() {
+    return OSArray::withCapacity(10);
+  }
+
+  OSArray *createArraySourceUnknown();
+
+  OSArray *getArraySourceUnknown();
+};
+
+void check_confusing_getters() {
+  OSArray *arr = OSArray::withCapacity(10);
+
+  ArrayOwner *AO = ArrayOwner::create(arr);
+  AO->getArray();
+
+  AO->release();
+  arr->release();
+}
+
 void check_rc_consumed() {
   OSArray *arr = OSArray::withCapacity(10);
   OSArray::consumeArray(arr);
 }
 
 void check_rc_consume_temporary() {
   OSArray::consumeArray(OSArray::withCapacity(10));
 }

 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 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}}
+  // FIXME: summaries are not applied in case the source of the getter/setter
+  // is known.
+  // rdar://45681203
+  OSArray *arr = owner->getArray();
+  arr->release();
   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;