[CFG] Fix crash on CFG building when deriving from a template.

[courbet]

ping

[NoQ]

Hi, interesting, the code looks great but at a glance I don't see why would you even want a CFG for an uninstantiated template. Every time you want to analyze the actual runtime behavior of a program, you'll have a fully instantiated template AST to build the CFG from.

[courbet]

Thanks !

In D121365#3720171, @NoQ wrote:

Hi, interesting, the code looks great but at a glance I don't see why would you even want a CFG for an uninstantiated template. Every time you want to analyze the actual runtime behavior of a program, you'll have a fully instantiated template AST to build the CFG from.

I'm hitting this for an analysis that is eagerly analyzing templates before instantiation, and does not require the base classes. It's always better to not crash anyway :)

clang/lib/Analysis/CFG.cpp

void CFGBuilder::addImplicitDtorsForDestructor(const CXXDestructorDecl *DD) {
   const CXXRecordDecl *RD = DD->getParent();
 
   // At the end destroy virtual base objects.
   for (const auto &VI : RD->vbases()) {
     // TODO: Add a VirtualBaseBranch to see if the most derived class
     // (which is different from the current class) is responsible for
     // destroying them.
     const CXXRecordDecl *CD = VI.getType()->getAsCXXRecordDecl();
-    if (!CD->hasTrivialDestructor()) {
+    if (CD && !CD->hasTrivialDestructor()) {
       autoCreateBlock();
       appendBaseDtor(Block, &VI);
     }
   }
 
   // Before virtual bases destroy direct base objects.
   for (const auto &BI : RD->bases()) {
     if (!BI.isVirtual()) {
       const CXXRecordDecl *CD = BI.getType()->getAsCXXRecordDecl();
-      if (!CD->hasTrivialDestructor()) {
+      if (CD && !CD->hasTrivialDestructor()) {
         autoCreateBlock();
         appendBaseDtor(Block, &BI);
       }
     }
   }
 
   // First destroy member objects.
   for (auto *FI : RD->fields()) {

clang/unittests/Analysis/CFGBuildResult.h

public:
   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))
+            CFG::buildCFG(Func, Body, Result.Context, Options))
       TheBuildResult = {BuildResult::BuiltCFG, Func, std::move(Cfg),
                         std::move(AST)};
   }
 };
 
-inline BuildResult BuildCFG(const char *Code, CFG::BuildOptions Options = {}) {
+template <typename FuncMatcherT = ast_matchers::internal::TrueMatcher>
+BuildResult BuildCFG(const char *Code, CFG::BuildOptions Options = {},
+                     FuncMatcherT FuncMatcher = ast_matchers::anything()) {
   std::vector<std::string> Args = {"-std=c++11",
                                    "-fno-delayed-template-parsing"};
   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.addMatcher(ast_matchers::functionDecl(FuncMatcher).bind("func"),
+                    &Callback);
 
   Finder.matchAST(Callback.AST->getASTContext());
   return std::move(Callback.TheBuildResult);
 }
 
 } // namespace analysis
 } // namespace clang

clang/unittests/Analysis/CFGTest.cpp

 TEST(CFG, VariableOfIncompleteType) {
   const char *Code = "template<class T> void f() {\n"
                      "  class Undefined;\n"
                      "  Undefined u;\n"
                      "}\n";
   EXPECT_EQ(BuildResult::BuiltCFG, BuildCFG(Code).getStatus());
 }
 
+// Constructing a CFG with a dependent base should not crash.
+TEST(CFG, DependantBaseAddImplicitDtors) {
+  const char *Code = R"(
+    template <class T>
+    struct Base {
+      virtual ~Base() {}
+    };
+
+    template <typename T>
+    struct Derived : public Base<T> {
+      virtual ~Derived() {}
+    };
+  )";
+  CFG::BuildOptions Options;
+  Options.AddImplicitDtors = true;
+  Options.setAllAlwaysAdd();
+  EXPECT_EQ(BuildResult::BuiltCFG,
+            BuildCFG(Code, Options, ast_matchers::hasName("~Derived<T>"))
+                .getStatus());
+}
+
 TEST(CFG, IsLinear) {
   auto expectLinear = [](bool IsLinear, const char *Code) {
     BuildResult B = BuildCFG(Code);
     EXPECT_EQ(BuildResult::BuiltCFG, B.getStatus());
     EXPECT_EQ(IsLinear, B.getCFG()->isLinear());
   };
 
   expectLinear(true, "void foo() {}");