From 4bcc32b6e5a4614fde28537725d336e9988c3057 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 67b05bc9ab77d4..8be19fcc8086ca 100644 --- a/src/coreclr/nativeaot/Runtime/amd64/ExceptionHandling.S +++ b/src/coreclr/nativeaot/Runtime/amd64/ExceptionHandling.S @@ -464,6 +464,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 ae147ffee8e9bd..14cb0f383a9bf6 100644 --- a/src/coreclr/nativeaot/Runtime/amd64/ExceptionHandling.asm +++ b/src/coreclr/nativeaot/Runtime/amd64/ExceptionHandling.asm @@ -617,6 +617,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 25f7ccab53deea451b1e065da7972a063ef51542 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 e90af6c523de95..f9937d4baee8a9 100644 --- a/docs/design/coreclr/botr/clr-abi.md +++ b/docs/design/coreclr/botr/clr-abi.md @@ -385,7 +385,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 1936ed2b3cd63423f90d3355aa1014fcc790eded 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 763308996b7e63f3a43ef6c12edb211d2db28453 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 1b449210e50af9281c42a13c1415577f5a9d2583 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 1f0c5f415932f132dfe117e04e0f59a95630f2d8 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 14cb0f383a9bf6..054bf2256bfcdb 100644 --- a/src/coreclr/nativeaot/Runtime/amd64/ExceptionHandling.asm +++ b/src/coreclr/nativeaot/Runtime/amd64/ExceptionHandling.asm @@ -636,17 +636,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 b839e42d9cc4bc9dc7379a7c4d77cbd49ca57cce 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 f9937d4baee8a9..0539c9bad99792 100644 --- a/docs/design/coreclr/botr/clr-abi.md +++ b/docs/design/coreclr/botr/clr-abi.md @@ -385,7 +385,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