Fix isolation between dynamic Objective-C classes in multiple frameworks

Also improve handling duplicates for @ExportObjCClass

backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/KotlinObjCClassInfoGenerator.kt

@@ -42,10 +42,13 @@ internal class KotlinObjCClassInfoGenerator(override val context: Context) : Con
         val bodySize =
                 LLVMStoreSizeOfType(llvmTargetData, context.llvmDeclarations.forClass(irClass).bodyType).toInt()
 
-        val className = selectClassName(irClass)?.let { staticData.cStringLiteral(it) } ?: NullPointer(int8Type)
+        val exportedClassName = selectExportedClassName(irClass)
+        val className = exportedClassName ?: selectInternalClassName(irClass)
 
+        val classNameLiteral = className?.let { staticData.cStringLiteral(it) } ?: NullPointer(int8Type)
         val info = Struct(runtime.kotlinObjCClassInfo,
-                className,
+                classNameLiteral,
+                Int32(if (exportedClassName != null) 1 else 0),
 
                 staticData.cStringLiteral(superclassName),
                 staticData.placeGlobalConstArray("", int8TypePtr,
@@ -91,16 +94,18 @@ internal class KotlinObjCClassInfoGenerator(override val context: Context) : Con
         }
     }
 
-    private fun selectClassName(irClass: IrClass): String? {
+    private fun selectExportedClassName(irClass: IrClass): String? {
         val exportObjCClassAnnotation = context.interopBuiltIns.exportObjCClass.fqNameSafe
-        return irClass.getAnnotationArgumentValue(exportObjCClassAnnotation, "name")
+        val explicitName = irClass.getAnnotationArgumentValue<String>(exportObjCClassAnnotation, "name")
-            ?: if (irClass.annotations.hasAnnotation(exportObjCClassAnnotation))
+        if (explicitName != null) return explicitName
-                irClass.name.asString()
+
-            else if (irClass.isExported()) {
+        return if (irClass.annotations.hasAnnotation(exportObjCClassAnnotation)) irClass.name.asString() else null
-                irClass.fqNameForIrSerialization.asString()
+    }
-            } else {
+
-                null // Generate as anonymous.
+    private fun selectInternalClassName(irClass: IrClass): String? = if (irClass.isExported()) {
-            }
+        irClass.fqNameForIrSerialization.asString()
+    } else {
+        null // Generate as anonymous.
     }
 
     private val impType = pointerType(functionType(int8TypePtr, true, int8TypePtr, int8TypePtr))

backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/objcexport/ObjCExportCodeGenerator.kt

@@ -253,6 +253,13 @@ internal class ObjCExportCodeGenerator(
 
         emitTypeAdapters()
 
+        // Replace runtime global with weak linkage:
+        replaceExternalWeakOrCommonGlobal(
+                "Kotlin_ObjCInterop_uniquePrefix",
+                codegen.staticData.cStringLiteral(namer.topLevelNamePrefix),
+                context.standardLlvmSymbolsOrigin
+        )
+
         emitSelectorsHolder()
 
         emitStaticInitializers()

backend.native/tests/build.gradle

@@ -3540,7 +3540,8 @@ interopTest("interop_objc_smoke") {
             "5\n" +
             "String macro\n" +
             "CFString macro\n" +
-            "Deallocated\nDeallocated\n"
+            "Deallocated\nDeallocated\n" +
+            "Class TestExportObjCClass34 has multiple implementations. Which one will be used is undefined.\n"
 
     source = "interop/objc/smoke.kt"
     interop = 'objcSmoke'

backend.native/tests/framework/multiple/framework1/first.kt

@@ -15,6 +15,17 @@ class I1Impl : I1 {
     override fun getFortyTwo(): Int = 42
 }
 
+fun getI1() = object : I1 {
+    override fun getFortyTwo(): Int = 42
+}
+
 class C
 
-fun getUnit(): Unit? = Unit
+fun getUnit(): Unit? = Unit
+
+/*
+// Disabled for now to avoid depending on platform libs.
+fun getAnonymousObject() = object : platform.darwin.NSObject() {}
+class NamedObject : platform.darwin.NSObject()
+fun getNamedObject() = NamedObject()
+ */

backend.native/tests/framework/multiple/framework2/second.kt

@@ -13,6 +13,17 @@ interface I2 {
 
 fun getFortyTwoFrom(i2: I2): Int = i2.getFortyTwo()
 
+fun getI2() = object : I2 {
+    override fun getFortyTwo(): Int = 42
+}
+
 class C
 
-fun isUnit(obj: Any?): Boolean = (obj === Unit)
+fun isUnit(obj: Any?): Boolean = (obj === Unit)
+
+/*
+// Disabled for now to avoid depending on platform libs.
+fun getAnonymousObject() = object : platform.darwin.NSObject() {}
+class NamedObject : platform.darwin.NSObject()
+fun getNamedObject() = NamedObject()
+ */

backend.native/tests/framework/multiple/multiple.swift

@@ -25,7 +25,7 @@ func testInteraction() throws {
     try assertEquals(actual: SecondKt.getFortyTwoFrom(i2: I1Impl()), expected: 42)
 }
 
-func testIsolation() throws {
+func testIsolation1() throws {
     try assertFalse(SecondKt.isUnit(obj: FirstKt.getUnit()))
 
     // Ensure frameworks don't share the same runtime (state):
@@ -36,6 +36,20 @@ func testIsolation() throws {
     try assertTrue(Second.RuntimeState().consumeChange())
 }
 
+func testIsolation2() throws {
+    try assertEquals(actual: FirstKt.getI1().getFortyTwo(), expected: 42)
+    try assertEquals(actual: SecondKt.getI2().getFortyTwo(), expected: 42)
+}
+
+func testIsolation3() throws {
+#if false // Disabled for now to avoid depending on platform libs.
+    FirstKt.getAnonymousObject()
+    SecondKt.getAnonymousObject()
+    FirstKt.getNamedObject()
+    SecondKt.getNamedObject()
+#endif
+}
+
 class MultipleFrameworksTests : TestProvider {
     var tests: [TestCase] = []
 
@@ -43,7 +57,9 @@ class MultipleFrameworksTests : TestProvider {
         tests = [
             TestCase(name: "TestClashingNames", method: withAutorelease(testClashingNames)),
             TestCase(name: "TestInteraction", method: withAutorelease(testInteraction)),
-            TestCase(name: "TestIsolation", method: withAutorelease(testIsolation)),
+            TestCase(name: "TestIsolation1", method: withAutorelease(testIsolation1)),
+            TestCase(name: "TestIsolation2", method: withAutorelease(testIsolation2)),
+            TestCase(name: "TestIsolation3", method: withAutorelease(testIsolation3)),
         ]
         providers.append(self)
     }

backend.native/tests/interop/objc/smoke.kt

@@ -445,9 +445,16 @@ private const val TestExportObjCClass1Name = "TestExportObjCClass"
 
 @ExportObjCClass class TestExportObjCClass2 : NSObject()
 
+const val TestExportObjCClass34Name = "TestExportObjCClass34"
+@ExportObjCClass(TestExportObjCClass34Name) class TestExportObjCClass3 : NSObject()
+@ExportObjCClass(TestExportObjCClass34Name) class TestExportObjCClass4 : NSObject()
+
 fun testExportObjCClass() {
     assertEquals(TestExportObjCClass1Name, TestExportObjCClass1().objCClassName)
     assertEquals("TestExportObjCClass2", TestExportObjCClass2().objCClassName)
+
+    assertTrue((TestExportObjCClass3().objCClassName == TestExportObjCClass34Name)
+            xor (TestExportObjCClass4().objCClassName == TestExportObjCClass34Name))
 }
 
 private val Any.objCClassName: String

runtime/src/main/cpp/ObjCExport.mm

@@ -17,6 +17,7 @@
 #import "Types.h"
 #import "Memory.h"
 #include "Natives.h"
+#include "ObjCInterop.h"
 
 #if KONAN_OBJC_INTEROP
 
@@ -976,12 +977,13 @@ static void addVirtualAdapters(Class clazz, const ObjCTypeAdapter* typeAdapter)
 static Class createClass(const TypeInfo* typeInfo, Class superClass) {
   RuntimeAssert(typeInfo->superType_ != nullptr, "");
 
-  char classNameBuffer[64];
+  int classIndex = (anonymousClassNextId++);
-  snprintf(classNameBuffer, sizeof(classNameBuffer), "kobjcc%d", anonymousClassNextId++);
+  KStdString className = Kotlin_ObjCInterop_getUniquePrefix();
-  const char* className = classNameBuffer;
+  className += "_kobjcc";
+  className += std::to_string(classIndex);
 
-  Class result = objc_allocateClassPair(superClass, className, 0);
+  Class result = objc_allocateClassPair(superClass, className.c_str(), 0);
-  RuntimeAssert(result != nullptr, "");
+  RuntimeCheck(result != nullptr, "");
 
   // TODO: optimize by adding virtual adapters only for overridden methods.
 

runtime/src/main/cpp/ObjCInterop.cpp

@@ -26,8 +26,19 @@
 #include "MemoryPrivate.hpp"
 
 #include "Natives.h"
+#include "ObjCInterop.h"
+#include "Types.h"
 #include "Utils.h"
 
+// Replaced in ObjCExportCodeGenerator.
+__attribute__((weak)) const char* Kotlin_ObjCInterop_uniquePrefix = nullptr;
+
+const char* Kotlin_ObjCInterop_getUniquePrefix() {
+  auto result = Kotlin_ObjCInterop_uniquePrefix;
+  RuntimeCheck(result != nullptr, "unique prefix is not initialized");
+  return result;
+}
+
 extern "C" {
 
 Class Kotlin_Interop_getObjCClass(const char* name);
@@ -99,6 +110,7 @@ struct ObjCMethodDescription {
 
 struct KotlinObjCClassInfo {
   const char* name;
+  int exported;
 
   const char* superclassName;
   const char** protocolNames;
@@ -129,6 +141,34 @@ static void AddMethods(Class clazz, const struct ObjCMethodDescription* methods,
 static SimpleMutex classCreationMutex;
 static int anonymousClassNextId = 0;
 
+static Class allocateClass(const KotlinObjCClassInfo* info) {
+  Class superclass = Kotlin_Interop_getObjCClass(info->superclassName);
+  size_t extraBytes = sizeof(struct KotlinClassData);
+
+  if (info->exported) {
+    RuntimeCheck(info->name != nullptr, "exported Objective-C class must have a name");
+    Class result = objc_allocateClassPair(superclass, info->name, extraBytes);
+    if (result != nullptr) return result;
+    // Similar to how Objective-C runtime handles this:
+    fprintf(stderr, "Class %s has multiple implementations. Which one will be used is undefined.\n", info->name);
+  }
+
+  KStdString className = Kotlin_ObjCInterop_getUniquePrefix();
+
+  if (info->name != nullptr) {
+    className += info->name;
+  } else {
+    className += "_kobjc";
+  }
+
+  int classId = anonymousClassNextId++;
+  className += std::to_string(classId);
+
+  Class result = objc_allocateClassPair(superclass, className.c_str(), extraBytes);
+  RuntimeCheck(result != nullptr, "Failed to allocate Objective-C class");
+  return result;
+}
+
 void* CreateKotlinObjCClass(const KotlinObjCClassInfo* info) {
   LockGuard<SimpleMutex> lockGuard(classCreationMutex);
 
@@ -137,15 +177,8 @@ void* CreateKotlinObjCClass(const KotlinObjCClassInfo* info) {
     return createdClass;
   }
 
-  char classNameBuffer[64];
+  Class newClass = allocateClass(info);
-  const char* className = info->name;
-  if (className == nullptr) {
-    snprintf(classNameBuffer, sizeof(classNameBuffer), "kobjc%d", anonymousClassNextId++);
-    className = classNameBuffer;
-  }
 
-  Class superclass = Kotlin_Interop_getObjCClass(info->superclassName);
-  Class newClass = objc_allocateClassPair(superclass, className, sizeof(struct KotlinClassData));
   RuntimeAssert(newClass != nullptr, "Failed to allocate Objective-C class");
 
   Class newMetaclass = object_getClass(reinterpret_cast<id>(newClass));

runtime/src/main/cpp/ObjCInterop.h

@@ -0,0 +1,15 @@
+/*
+ * Copyright 2010-2019 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license
+ * that can be found in the LICENSE file.
+ */
+
+#ifndef RUNTIME_OBJCINTEROP_H
+#define RUNTIME_OBJCINTEROP_H
+
+#if KONAN_OBJC_INTEROP
+
+const char* Kotlin_ObjCInterop_getUniquePrefix();
+
+#endif // KONAN_OBJC_INTEROP
+
+#endif // RUNTIME_OBJCINTEROP_H