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

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

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

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@345746 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp b/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
index 7db1465fa1b2c580cdde990d1000871841bb5af4..3431b1554a1d35aec1047be7b88ead6f7d54d8f8 100644 (file)
--- a/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
@@ -420,13 +420,6 @@ void RetainCountChecker::processSummaryOfInlined(const RetainSummary &Summ,
   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);
   }
@@ -1103,9 +1096,8 @@ RetainCountChecker::checkRegionChanges(ProgramStateRef state,
       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.

diff --git a/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp b/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp
index efaab64c77038367ef987492f3ee31fc9601abc3..b0e26bae961e1c93a774a143ceac1688c38075f8 100644 (file)
--- a/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp
+++ b/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp
@@ -101,10 +101,11 @@ static bool isOSObjectRelated(const CXXMethodDecl *MD) {
     return true;
 
   for (ParmVarDecl *Param : MD->parameters()) {
-    QualType PT = Param->getType();
-    if (CXXRecordDecl *RD = PT->getPointeeType()->getAsCXXRecordDecl())
-      if (isOSObjectSubclass(RD))
-        return true;
+    QualType PT = Param->getType()->getPointeeType();
+    if (!PT.isNull())
+      if (CXXRecordDecl *RD = PT->getAsCXXRecordDecl())
+        if (isOSObjectSubclass(RD))
+          return true;
   }
 
   return false;

diff --git a/test/Analysis/osobject-retain-release.cpp b/test/Analysis/osobject-retain-release.cpp
index 1159eaed0d8bd1ab5895c158f0619e5285bf7b7b..47601b0bada3e162176f6148d7ddd7a2240004be 100644 (file)
--- a/test/Analysis/osobject-retain-release.cpp
+++ b/test/Analysis/osobject-retain-release.cpp
@@ -2,10 +2,9 @@
 
 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)
 
@@ -62,6 +61,37 @@ void check_no_invalidation_other_struct() {
                           // 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);
@@ -140,31 +170,17 @@ void proper_cleanup() {
   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();
 }