diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/common/variableLiveness.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/common/variableLiveness.kt index e0f448f03c7..3c9947b529f 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/common/variableLiveness.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/common/variableLiveness.kt @@ -16,10 +16,14 @@ package org.jetbrains.kotlin.codegen.optimization.common +import org.jetbrains.kotlin.codegen.optimization.transformer.MethodTransformer +import org.jetbrains.org.objectweb.asm.Type import org.jetbrains.org.objectweb.asm.tree.AbstractInsnNode import org.jetbrains.org.objectweb.asm.tree.IincInsnNode import org.jetbrains.org.objectweb.asm.tree.MethodNode import org.jetbrains.org.objectweb.asm.tree.VarInsnNode +import org.jetbrains.org.objectweb.asm.tree.analysis.BasicValue +import org.jetbrains.org.objectweb.asm.tree.analysis.Frame import java.util.* @@ -49,16 +53,13 @@ class VariableLivenessFrame(val maxLocals: Int) { } fun analyzeLiveness(node: MethodNode): Array { + val typeAnnotatedFrames = MethodTransformer.analyze("fake", node, OptimizationBasicInterpreter()) + val graph = ControlFlowGraph.build(node) val insnList = node.instructions val frames = Array(insnList.size()) { VariableLivenessFrame(node.maxLocals) } val insnArray = insnList.toArray() - val varVisibility = Array(node.maxLocals) { BitSet(insnArray.size) } - for (localVar in node.localVariables) { - varVisibility[localVar.index].set(insnList.indexOf(localVar.start), insnList.indexOf(localVar.end), true) - } - // see Figure 9.16 from Dragon book var wereChanges: Boolean @@ -72,7 +73,7 @@ fun analyzeLiveness(node: MethodNode): Array { } def(newFrame, insn) - use(newFrame, insn, index, varVisibility) + use(newFrame, insn, node, typeAnnotatedFrames[index]) if (frames[index] != newFrame) { frames[index] = newFrame @@ -91,11 +92,19 @@ private fun def(frame: VariableLivenessFrame, insn: AbstractInsnNode) { } } -private fun use(frame: VariableLivenessFrame, insn: AbstractInsnNode, index: Int, varVisibility: Array) { - for (i in 0..frame.maxLocals - 1) { - if (varVisibility[i].get(index)) { - frame.markAlive(i) - } +private fun use( + frame: VariableLivenessFrame, + insn: AbstractInsnNode, + node: MethodNode, + // May be null in case of dead code + typeAnnotatedFrame: Frame? +) { + val index = node.instructions.indexOf(insn) + node.localVariables.filter { + node.instructions.indexOf(it.start) < index && index < node.instructions.indexOf(it.end) && + Type.getType(it.desc).sort == typeAnnotatedFrame?.getLocal(it.index)?.type?.sort + }.forEach { + frame.markAlive(it.index) } if (insn is VarInsnNode && insn.isLoadOperation()) { diff --git a/compiler/testData/codegen/box/coroutines/varValueConflictsWithTable.kt b/compiler/testData/codegen/box/coroutines/varValueConflictsWithTable.kt new file mode 100644 index 00000000000..798641da29a --- /dev/null +++ b/compiler/testData/codegen/box/coroutines/varValueConflictsWithTable.kt @@ -0,0 +1,36 @@ +class Controller { + suspend fun suspendHere(x: Continuation) { + x.resume("OK") + } +} + +fun builder(coroutine c: Controller.() -> Continuation) { + c(Controller()).resume(Unit) +} + +fun box(): String { + var result = "fail 1" + builder { + // Initialize var with Int value + for (i in 1..1) { + if (i != 1) continue + } + + // This variable should take the same slot as 'i' had + var s: String + + // We should not spill 's' to continuation field because it's not initialized + // More precisely it contains a value of wrong type (it conflicts with contents of local var table), + // so an attempt of spilling may lead to problems on Android + if (suspendHere() == "OK") { + s = "OK" + } + else { + s = "fail 2" + } + + result = s + } + + return result +} diff --git a/compiler/testData/codegen/box/coroutines/varValueConflictsWithTableSameSort.kt b/compiler/testData/codegen/box/coroutines/varValueConflictsWithTableSameSort.kt new file mode 100644 index 00000000000..29a3eb3136d --- /dev/null +++ b/compiler/testData/codegen/box/coroutines/varValueConflictsWithTableSameSort.kt @@ -0,0 +1,36 @@ +class Controller { + suspend fun suspendHere(x: Continuation) { + x.resume("OK") + } +} + +fun builder(coroutine c: Controller.() -> Continuation) { + c(Controller()).resume(Unit) +} + +fun box(): String { + var result = "fail 1" + builder { + // Initialize var with Int value + try { + var i: String = "abc" + i = "123" + } finally { } + + // This variable should take the same slot as 'i' had + var s: String + + // We shout not spill 's' to continuation field because it's not effectively initialized + // But we do this because it's not illegal (at least in Android/OpenJDK VM's) + if (suspendHere() == "OK") { + s = "OK" + } + else { + s = "fail 2" + } + + result = s + } + + return result +} diff --git a/compiler/testData/codegen/bytecodeText/coroutines/varValueConflictsWithTable.kt b/compiler/testData/codegen/bytecodeText/coroutines/varValueConflictsWithTable.kt new file mode 100644 index 00000000000..5578a8a0ecf --- /dev/null +++ b/compiler/testData/codegen/bytecodeText/coroutines/varValueConflictsWithTable.kt @@ -0,0 +1,42 @@ +class Controller { + suspend fun suspendHere(x: Continuation) { + x.resume("OK") + } +} + +fun builder(coroutine c: Controller.() -> Continuation) { + c(Controller()).resume(Unit) +} + +fun box(): String { + var result = "fail 1" + builder { + // Initialize var with Int value + for (i in 1..1) { + if ("".length > 0) continue + } + + // This variable should take the same slot as 'i' had + var s: String + + // We should not spill 's' to continuation field because it's not initialized + // More precisely it contains a value of wrong type (it conflicts with contents of local var table), + // so an attempt of spilling may lead to problems on Android + if (suspendHere() == "OK") { + s = "OK" + } + else { + s = "fail 2" + } + + result = s + } + + return result +} + +// 1 LOCALVARIABLE i I L.* 3 +// 1 LOCALVARIABLE s Ljava/lang/String; L.* 3 +// 0 PUTFIELD VarValueConflictsWithTableKt\$box\$1.I\$0 : I +/* 2 loads in cycle */ +// 2 ILOAD 3 diff --git a/compiler/testData/codegen/bytecodeText/coroutines/varValueConflictsWithTableSameSort.kt b/compiler/testData/codegen/bytecodeText/coroutines/varValueConflictsWithTableSameSort.kt new file mode 100644 index 00000000000..a5bf8f07b4e --- /dev/null +++ b/compiler/testData/codegen/bytecodeText/coroutines/varValueConflictsWithTableSameSort.kt @@ -0,0 +1,45 @@ +class Controller { + suspend fun suspendHere(x: Continuation) { + x.resume("OK") + } +} + +fun builder(coroutine c: Controller.() -> Continuation) { + c(Controller()).resume(Unit) +} + +fun box(): String { + var result = "fail 1" + + builder { + // Initialize var with Int value + try { + var i: String = "abc" + i = "123" + } finally { } + + // This variable should take the same slot as 'i' had + var s: String + + // We shout not spill 's' to continuation field because it's not effectively initialized + // But we do this because it's not illegal (at least in Android/OpenJDK VM's) + if (suspendHere() == "OK") { + s = "OK" + } + else { + s = "fail 2" + } + + result = s + } + + return result +} + +// 1 LOCALVARIABLE i Ljava/lang/String; L.* 3 +// 1 LOCALVARIABLE s Ljava/lang/String; L.* 3 +// 1 PUTFIELD VarValueConflictsWithTableSameSortKt\$box\$1.L\$0 : Ljava/lang/Object; +/* 1 load in try/finally */ +/* 1 load in result = s */ +/* 1 load before suspension point */ +// 3 ALOAD 3 diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java index 2ab6fc248bc..8e2b49c5a57 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java @@ -4386,6 +4386,18 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/tryFinallyWithHandleResult.kt"); doTest(fileName); } + + @TestMetadata("varValueConflictsWithTable.kt") + public void testVarValueConflictsWithTable() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/varValueConflictsWithTable.kt"); + doTest(fileName); + } + + @TestMetadata("varValueConflictsWithTableSameSort.kt") + public void testVarValueConflictsWithTableSameSort() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/varValueConflictsWithTableSameSort.kt"); + doTest(fileName); + } } @TestMetadata("compiler/testData/codegen/box/dataClasses") diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java index 37c381a4ddb..8e96e9d7543 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java @@ -773,6 +773,27 @@ public class BytecodeTextTestGenerated extends AbstractBytecodeTextTest { } } + @TestMetadata("compiler/testData/codegen/bytecodeText/coroutines") + @TestDataPath("$PROJECT_ROOT") + @RunWith(JUnit3RunnerWithInners.class) + public static class Coroutines extends AbstractBytecodeTextTest { + public void testAllFilesPresentInCoroutines() throws Exception { + KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/bytecodeText/coroutines"), Pattern.compile("^(.+)\\.kt$"), true); + } + + @TestMetadata("varValueConflictsWithTable.kt") + public void testVarValueConflictsWithTable() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/bytecodeText/coroutines/varValueConflictsWithTable.kt"); + doTest(fileName); + } + + @TestMetadata("varValueConflictsWithTableSameSort.kt") + public void testVarValueConflictsWithTableSameSort() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/bytecodeText/coroutines/varValueConflictsWithTableSameSort.kt"); + doTest(fileName); + } + } + @TestMetadata("compiler/testData/codegen/bytecodeText/deadCodeElimination") @TestDataPath("$PROJECT_ROOT") @RunWith(JUnit3RunnerWithInners.class)