[analyzer] Assume escape is possible through system functions taking void*

The analyzer assumes that system functions will not free memory or modify the
arguments in other ways, so we assume that arguments do not escape when
those are called. However, this may lead to false positive leak errors. For
example, in code like this where the pointers added to the rb_tree are freed
later on:

		struct alarm_event *e = calloc(1, sizeof(*e));
<snip>

		rb_tree_insert_node(&alarm_tree, e);

Add a heuristic to assume that calls to system functions taking void*
arguments allow for pointer escape.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@251449 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h b/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
index cf888b5040e5f24a6f3582d03b2a98b087838131..77d73cfd1eee31c34dde352c42561caeabdfccd0 100644 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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.