[wasm] Report GC locals held across a safepoint as pinned by lewing · Pull Request #131402 · dotnet/runtime

lewing · 2026-07-26T23:07:47Z

Alternative to #131374. Same bug, same test, narrower fix — opened as a draft so the two can be compared side by side rather than argued in the abstract.

Background

On wasm a GC reference loaded from its frame home onto the operand stack is a copy the GC can neither see nor update, so it goes stale if the referent is relocated at an intervening call. fgWasmSpillRefs already handles this for values with no home by spilling them to pinned slots, but it skips non-address-exposed GT_LCL_VAR:

If a value is just a GT_LCL_VAR that isn't address-exposed, by construction we ensure that it won't be mutated between its def (here) and its use (the call that would produce a spill) and we won't need to spill it.

That holds for IR mutation but not for relocation, which is where the hole comes from.

What this does

Stop skipping those locals; instead of spilling them to a second slot, report the home they already have as pinned.

lvPinned is deliberately not reused. As @AndyAyersMS noted, it has side effects — it is a source-level concept with ~15 consumers, all of which run before this phase, and setting it here breaks the lvPinned => !lvTracked invariant asserted at gcencode.cpp:4135 and gcinfo.cpp:608. That is not hypothetical: a variant doing exactly that fails 8 assertions (EC=133) during Emit GC+EH tables under a checked crossgen of CoreLib, where this change is EC=0. A wasm-only lvStackPinned bit, consulted only when encoding GC info, says just the thing that needs saying and nothing more.

Locals the call consumes directly are excluded — there is no safepoint between loading one and the call, and the home keeps the referent reachable meanwhile.

Versus #131374

@jakobbotsch's objection to #131374 was that it reports everything pinned always, trading GC behaviour for code size. This addresses that directly. Measured on a browser R2R composite against a common unfixed baseline, with pinning as the only variable:

	pinned slots	size
#131374 (pin all on-frame GC slots)	141,029 (100%)	+22,512 B
this change	21,161 (15.0%)	+14,256 B
spill to new pinned slots	n/a (bounded duration)	+2,028,932 B

Same denominator, so the comparison is exact: 85% of the pins #131374 takes are unnecessary, and targeting is slightly smaller rather than costing anything.

One caveat, stated plainly: this reduces the number of pins, not their duration. The spill approach zeroes its slots at end of block, releasing the pin; a report-pinned bit on a real local holds until the local is overwritten or the frame returns. On the fragmentation axis the ordering is spill (bounded) < this (6.7× fewer, unbounded) < #131374 (unbounded). GC-pressure measurement could not separate this from #131374 on the available workloads — within noise — so the static pin count is offered as the defensible proxy rather than a runtime claim.

Validation

Browser R2R rig, all variants sharing an identical base so pinning is the sole variable:

Byref GC-hole probe: unfixed ec=1 3/3; this change ec=100 3/3.
RuntimeCustomAttributeData NRE, 5 seeds, System.Text.Json 12166 discovered: 0 NREs on all five. So "held across a call" is not too narrow a criterion — it catches everything the encoder-wide version did.
Microsoft.Bcl.Memory 550/550 and System.Runtime.CompilerServices.Unsafe 128/128, R2R off and on.
Checked wasm crossgen of CoreLib: EC=0, no assertions.

Test

Runtime_131373 is unchanged from #131374. Four shapes look correct here but silently false-pass, which is worth recording:

A class field store. obj.Field = Call() lowers to STOREIND(ADD(LCL_VAR obj, offset), CALL) because the field is at a non-zero offset; that byref ADD has no home and was already spilled. A ref parameter store has no ADD.
A call argument. fgMorphArgs spills an earlier argument to a temp when a later one contains a call, so the load happens after the safepoint.
No allocation pressure. A compacting collect on a small unfragmented heap has nothing to relocate.
Test not crossgen'd per variant. The hole is in the test's own R2R code, so compiling it as IL against an R2R CoreLib passes even unfixed.

Only one of #131374 and this should be taken.

Note

This change was authored with GitHub Copilot.

azure-pipelines · 2026-07-26T23:08:32Z

Azure Pipelines: Successfully started running 6 pipeline(s). 10 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

Pull request overview

This PR adjusts the CoreCLR JIT’s wasm-specific GC reporting so that certain GC locals that can be held on the wasm operand stack across a safepoint are reported as pinned (via a wasm-only local flag), avoiding stale references after relocation. It also adds a JIT regression test that reproduces the issue under browser wasm R2R.

Changes:

Add a wasm-only local-descriptor bit (lvWasmReportPinned) and use it during GC slot encoding to emit GC_SLOT_PINNED when appropriate.
Extend fgWasmSpillRefs to stop exempting non-address-exposed GT_LCL_VAR and instead mark eligible locals for pinned reporting when they are live across a call.
Add the Runtime_131373 regression test project and test case, forcing crossgen on the browser leg.

Reviewed changes

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

Show a summary per file

File	Description
src/coreclr/jit/compiler.h	Adds wasm-only `lvWasmReportPinned` bit on `LclVarDsc`.
src/coreclr/jit/fgwasm.cpp	Marks certain locals as “report pinned” when held across calls; adds operand-check helper.
src/coreclr/jit/gcencode.cpp	Uses `lvWasmReportPinned` (wasm-only) to set `GC_SLOT_PINNED` when encoding stack GC slots.
src/tests/JIT/Regression/JitBlue/Runtime_131373/Runtime_131373.csproj	New regression test project; forces crossgen for `TargetOS=browser`.
src/tests/JIT/Regression/JitBlue/Runtime_131373/Runtime_131373.cs	New regression test intended to reproduce the stale-ref hole under wasm R2R.

Copilot

Pull request overview

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

Comments suppressed due to low confidence (1)

src/tests/JIT/Regression/JitBlue/Runtime_131373/Runtime_131373.cs:20

This test is specific to wasm R2R (browser) but currently runs the full allocation/compacting-GC loop on every target. That can add noticeable time to non-browser JIT regression runs while providing little additional coverage. Consider gating the test with ConditionalFact so it only executes on the browser leg where it can actually reproduce the issue.

    [Fact]
    public static void TestEntryPoint()

Alternative to dotnet#131374, which reports every on-frame GC slot pinned. This pins only the locals whose operand-stack copy actually spans a safepoint. On wasm a GC reference loaded from its frame home onto the operand stack is a copy the GC can neither see nor update, so it goes stale if the referent is relocated at an intervening call. fgWasmSpillRefs handles that for values with no home by spilling them to pinned slots, but it skips non-address-exposed GT_LCL_VAR on the grounds that such a local "won't be mutated between its def and its use" -- true of IR mutation, but not of relocation. Stop skipping those locals, and instead of spilling them to a second slot, report the home they already have as pinned. Reuse of lvPinned is deliberately avoided: it is a source-level concept with roughly fifteen consumers, all of which run before this phase, and setting it here breaks the lvPinned => !lvTracked invariant asserted in gcencode.cpp and gcinfo.cpp. A wasm-only lvStackPinned bit, read only when encoding GC info, says just the thing that needs saying. Locals the call consumes directly are excluded: there is no safepoint between loading one and the call, and the home keeps the referent reachable meanwhile. Measured against the same unfixed baseline on a browser R2R composite: pinned slots size dotnet#131374 (pin all) 141,029 (100%) +22,512 this change 21,161 (15.0%) +14,256 spill to new slots n/a (bounded) +2,028,932 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2d24becc-f8f3-40bf-b13b-5dd1df662196

dotnet-policy-service · 2026-07-26T23:29:02Z

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

jakobbotsch · 2026-07-27T08:24:46Z

+                    if (!fgWasmIsOperandOf(tree, def))
+                    {
+                        JITDUMP("Pinning V%02u, held across call\n", def->AsLclVarCommon()->GetLclNum());
+                        dsc->lvStackPinned = true;


Why can't we just set this inside the existing LCL_VAR check?

Ah ok, I see we do not push the def in that case. Can you just do that?

Remove LCL_VAR handling below

In the loop over defs below, handle LCL_VAR by setting this new bit, and then skip the spill logic

The FIXME below should be addressed and I think it should be done before the spilling. After a call the call arguments are the callees problem, not ours.

jakobbotsch · 2026-07-27T10:16:05Z

+            // A non-address-exposed GT_LCL_VAR is not exempt: the copy pushed onto the operand stack
+            //  goes stale if the referent moves, so record it and pin its home at the next call.


Suggested change

// A non-address-exposed GT_LCL_VAR is not exempt: the copy pushed onto the operand stack

// goes stale if the referent moves, so record it and pin its home at the next call.

// We have a ref that may be live across a future call.

Copilot AI review requested due to automatic review settings July 26, 2026 23:07

github-actions Bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jul 26, 2026

lewing mentioned this pull request Jul 26, 2026

[wasm] Report on-frame GC slots as pinned #131374

Open

dotnet-policy-service Bot assigned lewing Jul 26, 2026

Copilot started reviewing on behalf of lewing July 26, 2026 23:08 View session

lewing force-pushed the lewing-wasm-report-held-locals-pinned branch from 89191ca to d70dfec Compare July 26, 2026 23:13

Copilot AI reviewed Jul 26, 2026

View reviewed changes

Comment thread src/coreclr/jit/fgwasm.cpp

Comment thread src/tests/JIT/Regression/JitBlue/Runtime_131373/Runtime_131373.cs

Copilot AI review requested due to automatic review settings July 26, 2026 23:15

Copilot started reviewing on behalf of lewing July 26, 2026 23:15 View session

lewing requested review from AndyAyersMS and jakobbotsch July 26, 2026 23:21

Copilot AI reviewed Jul 26, 2026

View reviewed changes

lewing force-pushed the lewing-wasm-report-held-locals-pinned branch from d70dfec to a673984 Compare July 26, 2026 23:24

lewing added the arch-wasm WebAssembly architecture label Jul 26, 2026

build-analysis Bot mentioned this pull request Jul 27, 2026

Unable to pull image from mcr.microsoft.com #117164

Open

jakobbotsch reviewed Jul 27, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[wasm] Report GC locals held across a safepoint as pinned#131402

[wasm] Report GC locals held across a safepoint as pinned#131402
lewing wants to merge 1 commit into
dotnet:mainfrom
lewing:lewing-wasm-report-held-locals-pinned

lewing commented Jul 26, 2026 •

edited

Loading

Uh oh!

azure-pipelines Bot commented Jul 26, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

dotnet-policy-service Bot commented Jul 26, 2026

Uh oh!

jakobbotsch Jul 27, 2026

Uh oh!

jakobbotsch Jul 27, 2026 •

edited

Loading

Uh oh!

jakobbotsch Jul 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		// A non-address-exposed GT_LCL_VAR is not exempt: the copy pushed onto the operand stack
		// goes stale if the referent moves, so record it and pin its home at the next call.

	// A non-address-exposed GT_LCL_VAR is not exempt: the copy pushed onto the operand stack
	// goes stale if the referent moves, so record it and pin its home at the next call.
	// We have a ref that may be live across a future call.

Uh oh!

Conversation

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

Uh oh!

Background

What this does

Versus #131374

Validation

Test

Uh oh!

azure-pipelines Bot commented Jul 26, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

dotnet-policy-service Bot commented Jul 26, 2026

Uh oh!

jakobbotsch Jul 27, 2026

Choose a reason for hiding this comment

Uh oh!

jakobbotsch Jul 27, 2026 • 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!

jakobbotsch Jul 27, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

lewing commented Jul 26, 2026 •

edited

Loading

jakobbotsch Jul 27, 2026 •

edited

Loading