From 38f731f9860ba7b90c56a820d85f0fe31762f2fe Mon Sep 17 00:00:00 2001 From: Graham Date: Sat, 6 Jun 2020 13:27:50 +0100 Subject: [PATCH] Mark fields as final where possible Signed-off-by: Graham --- .../openrs2/asm/classpath/ClassMetadata.kt | 17 +++ .../dev/openrs2/deob/DeobfuscatorModule.kt | 2 + .../deob/analysis/FieldWriteAnalyzer.kt | 78 ++++++++++ .../openrs2/deob/analysis/FieldWriteCount.kt | 7 + .../openrs2/deob/analysis/ThisInterpreter.kt | 80 ++++++++++ .../dev/openrs2/deob/analysis/ThisValue.kt | 10 ++ .../deob/transform/FinalFieldTransformer.kt | 139 ++++++++++++++++++ 7 files changed, 333 insertions(+) create mode 100644 deob/src/main/java/dev/openrs2/deob/analysis/FieldWriteAnalyzer.kt create mode 100644 deob/src/main/java/dev/openrs2/deob/analysis/FieldWriteCount.kt create mode 100644 deob/src/main/java/dev/openrs2/deob/analysis/ThisInterpreter.kt create mode 100644 deob/src/main/java/dev/openrs2/deob/analysis/ThisValue.kt create mode 100644 deob/src/main/java/dev/openrs2/deob/transform/FinalFieldTransformer.kt diff --git a/asm/src/main/java/dev/openrs2/asm/classpath/ClassMetadata.kt b/asm/src/main/java/dev/openrs2/asm/classpath/ClassMetadata.kt index 08d639c3..2c20b65d 100644 --- a/asm/src/main/java/dev/openrs2/asm/classpath/ClassMetadata.kt +++ b/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 diff --git a/deob/src/main/java/dev/openrs2/deob/DeobfuscatorModule.kt b/deob/src/main/java/dev/openrs2/deob/DeobfuscatorModule.kt index f1896c8f..454bce22 100644 --- a/deob/src/main/java/dev/openrs2/deob/DeobfuscatorModule.kt +++ b/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) diff --git a/deob/src/main/java/dev/openrs2/deob/analysis/FieldWriteAnalyzer.kt b/deob/src/main/java/dev/openrs2/deob/analysis/FieldWriteAnalyzer.kt new file mode 100644 index 00000000..d958cdaa --- /dev/null +++ b/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> +) : DataFlowAnalyzer>(owner, method) { + override fun createInitialSet(): Map { + return emptyMap() + } + + override fun join( + set1: Map, + set2: Map + ): Map { + if (set1 == set2) { + return set1 + } + + val set = mutableMapOf() + + 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, + insn: AbstractInsnNode + ): Map { + 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)) + } + } +} diff --git a/deob/src/main/java/dev/openrs2/deob/analysis/FieldWriteCount.kt b/deob/src/main/java/dev/openrs2/deob/analysis/FieldWriteCount.kt new file mode 100644 index 00000000..6e0c4f76 --- /dev/null +++ b/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 +} diff --git a/deob/src/main/java/dev/openrs2/deob/analysis/ThisInterpreter.kt b/deob/src/main/java/dev/openrs2/deob/analysis/ThisInterpreter.kt new file mode 100644 index 00000000..08aed826 --- /dev/null +++ b/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(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? { + 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) + } +} diff --git a/deob/src/main/java/dev/openrs2/deob/analysis/ThisValue.kt b/deob/src/main/java/dev/openrs2/deob/analysis/ThisValue.kt new file mode 100644 index 00000000..79b7e3af --- /dev/null +++ b/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 + } +} diff --git a/deob/src/main/java/dev/openrs2/deob/transform/FinalFieldTransformer.kt b/deob/src/main/java/dev/openrs2/deob/transform/FinalFieldTransformer.kt new file mode 100644 index 00000000..0aec72c0 --- /dev/null +++ b/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 + private val nonFinalFields = mutableSetOf>() + + 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 == "" || method.name == "" + val thisCall = if (constructor && method.name == "") { + method.instructions.filterIsInstance() + .any { it.opcode == Opcodes.INVOKESPECIAL && it.owner == clazz.name && it.name == "" } + } 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() + } +}