Merge pull request #12 from 0x0ade/master

Make MethodInvoker.Handler operate on stack instead of locals
pardeike · Jun 18, 2017 · 05e5b66 · 05e5b66
2 parents e3dfcf2 + 15e51e2
commit 05e5b66
Show file tree

Hide file tree

Showing 4 changed files with 160 additions and 39 deletions.
diff --git a/Harmony/Extras/MethodInvoker.cs b/Harmony/Extras/MethodInvoker.cs
@@ -8,7 +8,7 @@ namespace Harmony
 
 	public delegate object FastInvokeHandler(object target, object[] paramters);
 
-	class MethodInvoker
+	public class MethodInvoker
 	{
 		public static FastInvokeHandler GetHandler(DynamicMethod methodInfo, Module module)
 		{
@@ -20,43 +20,82 @@ public static FastInvokeHandler GetHandler(MethodInfo methodInfo)
 			return Handler(methodInfo, methodInfo.DeclaringType.Module);
 		}
 
-		static FastInvokeHandler Handler(MethodInfo methodInfo, Module module)
+		static FastInvokeHandler Handler(MethodInfo methodInfo, Module module, bool directBoxValueAccess = false)
 		{
-			var dynamicMethod = new DynamicMethod("FastInvoke_" + methodInfo.Name, typeof(object), new Type[] { typeof(object), typeof(object[]) }, module);
+			var dynamicMethod = new DynamicMethod("FastInvoke_" + methodInfo.Name + "_" + (directBoxValueAccess ? "direct" : "indirect"), typeof(object), new Type[] { typeof(object), typeof(object[]) }, module);
 			var il = dynamicMethod.GetILGenerator();
 
+			bool generateLocalBoxValuePtr = true;
+
 			var ps = methodInfo.GetParameters();
-			var paramTypes = new Type[ps.Length];
-			for (int i = 0; i < paramTypes.Length; i++)
+
+			if (!methodInfo.IsStatic)
 			{
-				if (ps[i].ParameterType.IsByRef)
-					paramTypes[i] = ps[i].ParameterType.GetElementType();
-				else
-					paramTypes[i] = ps[i].ParameterType;
+				il.Emit(OpCodes.Ldarg_0);
+				EmitUnboxIfNeeded(il, methodInfo.DeclaringType);
 			}
 
-			var locals = new LocalBuilder[paramTypes.Length];
-			for (int i = 0; i < paramTypes.Length; i++)
-				locals[i] = il.DeclareLocal(paramTypes[i], true);
-
-			for (int i = 0; i < paramTypes.Length; i++)
+			for (int i = 0; i < ps.Length; i++)
 			{
+				Type argType = ps[i].ParameterType;
+				bool argIsByRef = argType.IsByRef;
+				if (argIsByRef)
+					argType = argType.GetElementType();
+				bool argIsValueType = argType.IsValueType;
+
+				if (argIsByRef && argIsValueType && !directBoxValueAccess)
+				{
+					// Used later when storing back the reference to the new box in the array.
+					il.Emit(OpCodes.Ldarg_1);
+					EmitFastInt(il, i);
+				}
+
 				il.Emit(OpCodes.Ldarg_1);
 				EmitFastInt(il, i);
-				il.Emit(OpCodes.Ldelem_Ref);
-				EmitCastToReference(il, paramTypes[i]);
-				il.Emit(OpCodes.Stloc, locals[i]);
-			}
 
-			if (!methodInfo.IsStatic)
-				il.Emit(OpCodes.Ldarg_0);
-
-			for (int i = 0; i < paramTypes.Length; i++)
-			{
-				if (ps[i].ParameterType.IsByRef)
-					il.Emit(OpCodes.Ldloca_S, locals[i]);
+				if (argIsByRef && !argIsValueType)
+				{
+					il.Emit(OpCodes.Ldelema, typeof(object));
+				}
 				else
-					il.Emit(OpCodes.Ldloc, locals[i]);
+				{
+					il.Emit(OpCodes.Ldelem_Ref);
+					if (argIsValueType)
+					{
+						if (!argIsByRef || !directBoxValueAccess)
+						{
+							// if !directBoxValueAccess, create a new box if required
+							il.Emit(OpCodes.Unbox_Any, argType);
+							if (argIsByRef)
+							{
+								// box back
+								il.Emit(OpCodes.Box, argType);
+
+								// store new box value address to local 0
+								il.Emit(OpCodes.Dup);
+								il.Emit(OpCodes.Unbox, argType);
+								if (generateLocalBoxValuePtr)
+								{
+									generateLocalBoxValuePtr = false;
+									// Yes, you're seeing this right - a local of type void* to store the box value address!
+									il.DeclareLocal(typeof(void*), true);
+								}
+								il.Emit(OpCodes.Stloc_0);
+
+								// arr and index set up already
+								il.Emit(OpCodes.Stelem_Ref);
+
+								// load address back to stack
+								il.Emit(OpCodes.Ldloc_0);
+							}
+						}
+						else
+						{
+							// if directBoxValueAccess, emit unbox (get value address)
+							il.Emit(OpCodes.Unbox, argType);
+						}
+					}
+				}
 			}
 
 #pragma warning disable XS0001
@@ -71,19 +110,6 @@ static FastInvokeHandler Handler(MethodInfo methodInfo, Module module)
 			else
 				EmitBoxIfNeeded(il, methodInfo.ReturnType);
 
-			for (int i = 0; i < paramTypes.Length; i++)
-			{
-				if (ps[i].ParameterType.IsByRef)
-				{
-					il.Emit(OpCodes.Ldarg_1);
-					EmitFastInt(il, i);
-					il.Emit(OpCodes.Ldloc, locals[i]);
-					if (locals[i].LocalType.IsValueType)
-						il.Emit(OpCodes.Box, locals[i].LocalType);
-					il.Emit(OpCodes.Stelem_Ref);
-				}
-			}
-
 			il.Emit(OpCodes.Ret);
 
 			var invoder = (FastInvokeHandler)dynamicMethod.CreateDelegate(typeof(FastInvokeHandler));
@@ -98,6 +124,12 @@ static void EmitCastToReference(ILGenerator il, Type type)
 				il.Emit(OpCodes.Castclass, type);
 		}
 
+		static void EmitUnboxIfNeeded(ILGenerator il, Type type)
+		{
+			if (type.IsValueType)
+				il.Emit(OpCodes.Unbox_Any, type);
+		}
+
 		static void EmitBoxIfNeeded(ILGenerator il, Type type)
 		{
 			if (type.IsValueType)

diff --git a/HarmonyTests/Extras/Assets/MethodInvokerClasses.cs b/HarmonyTests/Extras/Assets/MethodInvokerClasses.cs
@@ -0,0 +1,33 @@
+using Harmony;
+using System.Collections.Generic;
+using System.Reflection;
+using System.Runtime.CompilerServices;
+
+namespace HarmonyTests.Assets
+{
+    public class TestMethodInvokerObject
+    {
+        public int Value;
+        public void Method1(int a)
+        {
+            Value += a;
+        }
+    }
+    public struct TestMethodInvokerStruct
+    {
+        public int Value;
+    }
+    public static class MethodInvokerClass
+    {
+
+        public static void Method1(int a, ref int b, out int c, out TestMethodInvokerObject d, ref TestMethodInvokerStruct e)
+        {
+            b = b + 1;
+            c = b * 2;
+            d = new TestMethodInvokerObject();
+            d.Value = a;
+            e.Value = a;
+        }
+
+    }
+}
diff --git a/HarmonyTests/Extras/TestMethodInvoker.cs b/HarmonyTests/Extras/TestMethodInvoker.cs
@@ -0,0 +1,54 @@
+using Harmony;
+using Harmony.ILCopying;
+using HarmonyTests.Assets;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+using System;
+using System.Reflection;
+
+namespace HarmonyTests
+{
+	[TestClass]
+	public class TestMethodInvoker
+    {
+
+		[TestMethod]
+        public void TestMethodInvokerGeneral()
+        {
+            var type = typeof(MethodInvokerClass);
+            Assert.IsNotNull(type);
+            var method = type.GetMethod("Method1");
+            Assert.IsNotNull(method);
+
+            var handler = MethodInvoker.GetHandler(method);
+            Assert.IsNotNull(handler);
+
+            object[] args = new object[] { 1, 0, 0, /*out*/ null, /*ref*/ new TestMethodInvokerStruct() };
+            handler(null, args);
+            Assert.AreEqual(args[0], 1);
+            Assert.AreEqual(args[1], 1);
+            Assert.AreEqual(args[2], 2);
+            Assert.AreEqual(((TestMethodInvokerObject) args[3])?.Value, 1);
+            Assert.AreEqual(((TestMethodInvokerStruct) args[4]).Value, 1);
+        }
+
+        [TestMethod]
+        public void TestMethodInvokerSelfObject()
+        {
+            var type = typeof(TestMethodInvokerObject);
+            Assert.IsNotNull(type);
+            var method = type.GetMethod("Method1");
+            Assert.IsNotNull(method);
+
+            var handler = MethodInvoker.GetHandler(method);
+            Assert.IsNotNull(handler);
+
+            var instance = new TestMethodInvokerObject();
+            instance.Value = 1;
+
+            object[] args = new object[] { 2 };
+            handler(instance, args);
+            Assert.AreEqual(instance.Value, 3);
+        }
+
+    }
+}
diff --git a/HarmonyTests/HarmonyTests.csproj b/HarmonyTests/HarmonyTests.csproj
@@ -55,7 +55,9 @@
     </Otherwise>
   </Choose>
   <ItemGroup>
+    <Compile Include="Extras\Assets\MethodInvokerClasses.cs" />
     <Compile Include="Patching\Assets\PatchClasses.cs" />
+    <Compile Include="Extras\TestMethodInvoker.cs" />
     <Compile Include="Patching\StaticPatches.cs" />
     <Compile Include="Tools\Assets\AccessToolsClass.cs" />
     <Compile Include="Tools\Assets\AttributesClass.cs" />