Skip to content

[lld] Fix -ObjC load behavior with LTO #92162

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

Merged
merged 1 commit into from
May 15, 2024
Merged

Conversation

NuriAmari
Copy link
Contributor

When -ObjC is passed, the linker must force load any object files that contain special sections that store Objective-C / Swift information that is used at runtime.

This should work regadless if input files are bitcode or native, but it was not working with bitcode. This is because the sections that identify an object file that should be loaded were inconsistent when dealing with a native file vs bitcode file. In particular, bitcode files were not searched for __TEXT,__swift prefixed sections, while native files were.

This means LLD wasn't loading certain bitcode files and forcing the user to introduce --force-load to their linker invocation for that archive.

When -ObjC is passed, the linker must force load any object files that
contain special sections that store Objective-C / Swift information that
is used at runtime.

This should work regadless if input files are bitcode or native, but
it was not working with bitcode. This is because the sections that
identify an object file that should be loaded were inconsistent when
dealing with a native file vs bitcode file. In particular, bitcode files
were not searched for `__TEXT,__swift` prefixed sections, while native
files were.

This means LLD wasn't loading certain bitcode files and forcing the
user to introduce --force-load to their linker invocation for that
archive.
@NuriAmari NuriAmari force-pushed the users/nuriamari/fix-lto-ObjC branch from d90efc5 to 24da8f1 Compare May 14, 2024 18:58
@NuriAmari NuriAmari requested review from keith, int3 and kyulee-com May 14, 2024 18:59
Copy link
Contributor

@kyulee-com kyulee-com left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing this issue!
The related native code seems https://github.com/llvm/llvm-project/blob/main/lld/MachO/ObjC.cpp#L49.

@NuriAmari NuriAmari marked this pull request as ready for review May 14, 2024 22:00
@llvmbot
Copy link
Member

llvmbot commented May 14, 2024

@llvm/pr-subscribers-lld-macho

@llvm/pr-subscribers-lld

Author: Nuri Amari (NuriAmari)

Changes

When -ObjC is passed, the linker must force load any object files that contain special sections that store Objective-C / Swift information that is used at runtime.

This should work regadless if input files are bitcode or native, but it was not working with bitcode. This is because the sections that identify an object file that should be loaded were inconsistent when dealing with a native file vs bitcode file. In particular, bitcode files were not searched for __TEXT,__swift prefixed sections, while native files were.

This means LLD wasn't loading certain bitcode files and forcing the user to introduce --force-load to their linker invocation for that archive.


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

2 Files Affected:

  • (modified) lld/test/MachO/objc.s (+20-3)
  • (modified) llvm/lib/Bitcode/Reader/BitcodeReader.cpp (+2-1)
diff --git a/lld/test/MachO/objc.s b/lld/test/MachO/objc.s
index e7074141f0113..dbb9f1df27571 100644
--- a/lld/test/MachO/objc.s
+++ b/lld/test/MachO/objc.s
@@ -5,12 +5,14 @@
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/has-objc-category.s -o %t/has-objc-category.o
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/has-objc-symbol-and-category.s -o %t/has-objc-symbol-and-category.o
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/has-swift.s -o %t/has-swift.o
+# RUN: llvm-as %t/has-swift-ir-loaded.ll -o %t/has-swift-ir-loaded.o
+# RUN: llvm-as %t/has-swift-ir-not-loaded.ll -o %t/has-swift-ir-not-loaded.o
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/has-swift-proto.s -o %t/has-swift-proto.o
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/no-objc.s -o %t/no-objc.o
 ## Make sure we don't mis-parse a 32-bit file as 64-bit
 # RUN: llvm-mc -filetype=obj -triple=armv7-apple-watchos %t/no-objc.s -o %t/wrong-arch.o
-# RUN: llvm-ar rcs %t/libHasSomeObjC.a %t/no-objc.o %t/has-objc-symbol.o %t/has-objc-category.o %t/has-swift.o %t/has-swift-proto.o %t/wrong-arch.o
-# RUN: llvm-ar rcs %t/libHasSomeObjC2.a %t/no-objc.o %t/has-objc-symbol-and-category.o %t/has-swift.o %t/has-swift-proto.o %t/wrong-arch.o
+# RUN: llvm-ar rcs %t/libHasSomeObjC.a %t/no-objc.o %t/has-objc-symbol.o %t/has-objc-category.o %t/has-swift.o %t/has-swift-proto.o %t/has-swift-ir-loaded.o %t/has-swift-ir-not-loaded.o %t/wrong-arch.o
+# RUN: llvm-ar rcs %t/libHasSomeObjC2.a %t/no-objc.o %t/has-objc-symbol-and-category.o %t/has-swift.o %t/has-swift-proto.o %t/has-swift-ir-loaded.o %t/has-swift-ir-not-loaded.o %t/wrong-arch.o
 
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/test.s -o %t/test.o
 
@@ -20,7 +22,7 @@
 # RUN: %lld -lSystem %t/test.o -o %t/test -L%t -lHasSomeObjC2 -ObjC
 # RUN: llvm-objdump --section-headers --syms %t/test | FileCheck %s --check-prefix=OBJC
 
-# RUN: %no-fatal-warnings-lld -lSystem %t/test.o -o %t/test --start-lib %t/no-objc.o %t/has-objc-symbol.o %t/has-objc-category.o %t/has-swift.o %t/has-swift-proto.o %t/wrong-arch.o --end-lib -ObjC 2>&1 \
+# RUN: %no-fatal-warnings-lld -lSystem %t/test.o -o %t/test --start-lib %t/no-objc.o %t/has-objc-symbol.o %t/has-objc-category.o %t/has-swift.o %t/has-swift-proto.o %t/has-swift-ir-loaded.o %t/has-swift-ir-not-loaded.o %t/wrong-arch.o --end-lib -ObjC 2>&1 \
 # RUN:     | FileCheck -check-prefix=WARNING %s
 # RUN: llvm-objdump --section-headers --syms %t/test | FileCheck %s --check-prefix=OBJC
 
@@ -36,6 +38,7 @@
 # OBJC-NEXT:    4 has_objc_symbol {{.*}}      DATA
 # OBJC-EMPTY:
 # OBJC-NEXT:  SYMBOL TABLE:
+# OBJC-DAG:   g     O __TEXT,__swift _foo
 # OBJC-DAG:   g     F __TEXT,__text _main
 # OBJC-DAG:   g     F __TEXT,__text _OBJC_CLASS_$_MyObject
 # OBJC-DAG:   g     O __TEXT,__swift5_fieldmd $s7somelib4Blah_pMF
@@ -100,6 +103,20 @@ _has_dup:
 .section __TEXT,__swift
 .quad 0x1234
 
+#--- has-swift-ir-loaded.ll
+target datalayout = "e-m:o-i64:64-i128:128-n32:64-S128"
+target triple = "x86_64-apple-darwin"
+
+@foo = global i64 1234, section "__TEXT,__swift"
+@llvm.used = appending global [1 x ptr] [ptr @foo]
+
+#--- has-swift-ir-not-loaded.ll
+target datalayout = "e-m:o-i64:64-i128:128-n32:64-S128"
+target triple = "x86_64-apple-darwin"
+
+@bar = global i64 1234
+@llvm.used = appending global [1 x ptr] [ptr @bar]
+
 #--- has-swift-proto.s
 .section __TEXT,__swift5_fieldmd
 .globl $s7somelib4Blah_pMF
diff --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
index 19a15209f8b66..e64051cf53862 100644
--- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
+++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -306,7 +306,8 @@ static Expected<bool> hasObjCCategoryInModule(BitstreamCursor &Stream) {
         return error("Invalid section name record");
       // Check for the i386 and other (x86_64, ARM) conventions
       if (S.find("__DATA,__objc_catlist") != std::string::npos ||
-          S.find("__OBJC,__category") != std::string::npos)
+          S.find("__OBJC,__category") != std::string::npos ||
+          S.find("__TEXT,__swift") != std::string::npos)
         return true;
       break;
     }

@NuriAmari NuriAmari merged commit 141391a into main May 15, 2024
@NuriAmari NuriAmari deleted the users/nuriamari/fix-lto-ObjC branch May 15, 2024 16:21
AZero13 pushed a commit to AZero13/llvm-project that referenced this pull request May 17, 2024
When -ObjC is passed, the linker must force load any object files that
contain special sections that store Objective-C / Swift information that
is used at runtime.

This should work regadless if input files are bitcode or native, but it
was not working with bitcode. This is because the sections that identify
an object file that should be loaded were inconsistent when dealing with
a native file vs bitcode file. In particular, bitcode files were not
searched for `__TEXT,__swift` prefixed sections, while native files
were.

This means LLD wasn't loading certain bitcode files and forcing the user
to introduce --force-load to their linker invocation for that archive.

Co-authored-by: Nuri Amari <[email protected]>
AZero13 pushed a commit to AZero13/llvm-project that referenced this pull request May 30, 2024
When -ObjC is passed, the linker must force load any object files that
contain special sections that store Objective-C / Swift information that
is used at runtime.

This should work regadless if input files are bitcode or native, but it
was not working with bitcode. This is because the sections that identify
an object file that should be loaded were inconsistent when dealing with
a native file vs bitcode file. In particular, bitcode files were not
searched for `__TEXT,__swift` prefixed sections, while native files
were.

This means LLD wasn't loading certain bitcode files and forcing the user
to introduce --force-load to their linker invocation for that archive.

Co-authored-by: Nuri Amari <[email protected]>
AZero13 pushed a commit to AZero13/llvm-project that referenced this pull request Jun 5, 2024
When -ObjC is passed, the linker must force load any object files that
contain special sections that store Objective-C / Swift information that
is used at runtime.

This should work regadless if input files are bitcode or native, but it
was not working with bitcode. This is because the sections that identify
an object file that should be loaded were inconsistent when dealing with
a native file vs bitcode file. In particular, bitcode files were not
searched for `__TEXT,__swift` prefixed sections, while native files
were.

This means LLD wasn't loading certain bitcode files and forcing the user
to introduce --force-load to their linker invocation for that archive.

Co-authored-by: Nuri Amari <[email protected]>
@aleksandr-urakov
Copy link

Hello guys!

I just have faced this issue, and have a question about the fix. The problem is that sometimes name string contains a whitespace after the comma, e.g. __TEXT, __swift5_proto.

The current fix works in the case when there is at least one string without a whitespace (e.g. __TEXT,__swift5_typeref). I can't build an example without such section name though.

Can you tell, is the presence somehow guaranteed, or it is worth to make an additional check for the case __TEXT, __swift?

@NuriAmari
Copy link
Contributor Author

I just have faced this issue, and have a question about the fix. The problem is that sometimes name string contains a whitespace after the comma, e.g. __TEXT, __swift5_proto.

You have a case where the Swift compiler emits a section with a space? I haven't looked to closely

@aleksandr-urakov
Copy link

aleksandr-urakov commented Feb 27, 2025

You have a case where the Swift compiler emits a section with a space? I haven't looked to closely

Yes, sure! Actually __TEXT, __swift5_proto section always contains a whitespace, because it's so in the compiler sources.

It's easy reproducible:

% cat a.swift
protocol P {}
class C : P {}

% swiftc -emit-bc a.swift

aleksandr-urakov pushed a commit to aleksandr-urakov/llvm-project that referenced this pull request Jul 2, 2025
This is a fix additional to llvm#92162

In some cases, section names contain a whitespace between the segment name and
the actual section name (e.g. `__TEXT, __swift5_proto`). It is confirmed by
source code of the Swift compiler

This fix allows LTO to work correctly with the `-ObjC` flag in that rare case
when only a section with a whitespace in the name is present in the linked
bitcode module, but there are no sections containing `__TEXT,__swift`
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.

4 participants