From eded72d633b93a91a45c3619b830ba6be02e01ca Mon Sep 17 00:00:00 2001 From: Michael Nedzelsky Date: Tue, 23 Dec 2014 20:09:50 +0300 Subject: [PATCH] fix KT-6192: VerifyError when using spreads with primitive arrays #KT-6192 Fixed --- .../org/jetbrains/jet/codegen/AsmUtil.java | 7 + .../jet/codegen/ExpressionCodegen.java | 37 +++-- .../org/jetbrains/jet/codegen/StackValue.java | 23 +-- .../testData/codegen/box/vararg/kt6192.kt | 93 +++++++++++ .../BlackBoxCodegenTestGenerated.java | 6 + .../jvm/internal/PrimitiveSpreadBuilders.kt | 147 ++++++++++++++++++ .../kotlin/jvm/internal/SpreadBuilder.java | 32 +++- 7 files changed, 313 insertions(+), 32 deletions(-) create mode 100644 compiler/testData/codegen/box/vararg/kt6192.kt create mode 100644 core/runtime.jvm/src/kotlin/jvm/internal/PrimitiveSpreadBuilders.kt diff --git a/compiler/backend/src/org/jetbrains/jet/codegen/AsmUtil.java b/compiler/backend/src/org/jetbrains/jet/codegen/AsmUtil.java index c38eb54b0e4..ba9d4d05df6 100644 --- a/compiler/backend/src/org/jetbrains/jet/codegen/AsmUtil.java +++ b/compiler/backend/src/org/jetbrains/jet/codegen/AsmUtil.java @@ -45,6 +45,7 @@ import org.jetbrains.jet.lang.types.Approximation; import org.jetbrains.jet.lang.types.JetType; import org.jetbrains.jet.lang.types.TypesPackage; import org.jetbrains.jet.lang.types.lang.KotlinBuiltIns; +import org.jetbrains.jet.lang.types.lang.PrimitiveType; import org.jetbrains.jet.lexer.JetTokens; import org.jetbrains.org.objectweb.asm.*; import org.jetbrains.org.objectweb.asm.commons.InstructionAdapter; @@ -159,6 +160,12 @@ public class AsmUtil { return Type.getType(internalName.substring(1)); } + @Nullable + public static PrimitiveType asmPrimitiveTypeToLangPrimitiveType(Type type) { + JvmPrimitiveType jvmPrimitiveType = primitiveTypeByAsmSort.get(type.getSort()); + return jvmPrimitiveType != null ? jvmPrimitiveType.getPrimitiveType() : null; + } + @NotNull public static Method method(@NotNull String name, @NotNull Type returnType, @NotNull Type... parameterTypes) { return new Method(name, Type.getMethodDescriptor(returnType, parameterTypes)); diff --git a/compiler/backend/src/org/jetbrains/jet/codegen/ExpressionCodegen.java b/compiler/backend/src/org/jetbrains/jet/codegen/ExpressionCodegen.java index 4883f6bb2f6..51d556c4343 100644 --- a/compiler/backend/src/org/jetbrains/jet/codegen/ExpressionCodegen.java +++ b/compiler/backend/src/org/jetbrains/jet/codegen/ExpressionCodegen.java @@ -2524,10 +2524,25 @@ public class ExpressionCodegen extends JetVisitor implem gen(arguments.get(0).getArgumentExpression(), type); } else { - String owner = "kotlin/jvm/internal/SpreadBuilder"; + String owner; + String addDescriptor; + String toArrayDescriptor; + boolean arrayOfReferences = KotlinBuiltIns.isArray(outType); + if (arrayOfReferences) { + owner = "kotlin/jvm/internal/SpreadBuilder"; + addDescriptor = "(Ljava/lang/Object;)V"; + toArrayDescriptor = "([Ljava/lang/Object;)[Ljava/lang/Object;"; + } + else { + String spreadBuilderClassName = AsmUtil.asmPrimitiveTypeToLangPrimitiveType(elementType).getTypeName().getIdentifier() + "SpreadBuilder"; + owner = "kotlin/jvm/internal/" + spreadBuilderClassName; + addDescriptor = "(" + elementType.getDescriptor() + ")V"; + toArrayDescriptor = "()" + type.getDescriptor(); + } v.anew(Type.getObjectType(owner)); v.dup(); - v.invokespecial(owner, "", "()V", false); + v.iconst(size); + v.invokespecial(owner, "", "(I)V", false); for (int i = 0; i != size; ++i) { v.dup(); ValueArgument argument = arguments.get(i); @@ -2537,15 +2552,19 @@ public class ExpressionCodegen extends JetVisitor implem } else { gen(argument.getArgumentExpression(), elementType); - v.invokevirtual(owner, "add", "(Ljava/lang/Object;)Z", false); - v.pop(); + v.invokevirtual(owner, "add", addDescriptor, false); } } - v.dup(); - v.invokevirtual(owner, "size", "()I", false); - newArrayInstruction(outType); - v.invokevirtual(owner, "toArray", "([Ljava/lang/Object;)[Ljava/lang/Object;", false); - v.checkcast(type); + if (arrayOfReferences) { + v.dup(); + v.invokevirtual(owner, "size", "()I", false); + newArrayInstruction(outType); + v.invokevirtual(owner, "toArray", toArrayDescriptor, false); + v.checkcast(type); + } + else { + v.invokevirtual(owner, "toArray", toArrayDescriptor, false); + } } } else { diff --git a/compiler/backend/src/org/jetbrains/jet/codegen/StackValue.java b/compiler/backend/src/org/jetbrains/jet/codegen/StackValue.java index 49561b43c4a..ea2c0bc8108 100644 --- a/compiler/backend/src/org/jetbrains/jet/codegen/StackValue.java +++ b/compiler/backend/src/org/jetbrains/jet/codegen/StackValue.java @@ -35,6 +35,7 @@ import org.jetbrains.jet.lang.resolve.java.JvmAbi; import org.jetbrains.jet.lang.resolve.java.jvmSignature.JvmMethodParameterKind; import org.jetbrains.jet.lang.resolve.java.jvmSignature.JvmMethodParameterSignature; import org.jetbrains.jet.lang.resolve.scopes.receivers.ReceiverValue; +import org.jetbrains.jet.lang.types.lang.PrimitiveType; import org.jetbrains.jet.lexer.JetTokens; import org.jetbrains.org.objectweb.asm.Label; import org.jetbrains.org.objectweb.asm.Type; @@ -1229,24 +1230,12 @@ public abstract class StackValue { case Type.OBJECT: case Type.ARRAY: return OBJECT_REF_TYPE; - case Type.BYTE: - return Type.getObjectType("kotlin/jvm/internal/Ref$ByteRef"); - case Type.SHORT: - return Type.getObjectType("kotlin/jvm/internal/Ref$ShortRef"); - case Type.CHAR: - return Type.getObjectType("kotlin/jvm/internal/Ref$CharRef"); - case Type.INT: - return Type.getObjectType("kotlin/jvm/internal/Ref$IntRef"); - case Type.LONG: - return Type.getObjectType("kotlin/jvm/internal/Ref$LongRef"); - case Type.BOOLEAN: - return Type.getObjectType("kotlin/jvm/internal/Ref$BooleanRef"); - case Type.FLOAT: - return Type.getObjectType("kotlin/jvm/internal/Ref$FloatRef"); - case Type.DOUBLE: - return Type.getObjectType("kotlin/jvm/internal/Ref$DoubleRef"); default: - throw new UnsupportedOperationException(); + PrimitiveType primitiveType = AsmUtil.asmPrimitiveTypeToLangPrimitiveType(type); + if (primitiveType == null) throw new UnsupportedOperationException(); + + String typeName = primitiveType.getTypeName().getIdentifier(); + return Type.getObjectType("kotlin/jvm/internal/Ref$" + typeName + "Ref"); } } diff --git a/compiler/testData/codegen/box/vararg/kt6192.kt b/compiler/testData/codegen/box/vararg/kt6192.kt new file mode 100644 index 00000000000..1c75e53a2e6 --- /dev/null +++ b/compiler/testData/codegen/box/vararg/kt6192.kt @@ -0,0 +1,93 @@ + +fun barB(vararg args: Byte) = args +fun barC(vararg args: Char) = args +fun barD(vararg args: Double) = args +fun barF(vararg args: Float) = args +fun barI(vararg args: Int) = args +fun barJ(vararg args: Long) = args +fun barS(vararg args: Short) = args +fun barZ(vararg args: Boolean) = args + +fun sumInt(x: Int, vararg args: Int): Int { + var result = x + for(a in args) { + result += a + } + return result +} + +fun sumFunOnParameters(x: Int, vararg args: Int, f: (Int) -> Int): Int { + var result = f(x) + for(a in args) { + result += f(a) + } + return result +} + +fun concatParameters(vararg args: Int): String { + var result = "" + for(a in args) { + result += a.toString() + } + return result +} + +fun box(): String { + + val aB = ByteArray(3) + val aC = CharArray(3) + val aD = DoubleArray(3) + val aF = FloatArray(3) + val aI = IntArray(3) + aI[0] = 1 + aI[1] = 2 + aI[2] = 3 + val bI = IntArray(2) + bI[0] = 4 + bI[1] = 5 + val aJ = LongArray(3) + val aS = ShortArray(3) + val aZ = BooleanArray(3) + + + if (barB(*aB, 23: Byte).size() != 4) return "fail: Byte" + if (barB(11: Byte, *aB, 23: Byte, *aB).size() != 8) return "fail: Byte" + + if (barC(*aC, 'A').size() != 4) return "fail: Char" + if (barC('A', *aC, 'A', *aC).size() != 8) return "fail: Char" + + if (barD(*aD, 2.3).size() != 4) return "fail: Double" + if (barD(*aD, *aD, 2.3).size() != 7) return "fail: Double" + + if (barF(*aF, 2.3f).size() != 4) return "fail: Float" + if (barF(*aF, 2.3f, 1.1f).size() != 5) return "fail: Float" + + if (barI(*aI, 23).size() != 4) return "fail: Int" + if (barI(11, 10, *aI, 23).size() != 6) return "fail: Int" + if (barI(100, *aI, *aI).size() != 7) return "fail: Int 3" + + if (sumInt(100, *aI) != 106) return "fail: sumInt 1" + if (sumInt(100, *aI, 200) != 306) return "fail: sumInt 2" + if (sumInt(100, *aI, *aI) != 112) return "fail: sumInt 3" + if (sumFunOnParameters(100, *aI, 200) { 2*it } != 612) return "fail: sumFunOnParameters 1" + if (sumFunOnParameters(100, *aI, *aI) { 2*it } != 224) return "fail: sumFunOnParameters 2" + + if (concatParameters(1,2,3) != "123") return "fail: concatParameters 1" + if (concatParameters(*aI) != "123") return "fail: concatParameters 2" + if (concatParameters(4, 5, *aI) != "45123") return "fail: concatParameters 3" + if (concatParameters(*aI, 4, 5) != "12345") return "fail: concatParameters 4" + if (concatParameters(*aI, *bI) != "12345") return "fail: concatParameters 5" + if (concatParameters(*aI, 7, 8, *bI) != "1237845") return "fail: concatParameters 6" + if (concatParameters(*aI, 7, *bI, *aI, 9) != "1237451239") return "fail: concatParameters 7" + + if (barJ(*aJ, 23: Long).size() != 4) return "fail: Long" + if (barJ(*aJ, 23: Long, *aJ, *aJ).size() != 10) return "fail: Long" + + if (barS(*aS, 23: Short).size() != 4) return "fail: Short" + if (barS(*aS, *aS, 23: Short, *aS).size() != 10) return "fail: Short" + + if (barZ(*aZ, true).size() != 4) return "fail: Boolean" + if (barZ(false, *aZ, true, *aZ).size() != 8) return "fail: Boolean" + + return "OK" +} diff --git a/compiler/tests/org/jetbrains/jet/codegen/generated/BlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/jet/codegen/generated/BlackBoxCodegenTestGenerated.java index df21dca9d50..f210099a8d5 100644 --- a/compiler/tests/org/jetbrains/jet/codegen/generated/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/jet/codegen/generated/BlackBoxCodegenTestGenerated.java @@ -6874,6 +6874,12 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { doTest(fileName); } + @TestMetadata("kt6192.kt") + public void testKt6192() throws Exception { + String fileName = JetTestUtils.navigationMetadata("compiler/testData/codegen/box/vararg/kt6192.kt"); + doTest(fileName); + } + @TestMetadata("kt796_797.kt") public void testKt796_797() throws Exception { String fileName = JetTestUtils.navigationMetadata("compiler/testData/codegen/box/vararg/kt796_797.kt"); diff --git a/core/runtime.jvm/src/kotlin/jvm/internal/PrimitiveSpreadBuilders.kt b/core/runtime.jvm/src/kotlin/jvm/internal/PrimitiveSpreadBuilders.kt new file mode 100644 index 00000000000..0f6784f3bbe --- /dev/null +++ b/core/runtime.jvm/src/kotlin/jvm/internal/PrimitiveSpreadBuilders.kt @@ -0,0 +1,147 @@ +/* + * Copyright 2010-2014 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 kotlin.jvm.internal + +private abstract class PrimitiveSpreadBuilder(private val size: Int) { + abstract protected fun T.getSize(): Int + + protected var position: Int = 0 + private val spreads: Array = arrayOfNulls(size) as Array + + public fun addSpread(spreadArgument: T) { + spreads[position++] = spreadArgument + } + + protected fun size(): Int { + var totalLength = 0 + for (i in 0..size - 1) { + totalLength += spreads[i]?.getSize() ?: 1 + } + return totalLength + } + + protected fun toArray(values: T, result: T): T { + var dstIndex = 0 + var copyValuesFrom = 0 + for (i in 0..size - 1) { + val spreadArgument = spreads[i] + if (spreadArgument != null) { + if (copyValuesFrom < i) { + System.arraycopy(values, copyValuesFrom, result, dstIndex, i-copyValuesFrom) + dstIndex += i - copyValuesFrom + } + val spreadSize = spreadArgument.getSize() + System.arraycopy(spreadArgument, 0, result, dstIndex, spreadSize) + dstIndex += spreadSize + copyValuesFrom = i+1 + } + } + if (copyValuesFrom < size) { + System.arraycopy(values, copyValuesFrom, result, dstIndex, size-copyValuesFrom) + } + + return result + } +} + +public class ByteSpreadBuilder(size: Int) : PrimitiveSpreadBuilder(size) { + private val values: ByteArray = ByteArray(size) + override fun ByteArray.getSize(): Int = this.size() + + public fun add(value: Byte) { + values[position++] = value + } + + public fun toArray(): ByteArray = toArray(values, ByteArray(size())) +} + +public class CharSpreadBuilder(size: Int) : PrimitiveSpreadBuilder(size) { + private val values: CharArray = CharArray(size) + override fun CharArray.getSize(): Int = this.size() + + public fun add(value: Char) { + values[position++] = value + } + + public fun toArray(): CharArray = toArray(values, CharArray(size())) +} + +public class DoubleSpreadBuilder(size: Int) : PrimitiveSpreadBuilder(size) { + private val values: DoubleArray = DoubleArray(size) + override fun DoubleArray.getSize(): Int = this.size() + + public fun add(value: Double) { + values[position++] = value + } + + public fun toArray(): DoubleArray = toArray(values, DoubleArray(size())) +} + +public class FloatSpreadBuilder(size: Int) : PrimitiveSpreadBuilder(size) { + private val values: FloatArray = FloatArray(size) + override fun FloatArray.getSize(): Int = this.size() + + public fun add(value: Float) { + values[position++] = value + } + + public fun toArray(): FloatArray = toArray(values, FloatArray(size())) +} + +public class IntSpreadBuilder(size: Int) : PrimitiveSpreadBuilder(size) { + private val values: IntArray = IntArray(size) + override fun IntArray.getSize(): Int = this.size() + + public fun add(value: Int) { + values[position++] = value + } + + public fun toArray(): IntArray = toArray(values, IntArray(size())) +} + +public class LongSpreadBuilder(size: Int) : PrimitiveSpreadBuilder(size) { + private val values: LongArray = LongArray(size) + override fun LongArray.getSize(): Int = this.size() + + public fun add(value: Long) { + values[position++] = value + } + + public fun toArray(): LongArray = toArray(values, LongArray(size())) +} + +public class ShortSpreadBuilder(size: Int) : PrimitiveSpreadBuilder(size) { + private val values: ShortArray = ShortArray(size) + override fun ShortArray.getSize(): Int = this.size() + + public fun add(value: Short) { + values[position++] = value + } + + public fun toArray(): ShortArray = toArray(values, ShortArray(size())) +} + +public class BooleanSpreadBuilder(size: Int) : PrimitiveSpreadBuilder(size) { + private val values: BooleanArray = BooleanArray(size) + override fun BooleanArray.getSize(): Int = this.size() + + public fun add(value: Boolean) { + values[position++] = value + } + + public fun toArray(): BooleanArray = toArray(values, BooleanArray(size())) +} diff --git a/core/runtime.jvm/src/kotlin/jvm/internal/SpreadBuilder.java b/core/runtime.jvm/src/kotlin/jvm/internal/SpreadBuilder.java index 4d359670d62..aaef8fc6552 100644 --- a/core/runtime.jvm/src/kotlin/jvm/internal/SpreadBuilder.java +++ b/core/runtime.jvm/src/kotlin/jvm/internal/SpreadBuilder.java @@ -16,38 +16,58 @@ package kotlin.jvm.internal; +import java.lang.Object; import java.util.ArrayList; import java.util.Collection; import java.util.Iterator; -public class SpreadBuilder extends ArrayList { +public class SpreadBuilder { + + private final ArrayList list; + + public SpreadBuilder(int size) { + list = new ArrayList(size); + } + public void addSpread(Object container) { if (container == null) return; if (container instanceof Object[]) { Object[] array = (Object[]) container; if (array.length > 0) { - ensureCapacity(size() + array.length); + list.ensureCapacity(list.size() + array.length); for (Object element : array) { - add(element); + list.add(element); } } } else if (container instanceof Collection) { - addAll((Collection) container); + list.addAll((Collection) container); } else if (container instanceof Iterable) { for (Object element : (Iterable) container) { - add(element); + list.add(element); } } else if (container instanceof Iterator) { for (Iterator iterator = (Iterator) container; iterator.hasNext(); ) { - add(iterator.next()); + list.add(iterator.next()); } } else { throw new UnsupportedOperationException("Don't know how to spread " + container.getClass()); } } + + public int size() { + return list.size(); + } + + public void add(Object element) { + list.add(element); + } + + public Object[] toArray(Object[] a) { + return list.toArray(a); + } }