KT-19251 Process uninitialized stores in mandatory bytecode pass

See
https://youtrack.jetbrains.com/issue/KT-19251
puniverse/quasar#280
https://bugs.openjdk.java.net/browse/JDK-8046233

Inline function calls (as well as try/catch expressions) in constructor
arguments produce bytecode that spills stack, and stores uninitialized
objects (created by 'NEW C', but not initialized by 'C.<init>') to
local variables. Such bytecode is valid according to the JVM spec, but
confuses Quasar (and other bytecode postprocessing tools),
and fails to verify under some (buggy) versions of JDK 8.

In order to avoid that, we apply 'processUnitializedStores' already
implemented for coroutines. It moves 'NEW' instructions after the
constructor arguments evaluation, producing code like

<initialize class C using Class.forName>
<evaluate constructor arguments>
<store constructor arguments to variables>
NEW C
DUP
<load constructor arguments from variables>
INVOKESPECIAL C.<init>(...)

NB some other expressions, such as break/continue in the constructor
arguments, also can produce "weird" bytecode: object is created by a
'NEW C' instruction, but later (conditionally) POPped from stack and
left uninitialized. This, as we know, also can screw bytecode
postprocessing. However, it looks like we can get away with it ATM.
Otherwise it looks like we'd have to analyze constructor arguments, see
if the evaluation can "jump out", and perform argument linearization in
codegen.

compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/processUninitializedStores.kt

@@ -16,10 +16,8 @@
 
 package org.jetbrains.kotlin.codegen.coroutines
 
-import org.jetbrains.kotlin.codegen.optimization.common.CustomFramesMethodAnalyzer
-import org.jetbrains.kotlin.codegen.optimization.common.OptimizationBasicInterpreter
-import org.jetbrains.kotlin.codegen.optimization.common.StrictBasicValue
-import org.jetbrains.kotlin.codegen.optimization.common.insnListOf
+import org.jetbrains.kotlin.codegen.optimization.common.*
+import org.jetbrains.kotlin.codegen.optimization.fixStack.peek
 import org.jetbrains.org.objectweb.asm.Opcodes
 import org.jetbrains.org.objectweb.asm.Type
 import org.jetbrains.org.objectweb.asm.tree.*
@@ -71,124 +69,161 @@ import org.jetbrains.org.objectweb.asm.tree.analysis.Interpreter
  *   - restore constructor arguments
  */
 internal fun processUninitializedStores(methodNode: MethodNode) {
-    val interpreter = UninitializedNewValueMarkerInterpreter()
-    val frames = CustomFramesMethodAnalyzer("fake", methodNode, interpreter, ::UninitializedNewValueFrame).analyze()
-
-    for ((index, insn) in methodNode.instructions.toArray().withIndex()) {
-        val frame = frames[index] ?: continue
-        val uninitializedValue = frame.getUninitializedValueForConstructorCall(insn) ?: continue
-
-        val copyUsages: Set<AbstractInsnNode> = interpreter.uninitializedValuesToCopyUsages[uninitializedValue.newInsn]!!
-        assert(copyUsages.size > 0) { "At least DUP copy operation expected" }
-
-        // Value generated by NEW wasn't store to local/field (only DUPed)
-        if (copyUsages.size == 1) continue
+    UninitializedStoresProcessor(methodNode, forCoroutines = true).run()
+}
 
-        (copyUsages + uninitializedValue.newInsn).forEach {
-            methodNode.instructions.remove(it)
-        }
+class UninitializedStoresProcessor(private val methodNode: MethodNode, private val forCoroutines: Boolean) {
+    private val isInSpecialMethod = methodNode.name == "<init>" || methodNode.name == "<clinit>"
+
+    fun run() {
+        val interpreter = UninitializedNewValueMarkerInterpreter()
+
+        val frames = CustomFramesMethodAnalyzer(
+                "fake", methodNode, interpreter,
+                this::UninitializedNewValueFrame
+        ).analyze()
+
+        for ((index, insn) in methodNode.instructions.toArray().withIndex()) {
+            val frame = frames[index] ?: continue
+            val uninitializedValue = frame.getUninitializedValueForConstructorCall(insn) ?: continue
+
+            val newInsn = uninitializedValue.newInsn
+            val copyUsages: Set<AbstractInsnNode> = interpreter.uninitializedValuesToCopyUsages[newInsn]!!
+            assert(copyUsages.isNotEmpty()) { "At least DUP copy operation expected" }
+
+            // Value generated by NEW wasn't store to local/field (only DUPed)
+            if (copyUsages.size == 1) continue
+
+            methodNode.instructions.run {
+                removeAll(copyUsages)
+                if (forCoroutines) {
+                    remove(newInsn)
+                }
+                else {
+                    // Replace 'NEW C' instruction with "manual" initialization of class 'C':
+                    //      LDC [typeName for C]
+                    //      INVOKESTATIC java/lang/Class.forName (Ljava/lang/String;)Ljava/lang/Class;
+                    //      POP
+                    val typeNameForClass = newInsn.desc.replace('/', '.')
+                    insertBefore(newInsn, LdcInsnNode(typeNameForClass))
+                    insertBefore(
+                            newInsn,
+                            MethodInsnNode(
+                                    Opcodes.INVOKESTATIC,
+                                    "java/lang/Class", "forName", "(Ljava/lang/String;)Ljava/lang/Class;",
+                                    false
+                            )
+                    )
+                    set(newInsn, InsnNode(Opcodes.POP))
+                }
+            }
 
-        val indexOfConstructorArgumentFromTopOfStack = Type.getArgumentTypes((insn as MethodInsnNode).desc).size
-        val storedTypes = arrayListOf<Type>()
-        var nextVarIndex = methodNode.maxLocals
+            val indexOfConstructorArgumentFromTopOfStack = Type.getArgumentTypes((insn as MethodInsnNode).desc).size
+            val storedTypes = arrayListOf<Type>()
+            var nextVarIndex = methodNode.maxLocals
 
-        for (i in 0 until indexOfConstructorArgumentFromTopOfStack) {
-            val value = frame.getStack(frame.stackSize - 1 - i)
-            val type = value.type
-            methodNode.instructions.insertBefore(insn, VarInsnNode(type.getOpcode(Opcodes.ISTORE), nextVarIndex))
-            nextVarIndex += type.size
-            storedTypes.add(type)
-        }
-        methodNode.maxLocals = Math.max(methodNode.maxLocals, nextVarIndex)
+            for (i in 0 until indexOfConstructorArgumentFromTopOfStack) {
+                val value = frame.getStack(frame.stackSize - 1 - i)
+                val type = value.type
+                methodNode.instructions.insertBefore(insn, VarInsnNode(type.getOpcode(Opcodes.ISTORE), nextVarIndex))
+                nextVarIndex += type.size
+                storedTypes.add(type)
+            }
+            methodNode.maxLocals = Math.max(methodNode.maxLocals, nextVarIndex)
 
-        methodNode.instructions.insertBefore(insn, insnListOf(
-                TypeInsnNode(Opcodes.NEW, uninitializedValue.newInsn.desc),
-                InsnNode(Opcodes.DUP)
-        ))
+            methodNode.instructions.insertBefore(insn, insnListOf(
+                    TypeInsnNode(Opcodes.NEW, newInsn.desc),
+                    InsnNode(Opcodes.DUP)
+            ))
 
-        for (type in storedTypes.reversed()) {
-            nextVarIndex -= type.size
-            methodNode.instructions.insertBefore(insn, VarInsnNode(type.getOpcode(Opcodes.ILOAD), nextVarIndex))
+            for (type in storedTypes.reversed()) {
+                nextVarIndex -= type.size
+                methodNode.instructions.insertBefore(insn, VarInsnNode(type.getOpcode(Opcodes.ILOAD), nextVarIndex))
+            }
         }
     }
-}
-
-private class UninitializedNewValue(
-        val newInsn: TypeInsnNode, val internalName: String
-) : StrictBasicValue(Type.getObjectType(internalName)) {
-    override fun toString() = "UninitializedNewValue(internalName='$internalName')"
-}
 
-private class UninitializedNewValueFrame(nLocals: Int, nStack: Int) : Frame<BasicValue>(nLocals, nStack) {
-    override fun execute(insn: AbstractInsnNode, interpreter: Interpreter<BasicValue>?) {
-        val replaceTopValueWithInitialized = getUninitializedValueForConstructorCall(insn) != null
+    private inner class UninitializedNewValueFrame(nLocals: Int, nStack: Int) : Frame<BasicValue>(nLocals, nStack) {
+        override fun execute(insn: AbstractInsnNode, interpreter: Interpreter<BasicValue>?) {
+            val replaceTopValueWithInitialized = getUninitializedValueForConstructorCall(insn) != null
 
-        super.execute(insn, interpreter)
+            super.execute(insn, interpreter)
 
-        if (replaceTopValueWithInitialized) {
-            // Drop top value
-            val value = pop() as UninitializedNewValue
+            if (replaceTopValueWithInitialized) {
+                // Drop top value
+                val value = pop() as UninitializedNewValue
 
-            // uninitialized value become initialized after <init> call
-            push(StrictBasicValue(value.type))
+                // uninitialized value become initialized after <init> call
+                push(StrictBasicValue(value.type))
+            }
         }
     }
-}
 
-/**
- * @return value generated by NEW that used as 0-th argument of constructor call or null if current instruction is not constructor call
- */
-private fun Frame<BasicValue>.getUninitializedValueForConstructorCall(
-        insn: AbstractInsnNode
-): UninitializedNewValue? {
-    if (!insn.isConstructorCall()) return null
-
-    assert(insn.opcode == Opcodes.INVOKESPECIAL) { "Expected opcode Opcodes.INVOKESPECIAL for <init>, but ${insn.opcode} found" }
-    val paramsCountIncludingReceiver = Type.getArgumentTypes((insn as MethodInsnNode).desc).size + 1
-    val newValue = getStack(stackSize - (paramsCountIncludingReceiver + 1)) as? UninitializedNewValue ?: error("Expected value generated with NEW")
+    /**
+     * @return value generated by NEW that used as 0-th argument of constructor call or null if current instruction is not constructor call
+     */
+    private fun Frame<BasicValue>.getUninitializedValueForConstructorCall(insn: AbstractInsnNode): UninitializedNewValue? {
+        if (!insn.isConstructorCall()) return null
+
+        assert(insn.opcode == Opcodes.INVOKESPECIAL) { "Expected opcode Opcodes.INVOKESPECIAL for <init>, but ${insn.opcode} found" }
+        val paramsCountIncludingReceiver = Type.getArgumentTypes((insn as MethodInsnNode).desc).size + 1
+        val newValue = peek(paramsCountIncludingReceiver) as? UninitializedNewValue ?:
+                       if (isInSpecialMethod)
+                           return null
+                       else
+                           error("Expected value generated with NEW")
+
+        assert(peek(paramsCountIncludingReceiver - 1) is UninitializedNewValue) {
+            "Next value after NEW should be one generated by DUP"
+        }
 
-    assert(getStack(stackSize - paramsCountIncludingReceiver) is UninitializedNewValue) {
-        "Next value after NEW should be one generated by DUP"
+        return newValue
     }
 
-    return newValue
-}
+    private class UninitializedNewValue(
+            val newInsn: TypeInsnNode,
+            val internalName: String
+    ) : StrictBasicValue(Type.getObjectType(internalName)) {
+        override fun toString() = "UninitializedNewValue(internalName='$internalName')"
+    }
 
-private fun AbstractInsnNode.isConstructorCall() = this is MethodInsnNode && this.name == "<init>"
 
-private class UninitializedNewValueMarkerInterpreter : OptimizationBasicInterpreter() {
-    val uninitializedValuesToCopyUsages = hashMapOf<AbstractInsnNode, MutableSet<AbstractInsnNode>>()
-    override fun newOperation(insn: AbstractInsnNode): BasicValue? {
-        if (insn.opcode == Opcodes.NEW) {
-            uninitializedValuesToCopyUsages.getOrPut(insn) { mutableSetOf() }
-            return UninitializedNewValue(insn as TypeInsnNode, insn.desc)
-        }
-        return super.newOperation(insn)
-    }
+    private fun AbstractInsnNode.isConstructorCall() = this is MethodInsnNode && this.name == "<init>"
 
-    override fun copyOperation(insn: AbstractInsnNode, value: BasicValue?): BasicValue? {
-        if (value is UninitializedNewValue) {
-            uninitializedValuesToCopyUsages[value.newInsn]!!.add(insn)
-            return value
+    private class UninitializedNewValueMarkerInterpreter : OptimizationBasicInterpreter() {
+        val uninitializedValuesToCopyUsages = hashMapOf<AbstractInsnNode, MutableSet<AbstractInsnNode>>()
+        override fun newOperation(insn: AbstractInsnNode): BasicValue? {
+            if (insn.opcode == Opcodes.NEW) {
+                uninitializedValuesToCopyUsages.getOrPut(insn) { mutableSetOf() }
+                return UninitializedNewValue(insn as TypeInsnNode, insn.desc)
+            }
+            return super.newOperation(insn)
         }
-        return super.copyOperation(insn, value)
-    }
 
-    override fun merge(v: BasicValue, w: BasicValue): BasicValue {
-        if (v === w) return v
-        if (v === StrictBasicValue.UNINITIALIZED_VALUE || w === StrictBasicValue.UNINITIALIZED_VALUE) {
-            return StrictBasicValue.UNINITIALIZED_VALUE
+        override fun copyOperation(insn: AbstractInsnNode, value: BasicValue?): BasicValue? {
+            if (value is UninitializedNewValue) {
+                uninitializedValuesToCopyUsages[value.newInsn]!!.add(insn)
+                return value
+            }
+            return super.copyOperation(insn, value)
         }
 
-        if (v is UninitializedNewValue || w is UninitializedNewValue) {
-            if ((v as? UninitializedNewValue)?.newInsn !== (w as? UninitializedNewValue)?.newInsn) {
-                // Merge of two different ANEW result is possible, but such values should not be used further
+        override fun merge(v: BasicValue, w: BasicValue): BasicValue {
+            if (v === w) return v
+            if (v === StrictBasicValue.UNINITIALIZED_VALUE || w === StrictBasicValue.UNINITIALIZED_VALUE) {
                 return StrictBasicValue.UNINITIALIZED_VALUE
             }
 
-            return v
-        }
+            if (v is UninitializedNewValue || w is UninitializedNewValue) {
+                if ((v as? UninitializedNewValue)?.newInsn !== (w as? UninitializedNewValue)?.newInsn) {
+                    // Merge of two different ANEW result is possible, but such values should not be used further
+                    return StrictBasicValue.UNINITIALIZED_VALUE
+                }
 
-        return super.merge(v, w)
+                return v
+            }
+
+            return super.merge(v, w)
+        }
     }
 }

compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/OptimizationClassBuilderFactory.java

@@ -17,9 +17,7 @@
 package org.jetbrains.kotlin.codegen.optimization;
 
 import org.jetbrains.annotations.NotNull;
-import org.jetbrains.kotlin.codegen.ClassBuilder;
 import org.jetbrains.kotlin.codegen.ClassBuilderFactory;
-import org.jetbrains.kotlin.codegen.ClassBuilderMode;
 import org.jetbrains.kotlin.codegen.DelegatingClassBuilderFactory;
 import org.jetbrains.kotlin.resolve.jvm.diagnostics.JvmDeclarationOrigin;
 

compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/OptimizationMethodVisitor.kt

@@ -17,9 +17,9 @@
 package org.jetbrains.kotlin.codegen.optimization
 
 import org.jetbrains.kotlin.codegen.TransformationMethodVisitor
-import org.jetbrains.kotlin.codegen.optimization.boxing.StackPeepholeOptimizationsTransformer
-import org.jetbrains.kotlin.codegen.optimization.boxing.RedundantBoxingMethodTransformer
 import org.jetbrains.kotlin.codegen.optimization.boxing.PopBackwardPropagationTransformer
+import org.jetbrains.kotlin.codegen.optimization.boxing.RedundantBoxingMethodTransformer
+import org.jetbrains.kotlin.codegen.optimization.boxing.StackPeepholeOptimizationsTransformer
 import org.jetbrains.kotlin.codegen.optimization.common.prepareForEmitting
 import org.jetbrains.kotlin.codegen.optimization.nullCheck.RedundantNullCheckMethodTransformer
 import org.jetbrains.kotlin.codegen.optimization.transformer.CompositeMethodTransformer
@@ -35,24 +35,24 @@ class OptimizationMethodVisitor(
         signature: String?,
         exceptions: Array<String>?
 ) : TransformationMethodVisitor(delegate, access, name, desc, signature, exceptions) {
-
     override fun performTransformations(methodNode: MethodNode) {
-        MANDATORY_METHOD_TRANSFORMER.transform("fake", methodNode)
+        normalizationMethodTransformer.transform("fake", methodNode)
         if (canBeOptimized(methodNode) && !disableOptimization) {
-            OPTIMIZATION_TRANSFORMER.transform("fake", methodNode)
+            optimizationTransformer.transform("fake", methodNode)
         }
         methodNode.prepareForEmitting()
     }
 
     companion object {
         private val MEMORY_LIMIT_BY_METHOD_MB = 50
 
-        private val MANDATORY_METHOD_TRANSFORMER = CompositeMethodTransformer(
+        val normalizationMethodTransformer = CompositeMethodTransformer(
                 FixStackWithLabelNormalizationMethodTransformer(),
+                UninitializedStoresMethodTransformer(),
                 MethodVerifier("AFTER mandatory stack transformations")
         )
 
-        private val OPTIMIZATION_TRANSFORMER = CompositeMethodTransformer(
+        val optimizationTransformer = CompositeMethodTransformer(
                 CapturedVarsOptimizationMethodTransformer(),
                 RedundantNullCheckMethodTransformer(),
                 RedundantCheckCastEliminationMethodTransformer(),

compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/UninitializedStoresMethodTransformer.kt

@@ -0,0 +1,27 @@
+/*
+ * Copyright 2010-2017 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 org.jetbrains.kotlin.codegen.optimization
+
+import org.jetbrains.kotlin.codegen.coroutines.UninitializedStoresProcessor
+import org.jetbrains.kotlin.codegen.optimization.transformer.MethodTransformer
+import org.jetbrains.org.objectweb.asm.tree.MethodNode
+
+class UninitializedStoresMethodTransformer : MethodTransformer() {
+    override fun transform(internalClassName: String, methodNode: MethodNode) {
+        UninitializedStoresProcessor(methodNode, forCoroutines = false).run()
+    }
+}

compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/common/Util.kt

@@ -214,3 +214,6 @@ internal inline fun <reified T : AbstractInsnNode> AbstractInsnNode.isInsn(opcod
 internal inline fun <reified T : AbstractInsnNode> AbstractInsnNode.takeInsnIf(opcode: Int, condition: T.() -> Boolean): T? =
         takeIf { it.opcode == opcode }?.safeAs<T>()?.takeIf { it.condition() }
 
+fun InsnList.removeAll(nodes: Collection<AbstractInsnNode>) {
+    for (node in nodes) remove(node)
+}

compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/transformer/CompositeMethodTransformer.kt

@@ -18,8 +18,15 @@ package org.jetbrains.kotlin.codegen.optimization.transformer
 
 import org.jetbrains.org.objectweb.asm.tree.MethodNode
 
-open class CompositeMethodTransformer(private vararg val transformers: MethodTransformer) : MethodTransformer() {
+open class CompositeMethodTransformer(private val transformers: List<MethodTransformer>) : MethodTransformer() {
+    constructor(vararg transformers: MethodTransformer?) : this(transformers.filterNotNull())
+
     override fun transform(internalClassName: String, methodNode: MethodNode) {
         transformers.forEach { it.transform(internalClassName, methodNode) }
     }
+
+    companion object {
+        inline fun build(builder: MutableList<MethodTransformer>.() -> Unit) =
+                CompositeMethodTransformer(ArrayList<MethodTransformer>().apply { builder() })
+    }
 }

compiler/backend/src/org/jetbrains/kotlin/codegen/state/GenerationState.kt

@@ -179,7 +179,6 @@ class GenerationState @JvmOverloads constructor(
 
     val generateParametersMetadata: Boolean = configuration.getBoolean(JVMConfigurationKeys.PARAMETERS_METADATA)
 
-
     val shouldInlineConstVals = languageVersionSettings.supportsFeature(LanguageFeature.InlineConstVals)
 
     init {