[analyzer] Malloc Checker: reduce false negatives rate by assuming that

a pointer cannot escape through calls to system functions. Also, stop
after reporting the first use-after-free.

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

diff --git a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index ea4d7d29eab8b0a9f8f15f150ad02f3ec9fa91d3..f486a7e8c9a2f82032ee516d2c476081d653eaaf 100644 (file)
--- a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -20,6 +20,7 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
+#include "clang/Basic/SourceManager.h"
 #include "llvm/ADT/ImmutableMap.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/STLExtras.h"
@@ -260,20 +261,41 @@ void MallocChecker::checkPostStmt(const CallExpr *CE, CheckerContext &C) const {
       switch ((*i)->getOwnKind()) {
       case OwnershipAttr::Returns: {
         MallocMemReturnsAttr(C, CE, *i);
-        break;
+        return;
       }
       case OwnershipAttr::Takes:
       case OwnershipAttr::Holds: {
         FreeMemAttr(C, CE, *i);
-        break;
+        return;
       }
       }
     }
   }
 
+  // Check use after free, when a freed pointer is passed to a call.
+  ProgramStateRef State = C.getState();
+  for (CallExpr::const_arg_iterator I = CE->arg_begin(),
+                                    E = CE->arg_end(); I != E; ++I) {
+    const Expr *A = *I;
+    if (A->getType().getTypePtr()->isAnyPointerType()) {
+      SymbolRef Sym = State->getSVal(A, C.getLocationContext()).getAsSymbol();
+      if (!Sym)
+        continue;
+      if (checkUseAfterFree(Sym, C, A))
+        return;
+    }
+  }
+
+  // The pointer might escape through a function call.
+  // TODO: This should be rewritten to take into account inlining.
   if (Filter.CMallocPessimistic) {
+    SourceLocation FLoc = FD->getLocation();
+    // We assume that the pointers cannot escape through calls to system
+    // functions.
+    if (C.getSourceManager().isInSystemHeader(FLoc))
+      return;
+
     ProgramStateRef State = C.getState();
-    // The pointer might escape through a function call.
     for (CallExpr::const_arg_iterator I = CE->arg_begin(),
                                       E = CE->arg_end(); I != E; ++I) {
       const Expr *A = *I;
@@ -282,7 +304,6 @@ void MallocChecker::checkPostStmt(const CallExpr *CE, CheckerContext &C) const {
         if (!Sym)
           continue;
         checkEscape(Sym, A, C);
-        checkUseAfterFree(Sym, C, A);
       }
     }
   }
@@ -767,7 +788,8 @@ void MallocChecker::checkPreStmt(const ReturnStmt *S, CheckerContext &C) const {
     return;
 
   // Check if we are returning freed memory.
-  checkUseAfterFree(Sym, C, S);
+  if (checkUseAfterFree(Sym, C, S))
+    return;
 
   // Check if the symbol is escaping.
   checkEscape(Sym, S, C);
@@ -778,7 +800,7 @@ bool MallocChecker::checkUseAfterFree(SymbolRef Sym, CheckerContext &C,
   assert(Sym);
   const RefState *RS = C.getState()->get<RegionState>(Sym);
   if (RS && RS->isReleased()) {
-    if (ExplodedNode *N = C.addTransition()) {
+    if (ExplodedNode *N = C.generateSink()) {
       if (!BT_UseFree)
         BT_UseFree.reset(new BuiltinBug("Use of dynamically allocated memory "
             "after it is freed."));

diff --git a/test/Analysis/malloc.c b/test/Analysis/malloc.c
index b3095e0f302a6b999c62b1f95459c6ef40145257..0321f523a33762e12c1be3f95f5f4d262ad62edb 100644 (file)
--- a/test/Analysis/malloc.c
+++ b/test/Analysis/malloc.c
@@ -1,4 +1,6 @@
 // RUN: %clang_cc1 -analyze -analyzer-checker=core,experimental.deadcode.UnreachableCode,experimental.core.CastSize,experimental.unix.Malloc -analyzer-store=region -verify %s
+#include "system-header-simulator.h"
+
 typedef __typeof(sizeof(int)) size_t;
 void *malloc(size_t);
 void free(void *);
@@ -237,6 +239,11 @@ void mallocFreeUse_params() {
   int *p = malloc(12);
   free(p);
   myfoo(p); //expected-warning{{Use of dynamically allocated memory after it is freed}}
+}
+
+void mallocFreeUse_params2() {
+  int *p = malloc(12);
+  free(p);
   myfooint(*p); //expected-warning{{Use of dynamically allocated memory after it is freed}}
 }
 
@@ -376,6 +383,12 @@ void mallocAssert(int *g) {
   return;
 }
 
+void doNotInvalidateWhenPassedToSystemCalls(char *s) {
+  char *p = malloc(12);
+  strlen(p);
+  strcpy(p, s); // expected-warning {{leak}}
+}
+
 // Below are the known false positives.
 
 // TODO: There should be no warning here.

diff --git a/test/Analysis/system-header-simulator.h b/test/Analysis/system-header-simulator.h
index 68c66574df83878a778f57c698a1842ac7c0dd5b..1dd9c5b60746e10988d94eae275222102b1760b1 100644 (file)
--- a/test/Analysis/system-header-simulator.h
+++ b/test/Analysis/system-header-simulator.h
@@ -8,3 +8,6 @@ int fscanf(FILE *restrict stream, const char *restrict format, ...);
 extern int errno;
 
 unsigned long strlen(const char *);
+
+char *strcpy(char *restrict s1, const char *restrict s2);
+