[ObjC++] Never pass structs that transitively contain __weak fields in

registers.

This patch fixes a bug in r328731 that caused structs transitively
containing __weak fields to be passed in registers. The patch replaces
the flag RecordDecl::CanPassInRegisters with a 2-bit enum that indicates
whether the struct or structs containing the struct are forced to be
passed indirectly.

rdar://problem/39194693

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@329617 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/AST/Decl.h b/include/clang/AST/Decl.h
index f9f5cf858debd5568c04e6f54942ae3c0c60c252..515ab3b71162200767aa83f83800e63d3724c7a1 100644 (file)
--- a/include/clang/AST/Decl.h
+++ b/include/clang/AST/Decl.h
@@ -3541,6 +3541,31 @@ public:
 ///   union Y { int A, B; };     // Has body with members A and B (FieldDecls).
 /// This decl will be marked invalid if *any* members are invalid.
 class RecordDecl : public TagDecl {
+public:
+  /// Enum that represents the different ways arguments are passed to and
+  /// returned from function calls. This takes into account the target-specific
+  /// and version-specific rules along with the rules determined by the
+  /// language.
+  enum ArgPassingKind {
+    /// The argument of this type can be passed directly in registers.
+    APK_CanPassInRegs,
+
+    /// The argument of this type cannot be passed directly in registers.
+    /// Records containing this type as a subobject are not forced to be passed
+    /// indirectly. This value is used only in C++. This value is required by
+    /// C++ because, in uncommon situations, it is possible for a class to have
+    /// only trivial copy/move constructors even when one of its subobjects has
+    /// a non-trivial copy/move constructor (if e.g. the corresponding copy/move
+    /// constructor in the derived class is deleted).
+    APK_CannotPassInRegs,
+
+    /// The argument of this type cannot be passed directly in registers.
+    /// Records containing this type as a subobject are forced to be passed
+    /// indirectly.
+    APK_CanNeverPassInRegs
+  };
+
+private:
   friend class DeclContext;
 
   // FIXME: This can be packed into the bitfields in Decl.
@@ -3571,17 +3596,14 @@ class RecordDecl : public TagDecl {
   bool NonTrivialToPrimitiveCopy : 1;
   bool NonTrivialToPrimitiveDestroy : 1;
 
-  /// True if this class can be passed in a non-address-preserving fashion
-  /// (such as in registers).
-  /// This does not imply anything about how the ABI in use will actually
-  /// pass an object of this class.
-  bool CanPassInRegisters : 1;
-
   /// Indicates whether this struct is destroyed in the callee. This flag is
   /// meaningless when Microsoft ABI is used since parameters are always
   /// destroyed in the callee.
   bool ParamDestroyedInCallee : 1;
 
+  /// Represents the way this type is passed to a function.
+  ArgPassingKind ArgPassingRestrictions : 2;
+
 protected:
   RecordDecl(Kind DK, TagKind TK, const ASTContext &C, DeclContext *DC,
              SourceLocation StartLoc, SourceLocation IdLoc,
@@ -3669,12 +3691,15 @@ public:
   /// it must have at least one trivial, non-deleted copy or move constructor.
   /// FIXME: This should be set as part of completeDefinition.
   bool canPassInRegisters() const {
-    return CanPassInRegisters;
+    return ArgPassingRestrictions == APK_CanPassInRegs;
+  }
+
+  ArgPassingKind getArgPassingRestrictions() const {
+    return ArgPassingRestrictions;
   }
 
-  /// Set that we can pass this RecordDecl in registers.
-  void setCanPassInRegisters(bool CanPass) {
-    CanPassInRegisters = CanPass;
+  void setArgPassingRestrictions(ArgPassingKind Kind) {
+    ArgPassingRestrictions = Kind;
   }
 
   bool isParamDestroyedInCallee() const {

diff --git a/include/clang/AST/Type.h b/include/clang/AST/Type.h
index a97f5af5cefab8ae9810d5205b99f29d26e86402..ab6e113f2bdc39d11d3d14902b670ca496102d8a 100644 (file)
--- a/include/clang/AST/Type.h
+++ b/include/clang/AST/Type.h
@@ -1149,8 +1149,6 @@ public:
   /// source object is placed in an uninitialized state.
   PrimitiveCopyKind isNonTrivialToPrimitiveDestructiveMove() const;
 
-  bool canPassInRegisters() const;
-
   enum DestructionKind {
     DK_none,
     DK_cxx_destructor,

diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp
index b49fda08a7c19b739a309dfa28593dbe0026c258..18ef94ba225173e31b141fde004170f05d82e22e 100644 (file)
--- a/lib/AST/Decl.cpp
+++ b/lib/AST/Decl.cpp
@@ -3956,7 +3956,7 @@ RecordDecl::RecordDecl(Kind DK, TagKind TK, const ASTContext &C,
       LoadedFieldsFromExternalStorage(false),
       NonTrivialToPrimitiveDefaultInitialize(false),
       NonTrivialToPrimitiveCopy(false), NonTrivialToPrimitiveDestroy(false),
-      CanPassInRegisters(true), ParamDestroyedInCallee(false) {
+      ParamDestroyedInCallee(false), ArgPassingRestrictions(APK_CanPassInRegs) {
   assert(classof(static_cast<Decl*>(this)) && "Invalid Kind!");
 }
 

diff --git a/lib/AST/DeclCXX.cpp b/lib/AST/DeclCXX.cpp
index 99ff1ca31e77490a883f9ce4a87ca7139b628ce2..dd653d8f573068dca55d3892a856bca6055059c6 100644 (file)
--- a/lib/AST/DeclCXX.cpp
+++ b/lib/AST/DeclCXX.cpp
@@ -421,6 +421,10 @@ CXXRecordDecl::setBases(CXXBaseSpecifier const * const *Bases,
     if (BaseClassDecl->hasVolatileMember())
       setHasVolatileMember(true);
 
+    if (BaseClassDecl->getArgPassingRestrictions() ==
+        RecordDecl::APK_CanNeverPassInRegs)
+      setArgPassingRestrictions(RecordDecl::APK_CanNeverPassInRegs);
+
     // Keep track of the presence of mutable fields.
     if (BaseClassDecl->hasMutableFields()) {
       data().HasMutableFields = true;
@@ -950,7 +954,7 @@ void CXXRecordDecl::addedMember(Decl *D) {
 
         // Structs with __weak fields should never be passed directly.
         if (LT == Qualifiers::OCL_Weak)
-          setCanPassInRegisters(false);
+          setArgPassingRestrictions(RecordDecl::APK_CanNeverPassInRegs);
 
         Data.HasIrrelevantDestructor = false;
       } else if (!Context.getLangOpts().ObjCAutoRefCount) {
@@ -1117,6 +1121,9 @@ void CXXRecordDecl::addedMember(Decl *D) {
           setHasObjectMember(true);
         if (FieldRec->hasVolatileMember())
           setHasVolatileMember(true);
+        if (FieldRec->getArgPassingRestrictions() ==
+            RecordDecl::APK_CanNeverPassInRegs)
+          setArgPassingRestrictions(RecordDecl::APK_CanNeverPassInRegs);
 
         // C++0x [class]p7:
         //   A standard-layout class is a class that:

diff --git a/lib/AST/Type.cpp b/lib/AST/Type.cpp
index cca5ddc1e48003977a57ac99495f36ad53274237..a2a60772b770166992a90326c6b023a02c4f76ba 100644 (file)
--- a/lib/AST/Type.cpp
+++ b/lib/AST/Type.cpp
@@ -2239,17 +2239,6 @@ QualType::isNonTrivialToPrimitiveDestructiveMove() const {
   return isNonTrivialToPrimitiveCopy();
 }
 
-bool QualType::canPassInRegisters() const {
-  if (const auto *RT =
-          getTypePtr()->getBaseElementTypeUnsafe()->getAs<RecordType>())
-    return RT->getDecl()->canPassInRegisters();
-
-  if (getQualifiers().getObjCLifetime() == Qualifiers::OCL_Weak)
-    return false;
-
-  return true;
-}
-
 bool Type::isLiteralType(const ASTContext &Ctx) const {
   if (isDependentType())
     return false;

diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index f650e046c0d7fd35fa4d1cad5aaa5a89541a89c7..9228d65250cd56d01169e6af2394fa9db6d98d17 100644 (file)
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -15465,8 +15465,13 @@ void Sema::ActOnFields(Scope *S, SourceLocation RecLoc, Decl *EnclosingDecl,
         Record->setNonTrivialToPrimitiveDestroy(true);
         Record->setParamDestroyedInCallee(true);
       }
-      if (!FT.canPassInRegisters())
-        Record->setCanPassInRegisters(false);
+
+      if (const auto *RT = FT->getAs<RecordType>()) {
+        if (RT->getDecl()->getArgPassingRestrictions() ==
+            RecordDecl::APK_CanNeverPassInRegs)
+          Record->setArgPassingRestrictions(RecordDecl::APK_CanNeverPassInRegs);
+      } else if (FT.getQualifiers().getObjCLifetime() == Qualifiers::OCL_Weak)
+        Record->setArgPassingRestrictions(RecordDecl::APK_CanNeverPassInRegs);
     }
 
     if (Record && FD->getType().isVolatileQualified())

diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp
index 55c7a9ae24d43e85137a5bc4bdfd2fbc22d1a256..08d3bc21a2461529918e1621e9e297d0abf0a6c4 100644 (file)
--- a/lib/Sema/SemaDeclCXX.cpp
+++ b/lib/Sema/SemaDeclCXX.cpp
@@ -5847,20 +5847,20 @@ static bool paramCanBeDestroyedInCallee(Sema &S, CXXRecordDecl *D,
   return HasNonDeletedCopyOrMove;
 }
 
-static bool computeCanPassInRegister(bool DestroyedInCallee,
-                                     const CXXRecordDecl *RD,
-                                     TargetInfo::CallingConvKind CCK,
-                                     Sema &S) {
+static RecordDecl::ArgPassingKind
+computeArgPassingRestrictions(bool DestroyedInCallee, const CXXRecordDecl *RD,
+                              TargetInfo::CallingConvKind CCK, Sema &S) {
   if (RD->isDependentType() || RD->isInvalidDecl())
-    return true;
+    return RecordDecl::APK_CanPassInRegs;
 
-  // The param cannot be passed in registers if CanPassInRegisters is already
-  // set to false.
-  if (!RD->canPassInRegisters())
-    return false;
+  // The param cannot be passed in registers if ArgPassingRestrictions is set to
+  // APK_CanNeverPassInRegs.
+  if (RD->getArgPassingRestrictions() == RecordDecl::APK_CanNeverPassInRegs)
+    return RecordDecl::APK_CanNeverPassInRegs;
 
   if (CCK != TargetInfo::CCK_MicrosoftX86_64)
-    return DestroyedInCallee;
+    return DestroyedInCallee ? RecordDecl::APK_CanPassInRegs
+                             : RecordDecl::APK_CannotPassInRegs;
 
   bool CopyCtorIsTrivial = false, CopyCtorIsTrivialForCall = false;
   bool DtorIsTrivialForCall = false;
@@ -5900,7 +5900,7 @@ static bool computeCanPassInRegister(bool DestroyedInCallee,
 
   // If the copy ctor and dtor are both trivial-for-calls, pass direct.
   if (CopyCtorIsTrivialForCall && DtorIsTrivialForCall)
-    return true;
+    return RecordDecl::APK_CanPassInRegs;
 
   // If a class has a destructor, we'd really like to pass it indirectly
   // because it allows us to elide copies.  Unfortunately, MSVC makes that
@@ -5914,8 +5914,8 @@ static bool computeCanPassInRegister(bool DestroyedInCallee,
   // passed in registers, which is non-conforming.
   if (CopyCtorIsTrivial &&
       S.getASTContext().getTypeSize(RD->getTypeForDecl()) <= 64)
-    return true;
-  return false;
+    return RecordDecl::APK_CanPassInRegs;
+  return RecordDecl::APK_CannotPassInRegs;
 }
 
 /// \brief Perform semantic checks on a class definition that has been
@@ -6090,8 +6090,8 @@ void Sema::CheckCompletedCXXClass(CXXRecordDecl *Record) {
   if (Record->hasNonTrivialDestructor())
     Record->setParamDestroyedInCallee(DestroyedInCallee);
 
-  Record->setCanPassInRegisters(
-      computeCanPassInRegister(DestroyedInCallee, Record, CCK, *this));
+  Record->setArgPassingRestrictions(
+      computeArgPassingRestrictions(DestroyedInCallee, Record, CCK, *this));
 }
 
 /// Look up the special member function that would be called by a special

diff --git a/lib/Serialization/ASTReaderDecl.cpp b/lib/Serialization/ASTReaderDecl.cpp
index a6cc69d623fc07d88882fad3b8472a0c1952b78b..af4cf84b9b39d183efc2b0fdcb3ce252177f5a5e 100644 (file)
--- a/lib/Serialization/ASTReaderDecl.cpp
+++ b/lib/Serialization/ASTReaderDecl.cpp
@@ -742,8 +742,8 @@ ASTDeclReader::VisitRecordDeclImpl(RecordDecl *RD) {
   RD->setNonTrivialToPrimitiveDefaultInitialize(Record.readInt());
   RD->setNonTrivialToPrimitiveCopy(Record.readInt());
   RD->setNonTrivialToPrimitiveDestroy(Record.readInt());
-  RD->setCanPassInRegisters(Record.readInt());
   RD->setParamDestroyedInCallee(Record.readInt());
+  RD->setArgPassingRestrictions((RecordDecl::ArgPassingKind)Record.readInt());
   return Redecl;
 }
 
@@ -4114,8 +4114,9 @@ void ASTDeclReader::UpdateDecl(Decl *D,
       bool HadRealDefinition =
           OldDD && (OldDD->Definition != RD ||
                     !Reader.PendingFakeDefinitionData.count(OldDD));
-      RD->setCanPassInRegisters(Record.readInt());
       RD->setParamDestroyedInCallee(Record.readInt());
+      RD->setArgPassingRestrictions(
+          (RecordDecl::ArgPassingKind)Record.readInt());
       ReadCXXRecordDefinition(RD, /*Update*/true);
 
       // Visible update is handled separately.

diff --git a/lib/Serialization/ASTWriter.cpp b/lib/Serialization/ASTWriter.cpp
index 3369a543685e5ecced35d6ea994469fd24a4d80e..7f1199f1b6c84052d3c792503570f3ec513b9766 100644 (file)
--- a/lib/Serialization/ASTWriter.cpp
+++ b/lib/Serialization/ASTWriter.cpp
@@ -5193,8 +5193,8 @@ void ASTWriter::WriteDeclUpdatesBlocks(RecordDataImpl &OffsetsRecord) {
       case UPD_CXX_INSTANTIATED_CLASS_DEFINITION: {
         auto *RD = cast<CXXRecordDecl>(D);
         UpdatedDeclContexts.insert(RD->getPrimaryContext());
-        Record.push_back(RD->canPassInRegisters());
         Record.push_back(RD->isParamDestroyedInCallee());
+        Record.push_back(RD->getArgPassingRestrictions());
         Record.AddCXXDefinitionData(RD);
         Record.AddOffset(WriteDeclContextLexicalBlock(
             *Context, const_cast<CXXRecordDecl *>(RD)));

diff --git a/lib/Serialization/ASTWriterDecl.cpp b/lib/Serialization/ASTWriterDecl.cpp
index 06ea8e7f2bcd267aa0c2a15c5ff4782e8effab2a..189de14cff14b141081c4da6f242628177455cc5 100644 (file)
--- a/lib/Serialization/ASTWriterDecl.cpp
+++ b/lib/Serialization/ASTWriterDecl.cpp
@@ -469,8 +469,8 @@ void ASTDeclWriter::VisitRecordDecl(RecordDecl *D) {
   Record.push_back(D->isNonTrivialToPrimitiveDefaultInitialize());
   Record.push_back(D->isNonTrivialToPrimitiveCopy());
   Record.push_back(D->isNonTrivialToPrimitiveDestroy());
-  Record.push_back(D->canPassInRegisters());
   Record.push_back(D->isParamDestroyedInCallee());
+  Record.push_back(D->getArgPassingRestrictions());
 
   if (D->getDeclContext() == D->getLexicalDeclContext() &&
       !D->hasAttrs() &&
@@ -1913,9 +1913,10 @@ void ASTWriter::WriteDeclAbbrevs() {
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1));
   // isNonTrivialToPrimitiveDestroy
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1));
-  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // canPassInRegisters
   // isParamDestroyedInCallee
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1));
+  // getArgPassingRestrictions
+  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 2));
 
   // DC
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6));   // LexicalOffset

diff --git a/test/CodeGenObjCXX/objc-struct-cxx-abi.mm b/test/CodeGenObjCXX/objc-struct-cxx-abi.mm
index de8c5f9fcb804414854588c2588cfcc28db9f026..dd9b88b0234d09fa27e93c673082f5d9ede9811e 100644 (file)
--- a/test/CodeGenObjCXX/objc-struct-cxx-abi.mm
+++ b/test/CodeGenObjCXX/objc-struct-cxx-abi.mm
@@ -8,6 +8,8 @@
 // pointer fields are passed directly.
 
 // CHECK: %[[STRUCT_STRONGWEAK:.*]] = type { i8*, i8* }
+// CHECK: %[[STRUCT_CONTAINSSTRONGWEAK:.*]] = type { %[[STRUCT_STRONGWEAK]] }
+// CHECK: %[[STRUCT_DERIVEDSTRONGWEAK:.*]] = type { %[[STRUCT_STRONGWEAK]] }
 // CHECK: %[[STRUCT_STRONG:.*]] = type { i8* }
 // CHECK: %[[STRUCT_S:.*]] = type { i8* }
 // CHECK: %[[STRUCT_CONTAINSNONTRIVIAL:.*]] = type { %{{.*}}, i8* }
@@ -21,6 +23,21 @@ struct StrongWeak {
   __weak id fweak;
 };
 
+#ifdef TRIVIALABI
+struct __attribute__((trivial_abi)) ContainsStrongWeak {
+#else
+struct ContainsStrongWeak {
+#endif
+  StrongWeak sw;
+};
+
+#ifdef TRIVIALABI
+struct __attribute__((trivial_abi)) DerivedStrongWeak : StrongWeak {
+#else
+struct DerivedStrongWeak : StrongWeak {
+#endif
+};
+
 #ifdef TRIVIALABI
 struct __attribute__((trivial_abi)) Strong {
 #else
@@ -84,6 +101,18 @@ StrongWeak testReturnStrongWeak(StrongWeak *a) {
   return *a;
 }
 
+// CHECK: define void @_Z27testParamContainsStrongWeak18ContainsStrongWeak(%[[STRUCT_CONTAINSSTRONGWEAK]]* %[[A:.*]])
+// CHECK: call %[[STRUCT_CONTAINSSTRONGWEAK]]* @_ZN18ContainsStrongWeakD1Ev(%[[STRUCT_CONTAINSSTRONGWEAK]]* %[[A]])
+
+void testParamContainsStrongWeak(ContainsStrongWeak a) {
+}
+
+// CHECK: define void @_Z26testParamDerivedStrongWeak17DerivedStrongWeak(%[[STRUCT_DERIVEDSTRONGWEAK]]* %[[A:.*]])
+// CHECK: call %[[STRUCT_DERIVEDSTRONGWEAK]]* @_ZN17DerivedStrongWeakD1Ev(%[[STRUCT_DERIVEDSTRONGWEAK]]* %[[A]])
+
+void testParamDerivedStrongWeak(DerivedStrongWeak a) {
+}
+
 // CHECK: define void @_Z15testParamStrong6Strong(i64 %[[A_COERCE:.*]])
 // CHECK: %[[A:.*]] = alloca %[[STRUCT_STRONG]], align 8
 // CHECK: %[[COERCE_DIVE:.*]] = getelementptr inbounds %[[STRUCT_STRONG]], %[[STRUCT_STRONG]]* %[[A]], i32 0, i32 0