[analyzer] pr37270: Fix binding constructed object to DeclStmt when ConstructionContext is already lost.

[NoQ]

Summary

As explained in http://lists.llvm.org/pipermail/cfe-dev/2018-May/058055.html, findDirectConstructorForCurrentCFGElement() was not working correctly in the current example and erroneously made us think that we've constructed into a dummy temporary region rather than into the variable. Instead, it was proposed to track it in the program state every time we are performing construction correctly.

Additionally this information will be used to maintain liveness of the variable's Store bindings, because previously an incorrect Environment binding of the target region to the construct-expression was used for that purpose. Such binding is incorrect because the constructor is a prvalue of an object type and should therefore have a NonLoc representing its symbolic value. Therefore the hack implemented by isTemporaryPRValue() can be safely removed.

findDirectConstructorForCurrentCFGElement() cannot be removed yet because it is also used for constructor member initializers for the same purpose. It doesn't seem to introduce bugs though.

cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp

for (ExplodedNodeSet::iterator I = dstPreVisit.begin(), E = dstPreVisit.end();
     // Decls without InitExpr are not initialized explicitly.
     if (const Expr *InitEx = VD->getInit()) {
 
       // Note in the state that the initialization has occurred.
       ExplodedNode *UpdatedN = N;
       SVal InitVal = state->getSVal(InitEx, LC);
 
       assert(DS->isSingleDecl());
-      if (auto *CtorExpr = findDirectConstructorForCurrentCFGElement()) {
-        assert(InitEx->IgnoreImplicit() == CtorExpr);
-        (void)CtorExpr;
+      if (getObjectUnderConstruction(state, DS, LC)) {
+        state = finishObjectConstruction(state, DS, LC);
         // We constructed the object directly in the variable.
         // No need to bind anything.
         B.generateNode(DS, UpdatedN, state);
       } else {
-        // We bound the temp obj region to the CXXConstructExpr. Now recover
-        // the lazy compound value when the variable is not a reference.
-        if (AMgr.getLangOpts().CPlusPlus && VD->getType()->isRecordType() &&
-            !VD->getType()->isReferenceType()) {
-          if (Optional<loc::MemRegionVal> M =
-                  InitVal.getAs<loc::MemRegionVal>()) {
-            InitVal = state->getSVal(M->getRegion());
-            assert(InitVal.getAs<nonloc::LazyCompoundVal>());
-          }
-        }
-
         // Recover some path-sensitivity if a scalar value evaluated to
         // UnknownVal.
         if (InitVal.isUnknown()) {
           QualType Ty = InitEx->getType();
           if (InitEx->isGLValue()) {
             Ty = getContext().getPointerType(Ty);
           }
 

cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp

std::pair<ProgramStateRef, SVal> ExprEngine::prepareForObjectConstruction(
   if (CC) {
     switch (CC->getKind()) {
     case ConstructionContext::SimpleVariableKind: {
       const auto *DSCC = cast<SimpleVariableConstructionContext>(CC);
       const auto *DS = DSCC->getDeclStmt();
       const auto *Var = cast<VarDecl>(DS->getSingleDecl());
       SVal LValue = State->getLValue(Var, LCtx);
       QualType Ty = Var->getType();
-      return std::make_pair(
-          State,
-          makeZeroElementRegion(State, LValue, Ty, CallOpts.IsArrayCtorOrDtor));
+      LValue =
+          makeZeroElementRegion(State, LValue, Ty, CallOpts.IsArrayCtorOrDtor);
+      State =
+          addObjectUnderConstruction(State, DSCC->getDeclStmt(), LCtx, LValue);
+      return std::make_pair(State, LValue);
     }
     case ConstructionContext::SimpleConstructorInitializerKind: {
       const auto *ICC = cast<ConstructorInitializerConstructionContext>(CC);
       const auto *Init = ICC->getCXXCtorInitializer();
       assert(Init->isAnyMemberInitializer());
       const CXXMethodDecl *CurCtor = cast<CXXMethodDecl>(LCtx->getDecl());
       Loc ThisPtr =
       getSValBuilder().getCXXThis(CurCtor, LCtx->getCurrentStackFrame());

cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp

static bool wasDifferentDeclUsedForInlining(CallEventRef<> Call,
   const Decl *RuntimeCallee = calleeCtx->getDecl();
   const Decl *StaticDecl = Call->getDecl();
   assert(RuntimeCallee);
   if (!StaticDecl)
     return true;
   return RuntimeCallee->getCanonicalDecl() != StaticDecl->getCanonicalDecl();
 }
 
-/// Returns true if the CXXConstructExpr \p E was intended to construct a
-/// prvalue for the region in \p V.
-///
-/// Note that we can't just test for rvalue vs. glvalue because
-/// CXXConstructExprs embedded in DeclStmts and initializers are considered
-/// rvalues by the AST, and the analyzer would like to treat them as lvalues.
-static bool isTemporaryPRValue(const CXXConstructExpr *E, SVal V) {
-  if (E->isGLValue())
-    return false;
-
-  const MemRegion *MR = V.getAsRegion();
-  if (!MR)
-    return false;
-
-  return isa<CXXTempObjectRegion>(MR);
-}
-
 /// The call exit is simulated with a sequence of nodes, which occur between
 /// CallExitBegin and CallExitEnd. The following operations occur between the
 /// two program points:
 /// 1. CallExitBegin (triggers the start of call exit sequence)
 /// 2. Bind the return value
 /// 3. Run Remove dead bindings to clean up the dead symbols from the callee.
 /// 4. CallExitEnd (switch to the caller context)
 /// 5. PostStmt<CallExpr>
if (const ReturnStmt *RS = dyn_cast_or_null<ReturnStmt>(LastSt)) {
       state = state->BindExpr(CE, callerCtx, V);
     }
 
     // Bind the constructed object value to CXXConstructExpr.
     if (const CXXConstructExpr *CCE = dyn_cast<CXXConstructExpr>(CE)) {
       loc::MemRegionVal This =
         svalBuilder.getCXXThis(CCE->getConstructor()->getParent(), calleeCtx);
       SVal ThisV = state->getSVal(This);
-
-      // If the constructed object is a temporary prvalue, get its bindings.
-      if (isTemporaryPRValue(CCE, ThisV))
-        ThisV = state->getSVal(ThisV.castAs<Loc>());
-
       state = state->BindExpr(CCE, callerCtx, ThisV);
     }
 
     if (const auto *CNE = dyn_cast<CXXNewExpr>(CE)) {
       // We are currently evaluating a CXXNewAllocator CFGElement. It takes a
       // while to reach the actual CXXNewExpr element from here, so keep the
       // region for later use.
       // Additionally cast the return value of the inlined operator new
if (const ObjCMethodCall *Msg = dyn_cast<ObjCMethodCall>(&Call)) {
     case OMF_retain:
     case OMF_self: {
       // These methods return their receivers.
       return State->BindExpr(E, LCtx, Msg->getReceiverSVal());
     }
     }
   } else if (const CXXConstructorCall *C = dyn_cast<CXXConstructorCall>(&Call)){
     SVal ThisV = C->getCXXThisVal();
-
-    // If the constructed object is a temporary prvalue, get its bindings.
-    if (isTemporaryPRValue(cast<CXXConstructExpr>(E), ThisV))
-      ThisV = State->getSVal(ThisV.castAs<Loc>());
-
     return State->BindExpr(E, LCtx, ThisV);
   }
 
   SVal R;
   QualType ResultTy = Call.getResultType();
   unsigned Count = currBldrCtx->blockCount();
   if (auto RTC = getCurrentCFGElement().getAs<CFGCXXRecordTypedCall>()) {
     // Conjure a temporary if the function returns an object by value.

cfe/trunk/test/Analysis/cxx17-mandatory-elision.cpp

 // RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -std=c++11 -verify %s
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -std=c++17 -verify %s
 
 void clang_analyzer_eval(bool);
 
+namespace variable_functional_cast_crash {
+
+struct A {
+  A(int) {}
+};
+
+void foo() {
+  A a = A(0);
+}
+
+struct B {
+  A a;
+  B(): a(A(0)) {}
+};
+
+} // namespace variable_functional_cast_crash
+
+
+namespace address_vector_tests {
+
 template <typename T> struct AddressVector {
   T *buf[10];
   int len;
 
   AddressVector() : len(0) {}
 
   void push(T *t) {
     buf[len] = t;
#else
   clang_analyzer_eval(v.len == 5); // expected-warning{{TRUE}}
   clang_analyzer_eval(v.buf[0] != v.buf[1]); // expected-warning{{TRUE}}
   clang_analyzer_eval(v.buf[1] != v.buf[2]); // expected-warning{{TRUE}}
   clang_analyzer_eval(v.buf[2] != v.buf[3]); // expected-warning{{TRUE}}
   clang_analyzer_eval(v.buf[3] != v.buf[4]); // expected-warning{{TRUE}}
   clang_analyzer_eval(v.buf[4] == &c); // expected-warning{{TRUE}}
 #endif
 }
+
+} // namespace address_vector_tests