Skip to content

Commit

Permalink
Fix bug for small objects > 256 bytes not being collected by Heap.Col…
Browse files Browse the repository at this point in the history
…lect

Fix bug with reading incorrect bytes for GC count
The issue would trigger all objects size larger than 256 bytes to never be collected
Switched to explicit typing to make such errors easier to catch in future
Added additional behaviour to catch this case in future
  • Loading branch information
quajak committed Apr 25, 2023
1 parent ad8a556 commit 532b4e7
Show file tree
Hide file tree
Showing 7 changed files with 79 additions and 87 deletions.
2 changes: 1 addition & 1 deletion Tests/Cosmos.TestRunner.Core/Engine.Helpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ private void RunIL2CPU(string kernelFileName, string outputFile)
throw new Exception("Cannot run multiple kernels with in-process compilation!");
}

xArgs.Add("AllowComments:true");
//xArgs.Add("AllowComments:true"); // enable this line when debugging cosmos

RunIL2CPUInProc(xArgs.ToArray(), OutputHandler.LogMessage, OutputHandler.LogError);
}
Expand Down
6 changes: 3 additions & 3 deletions Tests/Cosmos.TestRunner.Full/DefaultEngineConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@ public virtual IEnumerable<RunTargetEnum> RunTargets
}
}

public virtual bool RunWithGDB => true;
public virtual bool StartBochsDebugGUI => true;
public virtual bool RunWithGDB => false;
public virtual bool StartBochsDebugGUI => false;

public virtual bool DebugIL2CPU => true;
public virtual bool DebugIL2CPU => false;
public virtual string KernelPkg => String.Empty;
public virtual TraceAssemblies TraceAssembliesLevel => TraceAssemblies.User;
public virtual bool EnableStackCorruptionChecks => true;
Expand Down
38 changes: 19 additions & 19 deletions Tests/Cosmos.TestRunner.Full/TestKernelSets.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,27 +15,27 @@ public static IEnumerable<Type> GetKernelTypesToRun()
// Stable kernel types: the ones that are stable and will run in AppVeyor
public static IEnumerable<Type> GetStableKernelTypes()
{
//yield return typeof(BoxingTests.Kernel);
yield return typeof(BoxingTests.Kernel);
yield return typeof(Compiler.Tests.TypeSystem.Kernel);
//yield return typeof(Compiler.Tests.Bcl.Kernel);
//yield return typeof(Compiler.Tests.Bcl.System.Kernel);
//yield return typeof(Cosmos.Compiler.Tests.Encryption.Kernel);
//yield return typeof(Compiler.Tests.Exceptions.Kernel);
//yield return typeof(Compiler.Tests.MethodTests.Kernel);
//yield return typeof(Compiler.Tests.SingleEchoTest.Kernel);
//yield return typeof(Kernel.Tests.Fat.Kernel);
//yield return typeof(Kernel.Tests.IO.Kernel);
//yield return typeof(SimpleStructsAndArraysTest.Kernel);
//yield return typeof(Kernel.Tests.DiskManager.Kernel);
yield return typeof(Compiler.Tests.Bcl.Kernel);
yield return typeof(Compiler.Tests.Bcl.System.Kernel);
yield return typeof(Cosmos.Compiler.Tests.Encryption.Kernel);
yield return typeof(Compiler.Tests.Exceptions.Kernel);
yield return typeof(Compiler.Tests.MethodTests.Kernel);
yield return typeof(Compiler.Tests.SingleEchoTest.Kernel);
yield return typeof(Kernel.Tests.Fat.Kernel);
yield return typeof(Kernel.Tests.IO.Kernel);
yield return typeof(SimpleStructsAndArraysTest.Kernel);
yield return typeof(Kernel.Tests.DiskManager.Kernel);

////yield return typeof(KernelGen3.Boot);
//yield return typeof(GraphicTest.Kernel);
//yield return typeof(NetworkTest.Kernel);
//yield return typeof(AudioTests.Kernel);
//// Please see the notes on the kernel itself before enabling it
//yield return typeof(ConsoleTest.Kernel);
//yield return typeof(MemoryOperationsTest.Kernel);
//yield return typeof(ProcessorTests.Kernel);
//yield return typeof(KernelGen3.Boot);
yield return typeof(GraphicTest.Kernel);
yield return typeof(NetworkTest.Kernel);
yield return typeof(AudioTests.Kernel);
// Please see the notes on the kernel itself before enabling it
yield return typeof(ConsoleTest.Kernel);
yield return typeof(MemoryOperationsTest.Kernel);
yield return typeof(ProcessorTests.Kernel);
}
}
}
88 changes: 44 additions & 44 deletions Tests/Kernels/Cosmos.Compiler.Tests.TypeSystem/Kernel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -201,16 +201,16 @@ class ClassWithStruct
public void RealMethodsTest()
{
Heap.Collect();
//int allocated = HeapSmall.GetAllocatedObjectCount();
//TestMethod6();
//Heap.Collect();
//int nowAllocated = HeapSmall.GetAllocatedObjectCount();
//Assert.AreEqual(allocated, nowAllocated, "Concentating and writing strings does not leak objects");

int allocated = HeapSmall.GetAllocatedObjectCount();
TestMethod7();
TestMethod6();
Heap.Collect();
int nowAllocated = HeapSmall.GetAllocatedObjectCount();
Assert.AreEqual(allocated, nowAllocated, "Concentating and writing strings does not leak objects");

allocated = HeapSmall.GetAllocatedObjectCount();
TestMethod7();
Heap.Collect();
nowAllocated = HeapSmall.GetAllocatedObjectCount();
Assert.AreEqual(allocated, nowAllocated, "TestMethod7 does not leak string objects");
}

Expand All @@ -236,55 +236,55 @@ protected override void Run()
{
mDebugger.Send("Run");

//object xString = "a";
//string xString2 = "b";
object xString = "a";
string xString2 = "b";

//Assert.IsTrue(xString.GetType() == typeof(string), "GetType or typeof() works for reference types!");
//Assert.IsTrue(xString.GetType() == xString2.GetType(), "GetType or typeof() works for reference types!");
//Assert.IsTrue(xString is ICloneable, "isinst works for interfaces on reference types!");
//Assert.IsTrue(xString is IEnumerable<char>, "isinst works for generic interfaces on reference types!");
//Assert.IsFalse(xString.GetType().IsValueType, "IsValueType works for reference types!");
Assert.IsTrue(xString.GetType() == typeof(string), "GetType or typeof() works for reference types!");
Assert.IsTrue(xString.GetType() == xString2.GetType(), "GetType or typeof() works for reference types!");
Assert.IsTrue(xString is ICloneable, "isinst works for interfaces on reference types!");
Assert.IsTrue(xString is IEnumerable<char>, "isinst works for generic interfaces on reference types!");
Assert.IsFalse(xString.GetType().IsValueType, "IsValueType works for reference types!");

//IComparable<int> xNumber = 3;
IComparable<int> xNumber = 3;

//Assert.IsTrue(xNumber.GetType() == typeof(int), "GetType or typeof() works for value types!");
//Assert.IsTrue(xNumber is IConvertible, "isinst works for interfaces on value types!");
//Assert.IsTrue(xNumber is IEquatable<int>, "isinst works for generic interfaces on value types!");
Assert.IsTrue(xNumber.GetType() == typeof(int), "GetType or typeof() works for value types!");
Assert.IsTrue(xNumber is IConvertible, "isinst works for interfaces on value types!");
Assert.IsTrue(xNumber is IEquatable<int>, "isinst works for generic interfaces on value types!");

//IEnumerable<string> xEnumerable = new List<string>();
IEnumerable<string> xEnumerable = new List<string>();

//Assert.IsTrue(xEnumerable.GetType() == typeof(List<string>), "GetType or typeof() works for reference types!");
//Assert.IsTrue(xEnumerable is global::System.Collections.IEnumerable, "isinst works for interfaces on generic reference types!");
//Assert.IsTrue(xEnumerable is IList<string>, "isinst works for generic interfaces on generic reference types!");
Assert.IsTrue(xEnumerable.GetType() == typeof(List<string>), "GetType or typeof() works for reference types!");
Assert.IsTrue(xEnumerable is global::System.Collections.IEnumerable, "isinst works for interfaces on generic reference types!");
Assert.IsTrue(xEnumerable is IList<string>, "isinst works for generic interfaces on generic reference types!");

//B b = new B();
//Assert.IsTrue(b.GetType() == typeof(B), "GetType or typeof() works for custom types!");
B b = new B();
Assert.IsTrue(b.GetType() == typeof(B), "GetType or typeof() works for custom types!");

//Type baseType = b.GetType().BaseType;
//Type objectType = typeof(A);
//Assert.IsTrue(baseType == objectType, "BaseType works for custom reference types!");
//Assert.IsTrue(b.GetType().BaseType == new B().GetType().BaseType, "BaseType works for custom reference types!");
//Assert.IsTrue(b.GetType().IsSubclassOf(typeof(A)), "IsSubClassOf works for custom reference types!");
Type baseType = b.GetType().BaseType;
Type objectType = typeof(A);
Assert.IsTrue(baseType == objectType, "BaseType works for custom reference types!");
Assert.IsTrue(b.GetType().BaseType == new B().GetType().BaseType, "BaseType works for custom reference types!");
Assert.IsTrue(b.GetType().IsSubclassOf(typeof(A)), "IsSubClassOf works for custom reference types!");

//byte xByte = 1;
//Assert.IsTrue(xByte.GetType() == typeof(byte), "GetType or typeof() works for value types!");
//Assert.IsTrue(xByte.GetType().IsSubclassOf(typeof(ValueType)), "IsSubClassOf works for value types!");
//Assert.IsTrue(xByte.GetType().IsValueType, "IsValueType works for value types!");
byte xByte = 1;
Assert.IsTrue(xByte.GetType() == typeof(byte), "GetType or typeof() works for value types!");
Assert.IsTrue(xByte.GetType().IsSubclassOf(typeof(ValueType)), "IsSubClassOf works for value types!");
Assert.IsTrue(xByte.GetType().IsValueType, "IsValueType works for value types!");

//Action a = () => { };
//Action<int> a1 = (i) => test++;
//Assert.IsTrue(a != null, "Anonymous type for action is created correctly");
//Assert.IsTrue(a1 != null, "Anonymous type for action<int> is created correctly");
Action a = () => { };
Action<int> a1 = (i) => test++;
Assert.IsTrue(a != null, "Anonymous type for action is created correctly");
Assert.IsTrue(a1 != null, "Anonymous type for action<int> is created correctly");

//var c = new { i = 1, n = "Test" };
//Assert.IsTrue(c != null, "Anonymous types are created correctly");
//Assert.IsTrue(c.i == 1 && c.n == "Test", "Anonymous types have correct values");
var c = new { i = 1, n = "Test" };
Assert.IsTrue(c != null, "Anonymous types are created correctly");
Assert.IsTrue(c.i == 1 && c.n == "Test", "Anonymous types have correct values");

//TestVTablesImpl();
//TestGarbageCollectorMethods();
//TestGarbageCollector();
TestVTablesImpl();
TestGarbageCollectorMethods();
TestGarbageCollector();
RealMethodsTest();
//TestReflection();
TestReflection();

TestController.Completed();
}
Expand Down
13 changes: 5 additions & 8 deletions source/Cosmos.Core/Memory/Heap.cs
Original file line number Diff line number Diff line change
Expand Up @@ -183,18 +183,19 @@ public static int Collect()
while (rootSMTPtr != null)
{
uint size = rootSMTPtr->Size;
var objectSize = size + HeapSmall.PrefixItemBytes;
uint objectSize = size + HeapSmall.PrefixItemBytes;
uint objectsPerPage = RAT.PageSize / objectSize;

var smtBlock = rootSMTPtr->First;
SMTBlock* smtBlock = rootSMTPtr->First;

while (smtBlock != null)
{
var pagePtr = smtBlock->PagePtr;
byte* pagePtr = smtBlock->PagePtr;
for (int i = 0; i < objectsPerPage; i++)
{

if (*(ushort*)(pagePtr + i * objectSize + 1) > 1) // 0 means not found and 1 means marked
if (*(ushort*)(pagePtr + i * objectSize + sizeof(ushort)) > 1) // 0 means not found and 1 means marked
// after the start of the object we first have one ushort of object size and then the ushort of gc info so + 2 == sizeof(ushort)
{
MarkAndSweepObject(pagePtr + i * objectSize + HeapSmall.PrefixItemBytes);
}
Expand All @@ -220,8 +221,6 @@ public static int Collect()
currentStackPointer += 1;
}

Debugger.DoBochsBreak();

// Free all unreferenced and reset hit flag
// This means we do the same transversal as we did before of the heap
// but we done have to touch the stack again
Expand All @@ -233,8 +232,6 @@ public static int Collect()
if (pageType == (byte)RAT.PageType.HeapMedium || pageType == (byte)RAT.PageType.HeapLarge)
{
byte* pagePointer = RAT.RamStart + ratIndex * RAT.PageSize;
Debugger.DoSendNumber((int)(pagePointer + HeapLarge.PrefixBytes - 2));
Debugger.DoSendNumber(*((ushort*)(pagePointer + HeapLarge.PrefixBytes - 2)));
if (*((ushort*)(pagePointer + HeapLarge.PrefixBytes - 2)) == 0)
{
Free(pagePointer + HeapLarge.PrefixBytes);
Expand Down
12 changes: 6 additions & 6 deletions source/Cosmos.Core/Memory/HeapSmall.cs
Original file line number Diff line number Diff line change
Expand Up @@ -409,9 +409,9 @@ static void CreatePage(SMTPage* aPage, uint aItemSize)
}

//now find position in the block
var page = (ushort*)pageBlock->PagePtr;
var elementSize = GetRoundedSize(aSize) + PrefixItemBytes;
var positions = RAT.PageSize / elementSize;
ushort* page = (ushort*)pageBlock->PagePtr;
uint elementSize = GetRoundedSize(aSize) + PrefixItemBytes;
uint positions = RAT.PageSize / elementSize;
for (int i = 0; i < positions; i++)
{
if (page[i * elementSize / 2] == 0)
Expand All @@ -422,7 +422,7 @@ static void CreatePage(SMTPage* aPage, uint aItemSize)
pageBlock->SpacesLeft--;

// set info in page
var heapObject = &page[i * elementSize / 2];
ushort* heapObject = &page[i * elementSize / 2];
heapObject[0] = aSize; // size of actual object being allocated
heapObject[1] = 0; // gc status starts as 0

Expand Down Expand Up @@ -542,8 +542,8 @@ private static int GetAllocatedObjectCount(SMTPage* aPage)
/// <returns></returns>
private static int GetAllocatedObjectCount(SMTPage* aPage, uint aSize)
{
var root = GetFirstBlock(aPage, aSize);
var ptr = root->First;
RootSMTBlock* root = GetFirstBlock(aPage, aSize);
SMTBlock* ptr = root->First;

uint size = root->Size;
int count = 0;
Expand Down
7 changes: 1 addition & 6 deletions source/Cosmos.Core/Memory/RAT.cs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public enum PageType
/// Debug flag.
/// </summary>
/// <remarks>Used to bypass certain checks that will fail during tests and debugging.</remarks>
static internal bool Debug = true;
static internal bool Debug = false;

/// <summary>
/// Native Intel page size.
Expand Down Expand Up @@ -114,11 +114,6 @@ public enum PageType
/// </exception>
public static void Init(byte* aStartPtr, uint aSize)
{
Debugger.DoSendNumber(0x1010101);
Debugger.DoSendNumber((int)aStartPtr);
Debugger.DoSendNumber((int)aSize);
Debugger.DoBochsBreak();

if ((uint)aStartPtr % PageSize != 0 && !Debug)
{
Debugger.DoSendNumber((uint)aStartPtr % PageSize);
Expand Down

0 comments on commit 532b4e7

Please sign in to comment.