adds better detection for methods that end in dead code

Harmony/Internal/Emitter.cs

@@ -208,17 +208,22 @@ internal void MarkBlockBefore(ExceptionBlock block, out Label? label)
 
 		internal void MarkBlockAfter(ExceptionBlock block)
 		{
-			if (block.blockType == ExceptionBlockType.EndExceptionBlock)
+			switch (block.blockType)
 			{
-				if (debug)
-				{
-					// fake log a LEAVE code since BeginCatchBlock() does add it
-					LogIL(OpCodes.Leave, new LeaveTry());
-
-					FileLog.ChangeIndent(-1);
-					FileLog.LogBuffered("} // end handler");
-				}
-				il.EndExceptionBlock();
+				case ExceptionBlockType.EndExceptionBlock:
+					if (debug)
+					{
+						// fake log a LEAVE code since BeginCatchBlock() does add it
+						LogIL(OpCodes.Leave, new LeaveTry());
+
+						FileLog.ChangeIndent(-1);
+						FileLog.LogBuffered("} // end handler");
+					}
+					il.EndExceptionBlock();
+					return;
+				default:
+					Console.WriteLine(block.ToString());
+					return;
 			}
 		}
 

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, out bool endsInThrow)
+		internal List<CodeInstruction> Finalize(Emitter emitter, List<Label> endLabels, out bool hasReturnCode, out bool methodEndsInDeadCode)
 		{
-			return reader.FinalizeILCodes(emitter, transpilers, endLabels, out hasReturnCode, out endsInThrow);
+			return reader.FinalizeILCodes(emitter, transpilers, endLabels, out hasReturnCode, out methodEndsInDeadCode);
 		}
 
 		internal static List<CodeInstruction> GetInstructions(ILGenerator generator, MethodBase method, int maxTranspilers)
@@ -315,10 +315,18 @@ void ParseExceptions()
 			{ OpCodes.Blt_Un_S, OpCodes.Blt_Un }
 		};
 
-		internal List<CodeInstruction> FinalizeILCodes(Emitter emitter, List<MethodInfo> transpilers, List<Label> endLabels, out bool hasReturnCode, out bool endsInThrow)
+		bool EndsInDeadCode(List<CodeInstruction> list)
+		{
+			var n = list.Count;
+			if (n < 2 || list.Last().opcode != OpCodes.Throw)
+				return false;
+			return list.GetRange(0, n - 1).Any(code => code.opcode == OpCodes.Ret);
+		}
+
+		internal List<CodeInstruction> FinalizeILCodes(Emitter emitter, List<MethodInfo> transpilers, List<Label> endLabels, out bool hasReturnCode, out bool methodEndsInDeadCode)
 		{
 			hasReturnCode = false;
-			endsInThrow = false;
+			methodEndsInDeadCode = false;
 			if (generator is null) return null;
 
 			// pass1 - define labels and add them to instructions that are target of a jump
@@ -382,7 +390,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;
+			methodEndsInDeadCode = EndsInDeadCode(codeInstructions);
 
 			// pass5 - remove RET if it appears at the end
 			//
@@ -460,7 +468,7 @@ internal List<CodeInstruction> FinalizeILCodes(Emitter emitter, List<MethodInfo>
 				codeInstruction.blocks.Do(block => emitter.MarkBlockAfter(block));
 			});
 
-			emitter.LogComment("end original");
+			emitter.LogComment("end original" + (methodEndsInDeadCode ? " (has dead code end)" : ""));
 			return codeInstructions;
 		}
 

Harmony/Internal/MethodPatcher.cs

@@ -139,7 +139,7 @@ internal MethodInfo CreateReplacement(out Dictionary<int, CodeInstruction> final
 			copier.AddTranspiler(PatchTools.m_GetExecutingAssemblyReplacementTranspiler);
 
 			var endLabels = new List<Label>();
-			_ = copier.Finalize(emitter, endLabels, out var hasReturnCode, out var endsInThrow);
+			_ = copier.Finalize(emitter, endLabels, out var hasReturnCode, out var methodEndsInDeadCode);
 
 			foreach (var label in endLabels)
 				emitter.MarkLabel(label);
@@ -205,7 +205,7 @@ internal MethodInfo CreateReplacement(out Dictionary<int, CodeInstruction> final
 					emitter.Emit(OpCodes.Ldloc, resultVariable);
 			}
 
-			if (endsInThrow == false || hasFinalizers) // methods ending in throw cannot have more code after the throw
+			if (methodEndsInDeadCode == false || hasFinalizers || postfixes.Count > 0)
 				emitter.Emit(OpCodes.Ret);
 
 			finalInstructions = emitter.GetInstructions();

Harmony/Public/Harmony.cs

@@ -1,3 +1,4 @@
+using MonoMod;
 using MonoMod.Core.Platforms;
 using System;
 using System.Collections.Generic;
@@ -66,6 +67,9 @@ public Harmony(string id)
 			}
 
 			Id = id;
+
+			// TODO: enable to switch to building methods with CECIL
+			// Switches.SetSwitchValue(Switches.DMDType, "cecil");
 		}
 
 		/// <summary>Searches the current assembly for Harmony annotations and uses them to create patches</summary>

HarmonyTests/Patching/Assets/PatchClasses.cs

@@ -1,4 +1,5 @@
 using HarmonyLib;
+using MonoMod.Utils.Cil;
 using System;
 using System.Collections;
 using System.Collections.Generic;
@@ -1424,12 +1425,25 @@ static IEnumerable<CodeInstruction> Transpiler(IEnumerable<CodeInstruction> inst
 		}
 	}
 
-	// disabled - see test case
-	/*
 	public class ClassExceptionFilter
 	{
+		static bool flag = false;
+
+		[MethodImpl(MethodImplOptions.NoInlining)]
+		public static void Method1()
+		{
+			try
+			{
+				Console.WriteLine("code");
+			}
+			catch (Exception e) when (flag)
+			{
+				Console.WriteLine(e.Message);
+			}
+		}
+
 		[MethodImpl(MethodImplOptions.NoInlining)]
-		public static int Method(Exception exception)
+		public static int Method2(Exception exception)
 		{
 			var result = 0;
 			try
@@ -1452,5 +1466,4 @@ public static int Method(Exception exception)
 			return result;
 		}
 	}
-	*/
 }

HarmonyTests/Patching/Assets/Specials.cs

@@ -34,26 +34,31 @@ public static void ResetTest()
 		}
 	}
 
+	// -----------------------------------------------------
+
 	public class DeadEndCode
 	{
 		public string Method()
 		{
-			throw new Exception();
+			throw new FormatException();
 		}
 	}
 
-	[HarmonyPatch(typeof(DeadEndCode), nameof(DeadEndCode.Method))]
+	// not using attributes here because we apply prefix first, then postfix
 	public class DeadEndCode_Patch1
 	{
-		static void Prefix()
+		public static bool prefixCalled = false;
+		public static bool postfixCalled = false;
+
+		public static void Prefix()
 		{
+			prefixCalled = true;
 		}
 
-		// cannot patch method that ends in throw with a postfix
-		//
-		// static void Postfix()
-		// {
-		// }
+		public static void Postfix()
+		{
+			prefixCalled = true;
+		}
 	}
 
 	[HarmonyPatch(typeof(DeadEndCode), nameof(DeadEndCode.Method))]
@@ -115,33 +120,73 @@ static Exception Cleanup()
 		}
 	}
 
-	public class LateThrowClass
+	// -----------------------------------------------------
+
+	public class LateThrowClass1
 	{
-		StringBuilder builder;
+		public void Method(string str)
+		{
+			if (str.Length == 2)
+				return;
+
+			// this throw is the last IL code before 'ret' in this method
+			throw new ArgumentException("fail");
+		}
+	}
+
+	[HarmonyPatch(typeof(LateThrowClass1), nameof(LateThrowClass1.Method))]
+	public class LateThrowClass_Patch1
+	{
+		public static bool prefixCalled = false;
+		public static bool postfixCalled = false;
+
+		static void Prefix()
+		{
+			prefixCalled = true;
+		}
 
-		public void Method()
+		static void Postfix()
 		{
-			if (builder == null)
-			{
-				builder = new StringBuilder();
-				var num = builder.Length == 123;
+			postfixCalled = true;
+		}
+	}
+
+	// -----------------------------------------------------
 
-				// this throw is the last IL code before 'ret' in this method
-				if (num == false)
-					throw new Exception("Test");
+	public class LateThrowClass2
+	{
+		public void Method(int i)
+		{
+			switch (i)
+			{
+				case 0:
+					Console.WriteLine("Test");
+					break;
+				default:
+					throw new ArgumentOutOfRangeException();
 			}
 		}
 	}
 
-	[HarmonyPatch(typeof(LateThrowClass), nameof(LateThrowClass.Method))]
-	public class LateThrowClass_Patch
+	[HarmonyPatch(typeof(LateThrowClass2), nameof(LateThrowClass2.Method))]
+	public class LateThrowClass_Patch2
 	{
-		static bool Prefix()
+		public static bool prefixCalled = false;
+		public static bool postfixCalled = false;
+
+		static void Prefix()
 		{
-			return false;
+			prefixCalled = true;
+		}
+
+		static void Postfix()
+		{
+			postfixCalled = true;
 		}
 	}
 
+	// -----------------------------------------------------
+
 	public struct SomeStruct
 	{
 		public bool accepted;

HarmonyTests/Patching/Exceptions.cs

@@ -1,30 +1,51 @@
+using HarmonyLib;
+using HarmonyLibTests.Assets;
+using NUnit.Framework;
+
 namespace HarmonyLibTests.Patching
 {
+	// TODO: DynamicMethod does not support 'catch .. when' and
+	//       MonoMod.Core has still a bug so for now we cannot enable
+
 	public class TestExceptionFilterBlock
 	{
-		// Filter exceptions are currently not supported in DynamicMethods
-		// Example:
-		// catch (Exception e) when (e.Message == "test") { }
+		[Test]
+		[Ignore("Filter exceptions are currently not supported in DynamicMethods")]
+		public void TestExceptionsWithFilter()
+		{
+			var originalClass = typeof(ClassExceptionFilter);
+			Assert.NotNull(originalClass);
+			var originalMethod = originalClass.GetMethod("Method1");
+			Assert.NotNull(originalMethod);
+
+			var instance = new Harmony("test");
+			Assert.NotNull(instance);
+
+			var patcher = new PatchProcessor(instance, originalMethod);
+			Assert.NotNull(patcher);
+			_ = patcher.Patch();
+
+			ClassExceptionFilter.Method1();
+		}
 
-		/*
 		[Test]
+		[Ignore("Filter exceptions are currently not supported in DynamicMethods")]
 		public void TestPlainMethodExceptions()
 		{
 			var originalClass = typeof(ClassExceptionFilter);
 			Assert.NotNull(originalClass);
-			var originalMethod = originalClass.GetMethod("Method");
+			var originalMethod = originalClass.GetMethod("Method2");
 			Assert.NotNull(originalMethod);
 
 			var instance = new Harmony("test");
 			Assert.NotNull(instance);
 
 			var patcher = new PatchProcessor(instance, originalMethod);
 			Assert.NotNull(patcher);
-			patcher.Patch();
+			_ = patcher.Patch();
 
-			var result = ClassExceptionFilter.Method(null);
+			var result = ClassExceptionFilter.Method2(null);
 			Assert.AreEqual(100, result);
 		}
-		*/
 	}
 }