[Mono/WASM] Marshal.GetFunctionPointerForDelegate crashes the runtime

[SingleAccretion]

Reproduction:

> dotnet new wasmconsole

using System;
using System.Runtime.InteropServices;

public unsafe class Program
{
    public static void Main()
    {
        nint fptr = Marshal.GetFunctionPointerForDelegate(new Action(() => Console.WriteLine("Managed method callee")));
        ((delegate* unmanaged<void>)fptr)();
    }
}

> dotnet run

Expected result: no crash.
Actual result:

RuntimeError: memory access out of bounds
    at strlen (wasm://wasm/03d4c862:wasm-function[12385]:0x28e52f)
    at get_native_to_interp (wasm://wasm/03d4c862:wasm-function[13709]:0x2b0d0f)
    at mono_wasm_get_native_to_interp_trampoline (wasm://wasm/03d4c862:wasm-function[4997]:0x14a8bb)
    at interp_create_method_pointer (wasm://wasm/03d4c862:wasm-function[155]:0x1a9f2)
    at jit_compile_method_with_opt_cb (wasm://wasm/03d4c862:wasm-function[4749]:0x134c4e)
    at mono_llvm_cpp_catch_exception (wasm://wasm/03d4c862:wasm-function[4999]:0x14a92f)
    at mono_llvm_catch_exception (wasm://wasm/03d4c862:wasm-function[5066]:0x14f5f8)
    at jit_compile_method_with_opt (wasm://wasm/03d4c862:wasm-function[4748]:0x134b5e)
    at mono_jit_compile_method (wasm://wasm/03d4c862:wasm-function[4746]:0x134981)
    at mono_compile_method_checked (wasm://wasm/03d4c862:wasm-function[3628]:0xf908b)
    at mono_delegate_to_ftnptr_impl (wasm://wasm/03d4c862:wasm-function[3142]:0xdb697)
    at ves_icall_System_Runtime_InteropServices_Marshal_GetFunctionPointerForDelegateInternal (wasm://wasm/03d4c862:wasm-function[3268]:0xe45dc)
    at ves_icall_System_Runtime_InteropServices_Marshal_GetFunctionPointerForDelegateInternal_raw (wasm://wasm/03d4c862:wasm-function[2835]:0xcc674)
    at do_icall (wasm://wasm/03d4c862:wasm-function[206]:0x1d577)
    at do_icall_wrapper (wasm://wasm/03d4c862:wasm-function[126]:0x19979)
    at mono_interp_exec_method (wasm://wasm/03d4c862:wasm-function[117]:0xb236)
    at interp_runtime_invoke (wasm://wasm/03d4c862:wasm-function[160]:0x1aed3)
    at mono_jit_runtime_invoke (wasm://wasm/03d4c862:wasm-function[4781]:0x1387a9)
    at do_runtime_invoke (wasm://wasm/03d4c862:wasm-function[3600]:0xf6dfe)
    at mono_runtime_invoke_checked (wasm://wasm/03d4c862:wasm-function[3599]:0xf6d98)
    at mono_runtime_try_invoke_byrefs (wasm://wasm/03d4c862:wasm-function[3697]:0xfd55c)
    at ves_icall_InternalInvoke (wasm://wasm/03d4c862:wasm-function[2620]:0xc15dd)
    at ves_icall_InternalInvoke_raw (wasm://wasm/03d4c862:wasm-function[2783]:0xca1b9)
    at do_icall (wasm://wasm/03d4c862:wasm-function[206]:0x1d64c)
    at do_icall_wrapper (wasm://wasm/03d4c862:wasm-function[126]:0x19979)
    at mono_interp_exec_method (wasm://wasm/03d4c862:wasm-function[117]:0xb236)
    at interp_runtime_invoke (wasm://wasm/03d4c862:wasm-function[160]:0x1aed3)
    at mono_jit_runtime_invoke (wasm://wasm/03d4c862:wasm-function[4781]:0x1387a9)
    at do_runtime_invoke (wasm://wasm/03d4c862:wasm-function[3600]:0xf6dfe)
    at mono_runtime_try_invoke (wasm://wasm/03d4c862:wasm-function[3609]:0xf787d)
    at mono_runtime_invoke (wasm://wasm/03d4c862:wasm-function[3646]:0xfa46c)
    at mono_wasm_invoke_jsexport (wasm://wasm/03d4c862:wasm-function[13673]:0x2af762)

More context: dotnet/runtimelab#2624.

[dotnet-policy-service]

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

[nazar554]

I don't think that it matters here, but isn't GC.KeepAlive required to keep the delegate alive?

[SingleAccretion]

I don't think that it matters here, but isn't GC.KeepAlive required to keep the delegate alive?

Yes, in actual code one needs to keep the delegate alive explicitly.

[dotnet-policy-service]

Tagging subscribers to this area: @BrzVlad, @kotlarmilos
See info in area-owners.md if you want to be subscribed.

[BrzVlad]

This particular crash is caused by a failure to obtain the managed method associated with a native to managed wrapper. However, even if we were to fix this, the testcase couldn't possible work on wasm interp nowadays. This is because, in order to obtain a ftnptr for entering execution of a method with interpreter, we require a thunk that receives all arguments of the call and calls into interpreter while passing an additional argument, the interp method to execute. Currently, these thunks are generated during app compilation, by build tasks in PInvokeTableGenerator.cs, only for methods with UnmanagedCallersOnly attribute. Given Marshal.GetFunctionPointerForDelegate requires a delegate, which I don't think can be created for a method with UnmanagedCallersOnly attribute, this API can't work on wasm interp.

I think a fix for this would be to completely stop relying, if possible, on the pinvoke table generator and always generate these thunks on the fly with the wasm jit support. Was this already brought into discussion ? @lewing @kg

[kg]

I think I've raised the possibility of jitting trampolines for this in browser WASM before but we've never had a discussion about whether to do it or when.

It seems like we could do it on android too (do we already?) but couldn't do it on iOS or wasi.

[lewing]

There are some separate issues here:

1. we're running strlen on a null string instead of throwing PNSE like it used to
2. on browser we could jit trampolines, should we?
3. on wasi we could do something terrible hacks with preallocating pools by signature if we really needed to

I would definitely like to fix 1.

[lewing]

I need to verify that this throws an exception rather than crashing now.