From 542d3c353210a5ada8af1be3ae23e7869acc288f Mon Sep 17 00:00:00 2001 From: Mads Ager Date: Fri, 1 Feb 2019 13:30:24 +0100 Subject: [PATCH] JVM_IR: Fix stack underflow for unit coercion of when expressions. Code such as ``` val b = getBoolean() if (b) 4 else if (b) 5 ``` didn't generate a value on the stack always and therefore would have control-flow paths leading to a pop instruction with nothing on the stack. Change-Id: I09d059f361e56a41880006e3f4e51e9acdbd167d --- .../kotlin/backend/jvm/JvmLoweringPhases.kt | 9 +++- .../jvm/lower/JvmTypeOperatorLoweringPhase.kt | 51 +++++++++++++++++++ .../codegen/box/when/noElseCoerceToUnit.kt | 22 ++++++++ .../codegen/BlackBoxCodegenTestGenerated.java | 5 ++ .../LightAnalysisModeTestGenerated.java | 5 ++ .../ir/IrBlackBoxCodegenTestGenerated.java | 5 ++ 6 files changed, 96 insertions(+), 1 deletion(-) create mode 100644 compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/JvmTypeOperatorLoweringPhase.kt create mode 100644 compiler/testData/codegen/box/when/noElseCoerceToUnit.kt diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/JvmLoweringPhases.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/JvmLoweringPhases.kt index b4ddf311a97..8f3cf6013cb 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/JvmLoweringPhases.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/JvmLoweringPhases.kt @@ -183,7 +183,6 @@ private val BridgePhase = makeJvmPhase( description = "Generate bridges" ) - private val JvmOverloadsAnnotationPhase = makeJvmPhase( { context, file -> JvmOverloadsAnnotationLowering(context).lower(file) }, name = "JvmOverloadsAnnotation", @@ -217,6 +216,13 @@ private val ToArrayPhase = makeJvmPhase( description = "Handle toArray functions" ) +private val JvmTypeOperatorLowering = makeJvmPhase( + { context, file -> JvmTypeOperatorLowering(context).lower(file) }, + name = "JvmTypeOperatorLoweringPhase", + description = "Handle JVM-specific type operator lowerings" +) + + private val JvmBuiltinOptimizationLowering = makeJvmPhase( { context, file -> JvmBuiltinOptimizationLowering(context).lower(file) }, name = "JvmBuiltinOptimizationLowering", @@ -273,6 +279,7 @@ val jvmPhases = listOf( TailrecPhase, ToArrayPhase, + JvmTypeOperatorLowering, JvmBuiltinOptimizationLowering, makePatchParentsPhase(3), diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/JvmTypeOperatorLoweringPhase.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/JvmTypeOperatorLoweringPhase.kt new file mode 100644 index 00000000000..e8815115ffe --- /dev/null +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/JvmTypeOperatorLoweringPhase.kt @@ -0,0 +1,51 @@ +/* + * Copyright 2010-2019 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license + * that can be found in the license/LICENSE.txt file. + */ + +package org.jetbrains.kotlin.backend.jvm.lower + +import org.jetbrains.kotlin.backend.common.FileLoweringPass +import org.jetbrains.kotlin.backend.jvm.JvmBackendContext +import org.jetbrains.kotlin.ir.declarations.IrFile +import org.jetbrains.kotlin.ir.expressions.* +import org.jetbrains.kotlin.ir.expressions.impl.IrBlockImpl +import org.jetbrains.kotlin.ir.expressions.impl.IrConstImpl +import org.jetbrains.kotlin.ir.expressions.impl.IrElseBranchImpl +import org.jetbrains.kotlin.ir.visitors.IrElementTransformerVoid +import org.jetbrains.kotlin.ir.visitors.transformChildrenVoid + +class JvmTypeOperatorLowering(val context: JvmBackendContext) : FileLoweringPass { + override fun lower(irFile: IrFile) { + irFile.transformChildrenVoid(object : IrElementTransformerVoid() { + + // Make sure that When expressions that are coerced to unit always produces a value. + // If the When expression do not have an else branch, we add one of the right type + // so that a value is always produced that can be pop off the stack. + // + // Otherwise, code such as + // + // val b = getBoolean() + // if (b) 5 + // else if (b) 4 + // + // leads to the generation of code that will underflow the stack. + override fun visitTypeOperator(expression: IrTypeOperatorCall): IrExpression { + expression.transformChildrenVoid(this) + if (expression.operator === IrTypeOperator.IMPLICIT_COERCION_TO_UNIT) { + val argument = expression.argument + if (argument is IrWhen + && argument.branches.size > 0 + && argument.branches.last() !is IrElseBranch) { + argument.branches.add( + IrElseBranchImpl( + IrConstImpl.constTrue(argument.startOffset, argument.endOffset, context.irBuiltIns.booleanType), + IrBlockImpl(argument.startOffset, argument.endOffset, argument.type)) + ) + } + } + return expression + } + }) + } +} diff --git a/compiler/testData/codegen/box/when/noElseCoerceToUnit.kt b/compiler/testData/codegen/box/when/noElseCoerceToUnit.kt new file mode 100644 index 00000000000..7eeb67d4e02 --- /dev/null +++ b/compiler/testData/codegen/box/when/noElseCoerceToUnit.kt @@ -0,0 +1,22 @@ +var result = "FAIL" +val d = 0.0 + +fun test(arg: Int) { + if (arg == 1) { + result = "firstResult" + d + } else if (arg == 2) { + result = "secondResult" + arg + } +} + +fun box(): String { + test(1) + if (result != "firstResult") + return "FAIL1" + test(2) + if (result != "secondResult") + return "FAIL2" + return "OK" +} \ No newline at end of file diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java index 33ee7fa71cb..74b15a2dfa2 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java @@ -24519,6 +24519,11 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { runTest("compiler/testData/codegen/box/when/multipleEntries.kt"); } + @TestMetadata("noElseCoerceToUnit.kt") + public void testNoElseCoerceToUnit() throws Exception { + runTest("compiler/testData/codegen/box/when/noElseCoerceToUnit.kt"); + } + @TestMetadata("noElseExhaustive.kt") public void testNoElseExhaustive() throws Exception { runTest("compiler/testData/codegen/box/when/noElseExhaustive.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java index c54cbeb13af..c3100cc8547 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java @@ -24519,6 +24519,11 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes runTest("compiler/testData/codegen/box/when/multipleEntries.kt"); } + @TestMetadata("noElseCoerceToUnit.kt") + public void testNoElseCoerceToUnit() throws Exception { + runTest("compiler/testData/codegen/box/when/noElseCoerceToUnit.kt"); + } + @TestMetadata("noElseExhaustive.kt") public void testNoElseExhaustive() throws Exception { runTest("compiler/testData/codegen/box/when/noElseExhaustive.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java index e2d3a192647..4264a245e56 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java @@ -24524,6 +24524,11 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes runTest("compiler/testData/codegen/box/when/multipleEntries.kt"); } + @TestMetadata("noElseCoerceToUnit.kt") + public void testNoElseCoerceToUnit() throws Exception { + runTest("compiler/testData/codegen/box/when/noElseCoerceToUnit.kt"); + } + @TestMetadata("noElseExhaustive.kt") public void testNoElseExhaustive() throws Exception { runTest("compiler/testData/codegen/box/when/noElseExhaustive.kt");