[lld-macho] Add support for non-lazy categories to ObjC category merger #91548
Conversation
This file contains hidden or 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
|
@llvm/pr-subscribers-lld @llvm/pr-subscribers-lld-macho Author: None (alx32) ChangesIn ObjC we can have categories that define a Full diff: https://github.com/llvm/llvm-project/pull/91548.diff 2 Files Affected:
diff --git a/lld/MachO/ObjC.cpp b/lld/MachO/ObjC.cpp
index 15b89a808b05e..96ec646095be8 100644
--- a/lld/MachO/ObjC.cpp
+++ b/lld/MachO/ObjC.cpp
@@ -428,6 +428,7 @@ class ObjcCategoryMerger {
static void doCleanup();
private:
+ DenseSet<const Symbol *> collectNlCategories();
void collectAndValidateCategoriesData();
void
mergeCategoriesIntoSingleCategory(std::vector<InfoInputCategory> &categories);
@@ -1060,7 +1061,27 @@ void ObjcCategoryMerger::createSymbolReference(Defined *refFrom,
refFrom->isec()->relocs.push_back(r);
}
+// Get the list of categories in the '__objc_nlcatlist' section. We can't
+// optimize these as they have a '+load' method that has to be called at
+// runtime.
+DenseSet<const Symbol *> ObjcCategoryMerger::collectNlCategories() {
+ DenseSet<const Symbol *> nlCategories;
+
+ for (InputSection *sec : allInputSections) {
+ if (sec->getName() != section_names::objcNonLazyCatList)
+ continue;
+
+ for (auto &r : sec->relocs) {
+ const Symbol *sym = r.referent.dyn_cast<Symbol *>();
+ nlCategories.insert(sym);
+ }
+ }
+ return nlCategories;
+}
+
void ObjcCategoryMerger::collectAndValidateCategoriesData() {
+ auto nlCategories = collectNlCategories();
+
for (InputSection *sec : allInputSections) {
if (sec->getName() != section_names::objcCatList)
continue;
@@ -1074,6 +1095,9 @@ void ObjcCategoryMerger::collectAndValidateCategoriesData() {
assert(categorySym &&
"Failed to get a valid category at __objc_catlit offset");
+ if (nlCategories.count(categorySym))
+ continue;
+
// We only support ObjC categories (no swift + @objc)
// TODO: Support swift + @objc categories also
if (!categorySym->getName().starts_with(objc::symbol_names::category))
diff --git a/lld/test/MachO/objc-category-merging-complete-test.s b/lld/test/MachO/objc-category-merging-complete-test.s
index d2d264a3f26c2..74400177b550d 100644
--- a/lld/test/MachO/objc-category-merging-complete-test.s
+++ b/lld/test/MachO/objc-category-merging-complete-test.s
@@ -88,6 +88,7 @@ MERGE_CATS-NEXT: name {{.*}} MyProtocol02Prop
MERGE_CATS-NEXT: attributes {{.*}} Ti,R,D
MERGE_CATS-NEXT: name {{.*}} MyProtocol03Prop
MERGE_CATS-NEXT: attributes {{.*}} Ti,R,D
+MERGE_CATS: __OBJC_$_CATEGORY_MyBaseClass_$_Category04
NO_MERGE_CATS-NOT: __OBJC_$_CATEGORY_MyBaseClass(Category02|Category03)
@@ -431,6 +432,15 @@ L_OBJC_IMAGE_INFO:
## @dynamic MyProtocol03Prop;
## @end
##
+## // This category shouldn't be merged
+## @interface MyBaseClass(Category04)
+## + (void)load;
+## @end
+##
+## @implementation MyBaseClass(Category04)
+## + (void)load {}
+## @end
+##
## int main() {
## return 0;
## }
@@ -493,6 +503,12 @@ L_OBJC_IMAGE_INFO:
b _OUTLINED_FUNCTION_0
.cfi_endproc
; -- End function
+ .p2align 2
+"+[MyBaseClass(Category04) load]":
+ .cfi_startproc
+; %bb.0:
+ ret
+ .cfi_endproc
.globl _main ; -- Begin function main
.p2align 2
_main: ; @main
@@ -746,11 +762,42 @@ __OBJC_$_CATEGORY_MyBaseClass_$_Category03:
.quad 0
.long 64 ; 0x40
.space 4
+ .section __TEXT,__objc_classname,cstring_literals
+l_OBJC_CLASS_NAME_.15:
+ .asciz "Category04"
+ .section __TEXT,__objc_methname,cstring_literals
+l_OBJC_METH_VAR_NAME_.16:
+ .asciz "load"
+ .section __DATA,__objc_const
+ .p2align 3, 0x0
+__OBJC_$_CATEGORY_CLASS_METHODS_MyBaseClass_$_Category04:
+ .long 24
+ .long 1
+ .quad l_OBJC_METH_VAR_NAME_.16
+ .quad l_OBJC_METH_VAR_TYPE_
+ .quad "+[MyBaseClass(Category04) load]"
+ .p2align 3, 0x0
+__OBJC_$_CATEGORY_MyBaseClass_$_Category04:
+ .quad l_OBJC_CLASS_NAME_.15
+ .quad _OBJC_CLASS_$_MyBaseClass
+ .quad 0
+ .quad __OBJC_$_CATEGORY_CLASS_METHODS_MyBaseClass_$_Category04
+ .quad 0
+ .quad 0
+ .quad 0
+ .long 64
+ .space 4
.section __DATA,__objc_catlist,regular,no_dead_strip
.p2align 3, 0x0 ; @"OBJC_LABEL_CATEGORY_$"
l_OBJC_LABEL_CATEGORY_$:
.quad __OBJC_$_CATEGORY_MyBaseClass_$_Category02
.quad __OBJC_$_CATEGORY_MyBaseClass_$_Category03
+ .quad __OBJC_$_CATEGORY_MyBaseClass_$_Category04
+ .section __DATA,__objc_nlcatlist,regular,no_dead_strip
+ .p2align 3, 0x0
+l_OBJC_LABEL_NONLAZY_CATEGORY_$:
+ .quad __OBJC_$_CATEGORY_MyBaseClass_$_Category04
+
.no_dead_strip __OBJC_LABEL_PROTOCOL_$_MyProtocol02
.no_dead_strip __OBJC_LABEL_PROTOCOL_$_MyProtocol03
.no_dead_strip __OBJC_PROTOCOL_$_MyProtocol02
|
|
This might have broken a buildbot: https://lab.llvm.org/buildbot/#/builders/5/builds/43261/steps/9/logs/stdio |
Thanks for pointing this out, I'll forward fix ASAP or revert later today. |
|
Fixed via #91680 |
alx32
added a commit
that referenced
this pull request
May 10, 2024
FIxing the address sanitizer issue reported in #91548 . The problem comes from the assignment `auto bodyData = newSectionData` which defaults to `SmallVector<uint8_t> data = newSectionData` - which actually creates a copy of the data, placed on the stack. By explicitly using `ArrayRef` instead, we make sure that the original copy is used. We also change the assignment in `ObjcCategoryMerger::newStringData` from `auto` to `SmallVector<uint8_t> &` to make it explicit.
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.
In ObjC we can have categories that define a
+loadmethod that is called when the category is loaded. In such cases, we shouldn't optimize the category. These categories are present in the__objc_nlcatlistsection. So we scan these section for such categories and ignore them from optimization.