From 48aef1eb2895c59bd6f77297933ea9e651019cad Mon Sep 17 00:00:00 2001 From: Michael Bogdanov Date: Wed, 15 Apr 2015 15:21:49 +0300 Subject: [PATCH] Fix for KT-6895: Compile error on Android (EXCEPTION FROM SIMULATION) when declaring locals in an inline function #KT-6895 Fixed --- .../inline/CoveringTryCatchNodeProcessor.kt | 37 +++++++++++-------- .../inline/InternalFinallyBlockInliner.java | 28 +++++++++----- .../codegen/inline/TryBlockClustering.kt | 18 +++++---- .../codegen/boxWithStdlib/inline/kt6895.kt | 22 +++++++++++ ...lackBoxWithStdlibCodegenTestGenerated.java | 15 ++++++++ 5 files changed, 89 insertions(+), 31 deletions(-) create mode 100644 compiler/testData/codegen/boxWithStdlib/inline/kt6895.kt diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/CoveringTryCatchNodeProcessor.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/CoveringTryCatchNodeProcessor.kt index 28cf8f30d2f..a5b9cf19ce2 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/CoveringTryCatchNodeProcessor.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/CoveringTryCatchNodeProcessor.kt @@ -19,6 +19,7 @@ package org.jetbrains.kotlin.codegen.inline import com.google.common.collect.LinkedListMultimap import java.util.ArrayList import com.intellij.util.containers.Stack +import org.jetbrains.kotlin.codegen.optimization.common.isMeaningful import org.jetbrains.org.objectweb.asm.Opcodes import org.jetbrains.org.objectweb.asm.tree.* import java.util.Comparator @@ -61,6 +62,7 @@ public abstract class CoveringTryCatchNodeProcessor(parameterSize: Int) { // pushing and popping it to stack entering and exiting tryCatchBlock start and end labels protected open fun updateCoveringTryBlocks(curIns: LabelNode, directOrder: Boolean) { for (startNode in tryBlocksMetaInfo.closeIntervals(curIns, directOrder)) { + assert(!startNode.isEmpty(), {"Try block should be non-empty"}) val pop = tryBlocksMetaInfo.currentIntervals.pop() //Temporary disabled cause during patched structure of exceptions changed // if (startNode != pop) { @@ -71,16 +73,21 @@ public abstract class CoveringTryCatchNodeProcessor(parameterSize: Int) { //Reversing list order cause we should pop external block before internal one // (originally internal blocks goes before external one, such invariant preserved via sortTryCatchBlocks method) for (info in tryBlocksMetaInfo.openIntervals(curIns, directOrder).reverse()) { + assert(!info.isEmpty(), {"Try block should be non-empty"}) tryBlocksMetaInfo.currentIntervals.add(info) } } protected open fun updateCoveringLocalVars(curIns: LabelNode, directOrder: Boolean) { - localVarsMetaInfo.closeIntervals(curIns, directOrder).forEach { + localVarsMetaInfo.closeIntervals(curIns, directOrder).filterNot { + it.isEmpty() + } forEach { localVarsMetaInfo.currentIntervals.pop() } - localVarsMetaInfo.openIntervals(curIns, directOrder).forEach { + localVarsMetaInfo.openIntervals(curIns, directOrder).filterNot { + it.isEmpty() + } forEach { localVarsMetaInfo.currentIntervals.add(it) } } @@ -105,7 +112,7 @@ public abstract class CoveringTryCatchNodeProcessor(parameterSize: Int) { protected fun substituteTryBlockNodes(node: MethodNode) { node.tryCatchBlocks.clear() - for (info in tryBlocksMetaInfo.getNonEmptyNodes()) { + for (info in tryBlocksMetaInfo.getMeaningfulIntervals()) { node.tryCatchBlocks.add(info.node) } } @@ -113,7 +120,7 @@ public abstract class CoveringTryCatchNodeProcessor(parameterSize: Int) { public fun substituteLocalVarTable(node: MethodNode) { node.localVariables.clear() - for (info in localVarsMetaInfo.getNonEmptyNodes()) { + for (info in localVarsMetaInfo.getMeaningfulIntervals()) { node.localVariables.add(info.node) } } @@ -160,22 +167,22 @@ class IntervalMetaInfo> { return splittedPair } - public fun getNonEmptyNodes(): List { - return allIntervals.filterNot { isEmptyInterval(it) } - } - fun closeIntervals(curIns: LabelNode, directOrder: Boolean) = if (!directOrder) intervalStarts.get(curIns) else intervalEnds.get(curIns) fun openIntervals(curIns: LabelNode, directOrder: Boolean) = if (directOrder) intervalStarts.get(curIns) else intervalEnds.get(curIns) +} - private fun isEmptyInterval(node: T): Boolean { - val start = node.startLabel - var end: AbstractInsnNode = node.endLabel - while (end != start && end is LabelNode) { - end = end.getPrevious() - } - return start == end +private fun Interval.isMeaningless(): Boolean { + val start = this.startLabel + var end: AbstractInsnNode = this.endLabel + while (end != start && !end.isMeaningful) { + end = end.getPrevious() } + return start == end +} + +public fun > IntervalMetaInfo.getMeaningfulIntervals(): List { + return allIntervals.filterNot { it.isMeaningless() } } public class DefaultProcessor(val node: MethodNode, parameterSize: Int) : CoveringTryCatchNodeProcessor(parameterSize) { diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/InternalFinallyBlockInliner.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/InternalFinallyBlockInliner.java index 13e145b4038..5423f6bd02a 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/InternalFinallyBlockInliner.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/InternalFinallyBlockInliner.java @@ -136,9 +136,10 @@ public class InternalFinallyBlockInliner extends CoveringTryCatchNodeProcessor { continue; } - AbstractInsnNode instrInsertFinallyBefore = curIns.getPrevious(); + AbstractInsnNode markedReturn = curIns; + final AbstractInsnNode instrInsertFinallyBefore = markedReturn.getPrevious(); AbstractInsnNode nextPrev = instrInsertFinallyBefore.getPrevious(); - Type nonLocalReturnType = InlineCodegenUtil.getReturnType(curIns.getOpcode()); + Type nonLocalReturnType = InlineCodegenUtil.getReturnType(markedReturn.getOpcode()); //Generally there could be several tryCatch blocks (group) on one code interval (same start and end labels, but maybe different handlers) - // all of them refer to one try/*catches*/finally or try/catches. @@ -190,7 +191,7 @@ public class InternalFinallyBlockInliner extends CoveringTryCatchNodeProcessor { !(currentIns instanceof JumpInsnNode) || labelsInsideFinally.contains(((JumpInsnNode) currentIns).label); - copyInstruction(nextTempNonLocalVarIndex, curIns, instrInsertFinallyBefore, nonLocalReturnType, finallyBlockCopy, + copyInstruction(nextTempNonLocalVarIndex, markedReturn, instrInsertFinallyBefore, nonLocalReturnType, finallyBlockCopy, currentIns, isInsOrJumpInsideFinally); currentIns = currentIns.getNext(); @@ -212,15 +213,25 @@ public class InternalFinallyBlockInliner extends CoveringTryCatchNodeProcessor { } //skip just inserted finally - curIns = curIns.getPrevious(); + curIns = markedReturn.getPrevious(); while (curIns != null && curIns != nextPrev) { processInstruction(curIns, false); curIns = curIns.getPrevious(); } + + //finally block inserted so we need split update localVarTable in lambda + if (instrInsertFinallyBefore.getPrevious() != nextPrev && curIns != null) { + LabelNode startNode = new LabelNode(); + LabelNode endNode = new LabelNode(); + instructions.insert(curIns, startNode); + //TODO: note that on return expression we have no variables + instructions.insert(markedReturn, endNode); + getLocalVarsMetaInfo().splitCurrentIntervals(new SimpleInterval(startNode, endNode), true); + } } substituteTryBlockNodes(inlineFun); - //substituteLocalVarTable(inlineFun); + substituteLocalVarTable(inlineFun); } private static void copyInstruction( @@ -378,19 +389,18 @@ public class InternalFinallyBlockInliner extends CoveringTryCatchNodeProcessor { toProcess.addAll(patched); toProcess.addAll(updatingClusterBlocks); patched.clear(); + SimpleInterval splitBy = new SimpleInterval((LabelNode) newFinallyStart.info, (LabelNode) newFinallyEnd.info); // Inserted finally shouldn't be handled by corresponding catches, // so we should split original interval by inserted finally one for (TryCatchBlockNodeInfo block : toProcess) { //update exception mapping - tryBlocksMetaInfo.split(block, new SimpleInterval((LabelNode) newFinallyStart.info, (LabelNode) newFinallyEnd.info), false); + tryBlocksMetaInfo.split(block, splitBy, false); //block patched in split method + assert !block.isEmpty() : "Finally block should be non-empty"; patched.add(block); //TODO add assert } - getLocalVarsMetaInfo().splitCurrentIntervals(new SimpleInterval((LabelNode) newFinallyStart.info, (LabelNode) newFinallyEnd.info), - false); - sortTryCatchBlocks(); } diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/TryBlockClustering.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/TryBlockClustering.kt index a2886071ee8..7975626e471 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/TryBlockClustering.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/TryBlockClustering.kt @@ -33,10 +33,14 @@ class SimpleInterval(override val startLabel: LabelNode, override val endLabel: trait Interval { val startLabel: LabelNode val endLabel: LabelNode + + /*note that some intervals are mutable */ + public fun isEmpty(): Boolean = startLabel == endLabel + } trait SplittableInterval : Interval { - fun split(split: Interval, keepStart: Boolean): SplittedPair + fun split(splitBy: Interval, keepStart: Boolean): SplittedPair } @@ -45,7 +49,7 @@ trait IntervalWithHandler : Interval { val type: String? } -open class TryCatchBlockNodeInfo(val node: TryCatchBlockNode, val onlyCopyNotProcess: Boolean) : IntervalWithHandler, SplittableInterval { +class TryCatchBlockNodeInfo(val node: TryCatchBlockNode, val onlyCopyNotProcess: Boolean) : IntervalWithHandler, SplittableInterval { override val startLabel: LabelNode get() = node.start override val endLabel: LabelNode @@ -55,16 +59,16 @@ open class TryCatchBlockNodeInfo(val node: TryCatchBlockNode, val onlyCopyNotPro override val type: String? get() = node.type - override fun split(by: Interval, keepStart: Boolean): SplittedPair { + override fun split(splitBy: Interval, keepStart: Boolean): SplittedPair { val newPartInterval = if (keepStart) { val oldEnd = endLabel - node.end = by.startLabel - Pair(by.endLabel, oldEnd) + node.end = splitBy.startLabel + Pair(splitBy.endLabel, oldEnd) } else { val oldStart = startLabel - node.start = by.endLabel - Pair(oldStart, by.startLabel) + node.start = splitBy.endLabel + Pair(oldStart, splitBy.startLabel) } return SplittedPair( this, diff --git a/compiler/testData/codegen/boxWithStdlib/inline/kt6895.kt b/compiler/testData/codegen/boxWithStdlib/inline/kt6895.kt new file mode 100644 index 00000000000..659aef9ac38 --- /dev/null +++ b/compiler/testData/codegen/boxWithStdlib/inline/kt6895.kt @@ -0,0 +1,22 @@ +import java.util.concurrent.locks.ReentrantReadWriteLock +import kotlin.concurrent.write + +class UpdateableThing { + private val lock = ReentrantReadWriteLock() + private var updateCount = 0 + + fun performUpdates(block: () -> T): T { + lock.write { + ++updateCount + val result = block() + --updateCount + + return result + } + } +} + + +fun box(): String { + return UpdateableThing().performUpdates { "OK" } +} \ No newline at end of file diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/generated/BlackBoxWithStdlibCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/generated/BlackBoxWithStdlibCodegenTestGenerated.java index a253f7c39f1..d391ea2affb 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/generated/BlackBoxWithStdlibCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/generated/BlackBoxWithStdlibCodegenTestGenerated.java @@ -1718,6 +1718,21 @@ public class BlackBoxWithStdlibCodegenTestGenerated extends AbstractBlackBoxCode } } + @TestMetadata("compiler/testData/codegen/boxWithStdlib/inline") + @TestDataPath("$PROJECT_ROOT") + @RunWith(JUnit3RunnerWithInners.class) + public static class Inline extends AbstractBlackBoxCodegenTest { + public void testAllFilesPresentInInline() throws Exception { + JetTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/boxWithStdlib/inline"), Pattern.compile("^(.+)\\.kt$"), true); + } + + @TestMetadata("kt6895.kt") + public void testKt6895() throws Exception { + String fileName = JetTestUtils.navigationMetadata("compiler/testData/codegen/boxWithStdlib/inline/kt6895.kt"); + doTestWithStdlib(fileName); + } + } + @TestMetadata("compiler/testData/codegen/boxWithStdlib/intrinsics") @TestDataPath("$PROJECT_ROOT") @RunWith(JUnit3RunnerWithInners.class)