From 41f8d5fc077613816adb3a6cab698233d23e8157 Mon Sep 17 00:00:00 2001 From: Igor Chevdar Date: Wed, 30 Oct 2019 12:40:29 +0300 Subject: [PATCH] Abstract class objc fix (#3517) Fixed itable for objc inheritors of abstract classes --- .../backend/konan/llvm/RTTIGenerator.kt | 35 ++++++++++--------- .../objcexport/ObjCExportCodeGenerator.kt | 12 +++++-- .../tests/framework/values/expectedLazy.h | 35 +++++++++++++++++++ .../tests/framework/values/values.kt | 25 ++++++++++++- .../tests/framework/values/values.swift | 32 +++++++++++++++++ runtime/src/main/cpp/ObjCExport.mm | 26 +++++++++----- 6 files changed, 136 insertions(+), 29 deletions(-) diff --git a/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/RTTIGenerator.kt b/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/RTTIGenerator.kt index c79f89d5a48..ebeb9955783 100644 --- a/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/RTTIGenerator.kt +++ b/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/RTTIGenerator.kt @@ -84,7 +84,7 @@ internal class RTTIGenerator(override val context: Context) : ContextUtils { methods: ConstValue, methodsCount: Int, interfaceTableSize: Int, - interfaceTable: ConstPointer?, + interfaceTable: ConstValue, packageName: String?, relativeName: String?, flags: Int, @@ -220,7 +220,15 @@ internal class RTTIGenerator(override val context: Context) : ContextUtils { val methodsPtr = staticData.placeGlobalConstArray("kmethods:$className", runtime.methodTableRecordType, methods) - val (interfaceTable, interfaceTableSize) = interfaceTable(irClass) + val needInterfaceTable = context.ghaEnabled() && !irClass.isInterface + && !irClass.isAbstract() && !irClass.isObjCClass() + val (interfaceTable, interfaceTableSize) = if (needInterfaceTable) { + interfaceTableRecords(irClass) + } else { + Pair(emptyList(), -1) + } + val interfaceTablePtr = staticData.placeGlobalConstArray("kifacetable:$className", + runtime.interfaceTableRecordType, interfaceTable) val reflectionInfo = getReflectionInfo(irClass) val typeInfoGlobal = llvmDeclarations.typeInfoGlobal @@ -236,7 +244,7 @@ internal class RTTIGenerator(override val context: Context) : ContextUtils { objOffsetsPtr, objOffsetsCount, interfacesPtr, interfaces.size, methodsPtr, methods.size, - interfaceTableSize, interfaceTable, + interfaceTableSize, interfaceTablePtr, reflectionInfo.packageName, reflectionInfo.relativeName, flagsFromClass(irClass), @@ -301,10 +309,7 @@ internal class RTTIGenerator(override val context: Context) : ContextUtils { }.sortedBy { it.nameSignature.value } } - private fun interfaceTable(irClass: IrClass): Pair { - val needInterfaceTable = context.ghaEnabled() && !irClass.isInterface - && !irClass.isAbstract() && !irClass.isObjCClass() - if (!needInterfaceTable) return Pair(null, 0) + fun interfaceTableRecords(irClass: IrClass): Pair, Int> { // The details are in ClassLayoutBuilder. val interfaceLayouts = irClass.implementedInterfaces.map { context.getLayoutBuilder(it) } val interfaceColors = interfaceLayouts.map { it.hierarchyInfo.interfaceColor } @@ -366,11 +371,7 @@ internal class RTTIGenerator(override val context: Context) : ContextUtils { } ) } - return Pair( - staticData.placeGlobalConstArray("kifacetable:$className", - runtime.interfaceTableRecordType, interfaceTableEntries), - if (conservative) -size else (size - 1) - ) + return Pair(interfaceTableEntries, if (conservative) -size else (size - 1)) } private fun mapRuntimeType(type: LLVMTypeRef): Int = @@ -489,19 +490,19 @@ internal class RTTIGenerator(override val context: Context) : ContextUtils { else ClassGlobalHierarchyInfo(-1, -1, 0, 0) - val interfaceTable = if (!context.ghaEnabled()) null else { + val interfaceTable = if (!context.ghaEnabled()) emptyList() else { val layoutBuilder = context.getLayoutBuilder(irClass) val vtableEntries = layoutBuilder.interfaceTableEntries.map { methodImpls[it]!!.bitcast(int8TypePtr) } val interfaceVTable = staticData.placeGlobalArray("", kInt8Ptr, vtableEntries) - staticData.placeGlobalConstArray("", - runtime.interfaceTableRecordType, listOf( + listOf( InterfaceTableRecord( Int32(layoutBuilder.hierarchyInfo.interfaceId), Int32(layoutBuilder.interfaceTableEntries.size), interfaceVTable.pointer.getElementPtr(0) ) - )) + ) } + val interfaceTablePtr = staticData.placeGlobalConstArray("", runtime.interfaceTableRecordType, interfaceTable) val typeInfoWithVtable = Struct(TypeInfo( selfPtr = result, @@ -512,7 +513,7 @@ internal class RTTIGenerator(override val context: Context) : ContextUtils { interfaces = interfacesPtr, interfacesCount = interfaces.size, methods = methodsPtr, methodsCount = methods.size, // 0 is a perfectly valid itable size (since 0 is in form of (2^k - 1). - interfaceTableSize = 0, interfaceTable = interfaceTable, + interfaceTableSize = 0, interfaceTable = interfaceTablePtr, packageName = reflectionInfo.packageName, relativeName = reflectionInfo.relativeName, flags = flagsFromClass(irClass) or (if (immutable) TF_IMMUTABLE else 0), diff --git a/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/objcexport/ObjCExportCodeGenerator.kt b/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/objcexport/ObjCExportCodeGenerator.kt index cd274a85953..b5bc498200c 100644 --- a/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/objcexport/ObjCExportCodeGenerator.kt +++ b/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/objcexport/ObjCExportCodeGenerator.kt @@ -380,6 +380,7 @@ internal class ObjCExportCodeGenerator( vtable: ConstPointer?, vtableSize: Int, methodTable: List, + itable: List, itableSize: Int, val objCName: String, directAdapters: List, @@ -396,6 +397,7 @@ internal class ObjCExportCodeGenerator( staticData.placeGlobalConstArray("", runtime.methodTableRecordType, methodTable), Int32(methodTable.size), + staticData.placeGlobalConstArray("", runtime.interfaceTableRecordType, itable), Int32(itableSize), staticData.cStringLiteral(objCName), @@ -1043,6 +1045,7 @@ private fun ObjCExportCodeGenerator.createTypeAdapterForFileClass( vtable = null, vtableSize = -1, methodTable = emptyList(), + itable = emptyList(), itableSize = -1, objCName = name, directAdapters = emptyList(), @@ -1117,9 +1120,11 @@ private fun ObjCExportCodeGenerator.createTypeAdapter( emptyList() } - val itableSize = if (irClass.isInterface) - context.getLayoutBuilder(irClass).interfaceTableEntries.size - else -1 + val (itable, itableSize) = when { + irClass.isInterface -> Pair(emptyList(), context.getLayoutBuilder(irClass).interfaceTableEntries.size) + irClass.isAbstract() && context.ghaEnabled() -> rttiGenerator.interfaceTableRecords(irClass) + else -> Pair(emptyList(), -1) + } when (irClass.kind) { ClassKind.OBJECT -> { @@ -1140,6 +1145,7 @@ private fun ObjCExportCodeGenerator.createTypeAdapter( vtable, vtableSize, methodTable, + itable, itableSize, objCName, adapters, diff --git a/backend.native/tests/framework/values/expectedLazy.h b/backend.native/tests/framework/values/expectedLazy.h index d3eb189ef24..da5129b86d4 100644 --- a/backend.native/tests/framework/values/expectedLazy.h +++ b/backend.native/tests/framework/values/expectedLazy.h @@ -981,6 +981,39 @@ __attribute__((swift_name("InterfaceForTypeCheck"))) @required @end; +__attribute__((swift_name("IAbstractInterface"))) +@protocol ValuesIAbstractInterface +@required +- (int32_t)foo __attribute__((swift_name("foo()"))); +@end; + +__attribute__((swift_name("IAbstractInterface2"))) +@protocol ValuesIAbstractInterface2 +@required +- (int32_t)foo __attribute__((swift_name("foo()"))); +@end; + +__attribute__((swift_name("AbstractInterfaceBase"))) +@interface ValuesAbstractInterfaceBase : ValuesBase +- (instancetype)init __attribute__((swift_name("init()"))) __attribute__((objc_designated_initializer)); ++ (instancetype)new __attribute__((availability(swift, unavailable, message="use object initializers instead"))); +- (int32_t)foo __attribute__((swift_name("foo()"))); +- (int32_t)bar __attribute__((swift_name("bar()"))); +@end; + +__attribute__((swift_name("AbstractInterfaceBase2"))) +@interface ValuesAbstractInterfaceBase2 : ValuesBase +- (instancetype)init __attribute__((swift_name("init()"))) __attribute__((objc_designated_initializer)); ++ (instancetype)new __attribute__((availability(swift, unavailable, message="use object initializers instead"))); +@end; + +__attribute__((swift_name("AbstractInterfaceBase3"))) +@interface ValuesAbstractInterfaceBase3 : ValuesBase +- (instancetype)init __attribute__((swift_name("init()"))) __attribute__((objc_designated_initializer)); ++ (instancetype)new __attribute__((availability(swift, unavailable, message="use object initializers instead"))); +- (int32_t)foo __attribute__((swift_name("foo()"))); +@end; + @interface ValuesEnumeration (ValuesKt) - (ValuesEnumeration *)getAnswer __attribute__((swift_name("getAnswer()"))); @end; @@ -1064,6 +1097,8 @@ __attribute__((swift_name("ValuesKt"))) + (void)gc __attribute__((swift_name("gc()"))); + (BOOL)testClassTypeCheckX:(id)x __attribute__((swift_name("testClassTypeCheck(x:)"))); + (BOOL)testInterfaceTypeCheckX:(id)x __attribute__((swift_name("testInterfaceTypeCheck(x:)"))); ++ (int32_t)testAbstractInterfaceCallX:(id)x __attribute__((swift_name("testAbstractInterfaceCall(x:)"))); ++ (int32_t)testAbstractInterfaceCall2X:(id)x __attribute__((swift_name("testAbstractInterfaceCall2(x:)"))); @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/backend.native/tests/framework/values/values.kt b/backend.native/tests/framework/values/values.kt index d533a70e81e..6ca3ce01918 100644 --- a/backend.native/tests/framework/values/values.kt +++ b/backend.native/tests/framework/values/values.kt @@ -787,4 +787,27 @@ fun testClassTypeCheck(x: Any) = x is ClassForTypeCheck interface InterfaceForTypeCheck -fun testInterfaceTypeCheck(x: Any) = x is InterfaceForTypeCheck \ No newline at end of file +fun testInterfaceTypeCheck(x: Any) = x is InterfaceForTypeCheck + +interface IAbstractInterface { + fun foo(): Int +} + +interface IAbstractInterface2 { + fun foo() = 42 +} + +fun testAbstractInterfaceCall(x: IAbstractInterface) = x.foo() +fun testAbstractInterfaceCall2(x: IAbstractInterface2) = x.foo() + +abstract class AbstractInterfaceBase : IAbstractInterface { + override fun foo() = bar() + + abstract fun bar(): Int +} + +abstract class AbstractInterfaceBase2 : IAbstractInterface2 + +abstract class AbstractInterfaceBase3 : IAbstractInterface { + abstract override fun foo(): Int +} \ No newline at end of file diff --git a/backend.native/tests/framework/values/values.swift b/backend.native/tests/framework/values/values.swift index c9860ab0818..de88a55e4a3 100644 --- a/backend.native/tests/framework/values/values.swift +++ b/backend.native/tests/framework/values/values.swift @@ -1029,6 +1029,35 @@ func testInterfaceTypeCheck() throws { try assertFalse(ValuesKt.testInterfaceTypeCheck(x: ClassForInterfaceTypeCheck_Fail())) } +class AbstractInterface : AbstractInterfaceBase { + override func bar() -> Int32 { + return 42 + } +} + +// See https://github.com/JetBrains/kotlin-native/issues/3503 +func testGH3503_1() throws { + try assertEquals(actual: ValuesKt.testAbstractInterfaceCall(x: AbstractInterface()), expected: 42) +} + +class AbstractInterface2 : AbstractInterfaceBase2 { + +} + +func testGH3503_2() throws { + try assertEquals(actual: ValuesKt.testAbstractInterfaceCall2(x: AbstractInterface2()), expected: 42) +} + +class AbstractInterface3 : AbstractInterfaceBase3 { + override func foo() -> Int32 { + return 42 + } +} + +func testGH3503_3() throws { + try assertEquals(actual: ValuesKt.testAbstractInterfaceCall(x: AbstractInterface3()), expected: 42) +} + // -------- Execution of the test -------- class ValuesTests : TestProvider { @@ -1080,6 +1109,9 @@ class ValuesTests : TestProvider { TestCase(name: "TestSharedRefs", method: withAutorelease(TestSharedRefs().test)), TestCase(name: "TestClassTypeCheck", method: withAutorelease(testClassTypeCheck)), TestCase(name: "TestInterfaceTypeCheck", method: withAutorelease(testInterfaceTypeCheck)), + TestCase(name: "TestGH3503_1", method: withAutorelease(testGH3503_1)), + TestCase(name: "TestGH3503_2", method: withAutorelease(testGH3503_2)), + TestCase(name: "TestGH3503_3", method: withAutorelease(testGH3503_3)), TestCase(name: "TestGH2931", method: withAutorelease(testGH2931)), ] } diff --git a/runtime/src/main/cpp/ObjCExport.mm b/runtime/src/main/cpp/ObjCExport.mm index e9c512a2bb9..c54905c8f01 100644 --- a/runtime/src/main/cpp/ObjCExport.mm +++ b/runtime/src/main/cpp/ObjCExport.mm @@ -67,6 +67,7 @@ struct ObjCTypeAdapter { const MethodTableRecord* kotlinMethodTable; int kotlinMethodTableSize; + const InterfaceTableRecord* kotlinItable; int kotlinItableSize; const char* objCName; @@ -634,6 +635,8 @@ static const TypeInfo* createTypeInfo( const KStdVector& vtable, const KStdVector& methodTable, const KStdOrderedMap>& interfaceVTables, + const InterfaceTableRecord* superItable, + int superItableSize, bool itableEqualsSuper ) { TypeInfo* result = (TypeInfo*)konanAllocMemory(sizeof(TypeInfo) + vtable.size() * sizeof(void*)); @@ -669,11 +672,10 @@ static const TypeInfo* createTypeInfo( result->implementedInterfaces_ = implementedInterfaces_; result->implementedInterfacesCount_ = implementedInterfaces.size(); - const InterfaceTableRecord* superItable = superType->interfaceTable_; if (superItable != nullptr) { if (itableEqualsSuper) { - result->interfaceTableSize_ = superType->interfaceTableSize_; - result->interfaceTable_ = superType->interfaceTable_; + result->interfaceTableSize_ = superItableSize; + result->interfaceTable_ = superItable; } else { buildITable(result, interfaceVTables); } @@ -803,6 +805,9 @@ static const TypeInfo* createTypeInfo(Class clazz, const TypeInfo* superType) { const MethodTableRecord* superMethodTable = nullptr; int superMethodTableSize = 0; + InterfaceTableRecord const* superITable = nullptr; + int superITableSize = 0; + if (superTypeAdapter != nullptr) { // Then super class is Kotlin class. @@ -811,6 +816,8 @@ static const TypeInfo* createTypeInfo(Class clazz, const TypeInfo* superType) { superVtable = superTypeAdapter->kotlinVtable; superMethodTable = superTypeAdapter->kotlinMethodTable; superMethodTableSize = superTypeAdapter->kotlinMethodTableSize; + superITable = superTypeAdapter->kotlinItable; + superITableSize = superTypeAdapter->kotlinItableSize; } if (superVtable == nullptr) superVtable = superType->vtable(); @@ -828,12 +835,14 @@ static const TypeInfo* createTypeInfo(Class clazz, const TypeInfo* superType) { superMethodTable, superMethodTable + superMethodTableSize ); - InterfaceTableRecord const* superITable = superType->interfaceTable_; + if (superITable == nullptr) { + superITable = superType->interfaceTable_; + superITableSize = superType->interfaceTableSize_; + } KStdOrderedMap> interfaceVTables; if (superITable != nullptr) { - int superITableSize = superType->interfaceTableSize_; - superITableSize = superITableSize >= 0 ? superITableSize + 1 : -superITableSize; - for (int i = 0; i < superITableSize; ++i) { + int actualItableSize = superITableSize >= 0 ? superITableSize + 1 : -superITableSize; + for (int i = 0; i < actualItableSize; ++i) { auto& record = superITable[i]; auto interfaceId = record.id; if (interfaceId == kInvalidInterfaceId) continue; @@ -919,7 +928,8 @@ static const TypeInfo* createTypeInfo(Class clazz, const TypeInfo* superType) { // TODO: consider forbidding the class being abstract. - const TypeInfo* result = createTypeInfo(superType, addedInterfaces, vtable, methodTable, interfaceVTables, itableEqualsSuper); + const TypeInfo* result = createTypeInfo(superType, addedInterfaces, vtable, methodTable, + interfaceVTables, superITable, superITableSize, itableEqualsSuper); // TODO: it will probably never be requested, since such a class can't be instantiated in Kotlin. result->writableInfo_->objCExport.objCClass = clazz;