[analyzer] Track class member initializer constructors path-sensitively.

The reasoning behind this change is similar to the previous commit, r334681.
Because members are already in scope when construction occurs, we are not
suffering from liveness problems, but we still want to figure out if the object
was constructed with construction context, because in this case we'll be able
to avoid trivial copy, which we don't always model perfectly. It'd also have
more importance when copy elision is implemented.

This also gets rid of the old CFG look-behind mechanism.

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

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

diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
index b5e2102324177fcf487ad3347b8a5e8dec562a1d..e469df0d15e448906868fd4c4de8a64edc36e752 100644 (file)
--- a/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ b/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -762,20 +762,23 @@ private:
   /// made within the constructor alive until its declaration actually
   /// goes into scope.
   static ProgramStateRef addObjectUnderConstruction(
-      ProgramStateRef State, const Stmt *S,
+      ProgramStateRef State,
+      llvm::PointerUnion<const Stmt *, const CXXCtorInitializer *> P,
       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);
+  static ProgramStateRef finishObjectConstruction(
+      ProgramStateRef State,
+      llvm::PointerUnion<const Stmt *, const CXXCtorInitializer *> P,
+      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);
+  static Optional<SVal> getObjectUnderConstruction(
+      ProgramStateRef State,
+      llvm::PointerUnion<const Stmt *, const CXXCtorInitializer *> P,
+      const LocationContext *LC);
 
   /// Check if all objects under construction have been fully constructed
   /// for the given context range (including FromLC, not including ToLC).

diff --git a/lib/StaticAnalyzer/Core/ExprEngine.cpp b/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 3fed9c088b85bdc600d09d828ec7e313e1481dd5..939b21944d943ef0df366878173b9276cc2a11f1 100644 (file)
--- a/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -109,7 +109,75 @@ STATISTIC(NumTimesRetriedWithoutInlining,
 // 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;
+/// ConstructedObjectKey is used for being able to find the path-sensitive
+/// memory region of a freshly constructed object while modeling the AST node
+/// that syntactically represents the object that is being constructed.
+/// Semantics of such nodes may sometimes require access to the region that's
+/// not otherwise present in the program state, or to the very fact that
+/// the construction context was present and contained references to these
+/// AST nodes.
+class ConstructedObjectKey {
+  typedef std::pair<
+      llvm::PointerUnion<const Stmt *, const CXXCtorInitializer *>,
+      const LocationContext *> ConstructedObjectKeyImpl;
+
+  ConstructedObjectKeyImpl Impl;
+
+  const void *getAnyASTNodePtr() const {
+    if (const Stmt *S = getStmt())
+      return S;
+    else
+      return getCXXCtorInitializer();
+  }
+
+public:
+  ConstructedObjectKey(
+      llvm::PointerUnion<const Stmt *, const CXXCtorInitializer *> P,
+      const LocationContext *LC)
+      : Impl(P, LC) {
+    // This is the full list of statements that require additional actions when
+    // encountered. This list may be expanded when new actions are implemented.
+    assert(getCXXCtorInitializer() || isa<DeclStmt>(getStmt()) ||
+           isa<CXXNewExpr>(getStmt()) || isa<CXXBindTemporaryExpr>(getStmt()) ||
+           isa<MaterializeTemporaryExpr>(getStmt()));
+  }
+
+  const Stmt *getStmt() const {
+    return Impl.first.dyn_cast<const Stmt *>();
+  }
+
+  const CXXCtorInitializer *getCXXCtorInitializer() const {
+    return Impl.first.dyn_cast<const CXXCtorInitializer *>();
+  }
+
+  const LocationContext *getLocationContext() const {
+    return Impl.second;
+  }
+
+  void print(llvm::raw_ostream &OS, PrinterHelper *Helper, PrintingPolicy &PP) {
+    OS << '(' << getLocationContext() << ',' << getAnyASTNodePtr() << ") ";
+    if (const Stmt *S = getStmt()) {
+      S->printPretty(OS, Helper, PP);
+    } else {
+      const CXXCtorInitializer *I = getCXXCtorInitializer();
+      OS << I->getAnyMember()->getNameAsString();
+    }
+  }
+
+  void Profile(llvm::FoldingSetNodeID &ID) const {
+    ID.AddPointer(Impl.first.getOpaqueValue());
+    ID.AddPointer(Impl.second);
+  }
+
+  bool operator==(const ConstructedObjectKey &RHS) const {
+    return Impl == RHS.Impl;
+  }
+
+  bool operator<(const ConstructedObjectKey &RHS) const {
+    return Impl < RHS.Impl;
+  }
+};
+
 typedef llvm::ImmutableMap<ConstructedObjectKey, SVal>
     ObjectsUnderConstructionMap;
 REGISTER_TRAIT_WITH_PROGRAMSTATE(ObjectsUnderConstruction,
@@ -378,26 +446,31 @@ ExprEngine::createTemporaryRegionIfNeeded(ProgramStateRef State,
   return State;
 }
 
-ProgramStateRef
-ExprEngine::addObjectUnderConstruction(ProgramStateRef State, const Stmt *S,
-                                       const LocationContext *LC, SVal V) {
-  ConstructedObjectKey Key(S, LC->getCurrentStackFrame());
+ProgramStateRef ExprEngine::addObjectUnderConstruction(
+    ProgramStateRef State,
+    llvm::PointerUnion<const Stmt *, const CXXCtorInitializer *> P,
+    const LocationContext *LC, SVal V) {
+  ConstructedObjectKey Key(P, LC->getCurrentStackFrame());
   // FIXME: Currently the state might already contain the marker due to
   // incorrect handling of temporaries bound to default parameters.
   assert(!State->get<ObjectsUnderConstruction>(Key) ||
-         isa<CXXBindTemporaryExpr>(S));
+         isa<CXXBindTemporaryExpr>(Key.getStmt()));
   return State->set<ObjectsUnderConstruction>(Key, V);
 }
 
-Optional<SVal> ExprEngine::getObjectUnderConstruction(ProgramStateRef State,
-    const Stmt *S, const LocationContext *LC) {
-  ConstructedObjectKey Key(S, LC->getCurrentStackFrame());
+Optional<SVal> ExprEngine::getObjectUnderConstruction(
+    ProgramStateRef State,
+    llvm::PointerUnion<const Stmt *, const CXXCtorInitializer *> P,
+    const LocationContext *LC) {
+  ConstructedObjectKey Key(P, LC->getCurrentStackFrame());
   return Optional<SVal>::create(State->get<ObjectsUnderConstruction>(Key));
 }
 
-ProgramStateRef ExprEngine::finishObjectConstruction(ProgramStateRef State,
-    const Stmt *S, const LocationContext *LC) {
-  ConstructedObjectKey Key(S, LC->getCurrentStackFrame());
+ProgramStateRef ExprEngine::finishObjectConstruction(
+    ProgramStateRef State,
+    llvm::PointerUnion<const Stmt *, const CXXCtorInitializer *> P,
+    const LocationContext *LC) {
+  ConstructedObjectKey Key(P, LC->getCurrentStackFrame());
   assert(State->contains<ObjectsUnderConstruction>(Key));
   return State->remove<ObjectsUnderConstruction>(Key);
 }
@@ -409,7 +482,7 @@ bool ExprEngine::areAllObjectsFullyConstructed(ProgramStateRef State,
   while (LC != ToLC) {
     assert(LC && "ToLC must be a parent of FromLC!");
     for (auto I : State->get<ObjectsUnderConstruction>())
-      if (I.first.second == LC)
+      if (I.first.getLocationContext() == LC)
         return false;
 
     LC = LC->getParent();
@@ -451,10 +524,9 @@ static void printObjectsUnderConstructionForContext(raw_ostream &Out,
   for (auto I : State->get<ObjectsUnderConstruction>()) {
     ConstructedObjectKey Key = I.first;
     SVal Value = I.second;
-    if (Key.second != LC)
+    if (Key.getLocationContext() != LC)
       continue;
-    Out << '(' << Key.second << ',' << Key.first << ") ";
-    Key.first->printPretty(Out, nullptr, PP);
+    Key.print(Out, nullptr, PP);
     Out << " : " << Value << NL;
   }
 }
@@ -677,9 +749,11 @@ void ExprEngine::ProcessLoopExit(const Stmt* S, ExplodedNode *Pred) {
   Engine.enqueue(Dst, currBldrCtx->getBlock(), currStmtIdx);
 }
 
-void ExprEngine::ProcessInitializer(const CFGInitializer Init,
+void ExprEngine::ProcessInitializer(const CFGInitializer CFGInit,
                                     ExplodedNode *Pred) {
-  const CXXCtorInitializer *BMI = Init.getInitializer();
+  const CXXCtorInitializer *BMI = CFGInit.getInitializer();
+  const Expr *Init = BMI->getInit()->IgnoreImplicit();
+  const LocationContext *LC = Pred->getLocationContext();
 
   PrettyStackTraceLoc CrashInfo(getContext().getSourceManager(),
                                 BMI->getSourceLocation(),
@@ -692,19 +766,21 @@ void ExprEngine::ProcessInitializer(const CFGInitializer Init,
   ProgramStateRef State = Pred->getState();
   SVal thisVal = State->getSVal(svalBuilder.getCXXThis(decl, stackFrame));
 
-  ExplodedNodeSet Tmp(Pred);
+  ExplodedNodeSet Tmp;
   SVal FieldLoc;
 
   // Evaluate the initializer, if necessary
   if (BMI->isAnyMemberInitializer()) {
     // Constructors build the object directly in the field,
     // but non-objects must be copied in from the initializer.
-    if (const auto *CtorExpr = findDirectConstructorForCurrentCFGElement()) {
-      assert(BMI->getInit()->IgnoreImplicit() == CtorExpr);
-      (void)CtorExpr;
+    if (getObjectUnderConstruction(State, BMI, LC)) {
       // The field was directly constructed, so there is no need to bind.
+      // But we still need to stop tracking the object under construction.
+      State = finishObjectConstruction(State, BMI, LC);
+      NodeBuilder Bldr(Pred, Tmp, *currBldrCtx);
+      PostStore PS(Init, LC, /*Loc*/ nullptr, /*tag*/ nullptr);
+      Bldr.generateNode(PS, State, Pred);
     } else {
-      const Expr *Init = BMI->getInit()->IgnoreImplicit();
       const ValueDecl *Field;
       if (BMI->isIndirectMemberInitializer()) {
         Field = BMI->getIndirectMember();
@@ -738,15 +814,12 @@ void ExprEngine::ProcessInitializer(const CFGInitializer Init,
         InitVal = State->getSVal(BMI->getInit(), stackFrame);
       }
 
-      assert(Tmp.size() == 1 && "have not generated any new nodes yet");
-      assert(*Tmp.begin() == Pred && "have not generated any new nodes yet");
-      Tmp.clear();
-
       PostInitializer PP(BMI, FieldLoc.getAsRegion(), stackFrame);
       evalBind(Tmp, Init, Pred, FieldLoc, InitVal, /*isInit=*/true, &PP);
     }
   } else {
     assert(BMI->isBaseInitializer() || BMI->isDelegatingInitializer());
+    Tmp.insert(Pred);
     // We already did all the work when visiting the CXXConstructExpr.
   }
 
@@ -755,8 +828,10 @@ void ExprEngine::ProcessInitializer(const CFGInitializer Init,
   PostInitializer PP(BMI, FieldLoc.getAsRegion(), stackFrame);
   ExplodedNodeSet Dst;
   NodeBuilder Bldr(Tmp, Dst, *currBldrCtx);
-  for (const auto I : Tmp)
-    Bldr.generateNode(PP, I->getState(), I);
+  for (const auto I : Tmp) {
+    ProgramStateRef State = I->getState();
+    Bldr.generateNode(PP, State, I);
+  }
 
   // Enqueue the new nodes onto the work list.
   Engine.enqueue(Dst, currBldrCtx->getBlock(), currStmtIdx);
@@ -2119,11 +2194,11 @@ void ExprEngine::processEndOfFunction(NodeBuilderContext& BC,
     while (LC != ToLC) {
       assert(LC && "ToLC must be a parent of FromLC!");
       for (auto I : State->get<ObjectsUnderConstruction>())
-        if (I.first.second == LC) {
+        if (I.first.getLocationContext() == 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));
+          assert(isa<CXXBindTemporaryExpr>(I.first.getStmt()));
           State = State->remove<ObjectsUnderConstruction>(I.first);
         }
 

diff --git a/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
index dd4660fff99aa0660e37701348e5121ee2a99a89..ada99e1369d9efc38401d52654d4d25d919583e2 100644 (file)
--- a/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -153,6 +153,7 @@ std::pair<ProgramStateRef, SVal> ExprEngine::prepareForObjectConstruction(
       QualType Ty = Field->getType();
       FieldVal = makeZeroElementRegion(State, FieldVal, Ty,
                                        CallOpts.IsArrayCtorOrDtor);
+      State = addObjectUnderConstruction(State, Init, LCtx, FieldVal);
       return std::make_pair(State, FieldVal);
     }
     case ConstructionContext::NewAllocatedObjectKind: {
@@ -272,35 +273,6 @@ std::pair<ProgramStateRef, SVal> ExprEngine::prepareForObjectConstruction(
       State, loc::MemRegionVal(MRMgr.getCXXTempObjectRegion(E, LCtx)));
 }
 
-const CXXConstructExpr *
-ExprEngine::findDirectConstructorForCurrentCFGElement() {
-  // Go backward in the CFG to see if the previous element (ignoring
-  // destructors) was a CXXConstructExpr. If so, that constructor
-  // was constructed directly into an existing region.
-  // This process is essentially the inverse of that performed in
-  // findElementDirectlyInitializedByCurrentConstructor().
-  if (currStmtIdx == 0)
-    return nullptr;
-
-  const CFGBlock *B = getBuilderContext().getBlock();
-
-  unsigned int PreviousStmtIdx = currStmtIdx - 1;
-  CFGElement Previous = (*B)[PreviousStmtIdx];
-
-  while (Previous.getAs<CFGImplicitDtor>() && PreviousStmtIdx > 0) {
-    --PreviousStmtIdx;
-    Previous = (*B)[PreviousStmtIdx];
-  }
-
-  if (Optional<CFGStmt> PrevStmtElem = Previous.getAs<CFGStmt>()) {
-    if (auto *CtorExpr = dyn_cast<CXXConstructExpr>(PrevStmtElem->getStmt())) {
-      return CtorExpr;
-    }
-  }
-
-  return nullptr;
-}
-
 void ExprEngine::VisitCXXConstructExpr(const CXXConstructExpr *CE,
                                        ExplodedNode *Pred,
                                        ExplodedNodeSet &destNodes) {

diff --git a/test/Analysis/cxx17-mandatory-elision.cpp b/test/Analysis/cxx17-mandatory-elision.cpp
index a1645a7efe11ced63f9927ad713984d612fbc093..6a88bbaad9b9b5521340a5a7e5ace3e31dc4d10d 100644 (file)
--- a/test/Analysis/cxx17-mandatory-elision.cpp
+++ b/test/Analysis/cxx17-mandatory-elision.cpp
@@ -21,6 +21,37 @@ struct B {
 } // namespace variable_functional_cast_crash
 
 
+namespace ctor_initializer {
+
+struct S {
+  int x, y, z;
+};
+
+struct T {
+  S s;
+  int w;
+  T(int w): s(), w(w) {}
+};
+
+class C {
+  T t;
+public:
+  C() : t(T(4)) {
+    S s = {1, 2, 3};
+    t.s = s;
+    // FIXME: Should be TRUE in C++11 as well.
+    clang_analyzer_eval(t.w == 4);
+#if __cplusplus >= 201703L
+    // expected-warning@-2{{TRUE}}
+#else
+    // expected-warning@-4{{UNKNOWN}}
+#endif
+  }
+};
+
+} // namespace ctor_initializer
+
+
 namespace address_vector_tests {
 
 template <typename T> struct AddressVector {