JVM: Improve line number handling for suspend calls.

Take branching and method calls into account when finding the line
number of the continuation. If there is no line number before
branching instructions or method calls, the following code is
still on the line of the suspend call itself.

This fixes a couple of issues with incorrect line numbers for
multiple throws on the same line or multipe suspend calls on
the same line.

In addition, it avoids the need to spam the method node with
repeated line number instructions in the IR backend.
This commit is contained in:
Mads Ager
2019-10-25 14:17:42 +02:00
committed by Ilmir Usmanov
parent 34d9959b17
commit 1713625718
9 changed files with 209 additions and 22 deletions
@@ -11,7 +11,6 @@ import org.jetbrains.kotlin.codegen.ClassBuilder
import org.jetbrains.kotlin.codegen.StackValue
import org.jetbrains.kotlin.codegen.TransformationMethodVisitor
import org.jetbrains.kotlin.codegen.inline.*
import org.jetbrains.kotlin.codegen.optimization.DeadCodeEliminationMethodTransformer
import org.jetbrains.kotlin.codegen.optimization.boxing.isUnitInstance
import org.jetbrains.kotlin.codegen.optimization.common.*
import org.jetbrains.kotlin.codegen.optimization.fixStack.FixStackMethodTransformer
@@ -146,7 +145,8 @@ class CoroutineTransformerMethodVisitor(
val suspensionPointLineNumbers = suspensionPoints.map { findSuspensionPointLineNumber(it) }
val continuationLabels = suspensionPoints.withIndex().map {
transformCallAndReturnContinuationLabel(it.index + 1, it.value, methodNode, suspendMarkerVarIndex)
transformCallAndReturnContinuationLabel(
it.index + 1, it.value, methodNode, suspendMarkerVarIndex, suspensionPointLineNumbers[it.index])
}
methodNode.instructions.apply {
@@ -702,12 +702,13 @@ class CoroutineTransformerMethodVisitor(
id: Int,
suspension: SuspensionPoint,
methodNode: MethodNode,
suspendMarkerVarIndex: Int
suspendMarkerVarIndex: Int,
suspendPointLineNumber: LineNumberNode?
): LabelNode {
val continuationLabel = LabelNode()
val continuationLabelAfterLoadedResult = LabelNode()
val suspendElementLineNumber = lineNumber
var nextLineNumberNode = suspension.suspensionCallEnd.findNextOrNull { it is LineNumberNode } as? LineNumberNode
var nextLineNumberNode = nextDefinitelyHitLineNumber(suspension)
with(methodNode.instructions) {
// Save state
insertBefore(
@@ -746,7 +747,6 @@ class CoroutineTransformerMethodVisitor(
}
remove(possibleTryCatchBlockStart.previous)
val afterSuspensionPointLineNumber = nextLineNumberNode?.line ?: suspendElementLineNumber
insert(possibleTryCatchBlockStart, withInstructionAdapter {
generateResumeWithExceptionCheck(languageVersionSettings.isReleaseCoroutines(), dataIndex, exceptionIndex)
@@ -755,17 +755,23 @@ class CoroutineTransformerMethodVisitor(
visitLabel(continuationLabelAfterLoadedResult.label)
// Extend next instruction linenumber. Can't use line number of suspension point here because both non-suspended execution
// and re-entering after suspension passes this label.
if (possibleTryCatchBlockStart.next?.opcode?.let {
it != Opcodes.ASTORE && it != Opcodes.CHECKCAST && it != Opcodes.INVOKESTATIC &&
it != Opcodes.INVOKEVIRTUAL && it != Opcodes.INVOKEINTERFACE
} == true
) {
visitLineNumber(afterSuspensionPointLineNumber, continuationLabelAfterLoadedResult.label)
} else {
// But keep the linenumber if the result of the call is is used afterwards
nextLineNumberNode = null
if (nextLineNumberNode != null) {
// If there is a clear next linenumber instruction, extend it. Can't use line number of suspension point
// here because both non-suspended execution and re-entering after suspension passes this label.
if (possibleTryCatchBlockStart.next?.opcode?.let {
it != Opcodes.ASTORE && it != Opcodes.CHECKCAST && it != Opcodes.INVOKESTATIC &&
it != Opcodes.INVOKEVIRTUAL && it != Opcodes.INVOKEINTERFACE
} == true
) {
visitLineNumber(nextLineNumberNode!!.line, continuationLabelAfterLoadedResult.label)
} else {
// But keep the linenumber if the result of the call is used afterwards
nextLineNumberNode = null
}
} else if (suspendPointLineNumber != null) {
// If there is no clear next linenumber instruction, the continuation is still on the
// same line as the suspend point.
visitLineNumber(suspendPointLineNumber.line, continuationLabelAfterLoadedResult.label)
}
})
@@ -779,6 +785,18 @@ class CoroutineTransformerMethodVisitor(
return continuationLabel
}
// Find the next line number instruction that is defintely hit. That is, a line number
// that comes before any branch or method call.
private fun nextDefinitelyHitLineNumber(suspension: SuspensionPoint): LineNumberNode? {
var next = suspension.suspensionCallEnd.next
while (next != null) {
if (next.isBranchOrCall) return null
else if (next is LineNumberNode) return next
else next = next.next
}
return next
}
// It's necessary to preserve some sensible invariants like there should be no jump in the middle of try-catch-block
// Also it's important that spilled variables are being restored outside of TCB,
// otherwise they would be treated as uninitialized within catch-block while they can be used there
@@ -34,6 +34,16 @@ val AbstractInsnNode.isMeaningful: Boolean
else -> true
}
val AbstractInsnNode.isBranchOrCall: Boolean
get() =
when(this.type) {
AbstractInsnNode.JUMP_INSN,
AbstractInsnNode.TABLESWITCH_INSN,
AbstractInsnNode.LOOKUPSWITCH_INSN,
AbstractInsnNode.METHOD_INSN -> true
else -> false
}
class InsnSequence(val from: AbstractInsnNode, val to: AbstractInsnNode?) : Sequence<AbstractInsnNode> {
constructor(insnList: InsnList) : this(insnList.first, null)
@@ -150,11 +150,7 @@ class ExpressionCodegen(
if (fileEntry != null) {
val lineNumber = fileEntry.getLineNumber(offset) + 1
assert(lineNumber > 0)
// State-machine builder splits the sequence of instructions into states inside state-machine, adding additional LINENUMBERs
// between them for debugger to stop on suspension. Thus, it requires as much LINENUMBER information as possible to be present,
// otherwise, any exception will have incorrect line number. See elvisLineNumber.kt test.
// TODO: Remove unneeded LINENUMBERs after building the state-machine.
if (lastLineNumber != lineNumber || irFunction.isSuspend || irFunction.isInvokeSuspendOfLambda(context)) {
if (lastLineNumber != lineNumber) {
lastLineNumber = lineNumber
mv.visitLineNumber(lineNumber, markNewLabel())
}
@@ -1,9 +1,9 @@
// TARGET_BACKEND: JVM
// WITH_RUNTIME
// WITH_COROUTINES
// FULL_JDK
import helpers.*
import kotlin.coroutines.*
import kotlin.coroutines.intrinsics.*
@@ -0,0 +1,94 @@
// TARGET_BACKEND: JVM
// WITH_RUNTIME
// WITH_COROUTINES
// FULL_JDK
import helpers.*
import kotlin.coroutines.*
import kotlin.coroutines.intrinsics.*
fun builder(c: suspend () -> Unit) {
c.startCoroutine(EmptyContinuation)
}
suspend fun mightThrow(b: Boolean): Int {
if (b) throw RuntimeException()
return 1
}
fun multipleCalls(b: Boolean) = builder {
mightThrow(b) + mightThrow(!b)
}
fun multipleCalls2(b: Boolean) = builder {
mightThrow(b) + mightThrow(!b)
throw RuntimeException()
}
var i = 0
suspend fun throwEverySecondCall(): Int {
if ((i++ % 2) == 1) throw RuntimeException()
return 1
}
fun multipleCalls3() = builder {
throwEverySecondCall() + throwEverySecondCall()
throwEverySecondCall()
}
fun multipleCalls4() = builder {
throwEverySecondCall() + throwEverySecondCall()
}
fun box(): String {
try {
multipleCalls(true)
return "FAIL 0"
} catch (e: RuntimeException) {
if (e.stackTrace[0].lineNumber != 15) return "FAIL 1 ${e.stackTrace[0].lineNumber}"
if (e.stackTrace[1].lineNumber != 20) return "FAIL 2 ${e.stackTrace[1].lineNumber}"
}
try {
multipleCalls(false)
return "FAIL 3"
} catch (e: RuntimeException) {
if (e.stackTrace[0].lineNumber != 15) return "FAIL 4 ${e.stackTrace[0].lineNumber}"
if (e.stackTrace[1].lineNumber != 20) return "FAIL 5 ${e.stackTrace[1].lineNumber}"
}
try {
multipleCalls2(true)
return "FAIL 6"
} catch (e: RuntimeException) {
if (e.stackTrace[0].lineNumber != 15) return "FAIL 7 ${e.stackTrace[0].lineNumber}"
if (e.stackTrace[1].lineNumber != 24) return "FAIL 8 ${e.stackTrace[1].lineNumber}"
}
try {
multipleCalls2(false)
return "FAIL 9"
} catch (e: RuntimeException) {
if (e.stackTrace[0].lineNumber != 15) return "FAIL 10 ${e.stackTrace[0].lineNumber}"
if (e.stackTrace[1].lineNumber != 24) return "FAIL 11 ${e.stackTrace[1].lineNumber}"
}
try {
multipleCalls3()
return "FAIL 12"
} catch (e: RuntimeException) {
if (e.stackTrace[0].lineNumber != 31) return "FAIL 13 ${e.stackTrace[0].lineNumber}"
if (e.stackTrace[1].lineNumber != 36) return "FAIL 14 ${e.stackTrace[1].lineNumber}"
}
try {
multipleCalls4()
return "FAIL 15"
} catch (e: RuntimeException) {
if (e.stackTrace[0].lineNumber != 31) return "FAIL 16 ${e.stackTrace[0].lineNumber}"
if (e.stackTrace[1].lineNumber != 41) return "FAIL 17 ${e.stackTrace[1].lineNumber}"
}
return "OK"
}
@@ -0,0 +1,39 @@
// TARGET_BACKEND: JVM
// WITH_RUNTIME
// WITH_COROUTINES
// FULL_JDK
import helpers.*
import kotlin.coroutines.*
import kotlin.coroutines.intrinsics.*
fun builder(c: suspend () -> Unit) {
c.startCoroutine(EmptyContinuation)
}
suspend fun mightReturnNull(b: Boolean): String? {
return if (b) null else "asdf"
}
fun throwOnSameLine(b: Boolean) = builder {
if (mightReturnNull(b) == null) throw RuntimeException() else throw RuntimeException()
throw RuntimeException()
}
fun box(): String {
try {
throwOnSameLine(true)
return "FAIL 0"
} catch (e: RuntimeException) {
if (e.stackTrace[0].lineNumber != 19) return "FAIL 1 ${e.stackTrace[0].lineNumber}"
}
try {
throwOnSameLine(false)
return "FAIL 2"
} catch (e: RuntimeException) {
if (e.stackTrace[0].lineNumber != 19) return "FAIL 3 ${e.stackTrace[0].lineNumber}"
}
return "OK"
}
@@ -6935,10 +6935,20 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest {
runTest("compiler/testData/codegen/box/coroutines/debug/fqName.kt");
}
@TestMetadata("multipleSuspendCallsOnSameLine.kt")
public void testMultipleSuspendCallsOnSameLine() throws Exception {
runTest("compiler/testData/codegen/box/coroutines/debug/multipleSuspendCallsOnSameLine.kt");
}
@TestMetadata("runtimeDebugMetadata.kt")
public void testRuntimeDebugMetadata() throws Exception {
runTest("compiler/testData/codegen/box/coroutines/debug/runtimeDebugMetadata.kt");
}
@TestMetadata("throwsOnSameLine.kt")
public void testThrowsOnSameLine() throws Exception {
runTest("compiler/testData/codegen/box/coroutines/debug/throwsOnSameLine.kt");
}
}
@TestMetadata("compiler/testData/codegen/box/coroutines/featureIntersection")
@@ -6935,10 +6935,20 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes
runTest("compiler/testData/codegen/box/coroutines/debug/fqName.kt");
}
@TestMetadata("multipleSuspendCallsOnSameLine.kt")
public void testMultipleSuspendCallsOnSameLine() throws Exception {
runTest("compiler/testData/codegen/box/coroutines/debug/multipleSuspendCallsOnSameLine.kt");
}
@TestMetadata("runtimeDebugMetadata.kt")
public void testRuntimeDebugMetadata() throws Exception {
runTest("compiler/testData/codegen/box/coroutines/debug/runtimeDebugMetadata.kt");
}
@TestMetadata("throwsOnSameLine.kt")
public void testThrowsOnSameLine() throws Exception {
runTest("compiler/testData/codegen/box/coroutines/debug/throwsOnSameLine.kt");
}
}
@TestMetadata("compiler/testData/codegen/box/coroutines/featureIntersection")
@@ -6385,10 +6385,20 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes
runTest("compiler/testData/codegen/box/coroutines/debug/fqName.kt");
}
@TestMetadata("multipleSuspendCallsOnSameLine.kt")
public void testMultipleSuspendCallsOnSameLine() throws Exception {
runTest("compiler/testData/codegen/box/coroutines/debug/multipleSuspendCallsOnSameLine.kt");
}
@TestMetadata("runtimeDebugMetadata.kt")
public void testRuntimeDebugMetadata() throws Exception {
runTest("compiler/testData/codegen/box/coroutines/debug/runtimeDebugMetadata.kt");
}
@TestMetadata("throwsOnSameLine.kt")
public void testThrowsOnSameLine() throws Exception {
runTest("compiler/testData/codegen/box/coroutines/debug/throwsOnSameLine.kt");
}
}
@TestMetadata("compiler/testData/codegen/box/coroutines/featureIntersection")