Skip to content
New issue

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

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

Already on GitHub? # to your account

Throw InvalidOperationException on RuntimeMethodHandle.GetFunctionPointer() for generic methods #104644

Merged
merged 3 commits into from
Jul 16, 2024

Conversation

steveharter
Copy link
Member

Fixes #101664

On CoreClr, the previous exception was InvalidProgramException. On Mono, the call caused a crash.

@steveharter steveharter added this to the 9.0.0 milestone Jul 9, 2024
@steveharter steveharter self-assigned this Jul 9, 2024
@lambdageek
Copy link
Member

lambdageek commented Jul 10, 2024

@steveharter what should this do:

public class G<T>
{
    public static T Method(T x) => x;
}

public class Program
{
    public static void Main()
    {
        var mi = typeof(G<>).GetMethod("Method", BindingFlags.Public|BindingFlags.Static);
        var h = mi.MethodHandle;
        var fp = h.GetFunctionPointer();
        Console.WriteLine(fp);
    }
}  

On CoreCLR this works (returns some kind of non-null value for fp).
On Mono interp it works too.
On Mono JIT it crashes:

* Assertion at /Users/runner/work/1/s/src/mono/mono/mini/mini-generic-sharing.c:3368, condition `!mono_class_is_gtd (class_vtable->klass)' not met

(same behavior on .NET 9 preview 6 and .NET 8, across both runtimes)

@@ -2938,6 +2938,11 @@ mono_jit_compile_method_jit_only (MonoMethod *method, MonoError *error)
static gpointer
get_ftnptr_for_method (MonoMethod *method, gboolean need_unbox, MonoError *error)
{
if (method->is_generic) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we want to also rule out methods from a generic type definition, then you may want to add || mono_class_is_gtd (method->klass)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes we should throw if the method is not callable meaning a "true" generic method and a generic method that uses the type parameters from the owning class. It seems like IsGenericMethod() should return true even if the type parameters come from the owning class, but that's a different problem...

I'll make another pass here. Thanks

@buyaa-n
Copy link
Contributor

buyaa-n commented Jul 16, 2024

Merging as @steveharter is out

@buyaa-n buyaa-n merged commit 9565550 into dotnet:main Jul 16, 2024
140 of 159 checks passed
@steveharter steveharter deleted the FncPtrReflection branch July 22, 2024 18:19
@github-actions github-actions bot locked and limited conversation to collaborators Aug 22, 2024
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RuntimeMethodHandle.GetFunctionPointer() should check if generic method is constructed
4 participants