Skip to content

Classify Wasm P/Invokes before R2R codegen - #133773

Merged
lewing merged 22 commits into
mainfrom
lewing-wasm-r2r-pinvoke-fallback
Sep 18, 2026
Merged

lewing merged 22 commits into
mainfrom
lewing-wasm-r2r-pinvoke-fallback

Conversation

@lewing

@lewing lewing commented Sep 12, 2026

Copy link
Copy Markdown
Member

Summary

Preserve .NET P/Invoke call-site semantics for CoreCLR WebAssembly ReadyToRun images without relying on runtime fallback to stripped IL.

Crossgen2 now distinguishes P/Invokes that can be emitted directly from those that require runtime resolution:

  • modules supplied through --directpinvoke, QCalls, and * static imports remain direct R2R P/Invokes;
  • WasmImportLinkageAttribute remains direct and resolves through the WebAssembly import table rather than the static native-module list;
  • ordinary imports not proven statically available are treated as requiring marshalling, so callers remain valid R2R methods and resolution occurs only when the P/Invoke is reached.

Browser and WASI app builds pass their supported module lists to Crossgen2. CoreLib and prebuilt browser framework R2R generation use the same canonical framework list as the portable call-helper generator. The canonical build configuration lives under eng/wasm and is packaged beside the browser runtime SDK target for out-of-tree builds. A shared WASM build task computes the effective module set for browser and WASI, including platform applicability, invariant globalization, supported native inputs, canonical module names, and deduplication.

The module list is written to an incremental-build manifest so adding or removing a native module invalidates the corresponding R2R images.

The earlier VM-level method-rejection implementation has been removed. This avoids a new R2R-to-IL fallback path and remains compatible with --strip-il-bodies.

Addresses the Runtime_58874 failure tracked by #133259 and the broader guarded-platform-probe behavior reported in #133614.

Validation

  • Crossgen2/CoreCLR build: passed.
  • NativeAOT compiler build: passed.
  • Browser-wasm CoreCLR runtime, CoreLib, framework R2R, and runtime pack build: passed.
  • WASI CoreCLR runtime and runtime pack build: passed.
  • Browser-wasm R2R Interop/PInvoke/Unresolved/UnresolvedPInvokeTests passed with --strip-il-bodies and --directpinvoke:libSystem.Native.
  • WASI R2R Interop/PInvoke/Unresolved/UnresolvedPInvokeTests passed with the same stripped-IL configuration.
  • Packaged workload WBT for CoreCLR R2R + NativeFileReference passed, including native relinking, native-lib module classification, and the P/Invoke result.
  • The preserved pre-policy browser runtime fails the call-site timing assertion under --strip-il-bodies; the new policy passes it.
  • R2RDump confirms the image has READYTORUN_FLAG_StrippedILBodies, configured libSystem.Native calls retain PINVOKE_TARGET fixups, and unresolved callers do not embed those fixups.
  • CoreCLR browser sample publish with R2R and native relinking: passed; its generated P/Invoke manifest matches the call-helper module list.
  • Native test execution: passed.

No benchmark is applicable to permanently unavailable imports. Configured direct P/Invokes retain the existing direct PINVOKE_TARGET codegen and add no runtime branch.

Note

This pull request description and changes were prepared with GitHub Copilot assistance.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

@lewing
lewing requested a review from pavelsavara September 12, 2026 02:49
@lewing lewing added the arch-wasm WebAssembly architecture label Sep 12, 2026
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

@lewing
lewing requested a review from radekdoulik September 12, 2026 02:57
Comment thread src/coreclr/vm/dllimport.cpp Outdated
Comment thread src/tests/JIT/Regression_o_2/Runtime_58874.cs Outdated
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Comment thread src/coreclr/vm/dllimport.cpp Outdated
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@lewing
lewing marked this pull request as ready for review September 12, 2026 05:56
Copilot AI lite review requested due to automatic review settings September 12, 2026 05:56
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Critical deferred-resolution and test-project configuration issues remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review tier: Lite
Findings: 3 High severity

Open findings (3)
What changed in this PR

This PR updates WASM ReadyToRun P/Invoke resolution to defer unresolved imports and adds regression coverage for call-site failure timing.

Changes:

  • Adds WASM-specific P/Invoke fallback resolution.
  • Removes the Runtime_58874 quarantine.
  • Adds unresolved P/Invoke timing tests and isolated test configuration.
File Summary
src/​tests/​JIT/​Regression_o_2/​Runtime_58874.cs Removes the WASM/R2R quarantine.
src/​tests/​Interop/​PInvoke/​Unresolved/​UnresolvedPInvokeTests.csproj Adds the isolated test project; critical: missing TestLibraryProjectPath reference may prevent the generated runner from compiling or running.
src/​tests/​Interop/​PInvoke/​Unresolved/​UnresolvedPInvokeTests.cs Adds call-site failure timing coverage.
src/​coreclr/​vm/​jitinterface.cpp Adds unresolved WASM R2R fallback; critical: interpreter compilation can still eagerly resolve imports or hit a portability assertion.
src/​coreclr/​vm/​dllimport.h Declares the WASM resolution helper.
src/​coreclr/​vm/​dllimport.cpp Implements override-based resolution; critical: fallback may still resolve before managed entry instead of deferring failure to the call site.

Comment thread src/coreclr/vm/dllimport.cpp Outdated
Comment thread src/coreclr/vm/jitinterface.cpp Outdated
Comment thread src/tests/Interop/PInvoke/Unresolved/UnresolvedPInvokeTests.csproj
@lewing lewing changed the title Fall back from R2R for unresolved WASM P/Invokes Fall back from R2R for unresolved Wasm P/Invokes Sep 12, 2026
Comment thread src/coreclr/vm/jitinterface.cpp Outdated
Comment thread src/coreclr/vm/dllimport.cpp Outdated
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 12, 2026 15:18
Comment thread src/coreclr/vm/dllimport.cpp
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🔵 Needs a closer look

Unresolved WebAssembly P/Invokes still fail during interpreter compilation instead of at the call site.

Review tier: Lite
Findings: 3 High severity

Open findings (3)

Copilot AI review requested due to automatic review settings September 12, 2026 15:32
Comment thread src/mono/browser/build/BrowserWasmApp.CoreCLR.targets
Comment thread src/mono/browser/build/BrowserWasmApp.CoreCLR.targets Outdated
Comment thread src/mono/browser/build/BrowserWasmApp.CoreCLR.targets
Comment thread src/mono/browser/build/BrowserWasmApp.CoreCLR.targets
Copilot AI review requested due to automatic review settings September 17, 2026 14:16

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🔵 Needs a closer look

Resolve the module-list/archive synchronization issue and update regression coverage for the canonical System.Native spelling.

Review effort: Lite
Findings: None

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 17, 2026 17:17
@lewing
lewing requested a review from ilonatommy as a code owner September 17, 2026 17:17

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🔵 Needs a closer look

The broad runtime/build changes include a moderate gap in proving direct R2R emission and warrant human review.

Review effort: Lite
Findings: None

@pavelsavara

Copy link
Copy Markdown
Member

Does this need to regenerate src\coreclr\vm\wasm\browser\callhelpers-pinvoke.cpp and src\coreclr\vm\wasm\wasi\callhelpers-pinvoke.cpp ?

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 17, 2026 20:19

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🔵 Needs a closer look

Targeted R2R coverage is still missing for NativeLibrary inputs, invariant globalization, and Browser/WASI module filtering.

Review effort: Lite
Findings: None

Previously missed (1)

In code that hasn't changed since last review

Medium severity Add R2R coverage for module policy branches

src/​tasks/​WasmAppBuilder/​ComputeWasmPInvokeModules.cs:65

These branches now determine which modules receive --directpinvoke for CoreCLR ReadyToRun, but the new ReadyToRun workload test covers only a custom NativeFileReference. The NativeLibrary path, invariant-globalization filtering, and Browser/WASI applicability are not exercised by an R2R test; the existing invariant test only validates the non-R2R native relink. Add focused R2R coverage for these cases so a regression cannot classify an unavailable module as direct.

[!NOTE] This review comment was generated with GitHub Copilot assistance.

@lewing

lewing commented Sep 17, 2026

Copy link
Copy Markdown
Member Author

Does this need to regenerate src\coreclr\vm\wasm\browser\callhelpers-pinvoke.cpp and src\coreclr\vm\wasm\wasi\callhelpers-pinvoke.cpp ?

Browser had become slightly stale again. Fixed.

I'm going to open a separate pr that adds a CI check to regenerate the baseline and compare to the checked in version to avoid drift.

@lewing
lewing merged commit 1d30ed0 into main Sep 18, 2026
162 of 165 checks passed
@lewing
lewing deleted the lewing-wasm-r2r-pinvoke-fallback branch September 18, 2026 07:58
lewing added a commit to lewing/runtime that referenced this pull request Sep 18, 2026
The P/Invoke classification fix from dotnet#133773 allows guarded platform probes to remain R2R compiled. Remove the temporary TestUtilities interpreter exception and its documentation.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

arch-wasm WebAssembly architecture area-VM-coreclr

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants