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

[Karthik Bhat]

================
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;
 
----------------
Jordan Rose wrote:
> I really don't want to increase the size of CallEvent. How about using `Origin` to either represent a CXXDestructorDecl or a CXXDeleteExpr?
Hi Jordan,
Origin already contains CXXDestructorDecl hence CallEvent->getDecl() will return CXXDestructorDecl but we will not be able to get the CXXDeleteExpr which triggered it. 
CXXDeleteExpr is required while reporting warning in MallocChecker as we are checking if argument to delete is already deleted or not.
If we try to save CXXDeleteExpr in Origin instead then we are will face problem when we try to inline code inside destructor call as CallEvent->getDecl() will be null in this case.

I assume Origin can have only 1 info at a time i.e. either Decl* or Expr* but in this case we might need both. Hence added a new Stmt to represent the triggered stmt.

I'm still trying to get a hold of clang SA code so please let me know if i'm wrong somewere or if there is some way to extract CXXDeleteExpr from CXXDestructorDecl. I tried to have a look but couldnt find one.

================
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);
+    }
+  }
+
----------------
Jordan Rose wrote:
> 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.
Hi Jordan,
I think we cannot get CXXDeleteExpr here without traversing pred nodes in this function as we only have ExplodedNode to operate with. This function is called while we try to emit a bugReport.
I think the problem in this particular case is for a code like -
 delete a;
what we are actually trying to report an error at 
 a->~A()
which is implicitly called but doest have a Stmt associated with it in which case S is null.
Please let me know if i'm missing something.
Thanks for the help!


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