Improve GCHeap::Promote debug validation

[jkotas]

• Mirrors the fix from Unconditionally skip GC reporting for non-interruptible aborted methods #119403 in x86 GCInfo decoder
• Improve GCHeap::Promote debug validation to repro this class of issues deterministically

Fixes#127581

[dotnet-policy-service]

Tagging subscribers to this area: @JulieLeeMSFT, @dotnet/gc
See info in area-owners.md if you want to be subscribed.

[Copilot]

Pull request overview

This PR aims to strengthen debug-time validation in GCHeap::Promote by validating candidate object pointers even on the early-return path when the pointer is outside the find_object bookkeeping range.

Changes:

• Add a _DEBUG-only CObjectHeader::Validate() call when !gc_heap::is_in_find_object_range(o) and the root is not an interior pointer.

[jkotas]

@copilot Change the priority for test119403.cs to 0 temporarily to make the test exercised in the CI

[Copilot]

@copilot Change the priority for test119403.cs to 0 temporarily to make the test exercised in the CI

Done in bfa4fd1. Changed CLRTestPriority from 1 to 0 in test119403.csproj.

[jkotas]

gc_heap::background_promote (background.cpp:547) and gc_heap::background_promote_callback

@copilot Address feeback

[Copilot]

gc_heap::background_promote (background.cpp:547) and gc_heap::background_promote_callback

@copilot Address feeback

Done in b05e42d. Applied the same fix to both background_promote and background_promote_callback: moved the DEBUG_DestroyedHandleValue check before is_in_find_object_range, and added Validate() in the early-return path (guarded by _DEBUG and !GC_CALL_INTERIOR).

[jkotas]

@copilot Implement the fix mentioned in #127581 (comment)

[Copilot]

@copilot Implement the fix mentioned in #127581 (comment)

Done in f66c09c. Added the equivalent of the gcinfodecoder.cpp fix to EnumGcRefsX86 in gc_unwind_x86.inl: when a partially-interruptible method is aborted (exception in flight) and the current offset is not within an interruptible range, skip all GC reporting. Untracked locals may not be initialized at the faulting offset, and since execution won't resume there, there's no need to report them.

[Copilot]

Pull request overview

This PR addresses a class of x86 GC hole issues (tracked in #127581) by aligning x86 GC reference enumeration behavior with the earlier fix from #119403 and by strengthening debug-time validation in GCHeap::Promote paths so invalid/out-of-range object references fail deterministically in debug builds.

Changes:

• Update x86 GCInfo/stack-walk enumeration to skip reporting entirely for execution-aborted, non-interruptible methods (where slots may not be initialized).
• Add additional _DEBUG validation when Promote/background promotion sees a non-null, non-interior pointer that is outside the GC “find_object” bookkeeping range.
• Adjust the regression test project priority to make the scenario easier to exercise in standard test runs.

Reviewed changes

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

File	Description
src/tests/Regressions/coreclr/GitHub_119403/test119403.csproj	Changes test priority to run as a default-priority regression.
src/coreclr/vm/gc_unwind_x86.inl	Skips GC reporting for execution-aborted non-interruptible methods on x86 to avoid reporting uninitialized GC slots.
src/coreclr/gc/interface.cpp	Adds debug validation in GCHeap::Promote when an out-of-range non-interior reference is encountered.
src/coreclr/gc/background.cpp	Adds the same debug validation behavior to background GC promotion paths.

[mangod9]

The fix is working:

CI Validation of the Fix

Build 1403154 — ❌ Failed (without x86 fix)

• Commit: bc54dbf (Apr 30 05:32 UTC)
• Contains: Validate change in interface.cpp + test promoted to Pri0 — but NOT the gc_unwind_x86.inl fix
• Result: coreclr Pri0 Runtime Tests Run windows x86 checked → failed
• Failure: Regressions workitem in Helix job ab237a9f-5703-40e6-9caa-e9eba9d566d0 Assert: !"Detected use of a
  corrupted OBJECTREF. Possible GC hole."
  Object::Validate + 0xA9
  → WKS::GCHeap::Promote + 0x169 ← new Validate() catches the bad ref
  → GcEnumObject
  → EnumGcRefsX86 ← reports untracked slot for aborted frame
  → SubProblem + 5 ← fault offset (before local 's' initialized)

Build 1403719 — ✅ Passed (with x86 fix)

• Commit: 9a9b245 → includes f66c09c "Fix EnumGcRefsX86 to skip reporting for non-interruptible aborted frames" (Apr 30 12:03 UTC)
• Contains: Validate change + test Pri0 + gc_unwind_x86.inl fix
• Result: coreclr Pri0 Runtime Tests Run windows x86 checked → succeeded (44m 3s)
• EnumGcRefsX86 now returns early for the aborted non-interruptible frame, so Promote/Validate is never called with the uninitialized slot

[github-actions]

🤖 Copilot Code Review — PR #127595

Note

This review was generated by GitHub Copilot.

Holistic Assessment

Motivation: The PR addresses a real correctness issue where EnumGcRefsX86 could report uninitialized GC slots for non-interruptible aborted frames, and improves debug validation in GC promote functions. These are genuine bugs that can cause GC heap corruption or crashes.

Approach: The changes are minimal and well-targeted — reordering the DEBUG_DestroyedHandleValue check before is_in_find_object_range (race condition correctness), adding debug-only Validate() calls for early diagnostics, and porting the existing gcinfodecoder.cpp logic to the x86 EnumGcRefsX86 path. The approach mirrors existing patterns already present in the codebase.

Summary: ✅ LGTM. The changes are correct, minimal, and consistent with established codebase patterns. All three GC promote functions (GCHeap::Promote, background_promote, background_promote_callback) are now consistent. The x86 fix is a straightforward port of logic already proven in gcinfodecoder.cpp (lines 808-812).

---

Detailed Findings

✅ Correctness — DEBUG_DestroyedHandleValue reordering (interface.cpp, background.cpp)

Moving the DEBUG_DestroyedHandleValue check before is_in_find_object_range is correct. During concurrent scanning, a handle can be destroyed (set to the sentinel value) while the GC is scanning. The destroyed sentinel value is not a valid heap address, so is_in_find_object_range would return false, but without the reordering, the new Validate() call (added in the !is_in_find_object_range path) could attempt to validate the sentinel value as an object, which would crash. Checking the sentinel first avoids this.

✅ Correctness — Debug validation in early-return path

Adding ((CObjectHeader*)o)->Validate() under _DEBUG for non-null, non-interior objects that fall outside the GC range is a sound diagnostic addition. The guards (o != NULL and !(flags & GC_CALL_INTERIOR)) are appropriate — null pointers obviously cannot be validated, and interior pointers do not point to object headers.

✅ Correctness — EnumGcRefsX86 aborted frame fix (gc_unwind_x86.inl)

The added condition ((flags & ExecutionAborted) && !info.interruptible) exactly matches the logic in gcinfodecoder.cpp at lines 808-812 (which skips reporting when countIntersections == 0 && executionAborted — meaning the frame is not fully interruptible). For non-interruptible methods where execution is aborted, GC slots may not be initialized at the current offset, so reporting them could cause the GC to trace garbage values. The existing _ASSERTE(flags & ExecutionAborted) at line 3103 already documents that this early-return path is only expected for aborted frames — the new condition simply widens the gate to include the non-interruptible aborted case alongside prolog/epilog.

✅ Test Priority — test119403.csproj

Changing priority from 0 to 1 follows the convention: "New JIT regression tests should typically be CLRTestPriority 1." The test was temporarily set to 0 earlier in the PR branch for verification, then correctly restored to 1 for merge.

✅ Consistency — All three promote functions aligned

GCHeap::Promote, background_promote, and background_promote_callback now all follow the same pattern: check DEBUG_DestroyedHandleValue → check is_in_find_object_range (with debug validation) → proceed. This eliminates an inconsistency that existed before this PR.

Generated by Code Review for issue #127595 · ◷

[Copilot]

Pull request overview

This PR targets a win-x86 GC root reporting hole by (1) aligning the x86 GCInfo unwinder behavior with the earlier fix in #119403 and (2) strengthening debug-time GC promotion validation so this class of corruption becomes easier to reproduce and diagnose deterministically.

Changes:

• Update x86 GC root enumeration to skip reporting for non-interruptible methods when execution has been aborted (in addition to prolog/epilog cases).
• Add _DEBUG-only validation in GCHeap::Promote and background promotion paths when an object reference falls outside the find_object bookkeeping range.

Reviewed changes

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

File	Description
src/coreclr/vm/gc_unwind_x86.inl	Extends the “skip reporting” condition to cover aborted + non-interruptible methods, matching the intended semantics from the earlier fix.
src/coreclr/gc/interface.cpp	Moves destroyed-handle sentinel check earlier and adds debug validation when the ref is outside is_in_find_object_range.
src/coreclr/gc/background.cpp	Adds similar debug validation in background promote paths (including callback).

[mangod9]

Btw, there is a libraries x86 failure but doesnt look like that is related to this change. The auto analysis is below. @VSadov surprised that ThreadAbort is being throw here?

!m_RedirectContextInUse Assert on win-x86 — Analysis

Assert

Assert failure (Thread 0x590): !m_RedirectContextInUse
File: src/coreclr/vm/threads.h:3104
Image: dotnet.exe (x86 Checked)
Exit code: -1073740286 (STATUS_FAIL_FAST_EXCEPTION)

Fires in Thread::MarkRedirectContextInUse when the runtime attempts to redirect (hijack) a thread whose redirect context is already occupied from a prior redirect.

Thread State at Crash

Thread	OSID	GC Mode	Role	Activity
11	0x590	Cooperative	Crash thread	Inside 1st redirect → HandleThreadAbort → constructing ThreadAbortException → 2nd redirect fires assert
12	0xccc	Cooperative	GC thread	Running GCExtendedTests.GetTotalAllocatedBytes() → GC.GetTotalAllocatedBytesPrecise() → triggered GC
6	0x101c	Preemptive	Tiered compilation	JIT compiling, blocked in WaitUntilGCComplete
5	0x1910	Preemptive	Finalizer	Waiting for finalizer event
0	0x1ad8	Preemptive	Main	xunit runner waiting

Full Stack — Thread 11 (OSID 0x590) — Native + Managed

Shows two nested redirects — the second one fires the assert:

=== 2nd redirect (fires the assert) ===
coreclr!FailFastOnAssert
coreclr!_DbgBreakCheck
coreclr!_DbgBreakCheckNoThrow
coreclr!DbgAssertDialog
coreclr!Thread::MarkRedirectContextInUse ← 2nd redirect: context already in use!
coreclr!Thread::RedirectedHandledJITCase ← 2nd redirect handler
coreclr!Thread::RedirectedHandledJITCaseForGCThreadControl
coreclr!RedirectedHandledJITCaseForGCThreadControl_Stub ← 2nd GC hijack entry point
=== Managed code running inside 1st redirect (ThreadAbortException construction) ===
System.IO.BinaryReader.ReadByte()
System.IO.BinaryReader.Read7BitEncodedInt()
System.Resources.ResourceReader.LoadString(Int32)
System.Resources.RuntimeResourceSet.ReadValue(...)
System.Resources.RuntimeResourceSet.GetObject(...)
System.Resources.RuntimeResourceSet.GetString(...)
System.Resources.ResourceManager.GetString(...)
System.SR.InternalGetResourceString(String)
System.SR.GetResourceString(String)
System.SR.get_Arg_SystemException()
System.SystemException..ctor()
System.Threading.ThreadAbortException..ctor()
System.Runtime.CompilerServices.RuntimeHelpers.CallDefaultConstructor(...)
=== 1st redirect processing the pending ThreadAbort ===
coreclr!CallDefaultConstructor
coreclr!EEException::CreateThrowable
coreclr!CLRException::GetThrowable
coreclr!CLRException::GetThrowableFromException
coreclr!Thread::HandleThreadAbort ← processes pending TS_AbortRequested
coreclr!Thread::RestoreContextSimulated
coreclr!Thread::RedirectedHandledJITCase ← 1st redirect handler
coreclr!Thread::RedirectedHandledJITCaseForGCThreadControl
coreclr!RedirectedHandledJITCaseForGCThreadControl_Stub ← 1st GC hijack entry point
=== Original managed code when 1st redirect hijacked the thread ===
[InlinedCallFrame] System.RuntimeMethodHandle.InvokeMethod(...)
System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object, IntPtr*)
System.Reflection.RuntimeMethodInfo.Invoke(...)
System.Reflection.MethodBase.Invoke(Object, Object[])
Xunit.Sdk.TestInvoker`1.CallTestMethod(Object)
Xunit.Sdk.TestInvoker`1+<>c__DisplayClass47_0.<<InvokeTestMethodAsync>b__1>d.MoveNext()
... (xunit async test invocation pipeline) ...
Xunit.Sdk.XunitTestCase.RunAsync(IMessageSink, IMessageBus, Object[], ExceptionAggregator, CancellationTokenSource)
Microsoft.DotNet.XUnitExtensions.SkippedFactTestCase.<>n__0(...)
Microsoft.DotNet.XUnitExtensions.SkippedFactTestCase+<RunAsync>d__2.MoveNext()
... (xunit async test runner pipeline) ...
Xunit.Sdk.XunitTestAssemblyRunner+<RunTestCollectionAsync>d__18.MoveNext()
System.Threading.ExecutionContext.RunInternal(...)
... (async continuations) ...
System.Threading.Tasks.Task+CancellationPromise`1[[Boolean]].ITaskCompletionAction.Invoke(Task)
System.Threading.Tasks.CompletionActionInvoker.Execute()
System.Threading.ThreadPoolWorkQueue.Dispatch()
System.Threading.PortableThreadPool+WorkerThread.WorkerDoWork(PortableThreadPool)
System.Threading.PortableThreadPool+WorkerThread.WorkerThreadStart()
System.Threading.Thread.StartCallback(Thread*)

GC Thread Stack (Thread 12, OSID 0xccc)

System.GC.GetTotalAllocatedBytesPrecise() ← triggers GC
System.GC.GetTotalAllocatedBytes(Boolean)
System.Tests.GCExtendedTests.GetTotalAllocatedBytes() ← the test
... (xunit async pipeline via reflection invoke) ...
System.Threading.PortableThreadPool+WorkerThread.WorkerDoWork(PortableThreadPool)
System.Threading.PortableThreadPool+WorkerThread.WorkerThreadStart()
System.Threading.Thread.StartCallback(Thread*)

Sequence of Events

1. Thread 12 runs GCExtendedTests.GetTotalAllocatedBytes() → GC.GetTotalAllocatedBytesPrecise() → triggers a GC collection
2. GC suspension begins. Thread 11 is in Cooperative mode running a test via SkippedFactTestCase.RunAsync. The runtime redirects (hijacks) Thread 11 via RedirectedHandledJITCaseForGCThreadControl_Stub (1st redirect)
3. Inside RedirectedHandledJITCase → RestoreContextSimulated, the runtime calls Thread::HandleThreadAbort — a pending thread abort (TS_AbortRequested) was set on Thread 11. The source of the abort is not captured in the dump. On modern .NET (6+), Thread.Abort() throws PlatformNotSupportedException; thread aborts can only be initiated by the runtime internally or via ControlledExecution.Run
4. HandleThreadAbort → CreateThrowable → CallDefaultConstructor for ThreadAbortException — this runs managed code to construct the exception (loading the resource string "Arg_SystemException" via SR.GetResourceString → ResourceReader.LoadString → BinaryReader.ReadByte)
5. The managed code runs in Cooperative mode, making Thread 11 eligible for GC hijacking again
6. A second GC triggers (or GC suspension retries). The runtime attempts to redirect Thread 11 again via RedirectedHandledJITCaseForGCThreadControl_Stub (2nd redirect)
7. MarkRedirectContextInUse finds m_RedirectContextInUse == true (still held from step 2) → assert fires

Key Finding

There is no separate cancellation thread visible in the dump. The ThreadAbort was a pending flag (TS_AbortRequested) already set on Thread 11. The source of the abort is not captured in the dump — on modern .NET, thread aborts can come from ControlledExecution.Run or other runtime-internal mechanisms. Thread::HandleThreadAbort processes this flag as part of the 1st redirect handler's normal flow (RestoreContextSimulated → HandleThreadAbort). The bug is that the 1st redirect handler runs managed code (exception construction) while still holding the redirect context, creating a window where a 2nd redirect can be attempted.

Root Cause

A double-redirect race condition on x86: the runtime hijacks a thread for GC suspension, processes a pending ThreadAbort inside the redirect handler, and the ThreadAbortException constructor runs managed code (resource string loading) in Cooperative mode. This makes the thread eligible for another GC redirect while the first redirect context is still held.

Why This Hasn't Been Seen Before

• Libraries Test Run checked coreclr windows x86 Debug only runs on PR builds, not main rolling builds
• Main rolling builds run Libraries Test Run checked coreclr windows x86 **Release** — Release libraries are faster, narrowing the timing window
• Debug libraries' slower execution (resource loading, extra checks) widens the race window between the first redirect and the second attempt
• The m_RedirectContextInUse assert only fires in Checked builds

Relation to PR #127595

Not related. The gc_unwind_x86.inl fix changes GC reporting logic inside EnumGcRefsX86. The Validate() change in interface.cpp adds a Debug assert in Promote. Neither affects thread redirect/hijack logic. This assert passed on the previous build of the same PR (build 1403154), confirming it's intermittent.

[jkotas]

• Mirrors the fix from Unconditionally skip GC reporting for non-interruptible aborted methods #119403 in x86 GCInfo decoder
• Improve GCHeap::Promote debug validation to repro this class of issues deterministically

Fixes#127581

[dotnet-policy-service]

Tagging subscribers to this area: @JulieLeeMSFT, @dotnet/gc
See info in area-owners.md if you want to be subscribed.

[Copilot]

Pull request overview

This PR aims to strengthen debug-time validation in GCHeap::Promote by validating candidate object pointers even on the early-return path when the pointer is outside the find_object bookkeeping range.

Changes:

• Add a _DEBUG-only CObjectHeader::Validate() call when !gc_heap::is_in_find_object_range(o) and the root is not an interior pointer.

[jkotas]

@copilot Change the priority for test119403.cs to 0 temporarily to make the test exercised in the CI

[Copilot]

@copilot Change the priority for test119403.cs to 0 temporarily to make the test exercised in the CI

Done in bfa4fd1. Changed CLRTestPriority from 1 to 0 in test119403.csproj.

[jkotas]

gc_heap::background_promote (background.cpp:547) and gc_heap::background_promote_callback

@copilot Address feeback

[Copilot]

gc_heap::background_promote (background.cpp:547) and gc_heap::background_promote_callback

@copilot Address feeback

Done in b05e42d. Applied the same fix to both background_promote and background_promote_callback: moved the DEBUG_DestroyedHandleValue check before is_in_find_object_range, and added Validate() in the early-return path (guarded by _DEBUG and !GC_CALL_INTERIOR).

[jkotas]

@copilot Implement the fix mentioned in #127581 (comment)

[Copilot]

@copilot Implement the fix mentioned in #127581 (comment)

Done in f66c09c. Added the equivalent of the gcinfodecoder.cpp fix to EnumGcRefsX86 in gc_unwind_x86.inl: when a partially-interruptible method is aborted (exception in flight) and the current offset is not within an interruptible range, skip all GC reporting. Untracked locals may not be initialized at the faulting offset, and since execution won't resume there, there's no need to report them.

[Copilot]

Pull request overview

This PR addresses a class of x86 GC hole issues (tracked in #127581) by aligning x86 GC reference enumeration behavior with the earlier fix from #119403 and by strengthening debug-time validation in GCHeap::Promote paths so invalid/out-of-range object references fail deterministically in debug builds.

Changes:

• Update x86 GCInfo/stack-walk enumeration to skip reporting entirely for execution-aborted, non-interruptible methods (where slots may not be initialized).
• Add additional _DEBUG validation when Promote/background promotion sees a non-null, non-interior pointer that is outside the GC “find_object” bookkeeping range.
• Adjust the regression test project priority to make the scenario easier to exercise in standard test runs.

Reviewed changes

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

File	Description
src/tests/Regressions/coreclr/GitHub_119403/test119403.csproj	Changes test priority to run as a default-priority regression.
src/coreclr/vm/gc_unwind_x86.inl	Skips GC reporting for execution-aborted non-interruptible methods on x86 to avoid reporting uninitialized GC slots.
src/coreclr/gc/interface.cpp	Adds debug validation in GCHeap::Promote when an out-of-range non-interior reference is encountered.
src/coreclr/gc/background.cpp	Adds the same debug validation behavior to background GC promotion paths.

[mangod9]

The fix is working:

CI Validation of the Fix

Build 1403154 — ❌ Failed (without x86 fix)

• Commit: bc54dbf (Apr 30 05:32 UTC)
• Contains: Validate change in interface.cpp + test promoted to Pri0 — but NOT the gc_unwind_x86.inl fix
• Result: coreclr Pri0 Runtime Tests Run windows x86 checked → failed
• Failure: Regressions workitem in Helix job ab237a9f-5703-40e6-9caa-e9eba9d566d0 Assert: !"Detected use of a
  corrupted OBJECTREF. Possible GC hole."
  Object::Validate + 0xA9
  → WKS::GCHeap::Promote + 0x169 ← new Validate() catches the bad ref
  → GcEnumObject
  → EnumGcRefsX86 ← reports untracked slot for aborted frame
  → SubProblem + 5 ← fault offset (before local 's' initialized)

Build 1403719 — ✅ Passed (with x86 fix)

• Commit: 9a9b245 → includes f66c09c "Fix EnumGcRefsX86 to skip reporting for non-interruptible aborted frames" (Apr 30 12:03 UTC)
• Contains: Validate change + test Pri0 + gc_unwind_x86.inl fix
• Result: coreclr Pri0 Runtime Tests Run windows x86 checked → succeeded (44m 3s)
• EnumGcRefsX86 now returns early for the aborted non-interruptible frame, so Promote/Validate is never called with the uninitialized slot

[github-actions]

🤖 Copilot Code Review — PR #127595

Note

This review was generated by GitHub Copilot.

Holistic Assessment

Motivation: The PR addresses a real correctness issue where EnumGcRefsX86 could report uninitialized GC slots for non-interruptible aborted frames, and improves debug validation in GC promote functions. These are genuine bugs that can cause GC heap corruption or crashes.

Approach: The changes are minimal and well-targeted — reordering the DEBUG_DestroyedHandleValue check before is_in_find_object_range (race condition correctness), adding debug-only Validate() calls for early diagnostics, and porting the existing gcinfodecoder.cpp logic to the x86 EnumGcRefsX86 path. The approach mirrors existing patterns already present in the codebase.

Summary: ✅ LGTM. The changes are correct, minimal, and consistent with established codebase patterns. All three GC promote functions (GCHeap::Promote, background_promote, background_promote_callback) are now consistent. The x86 fix is a straightforward port of logic already proven in gcinfodecoder.cpp (lines 808-812).

---

Detailed Findings

✅ Correctness — DEBUG_DestroyedHandleValue reordering (interface.cpp, background.cpp)

Moving the DEBUG_DestroyedHandleValue check before is_in_find_object_range is correct. During concurrent scanning, a handle can be destroyed (set to the sentinel value) while the GC is scanning. The destroyed sentinel value is not a valid heap address, so is_in_find_object_range would return false, but without the reordering, the new Validate() call (added in the !is_in_find_object_range path) could attempt to validate the sentinel value as an object, which would crash. Checking the sentinel first avoids this.

✅ Correctness — Debug validation in early-return path

Adding ((CObjectHeader*)o)->Validate() under _DEBUG for non-null, non-interior objects that fall outside the GC range is a sound diagnostic addition. The guards (o != NULL and !(flags & GC_CALL_INTERIOR)) are appropriate — null pointers obviously cannot be validated, and interior pointers do not point to object headers.

✅ Correctness — EnumGcRefsX86 aborted frame fix (gc_unwind_x86.inl)

The added condition ((flags & ExecutionAborted) && !info.interruptible) exactly matches the logic in gcinfodecoder.cpp at lines 808-812 (which skips reporting when countIntersections == 0 && executionAborted — meaning the frame is not fully interruptible). For non-interruptible methods where execution is aborted, GC slots may not be initialized at the current offset, so reporting them could cause the GC to trace garbage values. The existing _ASSERTE(flags & ExecutionAborted) at line 3103 already documents that this early-return path is only expected for aborted frames — the new condition simply widens the gate to include the non-interruptible aborted case alongside prolog/epilog.

✅ Test Priority — test119403.csproj

Changing priority from 0 to 1 follows the convention: "New JIT regression tests should typically be CLRTestPriority 1." The test was temporarily set to 0 earlier in the PR branch for verification, then correctly restored to 1 for merge.

✅ Consistency — All three promote functions aligned

GCHeap::Promote, background_promote, and background_promote_callback now all follow the same pattern: check DEBUG_DestroyedHandleValue → check is_in_find_object_range (with debug validation) → proceed. This eliminates an inconsistency that existed before this PR.

Generated by Code Review for issue #127595 · ◷

[Copilot]

Pull request overview

This PR targets a win-x86 GC root reporting hole by (1) aligning the x86 GCInfo unwinder behavior with the earlier fix in #119403 and (2) strengthening debug-time GC promotion validation so this class of corruption becomes easier to reproduce and diagnose deterministically.

Changes:

• Update x86 GC root enumeration to skip reporting for non-interruptible methods when execution has been aborted (in addition to prolog/epilog cases).
• Add _DEBUG-only validation in GCHeap::Promote and background promotion paths when an object reference falls outside the find_object bookkeeping range.

Reviewed changes

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

File	Description
src/coreclr/vm/gc_unwind_x86.inl	Extends the “skip reporting” condition to cover aborted + non-interruptible methods, matching the intended semantics from the earlier fix.
src/coreclr/gc/interface.cpp	Moves destroyed-handle sentinel check earlier and adds debug validation when the ref is outside is_in_find_object_range.
src/coreclr/gc/background.cpp	Adds similar debug validation in background promote paths (including callback).

[mangod9]

Btw, there is a libraries x86 failure but doesnt look like that is related to this change. The auto analysis is below. @VSadov surprised that ThreadAbort is being throw here?

!m_RedirectContextInUse Assert on win-x86 — Analysis

Assert

Assert failure (Thread 0x590): !m_RedirectContextInUse
File: src/coreclr/vm/threads.h:3104
Image: dotnet.exe (x86 Checked)
Exit code: -1073740286 (STATUS_FAIL_FAST_EXCEPTION)

Fires in Thread::MarkRedirectContextInUse when the runtime attempts to redirect (hijack) a thread whose redirect context is already occupied from a prior redirect.

Thread State at Crash

Thread	OSID	GC Mode	Role	Activity
11	0x590	Cooperative	Crash thread	Inside 1st redirect → HandleThreadAbort → constructing ThreadAbortException → 2nd redirect fires assert
12	0xccc	Cooperative	GC thread	Running GCExtendedTests.GetTotalAllocatedBytes() → GC.GetTotalAllocatedBytesPrecise() → triggered GC
6	0x101c	Preemptive	Tiered compilation	JIT compiling, blocked in WaitUntilGCComplete
5	0x1910	Preemptive	Finalizer	Waiting for finalizer event
0	0x1ad8	Preemptive	Main	xunit runner waiting

Full Stack — Thread 11 (OSID 0x590) — Native + Managed

Shows two nested redirects — the second one fires the assert:

=== 2nd redirect (fires the assert) ===
coreclr!FailFastOnAssert
coreclr!_DbgBreakCheck
coreclr!_DbgBreakCheckNoThrow
coreclr!DbgAssertDialog
coreclr!Thread::MarkRedirectContextInUse ← 2nd redirect: context already in use!
coreclr!Thread::RedirectedHandledJITCase ← 2nd redirect handler
coreclr!Thread::RedirectedHandledJITCaseForGCThreadControl
coreclr!RedirectedHandledJITCaseForGCThreadControl_Stub ← 2nd GC hijack entry point
=== Managed code running inside 1st redirect (ThreadAbortException construction) ===
System.IO.BinaryReader.ReadByte()
System.IO.BinaryReader.Read7BitEncodedInt()
System.Resources.ResourceReader.LoadString(Int32)
System.Resources.RuntimeResourceSet.ReadValue(...)
System.Resources.RuntimeResourceSet.GetObject(...)
System.Resources.RuntimeResourceSet.GetString(...)
System.Resources.ResourceManager.GetString(...)
System.SR.InternalGetResourceString(String)
System.SR.GetResourceString(String)
System.SR.get_Arg_SystemException()
System.SystemException..ctor()
System.Threading.ThreadAbortException..ctor()
System.Runtime.CompilerServices.RuntimeHelpers.CallDefaultConstructor(...)
=== 1st redirect processing the pending ThreadAbort ===
coreclr!CallDefaultConstructor
coreclr!EEException::CreateThrowable
coreclr!CLRException::GetThrowable
coreclr!CLRException::GetThrowableFromException
coreclr!Thread::HandleThreadAbort ← processes pending TS_AbortRequested
coreclr!Thread::RestoreContextSimulated
coreclr!Thread::RedirectedHandledJITCase ← 1st redirect handler
coreclr!Thread::RedirectedHandledJITCaseForGCThreadControl
coreclr!RedirectedHandledJITCaseForGCThreadControl_Stub ← 1st GC hijack entry point
=== Original managed code when 1st redirect hijacked the thread ===
[InlinedCallFrame] System.RuntimeMethodHandle.InvokeMethod(...)
System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object, IntPtr*)
System.Reflection.RuntimeMethodInfo.Invoke(...)
System.Reflection.MethodBase.Invoke(Object, Object[])
Xunit.Sdk.TestInvoker`1.CallTestMethod(Object)
Xunit.Sdk.TestInvoker`1+<>c__DisplayClass47_0.<<InvokeTestMethodAsync>b__1>d.MoveNext()
... (xunit async test invocation pipeline) ...
Xunit.Sdk.XunitTestCase.RunAsync(IMessageSink, IMessageBus, Object[], ExceptionAggregator, CancellationTokenSource)
Microsoft.DotNet.XUnitExtensions.SkippedFactTestCase.<>n__0(...)
Microsoft.DotNet.XUnitExtensions.SkippedFactTestCase+<RunAsync>d__2.MoveNext()
... (xunit async test runner pipeline) ...
Xunit.Sdk.XunitTestAssemblyRunner+<RunTestCollectionAsync>d__18.MoveNext()
System.Threading.ExecutionContext.RunInternal(...)
... (async continuations) ...
System.Threading.Tasks.Task+CancellationPromise`1[[Boolean]].ITaskCompletionAction.Invoke(Task)
System.Threading.Tasks.CompletionActionInvoker.Execute()
System.Threading.ThreadPoolWorkQueue.Dispatch()
System.Threading.PortableThreadPool+WorkerThread.WorkerDoWork(PortableThreadPool)
System.Threading.PortableThreadPool+WorkerThread.WorkerThreadStart()
System.Threading.Thread.StartCallback(Thread*)

GC Thread Stack (Thread 12, OSID 0xccc)

System.GC.GetTotalAllocatedBytesPrecise() ← triggers GC
System.GC.GetTotalAllocatedBytes(Boolean)
System.Tests.GCExtendedTests.GetTotalAllocatedBytes() ← the test
... (xunit async pipeline via reflection invoke) ...
System.Threading.PortableThreadPool+WorkerThread.WorkerDoWork(PortableThreadPool)
System.Threading.PortableThreadPool+WorkerThread.WorkerThreadStart()
System.Threading.Thread.StartCallback(Thread*)

Sequence of Events

1. Thread 12 runs GCExtendedTests.GetTotalAllocatedBytes() → GC.GetTotalAllocatedBytesPrecise() → triggers a GC collection
2. GC suspension begins. Thread 11 is in Cooperative mode running a test via SkippedFactTestCase.RunAsync. The runtime redirects (hijacks) Thread 11 via RedirectedHandledJITCaseForGCThreadControl_Stub (1st redirect)
3. Inside RedirectedHandledJITCase → RestoreContextSimulated, the runtime calls Thread::HandleThreadAbort — a pending thread abort (TS_AbortRequested) was set on Thread 11. The source of the abort is not captured in the dump. On modern .NET (6+), Thread.Abort() throws PlatformNotSupportedException; thread aborts can only be initiated by the runtime internally or via ControlledExecution.Run
4. HandleThreadAbort → CreateThrowable → CallDefaultConstructor for ThreadAbortException — this runs managed code to construct the exception (loading the resource string "Arg_SystemException" via SR.GetResourceString → ResourceReader.LoadString → BinaryReader.ReadByte)
5. The managed code runs in Cooperative mode, making Thread 11 eligible for GC hijacking again
6. A second GC triggers (or GC suspension retries). The runtime attempts to redirect Thread 11 again via RedirectedHandledJITCaseForGCThreadControl_Stub (2nd redirect)
7. MarkRedirectContextInUse finds m_RedirectContextInUse == true (still held from step 2) → assert fires

Key Finding

There is no separate cancellation thread visible in the dump. The ThreadAbort was a pending flag (TS_AbortRequested) already set on Thread 11. The source of the abort is not captured in the dump — on modern .NET, thread aborts can come from ControlledExecution.Run or other runtime-internal mechanisms. Thread::HandleThreadAbort processes this flag as part of the 1st redirect handler's normal flow (RestoreContextSimulated → HandleThreadAbort). The bug is that the 1st redirect handler runs managed code (exception construction) while still holding the redirect context, creating a window where a 2nd redirect can be attempted.

Root Cause

A double-redirect race condition on x86: the runtime hijacks a thread for GC suspension, processes a pending ThreadAbort inside the redirect handler, and the ThreadAbortException constructor runs managed code (resource string loading) in Cooperative mode. This makes the thread eligible for another GC redirect while the first redirect context is still held.

Why This Hasn't Been Seen Before

• Libraries Test Run checked coreclr windows x86 Debug only runs on PR builds, not main rolling builds
• Main rolling builds run Libraries Test Run checked coreclr windows x86 **Release** — Release libraries are faster, narrowing the timing window
• Debug libraries' slower execution (resource loading, extra checks) widens the race window between the first redirect and the second attempt
• The m_RedirectContextInUse assert only fires in Checked builds

Relation to PR #127595

Not related. The gc_unwind_x86.inl fix changes GC reporting logic inside EnumGcRefsX86. The Validate() change in interface.cpp adds a Debug assert in Promote. Neither affects thread redirect/hijack logic. This assert passed on the previous build of the same PR (build 1403154), confirming it's intermittent.

[jkotas]

• Mirrors the fix from Unconditionally skip GC reporting for non-interruptible aborted methods #119403 in x86 GCInfo decoder
• Improve GCHeap::Promote debug validation to repro this class of issues deterministically

Fixes#127581

[dotnet-policy-service]

Tagging subscribers to this area: @JulieLeeMSFT, @dotnet/gc
See info in area-owners.md if you want to be subscribed.

[Copilot]

Pull request overview

This PR aims to strengthen debug-time validation in GCHeap::Promote by validating candidate object pointers even on the early-return path when the pointer is outside the find_object bookkeeping range.

Changes:

• Add a _DEBUG-only CObjectHeader::Validate() call when !gc_heap::is_in_find_object_range(o) and the root is not an interior pointer.

[jkotas]

@copilot Change the priority for test119403.cs to 0 temporarily to make the test exercised in the CI

[Copilot]

@copilot Change the priority for test119403.cs to 0 temporarily to make the test exercised in the CI

Done in bfa4fd1. Changed CLRTestPriority from 1 to 0 in test119403.csproj.

[jkotas]

gc_heap::background_promote (background.cpp:547) and gc_heap::background_promote_callback

@copilot Address feeback

[Copilot]

gc_heap::background_promote (background.cpp:547) and gc_heap::background_promote_callback

@copilot Address feeback

Done in b05e42d. Applied the same fix to both background_promote and background_promote_callback: moved the DEBUG_DestroyedHandleValue check before is_in_find_object_range, and added Validate() in the early-return path (guarded by _DEBUG and !GC_CALL_INTERIOR).

[jkotas]

@copilot Implement the fix mentioned in #127581 (comment)

[Copilot]

@copilot Implement the fix mentioned in #127581 (comment)

Done in f66c09c. Added the equivalent of the gcinfodecoder.cpp fix to EnumGcRefsX86 in gc_unwind_x86.inl: when a partially-interruptible method is aborted (exception in flight) and the current offset is not within an interruptible range, skip all GC reporting. Untracked locals may not be initialized at the faulting offset, and since execution won't resume there, there's no need to report them.

[Copilot]

Pull request overview

This PR addresses a class of x86 GC hole issues (tracked in #127581) by aligning x86 GC reference enumeration behavior with the earlier fix from #119403 and by strengthening debug-time validation in GCHeap::Promote paths so invalid/out-of-range object references fail deterministically in debug builds.

Changes:

• Update x86 GCInfo/stack-walk enumeration to skip reporting entirely for execution-aborted, non-interruptible methods (where slots may not be initialized).
• Add additional _DEBUG validation when Promote/background promotion sees a non-null, non-interior pointer that is outside the GC “find_object” bookkeeping range.
• Adjust the regression test project priority to make the scenario easier to exercise in standard test runs.

Reviewed changes

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

File	Description
src/tests/Regressions/coreclr/GitHub_119403/test119403.csproj	Changes test priority to run as a default-priority regression.
src/coreclr/vm/gc_unwind_x86.inl	Skips GC reporting for execution-aborted non-interruptible methods on x86 to avoid reporting uninitialized GC slots.
src/coreclr/gc/interface.cpp	Adds debug validation in GCHeap::Promote when an out-of-range non-interior reference is encountered.
src/coreclr/gc/background.cpp	Adds the same debug validation behavior to background GC promotion paths.

[mangod9]

The fix is working:

CI Validation of the Fix

Build 1403154 — ❌ Failed (without x86 fix)

• Commit: bc54dbf (Apr 30 05:32 UTC)
• Contains: Validate change in interface.cpp + test promoted to Pri0 — but NOT the gc_unwind_x86.inl fix
• Result: coreclr Pri0 Runtime Tests Run windows x86 checked → failed
• Failure: Regressions workitem in Helix job ab237a9f-5703-40e6-9caa-e9eba9d566d0 Assert: !"Detected use of a
  corrupted OBJECTREF. Possible GC hole."
  Object::Validate + 0xA9
  → WKS::GCHeap::Promote + 0x169 ← new Validate() catches the bad ref
  → GcEnumObject
  → EnumGcRefsX86 ← reports untracked slot for aborted frame
  → SubProblem + 5 ← fault offset (before local 's' initialized)

Build 1403719 — ✅ Passed (with x86 fix)

• Commit: 9a9b245 → includes f66c09c "Fix EnumGcRefsX86 to skip reporting for non-interruptible aborted frames" (Apr 30 12:03 UTC)
• Contains: Validate change + test Pri0 + gc_unwind_x86.inl fix
• Result: coreclr Pri0 Runtime Tests Run windows x86 checked → succeeded (44m 3s)
• EnumGcRefsX86 now returns early for the aborted non-interruptible frame, so Promote/Validate is never called with the uninitialized slot

[github-actions]

🤖 Copilot Code Review — PR #127595

Note

This review was generated by GitHub Copilot.

Holistic Assessment

Motivation: The PR addresses a real correctness issue where EnumGcRefsX86 could report uninitialized GC slots for non-interruptible aborted frames, and improves debug validation in GC promote functions. These are genuine bugs that can cause GC heap corruption or crashes.

Approach: The changes are minimal and well-targeted — reordering the DEBUG_DestroyedHandleValue check before is_in_find_object_range (race condition correctness), adding debug-only Validate() calls for early diagnostics, and porting the existing gcinfodecoder.cpp logic to the x86 EnumGcRefsX86 path. The approach mirrors existing patterns already present in the codebase.

Summary: ✅ LGTM. The changes are correct, minimal, and consistent with established codebase patterns. All three GC promote functions (GCHeap::Promote, background_promote, background_promote_callback) are now consistent. The x86 fix is a straightforward port of logic already proven in gcinfodecoder.cpp (lines 808-812).

---

Detailed Findings

✅ Correctness — DEBUG_DestroyedHandleValue reordering (interface.cpp, background.cpp)

Moving the DEBUG_DestroyedHandleValue check before is_in_find_object_range is correct. During concurrent scanning, a handle can be destroyed (set to the sentinel value) while the GC is scanning. The destroyed sentinel value is not a valid heap address, so is_in_find_object_range would return false, but without the reordering, the new Validate() call (added in the !is_in_find_object_range path) could attempt to validate the sentinel value as an object, which would crash. Checking the sentinel first avoids this.

✅ Correctness — Debug validation in early-return path

Adding ((CObjectHeader*)o)->Validate() under _DEBUG for non-null, non-interior objects that fall outside the GC range is a sound diagnostic addition. The guards (o != NULL and !(flags & GC_CALL_INTERIOR)) are appropriate — null pointers obviously cannot be validated, and interior pointers do not point to object headers.

✅ Correctness — EnumGcRefsX86 aborted frame fix (gc_unwind_x86.inl)

The added condition ((flags & ExecutionAborted) && !info.interruptible) exactly matches the logic in gcinfodecoder.cpp at lines 808-812 (which skips reporting when countIntersections == 0 && executionAborted — meaning the frame is not fully interruptible). For non-interruptible methods where execution is aborted, GC slots may not be initialized at the current offset, so reporting them could cause the GC to trace garbage values. The existing _ASSERTE(flags & ExecutionAborted) at line 3103 already documents that this early-return path is only expected for aborted frames — the new condition simply widens the gate to include the non-interruptible aborted case alongside prolog/epilog.

✅ Test Priority — test119403.csproj

Changing priority from 0 to 1 follows the convention: "New JIT regression tests should typically be CLRTestPriority 1." The test was temporarily set to 0 earlier in the PR branch for verification, then correctly restored to 1 for merge.

✅ Consistency — All three promote functions aligned

GCHeap::Promote, background_promote, and background_promote_callback now all follow the same pattern: check DEBUG_DestroyedHandleValue → check is_in_find_object_range (with debug validation) → proceed. This eliminates an inconsistency that existed before this PR.

Generated by Code Review for issue #127595 · ◷

[Copilot]

Pull request overview

This PR targets a win-x86 GC root reporting hole by (1) aligning the x86 GCInfo unwinder behavior with the earlier fix in #119403 and (2) strengthening debug-time GC promotion validation so this class of corruption becomes easier to reproduce and diagnose deterministically.

Changes:

• Update x86 GC root enumeration to skip reporting for non-interruptible methods when execution has been aborted (in addition to prolog/epilog cases).
• Add _DEBUG-only validation in GCHeap::Promote and background promotion paths when an object reference falls outside the find_object bookkeeping range.

Reviewed changes

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

File	Description
src/coreclr/vm/gc_unwind_x86.inl	Extends the “skip reporting” condition to cover aborted + non-interruptible methods, matching the intended semantics from the earlier fix.
src/coreclr/gc/interface.cpp	Moves destroyed-handle sentinel check earlier and adds debug validation when the ref is outside is_in_find_object_range.
src/coreclr/gc/background.cpp	Adds similar debug validation in background promote paths (including callback).

[mangod9]

Btw, there is a libraries x86 failure but doesnt look like that is related to this change. The auto analysis is below. @VSadov surprised that ThreadAbort is being throw here?

!m_RedirectContextInUse Assert on win-x86 — Analysis

Assert

Assert failure (Thread 0x590): !m_RedirectContextInUse
File: src/coreclr/vm/threads.h:3104
Image: dotnet.exe (x86 Checked)
Exit code: -1073740286 (STATUS_FAIL_FAST_EXCEPTION)

Fires in Thread::MarkRedirectContextInUse when the runtime attempts to redirect (hijack) a thread whose redirect context is already occupied from a prior redirect.

Thread State at Crash

Thread	OSID	GC Mode	Role	Activity
11	0x590	Cooperative	Crash thread	Inside 1st redirect → HandleThreadAbort → constructing ThreadAbortException → 2nd redirect fires assert
12	0xccc	Cooperative	GC thread	Running GCExtendedTests.GetTotalAllocatedBytes() → GC.GetTotalAllocatedBytesPrecise() → triggered GC
6	0x101c	Preemptive	Tiered compilation	JIT compiling, blocked in WaitUntilGCComplete
5	0x1910	Preemptive	Finalizer	Waiting for finalizer event
0	0x1ad8	Preemptive	Main	xunit runner waiting

Full Stack — Thread 11 (OSID 0x590) — Native + Managed

Shows two nested redirects — the second one fires the assert:

=== 2nd redirect (fires the assert) ===
coreclr!FailFastOnAssert
coreclr!_DbgBreakCheck
coreclr!_DbgBreakCheckNoThrow
coreclr!DbgAssertDialog
coreclr!Thread::MarkRedirectContextInUse ← 2nd redirect: context already in use!
coreclr!Thread::RedirectedHandledJITCase ← 2nd redirect handler
coreclr!Thread::RedirectedHandledJITCaseForGCThreadControl
coreclr!RedirectedHandledJITCaseForGCThreadControl_Stub ← 2nd GC hijack entry point
=== Managed code running inside 1st redirect (ThreadAbortException construction) ===
System.IO.BinaryReader.ReadByte()
System.IO.BinaryReader.Read7BitEncodedInt()
System.Resources.ResourceReader.LoadString(Int32)
System.Resources.RuntimeResourceSet.ReadValue(...)
System.Resources.RuntimeResourceSet.GetObject(...)
System.Resources.RuntimeResourceSet.GetString(...)
System.Resources.ResourceManager.GetString(...)
System.SR.InternalGetResourceString(String)
System.SR.GetResourceString(String)
System.SR.get_Arg_SystemException()
System.SystemException..ctor()
System.Threading.ThreadAbortException..ctor()
System.Runtime.CompilerServices.RuntimeHelpers.CallDefaultConstructor(...)
=== 1st redirect processing the pending ThreadAbort ===
coreclr!CallDefaultConstructor
coreclr!EEException::CreateThrowable
coreclr!CLRException::GetThrowable
coreclr!CLRException::GetThrowableFromException
coreclr!Thread::HandleThreadAbort ← processes pending TS_AbortRequested
coreclr!Thread::RestoreContextSimulated
coreclr!Thread::RedirectedHandledJITCase ← 1st redirect handler
coreclr!Thread::RedirectedHandledJITCaseForGCThreadControl
coreclr!RedirectedHandledJITCaseForGCThreadControl_Stub ← 1st GC hijack entry point
=== Original managed code when 1st redirect hijacked the thread ===
[InlinedCallFrame] System.RuntimeMethodHandle.InvokeMethod(...)
System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object, IntPtr*)
System.Reflection.RuntimeMethodInfo.Invoke(...)
System.Reflection.MethodBase.Invoke(Object, Object[])
Xunit.Sdk.TestInvoker`1.CallTestMethod(Object)
Xunit.Sdk.TestInvoker`1+<>c__DisplayClass47_0.<<InvokeTestMethodAsync>b__1>d.MoveNext()
... (xunit async test invocation pipeline) ...
Xunit.Sdk.XunitTestCase.RunAsync(IMessageSink, IMessageBus, Object[], ExceptionAggregator, CancellationTokenSource)
Microsoft.DotNet.XUnitExtensions.SkippedFactTestCase.<>n__0(...)
Microsoft.DotNet.XUnitExtensions.SkippedFactTestCase+<RunAsync>d__2.MoveNext()
... (xunit async test runner pipeline) ...
Xunit.Sdk.XunitTestAssemblyRunner+<RunTestCollectionAsync>d__18.MoveNext()
System.Threading.ExecutionContext.RunInternal(...)
... (async continuations) ...
System.Threading.Tasks.Task+CancellationPromise`1[[Boolean]].ITaskCompletionAction.Invoke(Task)
System.Threading.Tasks.CompletionActionInvoker.Execute()
System.Threading.ThreadPoolWorkQueue.Dispatch()
System.Threading.PortableThreadPool+WorkerThread.WorkerDoWork(PortableThreadPool)
System.Threading.PortableThreadPool+WorkerThread.WorkerThreadStart()
System.Threading.Thread.StartCallback(Thread*)

GC Thread Stack (Thread 12, OSID 0xccc)

System.GC.GetTotalAllocatedBytesPrecise() ← triggers GC
System.GC.GetTotalAllocatedBytes(Boolean)
System.Tests.GCExtendedTests.GetTotalAllocatedBytes() ← the test
... (xunit async pipeline via reflection invoke) ...
System.Threading.PortableThreadPool+WorkerThread.WorkerDoWork(PortableThreadPool)
System.Threading.PortableThreadPool+WorkerThread.WorkerThreadStart()
System.Threading.Thread.StartCallback(Thread*)

Sequence of Events

1. Thread 12 runs GCExtendedTests.GetTotalAllocatedBytes() → GC.GetTotalAllocatedBytesPrecise() → triggers a GC collection
2. GC suspension begins. Thread 11 is in Cooperative mode running a test via SkippedFactTestCase.RunAsync. The runtime redirects (hijacks) Thread 11 via RedirectedHandledJITCaseForGCThreadControl_Stub (1st redirect)
3. Inside RedirectedHandledJITCase → RestoreContextSimulated, the runtime calls Thread::HandleThreadAbort — a pending thread abort (TS_AbortRequested) was set on Thread 11. The source of the abort is not captured in the dump. On modern .NET (6+), Thread.Abort() throws PlatformNotSupportedException; thread aborts can only be initiated by the runtime internally or via ControlledExecution.Run
4. HandleThreadAbort → CreateThrowable → CallDefaultConstructor for ThreadAbortException — this runs managed code to construct the exception (loading the resource string "Arg_SystemException" via SR.GetResourceString → ResourceReader.LoadString → BinaryReader.ReadByte)
5. The managed code runs in Cooperative mode, making Thread 11 eligible for GC hijacking again
6. A second GC triggers (or GC suspension retries). The runtime attempts to redirect Thread 11 again via RedirectedHandledJITCaseForGCThreadControl_Stub (2nd redirect)
7. MarkRedirectContextInUse finds m_RedirectContextInUse == true (still held from step 2) → assert fires

Key Finding

There is no separate cancellation thread visible in the dump. The ThreadAbort was a pending flag (TS_AbortRequested) already set on Thread 11. The source of the abort is not captured in the dump — on modern .NET, thread aborts can come from ControlledExecution.Run or other runtime-internal mechanisms. Thread::HandleThreadAbort processes this flag as part of the 1st redirect handler's normal flow (RestoreContextSimulated → HandleThreadAbort). The bug is that the 1st redirect handler runs managed code (exception construction) while still holding the redirect context, creating a window where a 2nd redirect can be attempted.

Root Cause

A double-redirect race condition on x86: the runtime hijacks a thread for GC suspension, processes a pending ThreadAbort inside the redirect handler, and the ThreadAbortException constructor runs managed code (resource string loading) in Cooperative mode. This makes the thread eligible for another GC redirect while the first redirect context is still held.

Why This Hasn't Been Seen Before

• Libraries Test Run checked coreclr windows x86 Debug only runs on PR builds, not main rolling builds
• Main rolling builds run Libraries Test Run checked coreclr windows x86 **Release** — Release libraries are faster, narrowing the timing window
• Debug libraries' slower execution (resource loading, extra checks) widens the race window between the first redirect and the second attempt
• The m_RedirectContextInUse assert only fires in Checked builds

Relation to PR #127595

Not related. The gc_unwind_x86.inl fix changes GC reporting logic inside EnumGcRefsX86. The Validate() change in interface.cpp adds a Debug assert in Promote. Neither affects thread redirect/hijack logic. This assert passed on the previous build of the same PR (build 1403154), confirming it's intermittent.

[jkotas]

• Mirrors the fix from Unconditionally skip GC reporting for non-interruptible aborted methods #119403 in x86 GCInfo decoder
• Improve GCHeap::Promote debug validation to repro this class of issues deterministically

Fixes#127581

[dotnet-policy-service]

Tagging subscribers to this area: @JulieLeeMSFT, @dotnet/gc
See info in area-owners.md if you want to be subscribed.

[Copilot]

Pull request overview

This PR aims to strengthen debug-time validation in GCHeap::Promote by validating candidate object pointers even on the early-return path when the pointer is outside the find_object bookkeeping range.

Changes:

• Add a _DEBUG-only CObjectHeader::Validate() call when !gc_heap::is_in_find_object_range(o) and the root is not an interior pointer.

[jkotas]

@copilot Change the priority for test119403.cs to 0 temporarily to make the test exercised in the CI

[Copilot]

@copilot Change the priority for test119403.cs to 0 temporarily to make the test exercised in the CI

Done in bfa4fd1. Changed CLRTestPriority from 1 to 0 in test119403.csproj.

[jkotas]

gc_heap::background_promote (background.cpp:547) and gc_heap::background_promote_callback

@copilot Address feeback

[Copilot]

gc_heap::background_promote (background.cpp:547) and gc_heap::background_promote_callback

@copilot Address feeback

Done in b05e42d. Applied the same fix to both background_promote and background_promote_callback: moved the DEBUG_DestroyedHandleValue check before is_in_find_object_range, and added Validate() in the early-return path (guarded by _DEBUG and !GC_CALL_INTERIOR).

[jkotas]

@copilot Implement the fix mentioned in #127581 (comment)

[Copilot]

@copilot Implement the fix mentioned in #127581 (comment)

Done in f66c09c. Added the equivalent of the gcinfodecoder.cpp fix to EnumGcRefsX86 in gc_unwind_x86.inl: when a partially-interruptible method is aborted (exception in flight) and the current offset is not within an interruptible range, skip all GC reporting. Untracked locals may not be initialized at the faulting offset, and since execution won't resume there, there's no need to report them.

[Copilot]

Pull request overview

This PR addresses a class of x86 GC hole issues (tracked in #127581) by aligning x86 GC reference enumeration behavior with the earlier fix from #119403 and by strengthening debug-time validation in GCHeap::Promote paths so invalid/out-of-range object references fail deterministically in debug builds.

Changes:

• Update x86 GCInfo/stack-walk enumeration to skip reporting entirely for execution-aborted, non-interruptible methods (where slots may not be initialized).
• Add additional _DEBUG validation when Promote/background promotion sees a non-null, non-interior pointer that is outside the GC “find_object” bookkeeping range.
• Adjust the regression test project priority to make the scenario easier to exercise in standard test runs.

Reviewed changes

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

File	Description
src/tests/Regressions/coreclr/GitHub_119403/test119403.csproj	Changes test priority to run as a default-priority regression.
src/coreclr/vm/gc_unwind_x86.inl	Skips GC reporting for execution-aborted non-interruptible methods on x86 to avoid reporting uninitialized GC slots.
src/coreclr/gc/interface.cpp	Adds debug validation in GCHeap::Promote when an out-of-range non-interior reference is encountered.
src/coreclr/gc/background.cpp	Adds the same debug validation behavior to background GC promotion paths.

[mangod9]

The fix is working:

CI Validation of the Fix

Build 1403154 — ❌ Failed (without x86 fix)

• Commit: bc54dbf (Apr 30 05:32 UTC)
• Contains: Validate change in interface.cpp + test promoted to Pri0 — but NOT the gc_unwind_x86.inl fix
• Result: coreclr Pri0 Runtime Tests Run windows x86 checked → failed
• Failure: Regressions workitem in Helix job ab237a9f-5703-40e6-9caa-e9eba9d566d0 Assert: !"Detected use of a
  corrupted OBJECTREF. Possible GC hole."
  Object::Validate + 0xA9
  → WKS::GCHeap::Promote + 0x169 ← new Validate() catches the bad ref
  → GcEnumObject
  → EnumGcRefsX86 ← reports untracked slot for aborted frame
  → SubProblem + 5 ← fault offset (before local 's' initialized)

Build 1403719 — ✅ Passed (with x86 fix)

• Commit: 9a9b245 → includes f66c09c "Fix EnumGcRefsX86 to skip reporting for non-interruptible aborted frames" (Apr 30 12:03 UTC)
• Contains: Validate change + test Pri0 + gc_unwind_x86.inl fix
• Result: coreclr Pri0 Runtime Tests Run windows x86 checked → succeeded (44m 3s)
• EnumGcRefsX86 now returns early for the aborted non-interruptible frame, so Promote/Validate is never called with the uninitialized slot

[github-actions]

🤖 Copilot Code Review — PR #127595

Note

This review was generated by GitHub Copilot.

Holistic Assessment

Motivation: The PR addresses a real correctness issue where EnumGcRefsX86 could report uninitialized GC slots for non-interruptible aborted frames, and improves debug validation in GC promote functions. These are genuine bugs that can cause GC heap corruption or crashes.

Approach: The changes are minimal and well-targeted — reordering the DEBUG_DestroyedHandleValue check before is_in_find_object_range (race condition correctness), adding debug-only Validate() calls for early diagnostics, and porting the existing gcinfodecoder.cpp logic to the x86 EnumGcRefsX86 path. The approach mirrors existing patterns already present in the codebase.

Summary: ✅ LGTM. The changes are correct, minimal, and consistent with established codebase patterns. All three GC promote functions (GCHeap::Promote, background_promote, background_promote_callback) are now consistent. The x86 fix is a straightforward port of logic already proven in gcinfodecoder.cpp (lines 808-812).

---

Detailed Findings

✅ Correctness — DEBUG_DestroyedHandleValue reordering (interface.cpp, background.cpp)

Moving the DEBUG_DestroyedHandleValue check before is_in_find_object_range is correct. During concurrent scanning, a handle can be destroyed (set to the sentinel value) while the GC is scanning. The destroyed sentinel value is not a valid heap address, so is_in_find_object_range would return false, but without the reordering, the new Validate() call (added in the !is_in_find_object_range path) could attempt to validate the sentinel value as an object, which would crash. Checking the sentinel first avoids this.

✅ Correctness — Debug validation in early-return path

Adding ((CObjectHeader*)o)->Validate() under _DEBUG for non-null, non-interior objects that fall outside the GC range is a sound diagnostic addition. The guards (o != NULL and !(flags & GC_CALL_INTERIOR)) are appropriate — null pointers obviously cannot be validated, and interior pointers do not point to object headers.

✅ Correctness — EnumGcRefsX86 aborted frame fix (gc_unwind_x86.inl)

The added condition ((flags & ExecutionAborted) && !info.interruptible) exactly matches the logic in gcinfodecoder.cpp at lines 808-812 (which skips reporting when countIntersections == 0 && executionAborted — meaning the frame is not fully interruptible). For non-interruptible methods where execution is aborted, GC slots may not be initialized at the current offset, so reporting them could cause the GC to trace garbage values. The existing _ASSERTE(flags & ExecutionAborted) at line 3103 already documents that this early-return path is only expected for aborted frames — the new condition simply widens the gate to include the non-interruptible aborted case alongside prolog/epilog.

✅ Test Priority — test119403.csproj

Changing priority from 0 to 1 follows the convention: "New JIT regression tests should typically be CLRTestPriority 1." The test was temporarily set to 0 earlier in the PR branch for verification, then correctly restored to 1 for merge.

✅ Consistency — All three promote functions aligned

GCHeap::Promote, background_promote, and background_promote_callback now all follow the same pattern: check DEBUG_DestroyedHandleValue → check is_in_find_object_range (with debug validation) → proceed. This eliminates an inconsistency that existed before this PR.

Generated by Code Review for issue #127595 · ◷

[Copilot]

Pull request overview

This PR targets a win-x86 GC root reporting hole by (1) aligning the x86 GCInfo unwinder behavior with the earlier fix in #119403 and (2) strengthening debug-time GC promotion validation so this class of corruption becomes easier to reproduce and diagnose deterministically.

Changes:

• Update x86 GC root enumeration to skip reporting for non-interruptible methods when execution has been aborted (in addition to prolog/epilog cases).
• Add _DEBUG-only validation in GCHeap::Promote and background promotion paths when an object reference falls outside the find_object bookkeeping range.

Reviewed changes

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

File	Description
src/coreclr/vm/gc_unwind_x86.inl	Extends the “skip reporting” condition to cover aborted + non-interruptible methods, matching the intended semantics from the earlier fix.
src/coreclr/gc/interface.cpp	Moves destroyed-handle sentinel check earlier and adds debug validation when the ref is outside is_in_find_object_range.
src/coreclr/gc/background.cpp	Adds similar debug validation in background promote paths (including callback).

[mangod9]

Btw, there is a libraries x86 failure but doesnt look like that is related to this change. The auto analysis is below. @VSadov surprised that ThreadAbort is being throw here?

!m_RedirectContextInUse Assert on win-x86 — Analysis

Assert

Assert failure (Thread 0x590): !m_RedirectContextInUse
File: src/coreclr/vm/threads.h:3104
Image: dotnet.exe (x86 Checked)
Exit code: -1073740286 (STATUS_FAIL_FAST_EXCEPTION)

Fires in Thread::MarkRedirectContextInUse when the runtime attempts to redirect (hijack) a thread whose redirect context is already occupied from a prior redirect.

Thread State at Crash

Thread	OSID	GC Mode	Role	Activity
11	0x590	Cooperative	Crash thread	Inside 1st redirect → HandleThreadAbort → constructing ThreadAbortException → 2nd redirect fires assert
12	0xccc	Cooperative	GC thread	Running GCExtendedTests.GetTotalAllocatedBytes() → GC.GetTotalAllocatedBytesPrecise() → triggered GC
6	0x101c	Preemptive	Tiered compilation	JIT compiling, blocked in WaitUntilGCComplete
5	0x1910	Preemptive	Finalizer	Waiting for finalizer event
0	0x1ad8	Preemptive	Main	xunit runner waiting

Full Stack — Thread 11 (OSID 0x590) — Native + Managed

Shows two nested redirects — the second one fires the assert:

=== 2nd redirect (fires the assert) ===
coreclr!FailFastOnAssert
coreclr!_DbgBreakCheck
coreclr!_DbgBreakCheckNoThrow
coreclr!DbgAssertDialog
coreclr!Thread::MarkRedirectContextInUse ← 2nd redirect: context already in use!
coreclr!Thread::RedirectedHandledJITCase ← 2nd redirect handler
coreclr!Thread::RedirectedHandledJITCaseForGCThreadControl
coreclr!RedirectedHandledJITCaseForGCThreadControl_Stub ← 2nd GC hijack entry point
=== Managed code running inside 1st redirect (ThreadAbortException construction) ===
System.IO.BinaryReader.ReadByte()
System.IO.BinaryReader.Read7BitEncodedInt()
System.Resources.ResourceReader.LoadString(Int32)
System.Resources.RuntimeResourceSet.ReadValue(...)
System.Resources.RuntimeResourceSet.GetObject(...)
System.Resources.RuntimeResourceSet.GetString(...)
System.Resources.ResourceManager.GetString(...)
System.SR.InternalGetResourceString(String)
System.SR.GetResourceString(String)
System.SR.get_Arg_SystemException()
System.SystemException..ctor()
System.Threading.ThreadAbortException..ctor()
System.Runtime.CompilerServices.RuntimeHelpers.CallDefaultConstructor(...)
=== 1st redirect processing the pending ThreadAbort ===
coreclr!CallDefaultConstructor
coreclr!EEException::CreateThrowable
coreclr!CLRException::GetThrowable
coreclr!CLRException::GetThrowableFromException
coreclr!Thread::HandleThreadAbort ← processes pending TS_AbortRequested
coreclr!Thread::RestoreContextSimulated
coreclr!Thread::RedirectedHandledJITCase ← 1st redirect handler
coreclr!Thread::RedirectedHandledJITCaseForGCThreadControl
coreclr!RedirectedHandledJITCaseForGCThreadControl_Stub ← 1st GC hijack entry point
=== Original managed code when 1st redirect hijacked the thread ===
[InlinedCallFrame] System.RuntimeMethodHandle.InvokeMethod(...)
System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object, IntPtr*)
System.Reflection.RuntimeMethodInfo.Invoke(...)
System.Reflection.MethodBase.Invoke(Object, Object[])
Xunit.Sdk.TestInvoker`1.CallTestMethod(Object)
Xunit.Sdk.TestInvoker`1+<>c__DisplayClass47_0.<<InvokeTestMethodAsync>b__1>d.MoveNext()
... (xunit async test invocation pipeline) ...
Xunit.Sdk.XunitTestCase.RunAsync(IMessageSink, IMessageBus, Object[], ExceptionAggregator, CancellationTokenSource)
Microsoft.DotNet.XUnitExtensions.SkippedFactTestCase.<>n__0(...)
Microsoft.DotNet.XUnitExtensions.SkippedFactTestCase+<RunAsync>d__2.MoveNext()
... (xunit async test runner pipeline) ...
Xunit.Sdk.XunitTestAssemblyRunner+<RunTestCollectionAsync>d__18.MoveNext()
System.Threading.ExecutionContext.RunInternal(...)
... (async continuations) ...
System.Threading.Tasks.Task+CancellationPromise`1[[Boolean]].ITaskCompletionAction.Invoke(Task)
System.Threading.Tasks.CompletionActionInvoker.Execute()
System.Threading.ThreadPoolWorkQueue.Dispatch()
System.Threading.PortableThreadPool+WorkerThread.WorkerDoWork(PortableThreadPool)
System.Threading.PortableThreadPool+WorkerThread.WorkerThreadStart()
System.Threading.Thread.StartCallback(Thread*)

GC Thread Stack (Thread 12, OSID 0xccc)

System.GC.GetTotalAllocatedBytesPrecise() ← triggers GC
System.GC.GetTotalAllocatedBytes(Boolean)
System.Tests.GCExtendedTests.GetTotalAllocatedBytes() ← the test
... (xunit async pipeline via reflection invoke) ...
System.Threading.PortableThreadPool+WorkerThread.WorkerDoWork(PortableThreadPool)
System.Threading.PortableThreadPool+WorkerThread.WorkerThreadStart()
System.Threading.Thread.StartCallback(Thread*)

Sequence of Events

1. Thread 12 runs GCExtendedTests.GetTotalAllocatedBytes() → GC.GetTotalAllocatedBytesPrecise() → triggers a GC collection
2. GC suspension begins. Thread 11 is in Cooperative mode running a test via SkippedFactTestCase.RunAsync. The runtime redirects (hijacks) Thread 11 via RedirectedHandledJITCaseForGCThreadControl_Stub (1st redirect)
3. Inside RedirectedHandledJITCase → RestoreContextSimulated, the runtime calls Thread::HandleThreadAbort — a pending thread abort (TS_AbortRequested) was set on Thread 11. The source of the abort is not captured in the dump. On modern .NET (6+), Thread.Abort() throws PlatformNotSupportedException; thread aborts can only be initiated by the runtime internally or via ControlledExecution.Run
4. HandleThreadAbort → CreateThrowable → CallDefaultConstructor for ThreadAbortException — this runs managed code to construct the exception (loading the resource string "Arg_SystemException" via SR.GetResourceString → ResourceReader.LoadString → BinaryReader.ReadByte)
5. The managed code runs in Cooperative mode, making Thread 11 eligible for GC hijacking again
6. A second GC triggers (or GC suspension retries). The runtime attempts to redirect Thread 11 again via RedirectedHandledJITCaseForGCThreadControl_Stub (2nd redirect)
7. MarkRedirectContextInUse finds m_RedirectContextInUse == true (still held from step 2) → assert fires

Key Finding

There is no separate cancellation thread visible in the dump. The ThreadAbort was a pending flag (TS_AbortRequested) already set on Thread 11. The source of the abort is not captured in the dump — on modern .NET, thread aborts can come from ControlledExecution.Run or other runtime-internal mechanisms. Thread::HandleThreadAbort processes this flag as part of the 1st redirect handler's normal flow (RestoreContextSimulated → HandleThreadAbort). The bug is that the 1st redirect handler runs managed code (exception construction) while still holding the redirect context, creating a window where a 2nd redirect can be attempted.

Root Cause

A double-redirect race condition on x86: the runtime hijacks a thread for GC suspension, processes a pending ThreadAbort inside the redirect handler, and the ThreadAbortException constructor runs managed code (resource string loading) in Cooperative mode. This makes the thread eligible for another GC redirect while the first redirect context is still held.

Why This Hasn't Been Seen Before

• Libraries Test Run checked coreclr windows x86 Debug only runs on PR builds, not main rolling builds
• Main rolling builds run Libraries Test Run checked coreclr windows x86 **Release** — Release libraries are faster, narrowing the timing window
• Debug libraries' slower execution (resource loading, extra checks) widens the race window between the first redirect and the second attempt
• The m_RedirectContextInUse assert only fires in Checked builds

Relation to PR #127595

Not related. The gc_unwind_x86.inl fix changes GC reporting logic inside EnumGcRefsX86. The Validate() change in interface.cpp adds a Debug assert in Promote. Neither affects thread redirect/hijack logic. This assert passed on the previous build of the same PR (build 1403154), confirming it's intermittent.

[jkotas]

• Mirrors the fix from Unconditionally skip GC reporting for non-interruptible aborted methods #119403 in x86 GCInfo decoder
• Improve GCHeap::Promote debug validation to repro this class of issues deterministically

Fixes#127581

[dotnet-policy-service]

Tagging subscribers to this area: @JulieLeeMSFT, @dotnet/gc
See info in area-owners.md if you want to be subscribed.

[Copilot]

Pull request overview

This PR aims to strengthen debug-time validation in GCHeap::Promote by validating candidate object pointers even on the early-return path when the pointer is outside the find_object bookkeeping range.

Changes:

• Add a _DEBUG-only CObjectHeader::Validate() call when !gc_heap::is_in_find_object_range(o) and the root is not an interior pointer.

[jkotas]

@copilot Change the priority for test119403.cs to 0 temporarily to make the test exercised in the CI

[Copilot]

@copilot Change the priority for test119403.cs to 0 temporarily to make the test exercised in the CI

Done in bfa4fd1. Changed CLRTestPriority from 1 to 0 in test119403.csproj.

[jkotas]

gc_heap::background_promote (background.cpp:547) and gc_heap::background_promote_callback

@copilot Address feeback

[Copilot]

gc_heap::background_promote (background.cpp:547) and gc_heap::background_promote_callback

@copilot Address feeback

Done in b05e42d. Applied the same fix to both background_promote and background_promote_callback: moved the DEBUG_DestroyedHandleValue check before is_in_find_object_range, and added Validate() in the early-return path (guarded by _DEBUG and !GC_CALL_INTERIOR).

[jkotas]

@copilot Implement the fix mentioned in #127581 (comment)

[Copilot]

@copilot Implement the fix mentioned in #127581 (comment)

Done in f66c09c. Added the equivalent of the gcinfodecoder.cpp fix to EnumGcRefsX86 in gc_unwind_x86.inl: when a partially-interruptible method is aborted (exception in flight) and the current offset is not within an interruptible range, skip all GC reporting. Untracked locals may not be initialized at the faulting offset, and since execution won't resume there, there's no need to report them.

[Copilot]

Pull request overview

This PR addresses a class of x86 GC hole issues (tracked in #127581) by aligning x86 GC reference enumeration behavior with the earlier fix from #119403 and by strengthening debug-time validation in GCHeap::Promote paths so invalid/out-of-range object references fail deterministically in debug builds.

Changes:

• Update x86 GCInfo/stack-walk enumeration to skip reporting entirely for execution-aborted, non-interruptible methods (where slots may not be initialized).
• Add additional _DEBUG validation when Promote/background promotion sees a non-null, non-interior pointer that is outside the GC “find_object” bookkeeping range.
• Adjust the regression test project priority to make the scenario easier to exercise in standard test runs.

Reviewed changes

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

File	Description
src/tests/Regressions/coreclr/GitHub_119403/test119403.csproj	Changes test priority to run as a default-priority regression.
src/coreclr/vm/gc_unwind_x86.inl	Skips GC reporting for execution-aborted non-interruptible methods on x86 to avoid reporting uninitialized GC slots.
src/coreclr/gc/interface.cpp	Adds debug validation in GCHeap::Promote when an out-of-range non-interior reference is encountered.
src/coreclr/gc/background.cpp	Adds the same debug validation behavior to background GC promotion paths.

[mangod9]

The fix is working:

CI Validation of the Fix

Build 1403154 — ❌ Failed (without x86 fix)

• Commit: bc54dbf (Apr 30 05:32 UTC)
• Contains: Validate change in interface.cpp + test promoted to Pri0 — but NOT the gc_unwind_x86.inl fix
• Result: coreclr Pri0 Runtime Tests Run windows x86 checked → failed
• Failure: Regressions workitem in Helix job ab237a9f-5703-40e6-9caa-e9eba9d566d0 Assert: !"Detected use of a
  corrupted OBJECTREF. Possible GC hole."
  Object::Validate + 0xA9
  → WKS::GCHeap::Promote + 0x169 ← new Validate() catches the bad ref
  → GcEnumObject
  → EnumGcRefsX86 ← reports untracked slot for aborted frame
  → SubProblem + 5 ← fault offset (before local 's' initialized)

Build 1403719 — ✅ Passed (with x86 fix)

• Commit: 9a9b245 → includes f66c09c "Fix EnumGcRefsX86 to skip reporting for non-interruptible aborted frames" (Apr 30 12:03 UTC)
• Contains: Validate change + test Pri0 + gc_unwind_x86.inl fix
• Result: coreclr Pri0 Runtime Tests Run windows x86 checked → succeeded (44m 3s)
• EnumGcRefsX86 now returns early for the aborted non-interruptible frame, so Promote/Validate is never called with the uninitialized slot

[github-actions]

🤖 Copilot Code Review — PR #127595

Note

This review was generated by GitHub Copilot.

Holistic Assessment

Motivation: The PR addresses a real correctness issue where EnumGcRefsX86 could report uninitialized GC slots for non-interruptible aborted frames, and improves debug validation in GC promote functions. These are genuine bugs that can cause GC heap corruption or crashes.

Approach: The changes are minimal and well-targeted — reordering the DEBUG_DestroyedHandleValue check before is_in_find_object_range (race condition correctness), adding debug-only Validate() calls for early diagnostics, and porting the existing gcinfodecoder.cpp logic to the x86 EnumGcRefsX86 path. The approach mirrors existing patterns already present in the codebase.

Summary: ✅ LGTM. The changes are correct, minimal, and consistent with established codebase patterns. All three GC promote functions (GCHeap::Promote, background_promote, background_promote_callback) are now consistent. The x86 fix is a straightforward port of logic already proven in gcinfodecoder.cpp (lines 808-812).

---

Detailed Findings

✅ Correctness — DEBUG_DestroyedHandleValue reordering (interface.cpp, background.cpp)

Moving the DEBUG_DestroyedHandleValue check before is_in_find_object_range is correct. During concurrent scanning, a handle can be destroyed (set to the sentinel value) while the GC is scanning. The destroyed sentinel value is not a valid heap address, so is_in_find_object_range would return false, but without the reordering, the new Validate() call (added in the !is_in_find_object_range path) could attempt to validate the sentinel value as an object, which would crash. Checking the sentinel first avoids this.

✅ Correctness — Debug validation in early-return path

Adding ((CObjectHeader*)o)->Validate() under _DEBUG for non-null, non-interior objects that fall outside the GC range is a sound diagnostic addition. The guards (o != NULL and !(flags & GC_CALL_INTERIOR)) are appropriate — null pointers obviously cannot be validated, and interior pointers do not point to object headers.

✅ Correctness — EnumGcRefsX86 aborted frame fix (gc_unwind_x86.inl)

The added condition ((flags & ExecutionAborted) && !info.interruptible) exactly matches the logic in gcinfodecoder.cpp at lines 808-812 (which skips reporting when countIntersections == 0 && executionAborted — meaning the frame is not fully interruptible). For non-interruptible methods where execution is aborted, GC slots may not be initialized at the current offset, so reporting them could cause the GC to trace garbage values. The existing _ASSERTE(flags & ExecutionAborted) at line 3103 already documents that this early-return path is only expected for aborted frames — the new condition simply widens the gate to include the non-interruptible aborted case alongside prolog/epilog.

✅ Test Priority — test119403.csproj

Changing priority from 0 to 1 follows the convention: "New JIT regression tests should typically be CLRTestPriority 1." The test was temporarily set to 0 earlier in the PR branch for verification, then correctly restored to 1 for merge.

✅ Consistency — All three promote functions aligned

GCHeap::Promote, background_promote, and background_promote_callback now all follow the same pattern: check DEBUG_DestroyedHandleValue → check is_in_find_object_range (with debug validation) → proceed. This eliminates an inconsistency that existed before this PR.

Generated by Code Review for issue #127595 · ◷

[Copilot]

Pull request overview

This PR targets a win-x86 GC root reporting hole by (1) aligning the x86 GCInfo unwinder behavior with the earlier fix in #119403 and (2) strengthening debug-time GC promotion validation so this class of corruption becomes easier to reproduce and diagnose deterministically.

Changes:

• Update x86 GC root enumeration to skip reporting for non-interruptible methods when execution has been aborted (in addition to prolog/epilog cases).
• Add _DEBUG-only validation in GCHeap::Promote and background promotion paths when an object reference falls outside the find_object bookkeeping range.

Reviewed changes

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

File	Description
src/coreclr/vm/gc_unwind_x86.inl	Extends the “skip reporting” condition to cover aborted + non-interruptible methods, matching the intended semantics from the earlier fix.
src/coreclr/gc/interface.cpp	Moves destroyed-handle sentinel check earlier and adds debug validation when the ref is outside is_in_find_object_range.
src/coreclr/gc/background.cpp	Adds similar debug validation in background promote paths (including callback).

[mangod9]

Btw, there is a libraries x86 failure but doesnt look like that is related to this change. The auto analysis is below. @VSadov surprised that ThreadAbort is being throw here?

!m_RedirectContextInUse Assert on win-x86 — Analysis

Assert

Assert failure (Thread 0x590): !m_RedirectContextInUse
File: src/coreclr/vm/threads.h:3104
Image: dotnet.exe (x86 Checked)
Exit code: -1073740286 (STATUS_FAIL_FAST_EXCEPTION)

Fires in Thread::MarkRedirectContextInUse when the runtime attempts to redirect (hijack) a thread whose redirect context is already occupied from a prior redirect.

Thread State at Crash

Thread	OSID	GC Mode	Role	Activity
11	0x590	Cooperative	Crash thread	Inside 1st redirect → HandleThreadAbort → constructing ThreadAbortException → 2nd redirect fires assert
12	0xccc	Cooperative	GC thread	Running GCExtendedTests.GetTotalAllocatedBytes() → GC.GetTotalAllocatedBytesPrecise() → triggered GC
6	0x101c	Preemptive	Tiered compilation	JIT compiling, blocked in WaitUntilGCComplete
5	0x1910	Preemptive	Finalizer	Waiting for finalizer event
0	0x1ad8	Preemptive	Main	xunit runner waiting

Full Stack — Thread 11 (OSID 0x590) — Native + Managed

Shows two nested redirects — the second one fires the assert:

=== 2nd redirect (fires the assert) ===
coreclr!FailFastOnAssert
coreclr!_DbgBreakCheck
coreclr!_DbgBreakCheckNoThrow
coreclr!DbgAssertDialog
coreclr!Thread::MarkRedirectContextInUse ← 2nd redirect: context already in use!
coreclr!Thread::RedirectedHandledJITCase ← 2nd redirect handler
coreclr!Thread::RedirectedHandledJITCaseForGCThreadControl
coreclr!RedirectedHandledJITCaseForGCThreadControl_Stub ← 2nd GC hijack entry point
=== Managed code running inside 1st redirect (ThreadAbortException construction) ===
System.IO.BinaryReader.ReadByte()
System.IO.BinaryReader.Read7BitEncodedInt()
System.Resources.ResourceReader.LoadString(Int32)
System.Resources.RuntimeResourceSet.ReadValue(...)
System.Resources.RuntimeResourceSet.GetObject(...)
System.Resources.RuntimeResourceSet.GetString(...)
System.Resources.ResourceManager.GetString(...)
System.SR.InternalGetResourceString(String)
System.SR.GetResourceString(String)
System.SR.get_Arg_SystemException()
System.SystemException..ctor()
System.Threading.ThreadAbortException..ctor()
System.Runtime.CompilerServices.RuntimeHelpers.CallDefaultConstructor(...)
=== 1st redirect processing the pending ThreadAbort ===
coreclr!CallDefaultConstructor
coreclr!EEException::CreateThrowable
coreclr!CLRException::GetThrowable
coreclr!CLRException::GetThrowableFromException
coreclr!Thread::HandleThreadAbort ← processes pending TS_AbortRequested
coreclr!Thread::RestoreContextSimulated
coreclr!Thread::RedirectedHandledJITCase ← 1st redirect handler
coreclr!Thread::RedirectedHandledJITCaseForGCThreadControl
coreclr!RedirectedHandledJITCaseForGCThreadControl_Stub ← 1st GC hijack entry point
=== Original managed code when 1st redirect hijacked the thread ===
[InlinedCallFrame] System.RuntimeMethodHandle.InvokeMethod(...)
System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object, IntPtr*)
System.Reflection.RuntimeMethodInfo.Invoke(...)
System.Reflection.MethodBase.Invoke(Object, Object[])
Xunit.Sdk.TestInvoker`1.CallTestMethod(Object)
Xunit.Sdk.TestInvoker`1+<>c__DisplayClass47_0.<<InvokeTestMethodAsync>b__1>d.MoveNext()
... (xunit async test invocation pipeline) ...
Xunit.Sdk.XunitTestCase.RunAsync(IMessageSink, IMessageBus, Object[], ExceptionAggregator, CancellationTokenSource)
Microsoft.DotNet.XUnitExtensions.SkippedFactTestCase.<>n__0(...)
Microsoft.DotNet.XUnitExtensions.SkippedFactTestCase+<RunAsync>d__2.MoveNext()
... (xunit async test runner pipeline) ...
Xunit.Sdk.XunitTestAssemblyRunner+<RunTestCollectionAsync>d__18.MoveNext()
System.Threading.ExecutionContext.RunInternal(...)
... (async continuations) ...
System.Threading.Tasks.Task+CancellationPromise`1[[Boolean]].ITaskCompletionAction.Invoke(Task)
System.Threading.Tasks.CompletionActionInvoker.Execute()
System.Threading.ThreadPoolWorkQueue.Dispatch()
System.Threading.PortableThreadPool+WorkerThread.WorkerDoWork(PortableThreadPool)
System.Threading.PortableThreadPool+WorkerThread.WorkerThreadStart()
System.Threading.Thread.StartCallback(Thread*)

GC Thread Stack (Thread 12, OSID 0xccc)

System.GC.GetTotalAllocatedBytesPrecise() ← triggers GC
System.GC.GetTotalAllocatedBytes(Boolean)
System.Tests.GCExtendedTests.GetTotalAllocatedBytes() ← the test
... (xunit async pipeline via reflection invoke) ...
System.Threading.PortableThreadPool+WorkerThread.WorkerDoWork(PortableThreadPool)
System.Threading.PortableThreadPool+WorkerThread.WorkerThreadStart()
System.Threading.Thread.StartCallback(Thread*)

Sequence of Events

1. Thread 12 runs GCExtendedTests.GetTotalAllocatedBytes() → GC.GetTotalAllocatedBytesPrecise() → triggers a GC collection
2. GC suspension begins. Thread 11 is in Cooperative mode running a test via SkippedFactTestCase.RunAsync. The runtime redirects (hijacks) Thread 11 via RedirectedHandledJITCaseForGCThreadControl_Stub (1st redirect)
3. Inside RedirectedHandledJITCase → RestoreContextSimulated, the runtime calls Thread::HandleThreadAbort — a pending thread abort (TS_AbortRequested) was set on Thread 11. The source of the abort is not captured in the dump. On modern .NET (6+), Thread.Abort() throws PlatformNotSupportedException; thread aborts can only be initiated by the runtime internally or via ControlledExecution.Run
4. HandleThreadAbort → CreateThrowable → CallDefaultConstructor for ThreadAbortException — this runs managed code to construct the exception (loading the resource string "Arg_SystemException" via SR.GetResourceString → ResourceReader.LoadString → BinaryReader.ReadByte)
5. The managed code runs in Cooperative mode, making Thread 11 eligible for GC hijacking again
6. A second GC triggers (or GC suspension retries). The runtime attempts to redirect Thread 11 again via RedirectedHandledJITCaseForGCThreadControl_Stub (2nd redirect)
7. MarkRedirectContextInUse finds m_RedirectContextInUse == true (still held from step 2) → assert fires

Key Finding

There is no separate cancellation thread visible in the dump. The ThreadAbort was a pending flag (TS_AbortRequested) already set on Thread 11. The source of the abort is not captured in the dump — on modern .NET, thread aborts can come from ControlledExecution.Run or other runtime-internal mechanisms. Thread::HandleThreadAbort processes this flag as part of the 1st redirect handler's normal flow (RestoreContextSimulated → HandleThreadAbort). The bug is that the 1st redirect handler runs managed code (exception construction) while still holding the redirect context, creating a window where a 2nd redirect can be attempted.

Root Cause

A double-redirect race condition on x86: the runtime hijacks a thread for GC suspension, processes a pending ThreadAbort inside the redirect handler, and the ThreadAbortException constructor runs managed code (resource string loading) in Cooperative mode. This makes the thread eligible for another GC redirect while the first redirect context is still held.

Why This Hasn't Been Seen Before

• Libraries Test Run checked coreclr windows x86 Debug only runs on PR builds, not main rolling builds
• Main rolling builds run Libraries Test Run checked coreclr windows x86 **Release** — Release libraries are faster, narrowing the timing window
• Debug libraries' slower execution (resource loading, extra checks) widens the race window between the first redirect and the second attempt
• The m_RedirectContextInUse assert only fires in Checked builds

Relation to PR #127595

Not related. The gc_unwind_x86.inl fix changes GC reporting logic inside EnumGcRefsX86. The Validate() change in interface.cpp adds a Debug assert in Promote. Neither affects thread redirect/hijack logic. This assert passed on the previous build of the same PR (build 1403154), confirming it's intermittent.

[jkotas]

• Mirrors the fix from Unconditionally skip GC reporting for non-interruptible aborted methods #119403 in x86 GCInfo decoder
• Improve GCHeap::Promote debug validation to repro this class of issues deterministically

Fixes#127581

[dotnet-policy-service]

Tagging subscribers to this area: @JulieLeeMSFT, @dotnet/gc
See info in area-owners.md if you want to be subscribed.

[Copilot]

Pull request overview

This PR aims to strengthen debug-time validation in GCHeap::Promote by validating candidate object pointers even on the early-return path when the pointer is outside the find_object bookkeeping range.

Changes:

• Add a _DEBUG-only CObjectHeader::Validate() call when !gc_heap::is_in_find_object_range(o) and the root is not an interior pointer.

[jkotas]

@copilot Change the priority for test119403.cs to 0 temporarily to make the test exercised in the CI

[Copilot]

@copilot Change the priority for test119403.cs to 0 temporarily to make the test exercised in the CI

Done in bfa4fd1. Changed CLRTestPriority from 1 to 0 in test119403.csproj.

[jkotas]

gc_heap::background_promote (background.cpp:547) and gc_heap::background_promote_callback

@copilot Address feeback

[Copilot]

gc_heap::background_promote (background.cpp:547) and gc_heap::background_promote_callback

@copilot Address feeback

Done in b05e42d. Applied the same fix to both background_promote and background_promote_callback: moved the DEBUG_DestroyedHandleValue check before is_in_find_object_range, and added Validate() in the early-return path (guarded by _DEBUG and !GC_CALL_INTERIOR).

[jkotas]

@copilot Implement the fix mentioned in #127581 (comment)

[Copilot]

@copilot Implement the fix mentioned in #127581 (comment)

Done in f66c09c. Added the equivalent of the gcinfodecoder.cpp fix to EnumGcRefsX86 in gc_unwind_x86.inl: when a partially-interruptible method is aborted (exception in flight) and the current offset is not within an interruptible range, skip all GC reporting. Untracked locals may not be initialized at the faulting offset, and since execution won't resume there, there's no need to report them.

[Copilot]

Pull request overview

This PR addresses a class of x86 GC hole issues (tracked in #127581) by aligning x86 GC reference enumeration behavior with the earlier fix from #119403 and by strengthening debug-time validation in GCHeap::Promote paths so invalid/out-of-range object references fail deterministically in debug builds.

Changes:

• Update x86 GCInfo/stack-walk enumeration to skip reporting entirely for execution-aborted, non-interruptible methods (where slots may not be initialized).
• Add additional _DEBUG validation when Promote/background promotion sees a non-null, non-interior pointer that is outside the GC “find_object” bookkeeping range.
• Adjust the regression test project priority to make the scenario easier to exercise in standard test runs.

Reviewed changes

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

File	Description
src/tests/Regressions/coreclr/GitHub_119403/test119403.csproj	Changes test priority to run as a default-priority regression.
src/coreclr/vm/gc_unwind_x86.inl	Skips GC reporting for execution-aborted non-interruptible methods on x86 to avoid reporting uninitialized GC slots.
src/coreclr/gc/interface.cpp	Adds debug validation in GCHeap::Promote when an out-of-range non-interior reference is encountered.
src/coreclr/gc/background.cpp	Adds the same debug validation behavior to background GC promotion paths.

[mangod9]

The fix is working:

CI Validation of the Fix

Build 1403154 — ❌ Failed (without x86 fix)

• Commit: bc54dbf (Apr 30 05:32 UTC)
• Contains: Validate change in interface.cpp + test promoted to Pri0 — but NOT the gc_unwind_x86.inl fix
• Result: coreclr Pri0 Runtime Tests Run windows x86 checked → failed
• Failure: Regressions workitem in Helix job ab237a9f-5703-40e6-9caa-e9eba9d566d0 Assert: !"Detected use of a
  corrupted OBJECTREF. Possible GC hole."
  Object::Validate + 0xA9
  → WKS::GCHeap::Promote + 0x169 ← new Validate() catches the bad ref
  → GcEnumObject
  → EnumGcRefsX86 ← reports untracked slot for aborted frame
  → SubProblem + 5 ← fault offset (before local 's' initialized)

Build 1403719 — ✅ Passed (with x86 fix)

• Commit: 9a9b245 → includes f66c09c "Fix EnumGcRefsX86 to skip reporting for non-interruptible aborted frames" (Apr 30 12:03 UTC)
• Contains: Validate change + test Pri0 + gc_unwind_x86.inl fix
• Result: coreclr Pri0 Runtime Tests Run windows x86 checked → succeeded (44m 3s)
• EnumGcRefsX86 now returns early for the aborted non-interruptible frame, so Promote/Validate is never called with the uninitialized slot

[github-actions]

🤖 Copilot Code Review — PR #127595

Note

This review was generated by GitHub Copilot.

Holistic Assessment

Motivation: The PR addresses a real correctness issue where EnumGcRefsX86 could report uninitialized GC slots for non-interruptible aborted frames, and improves debug validation in GC promote functions. These are genuine bugs that can cause GC heap corruption or crashes.

Approach: The changes are minimal and well-targeted — reordering the DEBUG_DestroyedHandleValue check before is_in_find_object_range (race condition correctness), adding debug-only Validate() calls for early diagnostics, and porting the existing gcinfodecoder.cpp logic to the x86 EnumGcRefsX86 path. The approach mirrors existing patterns already present in the codebase.

Summary: ✅ LGTM. The changes are correct, minimal, and consistent with established codebase patterns. All three GC promote functions (GCHeap::Promote, background_promote, background_promote_callback) are now consistent. The x86 fix is a straightforward port of logic already proven in gcinfodecoder.cpp (lines 808-812).

---

Detailed Findings

✅ Correctness — DEBUG_DestroyedHandleValue reordering (interface.cpp, background.cpp)

Moving the DEBUG_DestroyedHandleValue check before is_in_find_object_range is correct. During concurrent scanning, a handle can be destroyed (set to the sentinel value) while the GC is scanning. The destroyed sentinel value is not a valid heap address, so is_in_find_object_range would return false, but without the reordering, the new Validate() call (added in the !is_in_find_object_range path) could attempt to validate the sentinel value as an object, which would crash. Checking the sentinel first avoids this.

✅ Correctness — Debug validation in early-return path

Adding ((CObjectHeader*)o)->Validate() under _DEBUG for non-null, non-interior objects that fall outside the GC range is a sound diagnostic addition. The guards (o != NULL and !(flags & GC_CALL_INTERIOR)) are appropriate — null pointers obviously cannot be validated, and interior pointers do not point to object headers.

✅ Correctness — EnumGcRefsX86 aborted frame fix (gc_unwind_x86.inl)

The added condition ((flags & ExecutionAborted) && !info.interruptible) exactly matches the logic in gcinfodecoder.cpp at lines 808-812 (which skips reporting when countIntersections == 0 && executionAborted — meaning the frame is not fully interruptible). For non-interruptible methods where execution is aborted, GC slots may not be initialized at the current offset, so reporting them could cause the GC to trace garbage values. The existing _ASSERTE(flags & ExecutionAborted) at line 3103 already documents that this early-return path is only expected for aborted frames — the new condition simply widens the gate to include the non-interruptible aborted case alongside prolog/epilog.

✅ Test Priority — test119403.csproj

Changing priority from 0 to 1 follows the convention: "New JIT regression tests should typically be CLRTestPriority 1." The test was temporarily set to 0 earlier in the PR branch for verification, then correctly restored to 1 for merge.

✅ Consistency — All three promote functions aligned

GCHeap::Promote, background_promote, and background_promote_callback now all follow the same pattern: check DEBUG_DestroyedHandleValue → check is_in_find_object_range (with debug validation) → proceed. This eliminates an inconsistency that existed before this PR.

Generated by Code Review for issue #127595 · ◷

[Copilot]

Pull request overview

This PR targets a win-x86 GC root reporting hole by (1) aligning the x86 GCInfo unwinder behavior with the earlier fix in #119403 and (2) strengthening debug-time GC promotion validation so this class of corruption becomes easier to reproduce and diagnose deterministically.

Changes:

• Update x86 GC root enumeration to skip reporting for non-interruptible methods when execution has been aborted (in addition to prolog/epilog cases).
• Add _DEBUG-only validation in GCHeap::Promote and background promotion paths when an object reference falls outside the find_object bookkeeping range.

Reviewed changes

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

File	Description
src/coreclr/vm/gc_unwind_x86.inl	Extends the “skip reporting” condition to cover aborted + non-interruptible methods, matching the intended semantics from the earlier fix.
src/coreclr/gc/interface.cpp	Moves destroyed-handle sentinel check earlier and adds debug validation when the ref is outside is_in_find_object_range.
src/coreclr/gc/background.cpp	Adds similar debug validation in background promote paths (including callback).

[mangod9]

Btw, there is a libraries x86 failure but doesnt look like that is related to this change. The auto analysis is below. @VSadov surprised that ThreadAbort is being throw here?

!m_RedirectContextInUse Assert on win-x86 — Analysis

Assert

Assert failure (Thread 0x590): !m_RedirectContextInUse
File: src/coreclr/vm/threads.h:3104
Image: dotnet.exe (x86 Checked)
Exit code: -1073740286 (STATUS_FAIL_FAST_EXCEPTION)

Fires in Thread::MarkRedirectContextInUse when the runtime attempts to redirect (hijack) a thread whose redirect context is already occupied from a prior redirect.

Thread State at Crash

Thread	OSID	GC Mode	Role	Activity
11	0x590	Cooperative	Crash thread	Inside 1st redirect → HandleThreadAbort → constructing ThreadAbortException → 2nd redirect fires assert
12	0xccc	Cooperative	GC thread	Running GCExtendedTests.GetTotalAllocatedBytes() → GC.GetTotalAllocatedBytesPrecise() → triggered GC
6	0x101c	Preemptive	Tiered compilation	JIT compiling, blocked in WaitUntilGCComplete
5	0x1910	Preemptive	Finalizer	Waiting for finalizer event
0	0x1ad8	Preemptive	Main	xunit runner waiting

Full Stack — Thread 11 (OSID 0x590) — Native + Managed

Shows two nested redirects — the second one fires the assert:

=== 2nd redirect (fires the assert) ===
coreclr!FailFastOnAssert
coreclr!_DbgBreakCheck
coreclr!_DbgBreakCheckNoThrow
coreclr!DbgAssertDialog
coreclr!Thread::MarkRedirectContextInUse ← 2nd redirect: context already in use!
coreclr!Thread::RedirectedHandledJITCase ← 2nd redirect handler
coreclr!Thread::RedirectedHandledJITCaseForGCThreadControl
coreclr!RedirectedHandledJITCaseForGCThreadControl_Stub ← 2nd GC hijack entry point
=== Managed code running inside 1st redirect (ThreadAbortException construction) ===
System.IO.BinaryReader.ReadByte()
System.IO.BinaryReader.Read7BitEncodedInt()
System.Resources.ResourceReader.LoadString(Int32)
System.Resources.RuntimeResourceSet.ReadValue(...)
System.Resources.RuntimeResourceSet.GetObject(...)
System.Resources.RuntimeResourceSet.GetString(...)
System.Resources.ResourceManager.GetString(...)
System.SR.InternalGetResourceString(String)
System.SR.GetResourceString(String)
System.SR.get_Arg_SystemException()
System.SystemException..ctor()
System.Threading.ThreadAbortException..ctor()
System.Runtime.CompilerServices.RuntimeHelpers.CallDefaultConstructor(...)
=== 1st redirect processing the pending ThreadAbort ===
coreclr!CallDefaultConstructor
coreclr!EEException::CreateThrowable
coreclr!CLRException::GetThrowable
coreclr!CLRException::GetThrowableFromException
coreclr!Thread::HandleThreadAbort ← processes pending TS_AbortRequested
coreclr!Thread::RestoreContextSimulated
coreclr!Thread::RedirectedHandledJITCase ← 1st redirect handler
coreclr!Thread::RedirectedHandledJITCaseForGCThreadControl
coreclr!RedirectedHandledJITCaseForGCThreadControl_Stub ← 1st GC hijack entry point
=== Original managed code when 1st redirect hijacked the thread ===
[InlinedCallFrame] System.RuntimeMethodHandle.InvokeMethod(...)
System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object, IntPtr*)
System.Reflection.RuntimeMethodInfo.Invoke(...)
System.Reflection.MethodBase.Invoke(Object, Object[])
Xunit.Sdk.TestInvoker`1.CallTestMethod(Object)
Xunit.Sdk.TestInvoker`1+<>c__DisplayClass47_0.<<InvokeTestMethodAsync>b__1>d.MoveNext()
... (xunit async test invocation pipeline) ...
Xunit.Sdk.XunitTestCase.RunAsync(IMessageSink, IMessageBus, Object[], ExceptionAggregator, CancellationTokenSource)
Microsoft.DotNet.XUnitExtensions.SkippedFactTestCase.<>n__0(...)
Microsoft.DotNet.XUnitExtensions.SkippedFactTestCase+<RunAsync>d__2.MoveNext()
... (xunit async test runner pipeline) ...
Xunit.Sdk.XunitTestAssemblyRunner+<RunTestCollectionAsync>d__18.MoveNext()
System.Threading.ExecutionContext.RunInternal(...)
... (async continuations) ...
System.Threading.Tasks.Task+CancellationPromise`1[[Boolean]].ITaskCompletionAction.Invoke(Task)
System.Threading.Tasks.CompletionActionInvoker.Execute()
System.Threading.ThreadPoolWorkQueue.Dispatch()
System.Threading.PortableThreadPool+WorkerThread.WorkerDoWork(PortableThreadPool)
System.Threading.PortableThreadPool+WorkerThread.WorkerThreadStart()
System.Threading.Thread.StartCallback(Thread*)

GC Thread Stack (Thread 12, OSID 0xccc)

System.GC.GetTotalAllocatedBytesPrecise() ← triggers GC
System.GC.GetTotalAllocatedBytes(Boolean)
System.Tests.GCExtendedTests.GetTotalAllocatedBytes() ← the test
... (xunit async pipeline via reflection invoke) ...
System.Threading.PortableThreadPool+WorkerThread.WorkerDoWork(PortableThreadPool)
System.Threading.PortableThreadPool+WorkerThread.WorkerThreadStart()
System.Threading.Thread.StartCallback(Thread*)

Sequence of Events

1. Thread 12 runs GCExtendedTests.GetTotalAllocatedBytes() → GC.GetTotalAllocatedBytesPrecise() → triggers a GC collection
2. GC suspension begins. Thread 11 is in Cooperative mode running a test via SkippedFactTestCase.RunAsync. The runtime redirects (hijacks) Thread 11 via RedirectedHandledJITCaseForGCThreadControl_Stub (1st redirect)
3. Inside RedirectedHandledJITCase → RestoreContextSimulated, the runtime calls Thread::HandleThreadAbort — a pending thread abort (TS_AbortRequested) was set on Thread 11. The source of the abort is not captured in the dump. On modern .NET (6+), Thread.Abort() throws PlatformNotSupportedException; thread aborts can only be initiated by the runtime internally or via ControlledExecution.Run
4. HandleThreadAbort → CreateThrowable → CallDefaultConstructor for ThreadAbortException — this runs managed code to construct the exception (loading the resource string "Arg_SystemException" via SR.GetResourceString → ResourceReader.LoadString → BinaryReader.ReadByte)
5. The managed code runs in Cooperative mode, making Thread 11 eligible for GC hijacking again
6. A second GC triggers (or GC suspension retries). The runtime attempts to redirect Thread 11 again via RedirectedHandledJITCaseForGCThreadControl_Stub (2nd redirect)
7. MarkRedirectContextInUse finds m_RedirectContextInUse == true (still held from step 2) → assert fires

Key Finding

There is no separate cancellation thread visible in the dump. The ThreadAbort was a pending flag (TS_AbortRequested) already set on Thread 11. The source of the abort is not captured in the dump — on modern .NET, thread aborts can come from ControlledExecution.Run or other runtime-internal mechanisms. Thread::HandleThreadAbort processes this flag as part of the 1st redirect handler's normal flow (RestoreContextSimulated → HandleThreadAbort). The bug is that the 1st redirect handler runs managed code (exception construction) while still holding the redirect context, creating a window where a 2nd redirect can be attempted.

Root Cause

A double-redirect race condition on x86: the runtime hijacks a thread for GC suspension, processes a pending ThreadAbort inside the redirect handler, and the ThreadAbortException constructor runs managed code (resource string loading) in Cooperative mode. This makes the thread eligible for another GC redirect while the first redirect context is still held.

Why This Hasn't Been Seen Before

• Libraries Test Run checked coreclr windows x86 Debug only runs on PR builds, not main rolling builds
• Main rolling builds run Libraries Test Run checked coreclr windows x86 **Release** — Release libraries are faster, narrowing the timing window
• Debug libraries' slower execution (resource loading, extra checks) widens the race window between the first redirect and the second attempt
• The m_RedirectContextInUse assert only fires in Checked builds

Relation to PR #127595

Not related. The gc_unwind_x86.inl fix changes GC reporting logic inside EnumGcRefsX86. The Validate() change in interface.cpp adds a Debug assert in Promote. Neither affects thread redirect/hijack logic. This assert passed on the previous build of the same PR (build 1403154), confirming it's intermittent.

[jkotas]

• Mirrors the fix from Unconditionally skip GC reporting for non-interruptible aborted methods #119403 in x86 GCInfo decoder
• Improve GCHeap::Promote debug validation to repro this class of issues deterministically

Fixes#127581

[dotnet-policy-service]

Tagging subscribers to this area: @JulieLeeMSFT, @dotnet/gc
See info in area-owners.md if you want to be subscribed.

[Copilot]

Pull request overview

This PR aims to strengthen debug-time validation in GCHeap::Promote by validating candidate object pointers even on the early-return path when the pointer is outside the find_object bookkeeping range.

Changes:

• Add a _DEBUG-only CObjectHeader::Validate() call when !gc_heap::is_in_find_object_range(o) and the root is not an interior pointer.

[jkotas]

@copilot Change the priority for test119403.cs to 0 temporarily to make the test exercised in the CI

[Copilot]

@copilot Change the priority for test119403.cs to 0 temporarily to make the test exercised in the CI

Done in bfa4fd1. Changed CLRTestPriority from 1 to 0 in test119403.csproj.

[jkotas]

gc_heap::background_promote (background.cpp:547) and gc_heap::background_promote_callback

@copilot Address feeback

[Copilot]

gc_heap::background_promote (background.cpp:547) and gc_heap::background_promote_callback

@copilot Address feeback

Done in b05e42d. Applied the same fix to both background_promote and background_promote_callback: moved the DEBUG_DestroyedHandleValue check before is_in_find_object_range, and added Validate() in the early-return path (guarded by _DEBUG and !GC_CALL_INTERIOR).

[jkotas]

@copilot Implement the fix mentioned in #127581 (comment)

[Copilot]

@copilot Implement the fix mentioned in #127581 (comment)

Done in f66c09c. Added the equivalent of the gcinfodecoder.cpp fix to EnumGcRefsX86 in gc_unwind_x86.inl: when a partially-interruptible method is aborted (exception in flight) and the current offset is not within an interruptible range, skip all GC reporting. Untracked locals may not be initialized at the faulting offset, and since execution won't resume there, there's no need to report them.

[Copilot]

Pull request overview

This PR addresses a class of x86 GC hole issues (tracked in #127581) by aligning x86 GC reference enumeration behavior with the earlier fix from #119403 and by strengthening debug-time validation in GCHeap::Promote paths so invalid/out-of-range object references fail deterministically in debug builds.

Changes:

• Update x86 GCInfo/stack-walk enumeration to skip reporting entirely for execution-aborted, non-interruptible methods (where slots may not be initialized).
• Add additional _DEBUG validation when Promote/background promotion sees a non-null, non-interior pointer that is outside the GC “find_object” bookkeeping range.
• Adjust the regression test project priority to make the scenario easier to exercise in standard test runs.

Reviewed changes

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

File	Description
src/tests/Regressions/coreclr/GitHub_119403/test119403.csproj	Changes test priority to run as a default-priority regression.
src/coreclr/vm/gc_unwind_x86.inl	Skips GC reporting for execution-aborted non-interruptible methods on x86 to avoid reporting uninitialized GC slots.
src/coreclr/gc/interface.cpp	Adds debug validation in GCHeap::Promote when an out-of-range non-interior reference is encountered.
src/coreclr/gc/background.cpp	Adds the same debug validation behavior to background GC promotion paths.

[mangod9]

The fix is working:

CI Validation of the Fix

Build 1403154 — ❌ Failed (without x86 fix)

• Commit: bc54dbf (Apr 30 05:32 UTC)
• Contains: Validate change in interface.cpp + test promoted to Pri0 — but NOT the gc_unwind_x86.inl fix
• Result: coreclr Pri0 Runtime Tests Run windows x86 checked → failed
• Failure: Regressions workitem in Helix job ab237a9f-5703-40e6-9caa-e9eba9d566d0 Assert: !"Detected use of a
  corrupted OBJECTREF. Possible GC hole."
  Object::Validate + 0xA9
  → WKS::GCHeap::Promote + 0x169 ← new Validate() catches the bad ref
  → GcEnumObject
  → EnumGcRefsX86 ← reports untracked slot for aborted frame
  → SubProblem + 5 ← fault offset (before local 's' initialized)

Build 1403719 — ✅ Passed (with x86 fix)

• Commit: 9a9b245 → includes f66c09c "Fix EnumGcRefsX86 to skip reporting for non-interruptible aborted frames" (Apr 30 12:03 UTC)
• Contains: Validate change + test Pri0 + gc_unwind_x86.inl fix
• Result: coreclr Pri0 Runtime Tests Run windows x86 checked → succeeded (44m 3s)
• EnumGcRefsX86 now returns early for the aborted non-interruptible frame, so Promote/Validate is never called with the uninitialized slot

[github-actions]

🤖 Copilot Code Review — PR #127595

Note

This review was generated by GitHub Copilot.

Holistic Assessment

Motivation: The PR addresses a real correctness issue where EnumGcRefsX86 could report uninitialized GC slots for non-interruptible aborted frames, and improves debug validation in GC promote functions. These are genuine bugs that can cause GC heap corruption or crashes.

Approach: The changes are minimal and well-targeted — reordering the DEBUG_DestroyedHandleValue check before is_in_find_object_range (race condition correctness), adding debug-only Validate() calls for early diagnostics, and porting the existing gcinfodecoder.cpp logic to the x86 EnumGcRefsX86 path. The approach mirrors existing patterns already present in the codebase.

Summary: ✅ LGTM. The changes are correct, minimal, and consistent with established codebase patterns. All three GC promote functions (GCHeap::Promote, background_promote, background_promote_callback) are now consistent. The x86 fix is a straightforward port of logic already proven in gcinfodecoder.cpp (lines 808-812).

---

Detailed Findings

✅ Correctness — DEBUG_DestroyedHandleValue reordering (interface.cpp, background.cpp)

Moving the DEBUG_DestroyedHandleValue check before is_in_find_object_range is correct. During concurrent scanning, a handle can be destroyed (set to the sentinel value) while the GC is scanning. The destroyed sentinel value is not a valid heap address, so is_in_find_object_range would return false, but without the reordering, the new Validate() call (added in the !is_in_find_object_range path) could attempt to validate the sentinel value as an object, which would crash. Checking the sentinel first avoids this.

✅ Correctness — Debug validation in early-return path

Adding ((CObjectHeader*)o)->Validate() under _DEBUG for non-null, non-interior objects that fall outside the GC range is a sound diagnostic addition. The guards (o != NULL and !(flags & GC_CALL_INTERIOR)) are appropriate — null pointers obviously cannot be validated, and interior pointers do not point to object headers.

✅ Correctness — EnumGcRefsX86 aborted frame fix (gc_unwind_x86.inl)

The added condition ((flags & ExecutionAborted) && !info.interruptible) exactly matches the logic in gcinfodecoder.cpp at lines 808-812 (which skips reporting when countIntersections == 0 && executionAborted — meaning the frame is not fully interruptible). For non-interruptible methods where execution is aborted, GC slots may not be initialized at the current offset, so reporting them could cause the GC to trace garbage values. The existing _ASSERTE(flags & ExecutionAborted) at line 3103 already documents that this early-return path is only expected for aborted frames — the new condition simply widens the gate to include the non-interruptible aborted case alongside prolog/epilog.

✅ Test Priority — test119403.csproj

Changing priority from 0 to 1 follows the convention: "New JIT regression tests should typically be CLRTestPriority 1." The test was temporarily set to 0 earlier in the PR branch for verification, then correctly restored to 1 for merge.

✅ Consistency — All three promote functions aligned

GCHeap::Promote, background_promote, and background_promote_callback now all follow the same pattern: check DEBUG_DestroyedHandleValue → check is_in_find_object_range (with debug validation) → proceed. This eliminates an inconsistency that existed before this PR.

Generated by Code Review for issue #127595 · ◷

[Copilot]

Pull request overview

This PR targets a win-x86 GC root reporting hole by (1) aligning the x86 GCInfo unwinder behavior with the earlier fix in #119403 and (2) strengthening debug-time GC promotion validation so this class of corruption becomes easier to reproduce and diagnose deterministically.

Changes:

• Update x86 GC root enumeration to skip reporting for non-interruptible methods when execution has been aborted (in addition to prolog/epilog cases).
• Add _DEBUG-only validation in GCHeap::Promote and background promotion paths when an object reference falls outside the find_object bookkeeping range.

Reviewed changes

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

File	Description
src/coreclr/vm/gc_unwind_x86.inl	Extends the “skip reporting” condition to cover aborted + non-interruptible methods, matching the intended semantics from the earlier fix.
src/coreclr/gc/interface.cpp	Moves destroyed-handle sentinel check earlier and adds debug validation when the ref is outside is_in_find_object_range.
src/coreclr/gc/background.cpp	Adds similar debug validation in background promote paths (including callback).

[mangod9]

Btw, there is a libraries x86 failure but doesnt look like that is related to this change. The auto analysis is below. @VSadov surprised that ThreadAbort is being throw here?

!m_RedirectContextInUse Assert on win-x86 — Analysis

Assert

Assert failure (Thread 0x590): !m_RedirectContextInUse
File: src/coreclr/vm/threads.h:3104
Image: dotnet.exe (x86 Checked)
Exit code: -1073740286 (STATUS_FAIL_FAST_EXCEPTION)

Fires in Thread::MarkRedirectContextInUse when the runtime attempts to redirect (hijack) a thread whose redirect context is already occupied from a prior redirect.

Thread State at Crash

Thread	OSID	GC Mode	Role	Activity
11	0x590	Cooperative	Crash thread	Inside 1st redirect → HandleThreadAbort → constructing ThreadAbortException → 2nd redirect fires assert
12	0xccc	Cooperative	GC thread	Running GCExtendedTests.GetTotalAllocatedBytes() → GC.GetTotalAllocatedBytesPrecise() → triggered GC
6	0x101c	Preemptive	Tiered compilation	JIT compiling, blocked in WaitUntilGCComplete
5	0x1910	Preemptive	Finalizer	Waiting for finalizer event
0	0x1ad8	Preemptive	Main	xunit runner waiting

Full Stack — Thread 11 (OSID 0x590) — Native + Managed

Shows two nested redirects — the second one fires the assert:

=== 2nd redirect (fires the assert) ===
coreclr!FailFastOnAssert
coreclr!_DbgBreakCheck
coreclr!_DbgBreakCheckNoThrow
coreclr!DbgAssertDialog
coreclr!Thread::MarkRedirectContextInUse ← 2nd redirect: context already in use!
coreclr!Thread::RedirectedHandledJITCase ← 2nd redirect handler
coreclr!Thread::RedirectedHandledJITCaseForGCThreadControl
coreclr!RedirectedHandledJITCaseForGCThreadControl_Stub ← 2nd GC hijack entry point
=== Managed code running inside 1st redirect (ThreadAbortException construction) ===
System.IO.BinaryReader.ReadByte()
System.IO.BinaryReader.Read7BitEncodedInt()
System.Resources.ResourceReader.LoadString(Int32)
System.Resources.RuntimeResourceSet.ReadValue(...)
System.Resources.RuntimeResourceSet.GetObject(...)
System.Resources.RuntimeResourceSet.GetString(...)
System.Resources.ResourceManager.GetString(...)
System.SR.InternalGetResourceString(String)
System.SR.GetResourceString(String)
System.SR.get_Arg_SystemException()
System.SystemException..ctor()
System.Threading.ThreadAbortException..ctor()
System.Runtime.CompilerServices.RuntimeHelpers.CallDefaultConstructor(...)
=== 1st redirect processing the pending ThreadAbort ===
coreclr!CallDefaultConstructor
coreclr!EEException::CreateThrowable
coreclr!CLRException::GetThrowable
coreclr!CLRException::GetThrowableFromException
coreclr!Thread::HandleThreadAbort ← processes pending TS_AbortRequested
coreclr!Thread::RestoreContextSimulated
coreclr!Thread::RedirectedHandledJITCase ← 1st redirect handler
coreclr!Thread::RedirectedHandledJITCaseForGCThreadControl
coreclr!RedirectedHandledJITCaseForGCThreadControl_Stub ← 1st GC hijack entry point
=== Original managed code when 1st redirect hijacked the thread ===
[InlinedCallFrame] System.RuntimeMethodHandle.InvokeMethod(...)
System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object, IntPtr*)
System.Reflection.RuntimeMethodInfo.Invoke(...)
System.Reflection.MethodBase.Invoke(Object, Object[])
Xunit.Sdk.TestInvoker`1.CallTestMethod(Object)
Xunit.Sdk.TestInvoker`1+<>c__DisplayClass47_0.<<InvokeTestMethodAsync>b__1>d.MoveNext()
... (xunit async test invocation pipeline) ...
Xunit.Sdk.XunitTestCase.RunAsync(IMessageSink, IMessageBus, Object[], ExceptionAggregator, CancellationTokenSource)
Microsoft.DotNet.XUnitExtensions.SkippedFactTestCase.<>n__0(...)
Microsoft.DotNet.XUnitExtensions.SkippedFactTestCase+<RunAsync>d__2.MoveNext()
... (xunit async test runner pipeline) ...
Xunit.Sdk.XunitTestAssemblyRunner+<RunTestCollectionAsync>d__18.MoveNext()
System.Threading.ExecutionContext.RunInternal(...)
... (async continuations) ...
System.Threading.Tasks.Task+CancellationPromise`1[[Boolean]].ITaskCompletionAction.Invoke(Task)
System.Threading.Tasks.CompletionActionInvoker.Execute()
System.Threading.ThreadPoolWorkQueue.Dispatch()
System.Threading.PortableThreadPool+WorkerThread.WorkerDoWork(PortableThreadPool)
System.Threading.PortableThreadPool+WorkerThread.WorkerThreadStart()
System.Threading.Thread.StartCallback(Thread*)

GC Thread Stack (Thread 12, OSID 0xccc)

System.GC.GetTotalAllocatedBytesPrecise() ← triggers GC
System.GC.GetTotalAllocatedBytes(Boolean)
System.Tests.GCExtendedTests.GetTotalAllocatedBytes() ← the test
... (xunit async pipeline via reflection invoke) ...
System.Threading.PortableThreadPool+WorkerThread.WorkerDoWork(PortableThreadPool)
System.Threading.PortableThreadPool+WorkerThread.WorkerThreadStart()
System.Threading.Thread.StartCallback(Thread*)

Sequence of Events

1. Thread 12 runs GCExtendedTests.GetTotalAllocatedBytes() → GC.GetTotalAllocatedBytesPrecise() → triggers a GC collection
2. GC suspension begins. Thread 11 is in Cooperative mode running a test via SkippedFactTestCase.RunAsync. The runtime redirects (hijacks) Thread 11 via RedirectedHandledJITCaseForGCThreadControl_Stub (1st redirect)
3. Inside RedirectedHandledJITCase → RestoreContextSimulated, the runtime calls Thread::HandleThreadAbort — a pending thread abort (TS_AbortRequested) was set on Thread 11. The source of the abort is not captured in the dump. On modern .NET (6+), Thread.Abort() throws PlatformNotSupportedException; thread aborts can only be initiated by the runtime internally or via ControlledExecution.Run
4. HandleThreadAbort → CreateThrowable → CallDefaultConstructor for ThreadAbortException — this runs managed code to construct the exception (loading the resource string "Arg_SystemException" via SR.GetResourceString → ResourceReader.LoadString → BinaryReader.ReadByte)
5. The managed code runs in Cooperative mode, making Thread 11 eligible for GC hijacking again
6. A second GC triggers (or GC suspension retries). The runtime attempts to redirect Thread 11 again via RedirectedHandledJITCaseForGCThreadControl_Stub (2nd redirect)
7. MarkRedirectContextInUse finds m_RedirectContextInUse == true (still held from step 2) → assert fires

Key Finding

There is no separate cancellation thread visible in the dump. The ThreadAbort was a pending flag (TS_AbortRequested) already set on Thread 11. The source of the abort is not captured in the dump — on modern .NET, thread aborts can come from ControlledExecution.Run or other runtime-internal mechanisms. Thread::HandleThreadAbort processes this flag as part of the 1st redirect handler's normal flow (RestoreContextSimulated → HandleThreadAbort). The bug is that the 1st redirect handler runs managed code (exception construction) while still holding the redirect context, creating a window where a 2nd redirect can be attempted.

Root Cause

A double-redirect race condition on x86: the runtime hijacks a thread for GC suspension, processes a pending ThreadAbort inside the redirect handler, and the ThreadAbortException constructor runs managed code (resource string loading) in Cooperative mode. This makes the thread eligible for another GC redirect while the first redirect context is still held.

Why This Hasn't Been Seen Before

• Libraries Test Run checked coreclr windows x86 Debug only runs on PR builds, not main rolling builds
• Main rolling builds run Libraries Test Run checked coreclr windows x86 **Release** — Release libraries are faster, narrowing the timing window
• Debug libraries' slower execution (resource loading, extra checks) widens the race window between the first redirect and the second attempt
• The m_RedirectContextInUse assert only fires in Checked builds

Relation to PR #127595

Not related. The gc_unwind_x86.inl fix changes GC reporting logic inside EnumGcRefsX86. The Validate() change in interface.cpp adds a Debug assert in Promote. Neither affects thread redirect/hijack logic. This assert passed on the previous build of the same PR (build 1403154), confirming it's intermittent.

[jkotas]

• Mirrors the fix from Unconditionally skip GC reporting for non-interruptible aborted methods #119403 in x86 GCInfo decoder
• Improve GCHeap::Promote debug validation to repro this class of issues deterministically

Fixes#127581

[dotnet-policy-service]

Tagging subscribers to this area: @JulieLeeMSFT, @dotnet/gc
See info in area-owners.md if you want to be subscribed.

[Copilot]

Pull request overview

This PR aims to strengthen debug-time validation in GCHeap::Promote by validating candidate object pointers even on the early-return path when the pointer is outside the find_object bookkeeping range.

Changes:

• Add a _DEBUG-only CObjectHeader::Validate() call when !gc_heap::is_in_find_object_range(o) and the root is not an interior pointer.

[jkotas]

@copilot Change the priority for test119403.cs to 0 temporarily to make the test exercised in the CI

[Copilot]

@copilot Change the priority for test119403.cs to 0 temporarily to make the test exercised in the CI

Done in bfa4fd1. Changed CLRTestPriority from 1 to 0 in test119403.csproj.

[jkotas]

gc_heap::background_promote (background.cpp:547) and gc_heap::background_promote_callback

@copilot Address feeback

[Copilot]

gc_heap::background_promote (background.cpp:547) and gc_heap::background_promote_callback

@copilot Address feeback

Done in b05e42d. Applied the same fix to both background_promote and background_promote_callback: moved the DEBUG_DestroyedHandleValue check before is_in_find_object_range, and added Validate() in the early-return path (guarded by _DEBUG and !GC_CALL_INTERIOR).

[jkotas]

@copilot Implement the fix mentioned in #127581 (comment)

[Copilot]

@copilot Implement the fix mentioned in #127581 (comment)

Done in f66c09c. Added the equivalent of the gcinfodecoder.cpp fix to EnumGcRefsX86 in gc_unwind_x86.inl: when a partially-interruptible method is aborted (exception in flight) and the current offset is not within an interruptible range, skip all GC reporting. Untracked locals may not be initialized at the faulting offset, and since execution won't resume there, there's no need to report them.

[Copilot]

Pull request overview

This PR addresses a class of x86 GC hole issues (tracked in #127581) by aligning x86 GC reference enumeration behavior with the earlier fix from #119403 and by strengthening debug-time validation in GCHeap::Promote paths so invalid/out-of-range object references fail deterministically in debug builds.

Changes:

• Update x86 GCInfo/stack-walk enumeration to skip reporting entirely for execution-aborted, non-interruptible methods (where slots may not be initialized).
• Add additional _DEBUG validation when Promote/background promotion sees a non-null, non-interior pointer that is outside the GC “find_object” bookkeeping range.
• Adjust the regression test project priority to make the scenario easier to exercise in standard test runs.

Reviewed changes

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

File	Description
src/tests/Regressions/coreclr/GitHub_119403/test119403.csproj	Changes test priority to run as a default-priority regression.
src/coreclr/vm/gc_unwind_x86.inl	Skips GC reporting for execution-aborted non-interruptible methods on x86 to avoid reporting uninitialized GC slots.
src/coreclr/gc/interface.cpp	Adds debug validation in GCHeap::Promote when an out-of-range non-interior reference is encountered.
src/coreclr/gc/background.cpp	Adds the same debug validation behavior to background GC promotion paths.

[mangod9]

The fix is working:

CI Validation of the Fix

Build 1403154 — ❌ Failed (without x86 fix)

• Commit: bc54dbf (Apr 30 05:32 UTC)
• Contains: Validate change in interface.cpp + test promoted to Pri0 — but NOT the gc_unwind_x86.inl fix
• Result: coreclr Pri0 Runtime Tests Run windows x86 checked → failed
• Failure: Regressions workitem in Helix job ab237a9f-5703-40e6-9caa-e9eba9d566d0 Assert: !"Detected use of a
  corrupted OBJECTREF. Possible GC hole."
  Object::Validate + 0xA9
  → WKS::GCHeap::Promote + 0x169 ← new Validate() catches the bad ref
  → GcEnumObject
  → EnumGcRefsX86 ← reports untracked slot for aborted frame
  → SubProblem + 5 ← fault offset (before local 's' initialized)

Build 1403719 — ✅ Passed (with x86 fix)

• Commit: 9a9b245 → includes f66c09c "Fix EnumGcRefsX86 to skip reporting for non-interruptible aborted frames" (Apr 30 12:03 UTC)
• Contains: Validate change + test Pri0 + gc_unwind_x86.inl fix
• Result: coreclr Pri0 Runtime Tests Run windows x86 checked → succeeded (44m 3s)
• EnumGcRefsX86 now returns early for the aborted non-interruptible frame, so Promote/Validate is never called with the uninitialized slot

[github-actions]

🤖 Copilot Code Review — PR #127595

Note

This review was generated by GitHub Copilot.

Holistic Assessment

Motivation: The PR addresses a real correctness issue where EnumGcRefsX86 could report uninitialized GC slots for non-interruptible aborted frames, and improves debug validation in GC promote functions. These are genuine bugs that can cause GC heap corruption or crashes.

Approach: The changes are minimal and well-targeted — reordering the DEBUG_DestroyedHandleValue check before is_in_find_object_range (race condition correctness), adding debug-only Validate() calls for early diagnostics, and porting the existing gcinfodecoder.cpp logic to the x86 EnumGcRefsX86 path. The approach mirrors existing patterns already present in the codebase.

Summary: ✅ LGTM. The changes are correct, minimal, and consistent with established codebase patterns. All three GC promote functions (GCHeap::Promote, background_promote, background_promote_callback) are now consistent. The x86 fix is a straightforward port of logic already proven in gcinfodecoder.cpp (lines 808-812).

---

Detailed Findings

✅ Correctness — DEBUG_DestroyedHandleValue reordering (interface.cpp, background.cpp)

Moving the DEBUG_DestroyedHandleValue check before is_in_find_object_range is correct. During concurrent scanning, a handle can be destroyed (set to the sentinel value) while the GC is scanning. The destroyed sentinel value is not a valid heap address, so is_in_find_object_range would return false, but without the reordering, the new Validate() call (added in the !is_in_find_object_range path) could attempt to validate the sentinel value as an object, which would crash. Checking the sentinel first avoids this.

✅ Correctness — Debug validation in early-return path

Adding ((CObjectHeader*)o)->Validate() under _DEBUG for non-null, non-interior objects that fall outside the GC range is a sound diagnostic addition. The guards (o != NULL and !(flags & GC_CALL_INTERIOR)) are appropriate — null pointers obviously cannot be validated, and interior pointers do not point to object headers.

✅ Correctness — EnumGcRefsX86 aborted frame fix (gc_unwind_x86.inl)

The added condition ((flags & ExecutionAborted) && !info.interruptible) exactly matches the logic in gcinfodecoder.cpp at lines 808-812 (which skips reporting when countIntersections == 0 && executionAborted — meaning the frame is not fully interruptible). For non-interruptible methods where execution is aborted, GC slots may not be initialized at the current offset, so reporting them could cause the GC to trace garbage values. The existing _ASSERTE(flags & ExecutionAborted) at line 3103 already documents that this early-return path is only expected for aborted frames — the new condition simply widens the gate to include the non-interruptible aborted case alongside prolog/epilog.

✅ Test Priority — test119403.csproj

Changing priority from 0 to 1 follows the convention: "New JIT regression tests should typically be CLRTestPriority 1." The test was temporarily set to 0 earlier in the PR branch for verification, then correctly restored to 1 for merge.

✅ Consistency — All three promote functions aligned

GCHeap::Promote, background_promote, and background_promote_callback now all follow the same pattern: check DEBUG_DestroyedHandleValue → check is_in_find_object_range (with debug validation) → proceed. This eliminates an inconsistency that existed before this PR.

Generated by Code Review for issue #127595 · ◷

[Copilot]

Pull request overview

This PR targets a win-x86 GC root reporting hole by (1) aligning the x86 GCInfo unwinder behavior with the earlier fix in #119403 and (2) strengthening debug-time GC promotion validation so this class of corruption becomes easier to reproduce and diagnose deterministically.

Changes:

• Update x86 GC root enumeration to skip reporting for non-interruptible methods when execution has been aborted (in addition to prolog/epilog cases).
• Add _DEBUG-only validation in GCHeap::Promote and background promotion paths when an object reference falls outside the find_object bookkeeping range.

Reviewed changes

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

File	Description
src/coreclr/vm/gc_unwind_x86.inl	Extends the “skip reporting” condition to cover aborted + non-interruptible methods, matching the intended semantics from the earlier fix.
src/coreclr/gc/interface.cpp	Moves destroyed-handle sentinel check earlier and adds debug validation when the ref is outside is_in_find_object_range.
src/coreclr/gc/background.cpp	Adds similar debug validation in background promote paths (including callback).

[mangod9]

Btw, there is a libraries x86 failure but doesnt look like that is related to this change. The auto analysis is below. @VSadov surprised that ThreadAbort is being throw here?

!m_RedirectContextInUse Assert on win-x86 — Analysis

Assert

Assert failure (Thread 0x590): !m_RedirectContextInUse
File: src/coreclr/vm/threads.h:3104
Image: dotnet.exe (x86 Checked)
Exit code: -1073740286 (STATUS_FAIL_FAST_EXCEPTION)

Fires in Thread::MarkRedirectContextInUse when the runtime attempts to redirect (hijack) a thread whose redirect context is already occupied from a prior redirect.

Thread State at Crash

Thread	OSID	GC Mode	Role	Activity
11	0x590	Cooperative	Crash thread	Inside 1st redirect → HandleThreadAbort → constructing ThreadAbortException → 2nd redirect fires assert
12	0xccc	Cooperative	GC thread	Running GCExtendedTests.GetTotalAllocatedBytes() → GC.GetTotalAllocatedBytesPrecise() → triggered GC
6	0x101c	Preemptive	Tiered compilation	JIT compiling, blocked in WaitUntilGCComplete
5	0x1910	Preemptive	Finalizer	Waiting for finalizer event
0	0x1ad8	Preemptive	Main	xunit runner waiting

Full Stack — Thread 11 (OSID 0x590) — Native + Managed

Shows two nested redirects — the second one fires the assert:

=== 2nd redirect (fires the assert) ===
coreclr!FailFastOnAssert
coreclr!_DbgBreakCheck
coreclr!_DbgBreakCheckNoThrow
coreclr!DbgAssertDialog
coreclr!Thread::MarkRedirectContextInUse ← 2nd redirect: context already in use!
coreclr!Thread::RedirectedHandledJITCase ← 2nd redirect handler
coreclr!Thread::RedirectedHandledJITCaseForGCThreadControl
coreclr!RedirectedHandledJITCaseForGCThreadControl_Stub ← 2nd GC hijack entry point
=== Managed code running inside 1st redirect (ThreadAbortException construction) ===
System.IO.BinaryReader.ReadByte()
System.IO.BinaryReader.Read7BitEncodedInt()
System.Resources.ResourceReader.LoadString(Int32)
System.Resources.RuntimeResourceSet.ReadValue(...)
System.Resources.RuntimeResourceSet.GetObject(...)
System.Resources.RuntimeResourceSet.GetString(...)
System.Resources.ResourceManager.GetString(...)
System.SR.InternalGetResourceString(String)
System.SR.GetResourceString(String)
System.SR.get_Arg_SystemException()
System.SystemException..ctor()
System.Threading.ThreadAbortException..ctor()
System.Runtime.CompilerServices.RuntimeHelpers.CallDefaultConstructor(...)
=== 1st redirect processing the pending ThreadAbort ===
coreclr!CallDefaultConstructor
coreclr!EEException::CreateThrowable
coreclr!CLRException::GetThrowable
coreclr!CLRException::GetThrowableFromException
coreclr!Thread::HandleThreadAbort ← processes pending TS_AbortRequested
coreclr!Thread::RestoreContextSimulated
coreclr!Thread::RedirectedHandledJITCase ← 1st redirect handler
coreclr!Thread::RedirectedHandledJITCaseForGCThreadControl
coreclr!RedirectedHandledJITCaseForGCThreadControl_Stub ← 1st GC hijack entry point
=== Original managed code when 1st redirect hijacked the thread ===
[InlinedCallFrame] System.RuntimeMethodHandle.InvokeMethod(...)
System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object, IntPtr*)
System.Reflection.RuntimeMethodInfo.Invoke(...)
System.Reflection.MethodBase.Invoke(Object, Object[])
Xunit.Sdk.TestInvoker`1.CallTestMethod(Object)
Xunit.Sdk.TestInvoker`1+<>c__DisplayClass47_0.<<InvokeTestMethodAsync>b__1>d.MoveNext()
... (xunit async test invocation pipeline) ...
Xunit.Sdk.XunitTestCase.RunAsync(IMessageSink, IMessageBus, Object[], ExceptionAggregator, CancellationTokenSource)
Microsoft.DotNet.XUnitExtensions.SkippedFactTestCase.<>n__0(...)
Microsoft.DotNet.XUnitExtensions.SkippedFactTestCase+<RunAsync>d__2.MoveNext()
... (xunit async test runner pipeline) ...
Xunit.Sdk.XunitTestAssemblyRunner+<RunTestCollectionAsync>d__18.MoveNext()
System.Threading.ExecutionContext.RunInternal(...)
... (async continuations) ...
System.Threading.Tasks.Task+CancellationPromise`1[[Boolean]].ITaskCompletionAction.Invoke(Task)
System.Threading.Tasks.CompletionActionInvoker.Execute()
System.Threading.ThreadPoolWorkQueue.Dispatch()
System.Threading.PortableThreadPool+WorkerThread.WorkerDoWork(PortableThreadPool)
System.Threading.PortableThreadPool+WorkerThread.WorkerThreadStart()
System.Threading.Thread.StartCallback(Thread*)

GC Thread Stack (Thread 12, OSID 0xccc)

System.GC.GetTotalAllocatedBytesPrecise() ← triggers GC
System.GC.GetTotalAllocatedBytes(Boolean)
System.Tests.GCExtendedTests.GetTotalAllocatedBytes() ← the test
... (xunit async pipeline via reflection invoke) ...
System.Threading.PortableThreadPool+WorkerThread.WorkerDoWork(PortableThreadPool)
System.Threading.PortableThreadPool+WorkerThread.WorkerThreadStart()
System.Threading.Thread.StartCallback(Thread*)

Sequence of Events

1. Thread 12 runs GCExtendedTests.GetTotalAllocatedBytes() → GC.GetTotalAllocatedBytesPrecise() → triggers a GC collection
2. GC suspension begins. Thread 11 is in Cooperative mode running a test via SkippedFactTestCase.RunAsync. The runtime redirects (hijacks) Thread 11 via RedirectedHandledJITCaseForGCThreadControl_Stub (1st redirect)
3. Inside RedirectedHandledJITCase → RestoreContextSimulated, the runtime calls Thread::HandleThreadAbort — a pending thread abort (TS_AbortRequested) was set on Thread 11. The source of the abort is not captured in the dump. On modern .NET (6+), Thread.Abort() throws PlatformNotSupportedException; thread aborts can only be initiated by the runtime internally or via ControlledExecution.Run
4. HandleThreadAbort → CreateThrowable → CallDefaultConstructor for ThreadAbortException — this runs managed code to construct the exception (loading the resource string "Arg_SystemException" via SR.GetResourceString → ResourceReader.LoadString → BinaryReader.ReadByte)
5. The managed code runs in Cooperative mode, making Thread 11 eligible for GC hijacking again
6. A second GC triggers (or GC suspension retries). The runtime attempts to redirect Thread 11 again via RedirectedHandledJITCaseForGCThreadControl_Stub (2nd redirect)
7. MarkRedirectContextInUse finds m_RedirectContextInUse == true (still held from step 2) → assert fires

Key Finding

There is no separate cancellation thread visible in the dump. The ThreadAbort was a pending flag (TS_AbortRequested) already set on Thread 11. The source of the abort is not captured in the dump — on modern .NET, thread aborts can come from ControlledExecution.Run or other runtime-internal mechanisms. Thread::HandleThreadAbort processes this flag as part of the 1st redirect handler's normal flow (RestoreContextSimulated → HandleThreadAbort). The bug is that the 1st redirect handler runs managed code (exception construction) while still holding the redirect context, creating a window where a 2nd redirect can be attempted.

Root Cause

A double-redirect race condition on x86: the runtime hijacks a thread for GC suspension, processes a pending ThreadAbort inside the redirect handler, and the ThreadAbortException constructor runs managed code (resource string loading) in Cooperative mode. This makes the thread eligible for another GC redirect while the first redirect context is still held.

Why This Hasn't Been Seen Before

• Libraries Test Run checked coreclr windows x86 Debug only runs on PR builds, not main rolling builds
• Main rolling builds run Libraries Test Run checked coreclr windows x86 **Release** — Release libraries are faster, narrowing the timing window
• Debug libraries' slower execution (resource loading, extra checks) widens the race window between the first redirect and the second attempt
• The m_RedirectContextInUse assert only fires in Checked builds

Relation to PR #127595

Not related. The gc_unwind_x86.inl fix changes GC reporting logic inside EnumGcRefsX86. The Validate() change in interface.cpp adds a Debug assert in Promote. Neither affects thread redirect/hijack logic. This assert passed on the previous build of the same PR (build 1403154), confirming it's intermittent.