Skip to content

Commit

Permalink
Fix regression from RuntimeType.AllocateValueType rewrite (#101137)
Browse files Browse the repository at this point in the history
  • Loading branch information
jkoritzinsky committed Jun 5, 2024
1 parent 837eb27 commit 1753956
Show file tree
Hide file tree
Showing 12 changed files with 221 additions and 40 deletions.
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 @@ -432,32 +432,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)
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

0 comments on commit 1753956

Please sign in to comment.