[mono] UnsafeAccessor to a virtual method on a base class through a derived type asserts

[vitek-karas]

    class InheritanceBase
    {
        protected virtual string BaseVirtual() => $"{nameof(InheritanceBase)}.{nameof(BaseVirtual)}";
    }

    class InheritanceDerived : InheritanceBase
    {
    }

    [Fact]
    public static void Verify_InheritanceMethodResolution()
    {
        var instance = new InheritanceDerived();
        Assert.Equal($"{nameof(InheritanceBase)}.{nameof(BaseVirtual)}", BaseVirtual(instance));

        [UnsafeAccessor(UnsafeAccessorKind.Method, Name = nameof(BaseVirtual))]
        extern static string BaseVirtual(InheritanceDerived target);
    }

This fails with

* Assertion at F:\dotnet\runtime4\src\mono\mono\metadata\marshal-lightweight.c:2508, condition `target_method->klass == target_class' not met

@lambdageek - could you please route this to the right area/person

[vitek-karas]

Actually this fails on NativeAOT as well (at runtime!):

System.MissingMethodException: Method not found: 'BaseVirtual'.
   at Internal.Runtime.TypeLoaderExceptionHelper.CreateMissingMethodException(ExceptionStringID, String) + 0x63
   at Internal.Runtime.CompilerHelpers.ThrowHelpers.ThrowMissingMethodException(ExceptionStringID, String) + 0x25
   at UnsafeAccessorsTests.<Verify_InheritanceMethodResolution_BaseViaDerived>g__BaseVirtual|33_0(UnsafeAccessorsTests.InheritanceDerived target) + 0x1f
   at UnsafeAccessorsTests.Verify_InheritanceMethodResolution_BaseViaDerived() + 0x4b

CoreCLR works and the same code without UnsafeAccessor:

var instance = new InheritanceDerived();
instance.BaseVirtual();

will also work (probably on all runtimes, although I didn't try that).

@AaronRobinsonMSFT @MichalStrehovsky what do you think - should this work?

[MichalStrehovsky]

Well method BaseVirtual is not on InheritanceDerived so this is not expected to work.

This is what I brough up in #86932 (comment) and was rejected.

[AaronRobinsonMSFT]

@vitek-karas If I update as following it works, right?

        [UnsafeAccessor(UnsafeAccessorKind.Method, Name = nameof(BaseVirtual))]
        extern static string BaseVirtual(InheritanceBase target);

If so, I don't see why one would declare this as InheritanceDerived, when the target method is defined on a different type. It isn't as if I can know about non-public methods by looking at a derived type. The first thing I would do would be to look at InheritanceDerived, not see a non-public method and be rather confused.

[MichalStrehovsky]

Does this work with CoreCLR?

[vitek-karas]

Yes - it does work on CoreCLR.

[MichalStrehovsky]

Yes - it does work on CoreCLR.

@AaronRobinsonMSFT I think this should be in 8.0. We need to either block this in CoreCLR, or fix in NativeAOT/Mono.

[MichalStrehovsky]

Btw, I think the CoreCLR behavior is the correct one

[vitek-karas]

Personally I agree with Michal's comment in the other issue that this "should" work. That said I'd be fine if we said that we don't want this to work. But then all runtimes should behave consistently.
Especially differences between CoreCLR and NativeAOT are dangerous (since we promise that AOTed apps behave the same as non-AOTed app)

[AaronRobinsonMSFT]

Personally I agree with Michal's comment in the other issue that this "should" work.

What needs to happen then in NativeAOT? Based on some offline conversations with @fanyang-mono, I believe she is looking into enabling it on mono.

[MichalStrehovsky]

What needs to happen then in NativeAOT?

We'd walk into the base type, like we do when resolving a MemberRef:

runtime/src/coreclr/tools/Common/TypeSystem/Ecma/EcmaModule.cs

Line 525 in 9343b2f

// Try to resolve the name and signature in the current type, or any of the base types.

(I assume it matches what CoreCLR does in the UnsafeAccessor case too.)

[AaronRobinsonMSFT]

(I assume it matches what CoreCLR does in the UnsafeAccessor case too.)

Yep. I just used the MethodIterator. Seems like it does that under the covers but it wasn't obvious.

[lambdageek]

I think the work in Mono is just to replace the assert that the target class is equal to the method's class by a check that the target class is assignable to the method's class.

[fanyang-mono]

This has been resolved via #89217.