[analyzer] Switched to checkPreCall interface for detecting usage after free.

Now the check is also applied to arguments for Objective-C method calls and to 'this' pointer.

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

4 files changed, 101 insertions, 21 deletions

diff --git a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 03471a5236..64e017ed1c 100644
--- a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -147,7 +147,7 @@ class MallocChecker : public Checker<check::DeadSymbols,
                                      check::PointerEscape,
                                      check::ConstPointerEscape,
                                      check::PreStmt<ReturnStmt>,
-                                     check::PreStmt<CallExpr>,
+                                     check::PreCall,
                                      check::PostStmt<CallExpr>,
                                      check::PostStmt<CXXNewExpr>,
                                      check::PreStmt<CXXDeleteExpr>,
@@ -181,7 +181,7 @@ public:
 
   ChecksFilter Filter;
 
-  void checkPreStmt(const CallExpr *S, CheckerContext &C) const;
+  void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
   void checkPostStmt(const CallExpr *CE, CheckerContext &C) const;
   void checkPostStmt(const CXXNewExpr *NE, CheckerContext &C) const;
   void checkPreStmt(const CXXDeleteExpr *DE, CheckerContext &C) const;
@@ -1671,26 +1671,39 @@ void MallocChecker::checkDeadSymbols(SymbolReaper &SymReaper,
   C.addTransition(state->set<RegionState>(RS), N);
 }
 
-void MallocChecker::checkPreStmt(const CallExpr *CE, CheckerContext &C) const {
+void MallocChecker::checkPreCall(const CallEvent &Call,
+                                 CheckerContext &C) const {
+
   // We will check for double free in the post visit.
-  if ((Filter.CMallocOptimistic || Filter.CMallocPessimistic) &&
-      isFreeFunction(C.getCalleeDecl(CE), C.getASTContext()))
-    return;
+  if (const AnyFunctionCall *FC = dyn_cast<AnyFunctionCall>(&Call)) {
+    const FunctionDecl *FD = FC->getDecl();
+    if (!FD)
+      return;
 
-  if (Filter.CNewDeleteChecker &&
-      isStandardNewDelete(C.getCalleeDecl(CE), C.getASTContext()))
-    return;
+    if ((Filter.CMallocOptimistic || Filter.CMallocPessimistic) &&
+        isFreeFunction(FD, C.getASTContext()))
+      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 = C.getSVal(A).getAsSymbol();
+    if (Filter.CNewDeleteChecker &&
+        isStandardNewDelete(FD, C.getASTContext()))
+      return;
+  }
+
+  // Check if the callee of a method is deleted.
+  if (const CXXInstanceCall *CC = dyn_cast<CXXInstanceCall>(&Call)) {
+    SymbolRef Sym = CC->getCXXThisVal().getAsSymbol();
+    if (!Sym || checkUseAfterFree(Sym, C, CC->getCXXThisExpr()))
+      return;
+  }
+
+  // Check arguments for being used after free.
+  for (unsigned I = 0, E = Call.getNumArgs(); I != E; ++I) {
+    SVal ArgSVal = Call.getArgSVal(I);
+    if (ArgSVal.getAs<Loc>()) {
+      SymbolRef Sym = ArgSVal.getAsSymbol();
       if (!Sym)
         continue;
-      if (checkUseAfterFree(Sym, C, A))
+      if (checkUseAfterFree(Sym, C, Call.getArgExpr(I)))
         return;
     }
   }
diff --git a/test/Analysis/NewDelete-checker-test.cpp b/test/Analysis/NewDelete-checker-test.cpp
index bf95221eac..35125ccc8f 100644
--- a/test/Analysis/NewDelete-checker-test.cpp
+++ b/test/Analysis/NewDelete-checker-test.cpp
@@ -54,7 +54,6 @@ void testGlobalNoThrowPlacementExprNewBeforeOverload() {
 // expected-warning@-2{{Potential leak of memory pointed to by 'p'}}
 #endif
 
-
 //----- Standard pointer placement operators
 void testGlobalPointerPlacementNew() {
   int i;
@@ -91,14 +90,54 @@ void testNewInvalidationPlacement(PtrWrapper *w) {
 // other checks
 //---------------
 
-void f(int *);
+class SomeClass {
+public:
+  void f(int *p);
+};
+
+void f(int *p1, int *p2 = 0, int *p3 = 0);
+void g(SomeClass &c, ...);
 
-void testUseAfterDelete() {
+void testUseFirstArgAfterDelete() {
   int *p = new int;
   delete p;
   f(p); // expected-warning{{Use of memory after it is freed}}
 }
 
+void testUseMiddleArgAfterDelete(int *p) {
+  delete p;
+  f(0, p); // expected-warning{{Use of memory after it is freed}}
+}
+
+void testUseLastArgAfterDelete(int *p) {
+  delete p;
+  f(0, 0, p); // expected-warning{{Use of memory after it is freed}}
+}
+
+void testUseRefArgAfterDelete(SomeClass &c) {
+  delete &c;
+  g(c); // expected-warning{{Use of memory after it is freed}}
+}
+
+void testVariadicArgAfterDelete() {
+  SomeClass c;
+  int *p = new int;
+  delete p;
+  g(c, 0, p); // expected-warning{{Use of memory after it is freed}}
+}
+
+void testUseMethodArgAfterDelete(int *p) {
+  SomeClass *c = new SomeClass;
+  delete p;
+  c->f(p); // expected-warning{{Use of memory after it is freed}}
+}
+
+void testUseThisAfterDelete() {
+  SomeClass *c = new SomeClass;
+  delete c;
+  c->f(0); // expected-warning{{Use of memory after it is freed}}
+}
+
 void testDeleteAlloca() {
   int *p = (int *)__builtin_alloca(sizeof(int));
   delete p; // expected-warning{{Memory allocated by alloca() should not be deallocated}}
diff --git a/test/Analysis/malloc.mm b/test/Analysis/malloc.mm
index bd9d2d2a7f..c7fe86bf0b 100644
--- a/test/Analysis/malloc.mm
+++ b/test/Analysis/malloc.mm
@@ -81,7 +81,17 @@ void testRelinquished2() {
   void *data = malloc(42);
   NSData *nsdata;
   free(data);
-  [NSData dataWithBytesNoCopy:data length:42]; // expected-warning {{Attempt to free released memory}}
+  [NSData dataWithBytesNoCopy:data length:42]; // expected-warning {{Use of memory after it is freed}}
+}
+
+@interface My
++ (void)param:(void *)p;
+@end
+
+void testUseAfterFree() {
+  int *p = (int *)malloc(sizeof(int));
+  free(p);
+  [My param:p];  // expected-warning{{Use of memory after it is freed}}
 }
 
 void testNoCopy() {
diff --git a/test/Analysis/new.cpp b/test/Analysis/new.cpp
index 802243088e..8d3eee9baa 100644
--- a/test/Analysis/new.cpp
+++ b/test/Analysis/new.cpp
@@ -8,6 +8,12 @@ extern "C" void *malloc(size_t);
 extern "C" void free(void *);
 
 int someGlobal;
+
+class SomeClass {
+public:
+  void f(int *p);
+};
+
 void testImplicitlyDeclaredGlobalNew() {
   if (someGlobal != 0)
     return;
@@ -101,6 +107,12 @@ void testCacheOut(PtrWrapper w) {
   new (&w.x) (int*)(0); // we cache out here; don't crash
 }
 
+void testUseAfter(int *p) {
+  SomeClass *c = new SomeClass;
+  free(p);
+  c->f(p); // expected-warning{{Use of memory after it is freed}}
+  delete c;
+}
 
 //--------------------------------------------------------------------
 // Check for intersection with other checkers from MallocChecker.cpp 
@@ -152,6 +164,12 @@ void testCustomPlacementNewAfterFree() {
   p = new(0, p) int; // expected-warning{{Use of memory after it is freed}}
 }
 
+void testUsingThisAfterDelete() {
+  SomeClass *c = new SomeClass;
+  delete c;
+  c->f(0); // no-warning
+}
+
 //--------------------------------
 // Incorrectly-modelled behavior
 //--------------------------------