From 6d19eb1853ec39d0ccbc832beb095a60a12269a3 Mon Sep 17 00:00:00 2001 From: pyos Date: Fri, 24 May 2019 13:57:42 +0200 Subject: [PATCH] JVM_IR: sidestep defective getMethodAsmFlags when inlining lambdas It uses isStaticMethod to determine whether to set ACC_STATIC, which is not correct (see PR #2341). This results in using incorrectly typed opcodes (as all arguments are shifted by 1) when modifying the inlined lambda's bytecode. For example, in the test added by this commit, these opcodes are inserted to spill the stack into locals before calling another inline function. Because getMethodAsmFlags is used by the non-IR backend (see PR #2341 again for why changing stuff might not be a good idea), the proposed solution is to ditch it completely and override generateLambdaBody in IrExpressionLambdaImpl to use FunctionCodegen's IR-based flag computation logic. --- .../kotlin/codegen/inline/LambdaInfo.kt | 16 +------ .../codegen/inline/SourceCompilerForInline.kt | 24 +++++----- .../backend/jvm/codegen/ExpressionCodegen.kt | 1 - .../backend/jvm/codegen/FunctionCodegen.kt | 3 +- .../jvm/codegen/IrSourceCompilerForInline.kt | 45 ++++++------------- .../simple/inlineCallInInlineLambda.kt | 16 +++++++ .../BlackBoxInlineCodegenTestGenerated.java | 5 +++ ...otlinAgainstInlineKotlinTestGenerated.java | 5 +++ .../IrBlackBoxInlineCodegenTestGenerated.java | 5 +++ 9 files changed, 59 insertions(+), 61 deletions(-) create mode 100644 compiler/testData/codegen/boxInline/simple/inlineCallInInlineLambda.kt diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/LambdaInfo.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/LambdaInfo.kt index 0c79e5efab0..fc8e74ca07c 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/LambdaInfo.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/LambdaInfo.kt @@ -189,22 +189,8 @@ internal fun Type.boxReceiverForBoundReference(kotlinType: KotlinType, typeMappe AsmUtil.boxType(this, kotlinType, typeMapper) abstract class ExpressionLambda(protected val typeMapper: KotlinTypeMapper, isCrossInline: Boolean) : LambdaInfo(isCrossInline) { - override fun generateLambdaBody(sourceCompiler: SourceCompilerForInline, reifiedTypeInliner: ReifiedTypeInliner) { - val jvmMethodSignature = typeMapper.mapSignatureSkipGeneric(invokeMethodDescriptor) - val asmMethod = jvmMethodSignature.asmMethod - val methodNode = MethodNode( - Opcodes.API_VERSION, AsmUtil.getMethodAsmFlags(invokeMethodDescriptor, OwnerKind.IMPLEMENTATION, sourceCompiler.state), - asmMethod.name, asmMethod.descriptor, null, null - ) - - node = wrapWithMaxLocalCalc(methodNode).let { adapter -> - val smap = sourceCompiler.generateLambdaBody( - adapter, jvmMethodSignature, this - ) - adapter.visitMaxs(-1, -1) - SMAPAndMethodNode(methodNode, smap) - } + node = sourceCompiler.generateLambdaBody(this) } } diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/SourceCompilerForInline.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/SourceCompilerForInline.kt index 10cd9a50001..37084655710 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/SourceCompilerForInline.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/SourceCompilerForInline.kt @@ -48,11 +48,7 @@ interface SourceCompilerForInline { val lazySourceMapper: DefaultSourceMapper - fun generateLambdaBody( - adapter: MethodVisitor, - jvmMethodSignature: JvmMethodSignature, - lambdaInfo: ExpressionLambda - ): SMAP + fun generateLambdaBody(lambdaInfo: ExpressionLambda): SMAPAndMethodNode fun doCreateMethodNodeFromSource( callableDescriptor: FunctionDescriptor, @@ -133,13 +129,16 @@ class PsiSourceCompilerForInline(private val codegen: ExpressionCodegen, overrid override val lazySourceMapper get() = codegen.parentCodegen.orCreateSourceMapper - override fun generateLambdaBody( - adapter: MethodVisitor, - jvmMethodSignature: JvmMethodSignature, - lambdaInfo: ExpressionLambda - ): SMAP { + override fun generateLambdaBody(lambdaInfo: ExpressionLambda): SMAPAndMethodNode { lambdaInfo as? PsiExpressionLambda ?: error("TODO") val invokeMethodDescriptor = lambdaInfo.invokeMethodDescriptor + val jvmMethodSignature = state.typeMapper.mapSignatureSkipGeneric(invokeMethodDescriptor) + val asmMethod = jvmMethodSignature.asmMethod + val methodNode = MethodNode( + Opcodes.API_VERSION, AsmUtil.getMethodAsmFlags(invokeMethodDescriptor, OwnerKind.IMPLEMENTATION, state), + asmMethod.name, asmMethod.descriptor, null, null + ) + val adapter = wrapWithMaxLocalCalc(methodNode) val closureContext = when { lambdaInfo.isPropertyReference -> codegen.getContext().intoAnonymousClass(lambdaInfo.classDescriptor, codegen, OwnerKind.IMPLEMENTATION) @@ -150,12 +149,13 @@ class PsiSourceCompilerForInline(private val codegen: ExpressionCodegen, overrid else -> codegen.getContext().intoClosure(invokeMethodDescriptor, codegen, state.typeMapper) } val context = closureContext.intoInlinedLambda(invokeMethodDescriptor, lambdaInfo.isCrossInline, lambdaInfo.isPropertyReference) - - return generateMethodBody( + val smap = generateMethodBody( adapter, invokeMethodDescriptor, context, lambdaInfo.functionWithBodyOrCallableReference, jvmMethodSignature, lambdaInfo ) + adapter.visitMaxs(-1, -1) + return SMAPAndMethodNode(methodNode, smap) } private fun generateMethodBody( diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/ExpressionCodegen.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/ExpressionCodegen.kt index 0773de2e0f7..eb9212b94b0 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/ExpressionCodegen.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/ExpressionCodegen.kt @@ -174,7 +174,6 @@ class ExpressionCodegen( val endLabel = markNewLabel() writeLocalVariablesInTable(info, endLabel) writeParameterInLocalVariableTable(startLabel, endLabel) - mv.visitEnd() } private fun generateNonNullAssertions() { diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/FunctionCodegen.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/FunctionCodegen.kt index 5cac4621fdb..c21a3c246c0 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/FunctionCodegen.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/FunctionCodegen.kt @@ -58,11 +58,12 @@ open class FunctionCodegen( if (!state.classBuilderMode.generateBodies || flags.and(Opcodes.ACC_ABSTRACT) != 0 || irFunction.isExternal) { generateAnnotationDefaultValueIfNeeded(methodVisitor) - methodVisitor.visitEnd() } else { val frameMap = createFrameMapWithReceivers(signature) ExpressionCodegen(irFunction, frameMap, InstructionAdapter(methodVisitor), classCodegen, isInlineLambda).generate() + methodVisitor.visitMaxs(-1, -1) } + methodVisitor.visitEnd() return signature } diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/IrSourceCompilerForInline.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/IrSourceCompilerForInline.kt index 5764a881ff2..47774e920f1 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/IrSourceCompilerForInline.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/IrSourceCompilerForInline.kt @@ -38,7 +38,6 @@ class IrSourceCompilerForInline( private val data: BlockInfo ) : SourceCompilerForInline { - //TODO override val lookupLocation: LookupLocation get() = NoLookupLocation.FROM_BACKEND @@ -59,19 +58,24 @@ class IrSourceCompilerForInline( override val lazySourceMapper: DefaultSourceMapper get() = codegen.classCodegen.getOrCreateSourceMapper() - override fun generateLambdaBody(adapter: MethodVisitor, jvmMethodSignature: JvmMethodSignature, lambdaInfo: ExpressionLambda): SMAP { - lambdaInfo as? IrExpressionLambdaImpl ?: error("Expecting ir lambda, but $lambdaInfo") - - val functionCodegen = object : FunctionCodegen(lambdaInfo.function, codegen.classCodegen, true) { + private fun makeInlineNode(function: IrFunction, classCodegen: ClassCodegen, marker: CallSiteMarker?): SMAPAndMethodNode { + var node: MethodNode? = null + val functionCodegen = object : FunctionCodegen(function, classCodegen, isInlineLambda = marker == null) { override fun createMethod(flags: Int, signature: JvmMethodGenericSignature): MethodVisitor { - return adapter + val asmMethod = signature.asmMethod + node = MethodNode(Opcodes.API_VERSION, flags, asmMethod.name, asmMethod.descriptor, signature.genericsSignature, null) + return wrapWithMaxLocalCalc(node!!) } } + lazySourceMapper.callSiteMarker = marker functionCodegen.generate() - - return SMAP(codegen.classCodegen.getOrCreateSourceMapper().resultMappings) + lazySourceMapper.callSiteMarker = null + return SMAPAndMethodNode(node!!, SMAP(classCodegen.getOrCreateSourceMapper().resultMappings)) } + override fun generateLambdaBody(lambdaInfo: ExpressionLambda): SMAPAndMethodNode = + makeInlineNode((lambdaInfo as IrExpressionLambdaImpl).function, codegen.classCodegen, null) + override fun doCreateMethodNodeFromSource( callableDescriptor: FunctionDescriptor, jvmSignature: JvmMethodSignature, @@ -99,31 +103,8 @@ class IrSourceCompilerForInline( } } - //ExpressionCodegen() - var node: MethodNode? = null - var maxCalcAdapter: MethodVisitor? = null - val fakeClassCodegen = FakeClassCodegen(irFunction, codegen.classCodegen) - val functionCodegen = object : FunctionCodegen(irFunction, fakeClassCodegen) { - override fun createMethod(flags: Int, signature: JvmMethodGenericSignature): MethodVisitor { - node = MethodNode( - Opcodes.API_VERSION, - flags, - signature.asmMethod.name, signature.asmMethod.descriptor, - signature.genericsSignature, null - ) - maxCalcAdapter = wrapWithMaxLocalCalc(node!!) - return maxCalcAdapter!! - } - } - assert(codegen.lastLineNumber >= 0) - lazySourceMapper.callSiteMarker = CallSiteMarker(codegen.lastLineNumber) - functionCodegen.generate() - lazySourceMapper.callSiteMarker = null - maxCalcAdapter!!.visitMaxs(-1, -1) - maxCalcAdapter!!.visitEnd() - - return SMAPAndMethodNode(node!!, SMAP(fakeClassCodegen.getOrCreateSourceMapper().resultMappings)) + return makeInlineNode(irFunction, FakeClassCodegen(irFunction, codegen.classCodegen), CallSiteMarker(codegen.lastLineNumber)) } override fun hasFinallyBlocks() = data.hasFinallyBlocks() diff --git a/compiler/testData/codegen/boxInline/simple/inlineCallInInlineLambda.kt b/compiler/testData/codegen/boxInline/simple/inlineCallInInlineLambda.kt new file mode 100644 index 00000000000..f8b6a22d1c7 --- /dev/null +++ b/compiler/testData/codegen/boxInline/simple/inlineCallInInlineLambda.kt @@ -0,0 +1,16 @@ +// FILE: 1.kt + +package test + +inline fun f(g: (Int) -> Int) = g(2) + +inline fun h() = 1 + +// FILE: 2.kt + +import test.* + +fun box(): String { + val result = f { it + h() } + return if (result == 3) "OK" else "fail: $result" +} diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxInlineCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxInlineCodegenTestGenerated.java index 8da7cf8b7d9..fa94b3a32e2 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxInlineCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxInlineCodegenTestGenerated.java @@ -2914,6 +2914,11 @@ public class BlackBoxInlineCodegenTestGenerated extends AbstractBlackBoxInlineCo runTest("compiler/testData/codegen/boxInline/simple/funImportedFromObject.kt"); } + @TestMetadata("inlineCallInInlineLambda.kt") + public void testInlineCallInInlineLambda() throws Exception { + runTest("compiler/testData/codegen/boxInline/simple/inlineCallInInlineLambda.kt"); + } + @TestMetadata("kt17431.kt") public void testKt17431() throws Exception { runTest("compiler/testData/codegen/boxInline/simple/kt17431.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/CompileKotlinAgainstInlineKotlinTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/CompileKotlinAgainstInlineKotlinTestGenerated.java index 8b80a2f79f2..506c2a07dcc 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/CompileKotlinAgainstInlineKotlinTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/CompileKotlinAgainstInlineKotlinTestGenerated.java @@ -2914,6 +2914,11 @@ public class CompileKotlinAgainstInlineKotlinTestGenerated extends AbstractCompi runTest("compiler/testData/codegen/boxInline/simple/funImportedFromObject.kt"); } + @TestMetadata("inlineCallInInlineLambda.kt") + public void testInlineCallInInlineLambda() throws Exception { + runTest("compiler/testData/codegen/boxInline/simple/inlineCallInInlineLambda.kt"); + } + @TestMetadata("kt17431.kt") public void testKt17431() throws Exception { runTest("compiler/testData/codegen/boxInline/simple/kt17431.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxInlineCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxInlineCodegenTestGenerated.java index 33a76545dec..e4c58166a23 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxInlineCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxInlineCodegenTestGenerated.java @@ -2914,6 +2914,11 @@ public class IrBlackBoxInlineCodegenTestGenerated extends AbstractIrBlackBoxInli runTest("compiler/testData/codegen/boxInline/simple/funImportedFromObject.kt"); } + @TestMetadata("inlineCallInInlineLambda.kt") + public void testInlineCallInInlineLambda() throws Exception { + runTest("compiler/testData/codegen/boxInline/simple/inlineCallInInlineLambda.kt"); + } + @TestMetadata("kt17431.kt") public void testKt17431() throws Exception { runTest("compiler/testData/codegen/boxInline/simple/kt17431.kt");