From e17b7f01b64bc77b293b9b5391241c738c2e8072 Mon Sep 17 00:00:00 2001 From: Ilmir Usmanov Date: Mon, 1 Mar 2021 22:30:20 +0100 Subject: [PATCH] Do not remove dead variables' LVT records Also, extend liveness of alive variable to nearest suspension points #KT-44714 --- .../CoroutineTransformerMethodVisitor.kt | 61 +++++++++++++++++-- .../suspendFunctionDeadVariables.kt | 14 +++++ .../debug/localVariableCorrectLabel.kt | 2 +- .../completion/nonStaticStateMachine.kt | 2 +- .../suspend/completion/staticStateMachine.kt | 2 +- .../completion/staticStateMachineReceiver.kt | 2 +- .../suspend/localsStateMachineTransform.kt | 8 +-- .../localVariables/suspend/underscoreNames.kt | 2 +- ...CheckLocalVariablesTableTestGenerated.java | 5 ++ ...CheckLocalVariablesTableTestGenerated.java | 5 ++ 10 files changed, 89 insertions(+), 14 deletions(-) create mode 100644 compiler/testData/checkLocalVariablesTable/suspendFunctionDeadVariables.kt 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 eaa421dd181..627d2a4eaee 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/CoroutineTransformerMethodVisitor.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/CoroutineTransformerMethodVisitor.kt @@ -1217,6 +1217,27 @@ private fun updateLvtAccordingToLiveness(method: MethodNode, isForNamedFunction: fun isAlive(insnIndex: Int, variableIndex: Int): Boolean = liveness[insnIndex].isAlive(variableIndex) + fun nextSuspensionPointLabel(insn: AbstractInsnNode): LabelNode { + val suspensionPoint = + InsnSequence(insn, method.instructions.last).firstOrNull { isBeforeSuspendMarker(it) } ?: method.instructions.last + return suspensionPoint as? LabelNode ?: suspensionPoint.findPreviousOrNull { it is LabelNode } as LabelNode + } + + fun previousSuspensionPointLabel(insn: AbstractInsnNode): LabelNode { + val suspensionPoint = + InsnSequence(method.instructions.first, insn).lastOrNull { isAfterSuspendMarker(it) } ?: method.instructions.first + return suspensionPoint as? LabelNode ?: suspensionPoint.findNextOrNull { it is LabelNode } as LabelNode + } + + fun min(a: LabelNode, b: LabelNode): LabelNode = + if (method.instructions.indexOf(a) < method.instructions.indexOf(b)) a else b + + fun max(a: LabelNode, b: LabelNode): LabelNode = + if (method.instructions.indexOf(a) < method.instructions.indexOf(b)) b else a + + fun containsSuspensionPoint(a: LabelNode, b: LabelNode): Boolean = + InsnSequence(min(a, b), max(a, b)).none { isBeforeSuspendMarker(it) } + val oldLvt = arrayListOf() for (record in method.localVariables) { oldLvt += record @@ -1231,29 +1252,43 @@ private fun updateLvtAccordingToLiveness(method: MethodNode, isForNamedFunction: 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 + val lvtRecord = oldLvt.findRecord(insnIndex, variableIndex) ?: continue + startLabel = max(lvtRecord.start, previousSuspensionPointLabel(insn)) } if (isAlive(insnIndex, variableIndex) && !isAlive(insnIndex + 1, variableIndex)) { // No variable in LVT -> do not add one val lvtRecord = oldLvt.findRecord(insnIndex, variableIndex) ?: continue if (lvtRecord.name == CONTINUATION_VARIABLE_NAME) continue - val endLabel = insn as? LabelNode ?: insn.findNextOrNull { it is LabelNode } as? LabelNode ?: continue + // Extend lvt record to the next suspension point + val endLabel = min(lvtRecord.end, nextSuspensionPointLabel(insn)) // startLabel can be null in case of parameters @Suppress("NAME_SHADOWING") val startLabel = startLabel ?: lvtRecord.start // Attempt to extend existing local variable node corresponding to the record in // the original local variable table. - var recordToExtend: LocalVariableNode? = oldLvtNodeToLatestNewLvtNode[lvtRecord] - if (recordToExtend != null && InsnSequence(recordToExtend.end, startLabel).none { isBeforeSuspendMarker(it) }) { + val recordToExtend: LocalVariableNode? = oldLvtNodeToLatestNewLvtNode[lvtRecord] + if (recordToExtend != null && containsSuspensionPoint(recordToExtend.end, startLabel)) { recordToExtend.end = endLabel } else { val node = LocalVariableNode(lvtRecord.name, lvtRecord.desc, lvtRecord.signature, startLabel, endLabel, lvtRecord.index) - method.localVariables.add(node) + if (lvtRecord !in oldLvtNodeToLatestNewLvtNode) { + method.localVariables.add(node) + } oldLvtNodeToLatestNewLvtNode[lvtRecord] = node } } } } + val deadVariables = arrayListOf() + outer@for (variableIndex in start until method.maxLocals) { + if (oldLvt.none { it.index == variableIndex }) continue + if (oldLvt.none { it.index == variableIndex }) continue + for (insnIndex in 0 until (method.instructions.size() - 1)) { + if (isAlive(insnIndex, variableIndex)) continue@outer + } + deadVariables += variableIndex + } + for (variable in oldLvt) { // $continuation and $result are dead, but they are used by debugger, as well as fake inliner variables // For example, $continuation is used to create async stack trace @@ -1262,10 +1297,26 @@ private fun updateLvtAccordingToLiveness(method: MethodNode, isForNamedFunction: isFakeLocalVariableForInline(variable.name) ) { method.localVariables.add(variable) + continue } // this acts like $continuation for lambdas. For example, it is used by debugger to create async stack trace. Keep it. if (variable.name == "this" && !isForNamedFunction) { method.localVariables.add(variable) + continue + } + + // Shrink LVT records of dead variables to the next suspension point + if (variable.index in deadVariables) { + method.localVariables.add( + LocalVariableNode( + variable.name, + variable.desc, + variable.signature, + variable.start, + nextSuspensionPointLabel(variable.start), + variable.index + ) + ) } } } diff --git a/compiler/testData/checkLocalVariablesTable/suspendFunctionDeadVariables.kt b/compiler/testData/checkLocalVariablesTable/suspendFunctionDeadVariables.kt new file mode 100644 index 00000000000..2999a4f1a7d --- /dev/null +++ b/compiler/testData/checkLocalVariablesTable/suspendFunctionDeadVariables.kt @@ -0,0 +1,14 @@ +// WITH_RUNTIME + +suspend fun dummy() {} + +suspend fun test() { + dummy() + val a = 0 +} + +// METHOD : SuspendFunctionDeadVariablesKt.test(Lkotlin/coroutines/Continuation;)Ljava/lang/Object; + +// VARIABLE : NAME=a TYPE=I INDEX=1 +// VARIABLE : NAME=$continuation TYPE=Lkotlin/coroutines/Continuation; INDEX=3 +// VARIABLE : NAME=$result TYPE=Ljava/lang/Object; INDEX=2 \ No newline at end of file diff --git a/compiler/testData/codegen/bytecodeText/coroutines/debug/localVariableCorrectLabel.kt b/compiler/testData/codegen/bytecodeText/coroutines/debug/localVariableCorrectLabel.kt index b10d4d2438d..9d69e09e4c0 100644 --- a/compiler/testData/codegen/bytecodeText/coroutines/debug/localVariableCorrectLabel.kt +++ b/compiler/testData/codegen/bytecodeText/coroutines/debug/localVariableCorrectLabel.kt @@ -15,7 +15,7 @@ fun main(args: Array) { suspend fun SequenceScope.awaitSeq(): Int = 42 -// 1 LOCALVARIABLE a I L[0-9]+ L18 +// 1 LOCALVARIABLE a I L[0-9]+ L4 /* TODO: JVM_IR does not generate LINENUMBER at the end of the lambda */ // JVM_TEMPLATES diff --git a/compiler/testData/debug/localVariables/suspend/completion/nonStaticStateMachine.kt b/compiler/testData/debug/localVariables/suspend/completion/nonStaticStateMachine.kt index 8046fac136a..4e7fb338122 100644 --- a/compiler/testData/debug/localVariables/suspend/completion/nonStaticStateMachine.kt +++ b/compiler/testData/debug/localVariables/suspend/completion/nonStaticStateMachine.kt @@ -36,6 +36,6 @@ suspend fun box() { // test.kt:5 foo: $completion:kotlin.coroutines.Continuation=A$foo1$1 // test.kt:8 foo1: $continuation:kotlin.coroutines.Continuation=A$foo1$1, $result:java.lang.Object=null, l:long=42:long // test.kt:9 foo1: $continuation:kotlin.coroutines.Continuation=A$foo1$1, $result:java.lang.Object=null, l:long=42:long -// test.kt:10 foo1: $continuation:kotlin.coroutines.Continuation=A$foo1$1, $result:java.lang.Object=null +// test.kt:10 foo1: $continuation:kotlin.coroutines.Continuation=A$foo1$1, $result:java.lang.Object=null, l:long=42:long, dead:long=42:long // test.kt:14 box: $completion:kotlin.coroutines.Continuation=helpers.ResultContinuation // test.kt:15 box: $completion:kotlin.coroutines.Continuation=helpers.ResultContinuation \ No newline at end of file diff --git a/compiler/testData/debug/localVariables/suspend/completion/staticStateMachine.kt b/compiler/testData/debug/localVariables/suspend/completion/staticStateMachine.kt index 40d7c3024f7..6b387ac6457 100644 --- a/compiler/testData/debug/localVariables/suspend/completion/staticStateMachine.kt +++ b/compiler/testData/debug/localVariables/suspend/completion/staticStateMachine.kt @@ -32,6 +32,6 @@ suspend fun box() { // test.kt:4 foo: $completion:kotlin.coroutines.Continuation=TestKt$foo1$1 // test.kt:7 foo1: $continuation:kotlin.coroutines.Continuation=TestKt$foo1$1, $result:java.lang.Object=null, l:long=42:long // test.kt:8 foo1: $continuation:kotlin.coroutines.Continuation=TestKt$foo1$1, $result:java.lang.Object=null, l:long=42:long -// test.kt:9 foo1: $continuation:kotlin.coroutines.Continuation=TestKt$foo1$1, $result:java.lang.Object=null +// test.kt:9 foo1: $continuation:kotlin.coroutines.Continuation=TestKt$foo1$1, $result:java.lang.Object=null, l:long=42:long, dead:long=42:long // test.kt:12 box: $completion:kotlin.coroutines.Continuation=helpers.ResultContinuation // test.kt:13 box: $completion:kotlin.coroutines.Continuation=helpers.ResultContinuation \ No newline at end of file diff --git a/compiler/testData/debug/localVariables/suspend/completion/staticStateMachineReceiver.kt b/compiler/testData/debug/localVariables/suspend/completion/staticStateMachineReceiver.kt index 45a5a3bcfa2..91ebc4dd6c7 100644 --- a/compiler/testData/debug/localVariables/suspend/completion/staticStateMachineReceiver.kt +++ b/compiler/testData/debug/localVariables/suspend/completion/staticStateMachineReceiver.kt @@ -36,6 +36,6 @@ suspend fun box() { // test.kt:6 foo: $this$foo:A=A, $completion:kotlin.coroutines.Continuation=TestKt$foo1$1 // test.kt:9 foo1: $continuation:kotlin.coroutines.Continuation=TestKt$foo1$1, $result:java.lang.Object=null, l:long=42:long // test.kt:10 foo1: $continuation:kotlin.coroutines.Continuation=TestKt$foo1$1, $result:java.lang.Object=null, l:long=42:long -// test.kt:11 foo1: $continuation:kotlin.coroutines.Continuation=TestKt$foo1$1, $result:java.lang.Object=null +// test.kt:11 foo1: $continuation:kotlin.coroutines.Continuation=TestKt$foo1$1, $result:java.lang.Object=null, l:long=42:long, dead:long=42:long // test.kt:14 box: $completion:kotlin.coroutines.Continuation=helpers.ResultContinuation // test.kt:15 box: $completion:kotlin.coroutines.Continuation=helpers.ResultContinuation \ No newline at end of file diff --git a/compiler/testData/debug/localVariables/suspend/localsStateMachineTransform.kt b/compiler/testData/debug/localVariables/suspend/localsStateMachineTransform.kt index a68f0adf6a5..c29155d9f0a 100644 --- a/compiler/testData/debug/localVariables/suspend/localsStateMachineTransform.kt +++ b/compiler/testData/debug/localVariables/suspend/localsStateMachineTransform.kt @@ -31,19 +31,19 @@ suspend fun box() { // test.kt:11 box: $continuation:kotlin.coroutines.Continuation=TestKt$box$1, $result:java.lang.Object=null, x:int=0:int // test.kt:5 f: x:int=0:int // test.kt:11 box: $continuation:kotlin.coroutines.Continuation=TestKt$box$1, $result:java.lang.Object=null, x:int=0:int -// test.kt:10 box: $continuation:kotlin.coroutines.Continuation=TestKt$box$1, $result:java.lang.Object=null +// test.kt:10 box: $continuation:kotlin.coroutines.Continuation=TestKt$box$1, $result:java.lang.Object=null, x:int=0:int // test.kt:11 box: $continuation:kotlin.coroutines.Continuation=TestKt$box$1, $result:java.lang.Object=null, x:int=1:int // test.kt:5 f: x:int=1:int // test.kt:11 box: $continuation:kotlin.coroutines.Continuation=TestKt$box$1, $result:java.lang.Object=null, x:int=1:int -// test.kt:10 box: $continuation:kotlin.coroutines.Continuation=TestKt$box$1, $result:java.lang.Object=null +// test.kt:10 box: $continuation:kotlin.coroutines.Continuation=TestKt$box$1, $result:java.lang.Object=null, x:int=1:int // test.kt:14 box: $continuation:kotlin.coroutines.Continuation=TestKt$box$1, $result:java.lang.Object=null // test.kt:15 box: $continuation:kotlin.coroutines.Continuation=TestKt$box$1, $result:java.lang.Object=null // test.kt:16 box: $continuation:kotlin.coroutines.Continuation=TestKt$box$1, $result:java.lang.Object=null, x:int=0:int // test.kt:5 f: x:int=0:int // test.kt:16 box: $continuation:kotlin.coroutines.Continuation=TestKt$box$1, $result:java.lang.Object=null, x:int=0:int -// test.kt:15 box: $continuation:kotlin.coroutines.Continuation=TestKt$box$1, $result:java.lang.Object=null +// test.kt:15 box: $continuation:kotlin.coroutines.Continuation=TestKt$box$1, $result:java.lang.Object=null, x:int=0:int // test.kt:16 box: $continuation:kotlin.coroutines.Continuation=TestKt$box$1, $result:java.lang.Object=null, x:int=1:int // test.kt:5 f: x:int=1:int // test.kt:16 box: $continuation:kotlin.coroutines.Continuation=TestKt$box$1, $result:java.lang.Object=null, x:int=1:int -// test.kt:15 box: $continuation:kotlin.coroutines.Continuation=TestKt$box$1, $result:java.lang.Object=null +// test.kt:15 box: $continuation:kotlin.coroutines.Continuation=TestKt$box$1, $result:java.lang.Object=null, x:int=1:int // test.kt:18 box: $continuation:kotlin.coroutines.Continuation=TestKt$box$1, $result:java.lang.Object=null diff --git a/compiler/testData/debug/localVariables/suspend/underscoreNames.kt b/compiler/testData/debug/localVariables/suspend/underscoreNames.kt index 5f1f788120a..e7c4d05bb85 100644 --- a/compiler/testData/debug/localVariables/suspend/underscoreNames.kt +++ b/compiler/testData/debug/localVariables/suspend/underscoreNames.kt @@ -45,7 +45,7 @@ suspend fun box() = foo(A()) { (x_param, _, y_param) -> // test.kt:12 invokeSuspend: $result:java.lang.Object=kotlin.Unit, $dstr$x_param$_u24__u24$y_param:A=A, x_param:java.lang.String="O":java.lang.String // LOCAL VARIABLES -// test.kt:13 invokeSuspend: $result:java.lang.Object=kotlin.Unit, x_param:java.lang.String="O":java.lang.String, y_param:java.lang.String="K":java.lang.String +// test.kt:13 invokeSuspend: $result:java.lang.Object=kotlin.Unit, $dstr$x_param$_u24__u24$y_param:A=A, x_param:java.lang.String="O":java.lang.String, y_param:java.lang.String="K":java.lang.String // LOCAL VARIABLES JVM // test.kt:-1 invoke: diff --git a/compiler/tests-gen/org/jetbrains/kotlin/codegen/CheckLocalVariablesTableTestGenerated.java b/compiler/tests-gen/org/jetbrains/kotlin/codegen/CheckLocalVariablesTableTestGenerated.java index 9c0bc844e66..31b01d58d40 100644 --- a/compiler/tests-gen/org/jetbrains/kotlin/codegen/CheckLocalVariablesTableTestGenerated.java +++ b/compiler/tests-gen/org/jetbrains/kotlin/codegen/CheckLocalVariablesTableTestGenerated.java @@ -80,6 +80,11 @@ public class CheckLocalVariablesTableTestGenerated extends AbstractCheckLocalVar runTest("compiler/testData/checkLocalVariablesTable/objectInLocalPropertyDelegate.kt"); } + @TestMetadata("suspendFunctionDeadVariables.kt") + public void testSuspendFunctionDeadVariables() throws Exception { + runTest("compiler/testData/checkLocalVariablesTable/suspendFunctionDeadVariables.kt"); + } + @TestMetadata("underscoreNames.kt") public void testUnderscoreNames() throws Exception { runTest("compiler/testData/checkLocalVariablesTable/underscoreNames.kt"); diff --git a/compiler/tests-gen/org/jetbrains/kotlin/codegen/ir/IrCheckLocalVariablesTableTestGenerated.java b/compiler/tests-gen/org/jetbrains/kotlin/codegen/ir/IrCheckLocalVariablesTableTestGenerated.java index dca584a14d3..a72cb71b204 100644 --- a/compiler/tests-gen/org/jetbrains/kotlin/codegen/ir/IrCheckLocalVariablesTableTestGenerated.java +++ b/compiler/tests-gen/org/jetbrains/kotlin/codegen/ir/IrCheckLocalVariablesTableTestGenerated.java @@ -80,6 +80,11 @@ public class IrCheckLocalVariablesTableTestGenerated extends AbstractIrCheckLoca runTest("compiler/testData/checkLocalVariablesTable/objectInLocalPropertyDelegate.kt"); } + @TestMetadata("suspendFunctionDeadVariables.kt") + public void testSuspendFunctionDeadVariables() throws Exception { + runTest("compiler/testData/checkLocalVariablesTable/suspendFunctionDeadVariables.kt"); + } + @TestMetadata("underscoreNames.kt") public void testUnderscoreNames() throws Exception { runTest("compiler/testData/checkLocalVariablesTable/underscoreNames.kt");