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.
2017-07-13 22:53:30 +03:00
parent 9f0810f723
commit f2b5f37b22
14 changed files with 200 additions and 66 deletions
@@ -474,9 +474,16 @@ abstract class InlineCodegen<out T: BaseExpressionCodegen>(
                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
@@ -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)
+}
@@ -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))
-        }
-    }
-}
@@ -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");
@@ -93,7 +93,6 @@ public interface ErrorsJvm {
    DiagnosticFactory0<PsiElement> INTERFACE_STATIC_METHOD_CALL_FROM_JAVA6_TARGET = DiagnosticFactory0.create(WARNING);

    DiagnosticFactory2<PsiElement, String, String> INLINE_FROM_HIGHER_PLATFORM = DiagnosticFactory2.create(ERROR);
-    DiagnosticFactory0<PsiElement> OBSOLETE_SUSPEND_INLINE_FUNCTIONS_ABI = DiagnosticFactory0.create(ERROR);

    DiagnosticFactory1<PsiElement, String> JAVA_MODULE_DOES_NOT_DEPEND_ON_MODULE = DiagnosticFactory1.create(ERROR);
    DiagnosticFactory0<PsiElement> JAVA_MODULE_DOES_NOT_READ_UNNAMED_MODULE = DiagnosticFactory0.create(ERROR);
@@ -52,8 +52,7 @@ object JvmPlatformConfigurator : PlatformConfigurator(
                UnsupportedSyntheticCallableReferenceChecker(),
                SuperCallWithDefaultArgumentsChecker(),
                ProtectedSyntheticExtensionCallChecker,
-                ReifiedTypeParameterSubstitutionChecker(),
-                ObsoleteABIForInlineSuspendFunctionCallChecker
+                ReifiedTypeParameterSubstitutionChecker()
        ),

        additionalTypeCheckers = listOf(
@@ -1,4 +0,0 @@
-package library
-
-inline suspend fun foo() {}
-suspend fun bar() {}
@@ -1,6 +0,0 @@
-import library.*
-
-suspend fun test() {
-    foo()
-    bar()
-}
@@ -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
@@ -0,0 +1,31 @@
+package library
+
+import kotlin.coroutines.experimental.*
+
+var continuation: Continuation<Unit>? = 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<Unit> { continuation = it }
+}
+
+suspend fun bar() {
+    append("(bar)")
+    return suspendCoroutine<Unit> { continuation = it }
+}
+
+fun resumeLibrary() {
+    continuation?.let {
+        continuation = null
+        it.resume(Unit)
+    }
+}
@@ -0,0 +1 @@
+OK
@@ -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<Unit> {
+        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<String> {
+    val result = runBlockingLibrary {
+        test()
+    }
+    return arrayOf("[foo](foo)(block)[bar](bar)[test]:resumes=2", result)
+}
+
@@ -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)
@@ -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<String>
+        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<ByteArray, Int> {
+            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<out String>?): 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<AbstractInsnNode> =
+            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())