From 9b84d72a44ad7325e3d18d34451087e73b17da85 Mon Sep 17 00:00:00 2001 From: Igor Chevdar Date: Wed, 9 Dec 2020 20:13:16 +0500 Subject: [PATCH] [objc][codegen] Fixed bug with calling fake override interface methods --- .../konan/descriptors/ClassLayoutBuilder.kt | 6 +-- .../objcexport/ObjCExportCodeGenerator.kt | 1 + .../tests/objcexport/expectedLazy.h | 12 ++++++ .../backend.native/tests/objcexport/values.kt | 9 +++++ .../tests/objcexport/values.swift | 9 +++++ .../runtime/src/main/cpp/ObjCExport.mm | 37 ++++++++++++------- 6 files changed, 57 insertions(+), 17 deletions(-) diff --git a/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/descriptors/ClassLayoutBuilder.kt b/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/descriptors/ClassLayoutBuilder.kt index 7431e56aae7..75e9ace5c2f 100644 --- a/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/descriptors/ClassLayoutBuilder.kt +++ b/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/descriptors/ClassLayoutBuilder.kt @@ -363,9 +363,9 @@ internal class ClassLayoutBuilder(val irClass: IrClass, val context: Context, va .toList() } - data class InterfaceTablePlace(val interfaceId: Int, val methodIndex: Int) { + data class InterfaceTablePlace(val interfaceId: Int, val itableSize: Int, val methodIndex: Int) { companion object { - val INVALID = InterfaceTablePlace(0, -1) + val INVALID = InterfaceTablePlace(0, -1, -1) } } @@ -374,7 +374,7 @@ internal class ClassLayoutBuilder(val irClass: IrClass, val context: Context, va val itable = interfaceTableEntries val index = itable.indexOf(function) if (index >= 0) - return InterfaceTablePlace(hierarchyInfo.interfaceId, index) + return InterfaceTablePlace(hierarchyInfo.interfaceId, itable.size, index) val superFunction = function.overriddenSymbols.first().owner return context.getLayoutBuilder(superFunction.parentAsClass).itablePlace(superFunction) } diff --git a/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/objcexport/ObjCExportCodeGenerator.kt b/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/objcexport/ObjCExportCodeGenerator.kt index 48fc3c761bb..b1a25250c97 100644 --- a/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/objcexport/ObjCExportCodeGenerator.kt +++ b/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/objcexport/ObjCExportCodeGenerator.kt @@ -405,6 +405,7 @@ internal class ObjCExportCodeGenerator( staticData.cStringLiteral(selector), Int64(nameSignature), Int32(itablePlace.interfaceId), + Int32(itablePlace.itableSize), Int32(itablePlace.methodIndex), Int32(vtableIndex), kotlinImpl diff --git a/kotlin-native/backend.native/tests/objcexport/expectedLazy.h b/kotlin-native/backend.native/tests/objcexport/expectedLazy.h index 0bbb1375263..39dcf92fc9a 100644 --- a/kotlin-native/backend.native/tests/objcexport/expectedLazy.h +++ b/kotlin-native/backend.native/tests/objcexport/expectedLazy.h @@ -1937,6 +1937,17 @@ __attribute__((swift_name("GH3825KotlinImpl"))) - (BOOL)call2Callback:(void (^)(void))callback doThrow:(BOOL)doThrow error:(NSError * _Nullable * _Nullable)error __attribute__((swift_name("call2(callback:doThrow:)"))); @end; +__attribute__((swift_name("Foo_FakeOverrideInInterface"))) +@protocol KtFoo_FakeOverrideInInterface +@required +- (void)fooT:(id _Nullable)t __attribute__((swift_name("foo(t:)"))); +@end; + +__attribute__((swift_name("Bar_FakeOverrideInInterface"))) +@protocol KtBar_FakeOverrideInInterface +@required +@end; + @interface KtEnumeration (ValuesKt) - (KtEnumeration *)getAnswer __attribute__((swift_name("getAnswer()"))); @end; @@ -2088,6 +2099,7 @@ __attribute__((swift_name("ValuesKt"))) + (KtMutableDictionary *)mutULong2Long __attribute__((swift_name("mutULong2Long()"))); + (KtMutableDictionary *)mutFloat2Float __attribute__((swift_name("mutFloat2Float()"))); + (KtMutableDictionary *)mutDouble2String __attribute__((swift_name("mutDouble2String()"))); ++ (void)callFoo_FakeOverrideInInterfaceObj:(id)obj __attribute__((swift_name("callFoo_FakeOverrideInInterface(obj:)"))); @property (class, readonly) double dbl __attribute__((swift_name("dbl"))); @property (class, readonly) float flt __attribute__((swift_name("flt"))); @property (class, readonly) int32_t integer __attribute__((swift_name("integer"))); diff --git a/kotlin-native/backend.native/tests/objcexport/values.kt b/kotlin-native/backend.native/tests/objcexport/values.kt index 01753e1c599..6f6d224360c 100644 --- a/kotlin-native/backend.native/tests/objcexport/values.kt +++ b/kotlin-native/backend.native/tests/objcexport/values.kt @@ -1012,3 +1012,12 @@ fun mutULong2Long(): MutableMap = mutableMapOf(Pair(0x8000_0000_000 fun mutFloat2Float(): MutableMap = mutableMapOf(Pair(3.14f, 100f)) fun mutDouble2String(): MutableMap = mutableMapOf(Pair(2.718281828459045, "2.718281828459045")) +interface Foo_FakeOverrideInInterface { + fun foo(t: T?) +} + +interface Bar_FakeOverrideInInterface : Foo_FakeOverrideInInterface + +fun callFoo_FakeOverrideInInterface(obj: Bar_FakeOverrideInInterface) { + obj.foo(null) +} \ No newline at end of file diff --git a/kotlin-native/backend.native/tests/objcexport/values.swift b/kotlin-native/backend.native/tests/objcexport/values.swift index 8ae7fe998ed..2605878cd58 100644 --- a/kotlin-native/backend.native/tests/objcexport/values.swift +++ b/kotlin-native/backend.native/tests/objcexport/values.swift @@ -1318,6 +1318,14 @@ func testMapsExport() throws { try assertEquals(actual: (ValuesKt.mutDouble2String() as! [Double: String])[2.718281828459045], expected: "2.718281828459045") } +class Baz_FakeOverrideInInterface : Bar_FakeOverrideInInterface { + func foo(t: Any?) {} +} + +func testFakeOverrideInInterface() throws { + ValuesKt.callFoo_FakeOverrideInInterface(obj: Baz_FakeOverrideInInterface()) +} + // -------- Execution of the test -------- class ValuesTests : SimpleTestProvider { @@ -1374,6 +1382,7 @@ class ValuesTests : SimpleTestProvider { test("TestStringConversion", testStringConversion) test("TestGH3825", testGH3825) test("TestMapsExport", testMapsExport) + test("TestFakeOverrideInInterface", testFakeOverrideInInterface) // Stress test, must remain the last one: test("TestGH2931", testGH2931) diff --git a/kotlin-native/runtime/src/main/cpp/ObjCExport.mm b/kotlin-native/runtime/src/main/cpp/ObjCExport.mm index e615ef07ea2..ae9ead6feee 100644 --- a/kotlin-native/runtime/src/main/cpp/ObjCExport.mm +++ b/kotlin-native/runtime/src/main/cpp/ObjCExport.mm @@ -54,6 +54,7 @@ struct KotlinToObjCMethodAdapter { const char* selector; MethodNameHash nameSignature; ClassId interfaceId; + int itableSize; int itableIndex; int vtableIndex; const void* kotlinImpl; @@ -889,6 +890,8 @@ static const TypeInfo* createTypeInfo(Class clazz, const TypeInfo* superType, co supers.push_back(t); } + bool itableEqualsSuper = true; + auto addToITable = [&interfaceVTables](ClassId interfaceId, int methodIndex, VTableElement entry) { RuntimeAssert(interfaceId != kInvalidInterfaceId, ""); auto interfaceVTableIt = interfaceVTables.find(interfaceId); @@ -898,7 +901,18 @@ static const TypeInfo* createTypeInfo(Class clazz, const TypeInfo* superType, co interfaceVTable[methodIndex] = entry; }; - bool itableEqualsSuper = true; + auto addITable = [&interfaceVTables, &itableEqualsSuper](ClassId interfaceId, int itableSize) { + RuntimeAssert(itableSize >= 0, ""); + auto interfaceVTablesIt = interfaceVTables.find(interfaceId); + if (interfaceVTablesIt == interfaceVTables.end()) { + itableEqualsSuper = false; + interfaceVTables.emplace(interfaceId, KStdVector(itableSize)); + } else { + auto const& interfaceVTable = interfaceVTablesIt->second; + RuntimeAssert(interfaceVTable.size() == static_cast(itableSize), ""); + } + }; + for (const TypeInfo* t : supers) { const ObjCTypeAdapter* typeAdapter = getTypeAdapter(t); if (typeAdapter == nullptr) continue; @@ -924,17 +938,8 @@ static const TypeInfo* createTypeInfo(Class clazz, const TypeInfo* superType, co if (typeAdapter == nullptr) continue; if (superITable != nullptr) { - auto interfaceId = typeInfo->classId_; - int interfaceVTableSize = typeAdapter->kotlinItableSize; - RuntimeAssert(interfaceVTableSize >= 0, ""); - auto interfaceVTablesIt = interfaceVTables.find(interfaceId); - if (interfaceVTablesIt == interfaceVTables.end()) { - itableEqualsSuper = false; - interfaceVTables.emplace(interfaceId, KStdVector(interfaceVTableSize)); - } else { - auto const& interfaceVTable = interfaceVTablesIt->second; - RuntimeAssert(interfaceVTable.size() == static_cast(interfaceVTableSize), ""); - } + // The interface vtable has to be created always in order for type checks to work. + addITable(typeInfo->classId_, typeAdapter->kotlinItableSize); } for (int i = 0; i < typeAdapter->reverseAdapterNum; ++i) { @@ -945,8 +950,12 @@ static const TypeInfo* createTypeInfo(Class clazz, const TypeInfo* superType, co insertOrReplace(methodTable, adapter->nameSignature, const_cast(adapter->kotlinImpl)); RuntimeAssert(adapter->vtableIndex == -1, ""); - if (adapter->itableIndex != -1 && superITable != nullptr) - addToITable(adapter->interfaceId, adapter->itableIndex, adapter->kotlinImpl); + if (adapter->itableIndex != -1 && superITable != nullptr) { + // In general, [adapter->interfaceId] might not be equal to [typeInfo->classId_]. + auto interfaceId = adapter->interfaceId; + addITable(interfaceId, adapter->itableSize); + addToITable(interfaceId, adapter->itableIndex, adapter->kotlinImpl); + } } }