Mark fields as final where possible

Signed-off-by: Graham <gpe@openrs2.dev>

asm/src/main/java/dev/openrs2/asm/classpath/ClassMetadata.kt

@@ -71,6 +71,23 @@ abstract class ClassMetadata {
         return false
     }
 
+    fun resolveField(member: MemberDesc): ClassMetadata? {
+        // see https://docs.oracle.com/javase/specs/jvms/se11/html/jvms-5.html#jvms-5.4.3.2
+
+        if (fields.contains(member)) {
+            return this
+        }
+
+        for (superInterface in superInterfaces) {
+            val field = superInterface.resolveField(member)
+            if (field != null) {
+                return field
+            }
+        }
+
+        return superClass?.resolveField(member)
+    }
+
     override fun equals(other: Any?): Boolean {
         if (this === other) return true
         if (other !is ClassMetadata) return false

deob/src/main/java/dev/openrs2/deob/DeobfuscatorModule.kt

@@ -18,6 +18,7 @@ import dev.openrs2.deob.transform.EmptyClassTransformer
 import dev.openrs2.deob.transform.ExceptionTracingTransformer
 import dev.openrs2.deob.transform.FernflowerExceptionTransformer
 import dev.openrs2.deob.transform.FieldOrderTransformer
+import dev.openrs2.deob.transform.FinalFieldTransformer
 import dev.openrs2.deob.transform.FinalTransformer
 import dev.openrs2.deob.transform.InvokeSpecialTransformer
 import dev.openrs2.deob.transform.MethodOrderTransformer
@@ -69,6 +70,7 @@ object DeobfuscatorModule : AbstractModule() {
         binder.addBinding().to(MethodOrderTransformer::class.java)
         binder.addBinding().to(VisibilityTransformer::class.java)
         binder.addBinding().to(FinalTransformer::class.java)
+        binder.addBinding().to(FinalFieldTransformer::class.java)
         binder.addBinding().to(OverrideTransformer::class.java)
         binder.addBinding().to(RedundantGotoTransformer::class.java)
         binder.addBinding().to(OriginalPcRestoreTransformer::class.java)

deob/src/main/java/dev/openrs2/deob/analysis/FieldWriteAnalyzer.kt

@@ -0,0 +1,78 @@
+package dev.openrs2.deob.analysis
+
+import dev.openrs2.asm.MemberDesc
+import dev.openrs2.asm.classpath.ClassPath
+import org.objectweb.asm.Opcodes
+import org.objectweb.asm.tree.AbstractInsnNode
+import org.objectweb.asm.tree.FieldInsnNode
+import org.objectweb.asm.tree.MethodNode
+import org.objectweb.asm.tree.analysis.Frame
+
+class FieldWriteAnalyzer(
+    private val owner: String,
+    private val method: MethodNode,
+    private val classPath: ClassPath,
+    private val frames: Array<Frame<ThisValue>>
+) : DataFlowAnalyzer<Map<MemberDesc, FieldWriteCount>>(owner, method) {
+    override fun createInitialSet(): Map<MemberDesc, FieldWriteCount> {
+        return emptyMap()
+    }
+
+    override fun join(
+        set1: Map<MemberDesc, FieldWriteCount>,
+        set2: Map<MemberDesc, FieldWriteCount>
+    ): Map<MemberDesc, FieldWriteCount> {
+        if (set1 == set2) {
+            return set1
+        }
+
+        val set = mutableMapOf<MemberDesc, FieldWriteCount>()
+
+        for (member in set1.keys union set2.keys) {
+            val count1 = set1.getOrDefault(member, FieldWriteCount.NEVER)
+            val count2 = set2.getOrDefault(member, FieldWriteCount.NEVER)
+
+            set[member] = if (count1 == count2) {
+                count1
+            } else {
+                FieldWriteCount.UNKNOWN
+            }
+        }
+
+        return set
+    }
+
+    override fun transfer(
+        set: Map<MemberDesc, FieldWriteCount>,
+        insn: AbstractInsnNode
+    ): Map<MemberDesc, FieldWriteCount> {
+        if (insn !is FieldInsnNode) {
+            return set
+        } else if (insn.opcode != Opcodes.PUTFIELD && insn.opcode != Opcodes.PUTSTATIC) {
+            return set
+        }
+
+        val member = MemberDesc(insn)
+        val declaredOwner = classPath[insn.owner]!!.resolveField(member)!!.name
+        if (declaredOwner != owner) {
+            return set
+        }
+
+        val isThis = if (insn.opcode == Opcodes.PUTFIELD) {
+            val insnIndex = method.instructions.indexOf(insn)
+            val frame = frames[insnIndex]
+            frame.getStack(frame.stackSize - 2).isThis
+        } else {
+            true
+        }
+
+        val count = set.getOrDefault(member, FieldWriteCount.NEVER)
+        return when {
+            isThis && count == FieldWriteCount.NEVER -> set.plus(Pair(member, FieldWriteCount.EXACTLY_ONCE))
+            isThis && count == FieldWriteCount.EXACTLY_ONCE -> set.plus(Pair(member, FieldWriteCount.UNKNOWN))
+            // save an allocation if count is already set to UNKNOWN
+            count == FieldWriteCount.UNKNOWN -> set
+            else -> set.plus(Pair(member, FieldWriteCount.UNKNOWN))
+        }
+    }
+}

deob/src/main/java/dev/openrs2/deob/analysis/FieldWriteCount.kt

@@ -0,0 +1,7 @@
+package dev.openrs2.deob.analysis
+
+enum class FieldWriteCount {
+    NEVER,
+    EXACTLY_ONCE,
+    UNKNOWN
+}

deob/src/main/java/dev/openrs2/deob/analysis/ThisInterpreter.kt

@@ -0,0 +1,80 @@
+package dev.openrs2.deob.analysis
+
+import org.objectweb.asm.Opcodes
+import org.objectweb.asm.Type
+import org.objectweb.asm.tree.AbstractInsnNode
+import org.objectweb.asm.tree.analysis.BasicInterpreter
+import org.objectweb.asm.tree.analysis.Interpreter
+
+class ThisInterpreter : Interpreter<ThisValue>(Opcodes.ASM8) {
+    private val basicInterpreter = BasicInterpreter()
+
+    override fun newValue(type: Type?): ThisValue? {
+        val basicValue = basicInterpreter.newValue(type) ?: return null
+        return ThisValue(basicValue, false)
+    }
+
+    override fun newParameterValue(isInstanceMethod: Boolean, local: Int, type: Type): ThisValue {
+        val basicValue = basicInterpreter.newParameterValue(isInstanceMethod, local, type)
+        return ThisValue(basicValue, isInstanceMethod && local == 0)
+    }
+
+    override fun newOperation(insn: AbstractInsnNode): ThisValue {
+        val basicValue = basicInterpreter.newOperation(insn)
+        return ThisValue(basicValue, false)
+    }
+
+    override fun copyOperation(insn: AbstractInsnNode, value: ThisValue): ThisValue {
+        val basicValue = basicInterpreter.copyOperation(insn, value.basicValue)
+        /*
+         * Only allow "this"ness to propagate from a variable to the stack, not
+         * vice-versa. This is compatible with javac's analysis.
+         */
+        return if (insn.opcode == Opcodes.ASTORE) {
+            ThisValue(basicValue, false)
+        } else {
+            ThisValue(basicValue, value.isThis)
+        }
+    }
+
+    override fun unaryOperation(insn: AbstractInsnNode, value: ThisValue): ThisValue? {
+        val basicValue = basicInterpreter.unaryOperation(insn, value.basicValue) ?: return null
+        return ThisValue(basicValue, false)
+    }
+
+    override fun binaryOperation(insn: AbstractInsnNode, value1: ThisValue, value2: ThisValue): ThisValue? {
+        val basicValue = basicInterpreter.binaryOperation(insn, value1.basicValue, value2.basicValue)
+            ?: return null
+        return ThisValue(basicValue, false)
+    }
+
+    override fun ternaryOperation(
+        insn: AbstractInsnNode,
+        value1: ThisValue,
+        value2: ThisValue,
+        value3: ThisValue
+    ): ThisValue? {
+        val basicValue = basicInterpreter.ternaryOperation(
+            insn,
+            value1.basicValue,
+            value2.basicValue,
+            value3.basicValue
+        ) ?: return null
+        return ThisValue(basicValue, false)
+    }
+
+    override fun naryOperation(insn: AbstractInsnNode, values: List<ThisValue>): ThisValue? {
+        val args = values.map(ThisValue::basicValue)
+        val basicValue = basicInterpreter.naryOperation(insn, args) ?: return null
+        return ThisValue(basicValue, false)
+    }
+
+    override fun returnOperation(insn: AbstractInsnNode, value: ThisValue, expected: ThisValue) {
+        basicInterpreter.returnOperation(insn, value.basicValue, expected.basicValue)
+    }
+
+    override fun merge(value1: ThisValue, value2: ThisValue): ThisValue {
+        val basicValue = basicInterpreter.merge(value1.basicValue, value2.basicValue)
+        return ThisValue(basicValue, value1.isThis && value2.isThis)
+    }
+}

deob/src/main/java/dev/openrs2/deob/analysis/ThisValue.kt

@@ -0,0 +1,10 @@
+package dev.openrs2.deob.analysis
+
+import org.objectweb.asm.tree.analysis.BasicValue
+import org.objectweb.asm.tree.analysis.Value
+
+data class ThisValue(val basicValue: BasicValue, val isThis: Boolean) : Value {
+    override fun getSize(): Int {
+        return basicValue.size
+    }
+}

deob/src/main/java/dev/openrs2/deob/transform/FinalFieldTransformer.kt

@@ -0,0 +1,139 @@
+package dev.openrs2.deob.transform
+
+import com.github.michaelbull.logging.InlineLogger
+import dev.openrs2.asm.MemberDesc
+import dev.openrs2.asm.MemberRef
+import dev.openrs2.asm.classpath.ClassPath
+import dev.openrs2.asm.classpath.Library
+import dev.openrs2.asm.transform.Transformer
+import dev.openrs2.deob.analysis.FieldWriteAnalyzer
+import dev.openrs2.deob.analysis.FieldWriteCount
+import dev.openrs2.deob.analysis.ThisInterpreter
+import dev.openrs2.util.collect.DisjointSet
+import org.objectweb.asm.Opcodes
+import org.objectweb.asm.tree.ClassNode
+import org.objectweb.asm.tree.FieldInsnNode
+import org.objectweb.asm.tree.FieldNode
+import org.objectweb.asm.tree.MethodInsnNode
+import org.objectweb.asm.tree.MethodNode
+import org.objectweb.asm.tree.analysis.Analyzer
+
+class FinalFieldTransformer : Transformer() {
+    private lateinit var inheritedFieldSets: DisjointSet<MemberRef>
+    private val nonFinalFields = mutableSetOf<DisjointSet.Partition<MemberRef>>()
+
+    override fun preTransform(classPath: ClassPath) {
+        inheritedFieldSets = classPath.createInheritedFieldSets()
+        nonFinalFields.clear()
+    }
+
+    override fun transformCode(classPath: ClassPath, library: Library, clazz: ClassNode, method: MethodNode): Boolean {
+        val constructor = method.name == "<init>" || method.name == "<clinit>"
+        val thisCall = if (constructor && method.name == "<init>") {
+            method.instructions.filterIsInstance<MethodInsnNode>()
+                .any { it.opcode == Opcodes.INVOKESPECIAL && it.owner == clazz.name && it.name == "<init>" }
+        } else {
+            false
+        }
+
+        val constructorWithoutThisCall = constructor && !thisCall
+
+        val frames = if (constructorWithoutThisCall) {
+            Analyzer(ThisInterpreter()).analyze(clazz.name, method)
+        } else {
+            null
+        }
+
+        for (insn in method.instructions) {
+            if (insn !is FieldInsnNode) {
+                continue
+            } else if (insn.opcode != Opcodes.PUTFIELD && insn.opcode != Opcodes.PUTSTATIC) {
+                continue
+            }
+
+            if (constructorWithoutThisCall) {
+                val isThis = if (insn.opcode == Opcodes.PUTFIELD) {
+                    val insnIndex = method.instructions.indexOf(insn)
+                    val frame = frames!![insnIndex] ?: continue
+                    frame.getStack(frame.stackSize - 2).isThis
+                } else {
+                    true
+                }
+
+                val declaredOwner = classPath[insn.owner]!!.resolveField(MemberDesc(insn))!!.name
+                if (isThis && declaredOwner == clazz.name) {
+                    /*
+                     * Writes inside constructors without a this(...) call to
+                     * fields owned by the same class are analyzed separately - if
+                     * there is exactly one write in the constructor the field can
+                     * be made final.
+                     */
+                    continue
+                }
+            }
+
+            val partition = inheritedFieldSets[MemberRef(insn)] ?: continue
+            nonFinalFields += partition
+        }
+
+        if (constructorWithoutThisCall) {
+            val analyzer = FieldWriteAnalyzer(clazz.name, method, classPath, frames!!)
+            analyzer.analyze()
+
+            val exits = method.instructions.filter { it.opcode == Opcodes.RETURN }
+            for (insn in exits) {
+                val counts = analyzer.getOutSet(insn) ?: emptyMap()
+
+                for (field in clazz.fields) {
+                    val count = counts.getOrDefault(MemberDesc(field), FieldWriteCount.NEVER)
+                    if (count != FieldWriteCount.EXACTLY_ONCE) {
+                        val partition = inheritedFieldSets[MemberRef(clazz, field)]!!
+                        nonFinalFields += partition
+                    }
+                }
+            }
+        }
+
+        return false
+    }
+
+    override fun transformField(classPath: ClassPath, library: Library, clazz: ClassNode, field: FieldNode): Boolean {
+        if ((field.access and Opcodes.ACC_VOLATILE) != 0) {
+            val partition = inheritedFieldSets[MemberRef(clazz, field)]!!
+            nonFinalFields += partition
+        }
+
+        return false
+    }
+
+    override fun postTransform(classPath: ClassPath) {
+        var fieldsChanged = 0
+
+        for (library in classPath.libraries) {
+            for (clazz in library) {
+                for (field in clazz.fields) {
+                    val member = MemberRef(clazz, field)
+                    val partition = inheritedFieldSets[member]!!
+
+                    val access = field.access
+
+                    if (nonFinalFields.contains(partition)) {
+                        field.access = field.access and Opcodes.ACC_FINAL.inv()
+                    } else {
+                        field.access = field.access or Opcodes.ACC_FINAL
+                    }
+
+                    if (field.access != access) {
+                        fieldsChanged++
+                    }
+                }
+            }
+        }
+
+        logger.info { "Updated final modifier on $fieldsChanged fields" }
+    }
+
+    private companion object {
+        private val logger = InlineLogger()
+    }
+}