fix methodhooks

src/main/java/com/openosrs/injector/InjectUtil.java

@@ -5,6 +5,7 @@ import static com.openosrs.injector.rsapi.RSApi.*;
 import static com.openosrs.injector.rsapi.RSApi.API_BASE;
 import com.openosrs.injector.rsapi.RSApiClass;
 import com.openosrs.injector.rsapi.RSApiMethod;
+import java.util.List;
 import java.util.stream.Collectors;
 import net.runelite.asm.Annotated;
 import net.runelite.asm.ClassFile;
@@ -21,6 +22,8 @@ import net.runelite.asm.attributes.code.instructions.ALoad;
 import net.runelite.asm.attributes.code.instructions.DLoad;
 import net.runelite.asm.attributes.code.instructions.FLoad;
 import net.runelite.asm.attributes.code.instructions.ILoad;
+import net.runelite.asm.attributes.code.instructions.InvokeStatic;
+import net.runelite.asm.attributes.code.instructions.InvokeVirtual;
 import net.runelite.asm.attributes.code.instructions.LLoad;
 import net.runelite.asm.attributes.code.instructions.Return;
 import net.runelite.asm.attributes.code.instructions.VReturn;
@@ -59,11 +62,7 @@ public class InjectUtil
 
 		if (classHint != null)
 		{
-			ClassFile clazz = deob.findClass(classHint);
+			ClassFile clazz = findClassOrThrow(deob, classHint);
-			if (clazz == null)
-			{
-				throw new Injexception("Hint class " + classHint + " doesn't exist");
-			}
 
 			if (sig == null)
 			{
@@ -73,15 +72,13 @@ public class InjectUtil
 			{
 				method = clazz.findStaticMethod(name, sig);
 			}
+
+			if (method != null)
+				return data.toVanilla(method);
 		}
 
 		for (ClassFile clazz : deob)
 		{
-			if (method != null)
-			{
-				break;
-			}
-
 			if (sig == null)
 			{
 				method = clazz.findStaticMethod(name);
@@ -90,14 +87,21 @@ public class InjectUtil
 			{
 				method = clazz.findStaticMethod(name, sig);
 			}
+
+			if (method != null)
+				return data.toVanilla(method);
 		}
 
-		if (method == null)
+		throw new Injexception("Static method " + name + " doesn't exist");
-		{
+	}
-			throw new Injexception("Static method " + name + " doesn't exist");
-		}
 
-		return data.toVanilla(method);
+	private static ClassFile findClassOrThrow(ClassGroup group, String name) throws Injexception
+	{
+		ClassFile clazz = group.findClass(name);
+		if (clazz == null)
+			throw new Injexception("Hint class " + name + " doesn't exist");
+
+		return clazz;
 	}
 
 	/**
@@ -113,6 +117,37 @@ public class InjectUtil
 		return findStaticMethod(data, pool.getName(), pool.getClazz().getName(), pool.getType());
 	}
 
+	public static Method findMethodWithArgs(InjectData data, String name, String hintClass, Signature sig) throws Injexception
+	{
+		final ClassGroup deob = data.getDeobfuscated();
+		if (hintClass != null)
+		{
+			ClassFile clazz = findClassOrThrow(deob, hintClass);
+			Method method = clazz.findStaticMethod(name);
+
+			if (method != null && argsMatch(sig, method.getDescriptor()))
+				return data.toVanilla(method);
+		}
+
+		for (ClassFile c : deob)
+			for (Method m : c.getMethods())
+				if (m.getName().equals(name) && argsMatch(sig, m.getDescriptor()))
+					return data.toVanilla(m);
+
+		throw new Injexception("Method called " + name + " with args matching " + sig + " doesn't exist");
+	}
+
+	public static Method findMethodWithArgsDeep(InjectData data, ClassFile clazz, String name, Signature sig) throws Injexception
+	{
+		do
+			for (Method m : clazz.getMethods())
+				if (m.getName().equals(name) && argsMatch(sig, m.getDescriptor()))
+					return data.toVanilla(m);
+		while ((clazz = clazz.getParent()) != null);
+
+		throw new Injexception("Method called " + name + " with args matching " + sig + " doesn't exist");
+	}
+
 	/**
 	 * Fail-fast implementation of ClassGroup.findStaticMethod
 	 */
@@ -120,9 +155,8 @@ public class InjectUtil
 	{
 		Method m = group.findStaticMethod(name);
 		if (m == null)
-		{
 			throw new Injexception(String.format("Method %s couldn't be found", name));
-		}
+
 		return m;
 	}
 
@@ -175,7 +209,7 @@ public class InjectUtil
 		for (ClassFile clazz : group)
 		{
 			Field f = clazz.findField(name);
-			if (f != null)
+			if (f != null && f.isStatic())
 			{
 				return f;
 			}
@@ -200,11 +234,7 @@ public class InjectUtil
 
 		if (classHint != null)
 		{
-			ClassFile clazz = deob.findClass(classHint);
+			ClassFile clazz = findClassOrThrow(deob, classHint);
-			if (clazz == null)
-			{
-				throw new Injexception("Hint class " + classHint + " doesn't exist");
-			}
 
 			if (type == null)
 			{
@@ -214,15 +244,15 @@ public class InjectUtil
 			{
 				field = clazz.findField(name, type);
 			}
+
+			if (field != null)
+			{
+				return data.toVanilla(field);
+			}
 		}
 
 		for (ClassFile clazz : deob)
 		{
-			if (field != null)
-			{
-				break;
-			}
-
 			if (type == null)
 			{
 				field = clazz.findField(name);
@@ -231,14 +261,14 @@ public class InjectUtil
 			{
 				field = clazz.findField(name, type);
 			}
+
+			if (field != null)
+			{
+				return data.toVanilla(field);
+			}
 		}
 
-		if (field == null)
+		throw new Injexception(String.format("Static field %s doesn't exist", (type != null ? type + " " : "") + name));
-		{
-			throw new Injexception(String.format("Static field %s doesn't exist", (type != null ? type + " " : "") + name));
-		}
-
-		return data.toVanilla(field);
 	}
 
 	/**
@@ -282,11 +312,7 @@ public class InjectUtil
 		Field field;
 		if (hintClass != null)
 		{
-			ClassFile clazz = group.findClass(hintClass);
+			ClassFile clazz = findClassOrThrow(group, hintClass);
-			if (clazz == null)
-			{
-				throw new Injexception("Hint class " + hintClass + " doesn't exist");
-			}
 
 			field = clazz.findField(name);
 			if (field != null)
@@ -360,7 +386,7 @@ public class InjectUtil
 			}
 			while (changed);
 
-			return apiToDeob(data, new Type(highestKnown.getName()));
+			return apiToDeob(data, Type.getType(highestKnown.getName(), api.getDimensions()));
 		}
 
 		return api;
@@ -387,6 +413,27 @@ public class InjectUtil
 		return deobSig.equals(apiToDeob(data, apiSig));
 	}
 
+	public static boolean argsMatch(Signature a, Signature b)
+	{
+		List<Type> aa = a.getArguments();
+		List<Type> bb = b.getArguments();
+
+		if (aa.size() != bb.size())
+		{
+			return false;
+		}
+
+		for (int i = 0; i < aa.size(); i++)
+		{
+			if (!aa.get(i).equals(bb.get(i)))
+			{
+				return false;
+			}
+		}
+
+		return true;
+	}
+
 	/**
 	 * Gets the obfuscated name from something's annotations.
 	 *
@@ -466,4 +513,16 @@ public class InjectUtil
 				throw new IllegalStateException("Unknown type");
 		}
 	}
+
+	public static Instruction createInvokeFor(Instructions instructions, net.runelite.asm.pool.Method method, boolean isStatic)
+	{
+		if (isStatic)
+		{
+			return new InvokeStatic(instructions, method);
+		}
+		else
+		{
+			return new InvokeVirtual(instructions, method);
+		}
+	}
 }

src/main/java/com/openosrs/injector/Injection.java

@@ -68,7 +68,7 @@ public class Injection extends InjectData implements InjectTaskHandler
 
 		// inject(new HidePlayerAttacks(this));
 
-		new InjectorValidator(this).validate();
+		validate(new InjectorValidator(this));
 	}
 
 	public void save(File outputJar) throws IOException

src/main/java/com/openosrs/injector/injectors/InjectHook.java

@@ -26,8 +26,6 @@ import net.runelite.asm.attributes.code.instructions.ArrayStore;
 import net.runelite.asm.attributes.code.instructions.CheckCast;
 import net.runelite.asm.attributes.code.instructions.Dup;
 import net.runelite.asm.attributes.code.instructions.IMul;
-import net.runelite.asm.attributes.code.instructions.InvokeStatic;
-import net.runelite.asm.attributes.code.instructions.InvokeVirtual;
 import net.runelite.asm.attributes.code.instructions.LDC;
 import net.runelite.asm.attributes.code.instructions.LMul;
 import net.runelite.asm.attributes.code.instructions.PutField;
@@ -41,7 +39,6 @@ import net.runelite.deob.DeobAnnotations;
 
 public class InjectHook extends AbstractInjector
 {
-	private static final Signature HOOK_SIG = new Signature("(I)V");
 	private static final String CLINIT = "<clinit>";
 	private static final Type FIELDHOOK = new Type("Lnet/runelite/api/mixins/FieldHook;");
 
@@ -310,7 +307,7 @@ public class InjectHook extends AbstractInjector
 			idx = recursivelyPush(ins, idx, index);
 		}
 
-		Instruction invoke = getInvokeFor(ins, hookInfo, signature);
+		Instruction invoke = hookInfo.getInvoke(ins);
 		ins.getInstructions().add(idx++, invoke);
 	}
 
@@ -338,9 +335,7 @@ public class InjectHook extends AbstractInjector
 		if (!hookInfo.method.isStatic())
 		{
 			if (objectPusher == null)
-			{
 				throw new Injexception("Null object pusher");
-			}
 
 			idx = recursivelyPush(ins, idx, objectPusher);
 		}
@@ -354,41 +349,29 @@ public class InjectHook extends AbstractInjector
 			ins.getInstructions().add(idx++, new LDC(ins, -1));
 		}
 
-		Instruction invoke = getInvokeFor(ins, hookInfo);
+		Instruction invoke = hookInfo.getInvoke(ins);
 		ins.getInstructions().add(idx++, invoke);
 	}
 
-	private Instruction getInvokeFor(Instructions ins, HookInfo hook)
-	{
-		return getInvokeFor(ins, hook, HOOK_SIG);
-	}
-
-	private Instruction getInvokeFor(Instructions ins, HookInfo hook, Signature sig) {
-		if (hook.method.isStatic()) {
-			return new InvokeStatic(ins,
-				new net.runelite.asm.pool.Method(
-					hook.targetClass,
-					hook.method.getName(),
-					sig
-				)
-			);
-		} else {
-			return new InvokeVirtual(ins,
-				new net.runelite.asm.pool.Method(
-					hook.targetClass,
-					hook.method.getName(),
-					sig
-				)
-			);
-		}
-	}
-
 	@AllArgsConstructor
 	static class HookInfo
 	{
-		Class targetClass;
+		final Class targetClass;
-		Method method;
+		final Method method;
-		boolean before;
+		final boolean before;
-		Number getter;
+		final Number getter;
+
+		Instruction getInvoke(Instructions instructions)
+		{
+			return InjectUtil.createInvokeFor(
+				instructions,
+				new net.runelite.asm.pool.Method(
+					targetClass,
+					method.getName(),
+					method.getDescriptor()
+				),
+				method.isStatic()
+			);
+		}
 	}
 }

src/main/java/com/openosrs/injector/injectors/InjectHookMethod.java

@@ -3,31 +3,23 @@ package com.openosrs.injector.injectors;
 import com.openosrs.injector.InjectUtil;
 import com.openosrs.injector.Injexception;
 import com.openosrs.injector.injection.InjectData;
-import com.openosrs.injector.rsapi.RSApi;
-import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.List;
+import java.util.ListIterator;
 import java.util.Map;
-import java.util.stream.Collectors;
 import javax.inject.Provider;
 import net.runelite.asm.ClassFile;
 import net.runelite.asm.Method;
 import net.runelite.asm.Type;
-import net.runelite.asm.attributes.Code;
 import net.runelite.asm.attributes.annotation.Annotation;
 import net.runelite.asm.attributes.code.Instruction;
-import net.runelite.asm.attributes.code.InstructionType;
 import net.runelite.asm.attributes.code.Instructions;
-import net.runelite.asm.attributes.code.instruction.types.InvokeInstruction;
 import net.runelite.asm.attributes.code.instruction.types.ReturnInstruction;
 import net.runelite.asm.attributes.code.instructions.ALoad;
-import net.runelite.asm.attributes.code.instructions.InvokeStatic;
+import net.runelite.asm.attributes.code.instructions.InvokeSpecial;
-import net.runelite.asm.attributes.code.instructions.InvokeVirtual;
 import net.runelite.asm.signature.Signature;
 
 public class InjectHookMethod extends AbstractInjector
 {
-	private static final String HOOKS = "net/runelite/client/callback/Hooks";
 	private static final Type METHODHOOK = new Type("Lnet/runelite/api/mixins/MethodHook;");
 	private final Map<Provider<ClassFile>, List<ClassFile>> mixinTargets;
 
@@ -64,193 +56,106 @@ public class InjectHookMethod extends AbstractInjector
 					continue;
 				}
 
+				if (!mixinMethod.getDescriptor().isVoid())
+					throw new Injexception("Method hook " + mixinMethod.getPoolMethod() + " doesn't have void return type");
+
 				final String hookName = methodHook.getElement().getString();
 				final boolean end = methodHook.getElements().size() == 2 && methodHook.getElements().get(1).getValue().equals(true);
 
 				final ClassFile deobTarget = inject.toDeob(targetClass.getName());
-				final Method deobMethod;
+				final Signature deobSig = InjectUtil.apiToDeob(inject, mixinMethod.getDescriptor());
 
+				final Method targetMethod;
 				if (mixinMethod.isStatic())
 				{
-					deobMethod = InjectUtil.findStaticMethod(deobTarget.getGroup(), hookName);
+					targetMethod = InjectUtil.findMethodWithArgs(inject, hookName, deobTarget.getName(), deobSig); // , deobSig);
 				}
 				else
 				{
-					deobMethod = InjectUtil.findMethodDeep(deobTarget, hookName);
+					targetMethod = InjectUtil.findMethodWithArgsDeep(inject, deobTarget, hookName, deobSig);
 				}
 
-				assert mixinMethod.isStatic() == deobMethod.isStatic() : "Mixin method isn't static but deob has a static method named the same as the hook, and I was too lazy to do something about this bug";
+				assert mixinMethod.isStatic() == targetMethod.isStatic() : "Mixin method isn't static but deob has a static method named the same as the hook, and I was too lazy to do something about this bug";
 
-				inject(mixinMethod, deobMethod, hookName, end);
+				final net.runelite.asm.pool.Method hookMethod = new net.runelite.asm.pool.Method(
+					targetClass.getPoolClass(),
+					mixinMethod.getName(),
+					mixinMethod.getDescriptor()
+				);
+
+				inject(targetMethod, hookMethod, end);
+
+				log.debug("Injected method hook {} in {}", hookMethod, targetMethod);
 				++injected;
 			}
 		}
 	}
 
-	private void inject(Method mixinMethod, Method targetMethod, String name, boolean end) throws Injexception
+	private void inject(final Method method, final net.runelite.asm.pool.Method hookMethod, boolean end)
 	{
-		// Method is hooked
+		final Instructions ins = method.getCode().getInstructions();
-		// Find equivalent method in vanilla, and insert callback at the beginning
-		ClassFile deobClass = targetMethod.getClassFile();
-		String obfuscatedMethodName = InjectUtil.getObfuscatedName(targetMethod);
-		String obfuscatedClassName = InjectUtil.getObfuscatedName(deobClass);
-
-		assert obfuscatedClassName != null : "hook on method in class with no obfuscated name";
-		assert obfuscatedMethodName != null : "hook on method with no obfuscated name";
-
-		Signature obfuscatedSignature = targetMethod.getObfuscatedSignature();
-
-		ClassFile vanillaClass = inject.toVanilla(deobClass);
-		Method vanillaMethod = vanillaClass.findMethod(obfuscatedMethodName, obfuscatedSignature);
-		assert targetMethod.isStatic() == vanillaMethod.isStatic();
-
-		// Insert instructions at beginning of method
-		injectHookMethod(mixinMethod, name, end, targetMethod, vanillaMethod, false);
-	}
-
-	private void injectHookMethod(Method hookMethod, String hookName, boolean end, Method deobMethod, Method vanillaMethod, boolean useHooks) throws Injexception
-	{
-		Code code = vanillaMethod.getCode();
-		if (code == null)
-		{
-			log.warn(vanillaMethod + " code is null");
-			return;
-		}
-
-		Instructions instructions = code.getInstructions();
-
-		Signature.Builder builder = new Signature.Builder()
-			.setReturnType(Type.VOID); // Hooks always return void
-
-		for (Type type : deobMethod.getDescriptor().getArguments())
-		{
-			builder.addArgument(new Type("L" + RSApi.API_BASE + type.getInternalName() + ";"));
-		}
-
-		assert deobMethod.isStatic() == vanillaMethod.isStatic();
-
-		boolean modifiedSignature = false;
-		if (!deobMethod.isStatic() && useHooks)
-		{
-			// Add variable to signature
-			builder.addArgument(0, inject.deobfuscatedTypeToApiType(new Type(deobMethod.getClassFile().getName())));
-			modifiedSignature = true;
-		}
-
-		Signature signature = builder.build();
-
-		List<Integer> insertIndexes = findHookLocations(hookName, end, vanillaMethod);
-		insertIndexes.sort((a, b) -> Integer.compare(b, a));
-
-		for (int insertPos : insertIndexes)
-		{
-			if (!deobMethod.isStatic())
-			{
-				instructions.addInstruction(insertPos++, new ALoad(instructions, 0));
-			}
-
-			int signatureStart = modifiedSignature ? 1 : 0;
-			int index = deobMethod.isStatic() ? 0 : 1; // current variable index
-
-			for (int i = signatureStart; i < signature.size(); ++i)
-			{
-				Type type = signature.getTypeOfArg(i);
-
-				Instruction load = InjectUtil.createLoadForTypeIndex(instructions, type, index);
-				instructions.addInstruction(insertPos++, load);
-
-				index += type.getSize();
-			}
-
-			InvokeInstruction invoke;
-
-			// use old Hooks callback
-			if (useHooks)
-			{
-				// Invoke callback
-				invoke = new InvokeStatic(instructions,
-					new net.runelite.asm.pool.Method(
-						new net.runelite.asm.pool.Class(HOOKS),
-						hookName,
-						signature
-					)
-				);
-			}
-			else
-			{
-				// Invoke methodhook
-				assert hookMethod != null;
-
-				if (vanillaMethod.isStatic())
-				{
-					invoke = new InvokeStatic(instructions,
-						new net.runelite.asm.pool.Method(
-							new net.runelite.asm.pool.Class("client"), // Static methods are in client
-							hookMethod.getName(),
-							signature
-						)
-					);
-				}
-				else
-				{
-					// otherwise invoke member function
-					//instructions.addInstruction(insertPos++, new ALoad(instructions, 0));
-					invoke = new InvokeVirtual(instructions,
-						new net.runelite.asm.pool.Method(
-							new net.runelite.asm.pool.Class(vanillaMethod.getClassFile().getName()),
-							hookMethod.getName(),
-							hookMethod.getDescriptor()
-						)
-					);
-				}
-			}
-
-			instructions.addInstruction(insertPos++, (Instruction) invoke);
-		}
-
-		log.debug("Injected method hook {} in {} with {} args: {}",
-			hookName, vanillaMethod, signature.size(),
-			signature.getArguments());
-	}
-
-	private List<Integer> findHookLocations(String hookName, boolean end, Method vanillaMethod) throws Injexception
-	{
-		Instructions instructions = vanillaMethod.getCode().getInstructions();
 
 		if (end)
 		{
-			// find return
+			final ListIterator<Instruction> it = ins.listIterator(ins.size() - 1);
-			List<Instruction> returns = instructions.getInstructions().stream()
+			while (it.hasPrevious())
-				.filter(i -> i instanceof ReturnInstruction)
-				.collect(Collectors.toList());
-			List<Integer> indexes = new ArrayList<>();
-
-			for (Instruction ret : returns)
 			{
-				int idx = instructions.getInstructions().indexOf(ret);
+				if (it.previous() instanceof ReturnInstruction)
-				assert idx != -1;
+				{
-				indexes.add(idx);
+					insertVoke(method, hookMethod, it);
+				}
 			}
 
-			return indexes;
+			return;
 		}
 
-		if (!vanillaMethod.getName().equals("<init>"))
+		final ListIterator<Instruction> it = ins.listIterator();
-		{
-			return Arrays.asList(0);
-		}
 
-		// Find index after invokespecial
+		if (method.getName().equals("<init>"))
-		for (int i = 0; i < instructions.getInstructions().size(); ++i)
 		{
-			Instruction in = instructions.getInstructions().get(i);
+			while (it.hasNext())
-
-			if (in.getType() == InstructionType.INVOKESPECIAL)
 			{
-				return Arrays.asList(i + 1); // one after
+				if (it.next() instanceof InvokeSpecial)
+				{
+					break;
+				}
 			}
+
+			assert it.hasNext() : "Constructor without invokespecial";
 		}
 
-		throw new IllegalStateException("constructor with no invokespecial");
+		insertVoke(method, hookMethod, it);
+	}
+
+	private void insertVoke(final Method method, final net.runelite.asm.pool.Method hookMethod, ListIterator<Instruction> iterator)
+	{
+		final Instructions instructions = method.getCode().getInstructions();
+		int varIdx = 0;
+
+		if (!method.isStatic())
+		{
+			iterator.add(new ALoad(instructions, varIdx++));
+		}
+
+		for (Type type : hookMethod.getType().getArguments())
+		{
+			iterator.add(
+				InjectUtil.createLoadForTypeIndex(
+					instructions,
+					type,
+					varIdx
+				)
+			);
+
+			varIdx += type.getSize();
+		}
+
+		iterator.add(
+			InjectUtil.createInvokeFor(
+				instructions,
+				hookMethod,
+				method.isStatic()
+			)
+		);
 	}
 }

src/main/java/com/openosrs/injector/injectors/rsapi/InjectInvoke.java

@@ -40,8 +40,9 @@ public class InjectInvoke
 		int varIdx = 0;
 		if (!vanillaMethod.isStatic())
 		{
-			ins.add(new ALoad(instructions, varIdx++));
+			ins.add(new ALoad(instructions, varIdx));
 		}
+		++varIdx;
 
 		final Signature apiSig = apiMethod.getSignature();
 		final Signature vanSig = vanillaMethod.getDescriptor();