diff options
| -rw-r--r-- | lib/StaticAnalyzer/Checkers/MallocChecker.cpp | 118 | ||||
| -rw-r--r-- | test/Analysis/Inputs/system-header-simulator-for-malloc.h | 34 | ||||
| -rw-r--r-- | test/Analysis/malloc.mm | 47 |
3 files changed, 130 insertions, 69 deletions
diff --git a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp index f412c049c7..28a2999f04 100644 --- a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -168,12 +168,13 @@ public: private: void initIdentifierInfo(ASTContext &C) const; + ///@{ /// Check if this is one of the functions which can allocate/reallocate memory /// pointed to by one of its arguments. bool isMemFunction(const FunctionDecl *FD, ASTContext &C) const; bool isFreeFunction(const FunctionDecl *FD, ASTContext &C) const; bool isAllocationFunction(const FunctionDecl *FD, ASTContext &C) const; - + ///@} static ProgramStateRef MallocMemReturnsAttr(CheckerContext &C, const CallExpr *CE, const OwnershipAttr* Att); @@ -218,10 +219,13 @@ private: bool checkUseAfterFree(SymbolRef Sym, CheckerContext &C, const Stmt *S = 0) const; - /// Check if the function is not known to us. So, for example, we could - /// conservatively assume it can free/reallocate it's pointer arguments. - bool doesNotFreeMemory(const CallEvent *Call, - ProgramStateRef State) const; + /// Check if the function is known not to free memory, or if it is + /// "interesting" and should be modeled explicitly. + /// + /// 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; static bool SummarizeValue(raw_ostream &os, SVal V); static bool SummarizeRegion(raw_ostream &os, const MemRegion *MR); @@ -495,14 +499,30 @@ void MallocChecker::checkPostStmt(const CallExpr *CE, CheckerContext &C) const { C.addTransition(State); } -static bool isFreeWhenDoneSetToZero(const ObjCMethodCall &Call) { +static bool isKnownDeallocObjCMethodName(const ObjCMethodCall &Call) { + // If the first selector piece is one of the names below, assume that the + // object takes ownership of the memory, promising to eventually deallocate it + // with free(). + // Ex: [NSData dataWithBytesNoCopy:bytes length:10]; + // (...unless a 'freeWhenDone' parameter is false, but that's checked later.) + StringRef FirstSlot = Call.getSelector().getNameForSlot(0); + if (FirstSlot == "dataWithBytesNoCopy" || + FirstSlot == "initWithBytesNoCopy" || + FirstSlot == "initWithCharactersNoCopy") + return true; + + return false; +} + +static Optional<bool> getFreeWhenDoneArg(const ObjCMethodCall &Call) { Selector S = Call.getSelector(); + + // FIXME: We should not rely on fully-constrained symbols being folded. for (unsigned i = 1; i < S.getNumArgs(); ++i) if (S.getNameForSlot(i).equals("freeWhenDone")) - if (Call.getArgSVal(i).isConstant(0)) - return true; + return !Call.getArgSVal(i).isZeroConstant(); - return false; + return None; } void MallocChecker::checkPostObjCMessage(const ObjCMethodCall &Call, @@ -510,25 +530,20 @@ void MallocChecker::checkPostObjCMessage(const ObjCMethodCall &Call, if (C.wasInlined) return; - // If the first selector is dataWithBytesNoCopy, assume that the memory will - // be released with 'free' by the new object. - // Ex: [NSData dataWithBytesNoCopy:bytes length:10]; - // Unless 'freeWhenDone' param set to 0. - // TODO: Check that the memory was allocated with malloc. - bool ReleasedAllocatedMemory = false; - Selector S = Call.getSelector(); - if ((S.getNameForSlot(0) == "dataWithBytesNoCopy" || - S.getNameForSlot(0) == "initWithBytesNoCopy" || - S.getNameForSlot(0) == "initWithCharactersNoCopy") && - !isFreeWhenDoneSetToZero(Call)){ - unsigned int argIdx = 0; - ProgramStateRef State = FreeMemAux(C, Call.getArgExpr(argIdx), - Call.getOriginExpr(), C.getState(), true, - ReleasedAllocatedMemory, - /* RetNullOnFailure*/ true); - - C.addTransition(State); - } + if (!isKnownDeallocObjCMethodName(Call)) + return; + + if (Optional<bool> FreeWhenDone = getFreeWhenDoneArg(Call)) + if (!*FreeWhenDone) + return; + + bool ReleasedAllocatedMemory; + ProgramStateRef State = FreeMemAux(C, Call.getArgExpr(0), + Call.getOriginExpr(), C.getState(), + /*Hold=*/true, ReleasedAllocatedMemory, + /*RetNullOnFailure=*/true); + + C.addTransition(State); } ProgramStateRef MallocChecker::MallocMemReturnsAttr(CheckerContext &C, @@ -1356,12 +1371,8 @@ ProgramStateRef MallocChecker::evalAssume(ProgramStateRef state, return state; } -// Check if the function is known to us. So, for example, we could -// conservatively assume it can free/reallocate its pointer arguments. -// (We assume that the pointers cannot escape through calls to system -// functions not handled by this checker.) -bool MallocChecker::doesNotFreeMemory(const CallEvent *Call, - ProgramStateRef State) const { +bool MallocChecker::doesNotFreeMemOrInteresting(const CallEvent *Call, + ProgramStateRef State) const { assert(Call); // For now, assume that any C++ call can free memory. @@ -1378,24 +1389,23 @@ bool MallocChecker::doesNotFreeMemory(const CallEvent *Call, if (!Call->isInSystemHeader() || Call->hasNonZeroCallbackArg()) return false; - Selector S = Msg->getSelector(); - - // Whitelist the ObjC methods which do free memory. - // - Anything containing 'freeWhenDone' param set to 1. - // Ex: dataWithBytesNoCopy:length:freeWhenDone. - for (unsigned i = 1; i < S.getNumArgs(); ++i) { - if (S.getNameForSlot(i).equals("freeWhenDone")) { - if (Call->getArgSVal(i).isConstant(1)) - return false; - else - 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; - // If the first selector ends with NoCopy, assume that the ownership is - // transferred as well. - // Ex: [NSData dataWithBytesNoCopy:bytes length:10]; - StringRef FirstSlot = S.getNameForSlot(0); + // 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<bool> FreeWhenDone = getFreeWhenDoneArg(*Msg)) + return !*FreeWhenDone; + + // If the first selector piece ends with "NoCopy", and there is no + // "freeWhenDone" parameter set to zero, we know ownership is being + // transferred. Again, though, we can't be sure that the object will use + // free() to deallocate the memory, so we can't model it explicitly. + StringRef FirstSlot = Msg->getSelector().getNameForSlot(0); if (FirstSlot.endswith("NoCopy")) return false; @@ -1504,11 +1514,11 @@ ProgramStateRef MallocChecker::checkPointerEscape(ProgramStateRef State, const InvalidatedSymbols &Escaped, const CallEvent *Call, PointerEscapeKind Kind) const { - // If we know that the call does not free memory, keep tracking the top - // level arguments. + // If we know that the call does not free memory, or we want to process the + // call later, keep tracking the top level arguments. if ((Kind == PSK_DirectEscapeOnCall || Kind == PSK_IndirectEscapeOnCall) && - doesNotFreeMemory(Call, State)) { + doesNotFreeMemOrInteresting(Call, State)) { return State; } diff --git a/test/Analysis/Inputs/system-header-simulator-for-malloc.h b/test/Analysis/Inputs/system-header-simulator-for-malloc.h new file mode 100644 index 0000000000..e76455655e --- /dev/null +++ b/test/Analysis/Inputs/system-header-simulator-for-malloc.h @@ -0,0 +1,34 @@ +// Like the compiler, the static analyzer treats some functions differently if +// they come from a system header -- for example, it is assumed that system +// functions do not arbitrarily free() their parameters, and that some bugs +// found in system headers cannot be fixed by the user and should be +// suppressed. +#pragma clang system_header + +typedef __typeof(sizeof(int)) size_t; +void *malloc(size_t); +void *calloc(size_t, size_t); +void free(void *); + + +#if __OBJC__ + +#import "system-header-simulator-objc.h" + +@interface Wrapper : NSData +- (id)initWithBytesNoCopy:(void *)bytes length:(NSUInteger)len; +@end + +@implementation Wrapper +- (id)initWithBytesNoCopy:(void *)bytes length:(NSUInteger)len { + return [self initWithBytesNoCopy:bytes length:len freeWhenDone:1]; // no-warning +} +@end + +@interface CustomData : NSData ++ (id)somethingNoCopy:(char *)bytes; ++ (id)somethingNoCopy:(void *)bytes length:(NSUInteger)length freeWhenDone:(BOOL)freeBuffer; ++ (id)something:(char *)bytes freeWhenDone:(BOOL)freeBuffer; +@end + +#endif diff --git a/test/Analysis/malloc.mm b/test/Analysis/malloc.mm index f2a195ce1d..2f583b45fd 100644 --- a/test/Analysis/malloc.mm +++ b/test/Analysis/malloc.mm @@ -1,19 +1,6 @@ // RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc -analyzer-store=region -verify -fblocks %s -#include "Inputs/system-header-simulator-objc.h" - -typedef __typeof(sizeof(int)) size_t; -void *malloc(size_t); -void free(void *); - -@interface Wrapper : NSData -- (id)initWithBytesNoCopy:(void *)bytes length:(NSUInteger)len; -@end - -@implementation Wrapper -- (id)initWithBytesNoCopy:(void *)bytes length:(NSUInteger)len { - return [self initWithBytesNoCopy:bytes length:len freeWhenDone:1]; // no-warning -} -@end +#import "Inputs/system-header-simulator-objc.h" +#import "Inputs/system-header-simulator-for-malloc.h" // Done with headers. Start testing. void testNSDatafFreeWhenDoneNoError(NSUInteger dataLength) { @@ -79,6 +66,11 @@ void testNSStringFreeWhenDoneNO2(NSUInteger dataLength) { NSString *nsstr = [[NSString alloc] initWithCharactersNoCopy:data length:dataLength freeWhenDone:0]; // expected-warning{{leak}} } +void testOffsetFree() { + int *p = (int *)malloc(sizeof(int)); + NSData *nsdata = [NSData dataWithBytesNoCopy:++p length:sizeof(int) freeWhenDone:1]; // expected-warning{{Argument to free() is offset by 4 bytes from the start of memory allocated by malloc()}} +} + void testRelinquished1() { void *data = malloc(42); NSData *nsdata = [NSData dataWithBytesNoCopy:data length:42 freeWhenDone:1]; @@ -92,6 +84,31 @@ void testRelinquished2() { [NSData dataWithBytesNoCopy:data length:42]; // expected-warning {{Attempt to free released memory}} } +void testNoCopy() { + char *p = (char *)calloc(sizeof(int), 1); + CustomData *w = [CustomData somethingNoCopy:p]; // no-warning +} + +void testFreeWhenDone() { + char *p = (char *)calloc(sizeof(int), 1); + CustomData *w = [CustomData something:p freeWhenDone:1]; // no-warning +} + +void testFreeWhenDonePositive() { + char *p = (char *)calloc(sizeof(int), 1); + CustomData *w = [CustomData something:p freeWhenDone:0]; // expected-warning{{leak}} +} + +void testFreeWhenDoneNoCopy() { + int *p = (int *)malloc(sizeof(int)); + CustomData *w = [CustomData somethingNoCopy:p length:sizeof(int) freeWhenDone:1]; // no-warning +} + +void testFreeWhenDoneNoCopyPositive() { + int *p = (int *)malloc(sizeof(int)); + CustomData *w = [CustomData somethingNoCopy:p length:sizeof(int) freeWhenDone:0]; // expected-warning{{leak}} +} + // Test CF/NS...NoCopy. PR12100: Pointers can escape when custom deallocators are provided. void testNSDatafFreeWhenDone(NSUInteger dataLength) { CFStringRef str; |