[PATCH] [Clang SA] Fix double delete warning reported at wrong location.

[Karthik Bhat]

  Fix Indentation.

Hi jordan_rose,

http://llvm-reviews.chandlerc.com/D2441

CHANGE SINCE LAST DIFF
  http://llvm-reviews.chandlerc.com/D2441?vs=6185&id=6188#toc

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
  include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
  lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  lib/StaticAnalyzer/Core/PathDiagnostic.cpp
  test/Analysis/NewDelete-checker-test.cpp

Index: include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
===================================================================
--- include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
@@ -124,6 +124,7 @@
   void operator=(const CallEvent &) LLVM_DELETED_FUNCTION;
 
 protected:
+  const Stmt *TStmt;
   // This is user data for subclasses.
   const void *Data;
 
@@ -151,7 +152,8 @@
   // DO NOT MAKE PUBLIC
   CallEvent(const CallEvent &Original)
     : State(Original.State), LCtx(Original.LCtx), Origin(Original.Origin),
-      Data(Original.Data), Location(Original.Location), RefCount(0) {}
+      TStmt(Original.TStmt), Data(Original.Data), Location(Original.Location),
+      RefCount(0) {}
 
   /// Copies this CallEvent, with vtable intact, into a new block of memory.
   virtual void cloneTo(void *Dest) const = 0;
@@ -651,7 +653,6 @@
 /// of a full-expression (for temporaries), or as part of a delete.
 class CXXDestructorCall : public CXXInstanceCall {
   friend class CallEventManager;
-
 protected:
   typedef llvm::PointerIntPair<const MemRegion *, 1, bool> DtorDataTy;
 
@@ -667,6 +668,7 @@
                     ProgramStateRef St, const LocationContext *LCtx)
     : CXXInstanceCall(DD, St, LCtx) {
     Data = DtorDataTy(Target, IsBaseDestructor).getOpaqueValue();
+    TStmt = Trigger;
     Location = Trigger->getLocEnd();
   }
 
@@ -687,6 +689,10 @@
     return DtorDataTy::getFromOpaqueValue(Data).getInt();
   }
 
+  const Stmt *getTriggerStmt() const {
+    return TStmt;
+  }
+
   virtual Kind getKind() const { return CE_CXXDestructor; }
 
   static bool classof(const CallEvent *CA) {
Index: include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
===================================================================
--- include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
+++ include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
@@ -296,6 +296,8 @@
 
   /// \brief Retrieve the statement corresponding to the successor node.
   static const Stmt *getNextStmt(const ExplodedNode *N);
+
+  static const Stmt *getPrevStmt(const ExplodedNode *N);
 };
 
 class PathDiagnosticLocationPair {
Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -1710,6 +1710,15 @@
       return;
   }
 
+  if (const CXXDestructorCall *DC = dyn_cast<CXXDestructorCall>(&Call)) {
+    if (isa<CXXDeleteExpr>(DC->getTriggerStmt())) {
+      const CXXDeleteExpr *DE =  cast<CXXDeleteExpr>(DC->getTriggerStmt());
+      SymbolRef Sym = C.getSVal(DE->getArgument()).getAsSymbol();
+      if (!Sym || checkUseAfterFree(Sym, C, DE->getArgument()))
+        return;
+    }
+  }
+
   // Check if the callee of a method is deleted.
   if (const CXXInstanceCall *CC = dyn_cast<CXXInstanceCall>(&Call)) {
     SymbolRef Sym = CC->getCXXThisVal().getAsSymbol();
Index: lib/StaticAnalyzer/Core/PathDiagnostic.cpp
===================================================================
--- lib/StaticAnalyzer/Core/PathDiagnostic.cpp
+++ lib/StaticAnalyzer/Core/PathDiagnostic.cpp
@@ -706,6 +706,33 @@
   return 0;
 }
 
+const Stmt *PathDiagnosticLocation::getPrevStmt(const ExplodedNode *N) {
+  for (N = N->getFirstPred(); N; N = N->getFirstPred()) {
+    if (const Stmt *S = getStmt(N)) {
+      // Check if the statement is '?' or '&&'/'||'.  These are "merges",
+      // not actual statement points.
+      switch (S->getStmtClass()) {
+        case Stmt::ChooseExprClass:
+        case Stmt::BinaryConditionalOperatorClass:
+        case Stmt::ConditionalOperatorClass:
+          continue;
+        case Stmt::BinaryOperatorClass: {
+          BinaryOperatorKind Op = cast<BinaryOperator>(S)->getOpcode();
+          if (Op == BO_LAnd || Op == BO_LOr)
+            continue;
+          break;
+        }
+        default:
+          break;
+      }
+      // We found the statement, so return it.
+      return S;
+    }
+  }
+
+  return 0;
+}
+
 const Stmt *PathDiagnosticLocation::getNextStmt(const ExplodedNode *N) {
   for (N = N->getFirstSucc(); N; N = N->getFirstSucc()) {
     if (const Stmt *S = getStmt(N)) {
@@ -739,6 +766,14 @@
   assert(N && "Cannot create a location with a null node.");
   const Stmt *S = getStmt(N);
 
+  // If this is an implicit call find the stmt that triggered it.
+  if (N && !S) {
+    ProgramPoint P = N->getLocation();
+    if (P.getAs<ImplicitCallPoint>()) {
+      S = getPrevStmt(N);
+    }
+  }
+
   if (!S)
     S = getNextStmt(N);
 
Index: test/Analysis/NewDelete-checker-test.cpp
===================================================================
--- test/Analysis/NewDelete-checker-test.cpp
+++ test/Analysis/NewDelete-checker-test.cpp
@@ -338,13 +338,13 @@
 public:
   int *x;
   DerefClass() {}
-  ~DerefClass() {*x = 1;} //expected-warning {{Use of memory after it is freed}}
+  ~DerefClass() {*x = 1;} 
 };
 
 void testDoubleDeleteClassInstance() {
   DerefClass *foo = new DerefClass();
   delete foo;
-  delete foo; // FIXME: We should ideally report warning here instead of inside the destructor.
+  delete foo; //expected-warning {{Use of memory after it is freed}}
 }
 
 class EmptyClass{
@@ -356,5 +356,5 @@
 void testDoubleDeleteEmptyClass() {
   EmptyClass *foo = new EmptyClass();
   delete foo;
-  delete foo;  //expected-warning {{Attempt to free released memory}}
+  delete foo;  //expected-warning {{Use of memory after it is freed}}
 }
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D2441.2.patch
Type: text/x-patch
Size: 5573 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131219/81fdf832/attachment.bin>