From 2baf9bbf54b9949c0761535dcbd6ceb7e1d7fdf5 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Sat, 30 May 2026 19:13:03 +0200 Subject: [PATCH] JIT: fix x86 GC hole in stack-to-stack struct copies In LowerCopyBlockStore, the 'isNotHeap' optimization unconditionally set doCpObj=false but only set gtBlkOpGcUnsafe=true on non-JIT32 targets. On x86 (JIT32_GCENCODER) this routed stack-to-stack struct copies to the plain unroll path, which emits the copy as INS_mov through a scratch register without reporting the register as a byref and without disabling GC. A GC between the load and store of the byref slot left the scratch register stale; the stale value was then stored to the local, which the GC info correctly reports as a byref, pointing into a free object. Guard the 'isNotHeap' shortcut behind !JIT32_GCENCODER so x86 stays on the GC-aware CpObj path (per-slot decomposition for small structs, bulk write-barrier helper for large ones). Drop the !IsAddressNotOnHeap assert in LowerBlockStoreAsGcBulkCopyCall: the helper already handles off-heap destinations correctly (its notInHeap check skips GCHeapMemoryBarrier), so it is a valid fallback for large stack-target copies. Fixes https://github.com/dotnet/runtime/issues/128801 --- src/coreclr/jit/lower.cpp | 10 +++- .../JitBlue/Runtime_128801/Runtime_128801.cs | 57 +++++++++++++++++++ .../JIT/Regression/Regression_ro_2.csproj | 1 + 3 files changed, 65 insertions(+), 3 deletions(-) create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_128801/Runtime_128801.cs diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 5f136711aec316..5c9259c976e702 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -10146,7 +10146,6 @@ void Lowering::LowerBlockStoreAsGcBulkCopyCall(GenTreeBlk* blk) assert(blk->OperIs(GT_STORE_BLK)); assert(blk->GetLayout()->HasGCPtr()); assert(!blk->OperIsInitBlkOp()); - assert(!blk->IsAddressNotOnHeap(m_compiler)); assert(!blk->IsVolatile()); assert(!blk->Data()->OperIs(GT_IND) || !blk->Data()->AsIndir()->IsVolatile()); @@ -10286,21 +10285,26 @@ void Lowering::LowerCopyBlockStore(GenTreeBlk* blkNode) const unsigned unrollLimit = m_compiler->getUnrollThreshold(Compiler::UnrollKind::Memcpy, canUseSimd); #endif +#if !defined(JIT32_GCENCODER) if (doCpObj && isNotHeap) { // No write barriers are needed if the destination is known to be outside of the GC heap. // If the layout contains a byref, then we know it must live on the stack. doCpObj = false; -#if !defined(JIT32_GCENCODER) && !defined(TARGET_WASM) +#if !defined(TARGET_WASM) if (size <= unrollLimit) { // If the size is small enough to unroll then we need to mark the block as non-interruptible // to actually allow unrolling. The generated code does not report GC references loaded in the - // temporary register(s) used for copying. This is not supported for the JIT32_GCENCODER. + // temporary register(s) used for copying. This is not supported for the JIT32_GCENCODER, so + // on that target we keep doCpObj=true above and stay on the GC-aware CpObj path; the lowering + // heuristics in TryDecomposeBlockStoreAsIndirs then choose between per-slot decomposition and + // the bulk write-barrier helper. blkNode->gtBlkOpGcUnsafe = true; } #endif } +#endif // !JIT32_GCENCODER if (doCpObj) { diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_128801/Runtime_128801.cs b/src/tests/JIT/Regression/JitBlue/Runtime_128801/Runtime_128801.cs new file mode 100644 index 00000000000000..520863dd30d465 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_128801/Runtime_128801.cs @@ -0,0 +1,57 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +// Regression test for an x86 GC hole in Tier0/minopts code: a stack-to-stack +// copy of a ReadOnlySpan went through the unroll path that loaded the +// byref slot into a scratch register without reporting it in the GC info, +// so a moving GC between the load and the store could leave the local with +// a stale byref pointing into a free object. Needs GCStress to reliably +// reproduce; CI pipelines that exercise GCStress will catch a regression. + +namespace Runtime_128801; + +using System; +using System.Runtime.CompilerServices; +using Xunit; + +public class Runtime_128801 +{ + [Fact] + public static int TestEntryPoint() + { + byte[][] buffers = new byte[256][]; + for (int i = 0; i < buffers.Length; i++) + { + buffers[i] = new byte[2]; + } + + for (int i = 0; i < 50_000; i++) + { + if (!SpanCopyAndUse(buffers[i % buffers.Length])) + { + return -1; + } + } + + return 100; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static bool SpanCopyAndUse(Span s) + { + Span local = s; + if (local.Length < 2) + { + return false; + } + ref byte p = ref local[0]; + ref byte q = ref local[1]; + if (Unsafe.ByteOffset(ref p, ref q) != new IntPtr(1)) + { + return false; + } + p = 1; + q = 2; + return true; + } +} diff --git a/src/tests/JIT/Regression/Regression_ro_2.csproj b/src/tests/JIT/Regression/Regression_ro_2.csproj index bbc80fc0290b73..f8bedd81f9050b 100644 --- a/src/tests/JIT/Regression/Regression_ro_2.csproj +++ b/src/tests/JIT/Regression/Regression_ro_2.csproj @@ -99,6 +99,7 @@ +