From 022a481e6752d0c7d920695617f02c9a20b4c4c6 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Thu, 23 May 2024 14:38:56 -0700 Subject: [PATCH 1/8] Activator.CreateInstance similar to NAOT This represents an update of Activator.CreateInstance to naturally support byreflike generics. It is modeled off of the current native AOT implementation. --- .../src/System/RuntimeHandles.cs | 14 ++++--- .../src/System/RuntimeType.ActivatorCache.cs | 37 ++++++++++++++++--- .../src/System/RuntimeType.CoreCLR.cs | 30 ++++++++++++++- src/coreclr/vm/reflectioninvocation.cpp | 35 +++++++++++++----- src/coreclr/vm/runtimehandles.h | 1 + .../src/System/Activator.RuntimeType.cs | 12 +++++- 6 files changed, 106 insertions(+), 23 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/RuntimeHandles.cs b/src/coreclr/System.Private.CoreLib/src/System/RuntimeHandles.cs index 380981993451e9..74f6f5beeb3b63 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/RuntimeHandles.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/RuntimeHandles.cs @@ -270,24 +270,27 @@ internal static void GetActivationInfo( RuntimeType rt, out delegate* pfnAllocator, out void* vAllocatorFirstArg, - out delegate* pfnCtor, + out delegate* pfnRefCtor, + out delegate* pfnValueCtor, out bool ctorIsPublic) { Debug.Assert(rt != null); delegate* pfnAllocatorTemp = default; void* vAllocatorFirstArgTemp = default; - delegate* pfnCtorTemp = default; + delegate* pfnRefCtorTemp = default; + delegate* pfnValueCtorTemp = default; Interop.BOOL fCtorIsPublicTemp = default; GetActivationInfo( ObjectHandleOnStack.Create(ref rt), &pfnAllocatorTemp, &vAllocatorFirstArgTemp, - &pfnCtorTemp, &fCtorIsPublicTemp); + &pfnRefCtorTemp, &pfnValueCtorTemp, &fCtorIsPublicTemp); pfnAllocator = pfnAllocatorTemp; vAllocatorFirstArg = vAllocatorFirstArgTemp; - pfnCtor = pfnCtorTemp; + pfnRefCtor = pfnRefCtorTemp; + pfnValueCtor = pfnValueCtorTemp; ctorIsPublic = fCtorIsPublicTemp != Interop.BOOL.FALSE; } @@ -296,7 +299,8 @@ private static partial void GetActivationInfo( ObjectHandleOnStack pRuntimeType, delegate** ppfnAllocator, void** pvAllocatorFirstArg, - delegate** ppfnCtor, + delegate** ppfnRefCtor, + delegate** ppfnValueCtor, Interop.BOOL* pfCtorIsPublic); #if FEATURE_COMINTEROP diff --git a/src/coreclr/System.Private.CoreLib/src/System/RuntimeType.ActivatorCache.cs b/src/coreclr/System.Private.CoreLib/src/System/RuntimeType.ActivatorCache.cs index 4d73cfad391430..9c84e1c10a11e5 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/RuntimeType.ActivatorCache.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/RuntimeType.ActivatorCache.cs @@ -21,7 +21,8 @@ private sealed unsafe class ActivatorCache private readonly void* _allocatorFirstArg; // The managed calli to the parameterless ctor, taking "this" (as object) as its first argument. - private readonly delegate* _pfnCtor; + private readonly delegate* _pfnRefCtor; + private readonly delegate* _pfnValueCtor; private readonly bool _ctorIsPublic; private CreateUninitializedCache? _createUninitializedCache; @@ -48,7 +49,7 @@ internal ActivatorCache(RuntimeType rt) { RuntimeTypeHandle.GetActivationInfo(rt, out _pfnAllocator!, out _allocatorFirstArg, - out _pfnCtor!, out _ctorIsPublic); + out _pfnRefCtor!, out _pfnValueCtor!, out _ctorIsPublic); } catch (Exception ex) { @@ -87,14 +88,29 @@ internal ActivatorCache(RuntimeType rt) // would have thrown an exception if 'rt' were a normal reference type // without a ctor. - if (_pfnCtor == null) + if (_pfnRefCtor == null) { - static void CtorNoopStub(object? uninitializedObject) { } - _pfnCtor = &CtorNoopStub; // we use null singleton pattern if no ctor call is necessary + static void RefCtorNoopStub(object? uninitializedObject) { } + _pfnRefCtor = &RefCtorNoopStub; // we use null singleton pattern if no ctor call is necessary Debug.Assert(_ctorIsPublic); // implicit parameterless ctor is always considered public } + if (rt.IsValueType) + { + if (_pfnValueCtor == null) + { + static void ValueRefCtorNoopStub(ref byte uninitializedObject) { } + _pfnValueCtor = &ValueRefCtorNoopStub; // we use null singleton pattern if no ctor call is necessary + + Debug.Assert(_ctorIsPublic); // implicit parameterless ctor is always considered public + } + } + else + { + Debug.Assert(_pfnValueCtor == null); // Non-value types shouldn't have a value constructor. + } + // We don't need to worry about invoking cctors here. The runtime will figure it // out for us when the instance ctor is called. For value types, because we're // creating a boxed default(T), the static cctor is called when *any* instance @@ -120,7 +136,16 @@ static void CtorNoopStub(object? uninitializedObject) { } } [MethodImpl(MethodImplOptions.AggressiveInlining)] - internal void CallConstructor(object? uninitializedObject) => _pfnCtor(uninitializedObject); + internal void CallRefConstructor(object? uninitializedObject) => _pfnRefCtor(uninitializedObject); + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + internal void CallValueConstructor(ref byte uninitializedObject) + { +#if DEBUG + Debug.Assert(_originalRuntimeType.IsValueType); +#endif + _pfnValueCtor(ref uninitializedObject); + } [MethodImpl(MethodImplOptions.AggressiveInlining)] internal CreateUninitializedCache GetCreateUninitializedCache(RuntimeType rt) diff --git a/src/coreclr/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs index c6ea76fde9b85f..55244ec2343aa8 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs @@ -3932,7 +3932,7 @@ internal object GetUninitializedObject() object? obj = cache.CreateUninitializedObject(this); try { - cache.CallConstructor(obj); + cache.CallRefConstructor(obj); } catch (Exception e) when (wrapExceptions) { @@ -3961,7 +3961,7 @@ internal object GetUninitializedObject() object? obj = cache.CreateUninitializedObject(this); try { - cache.CallConstructor(obj); + cache.CallRefConstructor(obj); } catch (Exception e) { @@ -3971,6 +3971,32 @@ internal object GetUninitializedObject() return obj; } + // Specialized version of the above for Activator.CreateInstance() + [DebuggerStepThrough] + [DebuggerHidden] + internal void CallDefaultStructConstructor(ref byte data) + { + if (GenericCache is not ActivatorCache cache) + { + cache = new ActivatorCache(this); + GenericCache = cache; + } + + if (!cache.CtorIsPublic) + { + throw new MissingMethodException(SR.Format(SR.Arg_NoDefCTor, this)); + } + + try + { + cache.CallValueConstructor(ref data); + } + catch (Exception e) + { + throw new TargetInvocationException(e); + } + } + internal void InvalidateCachedNestedType() => Cache.InvalidateCachedNestedType(); internal bool IsGenericCOMObjectImpl() => RuntimeTypeHandle.IsComObject(this, true); diff --git a/src/coreclr/vm/reflectioninvocation.cpp b/src/coreclr/vm/reflectioninvocation.cpp index f57e21801253a3..6c240448ddfd67 100644 --- a/src/coreclr/vm/reflectioninvocation.cpp +++ b/src/coreclr/vm/reflectioninvocation.cpp @@ -1694,7 +1694,8 @@ extern "C" void QCALLTYPE RuntimeTypeHandle_GetActivationInfo( QCall::ObjectHandleOnStack pRuntimeType, PCODE* ppfnAllocator, void** pvAllocatorFirstArg, - PCODE* ppfnCtor, + PCODE* ppfnRefCtor, + PCODE* ppfnValueCtor, BOOL* pfCtorIsPublic ) { @@ -1703,11 +1704,13 @@ extern "C" void QCALLTYPE RuntimeTypeHandle_GetActivationInfo( QCALL_CHECK; PRECONDITION(CheckPointer(ppfnAllocator)); PRECONDITION(CheckPointer(pvAllocatorFirstArg)); - PRECONDITION(CheckPointer(ppfnCtor)); + PRECONDITION(CheckPointer(ppfnRefCtor)); + PRECONDITION(CheckPointer(ppfnValueCtor)); PRECONDITION(CheckPointer(pfCtorIsPublic)); PRECONDITION(*ppfnAllocator == NULL); PRECONDITION(*pvAllocatorFirstArg == NULL); - PRECONDITION(*ppfnCtor == NULL); + PRECONDITION(*ppfnRefCtor == NULL); + PRECONDITION(*ppfnValueCtor == NULL); PRECONDITION(*pfCtorIsPublic == FALSE); } CONTRACTL_END; @@ -1761,7 +1764,8 @@ extern "C" void QCALLTYPE RuntimeTypeHandle_GetActivationInfo( // managed sig: ComClassFactory* -> object (via FCALL) *ppfnAllocator = CoreLibBinder::GetMethod(METHOD__RT_TYPE_HANDLE__ALLOCATECOMOBJECT)->GetMultiCallableAddrOfCode(); *pvAllocatorFirstArg = pClassFactory; - *ppfnCtor = NULL; // no ctor call needed; activation handled entirely by the allocator + *ppfnRefCtor = NULL; // no ctor call needed; activation handled entirely by the allocator + *ppfnValueCtor = NULL; // no value ctor for reference type *pfCtorIsPublic = TRUE; // no ctor call needed => assume 'public' equivalent } else @@ -1771,7 +1775,8 @@ extern "C" void QCALLTYPE RuntimeTypeHandle_GetActivationInfo( // CreateInstance returns null given Nullable *ppfnAllocator = (PCODE)NULL; *pvAllocatorFirstArg = NULL; - *ppfnCtor = (PCODE)NULL; + *ppfnRefCtor = (PCODE)NULL; + *ppfnValueCtor = (PCODE)NULL; *pfCtorIsPublic = TRUE; // no ctor call needed => assume 'public' equivalent } else @@ -1781,22 +1786,34 @@ extern "C" void QCALLTYPE RuntimeTypeHandle_GetActivationInfo( *ppfnAllocator = CEEJitInfo::getHelperFtnStatic(CEEInfo::getNewHelperStatic(pMT, &fHasSideEffectsUnused)); *pvAllocatorFirstArg = pMT; + BOOL isValueType = pMT->IsValueType(); if (pMT->HasDefaultConstructor()) { // managed sig: object -> void // for ctors on value types, lookup boxed entry point stub - MethodDesc* pMD = pMT->GetDefaultConstructor(pMT->IsValueType() /* forceBoxedEntryPoint */); + MethodDesc* pMD = pMT->GetDefaultConstructor(isValueType /* forceBoxedEntryPoint */); _ASSERTE(pMD != NULL); PCODE pCode = pMD->GetMultiCallableAddrOfCode(); _ASSERTE(pCode != (PCODE)NULL); - *ppfnCtor = pCode; + *ppfnRefCtor = pCode; *pfCtorIsPublic = pMD->IsPublic(); + + // If we have a value type, get the non-boxing entry point too. + if (isValueType) + { + pMD = pMT->GetDefaultConstructor(FALSE /* forceBoxedEntryPoint */); + _ASSERTE(pMD != NULL); + pCode = pMD->GetMultiCallableAddrOfCode(); + _ASSERTE(pCode != (PCODE)NULL); + *ppfnValueCtor = pCode; + } } - else if (pMT->IsValueType()) + else if (isValueType) { - *ppfnCtor = (PCODE)NULL; // no ctor call needed; we're creating a boxed default(T) + *ppfnRefCtor = (PCODE)NULL; // no ctor call needed; we're creating a boxed default(T) + *ppfnValueCtor = (PCODE)NULL; *pfCtorIsPublic = TRUE; // no ctor call needed => assume 'public' equivalent } else diff --git a/src/coreclr/vm/runtimehandles.h b/src/coreclr/vm/runtimehandles.h index 56bb2df245f9f7..1adb6b25ebc3f4 100644 --- a/src/coreclr/vm/runtimehandles.h +++ b/src/coreclr/vm/runtimehandles.h @@ -179,6 +179,7 @@ extern "C" void QCALLTYPE RuntimeTypeHandle_GetActivationInfo( PCODE* ppfnAllocator, void** pvAllocatorFirstArg, PCODE* ppfnCtor, + PCODE* ppfnValueCtor, BOOL* pfCtorIsPublic); extern "C" void QCALLTYPE RuntimeTypeHandle_MakeByRef(QCall::TypeHandle pTypeHandle, QCall::ObjectHandleOnStack retType); extern "C" void QCALLTYPE RuntimeTypeHandle_MakePointer(QCall::TypeHandle pTypeHandle, QCall::ObjectHandleOnStack retType); 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 4d077d8ac3f9c8..0ab8ddc15a87aa 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Activator.RuntimeType.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Activator.RuntimeType.cs @@ -137,7 +137,17 @@ public static partial class Activator [Intrinsic] public static T CreateInstance<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)] T>() { - return (T)((RuntimeType)typeof(T)).CreateInstanceOfT()!; + var rtType = (RuntimeType)typeof(T); + if (!rtType.IsValueType) + { + return (T)rtType.CreateInstanceOfT()!; + } + else + { + T t = default!; + rtType.CallDefaultStructConstructor(ref Unsafe.As(ref t)); + return t; + } } private static T CreateDefaultInstance() where T : struct => default; From 4614dd3b85007b20c057fcc26cb52e4dccc37356 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Tue, 28 May 2024 15:55:51 -0700 Subject: [PATCH 2/8] Mono implementation now supports shared path with CoreCLR. --- .../src/System/RuntimeType.Mono.cs | 23 ++++++++++++++++++- src/mono/mono/metadata/icall-def.h | 1 + src/mono/mono/metadata/icall.c | 11 +++++++++ 3 files changed, 34 insertions(+), 1 deletion(-) diff --git a/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs b/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs index f289911d790f39..303ff086956c3d 100644 --- a/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs +++ b/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs @@ -37,7 +37,7 @@ internal enum TypeNameFormatFlags FormatFullInst } - internal partial class RuntimeType + internal unsafe partial class RuntimeType { #region Definitions @@ -1615,6 +1615,27 @@ private void CreateInstanceCheckThis() return CreateInstanceMono(false, true); } + // Specialized version of the above for Activator.CreateInstance() + [DebuggerStepThroughAttribute] + [Diagnostics.DebuggerHidden] + internal void CallDefaultStructConstructor(ref byte value) + { + Debug.Assert(IsValueType); + + RuntimeConstructorInfo? ctor = GetDefaultConstructor(); + if (ctor == null) + return; + + delegate* valueCtor = GetValueConstructor(ObjectHandleOnStack.Create(ref ctor)); + if (valueCtor == null) + throw new ExecutionEngineException(); + + valueCtor(ref value); + } + + [MethodImplAttribute(MethodImplOptions.InternalCall)] + private static extern delegate* GetValueConstructor(ObjectHandleOnStack rtCtorInfo); + #endregion private TypeCache? cache; diff --git a/src/mono/mono/metadata/icall-def.h b/src/mono/mono/metadata/icall-def.h index 53c95c8245faeb..56412533745781 100644 --- a/src/mono/mono/metadata/icall-def.h +++ b/src/mono/mono/metadata/icall-def.h @@ -510,6 +510,7 @@ HANDLES(RT_24, "GetNamespace", ves_icall_RuntimeType_GetNamespace, void, 2, (Mon HANDLES(RT_13, "GetNestedTypes_native", ves_icall_RuntimeType_GetNestedTypes_native, GPtrArray_ptr, 4, (MonoQCallTypeHandle, char_ptr, guint32, guint32)) HANDLES(RT_14, "GetPacking", ves_icall_RuntimeType_GetPacking, void, 3, (MonoQCallTypeHandle, guint32_ref, guint32_ref)) HANDLES(RT_15, "GetPropertiesByName_native", ves_icall_RuntimeType_GetPropertiesByName_native, GPtrArray_ptr, 4, (MonoQCallTypeHandle, char_ptr, guint32, guint32)) +HANDLES(RT_34, "GetValueConstructor", ves_icall_RuntimeType_GetValueConstructor, gpointer, 1, (MonoObjectHandleOnStack)) NOHANDLES(ICALL(RT_29, "IsUnmanagedFunctionPointerInternal", ves_icall_RuntimeType_IsUnmanagedFunctionPointerInternal)) HANDLES(RT_17, "MakeGenericType", ves_icall_RuntimeType_MakeGenericType, void, 3, (MonoReflectionType, MonoArray, MonoObjectHandleOnStack)) HANDLES(RT_19, "getFullName", ves_icall_System_RuntimeType_getFullName, void, 4, (MonoQCallTypeHandle, MonoObjectHandleOnStack, MonoBoolean, MonoBoolean)) diff --git a/src/mono/mono/metadata/icall.c b/src/mono/mono/metadata/icall.c index a1ab947afd36c1..269697120178c6 100644 --- a/src/mono/mono/metadata/icall.c +++ b/src/mono/mono/metadata/icall.c @@ -4246,6 +4246,17 @@ ves_icall_RuntimeType_GetPropertiesByName_native (MonoQCallTypeHandle type_handl return NULL; } +gpointer +ves_icall_RuntimeType_GetValueConstructor (MonoObjectHandleOnStack ref_ctor_info, MonoError *error) +{ + gpointer ctor_fptr; + MonoReflectionMethod* ctor = *(MonoReflectionMethod**)ref_ctor_info; + ctor_fptr = mono_compile_method_checked (ctor->method, error); + if (!is_ok (error)) + return NULL; + return ctor_fptr; +} + static guint event_hash (gconstpointer data) { From a12f418f02b73cbd953b350c0e875566f51be1f6 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Wed, 29 May 2024 11:19:08 -0700 Subject: [PATCH 3/8] Permit ref structs for Activator.CreateInstance and Unsafe.As(). Add test --- .../src/System/Activator.RuntimeType.cs | 4 +++- .../src/System/Runtime/CompilerServices/Unsafe.cs | 2 ++ src/libraries/System.Runtime/ref/System.Runtime.cs | 4 ++-- .../Loader/classloader/generics/ByRefLike/Validate.cs | 11 +++++++++++ 4 files changed, 18 insertions(+), 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 0ab8ddc15a87aa..c857d26dab8cb1 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Activator.RuntimeType.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Activator.RuntimeType.cs @@ -136,11 +136,13 @@ public static partial class Activator [Intrinsic] public static T CreateInstance<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)] T>() + where T : allows ref struct { var rtType = (RuntimeType)typeof(T); if (!rtType.IsValueType) { - return (T)rtType.CreateInstanceOfT()!; + object o = rtType.CreateInstanceOfT()!; + return Unsafe.As(ref o); } else { diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/Unsafe.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/Unsafe.cs index e59b30fd7633fc..9f0129aef0a728 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/Unsafe.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/Unsafe.cs @@ -83,6 +83,8 @@ public static T As(object? o) where T : class? [NonVersionable] [MethodImpl(MethodImplOptions.AggressiveInlining)] public static ref TTo As(ref TFrom source) + where TFrom : allows ref struct + where TTo : allows ref struct { throw new PlatformNotSupportedException(); diff --git a/src/libraries/System.Runtime/ref/System.Runtime.cs b/src/libraries/System.Runtime/ref/System.Runtime.cs index 37bd783042bdba..30c6aa55360f49 100644 --- a/src/libraries/System.Runtime/ref/System.Runtime.cs +++ b/src/libraries/System.Runtime/ref/System.Runtime.cs @@ -91,7 +91,7 @@ public static partial class Activator public static System.Runtime.Remoting.ObjectHandle? CreateInstanceFrom(string assemblyFile, string typeName, bool ignoreCase, System.Reflection.BindingFlags bindingAttr, System.Reflection.Binder? binder, object?[]? args, System.Globalization.CultureInfo? culture, object?[]? activationAttributes) { throw null; } [System.Diagnostics.CodeAnalysis.RequiresUnreferencedCodeAttribute("Type and its constructor could be removed")] public static System.Runtime.Remoting.ObjectHandle? CreateInstanceFrom(string assemblyFile, string typeName, object?[]? activationAttributes) { throw null; } - public static T CreateInstance<[System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute(System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)]T>() { throw null; } + public static T CreateInstance<[System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute(System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)]T>() where T : allows ref struct { throw null; } } public partial class AggregateException : System.Exception { @@ -13357,7 +13357,7 @@ public static partial class Unsafe public static ref T AsRef(scoped ref readonly T source) { throw null; } [return: System.Diagnostics.CodeAnalysis.NotNullIfNotNullAttribute("o")] public static T? As(object? o) where T : class? { throw null; } - public static ref TTo As(ref TFrom source) { throw null; } + public static ref TTo As (ref TFrom source) where TFrom : allows ref struct where TTo : allows ref struct { throw null; } public static TTo BitCast(TFrom source) { throw null; } public static System.IntPtr ByteOffset([System.Diagnostics.CodeAnalysis.AllowNull] ref readonly T origin, [System.Diagnostics.CodeAnalysis.AllowNull] ref readonly T target) { throw null; } [System.CLSCompliantAttribute(false)] diff --git a/src/tests/Loader/classloader/generics/ByRefLike/Validate.cs b/src/tests/Loader/classloader/generics/ByRefLike/Validate.cs index 867cccd32d5be3..45de0af7d211ba 100644 --- a/src/tests/Loader/classloader/generics/ByRefLike/Validate.cs +++ b/src/tests/Loader/classloader/generics/ByRefLike/Validate.cs @@ -11,6 +11,17 @@ public class Validate { + [Fact] + public static void Validate_Activation() + { + Console.WriteLine($"{nameof(Validate_Activation)}..."); + + Assert.Equal("System.Span[0]", Activator.CreateInstance>().ToString()); + Assert.Equal("System.Span[0]", Activator.CreateInstance>().ToString()); + Assert.Equal("System.ReadOnlySpan[0]", Activator.CreateInstance>().ToString()); + Assert.Equal("System.ReadOnlySpan[0]", Activator.CreateInstance>().ToString()); + } + [Fact] public static void Validate_TypeLoad() { From 0b91dbb0c424ee8f945847aa997d4464a817eb22 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Wed, 29 May 2024 16:33:51 -0700 Subject: [PATCH 4/8] Comment why Unsafe.As<> is safe. Wrap invocation in TargetInvocationException. --- .../src/System/Activator.RuntimeType.cs | 7 +++++++ .../src/System/RuntimeType.Mono.cs | 9 ++++++++- 2 files changed, 15 insertions(+), 1 deletion(-) 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 c857d26dab8cb1..b0837a535f00b7 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Activator.RuntimeType.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Activator.RuntimeType.cs @@ -142,6 +142,13 @@ public static partial class Activator if (!rtType.IsValueType) { object o = rtType.CreateInstanceOfT()!; + + // Casting the above object to T is technically invalid because + // T can be ByRefLike (that is, ref struct). Roslyn blocks the + // cast this in function with a "CS0030: Cannot convert type 'object' to 'T'", + // which is correct. However, since we are doing the IsValueType + // check above, we know this code path will only be taken with + // reference types and therefore the below Unsafe.As<> is safe. return Unsafe.As(ref o); } else diff --git a/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs b/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs index 303ff086956c3d..ae4f826cce8cb5 100644 --- a/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs +++ b/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs @@ -1630,7 +1630,14 @@ internal void CallDefaultStructConstructor(ref byte value) if (valueCtor == null) throw new ExecutionEngineException(); - valueCtor(ref value); + try + { + valueCtor(ref value); + } + catch (Exception e) + { + throw new TargetInvocationException(e); + } } [MethodImplAttribute(MethodImplOptions.InternalCall)] From 612a195b71df2b044d2771f5163bc2e400966efb Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Wed, 29 May 2024 23:51:33 -0400 Subject: [PATCH 5/8] Use MethodHandle.GetFunctionPointer to call the value constructor On the Mono interpreter, the ldftn opcode for a managed method returns a MonoFtnDesc*, not a native code pointer, and the calli opcode expects a MonoFtnDesc*. The MethodHandle.GetFunctionPointer() function is an interpreter intrinsic that returns such a MonoFtnDesc* --- .../src/System/RuntimeType.Mono.cs | 10 +++++----- src/mono/mono/metadata/icall-def.h | 1 - src/mono/mono/metadata/icall.c | 11 ----------- 3 files changed, 5 insertions(+), 17 deletions(-) diff --git a/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs b/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs index ae4f826cce8cb5..6c51ca7ab4c38d 100644 --- a/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs +++ b/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs @@ -1626,7 +1626,11 @@ internal void CallDefaultStructConstructor(ref byte value) if (ctor == null) return; - delegate* valueCtor = GetValueConstructor(ObjectHandleOnStack.Create(ref ctor)); + // Important: when using the interpreter, GetFunctionPointer is an intrinsic that + // returns a function descriptor suitable for casting to a managed function pointer. + // Other ways of obtraining a function pointer might not work. + IntPtr ptr = ctor.MethodHandle.GetFunctionPointer(); + delegate* valueCtor = (delegate*)ptr; if (valueCtor == null) throw new ExecutionEngineException(); @@ -1639,10 +1643,6 @@ internal void CallDefaultStructConstructor(ref byte value) throw new TargetInvocationException(e); } } - - [MethodImplAttribute(MethodImplOptions.InternalCall)] - private static extern delegate* GetValueConstructor(ObjectHandleOnStack rtCtorInfo); - #endregion private TypeCache? cache; diff --git a/src/mono/mono/metadata/icall-def.h b/src/mono/mono/metadata/icall-def.h index 56412533745781..53c95c8245faeb 100644 --- a/src/mono/mono/metadata/icall-def.h +++ b/src/mono/mono/metadata/icall-def.h @@ -510,7 +510,6 @@ HANDLES(RT_24, "GetNamespace", ves_icall_RuntimeType_GetNamespace, void, 2, (Mon HANDLES(RT_13, "GetNestedTypes_native", ves_icall_RuntimeType_GetNestedTypes_native, GPtrArray_ptr, 4, (MonoQCallTypeHandle, char_ptr, guint32, guint32)) HANDLES(RT_14, "GetPacking", ves_icall_RuntimeType_GetPacking, void, 3, (MonoQCallTypeHandle, guint32_ref, guint32_ref)) HANDLES(RT_15, "GetPropertiesByName_native", ves_icall_RuntimeType_GetPropertiesByName_native, GPtrArray_ptr, 4, (MonoQCallTypeHandle, char_ptr, guint32, guint32)) -HANDLES(RT_34, "GetValueConstructor", ves_icall_RuntimeType_GetValueConstructor, gpointer, 1, (MonoObjectHandleOnStack)) NOHANDLES(ICALL(RT_29, "IsUnmanagedFunctionPointerInternal", ves_icall_RuntimeType_IsUnmanagedFunctionPointerInternal)) HANDLES(RT_17, "MakeGenericType", ves_icall_RuntimeType_MakeGenericType, void, 3, (MonoReflectionType, MonoArray, MonoObjectHandleOnStack)) HANDLES(RT_19, "getFullName", ves_icall_System_RuntimeType_getFullName, void, 4, (MonoQCallTypeHandle, MonoObjectHandleOnStack, MonoBoolean, MonoBoolean)) diff --git a/src/mono/mono/metadata/icall.c b/src/mono/mono/metadata/icall.c index 269697120178c6..a1ab947afd36c1 100644 --- a/src/mono/mono/metadata/icall.c +++ b/src/mono/mono/metadata/icall.c @@ -4246,17 +4246,6 @@ ves_icall_RuntimeType_GetPropertiesByName_native (MonoQCallTypeHandle type_handl return NULL; } -gpointer -ves_icall_RuntimeType_GetValueConstructor (MonoObjectHandleOnStack ref_ctor_info, MonoError *error) -{ - gpointer ctor_fptr; - MonoReflectionMethod* ctor = *(MonoReflectionMethod**)ref_ctor_info; - ctor_fptr = mono_compile_method_checked (ctor->method, error); - if (!is_ok (error)) - return NULL; - return ctor_fptr; -} - static guint event_hash (gconstpointer data) { From 7b3e1726768b52e3e17488a51f38978f4f8cf4e4 Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Thu, 30 May 2024 08:35:55 -0700 Subject: [PATCH 6/8] Update src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs --- src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs b/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs index 6c51ca7ab4c38d..f74d0fbdc6dc79 100644 --- a/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs +++ b/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs @@ -1628,7 +1628,7 @@ internal void CallDefaultStructConstructor(ref byte value) // Important: when using the interpreter, GetFunctionPointer is an intrinsic that // returns a function descriptor suitable for casting to a managed function pointer. - // Other ways of obtraining a function pointer might not work. + // Other ways of obtaining a function pointer might not work. IntPtr ptr = ctor.MethodHandle.GetFunctionPointer(); delegate* valueCtor = (delegate*)ptr; if (valueCtor == null) From bcf269c547381224b0e07d09b0cd19a35c60cacb Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Thu, 30 May 2024 11:52:35 -0700 Subject: [PATCH 7/8] Missed public ctor check. --- .../src/System/RuntimeType.CoreCLR.cs | 8 +++++--- .../src/System/Activator.RuntimeType.cs | 2 +- .../src/System/RuntimeType.Mono.cs | 15 ++++++++++++--- 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs index 55244ec2343aa8..016b9d66d935a9 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs @@ -3942,7 +3942,7 @@ internal object GetUninitializedObject() return obj; } - // Specialized version of the above for Activator.CreateInstance() + // Specialized version of CreateInstanceDefaultCtor() for Activator.CreateInstance() [DebuggerStepThrough] [DebuggerHidden] internal object? CreateInstanceOfT() @@ -3971,11 +3971,13 @@ internal object GetUninitializedObject() return obj; } - // Specialized version of the above for Activator.CreateInstance() + // Specialized version of CreateInstanceDefaultCtor() for Activator.CreateInstance() [DebuggerStepThrough] [DebuggerHidden] - internal void CallDefaultStructConstructor(ref byte data) + internal void CallDefaultValueTypeConstructor(ref byte data) { + Debug.Assert(IsValueType); + if (GenericCache is not ActivatorCache cache) { cache = new ActivatorCache(this); 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 b0837a535f00b7..0d45af4e23bf08 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Activator.RuntimeType.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Activator.RuntimeType.cs @@ -154,7 +154,7 @@ public static partial class Activator else { T t = default!; - rtType.CallDefaultStructConstructor(ref Unsafe.As(ref t)); + rtType.CallDefaultValueTypeConstructor(ref Unsafe.As(ref t)); return t; } } diff --git a/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs b/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs index f74d0fbdc6dc79..df35604bd1c252 100644 --- a/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs +++ b/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs @@ -1607,7 +1607,7 @@ private void CreateInstanceCheckThis() return CreateInstanceMono(!publicOnly, wrapExceptions); } - // Specialized version of the above for Activator.CreateInstance() + // Specialized version of CreateInstanceDefaultCtor() for Activator.CreateInstance() [DebuggerStepThroughAttribute] [Diagnostics.DebuggerHidden] internal object? CreateInstanceOfT() @@ -1615,16 +1615,23 @@ private void CreateInstanceCheckThis() return CreateInstanceMono(false, true); } - // Specialized version of the above for Activator.CreateInstance() + // Specialized version of CreateInstanceDefaultCtor() for Activator.CreateInstance() [DebuggerStepThroughAttribute] [Diagnostics.DebuggerHidden] - internal void CallDefaultStructConstructor(ref byte value) + internal void CallDefaultValueTypeConstructor(ref byte value) { Debug.Assert(IsValueType); RuntimeConstructorInfo? ctor = GetDefaultConstructor(); if (ctor == null) + { return; + } + + if (!ctor.IsPublic) + { + throw new MissingMethodException(SR.Format(SR.Arg_NoDefCTor, this)); + } // Important: when using the interpreter, GetFunctionPointer is an intrinsic that // returns a function descriptor suitable for casting to a managed function pointer. @@ -1632,7 +1639,9 @@ internal void CallDefaultStructConstructor(ref byte value) IntPtr ptr = ctor.MethodHandle.GetFunctionPointer(); delegate* valueCtor = (delegate*)ptr; if (valueCtor == null) + { throw new ExecutionEngineException(); + } try { From 5cff86bd316121cb1e161261792e92ac80471a9d Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Thu, 30 May 2024 13:49:29 -0700 Subject: [PATCH 8/8] Keep API names consistent across runtimes. --- .../System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs | 2 +- .../System.Private.CoreLib/src/System/Activator.RuntimeType.cs | 2 +- src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs index 016b9d66d935a9..9eca40dfcad506 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs @@ -3974,7 +3974,7 @@ internal object GetUninitializedObject() // Specialized version of CreateInstanceDefaultCtor() for Activator.CreateInstance() [DebuggerStepThrough] [DebuggerHidden] - internal void CallDefaultValueTypeConstructor(ref byte data) + internal void CallDefaultStructConstructor(ref byte data) { Debug.Assert(IsValueType); 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 0d45af4e23bf08..b0837a535f00b7 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Activator.RuntimeType.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Activator.RuntimeType.cs @@ -154,7 +154,7 @@ public static partial class Activator else { T t = default!; - rtType.CallDefaultValueTypeConstructor(ref Unsafe.As(ref t)); + rtType.CallDefaultStructConstructor(ref Unsafe.As(ref t)); return t; } } diff --git a/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs b/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs index df35604bd1c252..f946ce7bc09daa 100644 --- a/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs +++ b/src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs @@ -1618,7 +1618,7 @@ private void CreateInstanceCheckThis() // Specialized version of CreateInstanceDefaultCtor() for Activator.CreateInstance() [DebuggerStepThroughAttribute] [Diagnostics.DebuggerHidden] - internal void CallDefaultValueTypeConstructor(ref byte value) + internal void CallDefaultStructConstructor(ref byte value) { Debug.Assert(IsValueType);