[compiler-rt] [orc][mach-o] Fix mixing objc and swift code in a single JITDylib (PR #69258)
Ben Langmuir via llvm-commits
llvm-commits at lists.llvm.org
Thu Oct 26 10:37:17 PDT 2023
https://github.com/benlangmuir updated https://github.com/llvm/llvm-project/pull/69258
>From 1ced4644c9f6daef875a6ec70cbd514f1b6b3e24 Mon Sep 17 00:00:00 2001
From: Ben Langmuir <blangmuir at apple.com>
Date: Mon, 16 Oct 2023 15:16:18 -0700
Subject: [PATCH 1/2] [orc][mach-o] Fix mixing objc and swift code in a single
JITDylib
The system linker merges __objc_imageinfo flags values to select a
compatible set of flags using the minimum swift version and only
erroring on incompatible ABIs. Match that behaviour in the orc macho
platform. One wrinkle is that the JIT can add new objects after the
dylib is running code. In that case we only check for known incompatible
flags and ignore the swift version. It's too late to change the flags at
that point and swift version is unlikely to change runtime behaviour in
practice.
---
.../TestCases/Darwin/arm64/objc-imageinfo.S | 111 +++++++++++++++++
.../TestCases/Darwin/x86-64/objc-imageinfo.S | 111 +++++++++++++++++
.../llvm/ExecutionEngine/Orc/MachOPlatform.h | 6 +
.../lib/ExecutionEngine/Orc/MachOPlatform.cpp | 112 +++++++++++++++++-
4 files changed, 336 insertions(+), 4 deletions(-)
create mode 100644 compiler-rt/test/orc/TestCases/Darwin/arm64/objc-imageinfo.S
create mode 100644 compiler-rt/test/orc/TestCases/Darwin/x86-64/objc-imageinfo.S
diff --git a/compiler-rt/test/orc/TestCases/Darwin/arm64/objc-imageinfo.S b/compiler-rt/test/orc/TestCases/Darwin/arm64/objc-imageinfo.S
new file mode 100644
index 000000000000000..d58943f9681dadb
--- /dev/null
+++ b/compiler-rt/test/orc/TestCases/Darwin/arm64/objc-imageinfo.S
@@ -0,0 +1,111 @@
+// Test merging of __objc_imageinfo flags and ensure we can run mixed objc and
+// swift code in a single jit dylib.
+
+// REQUIRES: system-darwin && asserts
+
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: (cd %t; %clang -c *.S)
+
+// Check individual versions are loadable.
+
+// RUN: %llvm_jitlink -debug-only=orc %t/main.o %t/objc_old.o 2>&1 | FileCheck %s -check-prefix=OLD
+// OLD: MachOPlatform: Registered __objc_imageinfo for main
+// OLD-SAME: flags = 0x0000
+// RUN: %llvm_jitlink -debug-only=orc %t/main.o %t/objc_new.o 2>&1 | FileCheck %s -check-prefix=NEW
+// NEW: MachOPlatform: Registered __objc_imageinfo for main
+// NEW-SAME: flags = 0x0040
+// RUN: %llvm_jitlink -debug-only=orc %t/main.o %t/swift_4.o 2>&1 | FileCheck %s -check-prefix=SWIFT_4
+// SWIFT_4: MachOPlatform: Registered __objc_imageinfo for main
+// SWIFT_4-SAME: flags = 0x0640
+// RUN: %llvm_jitlink -debug-only=orc %t/main.o %t/swift_5.o 2>&1 | FileCheck %s -check-prefix=SWIFT_5
+// SWIFT_5: MachOPlatform: Registered __objc_imageinfo for main
+// SWIFT_5-SAME: flags = 0x5000740
+// RUN: %llvm_jitlink -debug-only=orc %t/main.o %t/swift_59.o 2>&1 | FileCheck %s -check-prefix=SWIFT_59
+// SWIFT_59: MachOPlatform: Registered __objc_imageinfo for main
+// SWIFT_59-SAME: flags = 0x5090740
+
+// Check error conditions.
+
+// RUN: not %llvm_jitlink %t/main.o %t/objc_old.o %t/objc_new.o 2>&1 | FileCheck %s -check-prefix=CATEGORY
+// CATEGORY: ObjC category class property support in {{.*}} does not match first registered flags
+
+// RUN: not %llvm_jitlink %t/main.o %t/swift_4.o %t/swift_5.o 2>&1 | FileCheck %s -check-prefix=SWIFT_ABI
+// SWIFT_ABI: Swift ABI version in {{.*}} does not match first registered flags
+
+// Check merging.
+
+// Take the lowest swift version.
+// RUN: %llvm_jitlink -debug-only=orc %t/main.o %t/swift_59.o %t/swift_5.o 2>&1 | FileCheck %s -check-prefix=SWIFT_MIX1
+// SWIFT_MIX1: MachOPlatform: Merging __objc_imageinfo flags for main {{.*}} -> 0x5000740
+
+// Add swift to objc.
+// RUN: %llvm_jitlink -debug-only=orc %t/main.o %t/swift_59.o %t/objc_new.o 2>&1 | FileCheck %s -check-prefix=SWIFT_MIX2
+// SWIFT_MIX2: MachOPlatform: Merging __objc_imageinfo flags for main {{.*}} -> 0x5090740
+
+// Add multiple swift to objc.
+// RUN: %llvm_jitlink -debug-only=orc %t/main.o %t/swift_59.o %t/swift_5.o %t/objc_new.o 2>&1 | FileCheck %s -check-prefix=SWIFT_MIX3
+// SWIFT_MIX3: MachOPlatform: Merging __objc_imageinfo flags for main {{.*}} -> 0x5000740
+
+//--- main.S
+.section __TEXT,__text,regular,pure_instructions
+.globl _main
+_main:
+ mov w0, #0
+ ret
+
+//--- objc_old.S
+.section __TEXT,__text,regular,pure_instructions
+.globl _objc1
+_objc1:
+ ret
+
+ .section __DATA,__objc_imageinfo,regular,no_dead_strip
+L_OBJC_IMAGE_INFO:
+ .long 0
+ .long 0
+
+//--- objc_new.S
+.section __TEXT,__text,regular,pure_instructions
+.globl _objc2
+_objc2:
+ ret
+
+ .section __DATA,__objc_imageinfo,regular,no_dead_strip
+L_OBJC_IMAGE_INFO:
+ .long 0
+ .long 64
+
+//--- swift_4.S
+.section __TEXT,__text,regular,pure_instructions
+.globl _swift4
+_swift4:
+ ret
+
+ .section __DATA,__objc_imageinfo,regular,no_dead_strip
+L_OBJC_IMAGE_INFO:
+ .long 0
+ .long 1600
+
+//--- swift_5.S
+.section __TEXT,__text,regular,pure_instructions
+.globl _swift5
+_swift5:
+ ret
+
+ .section __DATA,__objc_imageinfo,regular,no_dead_strip
+L_OBJC_IMAGE_INFO:
+ .long 0
+ .long 83887936
+
+//--- swift_59.S
+.section __TEXT,__text,regular,pure_instructions
+.globl _swift59
+_swift59:
+ ret
+
+ .section __DATA,__objc_imageinfo,regular,no_dead_strip
+L_OBJC_IMAGE_INFO:
+ .long 0
+ .long 84477760
+
diff --git a/compiler-rt/test/orc/TestCases/Darwin/x86-64/objc-imageinfo.S b/compiler-rt/test/orc/TestCases/Darwin/x86-64/objc-imageinfo.S
new file mode 100644
index 000000000000000..90b5c3a38eebe05
--- /dev/null
+++ b/compiler-rt/test/orc/TestCases/Darwin/x86-64/objc-imageinfo.S
@@ -0,0 +1,111 @@
+// Test merging of __objc_imageinfo flags and ensure we can run mixed objc and
+// swift code in a single jit dylib.
+
+// REQUIRES: system-darwin && asserts
+
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: (cd %t; %clang -c *.S)
+
+// Check individual versions are loadable.
+
+// RUN: %llvm_jitlink -debug-only=orc %t/main.o %t/objc_old.o 2>&1 | FileCheck %s -check-prefix=OLD
+// OLD: MachOPlatform: Registered __objc_imageinfo for main
+// OLD-SAME: flags = 0x0000
+// RUN: %llvm_jitlink -debug-only=orc %t/main.o %t/objc_new.o 2>&1 | FileCheck %s -check-prefix=NEW
+// NEW: MachOPlatform: Registered __objc_imageinfo for main
+// NEW-SAME: flags = 0x0040
+// RUN: %llvm_jitlink -debug-only=orc %t/main.o %t/swift_4.o 2>&1 | FileCheck %s -check-prefix=SWIFT_4
+// SWIFT_4: MachOPlatform: Registered __objc_imageinfo for main
+// SWIFT_4-SAME: flags = 0x0640
+// RUN: %llvm_jitlink -debug-only=orc %t/main.o %t/swift_5.o 2>&1 | FileCheck %s -check-prefix=SWIFT_5
+// SWIFT_5: MachOPlatform: Registered __objc_imageinfo for main
+// SWIFT_5-SAME: flags = 0x5000740
+// RUN: %llvm_jitlink -debug-only=orc %t/main.o %t/swift_59.o 2>&1 | FileCheck %s -check-prefix=SWIFT_59
+// SWIFT_59: MachOPlatform: Registered __objc_imageinfo for main
+// SWIFT_59-SAME: flags = 0x5090740
+
+// Check error conditions.
+
+// RUN: not %llvm_jitlink %t/main.o %t/objc_old.o %t/objc_new.o 2>&1 | FileCheck %s -check-prefix=CATEGORY
+// CATEGORY: ObjC category class property support in {{.*}} does not match first registered flags
+
+// RUN: not %llvm_jitlink %t/main.o %t/swift_4.o %t/swift_5.o 2>&1 | FileCheck %s -check-prefix=SWIFT_ABI
+// SWIFT_ABI: Swift ABI version in {{.*}} does not match first registered flags
+
+// Check merging.
+
+// Take the lowest swift version.
+// RUN: %llvm_jitlink -debug-only=orc %t/main.o %t/swift_59.o %t/swift_5.o 2>&1 | FileCheck %s -check-prefix=SWIFT_MIX1
+// SWIFT_MIX1: MachOPlatform: Merging __objc_imageinfo flags for main {{.*}} -> 0x5000740
+
+// Add swift to objc.
+// RUN: %llvm_jitlink -debug-only=orc %t/main.o %t/swift_59.o %t/objc_new.o 2>&1 | FileCheck %s -check-prefix=SWIFT_MIX2
+// SWIFT_MIX2: MachOPlatform: Merging __objc_imageinfo flags for main {{.*}} -> 0x5090740
+
+// Add multiple swift to objc.
+// RUN: %llvm_jitlink -debug-only=orc %t/main.o %t/swift_59.o %t/swift_5.o %t/objc_new.o 2>&1 | FileCheck %s -check-prefix=SWIFT_MIX3
+// SWIFT_MIX3: MachOPlatform: Merging __objc_imageinfo flags for main {{.*}} -> 0x5000740
+
+//--- main.S
+.section __TEXT,__text,regular,pure_instructions
+.globl _main
+_main:
+ xorl %eax, %eax
+ ret
+
+//--- objc_old.S
+.section __TEXT,__text,regular,pure_instructions
+.globl _objc1
+_objc1:
+ ret
+
+ .section __DATA,__objc_imageinfo,regular,no_dead_strip
+L_OBJC_IMAGE_INFO:
+ .long 0
+ .long 0
+
+//--- objc_new.S
+.section __TEXT,__text,regular,pure_instructions
+.globl _objc2
+_objc2:
+ ret
+
+ .section __DATA,__objc_imageinfo,regular,no_dead_strip
+L_OBJC_IMAGE_INFO:
+ .long 0
+ .long 64
+
+//--- swift_4.S
+.section __TEXT,__text,regular,pure_instructions
+.globl _swift4
+_swift4:
+ ret
+
+ .section __DATA,__objc_imageinfo,regular,no_dead_strip
+L_OBJC_IMAGE_INFO:
+ .long 0
+ .long 1600
+
+//--- swift_5.S
+.section __TEXT,__text,regular,pure_instructions
+.globl _swift5
+_swift5:
+ ret
+
+ .section __DATA,__objc_imageinfo,regular,no_dead_strip
+L_OBJC_IMAGE_INFO:
+ .long 0
+ .long 83887936
+
+//--- swift_59.S
+.section __TEXT,__text,regular,pure_instructions
+.globl _swift59
+_swift59:
+ ret
+
+ .section __DATA,__objc_imageinfo,regular,no_dead_strip
+L_OBJC_IMAGE_INFO:
+ .long 0
+ .long 84477760
+
diff --git a/llvm/include/llvm/ExecutionEngine/Orc/MachOPlatform.h b/llvm/include/llvm/ExecutionEngine/Orc/MachOPlatform.h
index b9e6279432e5b24..d4af6c9234e3a42 100644
--- a/llvm/include/llvm/ExecutionEngine/Orc/MachOPlatform.h
+++ b/llvm/include/llvm/ExecutionEngine/Orc/MachOPlatform.h
@@ -159,6 +159,9 @@ class MachOPlatform : public Platform {
struct ObjCImageInfo {
uint32_t Version = 0;
uint32_t Flags = 0;
+ /// Whether this image info can no longer be mutated, as it may have been
+ /// registered with the objc runtime.
+ bool Finalized = false;
};
Error bootstrapPipelineStart(jitlink::LinkGraph &G);
@@ -173,6 +176,9 @@ class MachOPlatform : public Platform {
Error processObjCImageInfo(jitlink::LinkGraph &G,
MaterializationResponsibility &MR);
+ Error mergeImageInfoFlags(jitlink::LinkGraph &G,
+ MaterializationResponsibility &MR,
+ ObjCImageInfo &Info, uint32_t NewFlags);
Error fixTLVSectionsAndEdges(jitlink::LinkGraph &G, JITDylib &JD);
diff --git a/llvm/lib/ExecutionEngine/Orc/MachOPlatform.cpp b/llvm/lib/ExecutionEngine/Orc/MachOPlatform.cpp
index abc0bbbcaad2a39..22a9534ccdf0535 100644
--- a/llvm/lib/ExecutionEngine/Orc/MachOPlatform.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/MachOPlatform.cpp
@@ -266,6 +266,33 @@ static StringRef ObjCRuntimeObjectSectionName =
static StringRef ObjCImageInfoSymbolName =
"__llvm_jitlink_macho_objc_imageinfo";
+struct ObjCImageInfoFlags {
+ uint16_t SwiftABIVersion;
+ uint16_t SwiftVersion;
+ bool HasCategoryClassProperties;
+ bool HasSignedObjCClassROs;
+
+ static constexpr uint32_t SIGNED_CLASS_RO = (1 << 4);
+ static constexpr uint32_t HAS_CATEGORY_CLASS_PROPERTIES = (1 << 6);
+
+ explicit ObjCImageInfoFlags(uint32_t RawFlags) {
+ HasSignedObjCClassROs = RawFlags & SIGNED_CLASS_RO;
+ HasCategoryClassProperties = RawFlags & HAS_CATEGORY_CLASS_PROPERTIES;
+ SwiftABIVersion = (RawFlags >> 8) & 0xFF;
+ SwiftVersion = (RawFlags >> 16) & 0xFFFF;
+ }
+
+ uint32_t rawFlags() const {
+ uint32_t Result = 0;
+ if (HasCategoryClassProperties)
+ Result |= HAS_CATEGORY_CLASS_PROPERTIES;
+ if (HasSignedObjCClassROs)
+ Result |= SIGNED_CLASS_RO;
+ Result |= (SwiftABIVersion << 8);
+ Result |= (SwiftVersion << 16);
+ return Result;
+ }
+};
} // end anonymous namespace
namespace llvm {
@@ -1029,15 +1056,19 @@ Error MachOPlatform::MachOPlatformPlugin::processObjCImageInfo(
" does not match first registered version",
inconvertibleErrorCode());
if (ObjCImageInfoItr->second.Flags != Flags)
- return make_error<StringError>("ObjC flags in " + G.getName() +
- " do not match first registered flags",
- inconvertibleErrorCode());
+ if (Error E = mergeImageInfoFlags(G, MR, ObjCImageInfoItr->second, Flags))
+ return E;
// __objc_imageinfo is valid. Delete the block.
for (auto *S : ObjCImageInfo->symbols())
G.removeDefinedSymbol(*S);
G.removeBlock(ObjCImageInfoBlock);
} else {
+ LLVM_DEBUG({
+ dbgs() << "MachOPlatform: Registered __objc_imageinfo for "
+ << MR.getTargetJITDylib().getName() << " in " << G.getName()
+ << "; flags = " << formatv("{0:x4}", Flags) << "\n";
+ });
// We haven't registered an __objc_imageinfo section yet. Register and
// move on. The section should already be marked no-dead-strip.
G.addDefinedSymbol(ObjCImageInfoBlock, 0, ObjCImageInfoSymbolName,
@@ -1047,12 +1078,66 @@ Error MachOPlatform::MachOPlatformPlugin::processObjCImageInfo(
{{MR.getExecutionSession().intern(ObjCImageInfoSymbolName),
JITSymbolFlags()}}))
return Err;
- ObjCImageInfos[&MR.getTargetJITDylib()] = {Version, Flags};
+ ObjCImageInfos[&MR.getTargetJITDylib()] = {Version, Flags, false};
}
return Error::success();
}
+Error MachOPlatform::MachOPlatformPlugin::mergeImageInfoFlags(
+ jitlink::LinkGraph &G, MaterializationResponsibility &MR,
+ ObjCImageInfo &Info, uint32_t NewFlags) {
+ if (Info.Flags == NewFlags)
+ return Error::success();
+
+ ObjCImageInfoFlags Old(Info.Flags);
+ ObjCImageInfoFlags New(NewFlags);
+
+ // Check for incompatible flags.
+ if (Old.SwiftABIVersion && New.SwiftABIVersion &&
+ Old.SwiftABIVersion != New.SwiftABIVersion)
+ return make_error<StringError>("Swift ABI version in " + G.getName() +
+ " does not match first registered flags",
+ inconvertibleErrorCode());
+
+ if (Old.HasCategoryClassProperties != New.HasCategoryClassProperties)
+ return make_error<StringError>("ObjC category class property support in " +
+ G.getName() +
+ " does not match first registered flags",
+ inconvertibleErrorCode());
+ if (Old.HasSignedObjCClassROs != New.HasSignedObjCClassROs)
+ return make_error<StringError>("ObjC class_ro_t pointer signing in " +
+ G.getName() +
+ " does not match first registered flags",
+ inconvertibleErrorCode());
+
+ // If we cannot change the flags, ignore any remaining differences. Adding
+ // Swift or changing its version are unlikely to cause problems in practice.
+ if (Info.Finalized)
+ return Error::success();
+
+ // Use the minimum Swift version.
+ if (Old.SwiftVersion && New.SwiftVersion)
+ New.SwiftVersion = std::min(Old.SwiftVersion, New.SwiftVersion);
+ else if (Old.SwiftVersion)
+ New.SwiftVersion = Old.SwiftVersion;
+ // Add a Swift ABI version if it was pure objc before.
+ if (!New.SwiftABIVersion)
+ New.SwiftABIVersion = Old.SwiftABIVersion;
+
+ LLVM_DEBUG({
+ dbgs() << "MachOPlatform: Merging __objc_imageinfo flags for "
+ << MR.getTargetJITDylib().getName() << " (was "
+ << formatv("{0:x4}", Old.rawFlags()) << ")"
+ << " with " << G.getName() << " (" << formatv("{0:x4}", NewFlags)
+ << ")"
+ << " -> " << formatv("{0:x4}", New.rawFlags()) << "\n";
+ });
+
+ Info.Flags = New.rawFlags();
+ return Error::success();
+}
+
Error MachOPlatform::MachOPlatformPlugin::fixTLVSectionsAndEdges(
jitlink::LinkGraph &G, JITDylib &JD) {
@@ -1403,6 +1488,25 @@ Error MachOPlatform::MachOPlatformPlugin::populateObjCRuntimeObject(
for (auto *Sym : G.defined_symbols())
if (Sym->hasName() && Sym->getName() == ObjCImageInfoSymbolName) {
ObjCImageInfoSym = Sym;
+ std::optional<uint32_t> Flags;
+ {
+ std::lock_guard<std::mutex> Lock(PluginMutex);
+ auto It = ObjCImageInfos.find(&MR.getTargetJITDylib());
+ if (It != ObjCImageInfos.end()) {
+ It->second.Finalized = true;
+ Flags = It->second.Flags;
+ }
+ }
+
+ if (Flags) {
+ // We own the definition of __objc_image_info; write the final
+ // merged flags value.
+ auto Content = Sym->getBlock().getMutableContent(G);
+ assert(
+ Content.size() == sizeof(uint32_t) * 2 &&
+ "__objc_image_info size should have been verified already");
+ memcpy(&Content[sizeof(uint32_t)], &Flags, sizeof(uint32_t));
+ }
break;
}
if (!ObjCImageInfoSym)
>From 325a35753bbf96ce540ab8416aeca0879b86c37f Mon Sep 17 00:00:00 2001
From: Ben Langmuir <blangmuir at apple.com>
Date: Wed, 18 Oct 2023 09:54:55 -0700
Subject: [PATCH 2/2] Write flags with correct endianness, value
We were writing the flags with the JIT process' endianness, which could
be wrong in theory, and we were using memcpy on
`std::optional<uint32_t>` instead of the `uint32_t` that was expected.
That happens to work on some platforms, but obviously is not correct.
---
llvm/lib/ExecutionEngine/Orc/MachOPlatform.cpp | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/llvm/lib/ExecutionEngine/Orc/MachOPlatform.cpp b/llvm/lib/ExecutionEngine/Orc/MachOPlatform.cpp
index 22a9534ccdf0535..6d8e2396137337b 100644
--- a/llvm/lib/ExecutionEngine/Orc/MachOPlatform.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/MachOPlatform.cpp
@@ -1502,10 +1502,9 @@ Error MachOPlatform::MachOPlatformPlugin::populateObjCRuntimeObject(
// We own the definition of __objc_image_info; write the final
// merged flags value.
auto Content = Sym->getBlock().getMutableContent(G);
- assert(
- Content.size() == sizeof(uint32_t) * 2 &&
+ assert(Content.size() == 8 &&
"__objc_image_info size should have been verified already");
- memcpy(&Content[sizeof(uint32_t)], &Flags, sizeof(uint32_t));
+ support::endian::write32(&Content[4], *Flags, G.getEndianness());
}
break;
}
More information about the llvm-commits
mailing list