Fix handling of collectible thread statics and GC (#40671)

- NonGc Thread statics helper returns a ByRef, not a pointer for collectible statics
- Add test coverage for collectible statics
- Remove test coverage for converting thread static variable into byref and using that to keep alive the static data as it wasn't fully safe itself

src/coreclr/src/jit/flowgraph.cpp

@@ -7401,6 +7401,7 @@ GenTreeCall* Compiler::fgGetStaticsCCtorHelper(CORINFO_CLASS_HANDLE cls, CorInfo
         case CORINFO_HELP_GETSHARED_NONGCSTATIC_BASE_DYNAMICCLASS:
         case CORINFO_HELP_GETSHARED_GCTHREADSTATIC_BASE:
         case CORINFO_HELP_GETSHARED_GCTHREADSTATIC_BASE_DYNAMICCLASS:
+        case CORINFO_HELP_GETSHARED_NONGCTHREADSTATIC_BASE_DYNAMICCLASS:
             // type = TYP_BYREF;
             break;
 
@@ -7414,7 +7415,6 @@ GenTreeCall* Compiler::fgGetStaticsCCtorHelper(CORINFO_CLASS_HANDLE cls, CorInfo
 
         case CORINFO_HELP_GETSHARED_NONGCSTATIC_BASE:
         case CORINFO_HELP_GETSHARED_NONGCTHREADSTATIC_BASE:
-        case CORINFO_HELP_GETSHARED_NONGCTHREADSTATIC_BASE_DYNAMICCLASS:
         case CORINFO_HELP_CLASSINIT_SHARED_DYNAMICCLASS:
             type = TYP_I_IMPL;
             break;

src/coreclr/tests/issues.targets

@@ -961,6 +961,9 @@
         <ExcludeList Include="$(XunitTestBinBase)/Loader/CollectibleAssemblies/ByRefLocals/**">
             <Issue>https://github.com/dotnet/runtime/issues/40394</Issue>
         </ExcludeList>
+        <ExcludeList Include="$(XunitTestBinBase)/Loader/CollectibleAssemblies/Statics/**">
+            <Issue>https://github.com/dotnet/runtime/issues/40394</Issue>
+        </ExcludeList>
         <ExcludeList Include="$(XunitTestBinBase)/Interop/PInvoke/Miscellaneous/CopyCtor/**">
             <Issue>Handling for Copy constructors isn't present in mono interop</Issue>
         </ExcludeList>

src/tests/Loader/CollectibleAssemblies/ByRefLocals/ByRefLocals.cs

@@ -40,15 +40,6 @@ static int Main(string[] args)
     [MethodImpl(MethodImplOptions.NoInlining)]
     static int HoldAssembliesAliveThroughByRefFields(out GCHandle gch1, out GCHandle gch2)
     {
-        // ThreadStatic lifetime check. Here we don't require the actual assembly to remain loaded, but we do require the field to remain accessible
-        var spanThreadStatic = LoadAssemblyThreadStatic(out GCHandle gchThreadStatic);
-        GC.Collect(2);
-        GC.WaitForPendingFinalizers();
-        GC.Collect(2);
-        GC.WaitForPendingFinalizers();
-        GC.Collect(2);
-        GC.WaitForPendingFinalizers();
-
         var span1 = LoadAssembly(out gch1);
         GC.Collect(2);
         GC.WaitForPendingFinalizers();
@@ -61,7 +52,6 @@ static int HoldAssembliesAliveThroughByRefFields(out GCHandle gch1, out GCHandle
         {
             Console.WriteLine(span1[0]);
             Console.WriteLine(span2[0]);
-            Console.WriteLine(spanThreadStatic[0]);
             GC.Collect(2);
             GC.WaitForPendingFinalizers();
             if (gch1.Target == null)
@@ -72,11 +62,6 @@ static int HoldAssembliesAliveThroughByRefFields(out GCHandle gch1, out GCHandle
             {
                 return 2;
             }
-            if (spanThreadStatic[0] != 7)
-            {
-                Console.WriteLine($"spanThreadStatic[0] = {spanThreadStatic[0]}");
-                return 5;
-            }
         }
 
         return 100;
@@ -96,20 +81,6 @@ private static ReadOnlySpan<byte> LoadAssembly(out GCHandle gchToAssembly)
         return spanAccessor.GetSpan();
     }
 
-    [MethodImpl(MethodImplOptions.NoInlining)]
-    private static ReadOnlySpan<byte> LoadAssemblyThreadStatic(out GCHandle gchToAssembly)
-    {
-        var alc = new AssemblyLoadContext("test", isCollectible: true);
-        var a = alc.LoadFromAssemblyPath(Path.Combine(Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location), "Unloaded.dll"));
-        gchToAssembly = GCHandle.Alloc(a, GCHandleType.WeakTrackResurrection);
-
-        var spanAccessor = (IReturnSpan)Activator.CreateInstance(a.GetType("ThreadStaticSpanAccessor"));
-
-        alc.Unload();
-
-        return spanAccessor.GetSpan();
-    }
-
     [MethodImpl(MethodImplOptions.NoInlining)]
     private static ReadOnlySpan<byte> CreateAssemblyDynamically(out GCHandle gchToAssembly)
     {

src/tests/Loader/CollectibleAssemblies/ByRefLocals/Unloaded.cs

@@ -13,14 +13,3 @@ public ReadOnlySpan<byte> GetSpan()
         return RawData;
     }
 }
-
-public class ThreadStaticSpanAccessor : IReturnSpan
-{
-    [ThreadStatic]
-    public static byte ThreadStaticByte = 7;
-
-    public unsafe ReadOnlySpan<byte> GetSpan()
-    {
-        return new ReadOnlySpan<byte>(Unsafe.AsPointer(ref ThreadStaticByte), 1);
-    }
-}

src/tests/Loader/CollectibleAssemblies/Statics/Accessor.cs

@@ -0,0 +1,12 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+
+public interface IStaticTest
+{
+    void SetStatic(int val, int val2, int val3, int val4, int val5);
+    void GetStatic(out int val1, out int val2, out int val3, out int val4, out int val5);
+    void SetStaticObject(object val, object val2, object val3, object val4, object val5);
+    void GetStaticObject(out object val1, out object val2, out object val3, out object val4, out object val5);
+}

src/tests/Loader/CollectibleAssemblies/Statics/Accessor.csproj

@@ -0,0 +1,11 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
+    <OutputType>Library</OutputType>
+    <CLRTestKind>BuildOnly</CLRTestKind>
+    <CLRTestPriority>0</CLRTestPriority>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="Accessor.cs" />
+  </ItemGroup>
+</Project>

src/tests/Loader/CollectibleAssemblies/Statics/CollectibleStatics.cs

@@ -0,0 +1,54 @@
+// 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.IO;
+using System.Reflection;
+using System.Reflection.Emit;
+using System.Runtime.CompilerServices;
+using System.Runtime.InteropServices;
+using System.Runtime.Loader;
+
+class Program
+{
+    static int Main(string[] args)
+    {
+        var alc = new AssemblyLoadContext("test", isCollectible: true);
+        var a = alc.LoadFromAssemblyPath(Path.Combine(Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location), "Unloaded.dll"));
+
+        var accessor = (IStaticTest)Activator.CreateInstance(a.GetType("StaticTest"));
+        accessor.SetStatic(12759, 548739, 5468, 8518, 9995);
+        accessor.GetStatic(out int val1, out int val2, out int val3, out int val4, out int val5);
+        if (val1 != 12759)
+            return 1;
+        if (val2 != 548739)
+            return 2;
+        if (val3 != 5468)
+            return 3;
+        if (val4 != 8518)
+            return 4;
+        if (val5 != 9995)
+            return 5;
+
+        object obj1 = new object();
+        object obj2 = new object();
+        object obj3 = new object();
+        object obj4 = new object();
+        object obj5 = new object();
+        accessor.SetStaticObject(obj1, obj2, obj3, obj4, obj5);
+        accessor.GetStaticObject(out object val1Obj, out object val2Obj, out object val3Obj, out object val4Obj, out object val5Obj);
+        if (val1Obj != obj1)
+            return 11;
+        if (val2Obj != obj2)
+            return 12;
+        if (val3Obj != obj3)
+            return 13;
+        if (val4Obj != obj4)
+            return 14;
+        if (val5Obj != obj5)
+            return 15;
+
+        GC.KeepAlive(accessor);
+        return 100;
+    }
+}

src/tests/Loader/CollectibleAssemblies/Statics/CollectibleStatics.csproj

@@ -0,0 +1,13 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
+    <OutputType>Exe</OutputType>
+    <CLRTestKind>BuildAndRun</CLRTestKind>
+    <CLRTestPriority>0</CLRTestPriority>
+  </PropertyGroup>
+  <ItemGroup>
+    <ProjectReference Include="Unloaded.csproj" />
+    <ProjectReference Include="Accessor.csproj" />
+    <Compile Include="CollectibleStatics.cs" />
+  </ItemGroup>
+</Project>

src/tests/Loader/CollectibleAssemblies/Statics/Unloaded.cs

@@ -0,0 +1,113 @@
+// 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;
+
+// Use multiple classes to trigger multiple statics allocation events
+public class StaticTest2
+{
+    [ThreadStatic]
+    public static int ThreadStaticValue;
+    public static int StaticValue;
+}
+
+public class StaticTest3
+{
+    public static int StaticValue;
+}
+
+public class StaticTest4
+{
+    [ThreadStatic]
+    public static object ThreadStaticValue;
+    public static object StaticValue;
+}
+public class StaticTest5
+{
+    [ThreadStatic]
+    public static object ThreadStaticValue;
+    public static object StaticValue;
+}
+public class StaticTest6
+{
+    public static object StaticValue;
+}
+
+
+public class StaticTest : IStaticTest
+{
+    [ThreadStatic]
+    public static int ThreadStaticValue;
+    public static int StaticValue;
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static void SetValues(out int valTargetA, int valA
+                                , out int valTargetB, int valB
+                                , out int valTargetC, int valC
+                                , out int valTargetD, int valD
+                                , out int valTargetE, int valE
+                                )
+    {
+        valTargetA = valA;
+        valTargetB = valB;
+        valTargetC = valC;
+        valTargetD = valD;
+        valTargetE = valE;
+    }
+
+    public void SetStatic(int val, int val2, int val3, int val4, int val5)
+    {
+        // Use this odd pathway to increase the chance that in the presence of GCStress issues will be found
+        SetValues(out ThreadStaticValue, val
+                , out StaticValue, val2
+                , out StaticTest2.StaticValue, val3
+                , out StaticTest2.ThreadStaticValue, val4
+                , out StaticTest3.StaticValue, val5
+                );
+    }
+
+    public void GetStatic(out int val1, out int val2, out int val3, out int val4, out int val5)
+    {
+        val1 = ThreadStaticValue;
+        val2 = StaticValue;
+        val3 = StaticTest2.StaticValue;
+        val4 = StaticTest2.ThreadStaticValue;
+        val5 = StaticTest3.StaticValue;
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static void SetValuesObject(out object valTargetA, object valA
+                                , out object valTargetB, object valB
+                                , out object valTargetC, object valC
+                                , out object valTargetD, object valD
+                                , out object valTargetE, object valE
+                                )
+    {
+        valTargetA = valA;
+        valTargetB = valB;
+        valTargetC = valC;
+        valTargetD = valD;
+        valTargetE = valE;
+    }
+
+    public void SetStaticObject(object val, object val2, object val3, object val4, object val5)
+    {
+        // Use this odd pathway to increase the chance that in the presence of GCStress issues will be found
+        SetValuesObject(out StaticTest4.ThreadStaticValue, val
+                , out StaticTest4.StaticValue, val2
+                , out StaticTest5.StaticValue, val3
+                , out StaticTest5.ThreadStaticValue, val4
+                , out StaticTest6.StaticValue, val5
+                );
+    }
+
+    public void GetStaticObject(out object val1, out object val2, out object val3, out object val4, out object val5)
+    {
+        val1 = StaticTest4.ThreadStaticValue;
+        val2 = StaticTest4.StaticValue;
+        val3 = StaticTest5.StaticValue;
+        val4 = StaticTest5.ThreadStaticValue;
+        val5 = StaticTest6.StaticValue;
+    }
+}

src/tests/Loader/CollectibleAssemblies/Statics/Unloaded.csproj

@@ -0,0 +1,12 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
+    <OutputType>Library</OutputType>
+    <CLRTestKind>BuildOnly</CLRTestKind>
+    <CLRTestPriority>0</CLRTestPriority>
+  </PropertyGroup>
+  <ItemGroup>
+    <ProjectReference Include="Accessor.csproj" />
+    <Compile Include="Unloaded.cs" />
+  </ItemGroup>
+</Project>