From be45599a9c23a7042b6916d512f7e57dc59d9651 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Tue, 7 Nov 2023 07:14:28 +0900 Subject: [PATCH] Root less stuff for reflectable method signatures (#92994) --- .../src/System/InvokeUtils.cs | 5 +- .../System/Reflection/DynamicInvokeInfo.cs | 96 ++++++++++++------- .../ReflectionInvokeMapNode.cs | 5 + .../TrimmingBehaviors/DeadCodeElimination.cs | 27 ++++++ 4 files changed, 96 insertions(+), 37 deletions(-) diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/InvokeUtils.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/InvokeUtils.cs index 2563b696d9a72..538fb9b7e246e 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/InvokeUtils.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/InvokeUtils.cs @@ -251,8 +251,11 @@ private static Exception CreateChangeTypeException(EETypePtr srcEEType, EETypePt } internal static ArgumentException CreateChangeTypeArgumentException(EETypePtr srcEEType, EETypePtr dstEEType, bool destinationIsByRef = false) + => CreateChangeTypeArgumentException(srcEEType, Type.GetTypeFromHandle(new RuntimeTypeHandle(dstEEType)), destinationIsByRef); + + internal static ArgumentException CreateChangeTypeArgumentException(EETypePtr srcEEType, Type dstType, bool destinationIsByRef = false) { - object? destinationTypeName = Type.GetTypeFromHandle(new RuntimeTypeHandle(dstEEType)); + object? destinationTypeName = dstType; if (destinationIsByRef) destinationTypeName += "&"; return new ArgumentException(SR.Format(SR.Arg_ObjObjEx, Type.GetTypeFromHandle(new RuntimeTypeHandle(srcEEType)), destinationTypeName)); diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/DynamicInvokeInfo.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/DynamicInvokeInfo.cs index cb97cdb27f882..4f2e51afe59da 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/DynamicInvokeInfo.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/DynamicInvokeInfo.cs @@ -72,20 +72,21 @@ public DynamicInvokeInfo(MethodBase method, IntPtr invokeThunk) { Transform transform = default; - Type argumentType = parameters[i].ParameterType; + var argumentType = (RuntimeType)parameters[i].ParameterType; if (argumentType.IsByRef) { _needsCopyBack = true; transform |= Transform.ByRef; - argumentType = argumentType.GetElementType()!; + argumentType = (RuntimeType)argumentType.GetElementType()!; } Debug.Assert(!argumentType.IsByRef); - EETypePtr eeArgumentType = argumentType.TypeHandle.ToEETypePtr(); - - if (eeArgumentType.IsValueType) + // This can return a null MethodTable for reference types. + // The compiler makes sure it returns a non-null MT for everything else. + EETypePtr eeArgumentType = argumentType.ToEETypePtrMayBeNull(); + if (argumentType.IsValueType) { - Debug.Assert(argumentType.IsValueType); + Debug.Assert(eeArgumentType.IsValueType); if (eeArgumentType.IsByRefLike) _argumentCount = ArgumentCount_NotSupported_ByRefLike; @@ -93,15 +94,15 @@ public DynamicInvokeInfo(MethodBase method, IntPtr invokeThunk) if (eeArgumentType.IsNullable) transform |= Transform.Nullable; } - else if (eeArgumentType.IsPointer) + else if (argumentType.IsPointer) { - Debug.Assert(argumentType.IsPointer); + Debug.Assert(eeArgumentType.IsPointer); transform |= Transform.Pointer; } - else if (eeArgumentType.IsFunctionPointer) + else if (argumentType.IsFunctionPointer) { - Debug.Assert(argumentType.IsFunctionPointer); + Debug.Assert(eeArgumentType.IsFunctionPointer); transform |= Transform.FunctionPointer; } @@ -119,19 +120,18 @@ public DynamicInvokeInfo(MethodBase method, IntPtr invokeThunk) { Transform transform = default; - Type returnType = methodInfo.ReturnType; + var returnType = (RuntimeType)methodInfo.ReturnType; if (returnType.IsByRef) { transform |= Transform.ByRef; - returnType = returnType.GetElementType()!; + returnType = (RuntimeType)returnType.GetElementType()!; } Debug.Assert(!returnType.IsByRef); - EETypePtr eeReturnType = returnType.TypeHandle.ToEETypePtr(); - - if (eeReturnType.IsValueType) + EETypePtr eeReturnType = returnType.ToEETypePtrMayBeNull(); + if (returnType.IsValueType) { - Debug.Assert(returnType.IsValueType); + Debug.Assert(eeReturnType.IsValueType); if (returnType != typeof(void)) { @@ -150,17 +150,17 @@ public DynamicInvokeInfo(MethodBase method, IntPtr invokeThunk) _argumentCount = ArgumentCount_NotSupported; // ByRef to void return } } - else if (eeReturnType.IsPointer) + else if (returnType.IsPointer) { - Debug.Assert(returnType.IsPointer); + Debug.Assert(eeReturnType.IsPointer); transform |= Transform.Pointer; if ((transform & Transform.ByRef) == 0) transform |= Transform.AllocateReturnBox; } - else if (eeReturnType.IsFunctionPointer) + else if (returnType.IsFunctionPointer) { - Debug.Assert(returnType.IsFunctionPointer); + Debug.Assert(eeReturnType.IsFunctionPointer); transform |= Transform.FunctionPointer; if ((transform & Transform.ByRef) == 0) @@ -597,6 +597,12 @@ private unsafe ref byte InvokeDirectWithFewArguments( return defaultValue; } + private void ThrowForNeverValidNonNullArgument(EETypePtr srcEEType, int index) + { + Debug.Assert(index != 0 || _isStatic); + throw InvokeUtils.CreateChangeTypeArgumentException(srcEEType, Method.GetParametersAsSpan()[index - (_isStatic ? 0 : 1)].ParameterType, destinationIsByRef: false); + } + private unsafe void CheckArguments( Span copyOfParameters, void* byrefParameters, @@ -636,16 +642,25 @@ private unsafe void CheckArguments( EETypePtr srcEEType = arg.GetEETypePtr(); EETypePtr dstEEType = argumentInfo.Type; - if (!(srcEEType.RawValue == dstEEType.RawValue || - RuntimeImports.AreTypesAssignable(srcEEType, dstEEType) || - (dstEEType.IsInterface && arg is System.Runtime.InteropServices.IDynamicInterfaceCastable castable - && castable.IsInterfaceImplemented(new RuntimeTypeHandle(dstEEType), throwIfNotImplemented: false)))) + if (srcEEType.RawValue != dstEEType.RawValue) { - // ByRefs have to be exact match - if ((argumentInfo.Transform & Transform.ByRef) != 0) - throw InvokeUtils.CreateChangeTypeArgumentException(srcEEType, argumentInfo.Type, destinationIsByRef: true); + // Destination type can be null if we don't have a MethodTable for this type. This means one cannot + // possibly pass a valid non-null object instance here. + if (dstEEType.IsNull) + { + ThrowForNeverValidNonNullArgument(srcEEType, i); + } - arg = InvokeUtils.CheckArgumentConversions(arg, argumentInfo.Type, InvokeUtils.CheckArgumentSemantics.DynamicInvoke, binderBundle); + if (!(RuntimeImports.AreTypesAssignable(srcEEType, dstEEType) || + (dstEEType.IsInterface && arg is System.Runtime.InteropServices.IDynamicInterfaceCastable castable + && castable.IsInterfaceImplemented(new RuntimeTypeHandle(dstEEType), throwIfNotImplemented: false)))) + { + // ByRefs have to be exact match + if ((argumentInfo.Transform & Transform.ByRef) != 0) + throw InvokeUtils.CreateChangeTypeArgumentException(srcEEType, argumentInfo.Type, destinationIsByRef: true); + + arg = InvokeUtils.CheckArgumentConversions(arg, argumentInfo.Type, InvokeUtils.CheckArgumentSemantics.DynamicInvoke, binderBundle); + } } if ((argumentInfo.Transform & Transform.Reference) == 0) @@ -704,16 +719,25 @@ private unsafe void CheckArguments( EETypePtr srcEEType = arg.GetEETypePtr(); EETypePtr dstEEType = argumentInfo.Type; - if (!(srcEEType.RawValue == dstEEType.RawValue || - RuntimeImports.AreTypesAssignable(srcEEType, dstEEType) || - (dstEEType.IsInterface && arg is System.Runtime.InteropServices.IDynamicInterfaceCastable castable - && castable.IsInterfaceImplemented(new RuntimeTypeHandle(dstEEType), throwIfNotImplemented: false)))) + if (srcEEType.RawValue != dstEEType.RawValue) { - // ByRefs have to be exact match - if ((argumentInfo.Transform & Transform.ByRef) != 0) - throw InvokeUtils.CreateChangeTypeArgumentException(srcEEType, argumentInfo.Type, destinationIsByRef: true); + // Destination type can be null if we don't have a MethodTable for this type. This means one cannot + // possibly pass a valid non-null object instance here. + if (dstEEType.IsNull) + { + ThrowForNeverValidNonNullArgument(srcEEType, i); + } - arg = InvokeUtils.CheckArgumentConversions(arg, argumentInfo.Type, InvokeUtils.CheckArgumentSemantics.DynamicInvoke, binderBundle: null); + if (!(RuntimeImports.AreTypesAssignable(srcEEType, dstEEType) || + (dstEEType.IsInterface && arg is System.Runtime.InteropServices.IDynamicInterfaceCastable castable + && castable.IsInterfaceImplemented(new RuntimeTypeHandle(dstEEType), throwIfNotImplemented: false)))) + { + // ByRefs have to be exact match + if ((argumentInfo.Transform & Transform.ByRef) != 0) + throw InvokeUtils.CreateChangeTypeArgumentException(srcEEType, argumentInfo.Type, destinationIsByRef: true); + + arg = InvokeUtils.CheckArgumentConversions(arg, argumentInfo.Type, InvokeUtils.CheckArgumentSemantics.DynamicInvoke, binderBundle: null); + } } if ((argumentInfo.Transform & Transform.Reference) == 0) diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ReflectionInvokeMapNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ReflectionInvokeMapNode.cs index 5014526b677de..ceb0e7ecf76ee 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ReflectionInvokeMapNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ReflectionInvokeMapNode.cs @@ -104,6 +104,11 @@ internal static void AddSignatureDependency(ref DependencyList dependencies, Nod if (type.IsPrimitive || type.IsVoid) return; + // Reflection doesn't need the ability to generate MethodTables out of thin air for reference types. + // Skip generating the dependencies. + if (type.IsGCPointer) + return; + TypeDesc canonType = type.ConvertToCanonForm(CanonicalFormKind.Specific); if (canonType.IsCanonicalSubtype(CanonicalFormKind.Any)) GenericTypesTemplateMap.GetTemplateTypeDependencies(ref dependencies, factory, canonType); diff --git a/src/tests/nativeaot/SmokeTests/TrimmingBehaviors/DeadCodeElimination.cs b/src/tests/nativeaot/SmokeTests/TrimmingBehaviors/DeadCodeElimination.cs index c0c87c847995e..7e5a9e97c69f5 100644 --- a/src/tests/nativeaot/SmokeTests/TrimmingBehaviors/DeadCodeElimination.cs +++ b/src/tests/nativeaot/SmokeTests/TrimmingBehaviors/DeadCodeElimination.cs @@ -13,6 +13,7 @@ public static int Run() { SanityTest.Run(); TestInstanceMethodOptimization.Run(); + TestReflectionInvokeSignatures.Run(); TestAbstractTypeNeverDerivedVirtualsOptimization.Run(); TestAbstractNeverDerivedWithDevirtualizedCall.Run(); TestAbstractDerivedByUnrelatedTypeWithDevirtualizedCall.Run(); @@ -75,6 +76,32 @@ public static void Run() } } + class TestReflectionInvokeSignatures + { + public class Never1 { } + + public static void Invoke1(Never1 inst) { } + + public struct Allocated1 { } + + public static void Invoke2(out Allocated1 inst) { inst = default; } + + public static void Run() + { + { + MethodInfo mi = typeof(TestReflectionInvokeSignatures).GetMethod(nameof(Invoke1)); + mi.Invoke(null, new object[1]); + ThrowIfPresentWithUsableMethodTable(typeof(TestReflectionInvokeSignatures), nameof(Never1)); + } + + { + MethodInfo mi = typeof(TestReflectionInvokeSignatures).GetMethod(nameof(Invoke2)); + mi.Invoke(null, new object[1]); + ThrowIfNotPresent(typeof(TestReflectionInvokeSignatures), nameof(Allocated1)); + } + } + } + class TestAbstractTypeNeverDerivedVirtualsOptimization { class UnreferencedType1