[CFG] [analyzer] NFC: Allow more complicated construction contexts.

ConstructionContexts introduced in D42672 are an additional piece of information
included with CFGConstructor elements that help the client of the CFG (such as
the Static Analyzer) understand where the newly constructed object is stored.

The patch refactors the ConstructionContext class to prepare for including
multi-layered contexts that are being constructed gradually, layer-by-layer,
as the AST is traversed.

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

llvm-svn: 325966

diff --git a/clang/include/clang/Analysis/CFG.h b/clang/include/clang/Analysis/CFG.h
index fef486e..6ebcdbf 100644 (file)
--- a/clang/include/clang/Analysis/CFG.h
+++ b/clang/include/clang/Analysis/CFG.h
@@ -155,14 +155,35 @@ private:
   // new-expression triggers construction of the newly allocated object(s).
   TriggerTy Trigger;
 
-public:
+  // Sometimes a single trigger is not enough to describe the construction site.
+  // In this case we'd have a chain of "partial" construction contexts.
+  // Some examples:
+  // - A constructor within in an aggregate initializer list within a variable
+  //   would have a construction context of the initializer list with the parent
+  //   construction context of a variable.
+  // - A constructor for a temporary that needs to be both destroyed
+  //   and materialized into an elidable copy constructor would have a
+  //   construction context of a CXXBindTemporaryExpr with the parent
+  //   construction context of a MaterializeTemproraryExpr.
+  // Not all of these are currently supported.
+  const ConstructionContext *Parent = nullptr;
+
   ConstructionContext() = default;
-  ConstructionContext(TriggerTy Trigger)
-      : Trigger(Trigger) {}
+  ConstructionContext(TriggerTy Trigger, const ConstructionContext *Parent)
+      : Trigger(Trigger), Parent(Parent) {}
+
+public:
+  static const ConstructionContext *
+  create(BumpVectorContext &C, TriggerTy Trigger,
+         const ConstructionContext *Parent = nullptr) {
+    ConstructionContext *CC = C.getAllocator().Allocate<ConstructionContext>();
+    return new (CC) ConstructionContext(Trigger, Parent);
+  }
 
   bool isNull() const { return Trigger.isNull(); }
 
   TriggerTy getTrigger() const { return Trigger; }
+  const ConstructionContext *getParent() const { return Parent; }
 
   const Stmt *getTriggerStmt() const {
     return Trigger.dyn_cast<Stmt *>();
@@ -172,10 +193,27 @@ public:
     return Trigger.dyn_cast<CXXCtorInitializer *>();
   }
 
-  const ConstructionContext *getPersistentCopy(BumpVectorContext &C) const {
-    ConstructionContext *CC = C.getAllocator().Allocate<ConstructionContext>();
-    *CC = *this;
-    return CC;
+  bool isSameAsPartialContext(const ConstructionContext *Other) const {
+    assert(Other);
+    return (Trigger == Other->Trigger);
+  }
+
+  // See if Other is a proper initial segment of this construction context
+  // in terms of the parent chain - i.e. a few first parents coincide and
+  // then the other context terminates but our context goes further - i.e.,
+  // we are providing the same context that the other context provides,
+  // and a bit more above that.
+  bool isStrictlyMoreSpecificThan(const ConstructionContext *Other) const {
+    const ConstructionContext *Self = this;
+    while (true) {
+      if (!Other)
+        return Self;
+      if (!Self || !Self->isSameAsPartialContext(Other))
+        return false;
+      Self = Self->getParent();
+      Other = Other->getParent();
+    }
+    llvm_unreachable("The above loop can only be terminated via return!");
   }
 };
 
@@ -834,9 +872,9 @@ public:
     Elements.push_back(CFGStmt(statement), C);
   }
 
-  void appendConstructor(CXXConstructExpr *CE, const ConstructionContext &CC,
+  void appendConstructor(CXXConstructExpr *CE, const ConstructionContext *CC,
                          BumpVectorContext &C) {
-    Elements.push_back(CFGConstructor(CE, CC.getPersistentCopy(C)), C);
+    Elements.push_back(CFGConstructor(CE, CC), C);
   }
 
   void appendInitializer(CXXCtorInitializer *initializer,

diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp
index d2d1917..c65a652 100644 (file)
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -475,7 +475,8 @@ class CFGBuilder {
   // Information about the currently visited C++ object construction site.
   // This is set in the construction trigger and read when the constructor
   // itself is being visited.
-  ConstructionContext CurrentConstructionContext = {};
+  llvm::DenseMap<CXXConstructExpr *, const ConstructionContext *>
+      ConstructionContextMap{};
 
   bool badCFG = false;
   const CFG::BuildOptions &BuildOpts;
@@ -654,12 +655,12 @@ private:
   // to the trigger statement. The construction context will be unset once
   // it is consumed when the CFG building procedure processes the
   // construct-expression and adds the respective CFGConstructor element.
-  void EnterConstructionContextIfNecessary(
-      ConstructionContext::TriggerTy Trigger, Stmt *Child);
+  void findConstructionContexts(const ConstructionContext *ContextSoFar,
+                                Stmt *Child);
   // Unset the construction context after consuming it. This is done immediately
   // after adding the CFGConstructor element, so there's no need to
   // do this manually in every Visit... function.
-  void ExitConstructionContext();
+  void cleanupConstructionContext(CXXConstructExpr *CE);
 
   void autoCreateBlock() { if (!Block) Block = createBlock(); }
   CFGBlock *createBlock(bool add_successor = true);
@@ -702,10 +703,9 @@ private:
 
   void appendConstructor(CFGBlock *B, CXXConstructExpr *CE) {
     if (BuildOpts.AddRichCXXConstructors) {
-      if (!CurrentConstructionContext.isNull()) {
-        B->appendConstructor(CE, CurrentConstructionContext,
-                             cfg->getBumpVectorContext());
-        ExitConstructionContext();
+      if (const ConstructionContext *CC = ConstructionContextMap.lookup(CE)) {
+        B->appendConstructor(CE, CC, cfg->getBumpVectorContext());
+        cleanupConstructionContext(CE);
         return;
       }
     }
@@ -1148,25 +1148,38 @@ static const VariableArrayType *FindVA(const Type *t) {
   return nullptr;
 }
 
-void CFGBuilder::EnterConstructionContextIfNecessary(
-    ConstructionContext::TriggerTy Trigger, Stmt *Child) {
+void CFGBuilder::findConstructionContexts(
+    const ConstructionContext *ContextSoFar, Stmt *Child) {
   if (!BuildOpts.AddRichCXXConstructors)
     return;
   if (!Child)
     return;
-  if (isa<CXXConstructExpr>(Child)) {
-    assert(CurrentConstructionContext.isNull() &&
-           "Already within a construction context!");
-    CurrentConstructionContext = ConstructionContext(Trigger);
+  if (auto *CE = dyn_cast<CXXConstructExpr>(Child)) {
+    if (const ConstructionContext *PreviousContext =
+            ConstructionContextMap.lookup(CE)) {
+      // We might have visited this child when we were finding construction
+      // contexts within its parents.
+      assert(PreviousContext->isStrictlyMoreSpecificThan(ContextSoFar) &&
+             "Already within a different construction context!");
+    } else {
+      ConstructionContextMap[CE] = ContextSoFar;
+    }
   } else if (auto *Cleanups = dyn_cast<ExprWithCleanups>(Child)) {
-    EnterConstructionContextIfNecessary(Trigger, Cleanups->getSubExpr());
+    findConstructionContexts(ContextSoFar, Cleanups->getSubExpr());
+  } else if (auto *BTE = dyn_cast<CXXBindTemporaryExpr>(Child)) {
+    findConstructionContexts(
+        ConstructionContext::create(cfg->getBumpVectorContext(), BTE,
+                                    ContextSoFar),
+        BTE->getSubExpr());
   }
 }
 
-void CFGBuilder::ExitConstructionContext() {
-  assert(!CurrentConstructionContext.isNull() &&
+void CFGBuilder::cleanupConstructionContext(CXXConstructExpr *CE) {
+  assert(BuildOpts.AddRichCXXConstructors &&
+         "We should not be managing construction contexts!");
+  assert(ConstructionContextMap.count(CE) &&
          "Cannot exit construction context without the context!");
-  CurrentConstructionContext = ConstructionContext();
+  ConstructionContextMap.erase(CE);
 }
 
 
@@ -1250,6 +1263,10 @@ std::unique_ptr<CFG> CFGBuilder::buildCFG(const Decl *D, Stmt *Statement) {
   // Create an empty entry block that has no predecessors.
   cfg->setEntry(createBlock());
 
+  if (BuildOpts.AddRichCXXConstructors)
+    assert(ConstructionContextMap.empty() &&
+           "Not all construction contexts were cleaned up!");
+
   return std::move(cfg);
 }
 
@@ -1297,7 +1314,9 @@ CFGBlock *CFGBuilder::addInitializer(CXXCtorInitializer *I) {
   appendInitializer(Block, I);
 
   if (Init) {
-    EnterConstructionContextIfNecessary(I, Init);
+    findConstructionContexts(
+        ConstructionContext::create(cfg->getBumpVectorContext(), I),
+        Init);
 
     if (HasTemporaries) {
       // For expression with temporaries go directly to subexpression to omit
@@ -2383,7 +2402,9 @@ CFGBlock *CFGBuilder::VisitDeclSubExpr(DeclStmt *DS) {
   autoCreateBlock();
   appendStmt(Block, DS);
 
-  EnterConstructionContextIfNecessary(DS, Init);
+  findConstructionContexts(
+      ConstructionContext::create(cfg->getBumpVectorContext(), DS),
+      Init);
 
   // Keep track of the last non-null block, as 'Block' can be nulled out
   // if the initializer expression is something like a 'while' in a
@@ -2575,7 +2596,9 @@ CFGBlock *CFGBuilder::VisitReturnStmt(ReturnStmt *R) {
 
   addAutomaticObjHandling(ScopePos, LocalScope::const_iterator(), R);
 
-  EnterConstructionContextIfNecessary(R, R->getRetValue());
+  findConstructionContexts(
+      ConstructionContext::create(cfg->getBumpVectorContext(), R),
+      R->getRetValue());
 
   // If the one of the destructors does not return, we already have the Exit
   // block as a successor.
@@ -3923,7 +3946,9 @@ CFGBlock *CFGBuilder::VisitCXXBindTemporaryExpr(CXXBindTemporaryExpr *E,
     autoCreateBlock();
     appendStmt(Block, E);
 
-    EnterConstructionContextIfNecessary(E, E->getSubExpr());
+    findConstructionContexts(
+        ConstructionContext::create(cfg->getBumpVectorContext(), E),
+        E->getSubExpr());
 
     // We do not want to propagate the AlwaysAdd property.
     asc = asc.withAlwaysAdd(false);
@@ -3944,8 +3969,9 @@ CFGBlock *CFGBuilder::VisitCXXNewExpr(CXXNewExpr *NE,
   autoCreateBlock();
   appendStmt(Block, NE);
 
-  EnterConstructionContextIfNecessary(
-      NE, const_cast<CXXConstructExpr *>(NE->getConstructExpr()));
+  findConstructionContexts(
+      ConstructionContext::create(cfg->getBumpVectorContext(), NE),
+      const_cast<CXXConstructExpr *>(NE->getConstructExpr()));
 
   if (NE->getInitializer())
     Block = Visit(NE->getInitializer());