Index: include/clang/Analysis/CFG.h =================================================================== --- include/clang/Analysis/CFG.h +++ include/clang/Analysis/CFG.h @@ -155,14 +155,35 @@ // 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(); + 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(); @@ -172,10 +193,27 @@ return Trigger.dyn_cast(); } - const ConstructionContext *getPersistentCopy(BumpVectorContext &C) const { - ConstructionContext *CC = C.getAllocator().Allocate(); - *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 @@ 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, Index: lib/Analysis/CFG.cpp =================================================================== --- lib/Analysis/CFG.cpp +++ lib/Analysis/CFG.cpp @@ -475,7 +475,8 @@ // 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 + ConstructionContextMap{}; bool badCFG = false; const CFG::BuildOptions &BuildOpts; @@ -654,12 +655,12 @@ // 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 @@ 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 @@ 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(Child)) { - assert(CurrentConstructionContext.isNull() && - "Already within a construction context!"); - CurrentConstructionContext = ConstructionContext(Trigger); + if (auto *CE = dyn_cast(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(Child)) { - EnterConstructionContextIfNecessary(Trigger, Cleanups->getSubExpr()); + findConstructionContexts(ContextSoFar, Cleanups->getSubExpr()); + } else if (auto *BTE = dyn_cast(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 @@ // 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ autoCreateBlock(); appendStmt(Block, NE); - EnterConstructionContextIfNecessary( - NE, const_cast(NE->getConstructExpr())); + findConstructionContexts( + ConstructionContext::create(cfg->getBumpVectorContext(), NE), + const_cast(NE->getConstructExpr())); if (NE->getInitializer()) Block = Visit(NE->getInitializer());