Skip to content

Commit

Permalink
[CFG] Fix an assertion failure with static initializers
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Xazax-hun committed Dec 24, 2019
1 parent 25cf5d9 commit 379613d
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 14 deletions.
2 changes: 1 addition & 1 deletion clang/lib/Analysis/CFG.cpp
Expand Up @@ -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
Expand Down
35 changes: 22 additions & 13 deletions clang/unittests/Analysis/CFGBuildResult.h
Expand Up @@ -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 {
Expand All @@ -22,46 +23,54 @@ 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");
Stmt *Body = Func->getBody();
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);
}

Expand Down
16 changes: 16 additions & 0 deletions clang/unittests/Analysis/CFGTest.cpp
Expand Up @@ -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) {
Expand Down

0 comments on commit 379613d

Please sign in to comment.