IL2CPPDetourMethodPatcher: Improve struct handling on x64

* Add correct return buffer when return value is a struct
* Use IntPtr for all parameters on x64 instead of a FixedStruct

BepInEx.IL2CPP/Hook/IL2CPPDetourMethodPatcher.cs

@@ -4,19 +4,15 @@ using System.Linq;
 using System.Reflection;
 using System.Reflection.Emit;
 using System.Runtime.InteropServices;
-using System.Text;
 using BepInEx.Logging;
 using HarmonyLib;
 using HarmonyLib.Public.Patching;
-using Mono.Cecil.Cil;
 using MonoMod.Cil;
 using MonoMod.RuntimeDetour;
 using MonoMod.Utils;
 using UnhollowerBaseLib;
 using UnhollowerBaseLib.Runtime;
 using UnhollowerBaseLib.Runtime.VersionSpecific.MethodInfo;
-using OpCode = System.Reflection.Emit.OpCode;
-using OpCodes = System.Reflection.Emit.OpCodes;
 using ValueType = Il2CppSystem.ValueType;
 using Void = Il2CppSystem.Void;
 
@@ -39,7 +35,6 @@ public unsafe class IL2CPPDetourMethodPatcher : MethodPatcher
     private static readonly MethodInfo ReportExceptionMethodInfo
         = AccessTools.Method(typeof(IL2CPPDetourMethodPatcher), nameof(ReportException));
 
-
     private static readonly ManualLogSource DetourLogger = Logger.CreateLogSource("Detour");
 
     // Map each value type to correctly sized store opcode to prevent memory overwrite
@@ -100,7 +95,7 @@ public unsafe class IL2CPPDetourMethodPatcher : MethodPatcher
 
             // Get the native MethodInfo struct for the target method
             originalNativeMethodInfo =
-                UnityVersionHandler.Wrap((Il2CppMethodInfo*) (IntPtr) methodField.GetValue(null));
+                UnityVersionHandler.Wrap((Il2CppMethodInfo*)(IntPtr)methodField.GetValue(null));
 
             // Create a trampoline from the original target method
             var trampolinePtr =
@@ -206,19 +201,35 @@ public unsafe class IL2CPPDetourMethodPatcher : MethodPatcher
     {
         // managedParams are the unhollower types used on the managed side
         // unmanagedParams are IntPtr references that are used by IL2CPP compiled assembly
-        var paramStartIndex = Original.IsStatic ? 0 : 1;
+        var paramStartIndex = 0;
+
+        var managedReturnType = AccessTools.GetReturnedType(Original);
+        var hasReturnBuffer = managedReturnType.IsSubclassOf(typeof(ValueType)) &&
+                              PlatformHelper.Is(Platform.Bits64);
+        if (hasReturnBuffer)
+            // C compilers seem to return values larger than 64 bits by allocating a return buffer on caller's side and passing it as the first parameter
+            // TODO: Handle ARM
+            // TODO: Check if this applies to values other than structs
+            // TODO: Check if we can use the dummy struct generated by GetFixedSizeStructType() so that mono's marshaller can handle this
+            paramStartIndex++;
+
+        if (!Original.IsStatic)
+            paramStartIndex++;
+
         var managedParams = Original.GetParameters().Select(x => x.ParameterType).ToArray();
         var unmanagedParams =
             new Type[managedParams.Length + paramStartIndex +
-                     1]; // +1 for thisptr if needed, +1 for methodInfo at the end
+                     1]; // +1 for methodInfo at the end
 
-        if (!Original.IsStatic)
+        if (hasReturnBuffer)
+            // With GCC the return buffer seems to be the first param, same is likely with other compilers too
             unmanagedParams[0] = typeof(IntPtr);
+        if (!Original.IsStatic)
+            unmanagedParams[paramStartIndex - 1] = typeof(IntPtr);
         unmanagedParams[^1] = typeof(Il2CppMethodInfo*);
         Array.Copy(managedParams.Select(ConvertManagedTypeToIL2CPPType).ToArray(), 0,
                    unmanagedParams, paramStartIndex, managedParams.Length);
 
-        var managedReturnType = AccessTools.GetReturnedType(Original);
         var unmanagedReturnType = ConvertManagedTypeToIL2CPPType(managedReturnType);
 
         var dmd = new DynamicMethodDefinition("(il2cpp -> managed) " + Original.Name,
@@ -234,7 +245,7 @@ public unsafe class IL2CPPDetourMethodPatcher : MethodPatcher
         var indirectVariables = new LocalBuilder[managedParams.Length];
 
         if (!Original.IsStatic)
-            EmitConvertArgumentToManaged(il, 0, Original.DeclaringType, out _);
+            EmitConvertArgumentToManaged(il, paramStartIndex - 1, Original.DeclaringType, out _);
         for (var i = 0; i < managedParams.Length; ++i)
             EmitConvertArgumentToManaged(il, i + paramStartIndex, managedParams[i], out indirectVariables[i]);
 
@@ -270,8 +281,26 @@ public unsafe class IL2CPPDetourMethodPatcher : MethodPatcher
         // Convert the return value back to an IL2CPP friendly type (if there was a return value), and then return
         if (managedReturnVariable != null)
         {
-            il.Emit(OpCodes.Ldloc, managedReturnVariable);
-            EmitConvertManagedTypeToIL2CPP(il, managedReturnType);
+            if (hasReturnBuffer)
+            {
+                uint align = 0;
+                var size = UnhollowerBaseLib.IL2CPP.il2cpp_class_value_size(Il2CppTypeToClassPointer(managedReturnType),
+                                                                            ref align);
+
+                il.Emit(OpCodes.Ldarg_0);
+                il.Emit(OpCodes.Ldloc, managedReturnVariable);
+                il.Emit(OpCodes.Call, ObjectBaseToPtrMethodInfo);
+                il.Emit(OpCodes.Ldc_I4, size);
+                il.Emit(OpCodes.Cpblk);
+
+                // Return the same pointer to the return buffer
+                il.Emit(OpCodes.Ldarg_0);
+            }
+            else
+            {
+                il.Emit(OpCodes.Ldloc, managedReturnVariable);
+                EmitConvertManagedTypeToIL2CPP(il, managedReturnType);
+            }
         }
 
         il.Emit(OpCodes.Ret);
@@ -322,8 +351,8 @@ public unsafe class IL2CPPDetourMethodPatcher : MethodPatcher
     {
         if (type == typeof(void))
             return Il2CppClassPointerStore<Void>.NativeClassPtr;
-        return (IntPtr) typeof(Il2CppClassPointerStore<>).MakeGenericType(type).GetField("NativeClassPtr")
-                                                         .GetValue(null);
+        return (IntPtr)typeof(Il2CppClassPointerStore<>).MakeGenericType(type).GetField("NativeClassPtr")
+                                                        .GetValue(null);
     }
 
     private static void EmitConvertArgumentToManaged(ILGenerator il,
@@ -333,13 +362,14 @@ public unsafe class IL2CPPDetourMethodPatcher : MethodPatcher
     {
         variable = null;
 
-        if (managedParamType.IsSubclassOf(typeof(ValueType)) && !PlatformHelper.Is(Platform.Bits64))
+        if (managedParamType.IsSubclassOf(typeof(ValueType)))
         {
             // Box struct into object first before conversion
-            // This will likely incur struct copying down the line, but it shouldn't be a massive loss
             il.Emit(OpCodes.Ldc_I8, Il2CppTypeToClassPointer(managedParamType).ToInt64());
             il.Emit(OpCodes.Conv_I);
-            il.Emit(OpCodes.Ldarga_S, argIndex);
+            // On x64, struct is always a pointer but it is a non-pointer on x86
+            // We don't handle byref structs on x86 yet but we're yet to encounter those
+            il.Emit(PlatformHelper.Is(Platform.Bits64) ? OpCodes.Ldarg : OpCodes.Ldarga_S, argIndex);
             il.Emit(OpCodes.Call,
                     AccessTools.Method(typeof(UnhollowerBaseLib.IL2CPP),
                                        nameof(UnhollowerBaseLib.IL2CPP.il2cpp_value_box)));