Author: Gabor Horvath Date: 2019-12-23T16:35:37-08:00 New Revision: 379613d7c7fa9698a2c300f4067165b5309a5e6e
URL: https://github.com/llvm/llvm-project/commit/379613d7c7fa9698a2c300f4067165b5309a5e6e DIFF: https://github.com/llvm/llvm-project/commit/379613d7c7fa9698a2c300f4067165b5309a5e6e.diff LOG: [CFG] Fix an assertion failure with static initializers The CFGBlock::getLastCondition was not prepared for static initializer branches. This patch also revamps CFG unit tests. Earlier the lifetime of the AST was smaller than the CFG. So all the AST pointers within the CFG blocks were dangling. This was OK, since none of the tests dereferenced those pointers. This was, however, a timed bomb. There were patches in the past that were reverted partially due to this problem. Differential revision: https://reviews.llvm.org/D71791 Added: Modified: clang/lib/Analysis/CFG.cpp clang/unittests/Analysis/CFGBuildResult.h clang/unittests/Analysis/CFGTest.cpp Removed: ################################################################################ diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp index 07945a80a311..4c1ea8995f9f 100644 --- a/clang/lib/Analysis/CFG.cpp +++ b/clang/lib/Analysis/CFG.cpp @@ -5919,7 +5919,7 @@ const Expr *CFGBlock::getLastCondition() const { return nullptr; const Stmt *Cond = StmtElem->getStmt(); - if (isa<ObjCForCollectionStmt>(Cond)) + if (isa<ObjCForCollectionStmt>(Cond) || isa<DeclStmt>(Cond)) return nullptr; // Only ObjCForCollectionStmt is known not to be a non-Expr terminator, hence diff --git a/clang/unittests/Analysis/CFGBuildResult.h b/clang/unittests/Analysis/CFGBuildResult.h index 17511dcd46c8..252e608d28e8 100644 --- a/clang/unittests/Analysis/CFGBuildResult.h +++ b/clang/unittests/Analysis/CFGBuildResult.h @@ -6,9 +6,10 @@ // //===----------------------------------------------------------------------===// -#include "clang/Analysis/CFG.h" #include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Analysis/CFG.h" #include "clang/Tooling/Tooling.h" +#include <memory> namespace clang { namespace analysis { @@ -22,20 +23,27 @@ class BuildResult { BuiltCFG, }; - BuildResult(Status S, std::unique_ptr<CFG> Cfg = nullptr) - : S(S), Cfg(std::move(Cfg)) {} + BuildResult(Status S, std::unique_ptr<CFG> Cfg = nullptr, + std::unique_ptr<ASTUnit> AST = nullptr) + : S(S), Cfg(std::move(Cfg)), AST(std::move(AST)) {} Status getStatus() const { return S; } CFG *getCFG() const { return Cfg.get(); } + ASTUnit *getAST() const { return AST.get(); } private: Status S; std::unique_ptr<CFG> Cfg; + std::unique_ptr<ASTUnit> AST; }; class CFGCallback : public ast_matchers::MatchFinder::MatchCallback { public: + CFGCallback(std::unique_ptr<ASTUnit> AST) : AST(std::move(AST)) {} + + std::unique_ptr<ASTUnit> AST; BuildResult TheBuildResult = BuildResult::ToolRan; + CFG::BuildOptions Options; void run(const ast_matchers::MatchFinder::MatchResult &Result) override { const auto *Func = Result.Nodes.getNodeAs<FunctionDecl>("func"); @@ -43,25 +51,26 @@ class CFGCallback : public ast_matchers::MatchFinder::MatchCallback { if (!Body) return; TheBuildResult = BuildResult::SawFunctionBody; - CFG::BuildOptions Options; Options.AddImplicitDtors = true; if (std::unique_ptr<CFG> Cfg = CFG::buildCFG(nullptr, Body, Result.Context, Options)) - TheBuildResult = {BuildResult::BuiltCFG, std::move(Cfg)}; + TheBuildResult = {BuildResult::BuiltCFG, std::move(Cfg), std::move(AST)}; } }; -inline BuildResult BuildCFG(const char *Code) { - CFGCallback Callback; - - ast_matchers::MatchFinder Finder; - Finder.addMatcher(ast_matchers::functionDecl().bind("func"), &Callback); - std::unique_ptr<tooling::FrontendActionFactory> Factory( - tooling::newFrontendActionFactory(&Finder)); +inline BuildResult BuildCFG(const char *Code, CFG::BuildOptions Options = {}) { std::vector<std::string> Args = {"-std=c++11", "-fno-delayed-template-parsing"}; - if (!tooling::runToolOnCodeWithArgs(Factory->create(), Code, Args)) + std::unique_ptr<ASTUnit> AST = tooling::buildASTFromCodeWithArgs(Code, Args); + if (!AST) return BuildResult::ToolFailed; + + CFGCallback Callback(std::move(AST)); + Callback.Options = Options; + ast_matchers::MatchFinder Finder; + Finder.addMatcher(ast_matchers::functionDecl().bind("func"), &Callback); + + Finder.matchAST(Callback.AST->getASTContext()); return std::move(Callback.TheBuildResult); } diff --git a/clang/unittests/Analysis/CFGTest.cpp b/clang/unittests/Analysis/CFGTest.cpp index 27071dc5a5d9..1994658bed56 100644 --- a/clang/unittests/Analysis/CFGTest.cpp +++ b/clang/unittests/Analysis/CFGTest.cpp @@ -30,6 +30,22 @@ TEST(CFG, RangeBasedForOverDependentType) { EXPECT_EQ(BuildResult::SawFunctionBody, BuildCFG(Code).getStatus()); } +TEST(CFG, StaticInitializerLastCondition) { + const char *Code = "void f() {\n" + " int i = 5 ;\n" + " static int j = 3 ;\n" + "}\n"; + CFG::BuildOptions Options; + Options.AddStaticInitBranches = true; + Options.setAllAlwaysAdd(); + BuildResult B = BuildCFG(Code, Options); + EXPECT_EQ(BuildResult::BuiltCFG, B.getStatus()); + EXPECT_EQ(1u, B.getCFG()->getEntry().succ_size()); + CFGBlock *Block = *B.getCFG()->getEntry().succ_begin(); + EXPECT_TRUE(isa<DeclStmt>(Block->getTerminatorStmt())); + EXPECT_EQ(nullptr, Block->getLastCondition()); +} + // Constructing a CFG containing a delete expression on a dependent type should // not crash. TEST(CFG, DeleteExpressionOnDependentType) { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
