Skip to content
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

[lld-macho] Add support for category names in ConcatInputSection's #90850

Merged
merged 7 commits into from
May 6, 2024

Conversation

alx32
Copy link
Contributor

@alx32 alx32 commented May 2, 2024

In some cases we see strings from categories being part of "data" sections (Ex:__objc_const), not part of of sections marked as cstring_literals. Since lld treats these sections differently we need to explicitly implement support for reading strings from the non-cstring_literals sections.

Adding a test that previously would result in an assert.

@alx32 alx32 requested review from int3 and kyulee-com May 2, 2024 12:38
Copy link

github-actions bot commented May 2, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@alx32 alx32 force-pushed the 03_cat_name_concat_input_section branch from 137802e to c098eb0 Compare May 2, 2024 13:51
@alx32 alx32 marked this pull request as ready for review May 2, 2024 15:30
@llvmbot
Copy link
Collaborator

llvmbot commented May 2, 2024

@llvm/pr-subscribers-lld

Author: None (alx32)

Changes

In some cases we see strings from categories being part of "data" sections (Ex:__objc_const), not part of of sections marked as cstring_literals. Since lld treats these sections differently we need to explicitly implement support for reading strings from the non-cstring_literals sections.

Adding a test that previously would result in an assert.


Full diff: https://github.com/llvm/llvm-project/pull/90850.diff

2 Files Affected:

  • (modified) lld/MachO/ObjC.cpp (+14-3)
  • (modified) lld/test/MachO/objc-category-merging-extern-class-minimal.s (+1-1)
diff --git a/lld/MachO/ObjC.cpp b/lld/MachO/ObjC.cpp
index 95fe0c9374f150..68a8645532dff6 100644
--- a/lld/MachO/ObjC.cpp
+++ b/lld/MachO/ObjC.cpp
@@ -186,13 +186,24 @@ ObjcCategoryChecker::ObjcCategoryChecker()
       roClassLayout(target->wordSize), listHeaderLayout(target->wordSize),
       methodLayout(target->wordSize) {}
 
-// \p r must point to an offset within a cstring section.
+// \p r must point to an offset within a CStringInputSection or a
+// ConcatInputSection
 static StringRef getReferentString(const Reloc &r) {
   if (auto *isec = r.referent.dyn_cast<InputSection *>())
     return cast<CStringInputSection>(isec)->getStringRefAtOffset(r.addend);
   auto *sym = cast<Defined>(r.referent.get<Symbol *>());
-  return cast<CStringInputSection>(sym->isec())
-      ->getStringRefAtOffset(sym->value + r.addend);
+  if (isa<CStringInputSection>(sym->isec()))
+    return cast<CStringInputSection>(sym->isec())
+        ->getStringRefAtOffset(sym->value + r.addend);
+
+  if (isa<ConcatInputSection>(sym->isec())) {
+    auto &data = sym->isec()->data;
+    const char *pDataStart = reinterpret_cast<const char *>(data.data());
+    uint32_t len = strnlen(pDataStart + sym->value, data.size() - sym->value);
+    return StringRef(pDataStart + sym->value, len);
+  }
+
+  llvm_unreachable("unknown reference section in getReferentString");
 }
 
 void ObjcCategoryChecker::parseMethods(const ConcatInputSection *methodsIsec,
diff --git a/lld/test/MachO/objc-category-merging-extern-class-minimal.s b/lld/test/MachO/objc-category-merging-extern-class-minimal.s
index ede7ef5d9c32d4..3e220ef372ca0d 100644
--- a/lld/test/MachO/objc-category-merging-extern-class-minimal.s
+++ b/lld/test/MachO/objc-category-merging-extern-class-minimal.s
@@ -118,7 +118,7 @@ __OBJC_$_CATEGORY_MyBaseClass_$_Category01:
 	.quad	0
 	.long	64                              ; 0x40
 	.space	4
-	.section	__TEXT,__objc_classname,cstring_literals
+	.section	__DATA,__objc_const
 l_OBJC_CLASS_NAME_.1:                   ; @OBJC_CLASS_NAME_.1
 	.asciz	"Category02"
 	.section	__TEXT,__objc_methname,cstring_literals

@llvmbot
Copy link
Collaborator

llvmbot commented May 2, 2024

@llvm/pr-subscribers-lld-macho

Author: None (alx32)

Changes

In some cases we see strings from categories being part of "data" sections (Ex:__objc_const), not part of of sections marked as cstring_literals. Since lld treats these sections differently we need to explicitly implement support for reading strings from the non-cstring_literals sections.

Adding a test that previously would result in an assert.


Full diff: https://github.com/llvm/llvm-project/pull/90850.diff

2 Files Affected:

  • (modified) lld/MachO/ObjC.cpp (+14-3)
  • (modified) lld/test/MachO/objc-category-merging-extern-class-minimal.s (+1-1)
diff --git a/lld/MachO/ObjC.cpp b/lld/MachO/ObjC.cpp
index 95fe0c9374f150..68a8645532dff6 100644
--- a/lld/MachO/ObjC.cpp
+++ b/lld/MachO/ObjC.cpp
@@ -186,13 +186,24 @@ ObjcCategoryChecker::ObjcCategoryChecker()
       roClassLayout(target->wordSize), listHeaderLayout(target->wordSize),
       methodLayout(target->wordSize) {}
 
-// \p r must point to an offset within a cstring section.
+// \p r must point to an offset within a CStringInputSection or a
+// ConcatInputSection
 static StringRef getReferentString(const Reloc &r) {
   if (auto *isec = r.referent.dyn_cast<InputSection *>())
     return cast<CStringInputSection>(isec)->getStringRefAtOffset(r.addend);
   auto *sym = cast<Defined>(r.referent.get<Symbol *>());
-  return cast<CStringInputSection>(sym->isec())
-      ->getStringRefAtOffset(sym->value + r.addend);
+  if (isa<CStringInputSection>(sym->isec()))
+    return cast<CStringInputSection>(sym->isec())
+        ->getStringRefAtOffset(sym->value + r.addend);
+
+  if (isa<ConcatInputSection>(sym->isec())) {
+    auto &data = sym->isec()->data;
+    const char *pDataStart = reinterpret_cast<const char *>(data.data());
+    uint32_t len = strnlen(pDataStart + sym->value, data.size() - sym->value);
+    return StringRef(pDataStart + sym->value, len);
+  }
+
+  llvm_unreachable("unknown reference section in getReferentString");
 }
 
 void ObjcCategoryChecker::parseMethods(const ConcatInputSection *methodsIsec,
diff --git a/lld/test/MachO/objc-category-merging-extern-class-minimal.s b/lld/test/MachO/objc-category-merging-extern-class-minimal.s
index ede7ef5d9c32d4..3e220ef372ca0d 100644
--- a/lld/test/MachO/objc-category-merging-extern-class-minimal.s
+++ b/lld/test/MachO/objc-category-merging-extern-class-minimal.s
@@ -118,7 +118,7 @@ __OBJC_$_CATEGORY_MyBaseClass_$_Category01:
 	.quad	0
 	.long	64                              ; 0x40
 	.space	4
-	.section	__TEXT,__objc_classname,cstring_literals
+	.section	__DATA,__objc_const
 l_OBJC_CLASS_NAME_.1:                   ; @OBJC_CLASS_NAME_.1
 	.asciz	"Category02"
 	.section	__TEXT,__objc_methname,cstring_literals

@alx32 alx32 requested a review from thevinster May 2, 2024 16:10
lld/MachO/ObjC.cpp Outdated Show resolved Hide resolved
lld/MachO/ObjC.cpp Outdated Show resolved Hide resolved
@alx32 alx32 force-pushed the 03_cat_name_concat_input_section branch from 81f8dcc to c101bc0 Compare May 3, 2024 00:53
lld/MachO/ObjC.cpp Outdated Show resolved Hide resolved
lld/MachO/ObjC.cpp Outdated Show resolved Hide resolved
lld/MachO/ObjC.cpp Outdated Show resolved Hide resolved
lld/MachO/ObjC.cpp Outdated Show resolved Hide resolved
@alx32 alx32 merged commit 9fc0b18 into llvm:main May 6, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants