From f6f810de2ef77f3f59d274d6bad5759774d26315 Mon Sep 17 00:00:00 2001 From: Graham Date: Sat, 12 Oct 2019 17:39:49 +0100 Subject: [PATCH] Remove mutually-recursive dummy method calls This is a little bit grim and probably not completely safe in all cases, but it works well enough on the client. Ideally I think I'd do it with a dominator tree calculated from a call graph aware of integer constants and conditional calls, but that's quite complicated (especially given how the existing code in the DummyArgTransformer works). --- .../deob/transform/DummyArgTransformer.java | 162 +++++++++++++++++- 1 file changed, 155 insertions(+), 7 deletions(-) diff --git a/deob/src/main/java/dev/openrs2/deob/transform/DummyArgTransformer.java b/deob/src/main/java/dev/openrs2/deob/transform/DummyArgTransformer.java index cd586d57d9..7f6b095fb6 100644 --- a/deob/src/main/java/dev/openrs2/deob/transform/DummyArgTransformer.java +++ b/deob/src/main/java/dev/openrs2/deob/transform/DummyArgTransformer.java @@ -6,10 +6,12 @@ import java.util.HashMap; import java.util.Map; import java.util.Set; +import com.google.common.base.MoreObjects; import com.google.common.base.Preconditions; import com.google.common.collect.HashMultimap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Multimap; +import dev.openrs2.asm.InsnMatcher; import dev.openrs2.asm.InsnNodeUtils; import dev.openrs2.asm.MemberRef; import dev.openrs2.asm.StackMetadata; @@ -27,6 +29,7 @@ import org.objectweb.asm.tree.ClassNode; import org.objectweb.asm.tree.JumpInsnNode; import org.objectweb.asm.tree.MethodInsnNode; import org.objectweb.asm.tree.MethodNode; +import org.objectweb.asm.tree.VarInsnNode; import org.objectweb.asm.tree.analysis.Analyzer; import org.objectweb.asm.tree.analysis.AnalyzerException; import org.slf4j.Logger; @@ -35,6 +38,34 @@ import org.slf4j.LoggerFactory; public final class DummyArgTransformer extends Transformer { private static final Logger logger = LoggerFactory.getLogger(DummyArgTransformer.class); + private static final InsnMatcher CONDITIONAL_CALL_MATCHER = InsnMatcher.compile("ILOAD (IFEQ | IFNE | (ICONST | BIPUSH | SIPUSH | LDC) (IF_ICMPEQ | IF_ICMPNE | IF_ICMPLT | IF_ICMPGE | IF_ICMPGT | IF_ICMPLE)) ALOAD? (ICONST | FCONST | DCONST | BIPUSH | SIPUSH | LDC | ACONST_NULL CHECKCAST)+ (INVOKEVIRTUAL | INVOKESTATIC | INVOKEINTERFACE)"); + + private static final class ConditionalCall { + private final int conditionVar, conditionOpcode; + private final Integer conditionValue; + private final DisjointSet.Partition method; + private final Integer[] constArgs; + + public ConditionalCall(int conditionVar, int conditionOpcode, Integer conditionValue, DisjointSet.Partition method, Integer[] constArgs) { + this.conditionVar = conditionVar; + this.conditionOpcode = conditionOpcode; + this.conditionValue = conditionValue; + this.method = method; + this.constArgs = constArgs; + } + + @Override + public String toString() { + return MoreObjects.toStringHelper(this) + .add("conditionVar", conditionVar) + .add("conditionOpcode", conditionOpcode) + .add("conditionValue", conditionValue) + .add("method", method) + .add("constArgs", constArgs) + .toString(); + } + } + private enum BranchResult { ALWAYS_TAKEN, NEVER_TAKEN, @@ -115,7 +146,56 @@ public final class DummyArgTransformer extends Transformer { } } - private static ImmutableSet union(DisjointSet.Partition method, Collection intValues) { + private final Multimap argValues = HashMultimap.create(); + private final Multimap, ConditionalCall> conditionalCalls = HashMultimap.create(); + private final Map, ImmutableSet[]> constArgs = new HashMap<>(); + private DisjointSet inheritedMethodSets; + private int branchesSimplified, constantsInlined; + + private boolean isMutuallyRecursiveDummy(DisjointSet.Partition method, int arg, DisjointSet.Partition source, int value) { + for (var sourceToMethodCall : conditionalCalls.get(source)) { + if (!sourceToMethodCall.method.equals(method)) { + continue; + } + + for (var methodToSourceCall : conditionalCalls.get(method)) { + if (!methodToSourceCall.method.equals(source)) { + continue; + } + + if (methodToSourceCall.conditionVar != arg) { + continue; + } + + boolean taken; + if (methodToSourceCall.conditionValue != null) { + taken = evaluateBinaryBranch(methodToSourceCall.conditionOpcode, value, methodToSourceCall.conditionValue); + } else { + taken = evaluateUnaryBranch(methodToSourceCall.conditionOpcode, value); + } + + if (taken) { + continue; + } + + if (sourceToMethodCall.conditionValue != null) { + taken = evaluateBinaryBranch(sourceToMethodCall.conditionOpcode, methodToSourceCall.constArgs[sourceToMethodCall.conditionVar], sourceToMethodCall.conditionValue); + } else { + taken = evaluateUnaryBranch(sourceToMethodCall.conditionOpcode, methodToSourceCall.constArgs[sourceToMethodCall.conditionVar]); + } + + if (taken) { + continue; + } + + return true; + } + } + + return false; + } + + private ImmutableSet union(DisjointSet.Partition method, int arg, Collection intValues) { var builder = ImmutableSet.builder(); for (var value : intValues) { @@ -129,6 +209,12 @@ public final class DummyArgTransformer extends Transformer { continue; } + if (intValue.isSingleConstant()) { + if (isMutuallyRecursiveDummy(method, arg, source, intValue.getIntValue())) { + continue; + } + } + builder.addAll(intValue.getIntValues()); } @@ -140,11 +226,6 @@ public final class DummyArgTransformer extends Transformer { return set; } - private final Multimap argValues = HashMultimap.create(); - private final Map, ImmutableSet[]> constArgs = new HashMap<>(); - private DisjointSet inheritedMethodSets; - private int branchesSimplified, constantsInlined; - @Override protected void preTransform(ClassPath classPath) { inheritedMethodSets = classPath.createInheritedMethodSets(); @@ -155,11 +236,78 @@ public final class DummyArgTransformer extends Transformer { @Override protected void prePass(ClassPath classPath) { argValues.clear(); + conditionalCalls.clear(); } @Override protected boolean transformCode(ClassPath classPath, Library library, ClassNode clazz, MethodNode method) throws AnalyzerException { var parentMethod = inheritedMethodSets.get(new MemberRef(clazz.name, method.name, method.desc)); + + var stores = new boolean[method.maxLocals]; + + for (var it = method.instructions.iterator(); it.hasNext(); ) { + var insn = it.next(); + + var opcode = insn.getOpcode(); + if (opcode != Opcodes.ISTORE) { + continue; + } + + var store = (VarInsnNode) insn; + stores[store.var] = true; + } + + CONDITIONAL_CALL_MATCHER.match(method).forEach(match -> { + var matchIndex = 0; + var load = (VarInsnNode) match.get(matchIndex++); + if (stores[load.var]) { + return; + } + + var callerSlots = Type.getArgumentsAndReturnSizes(method.desc) >> 2; + if ((method.access & Opcodes.ACC_STATIC) != 0) { + callerSlots++; + } + + if (load.var >= callerSlots) { + return; + } + + Integer conditionValue; + var conditionOpcode = match.get(matchIndex).getOpcode(); + if (conditionOpcode == Opcodes.IFEQ || conditionOpcode == Opcodes.IFNE) { + conditionValue = null; + matchIndex++; + } else { + conditionValue = InsnNodeUtils.getIntConstant(match.get(matchIndex++)); + conditionOpcode = match.get(matchIndex++).getOpcode(); + } + + var invoke = (MethodInsnNode) match.get(match.size() - 1); + + var invokeArgTypes = Type.getArgumentTypes(invoke.desc).length; + var constArgs = new Integer[invokeArgTypes]; + + if (invoke.getOpcode() != Opcodes.INVOKESTATIC) { + matchIndex++; + } + + for (int i = 0; i < constArgs.length; i++) { + var insn = match.get(matchIndex++); + if (insn.getOpcode() == Opcodes.ACONST_NULL) { + matchIndex++; + } else if (InsnNodeUtils.isIntConstant(insn)) { + constArgs[i] = InsnNodeUtils.getIntConstant(insn); + } + } + + var callee = inheritedMethodSets.get(new MemberRef(invoke.owner, invoke.name, invoke.desc)); + if (callee == null) { + return; + } + conditionalCalls.put(parentMethod, new ConditionalCall(load.var, conditionOpcode, conditionValue, callee, constArgs)); + }); + var parameters = constArgs.get(parentMethod); var analyzer = new Analyzer<>(new IntInterpreter(parameters)); @@ -303,7 +451,7 @@ public final class DummyArgTransformer extends Transformer { @SuppressWarnings("unchecked") var parameters = (ImmutableSet[]) new ImmutableSet[args]; for (var i = 0; i < args; i++) { - var parameter = union(method, argValues.get(new ArgRef(method, i))); + var parameter = union(method, i, argValues.get(new ArgRef(method, i))); if (parameter != null) { allUnknown = false; }