UNREACHABLE executed in Decl::castFromDeclContext when inheriting from template parameter

[aaronpuchert]

Compiling the following file with -fsyntax-only -Wsuggest-lifetime-category results in executing llvm_unreachable("a decl that inherits DeclContext isn't handled"); in Decl::castFromDeclContext.

template<class...> struct X;
template<class Arg> struct X<Arg> : Arg {};

The output, abbreviated for clarity:

a decl that inherits DeclContext isn't handled
UNREACHABLE executed at ../clang/lib/AST/DeclBase.cpp:874!
Stack dump:
0.      Program arguments: clang++ -cc1 -fsyntax-only -Wsuggest-lifetime-category lifetime-bug.cpp 
1.      lifetime-bug.cpp:2:43: current parser token ';'
2.      lifetime-bug.cpp:2:21: parsing struct/union/class body 'X<Arg>'
[...]
 #7 0x00007f4a50458b34 [...]/llvm/lib/Support/ErrorHandling.cpp:209:3
 #8 0x00007f4a4a210404 clang::Decl::castFromDeclContext(clang::DeclContext const*) [...]/clang/lib/AST/DeclBase.cpp:876:1
 #9 0x00007f4a49df7ef5 clang::cast_convert_decl_context<clang::Decl, false>::doit(clang::DeclContext*) [...]/clang/include/clang/AST/DeclBase.h:2472:5
#10 0x00007f4a49df7d95 llvm::cast_convert_val<clang::Decl, clang::DeclContext*, clang::DeclContext*>::doit(clang::DeclContext*) [...]/clang/include/clang/AST/DeclBase.h:2527:5
#11 0x00007f4a49df7d4e llvm::cast_retty<clang::Decl, clang::DeclContext*>::ret_type llvm::cast<clang::Decl, clang::DeclContext>(clang::DeclContext*) [...]/llvm/include/llvm/Support/Casting.h:265:3
#12 0x00007f4a49df7f95 clang::DeclContext::getParent() [...]/clang/include/clang/AST/DeclBase.h:1785:30
#13 0x00007f4a49df7f75 clang::DeclContext::getParent() const [...]/clang/include/clang/AST/DeclBase.h:1788:5
#14 0x00007f4a4a20df08 clang::DeclContext::isDependentContext() const [...]/clang/lib/AST/DeclBase.cpp:1119:10
#15 0x00007f4a4a9eabd6 clang::lifetime::getPointeeType(clang::CXXRecordDecl const*) [...]/clang/lib/Analysis/LifetimeTypeCategory.cpp:347:9
#16 0x00007f4a4a9ea686 clang::lifetime::getPointeeTypeImpl(clang::Type const*) [...]/clang/lib/Analysis/LifetimeTypeCategory.cpp:397:22
#17 0x00007f4a4a9e9f46 clang::lifetime::getPointeeType(clang::Type const*) [...]/clang/lib/Analysis/LifetimeTypeCategory.cpp:424:12
#18 0x00007f4a4a9e8ed1 clang::lifetime::classifyTypeCategoryImpl(clang::Type const*) [...]/clang/lib/Analysis/LifetimeTypeCategory.cpp:195:22
#19 0x00007f4a4a9e8d48 clang::lifetime::classifyTypeCategory(clang::Type const*) [...]/clang/lib/Analysis/LifetimeTypeCategory.cpp:284:13
#20 0x00007f4a47b22dcd clang::lifetime::classifyTypeCategory(clang::QualType) [...]/clang/include/clang/Analysis/Analyses/LifetimeTypeCategory.h:58:10
#21 0x00007f4a47b1dfbe clang::Sema::suggestLifetimeAttribute(clang::CXXRecordDecl*) [...]/clang/lib/Sema/SemaAttr.cpp:165:7
#22 0x00007f4a47f2d34e clang::Sema::CheckCompletedCXXClass(clang::Scope*, clang::CXXRecordDecl*) [...]/clang/lib/Sema/SemaDeclCXX.cpp:6567:1
[...]

We have DeclKind = 120, but no such Decl::Kind exists. If I'm not mistaken, the highest value is TranslationUnit = 77.

[aaronpuchert]

It looks like memory corruption, and indeed with AddressSanitizer:

ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffc15f71b30 at pc 0x7ff221a20ae9 bp 0x7ffc15f717c0 sp 0x7ffc15f717b8
READ of size 3 at 0x7ffc15f71b30 thread T0
    #0 0x7ff221a20ae8 in clang::Type::getTypeClass() const [...]/clang/include/clang/AST/Type.h:1856:75
    #1 0x7ff221a702bd in clang::FunctionType::classof(clang::Type const*) [...]/clang/include/clang/AST/Type.h:3688:15
    #2 0x7ff221a701d0 in llvm::isa_impl_wrap<clang::FunctionType, clang::Type const* const, clang::Type const*>::doit(clang::Type const* const&) [...]/llvm/include/llvm/Support/Casting.h:122:12
    #3 0x7ff221a6ff5e in llvm::cast_retty<clang::FunctionType, clang::Type const*>::ret_type llvm::dyn_cast<clang::FunctionType, clang::Type const>(clang::Type const*) [...]/llvm/include/llvm/Support/Casting.h:343:10
    #4 0x7ff221a5630d in clang::FunctionType const* clang::Type::castAs<clang::FunctionType>() const [...]/clang/include/clang/AST/Type.h:6974:24
    #5 0x7ff21b643948 in clang::FunctionDecl::getReturnType() const [...]/clang/include/clang/AST/Decl.h:2438:23
    #6 0x7ff21b811ddd in clang::lifetime::getPointeeType(clang::CXXRecordDecl const*) [...]/clang/lib/Analysis/LifetimeTypeCategory.cpp:348:29
    #7 0x7ff21b811364 in clang::lifetime::getPointeeTypeImpl(clang::Type const*) [...]/clang/lib/Analysis/LifetimeTypeCategory.cpp:397:22
    #8 0x7ff21b81049b in clang::lifetime::getPointeeType(clang::Type const*) [...]/clang/lib/Analysis/LifetimeTypeCategory.cpp:424:12
    #9 0x7ff21b80e971 in clang::lifetime::classifyTypeCategoryImpl(clang::Type const*) [...]/clang/lib/Analysis/LifetimeTypeCategory.cpp:195:22
    #10 0x7ff21b80e694 in clang::lifetime::classifyTypeCategory(clang::Type const*) [...]/clang/lib/Analysis/LifetimeTypeCategory.cpp:284:13
    #11 0x7ff217022ba1 in clang::lifetime::classifyTypeCategory(clang::QualType) [...]/clang/include/clang/Analysis/Analyses/LifetimeTypeCategory.h:58:10
    #12 0x7ff21701a2ad in clang::Sema::suggestLifetimeAttribute(clang::CXXRecordDecl*) [...]/clang/lib/Sema/SemaAttr.cpp:165:7
    #13 0x7ff2174b7a1b in clang::Sema::CheckCompletedCXXClass(clang::Scope*, clang::CXXRecordDecl*) [...]/clang/lib/Sema/SemaDeclCXX.cpp:6566:5
[...]

Address 0x7ffc15f71b30 is located in stack of thread T0 at offset 208 in frame
    #0 0x7ff21b81178f in clang::lifetime::getPointeeType(clang::CXXRecordDecl const*) [...]/clang/lib/Analysis/LifetimeTypeCategory.cpp:328

  This frame has 19 object(s):
    [32, 40) 'retval'
    [64, 80) 'agg.tmp2'
    [96, 112) 'ref.tmp'
    [128, 136) '__begin3'
    [160, 176) 'agg.tmp19'
    [192, 200) 'ref.tmp22' <== Memory access at offset 208 overflows this variable
    [224, 248) 'Ops'
    [288, 292) 'ref.tmp40'
    [304, 308) 'ref.tmp41'
    [320, 324) 'ref.tmp42'
    [336, 340) 'ref.tmp43'
    [352, 356) 'ref.tmp45'
    [368, 372) 'ref.tmp46'
    [384, 392) 'F'
    [416, 424) 'PointeeType'
    [448, 456) 'FoundMD'
    [480, 496) 'agg.tmp87'
    [512, 520) 'PointeeType90'
    [544, 560) 'ref.tmp94'
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
      (longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow [...]/clang/include/clang/AST/Type.h:1856:75 in clang::Type::getTypeClass() const
Shadow bytes around the buggy address:
  0x100002be6310: 00 00 00 00 00 00 00 00 f1 f1 f1 f1 00 f3 f3 f3
  0x100002be6320: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x100002be6330: 00 00 00 00 f1 f1 f1 f1 00 f3 f3 f3 00 00 00 00
  0x100002be6340: 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1
  0x100002be6350: 00 f2 f2 f2 00 00 f2 f2 00 00 f2 f2 00 f2 f2 f2
=>0x100002be6360: 00 00 f2 f2 00 f2[f2]f2 00 00 00 f2 f2 f2 f2 f2
  0x100002be6370: 04 f2 04 f2 04 f2 04 f2 04 f2 04 f2 00 f2 f2 f2
  0x100002be6380: 00 f2 f2 f2 00 f2 f2 f2 00 00 f2 f2 00 f2 f2 f2
  0x100002be6390: 00 00 f3 f3 00 00 00 00 00 00 00 00 00 00 00 00
  0x100002be63a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x100002be63b0: f1 f1 f1 f1 00 f2 f2 f2 00 f3 f3 f3 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc

[aaronpuchert]

I think I've found the issue. In hasMethodLike from clang/lib/Analysis/LifetimeTypeCategory.cpp we use CXXRecordDecl::forallBases to determine if any base has a certain CXXMethodDecl. To that end, we pass a lambda that returns whether such a method has not been found, so it returns false if we have found the method and then forallBases stops.

In this case we have a base of TemplateTypeParmType instead of RecordType, and then forallBases also returns false, even though the lambda was never called. This makes some sense, because at that point we can't say if something is indeed true for all bases. Although it would be cool if there was a three-valued return type instead.

I'm not sure what the proper solution is, because it seems like we want to look into templates here. Maybe we should just ignore the return value of forallBases and also set AllowShortCircuit = true to make sure we don't miss anything in the case of type-dependent bases.

[aaronpuchert]

I'm not sure what the proper solution is, because it seems like we want to look into templates here. Maybe we should just ignore the return value of forallBases and also set AllowShortCircuit = true to make sure we don't miss anything in the case of type-dependent bases.

Now I'm about to remove the AllowShortCircuit parameter, because there are no uses of it in master. I think it wouldn't be the right solution anyway. The type categories that we're here to find out can be dependent, so maybe we shouldn't even attempt to determine them on a dependent type.