Fix check for invocation of async infrastructure methods via reflection (#121609)

MethodInfo.Invoke has multiple implementation strategies. The check for
async infrastructure methods needs to be placed in the code that is
shared by all implementation strategies.

Before this change, this check would not kick in under managed debugger
or when the test is run with
`Switch.System.Reflection.ForceEmitInvoke=true` appcontext switch.

Also, this check should fail with NotSupportedException, without
wrapping it in TargetInvocationException() to match existing behaviors.

Author: jkotas

src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.CoreCLR.cs

@@ -78,6 +78,9 @@ private ParameterInfo[] FetchNonReturnParameters() =>
 
         private ParameterInfo FetchReturnParameter() =>
             m_returnParameter ??= RuntimeParameterInfo.GetReturnParameter(this, this, Signature);
+
+        private bool IsDisallowedAsyncHelper =>
+            RuntimeMethodHandle.IsAsyncMethod(new RuntimeMethodHandleInternal(m_handle));
         #endregion
 
         #region Internal Members

src/coreclr/System.Private.CoreLib/src/System/RuntimeHandles.cs

@@ -1337,6 +1337,9 @@ internal static IRuntimeMethodInfo StripMethodInstantiation(IRuntimeMethodInfo m
         [MethodImpl(MethodImplOptions.InternalCall)]
         internal static extern bool IsConstructor(RuntimeMethodHandleInternal method);
 
+        [MethodImpl(MethodImplOptions.InternalCall)]
+        internal static extern bool IsAsyncMethod(RuntimeMethodHandleInternal method);
+
         [MethodImpl(MethodImplOptions.InternalCall)]
         private static extern LoaderAllocator GetLoaderAllocatorInternal(RuntimeMethodHandleInternal method);
 

src/coreclr/vm/ecalllist.h

@@ -148,6 +148,7 @@ FCFuncStart(gRuntimeMethodHandle)
     FCFuncElement("GetMethodFromCanonical", RuntimeMethodHandle::GetMethodFromCanonical)
     FCFuncElement("IsDynamicMethod", RuntimeMethodHandle::IsDynamicMethod)
     FCFuncElement("IsConstructor", RuntimeMethodHandle::IsConstructor)
+    FCFuncElement("IsAsyncMethod", RuntimeMethodHandle::IsAsyncMethod)
     FCFuncElement("GetResolver", RuntimeMethodHandle::GetResolver)
     FCFuncElement("GetLoaderAllocatorInternal", RuntimeMethodHandle::GetLoaderAllocatorInternal)
 FCFuncEnd()

src/coreclr/vm/reflectioninvocation.cpp

@@ -342,11 +342,6 @@ extern "C" void QCALLTYPE RuntimeMethodHandle_InvokeMethod(
         COMPlusThrow(kNotSupportedException, W("NotSupported_Type"));
     }
 
-    if (pMeth->IsAsyncMethod())
-    {
-        COMPlusThrow(kNotSupportedException, W("NotSupported_Async"));
-    }
-
 #ifdef _DEBUG
     if (g_pConfig->ShouldInvokeHalt(pMeth))
     {

src/coreclr/vm/runtimehandles.cpp

@@ -2120,8 +2120,19 @@ FCIMPL1(FC_BOOL_RET, RuntimeMethodHandle::IsConstructor, MethodDesc *pMethod)
     }
     CONTRACTL_END;
 
-    BOOL ret = (BOOL)pMethod->IsClassConstructorOrCtor();
-    FC_RETURN_BOOL(ret);
+    FC_RETURN_BOOL(pMethod->IsClassConstructorOrCtor());
+}
+FCIMPLEND
+
+FCIMPL1(FC_BOOL_RET, RuntimeMethodHandle::IsAsyncMethod, MethodDesc *pMethod)
+{
+    CONTRACTL {
+        FCALL_CHECK;
+        PRECONDITION(CheckPointer(pMethod));
+    }
+    CONTRACTL_END;
+
+    FC_RETURN_BOOL(pMethod->IsAsyncMethod());
 }
 FCIMPLEND
 

src/coreclr/vm/runtimehandles.h

@@ -215,6 +215,8 @@ class RuntimeMethodHandle
 
     static FCDECL1(FC_BOOL_RET, IsConstructor, MethodDesc *pMethod);
 
+    static FCDECL1(FC_BOOL_RET, IsAsyncMethod, MethodDesc *pMethod);
+
     static FCDECL1(Object*, GetLoaderAllocatorInternal, MethodDesc *pMethod);
 };
 

src/libraries/System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.cs

@@ -16,10 +16,12 @@ internal InvocationFlags ComputeAndUpdateInvocationFlags()
             InvocationFlags invocationFlags = InvocationFlags.Unknown;
 
             Type? declaringType = DeclaringType;
+            Type returnType = ReturnType;
 
             if (ContainsGenericParameters // Method has unbound generics
-                || IsDisallowedByRefType(ReturnType) // Return type is an invalid by-ref (i.e., by-ref-like or void*)
+                || IsDisallowedByRefType(returnType) // Return type is an invalid by-ref (i.e., by-ref-like or void*)
                 || (CallingConvention & CallingConventions.VarArgs) == CallingConventions.VarArgs // Managed varargs
+                || IsDisallowedAsyncHelper
                 )
             {
                 invocationFlags = InvocationFlags.NoInvoke;
@@ -38,7 +40,7 @@ internal InvocationFlags ComputeAndUpdateInvocationFlags()
                     }
                 }
 
-                if (ReturnType.IsByRefLike) // Check for byref-like types for return
+                if (returnType.IsByRefLike) // Check for byref-like types for return
                 {
                     invocationFlags |= InvocationFlags.ContainsStackPointers;
                 }
@@ -60,6 +62,8 @@ static bool IsDisallowedByRefType(Type type)
         [DoesNotReturn]
         internal void ThrowNoInvokeException()
         {
+            Type? declaringType = DeclaringType;
+
             // method is on a class that contains stack pointers
             if ((InvocationFlags & InvocationFlags.ContainsStackPointers) != 0)
             {
@@ -71,7 +75,7 @@ internal void ThrowNoInvokeException()
                 throw new NotSupportedException();
             }
             // method is generic or on a generic class
-            else if (DeclaringType!.ContainsGenericParameters || ContainsGenericParameters)
+            else if ((declaringType != null && declaringType.ContainsGenericParameters) || ContainsGenericParameters)
             {
                 throw new InvalidOperationException(SR.Arg_UnboundGenParam);
             }
@@ -88,6 +92,10 @@ internal void ThrowNoInvokeException()
                 if (elementType == typeof(void))
                     throw new NotSupportedException(SR.NotSupported_ByRefToVoidReturn);
             }
+            else if (IsDisallowedAsyncHelper)
+            {
+                throw new NotSupportedException(SR.NotSupported_Async);
+            }
 
             throw new TargetException();
         }

src/mono/System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.Mono.cs

@@ -705,6 +705,8 @@ public override IList<CustomAttributeData> GetCustomAttributesData()
         }
 
         public sealed override bool HasSameMetadataDefinitionAs(MemberInfo other) => HasSameMetadataDefinitionAsCore<RuntimeMethodInfo>(other);
+
+        private static bool IsDisallowedAsyncHelper => false;
     }
 #region Sync with _MonoReflectionMethod in object-internals.h
     [StructLayout(LayoutKind.Sequential)]

src/tests/async/reflection/reflection-simple.cs

@@ -27,11 +27,7 @@ public static void MethodInfo_Invoke_AsyncHelper()
     {
         var mi = typeof(System.Runtime.CompilerServices.AsyncHelpers).GetMethod("Await", BindingFlags.Static | BindingFlags.Public, new Type[] { typeof(Task) })!;
         Assert.NotNull(mi);
-
-        if (TestLibrary.Utilities.IsNativeAot)
-            Assert.Throws<NotSupportedException>(() => mi.Invoke(null, new object[] { FooTask() }));
-        else
-            Assert.Throws<TargetInvocationException>(() => mi.Invoke(null, new object[] { FooTask() }));
+        Assert.Throws<NotSupportedException>(() => mi.Invoke(null, new object[] { FooTask() }));
 
         // Sadly the following does not throw and results in UB
         // We cannot completely prevent putting a token of an Async method into IL stream.