Merge pull request #2264 from adityamandaleeka/dbg_exec_alloc

[WIP] Fix usage of executable heap memory

6 files changed, 552 insertions, 238 deletions

diff --git a/src/debug/ee/controller.cpp b/src/debug/ee/controller.cpp
index 4e1ab6557a..f3819af4a2 100644
--- a/src/debug/ee/controller.cpp
+++ b/src/debug/ee/controller.cpp
@@ -8445,9 +8445,9 @@ DebuggerFuncEvalComplete::DebuggerFuncEvalComplete(Thread *thread,
   : DebuggerController(thread, NULL)
 {
 #ifdef _TARGET_ARM_
-    m_pDE = reinterpret_cast<DebuggerEval*>(((DWORD)dest) & ~THUMB_CODE);
+    m_pDE = reinterpret_cast<DebuggerEvalBreakpointInfoSegment*>(((DWORD)dest) & ~THUMB_CODE)->m_associatedDebuggerEval;
 #else
-    m_pDE = reinterpret_cast<DebuggerEval*>(dest);
+    m_pDE = reinterpret_cast<DebuggerEvalBreakpointInfoSegment*>(dest)->m_associatedDebuggerEval;
 #endif
 
     // Add an unmanaged patch at the destination.
diff --git a/src/debug/ee/debugger.cpp b/src/debug/ee/debugger.cpp
index ddf2fadf14..12a7473e8e 100644
--- a/src/debug/ee/debugger.cpp
+++ b/src/debug/ee/debugger.cpp
@@ -40,7 +40,6 @@
 #include "dbgtransportsession.h"
 #endif // FEATURE_DBGIPC_TRANSPORT_VM
 
-
 #ifdef TEST_DATA_CONSISTENCY
 #include "datatest.h"
 #endif // TEST_DATA_CONSISTENCY
@@ -1405,11 +1404,14 @@ DebuggerEval::DebuggerEval(CONTEXT * pContext, DebuggerIPCE_FuncEvalInfo * pEval
 {
     WRAPPER_NO_CONTRACT;
 
+    // Allocate the breakpoint instruction info in executable memory.
+    m_bpInfoSegment = new (interopsafeEXEC, nothrow) DebuggerEvalBreakpointInfoSegment(this);
+
     // This must be non-zero so that the saved opcode is non-zero, and on IA64 we want it to be 0x16
     // so that we can have a breakpoint instruction in any slot in the bundle.
-    m_breakpointInstruction[0] = 0x16;
+    m_bpInfoSegment->m_breakpointInstruction[0] = 0x16;
 #if defined(_TARGET_ARM_)
-    USHORT *bp = (USHORT*)&m_breakpointInstruction;
+    USHORT *bp = (USHORT*)&m_bpInfoSegment->m_breakpointInstruction;
     *bp = CORDbg_BREAK_INSTRUCTION;
 #endif // _TARGET_ARM_
     m_thread = pEvalInfo->vmThreadToken.GetRawPtr();
@@ -1483,9 +1485,12 @@ DebuggerEval::DebuggerEval(CONTEXT * pContext, Thread * pThread, Thread::ThreadA
 {
     WRAPPER_NO_CONTRACT;
 
+    // Allocate the breakpoint instruction info in executable memory.
+    m_bpInfoSegment = new (interopsafeEXEC, nothrow) DebuggerEvalBreakpointInfoSegment(this);
+
     // This must be non-zero so that the saved opcode is non-zero, and on IA64 we want it to be 0x16
     // so that we can have a breakpoint instruction in any slot in the bundle.
-    m_breakpointInstruction[0] = 0x16;
+    m_bpInfoSegment->m_breakpointInstruction[0] = 0x16;
     m_thread = pThread;
     m_evalType = DB_IPCE_FET_RE_ABORT;
     m_methodToken = mdMethodDefNil;
@@ -15370,7 +15375,7 @@ HRESULT Debugger::FuncEvalSetup(DebuggerIPCE_FuncEvalInfo *pEvalInfo,
 
     // Create a DebuggerEval to hold info about this eval while its in progress. Constructor copies the thread's
     // CONTEXT.
-    DebuggerEval *pDE = new (interopsafeEXEC, nothrow) DebuggerEval(filterContext, pEvalInfo, fInException);
+    DebuggerEval *pDE = new (interopsafe, nothrow) DebuggerEval(filterContext, pEvalInfo, fInException);
 
     if (pDE == NULL)
     {
@@ -15502,7 +15507,7 @@ HRESULT Debugger::FuncEvalSetupReAbort(Thread *pThread, Thread::ThreadAbortReque
 
     // Create a DebuggerEval to hold info about this eval while its in progress. Constructor copies the thread's
     // CONTEXT.
-    DebuggerEval *pDE = new (interopsafeEXEC, nothrow) DebuggerEval(filterContext, pThread, requester);
+    DebuggerEval *pDE = new (interopsafe, nothrow) DebuggerEval(filterContext, pThread, requester);
 
     if (pDE == NULL)
     {
@@ -15692,7 +15697,8 @@ HRESULT Debugger::FuncEvalCleanup(DebuggerEval *debuggerEvalKey)
     LOG((LF_CORDB, LL_INFO1000, "D::FEC: pDE:%08x 0x%08x, id=0x%x\n",
          pDE, pDE->m_thread, GetThreadIdHelper(pDE->m_thread)));
 
-    DeleteInteropSafeExecutable(pDE);
+    DeleteInteropSafeExecutable(pDE->m_bpInfoSegment);
+    DeleteInteropSafe(pDE);
 
     return S_OK;
 }
@@ -16573,6 +16579,155 @@ void Debugger::ReleaseDebuggerDataLock(Debugger *pDebugger)
 #endif // DACCESS_COMPILE
 
 /* ------------------------------------------------------------------------ *
+ * Functions for DebuggerHeap executable memory allocations
+ * ------------------------------------------------------------------------ */
+
+DebuggerHeapExecutableMemoryAllocator::~DebuggerHeapExecutableMemoryAllocator()
+{
+    while (m_pages != NULL)
+    {
+        DebuggerHeapExecutableMemoryPage *temp = m_pages->GetNextPage();
+
+        // Free this page
+        INDEBUG(BOOL ret =) VirtualFree(m_pages, 0, MEM_RELEASE);
+        ASSERT(ret == TRUE);
+
+        m_pages = temp;
+    }
+
+    ASSERT(m_pages == NULL);
+}
+
+void* DebuggerHeapExecutableMemoryAllocator::Allocate(DWORD numberOfBytes)
+{
+    if (numberOfBytes > DBG_MAX_EXECUTABLE_ALLOC_SIZE)
+    {
+        ASSERT(!"Allocating more than DBG_MAX_EXECUTABLE_ALLOC_SIZE at once is unsupported and breaks our assumptions.");
+        return NULL;
+    }
+
+    if (numberOfBytes == 0)
+    {
+        // Should we allocate anything in this case?
+        ASSERT(!"Allocate called with 0 for numberOfBytes!");
+        return NULL;
+    }
+
+    CrstHolder execMemAllocCrstHolder(&m_execMemAllocMutex);
+
+    int chunkToUse = -1;
+    DebuggerHeapExecutableMemoryPage *pageToAllocateOn = NULL;
+    for (DebuggerHeapExecutableMemoryPage *currPage = m_pages; currPage != NULL; currPage = currPage->GetNextPage())
+    {
+        if (CheckPageForAvailability(currPage, &chunkToUse))
+        {
+            pageToAllocateOn = currPage;
+            break;
+        }
+    }
+
+    if (pageToAllocateOn == NULL)
+    {
+        // No existing page had availability, so create a new page and use that.
+        pageToAllocateOn = AddNewPage();
+        if (pageToAllocateOn == NULL)
+        {
+            ASSERT(!"Call to AddNewPage failed!");
+            return NULL;
+        }
+
+        if (!CheckPageForAvailability(pageToAllocateOn, &chunkToUse))
+        {
+            ASSERT(!"No availability on new page?");
+            return NULL;
+        }
+    }
+
+    return ChangePageUsage(pageToAllocateOn, chunkToUse, ChangePageUsageAction::ALLOCATE);
+}
+
+int 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;
+    }
+
+    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()
+{
+    void* newPageAddr = VirtualAlloc(NULL, sizeof(DebuggerHeapExecutableMemoryPage), MEM_COMMIT | MEM_RESERVE, PAGE_EXECUTE_READWRITE);
+
+    DebuggerHeapExecutableMemoryPage *newPage = new (newPageAddr) DebuggerHeapExecutableMemoryPage;
+    newPage->SetNextPage(m_pages);
+
+    // Add the new page to the linked list of pages
+    m_pages = newPage;
+    return newPage;
+}
+
+bool DebuggerHeapExecutableMemoryAllocator::CheckPageForAvailability(DebuggerHeapExecutableMemoryPage* page, /* _Out_ */ int* chunkToUse)
+{
+    uint64_t occupancy = page->GetPageOccupancy();
+    bool available = occupancy != UINT64_MAX;
+
+    if (!available)
+    {
+        if (chunkToUse)
+        {
+            *chunkToUse = -1;
+        }
+
+        return false;
+    }
+
+    if (chunkToUse)
+    {
+        // Start i at 62 because first chunk is reserved
+        for (int i = 62; i >= 0; i--)
+        {
+            uint64_t mask = ((uint64_t)1 << i);
+            if ((mask & occupancy) == 0)
+            {
+                *chunkToUse = 64 - i - 1;
+                break;
+            }
+        }
+    }
+
+    return true;
+}
+
+void* DebuggerHeapExecutableMemoryAllocator::ChangePageUsage(DebuggerHeapExecutableMemoryPage* page, int chunkNumber, ChangePageUsageAction action)
+{
+    ASSERT(action == ChangePageUsageAction::ALLOCATE || action == ChangePageUsageAction::FREE);
+
+    uint64_t mask = (uint64_t)0x1 << (64 - chunkNumber - 1);
+
+    uint64_t prevOccupancy = page->GetPageOccupancy();
+    uint64_t newOccupancy = (action == ChangePageUsageAction::ALLOCATE) ? (prevOccupancy | mask) : (prevOccupancy ^ mask);
+    page->SetPageOccupancy(newOccupancy);
+
+    return page->GetPointerToChunk(chunkNumber);
+}
+
+/* ------------------------------------------------------------------------ *
  * DebuggerHeap impl
  * ------------------------------------------------------------------------ */
 
@@ -16584,7 +16739,6 @@ DebuggerHeap::DebuggerHeap()
     m_fExecutable = FALSE;
 }
 
-
 DebuggerHeap::~DebuggerHeap()
 {
     CONTRACTL
@@ -16607,6 +16761,12 @@ void DebuggerHeap::Destroy()
         m_hHeap = NULL;
     }
 #endif
+#ifdef FEATURE_PAL
+    if (m_execMemAllocator != NULL)
+    {
+        delete m_execMemAllocator;
+    }
+#endif
 }
 
 bool DebuggerHeap::IsInit()
@@ -16654,6 +16814,16 @@ HRESULT DebuggerHeap::Init(BOOL fExecutable)
         return HRESULT_FROM_GetLastError();
     }
 #endif
+
+#ifdef FEATURE_PAL
+    m_execMemAllocator = new (nothrow) DebuggerHeapExecutableMemoryAllocator();
+    ASSERT(m_execMemAllocator != NULL);
+    if (m_execMemAllocator == NULL)
+    {
+        return E_OUTOFMEMORY;
+    }
+#endif
+
     return S_OK;
 }
 
@@ -16736,17 +16906,33 @@ void *DebuggerHeap::Alloc(DWORD size)
     ret = ::HeapAlloc(m_hHeap, HEAP_ZERO_MEMORY, size);
 #else // USE_INTEROPSAFE_HEAP
 
-#ifndef FEATURE_PAL
-    HANDLE hExecutableHeap  = ClrGetProcessExecutableHeap();
-#else // !FEATURE_PAL
-    HANDLE hExecutableHeap  = ClrGetProcessHeap();
-#endif // !FEATURE_PAL
+    bool allocateOnHeap = true;
+    HANDLE hExecutableHeap = NULL;
 
-    if (hExecutableHeap == NULL)
+#ifdef FEATURE_PAL
+    if (m_fExecutable)
     {
-        return NULL;
+        allocateOnHeap = false;
+        ret = m_execMemAllocator->Allocate(size);
     }
-    ret = ClrHeapAlloc(hExecutableHeap, NULL, S_SIZE_T(size));
+    else
+    {
+        hExecutableHeap  = ClrGetProcessHeap();
+    }
+#else // FEATURE_PAL
+    hExecutableHeap  = ClrGetProcessExecutableHeap();
+#endif
+
+    if (allocateOnHeap)
+    {
+        if (hExecutableHeap == NULL)
+        {
+            return NULL;
+        }
+
+        ret = ClrHeapAlloc(hExecutableHeap, NULL, S_SIZE_T(size));
+    }
+
 #endif // USE_INTEROPSAFE_HEAP
 
 #ifdef USE_INTEROPSAFE_CANARY
@@ -16758,25 +16944,9 @@ void *DebuggerHeap::Alloc(DWORD size)
     ret = pCanary->GetUserAddr();
 #endif
 
-#ifdef FEATURE_PAL
-    if (m_fExecutable)
-    {
-        // We don't have executable heap in PAL, but we still need to allocate 
-        // executable memory, that's why have change protection level for 
-        // each allocation. 
-        // UNIXTODO: We need to look how JIT solves this problem.
-        DWORD unusedFlags;
-        if (!VirtualProtect(ret, size, PAGE_EXECUTE_READWRITE, &unusedFlags))
-        {
-            _ASSERTE(!"VirtualProtect failed to make this memory executable");
-        }
-    }
-#endif // FEATURE_PAL
-
     return ret;
 }
 
-
 // Realloc memory.
 // If this fails, the original memory is still valid.
 void *DebuggerHeap::Realloc(void *pMem, DWORD newSize, DWORD oldSize)
@@ -16793,7 +16963,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)
+#if defined(USE_INTEROPSAFE_HEAP) && !defined(USE_INTEROPSAFE_CANARY) && !defined(FEATURE_PAL)
     // No canaries in this case.
     // Call into realloc.
     void *ret;
@@ -16815,22 +16985,6 @@ void *DebuggerHeap::Realloc(void *pMem, DWORD newSize, DWORD oldSize)
     this->Free(pMem);
 #endif
 
-
-#ifdef FEATURE_PAL
-    if (m_fExecutable)
-    {    
-        // We don't have executable heap in PAL, but we still need to allocate 
-        // executable memory, that's why have change protection level for 
-        // each allocation. 
-        // UNIXTODO: We need to look how JIT solves this problem.
-        DWORD unusedFlags;
-        if (!VirtualProtect(ret, newSize, PAGE_EXECUTE_READWRITE, &unusedFlags))
-        {
-            _ASSERTE(!"VirtualProtect failed to make this memory executable");
-        }
-    }
-#endif // FEATURE_PAL    
-
     return ret;
 }
 
@@ -16864,12 +17018,22 @@ void DebuggerHeap::Free(void *pMem)
     if (pMem != NULL)
     {
 #ifndef FEATURE_PAL
-        HANDLE hExecutableHeap  = ClrGetProcessExecutableHeap();
+        HANDLE hProcessExecutableHeap  = ClrGetProcessExecutableHeap();
+        _ASSERTE(hProcessExecutableHeap != NULL);
+        ClrHeapFree(hProcessExecutableHeap, NULL, pMem);
 #else // !FEATURE_PAL
-        HANDLE hExecutableHeap  = ClrGetProcessHeap();
+        if(!m_fExecutable)
+        {
+            HANDLE hProcessHeap  = ClrGetProcessHeap();
+            _ASSERTE(hProcessHeap != NULL);
+            ClrHeapFree(hProcessHeap, NULL, pMem);
+        }
+        else
+        {
+            INDEBUG(int ret =) m_execMemAllocator->Free(pMem);
+            _ASSERTE(ret == 0);
+        }
 #endif // !FEATURE_PAL
-        _ASSERTE(hExecutableHeap != NULL);
-        ClrHeapFree(hExecutableHeap, NULL, pMem);
     }
 #endif
 }
diff --git a/src/debug/ee/debugger.h b/src/debug/ee/debugger.h
index 9d25140cc0..b471fba601 100644
--- a/src/debug/ee/debugger.h
+++ b/src/debug/ee/debugger.h
@@ -1072,6 +1072,139 @@ protected:
     bool m_fHasInstrumentedILMap;
 };
 
+// ------------------------------------------------------------------------ *
+// Executable code memory management for the debugger heap.
+//
+//     Rather than allocating memory that needs to be executable on the process heap (which
+//     is forbidden on some flavors of SELinux and is generally a bad idea), we use the
+//     allocator below. It will handle allocating and managing the executable memory in a
+//     different part of the address space (not on the heap).
+// ------------------------------------------------------------------------ */
+
+#define DBG_MAX_EXECUTABLE_ALLOC_SIZE 48
+
+// Forward declaration
+struct DebuggerHeapExecutableMemoryPage;
+
+// ------------------------------------------------------------------------ */
+// DebuggerHeapExecutableMemoryChunk
+//
+// Each DebuggerHeapExecutableMemoryPage is divided into 64 of these chunks.
+// The first chunk is a BookkeepingChunk used for bookkeeping information
+// for the page, and the remaining ones are DataChunks and are handed out
+// by the allocator when it allocates memory.
+// ------------------------------------------------------------------------ */
+union DECLSPEC_ALIGN(64) DebuggerHeapExecutableMemoryChunk {
+
+    struct DataChunk
+    {
+        char data[DBG_MAX_EXECUTABLE_ALLOC_SIZE];
+
+        DebuggerHeapExecutableMemoryPage *startOfPage;
+
+        // The chunk number within the page.
+        uint8_t chunkNumber;
+
+    } data;
+
+    struct BookkeepingChunk
+    {
+        DebuggerHeapExecutableMemoryPage *nextPage;
+
+        uint64_t pageOccupancy;
+
+    } bookkeeping;
+};
+
+static_assert(sizeof(DebuggerHeapExecutableMemoryChunk) == 64, "DebuggerHeapExecutableMemoryChunk is expect to be 64 bytes.");
+
+// ------------------------------------------------------------------------ */
+// DebuggerHeapExecutableMemoryPage
+//
+// We allocate the size of DebuggerHeapExecutableMemoryPage each time we need
+// more memory and divide each page into DebuggerHeapExecutableMemoryChunks for
+// use. The pages are self describing; the first chunk contains information
+// about which of the other chunks are used/free as well as a pointer to 
+// the next page.
+// ------------------------------------------------------------------------ */
+struct DECLSPEC_ALIGN(4096) DebuggerHeapExecutableMemoryPage
+{
+    inline DebuggerHeapExecutableMemoryPage* GetNextPage()
+    {
+        return chunks[0].bookkeeping.nextPage;
+    }
+
+    inline void SetNextPage(DebuggerHeapExecutableMemoryPage* nextPage)
+    {
+        chunks[0].bookkeeping.nextPage = nextPage;
+    }
+
+    inline uint64_t GetPageOccupancy() const
+    {
+        return chunks[0].bookkeeping.pageOccupancy;
+    }
+
+    inline void SetPageOccupancy(uint64_t newOccupancy)
+    {
+        // Can't unset first bit of occupancy!
+        ASSERT((newOccupancy & 0x8000000000000000) != 0);
+
+        chunks[0].bookkeeping.pageOccupancy = newOccupancy;
+    }
+
+    inline void* GetPointerToChunk(int chunkNum) const
+    {
+        return (char*)this + chunkNum * sizeof(DebuggerHeapExecutableMemoryChunk);
+    }
+
+    DebuggerHeapExecutableMemoryPage()
+    {
+        SetPageOccupancy(0x8000000000000000); // only the first bit is set.
+        for (uint8_t i = 1; i < sizeof(chunks)/sizeof(chunks[0]); i++)
+        {
+            ASSERT(i != 0);
+            chunks[i].data.startOfPage = this;
+            chunks[i].data.chunkNumber = i;
+        }
+    }
+
+private:
+    DebuggerHeapExecutableMemoryChunk chunks[64];
+};
+
+// ------------------------------------------------------------------------ */
+// DebuggerHeapExecutableMemoryAllocator class
+// Handles allocation and freeing (and all necessary bookkeeping) for 
+// executable memory that the DebuggerHeap class needs. This is especially
+// useful on systems (like SELinux) where having executable code on the
+// heap is explicity disallowed for security reasons.
+// ------------------------------------------------------------------------ */
+
+class DebuggerHeapExecutableMemoryAllocator
+{
+public:
+    DebuggerHeapExecutableMemoryAllocator()
+    : m_pages(NULL)
+    , m_execMemAllocMutex(CrstDebuggerHeapExecMemLock, (CrstFlags)(CRST_UNSAFE_ANYMODE | CRST_REENTRANCY | CRST_DEBUGGER_THREAD))
+    { }
+
+    ~DebuggerHeapExecutableMemoryAllocator();
+
+    void* Allocate(DWORD numberOfBytes);
+    int Free(void* addr);
+
+private:
+    enum class ChangePageUsageAction {ALLOCATE, FREE};
+
+    DebuggerHeapExecutableMemoryPage* AddNewPage();
+    bool CheckPageForAvailability(DebuggerHeapExecutableMemoryPage* page, /* _Out_ */ int* chunkToUse);
+    void* ChangePageUsage(DebuggerHeapExecutableMemoryPage* page, int chunkNumber, ChangePageUsageAction action);
+
+private:
+    // Linked list of pages that have been allocated
+    DebuggerHeapExecutableMemoryPage* m_pages;
+    Crst m_execMemAllocMutex;
+};
 
 // ------------------------------------------------------------------------ *
 // DebuggerHeap class
@@ -1104,6 +1237,9 @@ protected:
     HANDLE m_hHeap;
 #endif
     BOOL m_fExecutable;
+
+private:
+    DebuggerHeapExecutableMemoryAllocator *m_execMemAllocator;
 };
 
 class DebuggerJitInfo;
@@ -3206,6 +3342,26 @@ public:
 #endif
 };
 
+class DebuggerEvalBreakpointInfoSegment
+{
+public:
+    // DebuggerEvalBreakpointInfoSegment contains just the breakpoint 
+    // instruction and a pointer to the associated DebuggerEval. It makes
+    // it easy to go from the instruction to the corresponding DebuggerEval
+    // object. It has been separated from the rest of the DebuggerEval 
+    // because it needs to be in a section of memory that's executable, 
+    // while the rest of DebuggerEval does not. By having it separate, we
+    // don't need to have the DebuggerEval contents in executable memory.
+    BYTE          m_breakpointInstruction[CORDbg_BREAK_INSTRUCTION_SIZE];
+    DebuggerEval *m_associatedDebuggerEval;
+
+    DebuggerEvalBreakpointInfoSegment(DebuggerEval* dbgEval)
+    : m_associatedDebuggerEval(dbgEval)
+    {
+        ASSERT(dbgEval != NULL);
+    }
+};
+
 /* ------------------------------------------------------------------------ *
  * DebuggerEval class
  *
@@ -3242,38 +3398,35 @@ public:
         FE_ABORT_RUDE = 2
     };
 
-    // Note: this first field must be big enough to hold a breakpoint
-    // instruction, and it MUST be the first field. (This
-    // is asserted in debugger.cpp)
-    BYTE                           m_breakpointInstruction[CORDbg_BREAK_INSTRUCTION_SIZE];
-    T_CONTEXT                      m_context;
-    Thread                        *m_thread;
-    DebuggerIPCE_FuncEvalType      m_evalType;
-    mdMethodDef                    m_methodToken;
-    mdTypeDef                      m_classToken;
-    ADID                           m_appDomainId;       // Safe even if AD unloaded
-    PTR_DebuggerModule             m_debuggerModule;     // Only valid if AD is still around
-    RSPTR_CORDBEVAL                m_funcEvalKey;
-    bool                           m_successful;        // Did the eval complete successfully
-    Debugger::AreValueTypesBoxed   m_retValueBoxing;        // Is the return value boxed?
-    unsigned int                   m_argCount;
-    unsigned int                   m_genericArgsCount;
-    unsigned int                   m_genericArgsNodeCount;
-    SIZE_T                         m_stringSize;
-    BYTE                          *m_argData;
-    MethodDesc                    *m_md;
-    PCODE                          m_targetCodeAddr;
-    INT64                          m_result;
-    TypeHandle                     m_resultType;
-    SIZE_T                         m_arrayRank;
-    FUNC_EVAL_ABORT_TYPE           m_aborting;          // Has an abort been requested, and what type.
-    bool                           m_aborted;           // Was this eval aborted
-    bool                           m_completed;          // Is the eval complete - successfully or by aborting
-    bool                           m_evalDuringException;
-    bool                           m_rethrowAbortException;
-    Thread::ThreadAbortRequester   m_requester;         // For aborts, what kind?
-    VMPTR_OBJECTHANDLE             m_vmObjectHandle;
-    TypeHandle                     m_ownerTypeHandle;
+    T_CONTEXT                          m_context;
+    Thread                            *m_thread;
+    DebuggerIPCE_FuncEvalType          m_evalType;
+    mdMethodDef                        m_methodToken;
+    mdTypeDef                          m_classToken;
+    ADID                               m_appDomainId;       // Safe even if AD unloaded
+    PTR_DebuggerModule                 m_debuggerModule;     // Only valid if AD is still around
+    RSPTR_CORDBEVAL                    m_funcEvalKey;
+    bool                               m_successful;        // Did the eval complete successfully
+    Debugger::AreValueTypesBoxed       m_retValueBoxing;        // Is the return value boxed?
+    unsigned int                       m_argCount;
+    unsigned int                       m_genericArgsCount;
+    unsigned int                       m_genericArgsNodeCount;
+    SIZE_T                             m_stringSize;
+    BYTE                              *m_argData;
+    MethodDesc                        *m_md;
+    PCODE                              m_targetCodeAddr;
+    INT64                              m_result;
+    TypeHandle                         m_resultType;
+    SIZE_T                             m_arrayRank;
+    FUNC_EVAL_ABORT_TYPE               m_aborting;          // Has an abort been requested, and what type.
+    bool                               m_aborted;           // Was this eval aborted
+    bool                               m_completed;          // Is the eval complete - successfully or by aborting
+    bool                               m_evalDuringException;
+    bool                               m_rethrowAbortException;
+    Thread::ThreadAbortRequester       m_requester;         // For aborts, what kind?
+    VMPTR_OBJECTHANDLE                 m_vmObjectHandle;
+    TypeHandle                         m_ownerTypeHandle;
+    DebuggerEvalBreakpointInfoSegment* m_bpInfoSegment;
 
     DebuggerEval(T_CONTEXT * pContext, DebuggerIPCE_FuncEvalInfo * pEvalInfo, bool fInException);
 
@@ -3282,11 +3435,10 @@ public:
 
     bool Init()
     {
-        _ASSERTE(DbgIsExecutable(&m_breakpointInstruction, sizeof(m_breakpointInstruction)));
+        _ASSERTE(DbgIsExecutable(&m_bpInfoSegment->m_breakpointInstruction, sizeof(m_bpInfoSegment->m_breakpointInstruction)));
         return true;
     }
 
-
     // The m_argData buffer holds both the type arg data (for generics) and the main argument data.
     //
     // For DB_IPCE_FET_NEW_STRING it holds the data specifying the string to create.
diff --git a/src/debug/ee/funceval.cpp b/src/debug/ee/funceval.cpp
index c7ec48d20c..5f9b6999b9 100644
--- a/src/debug/ee/funceval.cpp
+++ b/src/debug/ee/funceval.cpp
@@ -3872,21 +3872,12 @@ void * STDCALL FuncEvalHijackWorker(DebuggerEval *pDE)
     if (!pDE->m_evalDuringException)
     {
         // Signal to the helper thread that we're done with our func eval.  Start by creating a DebuggerFuncEvalComplete
-        // object. Give it an address at which to create the patch, which is a chunk of memory inside of our
+        // object. Give it an address at which to create the patch, which is a chunk of memory specified by our
         // DebuggerEval big enough to hold a breakpoint instruction.
 #ifdef _TARGET_ARM_
-        dest = (BYTE*)((DWORD)&(pDE->m_breakpointInstruction) | THUMB_CODE);
+        dest = (BYTE*)((DWORD)&(pDE->m_bpInfoSegment->m_breakpointInstruction) | THUMB_CODE);
 #else
-        dest = &(pDE->m_breakpointInstruction);
-#endif
-
-        // Here is kind of a cheat... we make sure that the address that we patch and jump to is actually also the ptr
-        // to our DebuggerEval. This works because m_breakpointInstruction is the first field of the DebuggerEval
-        // struct.
-#ifdef _TARGET_ARM_
-        _ASSERTE((((DWORD)dest) & ~THUMB_CODE) == (DWORD)pDE);
-#else
-        _ASSERTE(dest == pDE);
+        dest = &(pDE->m_bpInfoSegment->m_breakpointInstruction);
 #endif
 
         //
diff --git a/src/inc/CrstTypes.def b/src/inc/CrstTypes.def
index fe1da3eb4d..e59c489cd4 100644
--- a/src/inc/CrstTypes.def
+++ b/src/inc/CrstTypes.def
@@ -185,6 +185,10 @@ Crst DebuggerFavorLock
     AcquiredAfter DebuggerJitInfo DebuggerMutex
 End
 
+// This is the lock used by the DebuggerHeapExecutableMemoryAllocator for allocating/freeing memory.
+Crst DebuggerHeapExecMemLock
+End
+
 // Debugger Heap lock is the smallest of the debugger locks.
 Crst DebuggerHeapLock
     AcquiredAfter DebuggerFavorLock DebuggerJitInfo DebuggerMutex
diff --git a/src/inc/crsttypes.h b/src/inc/crsttypes.h
index bb60a300f1..2954ae4dd5 100644
--- a/src/inc/crsttypes.h
+++ b/src/inc/crsttypes.h
@@ -46,143 +46,144 @@ enum CrstType
     CrstDeadlockDetection = 28,
     CrstDebuggerController = 29,
     CrstDebuggerFavorLock = 30,
-    CrstDebuggerHeapLock = 31,
-    CrstDebuggerJitInfo = 32,
-    CrstDebuggerMutex = 33,
-    CrstDelegateToFPtrHash = 34,
-    CrstDomainLocalBlock = 35,
-    CrstDynamicIL = 36,
-    CrstDynamicMT = 37,
-    CrstDynLinkZapItems = 38,
-    CrstEtwTypeLogHash = 39,
-    CrstEventStore = 40,
-    CrstException = 41,
-    CrstExecuteManLock = 42,
-    CrstExecuteManRangeLock = 43,
-    CrstFCall = 44,
-    CrstFriendAccessCache = 45,
-    CrstFuncPtrStubs = 46,
-    CrstFusionAppCtx = 47,
-    CrstFusionAssemblyDownload = 48,
-    CrstFusionBindContext = 49,
-    CrstFusionBindResult = 50,
-    CrstFusionClb = 51,
-    CrstFusionClosure = 52,
-    CrstFusionClosureGraph = 53,
-    CrstFusionConfigSettings = 54,
-    CrstFusionDownload = 55,
-    CrstFusionIsoLibInit = 56,
-    CrstFusionLoadContext = 57,
-    CrstFusionLog = 58,
-    CrstFusionNgenIndex = 59,
-    CrstFusionNgenIndexPool = 60,
-    CrstFusionPcyCache = 61,
-    CrstFusionPolicyConfigPool = 62,
-    CrstFusionSingleUse = 63,
-    CrstFusionWarningLog = 64,
-    CrstGCMemoryPressure = 65,
-    CrstGlobalStrLiteralMap = 66,
-    CrstHandleTable = 67,
-    CrstHostAssemblyMap = 68,
-    CrstHostAssemblyMapAdd = 69,
-    CrstIbcProfile = 70,
-    CrstIJWFixupData = 71,
-    CrstIJWHash = 72,
-    CrstILFingerprintCache = 73,
-    CrstILStubGen = 74,
-    CrstInlineTrackingMap = 75,
-    CrstInstMethodHashTable = 76,
-    CrstInterfaceVTableMap = 77,
-    CrstInterop = 78,
-    CrstInteropData = 79,
-    CrstIOThreadpoolWorker = 80,
-    CrstIsJMCMethod = 81,
-    CrstISymUnmanagedReader = 82,
-    CrstJit = 83,
-    CrstJitGenericHandleCache = 84,
-    CrstJitPerf = 85,
-    CrstJumpStubCache = 86,
-    CrstLeafLock = 87,
-    CrstListLock = 88,
-    CrstLoaderAllocator = 89,
-    CrstLoaderAllocatorReferences = 90,
-    CrstLoaderHeap = 91,
-    CrstMda = 92,
-    CrstMetadataTracker = 93,
-    CrstModIntPairList = 94,
-    CrstModule = 95,
-    CrstModuleFixup = 96,
-    CrstModuleLookupTable = 97,
-    CrstMulticoreJitHash = 98,
-    CrstMulticoreJitManager = 99,
-    CrstMUThunkHash = 100,
-    CrstNativeBinderInit = 101,
-    CrstNativeImageCache = 102,
-    CrstNls = 103,
-    CrstObjectList = 104,
-    CrstOnEventManager = 105,
-    CrstPatchEntryPoint = 106,
-    CrstPEFileSecurityManager = 107,
-    CrstPEImage = 108,
-    CrstPEImagePDBStream = 109,
-    CrstPendingTypeLoadEntry = 110,
-    CrstPinHandle = 111,
-    CrstPinnedByrefValidation = 112,
-    CrstProfilerGCRefDataFreeList = 113,
-    CrstProfilingAPIStatus = 114,
-    CrstPublisherCertificate = 115,
-    CrstRCWCache = 116,
-    CrstRCWCleanupList = 117,
-    CrstRCWRefCache = 118,
-    CrstReDacl = 119,
-    CrstReflection = 120,
-    CrstReJITDomainTable = 121,
-    CrstReJITGlobalRequest = 122,
-    CrstReJITSharedDomainTable = 123,
-    CrstRemoting = 124,
-    CrstRetThunkCache = 125,
-    CrstRWLock = 126,
-    CrstSavedExceptionInfo = 127,
-    CrstSaveModuleProfileData = 128,
-    CrstSecurityPolicyCache = 129,
-    CrstSecurityPolicyInit = 130,
-    CrstSecurityStackwalkCache = 131,
-    CrstSharedAssemblyCreate = 132,
-    CrstSharedBaseDomain = 133,
-    CrstSigConvert = 134,
-    CrstSingleUseLock = 135,
-    CrstSpecialStatics = 136,
-    CrstSqmManager = 137,
-    CrstStackSampler = 138,
-    CrstStressLog = 139,
-    CrstStrongName = 140,
-    CrstStubCache = 141,
-    CrstStubDispatchCache = 142,
-    CrstStubUnwindInfoHeapSegments = 143,
-    CrstSyncBlockCache = 144,
-    CrstSyncHashLock = 145,
-    CrstSystemBaseDomain = 146,
-    CrstSystemDomain = 147,
-    CrstSystemDomainDelayedUnloadList = 148,
-    CrstThreadIdDispenser = 149,
-    CrstThreadpoolEventCache = 150,
-    CrstThreadpoolTimerQueue = 151,
-    CrstThreadpoolWaitThreads = 152,
-    CrstThreadpoolWorker = 153,
-    CrstThreadStaticDataHashTable = 154,
-    CrstThreadStore = 155,
-    CrstTPMethodTable = 156,
-    CrstTypeEquivalenceMap = 157,
-    CrstTypeIDMap = 158,
-    CrstUMEntryThunkCache = 159,
-    CrstUMThunkHash = 160,
-    CrstUniqueStack = 161,
-    CrstUnresolvedClassLock = 162,
-    CrstUnwindInfoTableLock = 163,
-    CrstVSDIndirectionCellLock = 164,
-    CrstWinRTFactoryCache = 165,
-    CrstWrapperTemplate = 166,
-    kNumberOfCrstTypes = 167
+    CrstDebuggerHeapExecMemLock = 31,
+    CrstDebuggerHeapLock = 32,
+    CrstDebuggerJitInfo = 33,
+    CrstDebuggerMutex = 34,
+    CrstDelegateToFPtrHash = 35,
+    CrstDomainLocalBlock = 36,
+    CrstDynamicIL = 37,
+    CrstDynamicMT = 38,
+    CrstDynLinkZapItems = 39,
+    CrstEtwTypeLogHash = 40,
+    CrstEventStore = 41,
+    CrstException = 42,
+    CrstExecuteManLock = 43,
+    CrstExecuteManRangeLock = 44,
+    CrstFCall = 45,
+    CrstFriendAccessCache = 46,
+    CrstFuncPtrStubs = 47,
+    CrstFusionAppCtx = 48,
+    CrstFusionAssemblyDownload = 49,
+    CrstFusionBindContext = 50,
+    CrstFusionBindResult = 51,
+    CrstFusionClb = 52,
+    CrstFusionClosure = 53,
+    CrstFusionClosureGraph = 54,
+    CrstFusionConfigSettings = 55,
+    CrstFusionDownload = 56,
+    CrstFusionIsoLibInit = 57,
+    CrstFusionLoadContext = 58,
+    CrstFusionLog = 59,
+    CrstFusionNgenIndex = 60,
+    CrstFusionNgenIndexPool = 61,
+    CrstFusionPcyCache = 62,
+    CrstFusionPolicyConfigPool = 63,
+    CrstFusionSingleUse = 64,
+    CrstFusionWarningLog = 65,
+    CrstGCMemoryPressure = 66,
+    CrstGlobalStrLiteralMap = 67,
+    CrstHandleTable = 68,
+    CrstHostAssemblyMap = 69,
+    CrstHostAssemblyMapAdd = 70,
+    CrstIbcProfile = 71,
+    CrstIJWFixupData = 72,
+    CrstIJWHash = 73,
+    CrstILFingerprintCache = 74,
+    CrstILStubGen = 75,
+    CrstInlineTrackingMap = 76,
+    CrstInstMethodHashTable = 77,
+    CrstInterfaceVTableMap = 78,
+    CrstInterop = 79,
+    CrstInteropData = 80,
+    CrstIOThreadpoolWorker = 81,
+    CrstIsJMCMethod = 82,
+    CrstISymUnmanagedReader = 83,
+    CrstJit = 84,
+    CrstJitGenericHandleCache = 85,
+    CrstJitPerf = 86,
+    CrstJumpStubCache = 87,
+    CrstLeafLock = 88,
+    CrstListLock = 89,
+    CrstLoaderAllocator = 90,
+    CrstLoaderAllocatorReferences = 91,
+    CrstLoaderHeap = 92,
+    CrstMda = 93,
+    CrstMetadataTracker = 94,
+    CrstModIntPairList = 95,
+    CrstModule = 96,
+    CrstModuleFixup = 97,
+    CrstModuleLookupTable = 98,
+    CrstMulticoreJitHash = 99,
+    CrstMulticoreJitManager = 100,
+    CrstMUThunkHash = 101,
+    CrstNativeBinderInit = 102,
+    CrstNativeImageCache = 103,
+    CrstNls = 104,
+    CrstObjectList = 105,
+    CrstOnEventManager = 106,
+    CrstPatchEntryPoint = 107,
+    CrstPEFileSecurityManager = 108,
+    CrstPEImage = 109,
+    CrstPEImagePDBStream = 110,
+    CrstPendingTypeLoadEntry = 111,
+    CrstPinHandle = 112,
+    CrstPinnedByrefValidation = 113,
+    CrstProfilerGCRefDataFreeList = 114,
+    CrstProfilingAPIStatus = 115,
+    CrstPublisherCertificate = 116,
+    CrstRCWCache = 117,
+    CrstRCWCleanupList = 118,
+    CrstRCWRefCache = 119,
+    CrstReDacl = 120,
+    CrstReflection = 121,
+    CrstReJITDomainTable = 122,
+    CrstReJITGlobalRequest = 123,
+    CrstReJITSharedDomainTable = 124,
+    CrstRemoting = 125,
+    CrstRetThunkCache = 126,
+    CrstRWLock = 127,
+    CrstSavedExceptionInfo = 128,
+    CrstSaveModuleProfileData = 129,
+    CrstSecurityPolicyCache = 130,
+    CrstSecurityPolicyInit = 131,
+    CrstSecurityStackwalkCache = 132,
+    CrstSharedAssemblyCreate = 133,
+    CrstSharedBaseDomain = 134,
+    CrstSigConvert = 135,
+    CrstSingleUseLock = 136,
+    CrstSpecialStatics = 137,
+    CrstSqmManager = 138,
+    CrstStackSampler = 139,
+    CrstStressLog = 140,
+    CrstStrongName = 141,
+    CrstStubCache = 142,
+    CrstStubDispatchCache = 143,
+    CrstStubUnwindInfoHeapSegments = 144,
+    CrstSyncBlockCache = 145,
+    CrstSyncHashLock = 146,
+    CrstSystemBaseDomain = 147,
+    CrstSystemDomain = 148,
+    CrstSystemDomainDelayedUnloadList = 149,
+    CrstThreadIdDispenser = 150,
+    CrstThreadpoolEventCache = 151,
+    CrstThreadpoolTimerQueue = 152,
+    CrstThreadpoolWaitThreads = 153,
+    CrstThreadpoolWorker = 154,
+    CrstThreadStaticDataHashTable = 155,
+    CrstThreadStore = 156,
+    CrstTPMethodTable = 157,
+    CrstTypeEquivalenceMap = 158,
+    CrstTypeIDMap = 159,
+    CrstUMEntryThunkCache = 160,
+    CrstUMThunkHash = 161,
+    CrstUniqueStack = 162,
+    CrstUnresolvedClassLock = 163,
+    CrstUnwindInfoTableLock = 164,
+    CrstVSDIndirectionCellLock = 165,
+    CrstWinRTFactoryCache = 166,
+    CrstWrapperTemplate = 167,
+    kNumberOfCrstTypes = 168
 };
 
 #endif // __CRST_TYPES_INCLUDED
@@ -224,6 +225,7 @@ int g_rgCrstLevelMap[] =
     0,			// CrstDeadlockDetection
     -1,			// CrstDebuggerController
     3,			// CrstDebuggerFavorLock
+    0,			// CrstDebuggerHeapExecMemLock
     0,			// CrstDebuggerHeapLock
     4,			// CrstDebuggerJitInfo
     11,			// CrstDebuggerMutex
@@ -396,6 +398,7 @@ LPCSTR g_rgCrstNameMap[] =
     "CrstDeadlockDetection",
     "CrstDebuggerController",
     "CrstDebuggerFavorLock",
+    "CrstDebuggerHeapExecMemLock",
     "CrstDebuggerHeapLock",
     "CrstDebuggerJitInfo",
     "CrstDebuggerMutex",