Fix crash in implicit default constructor creation for a template record specialization that has a field declaration with an initializer expression

[arphaman]

Summary

This patch fixes a NULL pointer crash that happens when clang is trying to create an implicit default constructor for a specialization of a record template which is defined in a specialization of a parent record template and has a field declaration with an initializer expression.

The following piece of code reproduces this problem:

template<typename T>
class A {
public:
  template<int i> 
  struct Inner;
};

template<>
template<int i>
struct A<int>::Inner {
  int member = 42;
};

int main(void) {
  return A<int>::Inner<0>().member;
}

The crash happens because CXXRecordDecl::getTemplateInstantiationPattern returns nil when instantiating A<int>::Inner<0>, because it tries to get the instantiation pattern from the declaration of Inner (which has no definition and thus no instantiation pattern), rather than the specialized definition of Inner. This patch fixes this by making sure that the instantiation pattern comes from the definition rather than the declaration of the record template.

[rsmith]

lib/AST/DeclCXX.cpp

1348–1349

We should be stopping at A<int>::Inner because it's a member specialization; whether the primary template had a declaration or a definition is irrelevant. It looks like the problem is that the member specialization check is checking the wrong declaration; we should be checking CTD not NewCTD here.

[arphaman]

The updated patch follows Richard's explanation and fixes the problem by checking if CTD is a member specialization without irrelevant definition checks.

lib/AST/DeclCXX.cpp

1348–1349

Yes, you're right. Thanks for pointing me in the right direction!

[arphaman]

Abandoning since I discovered that another patch (https://reviews.llvm.org/D13419) that's currently in review fixes this issue.

lib/AST/DeclCXX.cpp

CXXRecordDecl::setTemplateSpecializationKind(TemplateSpecializationKind TSK) {
   llvm_unreachable("Not a class template or member class specialization");
 }
 
 const CXXRecordDecl *CXXRecordDecl::getTemplateInstantiationPattern() const {
   // If it's a class template specialization, find the template or partial
   // specialization from which it was instantiated.
   if (auto *TD = dyn_cast<ClassTemplateSpecializationDecl>(this)) {
     auto From = TD->getInstantiatedFrom();
     if (auto *CTD = From.dyn_cast<ClassTemplateDecl *>()) {
-      while (auto *NewCTD = CTD->getInstantiatedFromMemberTemplate()) {
-        if (NewCTD->isMemberSpecialization())
-          break;
+      while (true) {
+        if (!CTD->isMemberSpecialization()) {
+          if (auto *NewCTD = CTD->getInstantiatedFromMemberTemplate()) {
             CTD = NewCTD;
+            continue;
+          }
+        }
+        break;
       }
       return CTD->getTemplatedDecl()->getDefinition();
     }
     if (auto *CTPSD =
             From.dyn_cast<ClassTemplatePartialSpecializationDecl *>()) {
       while (auto *NewCTPSD = CTPSD->getInstantiatedFromMember()) {
         if (NewCTPSD->isMemberSpecialization())
           break;

test/CodeGenCXX/cxx11-crashes.cpp

+// RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple -std=c++11 %s -o - | FileCheck %s
+
+namespace rd28886662 {
+
+template<typename T>
+class A {
+public:
+  template<int i>
+  struct Inner;
+};
+
+template<>
+template<int i>
+struct A<int>::Inner {
+  int member = 42; // should be ok
+};
+
+int foo() {
+  return A<int>::Inner<0>().member;
+}
+
+// CHECK: _ZN10rd288866621AIiE5InnerILi0EEC2Ev
+// CHECK: store i32 42, i32* %member
+
+template<typename T>
+class B {
+public:
+  template<int i>
+  struct Inner;
+};
+
+template<typename T>
+template<int i>
+struct B<T>::Inner {
+  int member = 21;
+};
+
+int bar() {
+  return B<int>::Inner<0>().member;
+}
+
+// CHECK: _ZN10rd288866621BIiE5InnerILi0EEC2Ev
+// CHECK: store i32 21, i32* %member
+
+}

test/Modules/cxx-templates.cpp

void test() {
     cls<void>::nested_cls *nk1; // ok
     cls<void>::nested_cls_t<int> *nk2; // ok
     cls<void>::nested_cls_t<char> *nk3; // ok
     cls<int> uk1; // expected-error 1+{{explicit specialization of 'cls<int>' must be imported}} expected-error 1+{{definition of}}
     cls<int*> uk3; // expected-error 1+{{partial specialization of 'cls<type-parameter-0-0 *>' must be imported}} expected-error 1+{{definition of}}
     cls<char*> uk4; // expected-error 1+{{partial specialization of 'cls<type-parameter-0-0 *>' must be imported}} expected-error 1+{{definition of}}
     cls<void>::nested_cls unk1; // expected-error 1+{{explicit specialization of 'nested_cls' must be imported}} expected-error 1+{{definition of}}
     cls<void>::nested_cls_t<int> unk2; // expected-error 1+{{explicit specialization of 'nested_cls_t' must be imported}} expected-error 1+{{definition of}}
-    cls<void>::nested_cls_t<char> unk3; // expected-error 1+{{explicit specialization of 'nested_cls_t' must be imported}}
+    cls<void>::nested_cls_t<char> unk3; // expected-error 1+{{explicit specialization of 'nested_cls_t' must be imported}} expected-error 1+{{definition of}}
 
     // For enums, uses that would trigger instantiations of definitions are not
     // allowed.
     cls<void>::nested_enum e; // ok
     (void)cls<void>::nested_enum::e; // expected-error 1+{{definition of 'nested_enum' must be imported}} expected-error 1+{{declaration of 'e'}}
 
     // For variable template specializations, no uses are allowed because
     // specializations can change the type.