Skip to content

Commit

Permalink
Move metadata off the executable heaps (#52912)
Browse files Browse the repository at this point in the history
* Move metadata off the executable heaps

This change moves metadata structures that manage blocks of heap memory
out of the heaps in preparation for the W^X changes that will make the
heap memory read-execute only and modifying the metadata would require
unnecessary mappings and unmappings of the memory as read-write.

The structures moved in this change are the following:
* LoaderHeapBlock
* FreeBlock
* HeapList

* Remove unnecessary m_pCurBlock from the UnlockedLoaderHeap
  • Loading branch information
janvorli committed May 20, 2021
1 parent 542ef8b commit 636f89d
Show file tree
Hide file tree
Showing 8 changed files with 148 additions and 94 deletions.
2 changes: 1 addition & 1 deletion src/coreclr/debug/daccess/fntableaccess.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ static NTSTATUS OutOfProcessFunctionTableCallback_JIT(IN ReadMemoryFunction

move(Hp, pHp);

if (pHp == MinAddress)
if (Hp.GetModuleBase() == MinAddress)
{
DWORD_PTR pThisHeader;
DWORD_PTR hdrOffset;
Expand Down
12 changes: 12 additions & 0 deletions src/coreclr/debug/daccess/fntableaccess.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,18 @@ struct FakeHeapList
DWORD_PTR pHdrMap; // changed from DWORD*
size_t maxCodeHeapSize;
size_t reserveForJumpStubs;
#if defined(TARGET_AMD64) || defined(TARGET_ARM64)
DWORD_PTR CLRPersonalityRoutine;
#endif

DWORD_PTR GetModuleBase()
{
#if defined(TARGET_AMD64) || defined(TARGET_ARM64)
return CLRPersonalityRoutine;
#else
return mapBase;
#endif
}
};

typedef struct _FakeHpRealCodeHdr
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/debug/daccess/request.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3481,7 +3481,7 @@ ClrDataAccess::TraverseLoaderHeap(CLRDATA_ADDRESS loaderHeapAddr, VISITHEAP pFun
TADDR addr = PTR_TO_TADDR(block->pVirtualAddress);
size_t size = block->dwVirtualSize;

BOOL bCurrentBlock = (block == pLoaderHeap->m_pCurBlock);
BOOL bCurrentBlock = (block == pLoaderHeap->m_pFirstBlock);

pFunc(addr,size,bCurrentBlock);

Expand Down Expand Up @@ -3538,7 +3538,7 @@ ClrDataAccess::TraverseVirtCallStubHeap(CLRDATA_ADDRESS pAppDomain, VCSHeapType
TADDR addr = PTR_TO_TADDR(block->pVirtualAddress);
size_t size = block->dwVirtualSize;

BOOL bCurrentBlock = (block == pLoaderHeap->m_pCurBlock);
BOOL bCurrentBlock = (block == pLoaderHeap->m_pFirstBlock);
pFunc(addr, size, bCurrentBlock);

block = block->pNext;
Expand Down
20 changes: 18 additions & 2 deletions src/coreclr/inc/loaderheap.h
Original file line number Diff line number Diff line change
Expand Up @@ -201,8 +201,6 @@ class UnlockedLoaderHeap
PTR_BYTE m_pPtrToEndOfCommittedRegion;
PTR_BYTE m_pEndReservedRegion;

PTR_LoaderHeapBlock m_pCurBlock;

// When we need to ClrVirtualAlloc() MEM_RESERVE a new set of pages, number of bytes to reserve
DWORD m_dwReserveBlockSize;

Expand Down Expand Up @@ -301,6 +299,12 @@ class UnlockedLoaderHeap
return m_pEndReservedRegion - m_pAllocPtr;
}

PTR_BYTE UnlockedGetAllocPtr()
{
LIMITED_METHOD_CONTRACT;
return m_pAllocPtr;
}

private:
// Get some more committed pages - either commit some more in the current reserved region, or, if it
// has run out, reserve another set of pages
Expand Down Expand Up @@ -848,6 +852,18 @@ class ExplicitControlLoaderHeap : public UnlockedLoaderHeap
WRAPPER_NO_CONTRACT;
return UnlockedGetReservedBytesFree();
}

PTR_BYTE GetAllocPtr()
{
WRAPPER_NO_CONTRACT;
return UnlockedGetAllocPtr();
}

void ReservePages(size_t size)
{
WRAPPER_NO_CONTRACT;
UnlockedReservePages(size);
}
};


Expand Down
74 changes: 30 additions & 44 deletions src/coreclr/utilcode/loaderheap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -910,7 +910,6 @@ UnlockedLoaderHeap::UnlockedLoaderHeap(DWORD dwReserveBlockSize,
}
CONTRACTL_END;

m_pCurBlock = NULL;
m_pFirstBlock = NULL;

m_dwReserveBlockSize = dwReserveBlockSize;
Expand Down Expand Up @@ -982,6 +981,8 @@ UnlockedLoaderHeap::~UnlockedLoaderHeap()
fSuccess = ClrVirtualFree(pVirtualAddress, 0, MEM_RELEASE);
_ASSERTE(fSuccess);
}

delete pSearch;
}

if (m_reservedBlock.m_fReleaseMemory)
Expand Down Expand Up @@ -1047,12 +1048,22 @@ size_t UnlockedLoaderHeap::GetBytesAvailReservedRegion()

#define SETUP_NEW_BLOCK(pData, dwSizeToCommit, dwSizeToReserve) \
m_pPtrToEndOfCommittedRegion = (BYTE *) (pData) + (dwSizeToCommit); \
m_pAllocPtr = (BYTE *) (pData) + sizeof(LoaderHeapBlock); \
m_pAllocPtr = (BYTE *) (pData); \
m_pEndReservedRegion = (BYTE *) (pData) + (dwSizeToReserve);


#ifndef DACCESS_COMPILE

void ReleaseReservedMemory(BYTE* value)
{
if (value)
{
ClrVirtualFree(value, 0, MEM_RELEASE);
}
}

using ReservedMemoryHolder = SpecializedWrapper<BYTE, ReleaseReservedMemory>;

BOOL UnlockedLoaderHeap::UnlockedReservePages(size_t dwSizeToCommit)
{
CONTRACTL
Expand All @@ -1065,21 +1076,18 @@ BOOL UnlockedLoaderHeap::UnlockedReservePages(size_t dwSizeToCommit)

size_t dwSizeToReserve;

// Add sizeof(LoaderHeapBlock)
dwSizeToCommit += sizeof(LoaderHeapBlock);

// Round to page size again
dwSizeToCommit = ALIGN_UP(dwSizeToCommit, GetOsPageSize());

void *pData = NULL;
ReservedMemoryHolder pData = NULL;
BOOL fReleaseMemory = TRUE;

// We were provided with a reserved memory block at instance creation time, so use it if it's big enough.
if (m_reservedBlock.pVirtualAddress != NULL &&
m_reservedBlock.dwVirtualSize >= dwSizeToCommit)
{
// Get the info out of the block.
pData = m_reservedBlock.pVirtualAddress;
pData = (PTR_BYTE)m_reservedBlock.pVirtualAddress;
dwSizeToReserve = m_reservedBlock.dwVirtualSize;
fReleaseMemory = m_reservedBlock.m_fReleaseMemory;

Expand Down Expand Up @@ -1126,16 +1134,17 @@ BOOL UnlockedLoaderHeap::UnlockedReservePages(size_t dwSizeToCommit)
return FALSE;
}

if (!fReleaseMemory)
{
pData.SuppressRelease();
}

// Commit first set of pages, since it will contain the LoaderHeapBlock
void *pTemp = ClrVirtualAlloc(pData, dwSizeToCommit, MEM_COMMIT, (m_Options & LHF_EXECUTABLE) ? PAGE_EXECUTE_READWRITE : PAGE_READWRITE);
if (pTemp == NULL)
{
//_ASSERTE(!"Unable to ClrVirtualAlloc commit in a loaderheap");

// Unable to commit - release pages
if (fReleaseMemory)
ClrVirtualFree(pData, 0, MEM_RELEASE);

return FALSE;
}

Expand All @@ -1147,44 +1156,27 @@ BOOL UnlockedLoaderHeap::UnlockedReservePages(size_t dwSizeToCommit)
((const BYTE *) pData) + dwSizeToReserve,
(void *) this))
{

if (fReleaseMemory)
ClrVirtualFree(pData, 0, MEM_RELEASE);

return FALSE;
}
}

m_dwTotalAlloc += dwSizeToCommit;

LoaderHeapBlock *pNewBlock;
LoaderHeapBlock *pNewBlock = new (nothrow) LoaderHeapBlock;
if (pNewBlock == NULL)
{
return FALSE;
}

#if defined(HOST_OSX) && defined(HOST_ARM64)
// Always assume we are touching executable heap
auto jitWriteEnableHolder = PAL_JITWriteEnable(true);
#endif // defined(HOST_OSX) && defined(HOST_ARM64)
m_dwTotalAlloc += dwSizeToCommit;

pNewBlock = (LoaderHeapBlock *) pData;
pData.SuppressRelease();

pNewBlock->dwVirtualSize = dwSizeToReserve;
pNewBlock->pVirtualAddress = pData;
pNewBlock->pNext = NULL;
pNewBlock->pNext = m_pFirstBlock;
pNewBlock->m_fReleaseMemory = fReleaseMemory;

LoaderHeapBlock *pCurBlock = m_pCurBlock;

// Add to linked list
while (pCurBlock != NULL &&
pCurBlock->pNext != NULL)
pCurBlock = pCurBlock->pNext;

if (pCurBlock != NULL)
m_pCurBlock->pNext = pNewBlock;
else
m_pFirstBlock = pNewBlock;

// If we want to use the memory immediately...
m_pCurBlock = pNewBlock;
// Add to the linked list
m_pFirstBlock = pNewBlock;

SETUP_NEW_BLOCK(pData, dwSizeToCommit, dwSizeToReserve);

Expand Down Expand Up @@ -1827,16 +1819,11 @@ void UnlockedLoaderHeap::DumpFreeList()
size_t dwsize = pBlock->m_dwSize;
BOOL ccbad = FALSE;
BOOL sizeunaligned = FALSE;
BOOL sizesmall = FALSE;

if ( 0 != (dwsize & ALLOC_ALIGN_CONSTANT) )
{
sizeunaligned = TRUE;
}
if ( dwsize < sizeof(LoaderHeapBlock))
{
sizesmall = TRUE;
}

for (size_t i = sizeof(LoaderHeapFreeBlock); i < dwsize; i++)
{
Expand All @@ -1850,7 +1837,6 @@ void UnlockedLoaderHeap::DumpFreeList()
printf("Addr = %pxh, Size = %lxh", pBlock, ((ULONG)dwsize));
if (ccbad) printf(" *** ERROR: NOT CC'd ***");
if (sizeunaligned) printf(" *** ERROR: size not a multiple of ALLOC_ALIGN_CONSTANT ***");
if (sizesmall) printf(" *** ERROR: size smaller than sizeof(LoaderHeapFreeBlock) ***");
printf("\n");

pBlock = pBlock->m_pNext;
Expand Down
Loading

0 comments on commit 636f89d

Please sign in to comment.