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

Fix NPE caused by nullability checks optimizations

Browse Source 
Mostly this commit replaces instances of original BasicValue from ASM
with ones of StrictBasicValue having strict equals implementation

Optimization issue was related to non-symmetric 'equals':
- NotNullBasicValue("java/lang/Object").equals(BasicValue("java/lang/Object")) == false
- BasicValue("java/lang/Object").equals(NotNullBasicValue("java/lang/Object")) == true

 #KT-14242 Fixed
This commit is contained in:
Denis Zharkov
2016-10-24 16:13:46 +03:00
parent 18f10860df
commit 3a197e8d7e
11 changed files with 122 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files
compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/boxing/BoxedBasicValue.kt
+3 -4
View File
@@ -18,19 +18,18 @@ package org.jetbrains.kotlin.codegen.optimization.boxing
import com.intellij.openapi.util.Pair
import org.jetbrains.kotlin.codegen.AsmUtil
import org.jetbrains.kotlin.codegen.optimization.common.StrictBasicValue
import org.jetbrains.kotlin.resolve.jvm.AsmTypes
import org.jetbrains.org.objectweb.asm.Type
import org.jetbrains.org.objectweb.asm.tree.AbstractInsnNode
import org.jetbrains.org.objectweb.asm.tree.analysis.BasicValue
import java.util.ArrayList
import java.util.HashSet
import java.util.*
class BoxedBasicValue(
boxedType: Type,
val boxingInsn: AbstractInsnNode,
val progressionIterator: ProgressionIteratorBasicValue?
) : BasicValue(boxedType) {
) : StrictBasicValue(boxedType) {
private val associatedInsns = HashSet<AbstractInsnNode>()
private val unboxingWithCastInsns = HashSet<Pair<AbstractInsnNode, Type>>()
private val associatedVariables = HashSet<Int>()
compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/boxing/BoxingInterpreter.kt
+4 -3
View File
@@ -20,6 +20,7 @@ import com.google.common.collect.ImmutableSet
import org.jetbrains.kotlin.codegen.AsmUtil
import org.jetbrains.kotlin.codegen.RangeCodegenUtil
import org.jetbrains.kotlin.codegen.optimization.common.OptimizationBasicInterpreter
import org.jetbrains.kotlin.codegen.optimization.common.StrictBasicValue
import org.jetbrains.kotlin.name.FqName
import org.jetbrains.kotlin.resolve.jvm.AsmTypes
import org.jetbrains.kotlin.resolve.jvm.JvmPrimitiveType
@@ -93,8 +94,8 @@ open class BoxingInterpreter(private val insnList: InsnList) : OptimizationBasic
override fun merge(v: BasicValue, w: BasicValue) =
when {
v == BasicValue.UNINITIALIZED_VALUE || w == BasicValue.UNINITIALIZED_VALUE -> {
BasicValue.UNINITIALIZED_VALUE
v == StrictBasicValue.UNINITIALIZED_VALUE || w == StrictBasicValue.UNINITIALIZED_VALUE -> {
StrictBasicValue.UNINITIALIZED_VALUE
}
v is BoxedBasicValue && v.typeEquals(w) -> {
onMergeSuccess(v, w as BoxedBasicValue)
@@ -199,4 +200,4 @@ private fun getValuesTypeOfProgressionClass(progressionClassInternalName: String
RangeCodegenUtil.getPrimitiveRangeOrProgressionElementType(buildFqNameByInternal(progressionClassInternalName))?.let {
type ->
type.typeName.asString()
} ?: error("type should be not null")
} ?: error("type should be not null")
compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/boxing/NullabilityInterpreter.kt
+4 -4
View File
@@ -16,7 +16,7 @@
package org.jetbrains.kotlin.codegen.optimization.boxing
import org.jetbrains.kotlin.codegen.optimization.common.BasicValueWrapper
import org.jetbrains.kotlin.codegen.optimization.common.StrictBasicValue
import org.jetbrains.org.objectweb.asm.Opcodes
import org.jetbrains.org.objectweb.asm.Type
import org.jetbrains.org.objectweb.asm.tree.AbstractInsnNode
@@ -31,21 +31,21 @@ class NullabilityInterpreter(insns: InsnList) : BoxingInterpreter(insns) {
override fun isExactValue(value: BasicValue) = super.isExactValue(value) || value is NotNullBasicValue
override fun createNewBoxing(insn: AbstractInsnNode, type: Type, progressionIterator: ProgressionIteratorBasicValue?) =
NotNullBasicValue(BasicValue(type))
NotNullBasicValue(type)
}
private fun makeNotNullIfNeeded(insn: AbstractInsnNode, value: BasicValue?): BasicValue? =
when (insn.opcode) {
Opcodes.ANEWARRAY, Opcodes.NEWARRAY, Opcodes.LDC, Opcodes.NEW ->
if (value?.type?.sort == Type.OBJECT || value?.type?.sort == Type.ARRAY)
NotNullBasicValue(value)
NotNullBasicValue(value.type)
else
value
else -> value
}
class NotNullBasicValue(wrappedValue: BasicValue?) : BasicValueWrapper(wrappedValue) {
class NotNullBasicValue(type: Type?) : StrictBasicValue(type) {
override fun equals(other: Any?): Boolean = other is NotNullBasicValue
// We do not differ not-nullable values, so we should always return the same hashCode
// Actually it doesn't really matter because analyzer is not supposed to store values in hashtables
compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/boxing/ProgressionIteratorBasicValue.java
+2 -2
View File
@@ -21,12 +21,12 @@ import org.jetbrains.annotations.NotNull;
import org.jetbrains.kotlin.builtins.PrimitiveType;
import org.jetbrains.kotlin.codegen.RangeCodegenUtil;
import org.jetbrains.kotlin.codegen.intrinsics.IteratorNext;
import org.jetbrains.kotlin.codegen.optimization.common.StrictBasicValue;
import org.jetbrains.kotlin.name.Name;
import org.jetbrains.kotlin.resolve.jvm.JvmPrimitiveType;
import org.jetbrains.org.objectweb.asm.Type;
import org.jetbrains.org.objectweb.asm.tree.analysis.BasicValue;
public class ProgressionIteratorBasicValue extends BasicValue {
public class ProgressionIteratorBasicValue extends StrictBasicValue {
private final static ImmutableMap<String, Type> VALUES_TYPENAME_TO_TYPE;
static {
compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/boxing/RedundantBoxingMethodTransformer.java
+2 -1
View File
@@ -22,6 +22,7 @@ import com.intellij.openapi.util.Pair;
import kotlin.collections.CollectionsKt;
import kotlin.jvm.functions.Function1;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.kotlin.codegen.optimization.common.StrictBasicValue;
import org.jetbrains.kotlin.codegen.optimization.transformer.MethodTransformer;
import org.jetbrains.org.objectweb.asm.Opcodes;
import org.jetbrains.org.objectweb.asm.Type;
@@ -114,7 +115,7 @@ public class RedundantBoxingMethodTransformer extends MethodTransformer {
if (CollectionsKt.any(usedValues, new Function1<BasicValue, Boolean>() {
@Override
public Boolean invoke(BasicValue input) {
if (input == BasicValue.UNINITIALIZED_VALUE) return false;
if (input == StrictBasicValue.UNINITIALIZED_VALUE) return false;
return input == null ||
!(input instanceof BoxedBasicValue) ||
!((BoxedBasicValue) input).isSafeToRemove() ||
compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/common/OptimizationBasicInterpreter.java
+19 -14
View File
@@ -25,22 +25,27 @@ import org.jetbrains.org.objectweb.asm.tree.analysis.AnalyzerException;
import org.jetbrains.org.objectweb.asm.tree.analysis.BasicInterpreter;
import org.jetbrains.org.objectweb.asm.tree.analysis.BasicValue;
public class OptimizationBasicInterpreter extends BasicInterpreter {
private static final BasicValue BOOLEAN_VALUE = new BasicValue(Type.BOOLEAN_TYPE);
private static final BasicValue CHAR_VALUE = new BasicValue(Type.CHAR_TYPE);
private static final BasicValue BYTE_VALUE = new BasicValue(Type.BYTE_TYPE);
private static final BasicValue SHORT_VALUE = new BasicValue(Type.SHORT_TYPE);
import static org.jetbrains.kotlin.codegen.optimization.common.StrictBasicValue.*;
public class OptimizationBasicInterpreter extends BasicInterpreter {
@Override
@Nullable
public BasicValue newValue(@Nullable Type type) {
public StrictBasicValue newValue(@Nullable Type type) {
if (type == null) {
return super.newValue(null);
return UNINITIALIZED_VALUE;
}
switch (type.getSort()) {
case Type.VOID:
return null;
case Type.INT:
return INT_VALUE;
case Type.FLOAT:
return FLOAT_VALUE;
case Type.LONG:
return LONG_VALUE;
case Type.DOUBLE:
return DOUBLE_VALUE;
case Type.BOOLEAN:
return BOOLEAN_VALUE;
case Type.CHAR:
@@ -51,9 +56,9 @@ public class OptimizationBasicInterpreter extends BasicInterpreter {
return SHORT_VALUE;
case Type.OBJECT:
case Type.ARRAY:
return new BasicValue(type);
return new StrictBasicValue(type);
default:
return super.newValue(type);
throw new IllegalArgumentException("Unknown type sort " + type.getSort());
}
}
@@ -88,23 +93,23 @@ public class OptimizationBasicInterpreter extends BasicInterpreter {
) {
if (v.equals(w)) return v;
if (v == BasicValue.UNINITIALIZED_VALUE || w == BasicValue.UNINITIALIZED_VALUE) {
return BasicValue.UNINITIALIZED_VALUE;
if (v == StrictBasicValue.UNINITIALIZED_VALUE || w == StrictBasicValue.UNINITIALIZED_VALUE) {
return StrictBasicValue.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;
return StrictBasicValue.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 StrictBasicValue.INT_VALUE;
}
return BasicValue.UNINITIALIZED_VALUE;
return StrictBasicValue.UNINITIALIZED_VALUE;
}
private static boolean isReference(@NotNull BasicValue v) {
compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/common/StrictBasicValue.kt
+63
View File
@@ -0,0 +1,63 @@
/*
* Copyright 2010-2016 JetBrains s.r.o.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.jetbrains.kotlin.codegen.optimization.common
import org.jetbrains.org.objectweb.asm.Type
import org.jetbrains.org.objectweb.asm.tree.analysis.BasicValue
/**
* This class has strict `equals` implementation, that does not accept its subclasses,
* unlike BasicValue that requires `other` instance only to be a subtype of BasicValue and must have the same `type`
*/
open class StrictBasicValue(type: Type?) : BasicValue(type) {
companion object {
@JvmField
val UNINITIALIZED_VALUE = StrictBasicValue(null)
@JvmField
val INT_VALUE = StrictBasicValue(Type.INT_TYPE)
@JvmField
val FLOAT_VALUE = StrictBasicValue(Type.FLOAT_TYPE)
@JvmField
val LONG_VALUE = StrictBasicValue(Type.LONG_TYPE)
@JvmField
val DOUBLE_VALUE = StrictBasicValue(Type.DOUBLE_TYPE)
@JvmField
val BOOLEAN_VALUE = StrictBasicValue(Type.BOOLEAN_TYPE)
@JvmField
val CHAR_VALUE = StrictBasicValue(Type.CHAR_TYPE)
@JvmField
val BYTE_VALUE = StrictBasicValue(Type.BYTE_TYPE)
@JvmField
val SHORT_VALUE = StrictBasicValue(Type.SHORT_TYPE)
@JvmField
val REFERENCE_VALUE = StrictBasicValue(Type.getObjectType("java/lang/Object"))
}
override fun equals(other: Any?): Boolean {
if (this === other) return true
if (other?.javaClass != javaClass) return false
if (!super.equals(other)) return false
other as StrictBasicValue
if (type != other.type) return false
return true
}
override fun hashCode() = (type?.hashCode() ?: 0)
}
compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/common/Util.kt
-9
View File
@@ -19,7 +19,6 @@ package org.jetbrains.kotlin.codegen.optimization.common
import org.jetbrains.org.objectweb.asm.Opcodes
import org.jetbrains.org.objectweb.asm.Opcodes.*
import org.jetbrains.org.objectweb.asm.tree.*
import org.jetbrains.org.objectweb.asm.tree.analysis.BasicValue
val AbstractInsnNode.isMeaningful: Boolean get() =
when (this.type) {
@@ -77,14 +76,6 @@ fun MethodNode.removeEmptyCatchBlocks() {
}
}
abstract class BasicValueWrapper(val wrappedValue: BasicValue?) : BasicValue(wrappedValue?.type) {
val basicValue: BasicValue? get() = (wrappedValue as? BasicValueWrapper)?.basicValue ?: wrappedValue
override fun equals(other: Any?): Boolean {
return super.equals(other) && this.javaClass == other?.javaClass
}
}
inline fun AbstractInsnNode.findNextOrNull(predicate: (AbstractInsnNode) -> Boolean): AbstractInsnNode? {
var finger = this.next
while (finger != null && !predicate(finger)) {
compiler/testData/codegen/box/regressions/hashCodeNPE.kt
Vendored
+13
View File
@@ -0,0 +1,13 @@
// See KT-14242
var x = 1
fun box(): String {
val any: Any? = when (1) {
x -> null
else -> Any()
}
// Must not be NPE here
val hashCode = any?.hashCode()
return hashCode?.toString() ?: "OK"
}
compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java
+6
View File
@@ -13846,6 +13846,12 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes
doTest(fileName);
}
@TestMetadata("hashCodeNPE.kt")
public void testHashCodeNPE() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/regressions/hashCodeNPE.kt");
doTest(fileName);
}
@TestMetadata("internalTopLevelOtherPackage.kt")
public void testInternalTopLevelOtherPackage() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/regressions/internalTopLevelOtherPackage.kt");
compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java
+6
View File
@@ -13846,6 +13846,12 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest {
doTest(fileName);
}
@TestMetadata("hashCodeNPE.kt")
public void testHashCodeNPE() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/regressions/hashCodeNPE.kt");
doTest(fileName);
}
@TestMetadata("internalTopLevelOtherPackage.kt")
public void testInternalTopLevelOtherPackage() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/regressions/internalTopLevelOtherPackage.kt");