[browser] throw PNSE from GetFunctionPointerForDelegate by pavelsavara · Pull Request #116107 · dotnet/runtime

pavelsavara · 2025-05-29T18:15:14Z

throw meaningful exception
WBT unit test

dotnet-policy-service · 2025-05-29T18:15:32Z

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

Copilot

Pull Request Overview

Adds support for throwing a PlatformNotSupportedException when GetFunctionPointerForDelegate is called on methods without an [UnmanagedCallersOnly] attribute, and wires up end-to-end tests in the Wasm Basic Test App and build tests.

Introduces a null‐wrapper check in interp_create_method_pointer to produce a PNSE with a descriptive message.
Extends main.js and the WebAssembly test app to invoke the new scenario.
Adds both an in-browser unit test (MiscTests.cs) and a build‐time Xunit test.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
src/mono/mono/mini/interp/interp.c	Checks for missing native‐to‐managed wrapper and throws PNSE
src/mono/browser/runtime/runtime.c	Adds an assertion to ensure `method` is not null
src/mono/wasm/testassets/WasmBasicTestApp/App/wwwroot/main.js	Adds “GetFunctionPointerForDelegate” case to the browser harness
src/mono/wasm/testassets/WasmBasicTestApp/App/MiscTests.cs	JSExport method to call `GetFunctionPointerForDelegate`
src/mono/wasm/Wasm.Build.Tests/MiscTests.cs	Xunit test to verify the PNSE and its message

Comments suppressed due to low confidence (2)

src/mono/wasm/testassets/WasmBasicTestApp/App/wwwroot/main.js:292

[nitpick] The variable name retMics is ambiguous; consider renaming it to something more descriptive (e.g., result or returnValue).

const retMics = testGetFunctionPointerForDelegate();

src/mono/mono/mini/interp/interp.c:3475

The variable error is not defined in this scope (the parameter is named e). This should call mono_error_set_platform_not_supported(e, msg).

mono_error_set_platform_not_supported (error, msg);

jkotas · 2025-05-29T18:23:22Z

throw PNSE from GetFunctionPointerForDelegate for methods without UnmanagedCallersOnly

Should GetFunctionPointerForDelegate throw PNSE unconditionally on browser?

GetFunctionPointerForDelegate does not work on methods with UnmanagedCallersOnly either (outside browser at least). For example, the following will fail to compile with error CS8902: 'MyMethod()' is attributed with 'UnmanagedCallersOnly' and cannot be converted to a delegate type.:

using System.Runtime.InteropServices;

Marshal.GetFunctionPointerForDelegate(new Action(MyMethod));

[UnmanagedCallersOnly]
static void MyMethod()
{
}

pavelsavara · 2025-05-29T19:02:30Z

Should GetFunctionPointerForDelegate throw PNSE unconditionally on browser?

Hmmm, good point.

I'm looking at mono/mono#20076 but I'm not any wiser.

@lewing do you know why we have

runtime/src/mono/mono/mini/interp/interp.c

Lines 3468 to 3499 in e5866fe

    
           #ifdef HOST_WASM 
        
           	if (method->wrapper_type == MONO_WRAPPER_NATIVE_TO_MANAGED) { 
        
           		WrapperInfo *info = mono_marshal_get_wrapper_info (method); 
        
           		MonoMethod *orig_method = info->d.native_to_managed.method; 
        
           		/* 
        
           		 * These are called from native code. Ask the host app for a trampoline. 
        
           		 */ 
        
           		MonoFtnDesc *ftndesc = g_new0 (MonoFtnDesc, 1); 
        
           		ftndesc->addr = entry_func; 
        
           		ftndesc->arg = imethod; 
        
           		addr = mono_wasm_get_native_to_interp_trampoline (orig_method, ftndesc); 
        
           		if (addr) { 
        
           			mono_memory_barrier (); 
        
           			imethod->jit_entry = addr; 
        
           			return addr; 
        
           		} 
        
           		/* 
        
           		 * The runtime expects a function pointer unique to method and 
        
           		 * the native caller expects a function pointer with the 
        
           		 * right signature, so fail right away. 
        
           		 */ 
        
           		char *s = mono_method_get_full_name (orig_method); 
        
           		char *msg = g_strdup_printf ("No native to managed transition for method '%s', missing [UnmanagedCallersOnly] attribute.", s); 
        
           		mono_error_set_platform_not_supported (error, msg); 
        
           		g_free (s); 
        
           		g_free (msg); 
        
           		return NULL; 
        
           	} 
        
           #endif

Is this for interp->AOT transitions only ?

pavelsavara · 2025-05-29T19:12:37Z

#56883 introduced the message about UnmanagedCallersOnly

lewing · 2025-05-29T22:08:21Z

I'd need to a few minutes to investigate to be sure, but I suspect this just a case where we are ending up in interp_create_method_pointer from different paths. We do expect a native to interp trampoline to exist for UCO methods (if the native build is enabled), that does not mean we intended GetFunctionPointerForDelegate to succeed if the attribute exists.

jkotas · 2025-05-31T03:44:44Z

BTW: You can reuse message added in https://github.com/dotnet/runtime/pull/116010/files#diff-d9a9b46d55b2e1fddcd071cc75b0408f92f33befa5ea92d5b6aa0c67ad437071R4365 to make this API unsupported on wasm.

src/mono/mono/mini/interp/interp.c

src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.cs

jkotas · 2025-06-04T11:39:39Z

dotnet/dotnet-api-docs#11401 is a docs updated related to this change:

src/mono/wasm/testassets/WasmBasicTestApp/App/MiscTests.cs

src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.cs

jkotas · 2025-06-09T17:17:56Z

src/libraries/Common/src/Interop/Interop.Ldap.cs

                {
                    _managed = managed;
                    _native.sizeofcallback = sizeof(Native);
+#pragma warning disable CA1416 // This call site is reachable on all platforms.


Should the Wasm compat check be disabled in .csproj files for libraries like Ldap instead?

pavelsavara added this to the 10.0.0 milestone May 29, 2025

pavelsavara self-assigned this May 29, 2025

Copilot AI review requested due to automatic review settings May 29, 2025 18:15

pavelsavara added arch-wasm WebAssembly architecture area-Interop-mono labels May 29, 2025

pavelsavara requested review from ilonatommy and maraf as code owners May 29, 2025 18:15

pavelsavara added the os-browser Browser variant of arch-wasm label May 29, 2025

pavelsavara requested review from BrzVlad, kotlarmilos, lewing, steveisok and vitek-karas as code owners May 29, 2025 18:15

Copilot AI reviewed May 29, 2025

View reviewed changes

build-analysis bot mentioned this pull request May 29, 2025

CI flakiness: mono interpreter build getting killed #114123

Open

This was referenced May 29, 2025

MSBuild crashing in the build #92290

Open

The Operation will be canceled. The next steps may not contain expected logs. dotnet/dnceng#3008

Open

jkotas mentioned this pull request May 30, 2025

ThunkPool doesn't work with PROCESS_MITIGATION_DYNAMIC_CODE_POLICY #113114

Closed

jkotas reviewed Jun 3, 2025

View reviewed changes

src/mono/mono/mini/interp/interp.c Outdated Show resolved Hide resolved

build-analysis bot mentioned this pull request Jun 3, 2025

System.Net.Http.Functional.Tests timeouts #115683

Closed

jkotas reviewed Jun 4, 2025

View reviewed changes

src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.cs Outdated Show resolved Hide resolved

pavelsavara changed the title ~~[browser] throw PNSE from GetFunctionPointerForDelegate for methods without UnmanagedCallersOnly~~ [browser] throw PNSE from GetFunctionPointerForDelegate Jun 4, 2025

akoeplinger reviewed Jun 6, 2025

View reviewed changes

src/mono/wasm/testassets/WasmBasicTestApp/App/MiscTests.cs Outdated Show resolved Hide resolved

src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.cs Outdated Show resolved Hide resolved

feeedback

a25e98d

pavelsavara force-pushed the get_native_to_interp branch from 27fed36 to a25e98d Compare June 9, 2025 11:11

pavelsavara added 2 commits June 9, 2025 13:39

more

0de1052

more

f98552d

build-analysis bot mentioned this pull request Jun 9, 2025

Intermittent build failure in AfterSourceBuild: "Could not write state file" #76488

Open

pavelsavara added 2 commits June 9, 2025 16:16

more

3f41716

more

132c53a

jkotas reviewed Jun 9, 2025

View reviewed changes

more

d4cbc38

build-analysis bot mentioned this pull request Jun 9, 2025

System.Net.Quic.Tests failed with "System.Net.Quic.QuicException : The connection timed out from inactivity." #105177

Closed

pavelsavara added 3 commits June 24, 2025 12:37

more

22b533f

more

afd5524

more

da9250c

Merge branch 'main' into get_native_to_interp

7a6f964

build-analysis bot mentioned this pull request Jun 26, 2025

Assertion failed 'm_dfsTree->Contains(dominator) && m_dfsTree->Contains(dominated)' #117039

Closed

pavelsavara marked this pull request as draft July 25, 2025 13:58

Merge branch 'main' into get_native_to_interp

0874e15

pavelsavara modified the milestones: 10.0.0, 11.0.0 Sep 1, 2025

pavelsavara closed this Sep 3, 2025

jkotas mentioned this pull request Sep 4, 2025

[wasm coreclr] Minor nits #119369

Merged

github-actions bot locked and limited conversation to collaborators Oct 4, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[browser] throw PNSE from GetFunctionPointerForDelegate#116107

[browser] throw PNSE from GetFunctionPointerForDelegate#116107
pavelsavara wants to merge 11 commits intodotnet:mainfrom
pavelsavara:get_native_to_interp

pavelsavara commented May 29, 2025 •

edited

Loading

Uh oh!

dotnet-policy-service bot commented May 29, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

jkotas commented May 29, 2025

Uh oh!

pavelsavara commented May 29, 2025

Uh oh!

pavelsavara commented May 29, 2025

Uh oh!

lewing commented May 29, 2025 •

edited

Loading

Uh oh!

jkotas commented May 31, 2025

Uh oh!

Uh oh!

Uh oh!

jkotas commented Jun 4, 2025

Uh oh!

Uh oh!

Uh oh!

jkotas Jun 9, 2025 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

pavelsavara commented May 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dotnet-policy-service bot commented May 29, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

jkotas commented May 29, 2025

Uh oh!

pavelsavara commented May 29, 2025

Uh oh!

pavelsavara commented May 29, 2025

Uh oh!

lewing commented May 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jkotas commented May 31, 2025

Uh oh!

Uh oh!

Uh oh!

jkotas commented Jun 4, 2025

Uh oh!

Uh oh!

Uh oh!

jkotas Jun 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

pavelsavara commented May 29, 2025 •

edited

Loading

lewing commented May 29, 2025 •

edited

Loading

jkotas Jun 9, 2025 •

edited

Loading