From da9673ffd84c36dc33cda77bf7c931feed5358ef Mon Sep 17 00:00:00 2001 From: pavelsavara Date: Fri, 14 Aug 2026 14:40:04 +0200 Subject: [PATCH 1/4] test --- .../System.Runtime.Tests/System/GCTests.cs | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/GCTests.cs b/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/GCTests.cs index 20b4fa37aa71f7..5aba4e2630a591 100644 --- a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/GCTests.cs +++ b/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/GCTests.cs @@ -1200,5 +1200,70 @@ private unsafe static void AllocateArrayCheckPinning() } } } + + private sealed class GcRootBox + { + public int Value; + public GcRootBox Self; + public byte[] Payload; + + public GcRootBox(int v) + { + Value = v; + Self = this; + Payload = new byte[16]; + Payload[0] = (byte)v; + } + + public bool IsIntact(int expected) => + Value == expected && ReferenceEquals(Self, this) && Payload is not null && Payload[0] == (byte)expected; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static GcRootBox MakeGcRootBox(int v) => new GcRootBox(v); + + // Taking the address forces the caller's local to be an address-taken variable. + [MethodImpl(MethodImplOptions.NoInlining)] + private static void TouchGcRootBox(ref GcRootBox b) + { + if (b is null) + { + throw new InvalidOperationException(); + } + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static void CollectAndScribble() + { + GC.Collect(); + byte[] scribble = new byte[64]; + scribble[0] = 1; + GC.KeepAlive(scribble); + } + + // 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() + { + GcRootBox cur = MakeGcRootBox(0); + TouchGcRootBox(ref cur); + + for (int i = 1; i <= 128; i++) + { + GcRootBox alias = cur; // ref MOVE; sole remaining reference to box (i - 1) + cur = MakeGcRootBox(i); // overwrites cur's stack slot + TouchGcRootBox(ref cur); + CollectAndScribble(); + + 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); + } + } } } From b5010370215164cfcd706041633dffb5f6d8a232 Mon Sep 17 00:00:00 2001 From: pavelsavara Date: Fri, 14 Aug 2026 19:42:38 +0200 Subject: [PATCH 2/4] test --- .../tests/System.Runtime.Tests/System/GCTests.cs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/GCTests.cs b/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/GCTests.cs index 5aba4e2630a591..f84afe95ca7af7 100644 --- a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/GCTests.cs +++ b/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/GCTests.cs @@ -1236,9 +1236,14 @@ private static void TouchGcRootBox(ref GcRootBox b) private static void CollectAndScribble() { GC.Collect(); - byte[] scribble = new byte[64]; - scribble[0] = 1; - GC.KeepAlive(scribble); + // Reclaim and overwrite any just-freed slot with same-sized objects so that an + // object lost across the collection is observable (it reads a filler's Value = -1). + GcRootBox[] filler = new GcRootBox[64]; + for (int j = 0; j < filler.Length; j++) + { + filler[j] = new GcRootBox(-1); + } + GC.KeepAlive(filler); } // Regression test for https://github.com/dotnet/runtime/issues/130592. On the Mono From 832c7387ea17e4fe18e3663977e7ceaa62330f46 Mon Sep 17 00:00:00 2001 From: pavelsavara Date: Mon, 17 Aug 2026 13:25:25 +0200 Subject: [PATCH 3/4] test --- .../System.Runtime.Tests/System/GCTests.cs | 55 ++++++++++++------- 1 file changed, 36 insertions(+), 19 deletions(-) diff --git a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/GCTests.cs b/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/GCTests.cs index f84afe95ca7af7..6cee2d6cdb0a35 100644 --- a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/GCTests.cs +++ b/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/GCTests.cs @@ -1236,37 +1236,54 @@ private static void TouchGcRootBox(ref GcRootBox b) private static void CollectAndScribble() { GC.Collect(); - // Reclaim and overwrite any just-freed slot with same-sized objects so that an - // object lost across the collection is observable (it reads a filler's Value = -1). - GcRootBox[] filler = new GcRootBox[64]; - for (int j = 0; j < filler.Length; j++) - { - filler[j] = new GcRootBox(-1); - } - GC.KeepAlive(filler); + // Refill the just-vacated nursery space so a stale reference reads different bytes. + byte[] scribble = new byte[64]; + scribble[0] = 1; + GC.KeepAlive(scribble); + } + + // 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 is not reported as a + // precise root, so when the GC relocates the object the alias keeps the pre-move address + // and silently reads whatever later reuses that memory. The invariant -- every live + // reference to an object denotes the same object after 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 async Task MovedAliasOfAddressTakenLocalIsRootedAcrossGC() + { + // Resume on a shallow stack: under a deep caller frame a stale copy of the reference + // is often found by the conservative stack scan, which pins the object and hides the bug. + await Task.Yield(); + RunMovedAliasLoop(); } - // 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() + [MethodImpl(MethodImplOptions.NoInlining)] + private static void RunMovedAliasLoop() { + // A heap slot is always fixed up when the GC relocates the object, so it is the + // ground truth to compare the stack alias against. + GcRootBox[] witness = new GcRootBox[1]; + GcRootBox cur = MakeGcRootBox(0); TouchGcRootBox(ref cur); for (int i = 1; i <= 128; i++) { - GcRootBox alias = cur; // ref MOVE; sole remaining reference to box (i - 1) + GcRootBox alias = cur; // ref MOVE; sole remaining stack reference to box (i - 1) + witness[0] = cur; cur = MakeGcRootBox(i); // overwrites cur's stack slot TouchGcRootBox(ref cur); CollectAndScribble(); - Assert.True(alias.IsIntact(i - 1), - $"object referenced by a live local was lost across GC at iteration {i} (read Value={alias.Value})"); + if (!ReferenceEquals(alias, witness[0])) + { + Assert.Fail($"live local was not updated when the GC relocated the object at iteration {i}"); + } + if (!alias.IsIntact(i - 1)) + { + Assert.Fail($"object referenced by a live local was lost across GC at iteration {i} (read Value={alias.Value})"); + } GC.KeepAlive(alias); } } From fc85d956391d4b335223ff7818b8b1b30c9e6d4d Mon Sep 17 00:00:00 2001 From: pavelsavara Date: Mon, 17 Aug 2026 18:49:56 +0200 Subject: [PATCH 4/4] [mono][wasm] Pin ref OP_MOVE dests whose source can stop being rooted On the wasm LLVM-AOT backend a ref OP_MOVE emits no LLVM instruction, so the dest is an SSA alias of the source and relied on the source staying rooted. When the source is an address-taken variable or is redefined, its pin slot is overwritten and the alias is left referencing an object rooted nowhere, so the GC relocates the object without updating the alias. Fixes https://github.com/dotnet/runtime/issues/130592 --- src/mono/mono/mini/mini-llvm.c | 48 +++++++++++++++++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-) diff --git a/src/mono/mono/mini/mini-llvm.c b/src/mono/mono/mini/mini-llvm.c index f3166c3d2881f1..667c8b7b0acb07 100644 --- a/src/mono/mono/mini/mini-llvm.c +++ b/src/mono/mono/mini/mini-llvm.c @@ -248,6 +248,8 @@ typedef struct { int *gc_var_indexes; int gc_var_indexes_len; Address *gc_pin_area; + /* Saturating count (0-2) of definitions per vreg, used to decide if a ref move needs pinning */ + guint8 *vreg_defcount; LLVMValueRef il_state; LLVMValueRef il_state_ret; } EmitContext; @@ -4010,6 +4012,27 @@ emit_gc_pin (EmitContext *ctx, LLVMBuilderRef builder, int vreg) LLVMValueRef addr = LLVMBuildGEP2 (builder, ctx->gc_pin_area->type, ctx->gc_pin_area->value, indexes, 2, ""); emit_store (builder, convert (ctx, ctx->values [vreg], IntPtrType ()), addr, TRUE); } + +/* + * A ref OP_MOVE aliases its source instead of producing a new value, so it only needs a pin + * slot of its own when the source can stop being rooted while the alias is still live. A vreg + * with a single definition keeps its pin slot for the rest of the method, but an address-taken + * variable has no pin slot at all: it lives in a stack slot which this method or a callee can + * overwrite at any point. + */ +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)); +} #endif /* @@ -4057,6 +4080,17 @@ emit_entry_bb (EmitContext *ctx, LLVMBuilderRef builder) LLVMTypeRef pin_area_type = LLVMArrayType (IntPtrType (), ngc_vars); LLVMValueRef gc_pin_area = build_alloca_llvm_type_name (ctx, pin_area_type, 0, "gc_pin"); ctx->gc_pin_area = create_address (ctx, gc_pin_area, pin_area_type); + + /* Runs after the OP_LDADDR pass in emit_method_inner, so MONO_INST_INDIRECT is already set */ + ctx->vreg_defcount = g_new0 (guint8, cfg->next_vreg); + for (MonoBasicBlock *dbb = cfg->bb_entry; dbb; dbb = dbb->next_bb) { + for (MonoInst *dins = dbb->code; dins; dins = dins->next) { + if (dins->dreg >= 0 && (guint32)dins->dreg < cfg->next_vreg && + LLVM_INS_INFO (dins->opcode) [MONO_INST_DEST] != ' ' && + ctx->vreg_defcount [dins->dreg] < 2) + ctx->vreg_defcount [dins->dreg] ++; + } + } #endif /* @@ -12752,7 +12786,18 @@ MONO_RESTORE_WARNING emit_volatile_store (ctx, ins->dreg); #ifdef TARGET_WASM //if (vreg_is_ref (cfg, ins->dreg) && ctx->values [ins->dreg]) - if (vreg_is_ref (cfg, ins->dreg) && ctx->values [ins->dreg] && ins->opcode != OP_MOVE && ins->opcode != OP_AOTCONST) + /* + * OP_MOVE dests need pinning too. A MOVE emits no LLVM instruction, so the dest is an SSA + * alias of the source and used to rely on the source staying rooted. That fails when the + * source is an address-taken variable: emit_gc_pin skips those (they live in their stack + * slot instead), so overwriting the variable drops the only root and leaves the alias + * holding an object rooted nowhere. See dotnet/runtime#130592. + * + * OP_AOTCONST stays excluded - those dests are GOT loads of ldstr literals and type/method + * handles, already rooted by the loader's interned tables. + */ + 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); #endif } @@ -12913,6 +12958,7 @@ free_ctx (EmitContext *ctx) g_free (ctx->is_dead); g_free (ctx->unreachable); g_free (ctx->gc_var_indexes); + g_free (ctx->vreg_defcount); g_free (ctx->param_etypes); g_ptr_array_free (ctx->phi_values, TRUE); g_free (ctx->bblocks);