From e525e25518f1c3b092b5c5cd084cac135ed74b0f Mon Sep 17 00:00:00 2001 From: Dmitry Petrov Date: Thu, 18 Nov 2021 20:12:38 +0300 Subject: [PATCH] JVM KT-47851 fix redundant checkcast elimination --- .../RedundantCheckCastElimination.kt | 41 ++++++++++++++++++- .../common/InstructionLivenessAnalyzer.kt | 12 ++++-- .../FirBlackBoxCodegenTestGenerated.java | 12 ++++++ .../box/checkcastOptimization/kt47851.kt | 21 ++++++++++ .../box/checkcastOptimization/kt47851a.kt | 30 ++++++++++++++ .../codegen/BlackBoxCodegenTestGenerated.java | 12 ++++++ .../IrBlackBoxCodegenTestGenerated.java | 12 ++++++ .../LightAnalysisModeTestGenerated.java | 10 +++++ .../js/test/JsCodegenBoxTestGenerated.java | 6 +++ .../test/ir/IrJsCodegenBoxTestGenerated.java | 6 +++ .../IrCodegenBoxWasmTestGenerated.java | 5 +++ .../NativeExtBlackBoxTestGenerated.java | 24 +++++++++++ 12 files changed, 187 insertions(+), 4 deletions(-) create mode 100644 compiler/testData/codegen/box/checkcastOptimization/kt47851.kt create mode 100644 compiler/testData/codegen/box/checkcastOptimization/kt47851a.kt diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/RedundantCheckCastElimination.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/RedundantCheckCastElimination.kt index d9c0e671c9a..72d9ed4e9f1 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/RedundantCheckCastElimination.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/RedundantCheckCastElimination.kt @@ -17,13 +17,17 @@ package org.jetbrains.kotlin.codegen.optimization import org.jetbrains.kotlin.codegen.inline.ReifiedTypeInliner +import org.jetbrains.kotlin.codegen.optimization.common.InstructionLivenessAnalyzer import org.jetbrains.kotlin.codegen.optimization.common.OptimizationBasicInterpreter import org.jetbrains.kotlin.codegen.optimization.fixStack.top import org.jetbrains.kotlin.codegen.optimization.transformer.MethodTransformer import org.jetbrains.org.objectweb.asm.Opcodes import org.jetbrains.org.objectweb.asm.Type +import org.jetbrains.org.objectweb.asm.tree.AbstractInsnNode import org.jetbrains.org.objectweb.asm.tree.MethodNode import org.jetbrains.org.objectweb.asm.tree.TypeInsnNode +import org.jetbrains.org.objectweb.asm.tree.VarInsnNode +import org.jetbrains.org.objectweb.asm.tree.analysis.BasicValue class RedundantCheckCastEliminationMethodTransformer : MethodTransformer() { override fun transform(internalClassName: String, methodNode: MethodNode) { @@ -31,9 +35,21 @@ class RedundantCheckCastEliminationMethodTransformer : MethodTransformer() { if (!insns.any { it.opcode == Opcodes.CHECKCAST }) return if (insns.any { ReifiedTypeInliner.isOperationReifiedMarker(it) }) return + val typeAdjustmentForALoads = getTypeAdjustmentForALoadInstructions(insns, methodNode) + val interpreter = object : OptimizationBasicInterpreter() { + override fun copyOperation(insn: AbstractInsnNode, value: BasicValue?): BasicValue { + val adjustedType = typeAdjustmentForALoads[insn] + return if (adjustedType != null) + newValue(adjustedType) + ?: throw AssertionError("Local variable type can't be VOID: $adjustedType") + else + super.copyOperation(insn, value) + } + } + val redundantCheckCasts = ArrayList() - val frames = analyze(internalClassName, methodNode, OptimizationBasicInterpreter()) + val frames = analyze(internalClassName, methodNode, interpreter) for (i in insns.indices) { val valueType = frames[i]?.top()?.type ?: continue val insn = insns[i] @@ -57,6 +73,29 @@ class RedundantCheckCastEliminationMethodTransformer : MethodTransformer() { } } + private fun getTypeAdjustmentForALoadInstructions( + insns: Array, + methodNode: MethodNode + ): Map { + val isNonHandler = InstructionLivenessAnalyzer(methodNode, visitExceptionHandlers = false).analyze() + + val result = HashMap() + for (lv in methodNode.localVariables) { + val startIndex = methodNode.instructions.indexOf(lv.start) + val endIndex = methodNode.instructions.indexOf(lv.end) + for (i in startIndex until endIndex) { + val insn = insns[i] + // If we are in exception handler (or in dead code, but it really doesn't matter here, since dead code should not be seen + // by data flow analyzer), treat ALOAD instructions as producing a value of declared local variable type. + // Otherwise, resulting bytecode might fail verification on JDK 1.8+ because of inexact frames (see KT-47851). + if (insn.opcode == Opcodes.ALOAD && (insn as VarInsnNode).`var` == lv.index && !isNonHandler[i]) { + result[insn] = Type.getType(lv.desc) + } + } + } + return result + } + private fun isTrivialSubtype(superType: Type, subType: Type) = superType == subType diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/common/InstructionLivenessAnalyzer.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/common/InstructionLivenessAnalyzer.kt index a15e04f870f..dfc2ea3ce1d 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/common/InstructionLivenessAnalyzer.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/common/InstructionLivenessAnalyzer.kt @@ -37,7 +37,10 @@ package org.jetbrains.kotlin.codegen.optimization.common import org.jetbrains.org.objectweb.asm.Opcodes import org.jetbrains.org.objectweb.asm.tree.* -class InstructionLivenessAnalyzer(val method: MethodNode) { +class InstructionLivenessAnalyzer( + val method: MethodNode, + val visitExceptionHandlers: Boolean = true +) { private val instructions = method.instructions private val nInsns = instructions.size() @@ -92,8 +95,10 @@ class InstructionLivenessAnalyzer(val method: MethodNode) { } } - handlers[insn]?.forEach { tcb -> - visitControlFlowEdge(tcb.handler.indexOf) + if (visitExceptionHandlers) { + handlers[insn]?.forEach { tcb -> + visitControlFlowEdge(tcb.handler.indexOf) + } } } } @@ -151,6 +156,7 @@ class InstructionLivenessAnalyzer(val method: MethodNode) { } private fun computeExceptionHandlersForEachInsn(m: MethodNode) { + if (!visitExceptionHandlers) return for (tcb in m.tryCatchBlocks) { val begin = tcb.start.indexOf val end = tcb.end.indexOf diff --git a/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBlackBoxCodegenTestGenerated.java b/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBlackBoxCodegenTestGenerated.java index ab92c749b2d..bfbaf060e26 100644 --- a/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBlackBoxCodegenTestGenerated.java +++ b/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBlackBoxCodegenTestGenerated.java @@ -4797,6 +4797,18 @@ public class FirBlackBoxCodegenTestGenerated extends AbstractFirBlackBoxCodegenT public void testKt19246() throws Exception { runTest("compiler/testData/codegen/box/checkcastOptimization/kt19246.kt"); } + + @Test + @TestMetadata("kt47851.kt") + public void testKt47851() throws Exception { + runTest("compiler/testData/codegen/box/checkcastOptimization/kt47851.kt"); + } + + @Test + @TestMetadata("kt47851a.kt") + public void testKt47851a() throws Exception { + runTest("compiler/testData/codegen/box/checkcastOptimization/kt47851a.kt"); + } } @Nested diff --git a/compiler/testData/codegen/box/checkcastOptimization/kt47851.kt b/compiler/testData/codegen/box/checkcastOptimization/kt47851.kt new file mode 100644 index 00000000000..f6bed4eb204 --- /dev/null +++ b/compiler/testData/codegen/box/checkcastOptimization/kt47851.kt @@ -0,0 +1,21 @@ +// IGNORE_BACKEND_FIR: JVM_IR +// FIR status: result.getMethod OK in FE1.0, unresolved in FIR + +class C(val value: String) { + fun getField() = value + fun getMethod() {} +} + +fun foo(): Any { + var result: Any = "" + result = C("OK") + try { + result = result.getField() + } catch (e: Exception) { + result.getMethod() + } + return result +} + + +fun box() = foo().toString() diff --git a/compiler/testData/codegen/box/checkcastOptimization/kt47851a.kt b/compiler/testData/codegen/box/checkcastOptimization/kt47851a.kt new file mode 100644 index 00000000000..40ad8f553f4 --- /dev/null +++ b/compiler/testData/codegen/box/checkcastOptimization/kt47851a.kt @@ -0,0 +1,30 @@ +// TARGET_BACKEND: JVM +// IGNORE_BACKEND_FIR: JVM_IR +// FIR status: result.getMethod OK in FE1.0, unresolved in FIR +// WITH_RUNTIME +// FULL_JDK + +val defaultStringConverter = fun(s: String): Any { + var result: Any = s + var m: Array? = arrayOf("1", "2", "3", "4") + if (m != null) { + val fname = m[4] + try { + result = Class.forName(m[1]) + if (fname != "") { + try { + val f = result.getField(fname) + result = f.get(null) + } catch (nfe: NoSuchFieldException) { + val meth = result.getMethod(fname) + } + } + } catch (cnfe: ClassNotFoundException) { + } + } + return result +} + +// Just check that there's no VerifyError. +// Semantics is checked in kt47851.kt. +fun box() = "OK" diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BlackBoxCodegenTestGenerated.java index 50bf2ba6f76..739a0a94c57 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BlackBoxCodegenTestGenerated.java @@ -4725,6 +4725,18 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { public void testKt19246() throws Exception { runTest("compiler/testData/codegen/box/checkcastOptimization/kt19246.kt"); } + + @Test + @TestMetadata("kt47851.kt") + public void testKt47851() throws Exception { + runTest("compiler/testData/codegen/box/checkcastOptimization/kt47851.kt"); + } + + @Test + @TestMetadata("kt47851a.kt") + public void testKt47851a() throws Exception { + runTest("compiler/testData/codegen/box/checkcastOptimization/kt47851a.kt"); + } } @Nested diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenTestGenerated.java index f31097020ae..c4977c8cda8 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenTestGenerated.java @@ -4797,6 +4797,18 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes public void testKt19246() throws Exception { runTest("compiler/testData/codegen/box/checkcastOptimization/kt19246.kt"); } + + @Test + @TestMetadata("kt47851.kt") + public void testKt47851() throws Exception { + runTest("compiler/testData/codegen/box/checkcastOptimization/kt47851.kt"); + } + + @Test + @TestMetadata("kt47851a.kt") + public void testKt47851a() throws Exception { + runTest("compiler/testData/codegen/box/checkcastOptimization/kt47851a.kt"); + } } @Nested diff --git a/compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java b/compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java index 138486661bf..5e903176cbc 100644 --- a/compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java +++ b/compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java @@ -4133,6 +4133,16 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes public void testKt19246() throws Exception { runTest("compiler/testData/codegen/box/checkcastOptimization/kt19246.kt"); } + + @TestMetadata("kt47851.kt") + public void testKt47851() throws Exception { + runTest("compiler/testData/codegen/box/checkcastOptimization/kt47851.kt"); + } + + @TestMetadata("kt47851a.kt") + public void testKt47851a() throws Exception { + runTest("compiler/testData/codegen/box/checkcastOptimization/kt47851a.kt"); + } } @TestMetadata("compiler/testData/codegen/box/classLiteral") diff --git a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/JsCodegenBoxTestGenerated.java b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/JsCodegenBoxTestGenerated.java index f26278ccda6..aedd1fe024e 100644 --- a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/JsCodegenBoxTestGenerated.java +++ b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/JsCodegenBoxTestGenerated.java @@ -3489,6 +3489,12 @@ public class JsCodegenBoxTestGenerated extends AbstractJsCodegenBoxTest { public void testKt19246() throws Exception { runTest("compiler/testData/codegen/box/checkcastOptimization/kt19246.kt"); } + + @Test + @TestMetadata("kt47851.kt") + public void testKt47851() throws Exception { + runTest("compiler/testData/codegen/box/checkcastOptimization/kt47851.kt"); + } } @Nested diff --git a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/ir/IrJsCodegenBoxTestGenerated.java b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/ir/IrJsCodegenBoxTestGenerated.java index fc6ae0570c6..b74a5d40219 100644 --- a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/ir/IrJsCodegenBoxTestGenerated.java +++ b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/ir/IrJsCodegenBoxTestGenerated.java @@ -3531,6 +3531,12 @@ public class IrJsCodegenBoxTestGenerated extends AbstractIrJsCodegenBoxTest { public void testKt19246() throws Exception { runTest("compiler/testData/codegen/box/checkcastOptimization/kt19246.kt"); } + + @Test + @TestMetadata("kt47851.kt") + public void testKt47851() throws Exception { + runTest("compiler/testData/codegen/box/checkcastOptimization/kt47851.kt"); + } } @Nested diff --git a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/testOld/wasm/semantics/IrCodegenBoxWasmTestGenerated.java b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/testOld/wasm/semantics/IrCodegenBoxWasmTestGenerated.java index 9f1ef572a3d..3ba37873b07 100644 --- a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/testOld/wasm/semantics/IrCodegenBoxWasmTestGenerated.java +++ b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/testOld/wasm/semantics/IrCodegenBoxWasmTestGenerated.java @@ -3128,6 +3128,11 @@ public class IrCodegenBoxWasmTestGenerated extends AbstractIrCodegenBoxWasmTest public void testKt19246() throws Exception { runTest("compiler/testData/codegen/box/checkcastOptimization/kt19246.kt"); } + + @TestMetadata("kt47851.kt") + public void testKt47851() throws Exception { + runTest("compiler/testData/codegen/box/checkcastOptimization/kt47851.kt"); + } } @TestMetadata("compiler/testData/codegen/box/classLiteral") diff --git a/native/native.tests/tests-gen/org/jetbrains/kotlin/konan/blackboxtest/NativeExtBlackBoxTestGenerated.java b/native/native.tests/tests-gen/org/jetbrains/kotlin/konan/blackboxtest/NativeExtBlackBoxTestGenerated.java index cdc05dacaad..a1319dd329d 100644 --- a/native/native.tests/tests-gen/org/jetbrains/kotlin/konan/blackboxtest/NativeExtBlackBoxTestGenerated.java +++ b/native/native.tests/tests-gen/org/jetbrains/kotlin/konan/blackboxtest/NativeExtBlackBoxTestGenerated.java @@ -4859,6 +4859,18 @@ public class NativeExtBlackBoxTestGenerated extends AbstractNativeBlackBoxTest { public void testKt19246() throws Exception { runTest("compiler/testData/codegen/box/checkcastOptimization/kt19246.kt"); } + + @Test + @TestMetadata("kt47851.kt") + public void testKt47851() throws Exception { + runTest("compiler/testData/codegen/box/checkcastOptimization/kt47851.kt"); + } + + @Test + @TestMetadata("kt47851a.kt") + public void testKt47851a() throws Exception { + runTest("compiler/testData/codegen/box/checkcastOptimization/kt47851a.kt"); + } } @Nested @@ -7956,6 +7968,18 @@ public class NativeExtBlackBoxTestGenerated extends AbstractNativeBlackBoxTest { runTest("compiler/testData/codegen/box/constructorCall/inlineFunInLocalClassConstructorCall.kt"); } + @Test + @TestMetadata("kt49615.kt") + public void testKt49615() throws Exception { + runTest("compiler/testData/codegen/box/constructorCall/kt49615.kt"); + } + + @Test + @TestMetadata("kt49615a.kt") + public void testKt49615a() throws Exception { + runTest("compiler/testData/codegen/box/constructorCall/kt49615a.kt"); + } + @Test @TestMetadata("loopInInlineFun.kt") public void testLoopInInlineFun() throws Exception {