From cc28fecacd4e981a78bdb3aeb421312142a1466e Mon Sep 17 00:00:00 2001 From: Denis Zharkov Date: Thu, 2 Feb 2017 14:58:40 +0300 Subject: [PATCH] Fix VerifyError in coroutines caused by null spilling While within a method by the JVM spec null-value has a special Nothing-like type, when we spill it for a coroutine, we must choose some real type to CHECKCAST to after restoring the variable's value. But the problem is that such a real type depends on usage of that null value, and there may be more than one usage. The solution is not to spill such variables into fields, but instead init them with ACONST_NULL after each suspension point #KT-16122 Fixed --- .../CoroutineTransformationClassBuilder.kt | 12 +++++ .../coroutines/varSpilling/nullSpilling.kt | 45 +++++++++++++++++++ .../coroutines/varSpilling/nullSpilling.txt | 34 ++++++++++++++ .../ir/IrBlackBoxCodegenTestGenerated.java | 15 +++++++ .../codegen/BlackBoxCodegenTestGenerated.java | 15 +++++++ ...LightAnalysisModeCodegenTestGenerated.java | 15 +++++++ .../semantics/JsCodegenBoxTestGenerated.java | 15 +++++++ 7 files changed, 151 insertions(+) create mode 100644 compiler/testData/codegen/box/coroutines/varSpilling/nullSpilling.kt create mode 100644 compiler/testData/codegen/light-analysis/coroutines/varSpilling/nullSpilling.txt diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/CoroutineTransformationClassBuilder.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/CoroutineTransformationClassBuilder.kt index 377c3d5bcf6..c05174d2d3a 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/CoroutineTransformationClassBuilder.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/CoroutineTransformationClassBuilder.kt @@ -229,6 +229,18 @@ class CoroutineTransformerMethodVisitor( } for ((index, basicValue) in variablesToSpill) { + if (basicValue === StrictBasicValue.NULL_VALUE) { + postponedActions.add { + with(instructions) { + insert(suspension.tryCatchBlockEndLabelAfterSuspensionCall, withInstructionAdapter { + aconst(null) + store(index, AsmTypes.OBJECT_TYPE) + }) + } + } + continue + } + val type = basicValue.type val normalizedType = type.normalize() diff --git a/compiler/testData/codegen/box/coroutines/varSpilling/nullSpilling.kt b/compiler/testData/codegen/box/coroutines/varSpilling/nullSpilling.kt new file mode 100644 index 00000000000..e3f75d35f15 --- /dev/null +++ b/compiler/testData/codegen/box/coroutines/varSpilling/nullSpilling.kt @@ -0,0 +1,45 @@ +// WITH_RUNTIME +// WITH_COROUTINES +import kotlin.coroutines.experimental.* +import kotlin.coroutines.experimental.intrinsics.* + +suspend fun foo(value: String): String = suspendCoroutineOrReturn { x -> + x.resume(value) + COROUTINE_SUSPENDED +} + +fun bar(x: String?, y: String, z: String): String { + if (x != null) throw RuntimeException("fail 0") + return y + z +} + +suspend fun baz1(): String { + return bar(null, foo("O"), foo("K")) +} + +suspend fun baz2(): String { + var x = null + + for (i in 1..3) { + x = null + } + + return bar(x, foo("O"), foo("K")) +} + +fun builder(c: suspend () -> Unit) { + c.startCoroutine(EmptyContinuation) +} + +fun box(): String { + var result = "" + + builder { + result = baz1() + + if (result != "OK") throw RuntimeException("fail 1") + result = baz2() + } + + return result +} diff --git a/compiler/testData/codegen/light-analysis/coroutines/varSpilling/nullSpilling.txt b/compiler/testData/codegen/light-analysis/coroutines/varSpilling/nullSpilling.txt new file mode 100644 index 00000000000..3940f0ef38a --- /dev/null +++ b/compiler/testData/codegen/light-analysis/coroutines/varSpilling/nullSpilling.txt @@ -0,0 +1,34 @@ +@kotlin.Metadata +public final class CoroutineUtilKt { + public final static @org.jetbrains.annotations.NotNull method handleExceptionContinuation(@org.jetbrains.annotations.NotNull p0: kotlin.jvm.functions.Function1): kotlin.coroutines.experimental.Continuation + public final static @org.jetbrains.annotations.NotNull method handleResultContinuation(@org.jetbrains.annotations.NotNull p0: kotlin.jvm.functions.Function1): kotlin.coroutines.experimental.Continuation +} + +@kotlin.Metadata +public class EmptyContinuation { + public final static field Companion: EmptyContinuation.Companion + private final @org.jetbrains.annotations.NotNull field context: kotlin.coroutines.experimental.CoroutineContext + inner class EmptyContinuation/Companion + public @synthetic.kotlin.jvm.GeneratedByJvmOverloads method (): void + public method (@org.jetbrains.annotations.NotNull p0: kotlin.coroutines.experimental.CoroutineContext): void + public synthetic method (p0: kotlin.coroutines.experimental.CoroutineContext, p1: int, p2: kotlin.jvm.internal.DefaultConstructorMarker): void + public @org.jetbrains.annotations.NotNull method getContext(): kotlin.coroutines.experimental.CoroutineContext + public method resume(@org.jetbrains.annotations.Nullable p0: java.lang.Object): void + public method resumeWithException(@org.jetbrains.annotations.NotNull p0: java.lang.Throwable): void +} + +@kotlin.Metadata +public final static class EmptyContinuation/Companion { + inner class EmptyContinuation/Companion + private method (): void +} + +@kotlin.Metadata +public final class NullSpillingKt { + public final static @org.jetbrains.annotations.NotNull method bar(@org.jetbrains.annotations.Nullable p0: java.lang.String, @org.jetbrains.annotations.NotNull p1: java.lang.String, @org.jetbrains.annotations.NotNull p2: java.lang.String): java.lang.String + public final static @org.jetbrains.annotations.Nullable method baz1(@org.jetbrains.annotations.NotNull p0: kotlin.coroutines.experimental.Continuation): java.lang.Object + public final static @org.jetbrains.annotations.Nullable method baz2(@org.jetbrains.annotations.NotNull p0: kotlin.coroutines.experimental.Continuation): java.lang.Object + public final static @org.jetbrains.annotations.NotNull method box(): java.lang.String + public final static method builder(@org.jetbrains.annotations.NotNull p0: kotlin.jvm.functions.Function1): void + public final static @org.jetbrains.annotations.Nullable method foo(@org.jetbrains.annotations.NotNull p0: java.lang.String, @org.jetbrains.annotations.NotNull p1: kotlin.coroutines.experimental.Continuation): java.lang.Object +} 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 1d87044f2ad..f809a1081d3 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 @@ -5359,6 +5359,21 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes doTest(fileName); } } + + @TestMetadata("compiler/testData/codegen/box/coroutines/varSpilling") + @TestDataPath("$PROJECT_ROOT") + @RunWith(JUnit3RunnerWithInners.class) + public static class VarSpilling extends AbstractIrBlackBoxCodegenTest { + public void testAllFilesPresentInVarSpilling() throws Exception { + KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/box/coroutines/varSpilling"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.JVM, true); + } + + @TestMetadata("nullSpilling.kt") + public void testNullSpilling() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/varSpilling/nullSpilling.kt"); + doTest(fileName); + } + } } @TestMetadata("compiler/testData/codegen/box/dataClasses") diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java index a937719911d..ea4f7aa9fc7 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java @@ -5359,6 +5359,21 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { doTest(fileName); } } + + @TestMetadata("compiler/testData/codegen/box/coroutines/varSpilling") + @TestDataPath("$PROJECT_ROOT") + @RunWith(JUnit3RunnerWithInners.class) + public static class VarSpilling extends AbstractBlackBoxCodegenTest { + public void testAllFilesPresentInVarSpilling() throws Exception { + KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/box/coroutines/varSpilling"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.JVM, true); + } + + @TestMetadata("nullSpilling.kt") + public void testNullSpilling() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/varSpilling/nullSpilling.kt"); + doTest(fileName); + } + } } @TestMetadata("compiler/testData/codegen/box/dataClasses") diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeCodegenTestGenerated.java index 029e884dabd..40ddbc8f34e 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeCodegenTestGenerated.java @@ -5359,6 +5359,21 @@ public class LightAnalysisModeCodegenTestGenerated extends AbstractLightAnalysis doTest(fileName); } } + + @TestMetadata("compiler/testData/codegen/box/coroutines/varSpilling") + @TestDataPath("$PROJECT_ROOT") + @RunWith(JUnit3RunnerWithInners.class) + public static class VarSpilling extends AbstractLightAnalysisModeCodegenTest { + public void testAllFilesPresentInVarSpilling() throws Exception { + KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/box/coroutines/varSpilling"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.JVM, true); + } + + @TestMetadata("nullSpilling.kt") + public void testNullSpilling() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/varSpilling/nullSpilling.kt"); + doTest(fileName); + } + } } @TestMetadata("compiler/testData/codegen/box/dataClasses") 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 3b140c4b0ca..b9f067573d1 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 @@ -6062,6 +6062,21 @@ public class JsCodegenBoxTestGenerated extends AbstractJsCodegenBoxTest { doTest(fileName); } } + + @TestMetadata("compiler/testData/codegen/box/coroutines/varSpilling") + @TestDataPath("$PROJECT_ROOT") + @RunWith(JUnit3RunnerWithInners.class) + public static class VarSpilling extends AbstractJsCodegenBoxTest { + public void testAllFilesPresentInVarSpilling() throws Exception { + KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/box/coroutines/varSpilling"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.JS, true); + } + + @TestMetadata("nullSpilling.kt") + public void testNullSpilling() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/coroutines/varSpilling/nullSpilling.kt"); + doTest(fileName); + } + } } @TestMetadata("compiler/testData/codegen/box/dataClasses")