Crash to desktop when a certain patch of a Harmony 2.0 mod is applied (worked fine in 1.2)

[boformer]

Describe the bug

I have a single Harmony 2.0 mod. it uses PatchAll to apply a single prefix patch to this method:

protected Vector4 GetPathTargetPosition(ushort instanceID, ref CitizenInstance citizenData, ref CitizenInstance.Frame frameData, float minSqrDistance)

Full method: https://gist.github.com/boformer/bd94cfb56720336fbf4a9becf2d93c48

As soon as I enable the mod, the game crashes to desktop.

The same patch is working fine when I compile the mod with Harmony 1.2 (unless I run a different mod with Harmony 2.0 in parallel)

To Reproduce

The harmony log, Unity log and mod source Code:

https://gist.github.com/boformer/cc554e51aa33d41102ae91c8a69e2f74

Looks like what leads to the final crash is:

KERNELBASE.dll caused an Access Violation (0xc0000005)

0x00007FF98FA5A388 (KERNELBASE) RaiseException
0x00007FF952C91158 (mono) set_vprintf_func
0x00007FF952C9117C (mono) set_vprintf_func
0x00007FF952CB1775 (mono) mono_class_get_full
0x00007FF952DA20D2 (mono) mono_set_break_policy
0x00007FF952DB3F6C (mono) mono_set_defaults
0x00007FF952DB5037 (mono) mono_set_defaults
0x00007FF952DB5655 (mono) mono_set_defaults
0x00007FF952DB56E8 (mono) mono_set_defaults
0x00007FF952CD23A0 (mono) mono_domain_finalize
0x00000000064DBC47 (Mono JIT Code) (wrapper managed-to-native) System.Delegate:CreateDelegate_internal (System.Type,object,System.Reflection.MethodInfo,bool)
0x00000000064DB5EC (Mono JIT Code) System.Delegate:CreateDelegate (System.Type,object,System.Reflection.MethodInfo,bool)
0x00000000064DAE60 (Mono JIT Code) System.Delegate:CreateDelegate (System.Type,System.Reflection.MethodInfo,bool)
0x00000000064DADFB (Mono JIT Code) System.Delegate:CreateDelegate (System.Type,System.Reflection.MethodInfo)
0x00000000064DA713 (Mono JIT Code) System.Reflection.Emit.DynamicMethod:CreateDelegate (System.Type)
0x00000000064D939D (Mono JIT Code) Harmony.NativeThisPointer:GetManagedSize (System.Type)
0x00000000064D8848 (Mono JIT Code) Harmony.NativeThisPointer:NeedsNativeThisPointerFix (System.Reflection.MethodBase)
0x00000000064D6204 (Mono JIT Code) Harmony.MethodPatcher:CreatePatchedMethod (System.Reflection.MethodBase,string,System.Collections.Generic.List`1<System.Reflection.MethodInfo>,System.Collections.Generic.List`1<System.Reflection.MethodInfo>,System.Collections.Generic.List`1<System.Reflection.MethodInfo>)
0x00000000064C1937 (Mono JIT Code) Harmony.PatchFunctions:UpdateWrapper (System.Reflection.MethodBase,Harmony.PatchInfo,string)
0x00000000064BABF8 (Mono JIT Code) Harmony.PatchProcessor:Patch ()
0x00000000064B17D9 (Mono JIT Code) Harmony.HarmonyInstance:<PatchAll>b__11_0 (System.Type)
0x00000000064AFA48 (Mono JIT Code) Harmony.CollectionExtensions:Do<object> (System.Collections.Generic.IEnumerable`1<object>,System.Action`1<object>)
0x00000000064B1738 (Mono JIT Code) Harmony.HarmonyInstance:PatchAll (System.Reflection.Assembly)
0x00000000064983F9 (Mono JIT Code) DrawBridgeLoader.AiReplaceMod:OnEnabled ()

Runtime environment (please complete the following information):

• OS: Windows 10, 64bit
• .NET version 3.5
• Harmony version 2.0
  • Name of game or host application: Cities: Skylines

[pardeike]

I really need you to test that again but without transpiler and without self-patching active to narrow the issue down.

Thanks for your excellent work on this so far!

[boformer]

Here are the results without SELF_PATCHING and without a transpiler (used empty prefix instead):

I started the game, enabled the 2.0 mod, then enabled the 1.2 mod, then disabled the 1.2 mod. No errors.

Harmony log: https://gist.github.com/boformer/7f75873511ed4e0a42a94340fc54bf3a

I have already tested with SELF_PATCHING, but without a transpiler. So it's not related to a transpiler. However it has something to do with the method that is patched and the combination of 1.2/2.0. Other methods can be patched without a problem.

I will now also check what happens when I compile the 1.2 mod with Harmony 2.0. Maybe it's a general problem with 2.0 that doesn't have anything to do with self patching...

[boformer]

Ok update on this:

It doesn't have anything to do with self-patching, it's a bug in Harmony 2.0

I recompiled the mod that was using 1.2 with Harmony 2.0, and ran the game with only that mod. As soon as I enabled the mod, it crashed to Desktop. Harmony 1.2 is no longer involved.

Here is the Harmony log, the Unity log and the source code of the mod

https://gist.github.com/boformer/cc554e51aa33d41102ae91c8a69e2f74

[pardeike]

Great work. Since we have a working and a non-working case this should be solvable. Since minimal functionality is involved I think it should be easy to find the difference.

[boformer]

Another method that cannot be patched with 2.0, resulting in the same crash:

public override Color GetColor(ushort nodeID, ref NetNode data, InfoManager.InfoMode infoMode)

The patch (tried prefix and postfix, both crash):

using System;
using System.Reflection;
using Harmony;

namespace NetworkSkins.Patches
{
    [HarmonyPatch]
    public static class RoadBaseAiGetNodeColorPatch
    {
        public static MethodBase TargetMethod()
        {
            // GetColor(ushort nodeID, ref NetNode data, InfoManager.InfoMode infoMode)
            return typeof(RoadBaseAI).GetMethod("GetColor", new Type[] { typeof(ushort), typeof(NetNode).MakeByRefType(), typeof(InfoManager.InfoMode) });
        }

        public static void Prefix()
        {
        }
    }
}

The log doesn't say much:

### Harmony id=boformer.NetworkSkins, version=2.0.0.0, location=F:\Steam\steamapps\common\data-0000000019327F00
### Started from NetworkSkins.NetworkSkinsMod.OnEnabled(), location F:\Steam\steamapps\common\data-0000000019358000
### At 2019-03-28 07.12.38
AccessTools.Method: Could not find method for type NetworkSkins.Patches.RoadBaseAiGetNodeColorPatch and name Prepare and parameters 
AccessTools.Method: Could not find method for type NetworkSkins.Patches.RoadBaseAiGetNodeColorPatch and name TargetMethods and parameters 
AccessTools.Method: Could not find method for type NetworkSkins.Patches.RoadBaseAiGetNodeColorPatch and name Postfix and parameters 
AccessTools.Method: Could not find method for type NetworkSkins.Patches.RoadBaseAiGetNodeColorPatch and name Transpiler and parameters 
AccessTools.Method: Could not find method for type NetworkSkins.Patches.RoadBaseAiGetNodeColorPatch and name Prepare and parameters 
### Patch RoadBaseAI, Color GetColor(UInt16, NetNode ByRef, InfoMode)

I also tried to patch the base method (public virtual Color GetColor(...)), it also results in a crash.

Manual patching with HarmonyInstance.Patch(…) produces the same result.

[pardeike]

I've fixed a CTD bug possibly relatedt to this. Can you re-test with latest master?

[boformer]

Both patches no longer crash to Desktop, but they still throw errors when applied:

FormatException: Method CitizenAI.GetPathTargetPosition(System.UInt16, CitizenInstance&, Frame&, System.Single) cannot be patched. Reason: Invalid IL code in (wrapper dynamic-method) CitizenAI:GetPathTargetPosition_Patch1 (intptr,CitizenAI,uint16,CitizenInstance&,CitizenInstance/Frame&,single): IL_001b: ldfld     0x00000009

  at Harmony.PatchFunctions.UpdateWrapper (System.Reflection.MethodBase original, Harmony.PatchInfo patchInfo, System.String instanceID) [0x00000] in <filename unknown>:0 
  at Harmony.PatchProcessor.Patch () [0x00000] in <filename unknown>:0 
  at Harmony.HarmonyInstance.<PatchAll>b__11_0 (System.Type type) [0x00000] in <filename unknown>:0 
  at Harmony.CollectionExtensions.Do[Type] (IEnumerable`1 sequence, System.Action`1 action) [0x00000] in <filename unknown>:0 
  at Harmony.HarmonyInstance.PatchAll (System.Reflection.Assembly assembly) [0x00000] in <filename unknown>:0 

FormatException: Method RoadBaseAI.GetColor(System.UInt16, NetNode&, InfoMode) cannot be patched. Reason: Invalid IL code in (wrapper dynamic-method) RoadBaseAI:GetColor_Patch1 (intptr,RoadBaseAI,uint16,NetNode&,InfoManager/InfoMode): IL_0012: switch    (IL_0250, IL_004f, IL_0847, IL_004f, IL_0847, IL_0847, IL_004f, IL_004f, IL_004f, IL_03bc, IL_05bd, IL_0847, IL_004f, IL_07be)

  at Harmony.PatchFunctions.UpdateWrapper (System.Reflection.MethodBase original, Harmony.PatchInfo patchInfo, System.String instanceID) [0x00000] in <filename unknown>:0 
  at Harmony.PatchProcessor.Patch () [0x00000] in <filename unknown>:0 
  at Harmony.HarmonyInstance.<PatchAll>b__11_0 (System.Type type) [0x00000] in <filename unknown>:0 
  at Harmony.CollectionExtensions.Do[Type] (IEnumerable`1 sequence, System.Action`1 action) [0x00000] in <filename unknown>:0 
  at Harmony.HarmonyInstance.PatchAll (System.Reflection.Assembly assembly) [0x00000] in <filename unknown>:0 

Again, both are just simple prefix Patches. Both Patches are working perfectly fine with Harmony 1.2

Logs for Harmony 1.2 and 2.0: https://gist.github.com/boformer/76baa9e8f3df714bf40f9855c403da35

[boformer]

I ran both logs through a diffchecker. For some reason Harmony 2.0 inserts an ldarg.1 instruction before the prefix call:

[boformer]

Another small update on this: I looked into the cause for the extra IL instructions in 2.0. TL;DR: It's not the reason for the "invalid IL Code" exception.

It happens because NeedsNativePointerFix() returns true for the methods so that firstArgIsReturnBuffer is true:

https://github.com/pardeike/Harmony/blob/10ca9fa787c4be7e516d3f2dc60598708625fbf2/Harmony/Internal/MethodPatcher.cs#L43

I'm not sure what's the reason for the pointer fix, but I commented it out in my local copy of Harmony so that firstArgIsReturnBuffer is always false.

After that both versions of Harmony output exactly the same instructions in harmony.log. But still, 1.2 runs through successfully, 2.0 fails with the same Invalid IL code exception that I posted above.

So it seems that the cause for this problem cannot be found in the harmony.log. (Here is the harmony.log for my experiment)

---

I noticed that both methods have 2 things in common:

• They both return a Unity struct type (Vector4 and Color)
• They both have a ref parameter

[pardeike]

That’s the problem. See https://github.com/dotnet/coreclr/blob/master/Documentation/botr/clr-abi.md for a discussion of the “return type is large struct on certain runtimes” problem. It’s a very specific optimisation that requires some workarounds since Harmony maps the original method signature onto a dynamic method which is always static and thus the original IL code for this specific situation does not match the original.

[pardeike]

After a lot of testing and research, I think I fixed the issue with commit 03fb3d52cb2706ff8156f3f35e7dda1cabe21c5b

[l-404-l]

After a lot of testing and research, I think I fixed the issue with commit 03fb3d5

Tested on latest (2.0.0.8) still crashing.