From ff0736f09ec866990e1d0026ae90dc62f68983ce Mon Sep 17 00:00:00 2001 From: Denis Zharkov Date: Tue, 8 Aug 2017 12:50:14 +0700 Subject: [PATCH] Fix exception after combination of `while (true)` + stack-spilling FixStack transformation divides on phases: - Fixing stack before break/continue - Fixing stack for inline markers/try-catch blocks After the first stage all ALWAYS_TRUE markers are replaced with simple GOTO's and if we're skipping break/continue edges we won't reach the code after while (true) statement. At the same time it's fine to not to skip them in the second phase as the stack for them is already corrected in the first phase #KT-19475 Fixed --- .../optimization/fixStack/FixStackAnalyzer.kt | 12 +++---- .../fixStack/FixStackMethodTransformer.kt | 2 +- .../tryCatchAfterWhileTrue.kt | 18 +++++++++++ .../box/coroutines/varSpilling/kt19475.kt | 32 +++++++++++++++++++ .../ir/IrBlackBoxCodegenTestGenerated.java | 12 +++++++ .../codegen/BlackBoxCodegenTestGenerated.java | 12 +++++++ .../LightAnalysisModeTestGenerated.java | 12 +++++++ .../semantics/JsCodegenBoxTestGenerated.java | 12 +++++++ 8 files changed, 104 insertions(+), 8 deletions(-) create mode 100644 compiler/testData/codegen/box/controlStructures/tryCatchInExpressions/tryCatchAfterWhileTrue.kt create mode 100644 compiler/testData/codegen/box/coroutines/varSpilling/kt19475.kt diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/fixStack/FixStackAnalyzer.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/fixStack/FixStackAnalyzer.kt index affee51a4a8..953bbccab62 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/fixStack/FixStackAnalyzer.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/fixStack/FixStackAnalyzer.kt @@ -32,7 +32,8 @@ import org.jetbrains.org.objectweb.asm.tree.analysis.Interpreter internal class FixStackAnalyzer( owner: String, val method: MethodNode, - val context: FixStackContext + val context: FixStackContext, + private val skipBreakContinueGotoEdges: Boolean = true ) { companion object { // Stack size is always non-negative @@ -65,17 +66,14 @@ internal class FixStackAnalyzer( } } - private val analyzer = InternalAnalyzer(owner, method, context) + private val analyzer = InternalAnalyzer(owner) - private class InternalAnalyzer( - owner: String, - method: MethodNode, - val context: FixStackContext - ) : MethodAnalyzer(owner, method, OptimizationBasicInterpreter()) { + private inner class InternalAnalyzer(owner: String) : MethodAnalyzer(owner, method, OptimizationBasicInterpreter()) { val spilledStacks = hashMapOf>() var maxExtraStackSize = 0; private set override fun visitControlFlowEdge(insn: Int, successor: Int): Boolean { + if (!skipBreakContinueGotoEdges) return true val insnNode = instructions[insn] return !(insnNode is JumpInsnNode && context.breakContinueGotoNodes.contains(insnNode)) } diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/fixStack/FixStackMethodTransformer.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/fixStack/FixStackMethodTransformer.kt index 6e522cce862..23cfc6fb085 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/fixStack/FixStackMethodTransformer.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/fixStack/FixStackMethodTransformer.kt @@ -64,7 +64,7 @@ class FixStackMethodTransformer : MethodTransformer() { } private fun analyzeAndTransformSaveRestoreStack(context: FixStackContext, internalClassName: String, methodNode: MethodNode) { - val analyzer = FixStackAnalyzer(internalClassName, methodNode, context) + val analyzer = FixStackAnalyzer(internalClassName, methodNode, context, skipBreakContinueGotoEdges = false) analyzer.analyze() val actions = arrayListOf<() -> Unit>() diff --git a/compiler/testData/codegen/box/controlStructures/tryCatchInExpressions/tryCatchAfterWhileTrue.kt b/compiler/testData/codegen/box/controlStructures/tryCatchInExpressions/tryCatchAfterWhileTrue.kt new file mode 100644 index 00000000000..4db8ad73a72 --- /dev/null +++ b/compiler/testData/codegen/box/controlStructures/tryCatchInExpressions/tryCatchAfterWhileTrue.kt @@ -0,0 +1,18 @@ +// WITH_RUNTIME +fun box(): String { + val a = arrayListOf() + + while (true) { + if (a.size == 0) { + break + } + } + + for (i in 1..2) { + a.add(try { foo() } catch (e: Throwable) { "OK" }) + } + + return a[0] +} + +fun foo(): String = throw RuntimeException() diff --git a/compiler/testData/codegen/box/coroutines/varSpilling/kt19475.kt b/compiler/testData/codegen/box/coroutines/varSpilling/kt19475.kt new file mode 100644 index 00000000000..b18b763100d --- /dev/null +++ b/compiler/testData/codegen/box/coroutines/varSpilling/kt19475.kt @@ -0,0 +1,32 @@ +// WITH_RUNTIME +// WITH_COROUTINES +import helpers.* +import kotlin.coroutines.experimental.* +import kotlin.coroutines.experimental.intrinsics.* + +suspend fun suspendHere(): String = suspendCoroutineOrReturn { x -> + x.resume("OK") + COROUTINE_SUSPENDED +} + +fun builder(c: suspend () -> Unit) { + c.startCoroutine(EmptyContinuation) +} + +fun box(): String { + var result = arrayListOf() + + builder { + while (true) { + if (result.size == 0) { + break + } + } + + for (i in 1..2) { + result.add(suspendHere()) + } + } + + return result[0] +} diff --git a/compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java b/compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java index e4fdd6e17b0..60339ea0f86 100644 --- a/compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java +++ b/compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java @@ -4969,6 +4969,12 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes doTest(fileName); } + @TestMetadata("tryCatchAfterWhileTrue.kt") + public void testTryCatchAfterWhileTrue() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/controlStructures/tryCatchInExpressions/tryCatchAfterWhileTrue.kt"); + doTest(fileName); + } + @TestMetadata("tryInsideCatch.kt") public void testTryInsideCatch() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/controlStructures/tryCatchInExpressions/tryInsideCatch.kt"); @@ -5944,6 +5950,12 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/box/coroutines/varSpilling"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.JVM, true); } + @TestMetadata("kt19475.kt") + public void testKt19475() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/varSpilling/kt19475.kt"); + doTest(fileName); + } + @TestMetadata("nullSpilling.kt") public void testNullSpilling() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/varSpilling/nullSpilling.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java index 9e9e4f868ef..3e3d9c8c7d1 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java @@ -4969,6 +4969,12 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { doTest(fileName); } + @TestMetadata("tryCatchAfterWhileTrue.kt") + public void testTryCatchAfterWhileTrue() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/controlStructures/tryCatchInExpressions/tryCatchAfterWhileTrue.kt"); + doTest(fileName); + } + @TestMetadata("tryInsideCatch.kt") public void testTryInsideCatch() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/controlStructures/tryCatchInExpressions/tryInsideCatch.kt"); @@ -5944,6 +5950,12 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/box/coroutines/varSpilling"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.JVM, true); } + @TestMetadata("kt19475.kt") + public void testKt19475() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/varSpilling/kt19475.kt"); + doTest(fileName); + } + @TestMetadata("nullSpilling.kt") public void testNullSpilling() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/varSpilling/nullSpilling.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java index 2fce78744ef..c6e0e3ca823 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java @@ -4969,6 +4969,12 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes doTest(fileName); } + @TestMetadata("tryCatchAfterWhileTrue.kt") + public void testTryCatchAfterWhileTrue() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/controlStructures/tryCatchInExpressions/tryCatchAfterWhileTrue.kt"); + doTest(fileName); + } + @TestMetadata("tryInsideCatch.kt") public void testTryInsideCatch() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/controlStructures/tryCatchInExpressions/tryInsideCatch.kt"); @@ -5944,6 +5950,12 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/box/coroutines/varSpilling"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.JVM, true); } + @TestMetadata("kt19475.kt") + public void testKt19475() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/varSpilling/kt19475.kt"); + doTest(fileName); + } + @TestMetadata("nullSpilling.kt") public void testNullSpilling() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/varSpilling/nullSpilling.kt"); diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java index 2e58cf88090..1033e077bd7 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java @@ -5671,6 +5671,12 @@ public class JsCodegenBoxTestGenerated extends AbstractJsCodegenBoxTest { doTest(fileName); } + @TestMetadata("tryCatchAfterWhileTrue.kt") + public void testTryCatchAfterWhileTrue() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/controlStructures/tryCatchInExpressions/tryCatchAfterWhileTrue.kt"); + doTest(fileName); + } + @TestMetadata("tryInsideCatch.kt") public void testTryInsideCatch() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/controlStructures/tryCatchInExpressions/tryInsideCatch.kt"); @@ -6628,6 +6634,12 @@ public class JsCodegenBoxTestGenerated extends AbstractJsCodegenBoxTest { KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/box/coroutines/varSpilling"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.JS, true); } + @TestMetadata("kt19475.kt") + public void testKt19475() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/varSpilling/kt19475.kt"); + doTest(fileName); + } + @TestMetadata("nullSpilling.kt") public void testNullSpilling() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/varSpilling/nullSpilling.kt");