[cfe-commits] r171770 - in /cfe/trunk: lib/StaticAnalyzer/Checkers/Checkers.td lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp test/Analysis/keychainAPI.m

[Anna Zaks]

Author: zaks
Date: Mon Jan  7 13:13:00 2013
New Revision: 171770

URL: http://llvm.org/viewvc/llvm-project?rev=171770&view=rev
Log:
[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.

Modified:
    cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td
    cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp
    cfe/trunk/test/Analysis/keychainAPI.m

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td?rev=171770&r1=171769&r2=171770&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td Mon Jan  7 13:13:00 2013
@@ -351,7 +351,7 @@
   HelpText<"Check for proper uses of Secure Keychain APIs">,
   DescFile<"MacOSKeychainAPIChecker.cpp">;
 
-} // end "macosx"
+} // end "osx"
 
 let ParentPackage = Cocoa in {
 

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp?rev=171770&r1=171769&r2=171770&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp Mon Jan  7 13:13:00 2013
@@ -393,16 +393,18 @@
           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

Modified: cfe/trunk/test/Analysis/keychainAPI.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/keychainAPI.m?rev=171770&r1=171769&r2=171770&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/keychainAPI.m (original)
+++ cfe/trunk/test/Analysis/keychainAPI.m Mon Jan  7 13:13:00 2013
@@ -305,6 +305,25 @@
   }
 }
 
+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;