[wasm][coreclr][R2R] Publish existing method bodies before interpreter fallback by lewing · Pull Request #134606 · dotnet/runtime

lewing · 2026-09-24T17:55:58Z

Summary

On browser-Wasm, ExternalMethodFixupWorker now prepares the target method with the standard ShouldCallPrestub()/DoPrestub() pattern before patching a non-virtual method import cell. This publishes an existing precompiled R2R body instead of leaving the import bound to an R2R-to-interpreter thunk.

This allows closed generic MethodSpec imports to call their already-emitted exact specialized bodies while preserving the interpreter path when no body exists.

Root cause

The original suspected four-argument-versus-five-argument adapter mismatch was not the actual hot call. MemoryExtensions.Contains<int> is inlined into the application caller; the remaining cross-module import targets SpanHelpers.NonPackedContainsValueType<int>, and both its import and exact R2R body use the same five-argument Wasm signature (the trailing argument is the portable entrypoint).

The failure was publication ordering: ExternalMethodFixupWorker read the portable entrypoint before the method had been prepared, so it stored a correctly typed interpreter thunk in the import cell even when the exact compiled body was present in the R2R image.

Implementation

Under FEATURE_PORTABLE_ENTRYPOINTS, in the non-virtual branch of ExternalMethodFixupWorker:

Call pMD->DoPrestub(NULL) when pMD->ShouldCallPrestub() is true. Portable entrypoints are stable, so this is safe before patching; DoPrestub publishes an R2R body when one exists (including unboxing/instantiating stub handling) and otherwise prepares the interpreter path.
Call EnsurePortableEntryPointIsCallableFromR2R on pMD->GetMethodEntryPoint() before storing it in the import cell, so R2R code can never observe an entrypoint it cannot call. FuncPtrStub selection is skipped because portable entrypoints do not need it.

In the virtual branch, EnsurePortableEntryPointIsCallableFromR2R now runs before the import cell is published, and the existing VirtualDispatchPortableEntryPoint publication is factored into PatchExternalMethodWithVirtualDispatchPortableEntryPoint. This replaces the shared post-branch block, so each path makes the entrypoint callable exactly once, before publishing.

The non-portable-entrypoint path is unchanged. No ReadyToRun format, ABI, signature encoding, or version change is required.

Validation

./build.sh -os browser -c Release -subset clr+libs (0 warnings, 0 errors)
Browser CoreCLR R2R AssemblyLoadEvent_ReentrantMethodCompilation: 1 passed, 0 failed
Browser CoreCLR R2R full System.Runtime.Loader.Tests: only the 21 ApplyUpdateTest cases fail, with NotImplementedException (FEATURE_METADATA_UPDATER is not enabled on Wasm)

A controlled MemoryExtensions.Contains<int> application was published with deterministic method-list MIBC profiles:

With the actual NonPackedContainsValueType<int> body rooted, Chrome CDP breakpoints at the compiled body's entry, vector setup, and v128 loop each fired (50/50), always called directly from the iiiip delay-load import with no WasmR2RToInterpreterThunk frame.
For 512 calls, lengths 8 and 512 both measured approximately 0.0-0.1 ms, eliminating the interpreted scan-length slope.
With no profile (body not emitted), the same calls measured approximately 2-2.5 ms at length 8 and 110-140 ms at length 512, confirming the interpreter fallback still works.

Testing limitation

A result-only managed regression would pass before and after this change because the interpreter fallback is functionally correct. The current Wasm R2R test infrastructure does not expose the selected portable-entrypoint target to managed tests, so the dispatch distinction was validated using positive and negative live CDP controls. An automated check based on runtime method-jitting events (asserting that the target method is never compiled for the interpreter) is planned as a follow-up.

This does not address #134200, where the required R2R body is genuinely absent.

Resolves #134565

Note

This pull request description was generated with GitHub Copilot assistance.

Resolve an existing precompiled body during external method fixup before leaving a portable entrypoint on its R2R-to-interpreter thunk. Reuse the same publication helper for unmanaged callers with the appropriate caller GC mode and notification policy. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b0f02cd6-bda1-42a0-b039-ec7b21696013

Keep the existing UnmanagedCallersOnly publication helper intact and add a focused helper for managed external method fixups. This avoids unnecessary overlap with the native-callback dispatch work while preserving cooperative-mode R2R publication. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b0f02cd6-bda1-42a0-b039-ec7b21696013

…asm-r2r-generic-adapter

azure-pipelines · 2026-09-24T17:56:47Z

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 · 2026-09-24T17:57:18Z

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

dotnet-policy-service · 2026-09-24T18:34:23Z

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

dotnet-policy-service · 2026-09-24T18:35:28Z

Tagging subscribers to this area: @dotnet/crossgen-contrib
See info in area-owners.md if you want to be subscribed.

Copilot

Copilot review overview

🟡 Changes recommended

Unboxing-stub handling must be corrected before approval; an automated regression is also recommended.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 · 1

Open (2)

Initialize unboxing stub metadata before publishing R2R bodies · New
Add regression coverage for portable entrypoint selection · New

What changed in this PR

Updates browser-Wasm CoreCLR ReadyToRun fixups to publish existing compiled bodies before interpreter fallback.

Changes:

Adds a cooperative R2R publication probe.
Integrates it into non-virtual external method fixups.
Declares the helper on MethodDesc.

File	Description
`src/coreclr/vm/prestub.cpp`	Implements and integrates R2R publication probing.
`src/coreclr/vm/method.hpp`	Declares the new publication helper.

Unboxing and instantiating stubs need portable-entrypoint state that is initialized only by their dedicated DoPrestub paths. Limit the external method fixup R2R publication probe to ordinary IL methods, matching the methods DoPrestub resolves through PrepareInitialCode. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b0f02cd6-bda1-42a0-b039-ec7b21696013

Move the VirtualDispatchPortableEntryPoint publication into PatchExternalMethodWithVirtualDispatchPortableEntryPoint and call it from the virtual fixup branch, after making the resolved target callable from R2R. This removes the trailing EnsurePortableEntryPointIsCallableFromR2R call, which was redundant for non-virtual fixups. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b0f02cd6-bda1-42a0-b039-ec7b21696013

Copilot

Copilot review overview

🔵 Needs a closer look

Exclude [UnmanagedCallersOnly] methods from the new preparation path to avoid managed-ABI import traps.

Review effort: Lite
Findings: None

…thod Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b0f02cd6-bda1-42a0-b039-ec7b21696013

Copilot

Copilot review overview

🔵 Needs a closer look

Exclude [UnmanagedCallersOnly] methods from the DoPrestub probe to avoid managed callers receiving an incompatible Wasm ABI.

Review effort: Lite
Findings: None

lewing · 2026-09-25T22:04:40Z

The CI failure is #134681 which is exposed by this fix

lewing · 2026-09-25T22:05:10Z

The readytorun/wasm/WasmInterpreterTransitions failure on the browser legs is a latent bug that this PR exposes; the PR doesn't cause it. Details are in #134681.

With this PR, TestEntryPoint runs as R2R instead of interpreted. Its delegate* unmanaged call into the R2R [UnmanagedCallersOnly] method StreamLengthProxy is then an inlined P/Invoke. A nested inlined P/Invoke under GetUserData<T> → CastHelpers.ChkCastAny_NoCacheLookup lowers __stack_pointer, and the UCO epilog never restores it. The outer JIT_PInvokeEnd then hits the Debug assert sp == stack_pointer_global_value (helpers.cpp:420).

The fix is a separate JIT change in #134686, which restores __stack_pointer in reverse P/Invoke epilogs. Locally, with that JIT, this test passes against the CI Checked payload from this PR, along with the other readytorun tests in the same work item. The two PRs are independent and can merge in either order, but this test stays red here until #134686 lands.

Note

This comment was generated with GitHub Copilot.

…134686) ## Problem R2R Wasm code keeps the shadow SP in a local and leaves the `__stack_pointer` global stale. `JIT_PInvokeBegin` and the SuppressGCTransition publish (#130924) set `__stack_pointer` to the caller's shadow SP and leave it there. So when a reverse P/Invoke (`UnmanagedCallersOnly`) method's body, or any R2R code it calls, did an inlined P/Invoke, the method returned to its native caller with the global still lowered. That breaks the native ABI, which requires `__stack_pointer` to be restored on return. In the observed case the P/Invoke was CoreLib's `CastHelpers.ChkCastAny_NoCacheLookup` under `GetUserData<T>`. When the UCO was reached through an R2R inlined P/Invoke (a `delegate* unmanaged` calli), the Debug check in `JIT_PInvokeEndImpl` fired: `sp == stack_pointer_global_value` at `src/coreclr/vm/wasm/helpers.cpp:420`. After that came GetFrame asserts and an out-of-bounds access. See #134681 for the full analysis. The bug is reachable on main whenever an R2R method calls a UCO that itself does an inlined P/Invoke (see the test below). #134606 exposes it more broadly by making `TestEntryPoint` in `readytorun/wasm/WasmInterpreterTransitions` run as R2R instead of interpreted, which also routes the existing `StreamLengthProxy` case through R2R. ## Fix In `CodeGen::genFnEpilog` (`codegenwasm.cpp`), reverse P/Invoke methods now emit this sequence before `return`/`end`: ``` local.get <FP local, or SP local if there is no frame pointer> i32.const genTotalFrameSize() i32.add global.set __stack_pointer ``` This restores the global to its entry value, the post-prolog SP plus the frame size. The sequence leaves the Wasm operand stack unchanged, so an already-pushed return value is preserved. It replaces the `TODO-WASM: shadow stack maintenance` comment. The emit pattern matches the SuppressGCTransition publish from #130924. ## Test `WasmInterpreterTransitions` gains `R2RCallsNestingUco`: an R2R method does a `delegate* unmanaged` calli into `UcoWithInlinedPInvoke`, which does a calli into a second UCO, `UcoLeaf`. Each calli is an inlined P/Invoke. The inner one lowers `__stack_pointer`, and the outer one's `JIT_PInvokeEnd` asserts that the global is back at the caller's SP. Run against the Checked browser Core_Root from main CI build 1612746 (without #134606), with crossgen2 and the universal wasm JIT built from main: | test | main JIT | fixed JIT | |---|---|---| | main `WasmInterpreterTransitions` | pass | pass | | with `R2RCallsNestingUco` | **assert** `sp == stack_pointer_global_value` (helpers.cpp:420), then GetFrame assert | pass (exit 100) | The test IL for this run was compiled locally with csc rather than through the repo's test build. ## Validation - Rebuilt `clrjit_universal_wasm_arm64`, swapped it into crossgen2, and recompiled `WasmInterpreterTransitions` against the CI Checked browser runtime from build 1612477, which includes #134606's runtime changes. - Old JIT: reproduces the CI failure (the helpers.cpp:420 assert, then GetFrame asserts and OOB). - New JIT: passes with exit 100 and no asserts. - Before the new test was added, a disassembly diff of `composite-r2r.wasm` showed exactly two changes: the two UCO epilogs (`StreamLengthProxy` and `UnmanagedCallerCallsInterpreted`), each with the 4-instruction restore added. Everything else was identical. - The other 6 tests in the Helix `readytorun` work item pass: readytorun, fieldlayout, crossgen2smoke_donotalwaysusecrossgen2, Breadth1Test, Depth1Test, DynamicMethodGCStress. - `./build.sh clr.alljits -c checked` on osx-arm64, which includes the universal wasm JIT, builds cleanly, and jit-format reports no changes. ## Related - Unblocks the `WasmInterpreterTransitions` failure in #134606. This PR is independent of #134606 and has its own regression test on main, and #134606 doesn't need to be stacked on it. - Helps the open #134355, where native callers dispatch directly to R2R UCO bodies. There the UCO epilog is the only place that can restore `__stack_pointer` for the native caller. Resolves #134681 > [!NOTE] > This PR description was generated with the help of GitHub Copilot. --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Co-authored-by: Andy Ayers <andya@microsoft.com> Copilot-Session: eea66043-479c-4397-ae20-f4c776bc3782

lewing · 2026-09-26T01:52:02Z

/azp run runtime

azure-pipelines · 2026-09-26T01:52:31Z

Azure Pipelines: Successfully started running 1 pipeline(s).

lewing · 2026-09-26T03:29:19Z

/ba-g failing test is fixed by #134686 and verified locally

lewing added 5 commits September 24, 2026 01:28

Merge commit '6c08d3879bd26bad726940cda89248ee4d9b6aa4' into lewing-w…

f6c79c3

…asm-r2r-generic-adapter

Merge commit '99f3a6778c67851f3db251bb544718ec9dd09b3e' into lewing-w…

cb73a28

…asm-r2r-generic-adapter

Merge commit '273641005792258dd0fb3ef98649c1c16c91fb33' into lewing-w…

a03cb9f

…asm-r2r-generic-adapter

github-actions Bot added the area-CodeGen-Interpreter-coreclr label Sep 24, 2026

dotnet-policy-service Bot assigned lewing Sep 24, 2026

lewing added the arch-wasm WebAssembly architecture label Sep 24, 2026

lewing added area-ReadyToRun tenet-performance Performance related issue and removed area-CodeGen-Interpreter-coreclr labels Sep 24, 2026

lewing assigned davidwrighton and unassigned davidwrighton Sep 24, 2026

lewing requested review from AndyAyersMS, davidwrighton, pavelsavara and radekdoulik September 24, 2026 19:52

This was referenced Sep 24, 2026

[wasm][coreclr][R2R] Delegate construction skips alternate-constructor lowering and remains interpreted #134564

Open

[wasm][coreclr][R2R] Closed generic MethodSpec imports cannot call specialized bodies without an instantiating adapter #134565

Closed

lewing marked this pull request as ready for review September 24, 2026 20:21

Copilot AI lite review requested due to automatic review settings September 24, 2026 20:21

Copilot started reviewing on behalf of lewing September 24, 2026 20:22 View session

Copilot AI reviewed Sep 24, 2026

View reviewed changes

Comment thread src/coreclr/vm/prestub.cpp Outdated

Comment thread src/coreclr/vm/prestub.cpp Outdated

lewing marked this pull request as draft September 24, 2026 20:33

lewing marked this pull request as ready for review September 24, 2026 20:33

jkotas reviewed Sep 25, 2026

View reviewed changes

Comment thread src/coreclr/vm/prestub.cpp Outdated

jkotas reviewed Sep 25, 2026

View reviewed changes

Comment thread src/coreclr/vm/prestub.cpp Outdated

Copilot AI review requested due to automatic review settings September 25, 2026 15:52

Copilot started reviewing on behalf of lewing September 25, 2026 15:53 View session

Copilot AI reviewed Sep 25, 2026

View reviewed changes

jkotas reviewed Sep 25, 2026

View reviewed changes

Comment thread src/coreclr/vm/prestub.cpp Outdated

jkotas reviewed Sep 25, 2026

View reviewed changes

Comment thread src/coreclr/vm/prestub.cpp Outdated

jkotas approved these changes Sep 25, 2026

View reviewed changes

Remove unused pImportSection parameter from PatchNonVirtualExternalMe…

52640bb

…thod Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b0f02cd6-bda1-42a0-b039-ec7b21696013

Copilot AI review requested due to automatic review settings September 25, 2026 16:45

Copilot started reviewing on behalf of lewing September 25, 2026 16:45 View session

jkotas approved these changes Sep 25, 2026

View reviewed changes

lewing enabled auto-merge (squash) September 25, 2026 16:50

Copilot AI reviewed Sep 25, 2026

View reviewed changes

davidwrighton approved these changes Sep 25, 2026

View reviewed changes

lewing merged commit bd36a2d into main Sep 26, 2026
110 of 112 checks passed

lewing deleted the lewing-wasm-r2r-generic-adapter branch September 26, 2026 03:29

dotnet-maestro Bot mentioned this pull request Sep 27, 2026

[main] Source code updates from dotnet/runtime dotnet/dotnet#9793

Merged

dotnet-milestone-bot Bot added this to the 12.0-preview1 milestone Sep 27, 2026

Conversation

lewing commented Sep 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Root cause

Implementation

Validation

Testing limitation

Uh oh!

azure-pipelines Bot commented Sep 24, 2026

Uh oh!

dotnet-policy-service Bot commented Sep 24, 2026

Uh oh!

dotnet-policy-service Bot commented Sep 24, 2026

Uh oh!

dotnet-policy-service Bot commented Sep 24, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

lewing commented Sep 25, 2026

Uh oh!

lewing commented Sep 25, 2026

Uh oh!

lewing commented Sep 26, 2026

Uh oh!

azure-pipelines Bot commented Sep 26, 2026

Uh oh!

lewing commented Sep 26, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

lewing commented Sep 24, 2026 •

edited

Loading