https://github.com/ahatanak updated
https://github.com/llvm/llvm-project/pull/126591
>From 5101ebcb921fb621f9abcd7371337732f2d307b5 Mon Sep 17 00:00:00 2001
From: Akira Hatanaka <ahata...@gmail.com>
Date: Wed, 5 Feb 2025 14:47:45 -0800
Subject: [PATCH] [CodeGen][ObjC] Invalidate cached ObjC class layout
information after parsing ObjC class implementations if new ivars are added
to the interface
The layout and the size of an ObjC interface can change after its
corresponding implementation is parsed when synthesized ivars or ivars
declared in categories are added to the interface's list of ivars. This
can cause clang to mis-compile if the optimization that emits fixed
offsets for ivars (see 923ddf65f4e21ec67018cf56e823895de18d83bc) uses
an ObjC class layout that is outdated and no longer reflects the current
state of the class.
For example, when compiling constant-non-fragile-ivar-offset.m, clang
emits 20 instead of 24 as the offset for IntermediateClass2Property as
the class layout for SuperClass2, which is created when the
implementation of IntermediateClass3 is parsed, is outdated when the
implementation of IntermediateClass2 is parsed.
This commit invalidates the stale layout information of the class and
its subclasses if new ivars are added to the interface.
With this change, we can also stop using ObjC implementation decls as
the key to retrieve ObjC class layouts information as the layout
retrieved using the ObjC interface as the key will always to up to date.
rdar://139531391
---
clang/include/clang/AST/ASTContext.h | 28 +++++------
clang/lib/AST/ASTContext.cpp | 27 +++++------
clang/lib/AST/RecordLayoutBuilder.cpp | 34 ++------------
clang/lib/CodeGen/CGObjCGNU.cpp | 13 +++--
clang/lib/CodeGen/CGObjCMac.cpp | 7 +--
clang/lib/CodeGen/CGObjCRuntime.cpp | 10 ++--
clang/lib/Sema/SemaDeclObjC.cpp | 7 +++
.../constant-non-fragile-ivar-offset.m | 47 +++++++++++++++++++
clang/test/CodeGenObjC/ivar-layout-64.m | 4 +-
9 files changed, 103 insertions(+), 74 deletions(-)
diff --git a/clang/include/clang/AST/ASTContext.h
b/clang/include/clang/AST/ASTContext.h
index 65be782c1ba43..7c8ab36af2b9d 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/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:
@@ -2630,13 +2635,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.
///
@@ -2675,7 +2673,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
@@ -3133,7 +3130,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
@@ -3523,9 +3525,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.
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index de617860b7004..436c03d8e5440 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/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,
@@ -8903,8 +8899,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);
@@ -11835,8 +11830,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
diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp
b/clang/lib/AST/RecordLayoutBuilder.cpp
index ae6d299024c6d..3e38ba0a43d98 100644
--- a/clang/lib/AST/RecordLayoutBuilder.cpp
+++ b/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;
}
diff --git a/clang/lib/CodeGen/CGObjCGNU.cpp b/clang/lib/CodeGen/CGObjCGNU.cpp
index ebd88bb38849e..d1876f47c0eea 100644
--- a/clang/lib/CodeGen/CGObjCGNU.cpp
+++ b/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;
diff --git a/clang/lib/CodeGen/CGObjCMac.cpp b/clang/lib/CodeGen/CGObjCMac.cpp
index 6c929a6431c0f..2e0508d2887e5 100644
--- a/clang/lib/CodeGen/CGObjCMac.cpp
+++ b/clang/lib/CodeGen/CGObjCMac.cpp
@@ -3522,8 +3522,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)
@@ -6414,7 +6415,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();
diff --git a/clang/lib/CodeGen/CGObjCRuntime.cpp
b/clang/lib/CodeGen/CGObjCRuntime.cpp
index a7f5c913f42fc..dfb0fd14d93ac 100644
--- a/clang/lib/CodeGen/CGObjCRuntime.cpp
+++ b/clang/lib/CodeGen/CGObjCRuntime.cpp
@@ -31,15 +31,14 @@ 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();
}
@@ -47,8 +46,7 @@ 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();
diff --git a/clang/lib/Sema/SemaDeclObjC.cpp b/clang/lib/Sema/SemaDeclObjC.cpp
index e665d0293dc84..ba9d3dcf19617 100644
--- a/clang/lib/Sema/SemaDeclObjC.cpp
+++ b/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);
}
diff --git a/clang/test/CodeGenObjC/constant-non-fragile-ivar-offset.m
b/clang/test/CodeGenObjC/constant-non-fragile-ivar-offset.m
index 8d55e6c7d2308..bc076b4656c9d 100644
--- a/clang/test/CodeGenObjC/constant-non-fragile-ivar-offset.m
+++ b/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
diff --git a/clang/test/CodeGenObjC/ivar-layout-64.m
b/clang/test/CodeGenObjC/ivar-layout-64.m
index d3ffdfe444c8b..409434ca3bef3 100644
--- a/clang/test/CodeGenObjC/ivar-layout-64.m
+++ b/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;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
