[PATCH] [StaticAnalyzer]Handle Destructor call generated by C++ delete expr

[Karthik Bhat]

  Hi Jordan,
  Thanks for the review. Implemented review comments.

  I think it makes sense to have function getDeleteExpr as const as the caller is not supposed to modify any member in the class. Removed the non const version of getDeleteExpr from CFG.h.

  Coming onto test cases-

  Except for -

  Use of delete[], which should at least run the destructor for the first element.

  all other test cases seem to pass.
  There seems to be some problem in VisitCXXDestructor as a result array types are not handled properly.
  This can be observed even in case of automatic destructor called on an array of class objects.
  I will debug more and try to address this in next revision.

  For now does this patch look good?

  Thanks
  Karthik Bhat

Hi jordan_rose,

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

CHANGE SINCE LAST DIFF
  http://llvm-reviews.chandlerc.com/D1594?vs=4014&id=4055#toc

Files:
  test/Analysis/new.cpp
  include/clang/Analysis/CFG.h
  lib/StaticAnalyzer/Core/CallEvent.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp

Index: test/Analysis/new.cpp
===================================================================
--- test/Analysis/new.cpp
+++ test/Analysis/new.cpp
@@ -206,3 +206,80 @@
   }
   return 1;
 }
+
+// Test modelling destructor call on call to delete
+class DestTest{
+  public:
+    int y;
+    int k;
+    DestTest() {};
+    ~DestTest() {k = k/y;}; //expected-warning{{Division by zero}}
+};
+
+void testCallToDestructor() {
+  DestTest* b = new DestTest();
+  b->y = 0;
+  b->k = 1;
+  delete b; // This results in divide by zero in destructor
+}
+
+// Test Deleting a value that's passed as an argument.
+class ArgDestTest {
+ public:
+  int x;
+  int y;
+  ArgDestTest() {}
+  ~ArgDestTest() {x = x/y;} //expected-warning{{Division by zero}}
+};
+
+void Fn(ArgDestTest* arg) {
+  delete arg; 
+}
+
+void test_delte_dtor_Arg() {
+  ArgDestTest* arg = new ArgDestTest();
+  arg->x = 1;
+  arg->y = 0;
+  Fn(arg); 
+}
+
+//Deleting the address of a local variable, null pointer
+void abort(void) __attribute__((noreturn));
+
+class LocalDestTest {
+ public:
+  LocalDestTest() {}
+  ~LocalDestTest() {abort();}
+};
+void test_delte_dtor_LocalVar() {
+  LocalDestTest* arg = 0;
+  LocalDestTest test;
+  delete arg;  // no warn or crash
+  delete &test;
+}
+
+//Deleting a non class pointer
+void test_dtor_reinterpretCast() {
+  int *v = new int(10);
+  int x = 0;
+  int y = 1;
+  LocalDestTest* test = reinterpret_cast<LocalDestTest*>(v);
+  delete test; // Destructor not called hence should not abort
+  x = y/x;  // expected-warning{{Division by zero}}
+}
+
+// Invalidate Region even in case of default destructor
+class InvalidateDestTest {
+  public:
+   int x;
+   ~InvalidateDestTest();
+};
+int test_member_invalidation() {
+  InvalidateDestTest* test = new InvalidateDestTest();
+  test->x = 5;
+  int* k = &(test->x);
+  clang_analyzer_eval(*k == 5); // expected-warning{{TRUE}}
+  delete test;
+  clang_analyzer_eval(*k == 5); // expected-warning{{UNKNOWN}}
+  return 0;
+}
Index: include/clang/Analysis/CFG.h
===================================================================
--- include/clang/Analysis/CFG.h
+++ include/clang/Analysis/CFG.h
@@ -200,7 +200,7 @@
   }
 
   // Get Delete expression which triggered the destructor call.
-  const CXXDeleteExpr *getDeleteExpr() {
+  const CXXDeleteExpr *getDeleteExpr() const {
     return static_cast<CXXDeleteExpr *>(Data2.getPointer());
   }
 
Index: lib/StaticAnalyzer/Core/CallEvent.cpp
===================================================================
--- lib/StaticAnalyzer/Core/CallEvent.cpp
+++ lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -957,6 +957,8 @@
   const Stmt *Trigger;
   if (Optional<CFGAutomaticObjDtor> AutoDtor = E.getAs<CFGAutomaticObjDtor>())
     Trigger = AutoDtor->getTriggerStmt();
+  else if (Optional<CFGDeleteDtor> DeleteDtor = E.getAs<CFGDeleteDtor>())
+    Trigger = cast<Stmt>(DeleteDtor->getDeleteExpr());
   else
     Trigger = Dtor->getBody();
 
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -569,7 +569,14 @@
 void ExprEngine::ProcessDeleteDtor(const CFGDeleteDtor Dtor,
                                    ExplodedNode *Pred,
                                    ExplodedNodeSet &Dst) {
-  //TODO: Handle DeleteDtor
+  const LocationContext *LCtx = Pred->getLocationContext();
+  const CXXDeleteExpr *DE = Dtor.getDeleteExpr();
+  const Stmt *Arg = cast<Stmt>(DE->getArgument());
+  SVal ArgVal = Pred->getState()->getSVal(Arg, LCtx);
+  VisitCXXDestructor(DE->getDestroyedType(),
+                     ArgVal.getAsRegion(),
+                     DE, /*IsBase=*/ false,
+                     Pred, Dst);
 }
 
 void ExprEngine::ProcessBaseDtor(const CFGBaseDtor D,
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D1594.2.patch
Type: text/x-patch
Size: 3820 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130905/9dc8159c/attachment.bin>