Skip to content

Commit

Permalink
Fix allocation of empty array in the frozen heap for collectible types (
Browse files Browse the repository at this point in the history
#100437)

Add assert for collectible in FrozenObjectHeapManager::TryAllocateObject

Fix another potential case

Add test

Fix test

Fix test bis

Fix test bis bis
  • Loading branch information
xoofx committed Mar 30, 2024
1 parent b7d91f2 commit 81578a1
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 3 deletions.
1 change: 1 addition & 0 deletions src/coreclr/vm/frozenobjectheap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
8 changes: 5 additions & 3 deletions src/coreclr/vm/gchelpers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -513,9 +513,11 @@ OBJECTREF TryAllocateFrozenSzArray(MethodTable* pArrayMT, INT32 cElements)

// The initial validation is copied from AllocateSzArray impl

if (pArrayMT->ContainsPointers() && cElements > 0)
if (pArrayMT->Collectible() || (pArrayMT->ContainsPointers() && cElements > 0))
{
// For arrays with GC pointers we can only work with empty arrays
// We cannot allocate in the frozen heap if:
// - the array type is collectible
// - or for non empty arrays with GC pointers
return NULL;
}

Expand Down Expand Up @@ -1122,7 +1124,7 @@ OBJECTREF TryAllocateFrozenObject(MethodTable* pObjMT)

SetTypeHandleOnThreadForAlloc(TypeHandle(pObjMT));

if (pObjMT->ContainsPointers() || pObjMT->IsComObjectType())
if (pObjMT->Collectible() || pObjMT->ContainsPointers() || pObjMT->IsComObjectType())
{
return NULL;
}
Expand Down
45 changes: 45 additions & 0 deletions src/tests/JIT/Regression/JitBlue/Runtime_100437/Runtime_100437.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
// 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 System.Runtime.Loader;
using Xunit;

public class Runtime_100437
{
[Fact]
public static int TestCollectibleEmptyArrayNotInFrozenHeap()
{
WeakReference[] wrs = new WeakReference[10];
for (int i = 0; i < wrs.Length; i++)
{
var alc = new MyAssemblyLoadContext();
var a = alc.LoadFromAssemblyPath(typeof(Runtime_100437).Assembly.Location);
wrs[i] = (WeakReference)a.GetType("Runtime_100437").GetMethod("Work").Invoke(null, null);
GC.Collect();
}

int result = 0;
foreach (var wr in wrs)
{
// This is testing that the empty array from Work(), if collected, should not have been allocated on the Frozen heap
// otherwise it will result in a random crash.
result += wr.Target?.ToString()?.GetHashCode() ?? 0;
}
return result;
}

public static WeakReference Work()
{
return new WeakReference(Array.Empty<Runtime_100437>());
}

private class MyAssemblyLoadContext : AssemblyLoadContext
{
public MyAssemblyLoadContext()
: base(isCollectible: true)
{
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<Optimize>True</Optimize>
</PropertyGroup>
<ItemGroup>
<Compile Include="$(MSBuildProjectName).cs" />
</ItemGroup>
</Project>

0 comments on commit 81578a1

Please sign in to comment.