From 78f7707d18e69abc7deea01e3aef169892c196be Mon Sep 17 00:00:00 2001 From: Alexandre Mutel Date: Tue, 2 Apr 2024 07:14:35 +0200 Subject: [PATCH] Fix allocation of empty array in the frozen heap for collectible types (#100444) * Fix allocation of empty array in the frozen heap for collectible types (#100437) * Remove Optimize from csproj * Add test for generic with static * Apply suggestions from code review * Better test * Disable tests on Mono --------- Co-authored-by: Jan Kotas --- src/coreclr/jit/importer.cpp | 3 +- src/coreclr/vm/frozenobjectheap.cpp | 1 + .../JitBlue/Runtime_100437/Runtime_100437.cs | 112 ++++++++++++++++++ .../Runtime_100437/Runtime_100437.csproj | 5 + 4 files changed, 120 insertions(+), 1 deletion(-) create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_100437/Runtime_100437.cs create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_100437/Runtime_100437.csproj diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 6087b1499301c..9bf654ffc35a2 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -9393,7 +9393,8 @@ void Compiler::impImportBlockCode(BasicBlock* block) CORINFO_FIELD_INFO fi; eeGetFieldInfo(&fldToken, CORINFO_ACCESS_SET, &fi); unsigned flagsToCheck = CORINFO_FLG_FIELD_STATIC | CORINFO_FLG_FIELD_FINAL; - if ((fi.fieldFlags & flagsToCheck) == flagsToCheck) + if (((fi.fieldFlags & flagsToCheck) == flagsToCheck) && + ((info.compCompHnd->getClassAttribs(info.compClassHnd) & CORINFO_FLG_SHAREDINST) == 0)) { #ifdef FEATURE_READYTORUN if (opts.IsReadyToRun()) diff --git a/src/coreclr/vm/frozenobjectheap.cpp b/src/coreclr/vm/frozenobjectheap.cpp index 8f11f3c8c74d6..bd25b4ba7b3e3 100644 --- a/src/coreclr/vm/frozenobjectheap.cpp +++ b/src/coreclr/vm/frozenobjectheap.cpp @@ -47,6 +47,7 @@ Object* FrozenObjectHeapManager::TryAllocateObject(PTR_MethodTable type, size_t _ASSERT(type != nullptr); _ASSERT(FOH_COMMIT_SIZE >= MIN_OBJECT_SIZE); + _ASSERT(!type->Collectible()); // Currently we don't support frozen objects with special alignment requirements // TODO: We should also give up on arrays of doubles on 32-bit platforms. diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_100437/Runtime_100437.cs b/src/tests/JIT/Regression/JitBlue/Runtime_100437/Runtime_100437.cs new file mode 100644 index 0000000000000..810b99acab307 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_100437/Runtime_100437.cs @@ -0,0 +1,112 @@ +// 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.Reflection; +using System.Runtime.CompilerServices; +using System.Runtime.Loader; +using Xunit; + +public class Runtime_100437 +{ + [Fact] + [SkipOnMono("PlatformDetection.IsPreciseGcSupported false on mono", TestPlatforms.Any)] + public static void TestNonCollectibleType() => TestCollectibleReadOnlyStatics(nameof(NonCollectibleType)); + + [Fact] + [SkipOnMono("PlatformDetection.IsPreciseGcSupported false on mono", TestPlatforms.Any)] + public static void TestNonCollectibleTypeInSharedGenericCode() => TestCollectibleReadOnlyStatics(nameof(NonCollectibleTypeInSharedGenericCode)); + + [Fact] + [SkipOnMono("PlatformDetection.IsPreciseGcSupported false on mono", TestPlatforms.Any)] + public static void TestNonCollectibleArrayTypeInSharedGenericCode() => TestCollectibleReadOnlyStatics(nameof(NonCollectibleArrayTypeInSharedGenericCode)); + + [Fact] + [SkipOnMono("PlatformDetection.IsPreciseGcSupported false on mono", TestPlatforms.Any)] + public static void TestCollectibleEmptyArray() => TestCollectibleReadOnlyStatics(nameof(CollectibleEmptyArray)); + + private static void TestCollectibleReadOnlyStatics(string methodName) + { + string assemblyPath = typeof(Runtime_100437).Assembly.Location; + + // Skip this test for single file + if (string.IsNullOrEmpty(assemblyPath)) + return; + + WeakReference wr = CreateReadOnlyStaticWeakReference(); + + for (int i = 0; i < 10; i++) + { + GC.Collect(); + GC.WaitForPendingFinalizers(); + + if (!IsTargetAlive(wr)) + return; + } + + throw new Exception("Test failed - readonly static has not been collected."); + + [MethodImpl(MethodImplOptions.NoInlining)] + WeakReference CreateReadOnlyStaticWeakReference() + { + AssemblyLoadContext alc = new CollectibleAssemblyLoadContext(); + Assembly a = alc.LoadFromAssemblyPath(assemblyPath); + return (WeakReference)a.GetType(nameof(Runtime_100437)).GetMethod(methodName).Invoke(null, new object[] { typeof(Runtime_100437).Assembly }); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + bool IsTargetAlive(WeakReference wr) + { + return wr.Target != null; + } + } + + public static WeakReference NonCollectibleType(Assembly assemblyInDefaultContext) + { + return new WeakReference(Holder.Singleton, trackResurrection: true); + } + + public static WeakReference NonCollectibleTypeInSharedGenericCode(Assembly assemblyInDefaultContext) + { + // Create instance of a non-collectible generic type definition over a collectible type + var type = assemblyInDefaultContext.GetType("Runtime_100437+GenericHolder`1", throwOnError: true).MakeGenericType(typeof(Runtime_100437)); + var field = type.GetField("Singleton", BindingFlags.Static | BindingFlags.Public); + return new WeakReference(field.GetValue(null), trackResurrection: true); + } + + public static WeakReference NonCollectibleArrayTypeInSharedGenericCode(Assembly assemblyInDefaultContext) + { + // Create instance of a non-collectible generic type definition over a collectible type + var type = assemblyInDefaultContext.GetType("Runtime_100437+GenericArrayHolder`1", throwOnError: true).MakeGenericType(typeof(Runtime_100437)); + var field = type.GetField("Singleton", BindingFlags.Static | BindingFlags.Public); + return new WeakReference(field.GetValue(null), trackResurrection: true); + } + + public static WeakReference CollectibleEmptyArray(Assembly assemblyInDefaultContext) + { + return new WeakReference(Array.Empty(), trackResurrection: true); + } + + private class CollectibleAssemblyLoadContext : AssemblyLoadContext + { + public CollectibleAssemblyLoadContext() + : base(isCollectible: true) + { + } + } + + private class Holder + { + public static readonly object Singleton = new object(); + } + + private class GenericHolder + { + public static readonly object Singleton = new object(); + } + + private class GenericArrayHolder + { + public static readonly int[] Singleton = new int[0]; + } +} diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_100437/Runtime_100437.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_100437/Runtime_100437.csproj new file mode 100644 index 0000000000000..197767e2c4e24 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_100437/Runtime_100437.csproj @@ -0,0 +1,5 @@ + + + + +