[analyzer] Inline destructors for non-array deletes.

[NoQ]

Summary

Similarly to how we allow (since D40560) inlining the constructor after operator new which isn't operator new[], even if the target region is an ElementRegion (which doesn't necessarily represent an array element - it may represent a result of pointer arithmetic or a cast), we should allow inlining the destructors for non-array-but-still-element regions, but not when they are part of operator delete[]. There aren't any known issues in this situation. We still aren't attempting to model array new/delete because it requires an unknown amount of constructor calls to be modeled symbolically.

Before the patch, in new.cpp tests testCallToDestructor() and test_delete_dtor_Arg() started failing under -analyzer-config c++-allocator-inlining=true, because the new behavior of operator new is to return an ElementRegion surrounding the void pointer, which disables destructor inlining; the old behavior is to call the destructor over a raw void pointer, which kind of worked. Additionally, some fixmes in new.cpp were fixed in the new mode. The change in testPlacementNew() also seems correct, even if it wasn't marked as a FIXME.

[NoQ]

lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp

692–693

I hope i'd be able to get rid of this ParentMap call - once i update D40560 as discussed there and rebase.

[NoQ]

because the new behavior of operator new is to return an ElementRegion surrounding the void pointer

The new behavior was reverted in D41250#971888 so this patch is no longer useful.

lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp

case CE_CXXDestructor: {
     const AnalysisDeclContext *ADC = CallerSFC->getAnalysisDeclContext();
     assert(ADC->getCFGBuildOptions().AddImplicitDtors && "No CFG destructors");
     (void)ADC;
 
     const CXXDestructorCall &Dtor = cast<CXXDestructorCall>(Call);
 
     // FIXME: We don't handle constructors or destructors for arrays properly.
     const MemRegion *Target = Dtor.getCXXThisVal().getAsRegion();
-    if (Target && isa<ElementRegion>(Target))
+    if (Target && isa<ElementRegion>(Target)) {
+      if (const Stmt *DtorExpr = Dtor.getOriginExpr())
+        if (const Stmt *ParentExpr =
+                CurLC->getParentMap().getParent(DtorExpr))
+          if (const CXXDeleteExpr *DeleteExpr =
+                  dyn_cast<CXXDeleteExpr>(ParentExpr))
+            if (DeleteExpr->isArrayForm())
               return CIP_DisallowedOnce;
 
+      if (const TypedValueRegion *TR = dyn_cast<TypedValueRegion>(
+              cast<SubRegion>(Target)->getSuperRegion()))
+        if (TR->getValueType()->isArrayType())
+          return CIP_DisallowedOnce;
+    }
+
     break;
   }
   case CE_CXXAllocator:
     if (Opts.mayInlineCXXAllocator())
       break;
     // Do not inline allocators until we model deallocators.
     // This is unfortunate, but basically necessary for smart pointers and such.
     return CIP_DisallowedAlways;

test/Analysis/new.cpp

 // RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-store region -std=c++11 -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-store region -analyzer-config c++-allocator-inlining=true -DALLOCATOR_INLINING=1 -std=c++11 -verify %s
 #include "Inputs/system-header-simulator-cxx.h"
 
 void clang_analyzer_eval(bool);
 
 typedef __typeof__(sizeof(int)) size_t;
 extern "C" void *malloc(size_t);
 extern "C" void free(void *);
 

 
 void *testPlacementNew() {
   int *x = (int *)malloc(sizeof(int));
   *x = 1;
   clang_analyzer_eval(*x == 1); // expected-warning{{TRUE}};
 
   void *y = new (x) int;
   clang_analyzer_eval(x == y); // expected-warning{{TRUE}};
-  clang_analyzer_eval(*x == 1); // expected-warning{{UNKNOWN}};
+  clang_analyzer_eval(*x == 1);
+#if ALLOCATOR_INLINING
+  // expected-warning@-2{{TRUE}};
+#else
+  // expected-warning@-4{{UNKNOWN}};
+#endif
 
   return y;
 }
 
 void *operator new(size_t, size_t, int *);
 void *testCustomNew() {
   int x[1] = {1};
   clang_analyzer_eval(*x == 1); // expected-warning{{TRUE}};
int testNoInitialization() {
   return 1;
 }
 
 int testNoInitializationPlacement() {
   int n;
   new (&n) int;
 
   // Should warn that n is uninitialized.
-  if (n) { // no-warning
+  if (n) {
+#if ALLOCATOR_INLINING
+    // expected-warning@-2{{Branch condition evaluates to a garbage value}}
+#endif
     return 0;
   }
   return 1;
 }
 
 // Test modelling destructor call on call to delete
 class IntPair{
 public:
void testArrayNull() {
   NoReturnDtor *fooArray = 0;
   delete[] fooArray; // should not call destructor, checked below
   clang_analyzer_eval(true); // expected-warning{{TRUE}}
 }
 
 void testArrayDestr() {
   NoReturnDtor *p = new NoReturnDtor[2];
   delete[] p; // Calls the base destructor which aborts, checked below
-  //TODO: clang_analyzer_eval should not be called
-  clang_analyzer_eval(true); // expected-warning{{TRUE}}
+  clang_analyzer_eval(true);
 }
 
 // Invalidate Region even in case of default destructor
 class InvalidateDestTest {
 public:
   int x;
   int *y;
   ~InvalidateDestTest();