[PATCH] D78760: Check a class has a definition before iterating over its base classes

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak created this revision.
ahatanak added a reviewer: rsmith.
ahatanak added a project: clang.
Herald added subscribers: ributzka, dexonsmith, jkorous.
ahatanak added a comment.

I'm not sure why this shouldn't be caught in `Sema::CheckBaseSpecifier`. But there is a check for the definition of the class before `findCircularInheritance` is called, so I'm guessing there is a reason the code shouldn't be rejected here.


This fixes a crash when a class defined in a method of a templated class inherits from a class that is forward-declared.

I considered checking whether the base class is defined in `Sema::CheckBaseSpecifier` and rejecting the code if it isn't, but it seems like that's not how this should be fixed.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D78760

Files:
  clang/lib/AST/DeclCXX.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/SemaCXX/class.cpp


Index: clang/test/SemaCXX/class.cpp
===================================================================
--- clang/test/SemaCXX/class.cpp
+++ clang/test/SemaCXX/class.cpp
@@ -211,3 +211,21 @@
 struct PR9989 { 
   static int const PR9989_Member = sizeof PR9989_Member; 
 };
+
+namespace forward_declaration {
+struct B1 {};
+
+template <class T>
+struct S0 {
+  void m0() {
+    struct B0; // expected-note {{member is declared here}}
+
+    struct D : B0, B1 { // expected-error {{implicit instantiation}} expected-note {{in instantiation of}}
+    };
+  }
+};
+
+void test() {
+  S0<int>().m0(); // expected-note {{in instantiation of}}
+}
+} // namespace forward_declaration
Index: clang/lib/Sema/SemaDeclCXX.cpp
===================================================================
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -2611,6 +2611,9 @@
   if (auto Rec = Type->getAs<RecordType>()) {
     auto Decl = Rec->getAsCXXRecordDecl();
 
+    if (!Decl->hasDefinition())
+      return;
+
     // Iterate over its bases.
     for (const auto &BaseSpec : Decl->bases()) {
       QualType Base = Context.getCanonicalType(BaseSpec.getType())
Index: clang/lib/AST/DeclCXX.cpp
===================================================================
--- clang/lib/AST/DeclCXX.cpp
+++ clang/lib/AST/DeclCXX.cpp
@@ -173,6 +173,8 @@
   SmallVector<const CXXRecordDecl*, 8> WorkList = {StartRD};
   while (!WorkList.empty()) {
     const CXXRecordDecl *RD = WorkList.pop_back_val();
+    if (!RD->hasDefinition())
+      continue;
     for (const CXXBaseSpecifier &BaseSpec : RD->bases()) {
       if (const CXXRecordDecl *B = BaseSpec.getType()->getAsCXXRecordDecl()) {
         if (!SeenBaseTypes.insert(B).second)


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D78760.259706.patch
Type: text/x-patch
Size: 1731 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200423/7bd1edbc/attachment-0001.bin>