[clang-ast] infinity templates indirect recursion cause custom clang-tidy rule to crash on trivial astMatcher in classIsDerivedFrom

[datacompboy]

Minimal repro is:

template<typename T> struct t1;
template<typename T> struct t2: t1< T > {};
template<typename T> struct t1: t2< T > {};

int main() {
  t1<int> a;
  return 0;
}

Causing stack-crash in infinity chain of

clang::ast_matchers::internal::(anonymous namespace)::MatchASTVisitor::classIsDerivedFrom()

this can't be compiled by clang either, but doesn't trigger recursion.

It does trigger recursion in clang with

template<typename T> struct t1: t2< t1< T > > {};

or

template<typename T> struct t1: t2< t2< T > > {};

but the recursion is cut with correct error:

mint.cpp:4:33: note: use -ftemplate-depth=N to increase recursive template instantiation depth

so only clang_tidy is affected.

[llvmbot]

@llvm/issue-subscribers-clang-tidy

[datacompboy]

Sent proposed fix as https://reviews.llvm.org/D126077

[njames93]

I can't reproduce the crash with a trunk build - release with assertions. Can you narrow down a reproducer that repros on trunk please. Also would it be possible to find out which check is causing the crash.

release-tidy --checks='*' crash.cpp
Error while trying to load a compilation database:
Could not auto-detect compilation database for file "crash.cpp"
No compilation database found in /home/nathan/test or any parent directory
fixed-compilation-database: Error while opening fixed database: No such file or directory
json-compilation-database: Error while opening JSON database: No such file or directory
Running without flags.
6 warnings and 1 error generated.
Error while processing /home/nathan/test/crash.cpp.
/home/nathan/test/crash.cpp:1:29: warning: declaration must be declared within the '__llvm_libc' namespace [llvmlibc-implementation-in-namespace]
template<typename T> struct t1;
                            ^
/home/nathan/test/crash.cpp:2:29: warning: declaration must be declared within the '__llvm_libc' namespace [llvmlibc-implementation-in-namespace]
template<typename T> struct t2: t1< T > {};
                            ^
/home/nathan/test/crash.cpp:2:33: error: base class has incomplete type [clang-diagnostic-error]
template<typename T> struct t2: t1< T > {};
                                ^~~~~~~
/home/nathan/test/crash.cpp:3:33: note: in instantiation of template class 't2<int>' requested here
template<typename T> struct t1: t2< T > {};
                                ^
/home/nathan/test/crash.cpp:7:11: note: in instantiation of template class 't1<int>' requested here
  t1<int> a;
          ^
/home/nathan/test/crash.cpp:3:29: note: definition of 't1<int>' is not complete until the closing '}'
template<typename T> struct t1: t2< T > {};
                            ^
/home/nathan/test/crash.cpp:3:29: warning: declaration must be declared within the '__llvm_libc' namespace [llvmlibc-implementation-in-namespace]
template<typename T> struct t1: t2< T > {};
                            ^
/home/nathan/test/crash.cpp:6:5: warning: declaration must be declared within the '__llvm_libc' namespace [llvmlibc-implementation-in-namespace]
int main() {
    ^
/home/nathan/test/crash.cpp:6:5: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
int main() {
~~~ ^
auto       -> int
/home/nathan/test/crash.cpp:7:11: warning: variable name 'a' is too short, expected at least 3 characters [readability-identifier-length]
  t1<int> a;
          ^
Found compiler error(s).

[datacompboy]

Thanks, I narrowed down to couple google internal checks that triggers this behaviour.

Triggering condition is trivial:

void $Some$Check::registerMatchers(MatchFinder* finder) {
  if (!getLangOpts().CPlusPlus) {
    return;
  }

  finder->addMatcher(
      ast_matchers::traverse(
          TK_AsIs,
          cxxRecordDecl(isDerivedFrom("::$Some$::$Base$"))
              .bind("$some$_$base$_class")),
      this);
}

And I don't see how could I add a test-only matcher for this case -- if you gave an suggestion i'd be happy to hear it -- otherwise I'll read more code around looking for a testing solution.

[datacompboy]

Oh, right, that could just be wrapped into a unit-test for ASTMatchers. I'll update a PR.

[datacompboy]

It looks like the condition is not used in any of checks included in clang and the direct matching is not used as is as well, so the bug is quite isolated to ASTMatchers / classIsDerivedFrom.