JVM_IR: never create temporaries in $default stubs

Given the strict pattern-matching in the inliner, this is the only way
to make it not crash when attempting to inline these stubs. Note that
the IR backend does not currently use the inliner's default method stub
handling; the crash only occurs when a module compiled with the non-IR
JVM backend is attempting to call an inline function with default
arguments defined in a module that was compiled with the IR backend.
This commit is contained in:
pyos
2019-10-16 13:23:57 +02:00
committed by max-kammerer
parent 95be7171bc
commit 55acc296a2
13 changed files with 136 additions and 19 deletions
@@ -31,6 +31,7 @@ class RedundantGotoMethodTransformer : MethodTransformer() {
* (1) subsequent labels
* ...
* goto Label (can be removed)
* nop (any number of them, or maybe none; will be removed by RedundantNopsCleanupMethodTransformer)
* Label:
* ...
* (2) indirect goto
@@ -49,6 +50,10 @@ class RedundantGotoMethodTransformer : MethodTransformer() {
var pendingGoto: JumpInsnNode? = null
for (insn in insns) {
// We can remove jumps over labels, NOPs, GOTOs with the same target, and fake
// instructions used to describe the current frame state. We have to keep jumps
// over line numbers, though, as otherwise something like an `if` with an empty
// `else` will trigger a breakpoint inside the `else` even when the condition is true.
when {
insn is LabelNode -> {
currentLabels.add(insn)
@@ -62,8 +67,7 @@ class RedundantGotoMethodTransformer : MethodTransformer() {
currentLabels.clear()
}
}
insn is LineNumberNode -> pendingGoto = null
insn.isMeaningful -> {
insn is LineNumberNode || (insn.isMeaningful && insn.opcode != Opcodes.NOP) -> {
currentLabels.clear()
pendingGoto = null
}
@@ -108,7 +108,6 @@ open class DefaultArgumentStubGenerator(
val expressionBody = valueParameter.defaultValue!!
expressionBody.patchDeclarationParents(newIrFunction)
expressionBody.transformChildrenVoid(object : IrElementTransformerVoid() {
override fun visitGetValue(expression: IrGetValue): IrExpression {
log { "GetValue: ${expression.symbol.owner}" }
@@ -117,13 +116,7 @@ open class DefaultArgumentStubGenerator(
}
})
val argument = irIfThenElse(
type = parameter.type,
condition = condition,
thenPart = expressionBody.expression,
elsePart = irGet(parameter)
)
createTmpVariable(argument, nameHint = parameter.name.asString())
selectArgumentOrDefault(condition, parameter, expressionBody.expression)
} else {
parameter
}
@@ -157,6 +150,15 @@ open class DefaultArgumentStubGenerator(
return listOf(irFunction, newIrFunction)
}
protected open fun IrBlockBodyBuilder.selectArgumentOrDefault(
shouldUseDefault: IrExpression,
parameter: IrValueParameter,
default: IrExpression
): IrValueDeclaration {
val value = irIfThenElse(parameter.type, shouldUseDefault, default, irGet(parameter))
return createTmpVariable(value, nameHint = parameter.name.asString())
}
private fun IrBlockBodyBuilder.dispatchToImplementation(
irFunction: IrSimpleFunction,
newIrFunction: IrFunction,
@@ -130,8 +130,8 @@ internal val localDeclarationsPhase = makeIrFilePhase<CommonBackendContext>(
prerequisite = setOf(callableReferencePhase, sharedVariablesPhase)
)
private val defaultArgumentStubPhase = makeIrFilePhase<CommonBackendContext>(
{ context -> DefaultArgumentStubGenerator(context, false) },
private val defaultArgumentStubPhase = makeIrFilePhase(
::JvmDefaultArgumentStubGenerator,
name = "DefaultArgumentsStubGenerator",
description = "Generate synthetic stubs for functions with default parameter values",
prerequisite = setOf(localDeclarationsPhase)
@@ -378,6 +378,18 @@ class JvmSymbols(
returnType = dst.defaultType
}.symbol
val reassignParameterIntrinsic: IrSimpleFunctionSymbol =
buildFun {
name = Name.special("<set-parameter>")
origin = IrDeclarationOrigin.IR_BUILTINS_STUB
}.apply {
parent = kotlinJvmInternalPackage
val type = addTypeParameter("T", irBuiltIns.anyNType)
addValueParameter("parameter", type.defaultType) // must be IrGetValue of an IrValueParameter
addValueParameter("value", type.defaultType)
returnType = irBuiltIns.unitType
}.symbol
private val collectionToArrayClass: IrClassSymbol = createClass(FqName("kotlin.jvm.internal.CollectionToArray")) { klass ->
klass.origin = JvmLoweredDeclarationOrigin.TO_ARRAY
@@ -36,6 +36,7 @@ import org.jetbrains.kotlin.ir.declarations.*
import org.jetbrains.kotlin.ir.expressions.*
import org.jetbrains.kotlin.ir.symbols.IrSymbol
import org.jetbrains.kotlin.ir.symbols.IrTypeParameterSymbol
import org.jetbrains.kotlin.ir.symbols.IrValueSymbol
import org.jetbrains.kotlin.ir.types.*
import org.jetbrains.kotlin.ir.util.*
import org.jetbrains.kotlin.ir.visitors.IrElementVisitor
@@ -131,7 +132,7 @@ class ExpressionCodegen(
val IrExpression.asmType: Type
get() = type.asmType
val IrVariable.asmType: Type
val IrValueDeclaration.asmType: Type
get() = type.asmType
// Assume this expression's result has already been materialized on the stack
@@ -512,12 +513,16 @@ class ExpressionCodegen(
override fun visitSetVariable(expression: IrSetVariable, data: BlockInfo): PromisedValue {
expression.markLineNumber(startOffset = true)
expression.value.markLineNumber(startOffset = true)
expression.value.accept(this, data).coerce(expression.symbol.owner.type).materialize()
mv.store(findLocalIndex(expression.symbol), expression.symbol.owner.asmType)
setVariable(expression.symbol, expression.value, data)
return defaultValue(expression.type)
}
fun setVariable(symbol: IrValueSymbol, value: IrExpression, data: BlockInfo) {
value.markLineNumber(startOffset = true)
value.accept(this, data).coerce(symbol.owner.type).materialize()
mv.store(findLocalIndex(symbol), symbol.owner.asmType)
}
override fun <T> visitConst(expression: IrConst<T>, data: BlockInfo): PromisedValue {
expression.markLineNumber(startOffset = true)
when (val value = expression.value) {
@@ -91,7 +91,8 @@ class IrIntrinsicMethods(val irBuiltIns: IrBuiltIns, val symbols: JvmSymbols) {
irBuiltIns.andandSymbol.toKey()!! to AndAnd,
irBuiltIns.ororSymbol.toKey()!! to OrOr,
symbols.unsafeCoerceIntrinsic.toKey()!! to UnsafeCoerce,
symbols.signatureStringIntrinsic.toKey()!! to SignatureString
symbols.signatureStringIntrinsic.toKey()!! to SignatureString,
symbols.reassignParameterIntrinsic.toKey()!! to ReassignParameter
) +
numberConversionMethods() +
unaryFunForPrimitives("plus", UnaryPlus) +
@@ -0,0 +1,33 @@
/*
* Copyright 2010-2019 JetBrains s.r.o. and Kotlin Programming Language contributors.
* Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file.
*/
package org.jetbrains.kotlin.backend.jvm.intrinsics
import org.jetbrains.kotlin.backend.jvm.codegen.*
import org.jetbrains.kotlin.ir.expressions.IrFunctionAccessExpression
import org.jetbrains.kotlin.ir.expressions.IrGetValue
import org.jetbrains.kotlin.ir.symbols.IrValueParameterSymbol
// A magic intrinsic that changes the supposedly-immutable parameter referenced by its argument:
//
// fun f(x: Int) {
// <set-parameter>(x, 1)
// return x
// }
// assert(f(2) == 1)
//
// This *only* works for parameters; any other variable should be made mutable instead.
//
// This is used to optimize default method stubs (and, more importantly, produce bytecode that
// the inliner can actually handle) in `JvmDefaultArgumentStubGenerator`.
object ReassignParameter : IntrinsicMethod() {
override fun invoke(expression: IrFunctionAccessExpression, codegen: ExpressionCodegen, data: BlockInfo): PromisedValue {
val parameterGet = expression.getValueArgument(0) as? IrGetValue
val parameter = parameterGet?.symbol as? IrValueParameterSymbol
?: throw AssertionError("${expression.getValueArgument(0)} is not a get of a parameter")
codegen.setVariable(parameter, expression.getValueArgument(1)!!, data)
return codegen.immaterialUnitValue
}
}
@@ -0,0 +1,46 @@
/*
* Copyright 2010-2019 JetBrains s.r.o. and Kotlin Programming Language contributors.
* Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file.
*/
package org.jetbrains.kotlin.backend.jvm.lower
import org.jetbrains.kotlin.backend.common.lower.DefaultArgumentStubGenerator
import org.jetbrains.kotlin.backend.common.lower.irIfThen
import org.jetbrains.kotlin.backend.jvm.JvmBackendContext
import org.jetbrains.kotlin.ir.builders.IrBlockBodyBuilder
import org.jetbrains.kotlin.ir.builders.irCall
import org.jetbrains.kotlin.ir.builders.irGet
import org.jetbrains.kotlin.ir.declarations.IrValueDeclaration
import org.jetbrains.kotlin.ir.declarations.IrValueParameter
import org.jetbrains.kotlin.ir.expressions.IrExpression
class JvmDefaultArgumentStubGenerator(override val context: JvmBackendContext) : DefaultArgumentStubGenerator(context, false, false) {
override fun IrBlockBodyBuilder.selectArgumentOrDefault(
shouldUseDefault: IrExpression,
parameter: IrValueParameter,
default: IrExpression
): IrValueDeclaration {
// We have to generate precisely this code because that results in the bytecode the inliner expects;
// see `expandMaskConditionsAndUpdateVariableNodes`. In short, the bytecode sequence should be
//
// -- no loads of the parameter here, as after inlining its value will be uninitialized
// ILOAD <mask>
// ICONST <bit>
// IAND
// IFEQ Lx
// -- any code inserted here is removed if the call site specifies the parameter
// STORE <n>
// -- no jumps here
// Lx
//
// This control flow limits us to an if-then (without an else), and this together with the
// restriction on loading the parameter in the default case means we cannot create any temporaries.
+irIfThen(shouldUseDefault, irCall(this@JvmDefaultArgumentStubGenerator.context.ir.symbols.reassignParameterIntrinsic).apply {
putTypeArgument(0, parameter.type)
putValueArgument(0, irGet(parameter))
putValueArgument(1, default)
})
return parameter
}
}
@@ -1,4 +1,3 @@
// IGNORE_BACKEND: JVM_IR
class A {
fun test(x: String? = "x", a: String?, b: String?, y: String? = "y") {
}
@@ -0,0 +1,6 @@
inline fun f(x: Int = 1) = x
// MUST contain these instructions to avoid breaking the inliner.
// See `expandMaskConditionsAndUpdateVariableNodes`.
// 1 ILOAD 1\s*ICONST_1\s*IAND\s*IFEQ L1
// 1 ICONST_1\s*ISTORE 0\s*(L\d\s*)*L1
@@ -1,4 +1,3 @@
// IGNORE_BACKEND: JVM_IR
fun test(a: Int, z: String = try{"1"} catch (e: Exception) {"2"}) {
"Default body"
}
@@ -1590,6 +1590,11 @@ public class BytecodeTextTestGenerated extends AbstractBytecodeTextTest {
runTest("compiler/testData/codegen/bytecodeText/defaultArguments/maskAndArgumentElimination.kt");
}
@TestMetadata("maskCheckSequence.kt")
public void testMaskCheckSequence() throws Exception {
runTest("compiler/testData/codegen/bytecodeText/defaultArguments/maskCheckSequence.kt");
}
@TestMetadata("methodHandlerElimination.kt")
public void testMethodHandlerElimination() throws Exception {
runTest("compiler/testData/codegen/bytecodeText/defaultArguments/methodHandlerElimination.kt");
@@ -1545,6 +1545,11 @@ public class IrBytecodeTextTestGenerated extends AbstractIrBytecodeTextTest {
runTest("compiler/testData/codegen/bytecodeText/defaultArguments/maskAndArgumentElimination.kt");
}
@TestMetadata("maskCheckSequence.kt")
public void testMaskCheckSequence() throws Exception {
runTest("compiler/testData/codegen/bytecodeText/defaultArguments/maskCheckSequence.kt");
}
@TestMetadata("methodHandlerElimination.kt")
public void testMethodHandlerElimination() throws Exception {
runTest("compiler/testData/codegen/bytecodeText/defaultArguments/methodHandlerElimination.kt");