From c2b2cc7de1fbc14b3cffc4b48f7e09b9c8bf92bf Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Fri, 19 Jun 2026 00:50:01 +0200 Subject: [PATCH 1/7] Fix NativeAOT GC hole issue There is a GC hole when: * an exception is rethrown from a funclet * the exception escapes that funclet * a finally is executed for this secondary exception * GC runs while the call chain of this finally is being executed * A reference in non-volatile register is pushed in a prolog of one of the functions in the finally call chain * the nonvolatile register holds a live reference up somewhere up in the call chain of the parent of the catch handler that catches the secondary exception * the nonvolatile register is not pushed anywhere between the parent of the catch and the frame where the nonvolatile register holds a live GC reference In this case, if GC relocates that reference, it is updated in the stack frame of the finally call chain, but not in the location referenced by the REGDISPLAY in the ExInfo of the secondary exception. So when we resume after catch, the stale reference is placed in the nonvolatile register and then it bubbles up the call chain until it reaches the frame where the register is supposed to hold live GC reference. The fix is to save the nonvolatile registers after returning from a finally funclet back to the location referenced by the REGDISPLAY passed to the RhpCallFinallyFunclet. Close #129010 --- .../Runtime/amd64/ExceptionHandling.S | 15 +++++++++ .../Runtime/amd64/ExceptionHandling.asm | 31 +++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/src/coreclr/nativeaot/Runtime/amd64/ExceptionHandling.S b/src/coreclr/nativeaot/Runtime/amd64/ExceptionHandling.S index d2bce874cecaca..9aa0aabcf2b096 100644 --- a/src/coreclr/nativeaot/Runtime/amd64/ExceptionHandling.S +++ b/src/coreclr/nativeaot/Runtime/amd64/ExceptionHandling.S @@ -439,6 +439,21 @@ NESTED_ENTRY RhpCallFinallyFunclet, _TEXT, NoHandler ALTERNATE_ENTRY RhpCallFinallyFunclet2 + mov rsi, [rsp + locArg1] // rsi <- regdisplay + + mov rax, [rsi + OFFSETOF__REGDISPLAY__pRbx] + mov [rax] , rbx + mov rax, [rsi + OFFSETOF__REGDISPLAY__pRbp] + mov [rax] , rbp + mov rax, [rsi + OFFSETOF__REGDISPLAY__pR12] + mov [rax] , r12 + mov rax, [rsi + OFFSETOF__REGDISPLAY__pR13] + mov [rax] , r13 + mov rax, [rsi + OFFSETOF__REGDISPLAY__pR14] + mov [rax] , r14 + mov rax, [rsi + OFFSETOF__REGDISPLAY__pR15] + mov [rax] , r15 + mov rax, [rsp + locThread] // rax <- Thread* lock or dword ptr [rax + OFFSETOF__Thread__m_ThreadStateFlags], TSF_DoNotTriggerGc diff --git a/src/coreclr/nativeaot/Runtime/amd64/ExceptionHandling.asm b/src/coreclr/nativeaot/Runtime/amd64/ExceptionHandling.asm index 9efa43b41114fe..071cbec53d1693 100644 --- a/src/coreclr/nativeaot/Runtime/amd64/ExceptionHandling.asm +++ b/src/coreclr/nativeaot/Runtime/amd64/ExceptionHandling.asm @@ -608,6 +608,37 @@ NESTED_ENTRY RhpCallFinallyFunclet, _TEXT ALTERNATE_ENTRY RhpCallFinallyFunclet2 + mov rdx, [rsp + rsp_offsetof_arguments + 8h] ;; rdx <- regdisplay + + mov rax, [rdx + OFFSETOF__REGDISPLAY__pRbx] + mov [rax] , rbx + mov rax, [rdx + OFFSETOF__REGDISPLAY__pRbp] + mov [rax] , rbp + mov rax, [rdx + OFFSETOF__REGDISPLAY__pRsi] + mov [rax] , rsi + mov rax, [rdx + OFFSETOF__REGDISPLAY__pRdi] + mov [rax] , rdi + mov rax, [rdx + OFFSETOF__REGDISPLAY__pR12] + mov [rax] , r12 + mov rax, [rdx + OFFSETOF__REGDISPLAY__pR13] + mov [rax] , r13 + mov rax, [rdx + OFFSETOF__REGDISPLAY__pR14] + mov [rax] , r14 + mov rax, [rdx + OFFSETOF__REGDISPLAY__pR15] + mov [rax] , r15 + + movdqa [rdx + OFFSETOF__REGDISPLAY__Xmm + 0*10h], xmm6 + movdqa [rdx + OFFSETOF__REGDISPLAY__Xmm + 1*10h], xmm7 + movdqa [rdx + OFFSETOF__REGDISPLAY__Xmm + 2*10h], xmm8 + movdqa [rdx + OFFSETOF__REGDISPLAY__Xmm + 3*10h], xmm9 + movdqa [rdx + OFFSETOF__REGDISPLAY__Xmm + 4*10h], xmm10 + + movdqa [rdx + OFFSETOF__REGDISPLAY__Xmm + 5*10h], xmm11 + movdqa [rdx + OFFSETOF__REGDISPLAY__Xmm + 6*10h], xmm12 + movdqa [rdx + OFFSETOF__REGDISPLAY__Xmm + 7*10h], xmm13 + movdqa [rdx + OFFSETOF__REGDISPLAY__Xmm + 8*10h], xmm14 + movdqa [rdx + OFFSETOF__REGDISPLAY__Xmm + 9*10h], xmm15 + mov rax, [rsp + rsp_offsetof_thread] ;; rax <- Thread* lock or dword ptr [rax + OFFSETOF__Thread__m_ThreadStateFlags], TSF_DoNotTriggerGc From 2cc16de0f4c4d87d31081b1485a9610cf9b41b58 Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Fri, 19 Jun 2026 10:43:11 +0200 Subject: [PATCH 2/7] Update ABI doc --- docs/design/coreclr/botr/clr-abi.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/design/coreclr/botr/clr-abi.md b/docs/design/coreclr/botr/clr-abi.md index af53e558712b5c..0b0d4f9bda5cf9 100644 --- a/docs/design/coreclr/botr/clr-abi.md +++ b/docs/design/coreclr/botr/clr-abi.md @@ -382,7 +382,7 @@ When a funclet finishes execution, and the VM returns execution to the function Any register value changes made in the funclet are lost. If a funclet wants to make a variable change known to the main function (or the funclet that contains the "try" region), that variable change needs to be made to the shared main function stack frame. This not a fundamental limitation. If necessary, the runtime can be updated to preserve non-volatile register changes made in funclets. -Funclets are not required to preserve non-volatile registers. +Funclets are not required to preserve non-volatile registers that are saved by main method body. # EH Info, GC Info, and Hot & Cold Splitting From d6bb11ccea830872f73009fa341ec006e5fc892c Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Fri, 19 Jun 2026 18:45:29 +0200 Subject: [PATCH 3/7] Regression test --- .../coreclr/GitHub_129010/Test129010.csproj | 12 ++ .../coreclr/GitHub_129010/test129010.cs | 103 ++++++++++++++++++ 2 files changed, 115 insertions(+) create mode 100644 src/tests/Regressions/coreclr/GitHub_129010/Test129010.csproj create mode 100644 src/tests/Regressions/coreclr/GitHub_129010/test129010.cs diff --git a/src/tests/Regressions/coreclr/GitHub_129010/Test129010.csproj b/src/tests/Regressions/coreclr/GitHub_129010/Test129010.csproj new file mode 100644 index 00000000000000..101d1bb49d1556 --- /dev/null +++ b/src/tests/Regressions/coreclr/GitHub_129010/Test129010.csproj @@ -0,0 +1,12 @@ + + + 1 + true + + + + + + + + diff --git a/src/tests/Regressions/coreclr/GitHub_129010/test129010.cs b/src/tests/Regressions/coreclr/GitHub_129010/test129010.cs new file mode 100644 index 00000000000000..67bcfe7c5eb02e --- /dev/null +++ b/src/tests/Regressions/coreclr/GitHub_129010/test129010.cs @@ -0,0 +1,103 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Runtime.CompilerServices; +using Xunit; + +public class Program +{ + static Exception ex = new Exception(); + static byte[] data = null; + static int count = 0; + + [MethodImpl(MethodImplOptions.NoInlining)] + private static void Alloc() + { + data = new byte[65536]; + if (count % 256 == 255) + { + GC.Collect(); + } + count++; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + public static void Test() + { + try + { + throw ex; + } + catch (Exception) + { + throw; + } + } + + [MethodImpl(MethodImplOptions.NoInlining)] + public static object Dummy(object a) + { + return a; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + public static void Finally(object a) + { + // This puts object a into the first nonvolatile register + object b = Dummy(a); + Alloc(); + Dummy(b); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + public static void Test2(object a) + { + try + { + try + { + Test(); + } + finally + { + Finally(a); + } + } + catch (Exception) + { + } + } + + + [MethodImpl(MethodImplOptions.NoInlining)] + private static object Foo(int x) + { + return x.ToString(); + } + + static int sum = 0; + [MethodImpl(MethodImplOptions.NoInlining)] + public static void Verify(object a) + { + sum += ((string)a).Length; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + public static void Test3() + { + // This puts object a into the first nonvolatile register + object a = Foo(5); + Test2(a); + Verify(a); + } + + [Fact] + public static void TestEntryPoint() + { + for (int i = 0; i < 1000000; i++) + { + Test3(); + } + } +} From 8e10db71e4e9c566cf168d20257e28a675664372 Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Fri, 19 Jun 2026 19:14:04 +0200 Subject: [PATCH 4/7] Update src/tests/Regressions/coreclr/GitHub_129010/Test129010.csproj Co-authored-by: Jan Kotas --- src/tests/Regressions/coreclr/GitHub_129010/Test129010.csproj | 1 - 1 file changed, 1 deletion(-) diff --git a/src/tests/Regressions/coreclr/GitHub_129010/Test129010.csproj b/src/tests/Regressions/coreclr/GitHub_129010/Test129010.csproj index 101d1bb49d1556..917fdfe044e22e 100644 --- a/src/tests/Regressions/coreclr/GitHub_129010/Test129010.csproj +++ b/src/tests/Regressions/coreclr/GitHub_129010/Test129010.csproj @@ -1,7 +1,6 @@ 1 - true From 794ab4a272cffe7c8355e1670647caa09cedc228 Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Fri, 19 Jun 2026 19:57:31 +0200 Subject: [PATCH 5/7] Apply suggestions from code review Co-authored-by: Jan Kotas --- src/tests/Regressions/coreclr/GitHub_129010/test129010.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/tests/Regressions/coreclr/GitHub_129010/test129010.cs b/src/tests/Regressions/coreclr/GitHub_129010/test129010.cs index 67bcfe7c5eb02e..10fe1165e3d19c 100644 --- a/src/tests/Regressions/coreclr/GitHub_129010/test129010.cs +++ b/src/tests/Regressions/coreclr/GitHub_129010/test129010.cs @@ -15,9 +15,10 @@ public class Program private static void Alloc() { data = new byte[65536]; - if (count % 256 == 255) + if (count % 16 == 0) { - GC.Collect(); + // Force compacting GC + GC.Collect(2, GCCollectionMode.Forced, true, true); } count++; } @@ -95,7 +96,7 @@ public static void Test3() [Fact] public static void TestEntryPoint() { - for (int i = 0; i < 1000000; i++) + for (int i = 0; i < 100; i++) { Test3(); } From d4a95a2b5f5913d28e1a0501558f3b1134d1b868 Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Fri, 19 Jun 2026 20:35:07 +0200 Subject: [PATCH 6/7] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../nativeaot/Runtime/amd64/ExceptionHandling.asm | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/src/coreclr/nativeaot/Runtime/amd64/ExceptionHandling.asm b/src/coreclr/nativeaot/Runtime/amd64/ExceptionHandling.asm index 071cbec53d1693..ad4484e8c11262 100644 --- a/src/coreclr/nativeaot/Runtime/amd64/ExceptionHandling.asm +++ b/src/coreclr/nativeaot/Runtime/amd64/ExceptionHandling.asm @@ -627,17 +627,7 @@ ALTERNATE_ENTRY RhpCallFinallyFunclet2 mov rax, [rdx + OFFSETOF__REGDISPLAY__pR15] mov [rax] , r15 - movdqa [rdx + OFFSETOF__REGDISPLAY__Xmm + 0*10h], xmm6 - movdqa [rdx + OFFSETOF__REGDISPLAY__Xmm + 1*10h], xmm7 - movdqa [rdx + OFFSETOF__REGDISPLAY__Xmm + 2*10h], xmm8 - movdqa [rdx + OFFSETOF__REGDISPLAY__Xmm + 3*10h], xmm9 - movdqa [rdx + OFFSETOF__REGDISPLAY__Xmm + 4*10h], xmm10 - - movdqa [rdx + OFFSETOF__REGDISPLAY__Xmm + 5*10h], xmm11 - movdqa [rdx + OFFSETOF__REGDISPLAY__Xmm + 6*10h], xmm12 - movdqa [rdx + OFFSETOF__REGDISPLAY__Xmm + 7*10h], xmm13 - movdqa [rdx + OFFSETOF__REGDISPLAY__Xmm + 8*10h], xmm14 - movdqa [rdx + OFFSETOF__REGDISPLAY__Xmm + 9*10h], xmm15 + ;; XMM6-15 do not need copy-back into REGDISPLAY (no GC adjustment required). mov rax, [rsp + rsp_offsetof_thread] ;; rax <- Thread* lock or dword ptr [rax + OFFSETOF__Thread__m_ThreadStateFlags], TSF_DoNotTriggerGc From d36e0e3db7ca85fbdf64495dcf54e5600780a079 Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Fri, 19 Jun 2026 12:18:13 -0700 Subject: [PATCH 7/7] Apply suggestion from @jkotas --- docs/design/coreclr/botr/clr-abi.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/design/coreclr/botr/clr-abi.md b/docs/design/coreclr/botr/clr-abi.md index 0b0d4f9bda5cf9..d822a2b79fd90d 100644 --- a/docs/design/coreclr/botr/clr-abi.md +++ b/docs/design/coreclr/botr/clr-abi.md @@ -382,7 +382,7 @@ When a funclet finishes execution, and the VM returns execution to the function Any register value changes made in the funclet are lost. If a funclet wants to make a variable change known to the main function (or the funclet that contains the "try" region), that variable change needs to be made to the shared main function stack frame. This not a fundamental limitation. If necessary, the runtime can be updated to preserve non-volatile register changes made in funclets. -Funclets are not required to preserve non-volatile registers that are saved by main method body. +Funclets are not required to preserve non-volatile registers that are saved by the main method body. # EH Info, GC Info, and Hot & Cold Splitting