Track multiple values in IntInterpreter

This helps us catch a few more cases in DummyTransformer.

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<IntValue> {
-	private final Interpreter<BasicValue> 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<BasicValue> basicInterpreter = new BasicInterpreter();
+	private final ImmutableSet<Integer>[] parameters;
 
-	public IntInterpreter(Integer[] parameters) {
+	public IntInterpreter(ImmutableSet<Integer>[] parameters) {
 		super(Opcodes.ASM7);
 		this.parameters = parameters;
 	}
@@ -86,25 +90,36 @@ public final class IntInterpreter extends Interpreter<IntValue> {
 			return null;
 		}
 
-		if (value.isConstant()) {
+		if (value.isUnknown()) {
-			var v = value.getIntValue();
+			return IntValue.newUnknown(basicValue);
+		}
+
+		var set = ImmutableSet.<Integer>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<IntValue> {
 			return null;
 		}
 
-		if (value1.isConstant() && value2.isConstant()) {
+		if (value1.isUnknown() || value2.isUnknown()) {
-			var v1 = value1.getIntValue();
+			return IntValue.newUnknown(basicValue);
-			var v2 = value2.getIntValue();
+		}
 
-			switch (insn.getOpcode()) {
+		var set = ImmutableSet.<Integer>builder();
-			case Opcodes.IADD:
+
-				return IntValue.newConstant(basicValue, v1 + v2);
+		for (var v1 : value1.getIntValues()) {
-			case Opcodes.ISUB:
+			for (var v2 : value2.getIntValues()) {
-				return IntValue.newConstant(basicValue, v1 - v2);
+				switch (insn.getOpcode()) {
-			case Opcodes.IMUL:
+				case Opcodes.IADD:
-				return IntValue.newConstant(basicValue, v1 * v2);
+					set.add(v1 + v2);
-			case Opcodes.IDIV:
+					break;
-				if (v2 == 0) {
+				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<IntValue> {
 			return null;
 		}
 
-		if (value1.isConstant() && value2.isConstant() && value1.getIntValue() == value2.getIntValue()) {
+		if (value1.isUnknown() || value2.isUnknown()) {
-			return IntValue.newConstant(basicValue, value1.getIntValue());
+			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);
 	}
 }

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<Integer> 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<Integer> intValues;
 
-	private IntValue(BasicValue basicValue, Integer intValue) {
+	private IntValue(BasicValue basicValue, ImmutableSet<Integer> intValues) {
 		this.basicValue = basicValue;
-		this.intValue = intValue;
+		this.intValues = intValues;
 	}
 
 	public BasicValue getBasicValue() {
 		return basicValue;
 	}
 
-	public boolean isConstant() {
+	public boolean isUnknown() {
-		return intValue != null;
+		return intValues.isEmpty();
+	}
+
+	public boolean isSingleConstant() {
+		return intValues.size() == 1;
 	}
 
 	public int getIntValue() {
-		Preconditions.checkState(intValue != null);
+		Preconditions.checkState(isSingleConstant());
-		return intValue;
+		return intValues.iterator().next();
+	}
+
+	public ImmutableSet<Integer> 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();
 	}
 }

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<Integer> 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<Integer> values1, Set<Integer> 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<Integer> union(Collection<IntValue> intValues) {
+		var builder = ImmutableSet.<Integer>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<ArgRef, IntValue> argValues = HashMultimap.create();
-	private final Map<DisjointSet.Partition<MemberRef>, Integer[]> constArgs = new HashMap<>();
+	private final Map<DisjointSet.Partition<MemberRef>, ImmutableSet<Integer>[]> constArgs = new HashMap<>();
 	private DisjointSet<MemberRef> 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());
+				var result = evaluateUnaryBranch(insn.getOpcode(), value.getIntValues());
-				if (taken) {
+				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());
+				result = evaluateBinaryBranch(insn.getOpcode(), value1.getIntValues(), value2.getIntValues());
-				if (taken) {
+				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<Integer>[]) new ImmutableSet<?>[args];
 			for (int i = 0; i < args; i++) {
-				var values = argValues.get(new ArgRef(method, i));
+				parameters[i] = union(argValues.get(new ArgRef(method, i)));
-				if (values.size() != 1) {
-					continue;
-				}
-
-				var value = values.iterator().next();
-				if (value.isConstant()) {
-					parameters[i] = value.getIntValue();
-				}
 			}
 			constArgs.put(method, parameters);
 		}