From ce265c049adf90118dcb7cfba1e8766670908295 Mon Sep 17 00:00:00 2001 From: Zalim Bashorov Date: Fri, 9 Dec 2022 18:50:43 +0100 Subject: [PATCH] [Wasm] Require location for `buildUnreachable` and fix all usages --- .../backend/wasm/ir2wasm/BodyGenerator.kt | 29 +++++++++---------- .../wasm/ir2wasm/DeclarationGenerator.kt | 2 +- .../wasm/ir2wasm/OptimisedWhenGenerator.kt | 4 ++- .../kotlin/wasm/ir/WasmExpressionBuilder.kt | 17 +++++++++-- 4 files changed, 33 insertions(+), 19 deletions(-) diff --git a/compiler/ir/backend.wasm/src/org/jetbrains/kotlin/backend/wasm/ir2wasm/BodyGenerator.kt b/compiler/ir/backend.wasm/src/org/jetbrains/kotlin/backend/wasm/ir2wasm/BodyGenerator.kt index 6a565186faa..9902032f22f 100644 --- a/compiler/ir/backend.wasm/src/org/jetbrains/kotlin/backend/wasm/ir2wasm/BodyGenerator.kt +++ b/compiler/ir/backend.wasm/src/org/jetbrains/kotlin/backend/wasm/ir2wasm/BodyGenerator.kt @@ -59,7 +59,8 @@ class BodyGenerator( elem.acceptVoid(this) if (elem is IrExpression && elem.type.isNothing()) { - body.buildUnreachable() + // TODO Ideally, we should generate unreachable only for specific cases and preferable on declaration site. + body.buildUnreachableAfterNothingType() } } @@ -437,7 +438,10 @@ class BodyGenerator( body.buildInstr(WasmOp.CALL_REF, WasmImmediate.TypeIdx(context.referenceFunctionType(function.symbol))) body.commentGroupEnd() } else { - body.buildUnreachable() + // We came here for a call to an interface method which interface is not implemented anywhere, + // so we don't have a slot in the itable and can't generate a correct call, + // and, anyway, the call effectively is unreachable. + body.buildUnreachableForVerifier() } } @@ -552,17 +556,11 @@ class BodyGenerator( val fromType = call.getTypeArgument(0)!! if (fromType.isNothing()) { - body.buildUnreachable() + body.buildUnreachableAfterNothingType() // TODO: Investigate why? return true } - // Workaround test codegen/box/elvis/nullNullOk.kt - if (fromType.makeNotNull().isNothing()) { - body.buildUnreachable() - return true - } - val toType = call.getTypeArgument(1)!! val klass: IrClass = backendContext.inlineClassesUtils.getInlinedClass(toType)!! val field = getInlineClassBackingField(klass) @@ -694,7 +692,7 @@ class BodyGenerator( private fun recoverToExpectedType(actualType: IrType, expectedType: IrType) { // TYPE -> NOTHING -> FALSE if (expectedType.isNothing()) { - body.buildUnreachable() + body.buildUnreachableAfterNothingType() return } @@ -724,11 +722,12 @@ class BodyGenerator( // NOT_NOTHING_TYPE -> NOTHING -> FALSE if (expectedTypeErased.isNothing() && !actualTypeErased.isNothing()) { - body.buildUnreachable() + body.buildUnreachableAfterNothingType() return } // TYPE -> BASE -> TRUE + // TODO Shouldn't we keep nullability for subtype check? if (actualClassErased.isSubclassOf(expectedClassErased)) { return } @@ -739,7 +738,8 @@ class BodyGenerator( // PRIMITIVE -> REF -> FALSE // REF -> PRIMITIVE -> FALSE if (expectedIsPrimitive != actualIsPrimitive) { - body.buildUnreachable() + // TODO Shouldn't we throw ICE instead? + body.buildUnreachableForVerifier() return } @@ -748,7 +748,7 @@ class BodyGenerator( if (expectedClassErased.isSubclassOf(actualClassErased)) { generateRefNullCast(actualTypeErased, expectedTypeErased) } else { - body.buildUnreachable() + body.buildUnreachableForVerifier() } } } @@ -758,7 +758,6 @@ class BodyGenerator( if (nonLocalReturnSymbol != null) { generateWithExpectedType(expression.value, nonLocalReturnSymbol.owner.type) body.buildBr(functionContext.referenceNonLocalReturnLevel(nonLocalReturnSymbol)) - body.buildUnreachable() } else { visitFunctionReturn(expression) } @@ -798,7 +797,7 @@ class BodyGenerator( body.buildGetUnit() } } else { - body.buildUnreachable() + error("'When' without else branch and non Unit type: ${expression.type.dumpKotlinLike()}") } } diff --git a/compiler/ir/backend.wasm/src/org/jetbrains/kotlin/backend/wasm/ir2wasm/DeclarationGenerator.kt b/compiler/ir/backend.wasm/src/org/jetbrains/kotlin/backend/wasm/ir2wasm/DeclarationGenerator.kt index e4301ac8831..0db2c6a67f9 100644 --- a/compiler/ir/backend.wasm/src/org/jetbrains/kotlin/backend/wasm/ir2wasm/DeclarationGenerator.kt +++ b/compiler/ir/backend.wasm/src/org/jetbrains/kotlin/backend/wasm/ir2wasm/DeclarationGenerator.kt @@ -171,7 +171,7 @@ class DeclarationGenerator( // Add unreachable if function returns something but not as a last instruction. // We can do a separate lowering which adds explicit returns everywhere instead. if (wasmFunctionType.resultTypes.isNotEmpty()) { - exprGen.buildUnreachable() + exprGen.buildUnreachableForVerifier() } context.defineFunction(declaration.symbol, function) diff --git a/compiler/ir/backend.wasm/src/org/jetbrains/kotlin/backend/wasm/ir2wasm/OptimisedWhenGenerator.kt b/compiler/ir/backend.wasm/src/org/jetbrains/kotlin/backend/wasm/ir2wasm/OptimisedWhenGenerator.kt index 58a06c6fd3d..7612ac37e8d 100644 --- a/compiler/ir/backend.wasm/src/org/jetbrains/kotlin/backend/wasm/ir2wasm/OptimisedWhenGenerator.kt +++ b/compiler/ir/backend.wasm/src/org/jetbrains/kotlin/backend/wasm/ir2wasm/OptimisedWhenGenerator.kt @@ -10,6 +10,7 @@ import org.jetbrains.kotlin.backend.wasm.WasmSymbols import org.jetbrains.kotlin.ir.expressions.* import org.jetbrains.kotlin.ir.types.IrType import org.jetbrains.kotlin.ir.types.isUnit +import org.jetbrains.kotlin.ir.util.dumpKotlinLike import org.jetbrains.kotlin.ir.util.isElseBranch import org.jetbrains.kotlin.wasm.ir.* import org.jetbrains.kotlin.wasm.ir.source.location.SourceLocation @@ -225,9 +226,10 @@ private fun BodyGenerator.createBinaryTable( // default else block if (resultType != null) { if (expectedType.isUnit()) { + // UnitToVoidLowering may optimize "a code" that execution didn't come here. body.buildGetUnit() } else { - body.buildUnreachable() + error("'When' without else branch and non Unit type: ${expectedType.dumpKotlinLike()}") } } } diff --git a/wasm/wasm.ir/src/org/jetbrains/kotlin/wasm/ir/WasmExpressionBuilder.kt b/wasm/wasm.ir/src/org/jetbrains/kotlin/wasm/ir/WasmExpressionBuilder.kt index f05d07d68d3..62fc55df617 100644 --- a/wasm/wasm.ir/src/org/jetbrains/kotlin/wasm/ir/WasmExpressionBuilder.kt +++ b/wasm/wasm.ir/src/org/jetbrains/kotlin/wasm/ir/WasmExpressionBuilder.kt @@ -36,8 +36,8 @@ abstract class WasmExpressionBuilder { buildInstr(WasmOp.I32_CONST, location, WasmImmediate.SymbolI32(value)) } - fun buildUnreachable() { - buildInstr(WasmOp.UNREACHABLE) + fun buildUnreachable(location: SourceLocation) { + buildInstr(WasmOp.UNREACHABLE, location) } @Suppress("UNUSED_PARAMETER") @@ -191,3 +191,16 @@ abstract class WasmExpressionBuilder { buildInstr(WasmOp.PSEUDO_COMMENT_GROUP_END) } } + +fun WasmExpressionBuilder.buildUnreachableForVerifier() { + buildUnreachable(SourceLocation.NoLocation("This instruction should never be reached, but required for wasm verifier")) +} + +fun WasmExpressionBuilder.buildUnreachableAfterNothingType() { + buildUnreachable( + SourceLocation.NoLocation( + "The unreachable instruction after an expression with Nothing type to make sure that " + + "execution doesn't come here (or it fails fast if so). It also might be required for wasm verifier." + ) + ) +}