azalea/kotlin-fork
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Generate CHECKCAST after ACONST_NULL in coroutines

Browse Source 
If we do not do this, the state-machine builder will not know the type
of the ACONST_NULL, defaulting to Object, leading to VerifyError.
Alternatively, we could use LVT to deduce the type, but getting types
from LVT is something I got rid of long time ago, and I have no desire
to return it back.

Generating CHECKCAST hints the state-machine builder the type of the
variable avoiding the issue of VerifyError. However, this CHECKCAST
replaces StrictBasicValue.NULL_VALUE with BasicValue in
OptimizationBasicInterpreter. To preserve optimization on not-spilling
known nulls, introduce BasicValues, which represent typed nulls and
create BasicInterpreter, which is aware of them. This way we have the
best of two worlds - we do not spill known nulls, and we know the type
of ACONST_NULL.

 #KT-51718 Fixed
This commit is contained in:
Ilmir Usmanov
2022-03-30 03:34:07 +02:00
committed by teamcity
parent 9b103b35cd
commit 7579be6c68
15 changed files with 136 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files
compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/CoroutineTransformerMethodVisitor.kt
+1 -1
View File
@@ -643,7 +643,7 @@ class CoroutineTransformerMethodVisitor(
val value = frame.getLocal(slot)
if (value.type == null || !livenessFrame.isAlive(slot)) continue
if (value == StrictBasicValue.NULL_VALUE) {
if (value == StrictBasicValue.NULL_VALUE || value is TypedNullValue) {
referencesToSpill += slot to null
continue
}
compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/SpilledVariableFieldTypesAnalysis.kt
+15 -1
View File
@@ -8,6 +8,7 @@ package org.jetbrains.kotlin.codegen.coroutines
import org.jetbrains.kotlin.codegen.StackValue
import org.jetbrains.kotlin.codegen.optimization.common.FastMethodAnalyzer
import org.jetbrains.kotlin.codegen.optimization.common.OptimizationBasicInterpreter
import org.jetbrains.kotlin.codegen.optimization.common.StrictBasicValue
import org.jetbrains.org.objectweb.asm.Label
import org.jetbrains.org.objectweb.asm.Opcodes
import org.jetbrains.org.objectweb.asm.Type
@@ -134,7 +135,7 @@ internal fun performSpilledVariableFieldTypesAnalysis(
for ((insn, type) in interpreter.needsToBeCoerced) {
methodNode.instructions.insert(insn, withInstructionAdapter { coerceInt(type, this) })
}
return FastMethodAnalyzer(thisName, methodNode, OptimizationBasicInterpreter()).analyze()
return FastMethodAnalyzer(thisName, methodNode, NullCheckcastAwareOptimizationBasicInterpreter()).analyze()
}
private fun coerceInt(to: Type, v: InstructionAdapter) {
@@ -157,4 +158,17 @@ private fun coerceInt(to: Type, v: InstructionAdapter) {
private fun Type.isIntLike(): Boolean = when (sort) {
Type.BOOLEAN, Type.BYTE, Type.CHAR, Type.SHORT -> true
else -> false
}
// Represents [ACONST_NULL, CHECKCAST Type] sequence result.
internal class TypedNullValue(type: Type) : BasicValue(type)
// Preserves nulls through CHECKCASTS.
private class NullCheckcastAwareOptimizationBasicInterpreter : OptimizationBasicInterpreter() {
override fun unaryOperation(insn: AbstractInsnNode, value: BasicValue?): BasicValue? {
if (insn.opcode == Opcodes.CHECKCAST && (value == StrictBasicValue.NULL_VALUE || value is TypedNullValue)) {
return TypedNullValue(Type.getObjectType((insn as TypeInsnNode).desc))
}
return super.unaryOperation(insn, value)
}
}
compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBlackBoxCodegenTestGenerated.java
Generated
+6
View File
@@ -10191,6 +10191,12 @@ public class FirBlackBoxCodegenTestGenerated extends AbstractFirBlackBoxCodegenT
runTest("compiler/testData/codegen/box/coroutines/kt50277.kt");
}
@Test
@TestMetadata("kt51718.kt")
public void testKt51718() throws Exception {
runTest("compiler/testData/codegen/box/coroutines/kt51718.kt");
}
@Test
@TestMetadata("lastExpressionIsLoop.kt")
public void testLastExpressionIsLoop() throws Exception {
compiler/ir/backend.jvm/codegen/src/org/jetbrains/kotlin/backend/jvm/codegen/ExpressionCodegen.kt
+13 -2
View File
@@ -45,8 +45,7 @@ import org.jetbrains.kotlin.ir.util.*
import org.jetbrains.kotlin.ir.visitors.IrElementVisitor
import org.jetbrains.kotlin.name.FqName
import org.jetbrains.kotlin.resolve.jvm.AsmTypes
import org.jetbrains.kotlin.resolve.jvm.AsmTypes.JAVA_STRING_TYPE
import org.jetbrains.kotlin.resolve.jvm.AsmTypes.OBJECT_TYPE
import org.jetbrains.kotlin.resolve.jvm.AsmTypes.*
import org.jetbrains.kotlin.resolve.jvm.jvmSignature.JvmMethodParameterKind
import org.jetbrains.kotlin.resolve.jvm.jvmSignature.JvmMethodSignature
import org.jetbrains.kotlin.types.TypeSystemCommonBackendContext
@@ -653,6 +652,18 @@ class ExpressionCodegen(
val value = initializer.accept(this, data)
initializer.markLineNumber(startOffset = true)
value.materializeAt(varType, declaration.type)
// We need to generate CHECKCAST from ACONST_NULL here for coroutines,
// since otherwise, upon spilling and then unspilling, we will get VerifyError,
// because state-machine builder does not know the type of the ACONST_NULL
// and assumes it to be Ljava/lang/Object;, which is incorrect.
// Generating CHECKCAST hints the state-machine builder the type of the variable
// avoiding the issue of VerifyError. See KT-51718
// Exception is Ljava/lang/Object;, since CHECKCAST Ljava/lang/Object; is effectively no-op.
if (initializer.isNullConst() && varType != OBJECT_TYPE &&
(irFunction.isSuspend || irFunction.isInvokeSuspendOfLambda())
) {
mv.checkcast(varType)
}
declaration.markLineNumber(startOffset = true)
mv.store(index, varType)
} else if (declaration.isVisibleInLVT) {
compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/ir/JvmIrCoroutineUtils.kt
+1 -1
View File
@@ -23,7 +23,7 @@ fun IrFunction.continuationParameter(): IrValueParameter? = when {
else -> valueParameters.singleOrNull { it.origin == JvmLoweredDeclarationOrigin.CONTINUATION_CLASS }
}
internal fun IrFunction.isInvokeSuspendOfLambda(): Boolean =
fun IrFunction.isInvokeSuspendOfLambda(): Boolean =
name.asString() == INVOKE_SUSPEND_METHOD_NAME && parentAsClass.origin == JvmLoweredDeclarationOrigin.SUSPEND_LAMBDA
private fun IrFunction.isInvokeSuspendForInlineOfLambda(): Boolean =
compiler/testData/codegen/box/coroutines/kt51718.kt
Vendored
+60
View File
@@ -0,0 +1,60 @@
// WITH_STDLIB
import kotlin.coroutines.*
fun builder(c: suspend () -> Unit) {
c.startCoroutine(Continuation(EmptyCoroutineContext) {
it.getOrThrow()
})
}
class Service {
suspend fun getCanalConfig() {
var start: Start? = null
var configuration: String? = null
val starts: Array<Start>? = null
if (starts != null && starts.isNotEmpty()) {
start = starts[0]
configuration = call2()
}
//start = start as Start // kotlin 1.5.0 need for produce correct bytecode
call(start!!)
}
val canalConfig = suspend {
var start: Start? = null
var configuration: String? = null
val starts: Array<Start>? = null
if (starts != null && starts.isNotEmpty()) {
start = starts[0]
configuration = call2()
}
//start = start as Start // kotlin 1.5.0 need for produce correct bytecode
call(start!!)
}
fun call(start: Start) {
}
suspend fun call2(
): String? {
return null
}
}
class Start()
fun box(): String {
builder {
try {
Service().getCanalConfig()
error("FAIL 1")
} catch (ignored: NullPointerException) {}
try {
Service().canalConfig()
error("FAIL 2")
} catch (ignored: NullPointerException) {}
}
return "OK"
}
compiler/testData/codegen/box/coroutines/tailCallOptimizations/crossinline.txt
Vendored
-5
View File
@@ -4,7 +4,6 @@ public final class CrossinlineKt$box$1$filter$$inlined$source$1$1 {
// source: 'crossinline.kt'
enclosing method CrossinlineKt$box$1$filter$$inlined$source$1.consume(LSink;Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
field L$0: java.lang.Object
field L$1: java.lang.Object
field label: int
synthetic field result: java.lang.Object
synthetic final field this$0: CrossinlineKt$box$1$filter$$inlined$source$1
@@ -89,7 +88,6 @@ public final class CrossinlineKt$box$1$invokeSuspend$$inlined$filter$1$1 {
// source: 'crossinline.kt'
enclosing method CrossinlineKt$box$1$invokeSuspend$$inlined$filter$1.consume(LSink;Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
field L$0: java.lang.Object
field L$1: java.lang.Object
field label: int
synthetic field result: java.lang.Object
synthetic final field this$0: CrossinlineKt$box$1$invokeSuspend$$inlined$filter$1
@@ -181,7 +179,6 @@ public final class CrossinlineKt$filter$$inlined$source$1$1 {
// source: 'crossinline.kt'
enclosing method CrossinlineKt$filter$$inlined$source$1.consume(LSink;Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
field L$0: java.lang.Object
field L$1: java.lang.Object
field label: int
synthetic field result: java.lang.Object
synthetic final field this$0: CrossinlineKt$filter$$inlined$source$1
@@ -282,7 +279,6 @@ public final class CrossinlineKt$range$$inlined$source$1$1 {
field I$1: int
field L$0: java.lang.Object
field L$1: java.lang.Object
field L$2: java.lang.Object
field label: int
synthetic field result: java.lang.Object
synthetic final field this$0: CrossinlineKt$range$$inlined$source$1
@@ -310,7 +306,6 @@ public final class CrossinlineKt$source$1$consume$1 {
// source: 'crossinline.kt'
enclosing method CrossinlineKt$source$1.consume(LSink;Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
field L$0: java.lang.Object
field L$1: java.lang.Object
field label: int
synthetic field result: java.lang.Object
synthetic final field this$0: CrossinlineKt$source$1
compiler/testData/codegen/bytecodeText/coroutines/cleanup/nullCleanup.kt
Vendored
-5
View File
@@ -14,10 +14,5 @@ suspend fun test() {
// 2 PUTFIELD .*L\$0 : Ljava/lang/Object;
// JVM_TEMPLATES:
// just before suspension point
// 1 ACONST_NULL
// JVM_IR_TEMPLATES:
// two stores to initialize the `a` variable and one null constant to store in the spill slot.
// 3 ACONST_NULL
compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BlackBoxCodegenTestGenerated.java
Generated
+6
View File
@@ -10065,6 +10065,12 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest {
runTest("compiler/testData/codegen/box/coroutines/kt50277.kt");
}
@Test
@TestMetadata("kt51718.kt")
public void testKt51718() throws Exception {
runTest("compiler/testData/codegen/box/coroutines/kt51718.kt");
}
@Test
@TestMetadata("lastExpressionIsLoop.kt")
public void testLastExpressionIsLoop() throws Exception {
compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenTestGenerated.java
Generated
+6
View File
@@ -10191,6 +10191,12 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes
runTest("compiler/testData/codegen/box/coroutines/kt50277.kt");
}
@Test
@TestMetadata("kt51718.kt")
public void testKt51718() throws Exception {
runTest("compiler/testData/codegen/box/coroutines/kt51718.kt");
}
@Test
@TestMetadata("lastExpressionIsLoop.kt")
public void testLastExpressionIsLoop() throws Exception {
compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java
Generated
+5
View File
@@ -7919,6 +7919,11 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes
runTest("compiler/testData/codegen/box/coroutines/kt50277.kt");
}
@TestMetadata("kt51718.kt")
public void testKt51718() throws Exception {
runTest("compiler/testData/codegen/box/coroutines/kt51718.kt");
}
@TestMetadata("lastExpressionIsLoop.kt")
public void testLastExpressionIsLoop() throws Exception {
runTest("compiler/testData/codegen/box/coroutines/lastExpressionIsLoop.kt");
js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/JsCodegenBoxTestGenerated.java
Generated
+6
View File
@@ -7157,6 +7157,12 @@ public class JsCodegenBoxTestGenerated extends AbstractJsCodegenBoxTest {
runTest("compiler/testData/codegen/box/coroutines/kt49168.kt");
}
@Test
@TestMetadata("kt51718.kt")
public void testKt51718() throws Exception {
runTest("compiler/testData/codegen/box/coroutines/kt51718.kt");
}
@Test
@TestMetadata("lastExpressionIsLoop.kt")
public void testLastExpressionIsLoop() throws Exception {
js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/ir/IrJsCodegenBoxTestGenerated.java
Generated
+6
View File
@@ -7199,6 +7199,12 @@ public class IrJsCodegenBoxTestGenerated extends AbstractIrJsCodegenBoxTest {
runTest("compiler/testData/codegen/box/coroutines/kt49168.kt");
}
@Test
@TestMetadata("kt51718.kt")
public void testKt51718() throws Exception {
runTest("compiler/testData/codegen/box/coroutines/kt51718.kt");
}
@Test
@TestMetadata("lastExpressionIsLoop.kt")
public void testLastExpressionIsLoop() throws Exception {
js/js.tests/tests-gen/org/jetbrains/kotlin/js/testOld/wasm/semantics/IrCodegenBoxWasmTestGenerated.java
Generated
+5
View File
@@ -6241,6 +6241,11 @@ public class IrCodegenBoxWasmTestGenerated extends AbstractIrCodegenBoxWasmTest
runTest("compiler/testData/codegen/box/coroutines/kt49168.kt");
}
@TestMetadata("kt51718.kt")
public void testKt51718() throws Exception {
runTest("compiler/testData/codegen/box/coroutines/kt51718.kt");
}
@TestMetadata("lastExpressionIsLoop.kt")
public void testLastExpressionIsLoop() throws Exception {
runTest("compiler/testData/codegen/box/coroutines/lastExpressionIsLoop.kt");
native/native.tests/tests-gen/org/jetbrains/kotlin/konan/blackboxtest/NativeCodegenBoxTestGenerated.java
Generated
+6
View File
@@ -8041,6 +8041,12 @@ public class NativeCodegenBoxTestGenerated extends AbstractNativeCodegenBoxTest
runTest("compiler/testData/codegen/box/coroutines/kt49168.kt");
}
@Test
@TestMetadata("kt51718.kt")
public void testKt51718() throws Exception {
runTest("compiler/testData/codegen/box/coroutines/kt51718.kt");
}
@Test
@TestMetadata("lastExpressionIsLoop.kt")
public void testLastExpressionIsLoop() throws Exception {