[PATCH] D111210: [Analysis][CFG] Fix CFG building for standalone `CaseStmt`s.

[Clement Courbet via Phabricator via cfe-commits]

courbet created this revision.
courbet added reviewers: Szelethus, NoQ.
courbet requested review of this revision.
Herald added a project: clang.

One is supposed ot be able to create a CFG for any statement, as per the
comment on `CFGBuilder::buildCFG`:

"The AST can represent an arbitrary statement.  Examples include a single
expression or a function body (compound statement)"

When building a CFG from a `CaseStmt`, we currently have a crash:

  clang/lib/Analysis/CFG.cpp:4242: clang::CFGBlock *(anonymous
  namespace)::CFGBuilder::VisitCaseStmt(clang::CaseStmt *): Assertion
  `SwitchTerminatedBlock' failed.

Add a unit test to show this, and avoid updating the parent `SwitchStmt`
block when none exists.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D111210

Files:
  clang/lib/Analysis/CFG.cpp
  clang/unittests/Analysis/CFGTest.cpp


Index: clang/unittests/Analysis/CFGTest.cpp
===================================================================
--- clang/unittests/Analysis/CFGTest.cpp
+++ clang/unittests/Analysis/CFGTest.cpp
@@ -268,6 +268,50 @@
   }
 }
 
+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;
+    Options.AddImplicitDtors = true;
+    if (std::unique_ptr<CFG> Cfg =
+            CFG::buildCFG(nullptr, Body, Result.Context, Options))
+      TheBuildResult = {BuildResult::BuiltCFG, Func, std::move(Cfg),
+                        std::move(AST)};
+  }
+};
+
+// It is valid to create a CFG for a single statement. Creating a CFG for an
+// isolated `CaseStmt` should not crash.`
+TEST(CFG, SwitchCase) {
+  const char *Code = "void f(int i) {\n"
+                     "  switch (i) {\n"
+                     "    case 0:\n"
+                     "      return;\n"
+                     "  }\n"
+                     "}\n";
+  std::vector<std::string> Args = {"-std=c++11",
+                                   "-fno-delayed-template-parsing"};
+  std::unique_ptr<ASTUnit> AST = tooling::buildASTFromCodeWithArgs(Code, Args);
+  ASSERT_NE(AST, nullptr);
+
+  auto Matches = ast_matchers::match(ast_matchers::caseStmt().bind("case"),
+                                     AST->getASTContext());
+  auto *Case = ast_matchers::selectFirst<CaseStmt>("case", Matches);
+  std::unique_ptr<CFG> Cfg =
+      CFG::buildCFG(nullptr, const_cast<CaseStmt *>(Case),
+                    &AST->getASTContext(), CFG::BuildOptions{});
+}
+
 } // namespace
 } // namespace analysis
 } // namespace clang
Index: clang/lib/Analysis/CFG.cpp
===================================================================
--- clang/lib/Analysis/CFG.cpp
+++ clang/lib/Analysis/CFG.cpp
@@ -4215,10 +4215,12 @@
       else
         TopBlock = currentBlock;
 
-      addSuccessor(SwitchTerminatedBlock,
-                   shouldAddCase(switchExclusivelyCovered, switchCond,
-                                 CS, *Context)
-                   ? currentBlock : nullptr);
+      if (SwitchTerminatedBlock)
+        addSuccessor(
+            SwitchTerminatedBlock,
+            shouldAddCase(switchExclusivelyCovered, switchCond, CS, *Context)
+                ? currentBlock
+                : nullptr);
 
       LastBlock = currentBlock;
       CS = cast<CaseStmt>(Sub);
@@ -4241,10 +4243,10 @@
 
   // Add this block to the list of successors for the block with the switch
   // statement.
-  assert(SwitchTerminatedBlock);
-  addSuccessor(SwitchTerminatedBlock, CaseBlock,
-               shouldAddCase(switchExclusivelyCovered, switchCond,
-                             CS, *Context));
+  if (SwitchTerminatedBlock)
+    addSuccessor(
+        SwitchTerminatedBlock, CaseBlock,
+        shouldAddCase(switchExclusivelyCovered, switchCond, CS, *Context));
 
   // We set Block to NULL to allow lazy creation of a new block (if necessary)
   Block = nullptr;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D111210.377455.patch
Type: text/x-patch
Size: 3391 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20211006/e3256204/attachment.bin>