[analyzer] Malloc: A pointer might escape through CFContainers APIs,

funopen, setvbuf.

Teach the checker and the engine about these APIs to resolve malloc
false positives. As I am adding more of these APIs, it is clear that all
this should be factored out into a separate callback (for example,
region escapes). Malloc, KeyChainAPI and RetainRelease checkers could
all use it.

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

7 files changed, 164 insertions, 10 deletions

diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/ObjCMessage.h b/include/clang/StaticAnalyzer/Core/PathSensitive/ObjCMessage.h
index e87e08fe82..21f80cdc4a 100644
--- a/include/clang/StaticAnalyzer/Core/PathSensitive/ObjCMessage.h
+++ b/include/clang/StaticAnalyzer/Core/PathSensitive/ObjCMessage.h
@@ -21,9 +21,11 @@
 #include "clang/AST/ExprCXX.h"
 #include "clang/Basic/SourceManager.h"
 #include "llvm/ADT/PointerUnion.h"
+#include "llvm/ADT/StringExtras.h"
 
 namespace clang {
 namespace ento {
+using llvm::StrInStrNoCase;
 
 /// \brief Represents both explicit ObjC message expressions and implicit
 /// messages that are sent for handling properties in dot syntax.
@@ -254,6 +256,33 @@ public:
     assert(isObjCMessage());
     return Msg.getReceiverSourceRange();
   }
+
+  /// \brief Check if the name corresponds to a CoreFoundation or CoreGraphics 
+  /// function that allows objects to escape.
+  ///
+  /// Many methods allow a tracked object to escape.  For example:
+  ///
+  ///   CFMutableDictionaryRef x = CFDictionaryCreateMutable(..., customDeallocator);
+  ///   CFDictionaryAddValue(y, key, x);
+  ///
+  /// We handle this and similar cases with the following heuristic.  If the
+  /// function name contains "InsertValue", "SetValue", "AddValue",
+  /// "AppendValue", or "SetAttribute", then we assume that arguments may
+  /// escape.
+  //
+  // TODO: To reduce false negatives here, we should track the container
+  // allocation site and check if a proper deallocator was set there.
+  static bool isCFCGAllowingEscape(StringRef FName) {
+    if (FName[0] == 'C' && (FName[1] == 'F' || FName[1] == 'G'))
+           if (StrInStrNoCase(FName, "InsertValue") != StringRef::npos||
+               StrInStrNoCase(FName, "AddValue") != StringRef::npos ||
+               StrInStrNoCase(FName, "SetValue") != StringRef::npos ||
+               StrInStrNoCase(FName, "AppendValue") != StringRef::npos||
+               StrInStrNoCase(FName, "SetAttribute") != StringRef::npos) {
+         return true;
+       }
+    return false;
+  }
 };
 
 }
diff --git a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 007eba19ab..ae81ad6eda 100644
--- a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -42,10 +42,8 @@ public:
   RefState(Kind k, const Stmt *s) : K(k), S(s) {}
 
   bool isAllocated() const { return K == AllocateUnchecked; }
-  //bool isFailed() const { return K == AllocateFailed; }
   bool isReleased() const { return K == Released; }
-  //bool isEscaped() const { return K == Escaped; }
-  //bool isRelinquished() const { return K == Relinquished; }
+
   const Stmt *getStmt() const { return S; }
 
   bool operator==(const RefState &X) const {
@@ -1122,6 +1120,43 @@ bool MallocChecker::doesNotFreeMemory(const CallOrObjCMessage *Call,
       return false;
     }
 
+    // PR12101
+    // Many CoreFoundation and CoreGraphics might allow a tracked object 
+    // to escape.
+    if (Call->isCFCGAllowingEscape(FName))
+      return false;
+
+    // Associating streams with malloced buffers. The pointer can escape if
+    // 'closefn' is specified (and if that function does free memory).
+    // Currently, we do not inspect the 'closefn' function (PR12101).
+    if (FName == "funopen")
+      if (Call->getNumArgs() >= 4 && !Call->getArgSVal(4).isConstant(0))
+        return false;
+
+    // Do not warn on pointers passed to 'setbuf' when used with std streams,
+    // these leaks might be intentional when setting the buffer for stdio.
+    // http://stackoverflow.com/questions/2671151/who-frees-setvbuf-buffer
+    if (FName == "setbuf" || FName =="setbuffer" ||
+        FName == "setlinebuf" || FName == "setvbuf") {
+      if (Call->getNumArgs() >= 1)
+        if (const DeclRefExpr *Arg =
+              dyn_cast<DeclRefExpr>(Call->getArg(0)->IgnoreParenCasts()))
+          if (const VarDecl *D = dyn_cast<VarDecl>(Arg->getDecl()))
+              if (D->getCanonicalDecl()->getName().find("std")
+                                                   != StringRef::npos)
+                return false;
+    }
+
+    // A bunch of other functions, which take ownership of a pointer (See retain
+    // release checker). Not all the parameters here are invalidated, but the
+    // Malloc checker cannot differentiate between them. The right way of doing
+    // this would be to implement a pointer escapes callback.
+    if (FName == "CVPixelBufferCreateWithBytes" ||
+        FName == "CGBitmapContextCreateWithData" ||
+        FName == "CVPixelBufferCreateWithPlanarBytes") {
+      return false;
+    }
+
     // Otherwise, assume that the function does not free memory.
     // Most system calls, do not free the memory.
     return true;
diff --git a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
index 7b6e0d75d6..5e23d44bdc 100644
--- a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -197,10 +197,15 @@ static void findPtrToConstParams(llvm::SmallSet<unsigned, 1> &PreserveArgs,
       // value into thread local storage. The value can later be retrieved with
       // 'void *ptheread_getspecific(pthread_key)'. So even thought the
       // parameter is 'const void *', the region escapes through the call.
+      //  - funopen - sets a buffer for future IO calls.
       //  - ObjC functions that end with "NoCopy" can free memory, of the passed
       // in buffer.
+      // - Many CF containers allow objects to escape through custom
+      // allocators/deallocators upon container construction.
       if (FName == "pthread_setspecific" ||
-          FName.endswith("NoCopy"))
+          FName == "funopen" ||
+          FName.endswith("NoCopy") ||
+          Call.isCFCGAllowingEscape(FName))
         return;
     }
 
diff --git a/test/Analysis/malloc.c b/test/Analysis/malloc.c
index f9e3663494..bfe1befb53 100644
--- a/test/Analysis/malloc.c
+++ b/test/Analysis/malloc.c
@@ -677,7 +677,9 @@ void testStrdupContentIsDefined(const char *s, unsigned validIndex) {
   free(s2);
 }
 
+// ----------------------------------------------------------------------------
 // Test the system library functions to which the pointer can escape.
+// This tests false positive suppression.
 
 // For now, we assume memory passed to pthread_specific escapes.
 // TODO: We could check that if a new pthread binding is set, the existing
@@ -687,6 +689,46 @@ void testPthereadSpecificEscape(pthread_key_t key) {
   pthread_setspecific(key, buf); // no warning
 }
 
+// PR12101: Test funopen().
+static int releasePtr(void *_ctx) {
+    free(_ctx);
+    return 0;
+}
+FILE *useFunOpen() {
+    void *ctx = malloc(sizeof(int));
+    FILE *f = funopen(ctx, 0, 0, 0, releasePtr); // no warning
+    if (f == 0) {
+        free(ctx);
+    }
+    return f;
+}
+FILE *useFunOpenNoReleaseFunction() {
+    void *ctx = malloc(sizeof(int));
+    FILE *f = funopen(ctx, 0, 0, 0, 0);
+    if (f == 0) {
+        free(ctx);
+    }
+    return f; // expected-warning{{leak}}
+}
+
+// Test setbuf, setvbuf.
+int my_main_no_warning() {
+    char *p = malloc(100);
+    setvbuf(stdout, p, 0, 100);
+    return 0;
+}
+int my_main_no_warning2() {
+    char *p = malloc(100);
+    setbuf(__stdoutp, p);
+    return 0;
+}
+int my_main_warn(FILE *f) {
+    char *p = malloc(100);
+    setvbuf(f, p, 0, 100);
+    return 0;// expected-warning {{leak}}
+}
+
+// ----------------------------------------------------------------------------
 // Below are the known false positives.
 
 // TODO: There should be no warning here. This one might be difficult to get rid of.
@@ -706,6 +748,7 @@ void dependsOnValueOfPtr(int *g, unsigned f) {
   return;
 }
 
+// ----------------------------------------------------------------------------
 // False negatives.
 
 // TODO: This requires tracking symbols stored inside the structs/arrays.
diff --git a/test/Analysis/malloc.mm b/test/Analysis/malloc.mm
index 88739db0f9..38667fd3e1 100644
--- a/test/Analysis/malloc.mm
+++ b/test/Analysis/malloc.mm
@@ -62,8 +62,7 @@ void testNSDatafFreeWhenDoneFN(NSUInteger dataLength) {
   free(data); // false negative
 }
 
-// Test CF/NS...NoCopy. PR12100.
-
+// Test CF/NS...NoCopy. PR12100: Pointers can escape when custom deallocators are provided.
 void testNSDatafFreeWhenDone(NSUInteger dataLength) {
   CFStringRef str;
   char *bytes = (char*)malloc(12);
@@ -83,3 +82,12 @@ void stringWithExternalContentsExample(void) {
     CFRelease(mutStr);
     //free(myBuffer);
 }
+
+// PR12101 : pointers can escape through custom deallocators set on creation of a container.
+void TestCallbackReleasesMemory(CFDictionaryKeyCallBacks keyCallbacks) {
+  void *key = malloc(12);
+  void *val = malloc(12);
+  CFMutableDictionaryRef x = CFDictionaryCreateMutable(kCFAllocatorDefault, 0, &keyCallbacks, &kCFTypeDictionaryValueCallBacks);
+  CFDictionarySetValue(x, key, val); 
+  return;// no-warning
+}
diff --git a/test/Analysis/system-header-simulator-objc.h b/test/Analysis/system-header-simulator-objc.h
index f5b6333a8f..3fe21920ae 100644
--- a/test/Analysis/system-header-simulator-objc.h
+++ b/test/Analysis/system-header-simulator-objc.h
@@ -92,6 +92,19 @@ typedef double NSTimeInterval;
 - (id)initWithBytesNoCopy:(void *)bytes length:(NSUInteger)length freeWhenDone:(BOOL)b;
 @end
 
+typedef struct {
+}
+CFDictionaryKeyCallBacks;
+extern const CFDictionaryKeyCallBacks kCFTypeDictionaryKeyCallBacks;
+typedef struct {
+}
+CFDictionaryValueCallBacks;
+extern const CFDictionaryValueCallBacks kCFTypeDictionaryValueCallBacks;
+typedef const struct __CFDictionary * CFDictionaryRef;
+typedef struct __CFDictionary * CFMutableDictionaryRef;
+extern CFMutableDictionaryRef CFDictionaryCreateMutable(CFAllocatorRef allocator, CFIndex capacity, const CFDictionaryKeyCallBacks *keyCallBacks, const CFDictionaryValueCallBacks *valueCallBacks);
+void CFDictionarySetValue(CFMutableDictionaryRef, const void *, const void *);
+
 
 extern void CFRelease(CFTypeRef cf);
 
diff --git a/test/Analysis/system-header-simulator.h b/test/Analysis/system-header-simulator.h
index 472cb5a616..6212131071 100644
--- a/test/Analysis/system-header-simulator.h
+++ b/test/Analysis/system-header-simulator.h
@@ -2,16 +2,37 @@
 
 typedef struct _FILE FILE;
 extern FILE *stdin;
-int fscanf(FILE *restrict stream, const char *restrict format, ...);
+extern FILE *stdout;
+extern FILE *stderr;
+// Include a variant of standard streams that occur in the pre-processed file.
+extern FILE *__stdinp;
+extern FILE *__stdoutp;
+extern FILE *__stderrp;
+
+
+int fscanf(FILE *restrict, const char *restrict, ...);
 
 // Note, on some platforms errno macro gets replaced with a function call.
 extern int errno;
 
 unsigned long strlen(const char *);
 
-char *strcpy(char *restrict s1, const char *restrict s2);
+char *strcpy(char *restrict, const char *restrict);
 
 typedef unsigned long __darwin_pthread_key_t;
 typedef __darwin_pthread_key_t pthread_key_t;
-int pthread_setspecific(pthread_key_t ,
-         const void *);
+int pthread_setspecific(pthread_key_t, const void *);
+
+typedef long long __int64_t;
+typedef __int64_t __darwin_off_t;
+typedef __darwin_off_t fpos_t;
+
+void setbuf(FILE * restrict, char * restrict);
+int setvbuf(FILE * restrict, char * restrict, int, size_t);
+
+FILE *funopen(const void *,
+                 int (*)(void *, char *, int),
+                 int (*)(void *, const char *, int),
+                 fpos_t (*)(void *, fpos_t, int),
+                 int (*)(void *));
+