Shrink and split LVT records of variables according to their liveness

Otherwise, debugger will show uninitialized values of dead variables
after resume.
 #KT-16222
 #KT-28016 Fixed
 #KT-20571 Fixed

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<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

@@ -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)
+        }
+    }
+}