1 files changed, 68 insertions, 24 deletions

diff --git a/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
index ed90dc5..eba4f94 100644
--- a/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -30,21 +30,7 @@ void ExprEngine::CreateCXXTemporaryObject(const MaterializeTemporaryExpr *ME,
   ProgramStateRef state = Pred->getState();
   const LocationContext *LCtx = Pred->getLocationContext();
 
-  SVal V = state->getSVal(tempExpr, LCtx);
-
-  // If the value is already a CXXTempObjectRegion, it is fine as it is.
-  // Otherwise, create a new CXXTempObjectRegion, and copy the value into it.
-  // This is an optimization for when an rvalue is constructed and then
-  // immediately materialized.
-  const MemRegion *MR = V.getAsRegion();
-  if (const CXXTempObjectRegion *TR =
-        dyn_cast_or_null<CXXTempObjectRegion>(MR)) {
-    if (getContext().hasSameUnqualifiedType(TR->getValueType(), ME->getType()))
-      state = state->BindExpr(ME, LCtx, V);
-  }
-
-  if (state == Pred->getState())
-    state = createTemporaryRegionIfNeeded(state, LCtx, tempExpr, ME);
+  state = createTemporaryRegionIfNeeded(state, LCtx, tempExpr, ME);
   Bldr.generateNode(ME, Pred, state);
 }
 
@@ -105,6 +91,12 @@ void ExprEngine::performTrivialCopy(NodeBuilder &Bldr, ExplodedNode *Pred,
 /// 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;
+
   SValBuilder &SVB = State->getStateManager().getSValBuilder();
   ASTContext &Ctx = SVB.getContext();
 
@@ -176,6 +168,7 @@ void ExprEngine::VisitCXXConstructExpr(const CXXConstructExpr *CE,
       }
 
       // FIXME: This will eventually need to handle new-expressions as well.
+      // Don't forget to update the pre-constructor initialization code below.
     }
 
     // If we couldn't find an existing region to construct into, assume we're
@@ -187,8 +180,26 @@ void ExprEngine::VisitCXXConstructExpr(const CXXConstructExpr *CE,
 
     break;
   }
-  case CXXConstructExpr::CK_NonVirtualBase:
   case CXXConstructExpr::CK_VirtualBase:
+    // Make sure we are not calling virtual base class initializers twice.
+    // Only the most-derived object should initialize virtual base classes.
+    if (const Stmt *Outer = LCtx->getCurrentStackFrame()->getCallSite()) {
+      const CXXConstructExpr *OuterCtor = dyn_cast<CXXConstructExpr>(Outer);
+      if (OuterCtor) {
+        switch (OuterCtor->getConstructionKind()) {
+        case CXXConstructExpr::CK_NonVirtualBase:
+        case CXXConstructExpr::CK_VirtualBase:
+          // Bail out!
+          destNodes.Add(Pred);
+          return;
+        case CXXConstructExpr::CK_Complete:
+        case CXXConstructExpr::CK_Delegating:
+          break;
+        }
+      }
+    }
+    // FALLTHROUGH
+  case CXXConstructExpr::CK_NonVirtualBase:
   case CXXConstructExpr::CK_Delegating: {
     const CXXMethodDecl *CurCtor = cast<CXXMethodDecl>(LCtx->getDecl());
     Loc ThisPtr = getSValBuilder().getCXXThis(CurCtor,
@@ -215,8 +226,38 @@ void ExprEngine::VisitCXXConstructExpr(const CXXConstructExpr *CE,
 
   ExplodedNodeSet DstPreVisit;
   getCheckerManager().runCheckersForPreStmt(DstPreVisit, Pred, CE, *this);
+
+  ExplodedNodeSet PreInitialized;
+  {
+    StmtNodeBuilder Bldr(DstPreVisit, PreInitialized, *currBldrCtx);
+    if (CE->requiresZeroInitialization()) {
+      // Type of the zero doesn't matter.
+      SVal ZeroVal = svalBuilder.makeZeroVal(getContext().CharTy);
+
+      for (ExplodedNodeSet::iterator I = DstPreVisit.begin(),
+                                     E = DstPreVisit.end();
+           I != E; ++I) {
+        ProgramStateRef State = (*I)->getState();
+        // FIXME: Once we properly handle constructors in new-expressions, we'll
+        // need to invalidate the region before setting a default value, to make
+        // sure there aren't any lingering bindings around. This probably needs
+        // to happen regardless of whether or not the object is zero-initialized
+        // to handle random fields of a placement-initialized object picking up
+        // old bindings. We might only want to do it when we need to, though.
+        // FIXME: This isn't actually correct for arrays -- we need to zero-
+        // initialize the entire array, not just the first element -- but our
+        // handling of arrays everywhere else is weak as well, so this shouldn't
+        // actually make things worse. Placement new makes this tricky as well,
+        // since it's then possible to be initializing one part of a multi-
+        // dimensional array.
+        State = State->bindDefault(loc::MemRegionVal(Target), ZeroVal);
+        Bldr.generateNode(CE, *I, State, /*tag=*/0, ProgramPoint::PreStmtKind);
+      }
+    }
+  }
+
   ExplodedNodeSet DstPreCall;
-  getCheckerManager().runCheckersForPreCall(DstPreCall, DstPreVisit,
+  getCheckerManager().runCheckersForPreCall(DstPreCall, PreInitialized,
                                             *Call, *this);
 
   ExplodedNodeSet DstEvaluated;
@@ -255,7 +296,9 @@ void ExprEngine::VisitCXXDestructor(QualType ObjectType,
   // 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).
-  SVal DestVal = loc::MemRegionVal(Dest);
+  SVal DestVal = UnknownVal();
+  if (Dest)
+    DestVal = loc::MemRegionVal(Dest);
   DestVal = makeZeroElementRegion(State, DestVal, ObjectType);
   Dest = DestVal.getAsRegion();
 
@@ -332,11 +375,14 @@ void ExprEngine::VisitCXXNewExpr(const CXXNewExpr *CNE, ExplodedNode *Pred,
   if (!State)
     return;
 
-  // If we're compiling with exceptions enabled, and this allocation function
-  // is not declared as non-throwing, failures /must/ be signalled by
-  // exceptions, and thus the return value will never be NULL.
+  // If this allocation function is not declared as non-throwing, failures
+  // /must/ be signalled by exceptions, and thus the return value will never be
+  // NULL. -fno-exceptions does not influence this semantics.
+  // FIXME: GCC has a -fcheck-new option, which forces it to consider the case
+  // where new can return NULL. If we end up supporting that option, we can
+  // consider adding a check for it here.
   // C++11 [basic.stc.dynamic.allocation]p3.
-  if (FD && getContext().getLangOpts().CXXExceptions) {
+  if (FD) {
     QualType Ty = FD->getType();
     if (const FunctionProtoType *ProtoType = Ty->getAs<FunctionProtoType>())
       if (!ProtoType->isNothrow(getContext()))
@@ -382,8 +428,6 @@ void ExprEngine::VisitCXXNewExpr(const CXXNewExpr *CNE, ExplodedNode *Pred,
     if (!isa<CXXConstructExpr>(Init)) {
       assert(Bldr.getResults().size() == 1);
       Bldr.takeNodes(NewN);
-
-      assert(!CNE->getType()->getPointeeCXXRecordDecl());
       evalBind(Dst, CNE, NewN, Result, State->getSVal(Init, LCtx),
                /*FirstInit=*/IsStandardGlobalOpNewFunction);
     }