From addc2a7bc0cd8665e2fd1393e98f75ee209796b5 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Mon, 4 Nov 2019 06:39:19 +0300 Subject: [PATCH] [netcore] Improve Array.CreateInstance (mono/mono#17673) * Rewrite Array.CreateInstance * Rewrite Array.CreateInstance * Address feedback * Convert the icall to NOHANDLES * fix build errors, remove NOHANDLES * Fix GC.KeepAlive usage * Remove icall definition for GC.KeepAlive (it's no an icall) * Fix failing tests (expect details in the AOORE) Commit migrated from https://github.com/mono/mono/commit/137ffe8b7c18d8a153f9311a8b30b53139ee2073 --- src/mono/mono/metadata/icall-def-netcore.h | 3 +- src/mono/mono/metadata/icall.c | 50 ++++++++ .../src/System/Array.Mono.cs | 141 +++++++++++---------- 3 files changed, 126 insertions(+), 68 deletions(-) diff --git a/src/mono/mono/metadata/icall-def-netcore.h b/src/mono/mono/metadata/icall-def-netcore.h index bb26e31..7091c0f 100644 --- a/src/mono/mono/metadata/icall-def-netcore.h +++ b/src/mono/mono/metadata/icall-def-netcore.h @@ -39,7 +39,6 @@ NOHANDLES(ICALL(ARGI_4, "Setup", ves_icall_System_ArgIterator_Se ICALL_TYPE(ARRAY, "System.Array", ARRAY_0) HANDLES(ARRAY_0, "CanChangePrimitive", ves_icall_System_Array_CanChangePrimitive, MonoBoolean, 3, (MonoReflectionType, MonoReflectionType, MonoBoolean)) HANDLES(ARRAY_1, "ClearInternal", ves_icall_System_Array_ClearInternal, void, 3, (MonoArray, int, int)) -HANDLES(ARRAY_3, "CreateInstanceImpl", ves_icall_System_Array_CreateInstanceImpl, MonoArray, 3, (MonoReflectionType, MonoArray, MonoArray)) HANDLES(ARRAY_4, "FastCopy", ves_icall_System_Array_FastCopy, MonoBoolean, 5, (MonoArray, int, MonoArray, int, int)) NOHANDLES(ICALL(ARRAY_5, "GetGenericValue_icall", ves_icall_System_Array_GetGenericValue_icall)) HANDLES(ARRAY_6, "GetLength", ves_icall_System_Array_GetLength, gint32, 2, (MonoArray, gint32)) @@ -48,6 +47,7 @@ HANDLES(ARRAY_7, "GetLowerBound", ves_icall_System_Array_GetLowerBound, gint3 HANDLES(ARRAY_8, "GetRank", ves_icall_System_Array_GetRank, gint32, 1, (MonoObject)) HANDLES(ARRAY_9, "GetValue", ves_icall_System_Array_GetValue, MonoObject, 2, (MonoArray, MonoArray)) HANDLES(ARRAY_10, "GetValueImpl", ves_icall_System_Array_GetValueImpl, MonoObject, 2, (MonoArray, guint32)) +HANDLES(ARRAY_10a, "InternalCreate", ves_icall_System_Array_InternalCreate, MonoArray, 4, (MonoType_ptr, gint32, gint32_ptr, gint32_ptr)) NOHANDLES(ICALL(ARRAY_11, "SetGenericValue_icall", ves_icall_System_Array_SetGenericValue_icall)) HANDLES(ARRAY_12, "SetValue", ves_icall_System_Array_SetValue, void, 3, (MonoArray, MonoObject, MonoArray)) HANDLES(ARRAY_13, "SetValueImpl", ves_icall_System_Array_SetValueImpl, void, 3, (MonoArray, MonoObject, guint32)) @@ -100,7 +100,6 @@ NOHANDLES(ICALL(GC_0b, "GetMaxGeneration", ves_icall_System_GC_GetMaxGeneration) HANDLES(GC_11, "GetTotalAllocatedBytes", ves_icall_System_GC_GetTotalAllocatedBytes, guint64, 1, (MonoBoolean)) NOHANDLES(ICALL(GC_1, "GetTotalMemory", ves_icall_System_GC_GetTotalMemory)) NOHANDLES(ICALL(GC_2, "InternalCollect", ves_icall_System_GC_InternalCollect)) -HANDLES(GC_3, "KeepAlive", ves_icall_System_GC_KeepAlive, void, 1, (MonoObject)) NOHANDLES(ICALL(GC_4a, "RecordPressure", ves_icall_System_GC_RecordPressure)) NOHANDLES(ICALL(GC_6, "WaitForPendingFinalizers", ves_icall_System_GC_WaitForPendingFinalizers)) HANDLES(GC_6b, "_ReRegisterForFinalize", ves_icall_System_GC_ReRegisterForFinalize, void, 1, (MonoObject)) diff --git a/src/mono/mono/metadata/icall.c b/src/mono/mono/metadata/icall.c index 3a28116..8417b08 100644 --- a/src/mono/mono/metadata/icall.c +++ b/src/mono/mono/metadata/icall.c @@ -817,6 +817,56 @@ ves_icall_System_Array_SetValue (MonoArrayHandle arr, MonoObjectHandle value, array_set_value_impl (arr, value, pos, TRUE, TRUE, error); } +#ifdef ENABLE_NETCORE +MonoArrayHandle +ves_icall_System_Array_InternalCreate (MonoType* type, gint32 rank, gint32* pLengths, gint32* pLowerBounds, MonoError *error) +{ + MonoClass* klass = mono_class_from_mono_type_internal (type); + if (!mono_class_init_checked (klass, error)) + return NULL_HANDLE_ARRAY; + + if (m_class_get_byval_arg (m_class_get_element_class (klass))->type == MONO_TYPE_VOID) { + mono_error_set_not_supported (error, "Arrays of System.Void are not supported."); + return NULL_HANDLE_ARRAY; + } + + if (type->byref || m_class_is_byreflike (klass)) { + mono_error_set_not_supported (error, NULL); + return NULL_HANDLE_ARRAY; + } + + MonoGenericClass *gklass = mono_class_try_get_generic_class (klass); + if (is_generic_parameter (type) || mono_class_is_gtd (klass) || (gklass && gklass->context.class_inst->is_open)) { + mono_error_set_not_supported (error, NULL); + return NULL_HANDLE_ARRAY; + } + + /* vectors are not the same as one dimensional arrays with non-zero bounds */ + const gboolean bounded = pLowerBounds != NULL && rank == 1 && pLowerBounds [0] != 0; + + MonoClass* const aklass = mono_class_create_bounded_array (klass, rank, bounded); + uintptr_t const aklass_rank = m_class_get_rank (aklass); + uintptr_t* const sizes = g_newa (uintptr_t, aklass_rank); + intptr_t* const lower_bounds = g_newa (intptr_t, aklass_rank); + + // Copy lengths and lower_bounds from gint32 to [u]intptr_t. + for (uintptr_t i = 0; i < aklass_rank; ++i) { + if (pLowerBounds != NULL) { + lower_bounds [i] = pLowerBounds [i]; + if ((gint64) pLowerBounds [i] + (gint64) pLengths [i] > G_MAXINT32) { + mono_error_set_argument_out_of_range (error, NULL, "Length + bound must not exceed Int32.MaxValue."); + return NULL_HANDLE_ARRAY; + } + } else { + lower_bounds [i] = 0; + } + sizes [i] = pLengths [i]; + } + + return mono_array_new_full_handle (mono_domain_get (), aklass, sizes, lower_bounds, error); +} +#endif + MonoArrayHandle ves_icall_System_Array_CreateInstanceImpl (MonoReflectionTypeHandle type, MonoArrayHandle lengths, MonoArrayHandle bounds, MonoError *error) { diff --git a/src/mono/netcore/System.Private.CoreLib/src/System/Array.Mono.cs b/src/mono/netcore/System.Private.CoreLib/src/System/Array.Mono.cs index 5ae3c7b..0b400ea 100644 --- a/src/mono/netcore/System.Private.CoreLib/src/System/Array.Mono.cs +++ b/src/mono/netcore/System.Private.CoreLib/src/System/Array.Mono.cs @@ -221,106 +221,118 @@ namespace System return false; } - public static Array CreateInstance (Type elementType, int length) + public static unsafe Array CreateInstance (Type elementType, int length) { + if (elementType is null) + ThrowHelper.ThrowArgumentNullException (ExceptionArgument.elementType); if (length < 0) - throw new ArgumentOutOfRangeException (nameof (length)); + ThrowHelper.ThrowLengthArgumentOutOfRange_ArgumentOutOfRange_NeedNonNegNum (); - int[] lengths = {length}; + RuntimeType? runtimeType = elementType.UnderlyingSystemType as RuntimeType; + if (runtimeType == null) + ThrowHelper.ThrowArgumentException (ExceptionResource.Arg_MustBeType, ExceptionArgument.elementType); - return CreateInstance (elementType, lengths); + Array array = InternalCreate (runtimeType._impl.Value, 1, &length, null); + GC.KeepAlive (runtimeType); + return array; } - public static Array CreateInstance (Type elementType, int length1, int length2) + public static unsafe Array CreateInstance (Type elementType, int length1, int length2) { + if (elementType is null) + ThrowHelper.ThrowArgumentNullException (ExceptionArgument.elementType); if (length1 < 0) - throw new ArgumentOutOfRangeException (nameof (length1)); + ThrowHelper.ThrowArgumentOutOfRangeException (ExceptionArgument.length1, ExceptionResource.ArgumentOutOfRange_NeedNonNegNum); if (length2 < 0) - throw new ArgumentOutOfRangeException (nameof (length2)); + ThrowHelper.ThrowArgumentOutOfRangeException (ExceptionArgument.length2, ExceptionResource.ArgumentOutOfRange_NeedNonNegNum); - int[] lengths = {length1, length2}; + RuntimeType? runtimeType = elementType.UnderlyingSystemType as RuntimeType; + if (runtimeType == null) + ThrowHelper.ThrowArgumentException (ExceptionResource.Arg_MustBeType, ExceptionArgument.elementType); - return CreateInstance (elementType, lengths); + int* lengths = stackalloc int [] { length1, length2 }; + Array array = InternalCreate (runtimeType._impl.Value, 2, lengths, null); + GC.KeepAlive (runtimeType); + return array; } - public static Array CreateInstance (Type elementType, int length1, int length2, int length3) + public static unsafe Array CreateInstance (Type elementType, int length1, int length2, int length3) { + if (elementType is null) + ThrowHelper.ThrowArgumentNullException (ExceptionArgument.elementType); if (length1 < 0) - throw new ArgumentOutOfRangeException (nameof (length1)); + ThrowHelper.ThrowArgumentOutOfRangeException (ExceptionArgument.length1, ExceptionResource.ArgumentOutOfRange_NeedNonNegNum); if (length2 < 0) - throw new ArgumentOutOfRangeException (nameof (length2)); + ThrowHelper.ThrowArgumentOutOfRangeException (ExceptionArgument.length2, ExceptionResource.ArgumentOutOfRange_NeedNonNegNum); if (length3 < 0) - throw new ArgumentOutOfRangeException (nameof (length3)); + ThrowHelper.ThrowArgumentOutOfRangeException (ExceptionArgument.length3, ExceptionResource.ArgumentOutOfRange_NeedNonNegNum); - int[] lengths = {length1, length2, length3}; + RuntimeType? runtimeType = elementType.UnderlyingSystemType as RuntimeType; + if (runtimeType == null) + ThrowHelper.ThrowArgumentException (ExceptionResource.Arg_MustBeType, ExceptionArgument.elementType); - return CreateInstance (elementType, lengths); + int* lengths = stackalloc int [] { length1, length2, length3 }; + Array array = InternalCreate (runtimeType._impl.Value, 3, lengths, null); + GC.KeepAlive (runtimeType); + return array; } - public static Array CreateInstance (Type elementType, params int[] lengths) + public static unsafe Array CreateInstance (Type elementType, params int[] lengths) { - if (elementType == null) - throw new ArgumentNullException ("elementType"); + if (elementType is null) + ThrowHelper.ThrowArgumentNullException (ExceptionArgument.elementType); if (lengths == null) - throw new ArgumentNullException ("lengths"); + ThrowHelper.ThrowArgumentNullException (ExceptionArgument.lengths); if (lengths.Length == 0) - throw new ArgumentException (nameof (lengths)); - if (lengths.Length > 255) - throw new TypeLoadException (); - for (int i = 0; i < lengths.Length; ++i) { + ThrowHelper.ThrowArgumentException (ExceptionResource.Arg_NeedAtLeast1Rank); + + RuntimeType? runtimeType = elementType.UnderlyingSystemType as RuntimeType; + if (runtimeType == null) + ThrowHelper.ThrowArgumentException (ExceptionResource.Arg_MustBeType, ExceptionArgument.elementType); + + for (int i = 0; i < lengths.Length; i++) if (lengths [i] < 0) - throw new ArgumentOutOfRangeException ($"lengths[{i}]", SR.ArgumentOutOfRange_NeedNonNegNum); - } + ThrowHelper.ThrowArgumentOutOfRangeException (ExceptionArgument.lengths, i, ExceptionResource.ArgumentOutOfRange_NeedNonNegNum); - if (!(elementType.UnderlyingSystemType is RuntimeType et)) - throw new ArgumentException ("Type must be a type provided by the runtime.", "elementType"); - if (et.Equals (typeof (void))) - throw new NotSupportedException ("Array type can not be void"); - if (et.ContainsGenericParameters) - throw new NotSupportedException ("Array type can not be an open generic type"); - if (et.IsByRef) - throw new NotSupportedException (SR.NotSupported_Type); - - return CreateInstanceImpl (et, lengths, null); + Array array; + fixed (int* pLengths = &lengths [0]) + array = InternalCreate (runtimeType._impl.Value, lengths.Length, pLengths, null); + GC.KeepAlive (runtimeType); + return array; } - public static Array CreateInstance (Type elementType, int[] lengths, int [] lowerBounds) + public static unsafe Array CreateInstance (Type elementType, int[] lengths, int[] lowerBounds) { if (elementType == null) - throw new ArgumentNullException ("elementType"); + ThrowHelper.ThrowArgumentNullException (ExceptionArgument.elementType); if (lengths == null) - throw new ArgumentNullException ("lengths"); + ThrowHelper.ThrowArgumentNullException (ExceptionArgument.lengths); if (lowerBounds == null) - throw new ArgumentNullException ("lowerBounds"); - - if (!(elementType.UnderlyingSystemType is RuntimeType rt)) - throw new ArgumentException ("Type must be a type provided by the runtime.", "elementType"); - if (rt.Equals (typeof (void))) - throw new NotSupportedException ("Array type can not be void"); - if (rt.ContainsGenericParameters) - throw new NotSupportedException ("Array type can not be an open generic type"); - if (rt.IsByRef) - throw new NotSupportedException (SR.NotSupported_Type); - - if (lengths.Length < 1) - throw new ArgumentException ("Arrays must contain >= 1 elements."); - - if (lengths.Length != lowerBounds.Length) - throw new ArgumentException ("Arrays must be of same size."); + ThrowHelper.ThrowArgumentNullException (ExceptionArgument.lowerBounds); + if (lengths.Length != lowerBounds!.Length) + ThrowHelper.ThrowArgumentException (ExceptionResource.Arg_RanksAndBounds); + if (lengths.Length == 0) + ThrowHelper.ThrowArgumentException (ExceptionResource.Arg_NeedAtLeast1Rank); - for (int j = 0; j < lowerBounds.Length; j ++) { - if (lengths [j] < 0) - throw new ArgumentOutOfRangeException ($"lengths[{j}]", "Each value has to be >= 0."); - if ((long)lowerBounds [j] + (long)lengths [j] > (long)Int32.MaxValue) - throw new ArgumentOutOfRangeException (null, "Length + bound must not exceed Int32.MaxValue."); - } + for (int i = 0; i < lengths.Length; i++) + if (lengths [i] < 0) + ThrowHelper.ThrowArgumentOutOfRangeException (ExceptionArgument.lengths, i, ExceptionResource.ArgumentOutOfRange_NeedNonNegNum); - if (lengths.Length > 255) - throw new TypeLoadException (); + RuntimeType? runtimeType = elementType.UnderlyingSystemType as RuntimeType; + if (runtimeType == null) + ThrowHelper.ThrowArgumentException (ExceptionResource.Arg_MustBeType, ExceptionArgument.elementType); - return CreateInstanceImpl (elementType, lengths, lowerBounds); + Array array; + fixed (int* pLengths = &lengths [0]) + fixed (int* pLowerBounds = &lowerBounds [0]) + array = InternalCreate (runtimeType._impl.Value, lengths.Length, pLengths, pLowerBounds); + GC.KeepAlive (runtimeType); + return array; } + [MethodImpl (MethodImplOptions.InternalCall)] + static extern unsafe Array InternalCreate (IntPtr elementType, int rank, int* pLengths, int* pLowerBounds); + public object GetValue (int index) { if (Rank != 1) @@ -476,9 +488,6 @@ namespace System extern static void ClearInternal (Array a, int index, int count); [MethodImplAttribute (MethodImplOptions.InternalCall)] - extern static Array CreateInstanceImpl (Type elementType, int[] lengths, int[]? bounds); - - [MethodImplAttribute (MethodImplOptions.InternalCall)] extern static bool CanChangePrimitive (Type srcType, Type dstType, bool reliable); [MethodImplAttribute (MethodImplOptions.InternalCall)] -- 2.7.4