GetMethodFromStackframe fix

pardeike · Mar 7, 2024 · 8cb43cb · 8cb43cb
1 parent 679d2d7
commit 8cb43cb
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 21 deletions.
diff --git a/Harmony/Internal/HarmonySharedState.cs b/Harmony/Internal/HarmonySharedState.cs
@@ -1,4 +1,5 @@
 using Mono.Cecil;
+using MonoMod.Core.Platforms;
 using MonoMod.Utils;
 using System;
 using System.Collections.Generic;
@@ -127,7 +128,10 @@ internal static void UpdatePatchInfo(MethodBase original, MethodInfo replacement
 
 		internal static MethodBase GetOriginal(MethodInfo replacement)
 		{
-			lock (originals) return originals.GetValueSafe(replacement);
+			// The runtime can return several different MethodInfo's that point to the same method. Use the correct one
+			replacement = PlatformTriple.Current.GetIdentifiable(replacement) as MethodInfo;
+			lock (originals)
+				return originals.GetValueSafe(replacement);
 		}
 
 		internal static MethodBase FindReplacement(StackFrame frame)

diff --git a/Harmony/Public/Harmony.cs b/Harmony/Public/Harmony.cs
@@ -1,4 +1,3 @@
-using MonoMod.Core.Platforms;
 using System;
 using System.Collections.Generic;
 using System.Diagnostics;
@@ -258,9 +257,7 @@ public IEnumerable<MethodBase> GetPatchedMethods()
 		public static MethodBase GetOriginalMethod(MethodInfo replacement)
 		{
 			if (replacement == null) throw new ArgumentNullException(nameof(replacement));
-			// The runtime can return several different MethodInfo's that point to the same method. Use the correct one
-			var identifiableReplacement = PlatformTriple.Current.GetIdentifiable(replacement) as MethodInfo;
-			return HarmonySharedState.GetOriginal(identifiableReplacement);
+			return HarmonySharedState.GetOriginal(replacement);
 		}
 
 		/// <summary>Tries to get the method from a stackframe including dynamic replacement methods</summary>

diff --git a/HarmonyTests/Extras/RetrieveOriginalMethod.cs b/HarmonyTests/Extras/RetrieveOriginalMethod.cs
@@ -3,6 +3,7 @@
 using System;
 using System.Diagnostics;
 using System.Reflection;
+using System.Runtime.CompilerServices;
 
 namespace HarmonyLibTests.Extras
 {
@@ -14,20 +15,20 @@ private static void CheckStackTraceFor(MethodBase expectedMethod)
 			Assert.NotNull(expectedMethod);
 
 			var st = new StackTrace(1, false);
-			var method = Harmony.GetMethodFromStackframe(st.GetFrame(0));
-
-			Assert.NotNull(method);
-
-			if (method is MethodInfo replacement)
-			{
-				var original = Harmony.GetOriginalMethod(replacement);
-				Assert.NotNull(original);
-				Assert.AreEqual(original, expectedMethod);
-			}
+			var frame = st.GetFrame(0);
+			Assert.NotNull(frame);
+
+			var methodFromStackframe = Harmony.GetMethodFromStackframe(frame);
+			Assert.NotNull(methodFromStackframe);
+			Assert.AreEqual(expectedMethod, methodFromStackframe);
+
+			var replacement = frame.GetMethod() as MethodInfo;
+			Assert.NotNull(replacement);
+			var original = Harmony.GetOriginalMethod(replacement);
+			Assert.NotNull(original);
+			Assert.AreEqual(expectedMethod, original);
 		}
 
-		/* TODO
-		 * 
 		[Test]
 		public void TestRegularMethod()
 		{
@@ -37,7 +38,7 @@ public void TestRegularMethod()
 			_ = harmony.Patch(originalMethod, new HarmonyMethod(dummyPrefix));
 			PatchTarget();
 		}
-		
+
 		[Test]
 		public void TestConstructor()
 		{
@@ -48,7 +49,6 @@ public void TestConstructor()
 			var inst = new NestedClass(5);
 			_ = inst.index;
 		}
-		*/
 
 		internal static void PatchTarget()
 		{
@@ -60,7 +60,7 @@ internal static void PatchTarget()
 			}
 		}
 
-		// [MethodImpl(MethodImplOptions.NoInlining)]
+		[MethodImpl(MethodImplOptions.NoInlining)]
 		internal static void DummyPrefix()
 		{
 		}
@@ -69,7 +69,7 @@ class NestedClass {
 			public NestedClass(int i)
 			{
 				try {
-					CheckStackTraceFor(AccessTools.Constructor(typeof(NestedClass), [typeof(int)])); 
+					CheckStackTraceFor(AccessTools.Constructor(typeof(NestedClass), [typeof(int)]));
 					throw new Exception();
 				} catch (Exception e)
 				{