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 f1ac72e789a..bd22ea4aeb6 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/CoroutineTransformerMethodVisitor.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/CoroutineTransformerMethodVisitor.kt @@ -132,6 +132,8 @@ class CoroutineTransformerMethodVisitor( UninitializedStoresProcessor(methodNode, shouldPreserveClassInitialization).run() + updateLvtAccordingToLiveness(methodNode) + val spilledToVariableMapping = spillVariables(suspensionPoints, methodNode) val suspendMarkerVarIndex = methodNode.maxLocals++ @@ -183,14 +185,6 @@ class CoroutineTransformerMethodVisitor( dropSuspensionMarkers(methodNode) methodNode.removeEmptyCatchBlocks() - // The parameters (and 'this') shall live throughout the method, otherwise, d8 emits warning about invalid debug info - val startLabel = LabelNode() - val endLabel = LabelNode() - methodNode.instructions.insertBefore(methodNode.instructions.first, startLabel) - methodNode.instructions.insert(methodNode.instructions.last, endLabel) - - fixLvtForParameters(methodNode, startLabel, endLabel) - if (languageVersionSettings.isReleaseCoroutines()) { writeDebugMetadata(methodNode, suspensionPointLineNumbers, spilledToVariableMapping) } @@ -311,31 +305,10 @@ class CoroutineTransformerMethodVisitor( } } - private fun fixLvtForParameters(methodNode: MethodNode, startLabel: LabelNode, endLabel: LabelNode) { - val paramsNum = - /* this */ (if (isStatic(methodNode.access)) 0 else 1) + - /* real params */ Type.getArgumentTypes(methodNode.desc).fold(0) { a, b -> a + b.size } - - for (i in 0 until paramsNum) { - fixRangeOfLvtRecord(methodNode, i, startLabel, endLabel) - } - } - - private fun fixRangeOfLvtRecord(methodNode: MethodNode, index: Int, startLabel: LabelNode, endLabel: LabelNode) { - val vars = methodNode.localVariables.filter { it.index == index } - assert(vars.size <= 1) { - "Someone else occupies parameter's slot at $index" - } - vars.firstOrNull()?.let { - it.start = startLabel - it.end = endLabel - } - } - private fun writeDebugMetadata( methodNode: MethodNode, suspensionPointLineNumbers: List, - spilledToLocalMapping: List> + spilledToLocalMapping: List> ) { val lines = suspensionPointLineNumbers.map { it?.line ?: -1 } val metadata = classBuilderForCoroutineState.newAnnotation(DEBUG_METADATA_ANNOTATION_ASM_TYPE.descriptor, true) @@ -590,7 +563,7 @@ class CoroutineTransformerMethodVisitor( } } - private fun spillVariables(suspensionPoints: List, methodNode: MethodNode): List> { + private fun spillVariables(suspensionPoints: List, methodNode: MethodNode): List> { val instructions = methodNode.instructions val frames = if (useOldSpilledVarTypeAnalysis) performRefinedTypeAnalysis(methodNode, containingClassInternalName) @@ -602,7 +575,7 @@ class CoroutineTransformerMethodVisitor( val postponedActions = mutableListOf<() -> Unit>() val maxVarsCountByType = mutableMapOf() val livenessFrames = analyzeLiveness(methodNode) - val spilledToVariableMapping = arrayListOf>() + val spilledToVariableMapping = arrayListOf>() for (suspension in suspensionPoints) { val suspensionCallBegin = suspension.suspensionCallBegin @@ -629,7 +602,7 @@ class CoroutineTransformerMethodVisitor( // NB: it's also rather useful for sake of optimization val livenessFrame = livenessFrames[suspensionCallBegin.index()] - val spilledToVariable = arrayListOf() + val spilledToVariable = arrayListOf() // 0 - this // 1 - parameter @@ -667,7 +640,7 @@ class CoroutineTransformerMethodVisitor( val fieldName = normalizedType.fieldNameForVar(indexBySort) localVariableName(methodNode, index, suspension.suspensionCallEnd.next.index()) - ?.let { spilledToVariable.add(SpilledVariableDescriptor(fieldName, it)) } + ?.let { spilledToVariable.add(SpilledVariableAndField(fieldName, it)) } postponedActions.add { with(instructions) { @@ -888,7 +861,7 @@ class CoroutineTransformerMethodVisitor( return } - private data class SpilledVariableDescriptor(val fieldName: String, val variableName: String) + private data class SpilledVariableAndField(val fieldName: String, val variableName: String) } internal fun InstructionAdapter.generateContinuationConstructorCall( 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 754e93fa676..0d680b49ca9 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 @@ -5,14 +5,10 @@ package org.jetbrains.kotlin.codegen.optimization.common +import org.jetbrains.kotlin.codegen.coroutines.SUSPEND_CALL_RESULT_NAME import org.jetbrains.kotlin.codegen.coroutines.SUSPEND_FUNCTION_COMPLETION_PARAMETER_NAME -import org.jetbrains.kotlin.codegen.optimization.transformer.MethodTransformer -import org.jetbrains.kotlin.load.java.JvmAbi -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.kotlin.codegen.inline.isFakeLocalVariableForInline +import org.jetbrains.org.objectweb.asm.tree.* import java.util.* @@ -39,10 +35,12 @@ class VariableLivenessFrame(val maxLocals: Int) : VarFrame = - analyze(node, object : BackwardAnalysisInterpreter { +fun analyzeLiveness(method: MethodNode): List = + analyze(method, object : BackwardAnalysisInterpreter { override fun newFrame(maxLocals: Int) = VariableLivenessFrame(maxLocals) override fun def(frame: VariableLivenessFrame, insn: AbstractInsnNode) = defVar(frame, insn) override fun use(frame: VariableLivenessFrame, insn: AbstractInsnNode) = @@ -61,4 +59,65 @@ private fun useVar(frame: VariableLivenessFrame, insn: AbstractInsnNode) { } else if (insn is IincInsnNode) { frame.markAlive(insn.`var`) } -} \ No newline at end of file +} + +/* We do not want to spill dead variables, thus, we shrink its LVT record to region, where the variable is alive, + * so, the variable will not be visible in debugger. User can still prolong life span of the variable by using it. + * + * This means, that function parameters do not longer span the whole function, including `this`. + * This might and will break some bytecode processors, including old versions of R8. See KT-24510. + */ +fun updateLvtAccordingToLiveness(method: MethodNode) { + val liveness = analyzeLiveness(method) + + fun List.findRecord(insnIndex: Int, variableIndex: Int): LocalVariableNode? { + for (variable in this) { + if (variable.index == variableIndex && + method.instructions.indexOf(variable.start) <= insnIndex && + insnIndex < method.instructions.indexOf(variable.end) + ) return variable + } + return null + } + + fun isAlive(insnIndex: Int, variableIndex: Int): Boolean = + liveness[insnIndex].isAlive(variableIndex) + + val oldLvt = arrayListOf() + for (record in method.localVariables) { + oldLvt += record + } + method.localVariables.clear() + for (variableIndex in 0 until method.maxLocals) { + if (oldLvt.none { it.index == variableIndex }) continue + var startLabel: LabelNode? = null + for (insnIndex in 0 until (method.instructions.size() - 1)) { + val insn = method.instructions[insnIndex] + if (!isAlive(insnIndex, variableIndex) && isAlive(insnIndex + 1, variableIndex)) { + startLabel = insn as? LabelNode ?: insn.findNextOrNull { it is LabelNode } as? LabelNode + } + if (isAlive(insnIndex, variableIndex) && !isAlive(insnIndex + 1, variableIndex)) { + // No variable in LVT -> do not add one + val lvtRecord = oldLvt.findRecord(insnIndex, variableIndex) ?: continue + val endLabel = insn as? LabelNode ?: insn.findNextOrNull { it is LabelNode } as? LabelNode ?: continue + // startLabel can be null in case of parameters + @Suppress("NAME_SHADOWING") val startLabel = startLabel ?: lvtRecord.start + // No LINENUMBER in range -> no way to put a breakpoint -> do not bother adding a record + if (InsnSequence(startLabel, endLabel).none { it is LineNumberNode }) continue + method.localVariables.add( + LocalVariableNode(lvtRecord.name, lvtRecord.desc, lvtRecord.signature, startLabel, endLabel, lvtRecord.index) + ) + } + } + } + + for (variable in oldLvt) { + // $completion and $result are dead, but they are used by debugger, as well as fake inliner variables + if (variable.name == SUSPEND_FUNCTION_COMPLETION_PARAMETER_NAME || + variable.name == SUSPEND_CALL_RESULT_NAME || + isFakeLocalVariableForInline(variable.name) + ) { + method.localVariables.add(variable) + } + } +} diff --git a/compiler/testData/codegen/bytecodeText/coroutines/debug/localVariableCorrectLabel.kt b/compiler/testData/codegen/bytecodeText/coroutines/debug/localVariableCorrectLabel.kt index 0e78d33f3b2..8974bbcc549 100644 --- a/compiler/testData/codegen/bytecodeText/coroutines/debug/localVariableCorrectLabel.kt +++ b/compiler/testData/codegen/bytecodeText/coroutines/debug/localVariableCorrectLabel.kt @@ -17,10 +17,9 @@ suspend fun SequenceScope.awaitSeq(): Int = 42 // label numbers differ in BEs // JVM_TEMPLATES -// 1 LOCALVARIABLE a I L[0-9]+ L20 -// 1 LINENUMBER 9 L20 +// 1 LOCALVARIABLE a I L[0-9]+ L18 +// 1 LINENUMBER 9 L19 /* TODO: JVM_IR does not generate LINENUMBER at the end of the lambda */ // JVM_IR_TEMPLATES -// 1 LOCALVARIABLE a I L[0-9]+ L5 -// 1 LINENUMBER 8 L14 +// 1 LOCALVARIABLE a I L[0-9]+ L16 diff --git a/compiler/testData/codegen/bytecodeText/coroutines/debug/shrinkLvtTopLevel.kt b/compiler/testData/codegen/bytecodeText/coroutines/debug/shrinkLvtTopLevel.kt new file mode 100644 index 00000000000..a072c28a570 --- /dev/null +++ b/compiler/testData/codegen/bytecodeText/coroutines/debug/shrinkLvtTopLevel.kt @@ -0,0 +1,9 @@ +suspend fun blackhole(a: Any) {} + +suspend fun topLevel(a: String, b: String) { + blackhole(a) // one spill + blackhole(b) // no spills +} + +// 1 PUTFIELD .*L\$0 : Ljava/lang/Object; +// 0 PUTFIELD .*L\$1 : Ljava/lang/Object; \ No newline at end of file diff --git a/compiler/testData/codegen/bytecodeText/coroutines/debug/thisAndResultInLvt.kt b/compiler/testData/codegen/bytecodeText/coroutines/debug/thisAndResultInLvt.kt index 24644724e05..814a3bdd596 100644 --- a/compiler/testData/codegen/bytecodeText/coroutines/debug/thisAndResultInLvt.kt +++ b/compiler/testData/codegen/bytecodeText/coroutines/debug/thisAndResultInLvt.kt @@ -5,37 +5,35 @@ val c: suspend () -> Unit = { dummy() } +fun blackhole(a: Any) {} + class A { suspend fun foo(a: A, s: String = "", block: suspend A.() -> Unit) { block() block() + blackhole(this) + blackhole(a) + blackhole(s) + blackhole(block) } } // BEs generate continuation classes differently, JVM_IR generates more correctly +// foo, c's lambda and foo's continuation +// 3 LOCALVARIABLE \$result Ljava/lang/Object; + +// foo and +// 2 LOCALVARIABLE this LA; +// 1 LOCALVARIABLE a LA; +// 1 LOCALVARIABLE s Ljava/lang/String; +// 1 LOCALVARIABLE block Lkotlin/jvm/functions/Function2; +// 1 LOCALVARIABLE \$continuation Lkotlin/coroutines/Continuation; + // JVM_TEMPLATES // invokeSuspend -// 1 LOCALVARIABLE this LThisAndResultInLvtKt\$c\$1; L0 L.* 0 -// c's lambda and foo's continuation -// 2 LOCALVARIABLE \$result Ljava/lang/Object; L0 L.* 1 - -// foo and -// 2 LOCALVARIABLE this LA; L0 L.* 0 -// 1 LOCALVARIABLE a LA; L0 L.* 1 -// 1 LOCALVARIABLE s Ljava/lang/String; L0 L.* 2 -// 1 LOCALVARIABLE block Lkotlin/jvm/functions/Function2; L0 L.* 3 -// 1 LOCALVARIABLE \$continuation Lkotlin/coroutines/Continuation; L2 L.* 6 +// 1 LOCALVARIABLE this LThisAndResultInLvtKt\$c\$1; // JVM_IR_TEMPLATES -// , invoke, invokeSuspend, create -// 4 LOCALVARIABLE this LThisAndResultInLvtKt\$c\$1; L0 L.* 0 -// c's lambda and foo's continuation -// 2 LOCALVARIABLE \$result Ljava/lang/Object; L0 L.* 1 - -// foo and -// 2 LOCALVARIABLE this LA; L0 L.* 0 -// 1 LOCALVARIABLE a LA; L0 L.* 1 -// 1 LOCALVARIABLE s Ljava/lang/String; L0 L.* 2 -// 1 LOCALVARIABLE block Lkotlin/jvm/functions/Function2; L0 L.* 3 -// 1 LOCALVARIABLE \$continuation Lkotlin/coroutines/Continuation; L2 L.* 6 \ No newline at end of file +// , invoke, invokeSuspend +// 3 LOCALVARIABLE this LThisAndResultInLvtKt\$c\$1; L0 L.* 0 diff --git a/compiler/testData/codegen/bytecodeText/coroutines/varValueConflictsWithTableSameSort.kt b/compiler/testData/codegen/bytecodeText/coroutines/varValueConflictsWithTableSameSort.kt index 0ea4632b030..bdfd1f67f0d 100644 --- a/compiler/testData/codegen/bytecodeText/coroutines/varValueConflictsWithTableSameSort.kt +++ b/compiler/testData/codegen/bytecodeText/coroutines/varValueConflictsWithTableSameSort.kt @@ -19,6 +19,8 @@ fun box(): String { try { var i: String = "abc" i = "123" + // We need to use the variable, otherwise, it is considered dead. + println(i) } finally { } // This variable should take the same slot as 'i' had @@ -40,7 +42,8 @@ fun box(): String { } // 1 LOCALVARIABLE i Ljava/lang/String; L.* 3 -// 1 LOCALVARIABLE s Ljava/lang/String; L.* 3 +// From liveness point of view, 's' is dead between 'println' and 's == "OK"', thus the range is split +// 2 LOCALVARIABLE s Ljava/lang/String; L.* 3 // 1 PUTFIELD VarValueConflictsWithTableSameSortKt\$box\$1.L\$0 : Ljava/lang/Object; /* 1 load in the catch (e: Throwable) { throw e } block which is implicitly wrapped around try/finally */ // 1 ALOAD 3\s+ATHROW @@ -48,7 +51,7 @@ fun box(): String { // 1 ALOAD 3\s+PUTFIELD kotlin/jvm/internal/Ref\$ObjectRef\.element /* 1 load in spill */ // 1 ALOAD 3\s+PUTFIELD VarValueConflictsWithTableSameSortKt\$box\$1\.L\$0 : Ljava/lang/Object; -/* 1 load in println(s) */ -// 1 ALOAD 3\s+INVOKEVIRTUAL java/io/PrintStream.println \(Ljava/lang/Object;\)V +/* 2 loads in println(s) */ +// 2 ALOAD 3\s+INVOKEVIRTUAL java/io/PrintStream.println \(Ljava/lang/Object;\)V /* But no further load when spilling 's' to the continuation */ -// 4 ALOAD 3 +// 5 ALOAD 3 diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java index 2f6e5601cb6..8a4c1d6bd0c 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java @@ -1478,6 +1478,11 @@ public class BytecodeTextTestGenerated extends AbstractBytecodeTextTest { runTest("compiler/testData/codegen/bytecodeText/coroutines/debug/probeCoroutineSuspended.kt"); } + @TestMetadata("shrinkLvtTopLevel.kt") + public void testShrinkLvtTopLevel() throws Exception { + runTest("compiler/testData/codegen/bytecodeText/coroutines/debug/shrinkLvtTopLevel.kt"); + } + @TestMetadata("thisAndResultInLvt.kt") public void testThisAndResultInLvt() throws Exception { runTest("compiler/testData/codegen/bytecodeText/coroutines/debug/thisAndResultInLvt.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java index 149b31a15f3..e7dc61889c7 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java @@ -7558,6 +7558,11 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes @TestDataPath("$PROJECT_ROOT") @RunWith(JUnit3RunnerWithInners.class) public static class Debug extends AbstractLightAnalysisModeTest { + @TestMetadata("debuggerMetadata_ir.kt") + public void ignoreDebuggerMetadata_ir() throws Exception { + runTest("compiler/testData/codegen/box/coroutines/debug/debuggerMetadata_ir.kt"); + } + private void runTest(String testDataFilePath) throws Exception { KotlinTestUtils.runTest(this::doTest, TargetBackend.JVM, testDataFilePath); } @@ -7571,11 +7576,6 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes runTest("compiler/testData/codegen/box/coroutines/debug/debuggerMetadata.kt"); } - @TestMetadata("debuggerMetadata_ir.kt") - public void testDebuggerMetadata_ir() throws Exception { - runTest("compiler/testData/codegen/box/coroutines/debug/debuggerMetadata_ir.kt"); - } - @TestMetadata("elvisLineNumber.kt") public void testElvisLineNumber() throws Exception { runTest("compiler/testData/codegen/box/coroutines/debug/elvisLineNumber.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBytecodeTextTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBytecodeTextTestGenerated.java index befed43539f..51aaea0811a 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBytecodeTextTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBytecodeTextTestGenerated.java @@ -1483,6 +1483,11 @@ public class IrBytecodeTextTestGenerated extends AbstractIrBytecodeTextTest { runTest("compiler/testData/codegen/bytecodeText/coroutines/debug/probeCoroutineSuspended.kt"); } + @TestMetadata("shrinkLvtTopLevel.kt") + public void testShrinkLvtTopLevel() throws Exception { + runTest("compiler/testData/codegen/bytecodeText/coroutines/debug/shrinkLvtTopLevel.kt"); + } + @TestMetadata("thisAndResultInLvt.kt") public void testThisAndResultInLvt() throws Exception { runTest("compiler/testData/codegen/bytecodeText/coroutines/debug/thisAndResultInLvt.kt");