From 198de589348a2e6b0ac59892adba8aa107cbde45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Petryka?= <35800402+MichalPetryka@users.noreply.github.com> Date: Thu, 14 Mar 2024 02:03:58 +0100 Subject: [PATCH 01/12] Remove UB from helpers --- .../Runtime/CompilerServices/CastHelpers.cs | 27 +++++++++++++------ 1 file changed, 19 insertions(+), 8 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 5f6c7070958ac..4c7f609902c61 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 @@ -395,9 +395,12 @@ private static ref byte Unbox(void* toTypeHnd, object obj) return ref Unbox_Helper(toTypeHnd, obj); } - internal struct ArrayElement + [DebuggerHidden] + [StackTraceHidden] + [DebuggerStepThrough] + private static void ThrowIndexOutOfRangeException() { - public object? Value; + throw new IndexOutOfRangeException(); } [DebuggerHidden] @@ -413,9 +416,13 @@ internal struct ArrayElement [DebuggerStepThrough] private static ref object? LdelemaRef(Array array, nint index, void* type) { - // this will throw appropriate exceptions if array is null or access is out of range. - ref object? element = ref Unsafe.As(array)[index].Value; - void* elementType = RuntimeHelpers.GetMethodTable(array)->ElementType; + Debug.Assert(array is object[]); + object[] a = Unsafe.As(array); + if (index > a.Length) + ThrowIndexOutOfRangeException(); + + ref object? element = ref Unsafe.Add(ref MemoryMarshal.GetArrayDataReference(a), index); + void* elementType = RuntimeHelpers.GetMethodTable(a)->ElementType; if (elementType == type) return ref element; @@ -428,9 +435,13 @@ internal struct ArrayElement [DebuggerStepThrough] private static void StelemRef(Array array, nint index, object? obj) { - // this will throw appropriate exceptions if array is null or access is out of range. - ref object? element = ref Unsafe.As(array)[index].Value; - void* elementType = RuntimeHelpers.GetMethodTable(array)->ElementType; + Debug.Assert(array is object[]); + object[] a = Unsafe.As(array); + if (index > a.Length) + ThrowIndexOutOfRangeException(); + + ref object? element = ref Unsafe.Add(ref MemoryMarshal.GetArrayDataReference(a), index); + void* elementType = RuntimeHelpers.GetMethodTable(a)->ElementType; if (obj == null) goto assigningNull; From 9d72f6c3c224e7d8ca342647f68203f3cd34f16e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Petryka?= <35800402+MichalPetryka@users.noreply.github.com> Date: Thu, 14 Mar 2024 03:07:28 +0100 Subject: [PATCH 02/12] Update CastHelpers.cs --- .../Runtime/CompilerServices/CastHelpers.cs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 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 4c7f609902c61..aa5535d043950 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 @@ -416,11 +416,11 @@ private static void ThrowIndexOutOfRangeException() [DebuggerStepThrough] private static ref object? LdelemaRef(Array array, nint index, void* type) { - Debug.Assert(array is object[]); - object[] a = Unsafe.As(array); - if (index > a.Length) + Debug.Assert(array is null or object?[]); + object?[] a = Unsafe.As(array); + if ((nuint)index >= (uint)a.Length) ThrowIndexOutOfRangeException(); - + ref object? element = ref Unsafe.Add(ref MemoryMarshal.GetArrayDataReference(a), index); void* elementType = RuntimeHelpers.GetMethodTable(a)->ElementType; @@ -435,11 +435,11 @@ private static void ThrowIndexOutOfRangeException() [DebuggerStepThrough] private static void StelemRef(Array array, nint index, object? obj) { - Debug.Assert(array is object[]); - object[] a = Unsafe.As(array); - if (index > a.Length) + Debug.Assert(array is null or object?[]); + object?[] a = Unsafe.As(array); + if ((nuint)index >= (uint)a.Length) ThrowIndexOutOfRangeException(); - + ref object? element = ref Unsafe.Add(ref MemoryMarshal.GetArrayDataReference(a), index); void* elementType = RuntimeHelpers.GetMethodTable(a)->ElementType; From e4b716eb0d67f5574e6e2a52abb9b903a19808d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Petryka?= <35800402+MichalPetryka@users.noreply.github.com> Date: Thu, 14 Mar 2024 16:42:15 +0100 Subject: [PATCH 03/12] Update CastHelpers.cs --- .../src/System/Runtime/CompilerServices/CastHelpers.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 aa5535d043950..2d64fc1e59045 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 @@ -458,7 +458,7 @@ private static void StelemRef(Array array, nint index, object? obj) return; notExactMatch: - if (array.GetType() == typeof(object[])) + if (a.GetType() == typeof(object[])) goto doWrite; StelemRef_Helper(ref element, elementType, obj); From 097207a380a63dac67c3aacf1f4b16d91ad28fe0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Petryka?= Date: Sat, 16 Mar 2024 20:51:33 +0100 Subject: [PATCH 04/12] Cleanup more helpers --- .../Runtime/CompilerServices/CastHelpers.cs | 78 ++++--------- .../src/System/Runtime/InternalCalls.cs | 2 +- .../src/System/Runtime/TypeCast.cs | 105 ++++++++++-------- .../Execution/TypeLoader/TypeCast.cs | 3 + 4 files changed, 82 insertions(+), 106 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 2d64fc1e59045..942b5ff3fb080 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 @@ -2,13 +2,12 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Diagnostics; -using System.Numerics; -using System.Runtime.CompilerServices; using System.Runtime.InteropServices; -using System.Threading; namespace System.Runtime.CompilerServices { + [StackTraceHidden] + [DebuggerStepThrough] internal static unsafe class CastHelpers { // In coreclr the table is allocated and written to on the native side. @@ -24,14 +23,12 @@ internal static unsafe class CastHelpers private static extern ref byte Unbox_Helper(void* toTypeHnd, object obj); [MethodImpl(MethodImplOptions.InternalCall)] - private static extern void WriteBarrier(ref object? dst, object obj); + private static extern void WriteBarrier(ref object? dst, object? obj); // 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 [DebuggerHidden] - [StackTraceHidden] - [DebuggerStepThrough] private static object? IsInstanceOfAny(void* toTypeHnd, object? obj) { if (obj != null) @@ -63,8 +60,6 @@ internal static unsafe class CastHelpers } [DebuggerHidden] - [StackTraceHidden] - [DebuggerStepThrough] private static object? IsInstanceOfInterface(void* toTypeHnd, object? obj) { const int unrollSize = 4; @@ -134,8 +129,6 @@ internal static unsafe class CastHelpers } [DebuggerHidden] - [StackTraceHidden] - [DebuggerStepThrough] private static object? IsInstanceOfClass(void* toTypeHnd, object? obj) { if (obj == null || RuntimeHelpers.GetMethodTable(obj) == toTypeHnd) @@ -184,8 +177,6 @@ internal static unsafe class CastHelpers } [DebuggerHidden] - [StackTraceHidden] - [DebuggerStepThrough] [MethodImpl(MethodImplOptions.NoInlining)] private static object? IsInstance_Helper(void* toTypeHnd, object obj) { @@ -207,8 +198,6 @@ internal static unsafe class CastHelpers // Unlike the ChkCastInterface and ChkCastClass functions, // this test must deal with all kinds of type tests [DebuggerHidden] - [StackTraceHidden] - [DebuggerStepThrough] internal static object? ChkCastAny(void* toTypeHnd, object? obj) { CastResult result; @@ -237,8 +226,6 @@ internal static unsafe class CastHelpers } [DebuggerHidden] - [StackTraceHidden] - [DebuggerStepThrough] [MethodImpl(MethodImplOptions.NoInlining)] private static object? ChkCast_Helper(void* toTypeHnd, object obj) { @@ -253,8 +240,6 @@ internal static unsafe class CastHelpers } [DebuggerHidden] - [StackTraceHidden] - [DebuggerStepThrough] private static object? ChkCastInterface(void* toTypeHnd, object? obj) { const int unrollSize = 4; @@ -321,8 +306,6 @@ internal static unsafe class CastHelpers } [DebuggerHidden] - [StackTraceHidden] - [DebuggerStepThrough] private static object? ChkCastClass(void* toTypeHnd, object? obj) { if (obj == null || RuntimeHelpers.GetMethodTable(obj) == toTypeHnd) @@ -336,8 +319,6 @@ internal static unsafe class CastHelpers // Optimized helper for classes. Assumes that the trivial cases // has been taken care of by the inlined check [DebuggerHidden] - [StackTraceHidden] - [DebuggerStepThrough] private static object? ChkCastClassSpecial(void* toTypeHnd, object obj) { MethodTable* mt = RuntimeHelpers.GetMethodTable(obj); @@ -384,8 +365,6 @@ internal static unsafe class CastHelpers } [DebuggerHidden] - [StackTraceHidden] - [DebuggerStepThrough] private static ref byte Unbox(void* toTypeHnd, object obj) { // this will throw NullReferenceException if obj is null, attributed to the user code, as expected. @@ -396,52 +375,42 @@ private static ref byte Unbox(void* toTypeHnd, object obj) } [DebuggerHidden] - [StackTraceHidden] - [DebuggerStepThrough] private static void ThrowIndexOutOfRangeException() { throw new IndexOutOfRangeException(); } [DebuggerHidden] - [StackTraceHidden] - [DebuggerStepThrough] - private static ref object? ThrowArrayMismatchException() + private static void ThrowArrayMismatchException() { throw new ArrayTypeMismatchException(); } [DebuggerHidden] - [StackTraceHidden] - [DebuggerStepThrough] - private static ref object? LdelemaRef(Array array, nint index, void* type) + private static ref object? LdelemaRef(object?[]? array, nint index, void* type) { - Debug.Assert(array is null or object?[]); - object?[] a = Unsafe.As(array); - if ((nuint)index >= (uint)a.Length) + if ((nuint)index >= (uint)array!.Length) ThrowIndexOutOfRangeException(); - ref object? element = ref Unsafe.Add(ref MemoryMarshal.GetArrayDataReference(a), index); - void* elementType = RuntimeHelpers.GetMethodTable(a)->ElementType; + Debug.Assert(index >= 0); + ref object? element = ref Unsafe.Add(ref MemoryMarshal.GetArrayDataReference(array), index); + void* elementType = RuntimeHelpers.GetMethodTable(array)->ElementType; - if (elementType == type) - return ref element; + if (elementType != type) + ThrowArrayMismatchException(); - return ref ThrowArrayMismatchException(); + return ref element; } [DebuggerHidden] - [StackTraceHidden] - [DebuggerStepThrough] - private static void StelemRef(Array array, nint index, object? obj) + private static void StelemRef(object?[]? array, nint index, object? obj) { - Debug.Assert(array is null or object?[]); - object?[] a = Unsafe.As(array); - if ((nuint)index >= (uint)a.Length) + if ((nuint)index >= (uint)array!.Length) ThrowIndexOutOfRangeException(); - ref object? element = ref Unsafe.Add(ref MemoryMarshal.GetArrayDataReference(a), index); - void* elementType = RuntimeHelpers.GetMethodTable(a)->ElementType; + Debug.Assert(index >= 0); + ref object? element = ref Unsafe.Add(ref MemoryMarshal.GetArrayDataReference(array), index); + void* elementType = RuntimeHelpers.GetMethodTable(array)->ElementType; if (obj == null) goto assigningNull; @@ -458,15 +427,13 @@ private static void StelemRef(Array array, nint index, object? obj) return; notExactMatch: - if (a.GetType() == typeof(object[])) + if (array.GetType() == typeof(object[])) goto doWrite; StelemRef_Helper(ref element, elementType, obj); } [DebuggerHidden] - [StackTraceHidden] - [DebuggerStepThrough] [MethodImpl(MethodImplOptions.NoInlining)] private static void StelemRef_Helper(ref object? element, void* elementType, object obj) { @@ -481,20 +448,17 @@ private static void StelemRef_Helper(ref object? element, void* elementType, obj } [DebuggerHidden] - [StackTraceHidden] - [DebuggerStepThrough] private static void StelemRef_Helper_NoCacheLookup(ref object? element, void* elementType, object obj) { Debug.Assert(obj != null); obj = IsInstanceOfAny_NoCacheLookup(elementType, obj); - if (obj != null) + if (obj == null) { - WriteBarrier(ref element, obj); - return; + ThrowArrayMismatchException(); } - throw new ArrayTypeMismatchException(); + WriteBarrier(ref element, obj); } } } diff --git a/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/InternalCalls.cs b/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/InternalCalls.cs index 8f5098ab31adc..7ea73ba7c2c38 100644 --- a/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/InternalCalls.cs +++ b/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/InternalCalls.cs @@ -150,7 +150,7 @@ internal static int RhEndNoGCRegion() [RuntimeImport(Redhawk.BaseName, "RhpAssignRef")] [MethodImpl(MethodImplOptions.InternalCall)] - internal static extern unsafe void RhpAssignRef(ref object address, object obj); + internal static extern unsafe void RhpAssignRef(ref object? address, object? obj); [MethodImplAttribute(MethodImplOptions.InternalCall)] [RuntimeImport(Redhawk.BaseName, "RhpGcSafeZeroMemory")] diff --git a/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/TypeCast.cs b/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/TypeCast.cs index 3dcf2bae02f24..8371a9f62d4fd 100644 --- a/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/TypeCast.cs +++ b/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/TypeCast.cs @@ -22,6 +22,8 @@ namespace System.Runtime // ///////////////////////////////////////////////////////////////////////////////////////////////////// + [StackTraceHidden] + [DebuggerStepThrough] [EagerStaticClassConstruction] internal static class TypeCast { @@ -737,23 +739,70 @@ public static unsafe void CheckArrayStore(object array, object obj) throw array.GetMethodTable()->GetClasslibException(ExceptionIDs.ArrayTypeMismatch); } - internal struct ArrayElement + private static unsafe void ThrowIndexOutOfRangeException(object?[] array) { - public object Value; + // Throw the index out of range exception defined by the classlib, using the input array's MethodTable* + // to find the correct classlib. + throw array.GetMethodTable()->GetClasslibException(ExceptionIDs.IndexOutOfRange); + } + + private static unsafe void ThrowArrayMismatchException(object?[] array) + { + // Throw the array type mismatch exception defined by the classlib, using the input array's MethodTable* + // to find the correct classlib. + throw array.GetMethodTable()->GetClasslibException(ExceptionIDs.ArrayTypeMismatch); } // // Array stelem/ldelema helpers with RyuJIT conventions // + + [RuntimeExport("RhpLdelemaRef")] + public static unsafe ref object? LdelemaRef(object?[]? array, nint index, IntPtr elementType) + { + Debug.Assert(array is null || array.GetMethodTable()->IsArray, "first argument must be an array"); + +#if INPLACE_RUNTIME + if ((nuint)index >= (uint)array.Length) + ThrowIndexOutOfRangeException(array); + + Debug.Assert(index >= 0); + ref object? element = ref Unsafe.Add(ref MemoryMarshal.GetArrayDataReference(array), index); +#else + if (array is null) + { + throw ((MethodTable*)elementType)->GetClasslibException(ExceptionIDs.NullReference); + } + if ((uint)index >= (uint)array.Length) + { + throw ((MethodTable*)elementType)->GetClasslibException(ExceptionIDs.IndexOutOfRange); + } + ref object rawData = ref Unsafe.As(ref Unsafe.As(array).Data); + ref object element = ref Unsafe.Add(ref rawData, index); +#endif + + MethodTable* elemType = (MethodTable*)elementType; + MethodTable* arrayElemType = array.GetMethodTable()->RelatedParameterType; + + if (elemType != arrayElemType) + ThrowArrayMismatchException(array); + + return ref element; + + } + [RuntimeExport("RhpStelemRef")] - public static unsafe void StelemRef(Array array, nint index, object obj) + public static unsafe void StelemRef(object?[]? array, nint index, object? obj) { // This is supported only on arrays - Debug.Assert(array.GetMethodTable()->IsArray, "first argument must be an array"); + Debug.Assert(array is null || array.GetMethodTable()->IsArray, "first argument must be an array"); #if INPLACE_RUNTIME - // this will throw appropriate exceptions if array is null or access is out of range. - ref object element = ref Unsafe.As(array)[index].Value; + if ((nuint)index >= (uint)array.Length) + ThrowIndexOutOfRangeException(array); + + Debug.Assert(index >= 0); + ref object? element = ref Unsafe.Add(ref MemoryMarshal.GetArrayDataReference(array), index); #else if (array is null) { @@ -796,7 +845,7 @@ public static unsafe void StelemRef(Array array, nint index, object obj) } [MethodImpl(MethodImplOptions.NoInlining)] - private static unsafe void StelemRef_Helper(ref object element, MethodTable* elementType, object obj) + private static unsafe void StelemRef_Helper(ref object? element, MethodTable* elementType, object obj) { CastResult result = s_castCache.TryGet((nuint)obj.GetMethodTable() + (int)AssignmentVariation.BoxedSource, (nuint)elementType); if (result == CastResult.CanCast) @@ -808,7 +857,7 @@ private static unsafe void StelemRef_Helper(ref object element, MethodTable* ele StelemRef_Helper_NoCacheLookup(ref element, elementType, obj); } - private static unsafe void StelemRef_Helper_NoCacheLookup(ref object element, MethodTable* elementType, object obj) + private static unsafe void StelemRef_Helper_NoCacheLookup(ref object? element, MethodTable* elementType, object obj) { object? castedObj = IsInstanceOfAny_NoCacheLookup(elementType, obj); if (castedObj != null) @@ -822,46 +871,6 @@ private static unsafe void StelemRef_Helper_NoCacheLookup(ref object element, Me throw elementType->GetClasslibException(ExceptionIDs.ArrayTypeMismatch); } - [RuntimeExport("RhpLdelemaRef")] - public static unsafe ref object LdelemaRef(Array array, nint index, IntPtr elementType) - { - Debug.Assert(array is null || array.GetMethodTable()->IsArray, "first argument must be an array"); - -#if INPLACE_RUNTIME - // this will throw appropriate exceptions if array is null or access is out of range. - ref object element = ref Unsafe.As(array)[index].Value; -#else - if (array is null) - { - throw ((MethodTable*)elementType)->GetClasslibException(ExceptionIDs.NullReference); - } - if ((uint)index >= (uint)array.Length) - { - throw ((MethodTable*)elementType)->GetClasslibException(ExceptionIDs.IndexOutOfRange); - } - ref object rawData = ref Unsafe.As(ref Unsafe.As(array).Data); - ref object element = ref Unsafe.Add(ref rawData, index); -#endif - - MethodTable* elemType = (MethodTable*)elementType; - MethodTable* arrayElemType = array.GetMethodTable()->RelatedParameterType; - - if (elemType == arrayElemType) - { - return ref element; - } - - return ref ThrowArrayMismatchException(array); - } - - // This weird structure is for parity with CoreCLR - allows potentially to be tailcalled - private static unsafe ref object ThrowArrayMismatchException(Array array) - { - // Throw the array type mismatch exception defined by the classlib, using the input array's MethodTable* - // to find the correct classlib. - throw array.GetMethodTable()->GetClasslibException(ExceptionIDs.ArrayTypeMismatch); - } - private static unsafe object IsInstanceOfArray(MethodTable* pTargetType, object obj) { MethodTable* pObjType = obj.GetMethodTable(); diff --git a/src/coreclr/nativeaot/System.Private.Reflection.Execution/src/Internal/Reflection/Execution/TypeLoader/TypeCast.cs b/src/coreclr/nativeaot/System.Private.Reflection.Execution/src/Internal/Reflection/Execution/TypeLoader/TypeCast.cs index c6baf2e2a3bae..b07b4da849ac7 100644 --- a/src/coreclr/nativeaot/System.Private.Reflection.Execution/src/Internal/Reflection/Execution/TypeLoader/TypeCast.cs +++ b/src/coreclr/nativeaot/System.Private.Reflection.Execution/src/Internal/Reflection/Execution/TypeLoader/TypeCast.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Reflection; @@ -21,6 +22,8 @@ namespace Internal.Reflection.Execution ///////////////////////////////////////////////////////////////////////////////////////////////////// // This is not a general purpose type comparison facility. It is limited to what constraint validation needs. + [StackTraceHidden] + [DebuggerStepThrough] internal static partial class ConstraintValidator { [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2070:UnrecognizedReflectionPattern", From 41608a5268cee00a80cb83c3b8269c22221704e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Petryka?= Date: Sat, 16 Mar 2024 20:58:11 +0100 Subject: [PATCH 05/12] Further cleanup --- .../Runtime.Base/src/System/Runtime/TypeCast.cs | 13 +++++-------- .../Reflection/Execution/TypeLoader/TypeCast.cs | 2 -- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/TypeCast.cs b/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/TypeCast.cs index 8371a9f62d4fd..6e2762301e7a7 100644 --- a/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/TypeCast.cs +++ b/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/TypeCast.cs @@ -758,7 +758,7 @@ private static unsafe void ThrowArrayMismatchException(object?[] array) // [RuntimeExport("RhpLdelemaRef")] - public static unsafe ref object? LdelemaRef(object?[]? array, nint index, IntPtr elementType) + public static unsafe ref object? LdelemaRef(object?[]? array, nint index, MethodTable* elementType) { Debug.Assert(array is null || array.GetMethodTable()->IsArray, "first argument must be an array"); @@ -771,24 +771,21 @@ private static unsafe void ThrowArrayMismatchException(object?[] array) #else if (array is null) { - throw ((MethodTable*)elementType)->GetClasslibException(ExceptionIDs.NullReference); + throw elementType->GetClasslibException(ExceptionIDs.NullReference); } - if ((uint)index >= (uint)array.Length) + if ((nuint)index >= (uint)array.Length) { - throw ((MethodTable*)elementType)->GetClasslibException(ExceptionIDs.IndexOutOfRange); + throw elementType->GetClasslibException(ExceptionIDs.IndexOutOfRange); } ref object rawData = ref Unsafe.As(ref Unsafe.As(array).Data); ref object element = ref Unsafe.Add(ref rawData, index); #endif - - MethodTable* elemType = (MethodTable*)elementType; MethodTable* arrayElemType = array.GetMethodTable()->RelatedParameterType; - if (elemType != arrayElemType) + if (elementType != arrayElemType) ThrowArrayMismatchException(array); return ref element; - } [RuntimeExport("RhpStelemRef")] diff --git a/src/coreclr/nativeaot/System.Private.Reflection.Execution/src/Internal/Reflection/Execution/TypeLoader/TypeCast.cs b/src/coreclr/nativeaot/System.Private.Reflection.Execution/src/Internal/Reflection/Execution/TypeLoader/TypeCast.cs index b07b4da849ac7..9cd60dec778a9 100644 --- a/src/coreclr/nativeaot/System.Private.Reflection.Execution/src/Internal/Reflection/Execution/TypeLoader/TypeCast.cs +++ b/src/coreclr/nativeaot/System.Private.Reflection.Execution/src/Internal/Reflection/Execution/TypeLoader/TypeCast.cs @@ -22,8 +22,6 @@ namespace Internal.Reflection.Execution ///////////////////////////////////////////////////////////////////////////////////////////////////// // This is not a general purpose type comparison facility. It is limited to what constraint validation needs. - [StackTraceHidden] - [DebuggerStepThrough] internal static partial class ConstraintValidator { [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2070:UnrecognizedReflectionPattern", From 55aaf7da0e5307f6f4365be298817eb32df2e2ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Petryka?= Date: Sat, 16 Mar 2024 20:59:32 +0100 Subject: [PATCH 06/12] Remove using --- .../src/Internal/Reflection/Execution/TypeLoader/TypeCast.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/coreclr/nativeaot/System.Private.Reflection.Execution/src/Internal/Reflection/Execution/TypeLoader/TypeCast.cs b/src/coreclr/nativeaot/System.Private.Reflection.Execution/src/Internal/Reflection/Execution/TypeLoader/TypeCast.cs index 9cd60dec778a9..c6baf2e2a3bae 100644 --- a/src/coreclr/nativeaot/System.Private.Reflection.Execution/src/Internal/Reflection/Execution/TypeLoader/TypeCast.cs +++ b/src/coreclr/nativeaot/System.Private.Reflection.Execution/src/Internal/Reflection/Execution/TypeLoader/TypeCast.cs @@ -3,7 +3,6 @@ using System; using System.Collections.Generic; -using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Reflection; From ea23fe4fab84e6cb4936a172b7a1fe529ce542f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Petryka?= Date: Sat, 16 Mar 2024 21:38:37 +0100 Subject: [PATCH 07/12] Fix Test.CoreLib --- .../DebuggerStepThroughAttribute.cs | 11 +++++++++++ .../Diagnostics/StackTraceHiddenAttribute.cs | 18 ++++++++++++++++++ .../Test.CoreLib/src/Test.CoreLib.csproj | 2 ++ 3 files changed, 31 insertions(+) create mode 100644 src/coreclr/nativeaot/Test.CoreLib/src/System/Diagnostics/DebuggerStepThroughAttribute.cs create mode 100644 src/coreclr/nativeaot/Test.CoreLib/src/System/Diagnostics/StackTraceHiddenAttribute.cs diff --git a/src/coreclr/nativeaot/Test.CoreLib/src/System/Diagnostics/DebuggerStepThroughAttribute.cs b/src/coreclr/nativeaot/Test.CoreLib/src/System/Diagnostics/DebuggerStepThroughAttribute.cs new file mode 100644 index 0000000000000..732a27d5da64b --- /dev/null +++ b/src/coreclr/nativeaot/Test.CoreLib/src/System/Diagnostics/DebuggerStepThroughAttribute.cs @@ -0,0 +1,11 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace System.Diagnostics +{ + [AttributeUsage(AttributeTargets.Class | AttributeTargets.Struct | AttributeTargets.Method | AttributeTargets.Constructor, Inherited = false)] + public sealed class DebuggerStepThroughAttribute : Attribute + { + public DebuggerStepThroughAttribute() { } + } +} diff --git a/src/coreclr/nativeaot/Test.CoreLib/src/System/Diagnostics/StackTraceHiddenAttribute.cs b/src/coreclr/nativeaot/Test.CoreLib/src/System/Diagnostics/StackTraceHiddenAttribute.cs new file mode 100644 index 0000000000000..cc3efc2c154d6 --- /dev/null +++ b/src/coreclr/nativeaot/Test.CoreLib/src/System/Diagnostics/StackTraceHiddenAttribute.cs @@ -0,0 +1,18 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace System.Diagnostics +{ + /// + /// Types and Methods attributed with StackTraceHidden will be omitted from the stack trace text shown in StackTrace.ToString() + /// and Exception.StackTrace + /// + [AttributeUsage(AttributeTargets.Class | AttributeTargets.Method | AttributeTargets.Constructor | AttributeTargets.Struct, Inherited = false)] + public sealed class StackTraceHiddenAttribute : Attribute + { + /// + /// Initializes a new instance of the class. + /// + public StackTraceHiddenAttribute() { } + } +} diff --git a/src/coreclr/nativeaot/Test.CoreLib/src/Test.CoreLib.csproj b/src/coreclr/nativeaot/Test.CoreLib/src/Test.CoreLib.csproj index fc0d6ab6198c7..1914fec1d039d 100644 --- a/src/coreclr/nativeaot/Test.CoreLib/src/Test.CoreLib.csproj +++ b/src/coreclr/nativeaot/Test.CoreLib/src/Test.CoreLib.csproj @@ -221,6 +221,8 @@ + + From 463678abc66ddc0ec8a6ef1600355ce15e8508c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Petryka?= Date: Sun, 17 Mar 2024 18:45:28 +0100 Subject: [PATCH 08/12] Fix build errors --- .../Runtime/InteropServices/MemoryMarshal.cs | 14 ++++++++++++++ .../nativeaot/Test.CoreLib/src/Test.CoreLib.csproj | 4 ++++ 2 files changed, 18 insertions(+) create mode 100644 src/coreclr/nativeaot/Test.CoreLib/src/System/Runtime/InteropServices/MemoryMarshal.cs diff --git a/src/coreclr/nativeaot/Test.CoreLib/src/System/Runtime/InteropServices/MemoryMarshal.cs b/src/coreclr/nativeaot/Test.CoreLib/src/System/Runtime/InteropServices/MemoryMarshal.cs new file mode 100644 index 0000000000000..fa8e7dce0a031 --- /dev/null +++ b/src/coreclr/nativeaot/Test.CoreLib/src/System/Runtime/InteropServices/MemoryMarshal.cs @@ -0,0 +1,14 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Runtime.CompilerServices; + +namespace System.Runtime.InteropServices +{ + public static class MemoryMarshal + { + [Intrinsic] + public static ref T GetArrayDataReference(T[] array) => + ref GetArrayDataReference(array); + } +} diff --git a/src/coreclr/nativeaot/Test.CoreLib/src/Test.CoreLib.csproj b/src/coreclr/nativeaot/Test.CoreLib/src/Test.CoreLib.csproj index 1914fec1d039d..05cc9bd8f05a7 100644 --- a/src/coreclr/nativeaot/Test.CoreLib/src/Test.CoreLib.csproj +++ b/src/coreclr/nativeaot/Test.CoreLib/src/Test.CoreLib.csproj @@ -225,6 +225,7 @@ + @@ -238,4 +239,7 @@ Internal\Runtime\TypeManagerHandle.cs + + + From 0f6104cd7ff229b30d0df6bf586e211b2e939629 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Petryka?= <35800402+MichalPetryka@users.noreply.github.com> Date: Sun, 17 Mar 2024 18:51:49 +0100 Subject: [PATCH 09/12] Update src/coreclr/nativeaot/Test.CoreLib/src/Test.CoreLib.csproj Co-authored-by: Jan Kotas --- src/coreclr/nativeaot/Test.CoreLib/src/Test.CoreLib.csproj | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/coreclr/nativeaot/Test.CoreLib/src/Test.CoreLib.csproj b/src/coreclr/nativeaot/Test.CoreLib/src/Test.CoreLib.csproj index 05cc9bd8f05a7..498dc441b9763 100644 --- a/src/coreclr/nativeaot/Test.CoreLib/src/Test.CoreLib.csproj +++ b/src/coreclr/nativeaot/Test.CoreLib/src/Test.CoreLib.csproj @@ -239,7 +239,4 @@ Internal\Runtime\TypeManagerHandle.cs - - - From 8b98907f907a9aa45b56a237fbe4095e5b9df4e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Petryka?= Date: Sun, 17 Mar 2024 20:49:42 +0100 Subject: [PATCH 10/12] Actually fix CoreCLR too --- src/coreclr/vm/corelib.h | 4 ++-- src/coreclr/vm/metasig.h | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/coreclr/vm/corelib.h b/src/coreclr/vm/corelib.h index c52c58954165a..45bbe3a70ee25 100644 --- a/src/coreclr/vm/corelib.h +++ b/src/coreclr/vm/corelib.h @@ -1151,8 +1151,8 @@ DEFINE_METHOD(CASTHELPERS, CHKCASTINTERFACE, ChkCastInterface, SM_Ptr DEFINE_METHOD(CASTHELPERS, CHKCASTCLASS, ChkCastClass, SM_PtrVoid_Obj_RetObj) DEFINE_METHOD(CASTHELPERS, CHKCASTCLASSSPECIAL, ChkCastClassSpecial, SM_PtrVoid_Obj_RetObj) DEFINE_METHOD(CASTHELPERS, UNBOX, Unbox, SM_PtrVoid_Obj_RetRefByte) -DEFINE_METHOD(CASTHELPERS, STELEMREF, StelemRef, SM_Array_IntPtr_Obj_RetVoid) -DEFINE_METHOD(CASTHELPERS, LDELEMAREF, LdelemaRef, SM_Array_IntPtr_PtrVoid_RetRefObj) +DEFINE_METHOD(CASTHELPERS, STELEMREF, StelemRef, SM_ArrObject_IntPtr_Obj_RetVoid) +DEFINE_METHOD(CASTHELPERS, LDELEMAREF, LdelemaRef, SM_ArrObject_IntPtr_PtrVoid_RetRefObj) #ifdef FEATURE_EH_FUNCLETS DEFINE_CLASS(EH, Runtime, EH) diff --git a/src/coreclr/vm/metasig.h b/src/coreclr/vm/metasig.h index 182acc55e643f..7e52c98d0e67a 100644 --- a/src/coreclr/vm/metasig.h +++ b/src/coreclr/vm/metasig.h @@ -610,6 +610,8 @@ DEFINE_METASIG_T(SM(Array_Int_Obj_RetVoid, C(ARRAY) i j, v)) DEFINE_METASIG_T(SM(Array_Int_PtrVoid_RetRefObj, C(ARRAY) i P(v), r(j))) DEFINE_METASIG_T(SM(Array_IntPtr_Obj_RetVoid, C(ARRAY) I j, v)) DEFINE_METASIG_T(SM(Array_IntPtr_PtrVoid_RetRefObj, C(ARRAY) I P(v), r(j))) +DEFINE_METASIG(SM(ArrObject_IntPtr_Obj_RetVoid, a(j) I j, v)) +DEFINE_METASIG(SM(ArrObject_IntPtr_PtrVoid_RetRefObj, a(j) I P(v), r(j))) DEFINE_METASIG(SM(Obj_IntPtr_Bool_RetVoid, j I F, v)) DEFINE_METASIG(SM(IntPtr_Obj_RetVoid, I j, v)) From ebf890abe64b94b3c653a24c0b865aedf378fc8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Petryka?= <35800402+MichalPetryka@users.noreply.github.com> Date: Sun, 17 Mar 2024 21:51:26 +0100 Subject: [PATCH 11/12] Update src/coreclr/vm/metasig.h Co-authored-by: Jan Kotas --- src/coreclr/vm/metasig.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/coreclr/vm/metasig.h b/src/coreclr/vm/metasig.h index 7e52c98d0e67a..f2d635f944669 100644 --- a/src/coreclr/vm/metasig.h +++ b/src/coreclr/vm/metasig.h @@ -608,8 +608,6 @@ DEFINE_METASIG(GM(RetT, IMAGE_CEE_CS_CALLCONV_DEFAULT, 1, _, M(0))) DEFINE_METASIG_T(SM(Array_Int_Array_Int_Int_RetVoid, C(ARRAY) i C(ARRAY) i i, v)) DEFINE_METASIG_T(SM(Array_Int_Obj_RetVoid, C(ARRAY) i j, v)) DEFINE_METASIG_T(SM(Array_Int_PtrVoid_RetRefObj, C(ARRAY) i P(v), r(j))) -DEFINE_METASIG_T(SM(Array_IntPtr_Obj_RetVoid, C(ARRAY) I j, v)) -DEFINE_METASIG_T(SM(Array_IntPtr_PtrVoid_RetRefObj, C(ARRAY) I P(v), r(j))) DEFINE_METASIG(SM(ArrObject_IntPtr_Obj_RetVoid, a(j) I j, v)) DEFINE_METASIG(SM(ArrObject_IntPtr_PtrVoid_RetRefObj, a(j) I P(v), r(j))) From 1bdb1f60f093f915c09cc913ee4c5e777ee09e9e Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Wed, 20 Mar 2024 03:13:43 -0700 Subject: [PATCH 12/12] Cleanup --- .../Runtime/CompilerServices/CastHelpers.cs | 12 +++++++----- .../Runtime.Base/src/System/Runtime/TypeCast.cs | 17 +++++++++-------- 2 files changed, 16 insertions(+), 13 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 942b5ff3fb080..0c640dd1c2f0a 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 @@ -367,7 +367,7 @@ internal static unsafe class CastHelpers [DebuggerHidden] private static ref byte Unbox(void* toTypeHnd, object obj) { - // this will throw NullReferenceException if obj is null, attributed to the user code, as expected. + // This will throw NullReferenceException if obj is null. if (RuntimeHelpers.GetMethodTable(obj) == toTypeHnd) return ref obj.GetRawData(); @@ -387,9 +387,10 @@ private static void ThrowArrayMismatchException() } [DebuggerHidden] - private static ref object? LdelemaRef(object?[]? array, nint index, void* type) + private static ref object? LdelemaRef(object?[] array, nint index, void* type) { - if ((nuint)index >= (uint)array!.Length) + // This will throw NullReferenceException if array is null. + if ((nuint)index >= (uint)array.Length) ThrowIndexOutOfRangeException(); Debug.Assert(index >= 0); @@ -403,9 +404,10 @@ private static void ThrowArrayMismatchException() } [DebuggerHidden] - private static void StelemRef(object?[]? array, nint index, object? obj) + private static void StelemRef(object?[] array, nint index, object? obj) { - if ((nuint)index >= (uint)array!.Length) + // This will throw NullReferenceException if array is null. + if ((nuint)index >= (uint)array.Length) ThrowIndexOutOfRangeException(); Debug.Assert(index >= 0); diff --git a/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/TypeCast.cs b/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/TypeCast.cs index 6e2762301e7a7..72c036d29b617 100644 --- a/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/TypeCast.cs +++ b/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/TypeCast.cs @@ -758,11 +758,12 @@ private static unsafe void ThrowArrayMismatchException(object?[] array) // [RuntimeExport("RhpLdelemaRef")] - public static unsafe ref object? LdelemaRef(object?[]? array, nint index, MethodTable* elementType) + public static unsafe ref object? LdelemaRef(object?[] array, nint index, MethodTable* elementType) { Debug.Assert(array is null || array.GetMethodTable()->IsArray, "first argument must be an array"); #if INPLACE_RUNTIME + // This will throw NullReferenceException if obj is null. if ((nuint)index >= (uint)array.Length) ThrowIndexOutOfRangeException(array); @@ -789,12 +790,13 @@ private static unsafe void ThrowArrayMismatchException(object?[] array) } [RuntimeExport("RhpStelemRef")] - public static unsafe void StelemRef(object?[]? array, nint index, object? obj) + public static unsafe void StelemRef(object?[] array, nint index, object? obj) { // This is supported only on arrays Debug.Assert(array is null || array.GetMethodTable()->IsArray, "first argument must be an array"); #if INPLACE_RUNTIME + // This will throw NullReferenceException if obj is null. if ((nuint)index >= (uint)array.Length) ThrowIndexOutOfRangeException(array); @@ -857,15 +859,14 @@ private static unsafe void StelemRef_Helper(ref object? element, MethodTable* el private static unsafe void StelemRef_Helper_NoCacheLookup(ref object? element, MethodTable* elementType, object obj) { object? castedObj = IsInstanceOfAny_NoCacheLookup(elementType, obj); - if (castedObj != null) + if (castedObj == null) { - InternalCalls.RhpAssignRef(ref element, obj); - return; + // Throw the array type mismatch exception defined by the classlib, using the input array's + // MethodTable* to find the correct classlib. + throw elementType->GetClasslibException(ExceptionIDs.ArrayTypeMismatch); } - // Throw the array type mismatch exception defined by the classlib, using the input array's - // MethodTable* to find the correct classlib. - throw elementType->GetClasslibException(ExceptionIDs.ArrayTypeMismatch); + InternalCalls.RhpAssignRef(ref element, obj); } private static unsafe object IsInstanceOfArray(MethodTable* pTargetType, object obj)