Skip to content

Commit

Permalink
[mono] Unify invoke code with coreclr (#72717)
Browse files Browse the repository at this point in the history 
* [mono] Add icalls to be used by new invoke machinery

* [mono] Fix IsInstanceOfType check

* [mono] Reuse most of the managed invoke path used by coreclr

* [mono] Implement new version of InternalInvoke icall

This version doesn't receive a MonoArray of params as objects, rather a simple array of byrefs. There is also no need for special handling for nullables. The runtime invoke wrapper receives a boxed nullable, this conversion happens in managed code using ReboxFromNullable and ReboxToNullable. This change might have some side effects on other API making use of the runtime invoke wrapper!

* [mono] Throw NRE for a null byref parameter

Behavior was changed recently.

* [tests] Enable reflection invoke tests on mono

* [mono] Avoid loading reflection invoke machinery during startup

Aside from perf optimization, this allows the usage of LocalAppContextSwitches for invoke tests. Before this change, invoke machinery was initialized before the appcontext properties were set, failing to detect ForceInterpretedInvoke and ForceEmitInvoke properties. CoreCLR also hardcodes string type param.

* [mono] Remove nullable handling case

This API already receives a boxed nullable now.

* [mono] Disable inlining into Invoke stubs

CoreCLR achieves this via NextCallReturnAddress. Add another intrinsic to be used for mono.

* [mono] Fix handling of nullables when invoking from debugger

* [mono] Update code for dyn runtime invokes and llvmonly invoke

Remove special handling for nullables and throwing of NullByRefReturnException.

* [mono] Remove old invoke code

Which was still used only by mono_runtime_invoke_array

* [mono] Move invoke code back in object.c

* [mono] Simplify nullable ctor case

Nullable<T> only has 1 param constructor that creates a boxed vtype

* [mono] Extract invoke code to be reused by mono_runtime_invoke_array

* [mono] Add back support for the embedding api

With same behavior as before, except we have to handle conversions between nullable and boxedvt ourserlves, since it is no longer done by the runtime invoke wrapper.

* [mono] Don't trigger ambiguous method exception when having duplicate method overrides

Seems to be possible via reflection, ex System.Reflection.Emit.Tests.MethodBuilderDefineMethodOverride.DefineMethodOverride_CalledTwiceWithSameBodies_Works

* Disable some warnings

* Disable tests on wasm aot since they rely on stacktraces
  • Loading branch information
@BrzVlad
BrzVlad authored Nov 18, 2022
1 parent 2725a84 commit aa012f4
Show file tree
Hide file tree
Showing 36 changed files with 779 additions and 1,014 deletions.
13 changes: 0 additions & 13 deletions src/coreclr/System.Private.CoreLib/src/System/RuntimeHandles.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,19 +141,6 @@ internal static bool IsByRef(RuntimeType type)
return corElemType == CorElementType.ELEMENT_TYPE_BYREF;
}

internal static bool TryGetByRefElementType(RuntimeType type, [NotNullWhen(true)] out RuntimeType? elementType)
{
CorElementType corElemType = GetCorElementType(type);
if (corElemType == CorElementType.ELEMENT_TYPE_BYREF)
{
elementType = GetElementType(type);
return true;
}

elementType = null;
return false;
}

internal static bool IsPointer(RuntimeType type)
{
CorElementType corElemType = GetCorElementType(type);
Expand Down
206 changes: 2 additions & 204 deletions src/coreclr/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3582,180 +3582,10 @@ public override Type MakeArrayType(int rank)
[MethodImpl(MethodImplOptions.InternalCall)]
private static extern object AllocateValueType(RuntimeType type, object? value);

private enum CheckValueStatus
{
Success = 0,
ArgumentException,
NotSupported_ByRefLike
}

/// <summary>
/// Verify <paramref name="value"/> and optionally convert the value for special cases.
/// </summary>
/// <returns>True if <paramref name="value"/> is a value type, False otherwise</returns>
internal bool CheckValue(
ref object? value,
ref ParameterCopyBackAction copyBack,
Binder? binder,
CultureInfo? culture,
BindingFlags invokeAttr)
{
// Already fast-pathed by the caller.
Debug.Assert(!ReferenceEquals(value?.GetType(), this));

// Since this cannot be a generic parameter, we use RuntimeTypeHandle.IsValueType here
// because it is faster than IsValueType
Debug.Assert(!IsGenericParameter);

// Fast path to whether a value can be assigned without conversion.
if (IsInstanceOfType(value))
{
if (IsNullableOfT)
{
// Pass as a true boxed Nullable<T>, not as a T or null.
value = RuntimeMethodHandle.ReboxToNullable(value, this);
return true;
}

// Other value types won't get here since Type equality was previous checked.
Debug.Assert(!RuntimeTypeHandle.IsValueType(this));

return false;
}

bool isValueType;
CheckValueStatus result = TryChangeType(ref value, ref copyBack, out isValueType);
if (result == CheckValueStatus.Success)
{
return isValueType;
}

if (result == CheckValueStatus.ArgumentException && (invokeAttr & BindingFlags.ExactBinding) == 0)
{
Debug.Assert(value != null);

// Use the binder
if (binder != null && binder != DefaultBinder)
{
value = binder.ChangeType(value, this, culture);
if (IsInstanceOfType(value))
{
if (IsNullableOfT)
{
// Pass as a true boxed Nullable<T>, not as a T or null.
value = RuntimeMethodHandle.ReboxToNullable(value, this);
copyBack = ParameterCopyBackAction.CopyNullable;
}
else
{
copyBack = ParameterCopyBackAction.Copy;
}

return IsValueType; // Note the call to IsValueType, not the variable.
}

result = TryChangeType(ref value, ref copyBack, out isValueType);
if (result == CheckValueStatus.Success)
{
return isValueType;
}
}
}

switch (result)
{
case CheckValueStatus.ArgumentException:
throw new ArgumentException(SR.Format(SR.Arg_ObjObjEx, value?.GetType(), this));
case CheckValueStatus.NotSupported_ByRefLike:
throw new NotSupportedException(SR.NotSupported_ByRefLike);
}

Debug.Fail("Error result not expected");
return false;
}

private CheckValueStatus TryChangeType(
ref object? value,
ref ParameterCopyBackAction copyBack,
private CheckValueStatus TryChangeTypeSpecial(
ref object value,
out bool isValueType)
{
RuntimeType? sigElementType;
if (RuntimeTypeHandle.TryGetByRefElementType(this, out sigElementType))
{
copyBack = ParameterCopyBackAction.Copy;
Debug.Assert(!sigElementType.IsGenericParameter);

if (sigElementType.IsInstanceOfType(value))
{
isValueType = RuntimeTypeHandle.IsValueType(sigElementType);
if (isValueType)
{
if (sigElementType.IsNullableOfT)
{
// Pass as a true boxed Nullable<T>, not as a T or null.
value = RuntimeMethodHandle.ReboxToNullable(value, sigElementType);
copyBack = ParameterCopyBackAction.CopyNullable;
}
else
{
// Make a copy to prevent the boxed instance from being directly modified by the method.
value = AllocateValueType(sigElementType, value);
}
}

return CheckValueStatus.Success;
}

if (value == null)
{
isValueType = RuntimeTypeHandle.IsValueType(sigElementType);
if (!isValueType)
{
// Normally we don't get here since 'null' was previosuly checked, but due to binders we can.
return CheckValueStatus.Success;
}

if (sigElementType.IsByRefLike)
{
return CheckValueStatus.NotSupported_ByRefLike;
}

// Allocate default<T>.
value = AllocateValueType(sigElementType, value: null);
copyBack = sigElementType.IsNullableOfT ? ParameterCopyBackAction.CopyNullable : ParameterCopyBackAction.Copy;
return CheckValueStatus.Success;
}

isValueType = false;
return CheckValueStatus.ArgumentException;
}

if (value == null)
{
isValueType = RuntimeTypeHandle.IsValueType(this);
if (!isValueType)
{
// Normally we don't get here since 'null' was previosuly checked, but due to binders we can.
return CheckValueStatus.Success;
}

if (IsByRefLike)
{
return CheckValueStatus.NotSupported_ByRefLike;
}

// Allocate default<T>.
value = AllocateValueType(this, value: null);
return CheckValueStatus.Success;
}

// Check the strange ones courtesy of reflection:
// - Implicit cast between primitives
// - Enum treated as underlying type
// - Pointer (*) types to IntPtr (if dest is IntPtr)
// - System.Reflection.Pointer to appropriate pointer (*) type (if dest is pointer type)
if (IsPointer || IsEnum || IsPrimitive)
{
Pointer? pointer = value as Pointer;
RuntimeType srcType = pointer != null ? pointer.GetPointerType() : (RuntimeType)value.GetType();

Expand All @@ -3781,38 +3611,6 @@ private CheckValueStatus TryChangeType(

isValueType = true;
return CheckValueStatus.Success;
}

isValueType = false;
return CheckValueStatus.ArgumentException;
}

internal bool TryByRefFastPath(ref object arg, ref bool isValueType)
{
if (RuntimeTypeHandle.TryGetByRefElementType(this, out RuntimeType? sigElementType) &&
ReferenceEquals(sigElementType, arg.GetType()))
{
isValueType = sigElementType.IsValueType;
if (isValueType)
{
// Make a copy to prevent the boxed instance from being directly modified by the method.
arg = AllocateValueType(sigElementType, arg);
}

return true;
}

return false;
}

internal static CorElementType GetUnderlyingType(RuntimeType type)
{
if (type.IsActualEnum)
{
type = (RuntimeType)Enum.GetUnderlyingType(type);
}

return RuntimeTypeHandle.GetCorElementType(type);
}

#endregion
Expand Down
3 changes: 1 addition & 2 deletions src/libraries/Common/tests/System/Reflection/InvokeEmitTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,7 @@ public static void VerifyInvokeIsUsingEmit_Constructor()
private static bool IsEmitInvokeSupported()
{
// Emit is only used for Invoke when RuntimeFeature.IsDynamicCodeCompiled is true.
return RuntimeFeature.IsDynamicCodeCompiled
&& !PlatformDetection.IsMonoRuntime; // Temporary until Mono is updated.
return RuntimeFeature.IsDynamicCodeCompiled;
}

private static string InterpretedMethodName => PlatformDetection.IsMonoRuntime ?
Expand Down
2 changes: 2 additions & 0 deletions src/libraries/Common/tests/System/Reflection/InvokeInterpretedTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ namespace System.Reflection.Tests
public class InvokeInterpretedTests
{
[Fact]
[ActiveIssue("https://github.com/dotnet/runtime/issues/50957", typeof(PlatformDetection), nameof(PlatformDetection.IsBrowser), nameof(PlatformDetection.IsMonoAOT))]
public static void VerifyInvokeIsUsingEmit_Method()
{
MethodInfo method = typeof(TestClassThatThrows).GetMethod(nameof(TestClassThatThrows.Throw))!;
Expand All @@ -24,6 +25,7 @@ string InterpretedMethodName() => PlatformDetection.IsMonoRuntime ?
}

[Fact]
[ActiveIssue("https://github.com/dotnet/runtime/issues/50957", typeof(PlatformDetection), nameof(PlatformDetection.IsBrowser), nameof(PlatformDetection.IsMonoAOT))]
public static void VerifyInvokeIsUsingEmit_Constructor()
{
ConstructorInfo ctor = typeof(TestClassThatThrows).GetConstructor(Type.EmptyTypes)!;
Expand Down
10 changes: 0 additions & 10 deletions src/libraries/System.Private.CoreLib/src/System/Reflection/ConstructorInvoker.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,14 @@ internal sealed partial class ConstructorInvoker
{
private readonly RuntimeConstructorInfo _method;

#if !MONO // Temporary until Mono is updated.
private bool _invoked;
private bool _strategyDetermined;
private InvokerEmitUtil.InvokeFunc? _invokeFunc;
#endif

public ConstructorInvoker(RuntimeConstructorInfo constructorInfo)
{
_method = constructorInfo;

#if !MONO // Temporary until Mono is updated.
if (LocalAppContextSwitches.ForceInterpretedInvoke && !LocalAppContextSwitches.ForceEmitInvoke)
{
// Always use the native invoke; useful for testing.
Expand All @@ -31,13 +28,9 @@ public ConstructorInvoker(RuntimeConstructorInfo constructorInfo)
// Always use emit invoke (if IsDynamicCodeCompiled == true); useful for testing.
_invoked = true;
}
#endif
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
#if MONO // Temporary until Mono is updated.
public unsafe object? InlinedInvoke(object? obj, Span<object?> args, BindingFlags invokeAttr) => InterpretedInvoke(obj, args, invokeAttr);
#else
public unsafe object? InlinedInvoke(object? obj, IntPtr* args, BindingFlags invokeAttr)
{
if (_invokeFunc != null && (invokeAttr & BindingFlags.DoNotWrapExceptions) != 0 && obj == null)
Expand All @@ -46,9 +39,7 @@ public ConstructorInvoker(RuntimeConstructorInfo constructorInfo)
}
return Invoke(obj, args, invokeAttr);
}
#endif

#if !MONO // Temporary until Mono is updated.
[DebuggerStepThrough]
[DebuggerHidden]
private unsafe object? Invoke(object? obj, IntPtr* args, BindingFlags invokeAttr)
Expand Down Expand Up @@ -102,6 +93,5 @@ public ConstructorInvoker(RuntimeConstructorInfo constructorInfo)

return ret;
}
#endif
}
}
13 changes: 10 additions & 3 deletions src/libraries/System.Private.CoreLib/src/System/Reflection/InvokerEmitUtil.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,11 @@ public static unsafe InvokeFunc CreateInvokeDelegate(MethodBase method)
}

// Invoke the method.
#if !MONO
il.Emit(OpCodes.Call, Methods.NextCallReturnAddress()); // For CallStack reasons, don't inline target method.
// For CallStack reasons, don't inline target method.
#if MONO
il.Emit(OpCodes.Call, Methods.DisableInline());
#else
il.Emit(OpCodes.Call, Methods.NextCallReturnAddress());
il.Emit(OpCodes.Pop);
#endif

Expand Down Expand Up @@ -182,7 +185,11 @@ public static MethodInfo Pointer_Box() =>
public static MethodInfo Type_GetTypeFromHandle() =>
s_Type_GetTypeFromHandle ??= typeof(Type).GetMethod(nameof(Type.GetTypeFromHandle), new[] { typeof(RuntimeTypeHandle) })!;

#if !MONO
#if MONO
private static MethodInfo? s_DisableInline;
public static MethodInfo DisableInline() =>
s_DisableInline ??= typeof(System.Runtime.CompilerServices.JitHelpers).GetMethod(nameof(System.Runtime.CompilerServices.JitHelpers.DisableInline), BindingFlags.NonPublic | BindingFlags.Static)!;
#else
private static MethodInfo? s_NextCallReturnAddress;
public static MethodInfo NextCallReturnAddress() =>
s_NextCallReturnAddress ??= typeof(System.StubHelpers.StubHelpers).GetMethod(nameof(System.StubHelpers.StubHelpers.NextCallReturnAddress), BindingFlags.NonPublic | BindingFlags.Static)!;
Expand Down
2 changes: 0 additions & 2 deletions src/libraries/System.Private.CoreLib/src/System/Reflection/MethodBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -239,14 +239,12 @@ BindingFlags invokeAttr
#pragma warning disable 8500
if (isValueType)
{
#if !MONO // Temporary until Mono is updated.
Debug.Assert(arg != null);
Debug.Assert(
arg.GetType() == sigType ||
(sigType.IsPointer && (arg.GetType() == typeof(IntPtr) || arg.GetType() == typeof(UIntPtr))) ||
(sigType.IsByRef && arg.GetType() == RuntimeTypeHandle.GetElementType(sigType)) ||
((sigType.IsEnum || arg.GetType().IsEnum) && RuntimeType.GetUnderlyingType((RuntimeType)arg.GetType()) == RuntimeType.GetUnderlyingType(sigType)));
#endif
ByReference valueTypeRef = ByReference.Create(ref copyOfParameters[i]!.GetRawData());
*(ByReference*)(byrefParameters + i) = valueTypeRef;
}
Expand Down
6 changes: 0 additions & 6 deletions src/libraries/System.Private.CoreLib/src/System/Reflection/MethodInvoker.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,6 @@ internal sealed partial class MethodInvoker
private readonly MethodBase _method;

[MethodImpl(MethodImplOptions.AggressiveInlining)]
#if MONO // Temporary until Mono is updated.
public unsafe object? InlinedInvoke(object? obj, Span<object?> args, BindingFlags invokeAttr) => InterpretedInvoke(obj, args, invokeAttr);
#else
public unsafe object? InlinedInvoke(object? obj, IntPtr* args, BindingFlags invokeAttr)
{
if (_invokeFunc != null && (invokeAttr & BindingFlags.DoNotWrapExceptions) != 0)
Expand All @@ -22,9 +19,7 @@ internal sealed partial class MethodInvoker
}
return Invoke(obj, args, invokeAttr);
}
#endif

#if !MONO // Temporary until Mono is updated.
private bool _invoked;
private bool _strategyDetermined;
private InvokerEmitUtil.InvokeFunc? _invokeFunc;
Expand Down Expand Up @@ -80,6 +75,5 @@ internal sealed partial class MethodInvoker

return ret;
}
#endif
}
}
Loading

0 comments on commit aa012f4

Please sign in to comment.