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

[Jordan Rose]

  This is a good start, but I think it can be made a little simpler.


================
Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:127-129
@@ -126,4 +126,5 @@
 protected:
+  const Stmt *TStmt;
   // This is user data for subclasses.
   const void *Data;
 
----------------
I really don't want to increase the size of CallEvent. How about using `Origin` to either represent a CXXDestructorDecl or a CXXDeleteExpr?

================
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1714-1715
@@ +1713,4 @@
+  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();
----------------
This should be spelled `if (const CXXDeleteExpr *DE = dyn_cast<CXXDeleteExpr>(DC->getTriggerStmt()))` (or `getOriginExpr()`).

================
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1717
@@ +1716,3 @@
+      SymbolRef Sym = C.getSVal(DE->getArgument()).getAsSymbol();
+      if (!Sym || checkUseAfterFree(Sym, C, DE->getArgument()))
+        return;
----------------
Can we get it to give the "double delete" message instead of the "use after free" message?

================
Comment at: lib/StaticAnalyzer/Core/PathDiagnostic.cpp:769-775
@@ -741,1 +768,9 @@
 
+  // 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);
+    }
+  }
+
----------------
I think rather than worry about finding the end of the previous statement, we should just use the CXXDeleteExpr as the (non-implicit) location. That's fully-recoverable from the information we have.


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