[JVM] Fix various undefined locals issues.

CoroutineTransformermethodVisitor attempts to extend the ranges
of local variables in various situations. Probably in an attempt
to give a better debugging experience. However, all of these
range extensions lead to invalid local variable tables where
something is in the local variable table where nothing is in the
corresponding slot.

The code that extends variables to the next suspension point
instead of ending them when they are no longer live has issues
with loops. When resuming and reentering the loop, the locals
table will mention a local that we did not spill and which
is therefore not restored when resuming.

The code that extends local variable table entries if there
are no suspension points between two entries doesn't work
for code such as:

```
var s: String
if (suspendHere() == "OK") {
  s = "OK"
} else {
  s = "FAIL"
}
```

If the local variable ranges are collapsed into one, one of
the branches will have the local defined in the local variable
table before the slot is initialized.

build.gradle.kts

@@ -182,7 +182,7 @@ extra["versions.jflex"] = "1.7.0"
 extra["versions.markdown"] = "0.1.25"
 extra["versions.trove4j"] = "1.0.20181211"
 extra["versions.completion-ranking-kotlin"] = "0.1.3"
-extra["versions.r8"] = "2.1.96"
+extra["versions.r8"] = "2.2.64"
 val immutablesVersion = "0.3.1"
 extra["versions.kotlinx-collections-immutable"] = immutablesVersion
 extra["versions.kotlinx-collections-immutable-jvm"] = immutablesVersion

compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/CoroutineTransformerMethodVisitor.kt

@@ -1276,16 +1276,13 @@ private fun updateLvtAccordingToLiveness(method: MethodNode, isForNamedFunction:
     fun isAlive(insnIndex: Int, variableIndex: Int): Boolean =
         liveness[insnIndex].isAlive(variableIndex)
 
-    fun nextSuspensionPointEndLabel(insn: AbstractInsnNode): LabelNode {
-        val suspensionPoint =
-            InsnSequence(insn, method.instructions.last).firstOrNull { isAfterSuspendMarker(it) } ?: method.instructions.last
-        return suspensionPoint as? LabelNode ?: suspensionPoint.findNextOrNull { it is LabelNode } as LabelNode
-    }
-
-    fun nextSuspensionPointStartLabel(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 nextLabel(node: AbstractInsnNode?): LabelNode? {
+        var current = node
+        while (current != null) {
+            if (current is LabelNode) return current
+            current = current.next
+        }
+        return null
     }
 
     fun min(a: LabelNode, b: LabelNode): LabelNode =
@@ -1294,9 +1291,6 @@ private fun updateLvtAccordingToLiveness(method: MethodNode, isForNamedFunction:
     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<LocalVariableNode>()
     for (record in method.localVariables) {
         oldLvt += record
@@ -1317,35 +1311,40 @@ private fun updateLvtAccordingToLiveness(method: MethodNode, isForNamedFunction:
                 // No variable in LVT -> do not add one
                 val lvtRecord = oldLvt.findRecord(insnIndex, variableIndex) ?: continue
                 if (lvtRecord.name == CONTINUATION_VARIABLE_NAME) continue
-                // Extend lvt record to the next suspension point
-                val endLabel = min(lvtRecord.end, nextSuspensionPointEndLabel(insn))
+                // End the local when it is no longer live. Since it is not live, we will not spill and unspill it across
+                // suspension points. It is tempting to keep it alive until the next suspension point to leave it visible in
+                // the debugger for as long as possible. However, in the case of loops, the resumption after suspension can
+                // have a backwards edge targeting instruction between the point of death and the next suspension point.
+                //
+                // For example, code such as the following:
+                //
+                //    listOf<String>.forEach {
+                //       yield(it)
+                //    }
+                //
+                // Generates code of this form with a back edge after resumption that will lead to invalid locals tables
+                // if the local range is extended to the next suspension point.
+                //
+                //        iterator = iterable.iterator()
+                //    L1: (iterable dies here)
+                //        load iterator.next if there
+                //        yield suspension point
+                //
+                //    L2: (resumption point)
+                //        restore live variables (not including iterable)
+                //        goto L1 (iterator not restored here, so we cannot not have iterator live at L1)
+                val endLabel = nextLabel(insn.next)?.let { min(lvtRecord.end, it) } ?: lvtRecord.end
                 // 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.
-                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)
-                    if (lvtRecord !in oldLvtNodeToLatestNewLvtNode) {
-                        method.localVariables.add(node)
-                    }
-                    oldLvtNodeToLatestNewLvtNode[lvtRecord] = node
+                val node = LocalVariableNode(lvtRecord.name, lvtRecord.desc, lvtRecord.signature, startLabel, endLabel, lvtRecord.index)
+                if (lvtRecord !in oldLvtNodeToLatestNewLvtNode) {
+                    method.localVariables.add(node)
                 }
+                oldLvtNodeToLatestNewLvtNode[lvtRecord] = node
             }
         }
     }
 
-    val deadVariables = arrayListOf<Int>()
-    outer@for (variableIndex in start until method.maxLocals) {
-        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
@@ -1361,19 +1360,5 @@ private fun updateLvtAccordingToLiveness(method: MethodNode, 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,
-                    min(variable.end, nextSuspensionPointStartLabel(variable.start)),
-                    variable.index
-                )
-            )
-        }
     }
 }

compiler/testData/checkLocalVariablesTable/suspendFunctionDeadVariables.kt

@@ -9,6 +9,5 @@ suspend fun test() {
 
 // 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
+// VARIABLE : NAME=$result TYPE=Ljava/lang/Object; INDEX=2

compiler/testData/codegen/bytecodeText/coroutines/debug/localVariableCorrectLabel.kt

@@ -15,12 +15,6 @@ fun main(args: Array<String>) {
 suspend fun SequenceScope<Int>.awaitSeq(): Int = 42
 
 // 1 LINENUMBER 9 L19
-
-// JVM_IR_TEMPLATES
-// 1 LOCALVARIABLE a I L[0-9]+ L4
-
-// JVM_TEMPLATES
-// 1 LOCALVARIABLE a I L[0-9]+ L19
-// TODO: Old BE generates LINENUMBER label after suspension point, unlike JVM_BE
+// 1 LOCALVARIABLE a I L[0-9]+ L18
 
 // IGNORE_BACKEND_FIR: JVM_IR

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, l:long=42:long, dead:long=42:long
+// test.kt:10 foo1: $continuation:kotlin.coroutines.Continuation=A$foo1$1, $result:java.lang.Object=null
 // test.kt:14 box: $completion:kotlin.coroutines.Continuation=helpers.ResultContinuation
-// test.kt:15 box: $completion:kotlin.coroutines.Continuation=helpers.ResultContinuation
+// test.kt:15 box: $completion:kotlin.coroutines.Continuation=helpers.ResultContinuation

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, l:long=42:long, dead:long=42:long
+// test.kt:9 foo1: $continuation:kotlin.coroutines.Continuation=TestKt$foo1$1, $result:java.lang.Object=null
 // test.kt:12 box: $completion:kotlin.coroutines.Continuation=helpers.ResultContinuation
-// test.kt:13 box: $completion:kotlin.coroutines.Continuation=helpers.ResultContinuation
+// test.kt:13 box: $completion:kotlin.coroutines.Continuation=helpers.ResultContinuation

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, l:long=42:long, dead:long=42:long
+// test.kt:11 foo1: $continuation:kotlin.coroutines.Continuation=TestKt$foo1$1, $result:java.lang.Object=null
 // test.kt:14 box: $completion:kotlin.coroutines.Continuation=helpers.ResultContinuation
 // test.kt:15 box: $completion:kotlin.coroutines.Continuation=helpers.ResultContinuation

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, x:int=0:int
+// test.kt:10 box: $continuation:kotlin.coroutines.Continuation=TestKt$box$1, $result:java.lang.Object=null
 // 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, x:int=1:int
+// test.kt:10 box: $continuation:kotlin.coroutines.Continuation=TestKt$box$1, $result:java.lang.Object=null
 // 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, x:int=0:int
+// 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=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, x:int=1:int
+// test.kt:15 box: $continuation:kotlin.coroutines.Continuation=TestKt$box$1, $result:java.lang.Object=null
 // test.kt:18 box: $continuation:kotlin.coroutines.Continuation=TestKt$box$1, $result:java.lang.Object=null

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, $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
+// 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
 
 // LOCAL VARIABLES JVM
 // test.kt:-1 invoke:
@@ -55,4 +55,4 @@ suspend fun box() = foo(A()) { (x_param, _, y_param) ->
 
 // LOCAL VARIABLES
 // test.kt:10 foo: a:A=A, block:kotlin.jvm.functions.Function2=TestKt$box$2, $completion:kotlin.coroutines.Continuation=helpers.ResultContinuation
-// test.kt:14 box: $completion:kotlin.coroutines.Continuation=helpers.ResultContinuation
+// test.kt:14 box: $completion:kotlin.coroutines.Continuation=helpers.ResultContinuation

gradle/verification-metadata.xml

@@ -333,10 +333,10 @@
             <sha256 value="6232de47feacc346126def6e6255759080a0e71fa5a11862bcada6dd8e0d4bbe" origin="Generated by Gradle"/>
          </artifact>
       </component>
-      <component group="com.android.tools" name="r8" version="2.1.96">
-         <artifact name="r8-2.1.96.jar">
-            <md5 value="02d84103577d5d145a6146d889c4dada" origin="Generated by Gradle"/>
-            <sha256 value="0b1ba7410a2f85737834429c1602f2211d0e2d8c10b21f73fb7c7dff0c02857a" origin="Generated by Gradle"/>
+      <component group="com.android.tools" name="r8" version="2.2.64">
+         <artifact name="r8-2.2.64.jar">
+            <md5 value="33a12f64ca1e4266bffe3772d798d046" origin="Generated by Gradle"/>
+            <sha256 value="5665e292c435ada26c1afde71f4d893303594c6e35648039f7c3fa31d9ecdff5" origin="Generated by Gradle"/>
          </artifact>
       </component>
       <component group="com.android.tools" name="repository" version="26.0.0">