dotnet / dotnet/runtime

[wasm] Mark the Marshal function-pointer/delegate APIs unsupported on wasm instead of custom build warnings

Open
#133,123 5 comments 0 reactions 1 assignee Claimed by @radekdoulik View on GitHub
arch-wasm area-System.Runtime.InteropServices
Dominant language
C#
Stars
18.3k
Forks
5.6k
PR merge metrics
PR metrics pending

Description

## Background

This comes out of review feedback on https://github.com/dotnet/runtime/pull/131877, where the CoreCLR wasm interop generator had grown a build warning (`WASM0065`) for methods annotated with `[MonoPInvokeCallback]`, telling the user that the attribute does not make the method callable from native code.

@jkotas pushed back on that approach ([comment](https://github.com/dotnet/runtime/pull/131877#discussion_r3908129061)):

> The correct way to do this would be to mark `Marshal.GetDelegateForFunctionPointer` as unsupported on wasm. It will catch all use cases and not just the ones that somebody happened to mark with `MonoPInvokeCallbackAttribute`, it is regular warning that can be disabled if the user knows what they are doing, etc. You can open an issue about this - I believe it was discussed before, but I am not able to find it.
>
> I do not think we should be building parallel platform compatibility analyzers like this.

The warning has since been removed from that PR, so nothing diagnoses this today at build time.

## The ask

Use the platform compatibility analyzer (`[UnsupportedOSPlatform]`) for the `Marshal` APIs that cannot work on wasm, rather than hand-rolled per-attribute warnings in a wasm build task.

Advantages over the removed warning:

- it covers every use, not only the methods someone happened to annotate with a Mono-specific attribute
- it is a regular, suppressible warning (`CA1416`) for users who know what they are doing
- there is one platform analyzer rather than a parallel one per toolchain

## Which API

The comment names `Marshal.GetDelegateForFunctionPointer`, but the `[MonoPInvokeCallback]` pattern that motivated the warning goes the other direction — a managed method handed to native code, i.e. `Marshal.GetFunctionPointerForDelegate`:

```csharp
[MonoPInvokeCallback(typeof(MyCallback))]
static void Callback(int x) { }

IntPtr fn = Marshal.GetFunctionPointerForDelegate(new MyCallback(Callback));
```

So part of this issue is deciding the exact set — plausibly both directions, and both the generic and non-generic overloads.

## Current state

Neither API carries a platform annotation today; in `src/libraries/System.Runtime.InteropServices/ref/System.Runtime.InteropServices.cs` the only annotation on the non-generic overloads is `RequiresDynamicCode`:

```csharp
[RequiresDynamicCode("Marshalling code for the delegate might not be available. Use the GetFunctionPointerForDelegate overload instead.")]
public static IntPtr GetFunctionPointerForDelegate(Delegate d) { throw null; }
public static IntPtr GetFunctionPointerForDelegate(TDelegate d) where TDelegate : notnull { throw null; }
```

## Open questions

- Which platforms: `browser` only, or `wasi` too? Does this differ between Mono and CoreCLR wasm?
- Does any supported scenario still rely on these APIs on wasm, which would make a blanket annotation too broad?
- Is annotating a shipping API this way something that needs to go through API review?

## Possibly the earlier discussion

@jkotas thought this had been discussed before. These look related, though none of them is a proposal to add the platform annotation:

- https://github.com/dotnet/runtime/issues/104391 — `[Mono/WASM] Marshal.GetFunctionPointerForDelegate crashes the runtime` (open)
- https://github.com/dotnet/runtime/issues/39187 — `GetFunctionPointerForDelegateTests tests fail with Specified cast is not valid on Browser` (open)
- https://github.com/dotnet/runtime/issues/101736 — `[Mono][Wasm][Interp] Assertion fail in Marshal.GetFunctionPointerForDelegate` (open)
- https://github.com/dotnet/runtime/pull/116107 — `[browser] throw PNSE from GetFunctionPointerForDelegate`, which targeted the runtime-side behaviour rather than a build-time diagnostic

cc @jkotas @lewing @pavelsavara

> [!NOTE]
> This issue was drafted with GitHub Copilot.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.