[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
Wed May 9 10:52:25 PDT 2012
I added the selectors as callbacks (though which pointers are assumed to escape) in r156481. Relieving people of false positives due to common use patterns of ObjC APIs wins over possible design issues ...
Thanks Jordy,
Anna.
On May 3, 2012, at 8:44 PM, Jordy Rose wrote:
> 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