Fix for KT-6895: Compile error on Android (EXCEPTION FROM SIMULATION) when declaring locals in an inline function

#KT-6895 Fixed

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<T : SplittableInterval<T>> {
         return splittedPair
     }
 
-    public fun getNonEmptyNodes(): List<T> {
-        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 {
+private fun Interval.isMeaningless(): Boolean {
-        val start = node.startLabel
+    val start = this.startLabel
-        var end: AbstractInsnNode = node.endLabel
+    var end: AbstractInsnNode = this.endLabel
-        while (end != start && end is LabelNode) {
+    while (end != start && !end.isMeaningful) {
-            end = end.getPrevious()
+        end = end.getPrevious()
-        }
-        return start == end
     }
+    return start == end
+}
+
+public fun <T : SplittableInterval<T>> IntervalMetaInfo<T>.getMeaningfulIntervals(): List<T> {
+    return allIntervals.filterNot { it.isMeaningless() }
 }
 
 public class DefaultProcessor(val node: MethodNode, parameterSize: Int) : CoveringTryCatchNodeProcessor(parameterSize) {

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();
     }
 

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<T: Interval> : Interval {
-    fun split(split: Interval, keepStart: Boolean): SplittedPair<T>
+    fun split(splitBy: Interval, keepStart: Boolean): SplittedPair<T>
 }
 
 
@@ -45,7 +49,7 @@ trait IntervalWithHandler : Interval {
     val type: String?
 }
 
-open class TryCatchBlockNodeInfo(val node: TryCatchBlockNode, val onlyCopyNotProcess: Boolean) : IntervalWithHandler, SplittableInterval<TryCatchBlockNodeInfo> {
+class TryCatchBlockNodeInfo(val node: TryCatchBlockNode, val onlyCopyNotProcess: Boolean) : IntervalWithHandler, SplittableInterval<TryCatchBlockNodeInfo> {
     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<TryCatchBlockNodeInfo> {
+    override fun split(splitBy: Interval, keepStart: Boolean): SplittedPair<TryCatchBlockNodeInfo> {
         val newPartInterval = if (keepStart) {
             val oldEnd = endLabel
-            node.end = by.startLabel
+            node.end = splitBy.startLabel
-            Pair(by.endLabel, oldEnd)
+            Pair(splitBy.endLabel, oldEnd)
         }
         else {
             val oldStart = startLabel
-            node.start = by.endLabel
+            node.start = splitBy.endLabel
-            Pair(oldStart, by.startLabel)
+            Pair(oldStart, splitBy.startLabel)
         }
         return SplittedPair(
                 this,

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<T>(block: () -> T): T {
+        lock.write {
+            ++updateCount
+            val result = block()
+            --updateCount
+
+            return result
+        }
+    }
+}
+
+
+fun box(): String {
+    return UpdateableThing().performUpdates { "OK" }
+}

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)