From 1e444e0451974acb1db335c2ab97b8cd344c408d Mon Sep 17 00:00:00 2001 From: Mads Ager Date: Tue, 5 Apr 2022 14:17:55 +0200 Subject: [PATCH] [JVM IR] Limit local variables in do-while conditions. Locals introduced in the body of a do-while loop are not necessarily live at the do-while condition. For example, if there is a continue in the body before the declaration: do { if (shouldContinue(x)) continue val y = 32 // not always defined in the condition doSomething(y) } while (x < 2) For locals referenced in the condition such code is rejected by the frontend because a local referenced in the condition must be always defined when you get there. However, locals that are not used in the condition were always put in the local variable table. This leads to invalid locals information which can trip of debuggers and other build tools such as the D8 dexer. This patch only puts in locals information for locals actually referenced in the local variable table for the condition. ^Fixes KT-51754 --- .../FirLocalVariableTestGenerated.java | 6 +++ .../backend/jvm/codegen/ExpressionCodegen.kt | 48 ++++++++++++++++++- .../underscoreNames.kt | 2 +- .../testData/debug/localVariables/doWhile.kt | 33 +++++++++++++ .../codegen/IrLocalVariableTestGenerated.java | 6 +++ .../codegen/LocalVariableTestGenerated.java | 6 +++ 6 files changed, 98 insertions(+), 3 deletions(-) create mode 100644 compiler/testData/debug/localVariables/doWhile.kt diff --git a/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirLocalVariableTestGenerated.java b/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirLocalVariableTestGenerated.java index b7df7a286e1..d9c53e704ce 100644 --- a/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirLocalVariableTestGenerated.java +++ b/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirLocalVariableTestGenerated.java @@ -49,6 +49,12 @@ public class FirLocalVariableTestGenerated extends AbstractFirLocalVariableTest runTest("compiler/testData/debug/localVariables/destructuringInLambdas.kt"); } + @Test + @TestMetadata("doWhile.kt") + public void testDoWhile() throws Exception { + runTest("compiler/testData/debug/localVariables/doWhile.kt"); + } + @Test @TestMetadata("emptyFun.kt") public void testEmptyFun() throws Exception { diff --git a/compiler/ir/backend.jvm/codegen/src/org/jetbrains/kotlin/backend/jvm/codegen/ExpressionCodegen.kt b/compiler/ir/backend.jvm/codegen/src/org/jetbrains/kotlin/backend/jvm/codegen/ExpressionCodegen.kt index b49cd7f6721..64f2c1f0e18 100644 --- a/compiler/ir/backend.jvm/codegen/src/org/jetbrains/kotlin/backend/jvm/codegen/ExpressionCodegen.kt +++ b/compiler/ir/backend.jvm/codegen/src/org/jetbrains/kotlin/backend/jvm/codegen/ExpressionCodegen.kt @@ -40,12 +40,17 @@ import org.jetbrains.kotlin.ir.expressions.impl.IrCompositeImpl import org.jetbrains.kotlin.ir.symbols.IrSymbol import org.jetbrains.kotlin.ir.symbols.IrTypeParameterSymbol import org.jetbrains.kotlin.ir.symbols.IrValueParameterSymbol +import org.jetbrains.kotlin.ir.symbols.IrValueSymbol import org.jetbrains.kotlin.ir.types.* import org.jetbrains.kotlin.ir.util.* import org.jetbrains.kotlin.ir.visitors.IrElementVisitor +import org.jetbrains.kotlin.ir.visitors.IrElementVisitorVoid +import org.jetbrains.kotlin.ir.visitors.acceptChildrenVoid +import org.jetbrains.kotlin.ir.visitors.acceptVoid import org.jetbrains.kotlin.name.FqName import org.jetbrains.kotlin.resolve.jvm.AsmTypes -import org.jetbrains.kotlin.resolve.jvm.AsmTypes.* +import org.jetbrains.kotlin.resolve.jvm.AsmTypes.JAVA_STRING_TYPE +import org.jetbrains.kotlin.resolve.jvm.AsmTypes.OBJECT_TYPE import org.jetbrains.kotlin.resolve.jvm.jvmSignature.JvmMethodParameterKind import org.jetbrains.kotlin.resolve.jvm.jvmSignature.JvmMethodSignature import org.jetbrains.kotlin.types.TypeSystemCommonBackendContext @@ -125,6 +130,7 @@ class Gap(val start: Label, val end: Label) class VariableInfo(val declaration: IrVariable, val index: Int, val type: Type, val startLabel: Label) { val gaps = mutableListOf() + var explicitEndLabel: Label? = null } class ExpressionCodegen( @@ -424,7 +430,8 @@ class ExpressionCodegen( mv.visitLocalVariable(it.declaration.name.asString(), it.type.descriptor, null, start, gap.start, it.index) start = gap.end } - mv.visitLocalVariable(it.declaration.name.asString(), it.type.descriptor, null, start, endLabel, it.index) + val end = it.explicitEndLabel ?: endLabel + mv.visitLocalVariable(it.declaration.name.asString(), it.type.descriptor, null, start, end, it.index) } } @@ -1102,17 +1109,54 @@ class ExpressionCodegen( // We have a regular 'do-while' loop. Proceed as usual. mv.fakeAlwaysFalseIfeq(continueLabel) mv.fakeAlwaysFalseIfeq(endLabel) + data.withBlock(loopInfo) { loop.body?.accept(this, data)?.discard() mv.visitLabel(continueLabel) loop.condition.markLineNumber(true) loop.condition.accept(this, data).coerceToBoolean().jumpIfTrue(entry) + endUnreferencedDoWhileLocals(data, loop, continueLabel) } mv.mark(endLabel) addInlineMarker(mv, false) return unitValue } + // Locals introduced in the body of a do-while loop are not necessarily declared when the condition is + // reached. For example, there could be a continue from the body before the local is declared: + // + // do { + // if (shouldContinue(x)) { + // continue + // } + // var y = 32 // this variable is not necessarily declared on the do-while condition + // doSomething(y) + // } while (x < 2) + // + // This is all fine for variables used in the condition. If a variable that is not definitely + // assigned is used in the condition, the frontend rightly rejects the code. However, for variables + // that are *not* referenced in the condition, we have to be conservative and make sure that + // they do not end up in the local variable table. Otherwise, the debugger and other build tools + // such as D8 will see locals information that makes no sense. + private fun endUnreferencedDoWhileLocals(blockInfo: BlockInfo, loop: IrDoWhileLoop, continueLabel: Label) { + val referencedValues = hashSetOf() + loop.condition.acceptVoid(object : IrElementVisitorVoid { + override fun visitElement(element: IrElement) { + element.acceptChildrenVoid(this) + } + + override fun visitGetValue(expression: IrGetValue) { + referencedValues.add(expression.symbol) + super.visitGetValue(expression) + } + }) + blockInfo.variables.forEach { + if (it.declaration.symbol !in referencedValues) { + it.explicitEndLabel = continueLabel + } + } + } + private fun unwindBlockStack( endLabel: Label, data: BlockInfo, diff --git a/compiler/testData/checkLocalVariablesTable/underscoreNames.kt b/compiler/testData/checkLocalVariablesTable/underscoreNames.kt index 8e657844915..3ee2514b9f1 100644 --- a/compiler/testData/checkLocalVariablesTable/underscoreNames.kt +++ b/compiler/testData/checkLocalVariablesTable/underscoreNames.kt @@ -13,7 +13,7 @@ fun box() { val (_, `_`, d) = A() for ((_, q) in arrayOfA) { - + Unit } "" diff --git a/compiler/testData/debug/localVariables/doWhile.kt b/compiler/testData/debug/localVariables/doWhile.kt new file mode 100644 index 00000000000..c08cee1b36d --- /dev/null +++ b/compiler/testData/debug/localVariables/doWhile.kt @@ -0,0 +1,33 @@ +// IGNORE_BACKEND: JVM +// FILE: test.kt +fun shouldContinue(i: Int) = i < 1 + +fun box() { + var x = 0 + do { + var z = 2 + if (shouldContinue(x++)) { + continue + } + // Introduce a variable `y` which is not defined on all control-flow + // paths to the while condition. Therefore, it should not be in the + // local variable table at the condition. + var y = 12 + } while (x < z) +} + +// EXPECTATIONS +// test.kt:6 box: +// test.kt:8 box: x:int=0:int +// test.kt:9 box: x:int=0:int, z:int=2:int +// test.kt:3 shouldContinue: i:int=0:int +// test.kt:9 box: x:int=1:int, z:int=2:int +// test.kt:10 box: x:int=1:int, z:int=2:int +// test.kt:16 box: x:int=1:int, z:int=2:int +// test.kt:8 box: x:int=1:int +// test.kt:9 box: x:int=1:int, z:int=2:int +// test.kt:3 shouldContinue: i:int=1:int +// test.kt:9 box: x:int=2:int, z:int=2:int +// test.kt:15 box: x:int=2:int, z:int=2:int +// test.kt:16 box: x:int=2:int, z:int=2:int +// test.kt:17 box: x:int=2:int \ No newline at end of file diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrLocalVariableTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrLocalVariableTestGenerated.java index ed2f43c53bf..524f6c5899d 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrLocalVariableTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrLocalVariableTestGenerated.java @@ -49,6 +49,12 @@ public class IrLocalVariableTestGenerated extends AbstractIrLocalVariableTest { runTest("compiler/testData/debug/localVariables/destructuringInLambdas.kt"); } + @Test + @TestMetadata("doWhile.kt") + public void testDoWhile() throws Exception { + runTest("compiler/testData/debug/localVariables/doWhile.kt"); + } + @Test @TestMetadata("emptyFun.kt") public void testEmptyFun() throws Exception { diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/LocalVariableTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/LocalVariableTestGenerated.java index b20d439a615..332011a5bea 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/LocalVariableTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/LocalVariableTestGenerated.java @@ -49,6 +49,12 @@ public class LocalVariableTestGenerated extends AbstractLocalVariableTest { runTest("compiler/testData/debug/localVariables/destructuringInLambdas.kt"); } + @Test + @TestMetadata("doWhile.kt") + public void testDoWhile() throws Exception { + runTest("compiler/testData/debug/localVariables/doWhile.kt"); + } + @Test @TestMetadata("emptyFun.kt") public void testEmptyFun() throws Exception {