Skip to content

Commit

Permalink
Fix FOH for SOS (#76586)
Browse files Browse the repository at this point in the history
The issue that FOH reported sizeof(ObjHeader)-bytes less than needed in UpdateFrozenSegment. It used to report the actual end of the last object while it's expected to be actual end + sizeof(ObjHeader) (for next object)
  • Loading branch information
EgorBo authored Oct 5, 2022
1 parent c27f486 commit 3e89264
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 8 deletions.
25 changes: 17 additions & 8 deletions src/coreclr/vm/frozenobjectheap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ FrozenObjectSegment::FrozenObjectSegment():
}

m_pStart = static_cast<uint8_t*>(committedAlloc);
m_pCurrent = m_pStart;
m_pCurrent = m_pStart + sizeof(ObjHeader);
m_SizeCommitted = si.ibCommit;
INDEBUG(m_ObjectsCount = 0);
return;
Expand All @@ -136,18 +136,27 @@ Object* FrozenObjectSegment::TryAllocateObject(PTR_MethodTable type, size_t obje
{
_ASSERT(m_pStart != nullptr && FOH_SEGMENT_SIZE > 0 && m_SegmentHandle != nullptr); // Expected to be inited
_ASSERT(IS_ALIGNED(m_pCurrent, DATA_ALIGNMENT));
_ASSERT(objectSize <= FOH_COMMIT_SIZE);
_ASSERT(m_pCurrent >= m_pStart + sizeof(ObjHeader));

uint8_t* obj = m_pCurrent;
if (reinterpret_cast<size_t>(m_pStart + FOH_SEGMENT_SIZE) < reinterpret_cast<size_t>(obj + objectSize))
const size_t spaceUsed = (size_t)(m_pCurrent - m_pStart);
const size_t spaceLeft = FOH_SEGMENT_SIZE - spaceUsed;

_ASSERT(spaceUsed >= sizeof(ObjHeader));
_ASSERT(spaceLeft >= sizeof(ObjHeader));

// Test if we have a room for the given object (including extra sizeof(ObjHeader) for next object)
if (spaceLeft - sizeof(ObjHeader) < objectSize)
{
// Segment is full
return nullptr;
}

// Check if we need to commit a new chunk
if (reinterpret_cast<size_t>(m_pStart + m_SizeCommitted) < reinterpret_cast<size_t>(obj + objectSize))
if (spaceUsed + objectSize + sizeof(ObjHeader) > m_SizeCommitted)
{
// Make sure we don't go out of bounds during this commit
_ASSERT(m_SizeCommitted + FOH_COMMIT_SIZE <= FOH_SEGMENT_SIZE);

if (ClrVirtualAlloc(m_pStart + m_SizeCommitted, FOH_COMMIT_SIZE, MEM_COMMIT, PAGE_READWRITE) == nullptr)
{
ClrVirtualFree(m_pStart, 0, MEM_RELEASE);
Expand All @@ -158,11 +167,11 @@ Object* FrozenObjectSegment::TryAllocateObject(PTR_MethodTable type, size_t obje

INDEBUG(m_ObjectsCount++);

m_pCurrent = obj + objectSize;

Object* object = reinterpret_cast<Object*>(obj + sizeof(ObjHeader));
Object* object = reinterpret_cast<Object*>(m_pCurrent);
object->SetMethodTable(type);

m_pCurrent += objectSize;

// Notify GC that we bumped the pointer and, probably, committed more memory in the reserved part
GCHeapUtilities::GetGCHeap()->UpdateFrozenSegment(m_SegmentHandle, m_pCurrent, m_pStart + m_SizeCommitted);

Expand Down
11 changes: 11 additions & 0 deletions src/coreclr/vm/frozenobjectheap.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,20 @@ class FrozenObjectSegment
Object* TryAllocateObject(PTR_MethodTable type, size_t objectSize);

private:
// Start of the reserved memory, the first object starts at "m_pStart + sizeof(ObjHeader)" (its pMT)
uint8_t* m_pStart;

// Pointer to the end of the current segment, ready to be used as a pMT for a new object
// meaning that "m_pCurrent - sizeof(ObjHeader)" is the actual start of the new object (header).
//
// m_pCurrent <= m_SizeCommitted
uint8_t* m_pCurrent;

// Memory committed in the current segment
//
// m_SizeCommitted <= m_pStart + FOH_SIZE_RESERVED
size_t m_SizeCommitted;

segment_handle m_SegmentHandle;
INDEBUG(size_t m_ObjectsCount);
};
Expand Down

0 comments on commit 3e89264

Please sign in to comment.