From 0905bf3e381f746c2b3c7c62412404d9412bc3d6 Mon Sep 17 00:00:00 2001 From: Denis Zharkov Date: Fri, 7 Oct 2016 18:59:22 +0300 Subject: [PATCH] Refine redundant null checks interpreter The main change here is addition of a check that NotNullBasicValue instances are not being created for non-reference types Exactly this change should be used instead of f25f0db10e634b38735ca21626b6e72523bc8a28 The latter commit lead to problem described in the KT-14242 issue: v.getType().getSort() == w.getType().getSort() && (v.getType().getSort() != Type.OBJECT || v.equals(w)) Problem is that the condition above returns true without calling `v.equals(w)`, because the sort of type is ARRAY, not OBJECT, so testArray was being treated as NotNullable erroneously So the second part of this change is effectively revering mentioned commit #KT-14242 Fixed --- .../boxing/NullabilityInterpreter.kt | 13 ++++--- .../common/OptimizationBasicInterpreter.java | 35 ++++++++++--------- .../codegen/box/regressions/arrayLengthNPE.kt | 13 +++++++ .../codegen/BlackBoxCodegenTestGenerated.java | 6 ++++ 4 files changed, 46 insertions(+), 21 deletions(-) create mode 100644 compiler/testData/codegen/box/regressions/arrayLengthNPE.kt diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/boxing/NullabilityInterpreter.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/boxing/NullabilityInterpreter.kt index bcb7a296e35..194facb2efd 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/boxing/NullabilityInterpreter.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/boxing/NullabilityInterpreter.kt @@ -16,12 +16,12 @@ package org.jetbrains.kotlin.codegen.optimization.boxing -import org.jetbrains.org.objectweb.asm.tree.InsnList -import org.jetbrains.org.objectweb.asm.tree.analysis.BasicValue import org.jetbrains.kotlin.codegen.optimization.common.BasicValueWrapper +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.Opcodes +import org.jetbrains.org.objectweb.asm.tree.InsnList +import org.jetbrains.org.objectweb.asm.tree.analysis.BasicValue class NullabilityInterpreter(insns: InsnList) : BoxingInterpreter(insns) { override fun unaryOperation(insn: AbstractInsnNode, value: BasicValue) = makeNotNullIfNeeded(insn, super.unaryOperation(insn, value)) @@ -36,7 +36,12 @@ class NullabilityInterpreter(insns: InsnList) : BoxingInterpreter(insns) { private fun makeNotNullIfNeeded(insn: AbstractInsnNode, value: BasicValue?): BasicValue? = when (insn.opcode) { - Opcodes.ANEWARRAY, Opcodes.NEWARRAY, Opcodes.LDC, Opcodes.NEW -> NotNullBasicValue(value) + Opcodes.ANEWARRAY, Opcodes.NEWARRAY, Opcodes.LDC, Opcodes.NEW -> + if (value?.type?.sort == Type.OBJECT || value?.type?.sort == Type.ARRAY) + NotNullBasicValue(value) + else + value + else -> value } diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/common/OptimizationBasicInterpreter.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/common/OptimizationBasicInterpreter.java index 494fea25ba3..8e0f4057d28 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/common/OptimizationBasicInterpreter.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/common/OptimizationBasicInterpreter.java @@ -86,26 +86,27 @@ public class OptimizationBasicInterpreter extends BasicInterpreter { public BasicValue merge( @NotNull BasicValue v, @NotNull BasicValue w ) { - if (v == BasicValue.UNINITIALIZED_VALUE || w == BasicValue.UNINITIALIZED_VALUE) { + if (!v.equals(w)) { + if (v == BasicValue.UNINITIALIZED_VALUE || w == BasicValue.UNINITIALIZED_VALUE) { + return BasicValue.UNINITIALIZED_VALUE; + } + + // if merge of two references then `lub` is java/lang/Object + // arrays also are BasicValues with reference type's + if (isReference(v) && isReference(w)) { + return BasicValue.REFERENCE_VALUE; + } + + // if merge of something can be stored in int var (int, char, boolean, byte, character) + if (v.getType().getOpcode(Opcodes.ISTORE) == Opcodes.ISTORE && + w.getType().getOpcode(Opcodes.ISTORE) == Opcodes.ISTORE) { + return BasicValue.INT_VALUE; + } + return BasicValue.UNINITIALIZED_VALUE; } - // Objects must be equal, others can just have the same sort - if (v.getType().getSort() == w.getType().getSort() && (v.getType().getSort() != Type.OBJECT || v.equals(w))) { - return v; - } - // if merge of two references then `lub` is java/lang/Object - if (isReference(v) && isReference(w)) { - return BasicValue.REFERENCE_VALUE; - } - - // if merge of something can be stored in int var (int, char, boolean, byte, character) - if (v.getType().getOpcode(Opcodes.ISTORE) == Opcodes.ISTORE && - w.getType().getOpcode(Opcodes.ISTORE) == Opcodes.ISTORE) { - return BasicValue.INT_VALUE; - } - - return BasicValue.UNINITIALIZED_VALUE; + return v; } private static boolean isReference(@NotNull BasicValue v) { diff --git a/compiler/testData/codegen/box/regressions/arrayLengthNPE.kt b/compiler/testData/codegen/box/regressions/arrayLengthNPE.kt new file mode 100644 index 00000000000..e3c91e64aaf --- /dev/null +++ b/compiler/testData/codegen/box/regressions/arrayLengthNPE.kt @@ -0,0 +1,13 @@ +// See KT-14242 +var x = 1 +fun box(): String { + val testArray: Array? = when (1) { + x -> null + else -> arrayOfNulls(0) + } + + // Must not be NPE here + val size = testArray?.size + + return size?.toString() ?: "OK" +} diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java index a2ac0b06c9d..a2991f3971d 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java @@ -13639,6 +13639,12 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/box/regressions"), Pattern.compile("^(.+)\\.kt$"), true); } + @TestMetadata("arrayLengthNPE.kt") + public void testArrayLengthNPE() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/regressions/arrayLengthNPE.kt"); + doTest(fileName); + } + @TestMetadata("collections.kt") public void testCollections() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/regressions/collections.kt");