Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix prefix skipping for dead-end code #584

Merged
merged 1 commit into from Mar 2, 2024
Merged

Fix prefix skipping for dead-end code #584

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions Harmony/Internal/MethodPatcher.cs
View file
Open in desktop
Expand Up @@ -164,7 +164,7 @@ internal MethodInfo CreateReplacement(out Dictionary<int, CodeInstruction> final

_ = AddPostfixes(privateVars, runOriginalVariable, false);

if (resultVariable is not null && hasReturnCode)
if (resultVariable is not null && (hasReturnCode || (methodEndsInDeadCode && skipOriginalLabel is not null)))
emitter.Emit(OpCodes.Ldloc, resultVariable);

var needsToStorePassthroughResult = AddPostfixes(privateVars, runOriginalVariable, true);
Expand Down Expand Up @@ -219,7 +219,7 @@ internal MethodInfo CreateReplacement(out Dictionary<int, CodeInstruction> final
emitter.Emit(OpCodes.Ldloc, resultVariable);
}

if (methodEndsInDeadCode == false || hasFinalizers || postfixes.Count > 0)
if (methodEndsInDeadCode == false || skipOriginalLabel is not null || hasFinalizers || postfixes.Count > 0)
emitter.Emit(OpCodes.Ret);

finalInstructions = emitter.GetInstructions();
Expand Down
2 changes: 2 additions & 0 deletions HarmonyTests/Patching/Assets/Specials.cs
View file
Open in desktop
Expand Up @@ -104,6 +104,8 @@ public class DeadEndCode_Patch1
public static void Prefix() => prefixCalled = true;

public static void Postfix() => postfixCalled = true;

public static bool PrefixWithControl() => false;
}

[HarmonyPatch(typeof(DeadEndCode), nameof(DeadEndCode.Method))]
Expand Down
22 changes: 19 additions & 3 deletions HarmonyTests/Patching/Specials.cs
View file
Open in desktop
Expand Up @@ -179,6 +179,9 @@ public void Test_PatchException()
Assert.NotNull(prefix);
var postfix = AccessTools.Method(typeof(DeadEndCode_Patch1), nameof(DeadEndCode_Patch1.Postfix));
Assert.NotNull(postfix);
var prefixWithControl =
AccessTools.Method(typeof(DeadEndCode_Patch1), nameof(DeadEndCode_Patch1.PrefixWithControl));
Assert.NotNull(postfix);

// run original
try
Expand Down Expand Up @@ -208,13 +211,16 @@ public void Test_PatchException()
}
Assert.True(DeadEndCode_Patch1.prefixCalled);


var isMonoRuntime = AccessTools.IsMonoRuntime;
var harmonyMethodPostfix = new HarmonyMethod(postfix);
// patch: +postfix
if (AccessTools.IsMonoRuntime)
if (isMonoRuntime)
{
// mono should fail
try
{
_ = instance.Patch(original, postfix: new HarmonyMethod(postfix));
_ = instance.Patch(original, postfix: harmonyMethodPostfix);
Assert.Fail("expecting patch exception");
}
catch (Exception ex)
Expand All @@ -226,7 +232,7 @@ public void Test_PatchException()
{
// non-mono can add postfix to methods ending in dead code

_ = instance.Patch(original, postfix: new HarmonyMethod(prefix));
_ = instance.Patch(original, postfix: harmonyMethodPostfix);
DeadEndCode_Patch1.prefixCalled = false;
DeadEndCode_Patch1.postfixCalled = false;
// run original
Expand All @@ -242,6 +248,16 @@ public void Test_PatchException()
Assert.False(DeadEndCode_Patch1.postfixCalled);
}
}

_ = instance.Patch(original,
prefix: new HarmonyMethod(prefixWithControl),
postfix: isMonoRuntime ? harmonyMethodPostfix : null
);
DeadEndCode_Patch1.prefixCalled = false;
DeadEndCode_Patch1.postfixCalled = false;
test.Method();
Assert.True(DeadEndCode_Patch1.prefixCalled);
Assert.True(DeadEndCode_Patch1.postfixCalled);
}

[Test]
Expand Down