-
Notifications
You must be signed in to change notification settings - Fork 10.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
release/18.x: [ObjC] Check entire chain of superclasses to determine class layout #84093
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
Clang issues not falling into any other category
clang:codegen
labels
Mar 5, 2024
@llvm/pr-subscribers-clang-codegen @llvm/pr-subscribers-clang Author: AtariDreams (AtariDreams) ChangesBackport of 923ddf6 Full diff: https://github.com/llvm/llvm-project/pull/84093.diff 2 Files Affected:
diff --git a/clang/lib/CodeGen/CGObjCMac.cpp b/clang/lib/CodeGen/CGObjCMac.cpp
index 517f7cddebc1a2..be628a909d89be 100644
--- a/clang/lib/CodeGen/CGObjCMac.cpp
+++ b/clang/lib/CodeGen/CGObjCMac.cpp
@@ -1593,12 +1593,20 @@ class CGObjCNonFragileABIMac : public CGObjCCommonMac {
}
bool isClassLayoutKnownStatically(const ObjCInterfaceDecl *ID) {
- // NSObject is a fixed size. If we can see the @implementation of a class
- // which inherits from NSObject then we know that all it's offsets also must
- // be fixed. FIXME: Can we do this if see a chain of super classes with
- // implementations leading to NSObject?
- return ID->getImplementation() && ID->getSuperClass() &&
- ID->getSuperClass()->getName() == "NSObject";
+ // Test a class by checking its superclasses up to
+ // its base class if it has one.
+ for (; ID; ID = ID->getSuperClass()) {
+ // The layout of base class NSObject
+ // is guaranteed to be statically known
+ if (ID->getIdentifier()->getName() == "NSObject")
+ return true;
+
+ // If we cannot see the @implementation of a class,
+ // we cannot statically know the class layout.
+ if (!ID->getImplementation())
+ return false;
+ }
+ return false;
}
public:
diff --git a/clang/test/CodeGenObjC/constant-non-fragile-ivar-offset.m b/clang/test/CodeGenObjC/constant-non-fragile-ivar-offset.m
index 788b3220af3067..8d55e6c7d23081 100644
--- a/clang/test/CodeGenObjC/constant-non-fragile-ivar-offset.m
+++ b/clang/test/CodeGenObjC/constant-non-fragile-ivar-offset.m
@@ -1,6 +1,13 @@
// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 -emit-llvm %s -o - | FileCheck %s
// CHECK: @"OBJC_IVAR_$_StaticLayout.static_layout_ivar" = hidden constant i64 20
+// CHECK: @"OBJC_IVAR_$_SuperClass.superClassIvar" = hidden constant i64 20
+// CHECK: @"OBJC_IVAR_$_SuperClass._superClassProperty" = hidden constant i64 24
+// CHECK: @"OBJC_IVAR_$_IntermediateClass.intermediateClassIvar" = constant i64 32
+// CHECK: @"OBJC_IVAR_$_IntermediateClass.intermediateClassIvar2" = constant i64 40
+// CHECK: @"OBJC_IVAR_$_IntermediateClass._intermediateProperty" = hidden constant i64 48
+// 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
@interface NSObject {
@@ -14,12 +21,105 @@ @interface StaticLayout : NSObject
@implementation StaticLayout {
int static_layout_ivar;
}
+
+// CHECK-LABEL: define internal void @"\01-[StaticLayout meth]"
-(void)meth {
static_layout_ivar = 0;
// CHECK-NOT: load i64, ptr @"OBJC_IVAR_$_StaticLayout
+ // CHECK: getelementptr inbounds i8, ptr %0, i64 20
+}
+@end
+
+@interface SuperClass : NSObject
+@property (nonatomic, assign) int superClassProperty;
+@end
+
+@implementation SuperClass {
+ int superClassIvar; // Declare an ivar
+}
+
+// CHECK-LABEL: define internal void @"\01-[SuperClass superClassMethod]"
+- (void)superClassMethod {
+ _superClassProperty = 42;
+ superClassIvar = 10;
+ // CHECK-NOT: load i64, ptr @"OBJC_IVAR_$_SuperClass
+ // CHECK: getelementptr inbounds i8, ptr %1, i64 20
+}
+
+// Implicitly synthesized method here
+// CHECK-LABEL: define internal i32 @"\01-[SuperClass superClassProperty]"
+// CHECK: getelementptr inbounds i8, ptr %0, i64 24
+
+// CHECK-LABEL: define internal void @"\01-[SuperClass setSuperClassProperty:]"
+// CHECK: getelementptr inbounds i8, ptr %1, i64 24
+@end
+
+@interface IntermediateClass : SuperClass {
+ double intermediateClassIvar;
+
+ @protected
+ int intermediateClassIvar2;
+}
+@property (nonatomic, strong) SuperClass *intermediateProperty;
+@end
+
+@implementation IntermediateClass
+@synthesize intermediateProperty = _intermediateProperty;
+
+// CHECK-LABEL: define internal void @"\01-[IntermediateClass intermediateClassMethod]"
+- (void)intermediateClassMethod {
+ intermediateClassIvar = 3.14;
+ // CHECK-NOT: load i64, ptr @"OBJC_IVAR_$_IntermediateClass
+ // CHECK: getelementptr inbounds i8, ptr %0, i64 32
+}
+
+// CHECK-LABEL: define internal void @"\01-[IntermediateClass intermediateClassPropertyMethod]"
+- (void)intermediateClassPropertyMethod {
+ self.intermediateProperty = 0;
+ // CHECK: load ptr, ptr @OBJC_SELECTOR_REFERENCES_
+ // CHECK: call void @objc_msgSend(ptr noundef %0, ptr noundef %1, ptr noundef null)
+}
+
+// CHECK-LABEL: define internal void @"\01-[IntermediateClass intermediateClassPropertyMethodDirect]"
+- (void)intermediateClassPropertyMethodDirect {
+ _intermediateProperty = 0;
+ // CHECK-NOT: load i64, ptr @"OBJC_IVAR_$_IntermediateClass._intermediateProperty"
+ // CHECK: getelementptr inbounds i8, ptr %0, i64 48
}
@end
+@interface SubClass : IntermediateClass {
+ double subClassIvar;
+}
+@property (nonatomic, assign) SubClass *subClassProperty;
+@end
+
+@implementation SubClass
+
+// CHECK-LABEL: define internal void @"\01-[SubClass subclassVar]"
+- (void)subclassVar {
+ subClassIvar = 6.28;
+ // CHECK-NOT: load i64, ptr @"OBJC_IVAR_$_SubClass
+ // CHECK: getelementptr inbounds i8, ptr %0, i64 56
+}
+
+// CHECK-LABEL: define internal void @"\01-[SubClass intermediateSubclassVar]"
+-(void)intermediateSubclassVar {
+ intermediateClassIvar = 3.14;
+ // CHECK-NOT: load i64, ptr @"OBJC_IVAR_$_IntermediateClass
+ // CHECK: getelementptr inbounds i8, ptr %0, i64 32
+}
+
+// Implicit synthesized method here:
+// CHECK-LABEL: define internal ptr @"\01-[SubClass subClassProperty]"
+// CHECK-NOT: load i64, ptr @"OBJC_IVAR_$_SubClass._subClassProperty"
+// CHECK: getelementptr inbounds i8, ptr %0, i64 64
+
+// CHECK-LABEL: define internal void @"\01-[SubClass setSubClassProperty:]"
+// CHECK-NOT: load i64, ptr @"OBJC_IVAR_$_SubClass._subClassProperty"
+// CHECK: getelementptr inbounds i8, ptr %1, i64 64
+@end
+
@interface NotNSObject {
int these, might, change;
}
@@ -31,6 +131,8 @@ @interface NotStaticLayout : NotNSObject
@implementation NotStaticLayout {
int not_static_layout_ivar;
}
+
+// CHECK-LABEL: define internal void @"\01-[NotStaticLayout meth]"
-(void)meth {
not_static_layout_ivar = 0;
// CHECK: load i64, ptr @"OBJC_IVAR_$_NotStaticLayout.not_static_layout_ivar
|
…tatically known As of now, we only check if a class directly inherits from NSObject to determine if said class has fixed offsets and can therefore have its memory layout statically known. However, if an NSObject subclass has fixed offsets, then so must the subclasses of that subclass, so this allows us to optimize instances of subclasses of subclasses that inherit from NSObject and so on. To determine this, we need to find that the compiler can see the implementation of each intermediate class, as that means it is statically linked.
efriedma-quic
requested changes
Mar 11, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We usually only take bugfixes on release branches (miscompiles/crashes/etc.).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backport of 923ddf6