[analyzer] Check that the argument to CFMakeCollectable is non-NULL.

Patch by Sean McBride!

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

6 files changed, 37 insertions, 19 deletions

diff --git a/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp b/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
index 0dd64780f1..eba534e08f 100644
--- a/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
+++ b/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
@@ -363,15 +363,15 @@ void CFNumberCreateChecker::checkPreStmt(const CallExpr *CE,
 }
 
 //===----------------------------------------------------------------------===//
-// CFRetain/CFRelease checking for null arguments.
+// CFRetain/CFRelease/CFMakeCollectable checking for null arguments.
 //===----------------------------------------------------------------------===//
 
 namespace {
 class CFRetainReleaseChecker : public Checker< check::PreStmt<CallExpr> > {
   mutable OwningPtr<APIMisuse> BT;
-  mutable IdentifierInfo *Retain, *Release;
+  mutable IdentifierInfo *Retain, *Release, *MakeCollectable;
 public:
-  CFRetainReleaseChecker(): Retain(0), Release(0) {}
+  CFRetainReleaseChecker(): Retain(0), Release(0), MakeCollectable(0) {}
   void checkPreStmt(const CallExpr *CE, CheckerContext &C) const;
 };
 } // end anonymous namespace
@@ -392,12 +392,14 @@ void CFRetainReleaseChecker::checkPreStmt(const CallExpr *CE,
     ASTContext &Ctx = C.getASTContext();
     Retain = &Ctx.Idents.get("CFRetain");
     Release = &Ctx.Idents.get("CFRelease");
-    BT.reset(new APIMisuse("null passed to CFRetain/CFRelease"));
+    MakeCollectable = &Ctx.Idents.get("CFMakeCollectable");
+    BT.reset(
+      new APIMisuse("null passed to CFRetain/CFRelease/CFMakeCollectable"));
   }
 
-  // Check if we called CFRetain/CFRelease.
+  // Check if we called CFRetain/CFRelease/CFMakeCollectable.
   const IdentifierInfo *FuncII = FD->getIdentifier();
-  if (!(FuncII == Retain || FuncII == Release))
+  if (!(FuncII == Retain || FuncII == Release || FuncII == MakeCollectable))
     return;
 
   // FIXME: The rest of this just checks that the argument is non-null.
@@ -426,9 +428,15 @@ void CFRetainReleaseChecker::checkPreStmt(const CallExpr *CE,
     if (!N)
       return;
 
-    const char *description = (FuncII == Retain)
-                            ? "Null pointer argument in call to CFRetain"
-                            : "Null pointer argument in call to CFRelease";
+    const char *description;
+    if (FuncII == Retain)
+      description = "Null pointer argument in call to CFRetain";
+    else if (FuncII == Release)
+      description = "Null pointer argument in call to CFRelease";
+    else if (FuncII == MakeCollectable)
+      description = "Null pointer argument in call to CFMakeCollectable";
+    else
+      llvm_unreachable("impossible case");
 
     BugReport *report = new BugReport(*BT, description, N);
     report->addRange(Arg->getSourceRange());
diff --git a/lib/StaticAnalyzer/Checkers/Checkers.td b/lib/StaticAnalyzer/Checkers/Checkers.td
index b4b40fae2b..235e63306f 100644
--- a/lib/StaticAnalyzer/Checkers/Checkers.td
+++ b/lib/StaticAnalyzer/Checkers/Checkers.td
@@ -433,7 +433,7 @@ def CFNumberCreateChecker : Checker<"CFNumber">,
   DescFile<"BasicObjCFoundationChecks.cpp">;
 
 def CFRetainReleaseChecker : Checker<"CFRetainRelease">,
-  HelpText<"Check for null arguments to CFRetain/CFRelease">,
+  HelpText<"Check for null arguments to CFRetain/CFRelease/CFMakeCollectable">,
   DescFile<"BasicObjCFoundationChecks.cpp">;
 
 def CFErrorChecker : Checker<"CFError">,
diff --git a/test/Analysis/diagnostics/undef-value-param.m b/test/Analysis/diagnostics/undef-value-param.m
index ae82839c32..d2a7a087b8 100644
--- a/test/Analysis/diagnostics/undef-value-param.m
+++ b/test/Analysis/diagnostics/undef-value-param.m
@@ -460,7 +460,7 @@ static void CreateRef(SCDynamicStoreRef *storeRef, unsigned x) {
 //CHECK:     </array>
 //CHECK:     <key>description</key><string>Null pointer argument in call to CFRelease</string>
 //CHECK:     <key>category</key><string>API Misuse (Apple)</string>
-//CHECK:     <key>type</key><string>null passed to CFRetain/CFRelease</string>
+//CHECK:     <key>type</key><string>null passed to CFRetain/CFRelease/CFMakeCollectable</string>
 //CHECK:    <key>issue_context_kind</key><string>Objective-C method</string>
 //CHECK:    <key>issue_context</key><string>test</string>
 //CHECK:    <key>issue_hash</key><integer>5</integer>
diff --git a/test/Analysis/retain-release.m b/test/Analysis/retain-release.m
index 514bd1208e..eb2554f889 100644
--- a/test/Analysis/retain-release.m
+++ b/test/Analysis/retain-release.m
@@ -62,6 +62,7 @@ typedef const struct __CFAllocator * CFAllocatorRef;
 extern const CFAllocatorRef kCFAllocatorDefault;
 extern CFTypeRef CFRetain(CFTypeRef cf);
 extern void CFRelease(CFTypeRef cf);
+extern CFTypeRef CFMakeCollectable(CFTypeRef cf);
 typedef struct {
 }
 CFArrayCallBacks;
@@ -508,31 +509,39 @@ void f15() {
   CFRelease(*B);  // no-warning
 }
 
-// Test when we pass NULL to CFRetain/CFRelease.
+// Test when we pass NULL to CFRetain/CFRelease/CFMakeCollectable.
 void f16(int x, CFTypeRef p) {
   if (p)
     return;
 
-  if (x) {
+  if (x > 0) {
     CFRelease(p); // expected-warning{{Null pointer argument in call to CFRelease}}
   }
-  else {
+  else if (x < 0) {
     CFRetain(p); // expected-warning{{Null pointer argument in call to CFRetain}}
   }
+  else {
+    CFMakeCollectable(p); // expected-warning{{Null pointer argument in call to CFMakeCollectable}}
+  }
 }
 
 // Test that an object is non-null after being CFRetained/CFReleased.
 void f17(int x, CFTypeRef p) {
-  if (x) {
+  if (x > 0) {
     CFRelease(p);
     if (!p)
       CFRelease(0); // no-warning
   }
-  else {
+  else if (x < 0) {
     CFRetain(p);
     if (!p)
       CFRetain(0); // no-warning
   }
+  else {
+    CFMakeCollectable(p);
+    if (!p)
+      CFMakeCollectable(0); // no-warning
+  }
 }
 
 // Test basic tracking of ivars associated with 'self'.  For the retain/release
diff --git a/tools/scan-build/scan-build.1 b/tools/scan-build/scan-build.1
index e53acdb755..10ddc7fafb 100644
--- a/tools/scan-build/scan-build.1
+++ b/tools/scan-build/scan-build.1
@@ -270,9 +270,10 @@ Check for proper uses of
 .Fn CFNumberCreate .
 .It osx.coreFoundation.CFRetainRelease
 Check for null arguments to
-.Fn CFRetain
+.Fn CFRetain ,
+.Fn CFRelease ,
 and
-.Fn CFRelease .
+.Fn CFMakeCollectable .
 .It osx.coreFoundation.containers.OutOfBounds
 Checks for index out-of-bounds when using the
 .Vt CFArray
diff --git a/www/analyzer/available_checks.html b/www/analyzer/available_checks.html
index 12d836c483..4f8971c550 100644
--- a/www/analyzer/available_checks.html
+++ b/www/analyzer/available_checks.html
@@ -125,7 +125,7 @@
 <td><b>osx.coreFoundation.CFNumber</b></td><td>Check for proper uses of CFNumberCreate.</td>
 </tr>
 <tr>
-<td><b>osx.coreFoundation.CFRetainRelease</b></td><td>Check for null arguments to CFRetain/CFRelease.</td>
+<td><b>osx.coreFoundation.CFRetainRelease</b></td><td>Check for null arguments to CFRetain/CFRelease/CFMakeCollectable.</td>
 </tr>
 <tr>
 <td><b>osx.coreFoundation.containers.OutOfBounds</b></td><td>Checks for index out-of-bounds when using 'CFArray' API.</td>