NoQ updated this revision to Diff 132053. NoQ added a comment. Remove the stack of contexts for now. We're not using it anywhere yet, and i'm not sure if it'd be necessary.
https://reviews.llvm.org/D42672 Files: include/clang/Analysis/AnalysisDeclContext.h include/clang/Analysis/CFG.h include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h lib/Analysis/AnalysisDeclContext.cpp lib/Analysis/CFG.cpp lib/Analysis/LiveVariables.cpp lib/StaticAnalyzer/Core/AnalysisManager.cpp lib/StaticAnalyzer/Core/CoreEngine.cpp lib/StaticAnalyzer/Core/ExprEngine.cpp lib/StaticAnalyzer/Core/ExprEngineCXX.cpp lib/StaticAnalyzer/Core/PathDiagnostic.cpp test/Analysis/cfg.cpp
Index: test/Analysis/cfg.cpp =================================================================== --- test/Analysis/cfg.cpp +++ test/Analysis/cfg.cpp @@ -116,7 +116,7 @@ // CHECK-NEXT: Succs (1): B1 // CHECK: [B1] // CHECK-NEXT: 1: CFGNewAllocator(A *) -// CHECK-NEXT: 2: (CXXConstructExpr, class A) +// CHECK-NEXT: 2: (CXXConstructExpr, [B1.3], class A) // CHECK-NEXT: 3: new A([B1.2]) // CHECK-NEXT: 4: A *a = new A(); // CHECK-NEXT: 5: a @@ -138,7 +138,7 @@ // CHECK: [B1] // CHECK-NEXT: 1: 5 // CHECK-NEXT: 2: CFGNewAllocator(A *) -// CHECK-NEXT: 3: (CXXConstructExpr, class A [5]) +// CHECK-NEXT: 3: (CXXConstructExpr, [B1.4], class A [5]) // CHECK-NEXT: 4: new A {{\[\[}}B1.1]] // CHECK-NEXT: 5: A *a = new A [5]; // CHECK-NEXT: 6: a @@ -331,7 +331,7 @@ // CHECK-NEXT: 3: [B1.2] (ImplicitCastExpr, ArrayToPointerDecay, int *) // CHECK-NEXT: 4: [B1.3] (ImplicitCastExpr, BitCast, void *) // CHECK-NEXT: 5: CFGNewAllocator(MyClass *) -// CHECK-NEXT: 6: (CXXConstructExpr, class MyClass) +// CHECK-NEXT: 6: (CXXConstructExpr, [B1.7], class MyClass) // CHECK-NEXT: 7: new ([B1.4]) MyClass([B1.6]) // CHECK-NEXT: 8: MyClass *obj = new (buffer) MyClass(); // CHECK-NEXT: Preds (1): B2 @@ -363,7 +363,7 @@ // CHECK-NEXT: 4: [B1.3] (ImplicitCastExpr, BitCast, void *) // CHECK-NEXT: 5: 5 // CHECK-NEXT: 6: CFGNewAllocator(MyClass *) -// CHECK-NEXT: 7: (CXXConstructExpr, class MyClass [5]) +// CHECK-NEXT: 7: (CXXConstructExpr, [B1.8], class MyClass [5]) // CHECK-NEXT: 8: new ([B1.4]) MyClass {{\[\[}}B1.5]] // CHECK-NEXT: 9: MyClass *obj = new (buffer) MyClass [5]; // CHECK-NEXT: Preds (1): B2 Index: lib/StaticAnalyzer/Core/PathDiagnostic.cpp =================================================================== --- lib/StaticAnalyzer/Core/PathDiagnostic.cpp +++ lib/StaticAnalyzer/Core/PathDiagnostic.cpp @@ -553,6 +553,9 @@ case CFGElement::Statement: return PathDiagnosticLocation(Source.castAs<CFGStmt>().getStmt(), SM, CallerCtx); + case CFGElement::Constructor: + return PathDiagnosticLocation( + Source.castAs<CFGConstructor>().getConstructor(), SM, CallerCtx); case CFGElement::Initializer: { const CFGInitializer &Init = Source.castAs<CFGInitializer>(); return PathDiagnosticLocation(Init.getInitializer()->getInit(), Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp =================================================================== --- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -209,7 +209,10 @@ // See if we're constructing an existing region by looking at the next // element in the CFG. const CFGBlock *B = CurrBldrCtx.getBlock(); - assert(isa<CXXConstructExpr>(((*B)[currStmtIdx]).castAs<CFGStmt>().getStmt())); + // TODO: Retrieve construction target from CFGConstructor directly. + assert( + (*B)[currStmtIdx].getAs<CFGConstructor>() || + isa<CXXConstructExpr>(((*B)[currStmtIdx]).castAs<CFGStmt>().getStmt())); unsigned int NextStmtIdx = currStmtIdx + 1; if (NextStmtIdx >= B->size()) return None; @@ -250,10 +253,14 @@ Previous = (*B)[PreviousStmtIdx]; } - if (Optional<CFGStmt> PrevStmtElem = Previous.getAs<CFGStmt>()) { + if (auto PrevStmtElem = Previous.getAs<CFGStmt>()) { if (auto *CtorExpr = dyn_cast<CXXConstructExpr>(PrevStmtElem->getStmt())) { return CtorExpr; } + } else if (auto PrevCtorElem = Previous.getAs<CFGConstructor>()) { + if (auto *CtorExpr = PrevCtorElem->getConstructor()) { + return CtorExpr; + } } return nullptr; Index: lib/StaticAnalyzer/Core/ExprEngine.cpp =================================================================== --- lib/StaticAnalyzer/Core/ExprEngine.cpp +++ lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -454,10 +454,13 @@ switch (E.getKind()) { case CFGElement::Statement: - ProcessStmt(const_cast<Stmt*>(E.castAs<CFGStmt>().getStmt()), Pred); + ProcessStmt(E.castAs<CFGStmt>(), Pred); + return; + case CFGElement::Constructor: + ProcessConstructor(E.castAs<CFGConstructor>(), Pred); return; case CFGElement::Initializer: - ProcessInitializer(E.castAs<CFGInitializer>().getInitializer(), Pred); + ProcessInitializer(E.castAs<CFGInitializer>(), Pred); return; case CFGElement::NewAllocator: ProcessNewAllocator(E.castAs<CFGNewAllocator>().getAllocatorExpr(), @@ -479,7 +482,7 @@ } static bool shouldRemoveDeadBindings(AnalysisManager &AMgr, - const CFGStmt S, + const Stmt *S, const ExplodedNode *Pred, const LocationContext *LC) { @@ -492,17 +495,17 @@ return true; // Is this on a non-expression? - if (!isa<Expr>(S.getStmt())) + if (!isa<Expr>(S)) return true; // Run before processing a call. - if (CallEvent::isCallStmt(S.getStmt())) + if (CallEvent::isCallStmt(S)) return true; // Is this an expression that is consumed by another expression? If so, // postpone cleaning out the state. ParentMap &PM = LC->getAnalysisDeclContext()->getParentMap(); - return !PM.isConsumedExpr(cast<Expr>(S.getStmt())); + return !PM.isConsumedExpr(cast<Expr>(S)); } void ExprEngine::removeDead(ExplodedNode *Pred, ExplodedNodeSet &Out, @@ -606,18 +609,48 @@ // Remove dead bindings and symbols. ExplodedNodeSet CleanedStates; - if (shouldRemoveDeadBindings(AMgr, S, Pred, Pred->getLocationContext())){ - removeDead(Pred, CleanedStates, currStmt, Pred->getLocationContext()); + if (shouldRemoveDeadBindings(AMgr, currStmt, Pred, + Pred->getLocationContext())) { + removeDead(Pred, CleanedStates, currStmt, + Pred->getLocationContext()); } else CleanedStates.Add(Pred); // Visit the statement. ExplodedNodeSet Dst; - for (ExplodedNodeSet::iterator I = CleanedStates.begin(), - E = CleanedStates.end(); I != E; ++I) { + for (auto I: CleanedStates) { ExplodedNodeSet DstI; // Visit the statement. - Visit(currStmt, *I, DstI); + Visit(currStmt, I, DstI); + Dst.insert(DstI); + } + + // Enqueue the new nodes onto the work list. + Engine.enqueue(Dst, currBldrCtx->getBlock(), currStmtIdx); +} + +void ExprEngine::ProcessConstructor(const CFGConstructor C, + ExplodedNode *Pred) { + // Reclaim any unnecessary nodes in the ExplodedGraph. + G.reclaimRecentlyAllocatedNodes(); + + const CXXConstructExpr *CE = C.getConstructor(); + PrettyStackTraceLoc CrashInfo(getContext().getSourceManager(), + CE->getLocStart(), + "Error evaluating statement"); + + // Remove dead bindings and symbols. + ExplodedNodeSet CleanedStates; + if (shouldRemoveDeadBindings(AMgr, CE, Pred, Pred->getLocationContext())){ + removeDead(Pred, CleanedStates, CE, Pred->getLocationContext()); + } else + CleanedStates.Add(Pred); + + // Visit the statement. + ExplodedNodeSet Dst; + for (auto I : CleanedStates) { + ExplodedNodeSet DstI; + VisitCXXConstructExpr(CE, I, DstI); Dst.insert(DstI); } Index: lib/StaticAnalyzer/Core/CoreEngine.cpp =================================================================== --- lib/StaticAnalyzer/Core/CoreEngine.cpp +++ lib/StaticAnalyzer/Core/CoreEngine.cpp @@ -585,8 +585,14 @@ } // At this point, we know we're processing a normal statement. - CFGStmt CS = (*Block)[Idx].castAs<CFGStmt>(); - PostStmt Loc(CS.getStmt(), N->getLocationContext()); + const Stmt *S = nullptr; + if (Optional<CFGStmt> CS = (*Block)[Idx].getAs<CFGStmt>()) + S = CS->getStmt(); + else + S = (*Block)[Idx].castAs<CFGConstructor>().getConstructor(); + + assert(S); + PostStmt Loc(S, N->getLocationContext()); if (Loc == N->getLocation().withTag(nullptr)) { // Note: 'N' should be a fresh node because otherwise it shouldn't be Index: lib/StaticAnalyzer/Core/AnalysisManager.cpp =================================================================== --- lib/StaticAnalyzer/Core/AnalysisManager.cpp +++ lib/StaticAnalyzer/Core/AnalysisManager.cpp @@ -29,6 +29,7 @@ Options.shouldSynthesizeBodies(), Options.shouldConditionalizeStaticInitializers(), /*addCXXNewAllocator=*/true, + /*addRichCXXConstructors=*/true, injector), Ctx(ASTCtx), Diags(diags), LangOpts(lang), PathConsumers(PDC), CreateStoreMgr(storemgr), CreateConstraintMgr(constraintmgr), Index: lib/Analysis/LiveVariables.cpp =================================================================== --- lib/Analysis/LiveVariables.cpp +++ lib/Analysis/LiveVariables.cpp @@ -452,10 +452,15 @@ continue; } - if (!elem.getAs<CFGStmt>()) + const Stmt *S = nullptr; + if (auto CS = elem.getAs<CFGStmt>()) + S = CS->getStmt(); + else if (auto CC = elem.getAs<CFGConstructor>()) + S = CC->getConstructor(); + else continue; - - const Stmt *S = elem.castAs<CFGStmt>().getStmt(); + assert(S); + TF.Visit(const_cast<Stmt*>(S)); stmtsToLiveness[S] = val; } Index: lib/Analysis/CFG.cpp =================================================================== --- lib/Analysis/CFG.cpp +++ lib/Analysis/CFG.cpp @@ -472,6 +472,8 @@ using LabelSetTy = llvm::SmallSetVector<LabelDecl *, 8>; LabelSetTy AddressTakenLabels; + ConstructionContext CurrentConstructionContext = {nullptr, nullptr}; + bool badCFG = false; const CFG::BuildOptions &BuildOpts; @@ -682,6 +684,17 @@ B->appendStmt(const_cast<Stmt*>(S), cfg->getBumpVectorContext()); } + void appendConstructor(CFGBlock *B, CXXConstructExpr *CE) { + if (CurrentConstructionContext.Constructor == CE) { + B->appendConstructor(CurrentConstructionContext, + cfg->getBumpVectorContext()); + return; + } + + // No valid construction context found. Fall back to statement. + B->appendStmt(CE, cfg->getBumpVectorContext()); + } + void appendInitializer(CFGBlock *B, CXXCtorInitializer *I) { B->appendInitializer(I, cfg->getBumpVectorContext()); } @@ -3872,7 +3885,7 @@ CFGBlock *CFGBuilder::VisitCXXConstructExpr(CXXConstructExpr *C, AddStmtChoice asc) { autoCreateBlock(); - appendStmt(Block, C); + appendConstructor(Block, C); return VisitChildren(C); } @@ -3882,15 +3895,22 @@ autoCreateBlock(); appendStmt(Block, NE); + if (auto *CE = const_cast<CXXConstructExpr *>(NE->getConstructExpr())) + CurrentConstructionContext = {/*Constructor=*/CE, /*Trigger=*/NE}; + if (NE->getInitializer()) Block = Visit(NE->getInitializer()); + if (BuildOpts.AddCXXNewAllocator) appendNewAllocator(Block, NE); + if (NE->isArray()) Block = Visit(NE->getArraySize()); + for (CXXNewExpr::arg_iterator I = NE->placement_arg_begin(), E = NE->placement_arg_end(); I != E; ++I) Block = Visit(*I); + return Block; } @@ -4210,11 +4230,12 @@ const CXXDestructorDecl * CFGImplicitDtor::getDestructorDecl(ASTContext &astContext) const { switch (getKind()) { - case CFGElement::Statement: case CFGElement::Initializer: case CFGElement::NewAllocator: case CFGElement::LoopExit: case CFGElement::LifetimeEnds: + case CFGElement::Statement: + case CFGElement::Constructor: llvm_unreachable("getDestructorDecl should only be used with " "ImplicitDtors"); case CFGElement::AutomaticObjectDtor: { @@ -4335,52 +4356,54 @@ for (CFG::const_iterator I = cfg->begin(), E = cfg->end(); I != E; ++I ) { unsigned j = 1; for (CFGBlock::const_iterator BI = (*I)->begin(), BEnd = (*I)->end() ; - BI != BEnd; ++BI, ++j ) { - if (Optional<CFGStmt> SE = BI->getAs<CFGStmt>()) { - const Stmt *stmt= SE->getStmt(); - std::pair<unsigned, unsigned> P((*I)->getBlockID(), j); - StmtMap[stmt] = P; - - switch (stmt->getStmtClass()) { - case Stmt::DeclStmtClass: - DeclMap[cast<DeclStmt>(stmt)->getSingleDecl()] = P; - break; - case Stmt::IfStmtClass: { - const VarDecl *var = cast<IfStmt>(stmt)->getConditionVariable(); - if (var) - DeclMap[var] = P; - break; - } - case Stmt::ForStmtClass: { - const VarDecl *var = cast<ForStmt>(stmt)->getConditionVariable(); - if (var) - DeclMap[var] = P; - break; - } - case Stmt::WhileStmtClass: { - const VarDecl *var = - cast<WhileStmt>(stmt)->getConditionVariable(); - if (var) - DeclMap[var] = P; - break; - } - case Stmt::SwitchStmtClass: { - const VarDecl *var = - cast<SwitchStmt>(stmt)->getConditionVariable(); - if (var) - DeclMap[var] = P; - break; - } - case Stmt::CXXCatchStmtClass: { - const VarDecl *var = - cast<CXXCatchStmt>(stmt)->getExceptionDecl(); - if (var) - DeclMap[var] = P; - break; - } - default: - break; - } + BI != BEnd; ++BI, ++j ) { + const Stmt *stmt = nullptr; + if (auto SE = BI->getAs<CFGStmt>()) + stmt = SE->getStmt(); + else if (auto CE = BI->getAs<CFGConstructor>()) + stmt = CE->getConstructor(); + else + continue; + + std::pair<unsigned, unsigned> P((*I)->getBlockID(), j); + StmtMap[stmt] = P; + + switch (stmt->getStmtClass()) { + case Stmt::DeclStmtClass: + DeclMap[cast<DeclStmt>(stmt)->getSingleDecl()] = P; + break; + case Stmt::IfStmtClass: { + const VarDecl *var = cast<IfStmt>(stmt)->getConditionVariable(); + if (var) + DeclMap[var] = P; + break; + } + case Stmt::ForStmtClass: { + const VarDecl *var = cast<ForStmt>(stmt)->getConditionVariable(); + if (var) + DeclMap[var] = P; + break; + } + case Stmt::WhileStmtClass: { + const VarDecl *var = cast<WhileStmt>(stmt)->getConditionVariable(); + if (var) + DeclMap[var] = P; + break; + } + case Stmt::SwitchStmtClass: { + const VarDecl *var = cast<SwitchStmt>(stmt)->getConditionVariable(); + if (var) + DeclMap[var] = P; + break; + } + case Stmt::CXXCatchStmtClass: { + const VarDecl *var = cast<CXXCatchStmt>(stmt)->getExceptionDecl(); + if (var) + DeclMap[var] = P; + break; + } + default: + break; } } } @@ -4575,14 +4598,11 @@ if (isa<CXXOperatorCallExpr>(S)) { OS << " (OperatorCall)"; - } - else if (isa<CXXBindTemporaryExpr>(S)) { + } else if (isa<CXXBindTemporaryExpr>(S)) { OS << " (BindTemporary)"; - } - else if (const CXXConstructExpr *CCE = dyn_cast<CXXConstructExpr>(S)) { - OS << " (CXXConstructExpr, " << CCE->getType().getAsString() << ")"; - } - else if (const CastExpr *CE = dyn_cast<CastExpr>(S)) { + } else if (const CXXConstructExpr *CCE = dyn_cast<CXXConstructExpr>(S)) { + OS << " (CXXConstructExpr, " << CCE->getType().getAsString() << ')'; + } else if (const CastExpr *CE = dyn_cast<CastExpr>(S)) { OS << " (" << CE->getStmtClassName() << ", " << CE->getCastKindName() << ", " << CE->getType().getAsString() @@ -4592,6 +4612,12 @@ // Expressions need a newline. if (isa<Expr>(S)) OS << '\n'; + } else if (Optional<CFGConstructor> CE = E.getAs<CFGConstructor>()) { + CE->getConstructor()->printPretty(OS, &Helper, + PrintingPolicy(Helper.getLangOpts())); + OS << " (CXXConstructExpr, "; + Helper.handledStmt((CE->getTriggerStmt()), OS); + OS << ", " << CE->getType().getAsString() << ")\n"; } else if (Optional<CFGInitializer> IE = E.getAs<CFGInitializer>()) { const CXXCtorInitializer *I = IE->getInitializer(); if (I->isBaseInitializer()) Index: lib/Analysis/AnalysisDeclContext.cpp =================================================================== --- lib/Analysis/AnalysisDeclContext.cpp +++ lib/Analysis/AnalysisDeclContext.cpp @@ -67,7 +67,8 @@ ASTContext &ASTCtx, bool useUnoptimizedCFG, bool addImplicitDtors, bool addInitializers, bool addTemporaryDtors, bool addLifetime, bool addLoopExit, bool synthesizeBodies, bool addStaticInitBranch, - bool addCXXNewAllocator, CodeInjector *injector) + bool addCXXNewAllocator, bool addRichCXXConstructors, + CodeInjector *injector) : Injector(injector), FunctionBodyFarm(ASTCtx, injector), SynthesizeBodies(synthesizeBodies) { cfgBuildOptions.PruneTriviallyFalseEdges = !useUnoptimizedCFG; @@ -78,6 +79,7 @@ cfgBuildOptions.AddLoopExit = addLoopExit; cfgBuildOptions.AddStaticInitBranches = addStaticInitBranch; cfgBuildOptions.AddCXXNewAllocator = addCXXNewAllocator; + cfgBuildOptions.AddRichCXXConstructors = addRichCXXConstructors; } void AnalysisDeclContextManager::clear() { Contexts.clear(); } Index: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h =================================================================== --- include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h +++ include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h @@ -196,6 +196,8 @@ void ProcessStmt(const CFGStmt S, ExplodedNode *Pred); + void ProcessConstructor(const CFGConstructor C, ExplodedNode *Pred); + void ProcessLoopExit(const Stmt* S, ExplodedNode *Pred); void ProcessInitializer(const CFGInitializer I, ExplodedNode *Pred); Index: include/clang/Analysis/CFG.h =================================================================== --- include/clang/Analysis/CFG.h +++ include/clang/Analysis/CFG.h @@ -15,7 +15,7 @@ #ifndef LLVM_CLANG_ANALYSIS_CFG_H #define LLVM_CLANG_ANALYSIS_CFG_H -#include "clang/AST/Stmt.h" +#include "clang/AST/ExprCXX.h" #include "clang/Analysis/Support/BumpVector.h" #include "clang/Basic/LLVM.h" #include "llvm/ADT/DenseMap.h" @@ -56,6 +56,7 @@ enum Kind { // main kind Statement, + Constructor, Initializer, NewAllocator, LifetimeEnds, @@ -117,7 +118,7 @@ class CFGStmt : public CFGElement { public: - CFGStmt(Stmt *S) : CFGElement(Statement, S) {} + explicit CFGStmt(Stmt *S) : CFGElement(Statement, S) {} const Stmt *getStmt() const { return static_cast<const Stmt *>(Data1.getPointer()); @@ -133,11 +134,56 @@ } }; +// This is bulky data for CFGConstructor which would not fit into the +// CFGElement's room (pair of pointers). Contains the information +// necessary to express what memory is being initialized by +// the construction. +struct ConstructionContext { + CXXConstructExpr *Constructor; + Stmt *Trigger; + + const ConstructionContext *getPersistentCopy(BumpVectorContext &C) const { + ConstructionContext *CC = C.getAllocator().Allocate<ConstructionContext>(); + *CC = *this; + return CC; + } +}; + +/// CFGConstructor - Represents C++ constructor call. Maintains information +/// necessary to figure out what memory is being initialized by the +/// constructor expression. +class CFGConstructor : public CFGElement { +public: + explicit CFGConstructor(const ConstructionContext *C) + : CFGElement(Constructor, C) {} + + const ConstructionContext *getConstructionContext() const { + return static_cast<ConstructionContext *>(Data1.getPointer()); + } + + CXXConstructExpr *getConstructor() const { + return getConstructionContext()->Constructor; + } + + QualType getType() const { return getConstructor()->getType(); } + + Stmt *getTriggerStmt() const { return getConstructionContext()->Trigger; } + +private: + friend class CFGElement; + + CFGConstructor() = default; + + static bool isKind(const CFGElement &E) { + return E.getKind() == Constructor; + } +}; + /// CFGInitializer - Represents C++ base or member initializer from /// constructor's initialization list. class CFGInitializer : public CFGElement { public: - CFGInitializer(CXXCtorInitializer *initializer) + explicit CFGInitializer(CXXCtorInitializer *initializer) : CFGElement(Initializer, initializer) {} CXXCtorInitializer* getInitializer() const { @@ -747,6 +793,10 @@ Elements.push_back(CFGStmt(statement), C); } + void appendConstructor(const ConstructionContext &CC, BumpVectorContext &C) { + Elements.push_back(CFGConstructor(CC.getPersistentCopy(C)), C); + } + void appendInitializer(CXXCtorInitializer *initializer, BumpVectorContext &C) { Elements.push_back(CFGInitializer(initializer), C); @@ -855,6 +905,7 @@ bool AddStaticInitBranches = false; bool AddCXXNewAllocator = false; bool AddCXXDefaultInitExprInCtors = false; + bool AddRichCXXConstructors = false; BuildOptions() = default; Index: include/clang/Analysis/AnalysisDeclContext.h =================================================================== --- include/clang/Analysis/AnalysisDeclContext.h +++ include/clang/Analysis/AnalysisDeclContext.h @@ -439,6 +439,7 @@ bool synthesizeBodies = false, bool addStaticInitBranches = false, bool addCXXNewAllocator = true, + bool addRichCXXConstructors = true, CodeInjector *injector = nullptr); AnalysisDeclContext *getContext(const Decl *D);
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits