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

[JVM] Fix uninitialized locals after coroutine transformation.

Browse Source 
The coroutine transformation would leave locals in the local
variable table across the code that reloads local variables from
the continuation on reentry. However, when reentering the function
the local has no value until after the reloads from the
continuation.

This change splits the locals in the local variable table to
avoid such uninitialized locals. A local alive across a
suspension point has its range split in two. One that goes
from the original start to the state label for the restart
after the suspension. The other goes from after the local
has been reloaded from the continuation until the previous
end of the local.
This commit is contained in:
Mads Ager
2021-04-27 16:24:31 +02:00
committed by Ilmir Usmanov
parent 83e3a702c5
commit df225c0c7f
4 changed files with 81 additions and 40 deletions
Download Patch File Download Diff File Expand all files Collapse all files
compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/CoroutineTransformerMethodVisitor.kt
+72 -31
View File
@@ -650,7 +650,7 @@ class CoroutineTransformerMethodVisitor(
val livenessFrames = analyzeLiveness(methodNode)
// References shall be cleaned up after uspill (during spill in next suspension point) to prevent memory leaks,
// References shall be cleaned up after unspill (during spill in next suspension point) to prevent memory leaks,
val referencesToSpillBySuspensionPointIndex = arrayListOf<List<ReferenceToSpill>>()
// while primitives shall not
val primitivesToSpillBySuspensionPointIndex = arrayListOf<List<PrimitiveToSpill>>()
@@ -759,6 +759,35 @@ class CoroutineTransformerMethodVisitor(
referencesToCleanBySuspensionPointIndex += currentSpilledReferencesCount to predSpilledReferencesCount
}
// Calculate debug metadata mapping before modifying method node to make it easier to locate
// locals alive across suspension points.
fun calculateSpilledVariableAndField(
suspension: SuspensionPoint,
slot: Int,
spillableVariable: SpillableVariable?
): SpilledVariableAndField? {
if (spillableVariable == null) return null
val name = localVariableName(methodNode, slot, suspension.suspensionCallBegin.index()) ?: return null
return SpilledVariableAndField(spillableVariable.fieldName, name)
}
val spilledToVariableMapping = arrayListOf<List<SpilledVariableAndField>>()
for (suspensionPointIndex in suspensionPoints.indices) {
val suspension = suspensionPoints[suspensionPointIndex]
val spilledToVariable = arrayListOf<SpilledVariableAndField>()
referencesToSpillBySuspensionPointIndex[suspensionPointIndex].mapNotNullTo(spilledToVariable) { (slot, spillableVariable) ->
calculateSpilledVariableAndField(suspension, slot, spillableVariable)
}
primitivesToSpillBySuspensionPointIndex[suspensionPointIndex].mapNotNullTo(spilledToVariable) { (slot, spillableVariable) ->
calculateSpilledVariableAndField(suspension, slot, spillableVariable)
}
spilledToVariableMapping += spilledToVariable
}
// Mutate method node
fun generateSpillAndUnspill(suspension: SuspensionPoint, slot: Int, spillableVariable: SpillableVariable?) {
@@ -772,6 +801,22 @@ class CoroutineTransformerMethodVisitor(
return
}
// Find and remove the local variable node, if any, in the local variable table corresponding to the slot that is spilled.
var local: LocalVariableNode? = null
val localRestart = LabelNode().linkWithLabel()
val iterator = methodNode.localVariables.listIterator()
while (iterator.hasNext()) {
val node = iterator.next()
if (node.index == slot &&
methodNode.instructions.indexOf(node.start) <= methodNode.instructions.indexOf(suspension.suspensionCallBegin) &&
methodNode.instructions.indexOf(node.end) > methodNode.instructions.indexOf(suspension.tryCatchBlockEndLabelAfterSuspensionCall)
) {
local = node
iterator.remove()
break
}
}
with(instructions) {
// store variable before suspension call
insertBefore(suspension.suspensionCallBegin, withInstructionAdapter {
@@ -795,8 +840,31 @@ class CoroutineTransformerMethodVisitor(
)
StackValue.coerce(spillableVariable.normalizedType, spillableVariable.type, this)
store(slot, spillableVariable.type)
if (local != null) {
visitLabel(localRestart.label)
}
})
}
// Split the local variable range for the local so that it is visible until the next state label, but is
// not visible until it has been unspilled from the continuation on the reentry path.
if (local != null) {
val previousEnd = local.end
local.end = suspension.stateLabel
// Add the local back, but end it at the next state label.
methodNode.localVariables.add(local)
// Add a new entry that starts after the local variable is restored from the continuation.
methodNode.localVariables.add(
LocalVariableNode(
local.name,
local.desc,
local.signature,
localRestart,
previousEnd,
local.index
)
)
}
}
fun cleanUpField(suspension: SuspensionPoint, fieldIndex: Int) {
@@ -839,33 +907,6 @@ class CoroutineTransformerMethodVisitor(
}
}
// Calculate debug metadata mapping
fun calculateSpilledVariableAndField(
suspension: SuspensionPoint,
slot: Int,
spillableVariable: SpillableVariable?
): SpilledVariableAndField? {
if (spillableVariable == null) return null
val name = localVariableName(methodNode, slot, suspension.suspensionCallEnd.next.index()) ?: return null
return SpilledVariableAndField(spillableVariable.fieldName, name)
}
val spilledToVariableMapping = arrayListOf<List<SpilledVariableAndField>>()
for (suspensionPointIndex in suspensionPoints.indices) {
val suspension = suspensionPoints[suspensionPointIndex]
val spilledToVariable = arrayListOf<SpilledVariableAndField>()
referencesToSpillBySuspensionPointIndex[suspensionPointIndex].mapNotNullTo(spilledToVariable) { (slot, spillableVariable) ->
calculateSpilledVariableAndField(suspension, slot, spillableVariable)
}
primitivesToSpillBySuspensionPointIndex[suspensionPointIndex].mapNotNullTo(spilledToVariable) { (slot, spillableVariable) ->
calculateSpilledVariableAndField(suspension, slot, spillableVariable)
}
spilledToVariableMapping += spilledToVariable
}
return spilledToVariableMapping
}
@@ -901,7 +942,6 @@ class CoroutineTransformerMethodVisitor(
suspendMarkerVarIndex: Int,
suspendPointLineNumber: LineNumberNode?
): LabelNode {
val stateLabel = LabelNode().linkWithLabel()
val continuationLabelAfterLoadedResult = LabelNode()
val suspendElementLineNumber = lineNumber
var nextLineNumberNode = nextDefinitelyHitLineNumber(suspension)
@@ -929,7 +969,7 @@ class CoroutineTransformerMethodVisitor(
load(suspendMarkerVarIndex, AsmTypes.OBJECT_TYPE)
areturn(AsmTypes.OBJECT_TYPE)
// Mark place for continuation
visitLabel(stateLabel.label)
visitLabel(suspension.stateLabel.label)
})
// After suspension point there is always three nodes: L1, NOP, L2
@@ -985,7 +1025,7 @@ class CoroutineTransformerMethodVisitor(
}
}
return stateLabel
return suspension.stateLabel
}
// Find the next line number instruction that is defintely hit. That is, a line number
@@ -1154,6 +1194,7 @@ internal class SuspensionPoint(
) {
lateinit var tryCatchBlocksContinuationLabel: LabelNode
val stateLabel = LabelNode().linkWithLabel()
val unboxInlineClassInstructions: List<AbstractInsnNode> = findUnboxInlineClassInstructions()
private fun findUnboxInlineClassInstructions(): List<AbstractInsnNode> {
compiler/testData/codegen/bytecodeText/coroutines/debug/thisAndResultInLvt.kt
Vendored
+5 -5
View File
@@ -23,11 +23,11 @@ class A {
// 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;
// foo x 3 since we split the local over restore code for the two calls to block(), and <init>
// 4 LOCALVARIABLE this LA;
// 3 LOCALVARIABLE a LA;
// 3 LOCALVARIABLE s Ljava/lang/String;
// 3 LOCALVARIABLE block Lkotlin/jvm/functions/Function2;
// 1 LOCALVARIABLE \$continuation Lkotlin/coroutines/Continuation;
// JVM_TEMPLATES
compiler/testData/codegen/bytecodeText/coroutines/varValueConflictsWithTableSameSort.kt
Vendored
+2 -2
View File
@@ -42,8 +42,8 @@ fun box(): String {
}
// 1 LOCALVARIABLE i Ljava/lang/String; L.* 3
// We merge LVT records for two consequent branches.
// 1 LOCALVARIABLE s Ljava/lang/String; L.* 3
// We merge LVT records for two consequent branches, but we split the local over the restore code.
// 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
compiler/testData/debug/localVariables/suspend/completion/staticStateMachineReceiver.kt
Vendored
+2 -2
View File
@@ -32,10 +32,10 @@ suspend fun box() {
// test.kt:8 foo1: $continuation:kotlin.coroutines.Continuation=TestKt$foo1$1, $result:java.lang.Object=null, $this$foo1:A=A, l:long=42:long
// test.kt:6 foo: $this$foo:A=A, $completion:kotlin.coroutines.Continuation=TestKt$foo1$1
// test.kt:8 foo1: $continuation:kotlin.coroutines.Continuation=TestKt$foo1$1, $result:java.lang.Object=null, $this$foo1:A=A, l:long=42:long
// test.kt:9 foo1: $continuation:kotlin.coroutines.Continuation=TestKt$foo1$1, $result:java.lang.Object=null, $this$foo1:A=A, l:long=42:long
// test.kt:9 foo1: $continuation:kotlin.coroutines.Continuation=TestKt$foo1$1, $result:java.lang.Object=null, l:long=42:long, $this$foo1:A=A
// 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: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