From f2b5f37b22db1a77e158e9b47ad221c0b572d02c Mon Sep 17 00:00:00 2001 From: Roman Elizarov Date: Thu, 13 Jul 2017 22:53:30 +0300 Subject: [PATCH] Support inline suspend functions built with compiler version less than 1.1.4/1.2-M1 The error message is removed and is replaced with a code that adapts inline suspend functions produced by the old compiler with the suspension markers that new compiler expects. --- .../kotlin/codegen/inline/InlineCodegen.kt | 11 ++- .../codegen/inline/LegacyInlineSuspendUtil.kt | 54 ++++++++++++++ ...eABIForInlineSuspendFunctionCallChecker.kt | 38 ---------- .../diagnostics/DefaultErrorMessagesJvm.java | 1 - .../resolve/jvm/diagnostics/ErrorsJvm.java | 1 - .../jvm/platform/JvmPlatformConfigurator.kt | 3 +- .../obsoleteInlineSuspend/A.kt | 4 -- .../obsoleteInlineSuspend/B.kt | 6 -- .../obsoleteInlineSuspend/output.txt | 4 -- .../obsoleteInlineSuspend/library/a.kt | 31 ++++++++ .../obsoleteInlineSuspend/output.txt | 1 + .../obsoleteInlineSuspend/source.kt | 36 ++++++++++ .../kotlin/cli/WrongBytecodeVersionTest.kt | 4 -- .../CompileKotlinAgainstCustomBinariesTest.kt | 72 +++++++++++++++++-- 14 files changed, 200 insertions(+), 66 deletions(-) create mode 100644 compiler/backend/src/org/jetbrains/kotlin/codegen/inline/LegacyInlineSuspendUtil.kt delete mode 100644 compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/ObsoleteABIForInlineSuspendFunctionCallChecker.kt delete mode 100644 compiler/testData/bytecodeVersion/obsoleteInlineSuspend/A.kt delete mode 100644 compiler/testData/bytecodeVersion/obsoleteInlineSuspend/B.kt delete mode 100644 compiler/testData/bytecodeVersion/obsoleteInlineSuspend/output.txt create mode 100644 compiler/testData/compileKotlinAgainstCustomBinaries/obsoleteInlineSuspend/library/a.kt create mode 100644 compiler/testData/compileKotlinAgainstCustomBinaries/obsoleteInlineSuspend/output.txt create mode 100644 compiler/testData/compileKotlinAgainstCustomBinaries/obsoleteInlineSuspend/source.kt diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/InlineCodegen.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/InlineCodegen.kt index dcf94f12a9b..f54bbd90b15 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/InlineCodegen.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/InlineCodegen.kt @@ -474,9 +474,16 @@ abstract class InlineCodegen( throw IllegalStateException("Couldn't find declaration file for " + containerId) } - return getMethodNode(bytes, asmMethod.name, asmMethod.descriptor, containerId.asString()) - } + val methodNode = getMethodNode(bytes, asmMethod.name, asmMethod.descriptor, containerId.asString()) ?: return null + // KLUDGE: Inline suspend function built with compiler version less than 1.1.4/1.2-M1 did not contain proper + // before/after suspension point marks, so we detect those functions here and insert the corresponding marks + if (isLegacySuspendInlineFunction(callableDescriptor)) { + insertLegacySuspendInlineMarks(methodNode.node) + } + + return methodNode + } private fun isBuiltInArrayIntrinsic(callableDescriptor: CallableMemberDescriptor): Boolean { if (callableDescriptor is FictitiousArrayConstructor) return true diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/LegacyInlineSuspendUtil.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/LegacyInlineSuspendUtil.kt new file mode 100644 index 00000000000..1fd3c25d74c --- /dev/null +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/LegacyInlineSuspendUtil.kt @@ -0,0 +1,54 @@ +/* + * Copyright 2010-2017 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.jetbrains.kotlin.codegen.inline + +import jdk.internal.org.objectweb.asm.Opcodes +import org.jetbrains.kotlin.descriptors.CallableMemberDescriptor +import org.jetbrains.kotlin.descriptors.FunctionDescriptor +import org.jetbrains.kotlin.load.kotlin.getContainingKotlinJvmBinaryClass +import org.jetbrains.org.objectweb.asm.commons.InstructionAdapter +import org.jetbrains.org.objectweb.asm.tree.AbstractInsnNode +import org.jetbrains.org.objectweb.asm.tree.MethodNode + +// KLUDGE: Inline suspend function built with compiler version less than 1.1.4/1.2-M1 did not contain proper +// before/after suspension point marks, so we detect those functions here and insert the corresponding marks + +fun insertLegacySuspendInlineMarks(node: MethodNode) { + with (node.instructions) { + // look for return instruction before the end and insert "afterSuspendMarker" there + insertBefore(findLastReturn(last) ?: return, produceSuspendMarker(false).instructions) + // insert "beforeSuspendMarker" at the beginning + insertBefore(first, produceSuspendMarker(true).instructions) + } + node.maxStack = node.maxStack.coerceAtLeast(2) // min stack need for suspend marker before return +} + +fun findLastReturn(node: AbstractInsnNode?): AbstractInsnNode? { + var cur = node + while (cur != null && cur.opcode != Opcodes.ARETURN) cur = cur.previous + return cur +} + +private fun produceSuspendMarker(isStartNotEnd: Boolean): MethodNode = + MethodNode().also { addSuspendMarker(InstructionAdapter(it), isStartNotEnd) } + +fun isLegacySuspendInlineFunction(descriptor: CallableMemberDescriptor): Boolean { + if (descriptor !is FunctionDescriptor) return false + if (!descriptor.isSuspend || !descriptor.isInline) return false + val jvmBytecodeVersion = descriptor.getContainingKotlinJvmBinaryClass()?.classHeader?.bytecodeVersion ?: return false + return !jvmBytecodeVersion.isAtLeast(1, 0, 2) +} diff --git a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/ObsoleteABIForInlineSuspendFunctionCallChecker.kt b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/ObsoleteABIForInlineSuspendFunctionCallChecker.kt deleted file mode 100644 index 6001abda9b1..00000000000 --- a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/ObsoleteABIForInlineSuspendFunctionCallChecker.kt +++ /dev/null @@ -1,38 +0,0 @@ -/* - * Copyright 2010-2017 JetBrains s.r.o. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.jetbrains.kotlin.resolve.jvm.checkers - -import com.intellij.psi.PsiElement -import org.jetbrains.kotlin.descriptors.FunctionDescriptor -import org.jetbrains.kotlin.load.kotlin.getContainingKotlinJvmBinaryClass -import org.jetbrains.kotlin.resolve.calls.checkers.CallChecker -import org.jetbrains.kotlin.resolve.calls.checkers.CallCheckerContext -import org.jetbrains.kotlin.resolve.calls.model.ResolvedCall -import org.jetbrains.kotlin.resolve.jvm.diagnostics.ErrorsJvm - -object ObsoleteABIForInlineSuspendFunctionCallChecker : CallChecker { - override fun check(resolvedCall: ResolvedCall<*>, reportOn: PsiElement, context: CallCheckerContext) { - val candidateDescriptor = resolvedCall.candidateDescriptor as? FunctionDescriptor ?: return - if (!candidateDescriptor.isSuspend || !candidateDescriptor.isInline) return - - val jvmBytecodeVersion = candidateDescriptor.getContainingKotlinJvmBinaryClass()?.classHeader?.bytecodeVersion ?: return - - if (!jvmBytecodeVersion.isAtLeast(1, 0, 2)) { - context.trace.report(ErrorsJvm.OBSOLETE_SUSPEND_INLINE_FUNCTIONS_ABI.on(reportOn)) - } - } -} diff --git a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/DefaultErrorMessagesJvm.java b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/DefaultErrorMessagesJvm.java index ce69bc3ac01..f0b67ec5ee7 100644 --- a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/DefaultErrorMessagesJvm.java +++ b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/DefaultErrorMessagesJvm.java @@ -112,7 +112,6 @@ public class DefaultErrorMessagesJvm implements DefaultErrorMessages.Extension { MAP.put(INTERFACE_STATIC_METHOD_CALL_FROM_JAVA6_TARGET, "Calls to static methods in Java interfaces are deprecated in JVM target 1.6. Recompile with '-jvm-target 1.8'"); MAP.put(INLINE_FROM_HIGHER_PLATFORM, "Cannot inline bytecode built with {0} into bytecode that is being built with {1}. Please specify proper ''-jvm-target'' option", STRING, STRING); - MAP.put(OBSOLETE_SUSPEND_INLINE_FUNCTIONS_ABI, "Cannot inline suspend function built with compiler version less than 1.1.4/1.2-M1"); MAP.put(JAVA_MODULE_DOES_NOT_DEPEND_ON_MODULE, "Symbol is declared in module ''{0}'' which current module does not depend on", STRING); MAP.put(JAVA_MODULE_DOES_NOT_READ_UNNAMED_MODULE, "Symbol is declared in unnamed module which is not read by current module"); diff --git a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/ErrorsJvm.java b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/ErrorsJvm.java index 030bb7beae5..334d108a254 100644 --- a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/ErrorsJvm.java +++ b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/ErrorsJvm.java @@ -93,7 +93,6 @@ public interface ErrorsJvm { DiagnosticFactory0 INTERFACE_STATIC_METHOD_CALL_FROM_JAVA6_TARGET = DiagnosticFactory0.create(WARNING); DiagnosticFactory2 INLINE_FROM_HIGHER_PLATFORM = DiagnosticFactory2.create(ERROR); - DiagnosticFactory0 OBSOLETE_SUSPEND_INLINE_FUNCTIONS_ABI = DiagnosticFactory0.create(ERROR); DiagnosticFactory1 JAVA_MODULE_DOES_NOT_DEPEND_ON_MODULE = DiagnosticFactory1.create(ERROR); DiagnosticFactory0 JAVA_MODULE_DOES_NOT_READ_UNNAMED_MODULE = DiagnosticFactory0.create(ERROR); diff --git a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/platform/JvmPlatformConfigurator.kt b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/platform/JvmPlatformConfigurator.kt index 91bb1fead10..1aff0040597 100644 --- a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/platform/JvmPlatformConfigurator.kt +++ b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/platform/JvmPlatformConfigurator.kt @@ -52,8 +52,7 @@ object JvmPlatformConfigurator : PlatformConfigurator( UnsupportedSyntheticCallableReferenceChecker(), SuperCallWithDefaultArgumentsChecker(), ProtectedSyntheticExtensionCallChecker, - ReifiedTypeParameterSubstitutionChecker(), - ObsoleteABIForInlineSuspendFunctionCallChecker + ReifiedTypeParameterSubstitutionChecker() ), additionalTypeCheckers = listOf( diff --git a/compiler/testData/bytecodeVersion/obsoleteInlineSuspend/A.kt b/compiler/testData/bytecodeVersion/obsoleteInlineSuspend/A.kt deleted file mode 100644 index 56b90c2015d..00000000000 --- a/compiler/testData/bytecodeVersion/obsoleteInlineSuspend/A.kt +++ /dev/null @@ -1,4 +0,0 @@ -package library - -inline suspend fun foo() {} -suspend fun bar() {} diff --git a/compiler/testData/bytecodeVersion/obsoleteInlineSuspend/B.kt b/compiler/testData/bytecodeVersion/obsoleteInlineSuspend/B.kt deleted file mode 100644 index 74ef9b1aad0..00000000000 --- a/compiler/testData/bytecodeVersion/obsoleteInlineSuspend/B.kt +++ /dev/null @@ -1,6 +0,0 @@ -import library.* - -suspend fun test() { - foo() - bar() -} diff --git a/compiler/testData/bytecodeVersion/obsoleteInlineSuspend/output.txt b/compiler/testData/bytecodeVersion/obsoleteInlineSuspend/output.txt deleted file mode 100644 index ee9f65b96a0..00000000000 --- a/compiler/testData/bytecodeVersion/obsoleteInlineSuspend/output.txt +++ /dev/null @@ -1,4 +0,0 @@ -compiler/testData/bytecodeVersion/obsoleteInlineSuspend/B.kt:4:5: error: cannot inline suspend function built with compiler version less than 1.1.4/1.2-M1 - foo() - ^ -COMPILATION_ERROR diff --git a/compiler/testData/compileKotlinAgainstCustomBinaries/obsoleteInlineSuspend/library/a.kt b/compiler/testData/compileKotlinAgainstCustomBinaries/obsoleteInlineSuspend/library/a.kt new file mode 100644 index 00000000000..0c55e519627 --- /dev/null +++ b/compiler/testData/compileKotlinAgainstCustomBinaries/obsoleteInlineSuspend/library/a.kt @@ -0,0 +1,31 @@ +package library + +import kotlin.coroutines.experimental.* + +var continuation: Continuation? = null +val sb = java.lang.StringBuilder() + +fun append(s: String) { sb.append(s) } + +val libraryResult: String get() = sb.toString() + +// this is an inline tail-suspend function that should work properly despite being compiler by +// compiler before 1.1.4 version that did not include suspension marks into bytecode. +// In order to test that it works properly it shall actually suspend during its execution +inline suspend fun foo(block: () -> Unit) { + append("(foo)") + block() + return suspendCoroutine { continuation = it } +} + +suspend fun bar() { + append("(bar)") + return suspendCoroutine { continuation = it } +} + +fun resumeLibrary() { + continuation?.let { + continuation = null + it.resume(Unit) + } +} diff --git a/compiler/testData/compileKotlinAgainstCustomBinaries/obsoleteInlineSuspend/output.txt b/compiler/testData/compileKotlinAgainstCustomBinaries/obsoleteInlineSuspend/output.txt new file mode 100644 index 00000000000..a0aba9318ad --- /dev/null +++ b/compiler/testData/compileKotlinAgainstCustomBinaries/obsoleteInlineSuspend/output.txt @@ -0,0 +1 @@ +OK \ No newline at end of file diff --git a/compiler/testData/compileKotlinAgainstCustomBinaries/obsoleteInlineSuspend/source.kt b/compiler/testData/compileKotlinAgainstCustomBinaries/obsoleteInlineSuspend/source.kt new file mode 100644 index 00000000000..02c640eeb76 --- /dev/null +++ b/compiler/testData/compileKotlinAgainstCustomBinaries/obsoleteInlineSuspend/source.kt @@ -0,0 +1,36 @@ +import library.* +import kotlin.coroutines.experimental.* + +suspend fun test() { + append("[foo]") + foo { // we are inlining foo here + append("(block)") + } + append("[bar]") + bar() // and invoking suspending function bar + append("[test]") +} + +fun runBlockingLibrary(block: suspend () -> Unit): String { + var done = false + block.startCoroutine(object : Continuation { + override val context: CoroutineContext = EmptyCoroutineContext + override fun resume(value: Unit) { done = true } + override fun resumeWithException(exception: Throwable) { throw exception } + }) + var resumeCounter = 0 + while (!done) { + resumeCounter++ + resumeLibrary() + } + return "$libraryResult:resumes=$resumeCounter" +} + +// Retruns array of expected and received string +fun run(): Array { + val result = runBlockingLibrary { + test() + } + return arrayOf("[foo](foo)(block)[bar](bar)[test]:resumes=2", result) +} + diff --git a/compiler/tests/org/jetbrains/kotlin/cli/WrongBytecodeVersionTest.kt b/compiler/tests/org/jetbrains/kotlin/cli/WrongBytecodeVersionTest.kt index 946a8e0e81f..1f31d3346f9 100644 --- a/compiler/tests/org/jetbrains/kotlin/cli/WrongBytecodeVersionTest.kt +++ b/compiler/tests/org/jetbrains/kotlin/cli/WrongBytecodeVersionTest.kt @@ -64,10 +64,6 @@ class WrongBytecodeVersionTest : KtUsefulTestCase() { doTest("/bytecodeVersion/simple") } - fun testObsoleteInlineSuspend() { - doTest("/bytecodeVersion/obsoleteInlineSuspend", intArrayOf(1, 0, 1)) - } - companion object { fun transformMetadataInClassFile(bytes: ByteArray, transform: (fieldName: String, value: Any?) -> Any?): ByteArray { val writer = ClassWriter(0) diff --git a/compiler/tests/org/jetbrains/kotlin/jvm/compiler/CompileKotlinAgainstCustomBinariesTest.kt b/compiler/tests/org/jetbrains/kotlin/jvm/compiler/CompileKotlinAgainstCustomBinariesTest.kt index 92e7dff3aa1..28c45dbee28 100644 --- a/compiler/tests/org/jetbrains/kotlin/jvm/compiler/CompileKotlinAgainstCustomBinariesTest.kt +++ b/compiler/tests/org/jetbrains/kotlin/jvm/compiler/CompileKotlinAgainstCustomBinariesTest.kt @@ -26,7 +26,11 @@ import org.jetbrains.kotlin.cli.js.K2JSCompiler import org.jetbrains.kotlin.cli.jvm.K2JVMCompiler import org.jetbrains.kotlin.cli.jvm.compiler.EnvironmentConfigFiles import org.jetbrains.kotlin.cli.jvm.compiler.KotlinCoreEnvironment +import org.jetbrains.kotlin.codegen.forTestCompile.ForTestCompileRuntime import org.jetbrains.kotlin.codegen.inline.GENERATE_SMAP +import org.jetbrains.kotlin.codegen.inline.remove +import org.jetbrains.kotlin.codegen.optimization.common.asSequence +import org.jetbrains.kotlin.codegen.optimization.common.intConstant import org.jetbrains.kotlin.config.KotlinCompilerVersion.TEST_IS_PRE_RELEASE_SYSTEM_PROPERTY import org.jetbrains.kotlin.descriptors.DeclarationDescriptor import org.jetbrains.kotlin.descriptors.PackageViewDescriptor @@ -41,11 +45,12 @@ import org.jetbrains.kotlin.test.MockLibraryUtil import org.jetbrains.kotlin.test.TestJdkKind import org.jetbrains.kotlin.test.util.RecursiveDescriptorComparator.validateAndCompareDescriptorWithFile import org.jetbrains.kotlin.utils.JsMetadataVersion -import org.jetbrains.org.objectweb.asm.ClassReader -import org.jetbrains.org.objectweb.asm.ClassVisitor -import org.jetbrains.org.objectweb.asm.ClassWriter -import org.jetbrains.org.objectweb.asm.Opcodes +import org.jetbrains.org.objectweb.asm.* +import org.jetbrains.org.objectweb.asm.tree.AbstractInsnNode +import org.jetbrains.org.objectweb.asm.tree.MethodInsnNode +import org.jetbrains.org.objectweb.asm.tree.MethodNode import java.io.File +import java.net.URLClassLoader import java.util.jar.JarEntry import java.util.jar.JarFile import java.util.zip.ZipOutputStream @@ -384,7 +389,66 @@ class CompileKotlinAgainstCustomBinariesTest : AbstractKotlinCompilerIntegration compileKotlin("source.kt", tmpdir, listOf(library)) } + fun testObsoleteInlineSuspend() { + val version = intArrayOf(1, 0, 1) // legacy coroutines metadata + val options = listOf("-Xcoroutines=enable") + val library = transformJar( + compileLibrary("library", additionalOptions = options), + { _, bytes -> + val (resultBytes, removedCounter) = stripSuspensionMarksToImitateLegacyCompiler( + WrongBytecodeVersionTest.transformMetadataInClassFile(bytes) { name, _ -> + if (name == JvmAnnotationNames.BYTECODE_VERSION_FIELD_NAME) version else null + }) + // we expect 4 instructions to be removed in this test library + assertEquals(4, removedCounter) + resultBytes + }) + compileKotlin("source.kt", tmpdir, listOf(library), K2JVMCompiler(), + additionalOptions = options) + val classLoader = URLClassLoader(arrayOf(library.toURI().toURL(), tmpdir.toURI().toURL()), + ForTestCompileRuntime.runtimeJarClassLoader()) + @Suppress("UNCHECKED_CAST") + val result = classLoader + .loadClass("SourceKt") + .getDeclaredMethod("run") + .invoke(null) as Array + assertEquals(result[0], result[1]) + } + companion object { + // compiler before 1.1.4 version did not include suspension marks into bytecode. + private fun stripSuspensionMarksToImitateLegacyCompiler(bytes: ByteArray): Pair { + val writer = ClassWriter(0) + var removedCounter = 0 + ClassReader(bytes).accept(object : ClassVisitor(Opcodes.ASM5, writer) { + override fun visitMethod(access: Int, name: String?, desc: String?, signature: String?, exceptions: Array?): MethodVisitor { + val superMV = super.visitMethod(access, name, desc, signature, exceptions) + return object : MethodNode(Opcodes.ASM5, access, name, desc, signature, exceptions) { + override fun visitEnd() { + val removeList = instructions.asSequence() + .flatMap { suspendMarkerInsns(it).asSequence() }.toList() + remove(removeList) + removedCounter += removeList.size + accept(superMV) + } + } + } + }, 0) + return writer.toByteArray() to removedCounter + } + + // KLUDGE: here is a simplified copy of compiler's logic for suspend markers + + private fun suspendMarkerInsns(insn: AbstractInsnNode): List = + if (insn is MethodInsnNode + && insn.opcode == Opcodes.INVOKESTATIC + && insn.owner == "kotlin/jvm/internal/InlineMarker" + && insn.name == "mark" + && insn.previous.intConstant in 0..1) listOf(insn, insn.previous) + else emptyList() + + // ----- + private fun copyJarFileWithoutEntry(jarPath: File, vararg entriesToDelete: String): File = transformJar(jarPath, { _, bytes -> bytes }, entriesToDelete.toSet())