[wasm] Report arguments of VM-to-managed calls while running the prestub - #134779
Merged
Merged
Conversation
CallDescrWorkerInternal ran DoPrestub (GC-capable) with no frame reporting the call's arguments, and did so on every call to an R2R target. - Only call DoPrestub when MethodDesc::ShouldCallPrestub(), matching the interpreter call path. - Pass the TransitionBlock from MethodDescCallSite and reflection invoke through CallDescrData and push a PrestubMethodFrame over it around DoPrestub so the arguments are reported during a GC. Fixes dotnet#134777 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
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. |
Contributor
|
Tagging subscribers to this area: @agocke |
Contributor
|
Tagging subscribers to 'arch-wasm': @lewing, @pavelsavara |
jkotas
reviewed
Sep 28, 2026
…escrWorkerInternal Mirror PreStubWorker's exception handling around DoPrestub: rethrow to the native caller and append the method being prepared to the exception's stack trace, so traces for exceptions thrown while preparing a VM-to-managed call target match other platforms. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
jkotas
approved these changes
Sep 30, 2026
lewing
enabled auto-merge (squash)
September 30, 2026 00:02
lewing
added a commit
that referenced
this pull request
Sep 30, 2026
## Summary Fixes a GC hole during exception dispatch in Wasm R2R code. It shows up in CI as a rare `SanityCheck()` assert in nested EH tests on the browser-wasm R2R leg, for example `Test_throwinfinallynestedintry_30`. ## Root cause Wasm R2R code has no interruptible ranges, and it reports every GC ref on the frame as an untracked (pinned) slot. A method and its funclets share those frame slots. When an exception is thrown from a funclet, that funclet's frame is `ExecutionAborted`. `GcInfoDecoder::EnumerateLiveSlots` finds no interruptible range covering the offset and returns without reporting anything, including the untracked slots. The parent frames (the caller of an out-of-line finally, and the main method body) are skipped as already reported by the funclet. So during dispatch nothing reports the method's untracked slots. A GC in that window can free an object still held in one of them. In the repro it's a boxed `Int32`. The next time the slot is reported, it points at freed memory and hits `SanityCheck()`. Other targets avoid this in the JIT. `CodeGenInterface::setFramePointerRequiredEH` forces every method with EH to be fully interruptible, because `EnumGcRefs` only reports slots in aborted frames that are fully interruptible. Wasm is explicitly excluded there, and Wasm R2R code can't be fully interruptible. ## Fix Add a `HAS_INTERRUPTIBLE_RANGES` trait to each `GcInfoEncoding`, `false` only for `Wasm32GcInfoEncoding`. - `EnumerateLiveSlots` skips the interruptible-range handling for that encoding and reports only untracked slots outside safe points, including for aborted frames. - The count of interruptible ranges is no longer serialized for that encoding. The encoder, the runtime decoder, the cDAC decoder and R2RDump all skip it, and `DefineInterruptibleRange` asserts it's never used for that encoding. - There's no R2R version bump: Wasm hasn't shipped, so Wasm-specific format changes don't need one. The gate is on the encoding rather than `TARGET_WASM`: on Wasm the interpreter's GC info goes through the same decoder, and interpreter code does define an interruptible range. Native targets are unchanged. The cDAC `GCInfoDecoder` mirrors the change through the same trait (default `true`), and `docs/design/datacontracts/GCInfo.md` documents it. `main` has no Wasm32 GC info traits in the cDAC yet; #133890 adds them. I fixed this in the decoder rather than having the JIT declare Wasm methods interruptible over their whole range. That claim isn't true for Wasm, and other checks rely on it (GC stress, the safe-point asserts). ## Size On browser-wasm `System.Private.CoreLib.wasm` (CI crossgen options), omitting the count saves 1,088 bytes (0.003%). A GC info blob only shrinks when the 2 saved bits cross a byte boundary (11,606 of 58,253 methods), and identical blobs are shared (5,331 distinct unwind entries across 60,323 methods and funclets). ## Validation I ran the CI Helix payload for `Methodical_d1` from build 1613981 locally, with crossgen2 built from this branch and a browser-wasm `corerun` built with and without the fix. For the final format I recompiled CoreLib and the test assemblies with this branch's crossgen2. | | Without fix | With fix | |---|---|---| | `Test_throwinfinallynestedintry_30`, `DOTNET_GCStress=0x1` | same `SanityCheck()` assert as CI, every run | pass | | `throwincascadedexcept_d`, `throwincascadedexceptnofin_d`, `DOTNET_GCStress=0x1` | assert, then timeout | pass | | `Methodical_d1` merged runner, no stress | 128 passed | 128 passed, same results | - Native osx-arm64 and browser-wasm checked builds pass. - R2RDump and the cDAC unit tests pass (3179/3179). - The final format gives the same no-stress results as a control that still writes and reads the count. - All 84 `Methodical_d1` assemblies, R2R-compiled with the checked Wasm JIT, never hit the new encoder assert. - In every full run, the same 18 out-of-process tests fail because my local runner doesn't handle out-of-process tests. - The final revert of the version bump only restores `readytorun.h` and the two `ModuleHeaders` files to `main`; it wasn't rebuilt. With this fix, the full `Methodical_d1` suite under `DOTNET_GCStress=0x1` was clean in 3 of 4 runs. The 4th hit a `RawGetMethodTable()` assert in `throw_SEH`. That's a separate, pre-existing hole, #134777: `CallDescrWorkerInternal` leaves the call's arguments unreported while `DoPrestub` runs. It reproduces identically with and without this change and is fixed in #134779. With both fixes, the full suite under stress passed 4 of 4 runs (126 passed), in a run before the count was removed from the format. I also checked native (osx-arm64, release 11.0 rc1 runtime, FullOpts JIT) with a small app. An object is held only in an address-exposed local, a `finally` throws, and the same method catches it with a filter that forces a GC. The object stayed alive 20/20 times, and JitDisasm shows the method as `; fully interruptible`, as `setFramePointerRequiredEH` requires. CI doesn't run GC stress on the Wasm R2R leg, which is why this showed up as a ~0.5% flake rather than a hard failure. Resolves #134768 > [!NOTE] > This PR description was drafted with GitHub Copilot. --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
On Wasm,
CallDescrWorkerInternalranDoPrestubwith no frame reporting the call's arguments, andDoPrestubcan trigger a GC. A GC in that window can free a reference argument before the callee receives it. It also ranDoPrestubon every call to an R2R-compiled target, not just the first, because R2R methods never have interpreter code.One visible symptom:
CustomAttribute_CreateCustomAttributeInstanceparses a string constructor argument from the attribute blob, ends its GC protection, and calls the constructor throughMethodDescCallSite. A GC during the prestub collects the string, and the attribute stores a dangling reference.Fix
DoPrestubonly whenMethodDesc::ShouldCallPrestub(), matching the interpreter call path.MethodDescCallSiteand reflection invoke already place the arguments right after aTransitionBlock. Pass it throughCallDescrDataand push aPrestubMethodFrameover it aroundDoPrestub, the same mechanism the native prestub and the Wasm R2R-to-interpreter thunk path use.DispatchCallSimplehas noTransitionBlock, so its arguments are still not reported duringDoPrestub(now only on the first call). Left as a follow-up.Validation
Browser-wasm checked runtime with the
Methodical_d1Helix payload and R2R-compiled test code,DOTNET_TieredCompilation=0:Assembly.GetCustomAttributes(typeof(NeutralResourcesLanguageAttribute), false)in a loop,DOTNET_GCStress=0x1SanityCheck()assert, 4/4 runsMethodical_d1merged runner,DOTNET_GCStress=0x1, 4 runs in parallel (with #134769 applied)RawGetMethodTable()assert inthrow_SEHMethodical_d1merged runner, no stressIn every full run, 18 out-of-process tests fail the same way because the local runner doesn't handle out-of-process tests. There is no new regression test: the hole only shows under GC stress, and no Wasm R2R CI leg runs GC stress today.
Resolves #134777
Note
This PR description was drafted with GitHub Copilot.