[analyzer] NFC: Merge code for finding and tracking construction target.

When analyzing C++ code, a common operation in the analyzer is to discover
target region for object construction by looking at CFG metadata ("construction
contexts"), and then track the region path-sensitively until object construction
is resolved, where the amount of information, again, depends on construction
context.

Scan construction context only once for both purposes.

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


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

include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h

@@ -744,14 +744,15 @@ private:
   /// constructing into an existing region.
   const CXXConstructExpr *findDirectConstructorForCurrentCFGElement();
 
-  /// For a given constructor, look forward in the current CFG block to
-  /// determine the region into which an object will be constructed by \p CE.
-  /// When the lookahead fails, a temporary region is returned, and the
+  /// Update the program state with all the path-sensitive information
+  /// that's necessary to perform construction of an object with a given
+  /// syntactic construction context. If the construction context is unavailable
+  /// or unusable for any reason, a dummy temporary region is returned, and the
   /// IsConstructorWithImproperlyModeledTargetRegion flag is set in \p CallOpts.
-  SVal getLocationForConstructedObject(const CXXConstructExpr *CE,
-                                       ExplodedNode *Pred,
-                                       const ConstructionContext *CC,
-                                       EvalCallOptions &CallOpts);
+  /// Returns the updated program state and the new object's this-region.
+  std::pair<ProgramStateRef, SVal> prepareForObjectConstruction(
+      const Expr *E, ProgramStateRef State, const LocationContext *LCtx,
+      const ConstructionContext *CC, EvalCallOptions &CallOpts);
 
   /// Store the location of a C++ object corresponding to a statement
   /// until the statement is actually encountered. For example, if a DeclStmt
@@ -782,14 +783,6 @@ private:
   static bool areAllObjectsFullyConstructed(ProgramStateRef State,
                                             const LocationContext *FromLC,
                                             const LocationContext *ToLC);
-
-  /// Adds an initialized temporary and/or a materialization, whichever is
-  /// necessary, by looking at the whole construction context. Handles
-  /// function return values, which need the construction context of the parent
-  /// stack frame, automagically.
-  ProgramStateRef markStatementsCorrespondingToConstructedObject(
-      ProgramStateRef State, const ConstructionContext *CC,
-      const LocationContext *LC, SVal V);
 };
 
 /// Traits for storing the call processing policy inside GDM.

lib/StaticAnalyzer/Core/ExprEngine.cpp

@@ -417,81 +417,6 @@ bool ExprEngine::areAllObjectsFullyConstructed(ProgramStateRef State,
   return true;
 }
 
-ProgramStateRef ExprEngine::markStatementsCorrespondingToConstructedObject(
-    ProgramStateRef State, const ConstructionContext *CC,
-    const LocationContext *LC, SVal V) {
-  if (CC) {
-    // If the temporary is being returned from the function, it will be
-    // destroyed or lifetime-extended in the caller stack frame.
-    if (isa<ReturnedValueConstructionContext>(CC)) {
-      const StackFrameContext *SFC = LC->getCurrentStackFrame();
-      assert(SFC);
-      LC = SFC->getParent();
-      if (!LC) {
-        // We are on the top frame. We won't ever need any info
-        // for this temporary, so don't set anything.
-        return State;
-      }
-      const CFGElement &CallElem =
-          (*SFC->getCallSiteBlock())[SFC->getIndex()];
-      auto RTCElem = CallElem.getAs<CFGCXXRecordTypedCall>();
-      if (!RTCElem) {
-        // We have a parent stack frame, but no construction context for the
-        // return value. Give up until we provide the construction context
-        // at the call site.
-        return State;
-      }
-      // We use the ReturnedValueConstructionContext as an indication that we
-      // need to look for the actual construction context on the parent stack
-      // frame. This purpose has been fulfilled, so now we replace CC with the
-      // actual construction context.
-      CC = RTCElem->getConstructionContext();
-      if (!isa<TemporaryObjectConstructionContext>(CC)) {
-        // TODO: We are not returning an object into a temporary. There must
-        // be copy elision happening at the call site. We still need to
-        // explicitly support the situation when the return value is put
-        // into another return statement, i.e.
-        // ReturnedValueConstructionContexts are chained through multiple
-        // stack frames before finally settling in a temporary.
-        // We don't seem to need to explicitly support construction into
-        // a variable after a return.
-        return State;
-      }
-      // Proceed to deal with the temporary we've found on the parent
-      // stack frame.
-    }
-    // In case of temporary object construction, extract data necessary for
-    // destruction and lifetime extension.
-    if (const auto *TCC = dyn_cast<TemporaryObjectConstructionContext>(CC)) {
-      if (AMgr.getAnalyzerOptions().includeTemporaryDtorsInCFG()) {
-        const CXXBindTemporaryExpr *BTE =
-            TCC->getCXXBindTemporaryExpr();
-        const MaterializeTemporaryExpr *MTE =
-            TCC->getMaterializedTemporaryExpr();
-        if (!BTE) {
-          // FIXME: Lifetime extension for temporaries without destructors
-          // is not implemented yet.
-          MTE = nullptr;
-        }
-        if (MTE && MTE->getStorageDuration() != SD_FullExpression) {
-          // If the temporary is lifetime-extended, don't save the BTE,
-          // because we don't need a temporary destructor, but an automatic
-          // destructor.
-          BTE = nullptr;
-        }
-
-        if (BTE)
-          State = addObjectUnderConstruction(State, BTE, LC, V);
-
-        if (MTE)
-          State = addObjectUnderConstruction(State, MTE, LC, V);
-      }
-    }
-  }
-
-  return State;
-}
-
 
 //===----------------------------------------------------------------------===//
 // Top-level transfer function logic (Dispatcher).

lib/StaticAnalyzer/Core/ExprEngineCXX.cpp

@@ -110,13 +110,9 @@ SVal ExprEngine::makeZeroElementRegion(ProgramStateRef State, SVal LValue,
   return LValue;
 }
 
-
-SVal ExprEngine::getLocationForConstructedObject(const CXXConstructExpr *CE,
-                                                 ExplodedNode *Pred,
-                                                 const ConstructionContext *CC,
-                                                 EvalCallOptions &CallOpts) {
-  const LocationContext *LCtx = Pred->getLocationContext();
-  ProgramStateRef State = Pred->getState();
+std::pair<ProgramStateRef, SVal> ExprEngine::prepareForObjectConstruction(
+    const Expr *E, ProgramStateRef State, const LocationContext *LCtx,
+    const ConstructionContext *CC, EvalCallOptions &CallOpts) {
   MemRegionManager &MRMgr = getSValBuilder().getRegionManager();
 
   // See if we're constructing an existing region by looking at the
@@ -129,8 +125,9 @@ SVal ExprEngine::getLocationForConstructedObject(const CXXConstructExpr *CE,
       const auto *Var = cast<VarDecl>(DS->getSingleDecl());
       SVal LValue = State->getLValue(Var, LCtx);
       QualType Ty = Var->getType();
-      return makeZeroElementRegion(State, LValue, Ty,
-                                   CallOpts.IsArrayCtorOrDtor);
+      return std::make_pair(
+          State,
+          makeZeroElementRegion(State, LValue, Ty, CallOpts.IsArrayCtorOrDtor));
     }
     case ConstructionContext::SimpleConstructorInitializerKind: {
       const auto *ICC = cast<ConstructorInitializerConstructionContext>(CC);
@@ -154,7 +151,7 @@ SVal ExprEngine::getLocationForConstructedObject(const CXXConstructExpr *CE,
       QualType Ty = Field->getType();
       FieldVal = makeZeroElementRegion(State, FieldVal, Ty,
                                        CallOpts.IsArrayCtorOrDtor);
-      return FieldVal;
+      return std::make_pair(State, FieldVal);
     }
     case ConstructionContext::NewAllocatedObjectKind: {
       if (AMgr.getAnalyzerOptions().mayInlineCXXAllocator()) {
@@ -167,44 +164,21 @@ SVal ExprEngine::getLocationForConstructedObject(const CXXConstructExpr *CE,
             // TODO: In fact, we need to call the constructor for every
             // allocated element, not just the first one!
             CallOpts.IsArrayCtorOrDtor = true;
-            return loc::MemRegionVal(getStoreManager().GetElementZeroRegion(
-                MR, NE->getType()->getPointeeType()));
+            return std::make_pair(
+                State, loc::MemRegionVal(getStoreManager().GetElementZeroRegion(
+                           MR, NE->getType()->getPointeeType())));
           }
-          return V;
+          return std::make_pair(State, V);
         }
         // TODO: Detect when the allocator returns a null pointer.
         // Constructor shall not be called in this case.
       }
       break;
     }
-    case ConstructionContext::TemporaryObjectKind: {
-      const auto *TOCC = cast<TemporaryObjectConstructionContext>(CC);
-      if (const auto *MTE = TOCC->getMaterializedTemporaryExpr()) {
-        if (const ValueDecl *VD = MTE->getExtendingDecl()) {
-          assert(MTE->getStorageDuration() != SD_FullExpression);
-          if (!VD->getType()->isReferenceType()) {
-            // We're lifetime-extended by a surrounding aggregate.
-            // Automatic destructors aren't quite working in this case
-            // on the CFG side. We should warn the caller about that.
-            // FIXME: Is there a better way to retrieve this information from
-            // the MaterializeTemporaryExpr?
-            CallOpts.IsTemporaryLifetimeExtendedViaAggregate = true;
-          }
-        }
-      }
-      // TODO: Support temporaries lifetime-extended via static references.
-      // They'd need a getCXXStaticTempObjectRegion().
-      CallOpts.IsTemporaryCtorOrDtor = true;
-      return loc::MemRegionVal(MRMgr.getCXXTempObjectRegion(CE, LCtx));
-    }
     case ConstructionContext::SimpleReturnedValueKind: {
       // The temporary is to be managed by the parent stack frame.
       // So build it in the parent stack frame if we're not in the
       // top frame of the analysis.
-      // TODO: What exactly happens when we are? Does the temporary object live
-      // long enough in the region store in this case? Would checkers think
-      // that this object immediately goes out of scope?
-      const LocationContext *TempLCtx = LCtx;
       const StackFrameContext *SFC = LCtx->getCurrentStackFrame();
       if (const LocationContext *CallerLCtx = SFC->getParent()) {
         auto RTC = (*SFC->getCallSiteBlock())[SFC->getIndex()]
@@ -213,19 +187,74 @@ SVal ExprEngine::getLocationForConstructedObject(const CXXConstructExpr *CE,
           // We were unable to find the correct construction context for the
           // call in the parent stack frame. This is equivalent to not being
           // able to find construction context at all.
-          CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion = true;
+          break;
         } else if (!isa<TemporaryObjectConstructionContext>(
                        RTC->getConstructionContext())) {
           // FIXME: The return value is constructed directly into a
           // non-temporary due to C++17 mandatory copy elision. This is not
           // implemented yet.
           assert(getContext().getLangOpts().CPlusPlus17);
-          CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion = true;
+          break;
+        }
+        CC = RTC->getConstructionContext();
+        LCtx = CallerLCtx;
+      } else {
+        // We are on the top frame of the analysis.
+        // TODO: What exactly happens when we are? Does the temporary object
+        // live long enough in the region store in this case? Would checkers
+        // think that this object immediately goes out of scope?
+        CallOpts.IsTemporaryCtorOrDtor = true;
+        SVal V = loc::MemRegionVal(MRMgr.getCXXTempObjectRegion(E, LCtx));
+        return std::make_pair(State, V);
+      }
+
+      // Continue as if we have a temporary with a different location context.
+      // FALLTHROUGH.
+    }
+    case ConstructionContext::TemporaryObjectKind: {
+      const auto *TCC = cast<TemporaryObjectConstructionContext>(CC);
+      const CXXBindTemporaryExpr *BTE = TCC->getCXXBindTemporaryExpr();
+      const MaterializeTemporaryExpr *MTE = TCC->getMaterializedTemporaryExpr();
+
+      if (!BTE) {
+        // FIXME: Lifetime extension for temporaries without destructors
+        // is not implemented yet.
+        MTE = nullptr;
+      }
+
+      if (MTE) {
+        if (const ValueDecl *VD = MTE->getExtendingDecl()) {
+          assert(MTE->getStorageDuration() != SD_FullExpression);
+          if (!VD->getType()->isReferenceType()) {
+            // We're lifetime-extended by a surrounding aggregate.
+            // Automatic destructors aren't quite working in this case
+            // on the CFG side. We should warn the caller about that.
+            // FIXME: Is there a better way to retrieve this information from
+            // the MaterializeTemporaryExpr?
+            CallOpts.IsTemporaryLifetimeExtendedViaAggregate = true;
+          }
         }
-        TempLCtx = CallerLCtx;
       }
+
+      if (MTE && MTE->getStorageDuration() != SD_FullExpression) {
+        // If the temporary is lifetime-extended, don't save the BTE,
+        // because we don't need a temporary destructor, but an automatic
+        // destructor.
+        BTE = nullptr;
+      }
+
+      // FIXME: Support temporaries lifetime-extended via static references.
+      // They'd need a getCXXStaticTempObjectRegion().
+      SVal V = loc::MemRegionVal(MRMgr.getCXXTempObjectRegion(E, LCtx));
+
+      if (BTE)
+        State = addObjectUnderConstruction(State, BTE, LCtx, V);
+
+      if (MTE)
+        State = addObjectUnderConstruction(State, MTE, LCtx, V);
+
       CallOpts.IsTemporaryCtorOrDtor = true;
-      return loc::MemRegionVal(MRMgr.getCXXTempObjectRegion(CE, TempLCtx));
+      return std::make_pair(State, V);
     }
     case ConstructionContext::CXX17ElidedCopyVariableKind:
     case ConstructionContext::CXX17ElidedCopyReturnedValueKind:
@@ -237,7 +266,8 @@ SVal ExprEngine::getLocationForConstructedObject(const CXXConstructExpr *CE,
   // If we couldn't find an existing region to construct into, assume we're
   // constructing a temporary. Notify the caller of our failure.
   CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion = true;
-  return loc::MemRegionVal(MRMgr.getCXXTempObjectRegion(CE, LCtx));
+  return std::make_pair(
+      State, loc::MemRegionVal(MRMgr.getCXXTempObjectRegion(E, LCtx)));
 }
 
 const CXXConstructExpr *
@@ -288,7 +318,8 @@ void ExprEngine::VisitCXXConstructExpr(const CXXConstructExpr *CE,
 
   switch (CE->getConstructionKind()) {
   case CXXConstructExpr::CK_Complete: {
-    Target = getLocationForConstructedObject(CE, Pred, CC, CallOpts);
+    std::tie(State, Target) =
+        prepareForObjectConstruction(CE, State, LCtx, CC, CallOpts);
     break;
   }
   case CXXConstructExpr::CK_VirtualBase:
@@ -349,6 +380,18 @@ void ExprEngine::VisitCXXConstructExpr(const CXXConstructExpr *CE,
   }
   }
 
+  if (State != Pred->getState()) {
+    static SimpleProgramPointTag T("ExprEngine",
+                                   "Prepare for object construction");
+    ExplodedNodeSet DstPrepare;
+    StmtNodeBuilder BldrPrepare(Pred, DstPrepare, *currBldrCtx);
+    BldrPrepare.generateNode(CE, Pred, State, &T, ProgramPoint::PreStmtKind);
+    assert(DstPrepare.size() <= 1);
+    if (DstPrepare.size() == 0)
+      return;
+    Pred = *BldrPrepare.begin();
+  }
+
   CallEventManager &CEMgr = getStateManager().getCallEventManager();
   CallEventRef<CXXConstructorCall> Call =
     CEMgr.getCXXConstructorCall(CE, Target.getAsRegion(), State, LCtx);
@@ -380,9 +423,6 @@ void ExprEngine::VisitCXXConstructExpr(const CXXConstructExpr *CE,
         State = State->bindDefaultZero(Target, LCtx);
       }
 
-      State = markStatementsCorrespondingToConstructedObject(State, CC, LCtx,
-                                                             Target);
-
       Bldr.generateNode(CE, *I, State, /*tag=*/nullptr,
                         ProgramPoint::PreStmtKind);
     }

lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp

@@ -587,18 +587,20 @@ ProgramStateRef ExprEngine::bindReturnValue(const CallEvent &Call,
   unsigned Count = currBldrCtx->blockCount();
   if (auto RTC = getCurrentCFGElement().getAs<CFGCXXRecordTypedCall>()) {
     // Conjure a temporary if the function returns an object by value.
-    MemRegionManager &MRMgr = svalBuilder.getRegionManager();
-    const CXXTempObjectRegion *TR = MRMgr.getCXXTempObjectRegion(E, LCtx);
-    State = markStatementsCorrespondingToConstructedObject(
-        State, RTC->getConstructionContext(), LCtx, loc::MemRegionVal(TR));
-
+    SVal Target;
+    assert(RTC->getStmt() == Call.getOriginExpr());
+    EvalCallOptions CallOpts; // FIXME: We won't really need those.
+    std::tie(State, Target) =
+        prepareForObjectConstruction(Call.getOriginExpr(), State, LCtx,
+                                     RTC->getConstructionContext(), CallOpts);
+    assert(Target.getAsRegion());
     // Invalidate the region so that it didn't look uninitialized. Don't notify
     // the checkers.
-    State = State->invalidateRegions(TR, E, Count, LCtx,
+    State = State->invalidateRegions(Target.getAsRegion(), E, Count, LCtx,
                                      /* CausedByPointerEscape=*/false, nullptr,
                                      &Call, nullptr);
 
-    R = State->getSVal(TR, E->getType());
+    R = State->getSVal(Target.castAs<Loc>(), E->getType());
   } else {
     // Conjure a symbol if the return value is unknown.