r345100 - [analyzer] Do not stop tracking CXX methods touching OSObject.

[George Karpenkov via cfe-commits]

Author: george.karpenkov
Date: Tue Oct 23 16:11:50 2018
New Revision: 345100

URL: http://llvm.org/viewvc/llvm-project?rev=345100&view=rev
Log:
[analyzer] Do not stop tracking CXX methods touching OSObject.

Trust generalized annotations for OSObject.

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

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=345100&r1=345099&r2=345100&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp Tue Oct 23 16:11:50 2018
@@ -45,7 +45,7 @@ ProgramStateRef removeRefBinding(Program
 
 void RefVal::print(raw_ostream &Out) const {
   if (!T.isNull())
-    Out << "Tracked " << T.getAsString() << '/';
+    Out << "Tracked " << T.getAsString() << " | ";
 
   switch (getKind()) {
     default: llvm_unreachable("Invalid RefVal kind");

Modified: cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp?rev=345100&r1=345099&r2=345100&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp Tue Oct 23 16:11:50 2018
@@ -8,8 +8,8 @@
 //===----------------------------------------------------------------------===//
 //
 //  This file defines summaries implementation for retain counting, which
-//  implements a reference count checker for Core Foundation and Cocoa
-//  on (Mac OS X).
+//  implements a reference count checker for Core Foundation, Cocoa
+//  and OSObject (on Mac OS X).
 //
 //===----------------------------------------------------------------------===//
 
@@ -94,6 +94,22 @@ static bool isMakeCollectable(StringRef
   return FName.contains_lower("MakeCollectable");
 }
 
+/// 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())
+      if (isOSObjectSubclass(RD))
+        return true;
+  }
+
+  return false;
+}
+
 const RetainSummary *
 RetainSummaryManager::generateSummary(const FunctionDecl *FD,
                                       bool &AllowAnnotations) {
@@ -322,12 +338,10 @@ RetainSummaryManager::generateSummary(co
     }
   }
 
-  if (isa<CXXMethodDecl>(FD)) {
-
-    // Stop tracking arguments passed to C++ methods, as those might be
-    // wrapping smart pointers.
-    return getPersistentSummary(RetEffect::MakeNoRet(), DoNothing, StopTracking,
-                                DoNothing);
+  if (const auto *MD = dyn_cast<CXXMethodDecl>(FD)) {
+    if (!(TrackOSObjects && isOSObjectRelated(MD)))
+      return getPersistentSummary(RetEffect::MakeNoRet(), DoNothing, StopTracking,
+                                  DoNothing);
   }
 
   return getDefaultSummary();
@@ -642,6 +656,8 @@ RetainSummaryManager::getRetEffectFromAn
 
   if (D->hasAttr<CFReturnsNotRetainedAttr>())
     return RetEffect::MakeNotOwned(RetEffect::CF);
+  else if (hasRCAnnotation(D, "rc_ownership_returns_not_retained"))
+    return RetEffect::MakeNotOwned(RetEffect::Generalized);
 
   return None;
 }

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=345100&r1=345099&r2=345100&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/osobject-retain-release.cpp (original)
+++ cfe/trunk/test/Analysis/osobject-retain-release.cpp Tue Oct 23 16:11:50 2018
@@ -2,6 +2,11 @@
 
 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 OSTypeID(type)   (type::metaClass)
 
 #define OSDynamicCast(type, inst)   \
@@ -21,14 +26,53 @@ struct OSArray : public OSObject {
   unsigned int getCount();
 
   static OSArray *withCapacity(unsigned int capacity);
+  static void consumeArray(OS_CONSUME OSArray * array);
+
+  static OSArray* consumeArrayHasCode(OS_CONSUME OSArray * array) {
+    return nullptr;
+  }
+
+  static OS_RETURNS_NOT_RETAINED OSArray *MaskedGetter();
+  static OS_RETURNS_RETAINED OSArray *getOoopsActuallyCreate();
+
 
   static const OSMetaClass * const metaClass;
 };
 
+struct OtherStruct {
+  static void doNothingToArray(OSArray *array);
+};
+
 struct OSMetaClassBase {
   static OSObject *safeMetaCast(const OSObject *inst, const OSMetaClass *meta);
 };
 
+void check_no_invalidation() {
+  OSArray *arr = OSArray::withCapacity(10); // expected-note{{Call to function 'withCapacity' returns an OSObject of type struct OSArray * with a +1 retain count}}
+  OtherStruct::doNothingToArray(arr);
+} // expected-warning{{Potential leak of an object stored into 'arr'}}
+  // expected-note at -1{{Object leaked}}
+
+void check_rc_consumed() {
+  OSArray *arr = OSArray::withCapacity(10);
+  OSArray::consumeArray(arr);
+}
+
+void check_rc_consume_temporary() {
+  OSArray::consumeArray(OSArray::withCapacity(10));
+}
+
+void check_rc_getter() {
+  OSArray *arr = OSArray::MaskedGetter();
+  (void)arr;
+}
+
+void check_rc_create() {
+  OSArray *arr = OSArray::getOoopsActuallyCreate();
+  arr->release();
+}
+
+
 void check_dynamic_cast() {
   OSArray *arr = OSDynamicCast(OSArray, OSObject::generateObject(1));
   arr->release();
@@ -80,11 +124,6 @@ struct ArrayOwner {
   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();