]> granicus.if.org Git - clang/commitdiff
[analyzer] Allow pointers escape through calls containing callback args.
authorAnna Zaks <ganna@apple.com>
Thu, 3 May 2012 23:50:28 +0000 (23:50 +0000)
committerAnna Zaks <ganna@apple.com>
Thu, 3 May 2012 23:50:28 +0000 (23:50 +0000)
(Since we don't have a generic pointer escape callback, modify
ExprEngineCallAndReturn as well as the malloc checker.)

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

include/clang/StaticAnalyzer/Core/PathSensitive/ObjCMessage.h
lib/StaticAnalyzer/Checkers/MallocChecker.cpp
lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
lib/StaticAnalyzer/Core/ObjCMessage.cpp
test/Analysis/malloc.c
test/Analysis/malloc.cpp
test/Analysis/malloc.m
test/Analysis/malloc.mm
test/Analysis/system-header-simulator-objc.h
test/Analysis/system-header-simulator.h

index d8aec0991d9a9f5cf93c5156dd7eed96a1d325a6..0b260e4879678edc9e558d4f2ca4e6e95f6f35bc 100644 (file)
@@ -164,6 +164,9 @@ class CallOrObjCMessage {
   ObjCMessage Msg;
   ProgramStateRef State;
   const LocationContext *LCtx;
+
+  bool isCallbackArg(unsigned Idx, const Type *T) const;
+
 public:
   CallOrObjCMessage(const CallExpr *callE, ProgramStateRef state,
                     const LocationContext *lctx)
@@ -258,6 +261,10 @@ public:
     return Msg.getReceiverSourceRange();
   }
 
+  /// \brief Check if one of the arguments might be a callback.
+  bool hasNonZeroCallbackArg() const;
+
+
   /// \brief Check if the name corresponds to a CoreFoundation or CoreGraphics 
   /// function that allows objects to escape.
   ///
@@ -273,18 +280,7 @@ public:
   //
   // 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, "WithData") != StringRef::npos ||
-               StrInStrNoCase(FName, "AppendValue") != StringRef::npos||
-               StrInStrNoCase(FName, "SetAttribute") != StringRef::npos) {
-         return true;
-       }
-    return false;
-  }
+  static bool isCFCGAllowingEscape(StringRef FName);
 };
 
 }
index 55c32ec1ebbd3a532aa29b535f19e9c079e2d8a7..141518403344df3225d27489ad01d3d06cac6fba 100644 (file)
@@ -1285,6 +1285,11 @@ bool MallocChecker::doesNotFreeMemory(const CallOrObjCMessage *Call,
     if (FName.startswith("NS") && (FName.find("Insert") != StringRef::npos))
       return false;
 
+    // If the call has a callback as an argument, assume the memory
+    // can be freed.
+    if (Call->hasNonZeroCallbackArg())
+      return false;
+
     // Otherwise, assume that the function does not free memory.
     // Most system calls, do not free the memory.
     return true;
@@ -1312,6 +1317,11 @@ bool MallocChecker::doesNotFreeMemory(const CallOrObjCMessage *Call,
       return false;
     }
 
+    // If the call has a callback as an argument, assume the memory
+    // can be freed.
+    if (Call->hasNonZeroCallbackArg())
+      return false;
+
     // Otherwise, assume that the function does not free memory.
     // Most system calls, do not free the memory.
     return true;
index 4d0da06427588b47cb9cdd1a38a0855e579ffff2..963dc90cdfa588fcff5f8230c49905c5fc118983 100644 (file)
@@ -323,12 +323,14 @@ static void findPtrToConstParams(llvm::SmallSet<unsigned, 1> &PreserveArgs,
       // allocators/deallocators upon container construction.
       // - NSXXInsertXX, for example NSMapInsertIfAbsent, since they can
       // be deallocated by NSMapRemove.
+      // - Any call that has a callback as one of the arguments.
       if (FName == "pthread_setspecific" ||
           FName == "funopen" ||
           FName.endswith("NoCopy") ||
           (FName.startswith("NS") &&
             (FName.find("Insert") != StringRef::npos)) ||
-          Call.isCFCGAllowingEscape(FName))
+          Call.isCFCGAllowingEscape(FName) ||
+          Call.hasNonZeroCallbackArg())
         return;
     }
 
@@ -344,6 +346,10 @@ static void findPtrToConstParams(llvm::SmallSet<unsigned, 1> &PreserveArgs,
   if (const ObjCMethodDecl *MDecl = dyn_cast<ObjCMethodDecl>(CallDecl)) {
     assert(MDecl->param_size() <= Call.getNumArgs());
     unsigned Idx = 0;
+
+    if (Call.hasNonZeroCallbackArg())
+      return;
+
     for (clang::ObjCMethodDecl::param_const_iterator
          I = MDecl->param_begin(), E = MDecl->param_end(); I != E; ++I, ++Idx) {
       if (isPointerToConst(*I))
index 65cdcd9d992459e785f1813e5cd695cc0f25d4e0..dc24e818ffa92ba4fccfbd0e3ddc5768bfdd0afa 100644 (file)
@@ -88,3 +88,69 @@ const Decl *CallOrObjCMessage::getDecl() const {
   return 0;
 }
 
+bool CallOrObjCMessage::isCallbackArg(unsigned Idx, const Type *T) const {
+  // Should we dig into struct fields, arrays ect?
+  if (T->isBlockPointerType() || T->isFunctionPointerType())
+    if (!getArgSVal(Idx).isZeroConstant())
+      return true;
+  return false;
+}
+
+bool CallOrObjCMessage::hasNonZeroCallbackArg() const {
+  unsigned NumOfArgs = getNumArgs();
+
+  // Process ObjC message first.
+  if (!CallE) {
+    const ObjCMethodDecl *D = Msg.getMethodDecl();
+    unsigned Idx = 0;
+    for (ObjCMethodDecl::param_const_iterator I = D->param_begin(),
+                                     E = D->param_end(); I != E; ++I, ++Idx) {
+      if (NumOfArgs <= Idx)
+        break;
+
+      if (isCallbackArg(Idx, (*I)->getType().getTypePtr()))
+        return true;
+    }
+    return false;
+  }
+
+  // Else, assume we are dealing with a Function call.
+  const FunctionDecl *FD = 0;
+  if (const CXXConstructExpr *Ctor =
+        CallE.dyn_cast<const CXXConstructExpr *>())
+    FD = Ctor->getConstructor();
+
+  const CallExpr * CE = CallE.get<const CallExpr *>();
+  FD = dyn_cast<FunctionDecl>(CE->getCalleeDecl());
+
+  // If calling using a function pointer, assume the function does not
+  // have a callback. TODO: We could check the types of the arguments here.
+  if (!FD)
+    return false;
+
+  unsigned Idx = 0;
+  for (FunctionDecl::param_const_iterator I = FD->param_begin(),
+                                      E = FD->param_end(); I != E; ++I, ++Idx) {
+    if (NumOfArgs <= Idx)
+      break;
+
+    if (isCallbackArg(Idx, (*I)->getType().getTypePtr()))
+      return true;
+  }
+  return false;
+}
+
+bool CallOrObjCMessage::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, "WithData") != StringRef::npos ||
+             StrInStrNoCase(FName, "AppendValue") != StringRef::npos||
+             StrInStrNoCase(FName, "SetAttribute") != StringRef::npos) {
+       return true;
+     }
+  return false;
+}
+
+
index 9c09051c31b49b17e7f53bd90c1b97a8946dae0d..f5bff4fa43b1326fa8bc1f5951e967cf85f56821 100644 (file)
@@ -798,6 +798,27 @@ void radar_11358224_test_double_assign_ints_positive_2()
   ptr = ptr; // expected-warning {{leak}}
 }
 
+// Assume that functions which take a function pointer can free memory even if
+// they are defined in system headers and take the const pointer to the
+// allocated memory. (radar://11160612)
+int const_ptr_and_callback(int, const char*, int n, void(*)(void*));
+void r11160612_1() {
+  char *x = malloc(12);
+  const_ptr_and_callback(0, x, 12, free); // no - warning
+}
+
+// Null is passed as callback.
+void r11160612_2() {
+  char *x = malloc(12);
+  const_ptr_and_callback(0, x, 12, 0); // expected-warning {{leak}}
+}
+
+// Callback is passed to a function defined in a system header.
+void r11160612_4() {
+  char *x = malloc(12);
+  sqlite3_bind_text_my(0, x, 12, free); // no - warning
+}
+
 // ----------------------------------------------------------------------------
 // Below are the known false positives.
 
index 8f80b2b76f2938ef0c5e02cc310033252a4cf3a3..f36d8fcb7b24deba4e2a17fbab477a944d727011 100644 (file)
@@ -14,3 +14,13 @@ struct Foo {
 Foo aFunction() {
     return malloc(10);
 }
+
+// Assume that functions which take a function pointer can free memory even if
+// they are defined in system headers and take the const pointer to the
+// allocated memory. (radar://11160612)
+// Test default parameter.
+int const_ptr_and_callback_def_param(int, const char*, int n, void(*)(void*) = 0);
+void r11160612_3() {
+  char *x = (char*)malloc(12);
+  const_ptr_and_callback_def_param(0, x, 12);
+}
index 6c94118286ab32b60d721c919ceee3ba50e27e51..08206f3bf14ccfc9022702953d9c5c35693483bc 100644 (file)
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc -analyzer-store=region -verify -Wno-objc-root-class %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc -analyzer-store=region -verify -Wno-objc-root-class -fblocks %s
 #include "system-header-simulator-objc.h"
 
 @class NSString;
index 3515a4f99af0000f51427c0dea890ad1e125c97d..ef1adda5a128c6f97874f72708c47b1b74e751d2 100644 (file)
@@ -153,4 +153,29 @@ static void releaseDataCallback (void *info, const void *data, size_t size) {
 void testCGDataProviderCreateWithData() { 
   void* b = calloc(8, 8);
   CGDataProviderRef p = CGDataProviderCreateWithData(0, b, 8*8, releaseDataCallback);
+}
+
+// Assume that functions which take a function pointer can free memory even if
+// they are defined in system headers and take the const pointer to the
+// allocated memory. (radar://11160612)
+extern CGDataProviderRef UnknownFunWithCallback(void *info,
+    const void *data, size_t size,
+    CGDataProviderReleaseDataCallback releaseData)
+    __attribute__((visibility("default")));
+void testUnknownFunWithCallBack() { 
+  void* b = calloc(8, 8);
+  CGDataProviderRef p = UnknownFunWithCallback(0, b, 8*8, releaseDataCallback);
+}
+
+// Test blocks.
+void acceptBlockParam(void *, void (^block)(void *), unsigned);
+void testCallWithBlockCallback() {
+  void *l = malloc(12);
+  acceptBlockParam(l, ^(void *i) { free(i); }, sizeof(char *));
+}
+
+// Test blocks in system headers.
+void testCallWithBlockCallbackInSystem() {
+  void *l = malloc(12);
+  SystemHeaderFunctionWithBlockParam(l, ^(void *i) { free(i); }, sizeof(char *));
 }
\ No newline at end of file
index 92d5899abf8d910bdec47daf99d07aed52721b23..4626a4ec4f5b1ef48a3a2677acc8db92f01f04a1 100644 (file)
@@ -112,3 +112,5 @@ extern void CFRelease(CFTypeRef cf);
 extern CFMutableStringRef CFStringCreateMutableWithExternalCharactersNoCopy(CFAllocatorRef alloc, UniChar *chars, CFIndex numChars, CFIndex capacity, CFAllocatorRef externalCharactersAllocator);
 extern CFStringRef CFStringCreateWithCStringNoCopy(CFAllocatorRef alloc, const char *cStr, CFStringEncoding encoding, CFAllocatorRef contentsDeallocator);
 extern void CFStringAppend(CFMutableStringRef theString, CFStringRef appendedString);
+
+void SystemHeaderFunctionWithBlockParam(void *, void (^block)(void *), unsigned);
index 6212131071de850a80d177bd0739d551fe411d29..c910ad9694c44b7bdbc0f396d3b691fcfc7cbacf 100644 (file)
@@ -36,3 +36,4 @@ FILE *funopen(const void *,
                  fpos_t (*)(void *, fpos_t, int),
                  int (*)(void *));
 
+int sqlite3_bind_text_my(int, const char*, int n, void(*)(void*));