[analyzer] Don't crash on array constructors and destructors.

This workaround is fairly lame: we simulate the first element's constructor
and destructor and rely on the region invalidation to "initialize" the rest
of the elements.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@160809 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/StaticAnalyzer/Core/ExprEngineC.cpp b/lib/StaticAnalyzer/Core/ExprEngineC.cpp
index 4bcc0fbd100ca8d622d45daca3374decd910b04a..fd467c2b7aad23f72d86111569325af7f7c96b8b 100644 (file)
--- a/lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -453,16 +453,17 @@ void ExprEngine::VisitDeclStmt(const DeclStmt *DS, ExplodedNode *Pred,
     const LocationContext *LC = N->getLocationContext();
     
     if (const Expr *InitEx = VD->getInit()) {
-      SVal InitVal = state->getSVal(InitEx, Pred->getLocationContext());
+      SVal InitVal = state->getSVal(InitEx, LC);
 
-      if (InitVal == state->getLValue(VD, LC)) {
+      if (InitVal == state->getLValue(VD, LC) ||
+          (VD->getType()->isArrayType() &&
+           isa<CXXConstructExpr>(InitEx->IgnoreImplicit()))) {
         // We constructed the object directly in the variable.
         // No need to bind anything.
         B.generateNode(DS, N, state);
       } else {
         // We bound the temp obj region to the CXXConstructExpr. Now recover
         // the lazy compound value when the variable is not a reference.
-        // FIXME: This is probably not correct for most constructors!
         if (AMgr.getLangOpts().CPlusPlus && VD->getType()->isRecordType() && 
             !VD->getType()->isReferenceType() && isa<loc::MemRegionVal>(InitVal)){
           InitVal = state->getSVal(cast<loc::MemRegionVal>(InitVal).getRegion());

diff --git a/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
index 88fba2919e81d231d9be17e50a62f21aa12203ce..bb7f8c7596bc40c02aba16c78367379b453b53ab 100644 (file)
--- a/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -57,12 +57,27 @@ void ExprEngine::VisitCXXConstructExpr(const CXXConstructExpr *CE,
       CFGElement Next = (*B)[currentStmtIdx+1];
 
       // Is this a constructor for a local variable?
-      if (const CFGStmt *StmtElem = dyn_cast<CFGStmt>(&Next))
-        if (const DeclStmt *DS = dyn_cast<DeclStmt>(StmtElem->getStmt()))
-          if (const VarDecl *Var = dyn_cast<VarDecl>(DS->getSingleDecl()))
-            if (Var->getInit() == CE)
-              Target = State->getLValue(Var, LCtx).getAsRegion();
-
+      if (const CFGStmt *StmtElem = dyn_cast<CFGStmt>(&Next)) {
+        if (const DeclStmt *DS = dyn_cast<DeclStmt>(StmtElem->getStmt())) {
+          if (const VarDecl *Var = dyn_cast<VarDecl>(DS->getSingleDecl())) {
+            if (Var->getInit()->IgnoreImplicit() == CE) {
+              QualType Ty = Var->getType();
+              if (const ArrayType *AT = getContext().getAsArrayType(Ty)) {
+                // FIXME: Handle arrays, which run the same constructor for
+                // every element. This workaround will just run the first
+                // constructor (which should still invalidate the entire array).
+                SVal Base = State->getLValue(Var, LCtx);
+                Target = State->getLValue(AT->getElementType(),
+                                          getSValBuilder().makeZeroArrayIndex(),
+                                          Base).getAsRegion();
+              } else {
+                Target = State->getLValue(Var, LCtx).getAsRegion();
+              }
+            }
+          }
+        }
+      }
+      
       // Is this a constructor for a member?
       if (const CFGInitializer *InitElem = dyn_cast<CFGInitializer>(&Next)) {
         const CXXCtorInitializer *Init = InitElem->getInitializer();
@@ -75,10 +90,10 @@ void ExprEngine::VisitCXXConstructExpr(const CXXConstructExpr *CE,
 
         if (Init->isIndirectMemberInitializer()) {
           SVal Field = State->getLValue(Init->getIndirectMember(), ThisVal);
-          Target = cast<loc::MemRegionVal>(Field).getRegion();
+          Target = Field.getAsRegion();
         } else {
           SVal Field = State->getLValue(Init->getMember(), ThisVal);
-          Target = cast<loc::MemRegionVal>(Field).getRegion();
+          Target = Field.getAsRegion();
         }
       }
 
@@ -104,7 +119,7 @@ void ExprEngine::VisitCXXConstructExpr(const CXXConstructExpr *CE,
       // Cast to the base type.
       QualType BaseTy = CE->getType();
       SVal BaseVal = getStoreManager().evalDerivedToBase(ThisVal, BaseTy);
-      Target = cast<loc::MemRegionVal>(BaseVal).getRegion();
+      Target = BaseVal.getAsRegion();
     }
     break;
   }
@@ -135,6 +150,16 @@ void ExprEngine::VisitCXXDestructor(QualType ObjectType,
                                     const Stmt *S,
                                     ExplodedNode *Pred, 
                                     ExplodedNodeSet &Dst) {
+  // FIXME: We need to run the same destructor on every element of the array.
+  // This workaround will just run the first destructor (which will still
+  // invalidate the entire array).
+  if (const ArrayType *AT = getContext().getAsArrayType(ObjectType)) {
+    ObjectType = AT->getElementType();
+    Dest = Pred->getState()->getLValue(ObjectType,
+                                       getSValBuilder().makeZeroArrayIndex(),
+                                       loc::MemRegionVal(Dest)).getAsRegion();
+  }
+
   const CXXRecordDecl *RecordDecl = ObjectType->getAsCXXRecordDecl();
   assert(RecordDecl && "Only CXXRecordDecls should have destructors");
   const CXXDestructorDecl *DtorDecl = RecordDecl->getDestructor();

diff --git a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
index bc6aeaa0368da1cf86e812580cb1b111d0480f87..fb6a9e0e7de77737ee88e0aab87229b4fb904f47 100644 (file)
--- a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -306,14 +306,18 @@ bool ExprEngine::inlineCall(const CallEvent &Call,
         !ADC->getCFGBuildOptions().AddInitializers)
       return false;
 
+    // FIXME: We don't handle constructors or destructors for arrays properly.
+    const MemRegion *Target = Call.getCXXThisVal().getAsRegion();
+    if (Target && isa<ElementRegion>(Target))
+      return false;
+
     // FIXME: This is a hack. We don't handle temporary destructors
     // right now, so we shouldn't inline their constructors.
     if (const CXXConstructorCall *Ctor = dyn_cast<CXXConstructorCall>(&Call)) {
       const CXXConstructExpr *CtorExpr = Ctor->getOriginExpr();
       if (CtorExpr->getConstructionKind() == CXXConstructExpr::CK_Complete)
-        if (const MemRegion *Target = Ctor->getCXXThisVal().getAsRegion())
-          if (!isa<DeclRegion>(Target))
-            return false;
+        if (!Target || !isa<DeclRegion>(Target))
+          return false;
     }
     break;
   }
@@ -461,6 +465,8 @@ ProgramStateRef ExprEngine::bindReturnValue(const CallEvent &Call,
       return State->BindExpr(E, LCtx, Msg->getReceiverSVal());
     }
     }
+  } else if (const CXXConstructorCall *C = dyn_cast<CXXConstructorCall>(&Call)){
+    return State->BindExpr(E, LCtx, C->getCXXThisVal());
   }
 
   // Conjure a symbol if the return value is unknown.

diff --git a/test/Analysis/dtor.cpp b/test/Analysis/dtor.cpp
index f5837539cb7b41fd5aabec16e4ef4491037b6251..4e3c0017f4a92efc0ef1197e1028d48d65ca328a 100644 (file)
--- a/test/Analysis/dtor.cpp
+++ b/test/Analysis/dtor.cpp
@@ -1,4 +1,6 @@
-// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc -analyzer-store region -analyzer-ipa=inlining -cfg-add-implicit-dtors -cfg-add-initializers -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-store region -analyzer-ipa=inlining -cfg-add-implicit-dtors -cfg-add-initializers -verify %s
+
+void clang_analyzer_eval(bool);
 
 class A {
 public:
@@ -100,7 +102,7 @@ void testMultipleInheritance3() {
     // Remove dead bindings...
     doSomething();
     // destructor called here
-    // expected-warning@25 {{Attempt to free released memory}}
+    // expected-warning@27 {{Attempt to free released memory}}
   }
 }
 
@@ -121,3 +123,34 @@ void testSmartPointerMember() {
   }
   *mem = 0; // expected-warning{{Use of memory after it is freed}}
 }
+
+
+struct IntWrapper {
+  IntWrapper() : x(0) {}
+  ~IntWrapper();
+  int *x;
+};
+
+void testArrayInvalidation() {
+  int i = 42;
+  int j = 42;
+
+  {
+    IntWrapper arr[2];
+
+    // There should be no undefined value warnings here.
+    // Eventually these should be TRUE as well, but right now
+    // we can't handle array constructors.
+    clang_analyzer_eval(arr[0].x == 0); // expected-warning{{UNKNOWN}}
+    clang_analyzer_eval(arr[1].x == 0); // expected-warning{{UNKNOWN}}
+
+    arr[0].x = &i;
+    arr[1].x = &j;
+    clang_analyzer_eval(*arr[0].x == 42); // expected-warning{{TRUE}}
+    clang_analyzer_eval(*arr[1].x == 42); // expected-warning{{TRUE}}
+  }
+
+  // The destructors should have invalidated i and j.
+  clang_analyzer_eval(i == 42); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(j == 42); // expected-warning{{UNKNOWN}}
+}