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

Fix regression from RuntimeType.AllocateValueType rewrite #101137

Merged
merged 4 commits into from
Jun 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,7 @@
<Compile Include="$(CommonPath)System\Collections\Generic\ArrayBuilder.cs">
<Link>Common\System\Collections\Generic\ArrayBuilder.cs</Link>
</Compile>
<Compile Include="src\System\RuntimeType.BoxCache.cs" />
<Compile Include="src\System\RuntimeType.CreateUninitializedCache.CoreCLR.cs" />
<Compile Include="src\System\RuntimeType.GenericCache.cs" />
</ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -431,32 +431,7 @@ private static unsafe void DispatchTailCalls(
if (type.IsNullHandle())
ThrowHelper.ThrowArgumentNullException(ExceptionArgument.type);

TypeHandle handle = type.GetNativeTypeHandle();

if (handle.IsTypeDesc)
throw new ArgumentException(SR.Arg_TypeNotSupported);

MethodTable* pMT = handle.AsMethodTable();

if (pMT->ContainsGenericVariables)
throw new ArgumentException(SR.Arg_TypeNotSupported);

if (pMT->IsValueType)
{
if (pMT->IsByRefLike)
throw new NotSupportedException(SR.NotSupported_ByRefLike);

if (MethodTable.AreSameType(pMT, (MethodTable*)RuntimeTypeHandle.ToIntPtr(typeof(void).TypeHandle)))
throw new ArgumentException(SR.Arg_TypeNotSupported);

object? result = Box(pMT, ref target);
GC.KeepAlive(type);
return result;
}
else
{
return Unsafe.As<byte, object?>(ref target);
}
return type.GetRuntimeType().Box(ref target);
}

[LibraryImport(QCall, EntryPoint = "ReflectionInvocation_SizeOf")]
Expand Down
146 changes: 146 additions & 0 deletions src/coreclr/System.Private.CoreLib/src/System/RuntimeType.BoxCache.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Diagnostics;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;

namespace System
{
internal sealed partial class RuntimeType
{
/// <summary>
/// A cache which allows optimizing <see cref="RuntimeHelpers.Box(ref byte, RuntimeTypeHandle)"/>.
/// </summary>
internal sealed unsafe partial class BoxCache : IGenericCacheEntry<BoxCache>
{
public static BoxCache Create(RuntimeType type) => new(type);
public void InitializeCompositeCache(CompositeCacheEntry compositeEntry) => compositeEntry._boxCache = this;
public static ref BoxCache? GetStorageRef(CompositeCacheEntry compositeEntry) => ref compositeEntry._boxCache;

// The managed calli to the newobj allocator, plus its first argument
private readonly delegate*<void*, object> _pfnAllocator;
private readonly void* _allocatorFirstArg;
private readonly int _nullableValueOffset;
private readonly uint _valueTypeSize;
private readonly MethodTable* _pMT;

#if DEBUG
private readonly RuntimeType _originalRuntimeType;
#endif

private BoxCache(RuntimeType rt)
{
Debug.Assert(rt != null);

#if DEBUG
_originalRuntimeType = rt;
#endif

TypeHandle handle = rt.TypeHandle.GetNativeTypeHandle();

if (handle.IsTypeDesc)
throw new ArgumentException(SR.Arg_TypeNotSupported);

_pMT = handle.AsMethodTable();

// For value types, this is checked in GetBoxInfo,
// but for non-value types, we still need to check this case for consistent behavior.
if (_pMT->ContainsGenericVariables)
jkotas marked this conversation as resolved.
Show resolved Hide resolved
throw new ArgumentException(SR.Arg_TypeNotSupported);

if (_pMT->IsValueType)
{
GetBoxInfo(rt, out _pfnAllocator, out _allocatorFirstArg, out _nullableValueOffset, out _valueTypeSize);
}
}

internal object? Box(RuntimeType rt, ref byte data)
{
#if DEBUG
if (_originalRuntimeType != rt)
{
Debug.Fail("Caller passed the wrong RuntimeType to this routine."
+ Environment.NewLineConst + "Expected: " + (_originalRuntimeType ?? (object)"<null>")
+ Environment.NewLineConst + "Actual: " + (rt ?? (object)"<null>"));
}
#endif
if (_pfnAllocator == null)
{
// If the allocator is null, then we shouldn't allocate and make a copy,
// we should return the data as the object it currently is.
return Unsafe.As<byte, object>(ref data);
}

ref byte source = ref data;

byte maybeNullableHasValue = Unsafe.ReadUnaligned<byte>(ref source);

if (_nullableValueOffset != 0)
{
if (maybeNullableHasValue == 0)
{
return null;
}
source = ref Unsafe.Add(ref source, _nullableValueOffset);
}

object result = _pfnAllocator(_allocatorFirstArg);
GC.KeepAlive(rt);

if (_pMT->ContainsGCPointers)
{
Buffer.BulkMoveWithWriteBarrier(ref result.GetRawData(), ref source, _valueTypeSize);
}
else
{
SpanHelpers.Memmove(ref result.GetRawData(), ref source, _valueTypeSize);
}

return result;
}

/// <summary>
/// Given a RuntimeType, returns information about how to box instances
/// of it via calli semantics.
/// </summary>
private static void GetBoxInfo(
RuntimeType rt,
out delegate*<void*, object> pfnAllocator,
out void* vAllocatorFirstArg,
out int nullableValueOffset,
out uint valueTypeSize)
{
Debug.Assert(rt != null);

delegate*<void*, object> pfnAllocatorTemp = default;
void* vAllocatorFirstArgTemp = default;
int nullableValueOffsetTemp = default;
uint valueTypeSizeTemp = default;

GetBoxInfo(
new QCallTypeHandle(ref rt),
&pfnAllocatorTemp, &vAllocatorFirstArgTemp,
&nullableValueOffsetTemp, &valueTypeSizeTemp);

pfnAllocator = pfnAllocatorTemp;
vAllocatorFirstArg = vAllocatorFirstArgTemp;
nullableValueOffset = nullableValueOffsetTemp;
valueTypeSize = valueTypeSizeTemp;
}

[LibraryImport(RuntimeHelpers.QCall, EntryPoint = "ReflectionInvocation_GetBoxInfo")]
private static partial void GetBoxInfo(
QCallTypeHandle type,
delegate*<void*, object>* ppfnAllocator,
void** pvAllocatorFirstArg,
int* pNullableValueOffset,
uint* pValueTypeSize);
}

internal object? Box(ref byte data)
{
return GetOrCreateCacheEntry<BoxCache>().Box(this, ref data);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ internal sealed class CompositeCacheEntry : IGenericCacheEntry
internal RuntimeTypeCache.FunctionPointerCache? _functionPointerCache;
internal Array.ArrayInitializeCache? _arrayInitializeCache;
internal IGenericCacheEntry? _enumInfo;
internal BoxCache? _boxCache;

void IGenericCacheEntry.InitializeCompositeCache(CompositeCacheEntry compositeEntry) => throw new UnreachableException();
}
Expand Down
9 changes: 9 additions & 0 deletions src/coreclr/vm/object.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1623,6 +1623,15 @@ BOOL Nullable::IsNullableForTypeHelperNoGC(MethodTable* nullableMT, MethodTable*
}

//===============================================================================
int32_t Nullable::GetValueAddrOffset(MethodTable* nullableMT)
{
LIMITED_METHOD_CONTRACT;

_ASSERTE(IsNullableType(nullableMT));
_ASSERTE(strcmp(nullableMT->GetApproxFieldDescListRaw()[1].GetDebugName(), "value") == 0);
return nullableMT->GetApproxFieldDescListRaw()[1].GetOffset();
}

CLR_BOOL* Nullable::HasValueAddr(MethodTable* nullableMT) {

LIMITED_METHOD_CONTRACT;
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/vm/object.h
Original file line number Diff line number Diff line change
Expand Up @@ -2473,6 +2473,8 @@ class Nullable {
return nullable->ValueAddr(nullableMT);
}

static int32_t GetValueAddrOffset(MethodTable* nullableMT);

private:
static BOOL IsNullableForTypeHelper(MethodTable* nullableMT, MethodTable* paramMT);
static BOOL IsNullableForTypeHelperNoGC(MethodTable* nullableMT, MethodTable* paramMT);
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/vm/qcallentrypoints.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,7 @@ static const Entry s_QCall[] =
DllImportEntry(ReflectionInvocation_CompileMethod)
DllImportEntry(ReflectionInvocation_PrepareMethod)
DllImportEntry(ReflectionInvocation_SizeOf)
DllImportEntry(ReflectionInvocation_GetBoxInfo)
DllImportEntry(ReflectionSerialization_GetCreateUninitializedObjectInfo)
#if defined(FEATURE_COMWRAPPERS)
DllImportEntry(ComWrappers_GetIUnknownImpl)
Expand Down
46 changes: 46 additions & 0 deletions src/coreclr/vm/reflectioninvocation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2069,3 +2069,49 @@ extern "C" int32_t QCALLTYPE ReflectionInvocation_SizeOf(QCall::TypeHandle pType

return handle.GetSize();
}

extern "C" void QCALLTYPE ReflectionInvocation_GetBoxInfo(
QCall::TypeHandle pType,
PCODE* ppfnAllocator,
void** pvAllocatorFirstArg,
int32_t* pValueOffset,
uint32_t* pValueSize)
{
CONTRACTL
{
QCALL_CHECK;
PRECONDITION(CheckPointer(ppfnAllocator));
PRECONDITION(CheckPointer(pvAllocatorFirstArg));
PRECONDITION(*ppfnAllocator == NULL);
PRECONDITION(*pvAllocatorFirstArg == NULL);
}
CONTRACTL_END;

BEGIN_QCALL;

TypeHandle type = pType.AsTypeHandle();

RuntimeTypeHandle::ValidateTypeAbleToBeInstantiated(type, true /* fForGetUninitializedInstance */);

MethodTable* pMT = type.AsMethodTable();

_ASSERTE(pMT->IsValueType() || pMT->IsNullable() || pMT->IsEnum() || pMT->IsTruePrimitive());

*pValueOffset = 0;

// If it is a nullable, return the allocator for the underlying type instead.
if (pMT->IsNullable())
{
*pValueOffset = Nullable::GetValueAddrOffset(pMT);
pMT = pMT->GetInstantiation()[0].GetMethodTable();
}

bool fHasSideEffectsUnused;
*ppfnAllocator = CEEJitInfo::getHelperFtnStatic(CEEInfo::getNewHelperStatic(pMT, &fHasSideEffectsUnused));
*pvAllocatorFirstArg = pMT;
*pValueSize = pMT->GetNumInstanceFieldBytes();

pMT->EnsureInstanceActive();

END_QCALL;
}
7 changes: 7 additions & 0 deletions src/coreclr/vm/reflectioninvocation.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,13 @@ extern "C" void QCALLTYPE ReflectionInvocation_PrepareMethod(MethodDesc* pMD, Ty

extern "C" void QCALLTYPE ReflectionSerialization_GetCreateUninitializedObjectInfo(QCall::TypeHandle pType, PCODE* ppfnAllocator, void** pvAllocatorFirstArg);

extern "C" void QCALLTYPE ReflectionInvocation_GetBoxInfo(
QCall::TypeHandle pType,
PCODE* ppfnAllocator,
void** pvAllocatorFirstArg,
int32_t* pValueOffset,
uint32_t* pValueSize);

class ReflectionEnum {
public:
static FCDECL1(INT32, InternalGetCorElementType, MethodTable* pMT);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ private static extern unsafe IntPtr GetSpanDataFrom(
private static extern bool SufficientExecutionStack();

[MethodImplAttribute(MethodImplOptions.InternalCall)]
private static extern void InternalBox(QCallTypeHandle type, ref byte target, ObjectHandleOnStack result);
private static extern object InternalBox(QCallTypeHandle type, ref byte target);

/// <summary>
/// Create a boxed object of the specified type from the data located at the target reference.
Expand Down Expand Up @@ -255,8 +255,7 @@ private static extern unsafe IntPtr GetSpanDataFrom(
if (rtType.IsByRefLike)
throw new NotSupportedException(SR.NotSupported_ByRefLike);

object? result = null;
InternalBox(new QCallTypeHandle(ref rtType), ref target, ObjectHandleOnStack.Create(ref result));
object? result = InternalBox(new QCallTypeHandle(ref rtType), ref target);
return result;
}

Expand Down
2 changes: 1 addition & 1 deletion src/mono/mono/metadata/icall-def.h
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,7 @@ HANDLES(RUNH_1, "GetObjectValue", ves_icall_System_Runtime_CompilerServices_Runt
HANDLES(RUNH_6, "GetSpanDataFrom", ves_icall_System_Runtime_CompilerServices_RuntimeHelpers_GetSpanDataFrom, gpointer, 3, (MonoClassField_ptr, MonoType_ptr, gpointer))
HANDLES(RUNH_2, "GetUninitializedObjectInternal", ves_icall_System_Runtime_CompilerServices_RuntimeHelpers_GetUninitializedObjectInternal, MonoObject, 1, (MonoType_ptr))
HANDLES(RUNH_3, "InitializeArray", ves_icall_System_Runtime_CompilerServices_RuntimeHelpers_InitializeArray, void, 2, (MonoArray, MonoClassField_ptr))
HANDLES(RUNH_8, "InternalBox", ves_icall_System_Runtime_CompilerServices_RuntimeHelpers_InternalBox, void, 3, (MonoQCallTypeHandle, char_ref, MonoObjectHandleOnStack))
HANDLES(RUNH_8, "InternalBox", ves_icall_System_Runtime_CompilerServices_RuntimeHelpers_InternalBox, MonoObject, 2, (MonoQCallTypeHandle, char_ref))
HANDLES(RUNH_7, "InternalGetHashCode", ves_icall_System_Runtime_CompilerServices_RuntimeHelpers_InternalGetHashCode, int, 1, (MonoObject))
HANDLES(RUNH_3a, "PrepareMethod", ves_icall_System_Runtime_CompilerServices_RuntimeHelpers_PrepareMethod, void, 3, (MonoMethod_ptr, gpointer, int))
HANDLES(RUNH_4, "RunClassConstructor", ves_icall_System_Runtime_CompilerServices_RuntimeHelpers_RunClassConstructor, void, 1, (MonoType_ptr))
Expand Down
14 changes: 4 additions & 10 deletions src/mono/mono/metadata/icall.c
Original file line number Diff line number Diff line change
Expand Up @@ -1213,24 +1213,18 @@ ves_icall_System_Runtime_CompilerServices_RuntimeHelpers_PrepareMethod (MonoMeth
// FIXME: Implement
}

void
ves_icall_System_Runtime_CompilerServices_RuntimeHelpers_InternalBox (MonoQCallTypeHandle type_handle, char* data, MonoObjectHandleOnStack obj, MonoError *error)
MonoObjectHandle
ves_icall_System_Runtime_CompilerServices_RuntimeHelpers_InternalBox (MonoQCallTypeHandle type_handle, char* data, MonoError *error)
{
MonoType *type = type_handle.type;
MonoClass *klass = mono_class_from_mono_type_internal (type);

g_assert (m_class_is_valuetype (klass));

mono_class_init_checked (klass, error);
goto_if_nok (error, error_ret);

MonoObject* raw_obj = mono_value_box_checked (klass, data, error);
goto_if_nok (error, error_ret);
return_val_if_nok (error, NULL_HANDLE);

HANDLE_ON_STACK_SET(obj, raw_obj);
return;
error_ret:
HANDLE_ON_STACK_SET (obj, NULL);
return mono_value_box_handle (klass, data, error);
}

gint32
Expand Down
Loading