From 5822ef770bd0ffc706292c174dbda822db8ef772 Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Mon, 8 Nov 2021 19:31:20 -0800 Subject: [PATCH 01/16] coreclr/ Remove always false IsStructRequiringStackAllocRetBuf(). Extract the conversion of array constructor arguments into managed. Expose new FCall for Array ctor call. --- .../RuntimeConstructorInfo.CoreCLR.cs | 7 ++ .../src/System/RuntimeHandles.cs | 5 + src/coreclr/vm/ecalllist.h | 1 + src/coreclr/vm/jitinterface.cpp | 6 +- src/coreclr/vm/methodtable.cpp | 14 --- src/coreclr/vm/methodtable.h | 8 -- src/coreclr/vm/object.h | 6 + src/coreclr/vm/reflectioninvocation.cpp | 112 ++++++------------ src/coreclr/vm/runtimehandles.h | 2 + 9 files changed, 60 insertions(+), 101 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.CoreCLR.cs index 8ac8a44be9448c..a58428f14ebb62 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.CoreCLR.cs @@ -225,6 +225,13 @@ private object InvokeCtorWorker(BindingFlags invokeAttr, in Span argume return RuntimeMethodHandle.InvokeMethod(null, arguments, Signature, true, wrapExceptions)!; } + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private object InvokeArrayCtorWorker(in Span 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.")] public override MethodBody? GetMethodBody() { diff --git a/src/coreclr/System.Private.CoreLib/src/System/RuntimeHandles.cs b/src/coreclr/System.Private.CoreLib/src/System/RuntimeHandles.cs index ef07d063012b56..d01bc6024c7634 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/RuntimeHandles.cs +++ b/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 arguments, Signature sig, bool constructor, bool wrapExceptions); + [DebuggerStepThroughAttribute] + [Diagnostics.DebuggerHidden] + [MethodImpl(MethodImplOptions.InternalCall)] + internal static extern object InvokeArrayCtor(in Span arguments, Signature sig); + [DllImport(RuntimeHelpers.QCall, EntryPoint = "RuntimeMethodHandle_GetMethodInstantiation")] private static extern void GetMethodInstantiation(RuntimeMethodHandleInternal method, ObjectHandleOnStack types, Interop.BOOL fAsRuntimeTypeArray); diff --git a/src/coreclr/vm/ecalllist.h b/src/coreclr/vm/ecalllist.h index e02bad38f9430f..4938ea05149c1d 100644 --- a/src/coreclr/vm/ecalllist.h +++ b/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) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 43c4fc17a597fe..1dc6e15da1e304 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/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(); diff --git a/src/coreclr/vm/methodtable.cpp b/src/coreclr/vm/methodtable.cpp index a685151f9838d0..1d63103cb36f1b 100644 --- a/src/coreclr/vm/methodtable.cpp +++ b/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() { diff --git a/src/coreclr/vm/methodtable.h b/src/coreclr/vm/methodtable.h index 1fa0d1be434523..6c3c89e7710afe 100644 --- a/src/coreclr/vm/methodtable.h +++ b/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(); diff --git a/src/coreclr/vm/object.h b/src/coreclr/vm/object.h index a47adb8c7985c8..e4b9550159f79a 100644 --- a/src/coreclr/vm/object.h +++ b/src/coreclr/vm/object.h @@ -770,6 +770,12 @@ class Span { return _length; } + + // Get at the underlying pointer value. + explicit operator KIND*() const + { + return _pointer; + } }; /* a TypedByRef is a structure that is used to implement VB's BYREF variants. diff --git a/src/coreclr/vm/reflectioninvocation.cpp b/src/coreclr/vm/reflectioninvocation.cpp index ebf13351d4c06f..899f0d1794100b 100644 --- a/src/coreclr/vm/reflectioninvocation.cpp +++ b/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* 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::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,39 @@ FCIMPL5(Object*, RuntimeMethodHandle::InvokeMethod, } -Done: - ; + HELPER_METHOD_FRAME_END(); + + return OBJECTREFToObject(gc.retVal); +} +FCIMPLEND + + +FCIMPL2(Object*, RuntimeMethodHandle::InvokeArrayCtor, + Span* args, + 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); + + gc.retVal = AllocateArrayEx(th, static_cast(*args), argCnt); + HELPER_METHOD_FRAME_END(); return OBJECTREFToObject(gc.retVal); diff --git a/src/coreclr/vm/runtimehandles.h b/src/coreclr/vm/runtimehandles.h index 184f27a7a311fe..4e58c924fe47bd 100644 --- a/src/coreclr/vm/runtimehandles.h +++ b/src/coreclr/vm/runtimehandles.h @@ -224,6 +224,8 @@ class RuntimeMethodHandle { static FCDECL5(Object*, InvokeMethod, Object *target, Span* objs, SignatureNative* pSig, CLR_BOOL fConstructor, CLR_BOOL fWrapExceptions); + static FCDECL2(Object*, InvokeArrayCtor, Span* args, 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 From b5bdbfad0aac44a1b92ac7f1151728ef01303ac4 Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Mon, 8 Nov 2021 19:32:00 -0800 Subject: [PATCH 02/16] libraries/ Add special case for array construction during Invoke. --- .../src/System/Reflection/RuntimeConstructorInfo.cs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) 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 e3502601407cff..76ce815ad269d5 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.cs @@ -158,6 +158,17 @@ 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) + { + Span args = stackalloc int[actualCount]; + for (int i = 0; i < actualCount; ++i) + { + args[i] = (int)parameters![i]!; + } + return InvokeArrayCtorWorker(in args); + } + StackAllocedArguments stackArgs = default; Span arguments = default; if (actualCount != 0) @@ -165,7 +176,7 @@ public override object Invoke(BindingFlags invokeAttr, Binder? binder, object?[] arguments = CheckArguments(ref stackArgs, parameters, binder, invokeAttr, culture, ArgumentTypes); } - object retValue = InvokeCtorWorker(invokeAttr, arguments); + object retValue = InvokeCtorWorker(invokeAttr, in arguments); // copy out. This should be made only if ByRef are present. // n.b. cannot use Span.CopyTo, as parameters.GetType() might not actually be typeof(object[]) From cb13ebc0aec5454cc2df69e801cf26783499b9ab Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Mon, 8 Nov 2021 19:33:13 -0800 Subject: [PATCH 03/16] mono/ Create new icall for Array ctor call. Create macro define for SPAN_T. --- .../Reflection/RuntimeMethodInfo.Mono.cs | 11 +- src/mono/mono/metadata/icall-def-netcore.h | 1 + src/mono/mono/metadata/icall-table.h | 2 + src/mono/mono/metadata/icall.c | 109 ++++++++++-------- src/mono/mono/metadata/object-internals.h | 12 +- 5 files changed, 79 insertions(+), 56 deletions(-) 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 1c6e10f1db2339..2806e80114d70c 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 @@ -869,12 +869,18 @@ private void InvokeClassConstructor() } [MethodImpl(MethodImplOptions.AggressiveInlining)] - internal object InvokeCtorWorker(BindingFlags invokeAttr, Span arguments) + internal object InvokeCtorWorker(BindingFlags invokeAttr, in Span arguments) { bool wrapExceptions = (invokeAttr & BindingFlags.DoNotWrapExceptions) == 0; return InternalInvoke(null, arguments, wrapExceptions)!; } + [MethodImpl(MethodImplOptions.AggressiveInlining)] + internal object InvokeArrayCtorWorker(in Span arguments) + { + return InternalInvokeArrayCtor(in arguments); + } + /* * InternalInvoke() receives the parameters correctly converted by the binder * to match the types of the method signature. @@ -882,6 +888,9 @@ internal object InvokeCtorWorker(BindingFlags invokeAttr, Span argument [MethodImplAttribute(MethodImplOptions.InternalCall)] internal extern object InternalInvoke(object? obj, in Span parameters, out Exception exc); + [MethodImplAttribute(MethodImplOptions.InternalCall)] + private extern object InternalInvokeArrayCtor(in Span arguments); + private object? InternalInvoke(object? obj, Span parameters, bool wrapExceptions) { Exception exc; diff --git a/src/mono/mono/metadata/icall-def-netcore.h b/src/mono/mono/metadata/icall-def-netcore.h index 1f0a400008e5ef..dca422dbd8a462 100644 --- a/src/mono/mono/metadata/icall-def-netcore.h +++ b/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, MonoSpanOfInts_ref)) HANDLES_REUSE_WRAPPER(MCMETH_4, "get_metadata_token", ves_icall_reflection_get_token) ICALL_TYPE(CATTR_DATA, "System.Reflection.RuntimeCustomAttributeData", CATTR_DATA_1) diff --git a/src/mono/mono/metadata/icall-table.h b/src/mono/mono/metadata/icall-table.h index f5d303d392304e..65a661859d6771 100644 --- a/src/mono/mono/metadata/icall-table.h +++ b/src/mono/mono/metadata/icall-table.h @@ -70,6 +70,7 @@ typedef unsigned *unsigned_ptr; typedef mono_unichar2 *mono_unichar2_ptr; typedef mono_unichar4 *mono_unichar4_ptr; typedef MonoSpanOfObjects *MonoSpanOfObjects_ref; +typedef MonoSpanOfInts *MonoSpanOfInts_ref; typedef char **char_ptr_ref; typedef gint32 *gint32_ref; @@ -175,6 +176,7 @@ typedef MonoStringHandle MonoStringOutHandle; #define MONO_HANDLE_TYPE_WRAP_guint8_ptr_ref ICALL_HANDLES_WRAP_VALUETYPE_REF #define MONO_HANDLE_TYPE_WRAP_MonoResolveTokenError_ref ICALL_HANDLES_WRAP_VALUETYPE_REF #define MONO_HANDLE_TYPE_WRAP_MonoSpanOfObjects_ref ICALL_HANDLES_WRAP_VALUETYPE_REF +#define MONO_HANDLE_TYPE_WRAP_MonoSpanOfInts_ref ICALL_HANDLES_WRAP_VALUETYPE_REF // HANDLE is not used just to avoid duplicate typedef warnings with some compilers. // gpointer == void* == HANDLE == FILE_HANDLE == PROCESS_HANDLE. diff --git a/src/mono/mono/metadata/icall.c b/src/mono/mono/metadata/icall.c index 43d826dd67c6f6..15ee1d93eebdf1 100644 --- a/src/mono/mono/metadata/icall.c +++ b/src/mono/mono/metadata/icall.c @@ -3373,11 +3373,7 @@ ves_icall_InternalInvoke (MonoReflectionMethodHandle method_handle, MonoObjectHa MonoMethodSignature* const sig = mono_method_signature_internal (m); int pcount = 0; void *obj = this_arg; - char *this_name = NULL; - char *target_name = NULL; - char *msg = NULL; MonoObject *result = NULL; - MonoArray *arr = NULL; MonoException *exception = NULL; *MONO_HANDLE_REF (exception_out) = NULL; @@ -3403,65 +3399,76 @@ ves_icall_InternalInvoke (MonoReflectionMethodHandle method_handle, MonoObjectHa } } - if (m_class_get_rank (m->klass) && !strcmp (m->name, ".ctor")) { - int i; - pcount = mono_span_length (params_span); - uintptr_t * const lengths = g_newa (uintptr_t, pcount); - /* Note: the synthetized array .ctors have int32 as argument type */ - for (i = 0; i < pcount; ++i) - lengths [i] = *(int32_t*) ((char*)mono_span_get (params_span, MonoObject*, i) + sizeof (MonoObject)); + result = mono_runtime_invoke_span_checked (m, obj, params_span, error); + goto exit; +return_null: + result = NULL; +exit: + if (exception) { + MONO_HANDLE_NEW (MonoException, exception); // FIXME? overkill? + mono_gc_wbarrier_generic_store_internal (MONO_HANDLE_REF (exception_out), (MonoObject*)exception); + } + return result ? MONO_HANDLE_NEW (MonoObject, result) : NULL_HANDLE; +} - 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. */ - arr = mono_array_new_full_checked (m->klass, lengths, NULL, error); - goto_if_nok (error, return_null); +MonoObjectHandle +ves_icall_InternalInvokeArrayCtor (MonoReflectionMethodHandle method_handle, MonoSpanOfInts *params_span, + MonoError *error) +{ + MonoReflectionMethod* const method = MONO_HANDLE_RAW (method_handle); + g_assert (params_span != NULL); - MonoArrayHandle subarray_handle = MONO_HANDLE_NEW (MonoArray, NULL); + int32_t i; + MonoArray *arr = NULL; + MonoMethod *m = method->method; + MonoMethodSignature* const sig = mono_method_signature_internal (m); + g_assert (m_class_get_rank (m->klass) && !strcmp (m->name, ".ctor")); - for (i = 0; i < mono_array_length_internal (arr); ++i) { - MonoArray *subarray = mono_array_new_full_checked (m_class_get_element_class (m->klass), &lengths [1], NULL, error); - goto_if_nok (error, return_null); - MONO_HANDLE_ASSIGN_RAW (subarray_handle, subarray); // FIXME? Overkill? - mono_array_setref_fast (arr, i, subarray); - } - goto exit; - } + int32_t pcount = mono_span_length (params_span); + uintptr_t * const lengths = g_newa (uintptr_t, pcount); + /* Note: the synthetized array .ctors have int32 as argument type */ + for (i = 0; i < pcount; ++i) + lengths [i] = mono_span_get (params_span, int32_t, i); - if (m_class_get_rank (m->klass) == pcount) { - /* Only lengths provided. */ - arr = mono_array_new_full_checked (m->klass, lengths, NULL, error); - goto_if_nok (error, return_null); - goto exit; - } 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); + 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. */ + arr = mono_array_new_full_checked (m->klass, lengths, NULL, error); + goto_if_nok (error, return_null); - for (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)); - } + MonoArrayHandle subarray_handle = MONO_HANDLE_NEW (MonoArray, NULL); - arr = mono_array_new_full_checked (m->klass, lengths, lower_bounds, error); + for (i = 0; i < mono_array_length_internal (arr); ++i) { + MonoArray *subarray = mono_array_new_full_checked (m_class_get_element_class (m->klass), &lengths [1], NULL, error); goto_if_nok (error, return_null); - goto exit; + MONO_HANDLE_ASSIGN_RAW (subarray_handle, subarray); // FIXME? Overkill? + mono_array_setref_fast (arr, i, subarray); } + goto exit; + } + + if (m_class_get_rank (m->klass) == pcount) { + /* Only lengths provided. */ + arr = mono_array_new_full_checked (m->klass, lengths, NULL, error); + goto_if_nok (error, return_null); + goto exit; + } 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); + + for (i = 0; i < pcount / 2; ++i) { + lower_bounds [i] = mono_span_get (params_span, int32_t, (i * 2)); + lengths [i] = mono_span_get (params_span, int32_t, (i * 2) + 1); + } + + arr = mono_array_new_full_checked (m->klass, lengths, lower_bounds, error); + goto_if_nok (error, return_null); + goto exit; } - result = mono_runtime_invoke_span_checked (m, obj, params_span, error); - goto exit; return_null: - result = NULL; arr = NULL; exit: - if (exception) { - MONO_HANDLE_NEW (MonoException, exception); // FIXME? overkill? - mono_gc_wbarrier_generic_store_internal (MONO_HANDLE_REF (exception_out), (MonoObject*)exception); - } - g_free (target_name); - g_free (this_name); - g_free (msg); - g_assert (!result || !arr); // only one, or neither, should be set - return result ? MONO_HANDLE_NEW (MonoObject, result) : arr ? MONO_HANDLE_NEW (MonoObject, (MonoObject*)arr) : NULL_HANDLE; + return arr ? MONO_HANDLE_NEW (MonoObject, (MonoObject*)arr) : NULL_HANDLE; } static guint64 diff --git a/src/mono/mono/metadata/object-internals.h b/src/mono/mono/metadata/object-internals.h index 361a6dbcc01dc2..91eb1be7a7f353 100644 --- a/src/mono/mono/metadata/object-internals.h +++ b/src/mono/mono/metadata/object-internals.h @@ -176,10 +176,14 @@ struct _MonoArray { }; /* match the layout of the managed definition of Span */ -typedef struct { - MonoObject** _pointer; - int32_t _length; -} MonoSpanOfObjects; +#define MONO_DEFINE_SPAN_OF_T(name, type) \ + typedef struct { \ + type* _pointer; \ + uint32_t _length; \ + } name; + +MONO_DEFINE_SPAN_OF_T (MonoSpanOfObjects, MonoObject*) +MONO_DEFINE_SPAN_OF_T (MonoSpanOfInts, int) #define MONO_SIZEOF_MONO_ARRAY (MONO_STRUCT_OFFSET_CONSTANT (MonoArray, vector)) From de614407a9e6a095e245991271098cf41aafd3e5 Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Mon, 8 Nov 2021 20:04:28 -0800 Subject: [PATCH 04/16] mono/ Build issues. --- src/mono/mono/metadata/icall.c | 2 -- src/mono/mono/metadata/object-internals.h | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/mono/mono/metadata/icall.c b/src/mono/mono/metadata/icall.c index 15ee1d93eebdf1..c3a5fb34c04413 100644 --- a/src/mono/mono/metadata/icall.c +++ b/src/mono/mono/metadata/icall.c @@ -3370,8 +3370,6 @@ ves_icall_InternalInvoke (MonoReflectionMethodHandle method_handle, MonoObjectHa * greater flexibility. */ MonoMethod *m = method->method; - MonoMethodSignature* const sig = mono_method_signature_internal (m); - int pcount = 0; void *obj = this_arg; MonoObject *result = NULL; MonoException *exception = NULL; diff --git a/src/mono/mono/metadata/object-internals.h b/src/mono/mono/metadata/object-internals.h index 91eb1be7a7f353..3b7dc189496f5c 100644 --- a/src/mono/mono/metadata/object-internals.h +++ b/src/mono/mono/metadata/object-internals.h @@ -288,7 +288,7 @@ mono_handle_array_get_bounds_dim (MonoArrayHandle arr, gint32 dim, MonoArrayBoun #define mono_span_length(span) (span->_length) -#define mono_span_get(span,type,idx) (type)(!span->_pointer ? NULL : span->_pointer[idx]) +#define mono_span_get(span,type,idx) (type)(!span->_pointer ? (type)0 : span->_pointer[idx]) #define mono_span_addr(span,type,idx) (type*)(span->_pointer + idx) From 2c05914b9d309287e801438b52eb68b5eba3ac67 Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Tue, 9 Nov 2021 15:02:47 -0800 Subject: [PATCH 05/16] coreclr/ --- .../RuntimeConstructorInfo.CoreCLR.cs | 8 +++--- .../Reflection/RuntimeMethodInfo.CoreCLR.cs | 4 +-- .../src/System/RuntimeHandles.cs | 2 +- src/coreclr/vm/object.h | 6 ----- src/coreclr/vm/reflectioninvocation.cpp | 26 +++++++++++++++++-- src/coreclr/vm/runtimehandles.h | 2 +- 6 files changed, 32 insertions(+), 16 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.CoreCLR.cs index a58428f14ebb62..92ea666187ae1d 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.CoreCLR.cs @@ -212,21 +212,21 @@ private void InvokeClassConstructor() } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private object? InvokeWorker(object? obj, BindingFlags invokeAttr, in Span arguments) + private object? InvokeWorker(object? obj, BindingFlags invokeAttr, Span 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 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(in Span arguments) + private object InvokeArrayCtorWorker(Span arguments) { Debug.Assert(m_declaringType.IsArray); return RuntimeMethodHandle.InvokeArrayCtor(in arguments, Signature); diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.CoreCLR.cs index 4710e6d3b7e6e2..68c09394e9275e 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.CoreCLR.cs +++ b/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 arguments) + private object? InvokeWorker(object? obj, BindingFlags invokeAttr, Span 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] diff --git a/src/coreclr/System.Private.CoreLib/src/System/RuntimeHandles.cs b/src/coreclr/System.Private.CoreLib/src/System/RuntimeHandles.cs index d01bc6024c7634..d5b50aacee2900 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/RuntimeHandles.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/RuntimeHandles.cs @@ -985,7 +985,7 @@ internal static MdUtf8String GetUtf8Name(RuntimeMethodHandleInternal method) [DebuggerStepThroughAttribute] [Diagnostics.DebuggerHidden] [MethodImpl(MethodImplOptions.InternalCall)] - internal static extern object InvokeArrayCtor(in Span arguments, Signature sig); + internal static extern object InvokeArrayCtor(in Span arguments, Signature sig); [DllImport(RuntimeHelpers.QCall, EntryPoint = "RuntimeMethodHandle_GetMethodInstantiation")] private static extern void GetMethodInstantiation(RuntimeMethodHandleInternal method, ObjectHandleOnStack types, Interop.BOOL fAsRuntimeTypeArray); diff --git a/src/coreclr/vm/object.h b/src/coreclr/vm/object.h index e4b9550159f79a..a47adb8c7985c8 100644 --- a/src/coreclr/vm/object.h +++ b/src/coreclr/vm/object.h @@ -770,12 +770,6 @@ class Span { return _length; } - - // Get at the underlying pointer value. - explicit operator KIND*() const - { - return _pointer; - } }; /* a TypedByRef is a structure that is used to implement VB's BYREF variants. diff --git a/src/coreclr/vm/reflectioninvocation.cpp b/src/coreclr/vm/reflectioninvocation.cpp index 899f0d1794100b..988f510c4dd8c8 100644 --- a/src/coreclr/vm/reflectioninvocation.cpp +++ b/src/coreclr/vm/reflectioninvocation.cpp @@ -1103,7 +1103,7 @@ FCIMPLEND FCIMPL2(Object*, RuntimeMethodHandle::InvokeArrayCtor, - Span* args, + Span* objs, SignatureNative* pSigUNSAFE) { FCALL_CONTRACT; @@ -1126,7 +1126,29 @@ FCIMPL2(Object*, RuntimeMethodHandle::InvokeArrayCtor, _ASSERTE(argCnt == (int) th.GetRank() || argCnt == (int) th.GetRank() * 2 || th.GetInternalCorElementType() == ELEMENT_TYPE_SZARRAY); - gc.retVal = AllocateArrayEx(th, static_cast(*args), argCnt); + // Validate all of the parameters. These all typed as integers + int allocSize = 0; + if (!ClrSafeInt::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(); diff --git a/src/coreclr/vm/runtimehandles.h b/src/coreclr/vm/runtimehandles.h index 4e58c924fe47bd..6264dc5a9ea3ff 100644 --- a/src/coreclr/vm/runtimehandles.h +++ b/src/coreclr/vm/runtimehandles.h @@ -224,7 +224,7 @@ class RuntimeMethodHandle { static FCDECL5(Object*, InvokeMethod, Object *target, Span* objs, SignatureNative* pSig, CLR_BOOL fConstructor, CLR_BOOL fWrapExceptions); - static FCDECL2(Object*, InvokeArrayCtor, Span* args, SignatureNative* pSig); + static FCDECL2(Object*, InvokeArrayCtor, Span* objs, SignatureNative* pSig); struct StreamingContextData { Object * additionalContext; // additionalContex was changed from OBJECTREF to Object to avoid having a From 907f39750bb1d95c677d4b6db477f18f19dfa1f4 Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Tue, 9 Nov 2021 15:03:31 -0800 Subject: [PATCH 06/16] mono/ --- .../src/System/Reflection/RuntimeMethodInfo.Mono.cs | 10 +++++----- src/mono/mono/metadata/icall-def-netcore.h | 2 +- src/mono/mono/metadata/icall-table.h | 2 -- src/mono/mono/metadata/icall.c | 12 ++++++------ src/mono/mono/metadata/object-internals.h | 1 - 5 files changed, 12 insertions(+), 15 deletions(-) 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 2806e80114d70c..fc6d8d10024bbf 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 @@ -374,7 +374,7 @@ private RuntimeType[] ArgumentTypes internal extern object? InternalInvoke(object? obj, in Span parameters, out Exception? exc); [MethodImpl(MethodImplOptions.AggressiveInlining)] - private object? InvokeWorker(object? obj, BindingFlags invokeAttr, in Span parameters) + private object? InvokeWorker(object? obj, BindingFlags invokeAttr, Span parameters) { Exception? exc; object? o = null; @@ -862,21 +862,21 @@ private void InvokeClassConstructor() } [MethodImpl(MethodImplOptions.AggressiveInlining)] - internal object? InvokeWorker(object? obj, BindingFlags invokeAttr, in Span arguments) + internal object? InvokeWorker(object? obj, BindingFlags invokeAttr, Span arguments) { bool wrapExceptions = (invokeAttr & BindingFlags.DoNotWrapExceptions) == 0; return InternalInvoke(obj, arguments, wrapExceptions); } [MethodImpl(MethodImplOptions.AggressiveInlining)] - internal object InvokeCtorWorker(BindingFlags invokeAttr, in Span arguments) + internal object InvokeCtorWorker(BindingFlags invokeAttr, Span arguments) { bool wrapExceptions = (invokeAttr & BindingFlags.DoNotWrapExceptions) == 0; return InternalInvoke(null, arguments, wrapExceptions)!; } [MethodImpl(MethodImplOptions.AggressiveInlining)] - internal object InvokeArrayCtorWorker(in Span arguments) + internal object InvokeArrayCtorWorker(Span arguments) { return InternalInvokeArrayCtor(in arguments); } @@ -889,7 +889,7 @@ internal object InvokeArrayCtorWorker(in Span arguments) internal extern object InternalInvoke(object? obj, in Span parameters, out Exception exc); [MethodImplAttribute(MethodImplOptions.InternalCall)] - private extern object InternalInvokeArrayCtor(in Span arguments); + private extern object InternalInvokeArrayCtor(in Span arguments); private object? InternalInvoke(object? obj, Span parameters, bool wrapExceptions) { diff --git a/src/mono/mono/metadata/icall-def-netcore.h b/src/mono/mono/metadata/icall-def-netcore.h index dca422dbd8a462..037f6467931d6d 100644 --- a/src/mono/mono/metadata/icall-def-netcore.h +++ b/src/mono/mono/metadata/icall-def-netcore.h @@ -253,7 +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, MonoSpanOfInts_ref)) +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) diff --git a/src/mono/mono/metadata/icall-table.h b/src/mono/mono/metadata/icall-table.h index 65a661859d6771..f5d303d392304e 100644 --- a/src/mono/mono/metadata/icall-table.h +++ b/src/mono/mono/metadata/icall-table.h @@ -70,7 +70,6 @@ typedef unsigned *unsigned_ptr; typedef mono_unichar2 *mono_unichar2_ptr; typedef mono_unichar4 *mono_unichar4_ptr; typedef MonoSpanOfObjects *MonoSpanOfObjects_ref; -typedef MonoSpanOfInts *MonoSpanOfInts_ref; typedef char **char_ptr_ref; typedef gint32 *gint32_ref; @@ -176,7 +175,6 @@ typedef MonoStringHandle MonoStringOutHandle; #define MONO_HANDLE_TYPE_WRAP_guint8_ptr_ref ICALL_HANDLES_WRAP_VALUETYPE_REF #define MONO_HANDLE_TYPE_WRAP_MonoResolveTokenError_ref ICALL_HANDLES_WRAP_VALUETYPE_REF #define MONO_HANDLE_TYPE_WRAP_MonoSpanOfObjects_ref ICALL_HANDLES_WRAP_VALUETYPE_REF -#define MONO_HANDLE_TYPE_WRAP_MonoSpanOfInts_ref ICALL_HANDLES_WRAP_VALUETYPE_REF // HANDLE is not used just to avoid duplicate typedef warnings with some compilers. // gpointer == void* == HANDLE == FILE_HANDLE == PROCESS_HANDLE. diff --git a/src/mono/mono/metadata/icall.c b/src/mono/mono/metadata/icall.c index c3a5fb34c04413..3e894fd0ab8bf5 100644 --- a/src/mono/mono/metadata/icall.c +++ b/src/mono/mono/metadata/icall.c @@ -3410,23 +3410,23 @@ ves_icall_InternalInvoke (MonoReflectionMethodHandle method_handle, MonoObjectHa } MonoObjectHandle -ves_icall_InternalInvokeArrayCtor (MonoReflectionMethodHandle method_handle, MonoSpanOfInts *params_span, +ves_icall_InternalInvokeArrayCtor (MonoReflectionMethodHandle method_handle, MonoSpanOfObjects *params_span, MonoError *error) { MonoReflectionMethod* const method = MONO_HANDLE_RAW (method_handle); g_assert (params_span != NULL); - int32_t i; + uint32_t i; MonoArray *arr = NULL; MonoMethod *m = method->method; MonoMethodSignature* const sig = mono_method_signature_internal (m); g_assert (m_class_get_rank (m->klass) && !strcmp (m->name, ".ctor")); - int32_t pcount = mono_span_length (params_span); + uint32_t pcount = mono_span_length (params_span); uintptr_t * const lengths = g_newa (uintptr_t, pcount); /* Note: the synthetized array .ctors have int32 as argument type */ for (i = 0; i < pcount; ++i) - lengths [i] = mono_span_get (params_span, int32_t, i); + lengths [i] = *(int32_t*) ((char*)mono_span_get (params_span, MonoObject*, i) + sizeof (MonoObject)); 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. */ @@ -3455,8 +3455,8 @@ ves_icall_InternalInvokeArrayCtor (MonoReflectionMethodHandle method_handle, Mon intptr_t * const lower_bounds = (intptr_t *)g_alloca (sizeof (intptr_t) * pcount); for (i = 0; i < pcount / 2; ++i) { - lower_bounds [i] = mono_span_get (params_span, int32_t, (i * 2)); - lengths [i] = mono_span_get (params_span, int32_t, (i * 2) + 1); + 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)); } arr = mono_array_new_full_checked (m->klass, lengths, lower_bounds, error); diff --git a/src/mono/mono/metadata/object-internals.h b/src/mono/mono/metadata/object-internals.h index 3b7dc189496f5c..4a47bd81ddfb66 100644 --- a/src/mono/mono/metadata/object-internals.h +++ b/src/mono/mono/metadata/object-internals.h @@ -183,7 +183,6 @@ struct _MonoArray { } name; MONO_DEFINE_SPAN_OF_T (MonoSpanOfObjects, MonoObject*) -MONO_DEFINE_SPAN_OF_T (MonoSpanOfInts, int) #define MONO_SIZEOF_MONO_ARRAY (MONO_STRUCT_OFFSET_CONSTANT (MonoArray, vector)) From 8299b332d3f88fe8b8bf9e78d5dd9048d5d9bb49 Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Tue, 9 Nov 2021 15:03:49 -0800 Subject: [PATCH 07/16] libraries/ --- .../src/System/Reflection/RuntimeConstructorInfo.cs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) 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 76ce815ad269d5..8d4a1ab7154d17 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.cs @@ -161,12 +161,7 @@ public override object Invoke(BindingFlags invokeAttr, Binder? binder, object?[] // Array construction is a special case. if (DeclaringType is not null && DeclaringType.IsArray) { - Span args = stackalloc int[actualCount]; - for (int i = 0; i < actualCount; ++i) - { - args[i] = (int)parameters![i]!; - } - return InvokeArrayCtorWorker(in args); + return InvokeArrayCtorWorker(parameters!); } StackAllocedArguments stackArgs = default; @@ -176,7 +171,7 @@ public override object Invoke(BindingFlags invokeAttr, Binder? binder, object?[] arguments = CheckArguments(ref stackArgs, parameters, binder, invokeAttr, culture, ArgumentTypes); } - object retValue = InvokeCtorWorker(invokeAttr, in arguments); + object retValue = InvokeCtorWorker(invokeAttr, arguments); // copy out. This should be made only if ByRef are present. // n.b. cannot use Span.CopyTo, as parameters.GetType() might not actually be typeof(object[]) From 2a41c9dfb29a0c24c43c13f83268288d1222e665 Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Tue, 9 Nov 2021 20:21:05 -0800 Subject: [PATCH 08/16] libraries/ Add tests for Reflection Binder type conversion support during Invoke. --- .../Reflection/RuntimeConstructorInfo.cs | 12 +++++----- .../System.Reflection/tests/Common.cs | 23 +++++++++++++++++++ .../tests/ConstructorInfoTests.cs | 19 +++++++++++++-- .../tests/MethodInfoTests.cs | 7 ++++++ 4 files changed, 53 insertions(+), 8 deletions(-) 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 8d4a1ab7154d17..cc208e9ae5f9a5 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.cs @@ -158,12 +158,6 @@ 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 arguments = default; if (actualCount != 0) @@ -171,6 +165,12 @@ public override object Invoke(BindingFlags invokeAttr, Binder? binder, object?[] arguments = CheckArguments(ref stackArgs, parameters, binder, invokeAttr, culture, ArgumentTypes); } + // Array construction is a special case. + if (DeclaringType is not null && DeclaringType.IsArray) + { + return InvokeArrayCtorWorker(arguments); + } + object retValue = InvokeCtorWorker(invokeAttr, arguments); // copy out. This should be made only if ByRef are present. diff --git a/src/libraries/System.Reflection/tests/Common.cs b/src/libraries/System.Reflection/tests/Common.cs index 2c9e65368ec21f..e70336e7e330c7 100644 --- a/src/libraries/System.Reflection/tests/Common.cs +++ b/src/libraries/System.Reflection/tests/Common.cs @@ -1,6 +1,8 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Globalization; + namespace System.Reflection.Tests { public enum PublicEnum @@ -105,4 +107,25 @@ public class Helpers { public static Assembly ExecutingAssembly => typeof(Helpers).GetTypeInfo().Assembly; } + + public class ConvertStringToIntBinder : Binder + { + public override FieldInfo BindToField(BindingFlags bindingAttr, FieldInfo[] match, object value, CultureInfo? culture) + => throw new NotImplementedException(); + + public override MethodBase BindToMethod(BindingFlags bindingAttr, MethodBase[] match, ref object?[] args, ParameterModifier[]? modifiers, CultureInfo? culture, string[]? names, out object? state) + => throw new NotImplementedException(); + + public override object ChangeType(object value, Type type, CultureInfo? culture) + => int.Parse((string)value); + + public override void ReorderArgumentArray(ref object?[] args, object state) + => throw new NotImplementedException(); + + public override MethodBase? SelectMethod(BindingFlags bindingAttr, MethodBase[] match, Type[] types, ParameterModifier[]? modifiers) + => throw new NotImplementedException(); + + public override PropertyInfo? SelectProperty(BindingFlags bindingAttr, PropertyInfo[] match, Type? returnType, Type[]? indexes, ParameterModifier[]? modifiers) + => throw new NotImplementedException(); + } } diff --git a/src/libraries/System.Reflection/tests/ConstructorInfoTests.cs b/src/libraries/System.Reflection/tests/ConstructorInfoTests.cs index a1042452810748..8e2b35eb4b98fa 100644 --- a/src/libraries/System.Reflection/tests/ConstructorInfoTests.cs +++ b/src/libraries/System.Reflection/tests/ConstructorInfoTests.cs @@ -126,6 +126,14 @@ public void Invoke_OneDimensionalArray_NegativeLengths_ThrowsOverflowException() } } + [Fact] + public void Invoke_TwoDimensionalArray_CustomBinder_IncorrectTypeArguments() + { + var ctor = typeof(int[,]).GetConstructor(new[] { typeof(int), typeof(int) }); + var arr = (int[,])ctor.Invoke(BindingFlags.Default, new ConvertStringToIntBinder(), new object[] { "1", "2" }, null); + Assert.Equal(2, arr.Length); + } + [Fact] public void Invoke_OneParameter() { @@ -143,6 +151,15 @@ public void Invoke_TwoParameters() Assert.Equal("hello", obj.stringValue); } + [Fact] + public void Invoke_TwoParameters_CustomBinder_IncorrectTypeArgument() + { + ConstructorInfo[] constructors = GetConstructors(typeof(ClassWith3Constructors)); + ClassWith3Constructors obj = (ClassWith3Constructors)constructors[2].Invoke(BindingFlags.Default, new ConvertStringToIntBinder(), new object[] { "101", "hello" }, null); + Assert.Equal(101, obj.intValue); + Assert.Equal("hello", obj.stringValue); + } + [Fact] public void Invoke_NoParameters_ThowsTargetParameterCountException() { @@ -248,8 +265,6 @@ public ClassWith3Constructors(int intValue, string stringValue) this.intValue = intValue; this.stringValue = stringValue; } - - public string Method1(DateTime dt) => ""; } public static class ClassWithStaticConstructor diff --git a/src/libraries/System.Reflection/tests/MethodInfoTests.cs b/src/libraries/System.Reflection/tests/MethodInfoTests.cs index b8abc6e9118eb6..749758b1eabd43 100644 --- a/src/libraries/System.Reflection/tests/MethodInfoTests.cs +++ b/src/libraries/System.Reflection/tests/MethodInfoTests.cs @@ -381,6 +381,13 @@ public static void Invoke_OptionalParameterUnassingableFromMissing_WithMissingVa AssertExtensions.Throws(null, () => GetMethod(typeof(MethodInfoDefaultParameters), "OptionalStringParameter").Invoke(new MethodInfoDefaultParameters(), new object[] { Type.Missing })); } + [Fact] + public void Invoke_TwoParameters_CustomBinder_IncorrectTypeArguments() + { + MethodInfo method = GetMethod(typeof(MI_SubClass), nameof(MI_SubClass.StaticIntIntMethodReturningInt)); + Assert.Equal(110, method.Invoke(null, BindingFlags.Default, new ConvertStringToIntBinder(), new object[] { "10", "100" }, null)); + } + [Theory] [InlineData(typeof(MI_SubClass), nameof(MI_SubClass.GenericMethod1), new Type[] { typeof(int) })] [InlineData(typeof(MI_SubClass), nameof(MI_SubClass.GenericMethod2), new Type[] { typeof(string), typeof(int) })] From f9ec4f428873456e361f18415685fd5faa76c09a Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Tue, 9 Nov 2021 22:11:11 -0800 Subject: [PATCH 09/16] coreclr/ Make sure integral types respect sign extension during widen operation. Fix InvokeUtil::CreatePrimitiveValue to write destination size. --- src/coreclr/vm/invokeutil.cpp | 14 +++++++++++++- src/coreclr/vm/reflectioninvocation.cpp | 9 ++++++++- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/src/coreclr/vm/invokeutil.cpp b/src/coreclr/vm/invokeutil.cpp index b9779762d40cd7..c207d765097f49 100644 --- a/src/coreclr/vm/invokeutil.cpp +++ b/src/coreclr/vm/invokeutil.cpp @@ -405,19 +405,31 @@ void InvokeUtil::CreatePrimitiveValue(CorElementType dstType,CorElementType srcT } // Copy the data and return + int dstSize = 0; switch (dstType) { case ELEMENT_TYPE_BOOLEAN: case ELEMENT_TYPE_I1: case ELEMENT_TYPE_U1: + dstSize = 1; + goto SIZE_SET_LABEL; case ELEMENT_TYPE_CHAR: case ELEMENT_TYPE_I2: case ELEMENT_TYPE_U2: + dstSize = 2; + goto SIZE_SET_LABEL; case ELEMENT_TYPE_I4: case ELEMENT_TYPE_U4: + dstSize = 4; + goto SIZE_SET_LABEL; case ELEMENT_TYPE_I: case ELEMENT_TYPE_U: + dstSize = sizeof(void*); + goto SIZE_SET_LABEL; case ELEMENT_TYPE_I8: case ELEMENT_TYPE_U8: + dstSize = 8; + goto SIZE_SET_LABEL; + SIZE_SET_LABEL: switch (srcType) { case ELEMENT_TYPE_BOOLEAN: case ELEMENT_TYPE_I1: @@ -431,7 +443,7 @@ void InvokeUtil::CreatePrimitiveValue(CorElementType dstType,CorElementType srcT case ELEMENT_TYPE_U: case ELEMENT_TYPE_I8: case ELEMENT_TYPE_U8: - *pDst = data; + memcpy(pDst, &data, dstSize); break; case ELEMENT_TYPE_R4: *pDst = (I8)(*(R4*)pSrc); diff --git a/src/coreclr/vm/reflectioninvocation.cpp b/src/coreclr/vm/reflectioninvocation.cpp index 988f510c4dd8c8..d6d847327a2a93 100644 --- a/src/coreclr/vm/reflectioninvocation.cpp +++ b/src/coreclr/vm/reflectioninvocation.cpp @@ -1122,6 +1122,9 @@ FCIMPL2(Object*, RuntimeMethodHandle::InvokeArrayCtor, HELPER_METHOD_FRAME_BEGIN_RET_PROTECT(gc); + if (th.IsSharedByGenericInstantiations()) + COMPlusThrow(kNotSupportedException, W("NotSupported_Type")); + // 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); @@ -1145,7 +1148,11 @@ FCIMPL2(Object*, RuntimeMethodHandle::InvokeArrayCtor, if (!InvokeUtil::IsPrimitiveType(oType) || !InvokeUtil::CanPrimitiveWiden(ELEMENT_TYPE_I4,oType)) COMPlusThrow(kArgumentException,W("Arg_PrimWiden")); - memcpy(&indexes[i], objs->GetAt(i)->UnBox(), pMT->GetNumInstanceFieldBytes()); + InvokeUtil::CreatePrimitiveValue( + ELEMENT_TYPE_I4, + oType, + objs->GetAt(i), + (ARG_SLOT*)(indexes + i)); } gc.retVal = AllocateArrayEx(th, indexes, argCnt); From f8512a54bc5eb06ec0444dce0d556e850a61b2d4 Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Tue, 9 Nov 2021 22:11:53 -0800 Subject: [PATCH 10/16] libraries/ Validate arg count prior to running static constructor. --- .../System/Reflection/RuntimeConstructorInfo.cs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) 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 cc208e9ae5f9a5..1bea0af7c9477f 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.cs @@ -105,6 +105,13 @@ internal void ThrowNoInvokeException() ValidateInvokeTarget(obj); + // Correct number of arguments supplied + int actualCount = (parameters is null) ? 0 : parameters.Length; + if (ArgumentTypes.Length != actualCount) + { + throw new TargetParameterCountException(SR.Arg_ParmCnt); + } + if ((InvocationFlags & InvocationFlags.RunClassConstructor) != 0) { // Run the class constructor through the class constructor mechanism instead of the Invoke path. @@ -113,13 +120,6 @@ internal void ThrowNoInvokeException() return null; } - // Correct number of arguments supplied - int actualCount = (parameters is null) ? 0 : parameters.Length; - if (ArgumentTypes.Length != actualCount) - { - throw new TargetParameterCountException(SR.Arg_ParmCnt); - } - StackAllocedArguments stackArgs = default; Span arguments = default; if (actualCount != 0) From 27ae925863f7c21354141f07ac0b4ea2d1002aa8 Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Tue, 9 Nov 2021 22:35:21 -0800 Subject: [PATCH 11/16] coreclr/ Properly widen the input by respecting endianness. Revert changes to InvokeUtil::CreatePrimitiveValue. --- src/coreclr/vm/invokeutil.cpp | 14 +------------- src/coreclr/vm/reflectioninvocation.cpp | 8 +++----- 2 files changed, 4 insertions(+), 18 deletions(-) diff --git a/src/coreclr/vm/invokeutil.cpp b/src/coreclr/vm/invokeutil.cpp index c207d765097f49..b9779762d40cd7 100644 --- a/src/coreclr/vm/invokeutil.cpp +++ b/src/coreclr/vm/invokeutil.cpp @@ -405,31 +405,19 @@ void InvokeUtil::CreatePrimitiveValue(CorElementType dstType,CorElementType srcT } // Copy the data and return - int dstSize = 0; switch (dstType) { case ELEMENT_TYPE_BOOLEAN: case ELEMENT_TYPE_I1: case ELEMENT_TYPE_U1: - dstSize = 1; - goto SIZE_SET_LABEL; case ELEMENT_TYPE_CHAR: case ELEMENT_TYPE_I2: case ELEMENT_TYPE_U2: - dstSize = 2; - goto SIZE_SET_LABEL; case ELEMENT_TYPE_I4: case ELEMENT_TYPE_U4: - dstSize = 4; - goto SIZE_SET_LABEL; case ELEMENT_TYPE_I: case ELEMENT_TYPE_U: - dstSize = sizeof(void*); - goto SIZE_SET_LABEL; case ELEMENT_TYPE_I8: case ELEMENT_TYPE_U8: - dstSize = 8; - goto SIZE_SET_LABEL; - SIZE_SET_LABEL: switch (srcType) { case ELEMENT_TYPE_BOOLEAN: case ELEMENT_TYPE_I1: @@ -443,7 +431,7 @@ void InvokeUtil::CreatePrimitiveValue(CorElementType dstType,CorElementType srcT case ELEMENT_TYPE_U: case ELEMENT_TYPE_I8: case ELEMENT_TYPE_U8: - memcpy(pDst, &data, dstSize); + *pDst = data; break; case ELEMENT_TYPE_R4: *pDst = (I8)(*(R4*)pSrc); diff --git a/src/coreclr/vm/reflectioninvocation.cpp b/src/coreclr/vm/reflectioninvocation.cpp index d6d847327a2a93..4cc7b8ffc03889 100644 --- a/src/coreclr/vm/reflectioninvocation.cpp +++ b/src/coreclr/vm/reflectioninvocation.cpp @@ -1148,11 +1148,9 @@ FCIMPL2(Object*, RuntimeMethodHandle::InvokeArrayCtor, if (!InvokeUtil::IsPrimitiveType(oType) || !InvokeUtil::CanPrimitiveWiden(ELEMENT_TYPE_I4,oType)) COMPlusThrow(kArgumentException,W("Arg_PrimWiden")); - InvokeUtil::CreatePrimitiveValue( - ELEMENT_TYPE_I4, - oType, - objs->GetAt(i), - (ARG_SLOT*)(indexes + i)); + ARG_SLOT value; + InvokeUtil::CreatePrimitiveValue(ELEMENT_TYPE_I4, oType, objs->GetAt(i), &value); + memcpyNoGCRefs(indexes + i, ArgSlotEndianessFixup(&value, sizeof(INT32)), sizeof(INT32)); } gc.retVal = AllocateArrayEx(th, indexes, argCnt); From 220a9b133472b4ce0946686d1543eb0345f560e2 Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Tue, 9 Nov 2021 22:35:41 -0800 Subject: [PATCH 12/16] libraries/ --- .../src/System/Reflection/RuntimeConstructorInfo.cs | 10 +++++++--- .../System.Reflection/tests/ConstructorInfoTests.cs | 11 +++++++++-- .../System.Reflection/tests/MethodInfoTests.cs | 5 ++++- 3 files changed, 20 insertions(+), 6 deletions(-) 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 1bea0af7c9477f..ae0576d74ea077 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.cs @@ -165,13 +165,17 @@ public override object Invoke(BindingFlags invokeAttr, Binder? binder, object?[] arguments = CheckArguments(ref stackArgs, parameters, binder, invokeAttr, culture, ArgumentTypes); } + object retValue; + // Array construction is a special case. if (DeclaringType is not null && DeclaringType.IsArray) { - return InvokeArrayCtorWorker(arguments); + retValue = InvokeArrayCtorWorker(arguments); + } + else + { + retValue = InvokeCtorWorker(invokeAttr, arguments); } - - object retValue = InvokeCtorWorker(invokeAttr, arguments); // copy out. This should be made only if ByRef are present. // n.b. cannot use Span.CopyTo, as parameters.GetType() might not actually be typeof(object[]) diff --git a/src/libraries/System.Reflection/tests/ConstructorInfoTests.cs b/src/libraries/System.Reflection/tests/ConstructorInfoTests.cs index 8e2b35eb4b98fa..ea861aa1dcdeb8 100644 --- a/src/libraries/System.Reflection/tests/ConstructorInfoTests.cs +++ b/src/libraries/System.Reflection/tests/ConstructorInfoTests.cs @@ -130,8 +130,11 @@ public void Invoke_OneDimensionalArray_NegativeLengths_ThrowsOverflowException() public void Invoke_TwoDimensionalArray_CustomBinder_IncorrectTypeArguments() { var ctor = typeof(int[,]).GetConstructor(new[] { typeof(int), typeof(int) }); - var arr = (int[,])ctor.Invoke(BindingFlags.Default, new ConvertStringToIntBinder(), new object[] { "1", "2" }, null); + var args = new object[] { "1", "2" }; + var arr = (int[,])ctor.Invoke(BindingFlags.Default, new ConvertStringToIntBinder(), args, null); Assert.Equal(2, arr.Length); + Assert.True(args[0] is int); + Assert.True(args[1] is int); } [Fact] @@ -155,9 +158,13 @@ public void Invoke_TwoParameters() public void Invoke_TwoParameters_CustomBinder_IncorrectTypeArgument() { ConstructorInfo[] constructors = GetConstructors(typeof(ClassWith3Constructors)); - ClassWith3Constructors obj = (ClassWith3Constructors)constructors[2].Invoke(BindingFlags.Default, new ConvertStringToIntBinder(), new object[] { "101", "hello" }, null); + + var args = new object[] { "101", "hello" }; + ClassWith3Constructors obj = (ClassWith3Constructors)constructors[2].Invoke(BindingFlags.Default, new ConvertStringToIntBinder(), args, null); Assert.Equal(101, obj.intValue); Assert.Equal("hello", obj.stringValue); + Assert.True(args[0] is int); + Assert.True(args[1] is string); } [Fact] diff --git a/src/libraries/System.Reflection/tests/MethodInfoTests.cs b/src/libraries/System.Reflection/tests/MethodInfoTests.cs index 749758b1eabd43..a538fc9f3a9c9f 100644 --- a/src/libraries/System.Reflection/tests/MethodInfoTests.cs +++ b/src/libraries/System.Reflection/tests/MethodInfoTests.cs @@ -385,7 +385,10 @@ public static void Invoke_OptionalParameterUnassingableFromMissing_WithMissingVa public void Invoke_TwoParameters_CustomBinder_IncorrectTypeArguments() { MethodInfo method = GetMethod(typeof(MI_SubClass), nameof(MI_SubClass.StaticIntIntMethodReturningInt)); - Assert.Equal(110, method.Invoke(null, BindingFlags.Default, new ConvertStringToIntBinder(), new object[] { "10", "100" }, null)); + var args = new object[] { "10", "100" }; + Assert.Equal(110, method.Invoke(null, BindingFlags.Default, new ConvertStringToIntBinder(), args, null)); + Assert.True(args[0] is int); + Assert.True(args[1] is int); } [Theory] From e988f811fbefee31e7ee37603c2a19b45a043ea5 Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Wed, 10 Nov 2021 13:10:13 -0800 Subject: [PATCH 13/16] coreclr/ Revert some non-impactful changes --- .../RuntimeConstructorInfo.CoreCLR.cs | 9 +- .../src/System/RuntimeHandles.cs | 5 - src/coreclr/vm/ecalllist.h | 1 - src/coreclr/vm/reflectioninvocation.cpp | 124 ++++++++---------- src/coreclr/vm/runtimehandles.h | 2 - 5 files changed, 57 insertions(+), 84 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.CoreCLR.cs index 92ea666187ae1d..7f34d5438c6bea 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.CoreCLR.cs @@ -219,19 +219,12 @@ private void InvokeClassConstructor() } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private object InvokeCtorWorker(BindingFlags invokeAttr, in Span arguments) + private object InvokeCtorWorker(BindingFlags invokeAttr, Span arguments) { bool wrapExceptions = (invokeAttr & BindingFlags.DoNotWrapExceptions) == 0; return RuntimeMethodHandle.InvokeMethod(null, in arguments, Signature, true, wrapExceptions)!; } - [MethodImpl(MethodImplOptions.AggressiveInlining)] - private object InvokeArrayCtorWorker(Span 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.")] public override MethodBody? GetMethodBody() { diff --git a/src/coreclr/System.Private.CoreLib/src/System/RuntimeHandles.cs b/src/coreclr/System.Private.CoreLib/src/System/RuntimeHandles.cs index d5b50aacee2900..ef07d063012b56 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/RuntimeHandles.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/RuntimeHandles.cs @@ -982,11 +982,6 @@ internal static MdUtf8String GetUtf8Name(RuntimeMethodHandleInternal method) [MethodImpl(MethodImplOptions.InternalCall)] internal static extern object? InvokeMethod(object? target, in Span arguments, Signature sig, bool constructor, bool wrapExceptions); - [DebuggerStepThroughAttribute] - [Diagnostics.DebuggerHidden] - [MethodImpl(MethodImplOptions.InternalCall)] - internal static extern object InvokeArrayCtor(in Span arguments, Signature sig); - [DllImport(RuntimeHelpers.QCall, EntryPoint = "RuntimeMethodHandle_GetMethodInstantiation")] private static extern void GetMethodInstantiation(RuntimeMethodHandleInternal method, ObjectHandleOnStack types, Interop.BOOL fAsRuntimeTypeArray); diff --git a/src/coreclr/vm/ecalllist.h b/src/coreclr/vm/ecalllist.h index 4938ea05149c1d..e02bad38f9430f 100644 --- a/src/coreclr/vm/ecalllist.h +++ b/src/coreclr/vm/ecalllist.h @@ -233,7 +233,6 @@ 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) diff --git a/src/coreclr/vm/reflectioninvocation.cpp b/src/coreclr/vm/reflectioninvocation.cpp index 4cc7b8ffc03889..68268942e562f5 100644 --- a/src/coreclr/vm/reflectioninvocation.cpp +++ b/src/coreclr/vm/reflectioninvocation.cpp @@ -478,6 +478,46 @@ static void CallDescrWorkerReflectionWrapper(CallDescrData * pCallDescrData, Fra PAL_ENDTRY } // CallDescrWorkerReflectionWrapper +static OBJECTREF InvokeArrayConstructor(TypeHandle th, Span* 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::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")); + + ARG_SLOT value; + InvokeUtil::CreatePrimitiveValue(ELEMENT_TYPE_I4, oType, objs->GetAt(i), &value); + memcpyNoGCRefs(indexes + i, ArgSlotEndianessFixup(&value, sizeof(INT32)), sizeof(INT32)); + } + + return AllocateArrayEx(th, indexes, argCnt); +} + static BOOL IsActivationNeededForMethodInvoke(MethodDesc * pMD) { CONTRACTL { @@ -731,8 +771,6 @@ FCIMPL5(Object*, RuntimeMethodHandle::InvokeMethod, HELPER_METHOD_FRAME_BEGIN_RET_PROTECT(gc); - Assembly *pAssem = pMeth->GetAssembly(); - if (ownerType.IsSharedByGenericInstantiations()) COMPlusThrow(kNotSupportedException, W("NotSupported_Type")); @@ -747,13 +785,21 @@ FCIMPL5(Object*, RuntimeMethodHandle::InvokeMethod, if (fConstructor) { - MethodTable * pMT = ownerType.AsMethodTable(); - - { - fCtorOfVariableSizedObject = pMT->HasComponentSize(); - if (!fCtorOfVariableSizedObject) - gc.retVal = pMT->Allocate(); + // If we are invoking a constructor on an array then we must + // handle this specially. + if (ownerType.IsArray()) { + gc.retVal = InvokeArrayConstructor(ownerType, + objs, + gc.pSig->NumFixedArgs()); + goto Done; } + + // Variable sized objects, like String instances, allocate themselves + // so they are a special case. + MethodTable * pMT = ownerType.AsMethodTable(); + fCtorOfVariableSizedObject = pMT->HasComponentSize(); + if (!fCtorOfVariableSizedObject) + gc.retVal = pMT->Allocate(); } { @@ -1095,66 +1141,8 @@ FCIMPL5(Object*, RuntimeMethodHandle::InvokeMethod, } - HELPER_METHOD_FRAME_END(); - - return OBJECTREFToObject(gc.retVal); -} -FCIMPLEND - - -FCIMPL2(Object*, RuntimeMethodHandle::InvokeArrayCtor, - Span* 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); - - if (th.IsSharedByGenericInstantiations()) - COMPlusThrow(kNotSupportedException, W("NotSupported_Type")); - - // 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::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")); - - ARG_SLOT value; - InvokeUtil::CreatePrimitiveValue(ELEMENT_TYPE_I4, oType, objs->GetAt(i), &value); - memcpyNoGCRefs(indexes + i, ArgSlotEndianessFixup(&value, sizeof(INT32)), sizeof(INT32)); - } - - gc.retVal = AllocateArrayEx(th, indexes, argCnt); - +Done: + ; HELPER_METHOD_FRAME_END(); return OBJECTREFToObject(gc.retVal); diff --git a/src/coreclr/vm/runtimehandles.h b/src/coreclr/vm/runtimehandles.h index 6264dc5a9ea3ff..184f27a7a311fe 100644 --- a/src/coreclr/vm/runtimehandles.h +++ b/src/coreclr/vm/runtimehandles.h @@ -224,8 +224,6 @@ class RuntimeMethodHandle { static FCDECL5(Object*, InvokeMethod, Object *target, Span* objs, SignatureNative* pSig, CLR_BOOL fConstructor, CLR_BOOL fWrapExceptions); - static FCDECL2(Object*, InvokeArrayCtor, Span* 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 From ff14814864793fe48e4d9195210dc08e2f670ef1 Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Wed, 10 Nov 2021 13:10:25 -0800 Subject: [PATCH 14/16] libraries/ Revert some non-impactful changes --- .../src/System/Reflection/RuntimeConstructorInfo.cs | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) 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 ae0576d74ea077..8ee3b14a658ccf 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.cs @@ -165,17 +165,7 @@ public override object Invoke(BindingFlags invokeAttr, Binder? binder, object?[] arguments = CheckArguments(ref stackArgs, parameters, binder, invokeAttr, culture, ArgumentTypes); } - object retValue; - - // Array construction is a special case. - if (DeclaringType is not null && DeclaringType.IsArray) - { - retValue = InvokeArrayCtorWorker(arguments); - } - else - { - retValue = InvokeCtorWorker(invokeAttr, arguments); - } + object retValue = InvokeCtorWorker(invokeAttr, arguments); // copy out. This should be made only if ByRef are present. // n.b. cannot use Span.CopyTo, as parameters.GetType() might not actually be typeof(object[]) From 74ec0cef32f3d6e45f928313fc8ef3c615ba2f86 Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Wed, 10 Nov 2021 13:11:01 -0800 Subject: [PATCH 15/16] mono/ Revert some non-impactful changes --- .../Reflection/RuntimeMethodInfo.Mono.cs | 9 ---- src/mono/mono/metadata/icall-def-netcore.h | 1 - src/mono/mono/metadata/icall.c | 52 ++++++++++++++++++- 3 files changed, 51 insertions(+), 11 deletions(-) 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 fc6d8d10024bbf..b715e4e3869623 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 @@ -875,12 +875,6 @@ internal object InvokeCtorWorker(BindingFlags invokeAttr, Span argument return InternalInvoke(null, arguments, wrapExceptions)!; } - [MethodImpl(MethodImplOptions.AggressiveInlining)] - internal object InvokeArrayCtorWorker(Span arguments) - { - return InternalInvokeArrayCtor(in arguments); - } - /* * InternalInvoke() receives the parameters correctly converted by the binder * to match the types of the method signature. @@ -888,9 +882,6 @@ internal object InvokeArrayCtorWorker(Span arguments) [MethodImplAttribute(MethodImplOptions.InternalCall)] internal extern object InternalInvoke(object? obj, in Span parameters, out Exception exc); - [MethodImplAttribute(MethodImplOptions.InternalCall)] - private extern object InternalInvokeArrayCtor(in Span arguments); - private object? InternalInvoke(object? obj, Span parameters, bool wrapExceptions) { Exception exc; diff --git a/src/mono/mono/metadata/icall-def-netcore.h b/src/mono/mono/metadata/icall-def-netcore.h index 037f6467931d6d..1f0a400008e5ef 100644 --- a/src/mono/mono/metadata/icall-def-netcore.h +++ b/src/mono/mono/metadata/icall-def-netcore.h @@ -253,7 +253,6 @@ 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) diff --git a/src/mono/mono/metadata/icall.c b/src/mono/mono/metadata/icall.c index 3e894fd0ab8bf5..31ae5570f987a8 100644 --- a/src/mono/mono/metadata/icall.c +++ b/src/mono/mono/metadata/icall.c @@ -3370,8 +3370,11 @@ ves_icall_InternalInvoke (MonoReflectionMethodHandle method_handle, MonoObjectHa * greater flexibility. */ 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; MonoException *exception = NULL; *MONO_HANDLE_REF (exception_out) = NULL; @@ -3397,6 +3400,52 @@ ves_icall_InternalInvoke (MonoReflectionMethodHandle method_handle, MonoObjectHa } } + /* Array constructor */ + if (m_class_get_rank (m->klass) && !strcmp (m->name, ".ctor")) { + int i; + pcount = mono_span_length (params_span); + uintptr_t * const lengths = g_newa (uintptr_t, pcount); + /* Note: the synthetized array .ctors have int32 as argument type */ + for (i = 0; i < pcount; ++i) + lengths [i] = *(int32_t*) ((char*)mono_span_get (params_span, MonoObject*, i) + sizeof (MonoObject)); + + 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. */ + arr = mono_array_new_full_checked (m->klass, lengths, NULL, error); + goto_if_nok (error, return_null); + + MonoArrayHandle subarray_handle = MONO_HANDLE_NEW (MonoArray, NULL); + + for (i = 0; i < mono_array_length_internal (arr); ++i) { + MonoArray *subarray = mono_array_new_full_checked (m_class_get_element_class (m->klass), &lengths [1], NULL, error); + goto_if_nok (error, return_null); + MONO_HANDLE_ASSIGN_RAW (subarray_handle, subarray); // FIXME? Overkill? + mono_array_setref_fast (arr, i, subarray); + } + goto exit; + } + + if (m_class_get_rank (m->klass) == pcount) { + /* Only lengths provided. */ + arr = mono_array_new_full_checked (m->klass, lengths, NULL, error); + goto_if_nok (error, return_null); + goto exit; + } 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); + + for (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)); + } + + arr = mono_array_new_full_checked (m->klass, lengths, lower_bounds, error); + goto_if_nok (error, return_null); + goto exit; + } + } + result = mono_runtime_invoke_span_checked (m, obj, params_span, error); goto exit; return_null: @@ -3406,7 +3455,8 @@ ves_icall_InternalInvoke (MonoReflectionMethodHandle method_handle, MonoObjectHa MONO_HANDLE_NEW (MonoException, exception); // FIXME? overkill? mono_gc_wbarrier_generic_store_internal (MONO_HANDLE_REF (exception_out), (MonoObject*)exception); } - return result ? MONO_HANDLE_NEW (MonoObject, result) : NULL_HANDLE; + g_assert (!result || !arr); // only one, or neither, should be set + return result ? MONO_HANDLE_NEW (MonoObject, result) : arr ? MONO_HANDLE_NEW (MonoObject, (MonoObject*)arr) : NULL_HANDLE; } MonoObjectHandle From c10c7c453d8e8ef7a8c6fceaea78e213311a5ba1 Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Wed, 10 Nov 2021 13:16:17 -0800 Subject: [PATCH 16/16] mono/ Remove additional changes. --- src/mono/mono/metadata/icall.c | 62 +--------------------------------- 1 file changed, 1 insertion(+), 61 deletions(-) diff --git a/src/mono/mono/metadata/icall.c b/src/mono/mono/metadata/icall.c index 31ae5570f987a8..536c6cf617b203 100644 --- a/src/mono/mono/metadata/icall.c +++ b/src/mono/mono/metadata/icall.c @@ -3445,11 +3445,11 @@ ves_icall_InternalInvoke (MonoReflectionMethodHandle method_handle, MonoObjectHa goto exit; } } - result = mono_runtime_invoke_span_checked (m, obj, params_span, error); goto exit; return_null: result = NULL; + arr = NULL; exit: if (exception) { MONO_HANDLE_NEW (MonoException, exception); // FIXME? overkill? @@ -3459,66 +3459,6 @@ ves_icall_InternalInvoke (MonoReflectionMethodHandle method_handle, MonoObjectHa return result ? MONO_HANDLE_NEW (MonoObject, result) : arr ? MONO_HANDLE_NEW (MonoObject, (MonoObject*)arr) : NULL_HANDLE; } -MonoObjectHandle -ves_icall_InternalInvokeArrayCtor (MonoReflectionMethodHandle method_handle, MonoSpanOfObjects *params_span, - MonoError *error) -{ - MonoReflectionMethod* const method = MONO_HANDLE_RAW (method_handle); - g_assert (params_span != NULL); - - uint32_t i; - MonoArray *arr = NULL; - MonoMethod *m = method->method; - MonoMethodSignature* const sig = mono_method_signature_internal (m); - g_assert (m_class_get_rank (m->klass) && !strcmp (m->name, ".ctor")); - - uint32_t pcount = mono_span_length (params_span); - uintptr_t * const lengths = g_newa (uintptr_t, pcount); - /* Note: the synthetized array .ctors have int32 as argument type */ - for (i = 0; i < pcount; ++i) - lengths [i] = *(int32_t*) ((char*)mono_span_get (params_span, MonoObject*, i) + sizeof (MonoObject)); - - 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. */ - arr = mono_array_new_full_checked (m->klass, lengths, NULL, error); - goto_if_nok (error, return_null); - - MonoArrayHandle subarray_handle = MONO_HANDLE_NEW (MonoArray, NULL); - - for (i = 0; i < mono_array_length_internal (arr); ++i) { - MonoArray *subarray = mono_array_new_full_checked (m_class_get_element_class (m->klass), &lengths [1], NULL, error); - goto_if_nok (error, return_null); - MONO_HANDLE_ASSIGN_RAW (subarray_handle, subarray); // FIXME? Overkill? - mono_array_setref_fast (arr, i, subarray); - } - goto exit; - } - - if (m_class_get_rank (m->klass) == pcount) { - /* Only lengths provided. */ - arr = mono_array_new_full_checked (m->klass, lengths, NULL, error); - goto_if_nok (error, return_null); - goto exit; - } 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); - - for (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)); - } - - arr = mono_array_new_full_checked (m->klass, lengths, lower_bounds, error); - goto_if_nok (error, return_null); - goto exit; - } -return_null: - arr = NULL; -exit: - return arr ? MONO_HANDLE_NEW (MonoObject, (MonoObject*)arr) : NULL_HANDLE; -} - static guint64 read_enum_value (const char *mem, int type) {