From a071262fa2c5abed750cbe610458230e236e0f78 Mon Sep 17 00:00:00 2001 From: Levi Broderick Date: Thu, 19 Nov 2020 13:19:22 -0800 Subject: [PATCH 01/15] Rename GetNewobjHelperFnPtr -> GetAllocatorFtn --- .../src/System/RuntimeHandles.cs | 13 ++++++------ src/coreclr/src/vm/ecalllist.h | 2 +- src/coreclr/src/vm/reflectioninvocation.cpp | 20 ++++++++++++------- src/coreclr/src/vm/runtimehandles.h | 2 +- 4 files changed, 21 insertions(+), 16 deletions(-) diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeHandles.cs b/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeHandles.cs index 1f7cf1a4ba4ba..ac0c89a9507e2 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeHandles.cs +++ b/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeHandles.cs @@ -207,9 +207,8 @@ internal static bool HasElementType(RuntimeType type) /// /// Given a RuntimeType, returns both the address of the JIT's newobj helper for that type and the - /// MethodTable* corresponding to that type. If the type is closed over - /// some T, then returns the newobj helper and MethodTable* for the 'T'. - /// Return value signature is managed calli (MethodTable* pMT) -> object. + /// MethodTable* corresponding to that type. Return value signature is + /// managed calli (MethodTable* pMT) -> object. /// internal static delegate* GetNewobjHelperFnPtr( // This API doesn't call any constructors, but the type needs to be seen as constructed. @@ -218,25 +217,25 @@ internal static bool HasElementType(RuntimeType type) // constructor are an academic problem. Valuetypes with no constructors are a problem, // but IL Linker currently treats them as always implicitly boxed. [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)] RuntimeType type, - out MethodTable* pMT, bool unwrapNullable) + out MethodTable* pMT, bool forGetUninitializedInstance) { Debug.Assert(type != null); delegate* pNewobjHelperTemp = null; MethodTable* pMTTemp = null; - GetNewobjHelperFnPtr( + GetAllocatorFtn( new QCallTypeHandle(ref type), &pNewobjHelperTemp, &pMTTemp, - unwrapNullable ? Interop.BOOL.TRUE : Interop.BOOL.FALSE); + forGetUninitializedInstance ? Interop.BOOL.TRUE : Interop.BOOL.FALSE); pMT = pMTTemp; return pNewobjHelperTemp; } [DllImport(RuntimeHelpers.QCall, CharSet = CharSet.Unicode)] - private static extern void GetNewobjHelperFnPtr(QCallTypeHandle typeHandle, delegate** ppNewobjHelper, MethodTable** ppMT, Interop.BOOL fUnwrapNullable); + private static extern void GetAllocatorFtn(QCallTypeHandle typeHandle, delegate** ppNewobjHelper, MethodTable** ppMT, Interop.BOOL fGetUninitializedInstance); /// /// Returns the MethodDesc* for this type's parameterless instance ctor. diff --git a/src/coreclr/src/vm/ecalllist.h b/src/coreclr/src/vm/ecalllist.h index 2be4a6cc47eed..17ea5afe1e07b 100644 --- a/src/coreclr/src/vm/ecalllist.h +++ b/src/coreclr/src/vm/ecalllist.h @@ -238,7 +238,7 @@ FCFuncStart(gCOMTypeHandleFuncs) FCFuncElement("IsGenericTypeDefinition", RuntimeTypeHandle::IsGenericTypeDefinition) FCFuncElement("ContainsGenericVariables", RuntimeTypeHandle::ContainsGenericVariables) FCFuncElement("SatisfiesConstraints", RuntimeTypeHandle::SatisfiesConstraints) - QCFuncElement("GetNewobjHelperFnPtr", RuntimeTypeHandle::GetNewobjHelperFnPtr) + QCFuncElement("GetAllocatorFtn", RuntimeTypeHandle::GetAllocatorFtn) QCFuncElement("GetDefaultCtor", RuntimeTypeHandle::GetDefaultCtor) FCFuncElement("CompareCanonicalHandles", RuntimeTypeHandle::CompareCanonicalHandles) FCIntrinsic("GetValueInternal", RuntimeTypeHandle::GetValueInternal, CORINFO_INTRINSIC_RTH_GetValueInternal) diff --git a/src/coreclr/src/vm/reflectioninvocation.cpp b/src/coreclr/src/vm/reflectioninvocation.cpp index c8681497c1e9e..b9c16537e3695 100644 --- a/src/coreclr/src/vm/reflectioninvocation.cpp +++ b/src/coreclr/src/vm/reflectioninvocation.cpp @@ -2007,11 +2007,11 @@ FCIMPLEND * then if the input type handle is Nullable we'll return the newobj helper and * MethodTable* for the underlying T. */ -void QCALLTYPE RuntimeTypeHandle::GetNewobjHelperFnPtr( +void QCALLTYPE RuntimeTypeHandle::GetAllocatorFtn( QCall::TypeHandle pTypeHandle, PCODE* ppNewobjHelper, MethodTable** ppMT, - BOOL fUnwrapNullable) + BOOL fGetUninitializedInstance) { CONTRACTL{ QCALL_CHECK; @@ -2026,8 +2026,14 @@ void QCALLTYPE RuntimeTypeHandle::GetNewobjHelperFnPtr( TypeHandle typeHandle = pTypeHandle.AsTypeHandle(); - // Don't allow void, arrays, pointers, byrefs, or function pointers. - if (typeHandle.IsTypeDesc() || typeHandle.IsArray() || typeHandle.GetSignatureCorElementType() == ELEMENT_TYPE_VOID) + // Don't allow void + if (typeHandle.GetSignatureCorElementType() == ELEMENT_TYPE_VOID) + { + COMPlusThrow(kArgumentException, W("Argument_InvalidValue")); + } + + // Don't allow arrays, pointers, byrefs, or function pointers + if (typeHandle.IsTypeDesc() || typeHandle.IsArray()) { COMPlusThrow(kArgumentException, W("Argument_InvalidValue")); } @@ -2079,9 +2085,9 @@ void QCALLTYPE RuntimeTypeHandle::GetNewobjHelperFnPtr( } #endif // FEATURE_COMINTEROP - // If the caller passed Nullable and wanted nullables unwrapped, - // instead pretend they had passed the 'T' directly. - if (fUnwrapNullable && Nullable::IsNullableType(pMT)) + // If the caller is GetUninitializedInstance, they'll want a boxed T instead of a boxed Nullable. + // Other callers will get the MethodTable* corresponding to Nullable. + if (fGetUninitializedInstance && Nullable::IsNullableType(pMT)) { pMT = pMT->GetInstantiation()[0].GetMethodTable(); } diff --git a/src/coreclr/src/vm/runtimehandles.h b/src/coreclr/src/vm/runtimehandles.h index a175107a3b4d2..ef9f8a7a632aa 100644 --- a/src/coreclr/src/vm/runtimehandles.h +++ b/src/coreclr/src/vm/runtimehandles.h @@ -131,7 +131,7 @@ class RuntimeTypeHandle { CLR_BOOL *pbHasNoDefaultCtor); static - void QCALLTYPE GetNewobjHelperFnPtr(QCall::TypeHandle pTypeHandle, PCODE* ppNewobjHelper, MethodTable** ppMT, BOOL fUnwrapNullable); + void QCALLTYPE GetAllocatorFtn(QCall::TypeHandle pTypeHandle, PCODE* ppNewobjHelper, MethodTable** ppMT, BOOL fGetUninitializedInstance); static MethodDesc* QCALLTYPE GetDefaultCtor(QCall::TypeHandle pTypeHandle); From dc417fdfd6cc8b6e8f3206a525afc3c839b4ae93 Mon Sep 17 00:00:00 2001 From: Levi Broderick Date: Thu, 19 Nov 2020 14:04:50 -0800 Subject: [PATCH 02/15] Cleanup in the allocator routines --- .../RuntimeHelpers.CoreCLR.cs | 9 ++- .../src/System/RuntimeHandles.cs | 12 +-- .../src/System/RuntimeType.CoreCLR.cs | 78 ++++++++----------- src/coreclr/src/vm/reflectioninvocation.cpp | 11 +-- src/coreclr/src/vm/runtimehandles.h | 2 +- 5 files changed, 51 insertions(+), 61 deletions(-) diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs b/src/coreclr/src/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs index aafbf652d3753..0526bd8028da4 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs +++ b/src/coreclr/src/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs @@ -143,12 +143,13 @@ private static unsafe object GetUninitializedObjectInternal( RuntimeType rt = (RuntimeType)type; Debug.Assert(rt != null); - // If type is Nullable, get newobj for boxed T instead. - delegate* newobjHelper = RuntimeTypeHandle.GetNewobjHelperFnPtr(rt, out MethodTable* pMT, unwrapNullable: true); - Debug.Assert(newobjHelper != null); + // If type is Nullable, returns the allocator and MethodTable* for the underlying T. + delegate* pfnAllocator = RuntimeTypeHandle.GetAllocatorFtn(rt, out MethodTable* pMT, forGetUninitializedObject: true); + Debug.Assert(pfnAllocator != null); Debug.Assert(pMT != null); + Debug.Assert(!pMT->IsNullable, "Should've unwrapped any Nullable input."); - object retVal = newobjHelper(pMT); + object retVal = pfnAllocator(pMT); GC.KeepAlive(rt); // don't allow the type to be collected before the object is instantiated return retVal; diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeHandles.cs b/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeHandles.cs index ac0c89a9507e2..8c490b3ad9eff 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeHandles.cs +++ b/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeHandles.cs @@ -206,18 +206,18 @@ internal static bool HasElementType(RuntimeType type) internal static extern object CreateInstanceForAnotherGenericParameter(RuntimeType type, RuntimeType genericParameter); /// - /// Given a RuntimeType, returns both the address of the JIT's newobj helper for that type and the - /// MethodTable* corresponding to that type. Return value signature is + /// Given a RuntimeType, returns both the address of the JIT's newobj allocator helper for + /// that type and the MethodTable* corresponding to that type. Return value signature is /// managed calli (MethodTable* pMT) -> object. /// - internal static delegate* GetNewobjHelperFnPtr( + internal static delegate* GetAllocatorFtn( // This API doesn't call any constructors, but the type needs to be seen as constructed. // A type is seen as constructed if a constructor is kept. // This obviously won't cover a type with no constructor. Reference types with no // constructor are an academic problem. Valuetypes with no constructors are a problem, // but IL Linker currently treats them as always implicitly boxed. [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)] RuntimeType type, - out MethodTable* pMT, bool forGetUninitializedInstance) + out MethodTable* pMT, bool forGetUninitializedObject) { Debug.Assert(type != null); @@ -228,14 +228,14 @@ internal static bool HasElementType(RuntimeType type) new QCallTypeHandle(ref type), &pNewobjHelperTemp, &pMTTemp, - forGetUninitializedInstance ? Interop.BOOL.TRUE : Interop.BOOL.FALSE); + forGetUninitializedObject ? Interop.BOOL.TRUE : Interop.BOOL.FALSE); pMT = pMTTemp; return pNewobjHelperTemp; } [DllImport(RuntimeHelpers.QCall, CharSet = CharSet.Unicode)] - private static extern void GetAllocatorFtn(QCallTypeHandle typeHandle, delegate** ppNewobjHelper, MethodTable** ppMT, Interop.BOOL fGetUninitializedInstance); + private static extern void GetAllocatorFtn(QCallTypeHandle typeHandle, delegate** ppNewobjHelper, MethodTable** ppMT, Interop.BOOL fGetUninitializedObject); /// /// Returns the MethodDesc* for this type's parameterless instance ctor. diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs b/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs index f61090f8f553e..7080e6a4299f5 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs +++ b/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs @@ -3953,14 +3953,13 @@ private void CreateInstanceCheckThis() // the cache entry private sealed unsafe class ActivatorCache { - // The managed calli to the newobj routine, plus its first argument (MethodTable*). - private readonly delegate* _pfnNewobj; + // The managed calli to the newobj allocator, plus its first argument (MethodTable*). + private readonly delegate* _pfnAllocator; private readonly MethodTable* _pMT; - // The managed calli to the parameterless ctor, plus a state object. - // State object depends on the stub being called. - private readonly delegate* _pfnCtorStub; - private readonly IntPtr _ctorStubState; + // The managed calli to the parameterless ctor, taking "this" (as object) as its first argument. + // For value type ctors, we'll point to a special unboxing stub. + private readonly delegate* _pfnCtor; #if DEBUG private readonly RuntimeType _originalRT; @@ -3974,12 +3973,12 @@ internal ActivatorCache([DynamicallyAccessedMembers(DynamicallyAccessedMemberTyp _originalRT = rt; #endif - _pfnCtorStub = &CtorNoopStub; // by default, no ctor is run - _ctorStubState = IntPtr.Zero; + _pfnAllocator = RuntimeTypeHandle.GetAllocatorFtn(rt, out _pMT, forGetUninitializedObject: false); - _pfnNewobj = RuntimeTypeHandle.GetNewobjHelperFnPtr(rt, out _pMT, unwrapNullable: false); + static void CtorNoopStub(object? uninitializedObject) { } + _pfnCtor = &CtorNoopStub; - RuntimeMethodHandleInternal defaultCtorRMH = RuntimeTypeHandle.GetDefaultConstructor(rt); + RuntimeMethodHandleInternal defaultCtorRMH = RuntimeTypeHandle.GetDefaultConstructor(rt); // could be null if (_pMT->IsValueType) { @@ -3988,31 +3987,30 @@ internal ActivatorCache([DynamicallyAccessedMembers(DynamicallyAccessedMemberTyp // Activator.CreateInstance returns null given typeof(Nullable). static object? ReturnNull(MethodTable* pMT) => null; - _pfnNewobj = &ReturnNull; - _pMT = null; - - _pfnCtorStub = &CtorNoopStub; - _ctorStubState = IntPtr.Zero; + _pfnAllocator = &ReturnNull; } else if (_pMT->HasDefaultConstructor) { // ValueType with explicit parameterless ctor typed as (ref T) -> void. - // We'll pass the actual ctor address as the state object, then create - // an unboxing stub so that we can pass the boxed value to it. + // We'll point the ctor at our unboxing stub. It's not terribly efficient, + // but value types almost never have explicit parameterless ctors, so + // this shouldn't be a problem in practice. - static void ValueTypeUnboxingStub(object? @this, IntPtr state) + static void CtorUnboxValueTypeStub(object? uninitializedObject) { - ((delegate*)state)(ref @this!.GetRawData()); + Debug.Assert(uninitializedObject != null); + Debug.Assert(RuntimeHelpers.GetMethodTable(uninitializedObject)->IsValueType); + + RuntimeType rt = (RuntimeType)uninitializedObject.GetType(); + RuntimeMethodHandleInternal rmh = RuntimeTypeHandle.GetDefaultConstructor(rt); + IntPtr pfnCtor = RuntimeMethodHandle.GetFunctionPointer(rmh); + ((delegate*)pfnCtor)(ref uninitializedObject!.GetRawData()); } - _pfnCtorStub = &ValueTypeUnboxingStub; - _ctorStubState = RuntimeMethodHandle.GetFunctionPointer(defaultCtorRMH); + _pfnCtor = &CtorUnboxValueTypeStub; } else { // ValueType with no explicit parameterless ctor; assume ctor returns default(T) - - _pfnCtorStub = &CtorNoopStub; - _ctorStubState = IntPtr.Zero; } } else @@ -4029,22 +4027,13 @@ static void ValueTypeUnboxingStub(object? @this, IntPtr state) Debug.Assert(!rt.IsGenericCOMObjectImpl(), "__ComObject base class should've been blocked."); defaultCtorRMH = default; // ignore any parameterless ctor } + else if (!_pMT->HasDefaultConstructor) + { + throw new MissingMethodException(SR.Format(SR.Arg_NoDefCTor, rt)); + } else { - if (!_pMT->HasDefaultConstructor) - { - throw new MissingMethodException(SR.Format(SR.Arg_NoDefCTor, rt)); - } - - // Reference type with explicit parameterless ctor typed as (object) -> void. - // We'll pass the actual ctor address as the state object. - - static void ReferenceTypeStub(object? @this, IntPtr state) - { - ((delegate*)state)(@this!); - } - _pfnCtorStub = &ReferenceTypeStub; - _ctorStubState = RuntimeMethodHandle.GetFunctionPointer(defaultCtorRMH); + // Reference type with explicit parameterless ctor } } @@ -4057,14 +4046,13 @@ static void ReferenceTypeStub(object? @this, IntPtr state) CtorIsPublic = (RuntimeMethodHandle.GetAttributes(defaultCtorRMH) & MethodAttributes.Public) != 0; } - Debug.Assert(_pfnNewobj != null); - Debug.Assert(_pfnCtorStub != null); // we use null singleton pattern if no ctor call is necessary + Debug.Assert(_pfnAllocator != null); + Debug.Assert(_pMT != null); + Debug.Assert(_pfnCtor != null); // we use null singleton pattern if no ctor call is necessary } internal bool CtorIsPublic { get; } - private static void CtorNoopStub(object? @this, IntPtr state) { } - [MethodImpl(MethodImplOptions.AggressiveInlining)] internal object? CreateUninitializedObject(RuntimeType rt) { @@ -4077,13 +4065,13 @@ private static void CtorNoopStub(object? @this, IntPtr state) { } Debug.Assert(rt == _originalRT, "Caller passed the wrong RuntimeType to this routine."); #endif - object? retVal = _pfnNewobj(_pMT); + object? retVal = _pfnAllocator(_pMT); GC.KeepAlive(rt); return retVal; } [MethodImpl(MethodImplOptions.AggressiveInlining)] - internal void CallCtorOverUninitializedObject(object? newObj) => _pfnCtorStub(newObj, _ctorStubState); + internal void CallConstructor(object? uninitializedObject) => _pfnCtor(uninitializedObject); } // This method mimics the overload present in the mono codebase. It allows shared source callers @@ -4133,7 +4121,7 @@ private static void CtorNoopStub(object? @this, IntPtr state) { } object? obj = cache.CreateUninitializedObject(this); try { - cache.CallCtorOverUninitializedObject(obj); + cache.CallConstructor(obj); } catch (Exception e) when (wrapExceptions) { diff --git a/src/coreclr/src/vm/reflectioninvocation.cpp b/src/coreclr/src/vm/reflectioninvocation.cpp index b9c16537e3695..18781566291b8 100644 --- a/src/coreclr/src/vm/reflectioninvocation.cpp +++ b/src/coreclr/src/vm/reflectioninvocation.cpp @@ -2003,15 +2003,16 @@ FCIMPLEND * Given a TypeHandle, returns the address of the NEWOBJ helper function that creates * a zero-inited instance of this type. If NEWOBJ is not supported on this TypeHandle, * throws an exception. If TypeHandle is a value type, the NEWOBJ helper will create - * a boxed zero-inited instance of the value type. If fUnwrapNullable is specified, - * then if the input type handle is Nullable we'll return the newobj helper and - * MethodTable* for the underlying T. + * a boxed zero-inited instance of the value type. The "fGetUninitializedObject" + * parameter dictates whether the caller is RuntimeHelpers.GetUninitializedObject or + * Activator.CreateInstance, which have different behavior w.r.t. what exceptions are + * thrown on failure and how nullables are handled. */ void QCALLTYPE RuntimeTypeHandle::GetAllocatorFtn( QCall::TypeHandle pTypeHandle, PCODE* ppNewobjHelper, MethodTable** ppMT, - BOOL fGetUninitializedInstance) + BOOL fGetUninitializedObject) { CONTRACTL{ QCALL_CHECK; @@ -2087,7 +2088,7 @@ void QCALLTYPE RuntimeTypeHandle::GetAllocatorFtn( // If the caller is GetUninitializedInstance, they'll want a boxed T instead of a boxed Nullable. // Other callers will get the MethodTable* corresponding to Nullable. - if (fGetUninitializedInstance && Nullable::IsNullableType(pMT)) + if (fGetUninitializedObject && Nullable::IsNullableType(pMT)) { pMT = pMT->GetInstantiation()[0].GetMethodTable(); } diff --git a/src/coreclr/src/vm/runtimehandles.h b/src/coreclr/src/vm/runtimehandles.h index ef9f8a7a632aa..fe6c6f7db8e86 100644 --- a/src/coreclr/src/vm/runtimehandles.h +++ b/src/coreclr/src/vm/runtimehandles.h @@ -131,7 +131,7 @@ class RuntimeTypeHandle { CLR_BOOL *pbHasNoDefaultCtor); static - void QCALLTYPE GetAllocatorFtn(QCall::TypeHandle pTypeHandle, PCODE* ppNewobjHelper, MethodTable** ppMT, BOOL fGetUninitializedInstance); + void QCALLTYPE GetAllocatorFtn(QCall::TypeHandle pTypeHandle, PCODE* ppNewobjHelper, MethodTable** ppMT, BOOL fGetUninitializedObject); static MethodDesc* QCALLTYPE GetDefaultCtor(QCall::TypeHandle pTypeHandle); From 6904495141ffc6692afdb5decce0d908c7dacc3f Mon Sep 17 00:00:00 2001 From: Levi Broderick Date: Thu, 19 Nov 2020 14:25:41 -0800 Subject: [PATCH 03/15] Fix linker failures, add asserts --- .../src/System/RuntimeType.CoreCLR.cs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs b/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs index 7080e6a4299f5..f66f731450b82 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs +++ b/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs @@ -3996,6 +3996,8 @@ static void CtorNoopStub(object? uninitializedObject) { } // but value types almost never have explicit parameterless ctors, so // this shouldn't be a problem in practice. + [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2072:UnrecognizedReflectionPattern", + Justification = "The linker requirements are already satisfied by the enclosing method.")] static void CtorUnboxValueTypeStub(object? uninitializedObject) { Debug.Assert(uninitializedObject != null); @@ -4011,6 +4013,8 @@ static void CtorUnboxValueTypeStub(object? uninitializedObject) else { // ValueType with no explicit parameterless ctor; assume ctor returns default(T) + + Debug.Assert(defaultCtorRMH.IsNullHandle()); } } else @@ -4034,6 +4038,8 @@ static void CtorUnboxValueTypeStub(object? uninitializedObject) else { // Reference type with explicit parameterless ctor + + Debug.Assert(!defaultCtorRMH.IsNullHandle()); } } @@ -4109,7 +4115,7 @@ static void CtorUnboxValueTypeStub(object? uninitializedObject) GenericCache = cache; } - if (publicOnly & !cache.CtorIsPublic) + if (!cache.CtorIsPublic && publicOnly) { throw new MissingMethodException(SR.Format(SR.Arg_NoDefCTor, this)); } From 05c35a82452d1c1ca12024a689f95fc4ad62efc1 Mon Sep 17 00:00:00 2001 From: Levi Broderick Date: Thu, 19 Nov 2020 14:28:16 -0800 Subject: [PATCH 04/15] Remove unwanted CreateInstanceDefaultCtor overload --- .../src/System/RuntimeType.CoreCLR.cs | 15 --------------- .../src/System/Activator.RuntimeType.cs | 4 ++-- 2 files changed, 2 insertions(+), 17 deletions(-) diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs b/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs index f66f731450b82..465594760e8ad 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs +++ b/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs @@ -4080,21 +4080,6 @@ static void CtorUnboxValueTypeStub(object? uninitializedObject) internal void CallConstructor(object? uninitializedObject) => _pfnCtor(uninitializedObject); } - // This method mimics the overload present in the mono codebase. It allows shared source callers - // to target this overload and work across both runtimes. CoreCLR ignores the 'skipCheckThis' and - // 'fillCache' parameters. - [DebuggerStepThrough] - [DebuggerHidden] - [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2082:UnrecognizedReflectionPattern", - Justification = "Implementation detail of Activator that linker intrinsically recognizes")] - [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2085:UnrecognizedReflectionPattern", - Justification = "Implementation detail of Activator that linker intrinsically recognizes")] - [MethodImpl(MethodImplOptions.AggressiveInlining)] - internal object? CreateInstanceDefaultCtor(bool publicOnly, bool skipCheckThis, bool fillCache, bool wrapExceptions) - { - return CreateInstanceDefaultCtor(publicOnly, wrapExceptions); - } - /// /// Helper to invoke the default (parameterless) constructor. /// diff --git a/src/libraries/System.Private.CoreLib/src/System/Activator.RuntimeType.cs b/src/libraries/System.Private.CoreLib/src/System/Activator.RuntimeType.cs index 043ec1fd275d5..1008e9e186720 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Activator.RuntimeType.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Activator.RuntimeType.cs @@ -94,7 +94,7 @@ public static partial class Activator throw new ArgumentNullException(nameof(type)); if (type.UnderlyingSystemType is RuntimeType rt) - return rt.CreateInstanceDefaultCtor(publicOnly: !nonPublic, skipCheckThis: false, fillCache: true, wrapExceptions: wrapExceptions); + return rt.CreateInstanceDefaultCtor(publicOnly: !nonPublic, wrapExceptions: wrapExceptions); throw new ArgumentException(SR.Arg_MustBeType, nameof(type)); } @@ -150,7 +150,7 @@ public static partial class Activator [System.Runtime.CompilerServices.Intrinsic] public static T CreateInstance<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)]T>() { - return (T)((RuntimeType)typeof(T)).CreateInstanceDefaultCtor(publicOnly: true, skipCheckThis: true, fillCache: true, wrapExceptions: true)!; + return (T)((RuntimeType)typeof(T)).CreateInstanceDefaultCtor(publicOnly: true, wrapExceptions: true)!; } private static T CreateDefaultInstance() where T : struct => default; From 3d8515acab710dcf9b50279eb4ae6a0fb502e99c Mon Sep 17 00:00:00 2001 From: Levi Broderick Date: Thu, 19 Nov 2020 14:39:37 -0800 Subject: [PATCH 05/15] Use WeakRef in ActivatorCache --- .../src/System/RuntimeType.CoreCLR.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs b/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs index 465594760e8ad..0618e2db62023 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs +++ b/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs @@ -3962,7 +3962,7 @@ private sealed unsafe class ActivatorCache private readonly delegate* _pfnCtor; #if DEBUG - private readonly RuntimeType _originalRT; + private readonly WeakReference _originalRuntimeType; // don't prevent the RT from being collected #endif internal ActivatorCache([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)] RuntimeType rt) @@ -3970,7 +3970,7 @@ internal ActivatorCache([DynamicallyAccessedMembers(DynamicallyAccessedMemberTyp Debug.Assert(rt != null); #if DEBUG - _originalRT = rt; + _originalRuntimeType = new WeakReference(rt); #endif _pfnAllocator = RuntimeTypeHandle.GetAllocatorFtn(rt, out _pMT, forGetUninitializedObject: false); @@ -4068,7 +4068,8 @@ static void CtorUnboxValueTypeStub(object? uninitializedObject) // as the object itself will keep the type alive. #if DEBUG - Debug.Assert(rt == _originalRT, "Caller passed the wrong RuntimeType to this routine."); + Debug.Assert(_originalRuntimeType.TryGetTarget(out RuntimeType? originalRT) && originalRT == rt, + "Caller passed the wrong RuntimeType to this routine."); #endif object? retVal = _pfnAllocator(_pMT); From 5acef41a31828ec529ab0b673004b59cd50d391b Mon Sep 17 00:00:00 2001 From: Levi Broderick Date: Thu, 19 Nov 2020 17:29:47 -0800 Subject: [PATCH 06/15] Reintroduce COM specialization --- .../System.Private.CoreLib.csproj | 1 + .../src/System/ActivatorCache.CoreCLR.cs | 169 ++++++++++++++++++ .../src/System/RuntimeHandles.cs | 26 ++- .../src/System/RuntimeType.CoreCLR.cs | 131 -------------- src/coreclr/src/vm/ecalllist.h | 1 + src/coreclr/src/vm/reflectioninvocation.cpp | 49 ++++- src/coreclr/src/vm/runtimehandles.h | 10 +- 7 files changed, 240 insertions(+), 147 deletions(-) create mode 100644 src/coreclr/src/System.Private.CoreLib/src/System/ActivatorCache.CoreCLR.cs diff --git a/src/coreclr/src/System.Private.CoreLib/System.Private.CoreLib.csproj b/src/coreclr/src/System.Private.CoreLib/System.Private.CoreLib.csproj index 4de4ac0aa5860..c5bf5c9f45c20 100644 --- a/src/coreclr/src/System.Private.CoreLib/System.Private.CoreLib.csproj +++ b/src/coreclr/src/System.Private.CoreLib/System.Private.CoreLib.csproj @@ -111,6 +111,7 @@ + diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/ActivatorCache.CoreCLR.cs b/src/coreclr/src/System.Private.CoreLib/src/System/ActivatorCache.CoreCLR.cs new file mode 100644 index 0000000000000..9c991fceb057d --- /dev/null +++ b/src/coreclr/src/System.Private.CoreLib/src/System/ActivatorCache.CoreCLR.cs @@ -0,0 +1,169 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; +using System.Reflection; +using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; + +namespace System +{ + internal sealed partial class RuntimeType + { + /// + /// A cache which allows optimizing , + /// , and related APIs. + /// + private sealed unsafe class ActivatorCache + { + // The managed calli to the newobj allocator, plus its first argument (MethodTable*). + // In the case of the COM allocator, first arg is ComClassFactory*, not MethodTable*. + private readonly delegate* _pfnAllocator; + private readonly IntPtr _allocatorFirstArg; + + // The managed calli to the parameterless ctor, taking "this" (as object) as its first argument. + // For value type ctors, we'll point to a special unboxing stub. + private readonly delegate* _pfnCtor; + +#if DEBUG + private readonly WeakReference _originalRuntimeType; // don't prevent the RT from being collected +#endif + + internal ActivatorCache([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)] RuntimeType rt) + { + Debug.Assert(rt != null); + +#if DEBUG + _originalRuntimeType = new WeakReference(rt); +#endif + + _pfnAllocator = (delegate*)RuntimeTypeHandle.GetAllocatorFtn(rt, out MethodTable* pMT, forGetUninitializedObject: false); + _allocatorFirstArg = (IntPtr)pMT; + + static void CtorNoopStub(object? uninitializedObject) { } + _pfnCtor = &CtorNoopStub; + + RuntimeMethodHandleInternal defaultCtorRMH = RuntimeTypeHandle.GetDefaultConstructor(rt); // could be null + + if (pMT->IsValueType) + { + if (pMT->IsNullable) + { + // Activator.CreateInstance returns null given typeof(Nullable). + + static object? ReturnNull(IntPtr _) => null; + _pfnAllocator = &ReturnNull; + } + else if (pMT->HasDefaultConstructor) + { + // ValueType with explicit parameterless ctor typed as (ref T) -> void. + // We'll point the ctor at our unboxing stub. It's not terribly efficient, + // but value types almost never have explicit parameterless ctors, so + // this shouldn't be a problem in practice. + + [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2072:UnrecognizedReflectionPattern", + Justification = "The linker requirements are already satisfied by the enclosing method.")] + static void CtorUnboxValueTypeStub(object? uninitializedObject) + { + Debug.Assert(uninitializedObject != null); + Debug.Assert(RuntimeHelpers.GetMethodTable(uninitializedObject)->IsValueType); + + RuntimeType rt = (RuntimeType)uninitializedObject.GetType(); + RuntimeMethodHandleInternal rmh = RuntimeTypeHandle.GetDefaultConstructor(rt); + IntPtr pfnCtor = RuntimeMethodHandle.GetFunctionPointer(rmh); + ((delegate*)pfnCtor)(ref uninitializedObject!.GetRawData()); + } + _pfnCtor = &CtorUnboxValueTypeStub; + } + else + { + // ValueType with no explicit parameterless ctor; assume ctor returns default(T) + + Debug.Assert(defaultCtorRMH.IsNullHandle()); + } + } + else + { + // Reference type - we can't proceed unless there's a default ctor we can call. + + Debug.Assert(rt.IsClass); + + if (pMT->IsComObject) + { + if (rt.IsGenericCOMObjectImpl()) + { + // This is the __ComObject base type, which means that the MethodTable* we have + // doesn't contain CLSID information. The CLSID information is instead hanging + // off of the RuntimeType's sync block. We'll set the allocator to our stub, and + // instead of a MethodTable* we'll pass in the handle to the RuntimeType. The + // handles we create live for the lifetime of the app, but that's ok since it + // matches coreclr's internal implementation anyway (see GetComClassHelper). + + static object AllocateComObject(IntPtr runtimeTypeHandle) + { + RuntimeType rt = (RuntimeType)GCHandle.FromIntPtr(runtimeTypeHandle).Target!; + Debug.Assert(rt != null); + + return RuntimeTypeHandle.AllocateComObject(rt); + } + _pfnAllocator = &AllocateComObject; + _allocatorFirstArg = GCHandle.ToIntPtr(GCHandle.Alloc(rt)); + } + + // Neither __ComObject nor any derived type gets its parameterless ctor called. + // Activation is handled entirely by the allocator. + + defaultCtorRMH = default; + } + else if (!pMT->HasDefaultConstructor) + { + throw new MissingMethodException(SR.Format(SR.Arg_NoDefCTor, rt)); + } + else + { + // Reference type with explicit parameterless ctor + + Debug.Assert(!defaultCtorRMH.IsNullHandle()); + } + } + + if (defaultCtorRMH.IsNullHandle()) + { + CtorIsPublic = true; // implicit parameterless ctor is always considered public + } + else + { + CtorIsPublic = (RuntimeMethodHandle.GetAttributes(defaultCtorRMH) & MethodAttributes.Public) != 0; + } + + Debug.Assert(_pfnAllocator != null); + Debug.Assert(_allocatorFirstArg != IntPtr.Zero); + Debug.Assert(_pfnCtor != null); // we use null singleton pattern if no ctor call is necessary + } + + internal bool CtorIsPublic { get; } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + internal object? CreateUninitializedObject(RuntimeType rt) + { + // We don't use RuntimeType, but we force the caller to pass it so + // that we can keep it alive on their behalf. Once the object is + // constructed, we no longer need the reference to the type instance, + // as the object itself will keep the type alive. + +#if DEBUG + Debug.Assert(_originalRuntimeType.TryGetTarget(out RuntimeType? originalRT) && originalRT == rt, + "Caller passed the wrong RuntimeType to this routine."); +#endif + + object? retVal = _pfnAllocator(_allocatorFirstArg); + GC.KeepAlive(rt); + return retVal; + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + internal void CallConstructor(object? uninitializedObject) => _pfnCtor(uninitializedObject); + } + } +} diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeHandles.cs b/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeHandles.cs index 8c490b3ad9eff..c6b4e595668cc 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeHandles.cs +++ b/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeHandles.cs @@ -199,9 +199,6 @@ internal static bool HasElementType(RuntimeType type) [MethodImpl(MethodImplOptions.InternalCall)] internal static extern object CreateInstance(RuntimeType type, bool publicOnly, bool wrapExceptions, ref bool canBeCached, ref RuntimeMethodHandleInternal ctor, ref bool hasNoDefaultCtor); - [MethodImpl(MethodImplOptions.InternalCall)] - internal static extern object Allocate(RuntimeType type); - [MethodImpl(MethodImplOptions.InternalCall)] internal static extern object CreateInstanceForAnotherGenericParameter(RuntimeType type, RuntimeType genericParameter); @@ -254,6 +251,29 @@ internal static RuntimeMethodHandleInternal GetDefaultConstructor( [DllImport(RuntimeHelpers.QCall, CharSet = CharSet.Unicode)] private static extern RuntimeMethodHandleInternal GetDefaultCtor(QCallTypeHandle typeHandle); + /// + /// Given a RuntimeType which represents __ComObject, activates the class and creates + /// a RCW around it. + /// + /// No CLSID present, or invalid CLSID. + internal static object AllocateComObject( + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)] RuntimeType type) + { + Debug.Assert(type != null); + + // n.b. use ObjectHandleOnStack instead of QCallTypeHandle since runtime needs the actual RuntimeType instance, + // not just its underlying TypeHandle. + + object activatedInstance = null!; + AllocateComObject(ObjectHandleOnStack.Create(ref type), ObjectHandleOnStack.Create(ref activatedInstance)); + + Debug.Assert(activatedInstance != null); + return activatedInstance; + } + + [DllImport(RuntimeHelpers.QCall, CharSet = CharSet.Unicode)] + private static extern void AllocateComObject(ObjectHandleOnStack runtimeType, ObjectHandleOnStack activatedInstance); + internal RuntimeType GetRuntimeType() { return m_type; diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs b/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs index 0618e2db62023..8545a205015b1 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs +++ b/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs @@ -3950,137 +3950,6 @@ private void CreateInstanceCheckThis() return instance; } - // the cache entry - private sealed unsafe class ActivatorCache - { - // The managed calli to the newobj allocator, plus its first argument (MethodTable*). - private readonly delegate* _pfnAllocator; - private readonly MethodTable* _pMT; - - // The managed calli to the parameterless ctor, taking "this" (as object) as its first argument. - // For value type ctors, we'll point to a special unboxing stub. - private readonly delegate* _pfnCtor; - -#if DEBUG - private readonly WeakReference _originalRuntimeType; // don't prevent the RT from being collected -#endif - - internal ActivatorCache([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)] RuntimeType rt) - { - Debug.Assert(rt != null); - -#if DEBUG - _originalRuntimeType = new WeakReference(rt); -#endif - - _pfnAllocator = RuntimeTypeHandle.GetAllocatorFtn(rt, out _pMT, forGetUninitializedObject: false); - - static void CtorNoopStub(object? uninitializedObject) { } - _pfnCtor = &CtorNoopStub; - - RuntimeMethodHandleInternal defaultCtorRMH = RuntimeTypeHandle.GetDefaultConstructor(rt); // could be null - - if (_pMT->IsValueType) - { - if (_pMT->IsNullable) - { - // Activator.CreateInstance returns null given typeof(Nullable). - - static object? ReturnNull(MethodTable* pMT) => null; - _pfnAllocator = &ReturnNull; - } - else if (_pMT->HasDefaultConstructor) - { - // ValueType with explicit parameterless ctor typed as (ref T) -> void. - // We'll point the ctor at our unboxing stub. It's not terribly efficient, - // but value types almost never have explicit parameterless ctors, so - // this shouldn't be a problem in practice. - - [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2072:UnrecognizedReflectionPattern", - Justification = "The linker requirements are already satisfied by the enclosing method.")] - static void CtorUnboxValueTypeStub(object? uninitializedObject) - { - Debug.Assert(uninitializedObject != null); - Debug.Assert(RuntimeHelpers.GetMethodTable(uninitializedObject)->IsValueType); - - RuntimeType rt = (RuntimeType)uninitializedObject.GetType(); - RuntimeMethodHandleInternal rmh = RuntimeTypeHandle.GetDefaultConstructor(rt); - IntPtr pfnCtor = RuntimeMethodHandle.GetFunctionPointer(rmh); - ((delegate*)pfnCtor)(ref uninitializedObject!.GetRawData()); - } - _pfnCtor = &CtorUnboxValueTypeStub; - } - else - { - // ValueType with no explicit parameterless ctor; assume ctor returns default(T) - - Debug.Assert(defaultCtorRMH.IsNullHandle()); - } - } - else - { - // Reference type - we can't proceed unless there's a default ctor we can call. - - Debug.Assert(rt.IsClass); - - if (_pMT->IsComObject) - { - // COM RCW types go through the runtime's special allocator, but no ctor - // ever gets run over them. We won't overwrite the _pfnCtorStub field. - - Debug.Assert(!rt.IsGenericCOMObjectImpl(), "__ComObject base class should've been blocked."); - defaultCtorRMH = default; // ignore any parameterless ctor - } - else if (!_pMT->HasDefaultConstructor) - { - throw new MissingMethodException(SR.Format(SR.Arg_NoDefCTor, rt)); - } - else - { - // Reference type with explicit parameterless ctor - - Debug.Assert(!defaultCtorRMH.IsNullHandle()); - } - } - - if (defaultCtorRMH.IsNullHandle()) - { - CtorIsPublic = true; // implicit parameterless ctor is always considered public - } - else - { - CtorIsPublic = (RuntimeMethodHandle.GetAttributes(defaultCtorRMH) & MethodAttributes.Public) != 0; - } - - Debug.Assert(_pfnAllocator != null); - Debug.Assert(_pMT != null); - Debug.Assert(_pfnCtor != null); // we use null singleton pattern if no ctor call is necessary - } - - internal bool CtorIsPublic { get; } - - [MethodImpl(MethodImplOptions.AggressiveInlining)] - internal object? CreateUninitializedObject(RuntimeType rt) - { - // We don't use RuntimeType, but we force the caller to pass it so - // that we can keep it alive on their behalf. Once the object is - // constructed, we no longer need the reference to the type instance, - // as the object itself will keep the type alive. - -#if DEBUG - Debug.Assert(_originalRuntimeType.TryGetTarget(out RuntimeType? originalRT) && originalRT == rt, - "Caller passed the wrong RuntimeType to this routine."); -#endif - - object? retVal = _pfnAllocator(_pMT); - GC.KeepAlive(rt); - return retVal; - } - - [MethodImpl(MethodImplOptions.AggressiveInlining)] - internal void CallConstructor(object? uninitializedObject) => _pfnCtor(uninitializedObject); - } - /// /// Helper to invoke the default (parameterless) constructor. /// diff --git a/src/coreclr/src/vm/ecalllist.h b/src/coreclr/src/vm/ecalllist.h index 17ea5afe1e07b..542198a1d8c11 100644 --- a/src/coreclr/src/vm/ecalllist.h +++ b/src/coreclr/src/vm/ecalllist.h @@ -240,6 +240,7 @@ FCFuncStart(gCOMTypeHandleFuncs) FCFuncElement("SatisfiesConstraints", RuntimeTypeHandle::SatisfiesConstraints) QCFuncElement("GetAllocatorFtn", RuntimeTypeHandle::GetAllocatorFtn) QCFuncElement("GetDefaultCtor", RuntimeTypeHandle::GetDefaultCtor) + QCFuncElement("AllocateComObject", RuntimeTypeHandle::AllocateComObject) FCFuncElement("CompareCanonicalHandles", RuntimeTypeHandle::CompareCanonicalHandles) FCIntrinsic("GetValueInternal", RuntimeTypeHandle::GetValueInternal, CORINFO_INTRINSIC_RTH_GetValueInternal) FCFuncElement("IsEquivalentTo", RuntimeTypeHandle::IsEquivalentTo) diff --git a/src/coreclr/src/vm/reflectioninvocation.cpp b/src/coreclr/src/vm/reflectioninvocation.cpp index 18781566291b8..0198c23cd7970 100644 --- a/src/coreclr/src/vm/reflectioninvocation.cpp +++ b/src/coreclr/src/vm/reflectioninvocation.cpp @@ -2078,12 +2078,9 @@ void QCALLTYPE RuntimeTypeHandle::GetAllocatorFtn( // transparent proxy or the jit will get confused. #ifdef FEATURE_COMINTEROP - // Never allow instantiation of the __ComObject base type, only RCWs. - // In a COM-enabled runitme, getNewHelperStatic will return an RCW-aware allocator. - if (IsComObjectClass(typeHandle)) - { - COMPlusThrow(kNotSupportedException, W("NotSupported_ManagedActivation")); - } + // COM allocation can involve the __ComObject base type (with attached CLSID) or an RCW type. + // We'll optimistically return a reference to the allocator, which will fail if there's not + // a legal CLSID associated with the requested type. #endif // FEATURE_COMINTEROP // If the caller is GetUninitializedInstance, they'll want a boxed T instead of a boxed Nullable. @@ -2140,6 +2137,46 @@ MethodDesc* QCALLTYPE RuntimeTypeHandle::GetDefaultCtor( return pMethodDesc; } +/* + * Given a RuntimeType that represents __ComObject, activates an instance of the + * COM object and returns a RCW around it. Throws if activation fails or if the + * RuntimeType isn't __ComObject. + */ +void QCALLTYPE RuntimeTypeHandle::AllocateComObject( + QCall::ObjectHandleOnStack refRuntimeType, + QCall::ObjectHandleOnStack retInstance) +{ + QCALL_CONTRACT; + + REFLECTCLASSBASEREF refThis = (REFLECTCLASSBASEREF)refRuntimeType.Get(); + bool allocated = false; + + BEGIN_QCALL; + +#ifdef FEATURE_COMINTEROP +#ifdef FEATURE_COMINTEROP_UNMANAGED_ACTIVATION + if (IsComObjectClass(refThis->GetType())) + { + SyncBlock* pSyncBlock = refThis->GetSyncBlock(); + + void* pClassFactory = (void*)pSyncBlock->GetInteropInfo()->GetComClassFactory(); + if (pClassFactory) + { + retInstance.Set(((ComClassFactory*)pClassFactory)->CreateInstance(NULL)); + allocated = true; + } + } +#endif // FEATURE_COMINTEROP_UNMANAGED_ACTIVATION +#endif // FEATURE_COMINTEROP + + if (!allocated) + { + COMPlusThrow(kInvalidComObjectException, IDS_EE_NO_BACKING_CLASS_FACTORY); + } + + END_QCALL; +} + //************************************************************************************************* //************************************************************************************************* //************************************************************************************************* diff --git a/src/coreclr/src/vm/runtimehandles.h b/src/coreclr/src/vm/runtimehandles.h index fe6c6f7db8e86..224a28b69599d 100644 --- a/src/coreclr/src/vm/runtimehandles.h +++ b/src/coreclr/src/vm/runtimehandles.h @@ -122,13 +122,6 @@ class RuntimeTypeHandle { public: // Static method on RuntimeTypeHandle - static FCDECL1(Object*, Allocate, ReflectClassBaseObject *refType) ; //A.CI work - static FCDECL6(Object*, CreateInstance, ReflectClassBaseObject* refThisUNSAFE, - CLR_BOOL publicOnly, - CLR_BOOL wrapExceptions, - CLR_BOOL *pbCanBeCached, - MethodDesc** pConstructor, - CLR_BOOL *pbHasNoDefaultCtor); static void QCALLTYPE GetAllocatorFtn(QCall::TypeHandle pTypeHandle, PCODE* ppNewobjHelper, MethodTable** ppMT, BOOL fGetUninitializedObject); @@ -136,6 +129,9 @@ class RuntimeTypeHandle { static MethodDesc* QCALLTYPE GetDefaultCtor(QCall::TypeHandle pTypeHandle); + static + void QCALLTYPE AllocateComObject(QCall::ObjectHandleOnStack refRuntimeType, QCall::ObjectHandleOnStack retInstance); + static void QCALLTYPE MakeByRef(QCall::TypeHandle pTypeHandle, QCall::ObjectHandleOnStack retType); From adfae42a39a83a25791dfe94b4e9825ed7cdc3fa Mon Sep 17 00:00:00 2001 From: Levi Broderick Date: Thu, 19 Nov 2020 17:48:33 -0800 Subject: [PATCH 07/15] Allow runtime to create unboxing stub on my behalf --- .../src/System/ActivatorCache.CoreCLR.cs | 39 ++++++------------- .../src/System/RuntimeHandles.cs | 10 +++-- src/coreclr/src/vm/methodtable.cpp | 4 +- src/coreclr/src/vm/methodtable.h | 2 +- src/coreclr/src/vm/reflectioninvocation.cpp | 8 ++-- src/coreclr/src/vm/runtimehandles.h | 2 +- 6 files changed, 27 insertions(+), 38 deletions(-) diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/ActivatorCache.CoreCLR.cs b/src/coreclr/src/System.Private.CoreLib/src/System/ActivatorCache.CoreCLR.cs index 9c991fceb057d..25b7010c1b5e0 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/ActivatorCache.CoreCLR.cs +++ b/src/coreclr/src/System.Private.CoreLib/src/System/ActivatorCache.CoreCLR.cs @@ -41,10 +41,7 @@ internal ActivatorCache([DynamicallyAccessedMembers(DynamicallyAccessedMemberTyp _pfnAllocator = (delegate*)RuntimeTypeHandle.GetAllocatorFtn(rt, out MethodTable* pMT, forGetUninitializedObject: false); _allocatorFirstArg = (IntPtr)pMT; - static void CtorNoopStub(object? uninitializedObject) { } - _pfnCtor = &CtorNoopStub; - - RuntimeMethodHandleInternal defaultCtorRMH = RuntimeTypeHandle.GetDefaultConstructor(rt); // could be null + RuntimeMethodHandleInternal ctorHandle = RuntimeMethodHandleInternal.EmptyHandle; // default nullptr if (pMT->IsValueType) { @@ -57,30 +54,14 @@ static void CtorNoopStub(object? uninitializedObject) { } } else if (pMT->HasDefaultConstructor) { - // ValueType with explicit parameterless ctor typed as (ref T) -> void. - // We'll point the ctor at our unboxing stub. It's not terribly efficient, - // but value types almost never have explicit parameterless ctors, so - // this shouldn't be a problem in practice. - - [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2072:UnrecognizedReflectionPattern", - Justification = "The linker requirements are already satisfied by the enclosing method.")] - static void CtorUnboxValueTypeStub(object? uninitializedObject) - { - Debug.Assert(uninitializedObject != null); - Debug.Assert(RuntimeHelpers.GetMethodTable(uninitializedObject)->IsValueType); + // Value type with an explicit default ctor; we'll ask the runtime to create + // an unboxing stub on our behalf. - RuntimeType rt = (RuntimeType)uninitializedObject.GetType(); - RuntimeMethodHandleInternal rmh = RuntimeTypeHandle.GetDefaultConstructor(rt); - IntPtr pfnCtor = RuntimeMethodHandle.GetFunctionPointer(rmh); - ((delegate*)pfnCtor)(ref uninitializedObject!.GetRawData()); - } - _pfnCtor = &CtorUnboxValueTypeStub; + ctorHandle = RuntimeTypeHandle.GetDefaultConstructor(rt, forceBoxedEntryPoint: true); } else { // ValueType with no explicit parameterless ctor; assume ctor returns default(T) - - Debug.Assert(defaultCtorRMH.IsNullHandle()); } } else @@ -114,7 +95,7 @@ static object AllocateComObject(IntPtr runtimeTypeHandle) // Neither __ComObject nor any derived type gets its parameterless ctor called. // Activation is handled entirely by the allocator. - defaultCtorRMH = default; + ctorHandle = default; } else if (!pMT->HasDefaultConstructor) { @@ -124,17 +105,21 @@ static object AllocateComObject(IntPtr runtimeTypeHandle) { // Reference type with explicit parameterless ctor - Debug.Assert(!defaultCtorRMH.IsNullHandle()); + ctorHandle = RuntimeTypeHandle.GetDefaultConstructor(rt, forceBoxedEntryPoint: false); + Debug.Assert(!ctorHandle.IsNullHandle()); } } - if (defaultCtorRMH.IsNullHandle()) + if (ctorHandle.IsNullHandle()) { + static void CtorNoopStub(object? uninitializedObject) { } + _pfnCtor = &CtorNoopStub; // we use null singleton pattern if no ctor call is necessary CtorIsPublic = true; // implicit parameterless ctor is always considered public } else { - CtorIsPublic = (RuntimeMethodHandle.GetAttributes(defaultCtorRMH) & MethodAttributes.Public) != 0; + _pfnCtor = (delegate*)RuntimeMethodHandle.GetFunctionPointer(ctorHandle); + CtorIsPublic = (RuntimeMethodHandle.GetAttributes(ctorHandle) & MethodAttributes.Public) != 0; } Debug.Assert(_pfnAllocator != null); diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeHandles.cs b/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeHandles.cs index c6b4e595668cc..01b51b0cb3c60 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeHandles.cs +++ b/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeHandles.cs @@ -237,19 +237,21 @@ internal static bool HasElementType(RuntimeType type) /// /// Returns the MethodDesc* for this type's parameterless instance ctor. /// For reference types, signature is (object @this) -> void. - /// For value types, signature is (ref T @thisUnboxed) -> void. + /// For value types, unboxed signature is (ref T @thisUnboxed) -> void. + /// For value types, forced boxed signature is (object @this) -> void. /// Returns nullptr if no parameterless ctor is defined. /// internal static RuntimeMethodHandleInternal GetDefaultConstructor( - [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)] RuntimeType type) + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)] RuntimeType type, + bool forceBoxedEntryPoint) { Debug.Assert(type != null); - return GetDefaultCtor(new QCallTypeHandle(ref type)); + return GetDefaultCtor(new QCallTypeHandle(ref type), (forceBoxedEntryPoint) ? Interop.BOOL.TRUE : Interop.BOOL.FALSE); } [DllImport(RuntimeHelpers.QCall, CharSet = CharSet.Unicode)] - private static extern RuntimeMethodHandleInternal GetDefaultCtor(QCallTypeHandle typeHandle); + private static extern RuntimeMethodHandleInternal GetDefaultCtor(QCallTypeHandle typeHandle, Interop.BOOL forceBoxedEntryPoint); /// /// Given a RuntimeType which represents __ComObject, activates the class and creates diff --git a/src/coreclr/src/vm/methodtable.cpp b/src/coreclr/src/vm/methodtable.cpp index 845c655cef9df..5c600590f8f75 100644 --- a/src/coreclr/src/vm/methodtable.cpp +++ b/src/coreclr/src/vm/methodtable.cpp @@ -9150,7 +9150,7 @@ BOOL MethodTable::HasExplicitOrImplicitPublicDefaultConstructor() } //========================================================================================== -MethodDesc *MethodTable::GetDefaultConstructor() +MethodDesc *MethodTable::GetDefaultConstructor(BOOL forceBoxedEntryPoint) { WRAPPER_NO_CONTRACT; _ASSERTE(HasDefaultConstructor()); @@ -9161,7 +9161,7 @@ MethodDesc *MethodTable::GetDefaultConstructor() // returns pCanonMD immediately. return MethodDesc::FindOrCreateAssociatedMethodDesc(pCanonMD, this, - FALSE /* no BoxedEntryPointStub */, + forceBoxedEntryPoint, Instantiation(), /* no method instantiation */ FALSE /* no allowInstParam */); } diff --git a/src/coreclr/src/vm/methodtable.h b/src/coreclr/src/vm/methodtable.h index 850a13d388935..4f05c6a1868c2 100644 --- a/src/coreclr/src/vm/methodtable.h +++ b/src/coreclr/src/vm/methodtable.h @@ -823,7 +823,7 @@ class MethodTable BOOL HasDefaultConstructor(); void SetHasDefaultConstructor(); WORD GetDefaultConstructorSlot(); - MethodDesc *GetDefaultConstructor(); + MethodDesc *GetDefaultConstructor(BOOL forceBoxedEntryPoint = FALSE); BOOL HasExplicitOrImplicitPublicDefaultConstructor(); diff --git a/src/coreclr/src/vm/reflectioninvocation.cpp b/src/coreclr/src/vm/reflectioninvocation.cpp index 0198c23cd7970..b2dbfb30841f6 100644 --- a/src/coreclr/src/vm/reflectioninvocation.cpp +++ b/src/coreclr/src/vm/reflectioninvocation.cpp @@ -2110,11 +2110,13 @@ void QCALLTYPE RuntimeTypeHandle::GetAllocatorFtn( * Given a TypeHandle, returns the MethodDesc* for the default (parameterless) ctor, * or nullptr if the parameterless ctor doesn't exist. For reference types, the parameterless * ctor has a managed (object @this) -> void calling convention. For value types, - * the parameterless ctor has a managed (ref T @this) -> void calling convention. + * the parameterless ctor has a managed (ref T @this) -> void calling convention, unless + * fForceBoxedEntryPoint is set, then managed (object @this) -> void stub is returned. * The returned MethodDesc* is appropriately instantiated over any necessary generic args. */ MethodDesc* QCALLTYPE RuntimeTypeHandle::GetDefaultCtor( - QCall::TypeHandle pTypeHandle) + QCall::TypeHandle pTypeHandle, + BOOL fForceBoxedEntryPoint) { QCALL_CONTRACT; @@ -2129,7 +2131,7 @@ MethodDesc* QCALLTYPE RuntimeTypeHandle::GetDefaultCtor( pMT->EnsureInstanceActive(); if (pMT->HasDefaultConstructor()) { - pMethodDesc = pMT->GetDefaultConstructor(); + pMethodDesc = pMT->GetDefaultConstructor(fForceBoxedEntryPoint); } END_QCALL; diff --git a/src/coreclr/src/vm/runtimehandles.h b/src/coreclr/src/vm/runtimehandles.h index 224a28b69599d..609615b7edd64 100644 --- a/src/coreclr/src/vm/runtimehandles.h +++ b/src/coreclr/src/vm/runtimehandles.h @@ -127,7 +127,7 @@ class RuntimeTypeHandle { void QCALLTYPE GetAllocatorFtn(QCall::TypeHandle pTypeHandle, PCODE* ppNewobjHelper, MethodTable** ppMT, BOOL fGetUninitializedObject); static - MethodDesc* QCALLTYPE GetDefaultCtor(QCall::TypeHandle pTypeHandle); + MethodDesc* QCALLTYPE GetDefaultCtor(QCall::TypeHandle pTypeHandle, BOOL fForceBoxedEntryPoint); static void QCALLTYPE AllocateComObject(QCall::ObjectHandleOnStack refRuntimeType, QCall::ObjectHandleOnStack retInstance); From 49a8de11a0ffd1caf9924e0c722e80b3f9acb3db Mon Sep 17 00:00:00 2001 From: Levi Broderick Date: Thu, 19 Nov 2020 18:01:54 -0800 Subject: [PATCH 08/15] Fix linker warning --- .../System.Private.CoreLib/src/System/ActivatorCache.CoreCLR.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/ActivatorCache.CoreCLR.cs b/src/coreclr/src/System.Private.CoreLib/src/System/ActivatorCache.CoreCLR.cs index 25b7010c1b5e0..7b0a6a0fda8ea 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/ActivatorCache.CoreCLR.cs +++ b/src/coreclr/src/System.Private.CoreLib/src/System/ActivatorCache.CoreCLR.cs @@ -81,6 +81,8 @@ internal ActivatorCache([DynamicallyAccessedMembers(DynamicallyAccessedMemberTyp // handles we create live for the lifetime of the app, but that's ok since it // matches coreclr's internal implementation anyway (see GetComClassHelper). + [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2072:UnrecognizedReflectionPattern", + Justification = "Linker already saw this type through Activator/Type.CreateInstance.")] static object AllocateComObject(IntPtr runtimeTypeHandle) { RuntimeType rt = (RuntimeType)GCHandle.FromIntPtr(runtimeTypeHandle).Target!; From 98c7941d0b1078b8bdb5506a4c318eae89335772 Mon Sep 17 00:00:00 2001 From: Levi Broderick Date: Thu, 19 Nov 2020 18:12:17 -0800 Subject: [PATCH 09/15] Update error conditions --- src/coreclr/src/vm/reflectioninvocation.cpp | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/src/coreclr/src/vm/reflectioninvocation.cpp b/src/coreclr/src/vm/reflectioninvocation.cpp index b2dbfb30841f6..36991860a02c0 100644 --- a/src/coreclr/src/vm/reflectioninvocation.cpp +++ b/src/coreclr/src/vm/reflectioninvocation.cpp @@ -2006,7 +2006,7 @@ FCIMPLEND * a boxed zero-inited instance of the value type. The "fGetUninitializedObject" * parameter dictates whether the caller is RuntimeHelpers.GetUninitializedObject or * Activator.CreateInstance, which have different behavior w.r.t. what exceptions are - * thrown on failure and how nullables are handled. + * thrown on failure and how nullables and COM types are handled. */ void QCALLTYPE RuntimeTypeHandle::GetAllocatorFtn( QCall::TypeHandle pTypeHandle, @@ -2058,12 +2058,16 @@ void QCALLTYPE RuntimeTypeHandle::GetAllocatorFtn( // Don't allow abstract classes or interface types if (pMT->IsAbstract()) { - COMPlusThrow(kMemberAccessException, W("Acc_CreateAbst")); + RuntimeExceptionKind exKind = fGetUninitializedObject ? kMemberAccessException : kMissingMethodException; + if (pMT->IsInterface()) + COMPlusThrow(exKind, W("Acc_CreateInterface")); + else + COMPlusThrow(exKind, W("Acc_CreateAbst")); } // Don't allow open generics or generics instantiated over __Canon if (pMT->ContainsGenericVariables()) { - COMPlusThrow(kMemberAccessException, W("Acc_CreateGeneric")); + COMPlusThrow(fGetUninitializedObject ? kMemberAccessException : kArgumentException, W("Acc_CreateGeneric")); } if (pMT->IsSharedByGenericInstantiations()) { COMPlusThrow(kNotSupportedException, W("NotSupported_Type")); @@ -2079,8 +2083,14 @@ void QCALLTYPE RuntimeTypeHandle::GetAllocatorFtn( #ifdef FEATURE_COMINTEROP // COM allocation can involve the __ComObject base type (with attached CLSID) or an RCW type. - // We'll optimistically return a reference to the allocator, which will fail if there's not - // a legal CLSID associated with the requested type. + // For Activator.CreateInstance, we'll optimistically return a reference to the allocator, + // which will fail if there's not a legal CLSID associated with the requested type. + // For __ComObject, Activator.CreateInstance will special-case this and replace it with a stub. + // RuntimeHelpers.GetUninitializedObject always fails when it sees COM objects. + if (fGetUninitializedObject && pMT->IsComObjectType()) + { + COMPlusThrow(kNotSupportedException, W("NotSupported_ManagedActivation")); + } #endif // FEATURE_COMINTEROP // If the caller is GetUninitializedInstance, they'll want a boxed T instead of a boxed Nullable. From 0a443c2d43403c9a0135a992ee3f7411565ff5ea Mon Sep 17 00:00:00 2001 From: Levi Broderick Date: Thu, 19 Nov 2020 19:05:06 -0800 Subject: [PATCH 10/15] Friendlier exception messages when activation fails --- .../src/System/ActivatorCache.CoreCLR.cs | 39 ++++++++++++- src/coreclr/src/vm/reflectioninvocation.cpp | 10 ++-- .../src/Resources/Strings.resx | 57 ++++++++++--------- 3 files changed, 73 insertions(+), 33 deletions(-) diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/ActivatorCache.CoreCLR.cs b/src/coreclr/src/System.Private.CoreLib/src/System/ActivatorCache.CoreCLR.cs index 7b0a6a0fda8ea..ce712394bfb19 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/ActivatorCache.CoreCLR.cs +++ b/src/coreclr/src/System.Private.CoreLib/src/System/ActivatorCache.CoreCLR.cs @@ -38,7 +38,19 @@ internal ActivatorCache([DynamicallyAccessedMembers(DynamicallyAccessedMemberTyp _originalRuntimeType = new WeakReference(rt); #endif - _pfnAllocator = (delegate*)RuntimeTypeHandle.GetAllocatorFtn(rt, out MethodTable* pMT, forGetUninitializedObject: false); + MethodTable* pMT; + + try + { + _pfnAllocator = (delegate*)RuntimeTypeHandle.GetAllocatorFtn(rt, out pMT, forGetUninitializedObject: false); + } + catch (Exception ex) + { + Exception? friendlyException = CreateAllocatorLookupFailedException(rt, ex); + if (friendlyException != null) { throw friendlyException; } + throw; // throw original exception if we couldn't create a friendly one + } + _allocatorFirstArg = (IntPtr)pMT; RuntimeMethodHandleInternal ctorHandle = RuntimeMethodHandleInternal.EmptyHandle; // default nullptr @@ -151,6 +163,31 @@ static void CtorNoopStub(object? uninitializedObject) { } [MethodImpl(MethodImplOptions.AggressiveInlining)] internal void CallConstructor(object? uninitializedObject) => _pfnCtor(uninitializedObject); + + [StackTraceHidden] + private static Exception? CreateAllocatorLookupFailedException(RuntimeType rt, Exception ex) + { + // If the inner exception is populated, probably a failed cctor, and we should + // propagate the exception as-is. If the exception is a type that we understand, + // we should make the error message friendlier so as to include the name of the type + // that couldn't be instantiated. + + if (ex.InnerException is null) + { + string friendlyMessage = SR.Format(SR.ActivatorCache_CannotGetAllocator, rt, ex.Message); + + switch (ex) + { + case ArgumentException: return new ArgumentException(friendlyMessage); + case NotSupportedException: return new NotSupportedException(friendlyMessage); + case MethodAccessException: return new MethodAccessException(friendlyMessage); + case MissingMethodException: return new MissingMethodException(friendlyMessage); + case MemberAccessException: return new MemberAccessException(friendlyMessage); + } + } + + return null; // caller should rethrow + } } } } diff --git a/src/coreclr/src/vm/reflectioninvocation.cpp b/src/coreclr/src/vm/reflectioninvocation.cpp index 36991860a02c0..e42765e965301 100644 --- a/src/coreclr/src/vm/reflectioninvocation.cpp +++ b/src/coreclr/src/vm/reflectioninvocation.cpp @@ -2030,13 +2030,13 @@ void QCALLTYPE RuntimeTypeHandle::GetAllocatorFtn( // Don't allow void if (typeHandle.GetSignatureCorElementType() == ELEMENT_TYPE_VOID) { - COMPlusThrow(kArgumentException, W("Argument_InvalidValue")); + COMPlusThrow(kArgumentException, W("NotSupported_Type")); } // Don't allow arrays, pointers, byrefs, or function pointers if (typeHandle.IsTypeDesc() || typeHandle.IsArray()) { - COMPlusThrow(kArgumentException, W("Argument_InvalidValue")); + COMPlusThrow(kNotSupportedException, W("NotSupported_Type")); } MethodTable* pMT = typeHandle.AsMethodTable(); @@ -2044,10 +2044,10 @@ void QCALLTYPE RuntimeTypeHandle::GetAllocatorFtn( pMT->EnsureInstanceActive(); - // Don't allow creating instances of void or delegates - if (pMT == CoreLibBinder::GetElementType(ELEMENT_TYPE_VOID) || pMT->IsDelegate()) + // Don't allow creating instances of delegates + if (pMT->IsDelegate()) { - COMPlusThrow(kArgumentException, W("Argument_InvalidValue")); + COMPlusThrow(kArgumentException, W("NotSupported_Type")); } // Don't allow string or string-like (variable length) types. diff --git a/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx b/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx index f3a82588fc95c..ae5144f7abed5 100644 --- a/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx +++ b/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx @@ -1,17 +1,17 @@  - @@ -3775,4 +3775,7 @@ CodeBase is not supported on assemblies loaded from a single-file bundle. + + Cannot dynamically create an instance of type '{0}'. Reason: {1} + From 3b34ae6c22fbf7263dff141b229626d8325b2eee Mon Sep 17 00:00:00 2001 From: Levi Broderick Date: Thu, 19 Nov 2020 19:11:29 -0800 Subject: [PATCH 11/15] Cleanup usings --- .../src/System/Activator.RuntimeType.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Activator.RuntimeType.cs b/src/libraries/System.Private.CoreLib/src/System/Activator.RuntimeType.cs index 1008e9e186720..7607ec2ea838e 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Activator.RuntimeType.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Activator.RuntimeType.cs @@ -2,13 +2,11 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Diagnostics.CodeAnalysis; -using System.Reflection; using System.Globalization; -using System.Runtime.CompilerServices; +using System.Reflection; using System.Runtime.Loader; using System.Runtime.Remoting; using System.Threading; -using System.Diagnostics; namespace System { From 6e1b7cdedbdd0f2a9f32a0e6376d0c0d4cd216a2 Mon Sep 17 00:00:00 2001 From: Levi Broderick Date: Thu, 19 Nov 2020 21:39:53 -0800 Subject: [PATCH 12/15] Move friendly exception messages to common place --- .../src/System/ActivatorCache.CoreCLR.cs | 39 +------------------ .../src/System/RuntimeHandles.cs | 36 ++++++++++++++--- src/coreclr/src/vm/reflectioninvocation.cpp | 21 +++++++--- 3 files changed, 48 insertions(+), 48 deletions(-) diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/ActivatorCache.CoreCLR.cs b/src/coreclr/src/System.Private.CoreLib/src/System/ActivatorCache.CoreCLR.cs index ce712394bfb19..7b0a6a0fda8ea 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/ActivatorCache.CoreCLR.cs +++ b/src/coreclr/src/System.Private.CoreLib/src/System/ActivatorCache.CoreCLR.cs @@ -38,19 +38,7 @@ internal ActivatorCache([DynamicallyAccessedMembers(DynamicallyAccessedMemberTyp _originalRuntimeType = new WeakReference(rt); #endif - MethodTable* pMT; - - try - { - _pfnAllocator = (delegate*)RuntimeTypeHandle.GetAllocatorFtn(rt, out pMT, forGetUninitializedObject: false); - } - catch (Exception ex) - { - Exception? friendlyException = CreateAllocatorLookupFailedException(rt, ex); - if (friendlyException != null) { throw friendlyException; } - throw; // throw original exception if we couldn't create a friendly one - } - + _pfnAllocator = (delegate*)RuntimeTypeHandle.GetAllocatorFtn(rt, out MethodTable* pMT, forGetUninitializedObject: false); _allocatorFirstArg = (IntPtr)pMT; RuntimeMethodHandleInternal ctorHandle = RuntimeMethodHandleInternal.EmptyHandle; // default nullptr @@ -163,31 +151,6 @@ static void CtorNoopStub(object? uninitializedObject) { } [MethodImpl(MethodImplOptions.AggressiveInlining)] internal void CallConstructor(object? uninitializedObject) => _pfnCtor(uninitializedObject); - - [StackTraceHidden] - private static Exception? CreateAllocatorLookupFailedException(RuntimeType rt, Exception ex) - { - // If the inner exception is populated, probably a failed cctor, and we should - // propagate the exception as-is. If the exception is a type that we understand, - // we should make the error message friendlier so as to include the name of the type - // that couldn't be instantiated. - - if (ex.InnerException is null) - { - string friendlyMessage = SR.Format(SR.ActivatorCache_CannotGetAllocator, rt, ex.Message); - - switch (ex) - { - case ArgumentException: return new ArgumentException(friendlyMessage); - case NotSupportedException: return new NotSupportedException(friendlyMessage); - case MethodAccessException: return new MethodAccessException(friendlyMessage); - case MissingMethodException: return new MissingMethodException(friendlyMessage); - case MemberAccessException: return new MemberAccessException(friendlyMessage); - } - } - - return null; // caller should rethrow - } } } } diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeHandles.cs b/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeHandles.cs index 01b51b0cb3c60..7faff86d881ec 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeHandles.cs +++ b/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeHandles.cs @@ -221,11 +221,37 @@ internal static bool HasElementType(RuntimeType type) delegate* pNewobjHelperTemp = null; MethodTable* pMTTemp = null; - GetAllocatorFtn( - new QCallTypeHandle(ref type), - &pNewobjHelperTemp, - &pMTTemp, - forGetUninitializedObject ? Interop.BOOL.TRUE : Interop.BOOL.FALSE); + try + { + GetAllocatorFtn( + new QCallTypeHandle(ref type), + &pNewobjHelperTemp, + &pMTTemp, + forGetUninitializedObject ? Interop.BOOL.TRUE : Interop.BOOL.FALSE); + } + catch (Exception ex) + { + // If the inner exception is populated, probably a failed cctor, and we should + // propagate the exception as-is. If the exception is a type that we understand, + // we should make the error message friendlier so as to include the name of the type + // that couldn't be instantiated. + + if (ex.InnerException is null) + { + string friendlyMessage = SR.Format(SR.ActivatorCache_CannotGetAllocator, type, ex.Message); + + switch (ex) + { + case ArgumentException: throw new ArgumentException(friendlyMessage); + case NotSupportedException: throw new NotSupportedException(friendlyMessage); + case MethodAccessException: throw new MethodAccessException(friendlyMessage); + case MissingMethodException: throw new MissingMethodException(friendlyMessage); + case MemberAccessException: throw new MemberAccessException(friendlyMessage); + } + } + + throw; // can't make a friendlier message, rethrow original exception + } pMT = pMTTemp; return pNewobjHelperTemp; diff --git a/src/coreclr/src/vm/reflectioninvocation.cpp b/src/coreclr/src/vm/reflectioninvocation.cpp index e42765e965301..f3b41d1314422 100644 --- a/src/coreclr/src/vm/reflectioninvocation.cpp +++ b/src/coreclr/src/vm/reflectioninvocation.cpp @@ -2033,10 +2033,17 @@ void QCALLTYPE RuntimeTypeHandle::GetAllocatorFtn( COMPlusThrow(kArgumentException, W("NotSupported_Type")); } + // Don't allow generic variables (e.g., the 'T' from List) + // or open generic types (List<>) - Activator variation of check. + if (!fGetUninitializedObject && typeHandle.ContainsGenericVariables()) + { + COMPlusThrow(kArgumentException, W("Acc_CreateGeneric")); + } + // Don't allow arrays, pointers, byrefs, or function pointers if (typeHandle.IsTypeDesc() || typeHandle.IsArray()) { - COMPlusThrow(kNotSupportedException, W("NotSupported_Type")); + COMPlusThrow(fGetUninitializedObject ? kArgumentException : kMissingMethodException, W("NotSupported_Type")); } MethodTable* pMT = typeHandle.AsMethodTable(); @@ -2053,7 +2060,7 @@ void QCALLTYPE RuntimeTypeHandle::GetAllocatorFtn( // Don't allow string or string-like (variable length) types. if (pMT->HasComponentSize()) { - COMPlusThrow(kArgumentException, W("Argument_NoUninitializedStrings")); + COMPlusThrow(fGetUninitializedObject ? kArgumentException : kMissingMethodException, W("Argument_NoUninitializedStrings")); } // Don't allow abstract classes or interface types @@ -2065,10 +2072,14 @@ void QCALLTYPE RuntimeTypeHandle::GetAllocatorFtn( COMPlusThrow(exKind, W("Acc_CreateAbst")); } - // Don't allow open generics or generics instantiated over __Canon - if (pMT->ContainsGenericVariables()) { - COMPlusThrow(fGetUninitializedObject ? kMemberAccessException : kArgumentException, W("Acc_CreateGeneric")); + // Don't allow generic variables (e.g., the 'T' from List) + // or open generic types (List<>) - FormatterServices variation of check. + if (fGetUninitializedObject && typeHandle.ContainsGenericVariables()) + { + COMPlusThrow(kMemberAccessException, W("Acc_CreateGeneric")); } + + // Don't allow generics instantiated over __Canon if (pMT->IsSharedByGenericInstantiations()) { COMPlusThrow(kNotSupportedException, W("NotSupported_Type")); } From b588d5d62cbdd777009c0138747f1a1b00e6cb1e Mon Sep 17 00:00:00 2001 From: Levi Broderick Date: Thu, 19 Nov 2020 23:14:00 -0800 Subject: [PATCH 13/15] Fix TIE wrappers --- .../src/System/ActivatorCache.CoreCLR.cs | 6 ++- .../RuntimeHelpers.CoreCLR.cs | 2 +- .../src/System/RuntimeHandles.cs | 39 ++++++++++--------- .../src/System/RuntimeType.CoreCLR.cs | 2 +- src/coreclr/src/vm/reflectioninvocation.cpp | 7 +++- src/coreclr/src/vm/runtimehandles.h | 2 +- 6 files changed, 34 insertions(+), 24 deletions(-) diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/ActivatorCache.CoreCLR.cs b/src/coreclr/src/System.Private.CoreLib/src/System/ActivatorCache.CoreCLR.cs index 7b0a6a0fda8ea..62582b28200ad 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/ActivatorCache.CoreCLR.cs +++ b/src/coreclr/src/System.Private.CoreLib/src/System/ActivatorCache.CoreCLR.cs @@ -30,7 +30,9 @@ private sealed unsafe class ActivatorCache private readonly WeakReference _originalRuntimeType; // don't prevent the RT from being collected #endif - internal ActivatorCache([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)] RuntimeType rt) + internal ActivatorCache( + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)] RuntimeType rt, + bool wrapExceptions) { Debug.Assert(rt != null); @@ -38,7 +40,7 @@ internal ActivatorCache([DynamicallyAccessedMembers(DynamicallyAccessedMemberTyp _originalRuntimeType = new WeakReference(rt); #endif - _pfnAllocator = (delegate*)RuntimeTypeHandle.GetAllocatorFtn(rt, out MethodTable* pMT, forGetUninitializedObject: false); + _pfnAllocator = (delegate*)RuntimeTypeHandle.GetAllocatorFtn(rt, out MethodTable* pMT, forGetUninitializedObject: false, wrapExceptions); _allocatorFirstArg = (IntPtr)pMT; RuntimeMethodHandleInternal ctorHandle = RuntimeMethodHandleInternal.EmptyHandle; // default nullptr diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs b/src/coreclr/src/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs index 0526bd8028da4..b74fd6f0c4050 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs +++ b/src/coreclr/src/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs @@ -144,7 +144,7 @@ private static unsafe object GetUninitializedObjectInternal( Debug.Assert(rt != null); // If type is Nullable, returns the allocator and MethodTable* for the underlying T. - delegate* pfnAllocator = RuntimeTypeHandle.GetAllocatorFtn(rt, out MethodTable* pMT, forGetUninitializedObject: true); + delegate* pfnAllocator = RuntimeTypeHandle.GetAllocatorFtn(rt, out MethodTable* pMT, forGetUninitializedObject: true, wrapExceptions: false); Debug.Assert(pfnAllocator != null); Debug.Assert(pMT != null); Debug.Assert(!pMT->IsNullable, "Should've unwrapped any Nullable input."); diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeHandles.cs b/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeHandles.cs index 7faff86d881ec..7903d79a09e3c 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeHandles.cs +++ b/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeHandles.cs @@ -214,12 +214,13 @@ internal static bool HasElementType(RuntimeType type) // constructor are an academic problem. Valuetypes with no constructors are a problem, // but IL Linker currently treats them as always implicitly boxed. [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)] RuntimeType type, - out MethodTable* pMT, bool forGetUninitializedObject) + out MethodTable* pMT, bool forGetUninitializedObject, bool wrapExceptions) { Debug.Assert(type != null); delegate* pNewobjHelperTemp = null; MethodTable* pMTTemp = null; + Interop.BOOL fFailedWhileRunningCctor = Interop.BOOL.FALSE; try { @@ -227,27 +228,29 @@ internal static bool HasElementType(RuntimeType type) new QCallTypeHandle(ref type), &pNewobjHelperTemp, &pMTTemp, - forGetUninitializedObject ? Interop.BOOL.TRUE : Interop.BOOL.FALSE); + forGetUninitializedObject ? Interop.BOOL.TRUE : Interop.BOOL.FALSE, + &fFailedWhileRunningCctor); } catch (Exception ex) { - // If the inner exception is populated, probably a failed cctor, and we should - // propagate the exception as-is. If the exception is a type that we understand, - // we should make the error message friendlier so as to include the name of the type - // that couldn't be instantiated. + // If the cctor failed, propagate the exception as-is, wrapping in a TIE + // if needed. Otherwise, make the error message friendlier by including + // the name of the type that couldn't be instantiated. - if (ex.InnerException is null) + if (fFailedWhileRunningCctor != Interop.BOOL.FALSE) { - string friendlyMessage = SR.Format(SR.ActivatorCache_CannotGetAllocator, type, ex.Message); - - switch (ex) - { - case ArgumentException: throw new ArgumentException(friendlyMessage); - case NotSupportedException: throw new NotSupportedException(friendlyMessage); - case MethodAccessException: throw new MethodAccessException(friendlyMessage); - case MissingMethodException: throw new MissingMethodException(friendlyMessage); - case MemberAccessException: throw new MemberAccessException(friendlyMessage); - } + if (wrapExceptions) throw new TargetInvocationException(ex); + else throw; // rethrow original, no TIE + } + + string friendlyMessage = SR.Format(SR.ActivatorCache_CannotGetAllocator, type, ex.Message); + switch (ex) + { + case ArgumentException: throw new ArgumentException(friendlyMessage); + case NotSupportedException: throw new NotSupportedException(friendlyMessage); + case MethodAccessException: throw new MethodAccessException(friendlyMessage); + case MissingMethodException: throw new MissingMethodException(friendlyMessage); + case MemberAccessException: throw new MemberAccessException(friendlyMessage); } throw; // can't make a friendlier message, rethrow original exception @@ -258,7 +261,7 @@ internal static bool HasElementType(RuntimeType type) } [DllImport(RuntimeHelpers.QCall, CharSet = CharSet.Unicode)] - private static extern void GetAllocatorFtn(QCallTypeHandle typeHandle, delegate** ppNewobjHelper, MethodTable** ppMT, Interop.BOOL fGetUninitializedObject); + private static extern void GetAllocatorFtn(QCallTypeHandle typeHandle, delegate** ppNewobjHelper, MethodTable** ppMT, Interop.BOOL fGetUninitializedObject, Interop.BOOL* pfFailedWhileRunningCctor); /// /// Returns the MethodDesc* for this type's parameterless instance ctor. diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs b/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs index 8545a205015b1..3da5023d8215e 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs +++ b/src/coreclr/src/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs @@ -3966,7 +3966,7 @@ private void CreateInstanceCheckThis() if (GenericCache is not ActivatorCache cache) { - cache = new ActivatorCache(this); + cache = new ActivatorCache(this, wrapExceptions); GenericCache = cache; } diff --git a/src/coreclr/src/vm/reflectioninvocation.cpp b/src/coreclr/src/vm/reflectioninvocation.cpp index f3b41d1314422..82cf331b1dddc 100644 --- a/src/coreclr/src/vm/reflectioninvocation.cpp +++ b/src/coreclr/src/vm/reflectioninvocation.cpp @@ -2012,14 +2012,17 @@ void QCALLTYPE RuntimeTypeHandle::GetAllocatorFtn( QCall::TypeHandle pTypeHandle, PCODE* ppNewobjHelper, MethodTable** ppMT, - BOOL fGetUninitializedObject) + BOOL fGetUninitializedObject, + BOOL* pfFailedWhileRunningCctor) { CONTRACTL{ QCALL_CHECK; PRECONDITION(CheckPointer(ppNewobjHelper)); PRECONDITION(CheckPointer(ppMT)); + PRECONDITION(CheckPointer(pfFailedWhileRunningCctor)); PRECONDITION(*ppNewobjHelper == NULL); PRECONDITION(*ppMT == NULL); + PRECONDITION(!*pfFailedWhileRunningCctor); } CONTRACTL_END; @@ -2114,7 +2117,9 @@ void QCALLTYPE RuntimeTypeHandle::GetAllocatorFtn( // Run the type's cctor if needed (if not marked beforefieldinit) if (pMT->HasPreciseInitCctors()) { + *pfFailedWhileRunningCctor = TRUE; pMT->CheckRunClassInitAsIfConstructingThrowing(); + *pfFailedWhileRunningCctor = FALSE; } // And we're done! diff --git a/src/coreclr/src/vm/runtimehandles.h b/src/coreclr/src/vm/runtimehandles.h index 609615b7edd64..1bcecf8154e64 100644 --- a/src/coreclr/src/vm/runtimehandles.h +++ b/src/coreclr/src/vm/runtimehandles.h @@ -124,7 +124,7 @@ class RuntimeTypeHandle { // Static method on RuntimeTypeHandle static - void QCALLTYPE GetAllocatorFtn(QCall::TypeHandle pTypeHandle, PCODE* ppNewobjHelper, MethodTable** ppMT, BOOL fGetUninitializedObject); + void QCALLTYPE GetAllocatorFtn(QCall::TypeHandle pTypeHandle, PCODE* ppNewobjHelper, MethodTable** ppMT, BOOL fGetUninitializedObject, BOOL* pfFailedWhileRunningCctor); static MethodDesc* QCALLTYPE GetDefaultCtor(QCall::TypeHandle pTypeHandle, BOOL fForceBoxedEntryPoint); From 55aa1289b3acb969238a1304926b80d5d331e27d Mon Sep 17 00:00:00 2001 From: Levi Broderick Date: Thu, 19 Nov 2020 23:29:18 -0800 Subject: [PATCH 14/15] Update unit tests --- .../CompilerServices/RuntimeHelpers.CoreCLR.cs | 2 +- src/coreclr/src/vm/reflectioninvocation.cpp | 2 +- .../System.Runtime/tests/System/ActivatorTests.cs | 12 ++++++------ .../Runtime/CompilerServices/RuntimeHelpersTests.cs | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs b/src/coreclr/src/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs index b74fd6f0c4050..fed423b44ec6d 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs +++ b/src/coreclr/src/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs @@ -140,8 +140,8 @@ private static unsafe object GetUninitializedObjectInternal( [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)] Type type) { + Debug.Assert(type != null); RuntimeType rt = (RuntimeType)type; - Debug.Assert(rt != null); // If type is Nullable, returns the allocator and MethodTable* for the underlying T. delegate* pfnAllocator = RuntimeTypeHandle.GetAllocatorFtn(rt, out MethodTable* pMT, forGetUninitializedObject: true, wrapExceptions: false); diff --git a/src/coreclr/src/vm/reflectioninvocation.cpp b/src/coreclr/src/vm/reflectioninvocation.cpp index 82cf331b1dddc..e2b27cea88cf0 100644 --- a/src/coreclr/src/vm/reflectioninvocation.cpp +++ b/src/coreclr/src/vm/reflectioninvocation.cpp @@ -2033,7 +2033,7 @@ void QCALLTYPE RuntimeTypeHandle::GetAllocatorFtn( // Don't allow void if (typeHandle.GetSignatureCorElementType() == ELEMENT_TYPE_VOID) { - COMPlusThrow(kArgumentException, W("NotSupported_Type")); + COMPlusThrow(kNotSupportedException, W("NotSupported_Type")); } // Don't allow generic variables (e.g., the 'T' from List) diff --git a/src/libraries/System.Runtime/tests/System/ActivatorTests.cs b/src/libraries/System.Runtime/tests/System/ActivatorTests.cs index 05e07447a9616..e497b64291bb2 100644 --- a/src/libraries/System.Runtime/tests/System/ActivatorTests.cs +++ b/src/libraries/System.Runtime/tests/System/ActivatorTests.cs @@ -150,17 +150,17 @@ public void CreateInstance_ContainsGenericParameters_ThrowsArgumentException(Typ public static IEnumerable CreateInstance_InvalidType_TestData() { - yield return new object[] { typeof(void) }; - yield return new object[] { typeof(void).MakeArrayType() }; - yield return new object[] { typeof(ArgIterator) }; + yield return new object[] { typeof(void), typeof(NotSupportedException) }; + yield return new object[] { typeof(void).MakeArrayType(), typeof(MissingMethodException) }; + yield return new object[] { typeof(ArgIterator), typeof(NotSupportedException) }; } [Theory] [MemberData(nameof(CreateInstance_InvalidType_TestData))] - public void CreateInstance_InvalidType_ThrowsNotSupportedException(Type type) + public void CreateInstance_InvalidType_ThrowsNotSupportedException(Type typeToActivate, Type expectedExceptionType) { - Assert.Throws(() => Activator.CreateInstance(type)); - Assert.Throws(() => Activator.CreateInstance(type, new object[0])); + Assert.Throws(expectedExceptionType, () => Activator.CreateInstance(typeToActivate)); + Assert.Throws(expectedExceptionType, () => Activator.CreateInstance(typeToActivate, new object[0])); } [Fact] diff --git a/src/libraries/System.Runtime/tests/System/Runtime/CompilerServices/RuntimeHelpersTests.cs b/src/libraries/System.Runtime/tests/System/Runtime/CompilerServices/RuntimeHelpersTests.cs index 12620e947c06b..1653850fcd8e9 100644 --- a/src/libraries/System.Runtime/tests/System/Runtime/CompilerServices/RuntimeHelpersTests.cs +++ b/src/libraries/System.Runtime/tests/System/Runtime/CompilerServices/RuntimeHelpersTests.cs @@ -211,7 +211,7 @@ public static IEnumerable GetUninitializedObject_NegativeTestCases() yield return new[] { typeof(Delegate), typeof(MemberAccessException) }; // abstract type - yield return new[] { typeof(void), typeof(ArgumentException) }; // explicit block in place + yield return new[] { typeof(void), typeof(NotSupportedException) }; // explicit block in place yield return new[] { typeof(int).MakePointerType(), typeof(ArgumentException) }; // pointer typedesc yield return new[] { typeof(int).MakeByRefType(), typeof(ArgumentException) }; // byref typedesc From 0ef275c43f2e31fc0c8b4db7f97218a14c5f005b Mon Sep 17 00:00:00 2001 From: Levi Broderick Date: Thu, 19 Nov 2020 23:47:41 -0800 Subject: [PATCH 15/15] Call CreateInstanceCheckThis for better exception handling --- .../src/System/ActivatorCache.CoreCLR.cs | 6 ++++++ src/coreclr/src/vm/reflectioninvocation.cpp | 13 +++---------- .../System.Runtime/tests/System/ActivatorTests.cs | 12 ++++++------ .../Runtime/CompilerServices/RuntimeHelpersTests.cs | 2 +- 4 files changed, 16 insertions(+), 17 deletions(-) diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/ActivatorCache.CoreCLR.cs b/src/coreclr/src/System.Private.CoreLib/src/System/ActivatorCache.CoreCLR.cs index 62582b28200ad..1d789aa555581 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/ActivatorCache.CoreCLR.cs +++ b/src/coreclr/src/System.Private.CoreLib/src/System/ActivatorCache.CoreCLR.cs @@ -40,6 +40,12 @@ internal ActivatorCache( _originalRuntimeType = new WeakReference(rt); #endif + // The check below is redundant since these same checks are performed at the + // unmanaged layer, but this call will throw slightly different exceptions + // than the unmanaged layer, and callers might be dependent on this. + + rt.CreateInstanceCheckThis(); + _pfnAllocator = (delegate*)RuntimeTypeHandle.GetAllocatorFtn(rt, out MethodTable* pMT, forGetUninitializedObject: false, wrapExceptions); _allocatorFirstArg = (IntPtr)pMT; diff --git a/src/coreclr/src/vm/reflectioninvocation.cpp b/src/coreclr/src/vm/reflectioninvocation.cpp index e2b27cea88cf0..9cbfdd190e769 100644 --- a/src/coreclr/src/vm/reflectioninvocation.cpp +++ b/src/coreclr/src/vm/reflectioninvocation.cpp @@ -2033,14 +2033,7 @@ void QCALLTYPE RuntimeTypeHandle::GetAllocatorFtn( // Don't allow void if (typeHandle.GetSignatureCorElementType() == ELEMENT_TYPE_VOID) { - COMPlusThrow(kNotSupportedException, W("NotSupported_Type")); - } - - // Don't allow generic variables (e.g., the 'T' from List) - // or open generic types (List<>) - Activator variation of check. - if (!fGetUninitializedObject && typeHandle.ContainsGenericVariables()) - { - COMPlusThrow(kArgumentException, W("Acc_CreateGeneric")); + COMPlusThrow(kArgumentException, W("NotSupported_Type")); } // Don't allow arrays, pointers, byrefs, or function pointers @@ -2076,8 +2069,8 @@ void QCALLTYPE RuntimeTypeHandle::GetAllocatorFtn( } // Don't allow generic variables (e.g., the 'T' from List) - // or open generic types (List<>) - FormatterServices variation of check. - if (fGetUninitializedObject && typeHandle.ContainsGenericVariables()) + // or open generic types (List<>). + if (typeHandle.ContainsGenericVariables()) { COMPlusThrow(kMemberAccessException, W("Acc_CreateGeneric")); } diff --git a/src/libraries/System.Runtime/tests/System/ActivatorTests.cs b/src/libraries/System.Runtime/tests/System/ActivatorTests.cs index e497b64291bb2..05e07447a9616 100644 --- a/src/libraries/System.Runtime/tests/System/ActivatorTests.cs +++ b/src/libraries/System.Runtime/tests/System/ActivatorTests.cs @@ -150,17 +150,17 @@ public void CreateInstance_ContainsGenericParameters_ThrowsArgumentException(Typ public static IEnumerable CreateInstance_InvalidType_TestData() { - yield return new object[] { typeof(void), typeof(NotSupportedException) }; - yield return new object[] { typeof(void).MakeArrayType(), typeof(MissingMethodException) }; - yield return new object[] { typeof(ArgIterator), typeof(NotSupportedException) }; + yield return new object[] { typeof(void) }; + yield return new object[] { typeof(void).MakeArrayType() }; + yield return new object[] { typeof(ArgIterator) }; } [Theory] [MemberData(nameof(CreateInstance_InvalidType_TestData))] - public void CreateInstance_InvalidType_ThrowsNotSupportedException(Type typeToActivate, Type expectedExceptionType) + public void CreateInstance_InvalidType_ThrowsNotSupportedException(Type type) { - Assert.Throws(expectedExceptionType, () => Activator.CreateInstance(typeToActivate)); - Assert.Throws(expectedExceptionType, () => Activator.CreateInstance(typeToActivate, new object[0])); + Assert.Throws(() => Activator.CreateInstance(type)); + Assert.Throws(() => Activator.CreateInstance(type, new object[0])); } [Fact] diff --git a/src/libraries/System.Runtime/tests/System/Runtime/CompilerServices/RuntimeHelpersTests.cs b/src/libraries/System.Runtime/tests/System/Runtime/CompilerServices/RuntimeHelpersTests.cs index 1653850fcd8e9..12620e947c06b 100644 --- a/src/libraries/System.Runtime/tests/System/Runtime/CompilerServices/RuntimeHelpersTests.cs +++ b/src/libraries/System.Runtime/tests/System/Runtime/CompilerServices/RuntimeHelpersTests.cs @@ -211,7 +211,7 @@ public static IEnumerable GetUninitializedObject_NegativeTestCases() yield return new[] { typeof(Delegate), typeof(MemberAccessException) }; // abstract type - yield return new[] { typeof(void), typeof(NotSupportedException) }; // explicit block in place + yield return new[] { typeof(void), typeof(ArgumentException) }; // explicit block in place yield return new[] { typeof(int).MakePointerType(), typeof(ArgumentException) }; // pointer typedesc yield return new[] { typeof(int).MakeByRefType(), typeof(ArgumentException) }; // byref typedesc