[compiler-rt] 05a0d94 - [orc][mach-o] Fix mixing objc and swift code in a single JITDylib (#69258)

via llvm-commits llvm-commits at lists.llvm.org
Fri Nov 3 08:32:13 PDT 2023


Author: Ben Langmuir
Date: 2023-11-03T08:32:08-07:00
New Revision: 05a0d9441630ff417d6e4185b526478967d1fe15

URL: https://github.com/llvm/llvm-project/commit/05a0d9441630ff417d6e4185b526478967d1fe15
DIFF: https://github.com/llvm/llvm-project/commit/05a0d9441630ff417d6e4185b526478967d1fe15.diff

LOG: [orc][mach-o] Fix mixing objc and swift code in a single JITDylib (#69258)

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.

Added: 
    compiler-rt/test/orc/TestCases/Darwin/arm64/objc-imageinfo.S
    compiler-rt/test/orc/TestCases/Darwin/x86-64/objc-imageinfo.S

Modified: 
    llvm/include/llvm/ExecutionEngine/Orc/MachOPlatform.h
    llvm/lib/ExecutionEngine/Orc/MachOPlatform.cpp

Removed: 
    


################################################################################
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..6d8e2396137337b 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 
diff erences. 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,24 @@ 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() == 8 &&
+                  "__objc_image_info size should have been verified already");
+              support::endian::write32(&Content[4], *Flags, G.getEndianness());
+            }
             break;
           }
       if (!ObjCImageInfoSym)


        


More information about the llvm-commits mailing list