From e7a5c829540a452f30cd5a1c0609dddcb1af33ce Mon Sep 17 00:00:00 2001 From: Anna Zaks Date: Fri, 31 May 2013 23:47:32 +0000 Subject: [PATCH] =?utf8?q?[analyzer]=20Malloc=20checker=20should=20only=20?= =?utf8?q?escape=20the=20receiver=20when=20=E2=80=9C[O=20init..]=E2=80=9D?= =?utf8?q?=20is=20called.?= MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 --- lib/StaticAnalyzer/Checkers/MallocChecker.cpp | 74 +++++++++++-------- .../Inputs/system-header-simulator-objc.h | 1 + test/Analysis/malloc.m | 11 ++- 3 files changed, 54 insertions(+), 32 deletions(-) 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 -- 2.40.0