fixes a regression with methods ending in throw + ret

pardeike · Jan 11, 2024 · e63e183 · e63e183
1 parent 1350ab8
commit e63e183
Show file tree

Hide file tree

Showing 5 changed files with 63 additions and 8 deletions.
diff --git a/Directory.Build.props b/Directory.Build.props
@@ -2,8 +2,8 @@
 
 	<PropertyGroup>
 		<HarmonyVersion>2.3.0.0</HarmonyVersion>
-		<HarmonyPrerelease>-prerelease.4</HarmonyPrerelease>
-		<MonoModCoreVersion>1.1.0-prerelease.1</MonoModCoreVersion>
+		<HarmonyPrerelease>-prerelease.5</HarmonyPrerelease>
+		<MonoModCoreVersion>1.1.0-prerelease.2</MonoModCoreVersion>
 	</PropertyGroup>
 
 </Project>
diff --git a/Harmony/Internal/MethodCopier.cs b/Harmony/Internal/MethodCopier.cs
@@ -34,9 +34,9 @@ internal void AddTranspiler(MethodInfo transpiler)
 			transpilers.Add(transpiler);
 		}
 
-		internal List<CodeInstruction> Finalize(Emitter emitter, List<Label> endLabels, out bool hasReturnCode)
+		internal List<CodeInstruction> Finalize(Emitter emitter, List<Label> endLabels, out bool hasReturnCode, out bool endsInThrow)
 		{
-			return reader.FinalizeILCodes(emitter, transpilers, endLabels, out hasReturnCode);
+			return reader.FinalizeILCodes(emitter, transpilers, endLabels, out hasReturnCode, out endsInThrow);
 		}
 
 		internal static List<CodeInstruction> GetInstructions(ILGenerator generator, MethodBase method, int maxTranspilers)
@@ -57,7 +57,7 @@ internal static List<CodeInstruction> GetInstructions(ILGenerator generator, Met
 					copier.AddTranspiler(sortedTranspilers[i]);
 			}
 
-			return copier.Finalize(null, null, out var _);
+			return copier.Finalize(null, null, out var _, out var _);
 		}
 	}
 
@@ -315,9 +315,10 @@ void ParseExceptions()
 			{ OpCodes.Blt_Un_S, OpCodes.Blt_Un }
 		};
 
-		internal List<CodeInstruction> FinalizeILCodes(Emitter emitter, List<MethodInfo> transpilers, List<Label> endLabels, out bool hasReturnCode)
+		internal List<CodeInstruction> FinalizeILCodes(Emitter emitter, List<MethodInfo> transpilers, List<Label> endLabels, out bool hasReturnCode, out bool endsInThrow)
 		{
 			hasReturnCode = false;
+			endsInThrow = false;
 			if (generator is null) return null;
 
 			// pass1 - define labels and add them to instructions that are target of a jump
@@ -381,6 +382,7 @@ internal List<CodeInstruction> FinalizeILCodes(Emitter emitter, List<MethodInfo>
 			// pass4 - check for any RET
 			//
 			hasReturnCode = codeInstructions.Any(code => code.opcode == OpCodes.Ret);
+			endsInThrow = codeInstructions.LastOrDefault()?.opcode == OpCodes.Throw;
 
 			// pass5 - remove RET if it appears at the end
 			//

diff --git a/Harmony/Internal/MethodPatcher.cs b/Harmony/Internal/MethodPatcher.cs
@@ -139,8 +139,7 @@ internal MethodInfo CreateReplacement(out Dictionary<int, CodeInstruction> final
 			copier.AddTranspiler(PatchTools.m_GetExecutingAssemblyReplacementTranspiler);
 
 			var endLabels = new List<Label>();
-			var lastCode = copier.Finalize(emitter, endLabels, out var hasReturnCode).LastOrDefault();
-			var endsInThrow = lastCode != null && lastCode.opcode == OpCodes.Throw;
+			_ = copier.Finalize(emitter, endLabels, out var hasReturnCode, out var endsInThrow);
 
 			foreach (var label in endLabels)
 				emitter.MarkLabel(label);

diff --git a/HarmonyTests/Patching/Assets/Specials.cs b/HarmonyTests/Patching/Assets/Specials.cs
@@ -1,6 +1,8 @@
 using HarmonyLib;
 using System;
 using System.Collections.Generic;
+using System.Diagnostics;
+using System.IO;
 using System.Linq;
 using System.Reflection;
 using System.Reflection.Emit;
@@ -113,6 +115,33 @@ static Exception Cleanup()
 		}
 	}
 
+	public class LateThrowClass
+	{
+		StringBuilder builder;
+
+		public void Method()
+		{
+			if (builder == null)
+			{
+				builder = new StringBuilder();
+				var num = builder.Length == 123;
+
+				// this throw is the last IL code before 'ret' in this method
+				if (num == false)
+					throw new Exception("Test");
+			}
+		}
+	}
+
+	[HarmonyPatch(typeof(LateThrowClass), nameof(LateThrowClass.Method))]
+	public class LateThrowClass_Patch
+	{
+		static bool Prefix()
+		{
+			return false;
+		}
+	}
+
 	public struct SomeStruct
 	{
 		public bool accepted;

diff --git a/HarmonyTests/Patching/Specials.cs b/HarmonyTests/Patching/Specials.cs
@@ -156,6 +156,31 @@ public void Test_PatchException()
 			Assert.NotNull(exception, "expecting runtime exception");
 		}
 
+		[Test]
+		public void Test_PatchingLateThrow()
+		{
+			var patchClass = typeof(LateThrowClass_Patch);
+			Assert.NotNull(patchClass);
+
+			try
+			{
+				new LateThrowClass().Method();
+				Assert.Fail("expecting exception");
+			}
+			catch (Exception ex)
+			{
+				Assert.AreEqual(ex.Message, "Test");
+			}
+
+			var instance = new Harmony("test");
+			Assert.NotNull(instance);
+			var patcher = instance.CreateClassProcessor(patchClass);
+			Assert.NotNull(patcher);
+			Assert.NotNull(patcher.Patch());
+
+			new LateThrowClass().Method();
+		}
+
 		[Test]
 		public void Test_PatchExceptionWithCleanup2()
 		{