Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove invalid Unsafe.As from array helpers #99778

Merged
merged 15 commits into from
Mar 21, 2024
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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)
Expand Down Expand Up @@ -63,8 +60,6 @@ internal static unsafe class CastHelpers
}

[DebuggerHidden]
[StackTraceHidden]
[DebuggerStepThrough]
private static object? IsInstanceOfInterface(void* toTypeHnd, object? obj)
{
const int unrollSize = 4;
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -184,8 +177,6 @@ internal static unsafe class CastHelpers
}

[DebuggerHidden]
[StackTraceHidden]
[DebuggerStepThrough]
[MethodImpl(MethodImplOptions.NoInlining)]
private static object? IsInstance_Helper(void* toTypeHnd, object obj)
{
Expand All @@ -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;
Expand Down Expand Up @@ -237,8 +226,6 @@ internal static unsafe class CastHelpers
}

[DebuggerHidden]
[StackTraceHidden]
[DebuggerStepThrough]
[MethodImpl(MethodImplOptions.NoInlining)]
private static object? ChkCast_Helper(void* toTypeHnd, object obj)
{
Expand All @@ -253,8 +240,6 @@ internal static unsafe class CastHelpers
}

[DebuggerHidden]
[StackTraceHidden]
[DebuggerStepThrough]
private static object? ChkCastInterface(void* toTypeHnd, object? obj)
{
const int unrollSize = 4;
Expand Down Expand Up @@ -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)
Expand All @@ -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);
Expand Down Expand Up @@ -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.
Expand All @@ -395,41 +374,42 @@ private static ref byte Unbox(void* toTypeHnd, object obj)
return ref Unbox_Helper(toTypeHnd, obj);
}

internal struct ArrayElement
[DebuggerHidden]
private static void ThrowIndexOutOfRangeException()
{
public object? Value;
throw new IndexOutOfRangeException();
}

[DebuggerHidden]
[StackTraceHidden]
[DebuggerStepThrough]
private static ref object? ThrowArrayMismatchException()
private static void ThrowArrayMismatchException()
jkotas marked this conversation as resolved.
Show resolved Hide resolved
{
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)
{
// this will throw appropriate exceptions if array is null or access is out of range.
ref object? element = ref Unsafe.As<ArrayElement[]>(array)[index].Value;
if ((nuint)index >= (uint)array!.Length)
ThrowIndexOutOfRangeException();

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;
jkotas marked this conversation as resolved.
Show resolved Hide resolved
}

[DebuggerHidden]
[StackTraceHidden]
[DebuggerStepThrough]
private static void StelemRef(Array array, nint index, object? obj)
private static void StelemRef(object?[]? 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<ArrayElement[]>(array)[index].Value;
if ((nuint)index >= (uint)array!.Length)
ThrowIndexOutOfRangeException();

Debug.Assert(index >= 0);
ref object? element = ref Unsafe.Add(ref MemoryMarshal.GetArrayDataReference(array), index);
void* elementType = RuntimeHelpers.GetMethodTable(array)->ElementType;

if (obj == null)
Expand All @@ -454,8 +434,6 @@ private static void StelemRef(Array array, nint index, object? obj)
}

[DebuggerHidden]
[StackTraceHidden]
[DebuggerStepThrough]
[MethodImpl(MethodImplOptions.NoInlining)]
private static void StelemRef_Helper(ref object? element, void* elementType, object obj)
{
Expand All @@ -470,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);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand Down
102 changes: 54 additions & 48 deletions src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/TypeCast.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ namespace System.Runtime
//
/////////////////////////////////////////////////////////////////////////////////////////////////////

[StackTraceHidden]
[DebuggerStepThrough]
[EagerStaticClassConstruction]
internal static class TypeCast
{
Expand Down Expand Up @@ -737,23 +739,67 @@ 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, MethodTable* 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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can just use ThrowHelper.ThrowIndexOutOfRangeException given that is for INPLACE_RUNTIME. It is unnecessary to go through GetClasslibException indirection for INPLACE_RUNTIME. The main point of INPLACE_RUNTIME is to avoid these indirections.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to remove the !INPLACE_RUNTIME code in the next PR and to keep this like earlier for now.
Also I think that ThrowHelper is not in the Test.CoreLib (not sure what that is for tbh but right now there's a build failure here due to MemoryMarshal not being there too).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test.CoreLib is very minimal implementation of CoreLib for testing, debugging and various experiments. It is fine to add minimal implementations of MemoryMarshal or ThrowHelper to it to keep it working.


Debug.Assert(index >= 0);
ref object? element = ref Unsafe.Add(ref MemoryMarshal.GetArrayDataReference(array), index);
#else
if (array is null)
{
throw elementType->GetClasslibException(ExceptionIDs.NullReference);
}
if ((nuint)index >= (uint)array.Length)
{
throw elementType->GetClasslibException(ExceptionIDs.IndexOutOfRange);
}
ref object rawData = ref Unsafe.As<byte, object>(ref Unsafe.As<RawArrayData>(array).Data);
ref object element = ref Unsafe.Add(ref rawData, index);
#endif
MethodTable* arrayElemType = array.GetMethodTable()->RelatedParameterType;

if (elementType != 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<ArrayElement[]>(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)
{
Expand Down Expand Up @@ -796,7 +842,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)
Expand All @@ -808,7 +854,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)
Expand All @@ -822,46 +868,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<ArrayElement[]>(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<byte, object>(ref Unsafe.As<RawArrayData>(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();
Expand Down
Original file line number Diff line number Diff line change
@@ -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() { }
}
}
Original file line number Diff line number Diff line change
@@ -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
{
/// <summary>
/// Types and Methods attributed with StackTraceHidden will be omitted from the stack trace text shown in StackTrace.ToString()
/// and Exception.StackTrace
/// </summary>
[AttributeUsage(AttributeTargets.Class | AttributeTargets.Method | AttributeTargets.Constructor | AttributeTargets.Struct, Inherited = false)]
public sealed class StackTraceHiddenAttribute : Attribute
{
/// <summary>
/// Initializes a new instance of the <see cref="StackTraceHiddenAttribute"/> class.
/// </summary>
public StackTraceHiddenAttribute() { }
}
}
Loading
Loading