From 17c93bd590055faa40161125e444ed3cd3448549 Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Thu, 7 Jul 2022 14:53:14 +0300 Subject: [PATCH 01/19] [mono] Add icalls to be used by new invoke machinery --- .../src/System/RuntimeMethodHandle.cs | 21 +++++- .../src/System/RuntimeType.Mono.cs | 10 +++ src/mono/mono/metadata/icall-def.h | 5 +- src/mono/mono/metadata/icall.c | 66 +++++++++++++++++++ 4 files changed, 99 insertions(+), 3 deletions(-) diff --git a/src/mono/System.Private.CoreLib/src/System/RuntimeMethodHandle.cs b/src/mono/System.Private.CoreLib/src/System/RuntimeMethodHandle.cs index f1b30c29f7df5e..4280686a3c4f67 100644 --- a/src/mono/System.Private.CoreLib/src/System/RuntimeMethodHandle.cs +++ b/src/mono/System.Private.CoreLib/src/System/RuntimeMethodHandle.cs @@ -90,7 +90,24 @@ internal bool IsNullHandle() return value == IntPtr.Zero; } - // Temporary placeholder until Mono adds support for supporting boxing true Nullables. - internal static object? ReboxFromNullable(object? src) => src; + [MethodImplAttribute(MethodImplOptions.InternalCall)] + private static extern void ReboxFromNullable (object? src, ObjectHandleOnStack res); + + [MethodImplAttribute(MethodImplOptions.InternalCall)] + private static extern void ReboxToNullable (object? src, QCallTypeHandle destNullableType, ObjectHandleOnStack res); + + internal static object ReboxFromNullable(object? src) + { + object? res = null; + ReboxFromNullable(src, ObjectHandleOnStack.Create(ref res)); + return res!; + } + + internal static object ReboxToNullable(object? src, RuntimeType destNullableType) + { + object? res = null; + ReboxToNullable(src, new QCallTypeHandle(ref destNullableType), ObjectHandleOnStack.Create(ref res)); + return res!; + } } } diff --git a/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs b/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs index cbedc793bb2e7e..9f8fac739ceb44 100644 --- a/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs +++ b/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs @@ -2162,6 +2162,16 @@ public override string ToString() [MethodImplAttribute(MethodImplOptions.InternalCall)] private static extern object CreateInstanceInternal(QCallTypeHandle type); + [MethodImpl(MethodImplOptions.InternalCall)] + private static extern void AllocateValueType(QCallTypeHandle type, object? value, ObjectHandleOnStack res); + + internal static object AllocateValueType(RuntimeType type, object? value) + { + object? res = null; + AllocateValueType(new QCallTypeHandle(ref type), value, ObjectHandleOnStack.Create(ref res)); + return res!; + } + [MethodImplAttribute(MethodImplOptions.InternalCall)] private static extern void GetDeclaringMethod(QCallTypeHandle type, ObjectHandleOnStack res); diff --git a/src/mono/mono/metadata/icall-def.h b/src/mono/mono/metadata/icall-def.h index 71670c578440af..03e680f153f8ae 100644 --- a/src/mono/mono/metadata/icall-def.h +++ b/src/mono/mono/metadata/icall-def.h @@ -474,8 +474,11 @@ HANDLES_REUSE_WRAPPER(RFH_2, "SetValueInternal", ves_icall_RuntimeFieldInfo_SetV ICALL_TYPE(MHAN, "System.RuntimeMethodHandle", MHAN_1) HANDLES(MHAN_1, "GetFunctionPointer", ves_icall_RuntimeMethodHandle_GetFunctionPointer, gpointer, 1, (MonoMethod_ptr)) +HANDLES(MAHN_3, "ReboxFromNullable", ves_icall_RuntimeMethodHandle_ReboxFromNullable, void, 2, (MonoObject, MonoObjectHandleOnStack)) +HANDLES(MAHN_2, "ReboxToNullable", ves_icall_RuntimeMethodHandle_ReboxToNullable, void, 3, (MonoObject, MonoQCallTypeHandle, MonoObjectHandleOnStack)) -ICALL_TYPE(RT, "System.RuntimeType", RT_1) +ICALL_TYPE(RT, "System.RuntimeType", RT_31) +HANDLES(RT_31, "AllocateValueType", ves_icall_System_RuntimeType_AllocateValueType, void, 3, (MonoQCallTypeHandle, MonoObject, MonoObjectHandleOnStack)) HANDLES(RT_1, "CreateInstanceInternal", ves_icall_System_RuntimeType_CreateInstanceInternal, MonoObject, 1, (MonoQCallTypeHandle)) HANDLES(RT_2, "GetConstructors_native", ves_icall_RuntimeType_GetConstructors_native, GPtrArray_ptr, 2, (MonoQCallTypeHandle, guint32)) HANDLES(RT_30, "GetCorrespondingInflatedMethod", ves_icall_RuntimeType_GetCorrespondingInflatedMethod, MonoReflectionMethod, 2, (MonoQCallTypeHandle, MonoReflectionMethod)) diff --git a/src/mono/mono/metadata/icall.c b/src/mono/mono/metadata/icall.c index dc3a22f72c7cd8..0a33b40e8d2c78 100644 --- a/src/mono/mono/metadata/icall.c +++ b/src/mono/mono/metadata/icall.c @@ -1820,6 +1820,49 @@ ves_icall_RuntimeTypeHandle_IsInstanceOfType (MonoQCallTypeHandle type_handle, M return !MONO_HANDLE_IS_NULL (inst); } +void +ves_icall_RuntimeMethodHandle_ReboxToNullable (MonoObjectHandle obj, MonoQCallTypeHandle type_handle, MonoObjectHandleOnStack res, MonoError *error) +{ + MonoType *type = type_handle.type; + MonoClass *klass = mono_class_from_mono_type_internal (type); + + mono_class_init_checked (klass, error); + goto_if_nok (error, error_ret); + + MonoObject *obj_res = mono_object_new_checked (klass, error); + goto_if_nok (error, error_ret); + gpointer dest = mono_object_unbox_internal (obj_res); + + mono_nullable_init (dest, MONO_HANDLE_RAW (obj), klass); + + HANDLE_ON_STACK_SET (res, obj_res); + return; +error_ret: + HANDLE_ON_STACK_SET (res, NULL); +} + +void +ves_icall_RuntimeMethodHandle_ReboxFromNullable (MonoObjectHandle obj, MonoObjectHandleOnStack res, MonoError *error) +{ + if (MONO_HANDLE_IS_NULL (obj)) { + HANDLE_ON_STACK_SET (res, NULL); + return; + } + + MonoVTable *vtable = MONO_HANDLE_GETVAL (obj, vtable); + MonoClass *klass = vtable->klass; + MonoObject *obj_res; + + if (!mono_class_is_nullable (klass)) { + obj_res = MONO_HANDLE_RAW (obj); + } else { + gpointer vbuf = mono_object_unbox_internal (MONO_HANDLE_RAW (obj)); + obj_res = mono_nullable_box (vbuf, klass, error); + } + + HANDLE_ON_STACK_SET (res, obj_res); +} + guint32 ves_icall_RuntimeTypeHandle_GetAttributes (MonoQCallTypeHandle type_handle) { @@ -6157,6 +6200,29 @@ ves_icall_System_RuntimeType_CreateInstanceInternal (MonoQCallTypeHandle type_ha return mono_object_new_handle (klass, error); } +/* Only used for value types */ +void +ves_icall_System_RuntimeType_AllocateValueType (MonoQCallTypeHandle type_handle, MonoObjectHandle value_h, MonoObjectHandleOnStack res, MonoError *error) +{ + MonoType *type = type_handle.type; + MonoClass *klass = mono_class_from_mono_type_internal (type); + + mono_class_init_checked (klass, error); + goto_if_nok (error, error_ret); + + MonoObject *obj_res = mono_object_new_checked (klass, error); + goto_if_nok (error, error_ret); + + MonoObject *value = MONO_HANDLE_RAW (value_h); + if (value) + mono_value_copy_internal (mono_object_unbox_internal (obj_res), mono_object_unbox_internal (value), klass); + + HANDLE_ON_STACK_SET (res, obj_res); + return; +error_ret: + HANDLE_ON_STACK_SET (res, NULL); +} + MonoReflectionMethodHandle ves_icall_RuntimeMethodInfo_get_base_method (MonoReflectionMethodHandle m, MonoBoolean definition, MonoError *error) { From c6b7015e9e6cccc7748c81ffb8ddfaa747e72e78 Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Thu, 7 Jul 2022 14:54:17 +0300 Subject: [PATCH 02/19] [mono] Fix IsInstanceOfType check --- src/mono/mono/metadata/icall.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/mono/mono/metadata/icall.c b/src/mono/mono/metadata/icall.c index 0a33b40e8d2c78..bf0e3085a23223 100644 --- a/src/mono/mono/metadata/icall.c +++ b/src/mono/mono/metadata/icall.c @@ -1812,6 +1812,10 @@ guint32 ves_icall_RuntimeTypeHandle_IsInstanceOfType (MonoQCallTypeHandle type_handle, MonoObjectHandle obj, MonoError *error) { MonoType *type = type_handle.type; + + if (m_type_is_byref (type)) + return FALSE; + MonoClass *klass = mono_class_from_mono_type_internal (type); mono_class_init_checked (klass, error); return_val_if_nok (error, FALSE); From 6c2169fc9f75e3dc79b4de169ed3611db21ca8a5 Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Thu, 7 Jul 2022 15:57:03 +0300 Subject: [PATCH 03/19] [mono] Reuse most of the managed invoke path used by coreclr --- .../src/System/RuntimeHandles.cs | 13 -- .../src/System/RuntimeType.CoreCLR.cs | 206 +--------------- .../System/Reflection/ConstructorInvoker.cs | 10 - .../src/System/Reflection/MethodBase.cs | 2 - .../src/System/Reflection/MethodInvoker.cs | 6 - .../Reflection/RuntimeConstructorInfo.cs | 16 -- .../System/Reflection/RuntimeMethodInfo.cs | 8 - .../src/System/RuntimeType.cs | 220 ++++++++++++++++++ .../Reflection/ConstructorInvoker.Mono.cs | 29 +-- .../System/Reflection/MethodInvoker.Mono.cs | 52 +---- .../Reflection/RuntimeMethodInfo.Mono.cs | 8 +- .../Runtime/InteropServices/Marshal.Mono.cs | 13 +- .../src/System/RuntimeType.Mono.cs | 87 +------ 13 files changed, 257 insertions(+), 413 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/RuntimeHandles.cs b/src/coreclr/System.Private.CoreLib/src/System/RuntimeHandles.cs index 1959209bd7cc41..f1dfa36b702379 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/RuntimeHandles.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/RuntimeHandles.cs @@ -141,19 +141,6 @@ internal static bool IsByRef(RuntimeType type) return corElemType == CorElementType.ELEMENT_TYPE_BYREF; } - internal static bool TryGetByRefElementType(RuntimeType type, [NotNullWhen(true)] out RuntimeType? elementType) - { - CorElementType corElemType = GetCorElementType(type); - if (corElemType == CorElementType.ELEMENT_TYPE_BYREF) - { - elementType = GetElementType(type); - return true; - } - - elementType = null; - return false; - } - internal static bool IsPointer(RuntimeType type) { CorElementType corElemType = GetCorElementType(type); diff --git a/src/coreclr/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs index fdfc88b419e4a0..a60920aa83c20a 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs @@ -3582,180 +3582,10 @@ public override Type MakeArrayType(int rank) [MethodImpl(MethodImplOptions.InternalCall)] private static extern object AllocateValueType(RuntimeType type, object? value); - private enum CheckValueStatus - { - Success = 0, - ArgumentException, - NotSupported_ByRefLike - } - - /// - /// Verify and optionally convert the value for special cases. - /// - /// True if is a value type, False otherwise - internal bool CheckValue( - ref object? value, - ref ParameterCopyBackAction copyBack, - Binder? binder, - CultureInfo? culture, - BindingFlags invokeAttr) - { - // Already fast-pathed by the caller. - Debug.Assert(!ReferenceEquals(value?.GetType(), this)); - - // Since this cannot be a generic parameter, we use RuntimeTypeHandle.IsValueType here - // because it is faster than IsValueType - Debug.Assert(!IsGenericParameter); - - // Fast path to whether a value can be assigned without conversion. - if (IsInstanceOfType(value)) - { - if (IsNullableOfT) - { - // Pass as a true boxed Nullable, not as a T or null. - value = RuntimeMethodHandle.ReboxToNullable(value, this); - return true; - } - - // Other value types won't get here since Type equality was previous checked. - Debug.Assert(!RuntimeTypeHandle.IsValueType(this)); - - return false; - } - - bool isValueType; - CheckValueStatus result = TryChangeType(ref value, ref copyBack, out isValueType); - if (result == CheckValueStatus.Success) - { - return isValueType; - } - - if (result == CheckValueStatus.ArgumentException && (invokeAttr & BindingFlags.ExactBinding) == 0) - { - Debug.Assert(value != null); - - // Use the binder - if (binder != null && binder != DefaultBinder) - { - value = binder.ChangeType(value, this, culture); - if (IsInstanceOfType(value)) - { - if (IsNullableOfT) - { - // Pass as a true boxed Nullable, not as a T or null. - value = RuntimeMethodHandle.ReboxToNullable(value, this); - copyBack = ParameterCopyBackAction.CopyNullable; - } - else - { - copyBack = ParameterCopyBackAction.Copy; - } - - return IsValueType; // Note the call to IsValueType, not the variable. - } - - result = TryChangeType(ref value, ref copyBack, out isValueType); - if (result == CheckValueStatus.Success) - { - return isValueType; - } - } - } - - switch (result) - { - case CheckValueStatus.ArgumentException: - throw new ArgumentException(SR.Format(SR.Arg_ObjObjEx, value?.GetType(), this)); - case CheckValueStatus.NotSupported_ByRefLike: - throw new NotSupportedException(SR.NotSupported_ByRefLike); - } - - Debug.Fail("Error result not expected"); - return false; - } - - private CheckValueStatus TryChangeType( - ref object? value, - ref ParameterCopyBackAction copyBack, + private CheckValueStatus TryChangeTypeSpecial( + ref object value, out bool isValueType) { - RuntimeType? sigElementType; - if (RuntimeTypeHandle.TryGetByRefElementType(this, out sigElementType)) - { - copyBack = ParameterCopyBackAction.Copy; - Debug.Assert(!sigElementType.IsGenericParameter); - - if (sigElementType.IsInstanceOfType(value)) - { - isValueType = RuntimeTypeHandle.IsValueType(sigElementType); - if (isValueType) - { - if (sigElementType.IsNullableOfT) - { - // Pass as a true boxed Nullable, not as a T or null. - value = RuntimeMethodHandle.ReboxToNullable(value, sigElementType); - copyBack = ParameterCopyBackAction.CopyNullable; - } - else - { - // Make a copy to prevent the boxed instance from being directly modified by the method. - value = AllocateValueType(sigElementType, value); - } - } - - return CheckValueStatus.Success; - } - - if (value == null) - { - isValueType = RuntimeTypeHandle.IsValueType(sigElementType); - if (!isValueType) - { - // Normally we don't get here since 'null' was previosuly checked, but due to binders we can. - return CheckValueStatus.Success; - } - - if (sigElementType.IsByRefLike) - { - return CheckValueStatus.NotSupported_ByRefLike; - } - - // Allocate default. - value = AllocateValueType(sigElementType, value: null); - copyBack = sigElementType.IsNullableOfT ? ParameterCopyBackAction.CopyNullable : ParameterCopyBackAction.Copy; - return CheckValueStatus.Success; - } - - isValueType = false; - return CheckValueStatus.ArgumentException; - } - - if (value == null) - { - isValueType = RuntimeTypeHandle.IsValueType(this); - if (!isValueType) - { - // Normally we don't get here since 'null' was previosuly checked, but due to binders we can. - return CheckValueStatus.Success; - } - - if (IsByRefLike) - { - return CheckValueStatus.NotSupported_ByRefLike; - } - - // Allocate default. - value = AllocateValueType(this, value: null); - return CheckValueStatus.Success; - } - - // Check the strange ones courtesy of reflection: - // - Implicit cast between primitives - // - Enum treated as underlying type - // - Pointer (*) types to IntPtr (if dest is IntPtr) - // - System.Reflection.Pointer to appropriate pointer (*) type (if dest is pointer type) - if (IsPointer || IsEnum || IsPrimitive) - { Pointer? pointer = value as Pointer; RuntimeType srcType = pointer != null ? pointer.GetPointerType() : (RuntimeType)value.GetType(); @@ -3781,38 +3611,6 @@ private CheckValueStatus TryChangeType( isValueType = true; return CheckValueStatus.Success; - } - - isValueType = false; - return CheckValueStatus.ArgumentException; - } - - internal bool TryByRefFastPath(ref object arg, ref bool isValueType) - { - if (RuntimeTypeHandle.TryGetByRefElementType(this, out RuntimeType? sigElementType) && - ReferenceEquals(sigElementType, arg.GetType())) - { - isValueType = sigElementType.IsValueType; - if (isValueType) - { - // Make a copy to prevent the boxed instance from being directly modified by the method. - arg = AllocateValueType(sigElementType, arg); - } - - return true; - } - - return false; - } - - internal static CorElementType GetUnderlyingType(RuntimeType type) - { - if (type.IsActualEnum) - { - type = (RuntimeType)Enum.GetUnderlyingType(type); - } - - return RuntimeTypeHandle.GetCorElementType(type); } #endregion diff --git a/src/libraries/System.Private.CoreLib/src/System/Reflection/ConstructorInvoker.cs b/src/libraries/System.Private.CoreLib/src/System/Reflection/ConstructorInvoker.cs index d5f173eac18708..9b581a305afdef 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Reflection/ConstructorInvoker.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Reflection/ConstructorInvoker.cs @@ -10,17 +10,14 @@ internal sealed partial class ConstructorInvoker { private readonly RuntimeConstructorInfo _method; -#if !MONO // Temporary until Mono is updated. private bool _invoked; private bool _strategyDetermined; private InvokerEmitUtil.InvokeFunc? _invokeFunc; -#endif public ConstructorInvoker(RuntimeConstructorInfo constructorInfo) { _method = constructorInfo; -#if !MONO // Temporary until Mono is updated. if (LocalAppContextSwitches.ForceInterpretedInvoke && !LocalAppContextSwitches.ForceEmitInvoke) { // Always use the native invoke; useful for testing. @@ -31,13 +28,9 @@ public ConstructorInvoker(RuntimeConstructorInfo constructorInfo) // Always use emit invoke (if IsDynamicCodeCompiled == true); useful for testing. _invoked = true; } -#endif } [MethodImpl(MethodImplOptions.AggressiveInlining)] -#if MONO // Temporary until Mono is updated. - public unsafe object? InlinedInvoke(object? obj, Span args, BindingFlags invokeAttr) => InterpretedInvoke(obj, args, invokeAttr); -#else public unsafe object? InlinedInvoke(object? obj, IntPtr* args, BindingFlags invokeAttr) { if (_invokeFunc != null && (invokeAttr & BindingFlags.DoNotWrapExceptions) != 0 && obj == null) @@ -46,9 +39,7 @@ public ConstructorInvoker(RuntimeConstructorInfo constructorInfo) } return Invoke(obj, args, invokeAttr); } -#endif -#if !MONO // Temporary until Mono is updated. [DebuggerStepThrough] [DebuggerHidden] private unsafe object? Invoke(object? obj, IntPtr* args, BindingFlags invokeAttr) @@ -102,6 +93,5 @@ public ConstructorInvoker(RuntimeConstructorInfo constructorInfo) return ret; } -#endif } } diff --git a/src/libraries/System.Private.CoreLib/src/System/Reflection/MethodBase.cs b/src/libraries/System.Private.CoreLib/src/System/Reflection/MethodBase.cs index 98069a70ef55b4..2ab4287267048d 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Reflection/MethodBase.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Reflection/MethodBase.cs @@ -239,14 +239,12 @@ BindingFlags invokeAttr #pragma warning disable 8500 if (isValueType) { -#if !MONO // Temporary until Mono is updated. Debug.Assert(arg != null); Debug.Assert( arg.GetType() == sigType || (sigType.IsPointer && (arg.GetType() == typeof(IntPtr) || arg.GetType() == typeof(UIntPtr))) || (sigType.IsByRef && arg.GetType() == RuntimeTypeHandle.GetElementType(sigType)) || ((sigType.IsEnum || arg.GetType().IsEnum) && RuntimeType.GetUnderlyingType((RuntimeType)arg.GetType()) == RuntimeType.GetUnderlyingType(sigType))); -#endif ByReference valueTypeRef = ByReference.Create(ref copyOfParameters[i]!.GetRawData()); *(ByReference*)(byrefParameters + i) = valueTypeRef; } diff --git a/src/libraries/System.Private.CoreLib/src/System/Reflection/MethodInvoker.cs b/src/libraries/System.Private.CoreLib/src/System/Reflection/MethodInvoker.cs index 3adbad39eb21cc..a33ca063fa8777 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Reflection/MethodInvoker.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Reflection/MethodInvoker.cs @@ -11,9 +11,6 @@ internal sealed partial class MethodInvoker private readonly MethodBase _method; [MethodImpl(MethodImplOptions.AggressiveInlining)] -#if MONO // Temporary until Mono is updated. - public unsafe object? InlinedInvoke(object? obj, Span args, BindingFlags invokeAttr) => InterpretedInvoke(obj, args, invokeAttr); -#else public unsafe object? InlinedInvoke(object? obj, IntPtr* args, BindingFlags invokeAttr) { if (_invokeFunc != null && (invokeAttr & BindingFlags.DoNotWrapExceptions) != 0) @@ -22,9 +19,7 @@ internal sealed partial class MethodInvoker } return Invoke(obj, args, invokeAttr); } -#endif -#if !MONO // Temporary until Mono is updated. private bool _invoked; private bool _strategyDetermined; private InvokerEmitUtil.InvokeFunc? _invokeFunc; @@ -80,6 +75,5 @@ internal sealed partial class MethodInvoker return ret; } -#endif } } diff --git a/src/libraries/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.cs b/src/libraries/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.cs index 1fd9c177ddb8a0..a16f6f48531ae9 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.cs @@ -160,11 +160,7 @@ internal void ThrowNoInvokeException() culture, invokeAttr); -#if MONO // Temporary until Mono is updated. - Invoker.InlinedInvoke(obj, copyOfParameters, invokeAttr); -#else Invoker.InlinedInvoke(obj, pByRefStorage, invokeAttr); -#endif // Copy modified values out. This should be done only with ByRef or Type.Missing parameters. for (int i = 0; i < argCount; i++) @@ -230,11 +226,7 @@ private static unsafe void InvokeWithManyArguments( culture, invokeAttr); -#if MONO // Temporary until Mono is updated. - ci.Invoker.InlinedInvoke(obj, copyOfParameters, invokeAttr); -#else ci.Invoker.InlinedInvoke(obj, pByRefStorage, invokeAttr); -#endif } finally { @@ -315,11 +307,7 @@ public override object Invoke(BindingFlags invokeAttr, Binder? binder, object?[] culture, invokeAttr); -#if MONO // Temporary until Mono is updated. - retValue = Invoker.InlinedInvoke(obj: null, copyOfParameters, invokeAttr); -#else retValue = Invoker.InlinedInvoke(obj: null, pByRefStorage, invokeAttr); -#endif // Copy modified values out. This should be done only with ByRef or Type.Missing parameters. for (int i = 0; i < argCount; i++) @@ -388,11 +376,7 @@ public override object Invoke(BindingFlags invokeAttr, Binder? binder, object?[] culture, invokeAttr); -#if MONO // Temporary until Mono is updated. - retValue = ci.Invoker.InlinedInvoke(obj: null, copyOfParameters, invokeAttr); -#else retValue = ci.Invoker.InlinedInvoke(obj: null, pByRefStorage, invokeAttr); -#endif } finally { diff --git a/src/libraries/System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.cs b/src/libraries/System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.cs index bf534c58e70ed4..28d530bcfc6c20 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.cs @@ -157,11 +157,7 @@ private void ThrowNoInvokeException() culture, invokeAttr); -#if MONO // Temporary until Mono is updated. - retValue = Invoker.InlinedInvoke(obj, copyOfParameters, invokeAttr); -#else retValue = Invoker.InlinedInvoke(obj, pByRefStorage, invokeAttr); -#endif // Copy modified values out. This should be done only with ByRef or Type.Missing parameters. for (int i = 0; i < argCount; i++) @@ -228,11 +224,7 @@ private void ThrowNoInvokeException() culture, invokeAttr); -#if MONO // Temporary until Mono is updated. - retValue = mi.Invoker.InlinedInvoke(obj, copyOfParameters, invokeAttr); -#else retValue = mi.Invoker.InlinedInvoke(obj, pByRefStorage, invokeAttr); -#endif } finally { diff --git a/src/libraries/System.Private.CoreLib/src/System/RuntimeType.cs b/src/libraries/System.Private.CoreLib/src/System/RuntimeType.cs index 7472df80e998e7..bb257e676a4f07 100644 --- a/src/libraries/System.Private.CoreLib/src/System/RuntimeType.cs +++ b/src/libraries/System.Private.CoreLib/src/System/RuntimeType.cs @@ -827,5 +827,225 @@ internal static void SanityCheckGenericArguments(RuntimeType[] genericArguments, throw new ArgumentException( SR.Format(SR.Argument_NotEnoughGenArguments, genericArguments.Length, genericParameters.Length)); } + + internal static CorElementType GetUnderlyingType(RuntimeType type) + { + if (type.IsActualEnum) + { + type = (RuntimeType)Enum.GetUnderlyingType(type); + } + + return RuntimeTypeHandle.GetCorElementType(type); + } + + internal static bool TryGetByRefElementType(RuntimeType type, [NotNullWhen(true)] out RuntimeType? elementType) + { + CorElementType corElemType = RuntimeTypeHandle.GetCorElementType(type); + if (corElemType == CorElementType.ELEMENT_TYPE_BYREF) + { + elementType = RuntimeTypeHandle.GetElementType(type); + return true; + } + + elementType = null; + return false; + } + + private enum CheckValueStatus + { + Success = 0, + ArgumentException, + NotSupported_ByRefLike + } + + /// + /// Verify and optionally convert the value for special cases. + /// + /// True if is a value type, False otherwise + internal bool CheckValue( + ref object? value, + ref ParameterCopyBackAction copyBack, + Binder? binder, + CultureInfo? culture, + BindingFlags invokeAttr) + { + // Already fast-pathed by the caller. + Debug.Assert(!ReferenceEquals(value?.GetType(), this)); + + // Since this cannot be a generic parameter, we use RuntimeTypeHandle.IsValueType here + // because it is faster than IsValueType + Debug.Assert(!IsGenericParameter); + + // Fast path to whether a value can be assigned without conversion. + if (IsInstanceOfType(value)) + { + if (IsNullableOfT) + { + // Pass as a true boxed Nullable, not as a T or null. + value = RuntimeMethodHandle.ReboxToNullable(value, this); + return true; + } + + // Other value types won't get here since Type equality was previous checked. + Debug.Assert(!RuntimeTypeHandle.IsValueType(this)); + + return false; + } + + bool isValueType; + CheckValueStatus result = TryChangeType(ref value, ref copyBack, out isValueType); + if (result == CheckValueStatus.Success) + { + return isValueType; + } + + if (result == CheckValueStatus.ArgumentException && (invokeAttr & BindingFlags.ExactBinding) == 0) + { + Debug.Assert(value != null); + + // Use the binder + if (binder != null && binder != DefaultBinder) + { + value = binder.ChangeType(value, this, culture); + if (IsInstanceOfType(value)) + { + if (IsNullableOfT) + { + // Pass as a true boxed Nullable, not as a T or null. + value = RuntimeMethodHandle.ReboxToNullable(value, this); + copyBack = ParameterCopyBackAction.CopyNullable; + } + else + { + copyBack = ParameterCopyBackAction.Copy; + } + + return IsValueType; // Note the call to IsValueType, not the variable. + } + + result = TryChangeType(ref value, ref copyBack, out isValueType); + if (result == CheckValueStatus.Success) + { + return isValueType; + } + } + } + + switch (result) + { + case CheckValueStatus.ArgumentException: + throw new ArgumentException(SR.Format(SR.Arg_ObjObjEx, value?.GetType(), this)); + case CheckValueStatus.NotSupported_ByRefLike: + throw new NotSupportedException(SR.NotSupported_ByRefLike); + } + + Debug.Fail("Error result not expected"); + return false; + } + + private CheckValueStatus TryChangeType( + ref object? value, + ref ParameterCopyBackAction copyBack, + out bool isValueType) + { + RuntimeType? sigElementType; + if (TryGetByRefElementType(this, out sigElementType)) + { + copyBack = ParameterCopyBackAction.Copy; + Debug.Assert(!sigElementType.IsGenericParameter); + + if (sigElementType.IsInstanceOfType(value)) + { + isValueType = RuntimeTypeHandle.IsValueType(sigElementType); + if (isValueType) + { + if (sigElementType.IsNullableOfT) + { + // Pass as a true boxed Nullable, not as a T or null. + value = RuntimeMethodHandle.ReboxToNullable(value, sigElementType); + copyBack = ParameterCopyBackAction.CopyNullable; + } + else + { + // Make a copy to prevent the boxed instance from being directly modified by the method. + value = AllocateValueType(sigElementType, value); + } + } + + return CheckValueStatus.Success; + } + + if (value == null) + { + isValueType = RuntimeTypeHandle.IsValueType(sigElementType); + if (!isValueType) + { + // Normally we don't get here since 'null' was previosuly checked, but due to binders we can. + return CheckValueStatus.Success; + } + + if (sigElementType.IsByRefLike) + { + return CheckValueStatus.NotSupported_ByRefLike; + } + + // Allocate default. + value = AllocateValueType(sigElementType, value: null); + copyBack = sigElementType.IsNullableOfT ? ParameterCopyBackAction.CopyNullable : ParameterCopyBackAction.Copy; + return CheckValueStatus.Success; + } + + isValueType = false; + return CheckValueStatus.ArgumentException; + } + + if (value == null) + { + isValueType = RuntimeTypeHandle.IsValueType(this); + if (!isValueType) + { + // Normally we don't get here since 'null' was previosuly checked, but due to binders we can. + return CheckValueStatus.Success; + } + + if (IsByRefLike) + { + return CheckValueStatus.NotSupported_ByRefLike; + } + + // Allocate default. + value = AllocateValueType(this, value: null); + return CheckValueStatus.Success; + } + + // Check the strange ones courtesy of reflection: + // - Implicit cast between primitives + // - Enum treated as underlying type + // - Pointer (*) types to IntPtr (if dest is IntPtr) + // - System.Reflection.Pointer to appropriate pointer (*) type (if dest is pointer type) + if (IsPointer || IsEnum || IsPrimitive) + return TryChangeTypeSpecial(ref value, out isValueType); + + isValueType = false; + return CheckValueStatus.ArgumentException; + } + + internal bool TryByRefFastPath(ref object arg, ref bool isValueType) + { + if (TryGetByRefElementType(this, out RuntimeType? sigElementType) && + ReferenceEquals(sigElementType, arg.GetType())) + { + isValueType = sigElementType.IsValueType; + if (isValueType) + { + // Make a copy to prevent the boxed instance from being directly modified by the method. + arg = AllocateValueType(sigElementType, arg); + } + + return true; + } + + return false; + } } } diff --git a/src/mono/System.Private.CoreLib/src/System/Reflection/ConstructorInvoker.Mono.cs b/src/mono/System.Private.CoreLib/src/System/Reflection/ConstructorInvoker.Mono.cs index 49b13afc66511a..3bde0806a5c6ee 100644 --- a/src/mono/System.Private.CoreLib/src/System/Reflection/ConstructorInvoker.Mono.cs +++ b/src/mono/System.Private.CoreLib/src/System/Reflection/ConstructorInvoker.Mono.cs @@ -7,35 +7,10 @@ namespace System.Reflection { internal partial class ConstructorInvoker { - [MethodImpl(MethodImplOptions.AggressiveInlining)] - private unsafe object? InterpretedInvoke(object? obj, Span args, BindingFlags invokeAttr) + private unsafe object? InterpretedInvoke(object? obj, IntPtr *args) { Exception exc; - object? o; - - if ((invokeAttr & BindingFlags.DoNotWrapExceptions) == 0) - { - try - { - o = _method.InternalInvoke(obj, args, out exc); - } - catch (MethodAccessException) - { - throw; - } - catch (OverflowException) - { - throw; - } - catch (Exception e) - { - throw new TargetInvocationException(e); - } - } - else - { - o = _method.InternalInvoke(obj, args, out exc); - } + object? o = _method.InternalInvoke(obj, args, out exc); if (exc != null) throw exc; diff --git a/src/mono/System.Private.CoreLib/src/System/Reflection/MethodInvoker.Mono.cs b/src/mono/System.Private.CoreLib/src/System/Reflection/MethodInvoker.Mono.cs index 2ea61086c7a4f0..99df70de828fbb 100644 --- a/src/mono/System.Private.CoreLib/src/System/Reflection/MethodInvoker.Mono.cs +++ b/src/mono/System.Private.CoreLib/src/System/Reflection/MethodInvoker.Mono.cs @@ -11,51 +11,23 @@ public MethodInvoker(MethodBase method) { _method = method; -#if USE_NATIVE_INVOKE - // Always use the native invoke; useful for testing. - _strategyDetermined = true; -#elif USE_EMIT_INVOKE - // Always use emit invoke (if IsDynamicCodeCompiled == true); useful for testing. - _invoked = true; -#endif + if (LocalAppContextSwitches.ForceInterpretedInvoke && !LocalAppContextSwitches.ForceEmitInvoke) + { + // Always use the native invoke; useful for testing. + _strategyDetermined = true; + } + else if (LocalAppContextSwitches.ForceEmitInvoke && !LocalAppContextSwitches.ForceInterpretedInvoke) + { + // Always use emit invoke (if IsDynamicCodeCompiled == true); useful for testing. + _invoked = true; + } } - [MethodImpl(MethodImplOptions.AggressiveInlining)] - private unsafe object? InterpretedInvoke(object? obj, Span args, BindingFlags invokeAttr) + private unsafe object? InterpretedInvoke(object? obj, IntPtr *args) { Exception? exc; - object? o; - if ((invokeAttr & BindingFlags.DoNotWrapExceptions) == 0) - { - try - { - o = ((RuntimeMethodInfo)_method).InternalInvoke(obj, args, out exc); - } - catch (Mono.NullByRefReturnException) - { - throw new NullReferenceException(); - } - catch (OverflowException) - { - throw; - } - catch (Exception e) - { - throw new TargetInvocationException(e); - } - } - else - { - try - { - o = ((RuntimeMethodInfo)_method).InternalInvoke(obj, args, out exc); - } - catch (Mono.NullByRefReturnException) - { - throw new NullReferenceException(); - } - } + object? o = ((RuntimeMethodInfo)_method).InternalInvoke(obj, args, out exc); if (exc != null) throw exc; diff --git a/src/mono/System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.Mono.cs b/src/mono/System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.Mono.cs index 08f30f04fca829..6d43a9270910c0 100644 --- a/src/mono/System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.Mono.cs +++ b/src/mono/System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.Mono.cs @@ -140,7 +140,7 @@ internal static ParameterInfo GetReturnParameterInfo(RuntimeMethodInfo method) #region Sync with _MonoReflectionMethod in object-internals.h [StructLayout(LayoutKind.Sequential)] - internal sealed partial class RuntimeMethodInfo : MethodInfo + internal sealed unsafe partial class RuntimeMethodInfo : MethodInfo { #pragma warning disable 649 internal IntPtr mhandle; @@ -382,7 +382,7 @@ internal RuntimeType[] ArgumentTypes * Exceptions thrown by the called method propagate normally. */ [MethodImplAttribute(MethodImplOptions.InternalCall)] - internal extern object? InternalInvoke(object? obj, in Span parameters, out Exception? exc); + internal extern object? InternalInvoke(object? obj, IntPtr *args, out Exception? exc); public override RuntimeMethodHandle MethodHandle { @@ -710,7 +710,7 @@ public override IList GetCustomAttributesData() } #region Sync with _MonoReflectionMethod in object-internals.h [StructLayout(LayoutKind.Sequential)] - internal sealed partial class RuntimeConstructorInfo : ConstructorInfo + internal sealed unsafe partial class RuntimeConstructorInfo : ConstructorInfo { #pragma warning disable 649 internal IntPtr mhandle; @@ -816,7 +816,7 @@ private void InvokeClassConstructor() * to match the types of the method signature. */ [MethodImplAttribute(MethodImplOptions.InternalCall)] - internal extern object InternalInvoke(object? obj, in Span parameters, out Exception exc); + internal extern object InternalInvoke(object? obj, IntPtr *args, out Exception exc); public override RuntimeMethodHandle MethodHandle { diff --git a/src/mono/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.Mono.cs b/src/mono/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.Mono.cs index 8f3dca0db13bed..b626b4df3eb05b 100644 --- a/src/mono/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.Mono.cs +++ b/src/mono/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.Mono.cs @@ -118,9 +118,15 @@ public int GetHashCode((Type, string) key) private static Dictionary<(Type, string), ICustomMarshaler>? MarshalerInstanceCache; + private static unsafe void SetInvokeArgs(ref string cookie, IntPtr *params_byref) + { + ByReference objRef = ByReference.Create(ref cookie); + *(ByReference*)params_byref = objRef; + } + [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2070:UnrecognizedReflectionPattern", Justification = "Implementation detail of MarshalAs.CustomMarshaler")] - internal static ICustomMarshaler? GetCustomMarshalerInstance(Type type, string cookie) + internal static unsafe ICustomMarshaler? GetCustomMarshalerInstance(Type type, string cookie) { var key = (type, cookie); @@ -163,7 +169,10 @@ public int GetHashCode((Type, string) key) Exception? exc; try { - result = (ICustomMarshaler?)getInstanceMethod.InternalInvoke(null, new object[] { cookie }, out exc); + IntPtr byrefStorage = default; + IntPtr *pbyrefStorage = &byrefStorage; + SetInvokeArgs(ref cookie, pbyrefStorage); + result = (ICustomMarshaler?)getInstanceMethod.InternalInvoke(null, pbyrefStorage, out exc); } catch (Exception e) { diff --git a/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs b/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs index 9f8fac739ceb44..db2ecd2a079bc8 100644 --- a/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs +++ b/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs @@ -1648,83 +1648,12 @@ internal override FieldInfo GetField(FieldInfo fromNoninstanciated) } } - /// - /// Verify and optionally convert the value for special cases. - /// - /// Not yet implemented in Mono: True if the value should be considered a value type, False otherwise - internal bool CheckValue( - ref object? value, - ref ParameterCopyBackAction copyBack, - Binder? binder, - CultureInfo? culture, - BindingFlags invokeAttr) + // FIXME Reuse with coreclr + private CheckValueStatus TryChangeTypeSpecial( + ref object value, + out bool isValueType) { - // Already fast-pathed by the caller. - Debug.Assert(!ReferenceEquals(value?.GetType(), this)); - - copyBack = ParameterCopyBackAction.Copy; - - CheckValueStatus status = TryConvertToType(ref value); - if (status == CheckValueStatus.Success) - { - return true; - } - - if (status == CheckValueStatus.NotSupported_ByRefLike) - { - throw new NotSupportedException(SR.Format(SR.NotSupported_ByRefLike, value?.GetType(), this)); - } - - if ((invokeAttr & BindingFlags.ExactBinding) == BindingFlags.ExactBinding) - { - throw new ArgumentException(SR.Format(SR.Arg_ObjObjEx, value?.GetType(), this)); - } - - if (binder != null && binder != DefaultBinder) - { - value = binder.ChangeType(value!, this, culture); - return true; - } - - throw new ArgumentException(SR.Format(SR.Arg_ObjObjEx, value?.GetType(), this)); - } - - private enum CheckValueStatus - { - Success = 0, - ArgumentException, - NotSupported_ByRefLike - } - - private CheckValueStatus TryConvertToType(ref object? value) - { - if (IsInstanceOfType(value)) - return CheckValueStatus.Success; - - if (IsByRef) - { - Type elementType = GetElementType(); - if (elementType.IsByRefLike) - { - return CheckValueStatus.NotSupported_ByRefLike; - } - - if (value == null || elementType.IsInstanceOfType(value)) - { - return CheckValueStatus.Success; - } - } - - if (value == null) - { - if (IsByRefLike) - { - return CheckValueStatus.NotSupported_ByRefLike; - } - - return CheckValueStatus.Success; - } - + isValueType = true; if (IsEnum) { Type? type = Enum.GetUnderlyingType(this); @@ -1763,14 +1692,10 @@ private CheckValueStatus TryConvertToType(ref object? value) } } + isValueType = false; return CheckValueStatus.ArgumentException; } - // Stub method to allow for shared code with CoreClr. -#pragma warning disable CA1822, IDE0060 - internal bool TryByRefFastPath(ref object arg, ref bool isValueType) => false; -#pragma warning restore CA1822, IDE0060 - // Binder uses some incompatible conversion rules. For example // int value cannot be used with decimal parameter but in other // ways it's more flexible than normal convertor, for example From 5da8f5721db03b91aaacbc97adcb85bf6f0c1158 Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Fri, 22 Jul 2022 21:16:58 +0300 Subject: [PATCH 04/19] [mono] Implement new version of InternalInvoke icall This version doesn't receive a MonoArray of params as objects, rather a simple array of byrefs. There is also no need for special handling for nullables. The runtime invoke wrapper receives a boxed nullable, this conversion happens in managed code using ReboxFromNullable and ReboxToNullable. This change might have some side effects on other API making use of the runtime invoke wrapper! --- src/mono/mono/metadata/icall-def.h | 2 +- src/mono/mono/metadata/icall.c | 137 +++++++++++++++++-- src/mono/mono/metadata/marshal-lightweight.c | 43 +----- src/mono/mono/metadata/object-internals.h | 3 + src/mono/mono/metadata/object.c | 83 ++++++----- 5 files changed, 179 insertions(+), 89 deletions(-) diff --git a/src/mono/mono/metadata/icall-def.h b/src/mono/mono/metadata/icall-def.h index 03e680f153f8ae..f61cc2b688bed2 100644 --- a/src/mono/mono/metadata/icall-def.h +++ b/src/mono/mono/metadata/icall-def.h @@ -357,7 +357,7 @@ HANDLES(RASSEM_7, "InternalGetReferencedAssemblies", ves_icall_System_Reflection ICALL_TYPE(MCMETH, "System.Reflection.RuntimeConstructorInfo", MCMETH_1) HANDLES(MCMETH_1, "GetGenericMethodDefinition_impl", ves_icall_RuntimeMethodInfo_GetGenericMethodDefinition, MonoReflectionMethod, 1, (MonoReflectionMethod)) -HANDLES(MCMETH_2, "InternalInvoke", ves_icall_InternalInvoke, MonoObject, 4, (MonoReflectionMethod, MonoObject, MonoSpanOfObjects_ref, MonoExceptionOut)) +HANDLES(MCMETH_2, "InternalInvoke", ves_icall_InternalInvoke, MonoObject, 4, (MonoReflectionMethod, MonoObject, gpointer_ref, MonoExceptionOut)) HANDLES(MCMETH_5, "InvokeClassConstructor", ves_icall_InvokeClassConstructor, void, 1, (MonoQCallTypeHandle)) HANDLES_REUSE_WRAPPER(MCMETH_4, "get_metadata_token", ves_icall_reflection_get_token) diff --git a/src/mono/mono/metadata/icall.c b/src/mono/mono/metadata/icall.c index bf0e3085a23223..e83b82f52f3e79 100644 --- a/src/mono/mono/metadata/icall.c +++ b/src/mono/mono/metadata/icall.c @@ -3448,13 +3448,63 @@ ves_icall_RuntimeMethodInfo_GetGenericArguments (MonoReflectionMethodHandle ref_ return res; } +static gpointer +invoke_byrefs_extract_argument (gpointer *params_byref, int i, MonoType *t, MonoError *error) +{ + gpointer result = NULL; + error_init (error); +again: + switch (t->type) { + case MONO_TYPE_U1: + case MONO_TYPE_I1: + case MONO_TYPE_BOOLEAN: + case MONO_TYPE_U2: + case MONO_TYPE_I2: + case MONO_TYPE_CHAR: + case MONO_TYPE_U: + case MONO_TYPE_I: + case MONO_TYPE_U4: + case MONO_TYPE_I4: + case MONO_TYPE_U8: + case MONO_TYPE_I8: + case MONO_TYPE_R4: + case MONO_TYPE_R8: + case MONO_TYPE_VALUETYPE: + result = params_byref [i]; + break; + case MONO_TYPE_STRING: + case MONO_TYPE_OBJECT: + case MONO_TYPE_CLASS: + case MONO_TYPE_ARRAY: + case MONO_TYPE_SZARRAY: + if (m_type_is_byref (t)) + result = params_byref [i]; + else + result = *(MonoObject**)params_byref [i]; + break; + case MONO_TYPE_GENERICINST: + if (m_type_is_byref (t)) + t = m_class_get_this_arg (t->data.generic_class->container_class); + else + t = m_class_get_byval_arg (t->data.generic_class->container_class); + goto again; + case MONO_TYPE_PTR: { + result = *(gpointer*)params_byref [i]; + break; + } + default: + g_error ("type 0x%x not handled in ves_icall_InternalInvoke", t->type); + } + + return result; +} + MonoObjectHandle ves_icall_InternalInvoke (MonoReflectionMethodHandle method_handle, MonoObjectHandle this_arg_handle, - MonoSpanOfObjects *params_span, MonoExceptionHandleOut exception_out, MonoError *error) + gpointer *params_byref, MonoExceptionHandleOut exception_out, MonoError *error) { MonoReflectionMethod* const method = MONO_HANDLE_RAW (method_handle); MonoObject* const this_arg = MONO_HANDLE_RAW (this_arg_handle); - g_assert (params_span != NULL); /* * Invoke from reflection is supposed to always be a virtual call (the API @@ -3463,7 +3513,6 @@ ves_icall_InternalInvoke (MonoReflectionMethodHandle method_handle, MonoObjectHa */ MonoMethod *m = method->method; MonoMethodSignature* const sig = mono_method_signature_internal (m); - int pcount = 0; void *obj = this_arg; MonoObject *result = NULL; MonoArray *arr = NULL; @@ -3494,11 +3543,11 @@ ves_icall_InternalInvoke (MonoReflectionMethodHandle method_handle, MonoObjectHa /* Array constructor */ if (m_class_get_rank (m->klass) && !strcmp (m->name, ".ctor")) { - pcount = mono_span_length (params_span); - uintptr_t * const lengths = g_newa (uintptr_t, pcount); + int pcount = sig->param_count; + uintptr_t *lengths = g_newa (uintptr_t, pcount); /* Note: the synthetized array .ctors have int32 as argument type */ for (int i = 0; i < pcount; ++i) - lengths [i] = *(int32_t*) ((char*)mono_span_get (params_span, MonoObject*, i) + sizeof (MonoObject)); + lengths [i] = *(int32_t*) params_byref [i]; if (m_class_get_rank (m->klass) == 1 && sig->param_count == 2 && m_class_get_rank (m_class_get_element_class (m->klass))) { /* This is a ctor for jagged arrays. MS creates an array of arrays. */ @@ -3524,11 +3573,11 @@ ves_icall_InternalInvoke (MonoReflectionMethodHandle method_handle, MonoObjectHa } else { g_assert (pcount == (m_class_get_rank (m->klass) * 2)); /* The arguments are lower-bound-length pairs */ - intptr_t * const lower_bounds = (intptr_t *)g_alloca (sizeof (intptr_t) * pcount); + intptr_t *lower_bounds = (intptr_t *)g_alloca (sizeof (intptr_t) * pcount); for (int i = 0; i < pcount / 2; ++i) { - lower_bounds [i] = *(int32_t*) ((char*)mono_span_get (params_span, MonoObject*, (i * 2)) + sizeof (MonoObject)); - lengths [i] = *(int32_t*) ((char*)mono_span_get (params_span, MonoObject*, (i * 2) + 1) + sizeof (MonoObject)); + lower_bounds [i] = *(int32_t*) params_byref [i * 2]; + lengths [i] = *(int32_t*) params_byref [i * 2 + 1]; } arr = mono_array_new_full_checked (m->klass, lengths, lower_bounds, error); @@ -3536,7 +3585,75 @@ ves_icall_InternalInvoke (MonoReflectionMethodHandle method_handle, MonoObjectHa goto exit; } } - result = mono_runtime_invoke_span_checked (m, obj, params_span, error); + + int params_length = sig->param_count; + gpointer *pa = NULL; + if (params_length > 0) { + pa = g_newa (gpointer, params_length); + for (int i = 0; i < params_length; i++) { + MonoType *t = sig->params [i]; + pa [i] = invoke_byrefs_extract_argument (params_byref, i, t, error); + goto_if_nok (error, return_null); + } + } + if (!strcmp (m->name, ".ctor") && m->klass != mono_defaults.string_class) { + gpointer o = obj; + if (mono_class_is_nullable (m->klass)) { + /* Need to create a boxed vtype instead */ + g_assert (!obj); + + if (params_length == 0) { + goto_if_nok (error, return_null); + } else { + result = mono_value_box_checked (m_class_get_cast_class (m->klass), pa [0], error); + goto exit; + } + } + + if (!obj) { + MonoObjectHandle obj_h = mono_object_new_handle (m->klass, error); + goto_if_nok (error, return_null); + obj = MONO_HANDLE_RAW (obj_h); + g_assert (obj); /*maybe we should raise a TLE instead?*/ + if (m_class_is_valuetype (m->klass)) + o = (MonoObject *)mono_object_unbox_internal ((MonoObject *)obj); + else + o = obj; + } else if (m_class_is_valuetype (m->klass)) { + MonoObjectHandle obj_h = mono_value_box_handle (m->klass, obj, error); + goto_if_nok (error, return_null); + obj = MONO_HANDLE_RAW (obj_h); + } + + mono_runtime_invoke_checked (m, o, pa, error); + result = (MonoObject*)obj; + } else { + if (mono_class_is_nullable (m->klass)) { + if (m->flags & METHOD_ATTRIBUTE_STATIC) { + obj = NULL; + } else { + /* Convert the unboxed vtype into a Nullable structure */ + MonoObjectHandle nullable_h = mono_object_new_handle (m->klass, error); + goto_if_nok (error, return_null); + MonoObject* nullable = MONO_HANDLE_RAW (nullable_h); + + MonoObjectHandle boxed_h = mono_value_box_handle (m_class_get_cast_class (m->klass), obj, error); + goto_if_nok (error, return_null); + mono_nullable_init ((guint8 *)mono_object_unbox_internal (nullable), MONO_HANDLE_RAW (boxed_h), m->klass); + obj = mono_object_unbox_internal (nullable); + } + } + + result = mono_runtime_invoke_checked (m, obj, pa, error); + MONO_HANDLE_PIN (result); + goto_if_nok (error, return_null); + + if (sig->ret->type == MONO_TYPE_PTR) { + result = mono_boxed_intptr_to_pointer (result, sig->ret, error); + goto_if_nok (error, return_null); + } + } + goto exit; return_null: result = NULL; diff --git a/src/mono/mono/metadata/marshal-lightweight.c b/src/mono/mono/metadata/marshal-lightweight.c index cc79171907dc38..6306233d4d4796 100644 --- a/src/mono/mono/metadata/marshal-lightweight.c +++ b/src/mono/mono/metadata/marshal-lightweight.c @@ -302,7 +302,6 @@ emit_invoke_call (MonoMethodBuilder *mb, MonoMethod *method, gboolean virtual_, gboolean need_direct_wrapper) { int i; - int *tmp_nullable_locals; gboolean void_ret = FALSE; gboolean string_ctor = method && method->string_ctor; @@ -321,8 +320,6 @@ emit_invoke_call (MonoMethodBuilder *mb, MonoMethod *method, } } - tmp_nullable_locals = g_new0 (int, sig->param_count); - for (i = 0; i < sig->param_count; i++) { MonoType *t = sig->params [i]; int type; @@ -335,16 +332,6 @@ emit_invoke_call (MonoMethodBuilder *mb, MonoMethod *method, if (m_type_is_byref (t)) { mono_mb_emit_byte (mb, CEE_LDIND_I); - /* A Nullable type don't have a boxed form, it's either null or a boxed T. - * So to make this work we unbox it to a local variablee and push a reference to that. - */ - if (t->type == MONO_TYPE_GENERICINST && mono_class_is_nullable (mono_class_from_mono_type_internal (t))) { - tmp_nullable_locals [i] = mono_mb_add_local (mb, m_class_get_byval_arg (mono_class_from_mono_type_internal (t))); - - mono_mb_emit_op (mb, CEE_UNBOX_ANY, mono_class_from_mono_type_internal (t)); - mono_mb_emit_stloc (mb, tmp_nullable_locals [i]); - mono_mb_emit_ldloc_addr (mb, tmp_nullable_locals [i]); - } continue; } @@ -397,13 +384,7 @@ emit_invoke_call (MonoMethodBuilder *mb, MonoMethod *method, } mono_mb_emit_no_nullcheck (mb); mono_mb_emit_byte (mb, CEE_LDIND_I); - if (mono_class_is_nullable (mono_class_from_mono_type_internal (sig->params [i]))) { - /* Need to convert a boxed vtype to an mp to a Nullable struct */ - mono_mb_emit_op (mb, CEE_UNBOX, mono_class_from_mono_type_internal (sig->params [i])); - mono_mb_emit_op (mb, CEE_LDOBJ, mono_class_from_mono_type_internal (sig->params [i])); - } else { - mono_mb_emit_op (mb, CEE_LDOBJ, mono_class_from_mono_type_internal (sig->params [i])); - } + mono_mb_emit_op (mb, CEE_LDOBJ, mono_class_from_mono_type_internal (sig->params [i])); break; default: g_assert_not_reached (); @@ -483,28 +464,6 @@ emit_invoke_call (MonoMethodBuilder *mb, MonoMethod *method, if (!void_ret) mono_mb_emit_stloc (mb, loc_res); - - /* Convert back nullable-byref arguments */ - for (i = 0; i < sig->param_count; i++) { - MonoType *t = sig->params [i]; - - /* - * Box the result and put it back into the array, the caller will have - * to obtain it from there. - */ - if (m_type_is_byref (t) && t->type == MONO_TYPE_GENERICINST && mono_class_is_nullable (mono_class_from_mono_type_internal (t))) { - mono_mb_emit_ldarg (mb, 1); - mono_mb_emit_icon (mb, TARGET_SIZEOF_VOID_P * i); - mono_mb_emit_byte (mb, CEE_ADD); - - mono_mb_emit_ldloc (mb, tmp_nullable_locals [i]); - mono_mb_emit_op (mb, CEE_BOX, mono_class_from_mono_type_internal (t)); - - mono_mb_emit_byte (mb, CEE_STIND_REF); - } - } - - g_free (tmp_nullable_locals); } static void diff --git a/src/mono/mono/metadata/object-internals.h b/src/mono/mono/metadata/object-internals.h index 7e32a0a905ccaf..5c0d3141419c1e 100644 --- a/src/mono/mono/metadata/object-internals.h +++ b/src/mono/mono/metadata/object-internals.h @@ -1775,6 +1775,9 @@ mono_error_set_pending_exception (MonoError *error) MonoArray * mono_glist_to_array (GList *list, MonoClass *eclass, MonoError *error); +MonoObject* +mono_boxed_intptr_to_pointer (MonoObject *boxed_intptr, MonoType *ret_type, MonoError *error); + MONO_COMPONENT_API MonoObject * mono_object_new_checked (MonoClass *klass, MonoError *error); diff --git a/src/mono/mono/metadata/object.c b/src/mono/mono/metadata/object.c index fde5f8ed8476fc..c4d6ba2ac2afd7 100644 --- a/src/mono/mono/metadata/object.c +++ b/src/mono/mono/metadata/object.c @@ -4662,6 +4662,51 @@ invoke_span_extract_argument (MonoSpanOfObjects *params_span, int i, MonoType *t } return result; } + +MonoObject* +mono_boxed_intptr_to_pointer (MonoObject *boxed_intptr, MonoType *ret_type, MonoError *error) +{ + MonoClass *pointer_class; + void *box_args [2]; + MonoObject *box_exc; + + /* + * The runtime-invoke wrapper returns a boxed IntPtr, need to + * convert it to a Pointer object. + */ + pointer_class = mono_class_get_pointer_class (); + + MONO_STATIC_POINTER_INIT (MonoMethod, box_method) + box_method = mono_class_get_method_from_name_checked (pointer_class, "Box", -1, 0, error); + mono_error_assert_ok (error); + MONO_STATIC_POINTER_INIT_END (MonoMethod, box_method) + + if (boxed_intptr) { + g_assert (boxed_intptr->vtable->klass == mono_defaults.int_class); + box_args [0] = ((MonoIntPtr*)boxed_intptr)->m_value; + } else { + box_args [0] = NULL; + } + if (m_type_is_byref (ret_type)) { + // byref is already unboxed by the invoke code + MonoType *tmpret = mono_metadata_type_dup (NULL, ret_type); + tmpret->byref__ = FALSE; + MonoReflectionTypeHandle type_h = mono_type_get_object_handle (tmpret, error); + box_args [1] = MONO_HANDLE_RAW (type_h); + mono_metadata_free_type (tmpret); + } else { + MonoReflectionTypeHandle type_h = mono_type_get_object_handle (ret_type, error); + box_args [1] = MONO_HANDLE_RAW (type_h); + } + return_val_if_nok (error, NULL); + + MonoObject *res = mono_runtime_try_invoke (box_method, NULL, box_args, &box_exc, error); + g_assert (box_exc == NULL); + mono_error_assert_ok (error); + + return res; +} + /** * mono_runtime_invoke_array: * \param method method to invoke @@ -4721,6 +4766,7 @@ mono_runtime_invoke_array (MonoMethod *method, void *obj, MonoArray *params, return res; } +// FIXME Remove this method. Make all callers reuse the byref version from ves_icall_InternalInvoke static MonoObject* mono_runtime_try_invoke_span (MonoMethod *method, void *obj, MonoSpanOfObjects *params_span, MonoObject **exc, MonoError *error) @@ -4810,43 +4856,8 @@ mono_runtime_try_invoke_span (MonoMethod *method, void *obj, MonoSpanOfObjects * goto_if_nok (error, exit_null); if (sig->ret->type == MONO_TYPE_PTR) { - MonoClass *pointer_class; - void *box_args [2]; - MonoObject *box_exc; - - /* - * The runtime-invoke wrapper returns a boxed IntPtr, need to - * convert it to a Pointer object. - */ - pointer_class = mono_class_get_pointer_class (); - - MONO_STATIC_POINTER_INIT (MonoMethod, box_method) - box_method = mono_class_get_method_from_name_checked (pointer_class, "Box", -1, 0, error); - mono_error_assert_ok (error); - MONO_STATIC_POINTER_INIT_END (MonoMethod, box_method) - - if (res) { - g_assert (res->vtable->klass == mono_defaults.int_class); - box_args [0] = ((MonoIntPtr*)res)->m_value; - } else { - box_args [0] = NULL; - } - if (m_type_is_byref (sig->ret)) { - // byref is already unboxed by the invoke code - MonoType *tmpret = mono_metadata_type_dup (NULL, sig->ret); - tmpret->byref__ = FALSE; - MonoReflectionTypeHandle type_h = mono_type_get_object_handle (tmpret, error); - box_args [1] = MONO_HANDLE_RAW (type_h); - mono_metadata_free_type (tmpret); - } else { - MonoReflectionTypeHandle type_h = mono_type_get_object_handle (sig->ret, error); - box_args [1] = MONO_HANDLE_RAW (type_h); - } + res = mono_boxed_intptr_to_pointer (res, sig->ret, error); goto_if_nok (error, exit_null); - - res = mono_runtime_try_invoke (box_method, NULL, box_args, &box_exc, error); - g_assert (box_exc == NULL); - mono_error_assert_ok (error); } if (has_byref_nullables) { From 6501ef0f969f0173d10209c8dcc130385e93ef3a Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Fri, 22 Jul 2022 21:56:57 +0300 Subject: [PATCH 05/19] [mono] Throw NRE for a null byref parameter Behavior was changed recently. --- .../src/ILLink/ILLink.Descriptors.xml | 5 ----- src/mono/System.Private.CoreLib/src/Mono/RuntimeStructs.cs | 7 ------- src/mono/mono/metadata/marshal-lightweight.c | 7 ------- 3 files changed, 19 deletions(-) diff --git a/src/mono/System.Private.CoreLib/src/ILLink/ILLink.Descriptors.xml b/src/mono/System.Private.CoreLib/src/ILLink/ILLink.Descriptors.xml index ee6d7863ebd6b1..3d13f30e92bbed 100644 --- a/src/mono/System.Private.CoreLib/src/ILLink/ILLink.Descriptors.xml +++ b/src/mono/System.Private.CoreLib/src/ILLink/ILLink.Descriptors.xml @@ -632,11 +632,6 @@ - - - - - diff --git a/src/mono/System.Private.CoreLib/src/Mono/RuntimeStructs.cs b/src/mono/System.Private.CoreLib/src/Mono/RuntimeStructs.cs index fca9cf42dbc320..49989b0ec58ba5 100644 --- a/src/mono/System.Private.CoreLib/src/Mono/RuntimeStructs.cs +++ b/src/mono/System.Private.CoreLib/src/Mono/RuntimeStructs.cs @@ -149,11 +149,4 @@ internal enum I64Enum : long internal enum UI64Enum : ulong { } - - internal sealed class NullByRefReturnException : Exception - { - public NullByRefReturnException() - { - } - } } diff --git a/src/mono/mono/metadata/marshal-lightweight.c b/src/mono/mono/metadata/marshal-lightweight.c index 6306233d4d4796..3935fd32a52b42 100644 --- a/src/mono/mono/metadata/marshal-lightweight.c +++ b/src/mono/mono/metadata/marshal-lightweight.c @@ -402,16 +402,9 @@ emit_invoke_call (MonoMethodBuilder *mb, MonoMethod *method, if (m_type_is_byref (sig->ret)) { /* perform indirect load and return by value */ - int pos; - mono_mb_emit_byte (mb, CEE_DUP); - pos = mono_mb_emit_branch (mb, CEE_BRTRUE); - mono_mb_emit_exception_full (mb, "Mono", "NullByRefReturnException", NULL); - mono_mb_patch_branch (mb, pos); - guint8 ldind_op; MonoType* ret_byval = m_class_get_byval_arg (mono_class_from_mono_type_internal (sig->ret)); g_assert (!m_type_is_byref (ret_byval)); - // TODO: Handle null references ldind_op = mono_type_to_ldind (ret_byval); /* taken from similar code in mini-generic-sharing.c * we need to use mono_mb_emit_op to add method data when loading From e9a4c57b3e58465623f5a7adf4cd646b727b7189 Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Fri, 22 Jul 2022 21:32:11 +0300 Subject: [PATCH 06/19] [tests] Enable reflection invoke tests on mono --- .../Common/tests/System/Reflection/InvokeEmitTests.cs | 3 +-- .../tests/ConstructorInfo/ConstructorInfoInvokeArrayTests.cs | 3 --- src/libraries/System.Reflection/tests/ConstructorInfoTests.cs | 1 - .../System.Runtime/tests/System/Reflection/InvokeRefReturn.cs | 2 -- 4 files changed, 1 insertion(+), 8 deletions(-) diff --git a/src/libraries/Common/tests/System/Reflection/InvokeEmitTests.cs b/src/libraries/Common/tests/System/Reflection/InvokeEmitTests.cs index c81da708294f13..d6345d3e923141 100644 --- a/src/libraries/Common/tests/System/Reflection/InvokeEmitTests.cs +++ b/src/libraries/Common/tests/System/Reflection/InvokeEmitTests.cs @@ -35,8 +35,7 @@ public static void VerifyInvokeIsUsingEmit_Constructor() private static bool IsEmitInvokeSupported() { // Emit is only used for Invoke when RuntimeFeature.IsDynamicCodeCompiled is true. - return RuntimeFeature.IsDynamicCodeCompiled - && !PlatformDetection.IsMonoRuntime; // Temporary until Mono is updated. + return RuntimeFeature.IsDynamicCodeCompiled; } private static string InterpretedMethodName => PlatformDetection.IsMonoRuntime ? diff --git a/src/libraries/System.Reflection.TypeExtensions/tests/ConstructorInfo/ConstructorInfoInvokeArrayTests.cs b/src/libraries/System.Reflection.TypeExtensions/tests/ConstructorInfo/ConstructorInfoInvokeArrayTests.cs index 27d0327c53689d..f67acdae97ad55 100644 --- a/src/libraries/System.Reflection.TypeExtensions/tests/ConstructorInfo/ConstructorInfoInvokeArrayTests.cs +++ b/src/libraries/System.Reflection.TypeExtensions/tests/ConstructorInfo/ConstructorInfoInvokeArrayTests.cs @@ -8,7 +8,6 @@ namespace System.Reflection.Tests public class ConstructorInfoInvokeArrayTests { [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/67457", TestRuntimes.Mono)] public void Invoke_SZArrayConstructor() { Type type = Type.GetType("System.Object[]"); @@ -109,7 +108,6 @@ public void Invoke_1DArrayConstructor() } [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/67457", TestRuntimes.Mono)] public void Invoke_2DArrayConstructor() { Type type = Type.GetType("System.Int32[,]", false); @@ -247,7 +245,6 @@ public void Invoke_LargeDimensionalArrayConstructor() } [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/67457", TestRuntimes.Mono)] public void Invoke_JaggedArrayConstructor() { Type type = Type.GetType("System.String[][]"); diff --git a/src/libraries/System.Reflection/tests/ConstructorInfoTests.cs b/src/libraries/System.Reflection/tests/ConstructorInfoTests.cs index 5e41d80f197c47..34d04906a8055b 100644 --- a/src/libraries/System.Reflection/tests/ConstructorInfoTests.cs +++ b/src/libraries/System.Reflection/tests/ConstructorInfoTests.cs @@ -113,7 +113,6 @@ public void Invoke_OneDimensionalArray() } [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/67457", TestRuntimes.Mono)] public void Invoke_OneDimensionalArray_NegativeLengths_ThrowsOverflowException() { ConstructorInfo[] constructors = GetConstructors(typeof(object[])); diff --git a/src/libraries/System.Runtime/tests/System/Reflection/InvokeRefReturn.cs b/src/libraries/System.Runtime/tests/System/Reflection/InvokeRefReturn.cs index a86592b14ff2f9..48ea6f4bdfdf8c 100644 --- a/src/libraries/System.Runtime/tests/System/Reflection/InvokeRefReturn.cs +++ b/src/libraries/System.Runtime/tests/System/Reflection/InvokeRefReturn.cs @@ -38,7 +38,6 @@ public static void TestRefReturnNullableNoValue() [Theory] [MemberData(nameof(RefReturnInvokeTestData))] - [ActiveIssue("https://github.com/dotnet/runtime/issues/67457", TestRuntimes.Mono)] public static void TestNullRefReturnInvoke(T value) { TestClass tc = new TestClass(value); @@ -61,7 +60,6 @@ public static unsafe void TestRefReturnOfPointer() } [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/67457", TestRuntimes.Mono)] public static unsafe void TestNullRefReturnOfPointer() { TestClassIntPointer tc = new TestClassIntPointer(null); From b2bed4bfe07d4834cce12c1515177aaeae4c068a Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Mon, 25 Jul 2022 00:39:02 +0300 Subject: [PATCH 07/19] [mono] Avoid loading reflection invoke machinery during startup Aside from perf optimization, this allows the usage of LocalAppContextSwitches for invoke tests. Before this change, invoke machinery was initialized before the appcontext properties were set, failing to detect ForceInterpretedInvoke and ForceEmitInvoke properties. CoreCLR also hardcodes string type param. --- .../src/System/Collections/Generic/EqualityComparer.Mono.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/mono/System.Private.CoreLib/src/System/Collections/Generic/EqualityComparer.Mono.cs b/src/mono/System.Private.CoreLib/src/System/Collections/Generic/EqualityComparer.Mono.cs index 796c946eebb8d8..2d574506e39d51 100644 --- a/src/mono/System.Private.CoreLib/src/System/Collections/Generic/EqualityComparer.Mono.cs +++ b/src/mono/System.Private.CoreLib/src/System/Collections/Generic/EqualityComparer.Mono.cs @@ -39,6 +39,11 @@ private static EqualityComparer CreateComparer() { return (EqualityComparer)(object)(new ByteEqualityComparer()); } + else if (t == typeof(string)) + { + // Specialize for string, as EqualityComparer.Default is on the startup path + return (EqualityComparer)(object)(new GenericEqualityComparer()); + } if (typeof(IEquatable).IsAssignableFrom(t)) { From 5757632244f777ea7d86bfd2aaa28e0abb28b0f1 Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Mon, 5 Sep 2022 17:16:23 +0300 Subject: [PATCH 08/19] [mono] Remove nullable handling case This API already receives a boxed nullable now. --- src/mono/mono/metadata/icall.c | 28 +++------------------------- 1 file changed, 3 insertions(+), 25 deletions(-) diff --git a/src/mono/mono/metadata/icall.c b/src/mono/mono/metadata/icall.c index e83b82f52f3e79..d79af23f62fc1a 100644 --- a/src/mono/mono/metadata/icall.c +++ b/src/mono/mono/metadata/icall.c @@ -2161,31 +2161,9 @@ ves_icall_RuntimeFieldInfo_SetValueInternal (MonoReflectionFieldHandle field, Mo MonoGenericClass *gclass = type->data.generic_class; g_assert (!gclass->context.class_inst->is_open); - if (mono_class_is_nullable (mono_class_from_mono_type_internal (type))) { - MonoClass *nklass = mono_class_from_mono_type_internal (type); - - /* - * Convert the boxed vtype into a Nullable structure. - * This is complicated by the fact that Nullables have - * a variable structure. - */ - MonoObjectHandle nullable = mono_object_new_handle (nklass, error); - return_if_nok (error); - - MonoGCHandle nullable_gchandle = 0; - guint8 *nval = (guint8*)mono_object_handle_pin_unbox (nullable, &nullable_gchandle); - mono_nullable_init_from_handle (nval, value, nklass); - - isref = FALSE; - value_gchandle = nullable_gchandle; - v = (gchar*)nval; - } - else { - isref = !m_class_is_valuetype (gclass->container_class); - if (!isref && !MONO_HANDLE_IS_NULL (value)) { - v = (char*)mono_object_handle_pin_unbox (value, &value_gchandle); - }; - } + isref = !m_class_is_valuetype (gclass->container_class); + if (!isref && !MONO_HANDLE_IS_NULL (value)) + v = (char*)mono_object_handle_pin_unbox (value, &value_gchandle); break; } default: From 6375397d154f4c40a87cd9387abdd4fe3a4316e4 Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Mon, 5 Sep 2022 20:26:56 +0300 Subject: [PATCH 09/19] [mono] Disable inlining into Invoke stubs CoreCLR achieves this via NextCallReturnAddress. Add another intrinsic to be used for mono. --- .../src/System/Reflection/InvokerEmitUtil.cs | 13 ++++++++++--- .../System/Runtime/CompilerServices/JitHelpers.cs | 3 +++ src/mono/mono/mini/intrinsics.c | 5 +++++ 3 files changed, 18 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Reflection/InvokerEmitUtil.cs b/src/libraries/System.Private.CoreLib/src/System/Reflection/InvokerEmitUtil.cs index 4e4bab9aa36fc1..87bccf3173281f 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Reflection/InvokerEmitUtil.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Reflection/InvokerEmitUtil.cs @@ -65,8 +65,11 @@ public static unsafe InvokeFunc CreateInvokeDelegate(MethodBase method) } // Invoke the method. -#if !MONO - il.Emit(OpCodes.Call, Methods.NextCallReturnAddress()); // For CallStack reasons, don't inline target method. + // For CallStack reasons, don't inline target method. +#if MONO + il.Emit(OpCodes.Call, Methods.DisableInline()); +#else + il.Emit(OpCodes.Call, Methods.NextCallReturnAddress()); il.Emit(OpCodes.Pop); #endif @@ -182,7 +185,11 @@ public static MethodInfo Pointer_Box() => public static MethodInfo Type_GetTypeFromHandle() => s_Type_GetTypeFromHandle ??= typeof(Type).GetMethod(nameof(Type.GetTypeFromHandle), new[] { typeof(RuntimeTypeHandle) })!; -#if !MONO +#if MONO + private static MethodInfo? s_DisableInline; + public static MethodInfo DisableInline() => + s_DisableInline ??= typeof(System.Runtime.CompilerServices.JitHelpers).GetMethod(nameof(System.Runtime.CompilerServices.JitHelpers.DisableInline), BindingFlags.NonPublic | BindingFlags.Static)!; +#else private static MethodInfo? s_NextCallReturnAddress; public static MethodInfo NextCallReturnAddress() => s_NextCallReturnAddress ??= typeof(System.StubHelpers.StubHelpers).GetMethod(nameof(System.StubHelpers.StubHelpers.NextCallReturnAddress), BindingFlags.NonPublic | BindingFlags.Static)!; diff --git a/src/mono/System.Private.CoreLib/src/System/Runtime/CompilerServices/JitHelpers.cs b/src/mono/System.Private.CoreLib/src/System/Runtime/CompilerServices/JitHelpers.cs index 7dbf6f02b5bc90..40d30c6001599d 100644 --- a/src/mono/System.Private.CoreLib/src/System/Runtime/CompilerServices/JitHelpers.cs +++ b/src/mono/System.Private.CoreLib/src/System/Runtime/CompilerServices/JitHelpers.cs @@ -12,5 +12,8 @@ internal static class JitHelpers [Intrinsic] public static int EnumCompareTo(T x, T y) where T : struct, Enum => throw new NotImplementedException(); #pragma warning restore IDE0060 + + [Intrinsic] + internal static void DisableInline () => throw new NotImplementedException(); } } diff --git a/src/mono/mono/mini/intrinsics.c b/src/mono/mono/mini/intrinsics.c index eaa0b333baa14b..13be9d44131edd 100644 --- a/src/mono/mono/mini/intrinsics.c +++ b/src/mono/mono/mini/intrinsics.c @@ -718,6 +718,11 @@ emit_jit_helpers_intrinsics (MonoCompile *cfg, MonoMethod *cmethod, MonoMethodSi } } return ins; + } else if (!strcmp (cmethod->name, "DisableInline")) { + cfg->disable_inline = TRUE; + MONO_INST_NEW (cfg, ins, OP_NOP); + MONO_ADD_INS (cfg->cbb, ins); + return ins; } return NULL; From f71f9ed23fc9a692cd46f5c78873c350d68ff788 Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Tue, 6 Sep 2022 19:33:45 +0300 Subject: [PATCH 10/19] [mono] Fix handling of nullables when invoking from debugger --- src/mono/mono/component/debugger-agent.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/mono/mono/component/debugger-agent.c b/src/mono/mono/component/debugger-agent.c index 0d155aed44e2c7..3c992a4792d329 100644 --- a/src/mono/mono/component/debugger-agent.c +++ b/src/mono/mono/component/debugger-agent.c @@ -6175,12 +6175,7 @@ mono_do_invoke_method (DebuggerTlsData *tls, Buffer *buf, InvokeData *invoke, gu err = decode_value (sig->params [i], domain, arg_buf [i], p, &p, end, TRUE); if (err != ERR_NONE) break; - if (mono_class_is_nullable (arg_class)) { - args [i] = mono_nullable_box (arg_buf [i], arg_class, error); - mono_error_assert_ok (error); - } else { - args [i] = arg_buf [i]; - } + args [i] = arg_buf [i]; } } From c6dc7f3cb699d60b689ab62c3c841f53d3d25273 Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Tue, 13 Sep 2022 20:18:01 +0300 Subject: [PATCH 11/19] [mono] Update code for dyn runtime invokes and llvmonly invoke Remove special handling for nullables and throwing of NullByRefReturnException. --- src/mono/mono/mini/mini-amd64.c | 64 +++---------------------------- src/mono/mono/mini/mini-arm.c | 19 +-------- src/mono/mono/mini/mini-arm64.c | 57 ++------------------------- src/mono/mono/mini/mini-runtime.c | 34 ++-------------- 4 files changed, 14 insertions(+), 160 deletions(-) diff --git a/src/mono/mono/mini/mini-amd64.c b/src/mono/mono/mini/mini-amd64.c index bfede8ad36c7f9..20bacf6236cd01 100644 --- a/src/mono/mono/mini/mini-amd64.c +++ b/src/mono/mono/mini/mini-amd64.c @@ -2544,7 +2544,7 @@ mono_arch_emit_setret (MonoCompile *cfg, MonoMethod *method, MonoInst *val) typedef struct { MonoMethodSignature *sig; CallInfo *cinfo; - int nstack_args, nullable_area; + int nstack_args; } ArchDynCallInfo; static gboolean @@ -2596,7 +2596,7 @@ mono_arch_dyn_call_prepare (MonoMethodSignature *sig) { ArchDynCallInfo *info; CallInfo *cinfo; - int i, aindex; + int i; cinfo = get_call_info (NULL, sig); @@ -2622,34 +2622,6 @@ mono_arch_dyn_call_prepare (MonoMethodSignature *sig) break; } } - - for (aindex = 0; aindex < sig->param_count; aindex++) { - MonoType *t = sig->params [aindex]; - ArgInfo *ainfo = &cinfo->args [aindex + sig->hasthis]; - - if (m_type_is_byref (t)) - continue; - - switch (t->type) { - case MONO_TYPE_GENERICINST: - if (t->type == MONO_TYPE_GENERICINST && mono_class_is_nullable (mono_class_from_mono_type_internal (t))) { - MonoClass *klass = mono_class_from_mono_type_internal (t); - int size; - - if (!(ainfo->storage == ArgValuetypeInReg || ainfo->storage == ArgOnStack)) { - /* Nullables need a temporary buffer, its stored at the end of DynCallArgs.regs after the stack args */ - size = mono_class_value_size (klass, NULL); - info->nullable_area += size; - } - } - break; - default: - break; - } - } - - info->nullable_area = ALIGN_TO (info->nullable_area, 16); - /* Align to 16 bytes */ if (info->nstack_args & 1) info->nstack_args ++; @@ -2677,7 +2649,7 @@ mono_arch_dyn_call_get_buf_size (MonoDynCallInfo *info) ArchDynCallInfo *ainfo = (ArchDynCallInfo*)info; /* Extend the 'regs' field dynamically */ - return sizeof (DynCallArgs) + (ainfo->nstack_args * sizeof (target_mgreg_t)) + ainfo->nullable_area; + return sizeof (DynCallArgs) + (ainfo->nstack_args * sizeof (target_mgreg_t)); } #define PTR_TO_GREG(ptr) ((host_mgreg_t)(ptr)) @@ -2704,8 +2676,6 @@ mono_arch_start_dyn_call (MonoDynCallInfo *info, gpointer **args, guint8 *ret, g DynCallArgs *p = (DynCallArgs*)buf; int arg_index, greg, i, pindex; MonoMethodSignature *sig = dinfo->sig; - int buffer_offset = 0; - guint8 *nullable_buffer; static int general_param_reg_to_index [MONO_MAX_IREGS]; static int float_param_reg_to_index [MONO_MAX_FREGS]; @@ -2730,9 +2700,6 @@ mono_arch_start_dyn_call (MonoDynCallInfo *info, gpointer **args, guint8 *ret, g greg = 0; pindex = 0; - /* Stored after the stack arguments */ - nullable_buffer = (guint8*)&(p->regs [PARAM_REGS + dinfo->nstack_args]); - if (sig->hasthis || dinfo->cinfo->vret_arg_index == 1) { p->regs [greg ++] = PTR_TO_GREG(*(args [arg_index ++])); if (!sig->hasthis) @@ -2820,32 +2787,11 @@ mono_arch_start_dyn_call (MonoDynCallInfo *info, gpointer **args, guint8 *ret, g } break; case MONO_TYPE_GENERICINST: - if (MONO_TYPE_IS_REFERENCE (t)) { + if (MONO_TYPE_IS_REFERENCE (t)) { p->regs [slot] = PTR_TO_GREG (*(arg)); break; - } else if (t->type == MONO_TYPE_GENERICINST && mono_class_is_nullable (mono_class_from_mono_type_internal (t))) { - MonoClass *klass = mono_class_from_mono_type_internal (t); - guint8 *nullable_buf; - int size; - - size = mono_class_value_size (klass, NULL); - if (ainfo->storage == ArgValuetypeInReg || ainfo->storage == ArgOnStack) { - nullable_buf = g_alloca (size); - } else { - nullable_buf = nullable_buffer + buffer_offset; - buffer_offset += size; - g_assert (buffer_offset <= dinfo->nullable_area); - } - - /* The argument pointed to by arg is either a boxed vtype or null */ - mono_nullable_init (nullable_buf, (MonoObject*)arg, klass); - - arg = (gpointer*)nullable_buf; - /* Fall though */ - - } else { - /* Fall through */ } + /* Fall through */ case MONO_TYPE_VALUETYPE: { switch (ainfo->storage) { case ArgValuetypeInReg: diff --git a/src/mono/mono/mini/mini-arm.c b/src/mono/mono/mini/mini-arm.c index 323524484d05e9..84b13a5d68d6a1 100644 --- a/src/mono/mono/mini/mini-arm.c +++ b/src/mono/mono/mini/mini-arm.c @@ -3038,25 +3038,8 @@ mono_arch_start_dyn_call (MonoDynCallInfo *info, gpointer **args, guint8 *ret, g if (MONO_TYPE_IS_REFERENCE (t)) { p->regs [slot] = (host_mgreg_t)(gsize)*arg; break; - } else { - if (t->type == MONO_TYPE_GENERICINST && mono_class_is_nullable (mono_class_from_mono_type_internal (t))) { - MonoClass *klass = mono_class_from_mono_type_internal (t); - guint8 *nullable_buf; - int size; - - size = mono_class_value_size (klass, NULL); - nullable_buf = g_alloca (size); - g_assert (nullable_buf); - - /* The argument pointed to by arg is either a boxed vtype or null */ - mono_nullable_init (nullable_buf, (MonoObject*)arg, klass); - - arg = (gpointer*)nullable_buf; - /* Fall though */ - } else { - /* Fall though */ - } } + /* Fall though */ case MONO_TYPE_VALUETYPE: g_assert (ainfo->storage == RegTypeStructByVal); diff --git a/src/mono/mono/mini/mini-arm64.c b/src/mono/mono/mini/mini-arm64.c index 65a6bb42e1fc7e..0e2b70b5f12a97 100644 --- a/src/mono/mono/mini/mini-arm64.c +++ b/src/mono/mono/mini/mini-arm64.c @@ -1638,7 +1638,7 @@ typedef struct { CallInfo *cinfo; MonoType *rtype; MonoType **param_types; - int n_fpargs, n_fpret, nullable_area; + int n_fpargs, n_fpret; } ArchDynCallInfo; static gboolean @@ -1691,7 +1691,7 @@ mono_arch_dyn_call_prepare (MonoMethodSignature *sig) { ArchDynCallInfo *info; CallInfo *cinfo; - int i, aindex; + int i; cinfo = get_call_info (NULL, sig); @@ -1721,28 +1721,6 @@ mono_arch_dyn_call_prepare (MonoMethodSignature *sig) break; } - for (aindex = 0; aindex < sig->param_count; aindex++) { - MonoType *t = info->param_types [aindex]; - - if (m_type_is_byref (t)) - continue; - - switch (t->type) { - case MONO_TYPE_GENERICINST: - if (t->type == MONO_TYPE_GENERICINST && mono_class_is_nullable (mono_class_from_mono_type_internal (t))) { - MonoClass *klass = mono_class_from_mono_type_internal (t); - int size; - - /* Nullables need a temporary buffer, its stored at the end of DynCallArgs.regs after the stack args */ - size = mono_class_value_size (klass, NULL); - info->nullable_area += size; - } - break; - default: - break; - } - } - return (MonoDynCallInfo*)info; } @@ -1762,7 +1740,7 @@ mono_arch_dyn_call_get_buf_size (MonoDynCallInfo *info) ArchDynCallInfo *ainfo = (ArchDynCallInfo*)info; g_assert (ainfo->cinfo->stack_usage % MONO_ARCH_FRAME_ALIGNMENT == 0); - return sizeof (DynCallArgs) + ainfo->cinfo->stack_usage + ainfo->nullable_area; + return sizeof (DynCallArgs) + ainfo->cinfo->stack_usage; } static double @@ -1789,8 +1767,6 @@ mono_arch_start_dyn_call (MonoDynCallInfo *info, gpointer **args, guint8 *ret, g int aindex, arg_index, greg, i, pindex; MonoMethodSignature *sig = dinfo->sig; CallInfo *cinfo = dinfo->cinfo; - int buffer_offset = 0; - guint8 *nullable_buffer; p->res = 0; p->ret = ret; @@ -1802,9 +1778,6 @@ mono_arch_start_dyn_call (MonoDynCallInfo *info, gpointer **args, guint8 *ret, g greg = 0; pindex = 0; - /* Stored after the stack arguments */ - nullable_buffer = (guint8*)&(p->regs [PARAM_REGS + 1 + (cinfo->stack_usage / sizeof (host_mgreg_t))]); - if (sig->hasthis) p->regs [greg ++] = (host_mgreg_t)*(args [arg_index ++]); @@ -1899,30 +1872,8 @@ mono_arch_start_dyn_call (MonoDynCallInfo *info, gpointer **args, guint8 *ret, g if (MONO_TYPE_IS_REFERENCE (t)) { p->regs [slot] = (host_mgreg_t)*arg; break; - } else { - if (t->type == MONO_TYPE_GENERICINST && mono_class_is_nullable (mono_class_from_mono_type_internal (t))) { - MonoClass *klass = mono_class_from_mono_type_internal (t); - guint8 *nullable_buf; - int size; - - /* - * Use p->buffer as a temporary buffer since the data needs to be available after this call - * if the nullable param is passed by ref. - */ - size = mono_class_value_size (klass, NULL); - nullable_buf = nullable_buffer + buffer_offset; - buffer_offset += size; - g_assert (buffer_offset <= dinfo->nullable_area); - - /* The argument pointed to by arg is either a boxed vtype or null */ - mono_nullable_init (nullable_buf, (MonoObject*)arg, klass); - - arg = (gpointer*)nullable_buf; - /* Fall though */ - } else { - /* Fall though */ - } } + /* Fall through */ case MONO_TYPE_VALUETYPE: switch (ainfo->storage) { case ArgVtypeInIRegs: diff --git a/src/mono/mono/mini/mini-runtime.c b/src/mono/mono/mini/mini-runtime.c index b9791ce273d570..5dff72ecf068d2 100644 --- a/src/mono/mono/mini/mini-runtime.c +++ b/src/mono/mono/mini/mini-runtime.c @@ -3097,18 +3097,10 @@ create_runtime_invoke_info (MonoMethod *method, gpointer compiled_method, gboole #ifdef MONO_ARCH_DYN_CALL_SUPPORTED if (!mono_llvm_only && (mono_aot_only || mini_debug_options.dyn_runtime_invoke)) { gboolean supported = TRUE; - int i; if (method->string_ctor) sig = mono_marshal_get_string_ctor_signature (method); - for (i = 0; i < sig->param_count; ++i) { - MonoType *t = sig->params [i]; - - if (m_type_is_byref (t) && t->type == MONO_TYPE_GENERICINST && mono_class_is_nullable (mono_class_from_mono_type_internal (t))) - supported = FALSE; - } - if (!info->compiled_method) supported = FALSE; @@ -3217,8 +3209,6 @@ create_runtime_invoke_info (MonoMethod *method, gpointer compiled_method, gboole return ret; } -static GENERATE_GET_CLASS_WITH_CACHE (nullbyrefreturn_ex, "Mono", "NullByRefReturnException"); - static MonoObject* mono_llvmonly_runtime_invoke (MonoMethod *method, RuntimeInvokeInfo *info, void *obj, void **params, MonoObject **exc, MonoError *error) { @@ -3271,20 +3261,6 @@ mono_llvmonly_runtime_invoke (MonoMethod *method, RuntimeInvokeInfo *info, void for (i = 0; i < sig->param_count; ++i) { MonoType *t = sig->params [i]; - if (t->type == MONO_TYPE_GENERICINST && mono_class_is_nullable (mono_class_from_mono_type_internal (t))) { - MonoClass *klass = mono_class_from_mono_type_internal (t); - guint8 *nullable_buf; - int size; - - size = mono_class_value_size (klass, NULL); - nullable_buf = g_alloca (size); - g_assert (nullable_buf); - - /* The argument pointed to by params [i] is either a boxed vtype or null */ - mono_nullable_init (nullable_buf, (MonoObject*)params [i], klass); - params [i] = nullable_buf; - } - if (!m_type_is_byref (t) && (MONO_TYPE_IS_REFERENCE (t) || t->type == MONO_TYPE_PTR)) { param_refs [i] = params [i]; params [i] = &(param_refs [i]); @@ -3302,10 +3278,9 @@ mono_llvmonly_runtime_invoke (MonoMethod *method, RuntimeInvokeInfo *info, void if (m_type_is_byref (sig->ret)) { if (*(gpointer*)retval == NULL) { - MonoClass *klass = mono_class_get_nullbyrefreturn_ex_class (); - MonoObject *ex = mono_object_new_checked (klass, error); + MonoException *ex = mono_get_exception_null_reference (); mono_error_assert_ok (error); - mono_error_set_exception_instance (error, (MonoException*)ex); + mono_error_set_exception_instance (error, ex); return NULL; } } @@ -3532,10 +3507,9 @@ mono_jit_runtime_invoke (MonoMethod *method, void *obj, void **params, MonoObjec if (m_type_is_byref (sig->ret)) { if (*(gpointer*)retval == NULL) { - MonoClass *klass = mono_class_get_nullbyrefreturn_ex_class (); - MonoObject *ex = mono_object_new_checked (klass, error); + MonoException *ex = mono_get_exception_null_reference (); mono_error_assert_ok (error); - mono_error_set_exception_instance (error, (MonoException*)ex); + mono_error_set_exception_instance (error, ex); return NULL; } } From 0e5ddb47322b12be6a85e041266226bf06237870 Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Fri, 7 Oct 2022 19:41:30 +0300 Subject: [PATCH 12/19] [mono] Remove old invoke code Which was still used only by mono_runtime_invoke_array --- src/mono/mono/metadata/object-internals.h | 8 - src/mono/mono/metadata/object.c | 323 +--------------------- 2 files changed, 4 insertions(+), 327 deletions(-) diff --git a/src/mono/mono/metadata/object-internals.h b/src/mono/mono/metadata/object-internals.h index 5c0d3141419c1e..c20d5306917281 100644 --- a/src/mono/mono/metadata/object-internals.h +++ b/src/mono/mono/metadata/object-internals.h @@ -1881,14 +1881,6 @@ mono_runtime_invoke_handle (MonoMethod *method, MonoObjectHandle obj, void **par void mono_runtime_invoke_handle_void (MonoMethod *method, MonoObjectHandle obj, void **params, MonoError* error); -MonoObject* -mono_runtime_try_invoke_array (MonoMethod *method, void *obj, MonoArray *params, - MonoObject **exc, MonoError *error); - -MonoObject* -mono_runtime_invoke_span_checked (MonoMethod *method, void *obj, MonoSpanOfObjects *params, - MonoError *error); - void* mono_compile_method_checked (MonoMethod *method, MonoError *error); diff --git a/src/mono/mono/metadata/object.c b/src/mono/mono/metadata/object.c index c4d6ba2ac2afd7..77605175d95929 100644 --- a/src/mono/mono/metadata/object.c +++ b/src/mono/mono/metadata/object.c @@ -4554,115 +4554,6 @@ mono_runtime_try_exec_main (MonoMethod *method, MonoArray *args, MonoObject **ex return do_try_exec_main (method, args, exc); } -/** invoke_span_extract_argument: - * @params: span of object arguments to the method. - * @i: the index of the argument to extract. - * @t: ith type from the method signature. - * @has_byref_nullables: outarg - TRUE if method expects a byref nullable argument - * @error: set on error. - * - * Given an array of method arguments, return the ith one using the corresponding type - * to perform necessary unboxing. If method expects a ref nullable argument, writes TRUE to @has_byref_nullables. - * - * On failure sets @error and returns NULL. - */ -static gpointer -invoke_span_extract_argument (MonoSpanOfObjects *params_span, int i, MonoType *t, MonoObject **pa_obj, gboolean* has_byref_nullables, MonoError *error) -{ - MonoType *t_orig = t; - gpointer result = NULL; - *pa_obj = NULL; - error_init (error); - again: - switch (t->type) { - case MONO_TYPE_U1: - case MONO_TYPE_I1: - case MONO_TYPE_BOOLEAN: - case MONO_TYPE_U2: - case MONO_TYPE_I2: - case MONO_TYPE_CHAR: - case MONO_TYPE_U: - case MONO_TYPE_I: - case MONO_TYPE_U4: - case MONO_TYPE_I4: - case MONO_TYPE_U8: - case MONO_TYPE_I8: - case MONO_TYPE_R4: - case MONO_TYPE_R8: - case MONO_TYPE_VALUETYPE: - if (t->type == MONO_TYPE_VALUETYPE && mono_class_is_nullable (mono_class_from_mono_type_internal (t_orig))) { - /* The runtime invoke wrapper needs the original boxed vtype, it does handle byref values as well. */ - *pa_obj = mono_span_get (params_span, MonoObject*, i); - result = *pa_obj; - if (m_type_is_byref (t)) - *has_byref_nullables = TRUE; - } else { - /* MS seems to create the objects if a null is passed in */ - gboolean was_null = FALSE; - if (!mono_span_get (params_span, MonoObject*, i)) { - MonoObject *o = mono_object_new_checked (mono_class_from_mono_type_internal (t_orig), error); - return_val_if_nok (error, NULL); - mono_span_setref (params_span, i, o); - was_null = TRUE; - } - - if (m_type_is_byref (t)) { - /* - * We can't pass the unboxed vtype byref to the callee, since - * that would mean the callee would be able to modify boxed - * primitive types. So we (and MS) make a copy of the boxed - * object, pass that to the callee, and replace the original - * boxed object in the arg array with the copy. - */ - MonoObject *orig = mono_span_get (params_span, MonoObject*, i); - MonoObject *copy = mono_value_box_checked (orig->vtable->klass, mono_object_unbox_internal (orig), error); - return_val_if_nok (error, NULL); - mono_span_setref (params_span, i, copy); - } - *pa_obj = mono_span_get (params_span, MonoObject*, i); - result = mono_object_unbox_internal (*pa_obj); - if (!m_type_is_byref (t) && was_null) - mono_span_setref (params_span, i, NULL); - } - break; - case MONO_TYPE_STRING: - case MONO_TYPE_OBJECT: - case MONO_TYPE_CLASS: - case MONO_TYPE_ARRAY: - case MONO_TYPE_SZARRAY: - if (m_type_is_byref (t)) { - result = mono_span_addr (params_span, MonoObject*, i); - // FIXME: I need to check this code path - } else { - *pa_obj = mono_span_get (params_span, MonoObject*, i); - result = *pa_obj; - } - break; - case MONO_TYPE_GENERICINST: - if (m_type_is_byref (t)) - t = m_class_get_this_arg (t->data.generic_class->container_class); - else - t = m_class_get_byval_arg (t->data.generic_class->container_class); - goto again; - case MONO_TYPE_PTR: { - MonoObject *arg; - - /* The argument should be an IntPtr */ - arg = mono_span_get (params_span, MonoObject*, i); - if (arg == NULL) { - result = NULL; - } else { - g_assert (arg->vtable->klass == mono_defaults.int_class || arg->vtable->klass == mono_defaults.uint_class); - result = ((MonoIntPtr*)arg)->m_value; - } - break; - } - default: - g_error ("type 0x%x not handled in mono_runtime_invoke_array", t_orig->type); - } - return result; -} - MonoObject* mono_boxed_intptr_to_pointer (MonoObject *boxed_intptr, MonoType *ret_type, MonoError *error) { @@ -4751,7 +4642,8 @@ mono_runtime_invoke_array (MonoMethod *method, void *obj, MonoArray *params, MONO_ENTER_GC_UNSAFE; ERROR_DECL (error); if (exc) { - res = mono_runtime_try_invoke_array (method, obj, params, exc, error); + g_error ("FIXME"); + res = NULL; if (*exc) { res = NULL; mono_error_cleanup (error); @@ -4759,221 +4651,14 @@ mono_runtime_invoke_array (MonoMethod *method, void *obj, MonoArray *params, *exc = (MonoObject*)mono_error_convert_to_exception (error); } } else { - res = mono_runtime_try_invoke_array (method, obj, params, NULL, error); + g_error ("FIXME"); + res = NULL; mono_error_raise_exception_deprecated (error); /* OK to throw, external only without a good alternative */ } MONO_EXIT_GC_UNSAFE; return res; } -// FIXME Remove this method. Make all callers reuse the byref version from ves_icall_InternalInvoke -static MonoObject* -mono_runtime_try_invoke_span (MonoMethod *method, void *obj, MonoSpanOfObjects *params_span, - MonoObject **exc, MonoError *error) -{ - error_init (error); - - MonoMethodSignature *sig = mono_method_signature_internal (method); - gpointer *pa = NULL; - MonoObject *res = NULL; - int i; - gboolean has_byref_nullables = FALSE; - int params_length = mono_span_length (params_span); - - if (params_length > 0) { - pa = g_newa (gpointer, params_length); - for (i = 0; i < params_length; i++) { - MonoType *t = sig->params [i]; - MonoObject *pa_obj; - pa [i] = invoke_span_extract_argument (params_span, i, t, &pa_obj, &has_byref_nullables, error); - if (pa_obj) - MONO_HANDLE_PIN (pa_obj); - goto_if_nok (error, exit_null); - } - } - - if (!strcmp (method->name, ".ctor") && method->klass != mono_defaults.string_class) { - void *o = obj; - - if (mono_class_is_nullable (method->klass)) { - /* Need to create a boxed vtype instead */ - g_assert (!obj); - - if (params_length == 0) { - goto_if_nok (error, exit_null); - } else { - res = mono_value_box_checked (m_class_get_cast_class (method->klass), pa [0], error); - goto exit; - } - } - - if (!obj) { - MonoObjectHandle obj_h = mono_object_new_handle (method->klass, error); - goto_if_nok (error, exit_null); - obj = MONO_HANDLE_RAW (obj_h); - g_assert (obj); /*maybe we should raise a TLE instead?*/ - if (m_class_is_valuetype (method->klass)) - o = (MonoObject *)mono_object_unbox_internal ((MonoObject *)obj); - else - o = obj; - } else if (m_class_is_valuetype (method->klass)) { - MonoObjectHandle obj_h = mono_value_box_handle (method->klass, obj, error); - goto_if_nok (error, exit_null); - obj = MONO_HANDLE_RAW (obj_h); - } - - if (exc) { - mono_runtime_try_invoke (method, o, pa, exc, error); - } else { - mono_runtime_invoke_checked (method, o, pa, error); - } - - res = (MonoObject*)obj; - } else { - if (mono_class_is_nullable (method->klass)) { - if (method->flags & METHOD_ATTRIBUTE_STATIC) { - obj = NULL; - } else { - /* Convert the unboxed vtype into a Nullable structure */ - MonoObjectHandle nullable_h = mono_object_new_handle (method->klass, error); - goto_if_nok (error, exit_null); - MonoObject* nullable = MONO_HANDLE_RAW (nullable_h); - - MonoObjectHandle boxed_h = mono_value_box_handle (m_class_get_cast_class (method->klass), obj, error); - goto_if_nok (error, exit_null); - mono_nullable_init ((guint8 *)mono_object_unbox_internal (nullable), MONO_HANDLE_RAW (boxed_h), method->klass); - obj = mono_object_unbox_internal (nullable); - } - } - - /* obj must be already unboxed if needed */ - if (exc) { - res = mono_runtime_try_invoke (method, obj, pa, exc, error); - } else { - res = mono_runtime_invoke_checked (method, obj, pa, error); - } - MONO_HANDLE_PIN (res); - goto_if_nok (error, exit_null); - - if (sig->ret->type == MONO_TYPE_PTR) { - res = mono_boxed_intptr_to_pointer (res, sig->ret, error); - goto_if_nok (error, exit_null); - } - - if (has_byref_nullables) { - /* - * The runtime invoke wrapper already converted byref nullables back, - * and stored them in pa, we just need to copy them back to the - * managed array. - */ - for (i = 0; i < params_length; i++) { - MonoType *t = sig->params [i]; - - if (m_type_is_byref (t) && t->type == MONO_TYPE_GENERICINST && mono_class_is_nullable (mono_class_from_mono_type_internal (t))) - mono_span_setref (params_span, i, pa [i]); - } - } - } - goto exit; -exit_null: - res = NULL; -exit: - return res; -} - -/** - * mono_runtime_invoke_array_checked: - * \param method method to invoke - * \param obj object instance - * \param params arguments to the method - * \param error set on failure. - * Invokes the method represented by \p method on the object \p obj. - * - * \p obj is the \c this pointer, it should be NULL for static - * methods, a \c MonoObject* for object instances and a pointer to - * the value type for value types. - * - * The \p params span contains the arguments to the method with the - * same convention: \c MonoObject* pointers for object instances and - * pointers to the value type otherwise. The \c _invoke_array - * variant takes a C# \c object[] as the \p params argument (\c MonoArray*): - * in this case the value types are boxed inside the - * respective reference representation. - * - * From unmanaged code you'll usually use the - * mono_runtime_invoke_checked() variant. - * - * Note that this function doesn't handle virtual methods for - * you, it will exec the exact method you pass: we still need to - * expose a function to lookup the derived class implementation - * of a virtual method (there are examples of this in the code, - * though). - * - * On failure or exception, \p error will be set. In that case, you - * can't use the \c MonoObject* result from the function. - * - * If the method returns a value type, it is boxed in an object - * reference. - */ -MonoObject* -mono_runtime_invoke_span_checked (MonoMethod *method, void *obj, MonoSpanOfObjects *params, - MonoError *error) -{ - error_init (error); - return mono_runtime_try_invoke_span (method, obj, params, NULL, error); -} - -/** - * mono_runtime_try_invoke_array: - * \param method method to invoke - * \param obj object instance - * \param params arguments to the method - * \param exc exception information. - * \param error set on failure. - * Invokes the method represented by \p method on the object \p obj. - * - * \p obj is the \c this pointer, it should be NULL for static - * methods, a \c MonoObject* for object instances and a pointer to - * the value type for value types. - * - * The \p params array contains the arguments to the method with the - * same convention: \c MonoObject* pointers for object instances and - * pointers to the value type otherwise. The \c _invoke_array - * variant takes a C# \c object[] as the params argument (\c MonoArray*): - * in this case the value types are boxed inside the - * respective reference representation. - * - * From unmanaged code you'll usually use the - * mono_runtime_invoke_checked() variant. - * - * Note that this function doesn't handle virtual methods for - * you, it will exec the exact method you pass: we still need to - * expose a function to lookup the derived class implementation - * of a virtual method (there are examples of this in the code, - * though). - * - * You can pass NULL as the \p exc argument if you don't want to catch - * exceptions, otherwise, \c *exc will be set to the exception thrown, if - * any. On other failures, \p error will be set. If an exception is - * thrown or there's an error, you can't use the \c MonoObject* result - * from the function. - * - * If the method returns a value type, it is boxed in an object - * reference. - */ -MonoObject* -mono_runtime_try_invoke_array (MonoMethod *method, void *obj, MonoArray *params, - MonoObject **exc, MonoError *error) -{ - MONO_REQ_GC_UNSAFE_MODE; - HANDLE_FUNCTION_ENTER (); - - MonoSpanOfObjects params_span = mono_span_create_from_object_array (params); - MonoObject *res = mono_runtime_try_invoke_span (method, obj, ¶ms_span, exc, error); - - HANDLE_FUNCTION_RETURN_VAL (res); -} - // FIXME these will move to header soon static MonoObjectHandle mono_object_new_by_vtable (MonoVTable *vtable, MonoError *error); From 4f4cde982de58c29df99cdc0c48591d24ad02657 Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Fri, 7 Oct 2022 21:04:02 +0300 Subject: [PATCH 13/19] [mono] Move invoke code back in object.c --- src/mono/mono/metadata/icall.c | 119 +----------------- src/mono/mono/metadata/object-internals.h | 7 +- src/mono/mono/metadata/object.c | 139 +++++++++++++++++++++- 3 files changed, 143 insertions(+), 122 deletions(-) diff --git a/src/mono/mono/metadata/icall.c b/src/mono/mono/metadata/icall.c index d79af23f62fc1a..8c6c7329042b0c 100644 --- a/src/mono/mono/metadata/icall.c +++ b/src/mono/mono/metadata/icall.c @@ -3426,57 +3426,6 @@ ves_icall_RuntimeMethodInfo_GetGenericArguments (MonoReflectionMethodHandle ref_ return res; } -static gpointer -invoke_byrefs_extract_argument (gpointer *params_byref, int i, MonoType *t, MonoError *error) -{ - gpointer result = NULL; - error_init (error); -again: - switch (t->type) { - case MONO_TYPE_U1: - case MONO_TYPE_I1: - case MONO_TYPE_BOOLEAN: - case MONO_TYPE_U2: - case MONO_TYPE_I2: - case MONO_TYPE_CHAR: - case MONO_TYPE_U: - case MONO_TYPE_I: - case MONO_TYPE_U4: - case MONO_TYPE_I4: - case MONO_TYPE_U8: - case MONO_TYPE_I8: - case MONO_TYPE_R4: - case MONO_TYPE_R8: - case MONO_TYPE_VALUETYPE: - result = params_byref [i]; - break; - case MONO_TYPE_STRING: - case MONO_TYPE_OBJECT: - case MONO_TYPE_CLASS: - case MONO_TYPE_ARRAY: - case MONO_TYPE_SZARRAY: - if (m_type_is_byref (t)) - result = params_byref [i]; - else - result = *(MonoObject**)params_byref [i]; - break; - case MONO_TYPE_GENERICINST: - if (m_type_is_byref (t)) - t = m_class_get_this_arg (t->data.generic_class->container_class); - else - t = m_class_get_byval_arg (t->data.generic_class->container_class); - goto again; - case MONO_TYPE_PTR: { - result = *(gpointer*)params_byref [i]; - break; - } - default: - g_error ("type 0x%x not handled in ves_icall_InternalInvoke", t->type); - } - - return result; -} - MonoObjectHandle ves_icall_InternalInvoke (MonoReflectionMethodHandle method_handle, MonoObjectHandle this_arg_handle, gpointer *params_byref, MonoExceptionHandleOut exception_out, MonoError *error) @@ -3564,73 +3513,7 @@ ves_icall_InternalInvoke (MonoReflectionMethodHandle method_handle, MonoObjectHa } } - int params_length = sig->param_count; - gpointer *pa = NULL; - if (params_length > 0) { - pa = g_newa (gpointer, params_length); - for (int i = 0; i < params_length; i++) { - MonoType *t = sig->params [i]; - pa [i] = invoke_byrefs_extract_argument (params_byref, i, t, error); - goto_if_nok (error, return_null); - } - } - if (!strcmp (m->name, ".ctor") && m->klass != mono_defaults.string_class) { - gpointer o = obj; - if (mono_class_is_nullable (m->klass)) { - /* Need to create a boxed vtype instead */ - g_assert (!obj); - - if (params_length == 0) { - goto_if_nok (error, return_null); - } else { - result = mono_value_box_checked (m_class_get_cast_class (m->klass), pa [0], error); - goto exit; - } - } - - if (!obj) { - MonoObjectHandle obj_h = mono_object_new_handle (m->klass, error); - goto_if_nok (error, return_null); - obj = MONO_HANDLE_RAW (obj_h); - g_assert (obj); /*maybe we should raise a TLE instead?*/ - if (m_class_is_valuetype (m->klass)) - o = (MonoObject *)mono_object_unbox_internal ((MonoObject *)obj); - else - o = obj; - } else if (m_class_is_valuetype (m->klass)) { - MonoObjectHandle obj_h = mono_value_box_handle (m->klass, obj, error); - goto_if_nok (error, return_null); - obj = MONO_HANDLE_RAW (obj_h); - } - - mono_runtime_invoke_checked (m, o, pa, error); - result = (MonoObject*)obj; - } else { - if (mono_class_is_nullable (m->klass)) { - if (m->flags & METHOD_ATTRIBUTE_STATIC) { - obj = NULL; - } else { - /* Convert the unboxed vtype into a Nullable structure */ - MonoObjectHandle nullable_h = mono_object_new_handle (m->klass, error); - goto_if_nok (error, return_null); - MonoObject* nullable = MONO_HANDLE_RAW (nullable_h); - - MonoObjectHandle boxed_h = mono_value_box_handle (m_class_get_cast_class (m->klass), obj, error); - goto_if_nok (error, return_null); - mono_nullable_init ((guint8 *)mono_object_unbox_internal (nullable), MONO_HANDLE_RAW (boxed_h), m->klass); - obj = mono_object_unbox_internal (nullable); - } - } - - result = mono_runtime_invoke_checked (m, obj, pa, error); - MONO_HANDLE_PIN (result); - goto_if_nok (error, return_null); - - if (sig->ret->type == MONO_TYPE_PTR) { - result = mono_boxed_intptr_to_pointer (result, sig->ret, error); - goto_if_nok (error, return_null); - } - } + result = mono_runtime_try_invoke_byrefs (m, obj, params_byref, NULL, error); goto exit; return_null: diff --git a/src/mono/mono/metadata/object-internals.h b/src/mono/mono/metadata/object-internals.h index c20d5306917281..3967256d5bcbb4 100644 --- a/src/mono/mono/metadata/object-internals.h +++ b/src/mono/mono/metadata/object-internals.h @@ -1775,9 +1775,6 @@ mono_error_set_pending_exception (MonoError *error) MonoArray * mono_glist_to_array (GList *list, MonoClass *eclass, MonoError *error); -MonoObject* -mono_boxed_intptr_to_pointer (MonoObject *boxed_intptr, MonoType *ret_type, MonoError *error); - MONO_COMPONENT_API MonoObject * mono_object_new_checked (MonoClass *klass, MonoError *error); @@ -1881,6 +1878,10 @@ mono_runtime_invoke_handle (MonoMethod *method, MonoObjectHandle obj, void **par void mono_runtime_invoke_handle_void (MonoMethod *method, MonoObjectHandle obj, void **params, MonoError* error); +MonoObject* +mono_runtime_try_invoke_byrefs (MonoMethod *method, void *obj, gpointer *params_byref, + MonoObject **exc, MonoError *error); + void* mono_compile_method_checked (MonoMethod *method, MonoError *error); diff --git a/src/mono/mono/metadata/object.c b/src/mono/mono/metadata/object.c index 77605175d95929..e7d807e73c5c39 100644 --- a/src/mono/mono/metadata/object.c +++ b/src/mono/mono/metadata/object.c @@ -4554,7 +4554,7 @@ mono_runtime_try_exec_main (MonoMethod *method, MonoArray *args, MonoObject **ex return do_try_exec_main (method, args, exc); } -MonoObject* +static MonoObject* mono_boxed_intptr_to_pointer (MonoObject *boxed_intptr, MonoType *ret_type, MonoError *error) { MonoClass *pointer_class; @@ -4659,6 +4659,143 @@ mono_runtime_invoke_array (MonoMethod *method, void *obj, MonoArray *params, return res; } +static gpointer +invoke_byrefs_extract_argument (gpointer *params_byref, int i, MonoType *t) +{ + gpointer result = NULL; +again: + switch (t->type) { + case MONO_TYPE_U1: + case MONO_TYPE_I1: + case MONO_TYPE_BOOLEAN: + case MONO_TYPE_U2: + case MONO_TYPE_I2: + case MONO_TYPE_CHAR: + case MONO_TYPE_U: + case MONO_TYPE_I: + case MONO_TYPE_U4: + case MONO_TYPE_I4: + case MONO_TYPE_U8: + case MONO_TYPE_I8: + case MONO_TYPE_R4: + case MONO_TYPE_R8: + case MONO_TYPE_VALUETYPE: + result = params_byref [i]; + break; + case MONO_TYPE_STRING: + case MONO_TYPE_OBJECT: + case MONO_TYPE_CLASS: + case MONO_TYPE_ARRAY: + case MONO_TYPE_SZARRAY: + if (m_type_is_byref (t)) + result = params_byref [i]; + else + result = *(MonoObject**)params_byref [i]; + break; + case MONO_TYPE_GENERICINST: + if (m_type_is_byref (t)) + t = m_class_get_this_arg (t->data.generic_class->container_class); + else + t = m_class_get_byval_arg (t->data.generic_class->container_class); + goto again; + case MONO_TYPE_PTR: { + result = *(gpointer*)params_byref [i]; + break; + } + default: + g_error ("type 0x%x not handled in ves_icall_InternalInvoke", t->type); + } + + return result; +} + +MonoObject* +mono_runtime_try_invoke_byrefs (MonoMethod *method, void *obj, gpointer *params_byref, + MonoObject **exc, MonoError *error) +{ + MonoMethodSignature *sig = mono_method_signature_internal (method); + int params_length = sig->param_count; + gpointer *pa = NULL; + MonoObject *res; + + if (params_length > 0) { + pa = g_newa (gpointer, params_length); + for (int i = 0; i < params_length; i++) { + MonoType *t = sig->params [i]; + pa [i] = invoke_byrefs_extract_argument (params_byref, i, t); + goto_if_nok (error, exit_null); + } + } + if (!strcmp (method->name, ".ctor") && method->klass != mono_defaults.string_class) { + gpointer o = obj; + if (mono_class_is_nullable (method->klass)) { + /* Need to create a boxed vtype instead */ + g_assert (!obj); + + if (params_length == 0) { + goto_if_nok (error, exit_null); + } else { + res = mono_value_box_checked (m_class_get_cast_class (method->klass), pa [0], error); + goto exit; + } + } + + if (!obj) { + MonoObjectHandle obj_h = mono_object_new_handle (method->klass, error); + goto_if_nok (error, exit_null); + obj = MONO_HANDLE_RAW (obj_h); + g_assert (obj); + if (m_class_is_valuetype (method->klass)) + o = mono_object_unbox_internal ((MonoObject *)obj); + else + o = obj; + } else if (m_class_is_valuetype (method->klass)) { + MonoObjectHandle obj_h = mono_value_box_handle (method->klass, obj, error); + goto_if_nok (error, exit_null); + obj = MONO_HANDLE_RAW (obj_h); + } + + if (exc) + mono_runtime_try_invoke (method, o, pa, exc, error); + else + mono_runtime_invoke_checked (method, o, pa, error); + res = (MonoObject*)obj; + } else { + if (mono_class_is_nullable (method->klass)) { + if (method->flags & METHOD_ATTRIBUTE_STATIC) { + obj = NULL; + } else { + /* Convert the unboxed vtype into a Nullable structure */ + MonoObjectHandle nullable_h = mono_object_new_handle (method->klass, error); + goto_if_nok (error, exit_null); + MonoObject* nullable = MONO_HANDLE_RAW (nullable_h); + + mono_nullable_init_unboxed ((guint8 *)mono_object_unbox_internal (nullable), obj, method->klass); + obj = mono_object_unbox_internal (nullable); + } + } + + if (exc) + res = mono_runtime_try_invoke (method, obj, pa, exc, error); + else + res = mono_runtime_invoke_checked (method, obj, pa, error); + + MONO_HANDLE_PIN (res); + goto_if_nok (error, exit_null); + + if (sig->ret->type == MONO_TYPE_PTR) { + res = mono_boxed_intptr_to_pointer (res, sig->ret, error); + goto_if_nok (error, exit_null); + } + } + + goto exit; +exit_null: + res = NULL; +exit: + return res; +} + // FIXME these will move to header soon static MonoObjectHandle mono_object_new_by_vtable (MonoVTable *vtable, MonoError *error); From ec1dcb9c635e2548a0e2f068b4437874ea17c007 Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Tue, 18 Oct 2022 01:09:50 +0300 Subject: [PATCH 14/19] [mono] Simplify nullable ctor case Nullable only has 1 param constructor that creates a boxed vtype --- src/mono/mono/metadata/object.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/mono/mono/metadata/object.c b/src/mono/mono/metadata/object.c index e7d807e73c5c39..a55a501ce2886b 100644 --- a/src/mono/mono/metadata/object.c +++ b/src/mono/mono/metadata/object.c @@ -4731,13 +4731,8 @@ mono_runtime_try_invoke_byrefs (MonoMethod *method, void *obj, gpointer *params_ if (mono_class_is_nullable (method->klass)) { /* Need to create a boxed vtype instead */ g_assert (!obj); - - if (params_length == 0) { - goto_if_nok (error, exit_null); - } else { - res = mono_value_box_checked (m_class_get_cast_class (method->klass), pa [0], error); - goto exit; - } + res = mono_value_box_checked (m_class_get_cast_class (method->klass), pa [0], error); + goto exit; } if (!obj) { From 16aeff6c46b460b3d97d4e22681efd6016a09826 Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Thu, 13 Oct 2022 19:08:06 +0300 Subject: [PATCH 15/19] [mono] Extract invoke code to be reused by mono_runtime_invoke_array --- src/mono/mono/metadata/object.c | 105 ++++++++++++++++++-------------- 1 file changed, 58 insertions(+), 47 deletions(-) diff --git a/src/mono/mono/metadata/object.c b/src/mono/mono/metadata/object.c index a55a501ce2886b..1a7b3d529641df 100644 --- a/src/mono/mono/metadata/object.c +++ b/src/mono/mono/metadata/object.c @@ -4598,6 +4598,48 @@ mono_boxed_intptr_to_pointer (MonoObject *boxed_intptr, MonoType *ret_type, Mono return res; } +static gpointer +extract_this_ptr (MonoMethod *method, gpointer this_ptr, MonoObject **res, MonoError *error) +{ + gpointer new_this_ptr = this_ptr; + if (!strcmp (method->name, ".ctor") && method->klass != mono_defaults.string_class) { + MonoObject *newobj; + if (!this_ptr) { + newobj = mono_object_new_checked (method->klass, error); + MONO_HANDLE_PIN (newobj); + return_val_if_nok (error, NULL); + if (m_class_is_valuetype (method->klass)) + new_this_ptr = mono_object_unbox_internal (newobj); + else + new_this_ptr = newobj; + *res = newobj; + } else if (m_class_is_valuetype (method->klass)) { + newobj = mono_value_box_checked (method->klass, this_ptr, error); + MONO_HANDLE_PIN (newobj); + return_val_if_nok (error, NULL); + *res = newobj; + } else { + *res = (MonoObject*)this_ptr; + } + + } else { + if (mono_class_is_nullable (method->klass)) { + if (method->flags & METHOD_ATTRIBUTE_STATIC) { + new_this_ptr = NULL; + } else { + /* Convert the unboxed vtype into a Nullable structure */ + MonoObject *newobj = mono_object_new_checked (method->klass, error); + MONO_HANDLE_PIN (newobj); + + mono_nullable_init_unboxed ((guint8 *)mono_object_unbox_internal (newobj), this_ptr, method->klass); + new_this_ptr = mono_object_unbox_internal (newobj); + } + } + *res = NULL; + } + return new_this_ptr; +} + /** * mono_runtime_invoke_array: * \param method method to invoke @@ -4726,57 +4768,26 @@ mono_runtime_try_invoke_byrefs (MonoMethod *method, void *obj, gpointer *params_ goto_if_nok (error, exit_null); } } - if (!strcmp (method->name, ".ctor") && method->klass != mono_defaults.string_class) { - gpointer o = obj; - if (mono_class_is_nullable (method->klass)) { - /* Need to create a boxed vtype instead */ - g_assert (!obj); - res = mono_value_box_checked (m_class_get_cast_class (method->klass), pa [0], error); - goto exit; - } - - if (!obj) { - MonoObjectHandle obj_h = mono_object_new_handle (method->klass, error); - goto_if_nok (error, exit_null); - obj = MONO_HANDLE_RAW (obj_h); - g_assert (obj); - if (m_class_is_valuetype (method->klass)) - o = mono_object_unbox_internal ((MonoObject *)obj); - else - o = obj; - } else if (m_class_is_valuetype (method->klass)) { - MonoObjectHandle obj_h = mono_value_box_handle (method->klass, obj, error); - goto_if_nok (error, exit_null); - obj = MONO_HANDLE_RAW (obj_h); - } - - if (exc) - mono_runtime_try_invoke (method, o, pa, exc, error); - else - mono_runtime_invoke_checked (method, o, pa, error); - res = (MonoObject*)obj; - } else { - if (mono_class_is_nullable (method->klass)) { - if (method->flags & METHOD_ATTRIBUTE_STATIC) { - obj = NULL; - } else { - /* Convert the unboxed vtype into a Nullable structure */ - MonoObjectHandle nullable_h = mono_object_new_handle (method->klass, error); - goto_if_nok (error, exit_null); - MonoObject* nullable = MONO_HANDLE_RAW (nullable_h); + if (!strcmp (method->name, ".ctor") && mono_class_is_nullable (method->klass)) { + /* Need to create a boxed vtype instead */ + g_assert (!obj); + res = mono_value_box_checked (m_class_get_cast_class (method->klass), pa [0], error); + goto exit; + } - mono_nullable_init_unboxed ((guint8 *)mono_object_unbox_internal (nullable), obj, method->klass); - obj = mono_object_unbox_internal (nullable); - } - } + obj = extract_this_ptr (method, obj, &res, error); + goto_if_nok (error, exit_null); - if (exc) - res = mono_runtime_try_invoke (method, obj, pa, exc, error); - else - res = mono_runtime_invoke_checked (method, obj, pa, error); + MonoObject *invoke_res; + if (exc) + invoke_res = mono_runtime_try_invoke (method, obj, pa, exc, error); + else + invoke_res = mono_runtime_invoke_checked (method, obj, pa, error); + goto_if_nok (error, exit_null); + if (!res) { + res = invoke_res; MONO_HANDLE_PIN (res); - goto_if_nok (error, exit_null); if (sig->ret->type == MONO_TYPE_PTR) { res = mono_boxed_intptr_to_pointer (res, sig->ret, error); From 4f30ce50b0a7d8c7e0bbefe1fb3aa2fc0eaff8ce Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Wed, 12 Oct 2022 20:04:17 +0300 Subject: [PATCH 16/19] [mono] Add back support for the embedding api With same behavior as before, except we have to handle conversions between nullable and boxedvt ourserlves, since it is no longer done by the runtime invoke wrapper. --- src/mono/mono/metadata/object.c | 216 ++++++++++++++++++++++++++++++-- 1 file changed, 208 insertions(+), 8 deletions(-) diff --git a/src/mono/mono/metadata/object.c b/src/mono/mono/metadata/object.c index 1a7b3d529641df..6e8e933bce6357 100644 --- a/src/mono/mono/metadata/object.c +++ b/src/mono/mono/metadata/object.c @@ -2460,16 +2460,59 @@ mono_runtime_invoke (MonoMethod *method, void *obj, void **params, MonoObject ** MonoObject *res; MONO_ENTER_GC_UNSAFE; ERROR_DECL (error); + gboolean has_byref_nullables = FALSE; + void **params_copy = NULL; + void **params_arg; + + MonoMethodSignature *sig = mono_method_signature_internal (method); + for (int i = 0; i < sig->param_count; i++) { + MonoType *t = sig->params [i]; + if (t->type == MONO_TYPE_GENERICINST && t->data.generic_class->container_class == mono_defaults.generic_nullable_class) { + MonoClass *klass = mono_class_from_mono_type_internal (t); + MonoObject *boxed_vt = (MonoObject*)params [i]; + gpointer unboxed_vt = mono_object_unbox_internal (boxed_vt); + gpointer nullable_vt = g_alloca (mono_class_value_size (klass, NULL)); + + mono_nullable_init_unboxed (nullable_vt, unboxed_vt, klass); + if (!params_copy) { + params_copy = g_alloca (sig->param_count * sizeof (void*)); + memcpy (params_copy, params, sig->param_count * sizeof (void*)); + } + params_copy [i] = nullable_vt; + if (m_type_is_byref (t)) + has_byref_nullables = TRUE; + } + } + + if (params_copy != NULL) + params_arg = params_copy; + else + params_arg = params; + if (exc) { - res = mono_runtime_try_invoke (method, obj, params, exc, error); + res = mono_runtime_try_invoke (method, obj, params_arg, exc, error); if (*exc == NULL && !is_ok(error)) { *exc = (MonoObject*) mono_error_convert_to_exception (error); } else mono_error_cleanup (error); } else { - res = mono_runtime_invoke_checked (method, obj, params, error); + res = mono_runtime_invoke_checked (method, obj, params_arg, error); mono_error_raise_exception_deprecated (error); /* OK to throw, external only without a good alternative */ } + + if (has_byref_nullables) { + // The params_args will contain the ref to the modified nullable. We need + // to return it as boxed vt or NULL + for (int i = 0; i < sig->param_count; i++) { + MonoType *t = sig->params [i]; + if (t->type == MONO_TYPE_GENERICINST && m_type_is_byref (t) && t->data.generic_class->container_class == mono_defaults.generic_nullable_class) { + MonoClass *klass = mono_class_from_mono_type_internal (t); + gpointer nullable_vt = params_arg [i]; + params [i] = mono_nullable_box (nullable_vt, klass, error); + } + } + } + MONO_EXIT_GC_UNSAFE; return res; } @@ -4640,6 +4683,106 @@ extract_this_ptr (MonoMethod *method, gpointer this_ptr, MonoObject **res, MonoE return new_this_ptr; } +static gpointer +invoke_array_extract_argument (MonoArray *array, int i, MonoType *t, gboolean* has_byref_nullables, MonoError *error) +{ + MonoType *t_orig = t; + gpointer result = NULL; + error_init (error); +again: + switch (t->type) { + case MONO_TYPE_U1: + case MONO_TYPE_I1: + case MONO_TYPE_BOOLEAN: + case MONO_TYPE_U2: + case MONO_TYPE_I2: + case MONO_TYPE_CHAR: + case MONO_TYPE_U: + case MONO_TYPE_I: + case MONO_TYPE_U4: + case MONO_TYPE_I4: + case MONO_TYPE_U8: + case MONO_TYPE_I8: + case MONO_TYPE_R4: + case MONO_TYPE_R8: + case MONO_TYPE_VALUETYPE: + if (t->type == MONO_TYPE_VALUETYPE && mono_class_is_nullable (mono_class_from_mono_type_internal (t_orig))) { + // Convert from boxed_vt to pointer to nullable data + MonoClass *klass = mono_class_from_mono_type_internal (t_orig); + MonoObject *boxed_vt = mono_array_get_internal (array, MonoObject*, i); + MONO_HANDLE_PIN (boxed_vt); + gpointer unboxed_vt = mono_object_unbox_internal (boxed_vt); + MonoObject *nullable = mono_object_new_checked (klass, error); + MONO_HANDLE_PIN (nullable); + result = mono_object_unbox_internal (nullable); + mono_nullable_init_unboxed (result, unboxed_vt, klass); + if (m_type_is_byref (t)) + *has_byref_nullables = TRUE; + } else { + /* MS seems to create the objects if a null is passed in */ + gboolean was_null = FALSE; + if (!mono_array_get_internal (array, MonoObject*, i)) { + MonoObject *o = mono_object_new_checked (mono_class_from_mono_type_internal (t_orig), error); + return_val_if_nok (error, NULL); + mono_array_setref_internal (array, i, o); + was_null = TRUE; + } + if (m_type_is_byref (t)) { + /* + * We can't pass the unboxed vtype byref to the callee, since + * that would mean the callee would be able to modify boxed + * primitive types. So we (and MS) make a copy of the boxed + * object, pass that to the callee, and replace the original + * boxed object in the arg array with the copy. + */ + MonoObject *orig = mono_array_get_internal (array, MonoObject*, i); + MonoObject *copy = mono_value_box_checked (orig->vtable->klass, mono_object_unbox_internal (orig), error); + return_val_if_nok (error, NULL); + mono_array_setref_internal (array, i, copy); + } + result = mono_array_get_internal (array, MonoObject*, i); + MONO_HANDLE_PIN (result); + result = mono_object_unbox_internal (result); + if (!m_type_is_byref (t) && was_null) + mono_array_set_internal (array, MonoObject*, i, NULL); + } + break; + case MONO_TYPE_STRING: + case MONO_TYPE_OBJECT: + case MONO_TYPE_CLASS: + case MONO_TYPE_ARRAY: + case MONO_TYPE_SZARRAY: + if (m_type_is_byref (t)) { + result = mono_array_addr_internal (array, MonoObject*, i); + } else { + result = mono_array_get_internal (array, MonoObject*, i); + MONO_HANDLE_PIN (result); + } + break; + case MONO_TYPE_GENERICINST: + if (m_type_is_byref (t)) + t = m_class_get_this_arg (t->data.generic_class->container_class); + else + t = m_class_get_byval_arg (t->data.generic_class->container_class); + goto again; + case MONO_TYPE_PTR: { + MonoObject *arg; + /* The argument should be an IntPtr */ + arg = mono_array_get_internal (array, MonoObject*, i); + if (arg == NULL) { + result = NULL; + } else { + g_assert (arg->vtable->klass == mono_defaults.int_class || arg->vtable->klass == mono_defaults.uint_class); + result = ((MonoIntPtr*)arg)->m_value; + } + break; + } + default: + g_error ("type 0x%x not handled in mono_runtime_invoke_array", t_orig->type); + } + return result; +} + /** * mono_runtime_invoke_array: * \param method method to invoke @@ -4683,20 +4826,77 @@ mono_runtime_invoke_array (MonoMethod *method, void *obj, MonoArray *params, MonoObject *res; MONO_ENTER_GC_UNSAFE; ERROR_DECL (error); + gpointer *pa = NULL; + MonoMethodSignature *sig = mono_method_signature_internal (method); + gboolean has_byref_nullables = FALSE; + int param_count = sig->param_count; + + // This method has similar behavior as mono_runtime_try_invoke_byrefs. The only + // difference is that one receives an managed array of objects, passed over by + // the embedder, while the other one receives an array of byrefs, initialized by + // our reflection code. So the only difference lies in transforming these arguments + // to what is required by the runtime invoke wrapper. + + if (param_count) { + pa = g_newa (gpointer, param_count); + for (int i = 0; i < sig->param_count; i++) { + MonoType *t = sig->params [i]; + pa [i] = invoke_array_extract_argument (params, i, t, &has_byref_nullables, error); + goto_if_nok (error, return_error); + } + } + if (!strcmp (method->name, ".ctor") && mono_class_is_nullable (method->klass)) { + /* Need to create a boxed vtype instead */ + g_assert (!obj); + res = mono_value_box_checked (m_class_get_cast_class (method->klass), pa [0], error); + goto return_res; + } + + MonoObject *invoke_res; + obj = extract_this_ptr (method, obj, &res, error); + goto_if_nok (error, return_error); + if (exc) { - g_error ("FIXME"); - res = NULL; + invoke_res = mono_runtime_try_invoke (method, obj, pa, exc, error); if (*exc) { res = NULL; mono_error_cleanup (error); - } else if (!is_ok (error)) { - *exc = (MonoObject*)mono_error_convert_to_exception (error); + goto return_res; + } else { + goto_if_nok (error, return_error); } } else { - g_error ("FIXME"); - res = NULL; + invoke_res = mono_runtime_invoke_checked (method, obj, pa, error); mono_error_raise_exception_deprecated (error); /* OK to throw, external only without a good alternative */ } + + if (!res) + res = invoke_res; + if (sig->ret->type == MONO_TYPE_PTR) { + res = mono_boxed_intptr_to_pointer (res, sig->ret, error); + goto_if_nok (error, return_error); + } + + if (has_byref_nullables) { + // The params_args will contain the ref to the modified nullable. We need + // to return it as boxed vt or NULL + for (int i = 0; i < param_count; i++) { + MonoType *t = sig->params [i]; + if (t->type == MONO_TYPE_GENERICINST && m_type_is_byref (t) && t->data.generic_class->container_class == mono_defaults.generic_nullable_class) { + MonoClass *klass = mono_class_from_mono_type_internal (t); + MonoObject *boxed_vt = mono_nullable_box (pa [i], klass, error); + goto_if_nok (error, return_error); + mono_array_setref_internal (params, i, boxed_vt); + } + } + } + + goto return_res; +return_error: + res = NULL; + if (exc) + *exc = (MonoObject*)mono_error_convert_to_exception (error); +return_res: MONO_EXIT_GC_UNSAFE; return res; } From 2c0349352a815740416416c94b82cea5ef82476b Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Tue, 18 Oct 2022 11:19:02 +0300 Subject: [PATCH 17/19] [mono] Don't trigger ambiguous method exception when having duplicate method overrides Seems to be possible via reflection, ex System.Reflection.Emit.Tests.MethodBuilderDefineMethodOverride.DefineMethodOverride_CalledTwiceWithSameBodies_Works --- src/mono/mono/metadata/class-setup-vtable.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mono/mono/metadata/class-setup-vtable.c b/src/mono/mono/metadata/class-setup-vtable.c index e9dc6997d89e7b..19c32f25fb1446 100644 --- a/src/mono/mono/metadata/class-setup-vtable.c +++ b/src/mono/mono/metadata/class-setup-vtable.c @@ -1065,7 +1065,7 @@ handle_dim_conflicts (MonoMethod **vtable, MonoClass *klass, GHashTable *conflic int nentries = 0; MonoMethod *impl = NULL; for (l = entries; l; l = l->next) { - if (l->data) { + if (l->data && l->data != impl) { nentries ++; impl = (MonoMethod*)l->data; } From ecbf3c80bd554159b2633755d1dcccfb4927dae8 Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Sat, 15 Oct 2022 00:13:58 +0300 Subject: [PATCH 18/19] Disable some warnings --- .../src/System/Runtime/InteropServices/Marshal.Mono.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/mono/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.Mono.cs b/src/mono/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.Mono.cs index b626b4df3eb05b..602ae2999c6102 100644 --- a/src/mono/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.Mono.cs +++ b/src/mono/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.Mono.cs @@ -118,11 +118,15 @@ public int GetHashCode((Type, string) key) private static Dictionary<(Type, string), ICustomMarshaler>? MarshalerInstanceCache; +#pragma warning disable 8500 +#pragma warning disable 9080 private static unsafe void SetInvokeArgs(ref string cookie, IntPtr *params_byref) { ByReference objRef = ByReference.Create(ref cookie); *(ByReference*)params_byref = objRef; } +#pragma warning restore 9080 +#pragma warning restore 8500 [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2070:UnrecognizedReflectionPattern", Justification = "Implementation detail of MarshalAs.CustomMarshaler")] From 93ca85ffed7214cef576cb1dd5c946a8c269b649 Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Thu, 20 Oct 2022 16:20:52 +0300 Subject: [PATCH 19/19] Disable tests on wasm aot since they rely on stacktraces --- .../Common/tests/System/Reflection/InvokeInterpretedTests.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libraries/Common/tests/System/Reflection/InvokeInterpretedTests.cs b/src/libraries/Common/tests/System/Reflection/InvokeInterpretedTests.cs index 5d4347e82d8852..a197da333f16d5 100644 --- a/src/libraries/Common/tests/System/Reflection/InvokeInterpretedTests.cs +++ b/src/libraries/Common/tests/System/Reflection/InvokeInterpretedTests.cs @@ -8,6 +8,7 @@ namespace System.Reflection.Tests public class InvokeInterpretedTests { [Fact] + [ActiveIssue("https://github.com/dotnet/runtime/issues/50957", typeof(PlatformDetection), nameof(PlatformDetection.IsBrowser), nameof(PlatformDetection.IsMonoAOT))] public static void VerifyInvokeIsUsingEmit_Method() { MethodInfo method = typeof(TestClassThatThrows).GetMethod(nameof(TestClassThatThrows.Throw))!; @@ -24,6 +25,7 @@ string InterpretedMethodName() => PlatformDetection.IsMonoRuntime ? } [Fact] + [ActiveIssue("https://github.com/dotnet/runtime/issues/50957", typeof(PlatformDetection), nameof(PlatformDetection.IsBrowser), nameof(PlatformDetection.IsMonoAOT))] public static void VerifyInvokeIsUsingEmit_Constructor() { ConstructorInfo ctor = typeof(TestClassThatThrows).GetConstructor(Type.EmptyTypes)!;