BepInEx / HarmonyX

Harmony built on top of MonoMod.RuntimeDetours with additional features
MIT License
356 stars 44 forks source link

v2.11.0: patch with "ref string" on "static external" method produces garbage #107

Closed CptMoore closed 4 months ago

CptMoore commented 8 months ago

BattleTech, Unity 2018.4, Mono, 64bit

[HarmonyPatch(typeof(Assembly), nameof(Assembly.LoadFrom), typeof(string), typeof(bool))]
[HarmonyPrefix]
private static void LoadFrom_Prefix(ref string assemblyFile)

The patch can be applied, but when calling the patched method it does not work using the pre-release, it was fine with earlier versions. Either getting garbage in the assemblyFile string (emojis, chinese chars..) which leads to IO exceptions.. but also:

String conversion error: Illegal byte sequence encounted in the input. at (wrapper managed-to-native) System.Object.__icall_wrapper_ves_icall_string_new_wrapper(intptr)

Since it feels random (always happens but content and error differ based on if its even a valid string) it feels like some pointer pointing at the wrong memory region or so.

CptMoore commented 8 months ago

I've added an upstream bug report under https://github.com/MonoMod/MonoMod/issues/168

CptMoore commented 8 months ago

Upstream is not convinced its related to MonoMod detour and unfortunately I realized I haven't a clue what I am looking at in NativeDetourMethodPatcher. Any help in debugging this?

ManlyMarco commented 8 months ago

If no one else replies here try asking on the BepInEx discord server.

CptMoore commented 8 months ago

Old HarmonyX

[IL] Generated patch (System.Reflection.Assembly DMD<NativeDetour_Wrapper?0>?-804120320::NativeDetour_Wrapper?0(System.String,System.Boolean)): .locals init ( System.Reflection.Assembly V_0 System.Boolean V_1 ) IL_0000: ldc.i4.1 IL_0001: stloc V_1 IL_0005: ldarga assemblyFile IL_0009: call System.Void ModTekPreloader.Harmony12X.ShimInjectorPatches/AssemblyLoadPatches::LoadFrom_Prefix(System.String&) IL_000e: ldc.i4 0 IL_0013: call System.Delegate HarmonyLib.Public.Patching.NativeDetourMethodPatcher::GetTrampoline(System.Int32) IL_0018: ldarg assemblyFile IL_001c: ldarg refonly IL_0020: call System.Reflection.Assembly HarmonyDTFType1::Invoke(System.String,System.Boolean) IL_0025: br IL_002a IL_002a: ret

new HarmonyX:

Generated patch (System.Reflection.Assembly DMD<NativeHookProxy>?1282143488::NativeHookProxy(System.String,System.Boolean)): .locals init ( System.Reflection.Assembly V_0 System.Boolean V_1 ) IL_0000: ldc.i4.1 IL_0001: stloc V_1 IL_0005: ldarga IL_0009: call System.Void ModTekPreloader.Harmony12X.ShimInjectorPatches/AssemblyLoadPatches::LoadFrom_Prefix(System.String&) IL_000e: ldc.i4 2 IL_0013: ldc.i4 1654352256 IL_0018: call System.Object MonoMod.Utils.DynamicReferenceManager::GetValue(System.Int32,System.Int32) IL_001d: ldarg IL_0021: ldarg IL_0025: callvirt System.Reflection.Assembly HarmonyDTFType1::Invoke(System.String,System.Boolean) IL_002a: br IL_002f IL_002f: ret

Meivyn commented 7 months ago

The problem seems to be the use of NativeDetour again when it shouldn't be used. I'll see if I can fix that and will just open a PR with the fix for Assembly.GetExecutingAssembly as well.