azalea/kotlin-fork
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Shrink and split LVT records of variables according to their liveness

Browse Source 
Otherwise, debugger will show uninitialized values of dead variables
after resume.
 #KT-16222
 #KT-28016 Fixed
 #KT-20571 Fixed
This commit is contained in:
Ilmir Usmanov
2020-07-22 19:18:16 +02:00
parent e5995f0c12
commit 70e91bd5db
9 changed files with 130 additions and 79 deletions
Download Patch File Download Diff File Expand all files Collapse all files
compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/CoroutineTransformerMethodVisitor.kt
+8 -35
View File
@@ -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<LineNumberNode?>,
spilledToLocalMapping: List<List<SpilledVariableDescriptor>>
spilledToLocalMapping: List<List<SpilledVariableAndField>>
) {
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<SuspensionPoint>, methodNode: MethodNode): List<List<SpilledVariableDescriptor>> {
private fun spillVariables(suspensionPoints: List<SuspensionPoint>, methodNode: MethodNode): List<List<SpilledVariableAndField>> {
val instructions = methodNode.instructions
val frames =
if (useOldSpilledVarTypeAnalysis) performRefinedTypeAnalysis(methodNode, containingClassInternalName)
@@ -602,7 +575,7 @@ class CoroutineTransformerMethodVisitor(
val postponedActions = mutableListOf<() -> Unit>()
val maxVarsCountByType = mutableMapOf<Type, Int>()
val livenessFrames = analyzeLiveness(methodNode)
val spilledToVariableMapping = arrayListOf<List<SpilledVariableDescriptor>>()
val spilledToVariableMapping = arrayListOf<List<SpilledVariableAndField>>()
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<SpilledVariableDescriptor>()
val spilledToVariable = arrayListOf<SpilledVariableAndField>()
// 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(
compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/common/variableLiveness.kt
+69 -10
View File
@@ -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<VariableLivenessFrame
}
override fun hashCode() = bitSet.hashCode()
override fun toString(): String = (0 until maxLocals).map { if (bitSet[it]) '@' else '_' }.joinToString(separator = "")
}
fun analyzeLiveness(node: MethodNode): List<VariableLivenessFrame> =
analyze(node, object : BackwardAnalysisInterpreter<VariableLivenessFrame> {
fun analyzeLiveness(method: MethodNode): List<VariableLivenessFrame> =
analyze(method, object : BackwardAnalysisInterpreter<VariableLivenessFrame> {
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`)
}
}
}
/* 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<LocalVariableNode>.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<LocalVariableNode>()
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)
}
}
}
compiler/testData/codegen/bytecodeText/coroutines/debug/localVariableCorrectLabel.kt
Vendored
+3 -4
View File
@@ -17,10 +17,9 @@ suspend fun SequenceScope<Int>.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
compiler/testData/codegen/bytecodeText/coroutines/debug/shrinkLvtTopLevel.kt
Vendored
+9
View File
@@ -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;
compiler/testData/codegen/bytecodeText/coroutines/debug/thisAndResultInLvt.kt
Vendored
+19 -21
View File
@@ -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 <init>
// 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 <init>
// 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
// <init>, 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 <init>
// 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
// <init>, invoke, invokeSuspend
// 3 LOCALVARIABLE this LThisAndResultInLvtKt\$c\$1; L0 L.* 0
compiler/testData/codegen/bytecodeText/coroutines/varValueConflictsWithTableSameSort.kt
Vendored
+7 -4
View File
@@ -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
compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java
Generated
+5
View File
@@ -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");
compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java
Generated
+5 -5
View File
@@ -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");
compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBytecodeTextTestGenerated.java
Generated
+5
View File
@@ -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");