From d88d1d6189d2b12f831c4cce06ec7fa2e8c31a76 Mon Sep 17 00:00:00 2001 From: Denis Zharkov Date: Thu, 15 Jun 2017 15:40:28 +0300 Subject: [PATCH] Do not use non-existing class for suspend markers As they remain in inline functions now proguard emits a warning about them, even though inline suspend functions are effectively inline-only and these markers can't be executed at runtime #KT-18702 Fixed --- .../kotlin/codegen/ExpressionCodegen.java | 10 +---- .../CoroutineTransformerMethodVisitor.kt | 44 ++++++++++++------- .../coroutines/coroutineCodegenUtil.kt | 5 --- .../codegen/inline/inlineCodegenUtils.kt | 36 ++++++++++----- 4 files changed, 55 insertions(+), 40 deletions(-) diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/ExpressionCodegen.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/ExpressionCodegen.java index d4e8956f7ff..56a33d068cb 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/ExpressionCodegen.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/ExpressionCodegen.java @@ -2238,19 +2238,13 @@ public class ExpressionCodegen extends KtVisitor impleme } if (isSuspendCall) { - v.invokestatic( - CoroutineCodegenUtilKt.COROUTINE_MARKER_OWNER, - CoroutineCodegenUtilKt.BEFORE_SUSPENSION_POINT_MARKER_NAME, - "()V", false - ); + addSuspendMarker(v, true); } callGenerator.genCall(callableMethod, resolvedCall, defaultMaskWasGenerated, this); if (isSuspendCall) { - v.invokestatic( - CoroutineCodegenUtilKt.COROUTINE_MARKER_OWNER, - CoroutineCodegenUtilKt.AFTER_SUSPENSION_POINT_MARKER_NAME, "()V", false); + addSuspendMarker(v, false); addInlineMarker(v, false); } diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/CoroutineTransformerMethodVisitor.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/CoroutineTransformerMethodVisitor.kt index 0f3241b5414..38f62e34c9b 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/CoroutineTransformerMethodVisitor.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/CoroutineTransformerMethodVisitor.kt @@ -23,6 +23,8 @@ import org.jetbrains.kotlin.codegen.ClassBuilder import org.jetbrains.kotlin.codegen.StackValue import org.jetbrains.kotlin.codegen.TransformationMethodVisitor import org.jetbrains.kotlin.codegen.inline.MaxStackFrameSizeAndLocalsCalculator +import org.jetbrains.kotlin.codegen.inline.isAfterSuspendMarker +import org.jetbrains.kotlin.codegen.inline.isBeforeSuspendMarker import org.jetbrains.kotlin.codegen.inline.isInlineMarker import org.jetbrains.kotlin.codegen.optimization.DeadCodeEliminationMethodTransformer import org.jetbrains.kotlin.codegen.optimization.common.* @@ -302,7 +304,7 @@ class CoroutineTransformerMethodVisitor( // It doesn't introduce an additional state to the corresponding coroutine's FSM. suspensionPoints.forEach { if (dceResult.isAlive(it.suspensionCallBegin) && dceResult.isRemoved(it.suspensionCallEnd)) { - methodNode.instructions.remove(it.suspensionCallBegin) + it.removeBeforeSuspendMarker(methodNode) } } @@ -311,17 +313,15 @@ class CoroutineTransformerMethodVisitor( private fun collectSuspensionPoints(methodNode: MethodNode): MutableList { val suspensionPoints = mutableListOf() - val beforeSuspensionPointMarkerStack = Stack() + val beforeSuspensionPointMarkerStack = Stack() for (methodInsn in methodNode.instructions.toArray().filterIsInstance()) { - if (methodInsn.owner != COROUTINE_MARKER_OWNER) continue - - when (methodInsn.name) { - BEFORE_SUSPENSION_POINT_MARKER_NAME -> { - beforeSuspensionPointMarkerStack.add(methodInsn) + when { + isBeforeSuspendMarker(methodInsn) -> { + beforeSuspensionPointMarkerStack.add(methodInsn.previous) } - AFTER_SUSPENSION_POINT_MARKER_NAME -> { + isAfterSuspendMarker(methodInsn) -> { suspensionPoints.add(SuspensionPoint(beforeSuspensionPointMarkerStack.pop(), methodInsn)) } } @@ -335,10 +335,8 @@ class CoroutineTransformerMethodVisitor( private fun dropSuspensionMarkers(methodNode: MethodNode, suspensionPoints: List) { // Drop markers suspensionPoints.forEach { - // before-marker - methodNode.instructions.remove(it.suspensionCallBegin) - // after-marker - methodNode.instructions.remove(it.suspensionCallEnd) + it.removeBeforeSuspendMarker(methodNode) + it.removeAfterSuspendMarker(methodNode) } } @@ -615,18 +613,30 @@ private fun Type.normalize() = /** * Suspension call may consists of several instructions: - * INVOKESTATIC beforeSuspensionMarker + * ICONST_0 + * INVOKESTATIC InlineMarker.mark() * INVOKEVIRTUAL suspensionMethod()Ljava/lang/Object; // actually it could be some inline method instead of plain call * CHECKCAST Type - * INVOKESTATIC afterSuspensionMarker + * ICONST_1 + * INVOKESTATIC InlineMarker.mark() */ private class SuspensionPoint( - // INVOKESTATIC beforeSuspensionMarker + // ICONST_0 val suspensionCallBegin: AbstractInsnNode, - // INVOKESTATIC afterSuspensionMarker + // INVOKESTATIC InlineMarker.mark() val suspensionCallEnd: AbstractInsnNode ) { lateinit var tryCatchBlocksContinuationLabel: LabelNode + + fun removeBeforeSuspendMarker(methodNode: MethodNode) { + methodNode.instructions.remove(suspensionCallBegin.next) + methodNode.instructions.remove(suspensionCallBegin) + } + + fun removeAfterSuspendMarker(methodNode: MethodNode) { + methodNode.instructions.remove(suspensionCallEnd.previous) + methodNode.instructions.remove(suspensionCallEnd) + } } private fun getLastParameterIndex(desc: String, access: Int) = @@ -747,4 +757,4 @@ private fun AbstractInsnNode?.isInvisibleInDebugVarInsn(methodNode: MethodNode): } private val SAFE_OPCODES = - ((Opcodes.DUP..Opcodes.DUP2_X2) + Opcodes.POP + Opcodes.POP2 + (Opcodes.IFEQ..Opcodes.GOTO)).toSet() \ No newline at end of file + ((Opcodes.DUP..Opcodes.DUP2_X2) + Opcodes.POP + Opcodes.POP2 + (Opcodes.IFEQ..Opcodes.GOTO)).toSet() diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/coroutineCodegenUtil.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/coroutineCodegenUtil.kt index 07d64165d5e..e1aa95fb744 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/coroutineCodegenUtil.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/coroutineCodegenUtil.kt @@ -52,11 +52,6 @@ import org.jetbrains.org.objectweb.asm.commons.InstructionAdapter import org.jetbrains.org.objectweb.asm.commons.Method import org.jetbrains.org.objectweb.asm.tree.MethodNode -// These classes do not actually exist at runtime -const val COROUTINE_MARKER_OWNER = "kotlin/coroutines/Markers" -const val BEFORE_SUSPENSION_POINT_MARKER_NAME = "beforeSuspensionPoint" -const val AFTER_SUSPENSION_POINT_MARKER_NAME = "afterSuspensionPoint" - const val COROUTINE_LABEL_FIELD_NAME = "label" const val SUSPEND_FUNCTION_CREATE_METHOD_NAME = "create" const val DO_RESUME_METHOD_NAME = "doResume" diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/inlineCodegenUtils.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/inlineCodegenUtils.kt index 31e2a03ac4c..b6a460c1134 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/inlineCodegenUtils.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/inlineCodegenUtils.kt @@ -22,18 +22,19 @@ import org.jetbrains.kotlin.codegen.AsmUtil import org.jetbrains.kotlin.codegen.BaseExpressionCodegen import org.jetbrains.kotlin.codegen.ExpressionCodegen import org.jetbrains.kotlin.codegen.JvmCodegenUtil +import org.jetbrains.kotlin.codegen.`when`.WhenByEnumsMapping import org.jetbrains.kotlin.codegen.binding.CodegenBinding import org.jetbrains.kotlin.codegen.context.CodegenContext import org.jetbrains.kotlin.codegen.context.CodegenContextUtil import org.jetbrains.kotlin.codegen.context.InlineLambdaContext import org.jetbrains.kotlin.codegen.context.MethodContext -import org.jetbrains.kotlin.codegen.intrinsics.* +import org.jetbrains.kotlin.codegen.intrinsics.classId +import org.jetbrains.kotlin.codegen.optimization.common.intConstant import org.jetbrains.kotlin.codegen.state.GenerationState import org.jetbrains.kotlin.codegen.state.KotlinTypeMapper -import org.jetbrains.kotlin.codegen.`when`.WhenByEnumsMapping import org.jetbrains.kotlin.descriptors.* -import org.jetbrains.kotlin.fileClasses.* import org.jetbrains.kotlin.fileClasses.JvmFileClassesProvider +import org.jetbrains.kotlin.fileClasses.getFileClassType import org.jetbrains.kotlin.load.java.JvmAbi import org.jetbrains.kotlin.load.kotlin.KotlinJvmBinaryPackageSourceElement import org.jetbrains.kotlin.load.kotlin.KotlinJvmBinarySourceElement @@ -44,7 +45,11 @@ import org.jetbrains.kotlin.name.ClassId import org.jetbrains.kotlin.name.Name import org.jetbrains.kotlin.resolve.DescriptorToSourceUtils import org.jetbrains.kotlin.resolve.jvm.AsmTypes +import org.jetbrains.kotlin.resolve.jvm.AsmTypes.ENUM_TYPE +import org.jetbrains.kotlin.resolve.jvm.AsmTypes.JAVA_CLASS_TYPE import org.jetbrains.kotlin.resolve.jvm.JvmClassName +import org.jetbrains.kotlin.resolve.source.PsiSourceElement +import org.jetbrains.kotlin.serialization.deserialization.descriptors.DeserializedCallableMemberDescriptor import org.jetbrains.kotlin.types.KotlinType import org.jetbrains.kotlin.util.OperatorNameConventions import org.jetbrains.org.objectweb.asm.* @@ -53,15 +58,9 @@ import org.jetbrains.org.objectweb.asm.tree.* import org.jetbrains.org.objectweb.asm.util.Printer import org.jetbrains.org.objectweb.asm.util.Textifier import org.jetbrains.org.objectweb.asm.util.TraceMethodVisitor - import java.io.PrintWriter import java.io.StringWriter -import org.jetbrains.kotlin.resolve.jvm.AsmTypes.ENUM_TYPE -import org.jetbrains.kotlin.resolve.jvm.AsmTypes.JAVA_CLASS_TYPE -import org.jetbrains.kotlin.resolve.source.PsiSourceElement -import org.jetbrains.kotlin.serialization.deserialization.descriptors.DeserializedCallableMemberDescriptor - const val GENERATE_SMAP = true const val API = Opcodes.ASM5 @@ -88,6 +87,8 @@ private const val INLINE_MARKER_AFTER_METHOD_NAME = "afterInlineCall" private const val INLINE_MARKER_FINALLY_START = "finallyStart" private const val INLINE_MARKER_FINALLY_END = "finallyEnd" +private const val INLINE_MARKER_BEFORE_SUSPEND_ID = 0 +private const val INLINE_MARKER_AFTER_SUSPEND_ID = 1 internal fun getMethodNode( classData: ByteArray, @@ -430,6 +431,21 @@ internal fun addInlineMarker(v: InstructionAdapter, isStartNotEnd: Boolean) { ) } +internal fun addSuspendMarker(v: InstructionAdapter, isStartNotEnd: Boolean) { + v.iconst(if (isStartNotEnd) INLINE_MARKER_BEFORE_SUSPEND_ID else INLINE_MARKER_AFTER_SUSPEND_ID) + v.visitMethodInsn( + Opcodes.INVOKESTATIC, INLINE_MARKER_CLASS_NAME, + "mark", + "(I)V", false + ) +} + +internal fun isBeforeSuspendMarker(insn: AbstractInsnNode) = isSuspendMarker(insn, INLINE_MARKER_BEFORE_SUSPEND_ID) +internal fun isAfterSuspendMarker(insn: AbstractInsnNode) = isSuspendMarker(insn, INLINE_MARKER_AFTER_SUSPEND_ID) + +private fun isSuspendMarker(insn: AbstractInsnNode, id: Int) = + isInlineMarker(insn, "mark") && insn.previous.intConstant == id + internal fun isInlineMarker(insn: AbstractInsnNode): Boolean { return isInlineMarker(insn, null) } @@ -581,4 +597,4 @@ class InlineOnlySmapSkipper(codegen: BaseExpressionCodegen) { mv.visitLineNumber(callLineNumber, label) } } -} \ No newline at end of file +}