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.
This commit is contained in:
pyos
2019-05-24 13:57:42 +02:00
committed by max-kammerer
parent 82e4985aa0
commit 6d19eb1853
9 changed files with 59 additions and 61 deletions
@@ -189,22 +189,8 @@ internal fun Type.boxReceiverForBoundReference(kotlinType: KotlinType, typeMappe
AsmUtil.boxType(this, kotlinType, typeMapper) AsmUtil.boxType(this, kotlinType, typeMapper)
abstract class ExpressionLambda(protected val typeMapper: KotlinTypeMapper, isCrossInline: Boolean) : LambdaInfo(isCrossInline) { abstract class ExpressionLambda(protected val typeMapper: KotlinTypeMapper, isCrossInline: Boolean) : LambdaInfo(isCrossInline) {
override fun generateLambdaBody(sourceCompiler: SourceCompilerForInline, reifiedTypeInliner: ReifiedTypeInliner) { override fun generateLambdaBody(sourceCompiler: SourceCompilerForInline, reifiedTypeInliner: ReifiedTypeInliner) {
val jvmMethodSignature = typeMapper.mapSignatureSkipGeneric(invokeMethodDescriptor) node = sourceCompiler.generateLambdaBody(this)
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)
}
} }
} }
@@ -48,11 +48,7 @@ interface SourceCompilerForInline {
val lazySourceMapper: DefaultSourceMapper val lazySourceMapper: DefaultSourceMapper
fun generateLambdaBody( fun generateLambdaBody(lambdaInfo: ExpressionLambda): SMAPAndMethodNode
adapter: MethodVisitor,
jvmMethodSignature: JvmMethodSignature,
lambdaInfo: ExpressionLambda
): SMAP
fun doCreateMethodNodeFromSource( fun doCreateMethodNodeFromSource(
callableDescriptor: FunctionDescriptor, callableDescriptor: FunctionDescriptor,
@@ -133,13 +129,16 @@ class PsiSourceCompilerForInline(private val codegen: ExpressionCodegen, overrid
override val lazySourceMapper override val lazySourceMapper
get() = codegen.parentCodegen.orCreateSourceMapper get() = codegen.parentCodegen.orCreateSourceMapper
override fun generateLambdaBody( override fun generateLambdaBody(lambdaInfo: ExpressionLambda): SMAPAndMethodNode {
adapter: MethodVisitor,
jvmMethodSignature: JvmMethodSignature,
lambdaInfo: ExpressionLambda
): SMAP {
lambdaInfo as? PsiExpressionLambda ?: error("TODO") lambdaInfo as? PsiExpressionLambda ?: error("TODO")
val invokeMethodDescriptor = lambdaInfo.invokeMethodDescriptor 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 { val closureContext = when {
lambdaInfo.isPropertyReference -> lambdaInfo.isPropertyReference ->
codegen.getContext().intoAnonymousClass(lambdaInfo.classDescriptor, codegen, OwnerKind.IMPLEMENTATION) 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) else -> codegen.getContext().intoClosure(invokeMethodDescriptor, codegen, state.typeMapper)
} }
val context = closureContext.intoInlinedLambda(invokeMethodDescriptor, lambdaInfo.isCrossInline, lambdaInfo.isPropertyReference) val context = closureContext.intoInlinedLambda(invokeMethodDescriptor, lambdaInfo.isCrossInline, lambdaInfo.isPropertyReference)
val smap = generateMethodBody(
return generateMethodBody(
adapter, invokeMethodDescriptor, context, adapter, invokeMethodDescriptor, context,
lambdaInfo.functionWithBodyOrCallableReference, lambdaInfo.functionWithBodyOrCallableReference,
jvmMethodSignature, lambdaInfo jvmMethodSignature, lambdaInfo
) )
adapter.visitMaxs(-1, -1)
return SMAPAndMethodNode(methodNode, smap)
} }
private fun generateMethodBody( private fun generateMethodBody(
@@ -174,7 +174,6 @@ class ExpressionCodegen(
val endLabel = markNewLabel() val endLabel = markNewLabel()
writeLocalVariablesInTable(info, endLabel) writeLocalVariablesInTable(info, endLabel)
writeParameterInLocalVariableTable(startLabel, endLabel) writeParameterInLocalVariableTable(startLabel, endLabel)
mv.visitEnd()
} }
private fun generateNonNullAssertions() { private fun generateNonNullAssertions() {
@@ -58,11 +58,12 @@ open class FunctionCodegen(
if (!state.classBuilderMode.generateBodies || flags.and(Opcodes.ACC_ABSTRACT) != 0 || irFunction.isExternal) { if (!state.classBuilderMode.generateBodies || flags.and(Opcodes.ACC_ABSTRACT) != 0 || irFunction.isExternal) {
generateAnnotationDefaultValueIfNeeded(methodVisitor) generateAnnotationDefaultValueIfNeeded(methodVisitor)
methodVisitor.visitEnd()
} else { } else {
val frameMap = createFrameMapWithReceivers(signature) val frameMap = createFrameMapWithReceivers(signature)
ExpressionCodegen(irFunction, frameMap, InstructionAdapter(methodVisitor), classCodegen, isInlineLambda).generate() ExpressionCodegen(irFunction, frameMap, InstructionAdapter(methodVisitor), classCodegen, isInlineLambda).generate()
methodVisitor.visitMaxs(-1, -1)
} }
methodVisitor.visitEnd()
return signature return signature
} }
@@ -38,7 +38,6 @@ class IrSourceCompilerForInline(
private val data: BlockInfo private val data: BlockInfo
) : SourceCompilerForInline { ) : SourceCompilerForInline {
//TODO //TODO
override val lookupLocation: LookupLocation override val lookupLocation: LookupLocation
get() = NoLookupLocation.FROM_BACKEND get() = NoLookupLocation.FROM_BACKEND
@@ -59,19 +58,24 @@ class IrSourceCompilerForInline(
override val lazySourceMapper: DefaultSourceMapper override val lazySourceMapper: DefaultSourceMapper
get() = codegen.classCodegen.getOrCreateSourceMapper() get() = codegen.classCodegen.getOrCreateSourceMapper()
override fun generateLambdaBody(adapter: MethodVisitor, jvmMethodSignature: JvmMethodSignature, lambdaInfo: ExpressionLambda): SMAP { private fun makeInlineNode(function: IrFunction, classCodegen: ClassCodegen, marker: CallSiteMarker?): SMAPAndMethodNode {
lambdaInfo as? IrExpressionLambdaImpl ?: error("Expecting ir lambda, but $lambdaInfo") var node: MethodNode? = null
val functionCodegen = object : FunctionCodegen(function, classCodegen, isInlineLambda = marker == null) {
val functionCodegen = object : FunctionCodegen(lambdaInfo.function, codegen.classCodegen, true) {
override fun createMethod(flags: Int, signature: JvmMethodGenericSignature): MethodVisitor { 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() functionCodegen.generate()
lazySourceMapper.callSiteMarker = null
return SMAP(codegen.classCodegen.getOrCreateSourceMapper().resultMappings) return SMAPAndMethodNode(node!!, SMAP(classCodegen.getOrCreateSourceMapper().resultMappings))
} }
override fun generateLambdaBody(lambdaInfo: ExpressionLambda): SMAPAndMethodNode =
makeInlineNode((lambdaInfo as IrExpressionLambdaImpl).function, codegen.classCodegen, null)
override fun doCreateMethodNodeFromSource( override fun doCreateMethodNodeFromSource(
callableDescriptor: FunctionDescriptor, callableDescriptor: FunctionDescriptor,
jvmSignature: JvmMethodSignature, 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) assert(codegen.lastLineNumber >= 0)
lazySourceMapper.callSiteMarker = CallSiteMarker(codegen.lastLineNumber) return makeInlineNode(irFunction, FakeClassCodegen(irFunction, codegen.classCodegen), CallSiteMarker(codegen.lastLineNumber))
functionCodegen.generate()
lazySourceMapper.callSiteMarker = null
maxCalcAdapter!!.visitMaxs(-1, -1)
maxCalcAdapter!!.visitEnd()
return SMAPAndMethodNode(node!!, SMAP(fakeClassCodegen.getOrCreateSourceMapper().resultMappings))
} }
override fun hasFinallyBlocks() = data.hasFinallyBlocks() override fun hasFinallyBlocks() = data.hasFinallyBlocks()
@@ -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"
}
@@ -2914,6 +2914,11 @@ public class BlackBoxInlineCodegenTestGenerated extends AbstractBlackBoxInlineCo
runTest("compiler/testData/codegen/boxInline/simple/funImportedFromObject.kt"); 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") @TestMetadata("kt17431.kt")
public void testKt17431() throws Exception { public void testKt17431() throws Exception {
runTest("compiler/testData/codegen/boxInline/simple/kt17431.kt"); runTest("compiler/testData/codegen/boxInline/simple/kt17431.kt");
@@ -2914,6 +2914,11 @@ public class CompileKotlinAgainstInlineKotlinTestGenerated extends AbstractCompi
runTest("compiler/testData/codegen/boxInline/simple/funImportedFromObject.kt"); 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") @TestMetadata("kt17431.kt")
public void testKt17431() throws Exception { public void testKt17431() throws Exception {
runTest("compiler/testData/codegen/boxInline/simple/kt17431.kt"); runTest("compiler/testData/codegen/boxInline/simple/kt17431.kt");
@@ -2914,6 +2914,11 @@ public class IrBlackBoxInlineCodegenTestGenerated extends AbstractIrBlackBoxInli
runTest("compiler/testData/codegen/boxInline/simple/funImportedFromObject.kt"); 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") @TestMetadata("kt17431.kt")
public void testKt17431() throws Exception { public void testKt17431() throws Exception {
runTest("compiler/testData/codegen/boxInline/simple/kt17431.kt"); runTest("compiler/testData/codegen/boxInline/simple/kt17431.kt");