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.

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

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

Closed
CptMoore opened this issue Feb 4, 2024 · 5 comments

Comments

@CptMoore
Copy link

CptMoore commented Feb 4, 2024

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
Copy link
Author

CptMoore commented Feb 6, 2024

I've added an upstream bug report under MonoMod/MonoMod#168

@CptMoore
Copy link
Author

CptMoore commented Feb 6, 2024

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
Copy link
Member

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

@CptMoore
Copy link
Author

CptMoore commented Feb 6, 2024

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

@Meivyn
Copy link
Contributor

Meivyn commented Mar 7, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants