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

Artem Dergachev via cfe-commits cfe-commits at lists.llvm.org
Fri Feb 23 14:20:39 PST 2018


Author: dergachev
Date: Fri Feb 23 14:20:39 2018
New Revision: 325966

URL: http://llvm.org/viewvc/llvm-project?rev=325966&view=rev
Log:
[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

Modified:
    cfe/trunk/include/clang/Analysis/CFG.h
    cfe/trunk/lib/Analysis/CFG.cpp

Modified: cfe/trunk/include/clang/Analysis/CFG.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/CFG.h?rev=325966&r1=325965&r2=325966&view=diff
==============================================================================
--- cfe/trunk/include/clang/Analysis/CFG.h (original)
+++ cfe/trunk/include/clang/Analysis/CFG.h Fri Feb 23 14:20:39 2018
@@ -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,

Modified: cfe/trunk/lib/Analysis/CFG.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CFG.cpp?rev=325966&r1=325965&r2=325966&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/CFG.cpp (original)
+++ cfe/trunk/lib/Analysis/CFG.cpp Fri Feb 23 14:20:39 2018
@@ -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(c
   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::buildCF
   // 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(CXX
   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(D
   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(Re
 
   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::VisitCXXBindTempor
     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(CX
   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());




More information about the cfe-commits mailing list