Fix check for invocation of async infrastructure methods via reflection

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 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,9 +16,10 @@ 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
                 )
             {
@@ -36,9 +37,13 @@ internal InvocationFlags ComputeAndUpdateInvocationFlags()
                     {
                         invocationFlags |= InvocationFlags.ContainsStackPointers;
                     }
+                    else if (IsDisallowedAsyncHelper)
+                    {
+                        invocationFlags = InvocationFlags.NoInvoke;
+                    }
                 }
 
-                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 +65,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 +78,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 +95,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

@@ -930,5 +930,7 @@ public override int MetadataToken
 
         [MethodImplAttribute(MethodImplOptions.InternalCall)]
         internal static extern int get_metadata_token(RuntimeConstructorInfo method);
+
+        private IsDisallowedAsyncHelper => false;
     }
 }

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.