Skip to content

Commit

Permalink
Process uninitialized stores in mandatory bytecode pass
Browse files Browse the repository at this point in the history 
See
https://youtrack.jetbrains.com/issue/KT-19251
puniverse/quasar#280

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).
In order to avoid that, we apply 'processUnitializedStores' already
implemented for coroutines. It moves 'NEW' instructions after the
constructor arguments evaluation, producing code like

<evaluate constructor arguments>
<store constructor arguments to variables>
NEW C
DUP
<load constructor arguments from variables>
INVOKESPECIAL C.<init>(...)

This transformation changes evaluation order for the expression,
because 'NEW C' might cause 'C.<clinit>' invocation, thus we can enable
it only in 1.2+.

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.

TODO KT-19251 Fixed Target versions 1.2
  • Loading branch information
@dnpetrov
dnpetrov committed Jul 28, 2017
1 parent 6733665 commit 7d5015d
Show file tree
Hide file tree
Showing 15 changed files with 398 additions and 123 deletions.
193 changes: 105 additions & 88 deletions compiler/backend/src/org/jetbrains/kotlin/codegen/coroutines/processUninitializedStores.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import org.jetbrains.kotlin.codegen.optimization.common.CustomFramesMethodAnalyz
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.fixStack.peek
import org.jetbrains.org.objectweb.asm.Opcodes
import org.jetbrains.org.objectweb.asm.Type
import org.jetbrains.org.objectweb.asm.tree.*
Expand Down Expand Up @@ -71,124 +72,140 @@ 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()
UninitializedStoresProcessor(methodNode, forCoroutines = true).run()
}

for ((index, insn) in methodNode.instructions.toArray().withIndex()) {
val frame = frames[index] ?: continue
val uninitializedValue = frame.getUninitializedValueForConstructorCall(insn) ?: continue
class UninitializedStoresProcessor(private val methodNode: MethodNode, forCoroutines: Boolean) {
private val isInSpecialMethod = !forCoroutines && (methodNode.name == "<init>" || methodNode.name == "<clinit>")

val copyUsages: Set<AbstractInsnNode> = interpreter.uninitializedValuesToCopyUsages[uninitializedValue.newInsn]!!
assert(copyUsages.size > 0) { "At least DUP copy operation expected" }
fun run() {
val interpreter = UninitializedNewValueMarkerInterpreter()

// Value generated by NEW wasn't store to local/field (only DUPed)
if (copyUsages.size == 1) continue
val frames = CustomFramesMethodAnalyzer(
"fake", methodNode, interpreter,
this::UninitializedNewValueFrame
).analyze()

(copyUsages + uninitializedValue.newInsn).forEach {
methodNode.instructions.remove(it)
}
for ((index, insn) in methodNode.instructions.toArray().withIndex()) {
val frame = frames[index] ?: continue
val uninitializedValue = frame.getUninitializedValueForConstructorCall(insn) ?: continue

val indexOfConstructorArgumentFromTopOfStack = Type.getArgumentTypes((insn as MethodInsnNode).desc).size
val storedTypes = arrayListOf<Type>()
var nextVarIndex = methodNode.maxLocals
val copyUsages: Set<AbstractInsnNode> = interpreter.uninitializedValuesToCopyUsages[uninitializedValue.newInsn]!!
assert(copyUsages.isNotEmpty()) { "At least DUP copy operation expected" }

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)
// Value generated by NEW wasn't store to local/field (only DUPed)
if (copyUsages.size == 1) continue

(copyUsages + uninitializedValue.newInsn).forEach {
methodNode.instructions.remove(it)
}

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)

methodNode.instructions.insertBefore(insn, insnListOf(
TypeInsnNode(Opcodes.NEW, uninitializedValue.newInsn.desc),
InsnNode(Opcodes.DUP)
))
methodNode.instructions.insertBefore(insn, insnListOf(
TypeInsnNode(Opcodes.NEW, uninitializedValue.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)
}
}
}
11 changes: 9 additions & 2 deletions compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/OptimizationClassBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,23 @@
import org.jetbrains.annotations.Nullable;
import org.jetbrains.kotlin.codegen.ClassBuilder;
import org.jetbrains.kotlin.codegen.DelegatingClassBuilder;
import org.jetbrains.kotlin.config.LanguageVersionSettings;
import org.jetbrains.kotlin.resolve.jvm.diagnostics.JvmDeclarationOrigin;
import org.jetbrains.org.objectweb.asm.MethodVisitor;

public class OptimizationClassBuilder extends DelegatingClassBuilder {
private final ClassBuilder delegate;
private final boolean disableOptimization;
private final LanguageVersionSettings languageVersionSettings;

public OptimizationClassBuilder(@NotNull ClassBuilder delegate, boolean disableOptimization) {
public OptimizationClassBuilder(
@NotNull ClassBuilder delegate,
boolean disableOptimization,
LanguageVersionSettings languageVersionSettings
) {
this.delegate = delegate;
this.disableOptimization = disableOptimization;
this.languageVersionSettings = languageVersionSettings;
}

@NotNull
Expand All @@ -50,7 +57,7 @@ public MethodVisitor newMethod(
) {
return new OptimizationMethodVisitor(
super.newMethod(origin, access, name, desc, signature, exceptions),
disableOptimization,
disableOptimization, languageVersionSettings,
access, name, desc, signature, exceptions
);
}
Expand Down
11 changes: 9 additions & 2 deletions ...ackend/src/org/jetbrains/kotlin/codegen/optimization/OptimizationClassBuilderFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,19 +21,26 @@
import org.jetbrains.kotlin.codegen.ClassBuilderFactory;
import org.jetbrains.kotlin.codegen.ClassBuilderMode;
import org.jetbrains.kotlin.codegen.DelegatingClassBuilderFactory;
import org.jetbrains.kotlin.config.LanguageVersionSettings;
import org.jetbrains.kotlin.resolve.jvm.diagnostics.JvmDeclarationOrigin;

public class OptimizationClassBuilderFactory extends DelegatingClassBuilderFactory {
private final boolean disableOptimization;
private final LanguageVersionSettings languageVersionSettings;

public OptimizationClassBuilderFactory(ClassBuilderFactory delegate, boolean disableOptimization) {
public OptimizationClassBuilderFactory(
ClassBuilderFactory delegate,
boolean disableOptimization,
LanguageVersionSettings languageVersionSettings
) {
super(delegate);
this.disableOptimization = disableOptimization;
this.languageVersionSettings = languageVersionSettings;
}

@NotNull
@Override
public OptimizationClassBuilder newClassBuilder(@NotNull JvmDeclarationOrigin origin) {
return new OptimizationClassBuilder(getDelegate().newClassBuilder(origin), disableOptimization);
return new OptimizationClassBuilder(getDelegate().newClassBuilder(origin), disableOptimization, languageVersionSettings);
}
}
47 changes: 26 additions & 21 deletions compiler/backend/src/org/jetbrains/kotlin/codegen/optimization/OptimizationMethodVisitor.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,24 +17,48 @@
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
import org.jetbrains.kotlin.config.LanguageFeature
import org.jetbrains.kotlin.config.LanguageVersionSettings
import org.jetbrains.org.objectweb.asm.MethodVisitor
import org.jetbrains.org.objectweb.asm.tree.MethodNode

class OptimizationMethodVisitor(
delegate: MethodVisitor,
private val disableOptimization: Boolean,
languageVersionSettings: LanguageVersionSettings,
access: Int,
name: String,
desc: String,
signature: String?,
exceptions: Array<String>?
) : TransformationMethodVisitor(delegate, access, name, desc, signature, exceptions) {
private val MANDATORY_METHOD_TRANSFORMER = CompositeMethodTransformer(
FixStackWithLabelNormalizationMethodTransformer(),
UninitializedStoresMethodTransformer().takeIf {
languageVersionSettings.supportsFeature(LanguageFeature.NoUninitializedObjectStores)
},
MethodVerifier("AFTER mandatory stack transformations")
)

private val OPTIMIZATION_TRANSFORMER = CompositeMethodTransformer(
CapturedVarsOptimizationMethodTransformer(),
RedundantNullCheckMethodTransformer(),
RedundantCheckCastEliminationMethodTransformer(),
ConstantConditionEliminationMethodTransformer(),
RedundantBoxingMethodTransformer(),
StackPeepholeOptimizationsTransformer(),
PopBackwardPropagationTransformer(),
DeadCodeEliminationMethodTransformer(),
RedundantGotoMethodTransformer(),
RedundantNopsCleanupMethodTransformer(),
MethodVerifier("AFTER optimizations")
)

override fun performTransformations(methodNode: MethodNode) {
MANDATORY_METHOD_TRANSFORMER.transform("fake", methodNode)
Expand All @@ -47,25 +71,6 @@ class OptimizationMethodVisitor(
companion object {
private val MEMORY_LIMIT_BY_METHOD_MB = 50

private val MANDATORY_METHOD_TRANSFORMER = CompositeMethodTransformer(
FixStackWithLabelNormalizationMethodTransformer(),
MethodVerifier("AFTER mandatory stack transformations")
)

private val OPTIMIZATION_TRANSFORMER = CompositeMethodTransformer(
CapturedVarsOptimizationMethodTransformer(),
RedundantNullCheckMethodTransformer(),
RedundantCheckCastEliminationMethodTransformer(),
ConstantConditionEliminationMethodTransformer(),
RedundantBoxingMethodTransformer(),
StackPeepholeOptimizationsTransformer(),
PopBackwardPropagationTransformer(),
DeadCodeEliminationMethodTransformer(),
RedundantGotoMethodTransformer(),
RedundantNopsCleanupMethodTransformer(),
MethodVerifier("AFTER optimizations")
)

fun canBeOptimized(node: MethodNode): Boolean {
val totalFramesSizeMb = node.instructions.size() * (node.maxLocals + node.maxStack) / (1024 * 1024)
return totalFramesSizeMb < MEMORY_LIMIT_BY_METHOD_MB
Expand Down
Loading

0 comments on commit 7d5015d

Please sign in to comment.