HarmonyX icon indicating copy to clipboard operation
HarmonyX copied to clipboard

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

Open CptMoore opened this issue 1 year ago • 5 comments
trafficstars

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 avatar Feb 04 '24 19:02 CptMoore

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

CptMoore avatar Feb 06 '24 07:02 CptMoore

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?

CptMoore avatar Feb 06 '24 19:02 CptMoore

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

ManlyMarco avatar Feb 06 '24 21:02 ManlyMarco

Old HarmonyX

[IL] Generated patch (System.Reflection.Assembly DMD<NativeDetour_WrapperSystem.Reflection.Assembly::LoadFrom?0>?-804120320::NativeDetour_WrapperSystem.Reflection.Assembly::LoadFrom?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<NativeHookProxySystem.Reflection.Assembly:LoadFrom>?1282143488::NativeHookProxySystem.Reflection.Assembly:LoadFrom(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

CptMoore avatar Feb 06 '24 22:02 CptMoore

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.

Meivyn avatar Mar 07 '24 02:03 Meivyn