diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h b/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h index cf888b5040e5f24a6f3582d03b2a98b087838131..77d73cfd1eee31c34dde352c42561caeabdfccd0 100644 --- a/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h +++ b/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h @@ -254,9 +254,16 @@ public: /// which the return value has already been bound to the origin expression. SVal getReturnValue() const; + /// \brief Returns true if the type of any of the non-null arguments satisfies + /// the condition. + bool hasNonNullArgumentsWithType(bool (*Condition)(QualType)) const; + /// \brief Returns true if any of the arguments appear to represent callbacks. bool hasNonZeroCallbackArg() const; + /// \brief Returns true if any of the arguments is void*. + bool hasVoidPointerToNonConstArg() const; + /// \brief Returns true if any of the arguments are known to escape to long- /// term storage, even if this method will not modify them. // NOTE: The exact semantics of this are still being defined! diff --git a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp index f924a767789406b8e7a4f0569fe3ed7efc3a5be0..54b1a6ee0fd191971f1e3c6a39b33fe3aaa7e3b2 100644 --- a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -2390,7 +2390,7 @@ bool MallocChecker::mayFreeAnyEscapedMemoryOrIsModeledExplicitly( if (const ObjCMethodCall *Msg = dyn_cast<ObjCMethodCall>(Call)) { // If it's not a framework call, or if it takes a callback, assume it // can free memory. - if (!Call->isInSystemHeader() || Call->hasNonZeroCallbackArg()) + if (!Call->isInSystemHeader() || Call->argumentsMayEscape()) return true; // If it's a method we know about, handle it explicitly post-call. diff --git a/lib/StaticAnalyzer/Core/CallEvent.cpp b/lib/StaticAnalyzer/Core/CallEvent.cpp index f9d4ea2283078261f19bebc41d3b72d126ccd332..32c168819bf74d86347414f12df563836081e220 100644 --- a/lib/StaticAnalyzer/Core/CallEvent.cpp +++ b/lib/StaticAnalyzer/Core/CallEvent.cpp @@ -50,11 +50,7 @@ QualType CallEvent::getResultType() const { return ResultTy; } -static bool isCallbackArg(SVal V, QualType T) { - // If the parameter is 0, it's harmless. - if (V.isZeroConstant()) - return false; - +static bool isCallback(QualType T) { // If a parameter is a block or a callback, assume it can modify pointer. if (T->isBlockPointerType() || T->isFunctionPointerType() || @@ -75,32 +71,53 @@ static bool isCallbackArg(SVal V, QualType T) { return true; } } - return false; } -bool CallEvent::hasNonZeroCallbackArg() const { +static bool isVoidPointerToNonConst(QualType T) { + if (const PointerType *PT = T->getAs<PointerType>()) { + QualType PointeeTy = PT->getPointeeType(); + if (PointeeTy.isConstQualified()) + return false; + return PointeeTy->isVoidType(); + } else + return false; +} + +bool CallEvent::hasNonNullArgumentsWithType(bool (*Condition)(QualType)) const { unsigned NumOfArgs = getNumArgs(); // If calling using a function pointer, assume the function does not - // have a callback. TODO: We could check the types of the arguments here. + // satisfy the callback. + // TODO: We could check the types of the arguments here. if (!getDecl()) return false; unsigned Idx = 0; for (CallEvent::param_type_iterator I = param_type_begin(), - E = param_type_end(); + E = param_type_end(); I != E && Idx < NumOfArgs; ++I, ++Idx) { if (NumOfArgs <= Idx) break; - if (isCallbackArg(getArgSVal(Idx), *I)) + // If the parameter is 0, it's harmless. + if (getArgSVal(Idx).isZeroConstant()) + continue; + + if (Condition(*I)) return true; } - return false; } +bool CallEvent::hasNonZeroCallbackArg() const { + return hasNonNullArgumentsWithType(isCallback); +} + +bool CallEvent::hasVoidPointerToNonConstArg() const { + return hasNonNullArgumentsWithType(isVoidPointerToNonConst); +} + bool CallEvent::isGlobalCFunction(StringRef FunctionName) const { const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(getDecl()); if (!FD) @@ -326,7 +343,7 @@ void AnyFunctionCall::getInitialStackFrameContents( } bool AnyFunctionCall::argumentsMayEscape() const { - if (hasNonZeroCallbackArg()) + if (CallEvent::argumentsMayEscape() || hasVoidPointerToNonConstArg()) return true; const FunctionDecl *D = getDecl(); diff --git a/test/Analysis/Inputs/system-header-simulator.h b/test/Analysis/Inputs/system-header-simulator.h index 8b8c9c4fe17b6402d285852c838667560993ee65..8605a2cbcb30e3d22095148ced5b91276a08c585 100644 --- a/test/Analysis/Inputs/system-header-simulator.h +++ b/test/Analysis/Inputs/system-header-simulator.h @@ -82,6 +82,12 @@ void xpc_connection_resume(xpc_connection_t connection); void fakeSystemHeaderCallInt(int *); void fakeSystemHeaderCallIntPtr(int **); +// Some data strauctures may hold onto the pointer and free it later. +void fake_insque(void *, void *); +typedef struct fake_rb_tree { void *opaque[8]; } fake_rb_tree_t; +void fake_rb_tree_init(fake_rb_tree_t *, const void *); +void *fake_rb_tree_insert_node(fake_rb_tree_t *, void *); + typedef struct __SomeStruct { char * p; } SomeStruct; diff --git a/test/Analysis/malloc.c b/test/Analysis/malloc.c index 0962d0dbbfeba7451361412d31599c1b3ac78e60..881eb38ad8401dc978c5895407a887e2bdf2a8c1 100644 --- a/test/Analysis/malloc.c +++ b/test/Analysis/malloc.c @@ -1657,6 +1657,23 @@ int *radar15580979() { return p; } +// Some data structures may hold onto the pointer and free it later. +void testEscapeThroughSystemCallTakingVoidPointer1(void *queue) { + int *data = (int *)malloc(32); + fake_insque(queue, data); // no warning +} + +void testEscapeThroughSystemCallTakingVoidPointer2(fake_rb_tree_t *rbt) { + int *data = (int *)malloc(32); + fake_rb_tree_init(rbt, data); +} //expected-warning{{Potential leak}} + +void testEscapeThroughSystemCallTakingVoidPointer3(fake_rb_tree_t *rbt) { + int *data = (int *)malloc(32); + fake_rb_tree_init(rbt, data); + fake_rb_tree_insert_node(rbt, data); // no warning +} + // ---------------------------------------------------------------------------- // False negatives.