From 6f3f4cca505cd9838fcc13a5cf5d7978eee3b42c Mon Sep 17 00:00:00 2001 From: Adeel <3840695+am11@users.noreply.github.com> Date: Tue, 3 Dec 2024 09:00:24 +0200 Subject: [PATCH 1/5] Move GetRefAny (with FCThrow) to managed --- .../Runtime/CompilerServices/CastHelpers.cs | 17 +++++++++++++++++ .../src/System/TypedReference.CoreCLR.cs | 3 +++ src/coreclr/inc/jithelpers.h | 2 +- src/coreclr/vm/corelib.h | 5 +++-- src/coreclr/vm/jithelpers.cpp | 18 ------------------ 5 files changed, 24 insertions(+), 21 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs index 202a339b680251..61aac42a16ae6a 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Runtime.InteropServices; @@ -24,6 +25,9 @@ internal static void ThrowInvalidCastException(object fromType, void* toTypeHnd) throw null!; // Provide hint to the inliner that this method does not return } + [DoesNotReturn] + internal static void ThrowInvalidCastException() => throw new InvalidCastException(); + [MethodImpl(MethodImplOptions.InternalCall)] private static extern object IsInstanceOfAny_NoCacheLookup(void* toTypeHnd, object obj); @@ -33,6 +37,19 @@ internal static void ThrowInvalidCastException(object fromType, void* toTypeHnd) [MethodImpl(MethodImplOptions.InternalCall)] private static extern void WriteBarrier(ref object? dst, object? obj); + internal static ref byte GetRefAny(IntPtr clsHnd, TypedReference typedByRef) + { + // @TODO right now we check for precisely the correct type. + // do we want to allow inheritance? (watch out since value + // classes inherit from object but do not normal object layout). + if (clsHnd != typedByRef.Type) + { + ThrowInvalidCastException(); + } + + return ref typedByRef.Value; + } + // IsInstanceOf test used for unusual cases (naked type parameters, variant generic types) // Unlike the IsInstanceOfInterface and IsInstanceOfClass functions, // this test must deal with all kinds of type tests diff --git a/src/coreclr/System.Private.CoreLib/src/System/TypedReference.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/TypedReference.CoreCLR.cs index de5b8d36c5053b..3c93e45894090b 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/TypedReference.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/TypedReference.CoreCLR.cs @@ -16,6 +16,9 @@ public ref partial struct TypedReference private readonly ref byte _value; private readonly IntPtr _type; + internal readonly IntPtr Type => _type; + internal readonly ref byte Value => ref _value; + private TypedReference(ref byte target, RuntimeType type) { _value = ref target; diff --git a/src/coreclr/inc/jithelpers.h b/src/coreclr/inc/jithelpers.h index b163f80ab49416..6d0d7950d9a31d 100644 --- a/src/coreclr/inc/jithelpers.h +++ b/src/coreclr/inc/jithelpers.h @@ -117,7 +117,7 @@ DYNAMICJITHELPER(CORINFO_HELP_UNBOX_TYPETEST,NULL, METHOD__CASTHELPERS__UNBOX_TYPETEST) DYNAMICJITHELPER(CORINFO_HELP_UNBOX_NULLABLE,NULL, METHOD__CASTHELPERS__UNBOX_NULLABLE) - JITHELPER(CORINFO_HELP_GETREFANY, JIT_GetRefAny, METHOD__NIL) + DYNAMICJITHELPER(CORINFO_HELP_GETREFANY, NULL, METHOD__CASTHELPERS__GETREFANY) DYNAMICJITHELPER(CORINFO_HELP_ARRADDR_ST, NULL, METHOD__CASTHELPERS__STELEMREF) DYNAMICJITHELPER(CORINFO_HELP_LDELEMA_REF, NULL, METHOD__CASTHELPERS__LDELEMAREF) diff --git a/src/coreclr/vm/corelib.h b/src/coreclr/vm/corelib.h index 5f20ad2d3b54f0..bbadd4da7c6e52 100644 --- a/src/coreclr/vm/corelib.h +++ b/src/coreclr/vm/corelib.h @@ -1178,8 +1178,9 @@ DEFINE_METHOD(CASTHELPERS, UNBOX, Unbox, NoSig) DEFINE_METHOD(CASTHELPERS, STELEMREF, StelemRef, SM_ArrObject_IntPtr_Obj_RetVoid) DEFINE_METHOD(CASTHELPERS, LDELEMAREF, LdelemaRef, SM_ArrObject_IntPtr_PtrVoid_RetRefObj) DEFINE_METHOD(CASTHELPERS, ARRAYTYPECHECK, ArrayTypeCheck, SM_Obj_Array_RetVoid) -DEFINE_METHOD(CASTHELPERS, UNBOX_NULLABLE, Unbox_Nullable, NoSig) -DEFINE_METHOD(CASTHELPERS, UNBOX_TYPETEST, Unbox_TypeTest, NoSig) +DEFINE_METHOD(CASTHELPERS, UNBOX_NULLABLE, Unbox_Nullable, NoSig) +DEFINE_METHOD(CASTHELPERS, UNBOX_TYPETEST, Unbox_TypeTest, NoSig) +DEFINE_METHOD(CASTHELPERS, GETREFANY, GetRefAny, NoSig) DEFINE_CLASS(VIRTUALDISPATCHHELPERS, CompilerServices, VirtualDispatchHelpers) DEFINE_METHOD(VIRTUALDISPATCHHELPERS, VIRTUALFUNCTIONPOINTER, VirtualFunctionPointer, NoSig) diff --git a/src/coreclr/vm/jithelpers.cpp b/src/coreclr/vm/jithelpers.cpp index 88bb4981d65729..faa1276e5e3e17 100644 --- a/src/coreclr/vm/jithelpers.cpp +++ b/src/coreclr/vm/jithelpers.cpp @@ -1382,24 +1382,6 @@ HCIMPL2(Object*, JIT_Box, CORINFO_CLASS_HANDLE type, void* unboxedData) } HCIMPLEND -/*************************************************************/ -HCIMPL2_IV(LPVOID, JIT_GetRefAny, CORINFO_CLASS_HANDLE type, TypedByRef typedByRef) -{ - FCALL_CONTRACT; - - TypeHandle clsHnd(type); - // @TODO right now we check for precisely the correct type. - // do we want to allow inheritance? (watch out since value - // classes inherit from object but do not normal object layout). - if (clsHnd != typedByRef.type) { - FCThrow(kInvalidCastException); - } - - return(typedByRef.data); -} -HCIMPLEND - - /*************************************************************/ HCIMPL2(BOOL, JIT_IsInstanceOfException, CORINFO_CLASS_HANDLE type, Object* obj) { From dc3c90b8e389a2942dc65fd882c01170705a1089 Mon Sep 17 00:00:00 2001 From: Adeel Mujahid <3840695+am11@users.noreply.github.com> Date: Tue, 3 Dec 2024 19:02:55 +0200 Subject: [PATCH 2/5] Apply suggestions from code review --- .../src/System/Runtime/CompilerServices/CastHelpers.cs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs index 61aac42a16ae6a..2f363eceb0a117 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs @@ -26,7 +26,7 @@ internal static void ThrowInvalidCastException(object fromType, void* toTypeHnd) } [DoesNotReturn] - internal static void ThrowInvalidCastException() => throw new InvalidCastException(); + private static void ThrowInvalidCastException() => throw new InvalidCastException(); [MethodImpl(MethodImplOptions.InternalCall)] private static extern object IsInstanceOfAny_NoCacheLookup(void* toTypeHnd, object obj); @@ -39,9 +39,6 @@ internal static void ThrowInvalidCastException(object fromType, void* toTypeHnd) internal static ref byte GetRefAny(IntPtr clsHnd, TypedReference typedByRef) { - // @TODO right now we check for precisely the correct type. - // do we want to allow inheritance? (watch out since value - // classes inherit from object but do not normal object layout). if (clsHnd != typedByRef.Type) { ThrowInvalidCastException(); From e3eb068446cb6cd18a3dbcbba50df6c1aa3e749d Mon Sep 17 00:00:00 2001 From: Adeel Mujahid <3840695+am11@users.noreply.github.com> Date: Tue, 3 Dec 2024 23:43:38 +0200 Subject: [PATCH 3/5] Move impl. to TypeReference --- .../System/Runtime/CompilerServices/CastHelpers.cs | 14 -------------- .../src/System/TypedReference.CoreCLR.cs | 12 ++++++++++-- src/coreclr/inc/jithelpers.h | 2 +- src/coreclr/vm/binder.h | 1 - src/coreclr/vm/corelib.h | 4 +++- 5 files changed, 14 insertions(+), 19 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs index 2f363eceb0a117..202a339b680251 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs @@ -1,7 +1,6 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Runtime.InteropServices; @@ -25,9 +24,6 @@ internal static void ThrowInvalidCastException(object fromType, void* toTypeHnd) throw null!; // Provide hint to the inliner that this method does not return } - [DoesNotReturn] - private static void ThrowInvalidCastException() => throw new InvalidCastException(); - [MethodImpl(MethodImplOptions.InternalCall)] private static extern object IsInstanceOfAny_NoCacheLookup(void* toTypeHnd, object obj); @@ -37,16 +33,6 @@ internal static void ThrowInvalidCastException(object fromType, void* toTypeHnd) [MethodImpl(MethodImplOptions.InternalCall)] private static extern void WriteBarrier(ref object? dst, object? obj); - internal static ref byte GetRefAny(IntPtr clsHnd, TypedReference typedByRef) - { - if (clsHnd != typedByRef.Type) - { - ThrowInvalidCastException(); - } - - return ref typedByRef.Value; - } - // IsInstanceOf test used for unusual cases (naked type parameters, variant generic types) // Unlike the IsInstanceOfInterface and IsInstanceOfClass functions, // this test must deal with all kinds of type tests diff --git a/src/coreclr/System.Private.CoreLib/src/System/TypedReference.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/TypedReference.CoreCLR.cs index 3c93e45894090b..a0c60c7c0ce02d 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/TypedReference.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/TypedReference.CoreCLR.cs @@ -16,8 +16,16 @@ public ref partial struct TypedReference private readonly ref byte _value; private readonly IntPtr _type; - internal readonly IntPtr Type => _type; - internal readonly ref byte Value => ref _value; + // implementation of CORINFO_HELP_GETREFANY + internal static ref byte GetRefAny(IntPtr clsHnd, TypedReference typedByRef) + { + if (clsHnd != typedByRef._type) + { + throw new InvalidCastException(); + } + + return ref typedByRef._value; + } private TypedReference(ref byte target, RuntimeType type) { diff --git a/src/coreclr/inc/jithelpers.h b/src/coreclr/inc/jithelpers.h index 6d0d7950d9a31d..8b97677ea11f91 100644 --- a/src/coreclr/inc/jithelpers.h +++ b/src/coreclr/inc/jithelpers.h @@ -117,7 +117,7 @@ DYNAMICJITHELPER(CORINFO_HELP_UNBOX_TYPETEST,NULL, METHOD__CASTHELPERS__UNBOX_TYPETEST) DYNAMICJITHELPER(CORINFO_HELP_UNBOX_NULLABLE,NULL, METHOD__CASTHELPERS__UNBOX_NULLABLE) - DYNAMICJITHELPER(CORINFO_HELP_GETREFANY, NULL, METHOD__CASTHELPERS__GETREFANY) + DYNAMICJITHELPER(CORINFO_HELP_GETREFANY, NULL, METHOD__TYPED_REFERENCE__GETREFANY) DYNAMICJITHELPER(CORINFO_HELP_ARRADDR_ST, NULL, METHOD__CASTHELPERS__STELEMREF) DYNAMICJITHELPER(CORINFO_HELP_LDELEMA_REF, NULL, METHOD__CASTHELPERS__LDELEMAREF) diff --git a/src/coreclr/vm/binder.h b/src/coreclr/vm/binder.h index 8e9369cc85a1b0..cadcdc6ddac89c 100644 --- a/src/coreclr/vm/binder.h +++ b/src/coreclr/vm/binder.h @@ -65,7 +65,6 @@ enum BinderClassID CLASS__SINGLE = CLASS__ELEMENT_TYPE_R4, CLASS__DOUBLE = CLASS__ELEMENT_TYPE_R8, CLASS__STRING = CLASS__ELEMENT_TYPE_STRING, - CLASS__TYPED_REFERENCE = CLASS__ELEMENT_TYPE_TYPEDBYREF, CLASS__INTPTR = CLASS__ELEMENT_TYPE_I, CLASS__UINTPTR = CLASS__ELEMENT_TYPE_U, CLASS__OBJECT = CLASS__ELEMENT_TYPE_OBJECT diff --git a/src/coreclr/vm/corelib.h b/src/coreclr/vm/corelib.h index bbadd4da7c6e52..b3555048a3c669 100644 --- a/src/coreclr/vm/corelib.h +++ b/src/coreclr/vm/corelib.h @@ -344,6 +344,9 @@ DEFINE_METHOD(RT_TYPE_HANDLE, ALLOCATECOMOBJECT, AllocateComObject, #endif DEFINE_FIELD(RT_TYPE_HANDLE, M_TYPE, m_type) +DEFINE_CLASS(TYPED_REFERENCE, System, TypedReference) +DEFINE_METHOD(TYPED_REFERENCE, GETREFANY, GetRefAny, NoSig) + DEFINE_CLASS(TYPE_NAME_RESOLVER, Reflection, TypeNameResolver) DEFINE_METHOD(TYPE_NAME_RESOLVER, GET_TYPE_HELPER, GetTypeHelper, SM_Type_CharPtr_RuntimeAssembly_Bool_Bool_RetRuntimeType) @@ -1180,7 +1183,6 @@ DEFINE_METHOD(CASTHELPERS, LDELEMAREF, LdelemaRef, SM_Arr DEFINE_METHOD(CASTHELPERS, ARRAYTYPECHECK, ArrayTypeCheck, SM_Obj_Array_RetVoid) DEFINE_METHOD(CASTHELPERS, UNBOX_NULLABLE, Unbox_Nullable, NoSig) DEFINE_METHOD(CASTHELPERS, UNBOX_TYPETEST, Unbox_TypeTest, NoSig) -DEFINE_METHOD(CASTHELPERS, GETREFANY, GetRefAny, NoSig) DEFINE_CLASS(VIRTUALDISPATCHHELPERS, CompilerServices, VirtualDispatchHelpers) DEFINE_METHOD(VIRTUALDISPATCHHELPERS, VIRTUALFUNCTIONPOINTER, VirtualFunctionPointer, NoSig) From 3015a558303cd2e4cd8b67c0d046a2c1c39c5188 Mon Sep 17 00:00:00 2001 From: Adeel <3840695+am11@users.noreply.github.com> Date: Wed, 4 Dec 2024 00:26:33 +0200 Subject: [PATCH 4/5] fb --- .../src/System/TypedReference.CoreCLR.cs | 7 ++++++- src/coreclr/vm/binder.h | 1 + src/coreclr/vm/corelib.h | 2 +- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/TypedReference.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/TypedReference.CoreCLR.cs index a0c60c7c0ce02d..4498a6049e352b 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/TypedReference.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/TypedReference.CoreCLR.cs @@ -5,6 +5,7 @@ // These are blob that must be dealt with by the compiler. using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.Runtime.CompilerServices; using System.Runtime.Versioning; @@ -17,14 +18,18 @@ public ref partial struct TypedReference private readonly IntPtr _type; // implementation of CORINFO_HELP_GETREFANY + [StackTraceHidden] internal static ref byte GetRefAny(IntPtr clsHnd, TypedReference typedByRef) { if (clsHnd != typedByRef._type) { - throw new InvalidCastException(); + ThrowInvalidCastException(); } return ref typedByRef._value; + + [DoesNotReturn] + static void ThrowInvalidCastException() => throw new InvalidCastException(); } private TypedReference(ref byte target, RuntimeType type) diff --git a/src/coreclr/vm/binder.h b/src/coreclr/vm/binder.h index cadcdc6ddac89c..8e9369cc85a1b0 100644 --- a/src/coreclr/vm/binder.h +++ b/src/coreclr/vm/binder.h @@ -65,6 +65,7 @@ enum BinderClassID CLASS__SINGLE = CLASS__ELEMENT_TYPE_R4, CLASS__DOUBLE = CLASS__ELEMENT_TYPE_R8, CLASS__STRING = CLASS__ELEMENT_TYPE_STRING, + CLASS__TYPED_REFERENCE = CLASS__ELEMENT_TYPE_TYPEDBYREF, CLASS__INTPTR = CLASS__ELEMENT_TYPE_I, CLASS__UINTPTR = CLASS__ELEMENT_TYPE_U, CLASS__OBJECT = CLASS__ELEMENT_TYPE_OBJECT diff --git a/src/coreclr/vm/corelib.h b/src/coreclr/vm/corelib.h index b3555048a3c669..7a5fe8d99743b2 100644 --- a/src/coreclr/vm/corelib.h +++ b/src/coreclr/vm/corelib.h @@ -344,7 +344,7 @@ DEFINE_METHOD(RT_TYPE_HANDLE, ALLOCATECOMOBJECT, AllocateComObject, #endif DEFINE_FIELD(RT_TYPE_HANDLE, M_TYPE, m_type) -DEFINE_CLASS(TYPED_REFERENCE, System, TypedReference) +// DEFINE_CLASS(TYPED_REFERENCE, System, TypedReference) DEFINE_METHOD(TYPED_REFERENCE, GETREFANY, GetRefAny, NoSig) DEFINE_CLASS(TYPE_NAME_RESOLVER, Reflection, TypeNameResolver) From 2969b3b9627f26eb7cbbb3186963550f78208ab1 Mon Sep 17 00:00:00 2001 From: Adeel Mujahid <3840695+am11@users.noreply.github.com> Date: Wed, 4 Dec 2024 01:10:45 +0200 Subject: [PATCH 5/5] Update src/coreclr/System.Private.CoreLib/src/System/TypedReference.CoreCLR.cs --- .../System.Private.CoreLib/src/System/TypedReference.CoreCLR.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/coreclr/System.Private.CoreLib/src/System/TypedReference.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/TypedReference.CoreCLR.cs index 4498a6049e352b..5ae74e571295e9 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/TypedReference.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/TypedReference.CoreCLR.cs @@ -29,6 +29,7 @@ internal static ref byte GetRefAny(IntPtr clsHnd, TypedReference typedByRef) return ref typedByRef._value; [DoesNotReturn] + [StackTraceHidden] static void ThrowInvalidCastException() => throw new InvalidCastException(); }