[cfe-commits] r156134 - in /cfe/trunk: 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
Anna Zaks
ganna at apple.com
Thu May 3 16:50:28 PDT 2012
Author: zaks
Date: Thu May 3 18:50:28 2012
New Revision: 156134
URL: http://llvm.org/viewvc/llvm-project?rev=156134&view=rev
Log:
[analyzer] Allow pointers escape through calls containing callback args.
(Since we don't have a generic pointer escape callback, modify
ExprEngineCallAndReturn as well as the malloc checker.)
Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ObjCMessage.h
cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ObjCMessage.cpp
cfe/trunk/test/Analysis/malloc.c
cfe/trunk/test/Analysis/malloc.cpp
cfe/trunk/test/Analysis/malloc.m
cfe/trunk/test/Analysis/malloc.mm
cfe/trunk/test/Analysis/system-header-simulator-objc.h
cfe/trunk/test/Analysis/system-header-simulator.h
Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ObjCMessage.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ObjCMessage.h?rev=156134&r1=156133&r2=156134&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ObjCMessage.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ObjCMessage.h Thu May 3 18:50:28 2012
@@ -164,6 +164,9 @@
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 @@
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 @@
//
// 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);
};
}
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp?rev=156134&r1=156133&r2=156134&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp Thu May 3 18:50:28 2012
@@ -1285,6 +1285,11 @@
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 @@
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;
Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp?rev=156134&r1=156133&r2=156134&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp Thu May 3 18:50:28 2012
@@ -323,12 +323,14 @@
// 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 @@
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))
Modified: cfe/trunk/lib/StaticAnalyzer/Core/ObjCMessage.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ObjCMessage.cpp?rev=156134&r1=156133&r2=156134&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ObjCMessage.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ObjCMessage.cpp Thu May 3 18:50:28 2012
@@ -88,3 +88,69 @@
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;
+}
+
+
Modified: cfe/trunk/test/Analysis/malloc.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/malloc.c?rev=156134&r1=156133&r2=156134&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/malloc.c (original)
+++ cfe/trunk/test/Analysis/malloc.c Thu May 3 18:50:28 2012
@@ -798,6 +798,27 @@
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.
Modified: cfe/trunk/test/Analysis/malloc.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/malloc.cpp?rev=156134&r1=156133&r2=156134&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/malloc.cpp (original)
+++ cfe/trunk/test/Analysis/malloc.cpp Thu May 3 18:50:28 2012
@@ -14,3 +14,13 @@
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);
+}
Modified: cfe/trunk/test/Analysis/malloc.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/malloc.m?rev=156134&r1=156133&r2=156134&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/malloc.m (original)
+++ cfe/trunk/test/Analysis/malloc.m Thu May 3 18:50:28 2012
@@ -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;
Modified: cfe/trunk/test/Analysis/malloc.mm
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/malloc.mm?rev=156134&r1=156133&r2=156134&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/malloc.mm (original)
+++ cfe/trunk/test/Analysis/malloc.mm Thu May 3 18:50:28 2012
@@ -153,4 +153,29 @@
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
Modified: cfe/trunk/test/Analysis/system-header-simulator-objc.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/system-header-simulator-objc.h?rev=156134&r1=156133&r2=156134&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/system-header-simulator-objc.h (original)
+++ cfe/trunk/test/Analysis/system-header-simulator-objc.h Thu May 3 18:50:28 2012
@@ -112,3 +112,5 @@
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);
Modified: cfe/trunk/test/Analysis/system-header-simulator.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/system-header-simulator.h?rev=156134&r1=156133&r2=156134&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/system-header-simulator.h (original)
+++ cfe/trunk/test/Analysis/system-header-simulator.h Thu May 3 18:50:28 2012
@@ -36,3 +36,4 @@
fpos_t (*)(void *, fpos_t, int),
int (*)(void *));
+int sqlite3_bind_text_my(int, const char*, int n, void(*)(void*));
More information about the cfe-commits
mailing list