[analyzer] Fix a false positive in Secure Keychain API checker.

Better handle the blacklisting of known bad deallocators when symbol
escapes through a call to CFStringCreateWithBytesNoCopy.

Addresses radar://12702952.

llvm-svn: 171770

3 files changed, 29 insertions, 8 deletions

diff --git a/clang/lib/StaticAnalyzer/Checkers/Checkers.td b/clang/lib/StaticAnalyzer/Checkers/Checkers.td
index ef9e17d4783..f4ea9ebbad9 100644
--- a/clang/lib/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/lib/StaticAnalyzer/Checkers/Checkers.td
@@ -351,7 +351,7 @@ def MacOSKeychainAPIChecker : Checker<"SecKeychainAPI">,
   HelpText<"Check for proper uses of Secure Keychain APIs">,
   DescFile<"MacOSKeychainAPIChecker.cpp">;
 
-} // end "macosx"
+} // end "osx"
 
 let ParentPackage = Cocoa in {
 
diff --git a/clang/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp
index bb5d4f66f20..b899b6f9b74 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp
@@ -393,16 +393,18 @@ void MacOSKeychainAPIChecker::checkPreStmt(const CallExpr *CE,
           return;
         }
         // If kCFAllocatorNull, which does not deallocate, we still have to
-        // find the deallocator. Otherwise, assume that the user had written a
-        // custom deallocator which does the right thing.
-        if (DE->getFoundDecl()->getName() != "kCFAllocatorNull") {
-          State = State->remove<AllocatedData>(ArgSM);
-          C.addTransition(State);
+        // find the deallocator.
+        if (DE->getFoundDecl()->getName() == "kCFAllocatorNull")
           return;
-        }
       }
+      // In all other cases, assume the user supplied a correct deallocator
+      // that will free memory so stop tracking.
+      State = State->remove<AllocatedData>(ArgSM);
+      C.addTransition(State);
+      return;
     }
-    return;
+
+    llvm_unreachable("We know of no other possible APIs.");
   }
 
   // The call is deallocating a value we previously allocated, so remove it
diff --git a/clang/test/Analysis/keychainAPI.m b/clang/test/Analysis/keychainAPI.m
index 6eca8003d93..4fc48c066f9 100644
--- a/clang/test/Analysis/keychainAPI.m
+++ b/clang/test/Analysis/keychainAPI.m
@@ -305,6 +305,25 @@ void DellocWithCFStringCreate4(CFAllocatorRef alloc) {
   }
 }
 
+static CFAllocatorRef gKeychainDeallocator = 0;
+
+static CFAllocatorRef GetKeychainDeallocator() {  
+  return gKeychainDeallocator;
+}
+
+CFStringRef DellocWithCFStringCreate5(CFAllocatorRef alloc) {
+  unsigned int *ptr = 0;
+  OSStatus st = 0;
+  UInt32 length;
+  void *bytes;
+  char * x;
+  st = SecKeychainItemCopyContent(2, ptr, ptr, &length, &bytes);
+  if (st == noErr) {
+    return CFStringCreateWithBytesNoCopy(alloc, bytes, length, 5, 0, GetKeychainDeallocator()); // no-warning
+  }
+  return 0;
+}
+
 void radar10508828() {
   UInt32 pwdLen = 0;
   void*  pwdBytes = 0;