[CodeGen][ObjC] Invalidate cached ObjC class layout information after parsing ObjC class implementations if new ivars are added to the interface

clang/include/clang/AST/ASTContext.h

@@ -287,8 +287,8 @@ class ASTContext : public RefCountedBase<ASTContext> {
   /// This is lazily created.  This is intentionally not serialized.
   mutable llvm::DenseMap<const RecordDecl*, const ASTRecordLayout*>
     ASTRecordLayouts;
-  mutable llvm::DenseMap<const ObjCContainerDecl*, const ASTRecordLayout*>
-    ObjCLayouts;
+  mutable llvm::DenseMap<const ObjCInterfaceDecl *, const ASTRecordLayout *>
+      ObjCLayouts;
 
   /// A cache from types to size and alignment information.
   using TypeInfoMap = llvm::DenseMap<const Type *, struct TypeInfo>;
@@ -500,6 +500,11 @@ class ASTContext : public RefCountedBase<ASTContext> {
   static constexpr unsigned GeneralTypesLog2InitSize = 9;
   static constexpr unsigned FunctionProtoTypesLog2InitSize = 12;
 
+  /// A mapping from an ObjC class to its subclasses.
+  llvm::DenseMap<const ObjCInterfaceDecl *,
+                 SmallVector<const ObjCInterfaceDecl *, 4>>
+      ObjCSubClasses;
+
   ASTContext &this_() { return *this; }
 
 public:
@@ -2671,13 +2676,6 @@ class ASTContext : public RefCountedBase<ASTContext> {
   void DumpRecordLayout(const RecordDecl *RD, raw_ostream &OS,
                         bool Simple = false) const;
 
-  /// Get or compute information about the layout of the specified
-  /// Objective-C implementation.
-  ///
-  /// This may differ from the interface if synthesized ivars are present.
-  const ASTRecordLayout &
-  getASTObjCImplementationLayout(const ObjCImplementationDecl *D) const;
-
   /// Get our current best idea for the key function of the
   /// given record decl, or nullptr if there isn't one.
   ///
@@ -2716,7 +2714,6 @@ class ASTContext : public RefCountedBase<ASTContext> {
 
   /// Get the offset of an ObjCIvarDecl in bits.
   uint64_t lookupFieldBitOffset(const ObjCInterfaceDecl *OID,
-                                const ObjCImplementationDecl *ID,
                                 const ObjCIvarDecl *Ivar) const;
 
   /// Find the 'this' offset for the member path in a pointer-to-member
@@ -3174,7 +3171,12 @@ class ASTContext : public RefCountedBase<ASTContext> {
       bool &CanUseFirst, bool &CanUseSecond,
       SmallVectorImpl<FunctionProtoType::ExtParameterInfo> &NewParamInfos);
 
-  void ResetObjCLayout(const ObjCContainerDecl *CD);
+  void ResetObjCLayout(const ObjCInterfaceDecl *D);
+
+  void addObjCSubClass(const ObjCInterfaceDecl *D,
+                       const ObjCInterfaceDecl *SubClass) {
+    ObjCSubClasses[D].push_back(SubClass);
+  }
 
   //===--------------------------------------------------------------------===//
   //                    Integer Predicates
@@ -3564,9 +3566,7 @@ OPT_LIST(V)
   friend class DeclarationNameTable;
   friend class DeclContext;
 
-  const ASTRecordLayout &
-  getObjCLayout(const ObjCInterfaceDecl *D,
-                const ObjCImplementationDecl *Impl) const;
+  const ASTRecordLayout &getObjCLayout(const ObjCInterfaceDecl *D) const;
 
   /// A set of deallocations that should be performed when the
   /// ASTContext is destroyed.

clang/lib/AST/ASTContext.cpp

@@ -948,9 +948,11 @@ void ASTContext::cleanup() {
 
   // ASTRecordLayout objects in ASTRecordLayouts must always be destroyed
   // because they can contain DenseMaps.
-  for (llvm::DenseMap<const ObjCContainerDecl*,
-       const ASTRecordLayout*>::iterator
-       I = ObjCLayouts.begin(), E = ObjCLayouts.end(); I != E; )
+  for (llvm::DenseMap<const ObjCInterfaceDecl *,
+                      const ASTRecordLayout *>::iterator
+           I = ObjCLayouts.begin(),
+           E = ObjCLayouts.end();
+       I != E;)
     // Increment in loop to prevent using deallocated memory.
     if (auto *R = const_cast<ASTRecordLayout *>((I++)->second))
       R->Destroy(*this);
@@ -3092,13 +3094,7 @@ TypeSourceInfo *ASTContext::getTrivialTypeSourceInfo(QualType T,
 
 const ASTRecordLayout &
 ASTContext::getASTObjCInterfaceLayout(const ObjCInterfaceDecl *D) const {
-  return getObjCLayout(D, nullptr);
-}
-
-const ASTRecordLayout &
-ASTContext::getASTObjCImplementationLayout(
-                                        const ObjCImplementationDecl *D) const {
-  return getObjCLayout(D->getClassInterface(), D);
+  return getObjCLayout(D);
 }
 
 static auto getCanonicalTemplateArguments(const ASTContext &C,
@@ -8916,8 +8912,7 @@ static void EncodeBitField(const ASTContext *Ctx, std::string& S,
     uint64_t Offset;
 
     if (const auto *IVD = dyn_cast<ObjCIvarDecl>(FD)) {
-      Offset = Ctx->lookupFieldBitOffset(IVD->getContainingInterface(), nullptr,
-                                         IVD);
+      Offset = Ctx->lookupFieldBitOffset(IVD->getContainingInterface(), IVD);
     } else {
       const RecordDecl *RD = FD->getParent();
       const ASTRecordLayout &RL = Ctx->getASTRecordLayout(RD);
@@ -11848,8 +11843,12 @@ bool ASTContext::mergeExtParameterInfo(
   return true;
 }
 
-void ASTContext::ResetObjCLayout(const ObjCContainerDecl *CD) {
-  ObjCLayouts[CD] = nullptr;
+void ASTContext::ResetObjCLayout(const ObjCInterfaceDecl *D) {
+  if (ObjCLayouts.count(D)) {
+    ObjCLayouts[D] = nullptr;
+    for (auto *SubClass : ObjCSubClasses[D])
+      ResetObjCLayout(SubClass);
+  }
 }
 
 /// mergeObjCGCQualifiers - This routine merges ObjC's GC attribute of 'LHS' and

clang/lib/AST/RecordLayoutBuilder.cpp

@@ -3481,22 +3481,10 @@ uint64_t ASTContext::getFieldOffset(const ValueDecl *VD) const {
 }
 
 uint64_t ASTContext::lookupFieldBitOffset(const ObjCInterfaceDecl *OID,
-                                          const ObjCImplementationDecl *ID,
                                           const ObjCIvarDecl *Ivar) const {
   Ivar = Ivar->getCanonicalDecl();
   const ObjCInterfaceDecl *Container = Ivar->getContainingInterface();
-
-  // FIXME: We should eliminate the need to have ObjCImplementationDecl passed
-  // in here; it should never be necessary because that should be the lexical
-  // decl context for the ivar.
-
-  // If we know have an implementation (and the ivar is in it) then
-  // look up in the implementation layout.
-  const ASTRecordLayout *RL;
-  if (ID && declaresSameEntity(ID->getClassInterface(), Container))
-    RL = &getASTObjCImplementationLayout(ID);
-  else
-    RL = &getASTObjCInterfaceLayout(Container);
+  const ASTRecordLayout *RL = &getASTObjCInterfaceLayout(Container);
 
   // Compute field index.
   //
@@ -3522,8 +3510,7 @@ uint64_t ASTContext::lookupFieldBitOffset(const ObjCInterfaceDecl *OID,
 /// \param Impl - If given, also include the layout of the interface's
 /// implementation. This may differ by including synthesized ivars.
 const ASTRecordLayout &
-ASTContext::getObjCLayout(const ObjCInterfaceDecl *D,
-                          const ObjCImplementationDecl *Impl) const {
+ASTContext::getObjCLayout(const ObjCInterfaceDecl *D) const {
   // Retrieve the definition
   if (D->hasExternalLexicalStorage() && !D->getDefinition())
     getExternalSource()->CompleteType(const_cast<ObjCInterfaceDecl*>(D));
@@ -3532,22 +3519,9 @@ ASTContext::getObjCLayout(const ObjCInterfaceDecl *D,
          "Invalid interface decl!");
 
   // Look up this layout, if already laid out, return what we have.
-  const ObjCContainerDecl *Key =
-    Impl ? (const ObjCContainerDecl*) Impl : (const ObjCContainerDecl*) D;
-  if (const ASTRecordLayout *Entry = ObjCLayouts[Key])
+  if (const ASTRecordLayout *Entry = ObjCLayouts[D])
     return *Entry;
 
-  // Add in synthesized ivar count if laying out an implementation.
-  if (Impl) {
-    unsigned SynthCount = CountNonClassIvars(D);
-    // If there aren't any synthesized ivars then reuse the interface
-    // entry. Note we can't cache this because we simply free all
-    // entries later; however we shouldn't look up implementations
-    // frequently.
-    if (SynthCount == 0)
-      return getObjCLayout(D, nullptr);
-  }
-
   ItaniumRecordLayoutBuilder Builder(*this, /*EmptySubobjects=*/nullptr);
   Builder.Layout(D);
 
@@ -3557,7 +3531,7 @@ ASTContext::getObjCLayout(const ObjCInterfaceDecl *D,
       /*RequiredAlignment : used by MS-ABI)*/
       Builder.Alignment, Builder.getDataSize(), Builder.FieldOffsets);
 
-  ObjCLayouts[Key] = NewEntry;
+  ObjCLayouts[D] = NewEntry;
 
   return *NewEntry;
 }

clang/lib/CodeGen/CGObjCGNU.cpp

@@ -1826,9 +1826,11 @@ class CGObjCGNUstep2 : public CGObjCGNUstep {
       Context.getASTObjCInterfaceLayout(SuperClassDecl).getSize().getQuantity();
     // Instance size is negative for classes that have not yet had their ivar
     // layout calculated.
-    classFields.addInt(LongTy,
-      0 - (Context.getASTObjCImplementationLayout(OID).getSize().getQuantity() -
-      superInstanceSize));
+    classFields.addInt(
+        LongTy, 0 - (Context.getASTObjCInterfaceLayout(OID->getClassInterface())
+                         .getSize()
+                         .getQuantity() -
+                     superInstanceSize));
 
     if (classDecl->all_declared_ivar_begin() == nullptr)
       classFields.addNullPointer(PtrTy);
@@ -3639,8 +3641,9 @@ void CGObjCGNU::GenerateClass(const ObjCImplementationDecl *OID) {
   }
 
   // Get the size of instances.
-  int instanceSize =
-    Context.getASTObjCImplementationLayout(OID).getSize().getQuantity();
+  int instanceSize = Context.getASTObjCInterfaceLayout(OID->getClassInterface())
+                         .getSize()
+                         .getQuantity();
 
   // Collect information about instance variables.
   SmallVector<llvm::Constant*, 16> IvarNames;

clang/lib/CodeGen/CGObjCMac.cpp

@@ -3439,8 +3439,9 @@ void CGObjCMac::GenerateClass(const ObjCImplementationDecl *ID) {
   else if ((hasMRCWeak = hasMRCWeakIvars(CGM, ID)))
     Flags |= FragileABI_Class_HasMRCWeakIvars;
 
-  CharUnits Size =
-      CGM.getContext().getASTObjCImplementationLayout(ID).getSize();
+  CharUnits Size = CGM.getContext()
+                       .getASTObjCInterfaceLayout(ID->getClassInterface())
+                       .getSize();
 
   // FIXME: Set CXX-structors flag.
   if (ID->getClassInterface()->getVisibility() == HiddenVisibility)
@@ -6330,7 +6331,7 @@ void CGObjCNonFragileABIMac::GetClassSizeInfo(const ObjCImplementationDecl *OID,
                                               uint32_t &InstanceStart,
                                               uint32_t &InstanceSize) {
   const ASTRecordLayout &RL =
-      CGM.getContext().getASTObjCImplementationLayout(OID);
+      CGM.getContext().getASTObjCInterfaceLayout(OID->getClassInterface());
 
   // InstanceSize is really instance end.
   InstanceSize = RL.getDataSize().getQuantity();

clang/lib/CodeGen/CGObjCRuntime.cpp

@@ -31,24 +31,22 @@ using namespace CodeGen;
 uint64_t CGObjCRuntime::ComputeIvarBaseOffset(CodeGen::CodeGenModule &CGM,
                                               const ObjCInterfaceDecl *OID,
                                               const ObjCIvarDecl *Ivar) {
-  return CGM.getContext().lookupFieldBitOffset(OID, nullptr, Ivar) /
+  return CGM.getContext().lookupFieldBitOffset(OID, Ivar) /
          CGM.getContext().getCharWidth();
 }
 
 uint64_t CGObjCRuntime::ComputeIvarBaseOffset(CodeGen::CodeGenModule &CGM,
                                               const ObjCImplementationDecl *OID,
                                               const ObjCIvarDecl *Ivar) {
-  return CGM.getContext().lookupFieldBitOffset(OID->getClassInterface(), OID,
-                                               Ivar) /
+  return CGM.getContext().lookupFieldBitOffset(OID->getClassInterface(), Ivar) /
          CGM.getContext().getCharWidth();
 }
 
 unsigned CGObjCRuntime::ComputeBitfieldBitOffset(
     CodeGen::CodeGenModule &CGM,
     const ObjCInterfaceDecl *ID,
     const ObjCIvarDecl *Ivar) {
-  return CGM.getContext().lookupFieldBitOffset(ID, ID->getImplementation(),
-                                               Ivar);
+  return CGM.getContext().lookupFieldBitOffset(ID, Ivar);
 }
 
 LValue CGObjCRuntime::EmitValueForIvarAtOffset(CodeGen::CodeGenFunction &CGF,
@@ -86,7 +84,7 @@ LValue CGObjCRuntime::EmitValueForIvarAtOffset(CodeGen::CodeGenFunction &CGF,
   // non-synthesized ivars but we may be called for synthesized ivars.  However,
   // a synthesized ivar can never be a bit-field, so this is safe.
   uint64_t FieldBitOffset =
-      CGF.CGM.getContext().lookupFieldBitOffset(OID, nullptr, Ivar);
+      CGF.CGM.getContext().lookupFieldBitOffset(OID, Ivar);
   uint64_t BitOffset = FieldBitOffset % CGF.CGM.getContext().getCharWidth();
   uint64_t AlignmentBits = CGF.CGM.getTarget().getCharAlign();
   uint64_t BitFieldSize = Ivar->getBitWidthValue();

clang/lib/Sema/SemaDeclObjC.cpp

@@ -659,6 +659,7 @@ void SemaObjC::ActOnSuperClassOfClassInterface(
 
     IDecl->setSuperClass(SuperClassTInfo);
     IDecl->setEndOfDefinitionLoc(SuperClassTInfo->getTypeLoc().getEndLoc());
+    getASTContext().addObjCSubClass(IDecl->getSuperClass(), IDecl);
   }
 }
 
@@ -2129,6 +2130,12 @@ SemaObjC::ActOnFinishObjCImplementation(Decl *ObjCImpDecl,
 
   DeclsInGroup.push_back(ObjCImpDecl);
 
+  // Reset the cached layout if there are any ivars added to
+  // the implementation.
+  if (auto *ImplD = dyn_cast<ObjCImplementationDecl>(ObjCImpDecl))
+    if (!ImplD->ivar_empty())
+      getASTContext().ResetObjCLayout(ImplD->getClassInterface());
+
   return SemaRef.BuildDeclaratorGroup(DeclsInGroup);
 }
 

clang/test/CodeGenObjC/constant-non-fragile-ivar-offset.m

@@ -9,6 +9,9 @@
 // CHECK: @"OBJC_IVAR_$_SubClass.subClassIvar" = constant i64 56
 // CHECK: @"OBJC_IVAR_$_SubClass._subClassProperty" = hidden constant i64 64
 // CHECK: @"OBJC_IVAR_$_NotStaticLayout.not_static_layout_ivar" = hidden global i64 12
+// CHECK: @"OBJC_IVAR_$_SuperClass2._superClassProperty2" = hidden constant i64 20
+// CHECK: @"OBJC_IVAR_$_IntermediateClass2._IntermediateClass2Property" = hidden constant i64 24
+// CHECK: @"OBJC_IVAR_$_SubClass2._subClass2Property" = hidden constant i64 28
 
 @interface NSObject {
   int these, will, never, change, ever;
@@ -138,3 +141,47 @@ -(void)meth {
   // CHECK: load i64, ptr @"OBJC_IVAR_$_NotStaticLayout.not_static_layout_ivar
 }
 @end
+
+// CHECK: define internal i32 @"\01-[IntermediateClass2 IntermediateClass2Property]"(ptr noundef %[[SELF:.*]],
+// CHECK: %[[SELF_ADDR:.*]] = alloca ptr, align 8
+// CHECK: store ptr %[[SELF]], ptr %[[SELF_ADDR]], align 8
+// CHECK: %[[V0:.*]] = load ptr, ptr %[[SELF_ADDR]], align 8
+// CHECK: %[[ADD_PTR:.*]] = getelementptr inbounds i8, ptr %[[V0]], i64 24
+// CHECK: %[[LOAD:.*]] = load atomic i32, ptr %[[ADD_PTR]] unordered, align 4
+// CHECK: ret i32 %[[LOAD]]
+
+// CHECK: define internal i32 @"\01-[SubClass2 subClass2Property]"(ptr noundef %[[SELF:.*]],
+// CHECK: %[[SELF_ADDR:.*]] = alloca ptr, align 8
+// CHECK: store ptr %[[SELF]], ptr %[[SELF_ADDR]], align 8
+// CHECK: %[[V0:.*]] = load ptr, ptr %[[SELF_ADDR]], align 8
+// CHECK: %[[ADD_PTR:.*]] = getelementptr inbounds i8, ptr %[[V0]], i64 28
+// CHECK: %[[LOAD:.*]] = load atomic i32, ptr %[[ADD_PTR]] unordered, align 4
+// CHECK: ret i32 %[[LOAD]]
+
+@interface SuperClass2 : NSObject
+@property int superClassProperty2;
+@end
+
+@interface IntermediateClass2 : SuperClass2
+@property int IntermediateClass2Property;
+@end
+
+@interface IntermediateClass3 : SuperClass2
+@property int IntermediateClass3Property;
+@end
+
+@interface SubClass2 : IntermediateClass2
+@property int subClass2Property;
+@end
+
+@implementation IntermediateClass3
+@end
+
+@implementation SuperClass2
+@end
+
+@implementation IntermediateClass2
+@end
+
+@implementation SubClass2
+@end

clang/test/CodeGenObjC/ivar-layout-64.m

@@ -63,8 +63,8 @@ @interface D : A
 @end
 
 // CHECK: @OBJC_CLASS_NAME_{{.*}} = private unnamed_addr constant {{.*}} c"D\00"
-// CHECK: @OBJC_CLASS_NAME_{{.*}} = private unnamed_addr constant {{.*}} c"\11p\00"
-// CHECK: @OBJC_CLASS_NAME_{{.*}} = private unnamed_addr constant {{.*}} c"!`\00"
+// CHECK: @OBJC_CLASS_NAME_{{.*}} = private unnamed_addr constant {{.*}} c"\11\A0\00"
+// CHECK: @OBJC_CLASS_NAME_{{.*}} = private unnamed_addr constant {{.*}} c"!\90\00"
 
 @implementation D
 @synthesize p3 = _p3;