Streamline C heap allocations (dotnet#34289)

- Remove unnecessary layers in heap allocation APIs
- Delete HeapCreate/Alloc/Free/Destroy from the PAL

src/coreclr/src/ToolBox/superpmi/superpmi-shared/compileresult.cpp

@@ -30,8 +30,6 @@ CompileResult::CompileResult()
 
     allocGCInfoDets.retval = 0;
     allocGCInfoDets.size   = 0;
-
-    codeHeap = nullptr;
 }
 
 CompileResult::~CompileResult()
@@ -43,11 +41,6 @@ CompileResult::~CompileResult()
 
     if (CallTargetTypes != nullptr)
         delete CallTargetTypes;
-
-#ifndef TARGET_UNIX // PAL doesn't have HeapDestroy()
-    if (codeHeap != nullptr)
-        ::HeapDestroy(codeHeap);
-#endif // !TARGET_UNIX
 }
 
 // Is the CompileResult empty? Define this as whether all the maps that store information given by the JIT are empty.
@@ -64,18 +57,6 @@ bool CompileResult::IsEmpty()
     return isEmpty;
 }
 
-HANDLE CompileResult::getCodeHeap()
-{
-    if (codeHeap == nullptr)
-        codeHeap = ::HeapCreate(0, 0, 0);
-    if (codeHeap == nullptr)
-    {
-        LogError("CompileResult::codeHeap() failed to acquire a heap.");
-        __debugbreak();
-    }
-    return codeHeap;
-}
-
 void CompileResult::recAssert(const char* assertText)
 {
     if (AssertLog == nullptr)

src/coreclr/src/ToolBox/superpmi/superpmi-shared/compileresult.h

@@ -168,8 +168,6 @@ class CompileResult
 
     void dumpToConsole();
 
-    HANDLE getCodeHeap();
-
     void recAssert(const char* buff);
     void dmpAssertLog(DWORD key, DWORD value);
     const char* repAssert();
@@ -309,7 +307,6 @@ class CompileResult
     LightWeightMap<DWORDLONG, DWORD>* CallTargetTypes;
 
 private:
-    HANDLE                  codeHeap;
     Capture_AllocMemDetails allocMemDets;
     allocGCInfoDetails      allocGCInfoDets;
 };

src/coreclr/src/ToolBox/superpmi/superpmi-shared/runtimedetails.h

@@ -9,30 +9,11 @@
 #ifndef _RuntimeDetails
 #define _RuntimeDetails
 
-// Our little collection of enough of the CLR data to get the JIT up and working...
-#define FEATURE_CLRSQM
-
-#if !defined(TARGET_AMD64) && !defined(TARGET_X86) && !defined(TARGET_ARM64) && !defined(TARGET_ARM)
-#if defined(_M_X64)
-#define TARGET_AMD64 1
-#elif defined(_M_IX86)
-#define TARGET_X86 1
-#endif
-#endif // _TARGET_* not previously defined
-
-#define __EXCEPTION_RECORD_CLR // trick out clrntexception.h to not include another exception record....
-
 #include <mscoree.h>
 #include <corjit.h>
 #include <utilcode.h>
 #include <patchpointinfo.h>
 
-/// Turn back on direct access to a few OS level things...
-#undef HeapCreate
-#undef HeapAlloc
-#undef HeapFree
-#undef HeapDestroy
-
 // Jit Exports
 typedef ICorJitCompiler*(__stdcall* PgetJit)();
 typedef void(__stdcall* PjitStartup)(ICorJitHost* host);

src/coreclr/src/ToolBox/superpmi/superpmi/icorjitinfo.cpp

@@ -1588,12 +1588,12 @@ void MyICJI::allocMem(ULONG              hotCodeSize,   /* IN */
 {
     jitInstance->mc->cr->AddCall("allocMem");
     // TODO-Cleanup: investigate if we need to check roDataBlock as well. Could hot block size be ever 0?
-    *hotCodeBlock = HeapAlloc(jitInstance->mc->cr->getCodeHeap(), 0, hotCodeSize);
+    *hotCodeBlock = new BYTE[hotCodeSize];
     if (coldCodeSize > 0)
-        *coldCodeBlock = HeapAlloc(jitInstance->mc->cr->getCodeHeap(), 0, coldCodeSize);
+        *coldCodeBlock = new BYTE[coldCodeSize];
     else
         *coldCodeBlock = nullptr;
-    *roDataBlock       = HeapAlloc(jitInstance->mc->cr->getCodeHeap(), 0, roDataSize);
+    *roDataBlock       = new BYTE[roDataSize];
     jitInstance->mc->cr->recAllocMem(hotCodeSize, coldCodeSize, roDataSize, xcptnsCount, flag, hotCodeBlock,
                                      coldCodeBlock, roDataBlock);
 }
@@ -1657,7 +1657,7 @@ void* MyICJI::allocGCInfo(size_t size /* IN */
                           )
 {
     jitInstance->mc->cr->AddCall("allocGCInfo");
-    void* temp = (unsigned char*)HeapAlloc(jitInstance->mc->cr->getCodeHeap(), 0, size);
+    void* temp = (unsigned char*)new BYTE[size];
     jitInstance->mc->cr->recAllocGCInfo(size, temp);
 
     return temp;

src/coreclr/src/ToolBox/superpmi/superpmi/jitinstance.cpp

@@ -61,15 +61,6 @@ HRESULT JitInstance::StartUp(char* PathToJit, bool copyJit, bool breakOnDebugBre
     char lpTempPathBuffer[MAX_PATH];
     char szTempFileName[MAX_PATH];
 
-    // Get an allocator instance
-    // Note: we do this to keep cleanup somewhat simple...
-    ourHeap = ::HeapCreate(0, 0, 0);
-    if (ourHeap == nullptr)
-    {
-        LogError("Failed to get a new heap (0x%08x)", ::GetLastError());
-        return E_FAIL;
-    }
-
     // find the full jit path
     dwRetVal = ::GetFullPathNameA(PathToJit, MAX_PATH, pFullPathName, nullptr);
     if (dwRetVal == 0)
@@ -79,7 +70,7 @@ HRESULT JitInstance::StartUp(char* PathToJit, bool copyJit, bool breakOnDebugBre
     }
 
     // Store the full path to the jit
-    PathToOriginalJit = (char*)::HeapAlloc(ourHeap, 0, MAX_PATH);
+    PathToOriginalJit = (char*)malloc(MAX_PATH);
     if (PathToOriginalJit == nullptr)
     {
         LogError("1st HeapAlloc failed (0x%08x)", ::GetLastError());
@@ -111,7 +102,7 @@ HRESULT JitInstance::StartUp(char* PathToJit, bool copyJit, bool breakOnDebugBre
         dwRetVal = (DWORD)::strlen(szTempFileName);
 
         // Store the full path to the temp jit
-        PathToTempJit = (char*)::HeapAlloc(ourHeap, 0, MAX_PATH);
+        PathToTempJit = (char*)malloc(MAX_PATH);
         if (PathToTempJit == nullptr)
         {
             LogError("2nd HeapAlloc failed 0x%08x)", ::GetLastError());
@@ -436,14 +427,14 @@ const WCHAR* JitInstance::getOption(const WCHAR* key, LightWeightMap<DWORD, DWOR
 void* JitInstance::allocateArray(size_t cBytes)
 {
     mc->cr->AddCall("allocateArray");
-    return HeapAlloc(mc->cr->getCodeHeap(), 0, cBytes);
+    return new BYTE[cBytes];
 }
 
 // Used to allocate memory that needs to live as long as the jit
 // instance does.
 void* JitInstance::allocateLongLivedArray(size_t cBytes)
 {
-    return HeapAlloc(ourHeap, 0, cBytes);
+    return new BYTE[cBytes];
 }
 
 // JitCompiler will free arrays passed by the EE using this
@@ -453,13 +444,13 @@ void* JitInstance::allocateLongLivedArray(size_t cBytes)
 void JitInstance::freeArray(void* array)
 {
     mc->cr->AddCall("freeArray");
-    HeapFree(mc->cr->getCodeHeap(), 0, array);
+    delete [] (BYTE*)array;
 }
 
 // Used to free memory allocated by JitInstance::allocateLongLivedArray.
 void JitInstance::freeLongLivedArray(void* array)
 {
-    HeapFree(ourHeap, 0, array);
+    delete [] (BYTE*)array;
 }
 
 // Helper for calling pnjitStartup. Needed to allow SEH here.

src/coreclr/src/ToolBox/superpmi/superpmi/jitinstance.h

@@ -16,7 +16,6 @@ class JitInstance
 private:
     char*          PathToOriginalJit;
     char*          PathToTempJit;
-    HANDLE         ourHeap;
     HMODULE        hLib;
     PgetJit        pngetJit;
     PjitStartup    pnjitStartup;

src/coreclr/src/debug/daccess/fntableaccess.cpp

@@ -217,9 +217,13 @@ static NTSTATUS OutOfProcessFunctionTableCallback_JIT(IN  ReadMemoryFunction
                 OutOfProcessFindHeader(fpReadMemory, pUserContext, Hp.pHdrMap, hdrOffset, hdrOffset);
             }
 
-            pFunctions   = (PT_RUNTIME_FUNCTION)ClrHeapAlloc(ClrGetProcessHeap(), HEAP_ZERO_MEMORY, S_SIZE_T(nEntries) * S_SIZE_T(sizeof(T_RUNTIME_FUNCTION)));
-            *ppFunctions = pFunctions;
-            *pnEntries   = nEntries;
+            S_SIZE_T blockSize = S_SIZE_T(nEntries) * S_SIZE_T(sizeof(T_RUNTIME_FUNCTION));
+            if (blockSize.IsOverflow())
+                return STATUS_UNSUCCESSFUL;
+
+            pFunctions = (PT_RUNTIME_FUNCTION)HeapAlloc(GetProcessHeap(), 0, blockSize.Value());
+            if (pFunctions == NULL)
+                return STATUS_NO_MEMORY;
 
             //
             // walk the header map and copy the function tables
@@ -256,7 +260,7 @@ static NTSTATUS OutOfProcessFunctionTableCallback_JIT(IN  ReadMemoryFunction
                 OutOfProcessFindHeader(fpReadMemory, pUserContext, Hp.pHdrMap, hdrOffset, hdrOffset);
             }
 
-            // Return the final count.
+            *ppFunctions = pFunctions;
             *pnEntries = index;
             break;
         }
@@ -371,7 +375,14 @@ static NTSTATUS OutOfProcessFunctionTableCallback_Stub(IN  ReadMemoryFunction
 
             _ASSERTE(!nEntriesAllocated);
             nEntriesAllocated = nEntries;
-            rgFunctions = (PT_RUNTIME_FUNCTION)ClrHeapAlloc(ClrGetProcessHeap(), HEAP_ZERO_MEMORY, S_SIZE_T(nEntries) * S_SIZE_T(sizeof(T_RUNTIME_FUNCTION)));
+
+            S_SIZE_T blockSize = S_SIZE_T(nEntries) * S_SIZE_T(sizeof(T_RUNTIME_FUNCTION));
+            if (blockSize.IsOverflow())
+                return STATUS_UNSUCCESSFUL;
+
+            rgFunctions = (PT_RUNTIME_FUNCTION)HeapAlloc(GetProcessHeap(), 0, blockSize.Value());
+            if (rgFunctions == NULL)
+                return STATUS_NO_MEMORY;
             nEntries = 0;
         }
         else

src/coreclr/src/debug/ee/debugger.cpp

@@ -16506,28 +16506,21 @@ void* DebuggerHeapExecutableMemoryAllocator::Allocate(DWORD numberOfBytes)
     return ChangePageUsage(pageToAllocateOn, chunkToUse, ChangePageUsageAction::ALLOCATE);
 }
 
-int DebuggerHeapExecutableMemoryAllocator::Free(void* addr)
+void DebuggerHeapExecutableMemoryAllocator::Free(void* addr)
 {
     ASSERT(addr != NULL);
 
     CrstHolder execMemAllocCrstHolder(&m_execMemAllocMutex);
 
     DebuggerHeapExecutableMemoryPage *pageToFreeIn = static_cast<DebuggerHeapExecutableMemoryChunk*>(addr)->data.startOfPage;
-
-    if (pageToFreeIn == NULL)
-    {
-        ASSERT(!"Couldn't locate page in which to free!");
-        return -1;
-    }
+    _ASSERTE(pageToFreeIn != NULL);
 
     int chunkNum = static_cast<DebuggerHeapExecutableMemoryChunk*>(addr)->data.chunkNumber;
 
     // Sanity check: assert that the address really represents the start of a chunk.
     ASSERT(((uint64_t)addr - (uint64_t)pageToFreeIn) % 64 == 0);
 
     ChangePageUsage(pageToFreeIn, chunkNum, ChangePageUsageAction::FREE);
-
-    return 0;
 }
 
 DebuggerHeapExecutableMemoryPage* DebuggerHeapExecutableMemoryAllocator::AddNewPage()
@@ -16620,7 +16613,7 @@ void DebuggerHeap::Destroy()
         m_hHeap = NULL;
     }
 #endif
-#ifdef TARGET_UNIX
+#ifndef HOST_WINDOWS
     if (m_execMemAllocator != NULL)
     {
         delete m_execMemAllocator;
@@ -16660,10 +16653,6 @@ HRESULT DebuggerHeap::Init(BOOL fExecutable)
         return S_OK;
     }
 
-#ifndef HEAP_CREATE_ENABLE_EXECUTE
-#define HEAP_CREATE_ENABLE_EXECUTE      0x00040000      // winnt create heap with executable pages
-#endif
-
     // Create a standard, grow-able, thread-safe heap.
     DWORD dwFlags = ((fExecutable == TRUE)? HEAP_CREATE_ENABLE_EXECUTE : 0);
     m_hHeap = ::HeapCreate(dwFlags, 0, 0);
@@ -16673,7 +16662,7 @@ HRESULT DebuggerHeap::Init(BOOL fExecutable)
     }
 #endif
 
-#ifdef TARGET_UNIX
+#ifndef HOST_WINDOWS
     m_execMemAllocator = new (nothrow) DebuggerHeapExecutableMemoryAllocator();
     ASSERT(m_execMemAllocator != NULL);
     if (m_execMemAllocator == NULL)
@@ -16763,32 +16752,25 @@ void *DebuggerHeap::Alloc(DWORD size)
     ret = ::HeapAlloc(m_hHeap, HEAP_ZERO_MEMORY, size);
 #else // USE_INTEROPSAFE_HEAP
 
-    bool allocateOnHeap = true;
-    HANDLE hExecutableHeap = NULL;
+#ifdef HOST_WINDOWS
+    HANDLE hExecutableHeap  = ClrGetProcessExecutableHeap();
 
-#ifdef TARGET_UNIX
+    if (hExecutableHeap == NULL)
+    {
+        return NULL;
+    }
+
+    ret = ::HeapAlloc(hExecutableHeap, 0, size);
+#else // HOST_WINDOWS
     if (m_fExecutable)
     {
-        allocateOnHeap = false;
         ret = m_execMemAllocator->Allocate(size);
     }
     else
     {
-        hExecutableHeap  = ClrGetProcessHeap();
-    }
-#else // TARGET_UNIX
-    hExecutableHeap  = ClrGetProcessExecutableHeap();
-#endif
-
-    if (allocateOnHeap)
-    {
-        if (hExecutableHeap == NULL)
-        {
-            return NULL;
-        }
-
-        ret = ClrHeapAlloc(hExecutableHeap, NULL, S_SIZE_T(size));
+        ret = malloc(size);
     }
+#endif // HOST_WINDOWS
 
 #endif // USE_INTEROPSAFE_HEAP
 
@@ -16819,7 +16801,7 @@ void *DebuggerHeap::Realloc(void *pMem, DWORD newSize, DWORD oldSize)
     _ASSERTE(newSize != 0);
     _ASSERTE(oldSize != 0);
 
-#if defined(USE_INTEROPSAFE_HEAP) && !defined(USE_INTEROPSAFE_CANARY) && !defined(TARGET_UNIX)
+#if defined(USE_INTEROPSAFE_HEAP) && !defined(USE_INTEROPSAFE_CANARY) && defined(HOST_WINDOWS)
     // No canaries in this case.
     // Call into realloc.
     void *ret;
@@ -16872,23 +16854,20 @@ void DebuggerHeap::Free(void *pMem)
 #else
     if (pMem != NULL)
     {
-#ifndef TARGET_UNIX
+#ifdef HOST_WINDOWS
         HANDLE hProcessExecutableHeap  = ClrGetProcessExecutableHeap();
         _ASSERTE(hProcessExecutableHeap != NULL);
-        ClrHeapFree(hProcessExecutableHeap, NULL, pMem);
-#else // !TARGET_UNIX
-        if(!m_fExecutable)
+        ::HeapFree(hProcessExecutableHeap, NULL, pMem);
+#else // HOST_WINDOWS
+        if (m_fExecutable)
         {
-            HANDLE hProcessHeap  = ClrGetProcessHeap();
-            _ASSERTE(hProcessHeap != NULL);
-            ClrHeapFree(hProcessHeap, NULL, pMem);
+            m_execMemAllocator->Free(pMem);
         }
         else
         {
-            INDEBUG(int ret =) m_execMemAllocator->Free(pMem);
-            _ASSERTE(ret == 0);
+            free(pMem);
         }
-#endif // !TARGET_UNIX
+#endif // HOST_WINDOWS
     }
 #endif
 }

src/coreclr/src/debug/ee/debugger.h

@@ -1201,7 +1201,7 @@ class DebuggerHeapExecutableMemoryAllocator
     ~DebuggerHeapExecutableMemoryAllocator();
 
     void* Allocate(DWORD numberOfBytes);
-    int Free(void* addr);
+    void Free(void* addr);
 
 private:
     enum class ChangePageUsageAction {ALLOCATE, FREE};
@@ -1249,7 +1249,9 @@ class DebuggerHeap
     BOOL m_fExecutable;
 
 private:
+#ifndef HOST_WINDOWS
     DebuggerHeapExecutableMemoryAllocator *m_execMemAllocator;
+#endif
 };
 
 class DebuggerJitInfo;