Fix Array ctor integer widening and add Reflection tests

src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.CoreCLR.cs

@@ -212,17 +212,24 @@ private void InvokeClassConstructor()
         }
 
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
-        private object? InvokeWorker(object? obj, BindingFlags invokeAttr, in Span<object?> arguments)
+        private object? InvokeWorker(object? obj, BindingFlags invokeAttr, Span<object?> arguments)
         {
             bool wrapExceptions = (invokeAttr & BindingFlags.DoNotWrapExceptions) == 0;
-            return RuntimeMethodHandle.InvokeMethod(obj, arguments, Signature, false, wrapExceptions);
+            return RuntimeMethodHandle.InvokeMethod(obj, in arguments, Signature, false, wrapExceptions);
         }
 
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         private object InvokeCtorWorker(BindingFlags invokeAttr, in Span<object?> arguments)
         {
             bool wrapExceptions = (invokeAttr & BindingFlags.DoNotWrapExceptions) == 0;
-            return RuntimeMethodHandle.InvokeMethod(null, arguments, Signature, true, wrapExceptions)!;
+            return RuntimeMethodHandle.InvokeMethod(null, in arguments, Signature, true, wrapExceptions)!;
+        }
+
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        private object InvokeArrayCtorWorker(Span<object?> arguments)
+        {
+            Debug.Assert(m_declaringType.IsArray);
+            return RuntimeMethodHandle.InvokeArrayCtor(in arguments, Signature);
         }
 
         [RequiresUnreferencedCode("Trimming may change method bodies. For example it can change some instructions, remove branches or local variables.")]

src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.CoreCLR.cs

@@ -316,10 +316,10 @@ public override MethodImplAttributes GetMethodImplementationFlags()
 
         #region Invocation Logic(On MemberBase)
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
-        private object? InvokeWorker(object? obj, BindingFlags invokeAttr, in Span<object?> arguments)
+        private object? InvokeWorker(object? obj, BindingFlags invokeAttr, Span<object?> arguments)
         {
             bool wrapExceptions = (invokeAttr & BindingFlags.DoNotWrapExceptions) == 0;
-            return RuntimeMethodHandle.InvokeMethod(obj, arguments, Signature, false, wrapExceptions);
+            return RuntimeMethodHandle.InvokeMethod(obj, in arguments, Signature, false, wrapExceptions);
         }
 
         [DebuggerStepThroughAttribute]

src/coreclr/System.Private.CoreLib/src/System/RuntimeHandles.cs

@@ -982,6 +982,11 @@ internal static MdUtf8String GetUtf8Name(RuntimeMethodHandleInternal method)
         [MethodImpl(MethodImplOptions.InternalCall)]
         internal static extern object? InvokeMethod(object? target, in Span<object?> arguments, Signature sig, bool constructor, bool wrapExceptions);
 
+        [DebuggerStepThroughAttribute]
+        [Diagnostics.DebuggerHidden]
+        [MethodImpl(MethodImplOptions.InternalCall)]
+        internal static extern object InvokeArrayCtor(in Span<object?> arguments, Signature sig);
+
         [DllImport(RuntimeHelpers.QCall, EntryPoint = "RuntimeMethodHandle_GetMethodInstantiation")]
         private static extern void GetMethodInstantiation(RuntimeMethodHandleInternal method, ObjectHandleOnStack types, Interop.BOOL fAsRuntimeTypeArray);
 

src/coreclr/vm/ecalllist.h

@@ -233,6 +233,7 @@ FCFuncEnd()
 FCFuncStart(gRuntimeMethodHandle)
     FCFuncElement("_GetCurrentMethod", RuntimeMethodHandle::GetCurrentMethod)
     FCFuncElement("InvokeMethod", RuntimeMethodHandle::InvokeMethod)
+    FCFuncElement("InvokeArrayCtor", RuntimeMethodHandle::InvokeArrayCtor)
     FCFuncElement("GetImplAttributes", RuntimeMethodHandle::GetImplAttributes)
     FCFuncElement("GetAttributes", RuntimeMethodHandle::GetAttributes)
     FCFuncElement("GetDeclaringType", RuntimeMethodHandle::GetDeclaringType)

src/coreclr/vm/jitinterface.cpp

@@ -3627,9 +3627,9 @@ bool CEEInfo::isStructRequiringStackAllocRetBuf(CORINFO_CLASS_HANDLE clsHnd)
 
     JIT_TO_EE_TRANSITION_LEAF();
 
-    TypeHandle     VMClsHnd(clsHnd);
-    MethodTable * pMT = VMClsHnd.GetMethodTable();
-    ret = (pMT != NULL && pMT->IsStructRequiringStackAllocRetBuf());
+    // Disable this optimization. It has limited value (only kicks in on x86, and only for less common structs),
+    // causes bugs and introduces odd ABI differences not compatible with ReadyToRun.
+    ret = false;
 
     EE_TO_JIT_TRANSITION_LEAF();
 

src/coreclr/vm/methodtable.cpp

@@ -6002,20 +6002,6 @@ BOOL MethodTable::SanityCheck()
         return (pCanonMT == this) || IsArray();
 }
 
-//==========================================================================================
-
-// Structs containing GC pointers whose size is at most this are always stack-allocated.
-const unsigned MaxStructBytesForLocalVarRetBuffBytes = 2 * sizeof(void*);  // 4 pointer-widths.
-
-BOOL MethodTable::IsStructRequiringStackAllocRetBuf()
-{
-    LIMITED_METHOD_DAC_CONTRACT;
-
-    // Disable this optimization. It has limited value (only kicks in on x86, and only for less common structs),
-    // causes bugs and introduces odd ABI differences not compatible with ReadyToRun.
-    return FALSE;
-}
-
 //==========================================================================================
 unsigned MethodTable::GetTypeDefRid()
 {

src/coreclr/vm/methodtable.h

@@ -2511,14 +2511,6 @@ class MethodTable
     // Is this value type? Returns false for System.ValueType and System.Enum.
     inline BOOL IsValueType();
 
-    // Returns "TRUE" iff "this" is a struct type such that return buffers used for returning a value
-    // of this type must be stack-allocated.  This will generally be true only if the struct
-    // contains GC pointers, and does not exceed some size limit.  Maintaining this as an invariant allows
-    // an optimization: the JIT may assume that return buffer pointers for return types for which this predicate
-    // returns TRUE are always stack allocated, and thus, that stores to the GC-pointer fields of such return
-    // buffers do not require GC write barriers.
-    BOOL IsStructRequiringStackAllocRetBuf();
-
     // Is this enum? Returns false for System.Enum.
     inline BOOL IsEnum();
 

src/coreclr/vm/reflectioninvocation.cpp

@@ -451,7 +451,7 @@ struct ByRefToNullable  {
     }
 };
 
-void CallDescrWorkerReflectionWrapper(CallDescrData * pCallDescrData, Frame * pFrame)
+static void CallDescrWorkerReflectionWrapper(CallDescrData * pCallDescrData, Frame * pFrame)
 {
     // Use static contracts b/c we have SEH.
     STATIC_CONTRACT_THROWS;
@@ -478,44 +478,6 @@ void CallDescrWorkerReflectionWrapper(CallDescrData * pCallDescrData, Frame * pF
     PAL_ENDTRY
 } // CallDescrWorkerReflectionWrapper
 
-OBJECTREF InvokeArrayConstructor(TypeHandle th, MethodDesc* pMeth, Span<OBJECTREF>* objs, int argCnt)
-{
-    CONTRACTL {
-        THROWS;
-        GC_TRIGGERS;
-        MODE_COOPERATIVE;
-    }
-    CONTRACTL_END;
-
-    // Validate the argCnt an the Rank. Also allow nested SZARRAY's.
-    _ASSERTE(argCnt == (int) th.GetRank() || argCnt == (int) th.GetRank() * 2 ||
-             th.GetInternalCorElementType() == ELEMENT_TYPE_SZARRAY);
-
-    // Validate all of the parameters.  These all typed as integers
-    int allocSize = 0;
-    if (!ClrSafeInt<int>::multiply(sizeof(INT32), argCnt, allocSize))
-        COMPlusThrow(kArgumentException, IDS_EE_SIGTOOCOMPLEX);
-
-    INT32* indexes = (INT32*) _alloca((size_t)allocSize);
-    ZeroMemory(indexes, allocSize);
-
-    for (DWORD i=0; i<(DWORD)argCnt; i++)
-    {
-        if (!objs->GetAt(i))
-            COMPlusThrowArgumentException(W("parameters"), W("Arg_NullIndex"));
-
-        MethodTable* pMT = objs->GetAt(i)->GetMethodTable();
-        CorElementType oType = TypeHandle(pMT).GetVerifierCorElementType();
-
-        if (!InvokeUtil::IsPrimitiveType(oType) || !InvokeUtil::CanPrimitiveWiden(ELEMENT_TYPE_I4,oType))
-            COMPlusThrow(kArgumentException,W("Arg_PrimWiden"));
-
-        memcpy(&indexes[i], objs->GetAt(i)->UnBox(),pMT->GetNumInstanceFieldBytes());
-    }
-
-    return AllocateArrayEx(th, indexes, argCnt);
-}
-
 static BOOL IsActivationNeededForMethodInvoke(MethodDesc * pMD)
 {
     CONTRACTL {
@@ -785,17 +747,6 @@ FCIMPL5(Object*, RuntimeMethodHandle::InvokeMethod,
 
     if (fConstructor)
     {
-        // If we are invoking a constructor on an array then we must
-        // handle this specially.  String objects allocate themselves
-        // so they are a special case.
-        if (ownerType.IsArray()) {
-            gc.retVal = InvokeArrayConstructor(ownerType,
-                                               pMeth,
-                                               objs,
-                                               gc.pSig->NumFixedArgs());
-            goto Done;
-        }
-
         MethodTable * pMT = ownerType.AsMethodTable();
 
         {
@@ -936,8 +887,6 @@ FCIMPL5(Object*, RuntimeMethodHandle::InvokeMethod,
     // If an exception occurs a gc may happen but we are going to dump the stack anyway and we do
     // not need to protect anything.
 
-    PVOID pRetBufStackCopy = NULL;
-
     {
     BEGINFORBIDGC();
 #ifdef _DEBUG
@@ -947,24 +896,8 @@ FCIMPL5(Object*, RuntimeMethodHandle::InvokeMethod,
     // Take care of any return arguments
     if (fHasRetBuffArg)
     {
-        // We stack-allocate this ret buff, to preserve the invariant that ret-buffs are always in the
-        // caller's stack frame.  We'll copy into gc.retVal later.
-        TypeHandle retTH = gc.pSig->GetReturnTypeHandle();
-        MethodTable* pMT = retTH.GetMethodTable();
-        if (pMT->IsStructRequiringStackAllocRetBuf())
-        {
-            SIZE_T sz = pMT->GetNumInstanceFieldBytes();
-            pRetBufStackCopy = _alloca(sz);
-            memset(pRetBufStackCopy, 0, sz);
-
-            pValueClasses = new (_alloca(sizeof(ValueClassInfo))) ValueClassInfo(pRetBufStackCopy, pMT, pValueClasses);
-            *((LPVOID*) (pTransitionBlock + argit.GetRetBuffArgOffset())) = pRetBufStackCopy;
-        }
-        else
-        {
-            PVOID pRetBuff = gc.retVal->GetData();
-            *((LPVOID*) (pTransitionBlock + argit.GetRetBuffArgOffset())) = pRetBuff;
-        }
+        PVOID pRetBuff = gc.retVal->GetData();
+        *((LPVOID*) (pTransitionBlock + argit.GetRetBuffArgOffset())) = pRetBuff;
     }
 
     // copy args
@@ -1128,10 +1061,6 @@ FCIMPL5(Object*, RuntimeMethodHandle::InvokeMethod,
         {
             CopyValueClass(gc.retVal->GetData(), &callDescrData.returnValue, gc.retVal->GetMethodTable());
         }
-        else if (pRetBufStackCopy)
-        {
-            CopyValueClass(gc.retVal->GetData(), pRetBufStackCopy, gc.retVal->GetMethodTable());
-        }
         // From here on out, it is OK to have GCs since the return object (which may have had
         // GC pointers has been put into a GC object and thus protected.
 
@@ -1166,8 +1095,61 @@ FCIMPL5(Object*, RuntimeMethodHandle::InvokeMethod,
 
     }
 
-Done:
-    ;
+    HELPER_METHOD_FRAME_END();
+
+    return OBJECTREFToObject(gc.retVal);
+}
+FCIMPLEND
+
+
+FCIMPL2(Object*, RuntimeMethodHandle::InvokeArrayCtor,
+    Span<OBJECTREF>* objs,
+    SignatureNative* pSigUNSAFE)
+{
+    FCALL_CONTRACT;
+
+    struct
+    {
+        SIGNATURENATIVEREF pSig;
+        OBJECTREF retVal;
+    } gc;
+
+    gc.pSig = (SIGNATURENATIVEREF)pSigUNSAFE;
+    gc.retVal = NULL;
+
+    TypeHandle th = gc.pSig->GetDeclaringType();
+    int argCnt = gc.pSig->NumFixedArgs();
+
+    HELPER_METHOD_FRAME_BEGIN_RET_PROTECT(gc);
+
+    // Validate the argCnt an the Rank. Also allow nested SZARRAY's.
+    _ASSERTE(argCnt == (int) th.GetRank() || argCnt == (int) th.GetRank() * 2 ||
+             th.GetInternalCorElementType() == ELEMENT_TYPE_SZARRAY);
+
+    // Validate all of the parameters.  These all typed as integers
+    int allocSize = 0;
+    if (!ClrSafeInt<int>::multiply(sizeof(INT32), argCnt, allocSize))
+        COMPlusThrow(kArgumentException, IDS_EE_SIGTOOCOMPLEX);
+
+    INT32* indexes = (INT32*) _alloca((size_t)allocSize);
+    ZeroMemory(indexes, allocSize);
+
+    for (DWORD i = 0; i < (DWORD)argCnt; i++)
+    {
+        if (!objs->GetAt(i))
+            COMPlusThrowArgumentException(W("parameters"), W("Arg_NullIndex"));
+
+        MethodTable* pMT = objs->GetAt(i)->GetMethodTable();
+        CorElementType oType = TypeHandle(pMT).GetVerifierCorElementType();
+
+        if (!InvokeUtil::IsPrimitiveType(oType) || !InvokeUtil::CanPrimitiveWiden(ELEMENT_TYPE_I4,oType))
+            COMPlusThrow(kArgumentException,W("Arg_PrimWiden"));
+
+        memcpy(&indexes[i], objs->GetAt(i)->UnBox(), pMT->GetNumInstanceFieldBytes());
+    }
+
+    gc.retVal = AllocateArrayEx(th, indexes, argCnt);
+
     HELPER_METHOD_FRAME_END();
 
     return OBJECTREFToObject(gc.retVal);

src/coreclr/vm/runtimehandles.h

@@ -224,6 +224,8 @@ class RuntimeMethodHandle {
 
     static FCDECL5(Object*, InvokeMethod, Object *target, Span<OBJECTREF>* objs, SignatureNative* pSig, CLR_BOOL fConstructor, CLR_BOOL fWrapExceptions);
 
+    static FCDECL2(Object*, InvokeArrayCtor, Span<OBJECTREF>* objs, SignatureNative* pSig);
+
     struct StreamingContextData {
         Object * additionalContext;  // additionalContex was changed from OBJECTREF to Object to avoid having a
         INT32 contextStates;         // constructor in this struct. GCC doesn't allow structs with constructors to be

src/libraries/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.cs

@@ -158,6 +158,12 @@ public override object Invoke(BindingFlags invokeAttr, Binder? binder, object?[]
                 throw new TargetParameterCountException(SR.Arg_ParmCnt);
             }
 
+            // Array construction is a special case.
+            if (DeclaringType is not null && DeclaringType.IsArray)
+            {
+                return InvokeArrayCtorWorker(parameters!);
+            }
+
             StackAllocedArguments stackArgs = default;
             Span<object?> arguments = default;
             if (actualCount != 0)

src/mono/System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.Mono.cs

@@ -374,7 +374,7 @@ private RuntimeType[] ArgumentTypes
         internal extern object? InternalInvoke(object? obj, in Span<object?> parameters, out Exception? exc);
 
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
-        private object? InvokeWorker(object? obj, BindingFlags invokeAttr, in Span<object?> parameters)
+        private object? InvokeWorker(object? obj, BindingFlags invokeAttr, Span<object?> parameters)
         {
             Exception? exc;
             object? o = null;
@@ -862,7 +862,7 @@ private void InvokeClassConstructor()
         }
 
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
-        internal object? InvokeWorker(object? obj, BindingFlags invokeAttr, in Span<object?> arguments)
+        internal object? InvokeWorker(object? obj, BindingFlags invokeAttr, Span<object?> arguments)
         {
             bool wrapExceptions = (invokeAttr & BindingFlags.DoNotWrapExceptions) == 0;
             return InternalInvoke(obj, arguments, wrapExceptions);
@@ -875,13 +875,22 @@ internal object InvokeCtorWorker(BindingFlags invokeAttr, Span<object?> argument
             return InternalInvoke(null, arguments, wrapExceptions)!;
         }
 
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        internal object InvokeArrayCtorWorker(Span<object?> arguments)
+        {
+            return InternalInvokeArrayCtor(in arguments);
+        }
+
         /*
          * InternalInvoke() receives the parameters correctly converted by the binder
          * to match the types of the method signature.
          */
         [MethodImplAttribute(MethodImplOptions.InternalCall)]
         internal extern object InternalInvoke(object? obj, in Span<object?> parameters, out Exception exc);
 
+        [MethodImplAttribute(MethodImplOptions.InternalCall)]
+        private extern object InternalInvokeArrayCtor(in Span<object?> arguments);
+
         private object? InternalInvoke(object? obj, Span<object?> parameters, bool wrapExceptions)
         {
             Exception exc;

src/mono/mono/metadata/icall-def-netcore.h

@@ -253,6 +253,7 @@ HANDLES(RASSEM_12, "get_location", ves_icall_System_Reflection_RuntimeAssembly_g
 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_3, "InternalInvokeArrayCtor", ves_icall_InternalInvokeArrayCtor, MonoObject, 2, (MonoReflectionMethod, MonoSpanOfObjects_ref))
 HANDLES_REUSE_WRAPPER(MCMETH_4, "get_metadata_token", ves_icall_reflection_get_token)
 
 ICALL_TYPE(CATTR_DATA, "System.Reflection.RuntimeCustomAttributeData", CATTR_DATA_1)