[analyzer] pr37270: Track constructor target region, even if just a variable.

The very idea of construction context implies that first the object is
constructed, and then later, in a separate moment of time, the constructed
object goes into scope, i.e. becomes "live".

Most construction contexts require path-sensitive tracking of the constructed
object region in order to compute the outer expressions accordingly before
the object becomes live.

Semantics of simple variable construction contexts don't immediately require
that such tracking happens in path-sensitive manner, but shortcomings of the
analyzer force us to track it path-sensitively as well. Namely, whether
construction context was available at all during construction is a
path-sensitive information. Additionally, path-sensitive tracking takes care of
our liveness problems that kick in as the temporal gap between construction and
going-into-scope becomes larger (eg., due to copy elision).

Differential Revision: https://reviews.llvm.org/D47305

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

diff --git a/lib/StaticAnalyzer/Core/ExprEngineC.cpp b/lib/StaticAnalyzer/Core/ExprEngineC.cpp
index 462104c1d4fc9cc058cb7e1c7a65055d6d9ee8eb..c7b1a9ac82f0bf84c72ecdd003ed61535713a747 100644 (file)
--- a/lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -590,24 +590,12 @@ void ExprEngine::VisitDeclStmt(const DeclStmt *DS, ExplodedNode *Pred,
       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()) {

diff --git a/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
index 529761d6084e684a0390c2214e625e22838f6bd5..dd4660fff99aa0660e37701348e5121ee2a99a89 100644 (file)
--- a/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -125,9 +125,11 @@ std::pair<ProgramStateRef, SVal> ExprEngine::prepareForObjectConstruction(
       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);

diff --git a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
index 9cfe421109779ad61d9fb2db2608b7a5c56ba4d5..e15871c7a2e2191e582258fb5bd82707f24d7dd1 100644 (file)
--- a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -193,23 +193,6 @@ static bool wasDifferentDeclUsedForInlining(CallEventRef<> Call,
   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:
@@ -269,11 +252,7 @@ void ExprEngine::processCallExit(ExplodedNode *CEBNode) {
       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>());
-
+      ThisV = state->getSVal(ThisV.castAs<Loc>());
       state = state->BindExpr(CCE, callerCtx, ThisV);
     }
 
@@ -574,11 +553,7 @@ ProgramStateRef ExprEngine::bindReturnValue(const CallEvent &Call,
     }
   } 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>());
-
+    ThisV = State->getSVal(ThisV.castAs<Loc>());
     return State->BindExpr(E, LCtx, ThisV);
   }
 

diff --git a/test/Analysis/cxx17-mandatory-elision.cpp b/test/Analysis/cxx17-mandatory-elision.cpp
index 700888e47a96713e8754f121df619d69455c806b..a1645a7efe11ced63f9927ad713984d612fbc093 100644 (file)
--- a/test/Analysis/cxx17-mandatory-elision.cpp
+++ b/test/Analysis/cxx17-mandatory-elision.cpp
@@ -3,6 +3,26 @@
 
 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;
@@ -56,3 +76,5 @@ void testMultipleReturns() {
   clang_analyzer_eval(v.buf[4] == &c); // expected-warning{{TRUE}}
 #endif
 }
+
+} // namespace address_vector_tests