Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reflection Invoke does not check ContainsGenericParameters #95245

Closed
Suchiman opened this issue Mar 27, 2021 · 4 comments · Fixed by #98408
Closed

Reflection Invoke does not check ContainsGenericParameters #95245

Suchiman opened this issue Mar 27, 2021 · 4 comments · Fixed by #98408
Labels
area-NativeAOT-coreclr good first issue Issue should be easy to implement, good for first-time contributors help wanted [up-for-grabs] Good issue for external contributors

Comments

@Suchiman
Copy link
Contributor

Suchiman commented Mar 27, 2021

    class Program
    {
        static void Main(string[] args)
        {
            var m = typeof(SimpleType).GetMethod("GenericMethod");
            //m = m.MakeGenericMethod(typeof(string));
            m.Invoke(new SimpleType(), new object[0]);
        }
    }

    class SimpleType
    {
        public void GenericMethod<T>() { }
    }

Expected Behavior:
Throws

Unhandled exception. System.InvalidOperationException: Late bound operations cannot be performed on types or methods for which ContainsGenericParameters is true.
   at System.Reflection.RuntimeMethodInfo.ThrowNoInvokeException()
   at System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
   at System.Reflection.MethodBase.Invoke(Object obj, Object[] parameters)
   at Program.Main(String[] args)

Actual Behavior:
Throws

Unhandled exception. System.NotSupportedException: This object cannot be invoked because no code was generated for it: 'SimpleType.GenericMethod()'.
   at System.Reflection.Runtime.MethodInfos.RuntimeNamedMethodInfo`1.GetUncachedMethodInvoker(RuntimeTypeInfo[], MemberInfo) + 0x2f
   at System.Reflection.Runtime.MethodInfos.RuntimeMethodInfo.Invoke(Object, BindingFlags, Binder, Object[], CultureInfo) + 0x2b
   at Program.Main(String[] args) + 0x59
@MichalStrehovsky
Copy link
Member

We can probably follow dotnet/runtimelab#858 and add that check there too.

@MichalStrehovsky MichalStrehovsky added good first issue Issue should be easy to implement, good for first-time contributors help wanted [up-for-grabs] Good issue for external contributors labels Mar 28, 2021
@hez2010
Copy link
Contributor

hez2010 commented Apr 4, 2021

I don't think this should be fixed in this way.
The class SimpleType definitely can contain a method without any generic parameter public void GenericMethod() { }, and this method may be trimmed during compilation and cause MissingRuntimeArtifactException.

@MichalStrehovsky
Copy link
Member

If a non-generic method gets completely trimmed away, we don't throw an exception; the reflection stack acts as if it was never there.

Try:

using System;

class Program
{
    static Type programType = typeof(Program);
    static string unusedMethodString = "UnusedMethod";

    public static void UnusedMethod() => Console.WriteLine("Hello");

    static void Main()
    {
        programType.GetMethod(unusedMethodString).Invoke(null, Array.Empty<object>());
    }
}

This will throw a NullReferenceException (with trimming enabled) because UnusedMethod got trimmed.

You will also get a compile-time warning that there's a problem:

C:\git\runtime\src\coreclr\tools\aot\ILCompiler\repro\Program.cs(12): Trim analysis warning IL2080: Program.Main(): 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicMethods' in call to 'System.Type.GetMethod(String)'. The field 'Program.programType' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.

(You need to opt into the warning with <SuppressTrimAnalysisWarnings>false</SuppressTrimAnalysisWarnings> because they're suppressed by default - we're hoping to change the default in .NET 6).

(If you change the above program to not use static fields, the compiler can actually figure this reflection out and there will be no warnings.)

The fix I proposed won't change this experience.

@Suchiman
Copy link
Contributor Author

Still reproduces on .NET 9.0 daily builds, should probably be moved to dotnet/runtime now

@jkotas jkotas transferred this issue from dotnet/runtimelab Nov 26, 2023
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Nov 26, 2023
MichalStrehovsky added a commit to MichalStrehovsky/runtime that referenced this issue Feb 14, 2024
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Feb 14, 2024
@ghost ghost removed in-pr There is an active PR which will close this issue when it is merged untriaged New issue has not been triaged by the area owner labels Feb 14, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Mar 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-NativeAOT-coreclr good first issue Issue should be easy to implement, good for first-time contributors help wanted [up-for-grabs] Good issue for external contributors
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants