[PATCH] [analyzer] Fix crash with temporary destructors

[Pavel Labath]

Hi jordan_rose,

It turns out that moving the test out of the loop in makeZeroElementRegion was
not a valid optimization after all, since one of the side effects of the loop is
changing the element type.

To avoid this happening in the future I have changed the function to return the type,
instead of using by-ref parameter.

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

Files:
  lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  test/Analysis/temporaries.cpp

Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -84,28 +84,24 @@
 
 
 /// Returns a region representing the first element of a (possibly
-/// multi-dimensional) array.
-///
-/// On return, \p Ty will be set to the base type of the array.
+/// multi-dimensional) array and its type.
 ///
 /// If the type is not an array type at all, the original value is returned.
-static SVal makeZeroElementRegion(ProgramStateRef State, SVal LValue,
-                                  QualType &Ty) {
-  // FIXME: This check is just a temporary workaround, because
-  // ProcessTemporaryDtor sends us NULL regions. It will not be necessary once
-  // we can properly process temporary destructors.
-  if (!LValue.getAsRegion())
-    return LValue;
-
+static std::pair<SVal, QualType>
+makeZeroElementRegion(ProgramStateRef State, SVal LValue, QualType Ty) {
   SValBuilder &SVB = State->getStateManager().getSValBuilder();
   ASTContext &Ctx = SVB.getContext();
 
   while (const ArrayType *AT = Ctx.getAsArrayType(Ty)) {
     Ty = AT->getElementType();
-    LValue = State->getLValue(Ty, SVB.makeZeroArrayIndex(), LValue);
+    // FIXME: This check is just a temporary workaround, because
+    // ProcessTemporaryDtor sends us NULL regions. It will not be necessary once
+    // we can properly process temporary destructors.
+    if (LValue.getAsRegion())
+      LValue = State->getLValue(Ty, SVB.makeZeroArrayIndex(), LValue);
   }
 
-  return LValue;
+  return std::make_pair(LValue, Ty);
 }
 
 void ExprEngine::VisitCXXConstructExpr(const CXXConstructExpr *CE,
@@ -135,7 +131,7 @@
             if (Var->getInit()->IgnoreImplicit() == CE) {
               SVal LValue = State->getLValue(Var, LCtx);
               QualType Ty = Var->getType();
-              LValue = makeZeroElementRegion(State, LValue, Ty);
+              LValue = makeZeroElementRegion(State, LValue, Ty).first;
               Target = LValue.getAsRegion();
             }
           }
@@ -163,7 +159,7 @@
         }
 
         QualType Ty = Field->getType();
-        FieldVal = makeZeroElementRegion(State, FieldVal, Ty);
+        FieldVal = makeZeroElementRegion(State, FieldVal, Ty).first;
         Target = FieldVal.getAsRegion();
       }
 
@@ -297,7 +293,8 @@
   // This workaround will just run the first destructor (which will still
   // invalidate the entire array).
   SVal DestVal = loc::MemRegionVal(Dest);
-  DestVal = makeZeroElementRegion(State, DestVal, ObjectType);
+  llvm::tie(DestVal, ObjectType) =
+      makeZeroElementRegion(State, DestVal, ObjectType);
   Dest = DestVal.getAsRegion();
 
   const CXXRecordDecl *RecordDecl = ObjectType->getAsCXXRecordDecl();
Index: test/Analysis/temporaries.cpp
===================================================================
--- test/Analysis/temporaries.cpp
+++ test/Analysis/temporaries.cpp
@@ -1,6 +1,8 @@
 // RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -verify -w -std=c++03 %s
 // RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -verify -w -std=c++11 %s
-// RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -verify -w -analyzer-config cfg-temporary-dtors=true %s -DTEMPORARY_DTORS
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -verify -w -std=c++11 -analyzer-config cfg-temporary-dtors=true %s -DTEMPORARY_DTORS
+
+#include "Inputs/system-header-simulator-cxx.h"
 
 extern bool clang_analyzer_eval(bool);
 
@@ -247,3 +249,16 @@
     clang_analyzer_eval(x == 47); // expected-warning{{TRUE}}
   }
 }
+
+#if __cplusplus >= 201103L
+namespace array_destructors {
+  struct Triple {
+    ~Triple();
+  };
+
+  void f() {
+    Triple triple;
+    std::initializer_list<Triple> a{triple};
+  }
+}
+#endif
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D1615.1.patch
Type: text/x-patch
Size: 3886 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130906/23bb1e70/attachment.bin>