[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
Jordy Rose
jediknil at belkadan.com
Thu May 3 20:44:51 PDT 2012
Some day we'll look inside callback blocks, too, but this is good. How about Cocoa methods with callback selectors, like -[NSOpenPanel beginSheet:modalForWindow:modalDelegate:didEndSelector:contextInfo:]? Then again, there's a fairly strong case against it at http://boredzo.org/blog/archives/2009-04-03/dont-pass-objects-as-context-pointers .
And we really do need a central place for pointer escape...
Jordy
On May 3, 2012, at 19:50, Anna Zaks wrote:
> 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*));
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list