Skip to content

[mono][wasm] Pin OP_MOVE destinations in the GC pin area - #132306

Draft
pavelsavara wants to merge 2 commits into
dotnet:mainfrom
pavelsavara:MoveAliasGcRoot
Draft

[mono][wasm] Pin OP_MOVE destinations in the GC pin area#132306
pavelsavara wants to merge 2 commits into
dotnet:mainfrom
pavelsavara:MoveAliasGcRoot

Conversation

@pavelsavara

@pavelsavara pavelsavara commented Aug 14, 2026

Copy link
Copy Markdown
Member

Testing ....

Fixes the AOT codegen half of #130592.
Related #132180

Objects that are still referenced by a live local can be collected in Mono LLVM-AOT
browser-wasm code. The fix stops excluding OP_MOVE destinations from the GC pin area,
but only for the moves that actually need it, so the size cost is negligible
(+0.15% of AOT code, +186 bytes compressed).

-if (vreg_is_ref (cfg, ins->dreg) && ctx->values [ins->dreg] && ins->opcode != OP_MOVE && ins->opcode != OP_AOTCONST)
+if (vreg_is_ref (cfg, ins->dreg) && ctx->values [ins->dreg] && ins->opcode != OP_AOTCONST &&
+        (ins->opcode != OP_MOVE || move_needs_gc_pin (ctx, ins)))
         emit_gc_pin (ctx, builder, ins->dreg);

Background: how GC references stay alive in wasm AOT code

This part is unusual enough that the bug does not make sense without it.

On normal targets, SGen finds GC references by scanning the machine stack
conservatively: anything on the stack that looks like a heap pointer is treated as a
root, and the object it points to is pinned so the collector will not move it.

WebAssembly has no addressable machine stack. Local variables live in wasm locals
— slots in the VM's own execution state. They are not part of linear memory, so a
conservative scan of linear memory structurally cannot see them. A reference that lives
only in a wasm local is invisible to the GC.

Mono works around this with a GC pin area. For each method, the AOT compiler
allocates a small array in the method's linear-memory frame (emit_entry_bb), reserving
one slot per reference-typed virtual register. Every time a ref vreg is assigned, the
compiler emits a volatile store of that value into its slot (emit_gc_pin). Because the
pin area is in linear memory, the conservative scan finds it, and the objects are
pinned.

So on wasm the rule is: a reference is only a GC root if something wrote it into the pin
area (or into some other linear-memory location).
If the compiler skips that store, the
value is invisible to the GC even though the program is still using it.

Two details matter later:

  • emit_gc_pin deliberately skips address-taken (volatile/indirect) variables and
    returns early. Those variables already live in their own linear-memory stack slot
    (emit_volatile_store writes them there), so a pin slot would be redundant. They are
    not given a pin slot at all.
  • An OP_MOVE in Mono's IR emits no LLVM instruction. values[dreg] = values[sreg1]
    simply makes the destination an SSA alias of the source. There is no new value, so the
    original code assumed there was no new root to record.

The bug

OP_MOVE destinations were excluded from emit_gc_pin, on the assumption that the
source is already rooted, so the alias is covered.

That assumption breaks when the source is an address-taken variable:

  1. The source has no pin slot (emit_gc_pin skips it, as above). Its only root is its
    linear-memory stack slot.
  2. alias = source is an OP_MOVE, so under the old condition alias got no pin store
    either
    . alias exists only as a wasm local.
  3. Reassigning the source (source = something_else) overwrites that stack slot — the
    only root for the previous object.

The previous object is now referenced solely by alias, which lives in a wasm local that
the conservative scan cannot see. It is not pinned and not reachable, so the next
collection frees it (or moves it without updating the alias). The program then keeps using
a dangling reference.

That produces exactly the symptoms in #130592: reads returning garbage, mono_class_get_flags: unexpected GC filler class, out-of-bounds accesses, and hangs inside a collection.

Reproduction

src/tests/JIT/Directed/gcpin/MoveAliasGcRoot.cs:

Box cur = Make(0);
Touch(ref cur);                 // taking the address makes `cur` an address-taken variable

for (int i = 1; i <= 128; i++) {
    Box alias = cur;            // ref OP_MOVE; sole remaining reference to Box(i - 1)
    cur = Make(i);              // overwrites cur's stack slot -- the only root
    Touch(ref cur);
    Collect();                  // GC.Collect() + a small allocation to reuse freed space

    Assert.True(alias.IsIntact(i - 1));   // fails: object was collected
    GC.KeepAlive(alias);
}

The asserted invariant — an object reachable through a live local survives a collection
holds on every runtime, so this test is safe to run everywhere. It only has teeth on
wasm AOT.

Measured on browser-wasm, Release, RunAOTCompilation=true, via the console-node
sample:

build result
baseline FAIL 3/3lost at iteration 28 (read Value=3652889), byte-identical each run
this PR (conditional) PASS 2/2
unconditional variant PASS 3/3

Notes:

  • Deterministic. wasm is single-threaded, so the failure lands on the same iteration
    with the same garbage value every run.
  • No GC debug flags required. It fails on a plain AOT run; MONO_GC_DEBUG=clear-at-gc
    only makes it fail sooner (iteration 5 instead of 28).
  • Sensitive to allocation shape: increasing the churn inside Collect() from one small
    array to 32 made the failure disappear, presumably by changing promotion behaviour. The
    committed form is the one verified to fail.

How common is the shape?

A temporary census in the AOT compiler over the five assemblies AOT'd by the sample
(including System.Private.CoreLib):

count
unique methods containing ref OP_MOVEs 2,776
ref OP_MOVEs total 12,047
...with an address-taken/volatile source (the hazard) 1,457, across 252 methods
...with a multi-definition source (the originally claimed shape) 1,250, across 410 methods

So the vulnerable pattern is not exotic; it appears in a few hundred CoreLib methods
alone. Whether a given site actually strands an object additionally depends on the object
being the sole reference across a collection, which is why it surfaces as rare, random
corruption rather than a consistent failure.

These two rows are also what the conditional rule targets: roughly 2,300 of the 12,047 ref
moves need a pin slot, and the other ~80% are provably covered by their source and are
skipped. That ratio is what produces the size numbers below.

History of this code

Useful context, because the exclusion being removed was never a correctness decision.

PR change
#59352 (Sep 2021) Introduced the design. Previously ref variables were marked volatile and reloaded on every access; this replaced that with the pin area, storing each ref vreg after assignment. Done for code size and speed.
#69955 (Jun 2022) "Wasm AOT micro optimizations." Added three size optimizations on top, including != OP_MOVE, described as "Avoid storing arguments and results of moves into the gc_pin area." No correctness argument was given.
That batch was reverted.
#70465 (Jun 2022) "Enable a subset of the gc_pin area optimizations." Re-landed the OP_MOVE skip and the volatile/dead skip, but not the argument skip, with the note: "The OP_ARG one causes random crashes."
#81179 (Jan 2023) Added != OP_AOTCONST, which is genuinely safe — those are GOT loads of ldstr literals and type/method handles, already rooted by the loader's interned tables.

In other words: the OP_MOVE exclusion came from a size micro-optimization batch that was
already partially reverted for causing random crashes
, and the sibling exclusion from the
same batch was dropped for exactly the class of symptom seen here. This change removes the
remaining unsound member of that batch.

The fix

Pin OP_MOVE destinations, but only when the source can stop being rooted while the alias
is still live. OP_AOTCONST keeps its blanket exclusion.

static gboolean
move_needs_gc_pin (EmitContext *ctx, MonoInst *ins)
{
        MonoCompile *cfg = ctx->cfg;
        MonoInst *var;

        if (!ctx->vreg_defcount || ins->sreg1 < 0 || (guint32)ins->sreg1 >= cfg->next_vreg)
                return TRUE;
        if (ctx->vreg_defcount [ins->sreg1] > 1)
                return TRUE;
        var = get_vreg_to_inst (cfg, ins->sreg1);
        return var && (var->flags & (MONO_INST_VOLATILE | MONO_INST_INDIRECT | MONO_INST_IS_DEAD));
}

A move is skipped only when the source is provably rooted for the rest of the method:

  • Single definition (vreg_defcount <= 1). Such a vreg's pin slot is written once and
    never overwritten, so the object stays pinned. emit_entry_bb counts definitions in one
    extra walk over the IR, saturating at 2; it runs after the OP_LDADDR pass, so
    MONO_INST_INDIRECT is already set by then. Arguments have zero IR definitions and are
    pinned in the prologue, so they fall into this bucket too.
  • Not address-taken. MONO_INST_VOLATILE/INDIRECT/IS_DEAD variables have no pin
    slot at all and live in a stack slot that this method or a callee can overwrite through
    the escaped pointer — which is exactly the bug — so they are always pinned.

Everything else is pinned, so the rule is conservative by default: anything unknown
(no def-count array, out-of-range vreg) returns TRUE.

The rule is also transitively sound for chains of moves. c = MOVE b; b = MOVE a bottoms
out at a real definition: if a is single-def and not address-taken, its slot roots the
object for the whole method; if it is not, b gets pinned and — being single-def itself —
keeps its slot for the rest of the method, covering c.

Writing a MOVE destination's slot is safe by construction: emit_entry_bb already reserves
a pin slot for every non-volatile, non-dead ref vreg, using the same predicate
emit_gc_pin filters on. A MOVE destination therefore writes a slot that was already
allocated and left empty. It cannot collide with the source's slot, cannot grow the frame,
and cannot shift any other vreg's index.

Why not just pin every ref move?

That was the first version of this change and it also fixes the bug, but it costs about
9.5x more code size for no additional correctness (see below). Both variants were built
and measured; the numbers for the unconditional variant are kept in the table for
comparison.

Binary size impact

Measured on the console-node sample, browser-wasm, Release, RunAOTCompilation=true,
AOT'ing 5 assemblies including System.Private.CoreLib. All three configurations were built
from an identical tree with a full mono+libs rebuild and the identical app source; only the
pin condition differs. The baseline was reproduced twice, byte-identical.

Final shipped artifact — dotnet.native.wasm:

bytes vs baseline
baseline (today's code) 7,727,862
this PR (conditional) 7,745,349 +17,487 (+0.23%)
unconditional variant 7,872,152 +144,290 (+1.87%)
baseline, brotli 2,481,410
this PR, brotli 2,481,596 +186 (+0.007%)
unconditional variant, brotli 2,510,062 +28,652 (+1.15%)

Brotli is what users actually download, and there the cost of this PR is 186 bytes
indistinguishable from noise.

AOT-compiled managed code only (per-assembly object files), which isolates the codegen
change from the fixed runtime portion:

object baseline this PR unconditional
aot-instances.dll.o 6,731,828 +3,394 +78,676
System.Private.CoreLib.dll.o 4,416,021 +13,429 +82,387
System.Runtime.InteropServices.JavaScript.dll.o 210,834 +371 +3,622
System.Console.dll.o 38,981 +39 +419
Wasm.Console.Node.Sample.dll.o 17,745 +114 +267
total 11,415,409 +17,347 (+0.15%) +165,371 (+1.45%)

So the conditional rule costs +0.15% of AOT code size instead of +1.45%, a 9.5x
reduction
, while still fixing the bug (verified: the repro passes 2/2 with this version).
For reference, the original PR reported ~3.4 MB on a 65.5 MB application (~5%).

@pavelsavara pavelsavara added this to the 11.0.0 milestone Aug 14, 2026
@pavelsavara pavelsavara self-assigned this Aug 14, 2026
@pavelsavara pavelsavara added the arch-wasm WebAssembly architecture label Aug 14, 2026
Copilot AI lite review requested due to automatic review settings August 14, 2026 08:38
@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.

Pull request overview

Adds a new regression test intended to guard against the wasm/Mono LLVM-AOT OP_MOVE alias rooting issue described in #130592, by asserting that an object reachable from a live local survives GC across repeated address-taken-local reassignment + collection cycles.

Changes:

  • Add a new Directed JIT test project under src/tests/JIT/Directed/gcpin/.
  • Add a new xUnit test (MoveAliasGcRoot) that stress-loops through alias = cur; cur = Make(i); GC.Collect() and validates the aliased object remains intact.

Reviewed changes

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

File Description
src/tests/JIT/Directed/gcpin/MoveAliasGcRoot.csproj New test project definition for the regression test.
src/tests/JIT/Directed/gcpin/MoveAliasGcRoot.cs New xUnit regression test exercising MOVE-alias rooting across GC.

Comment on lines +1 to +5
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<Optimize>True</Optimize>
</PropertyGroup>
<ItemGroup>
Comment on lines +6 to +10
// On the Mono LLVM-AOT wasm backend, GC references are kept scannable by storing each
// ref vreg into a linear-memory "gc pin" area, because wasm value-stack locals are
// invisible to SGen's conservative stack scan. OP_MOVE dests used to be skipped there,
// on the assumption the source's pin slot covers them.
//
@pavelsavara

Copy link
Copy Markdown
Member Author

/azp run runtime-wasm

@azure-pipelines

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

Copilot AI review requested due to automatic review settings August 14, 2026 10:12
@pavelsavara pavelsavara changed the title [mono][wasm] emit_gc_pin: don't skip OP_MOVE dest [mono][wasm] Pin OP_MOVE destinations in the GC pin area Aug 14, 2026

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.

Pull request overview

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

Suppressed comments (1)

src/tests/JIT/Directed/Directed_ro.csproj:23

  • PR title/description indicate a runtime fix to wasm AOT codegen (mini-llvm.c emit_gc_pin logic for OP_MOVE), but this PR currently only adds the new gcpin test project reference here. In the current tree, src/mono/mono/mini/mini-llvm.c still has the old condition ... && ins->opcode != OP_MOVE && ins->opcode != OP_AOTCONST (around mini-llvm.c:12755), so the described fix does not appear to be included. Either add the runtime change to the PR or update the PR title/description to match what’s actually being submitted.
    <MergedWrapperProjectReference Include="FaultHandlers\Nesting\Nesting.ilproj" />
    <MergedWrapperProjectReference Include="FaultHandlers\Simple\Simple.ilproj" />
    <MergedWrapperProjectReference Include="gcpin\MoveAliasGcRoot.csproj" />
    <MergedWrapperProjectReference Include="IL\leave\leave1.ilproj" />

@pavelsavara

Copy link
Copy Markdown
Member Author

/azp run runtime-wasm

@azure-pipelines

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

Copilot AI review requested due to automatic review settings August 14, 2026 12:40
@pavelsavara

Copy link
Copy Markdown
Member Author

/azp run runtime-wasm

@azure-pipelines

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

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.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

src/libraries/System.Runtime/tests/System.Runtime.Tests/System/GCTests.cs:1265

  • The failure message interpolation allocates a new string on every loop iteration (even when the test passes). Consider constructing the message only on failure to avoid unnecessary allocations/extra work in a GC-focused test.
                Assert.True(alias.IsIntact(i - 1),
                    $"object referenced by a live local was lost across GC at iteration {i} (read Value={alias.Value})");
                GC.KeepAlive(alias);

Comment on lines +1244 to +1249
// Regression test for https://github.com/dotnet/runtime/issues/130592. On the Mono
// wasm LLVM-AOT backend a ref OP_MOVE alias of an address-taken local could be left
// rooted nowhere once the local was reassigned, letting the aliased object be collected
// while a live local still referenced it. The invariant -- an object reachable through a
// live local survives a collection -- holds on every runtime, so this only has teeth on
// wasm AOT (it must be in the browser Mono smoke set to run there).
@pavelsavara

Copy link
Copy Markdown
Member Author

/azp run runtime-wasm

@azure-pipelines

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

@pavelsavara

Copy link
Copy Markdown
Member Author

/azp run runtime-wasm

@azure-pipelines

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

Copilot AI review requested due to automatic review settings August 14, 2026 17:42

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.

Pull request overview

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

Suppressed comments (1)

src/libraries/System.Runtime/tests/System.Runtime.Tests/System/GCTests.cs:1256

  • The PR description says the wasm LLVM-AOT codegen change was made (pin OP_MOVE destinations conditionally via a helper like move_needs_gc_pin), but the current tree still unconditionally skips OP_MOVE at the only TARGET_WASM emit_gc_pin call site in src/mono/mono/mini/mini-llvm.c (it still has ins->opcode != OP_MOVE && ins->opcode != OP_AOTCONST). If this PR is meant to fix the codegen half of #130592, the runtime-side change appears to be missing from this branch.
        // Regression test for https://github.com/dotnet/runtime/issues/130592. On the Mono
        // wasm LLVM-AOT backend a ref OP_MOVE alias of an address-taken local could be left
        // rooted nowhere once the local was reassigned, letting the aliased object be collected
        // while a live local still referenced it. The invariant -- an object reachable through a
        // live local survives a collection -- holds on every runtime, so this only has teeth on
        // wasm AOT (it must be in the browser Mono smoke set to run there).
        [Fact]
        public static void MovedAliasOfAddressTakenLocalIsRootedAcrossGC()

@pavelsavara

Copy link
Copy Markdown
Member Author

/azp run runtime-wasm

@azure-pipelines

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

arch-wasm WebAssembly architecture area-Codegen-AOT-mono

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants