From 3b2843fe7a0c428f87609f326daa5ccf2ee74de0 Mon Sep 17 00:00:00 2001 From: Mads Ager Date: Thu, 26 Sep 2019 13:42:42 +0200 Subject: [PATCH] Introduce local variable type checker. CheckLocalVariablesTableTests will now check the validity of the locals table against types of locals computed based on the bytecode. These checks and the new destructuringInFor test act as a regression test for the changes in https://github.com/JetBrains/kotlin/pull/2613 These checks also caught a similar issue for destructuring lambda parameters, where the local is introduced before the value has been written to the local slot. This change also fixes that. Finally, this change fixes the asmLike tests to correctly look up the name of parameters in the locals table. --- .../codegen/ClosureGenerationStrategy.kt | 3 +- .../kotlin/codegen/FunctionCodegen.java | 45 +-- .../jetbrains/kotlin/codegen/codegenUtil.kt | 2 +- .../destructuringInFor.kt | 14 + .../destructuringInLambdas.kt | 4 +- .../destructuringInlineLambda.kt | 4 +- .../parametersInSuspendLambda/inline.kt | 4 +- .../underscoreNames.kt | 6 +- .../asmLike/receiverMangling/deepInline.txt | 4 +- .../receiverMangling/deepInlineWithLabels.txt | 4 +- .../deepNoinlineWithLabels_after.txt | 8 +- .../deepNoinlineWithLabels_before.txt | 8 +- .../receiverMangling/deepNoinline_after.txt | 8 +- .../receiverMangling/deepNoinline_before.txt | 8 +- .../receiverMangling/inlineReceivers.txt | 4 +- .../asmLike/receiverMangling/mangledNames.txt | 4 +- .../nonInlineReceivers_after.txt | 4 +- .../nonInlineReceivers_before.txt | 4 +- .../AbstractAsmLikeInstructionListingTest.kt | 6 +- .../AbstractCheckLocalVariablesTableTest.kt | 306 +++++++++++++++++- ...CheckLocalVariablesTableTestGenerated.java | 5 + ...CheckLocalVariablesTableTestGenerated.java | 5 + 22 files changed, 367 insertions(+), 93 deletions(-) create mode 100644 compiler/testData/checkLocalVariablesTable/destructuringInFor.kt diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/ClosureGenerationStrategy.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/ClosureGenerationStrategy.kt index bb57de8a86c..58c6f17a7b6 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/ClosureGenerationStrategy.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/ClosureGenerationStrategy.kt @@ -27,7 +27,8 @@ class ClosureGenerationStrategy( ) : FunctionGenerationStrategy.FunctionDefault(state, declaration) { override fun doGenerateBody(codegen: ExpressionCodegen, signature: JvmMethodSignature) { - initializeVariablesForDestructuredLambdaParameters(codegen, codegen.context.functionDescriptor.valueParameters) + initializeVariablesForDestructuredLambdaParameters( + codegen, codegen.context.functionDescriptor.valueParameters, codegen.context.methodEndLabel) if (declaration is KtFunctionLiteral) { recordCallLabelForLambdaArgument(declaration, state.bindingTrace) } diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/FunctionCodegen.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/FunctionCodegen.java index 32e2a34f3c0..8911acf6b02 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/FunctionCodegen.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/FunctionCodegen.java @@ -29,7 +29,6 @@ import org.jetbrains.kotlin.descriptors.*; import org.jetbrains.kotlin.descriptors.annotations.Annotated; import org.jetbrains.kotlin.descriptors.annotations.AnnotationDescriptor; import org.jetbrains.kotlin.descriptors.impl.AnonymousFunctionDescriptor; -import org.jetbrains.kotlin.descriptors.impl.ValueParameterDescriptorImpl; import org.jetbrains.kotlin.load.java.BuiltinMethodsWithSpecialGenericSignature; import org.jetbrains.kotlin.load.java.JvmAbi; import org.jetbrains.kotlin.load.java.SpecialBuiltinMembers; @@ -767,7 +766,7 @@ public class FunctionCodegen { generateLocalVariablesForParameters(mv, jvmMethodSignature, functionDescriptor, thisType, methodBegin, methodEnd, functionDescriptor.getValueParameters(), - AsmUtil.isStaticMethod(ownerKind, functionDescriptor), state, shiftForDestructuringVariables + AsmUtil.isStaticMethod(ownerKind, functionDescriptor), state ); } @@ -781,24 +780,6 @@ public class FunctionCodegen { Collection valueParameters, boolean isStatic, @NotNull GenerationState state - ) { - generateLocalVariablesForParameters( - mv, jvmMethodSignature, functionDescriptor, - thisType, methodBegin, methodEnd, valueParameters, isStatic, state, - 0); - } - - private static void generateLocalVariablesForParameters( - @NotNull MethodVisitor mv, - @NotNull JvmMethodSignature jvmMethodSignature, - @NotNull FunctionDescriptor functionDescriptor, - @Nullable Type thisType, - @NotNull Label methodBegin, - @NotNull Label methodEnd, - Collection valueParameters, - boolean isStatic, - @NotNull GenerationState state, - int shiftForDestructuringVariables ) { Iterator valueParameterIterator = valueParameters.iterator(); List params = jvmMethodSignature.getValueParameters(); @@ -845,30 +826,6 @@ public class FunctionCodegen { mv.visitLocalVariable(parameterName, type.getDescriptor(), null, methodBegin, methodEnd, shift); shift += type.getSize(); } - - shift += shiftForDestructuringVariables; - generateDestructuredParameterEntries(mv, methodBegin, methodEnd, valueParameters, typeMapper, shift); - } - - private static int generateDestructuredParameterEntries( - @NotNull MethodVisitor mv, - @NotNull Label methodBegin, - @NotNull Label methodEnd, - Collection valueParameters, - KotlinTypeMapper typeMapper, - int shift - ) { - for (ValueParameterDescriptor parameter : valueParameters) { - List destructuringVariables = ValueParameterDescriptorImpl.getDestructuringVariablesOrNull(parameter); - if (destructuringVariables == null) continue; - - for (VariableDescriptor entry : CodegenUtilKt.filterOutDescriptorsWithSpecialNames(destructuringVariables)) { - Type type = typeMapper.mapType(entry.getType()); - mv.visitLocalVariable(entry.getName().asString(), type.getDescriptor(), null, methodBegin, methodEnd, shift); - shift += type.getSize(); - } - } - return shift; } private static String computeParameterName(int i, ValueParameterDescriptor parameter) { diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/codegenUtil.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/codegenUtil.kt index 39f3b378414..d40edd2b67a 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/codegenUtil.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/codegenUtil.kt @@ -323,7 +323,7 @@ fun MemberDescriptor.isToArrayFromCollection(): Boolean { fun FqName.topLevelClassInternalName() = JvmClassName.byClassId(ClassId(parent(), shortName())).internalName fun FqName.topLevelClassAsmType(): Type = Type.getObjectType(topLevelClassInternalName()) -fun initializeVariablesForDestructuredLambdaParameters(codegen: ExpressionCodegen, valueParameters: List, endLabel: Label? = null) { +fun initializeVariablesForDestructuredLambdaParameters(codegen: ExpressionCodegen, valueParameters: List, endLabel: Label?) { // Do not write line numbers until destructuring happens // (otherwise destructuring variables will be uninitialized in the beginning of lambda) codegen.runWithShouldMarkLineNumbers(false) { diff --git a/compiler/testData/checkLocalVariablesTable/destructuringInFor.kt b/compiler/testData/checkLocalVariablesTable/destructuringInFor.kt new file mode 100644 index 00000000000..c68ccef4d3f --- /dev/null +++ b/compiler/testData/checkLocalVariablesTable/destructuringInFor.kt @@ -0,0 +1,14 @@ +// WITH_RUNTIME + +fun box() { + val map: Map = mapOf() + for ((a, b) in map) { + a + b + } +} + +// METHOD : DestructuringInForKt.box()V + +// VARIABLE : NAME=b TYPE=Ljava/lang/String; INDEX=4 +// VARIABLE : NAME=a TYPE=Ljava/lang/String; INDEX=3 +// VARIABLE : NAME=map TYPE=Ljava/util/Map; INDEX=0 \ No newline at end of file diff --git a/compiler/testData/checkLocalVariablesTable/destructuringInLambdas.kt b/compiler/testData/checkLocalVariablesTable/destructuringInLambdas.kt index 4370f63cf10..bc5d25b361d 100644 --- a/compiler/testData/checkLocalVariablesTable/destructuringInLambdas.kt +++ b/compiler/testData/checkLocalVariablesTable/destructuringInLambdas.kt @@ -8,7 +8,7 @@ fun box() { } // METHOD : DestructuringInLambdasKt$box$1.invoke(LA;)Ljava/lang/String; -// VARIABLE : NAME=this TYPE=LDestructuringInLambdasKt$box$1; INDEX=0 -// VARIABLE : NAME=$dstr$x$y TYPE=LA; INDEX=1 // VARIABLE : NAME=x TYPE=Ljava/lang/String; INDEX=2 // VARIABLE : NAME=y TYPE=I INDEX=3 +// VARIABLE : NAME=this TYPE=LDestructuringInLambdasKt$box$1; INDEX=0 +// VARIABLE : NAME=$dstr$x$y TYPE=LA; INDEX=1 \ No newline at end of file diff --git a/compiler/testData/checkLocalVariablesTable/destructuringInlineLambda.kt b/compiler/testData/checkLocalVariablesTable/destructuringInlineLambda.kt index 78e0405e8e6..05cc9338cf9 100644 --- a/compiler/testData/checkLocalVariablesTable/destructuringInlineLambda.kt +++ b/compiler/testData/checkLocalVariablesTable/destructuringInlineLambda.kt @@ -14,10 +14,10 @@ fun box(): String { } // METHOD : DestructuringInlineLambdaKt.box()Ljava/lang/String; -// VARIABLE : NAME=i TYPE=I INDEX=2 -// VARIABLE : NAME=$dstr$a1$a2$a3 TYPE=LStation; INDEX=1 // VARIABLE : NAME=a1 TYPE=Ljava/lang/String; INDEX=4 // VARIABLE : NAME=a2 TYPE=Ljava/lang/String; INDEX=5 // VARIABLE : NAME=a3 TYPE=I INDEX=6 +// VARIABLE : NAME=i TYPE=I INDEX=2 +// VARIABLE : NAME=$dstr$a1$a2$a3 TYPE=LStation; INDEX=1 // VARIABLE : NAME=$i$a$-foo-DestructuringInlineLambdaKt$box$1 TYPE=I INDEX=3 // VARIABLE : NAME=$i$f$foo TYPE=I INDEX=0 diff --git a/compiler/testData/checkLocalVariablesTable/parametersInSuspendLambda/inline.kt b/compiler/testData/checkLocalVariablesTable/parametersInSuspendLambda/inline.kt index 40006e93939..69e8801433b 100644 --- a/compiler/testData/checkLocalVariablesTable/parametersInSuspendLambda/inline.kt +++ b/compiler/testData/checkLocalVariablesTable/parametersInSuspendLambda/inline.kt @@ -7,10 +7,10 @@ suspend inline fun foo(a: A, block: suspend (A) -> String): String = block(a) suspend fun test() = foo(A("O", "K")) { (x_param, y_param) -> x_param + y_param } // METHOD : InlineKt.test(Lkotlin/coroutines/Continuation;)Ljava/lang/Object; -// VARIABLE : NAME=$dstr$x_param$y_param TYPE=LA; INDEX=4 -// VARIABLE : NAME=continuation TYPE=Lkotlin/coroutines/Continuation; INDEX=3 // VARIABLE : NAME=x_param TYPE=Ljava/lang/String; INDEX=6 // VARIABLE : NAME=y_param TYPE=Ljava/lang/String; INDEX=7 +// VARIABLE : NAME=$dstr$x_param$y_param TYPE=LA; INDEX=4 +// VARIABLE : NAME=continuation TYPE=Lkotlin/coroutines/Continuation; INDEX=3 // VARIABLE : NAME=$i$a$-foo-InlineKt$test$2 TYPE=I INDEX=5 // VARIABLE : NAME=a$iv TYPE=LA; INDEX=1 // VARIABLE : NAME=$i$f$foo TYPE=I INDEX=2 diff --git a/compiler/testData/checkLocalVariablesTable/underscoreNames.kt b/compiler/testData/checkLocalVariablesTable/underscoreNames.kt index 6ff93bc9f99..dfedc33a270 100644 --- a/compiler/testData/checkLocalVariablesTable/underscoreNames.kt +++ b/compiler/testData/checkLocalVariablesTable/underscoreNames.kt @@ -22,6 +22,8 @@ fun box() { } // METHOD : UnderscoreNamesKt$box$1.invoke(LA;Ljava/lang/String;I)Ljava/lang/String; +// VARIABLE : NAME=x TYPE=D INDEX=4 +// VARIABLE : NAME=y TYPE=C INDEX=6 // VARIABLE : NAME=q TYPE=Ljava/lang/String; INDEX=16 // VARIABLE : NAME=d TYPE=C INDEX=11 // VARIABLE : NAME=_ TYPE=Ljava/lang/String; INDEX=10 @@ -30,6 +32,4 @@ fun box() { // VARIABLE : NAME=this TYPE=LUnderscoreNamesKt$box$1; INDEX=0 // VARIABLE : NAME=$dstr$x$_u24__u24$y TYPE=LA; INDEX=1 // VARIABLE : NAME=$noName_1 TYPE=Ljava/lang/String; INDEX=2 -// VARIABLE : NAME=w TYPE=I INDEX=3 -// VARIABLE : NAME=x TYPE=D INDEX=4 -// VARIABLE : NAME=y TYPE=C INDEX=6 +// VARIABLE : NAME=w TYPE=I INDEX=3 \ No newline at end of file diff --git a/compiler/testData/codegen/asmLike/receiverMangling/deepInline.txt b/compiler/testData/codegen/asmLike/receiverMangling/deepInline.txt index 69ee7ef1026..0ad08afa5ad 100644 --- a/compiler/testData/codegen/asmLike/receiverMangling/deepInline.txt +++ b/compiler/testData/codegen/asmLike/receiverMangling/deepInline.txt @@ -5,7 +5,7 @@ public final class DeepInlineKt : java/lang/Object { 1 $i$f$block: I } - public final static void foo(java.lang.String $this$block, int $i$a$-block-DeepInlineKt$foo$1$1$1) { + public final static void foo(java.lang.String $this$foo, int count) { Local variables: 14 $this$block: J 16 $i$a$-block-DeepInlineKt$foo$1$1$1: I @@ -22,4 +22,4 @@ public final class DeepInlineKt : java/lang/Object { 0 $this$foo: Ljava/lang/String; 1 count: I } -} +} \ No newline at end of file diff --git a/compiler/testData/codegen/asmLike/receiverMangling/deepInlineWithLabels.txt b/compiler/testData/codegen/asmLike/receiverMangling/deepInlineWithLabels.txt index 9c702fdce02..d91e68f005b 100644 --- a/compiler/testData/codegen/asmLike/receiverMangling/deepInlineWithLabels.txt +++ b/compiler/testData/codegen/asmLike/receiverMangling/deepInlineWithLabels.txt @@ -5,7 +5,7 @@ public final class DeepInlineWithLabelsKt : java/lang/Object { 1 $i$f$block: I } - public final static void foo(java.lang.String $this$b3, int $i$a$-block-DeepInlineWithLabelsKt$foo$1$1$1) { + public final static void foo(java.lang.String $this$foo, int count) { Local variables: 14 $this$b3: J 16 $i$a$-block-DeepInlineWithLabelsKt$foo$1$1$1: I @@ -22,4 +22,4 @@ public final class DeepInlineWithLabelsKt : java/lang/Object { 0 $this$foo: Ljava/lang/String; 1 count: I } -} +} \ No newline at end of file diff --git a/compiler/testData/codegen/asmLike/receiverMangling/deepNoinlineWithLabels_after.txt b/compiler/testData/codegen/asmLike/receiverMangling/deepNoinlineWithLabels_after.txt index 8f3b6d25143..f2d7775b99f 100644 --- a/compiler/testData/codegen/asmLike/receiverMangling/deepNoinlineWithLabels_after.txt +++ b/compiler/testData/codegen/asmLike/receiverMangling/deepNoinlineWithLabels_after.txt @@ -27,7 +27,7 @@ final class DeepNoinlineWithLabels_afterKt$foo$1$1 : kotlin/jvm/internal/Lambda, public java.lang.Object invoke(java.lang.Object p0) - public final void invoke(long this) { + public final void invoke(long $this$b2) { Local variables: 3 z: Z 0 this: LDeepNoinlineWithLabels_afterKt$foo$1$1; @@ -46,7 +46,7 @@ final class DeepNoinlineWithLabels_afterKt$foo$1 : kotlin/jvm/internal/Lambda, k public java.lang.Object invoke(java.lang.Object p0) - public final void invoke(long this) { + public final void invoke(long $this$b1) { Local variables: 3 y: Z 0 this: LDeepNoinlineWithLabels_afterKt$foo$1; @@ -60,10 +60,10 @@ public final class DeepNoinlineWithLabels_afterKt : java/lang/Object { 0 block: Lkotlin/jvm/functions/Function1; } - public final static void foo(java.lang.String x, int $this$foo) { + public final static void foo(java.lang.String $this$foo, int count) { Local variables: 2 x: Z 0 $this$foo: Ljava/lang/String; 1 count: I } -} +} \ No newline at end of file diff --git a/compiler/testData/codegen/asmLike/receiverMangling/deepNoinlineWithLabels_before.txt b/compiler/testData/codegen/asmLike/receiverMangling/deepNoinlineWithLabels_before.txt index 806f8843357..1b84d6f6a47 100644 --- a/compiler/testData/codegen/asmLike/receiverMangling/deepNoinlineWithLabels_before.txt +++ b/compiler/testData/codegen/asmLike/receiverMangling/deepNoinlineWithLabels_before.txt @@ -27,7 +27,7 @@ final class DeepNoinlineWithLabels_beforeKt$foo$1$1 : kotlin/jvm/internal/Lambda public java.lang.Object invoke(java.lang.Object p0) - public final void invoke(long this) { + public final void invoke(long $receiver) { Local variables: 3 z: Z 0 this: LDeepNoinlineWithLabels_beforeKt$foo$1$1; @@ -46,7 +46,7 @@ final class DeepNoinlineWithLabels_beforeKt$foo$1 : kotlin/jvm/internal/Lambda, public java.lang.Object invoke(java.lang.Object p0) - public final void invoke(long this) { + public final void invoke(long $receiver) { Local variables: 3 y: Z 0 this: LDeepNoinlineWithLabels_beforeKt$foo$1; @@ -60,10 +60,10 @@ public final class DeepNoinlineWithLabels_beforeKt : java/lang/Object { 0 block: Lkotlin/jvm/functions/Function1; } - public final static void foo(java.lang.String x, int $receiver) { + public final static void foo(java.lang.String $receiver, int count) { Local variables: 2 x: Z 0 $receiver: Ljava/lang/String; 1 count: I } -} +} \ No newline at end of file diff --git a/compiler/testData/codegen/asmLike/receiverMangling/deepNoinline_after.txt b/compiler/testData/codegen/asmLike/receiverMangling/deepNoinline_after.txt index 8d88ab52263..160c7cab077 100644 --- a/compiler/testData/codegen/asmLike/receiverMangling/deepNoinline_after.txt +++ b/compiler/testData/codegen/asmLike/receiverMangling/deepNoinline_after.txt @@ -23,7 +23,7 @@ final class DeepNoinline_afterKt$foo$1$1 : kotlin/jvm/internal/Lambda, kotlin/jv public java.lang.Object invoke(java.lang.Object p0) - public final void invoke(long this) { + public final void invoke(long $this$block) { Local variables: 3 z: Z 0 this: LDeepNoinline_afterKt$foo$1$1; @@ -42,7 +42,7 @@ final class DeepNoinline_afterKt$foo$1 : kotlin/jvm/internal/Lambda, kotlin/jvm/ public java.lang.Object invoke(java.lang.Object p0) - public final void invoke(long this) { + public final void invoke(long $this$block) { Local variables: 3 y: Z 0 this: LDeepNoinline_afterKt$foo$1; @@ -56,10 +56,10 @@ public final class DeepNoinline_afterKt : java/lang/Object { 0 block: Lkotlin/jvm/functions/Function1; } - public final static void foo(java.lang.String x, int $this$foo) { + public final static void foo(java.lang.String $this$foo, int count) { Local variables: 2 x: Z 0 $this$foo: Ljava/lang/String; 1 count: I } -} +} \ No newline at end of file diff --git a/compiler/testData/codegen/asmLike/receiverMangling/deepNoinline_before.txt b/compiler/testData/codegen/asmLike/receiverMangling/deepNoinline_before.txt index e0f4648686a..d7373df0eab 100644 --- a/compiler/testData/codegen/asmLike/receiverMangling/deepNoinline_before.txt +++ b/compiler/testData/codegen/asmLike/receiverMangling/deepNoinline_before.txt @@ -23,7 +23,7 @@ final class DeepNoinline_beforeKt$foo$1$1 : kotlin/jvm/internal/Lambda, kotlin/j public java.lang.Object invoke(java.lang.Object p0) - public final void invoke(long this) { + public final void invoke(long $receiver) { Local variables: 3 z: Z 0 this: LDeepNoinline_beforeKt$foo$1$1; @@ -42,7 +42,7 @@ final class DeepNoinline_beforeKt$foo$1 : kotlin/jvm/internal/Lambda, kotlin/jvm public java.lang.Object invoke(java.lang.Object p0) - public final void invoke(long this) { + public final void invoke(long $receiver) { Local variables: 3 y: Z 0 this: LDeepNoinline_beforeKt$foo$1; @@ -56,10 +56,10 @@ public final class DeepNoinline_beforeKt : java/lang/Object { 0 block: Lkotlin/jvm/functions/Function1; } - public final static void foo(java.lang.String x, int $receiver) { + public final static void foo(java.lang.String $receiver, int count) { Local variables: 2 x: Z 0 $receiver: Ljava/lang/String; 1 count: I } -} +} \ No newline at end of file diff --git a/compiler/testData/codegen/asmLike/receiverMangling/inlineReceivers.txt b/compiler/testData/codegen/asmLike/receiverMangling/inlineReceivers.txt index 83bce2d60d9..f441d22c97c 100644 --- a/compiler/testData/codegen/asmLike/receiverMangling/inlineReceivers.txt +++ b/compiler/testData/codegen/asmLike/receiverMangling/inlineReceivers.txt @@ -5,7 +5,7 @@ public final class InlineReceiversKt : java/lang/Object { 1 $i$f$block: I } - public final static void foo(java.lang.String $this$block, int $i$a$-block-InlineReceiversKt$foo$1) { + public final static void foo(java.lang.String $this$foo, int count) { Local variables: 4 $this$block: J 6 $i$a$-block-InlineReceiversKt$foo$1: I @@ -14,4 +14,4 @@ public final class InlineReceiversKt : java/lang/Object { 0 $this$foo: Ljava/lang/String; 1 count: I } -} +} \ No newline at end of file diff --git a/compiler/testData/codegen/asmLike/receiverMangling/mangledNames.txt b/compiler/testData/codegen/asmLike/receiverMangling/mangledNames.txt index 3f6c0338d36..b3bb2ea72e1 100644 --- a/compiler/testData/codegen/asmLike/receiverMangling/mangledNames.txt +++ b/compiler/testData/codegen/asmLike/receiverMangling/mangledNames.txt @@ -103,13 +103,13 @@ final class MangledNamesKt$foo$1 : kotlin/jvm/internal/Lambda, kotlin/jvm/functi public final void invoke(Arr $dstr$a_u20b$b_u24c$c_u2dd$b_u24_u24c_u2d_u2dd$a_u28_u29§_u26_u2a_u26_u5e_u40あ化) { Local variables: - 0 this: LMangledNamesKt$foo$1; - 1 $dstr$a_u20b$b_u24c$c_u2dd$b_u24_u24c_u2d_u2dd$a_u28_u29§_u26_u2a_u26_u5e_u40あ化: LArr; 2 a b: I 3 b$c: I 4 c-d: I 5 b$$c--d: I 6 a()§&*&^@あ化: I + 0 this: LMangledNamesKt$foo$1; + 1 $dstr$a_u20b$b_u24c$c_u2dd$b_u24_u24c_u2d_u2dd$a_u28_u29§_u26_u2a_u26_u5e_u40あ化: LArr; } } diff --git a/compiler/testData/codegen/asmLike/receiverMangling/nonInlineReceivers_after.txt b/compiler/testData/codegen/asmLike/receiverMangling/nonInlineReceivers_after.txt index 735a1aba9f3..0fa1355bedc 100644 --- a/compiler/testData/codegen/asmLike/receiverMangling/nonInlineReceivers_after.txt +++ b/compiler/testData/codegen/asmLike/receiverMangling/nonInlineReceivers_after.txt @@ -22,10 +22,10 @@ public final class NonInlineReceivers_afterKt : java/lang/Object { 0 block: Lkotlin/jvm/functions/Function1; } - public final static void foo(java.lang.String x, int $this$foo) { + public final static void foo(java.lang.String $this$foo, int count) { Local variables: 2 x: Z 0 $this$foo: Ljava/lang/String; 1 count: I } -} +} \ No newline at end of file diff --git a/compiler/testData/codegen/asmLike/receiverMangling/nonInlineReceivers_before.txt b/compiler/testData/codegen/asmLike/receiverMangling/nonInlineReceivers_before.txt index 3e7e4057ac5..2b97e331484 100644 --- a/compiler/testData/codegen/asmLike/receiverMangling/nonInlineReceivers_before.txt +++ b/compiler/testData/codegen/asmLike/receiverMangling/nonInlineReceivers_before.txt @@ -22,10 +22,10 @@ public final class NonInlineReceivers_beforeKt : java/lang/Object { 0 block: Lkotlin/jvm/functions/Function1; } - public final static void foo(java.lang.String x, int $receiver) { + public final static void foo(java.lang.String $receiver, int count) { Local variables: 2 x: Z 0 $receiver: Ljava/lang/String; 1 count: I } -} +} \ No newline at end of file diff --git a/compiler/tests-common/tests/org/jetbrains/kotlin/codegen/AbstractAsmLikeInstructionListingTest.kt b/compiler/tests-common/tests/org/jetbrains/kotlin/codegen/AbstractAsmLikeInstructionListingTest.kt index 7fa90af3cd3..9ed1984bb34 100644 --- a/compiler/tests-common/tests/org/jetbrains/kotlin/codegen/AbstractAsmLikeInstructionListingTest.kt +++ b/compiler/tests-common/tests/org/jetbrains/kotlin/codegen/AbstractAsmLikeInstructionListingTest.kt @@ -132,8 +132,10 @@ abstract class AbstractAsmLikeInstructionListingTest : CodegenTestCase() { } val actualIndex = index + localVariableIndexOffset - val localVariables = method.localVariables?.takeIf { it.size > actualIndex } ?: return "p$index" - return localVariables[actualIndex].name + val localVariables = method.localVariables + return localVariables?.firstOrNull { + it.index == actualIndex + }?.name ?: "p$index" } private fun buildLocalVariableTable(method: MethodNode): String { diff --git a/compiler/tests-common/tests/org/jetbrains/kotlin/codegen/AbstractCheckLocalVariablesTableTest.kt b/compiler/tests-common/tests/org/jetbrains/kotlin/codegen/AbstractCheckLocalVariablesTableTest.kt index c911759fb18..fedfc4d6140 100644 --- a/compiler/tests-common/tests/org/jetbrains/kotlin/codegen/AbstractCheckLocalVariablesTableTest.kt +++ b/compiler/tests-common/tests/org/jetbrains/kotlin/codegen/AbstractCheckLocalVariablesTableTest.kt @@ -45,7 +45,9 @@ abstract class AbstractCheckLocalVariablesTableTest : CodegenTestCase() { val pathsString = outputFiles.joinToString { it.relativePath } assertNotNull("Couldn't find class file for pattern $classFileRegex in: $pathsString", outputFile) - val actualLocalVariables = readLocalVariable(ClassReader(outputFile.asByteArray()), methodName) + val classReader = ClassReader(outputFile.asByteArray()) + val actualLocalVariables = readLocalVariable(classReader, methodName) + checkLocalVariableTypes(classReader, methodName, actualLocalVariables) doCompare(wholeFile, files.single().content, actualLocalVariables) } catch (e: Throwable) { @@ -68,9 +70,11 @@ abstract class AbstractCheckLocalVariablesTableTest : CodegenTestCase() { } protected class LocalVariable internal constructor( - private val name: String, - private val type: String, - private val index: Int + val name: String, + val type: String, + val index: Int, + val startLabelNumber: Int, + val endLabelNumber: Int ) { override fun toString(): String { @@ -107,10 +111,20 @@ abstract class AbstractCheckLocalVariablesTableTest : CodegenTestCase() { return if (methodName == name + desc) { methodFound = true object : MethodVisitor(Opcodes.API_VERSION) { + // ASM labels cannot be easily compared across two visits of the same method. + // Therefore, we keep our own numbering that is consistent across multiple + // visits. + private var currentLabelNumber = 0 + private val labelToNumber: MutableMap = mutableMapOf() + override fun visitLocalVariable( name: String, desc: String, signature: String?, start: Label, end: Label, index: Int ) { - readVariables.add(LocalVariable(name, desc, index)) + readVariables.add(LocalVariable(name, desc, index, labelToNumber[start]!!, labelToNumber[end]!!)) + } + + override fun visitLabel(label: Label?) { + labelToNumber[label!!] = currentLabelNumber++ } } } else { @@ -120,13 +134,289 @@ abstract class AbstractCheckLocalVariablesTableTest : CodegenTestCase() { } val visitor = Visitor() - cr.accept(visitor, ClassReader.SKIP_FRAMES) - TestCase.assertTrue("method not found: $methodName", visitor.methodFound) - return visitor.readVariables } + + private fun checkLocalVariableTypes(cr: ClassReader, methodName: String, locals: List) { + + // Representation of local load and store instruction. + open class Instruction(val index: Int, val type: String) + class Load(index: Int, type: String): Instruction(index, type) + class Store(index: Int, type: String): Instruction(index, type) + + // Representation of basic blocks with the locals table at this block + // as well as the actual locals at entry to the block computed + // based on the recorded load and store instructions in the code. + class BasicBlock( + val successors: MutableSet = mutableSetOf(), + var endsWithUnconditionalJump: Boolean = false, + var localsTable: MutableMap = mutableMapOf(), + var localsAtEntry: MutableMap = mutableMapOf(), + val localsInstructions: MutableList = mutableListOf() + ) { + fun addInstruction(index: Int, opcode: Int) { + localsInstructions.add( + when (opcode) { + Opcodes.ILOAD -> Load(index, "I") + Opcodes.LLOAD -> Load(index, "J") + Opcodes.FLOAD -> Load(index, "F") + Opcodes.DLOAD -> Load(index, "D") + Opcodes.ALOAD -> Load(index, "Ljava/lang/Object;") + Opcodes.ISTORE -> Store(index, "I") + Opcodes.LSTORE -> Store(index, "J") + Opcodes.FSTORE -> Store(index, "F") + Opcodes.DSTORE -> Store(index, "D") + Opcodes.ASTORE -> Store(index, "Ljava/lang/Object;") + else -> throw Exception("Unsupported var instruction: $opcode") + } + ) + } + } + + // Visitor that builds a control-flow graph of a method. The control-flow graph + // consists of [BasicBlock]s containing the local load and store instructions + // and the locals table at each basic block. When the visitor ends, the type of + // local slots are computed based on instructions in the code and are checked + // for consistency with information in the locals table. + class Visitor : ClassVisitor(Opcodes.API_VERSION) { + var methodFound = false + var skipValidation = false + + var entryBlock = BasicBlock() + var allBlocks: MutableSet = mutableSetOf(entryBlock) + var currentBlock = entryBlock + var labelToBlock: MutableMap = mutableMapOf() + val currentLocalsTable: MutableMap = mutableMapOf() + + private fun ensureBlock(label: Label): BasicBlock { + return if (labelToBlock.containsKey(label)) { + labelToBlock[label]!! + } else { + val result = BasicBlock() + allBlocks.add(result) + labelToBlock[label] = result + result + } + } + + private fun recordLocalTypesForParameters(access: Int, desc: String) { + val localsAtEntry = entryBlock.localsAtEntry + var parameterIndex = 0 + if (access and Opcodes.ACC_STATIC == 0) { + localsAtEntry[parameterIndex++] = "Ljava/lang/Object;" + } + Type.getMethodType(desc).argumentTypes.forEach { + localsAtEntry[parameterIndex] = when (it) { + // Bytecode instructions use iload/istore for all of these so we do not distinguish. + Type.BOOLEAN_TYPE, + Type.CHAR_TYPE, + Type.BYTE_TYPE, + Type.SHORT_TYPE, + Type.INT_TYPE -> "I" + Type.FLOAT_TYPE-> "F" + Type.DOUBLE_TYPE -> "D" + Type.LONG_TYPE -> "J"; + else -> "Ljava/lang/Object;" + } + parameterIndex += it.size; + } + } + + override fun visitMethod( + access: Int, name: String, desc: String, signature: String?, exceptions: Array? + ): MethodVisitor? { + return if (methodName == name + desc) { + methodFound = true + + recordLocalTypesForParameters(access, desc) + + object : MethodVisitor(Opcodes.API_VERSION) { + // ASM labels cannot be easily compared across to visits of the same method. + // Therefore, we keep our own numbering that is consistent across multiple + // visits. + private var currentLabelNumber = 0 + + // Record local load and store instructions for each basic block. + override fun visitVarInsn(opcode: Int, index: Int) { + currentBlock.addInstruction(index, opcode) + } + + // Build control-flow graph based on jump instructions. + override fun visitJumpInsn(opcode: Int, label: Label?) { + when (opcode) { + Opcodes.IFEQ, + Opcodes.IFNE, + Opcodes.IFLT, + Opcodes.IFGE, + Opcodes.IFGT, + Opcodes.IFLE, + Opcodes.IF_ICMPEQ, + Opcodes.IF_ICMPNE, + Opcodes.IF_ICMPLT, + Opcodes.IF_ICMPGE, + Opcodes.IF_ICMPGT, + Opcodes.IF_ICMPLE, + Opcodes.IF_ACMPEQ, + Opcodes.IF_ACMPNE, + Opcodes.IFNULL, + Opcodes.IFNONNULL -> { + val target = ensureBlock(label!!) + val fallthrough = BasicBlock() + fallthrough.localsTable = currentLocalsTable.toMutableMap() + allBlocks.add(fallthrough) + currentBlock.successors.add(target) + currentBlock.successors.add(fallthrough) + currentBlock = fallthrough + } + + Opcodes.GOTO -> { + currentBlock.endsWithUnconditionalJump = true + currentBlock.successors.add(ensureBlock(label!!)) + } + + else -> throw Exception("Unsupported jump instruction $opcode") + } + } + + // Skip validation for control-flow we do not yet support. + // TODO: Implement these to extend coverage. + override fun visitTryCatchBlock(start: Label?, end: Label?, handler: Label?, type: String?) { + skipValidation = true + } + + override fun visitLookupSwitchInsn(dflt: Label?, keys: IntArray?, labels: Array?) { + skipValidation = true + } + + override fun visitTableSwitchInsn(min: Int, max: Int, dflt: Label?, vararg labels: Label?) { + skipValidation = true + } + + // Start a new basic block at all labels and compute the active locals table. + override fun visitLabel(label: Label?) { + val newBlock = ensureBlock(label!!) + if (!currentBlock.endsWithUnconditionalJump) { + currentBlock.successors.add(newBlock) + } + currentBlock = newBlock + // Compute locals table for new block. End expiring locals first and then + // introduce the ones that start at this label. + locals.forEach { + if (it.endLabelNumber == currentLabelNumber) { + currentLocalsTable.remove(it.index) + } + } + locals.forEach { + if (it.startLabelNumber == currentLabelNumber) { + if (currentLocalsTable.containsKey(it.index)) { + throw Exception("Locals table already contains info for slot ${it.index} at label: $label") + } + // We treat all integer types interchangably as the java bytecode uses + // iload and istore for all of them so we cannot distinguish in our + // analysis of the code. + currentLocalsTable[it.index] = when (it.type) { + "S", "Z", "C", "B" -> "I" + else -> it.type + } + } + } + currentBlock.localsTable = currentLocalsTable.toMutableMap() + currentLabelNumber++ + } + } + } else { + super.visitMethod(access, name, desc, signature, exceptions) + } + } + + override fun visitEnd() { + if (skipValidation) return + propagateAndCheckTypes() + } + + // Compute the types of locals based on the code and check that the locals table + // is consistent with the types computed for the code. + // + // Currently this only checks that basic types are consistent and that basic + // types and object types are not mixed. It does not check that two object types + // are actually related. We consider all object types (including arrays) as equal + // for this analysis. + private fun propagateAndCheckTypes() { + // We need to go through all blocks at least once to make sure + // we propagate everything. After that, it is a fixed-point + // algorithm using a worklist: when the entry state of a block + // changes we enqueue that block for reprocessing. + val worklist: MutableList = LinkedList(allBlocks) + while (worklist.size > 0) { + val currentBlock = worklist.removeAt(0) + val currentLocals = currentBlock.localsAtEntry.toMutableMap() + // Check consistency with the local table. + for ((index, type) in currentBlock.localsTable) { + currentLocals[index]?.let { + checkCompatible(index, type, it) + } ?: throw Exception("Uninitialized local in locals table: index: $index type: $type") + } + // Check that loads make sense and change the actual locals types based + // on store instructions to compute the locals types at exit from this + // block. + for (inst in currentBlock.localsInstructions) { + when (inst) { + is Load -> { + currentLocals[inst.index]?.let { + checkCompatible(inst.index, it, inst.type) + } ?: throw Exception("Uninitialized local read: index: ${inst.index} type: ${inst.type}") + } + + is Store -> currentLocals[inst.index] = inst.type + } + } + // Propagate the type information to successor blocks and enqueue successor + // blocks for reprocessing if the type of the locals at entry changed. + for (succ in currentBlock.successors) { + if (!succ.localsAtEntry.equals(currentLocals)) { + for ((index, type) in currentLocals) { + succ.localsAtEntry[index]?.let { + // If conflicting types flow to the same block for a local + // slot that is OK as long as the type is never used. Such + // conflicting types are represented with a CONFLICT type + // which will never match a read instruction or a type in + // a locals table. + if (!areCompatible(index, it, type)) { + currentLocals[index] = "CONFLICT" + } + } + } + succ.localsAtEntry = currentLocals.toMutableMap() + worklist.add(succ) + } + } + } + + } + + // We only check that there is no confusion between object types and basic types. + // Therefore, we map all arrays types to type Object when comparing. + private fun checkCompatible(index: Int, type0: String, type1: String) { + if (areCompatible(index, type0, type1)) return + throw Exception("Incompatible types for local $index: $type0 and $type1") + } + + private fun areCompatible(index: Int, type0: String, type1: String): Boolean { + val t0 = if (type0.startsWith("[")) "Ljava/lang/Object;" else type0 + val t1 = if (type1.startsWith("[")) "Ljava/lang/Object;" else type1 + if (t0.equals(t1)) return true + // If both are object descriptors we are fine, otherwise we have a mix + // of an object type and a basic type. + return t0.endsWith(";") == t1.endsWith(";") + } + } + + val visitor = Visitor() + cr.accept(visitor, ClassReader.SKIP_FRAMES) + TestCase.assertTrue("method not found: $methodName", visitor.methodFound) + } } } diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/CheckLocalVariablesTableTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/CheckLocalVariablesTableTestGenerated.java index dde71e5c041..4414051fefb 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/CheckLocalVariablesTableTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/CheckLocalVariablesTableTestGenerated.java @@ -39,6 +39,11 @@ public class CheckLocalVariablesTableTestGenerated extends AbstractCheckLocalVar runTest("compiler/testData/checkLocalVariablesTable/copyFunction.kt"); } + @TestMetadata("destructuringInFor.kt") + public void testDestructuringInFor() throws Exception { + runTest("compiler/testData/checkLocalVariablesTable/destructuringInFor.kt"); + } + @TestMetadata("destructuringInLambdas.kt") public void testDestructuringInLambdas() throws Exception { runTest("compiler/testData/checkLocalVariablesTable/destructuringInLambdas.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrCheckLocalVariablesTableTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrCheckLocalVariablesTableTestGenerated.java index af312f6bc2f..e5e191103de 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrCheckLocalVariablesTableTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrCheckLocalVariablesTableTestGenerated.java @@ -39,6 +39,11 @@ public class IrCheckLocalVariablesTableTestGenerated extends AbstractIrCheckLoca runTest("compiler/testData/checkLocalVariablesTable/copyFunction.kt"); } + @TestMetadata("destructuringInFor.kt") + public void testDestructuringInFor() throws Exception { + runTest("compiler/testData/checkLocalVariablesTable/destructuringInFor.kt"); + } + @TestMetadata("destructuringInLambdas.kt") public void testDestructuringInLambdas() throws Exception { runTest("compiler/testData/checkLocalVariablesTable/destructuringInLambdas.kt");