[netcore] Improve Array.CreateInstance (mono/mono#17673)
authorEgor Bogatov <egorbo@gmail.com>
Mon, 4 Nov 2019 03:39:19 +0000 (06:39 +0300)
committerZoltan Varga <vargaz@gmail.com>
Mon, 4 Nov 2019 03:39:19 +0000 (22:39 -0500)
* 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
src/mono/mono/metadata/icall.c
src/mono/netcore/System.Private.CoreLib/src/System/Array.Mono.cs

index bb26e31..7091c0f 100644 (file)
@@ -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))
index 3a28116..8417b08 100644 (file)
@@ -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)
 {
index 5ae3c7b..0b400ea 100644 (file)
@@ -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)]