From 966480a155e96994b66e026619b8d23cbb2b4e87 Mon Sep 17 00:00:00 2001 From: Yan Zhulanow Date: Fri, 2 Mar 2018 20:58:49 +0300 Subject: [PATCH] Debugger: A dirty fix for setting breakpoints inside `finally {}` in case if exception is thrown from `try {}` (#KT-22654) --- .../kotlin/codegen/ExpressionCodegen.java | 4 ++ .../lineNumber/custom/tryCatchFinally.kt | 2 +- .../testData/lineNumber/custom/tryFinally.kt | 2 +- .../idea/debugger/KotlinPositionManager.kt | 17 +++++-- .../src/stepping/custom/finallyBlock.kt | 49 +++++++++++++++++++ .../src/stepping/custom/finallyBlock.out | 19 +++++++ .../debugger/KotlinSteppingTestGenerated.java | 6 +++ 7 files changed, 92 insertions(+), 7 deletions(-) create mode 100644 idea/testData/debugger/tinyApp/src/stepping/custom/finallyBlock.kt create mode 100644 idea/testData/debugger/tinyApp/src/stepping/custom/finallyBlock.out diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/ExpressionCodegen.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/ExpressionCodegen.java index 612066da86b..2ecbce3e1a9 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/ExpressionCodegen.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/ExpressionCodegen.java @@ -1488,6 +1488,10 @@ public class ExpressionCodegen extends KtVisitor impleme } if (tryCatchBlockEnd != null) { + if (finallyBlockStackElement != null) { + markLineNumber(finallyBlockStackElement.expression, true); + } + v.goTo(tryCatchBlockEnd); } diff --git a/compiler/testData/lineNumber/custom/tryCatchFinally.kt b/compiler/testData/lineNumber/custom/tryCatchFinally.kt index 75e8a0ac02d..05218c3289d 100644 --- a/compiler/testData/lineNumber/custom/tryCatchFinally.kt +++ b/compiler/testData/lineNumber/custom/tryCatchFinally.kt @@ -16,4 +16,4 @@ fun foo() { } } -// 2 3 7 4 5 7 8 +10 11 15 12 13 15 10 17 \ No newline at end of file +// 2 3 7 8 4 5 7 8 7 8 +10 11 15 16 12 13 15 16 15 10 17 \ No newline at end of file diff --git a/compiler/testData/lineNumber/custom/tryFinally.kt b/compiler/testData/lineNumber/custom/tryFinally.kt index 5d635e59a93..51e03b80539 100644 --- a/compiler/testData/lineNumber/custom/tryFinally.kt +++ b/compiler/testData/lineNumber/custom/tryFinally.kt @@ -12,4 +12,4 @@ fun foo() { } } -// 2 3 5 6 +8 9 11 8 13 \ No newline at end of file +// 2 3 5 6 5 6 +8 9 11 12 11 8 13 \ No newline at end of file diff --git a/idea/idea-jvm/src/org/jetbrains/kotlin/idea/debugger/KotlinPositionManager.kt b/idea/idea-jvm/src/org/jetbrains/kotlin/idea/debugger/KotlinPositionManager.kt index 32d24bc6327..7952a3779eb 100644 --- a/idea/idea-jvm/src/org/jetbrains/kotlin/idea/debugger/KotlinPositionManager.kt +++ b/idea/idea-jvm/src/org/jetbrains/kotlin/idea/debugger/KotlinPositionManager.kt @@ -58,10 +58,8 @@ import org.jetbrains.kotlin.idea.util.ProjectRootsUtil import org.jetbrains.kotlin.idea.util.application.runReadAction import org.jetbrains.kotlin.load.java.JvmAbi import org.jetbrains.kotlin.name.FqName -import org.jetbrains.kotlin.psi.KtClass -import org.jetbrains.kotlin.psi.KtElement -import org.jetbrains.kotlin.psi.KtFile -import org.jetbrains.kotlin.psi.KtFunction +import org.jetbrains.kotlin.psi.* +import org.jetbrains.kotlin.psi.psiUtil.getStrictParentOfType import org.jetbrains.kotlin.resolve.inline.InlineUtil import org.jetbrains.kotlin.resolve.jvm.JvmClassName import com.intellij.debugger.engine.DebuggerUtils as JDebuggerUtils @@ -166,7 +164,16 @@ class KotlinPositionManager(private val myDebugProcess: DebugProcess) : MultiReq // have been merged into one), but it's impossible to correctly map locations to actual source expressions now. val locationIndex = sameLineLocations.indexOf(location) if (locationIndex > 0) { - return KotlinReentrantSourcePosition(SourcePosition.createFromLine(psiFile, sourceLineNumber)) + /* + `finally {}` block code is placed in the class file twice. + Unless the debugger metadata is available, we can't figure out if we are inside `finally {}`, so we have to check it using PSI. + This is conceptually wrong and won't work in some cases, but it's still better than nothing. + */ + val elementAt = psiFile.getLineStartOffset(lineNumber)?.let { psiFile.findElementAt(it) } + val isInsideDuplicatedFinally = elementAt != null && elementAt.getStrictParentOfType() != null + if (!isInsideDuplicatedFinally) { + return KotlinReentrantSourcePosition(SourcePosition.createFromLine(psiFile, sourceLineNumber)) + } } } diff --git a/idea/testData/debugger/tinyApp/src/stepping/custom/finallyBlock.kt b/idea/testData/debugger/tinyApp/src/stepping/custom/finallyBlock.kt new file mode 100644 index 00000000000..03dd68f23a4 --- /dev/null +++ b/idea/testData/debugger/tinyApp/src/stepping/custom/finallyBlock.kt @@ -0,0 +1,49 @@ +package finallyBlock + +fun throwException() { throw RuntimeException() } +fun foo() {} + +fun main(args: Array) { + fun wrap(f: () -> Unit) = try { f() } catch (e: Throwable) {} + + wrap(::test1) + wrap(::test2) + wrap(::test3) +} + +fun test1() { + try { + //Breakpoint! + throwException() + } finally { + //Breakpoint! + foo() + } +} + +fun test2() { + try { + //Breakpoint! + foo() + } finally { + //Breakpoint! + foo() + } +} + +fun test3() { + try { + //Breakpoint! + throwException() + } finally { + //Breakpoint! + try { + throwException() + } finally { + //Breakpoint! + foo() + } + } +} + +// RESUME: 7 \ No newline at end of file diff --git a/idea/testData/debugger/tinyApp/src/stepping/custom/finallyBlock.out b/idea/testData/debugger/tinyApp/src/stepping/custom/finallyBlock.out new file mode 100644 index 00000000000..afe2711168f --- /dev/null +++ b/idea/testData/debugger/tinyApp/src/stepping/custom/finallyBlock.out @@ -0,0 +1,19 @@ +LineBreakpoint created at finallyBlock.kt:17 +LineBreakpoint created at finallyBlock.kt:20 +LineBreakpoint created at finallyBlock.kt:27 +LineBreakpoint created at finallyBlock.kt:30 +LineBreakpoint created at finallyBlock.kt:37 +LineBreakpoint created at finallyBlock.kt:40 +LineBreakpoint created at finallyBlock.kt:44 +Run Java +Connected to the target VM +finallyBlock.kt:17 +finallyBlock.kt:20 +finallyBlock.kt:27 +finallyBlock.kt:30 +finallyBlock.kt:37 +finallyBlock.kt:40 +finallyBlock.kt:44 +Disconnected from the target VM + +Process finished with exit code 0 diff --git a/idea/tests/org/jetbrains/kotlin/idea/debugger/KotlinSteppingTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/debugger/KotlinSteppingTestGenerated.java index f781ee0f1de..cc8d2292bfb 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/debugger/KotlinSteppingTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/debugger/KotlinSteppingTestGenerated.java @@ -1176,6 +1176,12 @@ public class KotlinSteppingTestGenerated extends AbstractKotlinSteppingTest { doCustomTest(fileName); } + @TestMetadata("finallyBlock.kt") + public void testFinallyBlock() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/debugger/tinyApp/src/stepping/custom/finallyBlock.kt"); + doCustomTest(fileName); + } + @TestMetadata("funLiteral.kt") public void testFunLiteral() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/debugger/tinyApp/src/stepping/custom/funLiteral.kt");