From f128e5222ae69635352a3cf31af772f35b04e333 Mon Sep 17 00:00:00 2001 From: Mads Ager Date: Thu, 28 May 2020 13:04:59 +0200 Subject: [PATCH] [JVM_IR] Fix line number information for try-catch. In particular, the current line numbers could lead to stepping into the catch handler even when the code in the try did not throw an exception. This was caused by the code materializing the final value having the catch line number. This patch delays the materialization until the line number of the usage has been emitted. --- .../backend/jvm/codegen/ExpressionCodegen.kt | 25 +++++-- .../debug/stepping/tryCatchExpression.kt | 72 ++++++++++++++----- .../debug/stepping/tryCatchFinally.kt | 4 -- .../testData/debug/stepping/tryFinally.kt | 9 ++- 4 files changed, 77 insertions(+), 33 deletions(-) diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/ExpressionCodegen.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/ExpressionCodegen.kt index 6afc1faaabd..1af5a014d89 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/ExpressionCodegen.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/ExpressionCodegen.kt @@ -495,8 +495,9 @@ class ExpressionCodegen( val initializer = declaration.initializer if (initializer != null) { - initializer.accept(this, data).materializeAt(varType, declaration.type) + var value = initializer.accept(this, data) initializer.markLineNumber(startOffset = true) + value.materializeAt(varType, declaration.type) mv.store(index, varType) } else if (declaration.isVisibleInLVT) { pushDefaultValueOnStack(varType, mv) @@ -980,14 +981,24 @@ class ExpressionCodegen( } mv.mark(tryCatchBlockEnd) - // TODO: generate a common `finally` for try & catch blocks here? Right now this breaks the inliner. - if (savedValue != null) { - mv.load(savedValue, tryAsmType) - frameMap.leaveTemp(tryAsmType) - return aTry.onStack + return object : PromisedValue(this, tryAsmType, aTry.type) { + override fun materializeAt(target: Type, irTarget: IrType) { + if (savedValue != null) { + mv.load(savedValue, tryAsmType) + frameMap.leaveTemp(tryAsmType) + super.materializeAt(target, irTarget) + } else { + unitValue.materializeAt(target, irTarget) + } + } + + override fun discard() { + if (savedValue != null) { + frameMap.leaveTemp(tryAsmType) + } + } } - return unitValue } private fun genTryCatchCover(catchStart: Label, tryStart: Label, tryEnd: Label, tryGaps: List>, type: String?) { diff --git a/compiler/testData/debug/stepping/tryCatchExpression.kt b/compiler/testData/debug/stepping/tryCatchExpression.kt index 5efc1eacbd5..ef39533b0f2 100644 --- a/compiler/testData/debug/stepping/tryCatchExpression.kt +++ b/compiler/testData/debug/stepping/tryCatchExpression.kt @@ -12,10 +12,17 @@ fun foo() { } catch (e: Throwable) { return } + + val x = try { + mightThrow3() + } catch (e: Throwable) { + return + } } var throw1 = false var throw2 = false +var throw3 = false fun mightThrow() { if (throw1) throw Exception() @@ -25,7 +32,14 @@ fun mightThrow2() { if (throw2) throw Exception() } +fun mightThrow3(): Int { + if (throw3) throw Exception() + return 42 +} + fun box() { + foo() + throw3 = true foo() throw2 = true foo() @@ -33,41 +47,61 @@ fun box() { foo() } -// IGNORE_BACKEND: JVM_IR -// The JVM_IR backend will stop on line 13 even when mightThrow2 -// does not throw! - // LINENUMBERS -// test.kt:29 box +// test.kt:41 box // test.kt:4 foo // test.kt:5 foo -// test.kt:21 mightThrow -// test.kt:22 mightThrow +// test.kt:28 mightThrow +// test.kt:29 mightThrow // test.kt:5 foo // test.kt:10 foo // test.kt:11 foo -// test.kt:25 mightThrow2 -// test.kt:26 mightThrow2 +// test.kt:32 mightThrow2 +// test.kt:33 mightThrow2 // test.kt:11 foo // test.kt:10 foo -// test.kt:15 foo -// test.kt:30 box -// test.kt:31 box +// test.kt:16 foo +// test.kt:17 foo +// test.kt:36 mightThrow3 +// test.kt:37 mightThrow3 +// test.kt:17 foo +// test.kt:16 foo +// test.kt:21 foo +// test.kt:42 box +// test.kt:43 box // test.kt:4 foo // test.kt:5 foo -// test.kt:21 mightThrow -// test.kt:22 mightThrow +// test.kt:28 mightThrow +// test.kt:29 mightThrow // test.kt:5 foo // test.kt:10 foo // test.kt:11 foo -// test.kt:25 mightThrow2 +// test.kt:32 mightThrow2 +// test.kt:33 mightThrow2 +// test.kt:11 foo +// test.kt:10 foo +// test.kt:16 foo +// test.kt:17 foo +// test.kt:36 mightThrow3 +// test.kt:18 foo +// test.kt:19 foo +// test.kt:44 box +// test.kt:45 box +// test.kt:4 foo +// test.kt:5 foo +// test.kt:28 mightThrow +// test.kt:29 mightThrow +// test.kt:5 foo +// test.kt:10 foo +// test.kt:11 foo +// test.kt:32 mightThrow2 // test.kt:12 foo // test.kt:13 foo -// test.kt:32 box -// test.kt:33 box +// test.kt:46 box +// test.kt:47 box // test.kt:4 foo // test.kt:5 foo -// test.kt:21 mightThrow +// test.kt:28 mightThrow // test.kt:6 foo // test.kt:7 foo -// test.kt:34 box +// test.kt:48 box \ No newline at end of file diff --git a/compiler/testData/debug/stepping/tryCatchFinally.kt b/compiler/testData/debug/stepping/tryCatchFinally.kt index f1705d17ec1..07cb69ef6d1 100644 --- a/compiler/testData/debug/stepping/tryCatchFinally.kt +++ b/compiler/testData/debug/stepping/tryCatchFinally.kt @@ -37,10 +37,6 @@ fun box() { foo() } -// IGNORE_BACKEND: JVM_IR -// The JVM_IR backend goes back to line 17 after line 18. It has the -// sequence 17, 18, 17 which doesn't make sense. - // LINENUMBERS // test.kt:33 box // test.kt:4 foo diff --git a/compiler/testData/debug/stepping/tryFinally.kt b/compiler/testData/debug/stepping/tryFinally.kt index 82942faa00f..ab17e89f33a 100644 --- a/compiler/testData/debug/stepping/tryFinally.kt +++ b/compiler/testData/debug/stepping/tryFinally.kt @@ -34,9 +34,10 @@ fun box() { foo() } -// IGNORE_BACKEND: JVM_IR -// The JVM_IR backend goes back to line 13 after line 14. It has the -// sequence 13, 14, 13 which doesn't make sense. +// The JVM backend steps back to line 11 when leaving the +// `mightThrow2` call. The JVM_IR backend does not. The +// JVM_IR behavior is consistent with what happens for the +// try-finally where the value is discarded which seems good. // LINENUMBERS // test.kt:29 box @@ -50,7 +51,9 @@ fun box() { // test.kt:11 foo // test.kt:25 mightThrow2 // test.kt:26 mightThrow2 +// LINENUMBERS JVM // test.kt:11 foo +// LINENUMBERS // test.kt:13 foo // test.kt:14 foo // test.kt:10 foo