From 766b544fc111487ce1fa5d8c303d58b8ad0b44d3 Mon Sep 17 00:00:00 2001 From: Graham Date: Sun, 11 Aug 2019 09:46:35 +0100 Subject: [PATCH] Track multiple values in IntInterpreter This helps us catch a few more cases in DummyTransformer. --- .../openrs2/deob/analysis/IntInterpreter.java | 128 ++++++++++++------ .../dev/openrs2/deob/analysis/IntValue.java | 39 ++++-- .../deob/transform/DummyTransformer.java | 116 ++++++++++++---- 3 files changed, 202 insertions(+), 81 deletions(-) diff --git a/deob/src/main/java/dev/openrs2/deob/analysis/IntInterpreter.java b/deob/src/main/java/dev/openrs2/deob/analysis/IntInterpreter.java index 9cf47f2d..be16536f 100644 --- a/deob/src/main/java/dev/openrs2/deob/analysis/IntInterpreter.java +++ b/deob/src/main/java/dev/openrs2/deob/analysis/IntInterpreter.java @@ -3,6 +3,8 @@ package dev.openrs2.deob.analysis; import java.util.List; import java.util.stream.Collectors; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Sets; import dev.openrs2.asm.InsnNodeUtils; import org.objectweb.asm.Opcodes; import org.objectweb.asm.Type; @@ -14,11 +16,13 @@ import org.objectweb.asm.tree.analysis.BasicValue; import org.objectweb.asm.tree.analysis.Interpreter; public final class IntInterpreter extends Interpreter { - private final Interpreter basicInterpreter = new BasicInterpreter(); + // TODO(gpe): this is fairly arbitrary and will need tweaking + private static final int MAX_TRACKED_VALUES = 8; - private final Integer[] parameters; + private final Interpreter basicInterpreter = new BasicInterpreter(); + private final ImmutableSet[] parameters; - public IntInterpreter(Integer[] parameters) { + public IntInterpreter(ImmutableSet[] parameters) { super(Opcodes.ASM7); this.parameters = parameters; } @@ -86,25 +90,36 @@ public final class IntInterpreter extends Interpreter { return null; } - if (value.isConstant()) { - var v = value.getIntValue(); + if (value.isUnknown()) { + return IntValue.newUnknown(basicValue); + } + + var set = ImmutableSet.builder(); + for (var v : value.getIntValues()) { switch (insn.getOpcode()) { case Opcodes.INEG: - return IntValue.newConstant(basicValue, -v); + set.add(-v); + break; case Opcodes.IINC: var iinc = (IincInsnNode) insn; - return IntValue.newConstant(basicValue, v + iinc.incr); + set.add(v + iinc.incr); + break; case Opcodes.I2B: - return IntValue.newConstant(basicValue, (byte) v); + set.add((int) (byte) (int) v); + break; case Opcodes.I2C: - return IntValue.newConstant(basicValue, (char) v); + set.add((int) (char) (int) v); + break; case Opcodes.I2S: - return IntValue.newConstant(basicValue, (short) v); + set.add((int) (short) (int) v); + break; + default: + return IntValue.newUnknown(basicValue); } } - return IntValue.newUnknown(basicValue); + return IntValue.newConstant(basicValue, set.build()); } @Override @@ -114,43 +129,61 @@ public final class IntInterpreter extends Interpreter { return null; } - if (value1.isConstant() && value2.isConstant()) { - var v1 = value1.getIntValue(); - var v2 = value2.getIntValue(); + if (value1.isUnknown() || value2.isUnknown()) { + return IntValue.newUnknown(basicValue); + } - switch (insn.getOpcode()) { - case Opcodes.IADD: - return IntValue.newConstant(basicValue, v1 + v2); - case Opcodes.ISUB: - return IntValue.newConstant(basicValue, v1 - v2); - case Opcodes.IMUL: - return IntValue.newConstant(basicValue, v1 * v2); - case Opcodes.IDIV: - if (v2 == 0) { + var set = ImmutableSet.builder(); + + for (var v1 : value1.getIntValues()) { + for (var v2 : value2.getIntValues()) { + switch (insn.getOpcode()) { + case Opcodes.IADD: + set.add(v1 + v2); + break; + case Opcodes.ISUB: + set.add(v1 - v2); + break; + case Opcodes.IMUL: + set.add(v1 * v2); + break; + case Opcodes.IDIV: + if (v2 == 0) { + return IntValue.newUnknown(basicValue); + } + set.add(v1 / v2); + break; + case Opcodes.IREM: + if (v2 == 0) { + return IntValue.newUnknown(basicValue); + } + set.add(v1 % v2); + break; + case Opcodes.ISHL: + set.add(v1 << v2); + break; + case Opcodes.ISHR: + set.add(v1 >> v2); + break; + case Opcodes.IUSHR: + set.add(v1 >>> v2); + break; + case Opcodes.IAND: + set.add(v1 & v2); + break; + case Opcodes.IOR: + set.add(v1 | v2); + break; + case Opcodes.IXOR: + set.add(v1 ^ v2); + break; + default: return IntValue.newUnknown(basicValue); } - return IntValue.newConstant(basicValue, v1 / v2); - case Opcodes.IREM: - if (v2 == 0) { - return IntValue.newUnknown(basicValue); - } - return IntValue.newConstant(basicValue, v1 % v2); - case Opcodes.ISHL: - return IntValue.newConstant(basicValue, v1 << v2); - case Opcodes.ISHR: - return IntValue.newConstant(basicValue, v1 >> v2); - case Opcodes.IUSHR: - return IntValue.newConstant(basicValue, v1 >>> v2); - case Opcodes.IAND: - return IntValue.newConstant(basicValue, v1 & v2); - case Opcodes.IOR: - return IntValue.newConstant(basicValue, v1 | v2); - case Opcodes.IXOR: - return IntValue.newConstant(basicValue, v1 ^ v2); } } - return IntValue.newUnknown(basicValue); + return IntValue.newConstant(basicValue, set.build()); } @Override @@ -188,10 +221,15 @@ public final class IntInterpreter extends Interpreter { return null; } - if (value1.isConstant() && value2.isConstant() && value1.getIntValue() == value2.getIntValue()) { - return IntValue.newConstant(basicValue, value1.getIntValue()); + if (value1.isUnknown() || value2.isUnknown()) { + return IntValue.newUnknown(basicValue); } - return IntValue.newUnknown(basicValue); + var set = ImmutableSet.copyOf(Sets.union(value1.getIntValues(), value2.getIntValues())); + if (set.size() > MAX_TRACKED_VALUES) { + return IntValue.newUnknown(basicValue); + } + + return IntValue.newConstant(basicValue, set); } } diff --git a/deob/src/main/java/dev/openrs2/deob/analysis/IntValue.java b/deob/src/main/java/dev/openrs2/deob/analysis/IntValue.java index 946cd626..e539d369 100644 --- a/deob/src/main/java/dev/openrs2/deob/analysis/IntValue.java +++ b/deob/src/main/java/dev/openrs2/deob/analysis/IntValue.java @@ -4,39 +4,54 @@ import java.util.Objects; import com.google.common.base.MoreObjects; import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableSet; import org.objectweb.asm.tree.analysis.BasicValue; import org.objectweb.asm.tree.analysis.Value; public final class IntValue implements Value { public static IntValue newConstant(BasicValue basicValue, int intValue) { + return newConstant(basicValue, ImmutableSet.of(intValue)); + } + + public static IntValue newConstant(BasicValue basicValue, ImmutableSet intValues) { Preconditions.checkArgument(basicValue == BasicValue.INT_VALUE); - return new IntValue(basicValue, intValue); + Preconditions.checkArgument(!intValues.isEmpty()); + return new IntValue(basicValue, intValues); } public static IntValue newUnknown(BasicValue basicValue) { Preconditions.checkNotNull(basicValue); - return new IntValue(basicValue, null); + return new IntValue(basicValue, ImmutableSet.of()); } private final BasicValue basicValue; - private final Integer intValue; + private final ImmutableSet intValues; - private IntValue(BasicValue basicValue, Integer intValue) { + private IntValue(BasicValue basicValue, ImmutableSet intValues) { this.basicValue = basicValue; - this.intValue = intValue; + this.intValues = intValues; } public BasicValue getBasicValue() { return basicValue; } - public boolean isConstant() { - return intValue != null; + public boolean isUnknown() { + return intValues.isEmpty(); + } + + public boolean isSingleConstant() { + return intValues.size() == 1; } public int getIntValue() { - Preconditions.checkState(intValue != null); - return intValue; + Preconditions.checkState(isSingleConstant()); + return intValues.iterator().next(); + } + + public ImmutableSet getIntValues() { + Preconditions.checkArgument(!isUnknown()); + return intValues; } @Override @@ -54,19 +69,19 @@ public final class IntValue implements Value { } IntValue intValue1 = (IntValue) o; return basicValue.equals(intValue1.basicValue) && - Objects.equals(intValue, intValue1.intValue); + Objects.equals(intValues, intValue1.intValues); } @Override public int hashCode() { - return Objects.hash(basicValue, intValue); + return Objects.hash(basicValue, intValues); } @Override public String toString() { return MoreObjects.toStringHelper(this) .add("basicValue", basicValue) - .add("intValue", intValue) + .add("intValues", intValues) .toString(); } } diff --git a/deob/src/main/java/dev/openrs2/deob/transform/DummyTransformer.java b/deob/src/main/java/dev/openrs2/deob/transform/DummyTransformer.java index 623b5f7f..970462bd 100644 --- a/deob/src/main/java/dev/openrs2/deob/transform/DummyTransformer.java +++ b/deob/src/main/java/dev/openrs2/deob/transform/DummyTransformer.java @@ -1,11 +1,15 @@ package dev.openrs2.deob.transform; import java.util.ArrayList; +import java.util.Collection; import java.util.HashMap; import java.util.Map; import java.util.Objects; +import java.util.Set; +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.InsnNodeUtils; import dev.openrs2.asm.MemberRef; @@ -58,6 +62,39 @@ public final class DummyTransformer extends Transformer { } } + private enum BranchResult { + ALWAYS_TAKEN, + NEVER_TAKEN, + UNKNOWN; + + public static BranchResult fromTakenNotTaken(int taken, int notTaken) { + Preconditions.checkArgument(taken != 0 || notTaken != 0); + + if (taken == 0) { + return NEVER_TAKEN; + } else if (notTaken == 0) { + return ALWAYS_TAKEN; + } else { + return UNKNOWN; + } + } + } + + private static BranchResult evaluateUnaryBranch(int opcode, Set values) { + Preconditions.checkArgument(!values.isEmpty()); + + int taken = 0, notTaken = 0; + for (var v : values) { + if (evaluateUnaryBranch(opcode, v)) { + taken++; + } else { + notTaken++; + } + } + + return BranchResult.fromTakenNotTaken(taken, notTaken); + } + private static boolean evaluateUnaryBranch(int opcode, int value) { switch (opcode) { case Opcodes.IFEQ: @@ -69,6 +106,23 @@ public final class DummyTransformer extends Transformer { } } + private static BranchResult evaluateBinaryBranch(int opcode, Set values1, Set values2) { + Preconditions.checkArgument(!values1.isEmpty() && !values2.isEmpty()); + + int taken = 0, notTaken = 0; + for (var v1 : values1) { + for (var v2 : values2) { + if (evaluateBinaryBranch(opcode, v1, v2)) { + taken++; + } else { + notTaken++; + } + } + } + + return BranchResult.fromTakenNotTaken(taken, notTaken); + } + private static boolean evaluateBinaryBranch(int opcode, int value1, int value2) { switch (opcode) { case Opcodes.IF_ICMPEQ: @@ -88,8 +142,27 @@ public final class DummyTransformer extends Transformer { } } + private static ImmutableSet union(Collection intValues) { + var builder = ImmutableSet.builder(); + + for (var value : intValues) { + if (value.isUnknown()) { + return null; + } + + builder.addAll(value.getIntValues()); + } + + var set = builder.build(); + if (set.isEmpty()) { + return null; + } + + return set; + } + private final Multimap argValues = HashMultimap.create(); - private final Map, Integer[]> constArgs = new HashMap<>(); + private final Map, ImmutableSet[]> constArgs = new HashMap<>(); private DisjointSet inheritedMethodSets; private int loadsInlined, branchesSimplified; @@ -153,7 +226,7 @@ public final class DummyTransformer extends Transformer { case Opcodes.ILOAD: var iload = (VarInsnNode) insn; var value = frame.getLocal(iload.var); - if (value.isConstant()) { + if (value.isSingleConstant()) { method.instructions.set(insn, InsnNodeUtils.createIntConstant(value.getIntValue())); loadsInlined++; changed = true; @@ -162,15 +235,18 @@ public final class DummyTransformer extends Transformer { case Opcodes.IFEQ: case Opcodes.IFNE: value = frame.getStack(stackSize - 1); - if (!value.isConstant()) { + if (value.isUnknown()) { continue; } - var taken = evaluateUnaryBranch(insn.getOpcode(), value.getIntValue()); - if (taken) { + var result = evaluateUnaryBranch(insn.getOpcode(), value.getIntValues()); + switch (result) { + case ALWAYS_TAKEN: alwaysTakenUnaryBranches.add((JumpInsnNode) insn); - } else { + break; + case NEVER_TAKEN: neverTakenUnaryBranches.add((JumpInsnNode) insn); + break; } break; case Opcodes.IF_ICMPEQ: @@ -180,20 +256,19 @@ public final class DummyTransformer extends Transformer { case Opcodes.IF_ICMPGT: case Opcodes.IF_ICMPLE: var value1 = frame.getStack(stackSize - 2); - if (!value1.isConstant()) { - continue; - } - var value2 = frame.getStack(stackSize - 1); - if (!value2.isConstant()) { + if (value1.isUnknown() || value2.isUnknown()) { continue; } - taken = evaluateBinaryBranch(insn.getOpcode(), value1.getIntValue(), value2.getIntValue()); - if (taken) { + result = evaluateBinaryBranch(insn.getOpcode(), value1.getIntValues(), value2.getIntValues()); + switch (result) { + case ALWAYS_TAKEN: alwaysTakenBinaryBranches.add((JumpInsnNode) insn); - } else { + break; + case NEVER_TAKEN: neverTakenBinaryBranches.add((JumpInsnNode) insn); + break; } break; } @@ -235,17 +310,10 @@ public final class DummyTransformer extends Transformer { for (var method : inheritedMethodSets) { var args = (Type.getArgumentsAndReturnSizes(method.iterator().next().getDesc()) >> 2) - 1; - var parameters = new Integer[args]; + @SuppressWarnings("unchecked") + var parameters = (ImmutableSet[]) new ImmutableSet[args]; for (int i = 0; i < args; i++) { - var values = argValues.get(new ArgRef(method, i)); - if (values.size() != 1) { - continue; - } - - var value = values.iterator().next(); - if (value.isConstant()) { - parameters[i] = value.getIntValue(); - } + parameters[i] = union(argValues.get(new ArgRef(method, i))); } constArgs.put(method, parameters); }