[llvm-branch-commits] [clang] 997a719 - PR48434: Work around crashes due to deserialization cycles via typedefs.

[Richard Smith via llvm-branch-commits]

Author: Richard Smith
Date: 2020-12-09T12:22:35-08:00
New Revision: 997a719d5a70100fcbbec1e51ce44cd66041bddc

URL: https://github.com/llvm/llvm-project/commit/997a719d5a70100fcbbec1e51ce44cd66041bddc
DIFF: https://github.com/llvm/llvm-project/commit/997a719d5a70100fcbbec1e51ce44cd66041bddc.diff

LOG: PR48434: Work around crashes due to deserialization cycles via typedefs.

Ensure that we can deserialize a TypedefType even while in the middle of
deserializing its TypedefDecl, by removing the need to look at the
TypedefDecl while constructing the TypedefType.

This fixes all the currently-known failures for PR48434, but it's not a
complete fix, because we can still trigger deserialization cycles, which
are not supposed to happen.

Added: 
    

Modified: 
    clang/include/clang/AST/ASTContext.h
    clang/include/clang/AST/Type.h
    clang/include/clang/AST/TypeProperties.td
    clang/lib/AST/ASTContext.cpp
    clang/lib/AST/Type.cpp
    clang/test/PCH/cxx-templates.cpp
    clang/test/PCH/cxx-templates.h

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h
index 7d58b426a3df..0a635875207d 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -1430,7 +1430,7 @@ class ASTContext : public RefCountedBase<ASTContext> {
   /// Return the unique reference to the type for the specified
   /// typedef-name decl.
   QualType getTypedefType(const TypedefNameDecl *Decl,
-                          QualType Canon = QualType()) const;
+                          QualType Underlying = QualType()) const;
 
   QualType getRecordType(const RecordDecl *Decl) const;
 

diff  --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index 60b8ee0f1614..99cfa3ae76f5 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@@ -4362,10 +4362,11 @@ class UnresolvedUsingType : public Type {
 class TypedefType : public Type {
   TypedefNameDecl *Decl;
 
-protected:
+private:
   friend class ASTContext; // ASTContext creates these.
 
-  TypedefType(TypeClass tc, const TypedefNameDecl *D, QualType can);
+  TypedefType(TypeClass tc, const TypedefNameDecl *D, QualType underlying,
+              QualType can);
 
 public:
   TypedefNameDecl *getDecl() const { return Decl; }

diff  --git a/clang/include/clang/AST/TypeProperties.td b/clang/include/clang/AST/TypeProperties.td
index a183ac0479c6..b582395c44a6 100644
--- a/clang/include/clang/AST/TypeProperties.td
+++ b/clang/include/clang/AST/TypeProperties.td
@@ -484,8 +484,12 @@ let Class = TagType in {
     let Read = [{ node->isDependentType() }];
   }
   def : Property<"declaration", DeclRef> {
-    // Serializing a reference to the canonical declaration is apparently
-    // necessary to make module-merging work.
+    // We don't know which declaration was originally referenced here, and we
+    // cannot reference a declaration that follows the use (because that can
+    // introduce deserialization cycles), so conservatively generate a
+    // reference to the first declaration.
+    // FIXME: If this is a reference to a class template specialization, that
+    // can still introduce a deserialization cycle.
     let Read = [{ node->getDecl()->getCanonicalDecl() }];
   }
 }

diff  --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index c52369cd8a02..057574ec2b82 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -4449,15 +4449,15 @@ QualType ASTContext::getTypeDeclTypeSlow(const TypeDecl *Decl) const {
 
 /// getTypedefType - Return the unique reference to the type for the
 /// specified typedef name decl.
-QualType
-ASTContext::getTypedefType(const TypedefNameDecl *Decl,
-                           QualType Canonical) const {
+QualType ASTContext::getTypedefType(const TypedefNameDecl *Decl,
+                                    QualType Underlying) const {
   if (Decl->TypeForDecl) return QualType(Decl->TypeForDecl, 0);
 
-  if (Canonical.isNull())
-    Canonical = getCanonicalType(Decl->getUnderlyingType());
+  if (Underlying.isNull())
+    Underlying = Decl->getUnderlyingType();
+  QualType Canonical = getCanonicalType(Underlying);
   auto *newType = new (*this, TypeAlignment)
-    TypedefType(Type::Typedef, Decl, Canonical);
+      TypedefType(Type::Typedef, Decl, Underlying, Canonical);
   Decl->TypeForDecl = newType;
   Types.push_back(newType);
   return QualType(newType, 0);

diff  --git a/clang/lib/AST/Type.cpp b/clang/lib/AST/Type.cpp
index c07cab9a4006..aa623b000fb5 100644
--- a/clang/lib/AST/Type.cpp
+++ b/clang/lib/AST/Type.cpp
@@ -3369,8 +3369,9 @@ void FunctionProtoType::Profile(llvm::FoldingSetNodeID &ID,
           getExtProtoInfo(), Ctx, isCanonicalUnqualified());
 }
 
-TypedefType::TypedefType(TypeClass tc, const TypedefNameDecl *D, QualType can)
-    : Type(tc, can, D->getUnderlyingType()->getDependence()),
+TypedefType::TypedefType(TypeClass tc, const TypedefNameDecl *D,
+                         QualType underlying, QualType can)
+    : Type(tc, can, underlying->getDependence()),
       Decl(const_cast<TypedefNameDecl *>(D)) {
   assert(!isa<TypedefType>(can) && "Invalid canonical type");
 }

diff  --git a/clang/test/PCH/cxx-templates.cpp b/clang/test/PCH/cxx-templates.cpp
index 3e1c3cee79a3..c5694425585d 100644
--- a/clang/test/PCH/cxx-templates.cpp
+++ b/clang/test/PCH/cxx-templates.cpp
@@ -180,3 +180,8 @@ namespace DependentTemplateName {
     getWithIdentifier<HasMember>();
   }
 }
+
+namespace ClassTemplateCycle {
+  extern T t;
+  int k = M;
+}

diff  --git a/clang/test/PCH/cxx-templates.h b/clang/test/PCH/cxx-templates.h
index b4ea2c23b3cc..7819a1ecb8e1 100644
--- a/clang/test/PCH/cxx-templates.h
+++ b/clang/test/PCH/cxx-templates.h
@@ -456,3 +456,19 @@ namespace DependentTemplateName {
   template <class T>
   TakesClassTemplate<T::template Member> getWithIdentifier();
 }
+
+namespace ClassTemplateCycle {
+  // Create a cycle: the typedef T refers to A<0, 8>, whose template argument
+  // list refers back to T.
+  template<int, int> struct A;
+  using T = A<0, sizeof(void*)>;
+  template<int N> struct A<N, sizeof(T*)> {};
+  T t;
+
+  // Create a cycle: the variable M refers to A<1, 1>, whose template argument
+  // list list refers back to M.
+  template<int, int> struct A;
+  const decltype(sizeof(A<1, 1>*)) M = 1;
+  template<int N> struct A<N, M> {};
+  A<1, 1> u;
+}