Skip to content

Commit db91a6c

Browse files
github-actions[bot]janvorlijkotasCopilot
authored
[release/10.0] Fix NativeAOT GC hole issue (#129711)
Backport of #129598 to release/10.0 /cc @janvorli ## Customer Impact - [ ] Customer reported - [x] Found internally There is a GC hole in NativeAOT. As any other GC hole, it could lead to intermittent failures of applications due to unexpected NullReferenceException, AccessViolationException or just unexpected behavior. This GC hole occurs in some cases when GC scans stack with active exception handling when an exception thrown from a call chain of a funclet escapes the funclet and GC occurs when a finally handler of that secondary exception is being executed. ## Regression - [x] Yes - [ ] No Introduced by #115284 in .NET 10.0 ## Testing Libraries test that exposed the issue, directed regression test, CI coreclr and libraries tests. ## Risk Low. The change just ensures that a modified non-volatile register value is saved in the stack frame iterator of the pending exception handling, keeping that value up to date in case GC moves it. --------- Co-authored-by: Jan Vorlicek <janvorli@microsoft.com> Co-authored-by: Jan Kotas <jkotas@microsoft.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
1 parent 2f718e2 commit db91a6c

5 files changed

Lines changed: 152 additions & 1 deletion

File tree

‎docs/design/coreclr/botr/clr-abi.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -382,7 +382,7 @@ When a funclet finishes execution, and the VM returns execution to the function
382382

383383
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.
384384

385-
Funclets are not required to preserve non-volatile registers.
385+
Funclets are not required to preserve non-volatile registers that are saved by the main method body.
386386

387387
# EH Info, GC Info, and Hot & Cold Splitting
388388

‎src/coreclr/nativeaot/Runtime/amd64/ExceptionHandling.S‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -439,6 +439,21 @@ NESTED_ENTRY RhpCallFinallyFunclet, _TEXT, NoHandler
439439

440440
ALTERNATE_ENTRY RhpCallFinallyFunclet2
441441

442+
mov rsi, [rsp + locArg1] // rsi <- regdisplay
443+
444+
mov rax, [rsi + OFFSETOF__REGDISPLAY__pRbx]
445+
mov [rax] , rbx
446+
mov rax, [rsi + OFFSETOF__REGDISPLAY__pRbp]
447+
mov [rax] , rbp
448+
mov rax, [rsi + OFFSETOF__REGDISPLAY__pR12]
449+
mov [rax] , r12
450+
mov rax, [rsi + OFFSETOF__REGDISPLAY__pR13]
451+
mov [rax] , r13
452+
mov rax, [rsi + OFFSETOF__REGDISPLAY__pR14]
453+
mov [rax] , r14
454+
mov rax, [rsi + OFFSETOF__REGDISPLAY__pR15]
455+
mov [rax] , r15
456+
442457
mov rax, [rsp + locThread] // rax <- Thread*
443458
lock or dword ptr [rax + OFFSETOF__Thread__m_ThreadStateFlags], TSF_DoNotTriggerGc
444459

‎src/coreclr/nativeaot/Runtime/amd64/ExceptionHandling.asm‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -608,6 +608,27 @@ NESTED_ENTRY RhpCallFinallyFunclet, _TEXT
608608

609609
ALTERNATE_ENTRY RhpCallFinallyFunclet2
610610

611+
mov rdx, [rsp + rsp_offsetof_arguments + 8h] ;; rdx <- regdisplay
612+
613+
mov rax, [rdx + OFFSETOF__REGDISPLAY__pRbx]
614+
mov [rax] , rbx
615+
mov rax, [rdx + OFFSETOF__REGDISPLAY__pRbp]
616+
mov [rax] , rbp
617+
mov rax, [rdx + OFFSETOF__REGDISPLAY__pRsi]
618+
mov [rax] , rsi
619+
mov rax, [rdx + OFFSETOF__REGDISPLAY__pRdi]
620+
mov [rax] , rdi
621+
mov rax, [rdx + OFFSETOF__REGDISPLAY__pR12]
622+
mov [rax] , r12
623+
mov rax, [rdx + OFFSETOF__REGDISPLAY__pR13]
624+
mov [rax] , r13
625+
mov rax, [rdx + OFFSETOF__REGDISPLAY__pR14]
626+
mov [rax] , r14
627+
mov rax, [rdx + OFFSETOF__REGDISPLAY__pR15]
628+
mov [rax] , r15
629+
630+
;; XMM6-15 do not need copy-back into REGDISPLAY (no GC adjustment required).
631+
611632
mov rax, [rsp + rsp_offsetof_thread] ;; rax <- Thread*
612633
lock or dword ptr [rax + OFFSETOF__Thread__m_ThreadStateFlags], TSF_DoNotTriggerGc
613634

‎src/tests/Regressions/coreclr/GitHub_129010/Test129010.csproj‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
<Project Sdk="Microsoft.NET.Sdk">
2+
<PropertyGroup>
3+
<CLRTestPriority>1</CLRTestPriority>
4+
</PropertyGroup>
5+
<ItemGroup>
6+
<Compile Include="test129010.cs" />
7+
</ItemGroup>
8+
<ItemGroup>
9+
<ProjectReference Include="$(TestLibraryProjectPath)" />
10+
</ItemGroup>
11+
</Project>

‎src/tests/Regressions/coreclr/GitHub_129010/test129010.cs‎

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using System;
5+
using System.Runtime.CompilerServices;
6+
using Xunit;
7+
8+
public class Program
9+
{
10+
static Exception ex = new Exception();
11+
static byte[] data = null;
12+
static int count = 0;
13+
14+
[MethodImpl(MethodImplOptions.NoInlining)]
15+
private static void Alloc()
16+
{
17+
data = new byte[65536];
18+
if (count % 16 == 0)
19+
{
20+
// Force compacting GC
21+
GC.Collect(2, GCCollectionMode.Forced, true, true);
22+
}
23+
count++;
24+
}
25+
26+
[MethodImpl(MethodImplOptions.NoInlining)]
27+
public static void Test()
28+
{
29+
try
30+
{
31+
throw ex;
32+
}
33+
catch (Exception)
34+
{
35+
throw;
36+
}
37+
}
38+
39+
[MethodImpl(MethodImplOptions.NoInlining)]
40+
public static object Dummy(object a)
41+
{
42+
return a;
43+
}
44+
45+
[MethodImpl(MethodImplOptions.NoInlining)]
46+
public static void Finally(object a)
47+
{
48+
// This puts object a into the first nonvolatile register
49+
object b = Dummy(a);
50+
Alloc();
51+
Dummy(b);
52+
}
53+
54+
[MethodImpl(MethodImplOptions.NoInlining)]
55+
public static void Test2(object a)
56+
{
57+
try
58+
{
59+
try
60+
{
61+
Test();
62+
}
63+
finally
64+
{
65+
Finally(a);
66+
}
67+
}
68+
catch (Exception)
69+
{
70+
}
71+
}
72+
73+
74+
[MethodImpl(MethodImplOptions.NoInlining)]
75+
private static object Foo(int x)
76+
{
77+
return x.ToString();
78+
}
79+
80+
static int sum = 0;
81+
[MethodImpl(MethodImplOptions.NoInlining)]
82+
public static void Verify(object a)
83+
{
84+
sum += ((string)a).Length;
85+
}
86+
87+
[MethodImpl(MethodImplOptions.NoInlining)]
88+
public static void Test3()
89+
{
90+
// This puts object a into the first nonvolatile register
91+
object a = Foo(5);
92+
Test2(a);
93+
Verify(a);
94+
}
95+
96+
[Fact]
97+
public static void TestEntryPoint()
98+
{
99+
for (int i = 0; i < 100; i++)
100+
{
101+
Test3();
102+
}
103+
}
104+
}

0 commit comments

Comments
 (0)