Fixed bug in patch argument passing when first argument isn't instanc…

…e but return buffer pointer
pardeike · Feb 9, 2020 · cfbd9f3 · cfbd9f3
1 parent 25fede0
commit cfbd9f3
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 12 deletions.
diff --git a/Harmony/Internal/MethodPatcher.cs b/Harmony/Internal/MethodPatcher.cs
@@ -20,6 +20,7 @@ internal class MethodPatcher
 		public const string PARAM_INDEX_PREFIX = "__";
 		public const string INSTANCE_FIELD_PREFIX = "___";
 
+		readonly bool debug;
 		readonly MethodBase original;
 		readonly MethodBase source;
 		readonly List<MethodInfo> prefixes;
@@ -28,6 +29,7 @@ internal class MethodPatcher
 		readonly List<MethodInfo> finalizers;
 		readonly int idx;
 		readonly bool firstArgIsReturnBuffer;
+		readonly OpCode Ldarg_instance;
 		readonly Type returnType;
 		readonly DynamicMethodDefinition patch;
 		readonly ILGenerator il;
@@ -38,6 +40,7 @@ internal MethodPatcher(MethodBase original, MethodBase source, List<MethodInfo>
 			if (original == null)
 				throw new ArgumentNullException(nameof(original));
 
+			this.debug = debug;
 			this.original = original;
 			this.source = source;
 			this.prefixes = prefixes;
@@ -55,6 +58,8 @@ internal MethodPatcher(MethodBase original, MethodBase source, List<MethodInfo>
 
 			idx = prefixes.Count() + postfixes.Count() + finalizers.Count();
 			firstArgIsReturnBuffer = NativeThisPointer.NeedsNativeThisPointerFix(original);
+			Ldarg_instance = firstArgIsReturnBuffer ? OpCodes.Ldarg_1 : OpCodes.Ldarg_0;
+			if (debug && firstArgIsReturnBuffer) FileLog.Log($"### Special case: Ldarg.0 is return buffer, not instance!");
 			returnType = AccessTools.GetReturnedType(original);
 			patch = CreateDynamicMethod(original, $"_Patch{idx}");
 			if (patch == null)
@@ -64,7 +69,7 @@ internal MethodPatcher(MethodBase original, MethodBase source, List<MethodInfo>
 			emitter = new Emitter(il, debug);
 		}
 
-		internal MethodInfo CreateReplacement(bool debug, out Dictionary<int, CodeInstruction> finalInstructions)
+		internal MethodInfo CreateReplacement(out Dictionary<int, CodeInstruction> finalInstructions)
 		{
 			var originalVariables = DeclareLocalVariables(source ?? original);
 			var privateVars = new Dictionary<string, LocalBuilder>();
@@ -354,11 +359,11 @@ void EmitCallParameter(MethodInfo patch, Dictionary<string, LocalBuilder> variab
 						var parameterIsRef = patchParam.ParameterType.IsByRef;
 						if (instanceIsRef == parameterIsRef)
 						{
-							emitter.Emit(OpCodes.Ldarg_0);
+							emitter.Emit(Ldarg_instance);
 						}
 						if (instanceIsRef && parameterIsRef == false)
 						{
-							emitter.Emit(OpCodes.Ldarg_0);
+							emitter.Emit(Ldarg_instance);
 							emitter.Emit(OpCodes.Ldobj, original.DeclaringType);
 						}
 						if (instanceIsRef == false && parameterIsRef)
@@ -391,7 +396,7 @@ void EmitCallParameter(MethodInfo patch, Dictionary<string, LocalBuilder> variab
 						emitter.Emit(patchParam.ParameterType.IsByRef ? OpCodes.Ldsflda : OpCodes.Ldsfld, fieldInfo);
 					else
 					{
-						emitter.Emit(OpCodes.Ldarg_0);
+						emitter.Emit(Ldarg_instance);
 						emitter.Emit(patchParam.ParameterType.IsByRef ? OpCodes.Ldflda : OpCodes.Ldfld, fieldInfo);
 					}
 					continue;

diff --git a/Harmony/Internal/NativeThisPointerCheck.cs b/Harmony/Internal/NativeThisPointerCheck.cs
@@ -116,19 +116,23 @@ private static IEnumerable<CodeInstruction> ArgumentShiftTranspiler(IEnumerable<
 					continue;
 				}
 
+				if (instruction.opcode == OpCodes.Ldarg_0)
+				{
+					instruction.opcode = OpCodes.Ldarg_1;
+					yield return instruction;
+					continue;
+				}
+
 				if (instruction.opcode == OpCodes.Ldarg
 					|| instruction.opcode == OpCodes.Ldarga
 					|| instruction.opcode == OpCodes.Ldarga_S
 					|| instruction.opcode == OpCodes.Starg
 					|| instruction.opcode == OpCodes.Starg_S)
 				{
 					var n = (int)instruction.operand;
-					if (n > 0)
-					{
-						instruction.operand = n + 1;
-						yield return instruction;
-						continue;
-					}
+					instruction.operand = n + 1;
+					yield return instruction;
+					continue;
 				}
 
 				yield return instruction;

diff --git a/Harmony/Internal/PatchFunctions.cs b/Harmony/Internal/PatchFunctions.cs
@@ -158,7 +158,7 @@ internal static MethodInfo UpdateWrapper(MethodBase original, PatchInfo patchInf
 			var sortedFinalizers = GetSortedPatchMethods(original, patchInfo.finalizers, debug);
 
 			var patcher = new MethodPatcher(original, null, sortedPrefixes, sortedPostfixes, sortedTranspilers, sortedFinalizers, debug);
-			var replacement = patcher.CreateReplacement(debug, out var finalInstructions);
+			var replacement = patcher.CreateReplacement(out var finalInstructions);
 			if (replacement == null) throw new MissingMethodException($"Cannot create replacement for {original.FullDescription()}");
 
 			try
@@ -191,7 +191,7 @@ internal static MethodInfo ReversePatch(HarmonyMethod standin, MethodBase origin
 
 			var empty = new List<MethodInfo>();
 			var patcher = new MethodPatcher(standin.method, original, empty, empty, transpilers, empty, debug);
-			var replacement = patcher.CreateReplacement(debug, out var finalInstructions);
+			var replacement = patcher.CreateReplacement(out var finalInstructions);
 			if (replacement == null) throw new MissingMethodException($"Cannot create replacement for {standin.method.FullDescription()}");
 
 			try