From: Anna Zaks Date: Fri, 31 May 2013 23:47:32 +0000 (+0000) Subject: [analyzer] Malloc checker should only escape the receiver when “[O init..]” is called. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=e7a5c829540a452f30cd5a1c0609dddcb1af33ce;p=clang [analyzer] Malloc checker should only escape the receiver when “[O init..]” is called. Jordan has pointed out that it is valuable to warn in cases when the arguments to init escape. For example, NSData initWithBytes id not going to free the memory. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@183062 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp index bb2e2df2ac..9f8a1da0ce 100644 --- a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -279,13 +279,19 @@ private: bool checkUseAfterFree(SymbolRef Sym, CheckerContext &C, const Stmt *S) const; - /// Check if the function is known not to free memory, or if it is + /// Check if the function is known free memory, or if it is /// "interesting" and should be modeled explicitly. /// + /// \param EscapingSymbol A function might not free memory in general, but + /// could be known to free a particular symbol. In this case, false is + /// returned and the single escaping symbol is returned through the out + /// parameter. + /// /// We assume that pointers do not escape through calls to system functions /// not handled by this checker. - bool doesNotFreeMemOrInteresting(const CallEvent *Call, - ProgramStateRef State) const; + bool mayFreeAnyEscapedMemoryOrIsModelledExplicitely(const CallEvent *Call, + ProgramStateRef State, + SymbolRef &EscapingSymbol) const; // Implementation of the checkPointerEscape callabcks. ProgramStateRef checkPointerEscapeAux(ProgramStateRef State, @@ -1842,8 +1848,10 @@ ProgramStateRef MallocChecker::evalAssume(ProgramStateRef state, return state; } -bool MallocChecker::doesNotFreeMemOrInteresting(const CallEvent *Call, - ProgramStateRef State) const { +bool MallocChecker::mayFreeAnyEscapedMemoryOrIsModelledExplicitely( + const CallEvent *Call, + ProgramStateRef State, + SymbolRef &EscapingSymbol) const { assert(Call); // For now, assume that any C++ call can free memory. @@ -1851,26 +1859,26 @@ bool MallocChecker::doesNotFreeMemOrInteresting(const CallEvent *Call, // regions escape to C++ containers. They seem to do that even now, but for // mysterious reasons. if (!(isa(Call) || isa(Call))) - return false; + return true; // Check Objective-C messages by selector name. if (const ObjCMethodCall *Msg = dyn_cast(Call)) { // If it's not a framework call, or if it takes a callback, assume it // can free memory. if (!Call->isInSystemHeader() || Call->hasNonZeroCallbackArg()) - return false; + return true; // If it's a method we know about, handle it explicitly post-call. // This should happen before the "freeWhenDone" check below. if (isKnownDeallocObjCMethodName(*Msg)) - return true; + return false; // If there's a "freeWhenDone" parameter, but the method isn't one we know // about, we can't be sure that the object will use free() to deallocate the // memory, so we can't model it explicitly. The best we can do is use it to // decide whether the pointer escapes. if (Optional FreeWhenDone = getFreeWhenDoneArg(*Msg)) - return !*FreeWhenDone; + return *FreeWhenDone; // If the first selector piece ends with "NoCopy", and there is no // "freeWhenDone" parameter set to zero, we know ownership is being @@ -1878,7 +1886,7 @@ bool MallocChecker::doesNotFreeMemOrInteresting(const CallEvent *Call, // free() to deallocate the memory, so we can't model it explicitly. StringRef FirstSlot = Msg->getSelector().getNameForSlot(0); if (FirstSlot.endswith("NoCopy")) - return false; + return true; // If the first selector starts with addPointer, insertPointer, // or replacePointer, assume we are dealing with NSPointerArray or similar. @@ -1887,40 +1895,42 @@ bool MallocChecker::doesNotFreeMemOrInteresting(const CallEvent *Call, if (FirstSlot.startswith("addPointer") || FirstSlot.startswith("insertPointer") || FirstSlot.startswith("replacePointer")) { - return false; + return true; } - // We should escape on call to 'init'. This is especially relevant to the - // receiver, as the corresponding symbol is usually not referenced after - // the call. - if (Msg->getMethodFamily() == OMF_init) - return false; + // We should escape receiver on call to 'init'. This is especially relevant + // to the receiver, as the corresponding symbol is usually not referenced + // after the call. + if (Msg->getMethodFamily() == OMF_init) { + EscapingSymbol = Msg->getReceiverSVal().getAsSymbol(); + return true; + } // Otherwise, assume that the method does not free memory. // Most framework methods do not free memory. - return true; + return false; } // At this point the only thing left to handle is straight function calls. const FunctionDecl *FD = cast(Call)->getDecl(); if (!FD) - return false; + return true; ASTContext &ASTC = State->getStateManager().getContext(); // If it's one of the allocation functions we can reason about, we model // its behavior explicitly. if (isMemFunction(FD, ASTC)) - return true; + return false; // If it's not a system call, assume it frees memory. if (!Call->isInSystemHeader()) - return false; + return true; // White list the system functions whose arguments escape. const IdentifierInfo *II = FD->getIdentifier(); if (!II) - return false; + return true; StringRef FName = II->getName(); // White list the 'XXXNoCopy' CoreFoundation functions. @@ -1934,10 +1944,10 @@ bool MallocChecker::doesNotFreeMemOrInteresting(const CallEvent *Call, if (const DeclRefExpr *DE = dyn_cast(ArgE)) { StringRef DeallocatorName = DE->getFoundDecl()->getName(); if (DeallocatorName == "kCFAllocatorNull") - return true; + return false; } } - return false; + return true; } // Associating streams with malloced buffers. The pointer can escape if @@ -1946,7 +1956,7 @@ bool MallocChecker::doesNotFreeMemOrInteresting(const CallEvent *Call, // Currently, we do not inspect the 'closefn' function (PR12101). if (FName == "funopen") if (Call->getNumArgs() >= 4 && Call->getArgSVal(4).isConstant(0)) - return true; + 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. @@ -1958,7 +1968,7 @@ bool MallocChecker::doesNotFreeMemOrInteresting(const CallEvent *Call, if (const DeclRefExpr *ArgDRE = dyn_cast(ArgE)) if (const VarDecl *D = dyn_cast(ArgDRE->getDecl())) if (D->getCanonicalDecl()->getName().find("std") != StringRef::npos) - return false; + return true; } } @@ -1972,7 +1982,7 @@ bool MallocChecker::doesNotFreeMemOrInteresting(const CallEvent *Call, FName == "CVPixelBufferCreateWithBytes" || FName == "CVPixelBufferCreateWithPlanarBytes" || FName == "OSAtomicEnqueue") { - return false; + return true; } // Handle cases where we know a buffer's /address/ can escape. @@ -1980,11 +1990,11 @@ bool MallocChecker::doesNotFreeMemOrInteresting(const CallEvent *Call, // even though the address escapes, it's still our responsibility to free the // buffer. if (Call->argumentsMayEscape()) - return false; + return true; // Otherwise, assume that the function does not free memory. // Most system calls do not free the memory. - return true; + return false; } static bool retTrue(const RefState *RS) { @@ -2018,8 +2028,11 @@ ProgramStateRef MallocChecker::checkPointerEscapeAux(ProgramStateRef State, bool(*CheckRefState)(const RefState*)) const { // If we know that the call does not free memory, or we want to process the // call later, keep tracking the top level arguments. + SymbolRef EscapingSymbol = 0; if (Kind == PSK_DirectEscapeOnCall && - doesNotFreeMemOrInteresting(Call, State)) { + !mayFreeAnyEscapedMemoryOrIsModelledExplicitely(Call, State, + EscapingSymbol) && + !EscapingSymbol) { return State; } @@ -2028,6 +2041,9 @@ ProgramStateRef MallocChecker::checkPointerEscapeAux(ProgramStateRef State, I != E; ++I) { SymbolRef sym = *I; + if (EscapingSymbol && EscapingSymbol != sym) + continue; + if (const RefState *RS = State->get(sym)) { if (RS->isAllocated() && CheckRefState(RS)) { State = State->remove(sym); diff --git a/test/Analysis/Inputs/system-header-simulator-objc.h b/test/Analysis/Inputs/system-header-simulator-objc.h index ecc99e17c4..3e1d9555bb 100644 --- a/test/Analysis/Inputs/system-header-simulator-objc.h +++ b/test/Analysis/Inputs/system-header-simulator-objc.h @@ -102,6 +102,7 @@ typedef double NSTimeInterval; + (id)dataWithBytesNoCopy:(void *)bytes length:(NSUInteger)length freeWhenDone:(BOOL)b; - (id)initWithBytesNoCopy:(void *)bytes length:(NSUInteger)length; - (id)initWithBytesNoCopy:(void *)bytes length:(NSUInteger)length freeWhenDone:(BOOL)b; +- (id)initWithBytes:(void *)bytes length:(NSUInteger) length; @end typedef struct { diff --git a/test/Analysis/malloc.m b/test/Analysis/malloc.m index 4c1e161db2..ad16db52df 100644 --- a/test/Analysis/malloc.m +++ b/test/Analysis/malloc.m @@ -35,13 +35,18 @@ void rdar10579586(char x); } @end -@interface JKArray : NSObject { +@interface MyArray : NSObject { id * objects; } @end -void _JKArrayCreate() { - JKArray *array = (JKArray *)malloc(12); +void _ArrayCreate() { + MyArray *array = (MyArray *)malloc(12); array = [array init]; free(array); // no-warning +} + +void testNSDataTruePositiveLeak() { + char *b = (char *)malloc(12); + NSData *d = [[NSData alloc] initWithBytes: b length: 12]; // expected-warning {{Potential leak of memory pointed to by 'b'}} } \ No newline at end of file