r333719 - [analyzer] NFC: Track all constructed objects in a single state trait.

Artem Dergachev via cfe-commits cfe-commits at lists.llvm.org
Thu May 31 18:59:48 PDT 2018


Author: dergachev
Date: Thu May 31 18:59:48 2018
New Revision: 333719

URL: http://llvm.org/viewvc/llvm-project?rev=333719&view=rev
Log:
[analyzer] NFC: Track all constructed objects in a single state trait.

ExprEngine already maintains three internal program state traits to track
path-sensitive information related to object construction: pointer returned by
operator new, and pointer to temporary object for two different purposes - for
destruction and for lifetime extension. We'll need to add 2-3 more in a few
follow-up commits.

Merge these traits into one because they all essentially serve one purpose and
work similarly.

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

Modified:
    cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
    cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h?rev=333719&r1=333718&r2=333719&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h Thu May 31 18:59:48 2018
@@ -753,68 +753,48 @@ private:
   /// determine the region into which an object will be constructed by \p CE.
   /// When the lookahead fails, a temporary region is returned, and the
   /// IsConstructorWithImproperlyModeledTargetRegion flag is set in \p CallOpts.
-  const MemRegion *getRegionForConstructedObject(const CXXConstructExpr *CE,
-                                                 ExplodedNode *Pred,
-                                                 const ConstructionContext *CC,
-                                                 EvalCallOptions &CallOpts);
-
-  /// Store the region of a C++ temporary object corresponding to a
-  /// CXXBindTemporaryExpr for later destruction.
-  static ProgramStateRef addInitializedTemporary(
-      ProgramStateRef State, const CXXBindTemporaryExpr *BTE,
-      const LocationContext *LC, const CXXTempObjectRegion *R);
+  SVal getLocationForConstructedObject(const CXXConstructExpr *CE,
+                                       ExplodedNode *Pred,
+                                       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
+  /// has CXXConstructExpr as its initializer, the object would be considered
+  /// to be "under construction" between CXXConstructExpr and DeclStmt.
+  /// This allows, among other things, to keep bindings to variable's fields
+  /// made within the constructor alive until its declaration actually
+  /// goes into scope.
+  static ProgramStateRef addObjectUnderConstruction(
+      ProgramStateRef State, const Stmt *S,
+      const LocationContext *LC, SVal V);
+
+  /// Mark the object sa fully constructed, cleaning up the state trait
+  /// that tracks objects under construction.
+  static ProgramStateRef finishObjectConstruction(ProgramStateRef State,
+                                                  const Stmt *S,
+                                                  const LocationContext *LC);
+
+  /// If the given statement corresponds to an object under construction,
+  /// being part of its construciton context, retrieve that object's location.
+  static Optional<SVal> getObjectUnderConstruction(ProgramStateRef State,
+                                                   const Stmt *S,
+                                                   const LocationContext *LC);
 
-  /// Check if all initialized temporary regions are clear for the given
-  /// context range (including FromLC, not including ToLC).
+  /// Check if all objects under construction have been fully constructed
+  /// for the given context range (including FromLC, not including ToLC).
   /// This is useful for assertions.
-  static bool areInitializedTemporariesClear(ProgramStateRef State,
-                                             const LocationContext *FromLC,
-                                             const LocationContext *ToLC);
-
-  /// Store the region of a C++ temporary object corresponding to a
-  /// CXXBindTemporaryExpr for later destruction.
-  static ProgramStateRef addTemporaryMaterialization(
-      ProgramStateRef State, const MaterializeTemporaryExpr *MTE,
-      const LocationContext *LC, const CXXTempObjectRegion *R);
-
-  /// Check if all temporary materialization regions are clear for the given
-  /// context range (including FromLC, not including ToLC).
-  /// This is useful for assertions.
-  static bool areTemporaryMaterializationsClear(ProgramStateRef State,
-                                                const LocationContext *FromLC,
-                                                const LocationContext *ToLC);
+  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 addAllNecessaryTemporaryInfo(
+  ProgramStateRef markStatementsCorrespondingToConstructedObject(
       ProgramStateRef State, const ConstructionContext *CC,
-      const LocationContext *LC, const MemRegion *R);
-
-  /// Store the region returned by operator new() so that the constructor
-  /// that follows it knew what location to initialize. The value should be
-  /// cleared once the respective CXXNewExpr CFGStmt element is processed.
-  static ProgramStateRef
-  setCXXNewAllocatorValue(ProgramStateRef State, const CXXNewExpr *CNE,
-                          const LocationContext *CallerLC, SVal V);
-
-  /// Retrieve the location returned by the current operator new().
-  static SVal
-  getCXXNewAllocatorValue(ProgramStateRef State, const CXXNewExpr *CNE,
-                          const LocationContext *CallerLC);
-
-  /// Clear the location returned by the respective operator new(). This needs
-  /// to be done as soon as CXXNewExpr CFG block is evaluated.
-  static ProgramStateRef
-  clearCXXNewAllocatorValue(ProgramStateRef State, const CXXNewExpr *CNE,
-                            const LocationContext *CallerLC);
-
-  /// Check if all allocator values are clear for the given context range
-  /// (including FromLC, not including ToLC). This is useful for assertions.
-  static bool areCXXNewAllocatorValuesClear(ProgramStateRef State,
-                                            const LocationContext *FromLC,
-                                            const LocationContext *ToLC);
+      const LocationContext *LC, SVal V);
 };
 
 /// Traits for storing the call processing policy inside GDM.

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp?rev=333719&r1=333718&r2=333719&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp Thu May 31 18:59:48 2018
@@ -98,42 +98,23 @@ STATISTIC(NumMaxBlockCountReachedInInlin
 STATISTIC(NumTimesRetriedWithoutInlining,
             "The # of times we re-evaluated a call without inlining");
 
-using InitializedTemporariesMap =
-    llvm::ImmutableMap<std::pair<const CXXBindTemporaryExpr *,
-                                 const StackFrameContext *>,
-                       const CXXTempObjectRegion *>;
-
-// Keeps track of whether CXXBindTemporaryExpr nodes have been evaluated.
-// The StackFrameContext assures that nested calls due to inlined recursive
-// functions do not interfere.
-REGISTER_TRAIT_WITH_PROGRAMSTATE(InitializedTemporaries,
-                                 InitializedTemporariesMap)
-
-using TemporaryMaterializationMap =
-    llvm::ImmutableMap<std::pair<const MaterializeTemporaryExpr *,
-                                 const StackFrameContext *>,
-                       const CXXTempObjectRegion *>;
-
-// Keeps track of temporaries that will need to be materialized later.
-// The StackFrameContext assures that nested calls due to inlined recursive
-// functions do not interfere.
-REGISTER_TRAIT_WITH_PROGRAMSTATE(TemporaryMaterializations,
-                                 TemporaryMaterializationMap)
-
-using CXXNewAllocatorValuesMap =
-    llvm::ImmutableMap<std::pair<const CXXNewExpr *, const LocationContext *>,
-                       SVal>;
-
-// Keeps track of return values of various operator new() calls between
-// evaluation of the inlined operator new(), through the constructor call,
-// to the actual evaluation of the CXXNewExpr.
-// TODO: Refactor the key for this trait into a LocationContext sub-class,
-// which would be put on the stack of location contexts before operator new()
-// is evaluated, and removed from the stack when the whole CXXNewExpr
-// is fully evaluated.
-// Probably do something similar to the previous trait as well.
-REGISTER_TRAIT_WITH_PROGRAMSTATE(CXXNewAllocatorValues,
-                                 CXXNewAllocatorValuesMap)
+
+//===----------------------------------------------------------------------===//
+// Internal program state traits.
+//===----------------------------------------------------------------------===//
+
+// When modeling a C++ constructor, for a variety of reasons we need to track
+// the location of the object for the duration of its ConstructionContext.
+// ObjectsUnderConstruction maps statements within the construction context
+// to the object's location, so that on every such statement the location
+// could have been retrieved.
+
+typedef std::pair<const Stmt *, const LocationContext *> ConstructedObjectKey;
+typedef llvm::ImmutableMap<ConstructedObjectKey, SVal>
+    ObjectsUnderConstructionMap;
+REGISTER_TRAIT_WITH_PROGRAMSTATE(ObjectsUnderConstruction,
+                                 ObjectsUnderConstructionMap)
+
 
 //===----------------------------------------------------------------------===//
 // Engine construction and deletion.
@@ -312,13 +293,11 @@ ExprEngine::createTemporaryRegionIfNeede
   bool FoundOriginalMaterializationRegion = false;
   const TypedValueRegion *TR = nullptr;
   if (const auto *MT = dyn_cast<MaterializeTemporaryExpr>(Result)) {
-    auto Key = std::make_pair(MT, LC->getCurrentStackFrame());
-    if (const CXXTempObjectRegion *const *TRPtr =
-            State->get<TemporaryMaterializations>(Key)) {
+    if (Optional<SVal> V = getObjectUnderConstruction(State, MT, LC)) {
       FoundOriginalMaterializationRegion = true;
-      TR = *TRPtr;
+      TR = cast<CXXTempObjectRegion>(V->getAsRegion());
       assert(TR);
-      State = State->remove<TemporaryMaterializations>(Key);
+      State = finishObjectConstruction(State, MT, LC);
     } else {
       StorageDuration SD = MT->getStorageDuration();
       // If this object is bound to a reference with static storage duration, we
@@ -399,51 +378,37 @@ ExprEngine::createTemporaryRegionIfNeede
   return State;
 }
 
-ProgramStateRef ExprEngine::addInitializedTemporary(
-    ProgramStateRef State, const CXXBindTemporaryExpr *BTE,
-    const LocationContext *LC, const CXXTempObjectRegion *R) {
-  const auto &Key = std::make_pair(BTE, LC->getCurrentStackFrame());
-  if (!State->contains<InitializedTemporaries>(Key)) {
-    return State->set<InitializedTemporaries>(Key, R);
-  }
-
+ProgramStateRef
+ExprEngine::addObjectUnderConstruction(ProgramStateRef State, const Stmt *S,
+                                       const LocationContext *LC, SVal V) {
+  ConstructedObjectKey Key(S, LC->getCurrentStackFrame());
   // FIXME: Currently the state might already contain the marker due to
-  // incorrect handling of temporaries bound to default parameters; for
-  // those, we currently skip the CXXBindTemporaryExpr but rely on adding
-  // temporary destructor nodes. Otherwise, this branch should be unreachable.
-  return State;
+  // incorrect handling of temporaries bound to default parameters.
+  assert(!State->get<ObjectsUnderConstruction>(Key) ||
+         isa<CXXBindTemporaryExpr>(S));
+  return State->set<ObjectsUnderConstruction>(Key, V);
 }
 
-bool ExprEngine::areInitializedTemporariesClear(ProgramStateRef State,
-                                                const LocationContext *FromLC,
-                                                const LocationContext *ToLC) {
-  const LocationContext *LC = FromLC;
-  while (LC != ToLC) {
-    assert(LC && "ToLC must be a parent of FromLC!");
-    for (auto I : State->get<InitializedTemporaries>())
-      if (I.first.second == LC)
-        return false;
-
-    LC = LC->getParent();
-  }
-  return true;
+Optional<SVal> ExprEngine::getObjectUnderConstruction(ProgramStateRef State,
+    const Stmt *S, const LocationContext *LC) {
+  ConstructedObjectKey Key(S, LC->getCurrentStackFrame());
+  return Optional<SVal>::create(State->get<ObjectsUnderConstruction>(Key));
 }
 
-ProgramStateRef ExprEngine::addTemporaryMaterialization(
-    ProgramStateRef State, const MaterializeTemporaryExpr *MTE,
-    const LocationContext *LC, const CXXTempObjectRegion *R) {
-  const auto &Key = std::make_pair(MTE, LC->getCurrentStackFrame());
-  assert(!State->contains<TemporaryMaterializations>(Key));
-  return State->set<TemporaryMaterializations>(Key, R);
+ProgramStateRef ExprEngine::finishObjectConstruction(ProgramStateRef State,
+    const Stmt *S, const LocationContext *LC) {
+  ConstructedObjectKey Key(S, LC->getCurrentStackFrame());
+  assert(State->contains<ObjectsUnderConstruction>(Key));
+  return State->remove<ObjectsUnderConstruction>(Key);
 }
 
-bool ExprEngine::areTemporaryMaterializationsClear(
-    ProgramStateRef State, const LocationContext *FromLC,
-    const LocationContext *ToLC) {
+bool ExprEngine::areAllObjectsFullyConstructed(ProgramStateRef State,
+                                               const LocationContext *FromLC,
+                                               const LocationContext *ToLC) {
   const LocationContext *LC = FromLC;
   while (LC != ToLC) {
     assert(LC && "ToLC must be a parent of FromLC!");
-    for (auto I : State->get<TemporaryMaterializations>())
+    for (auto I : State->get<ObjectsUnderConstruction>())
       if (I.first.second == LC)
         return false;
 
@@ -452,12 +417,9 @@ bool ExprEngine::areTemporaryMaterializa
   return true;
 }
 
-ProgramStateRef ExprEngine::addAllNecessaryTemporaryInfo(
+ProgramStateRef ExprEngine::markStatementsCorrespondingToConstructedObject(
     ProgramStateRef State, const ConstructionContext *CC,
-    const LocationContext *LC, const MemRegion *R) {
-  const CXXBindTemporaryExpr *BTE = nullptr;
-  const MaterializeTemporaryExpr *MTE = nullptr;
-
+    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.
@@ -498,13 +460,14 @@ ProgramStateRef ExprEngine::addAllNecess
       // 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()) {
-        BTE = TCC->getCXXBindTemporaryExpr();
-        MTE = TCC->getMaterializedTemporaryExpr();
+        const CXXBindTemporaryExpr *BTE =
+            TCC->getCXXBindTemporaryExpr();
+        const MaterializeTemporaryExpr *MTE =
+            TCC->getMaterializedTemporaryExpr();
         if (!BTE) {
           // FIXME: Lifetime extension for temporaries without destructors
           // is not implemented yet.
@@ -516,59 +479,19 @@ ProgramStateRef ExprEngine::addAllNecess
           // destructor.
           BTE = nullptr;
         }
-      }
-    }
 
-    if (BTE) {
-      State = addInitializedTemporary(State, BTE, LC,
-                                      cast<CXXTempObjectRegion>(R));
-    }
+        if (BTE)
+          State = addObjectUnderConstruction(State, BTE, LC, V);
 
-    if (MTE) {
-      State = addTemporaryMaterialization(State, MTE, LC,
-                                          cast<CXXTempObjectRegion>(R));
+        if (MTE)
+          State = addObjectUnderConstruction(State, MTE, LC, V);
+      }
     }
   }
 
   return State;
 }
 
-ProgramStateRef
-ExprEngine::setCXXNewAllocatorValue(ProgramStateRef State,
-                                    const CXXNewExpr *CNE,
-                                    const LocationContext *CallerLC, SVal V) {
-  assert(!State->get<CXXNewAllocatorValues>(std::make_pair(CNE, CallerLC)) &&
-         "Allocator value already set!");
-  return State->set<CXXNewAllocatorValues>(std::make_pair(CNE, CallerLC), V);
-}
-
-SVal ExprEngine::getCXXNewAllocatorValue(ProgramStateRef State,
-                                         const CXXNewExpr *CNE,
-                                         const LocationContext *CallerLC) {
-  return *State->get<CXXNewAllocatorValues>(std::make_pair(CNE, CallerLC));
-}
-
-ProgramStateRef
-ExprEngine::clearCXXNewAllocatorValue(ProgramStateRef State,
-                                      const CXXNewExpr *CNE,
-                                      const LocationContext *CallerLC) {
-  return State->remove<CXXNewAllocatorValues>(std::make_pair(CNE, CallerLC));
-}
-
-bool ExprEngine::areCXXNewAllocatorValuesClear(ProgramStateRef State,
-                                               const LocationContext *FromLC,
-                                               const LocationContext *ToLC) {
-  const LocationContext *LC = FromLC;
-  while (LC != ToLC) {
-    assert(LC && "ToLC must be a parent of FromLC!");
-    for (auto I : State->get<CXXNewAllocatorValues>())
-      if (I.first.second == LC)
-        return false;
-
-    LC = LC->getParent();
-  }
-  return true;
-}
 
 //===----------------------------------------------------------------------===//
 // Top-level transfer function logic (Dispatcher).
@@ -593,55 +516,15 @@ ExprEngine::processRegionChanges(Program
                                                          LCtx, Call);
 }
 
-static void printInitializedTemporariesForContext(raw_ostream &Out,
-                                                  ProgramStateRef State,
-                                                  const char *NL,
-                                                  const char *Sep,
-                                                  const LocationContext *LC) {
-  PrintingPolicy PP =
-      LC->getAnalysisDeclContext()->getASTContext().getPrintingPolicy();
-  for (auto I : State->get<InitializedTemporaries>()) {
-    std::pair<const CXXBindTemporaryExpr *, const LocationContext *> Key =
-        I.first;
-    const MemRegion *Value = I.second;
-    if (Key.second != LC)
-      continue;
-    Out << '(' << Key.second << ',' << Key.first << ") ";
-    Key.first->printPretty(Out, nullptr, PP);
-    if (Value)
-      Out << " : " << Value;
-    Out << NL;
-  }
-}
-
-static void printTemporaryMaterializationsForContext(
-    raw_ostream &Out, ProgramStateRef State, const char *NL, const char *Sep,
-    const LocationContext *LC) {
-  PrintingPolicy PP =
-      LC->getAnalysisDeclContext()->getASTContext().getPrintingPolicy();
-  for (auto I : State->get<TemporaryMaterializations>()) {
-    std::pair<const MaterializeTemporaryExpr *, const LocationContext *> Key =
-        I.first;
-    const MemRegion *Value = I.second;
-    if (Key.second != LC)
-      continue;
-    Out << '(' << Key.second << ',' << Key.first << ") ";
-    Key.first->printPretty(Out, nullptr, PP);
-    assert(Value);
-    Out << " : " << Value << NL;
-  }
-}
-
-static void printCXXNewAllocatorValuesForContext(raw_ostream &Out,
-                                                 ProgramStateRef State,
-                                                 const char *NL,
-                                                 const char *Sep,
-                                                 const LocationContext *LC) {
+static void printObjectsUnderConstructionForContext(raw_ostream &Out,
+                                                    ProgramStateRef State,
+                                                    const char *NL,
+                                                    const char *Sep,
+                                                    const LocationContext *LC) {
   PrintingPolicy PP =
       LC->getAnalysisDeclContext()->getASTContext().getPrintingPolicy();
-
-  for (auto I : State->get<CXXNewAllocatorValues>()) {
-    std::pair<const CXXNewExpr *, const LocationContext *> Key = I.first;
+  for (auto I : State->get<ObjectsUnderConstruction>()) {
+    ConstructedObjectKey Key = I.first;
     SVal Value = I.second;
     if (Key.second != LC)
       continue;
@@ -655,27 +538,11 @@ void ExprEngine::printState(raw_ostream
                             const char *NL, const char *Sep,
                             const LocationContext *LCtx) {
   if (LCtx) {
-    if (!State->get<InitializedTemporaries>().isEmpty()) {
-      Out << Sep << "Initialized temporaries:" << NL;
+    if (!State->get<ObjectsUnderConstruction>().isEmpty()) {
+      Out << Sep << "Objects under construction:" << NL;
 
       LCtx->dumpStack(Out, "", NL, Sep, [&](const LocationContext *LC) {
-        printInitializedTemporariesForContext(Out, State, NL, Sep, LC);
-      });
-    }
-
-    if (!State->get<TemporaryMaterializations>().isEmpty()) {
-      Out << Sep << "Temporaries to be materialized:" << NL;
-
-      LCtx->dumpStack(Out, "", NL, Sep, [&](const LocationContext *LC) {
-        printTemporaryMaterializationsForContext(Out, State, NL, Sep, LC);
-      });
-    }
-
-    if (!State->get<CXXNewAllocatorValues>().isEmpty()) {
-      Out << Sep << "operator new() allocator return values:" << NL;
-
-      LCtx->dumpStack(Out, "", NL, Sep, [&](const LocationContext *LC) {
-        printCXXNewAllocatorValuesForContext(Out, State, NL, Sep, LC);
+        printObjectsUnderConstructionForContext(Out, State, NL, Sep, LC);
       });
     }
   }
@@ -779,19 +646,11 @@ void ExprEngine::removeDead(ExplodedNode
   const StackFrameContext *SFC = LC ? LC->getCurrentStackFrame() : nullptr;
   SymbolReaper SymReaper(SFC, ReferenceStmt, SymMgr, getStoreManager());
 
-  for (auto I : CleanedState->get<InitializedTemporaries>())
-    if (I.second)
-      SymReaper.markLive(I.second);
-
-  for (auto I : CleanedState->get<TemporaryMaterializations>())
-    if (I.second)
-      SymReaper.markLive(I.second);
-
-  for (auto I : CleanedState->get<CXXNewAllocatorValues>()) {
+  for (auto I : CleanedState->get<ObjectsUnderConstruction>()) {
     if (SymbolRef Sym = I.second.getAsSymbol())
       SymReaper.markLive(Sym);
     if (const MemRegion *MR = I.second.getAsRegion())
-      SymReaper.markElementIndicesLive(MR);
+      SymReaper.markLive(MR);
   }
 
   getCheckerManager().runCheckersForLiveSymbols(CleanedState, SymReaper);
@@ -1148,17 +1007,15 @@ void ExprEngine::ProcessTemporaryDtor(co
   StmtNodeBuilder StmtBldr(Pred, CleanDtorState, *currBldrCtx);
   ProgramStateRef State = Pred->getState();
   const MemRegion *MR = nullptr;
-  if (const CXXTempObjectRegion *const *MRPtr =
-          State->get<InitializedTemporaries>(std::make_pair(
-              D.getBindTemporaryExpr(), Pred->getStackFrame()))) {
+  if (Optional<SVal> V =
+          getObjectUnderConstruction(State, D.getBindTemporaryExpr(),
+                                     Pred->getLocationContext())) {
     // FIXME: Currently we insert temporary destructors for default parameters,
     // but we don't insert the constructors, so the entry in
-    // InitializedTemporaries may be missing.
-    State = State->remove<InitializedTemporaries>(
-        std::make_pair(D.getBindTemporaryExpr(), Pred->getStackFrame()));
-    // *MRPtr may still be null when the construction context for the temporary
-    // was not implemented.
-    MR = *MRPtr;
+    // ObjectsUnderConstruction may be missing.
+    State = finishObjectConstruction(State, D.getBindTemporaryExpr(),
+                                     Pred->getLocationContext());
+    MR = V->getAsRegion();
   }
   StmtBldr.generateNode(D.getBindTemporaryExpr(), Pred, State);
 
@@ -1197,13 +1054,14 @@ void ExprEngine::processCleanupTemporary
                                                const CFGBlock *DstT,
                                                const CFGBlock *DstF) {
   BranchNodeBuilder TempDtorBuilder(Pred, Dst, BldCtx, DstT, DstF);
-  if (Pred->getState()->contains<InitializedTemporaries>(
-          std::make_pair(BTE, Pred->getStackFrame()))) {
+  ProgramStateRef State = Pred->getState();
+  const LocationContext *LC = Pred->getLocationContext();
+  if (getObjectUnderConstruction(State, BTE, LC)) {
     TempDtorBuilder.markInfeasible(false);
-    TempDtorBuilder.generateNode(Pred->getState(), true, Pred);
+    TempDtorBuilder.generateNode(State, true, Pred);
   } else {
     TempDtorBuilder.markInfeasible(true);
-    TempDtorBuilder.generateNode(Pred->getState(), false, Pred);
+    TempDtorBuilder.generateNode(State, false, Pred);
   }
 }
 
@@ -1222,14 +1080,13 @@ void ExprEngine::VisitCXXBindTemporaryEx
   StmtNodeBuilder StmtBldr(PreVisit, Dst, *currBldrCtx);
   for (ExplodedNode *Node : PreVisit) {
     ProgramStateRef State = Node->getState();
-		const auto &Key = std::make_pair(BTE, Node->getStackFrame());
-
-    if (!State->contains<InitializedTemporaries>(Key)) {
+    const LocationContext *LC = Node->getLocationContext();
+    if (!getObjectUnderConstruction(State, BTE, LC)) {
       // FIXME: Currently the state might also already contain the marker due to
       // incorrect handling of temporaries bound to default parameters; for
       // those, we currently skip the CXXBindTemporaryExpr but rely on adding
       // temporary destructor nodes.
-      State = State->set<InitializedTemporaries>(Key, nullptr);
+      State = addObjectUnderConstruction(State, BTE, LC, UnknownVal());
     }
     StmtBldr.generateNode(BTE, Node, State);
   }
@@ -2320,11 +2177,6 @@ void ExprEngine::processBeginOfFunction(
 void ExprEngine::processEndOfFunction(NodeBuilderContext& BC,
                                       ExplodedNode *Pred,
                                       const ReturnStmt *RS) {
-  // See if we have any stale C++ allocator values.
-  assert(areCXXNewAllocatorValuesClear(Pred->getState(),
-                                       Pred->getLocationContext(),
-                                       Pred->getStackFrame()->getParent()));
-
   // FIXME: We currently cannot assert that temporaries are clear, because
   // lifetime extended temporaries are not always modelled correctly. In some
   // cases when we materialize the temporary, we do
@@ -2333,17 +2185,22 @@ void ExprEngine::processEndOfFunction(No
   // the state manually before asserting. Ideally, the code above the assertion
   // should go away, but the assertion should remain.
   {
-    ExplodedNodeSet CleanUpTemporaries;
-    NodeBuilder Bldr(Pred, CleanUpTemporaries, BC);
+    ExplodedNodeSet CleanUpObjects;
+    NodeBuilder Bldr(Pred, CleanUpObjects, BC);
     ProgramStateRef State = Pred->getState();
     const LocationContext *FromLC = Pred->getLocationContext();
     const LocationContext *ToLC = FromLC->getCurrentStackFrame()->getParent();
     const LocationContext *LC = FromLC;
     while (LC != ToLC) {
       assert(LC && "ToLC must be a parent of FromLC!");
-      for (auto I : State->get<InitializedTemporaries>())
-        if (I.first.second == LC)
-          State = State->remove<InitializedTemporaries>(I.first);
+      for (auto I : State->get<ObjectsUnderConstruction>())
+        if (I.first.second == LC) {
+          // The comment above only pardons us for not cleaning up a
+          // CXXBindTemporaryExpr. If any other statements are found here,
+          // it must be a separate problem.
+          assert(isa<CXXBindTemporaryExpr>(I.first.first));
+          State = State->remove<ObjectsUnderConstruction>(I.first);
+        }
 
       LC = LC->getParent();
     }
@@ -2356,12 +2213,9 @@ void ExprEngine::processEndOfFunction(No
       }
     }
   }
-  assert(areInitializedTemporariesClear(Pred->getState(),
-                                        Pred->getLocationContext(),
-                                        Pred->getStackFrame()->getParent()));
-  assert(areTemporaryMaterializationsClear(Pred->getState(),
-                                           Pred->getLocationContext(),
-                                           Pred->getStackFrame()->getParent()));
+  assert(areAllObjectsFullyConstructed(Pred->getState(),
+                                       Pred->getLocationContext(),
+                                       Pred->getStackFrame()->getParent()));
 
   PrettyStackTraceLocationContext CrashInfo(Pred->getLocationContext());
   StateMgr.EndPath(Pred->getState());

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp?rev=333719&r1=333718&r2=333719&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp Thu May 31 18:59:48 2018
@@ -111,11 +111,10 @@ SVal ExprEngine::makeZeroElementRegion(P
 }
 
 
-const MemRegion *
-ExprEngine::getRegionForConstructedObject(const CXXConstructExpr *CE,
-                                          ExplodedNode *Pred,
-                                          const ConstructionContext *CC,
-                                          EvalCallOptions &CallOpts) {
+SVal ExprEngine::getLocationForConstructedObject(const CXXConstructExpr *CE,
+                                                 ExplodedNode *Pred,
+                                                 const ConstructionContext *CC,
+                                                 EvalCallOptions &CallOpts) {
   const LocationContext *LCtx = Pred->getLocationContext();
   ProgramStateRef State = Pred->getState();
   MemRegionManager &MRMgr = getSValBuilder().getRegionManager();
@@ -130,9 +129,8 @@ ExprEngine::getRegionForConstructedObjec
       const auto *Var = cast<VarDecl>(DS->getSingleDecl());
       SVal LValue = State->getLValue(Var, LCtx);
       QualType Ty = Var->getType();
-      LValue =
-          makeZeroElementRegion(State, LValue, Ty, CallOpts.IsArrayCtorOrDtor);
-      return LValue.getAsRegion();
+      return makeZeroElementRegion(State, LValue, Ty,
+                                   CallOpts.IsArrayCtorOrDtor);
     }
     case ConstructionContext::SimpleConstructorInitializerKind: {
       const auto *ICC = cast<ConstructorInitializerConstructionContext>(CC);
@@ -156,25 +154,26 @@ ExprEngine::getRegionForConstructedObjec
       QualType Ty = Field->getType();
       FieldVal = makeZeroElementRegion(State, FieldVal, Ty,
                                        CallOpts.IsArrayCtorOrDtor);
-      return FieldVal.getAsRegion();
+      return FieldVal;
     }
     case ConstructionContext::NewAllocatedObjectKind: {
       if (AMgr.getAnalyzerOptions().mayInlineCXXAllocator()) {
         const auto *NECC = cast<NewAllocatedObjectConstructionContext>(CC);
         const auto *NE = NECC->getCXXNewExpr();
-        // TODO: Detect when the allocator returns a null pointer.
-        // Constructor shall not be called in this case.
-        if (const SubRegion *MR = dyn_cast_or_null<SubRegion>(
-                getCXXNewAllocatorValue(State, NE, LCtx).getAsRegion())) {
+        SVal V = *getObjectUnderConstruction(State, NE, LCtx);
+        if (const SubRegion *MR =
+                dyn_cast_or_null<SubRegion>(V.getAsRegion())) {
           if (NE->isArray()) {
             // TODO: In fact, we need to call the constructor for every
             // allocated element, not just the first one!
             CallOpts.IsArrayCtorOrDtor = true;
-            return getStoreManager().GetElementZeroRegion(
-                MR, NE->getType()->getPointeeType());
+            return loc::MemRegionVal(getStoreManager().GetElementZeroRegion(
+                MR, NE->getType()->getPointeeType()));
           }
-          return MR;
+          return V;
         }
+        // TODO: Detect when the allocator returns a null pointer.
+        // Constructor shall not be called in this case.
       }
       break;
     }
@@ -205,7 +204,7 @@ ExprEngine::getRegionForConstructedObjec
       // TODO: Support temporaries lifetime-extended via static references.
       // They'd need a getCXXStaticTempObjectRegion().
       CallOpts.IsTemporaryCtorOrDtor = true;
-      return MRMgr.getCXXTempObjectRegion(CE, LCtx);
+      return loc::MemRegionVal(MRMgr.getCXXTempObjectRegion(CE, LCtx));
     }
     case ConstructionContext::SimpleReturnedValueKind: {
       // The temporary is to be managed by the parent stack frame.
@@ -226,7 +225,7 @@ ExprEngine::getRegionForConstructedObjec
           CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion = true;
         } else if (!isa<TemporaryObjectConstructionContext>(
                        RTC->getConstructionContext())) {
-          // FXIME: The return value is constructed directly into a
+          // 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);
@@ -235,7 +234,7 @@ ExprEngine::getRegionForConstructedObjec
         TempLCtx = CallerLCtx;
       }
       CallOpts.IsTemporaryCtorOrDtor = true;
-      return MRMgr.getCXXTempObjectRegion(CE, TempLCtx);
+      return loc::MemRegionVal(MRMgr.getCXXTempObjectRegion(CE, TempLCtx));
     }
     case ConstructionContext::CXX17ElidedCopyVariableKind:
     case ConstructionContext::CXX17ElidedCopyReturnedValueKind:
@@ -247,7 +246,7 @@ ExprEngine::getRegionForConstructedObjec
   // 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 MRMgr.getCXXTempObjectRegion(CE, LCtx);
+  return loc::MemRegionVal(MRMgr.getCXXTempObjectRegion(CE, LCtx));
 }
 
 const CXXConstructExpr *
@@ -285,7 +284,7 @@ void ExprEngine::VisitCXXConstructExpr(c
   const LocationContext *LCtx = Pred->getLocationContext();
   ProgramStateRef State = Pred->getState();
 
-  const MemRegion *Target = nullptr;
+  SVal Target = UnknownVal();
 
   // FIXME: Handle arrays, which run the same constructor for every element.
   // For now, we just run the first constructor (which should still invalidate
@@ -298,7 +297,7 @@ void ExprEngine::VisitCXXConstructExpr(c
 
   switch (CE->getConstructionKind()) {
   case CXXConstructExpr::CK_Complete: {
-    Target = getRegionForConstructedObject(CE, Pred, CC, CallOpts);
+    Target = getLocationForConstructedObject(CE, Pred, CC, CallOpts);
     break;
   }
   case CXXConstructExpr::CK_VirtualBase:
@@ -334,7 +333,7 @@ void ExprEngine::VisitCXXConstructExpr(c
     // otherwise always available during construction.
     if (dyn_cast_or_null<InitListExpr>(LCtx->getParentMap().getParent(CE))) {
       MemRegionManager &MRMgr = getSValBuilder().getRegionManager();
-      Target = MRMgr.getCXXTempObjectRegion(CE, LCtx);
+      Target = loc::MemRegionVal(MRMgr.getCXXTempObjectRegion(CE, LCtx));
       CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion = true;
       break;
     }
@@ -346,14 +345,14 @@ void ExprEngine::VisitCXXConstructExpr(c
     SVal ThisVal = State->getSVal(ThisPtr);
 
     if (CE->getConstructionKind() == CXXConstructExpr::CK_Delegating) {
-      Target = ThisVal.getAsRegion();
+      Target = ThisVal;
     } else {
       // Cast to the base type.
       bool IsVirtual =
         (CE->getConstructionKind() == CXXConstructExpr::CK_VirtualBase);
       SVal BaseVal = getStoreManager().evalDerivedToBase(ThisVal, CE->getType(),
                                                          IsVirtual);
-      Target = BaseVal.getAsRegion();
+      Target = BaseVal;
     }
     break;
   }
@@ -361,7 +360,7 @@ void ExprEngine::VisitCXXConstructExpr(c
 
   CallEventManager &CEMgr = getStateManager().getCallEventManager();
   CallEventRef<CXXConstructorCall> Call =
-    CEMgr.getCXXConstructorCall(CE, Target, State, LCtx);
+    CEMgr.getCXXConstructorCall(CE, Target.getAsRegion(), State, LCtx);
 
   ExplodedNodeSet DstPreVisit;
   getCheckerManager().runCheckersForPreStmt(DstPreVisit, Pred, CE, *this);
@@ -387,10 +386,11 @@ void ExprEngine::VisitCXXConstructExpr(c
         // 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->bindDefaultZero(loc::MemRegionVal(Target), LCtx);
+        State = State->bindDefaultZero(Target, LCtx);
       }
 
-      State = addAllNecessaryTemporaryInfo(State, CC, LCtx, Target);
+      State = markStatementsCorrespondingToConstructedObject(State, CC, LCtx,
+                                                             Target);
 
       Bldr.generateNode(CE, *I, State, /*tag=*/nullptr,
                         ProgramPoint::PreStmtKind);
@@ -550,8 +550,8 @@ void ExprEngine::VisitCXXNewAllocatorCal
           State = State->assume(RetVal.castAs<DefinedOrUnknownSVal>(), true);
     }
 
-    ValueBldr.generateNode(CNE, I,
-                           setCXXNewAllocatorValue(State, CNE, LCtx, RetVal));
+    ValueBldr.generateNode(
+        CNE, I, addObjectUnderConstruction(State, CNE, LCtx, RetVal));
   }
 
   ExplodedNodeSet DstPostPostCallCallback;
@@ -559,7 +559,8 @@ void ExprEngine::VisitCXXNewAllocatorCal
                                              DstPostValue, *Call, *this);
   for (auto I : DstPostPostCallCallback) {
     getCheckerManager().runCheckersForNewAllocator(
-        CNE, getCXXNewAllocatorValue(I->getState(), CNE, LCtx), Dst, I, *this);
+        CNE, *getObjectUnderConstruction(I->getState(), CNE, LCtx), Dst, I,
+        *this);
   }
 }
 
@@ -582,8 +583,8 @@ void ExprEngine::VisitCXXNewExpr(const C
 
   // Retrieve the stored operator new() return value.
   if (AMgr.getAnalyzerOptions().mayInlineCXXAllocator()) {
-    symVal = getCXXNewAllocatorValue(State, CNE, LCtx);
-    State = clearCXXNewAllocatorValue(State, CNE, LCtx);
+    symVal = *getObjectUnderConstruction(State, CNE, LCtx);
+    State = finishObjectConstruction(State, CNE, LCtx);
   }
 
   // We assume all standard global 'operator new' functions allocate memory in

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp?rev=333719&r1=333718&r2=333719&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp Thu May 31 18:59:48 2018
@@ -288,8 +288,8 @@ void ExprEngine::processCallExit(Explode
           AllocV, CNE->getType(),
           getContext().getPointerType(getContext().VoidTy));
 
-      state =
-          setCXXNewAllocatorValue(state, CNE, calleeCtx->getParent(), AllocV);
+      state = addObjectUnderConstruction(state, CNE, calleeCtx->getParent(),
+                                         AllocV);
     }
   }
 
@@ -354,8 +354,9 @@ void ExprEngine::processCallExit(Explode
                                                  /*WasInlined=*/true);
       for (auto I : DstPostPostCallCallback) {
         getCheckerManager().runCheckersForNewAllocator(
-            CNE, getCXXNewAllocatorValue(I->getState(), CNE,
-                                         calleeCtx->getParent()),
+            CNE,
+            *getObjectUnderConstruction(I->getState(), CNE,
+                                        calleeCtx->getParent()),
             DstPostCall, I, *this,
             /*WasInlined=*/true);
       }
@@ -588,8 +589,8 @@ ProgramStateRef ExprEngine::bindReturnVa
     // Conjure a temporary if the function returns an object by value.
     MemRegionManager &MRMgr = svalBuilder.getRegionManager();
     const CXXTempObjectRegion *TR = MRMgr.getCXXTempObjectRegion(E, LCtx);
-    State = addAllNecessaryTemporaryInfo(State, RTC->getConstructionContext(),
-                                         LCtx, TR);
+    State = markStatementsCorrespondingToConstructedObject(
+        State, RTC->getConstructionContext(), LCtx, loc::MemRegionVal(TR));
 
     // Invalidate the region so that it didn't look uninitialized. Don't notify
     // the checkers.




More information about the cfe-commits mailing list