From 056ee746559b3499e3c3a3247b8c842aa832b97c Mon Sep 17 00:00:00 2001 From: Aditya Mandaleeka Date: Thu, 3 Dec 2015 16:54:43 -0800 Subject: Implement allocator in debugger code for handling allocations that need to be executable --- src/debug/ee/debugger.cpp | 252 +++++++++++++++++++++++++++++++++-------- src/debug/ee/debugger.h | 136 +++++++++++++++++++++++ src/inc/CrstTypes.def | 4 + src/inc/crsttypes.h | 277 +++++++++++++++++++++++----------------------- 4 files changed, 485 insertions(+), 184 deletions(-) (limited to 'src') diff --git a/src/debug/ee/debugger.cpp b/src/debug/ee/debugger.cpp index 613a64e847..1ee88ab228 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 @@ -16557,6 +16556,156 @@ void Debugger::ReleaseDebuggerDataLock(Debugger *pDebugger) } #endif // DACCESS_COMPILE +/* ------------------------------------------------------------------------ * + * Functions for DebuggerHeap executable memory allocations + * ------------------------------------------------------------------------ */ + +DebuggerHeapExecutableMemoryAllocator::~DebuggerHeapExecutableMemoryAllocator() +{ + DebuggerHeapExecutableMemoryPage *currPage = pages; + while (currPage != NULL) + { + currPage = currPage->GetNextPage(); + + // Free this page + INDEBUG(BOOL ret =) VirtualFree(currPage, 0, MEM_RELEASE); + ASSERT(ret == TRUE); + + pages = currPage; + } + + ASSERT(pages == NULL); +} + +void* DebuggerHeapExecutableMemoryAllocator::Allocate(DWORD numBytes) +{ + if (numBytes > 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 (numBytes == 0) + { + // Should we allocate anything in this case? + ASSERT(!"Allocate called with 0 for numBytes!"); + return NULL; + } + + CrstHolder execMemAllocCrstHolder(&m_execMemAllocMutex); + + int chunkToUse = -1; + DebuggerHeapExecutableMemoryPage *pageToAllocateOn = NULL; + for (DebuggerHeapExecutableMemoryPage *currPage = 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; + } + } + + void* addr = ChangePageUsage(pageToAllocateOn, chunkToUse, true /* allocate */); + return addr; +} + +int DebuggerHeapExecutableMemoryAllocator::Free(void* addr) +{ + CrstHolder execMemAllocCrstHolder(&m_execMemAllocMutex); + + DebuggerHeapExecutableMemoryPage *pageToFreeIn = static_cast(addr)->d.startOfPage; + + if (pageToFreeIn == NULL) + { + ASSERT(!"Couldn't locate page in which to free!"); + return -1; + } + + int chunkNum = static_cast(addr)->d.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, false /* indicates free */); + + return 0; +} + +DebuggerHeapExecutableMemoryPage* DebuggerHeapExecutableMemoryAllocator::AddNewPage() +{ + void* newPageAddr = VirtualAlloc(NULL, sizeof(DebuggerHeapExecutableMemoryPage), MEM_COMMIT | MEM_RESERVE, PAGE_EXECUTE_READWRITE); + + // Make sure new page is aligned to 4096. + ASSERT(((uint64_t)newPageAddr & 0xFFF) == 0); + + DebuggerHeapExecutableMemoryPage *newPage = new (newPageAddr) DebuggerHeapExecutableMemoryPage; + newPage->SetNextPage(pages); + + // Add the new page to the linked list of pages + 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 = ((unsigned long long)1 << i); + if ((mask & occupancy) == 0) + { + *chunkToUse = 64-i-1; + break; + } + } + } + + return true; +} + +void* DebuggerHeapExecutableMemoryAllocator::ChangePageUsage(DebuggerHeapExecutableMemoryPage* page, int chunkNum, bool allocateOrFree) +{ + uint64_t mask = (uint64_t)0x1 << (64 - chunkNum - 1); + + uint64_t prevOccupancy = page->GetPageOccupancy(); + uint64_t newOccupancy = allocateOrFree ? (prevOccupancy | mask) : (prevOccupancy ^ mask); + page->SetPageOccupancy(newOccupancy); + + return page->GetPointerToChunk(chunkNum); +} + /* ------------------------------------------------------------------------ * * DebuggerHeap impl * ------------------------------------------------------------------------ */ @@ -16569,7 +16718,6 @@ DebuggerHeap::DebuggerHeap() m_fExecutable = FALSE; } - DebuggerHeap::~DebuggerHeap() { CONTRACTL @@ -16592,6 +16740,12 @@ void DebuggerHeap::Destroy() m_hHeap = NULL; } #endif +#ifdef FEATURE_PAL + if (m_execMemAllocator != NULL) + { + delete m_execMemAllocator; + } +#endif } bool DebuggerHeap::IsInit() @@ -16639,6 +16793,16 @@ HRESULT DebuggerHeap::Init(BOOL fExecutable) return HRESULT_FROM_GetLastError(); } #endif + +#ifdef FEATURE_PAL + m_execMemAllocator = new DebuggerHeapExecutableMemoryAllocator(); + ASSERT(m_execMemAllocator != NULL); + if (m_execMemAllocator == NULL) + { + return E_OUTOFMEMORY; + } +#endif + return S_OK; } @@ -16721,17 +16885,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 @@ -16743,25 +16923,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) @@ -16778,7 +16942,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; @@ -16800,22 +16964,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; } @@ -16849,12 +16997,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..71801e79a3 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[48]; + + DebuggerHeapExecutableMemoryPage *startOfPage; + + // The chunk number within the page. + uint8_t chunkNumber; + + } d; + + struct BookkeepingChunk + { + DebuggerHeapExecutableMemoryPage *nextPage; + + uint64_t pageOccupancy; + + char padding[48]; + + } b; +}; + +// ------------------------------------------------------------------------ */ +// 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].b.nextPage; + } + + inline void SetNextPage(DebuggerHeapExecutableMemoryPage* nextPage) + { + chunks[0].b.nextPage = nextPage; + } + + inline uint64_t GetPageOccupancy() + { + return chunks[0].b.pageOccupancy; + } + + inline void SetPageOccupancy(uint64_t newOccupancy) + { + // Can't unset first bit of occupancy! + ASSERT((newOccupancy & 0x8000000000000000) != 0); + + chunks[0].b.pageOccupancy = newOccupancy; + } + + inline void* GetPointerToChunk(int chunkNum) + { + 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].d.startOfPage = this; + chunks[i].d.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_execMemAllocMutex(CrstDebuggerHeapExecMemLock, (CrstFlags)(CRST_UNSAFE_ANYMODE | CRST_REENTRANCY | CRST_DEBUGGER_THREAD)) + { + pages = NULL; + } + + ~DebuggerHeapExecutableMemoryAllocator(); + + void* Allocate(DWORD numBytes); + int Free(void* addr); + +private: + DebuggerHeapExecutableMemoryPage* AddNewPage(); + bool CheckPageForAvailability(DebuggerHeapExecutableMemoryPage* page, /* _Out_ */ int* chunkToUse); + void* ChangePageUsage(DebuggerHeapExecutableMemoryPage* page, int chunkNum, bool allocateOrFree); + +private: + // Linked list of pages that have been allocated + DebuggerHeapExecutableMemoryPage* pages; + Crst m_execMemAllocMutex; +}; // ------------------------------------------------------------------------ * // DebuggerHeap class @@ -1104,6 +1237,9 @@ protected: HANDLE m_hHeap; #endif BOOL m_fExecutable; + +private: + DebuggerHeapExecutableMemoryAllocator *m_execMemAllocator; }; class DebuggerJitInfo; 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", -- cgit v1.2.3 From 939730ffdc60fd05991ffa38999f9bddcf9c4d4d Mon Sep 17 00:00:00 2001 From: Aditya Mandaleeka Date: Tue, 29 Dec 2015 12:21:57 -0800 Subject: Separate DebuggerEval into executable and non-executable portions. --- src/debug/ee/controller.cpp | 4 +- src/debug/ee/debugger.cpp | 74 ++++++++++++++------------ src/debug/ee/debugger.h | 126 +++++++++++++++++++++++++------------------- src/debug/ee/funceval.cpp | 15 ++---- 4 files changed, 116 insertions(+), 103 deletions(-) (limited to 'src') 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(((DWORD)dest) & ~THUMB_CODE); + m_pDE = reinterpret_cast(((DWORD)dest) & ~THUMB_CODE)->m_associatedDebuggerEval; #else - m_pDE = reinterpret_cast(dest); + m_pDE = reinterpret_cast(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 1ee88ab228..1959d50d67 100644 --- a/src/debug/ee/debugger.cpp +++ b/src/debug/ee/debugger.cpp @@ -1390,11 +1390,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(); @@ -1468,9 +1471,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; @@ -15354,7 +15360,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) { @@ -15486,7 +15492,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) { @@ -15676,7 +15682,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; } @@ -16562,33 +16569,32 @@ void Debugger::ReleaseDebuggerDataLock(Debugger *pDebugger) DebuggerHeapExecutableMemoryAllocator::~DebuggerHeapExecutableMemoryAllocator() { - DebuggerHeapExecutableMemoryPage *currPage = pages; - while (currPage != NULL) + while (m_pages != NULL) { - currPage = currPage->GetNextPage(); + DebuggerHeapExecutableMemoryPage *temp = m_pages->GetNextPage(); // Free this page - INDEBUG(BOOL ret =) VirtualFree(currPage, 0, MEM_RELEASE); + INDEBUG(BOOL ret =) VirtualFree(m_pages, 0, MEM_RELEASE); ASSERT(ret == TRUE); - pages = currPage; + m_pages = temp; } - ASSERT(pages == NULL); + ASSERT(m_pages == NULL); } -void* DebuggerHeapExecutableMemoryAllocator::Allocate(DWORD numBytes) +void* DebuggerHeapExecutableMemoryAllocator::Allocate(DWORD numberOfBytes) { - if (numBytes > DBG_MAX_EXECUTABLE_ALLOC_SIZE) + 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 (numBytes == 0) + if (numberOfBytes == 0) { // Should we allocate anything in this case? - ASSERT(!"Allocate called with 0 for numBytes!"); + ASSERT(!"Allocate called with 0 for numberOfBytes!"); return NULL; } @@ -16596,7 +16602,7 @@ void* DebuggerHeapExecutableMemoryAllocator::Allocate(DWORD numBytes) int chunkToUse = -1; DebuggerHeapExecutableMemoryPage *pageToAllocateOn = NULL; - for (DebuggerHeapExecutableMemoryPage *currPage = pages; currPage != NULL; currPage = currPage->GetNextPage()) + for (DebuggerHeapExecutableMemoryPage *currPage = m_pages; currPage != NULL; currPage = currPage->GetNextPage()) { if (CheckPageForAvailability(currPage, &chunkToUse)) { @@ -16622,15 +16628,16 @@ void* DebuggerHeapExecutableMemoryAllocator::Allocate(DWORD numBytes) } } - void* addr = ChangePageUsage(pageToAllocateOn, chunkToUse, true /* allocate */); - return addr; + return ChangePageUsage(pageToAllocateOn, chunkToUse, ChangePageUsageAction::ALLOCATE); } int DebuggerHeapExecutableMemoryAllocator::Free(void* addr) { + ASSERT(addr != NULL); + CrstHolder execMemAllocCrstHolder(&m_execMemAllocMutex); - DebuggerHeapExecutableMemoryPage *pageToFreeIn = static_cast(addr)->d.startOfPage; + DebuggerHeapExecutableMemoryPage *pageToFreeIn = static_cast(addr)->data.startOfPage; if (pageToFreeIn == NULL) { @@ -16638,12 +16645,12 @@ int DebuggerHeapExecutableMemoryAllocator::Free(void* addr) return -1; } - int chunkNum = static_cast(addr)->d.chunkNumber; + int chunkNum = static_cast(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, false /* indicates free */); + ChangePageUsage(pageToFreeIn, chunkNum, ChangePageUsageAction::FREE); return 0; } @@ -16652,14 +16659,11 @@ DebuggerHeapExecutableMemoryPage* DebuggerHeapExecutableMemoryAllocator::AddNewP { void* newPageAddr = VirtualAlloc(NULL, sizeof(DebuggerHeapExecutableMemoryPage), MEM_COMMIT | MEM_RESERVE, PAGE_EXECUTE_READWRITE); - // Make sure new page is aligned to 4096. - ASSERT(((uint64_t)newPageAddr & 0xFFF) == 0); - DebuggerHeapExecutableMemoryPage *newPage = new (newPageAddr) DebuggerHeapExecutableMemoryPage; - newPage->SetNextPage(pages); + newPage->SetNextPage(m_pages); // Add the new page to the linked list of pages - pages = newPage; + m_pages = newPage; return newPage; } @@ -16683,10 +16687,10 @@ bool DebuggerHeapExecutableMemoryAllocator::CheckPageForAvailability(DebuggerHea // Start i at 62 because first chunk is reserved for (int i = 62; i >= 0; i--) { - uint64_t mask = ((unsigned long long)1 << i); + uint64_t mask = ((uint64_t)1 << i); if ((mask & occupancy) == 0) { - *chunkToUse = 64-i-1; + *chunkToUse = 64 - i - 1; break; } } @@ -16695,15 +16699,17 @@ bool DebuggerHeapExecutableMemoryAllocator::CheckPageForAvailability(DebuggerHea return true; } -void* DebuggerHeapExecutableMemoryAllocator::ChangePageUsage(DebuggerHeapExecutableMemoryPage* page, int chunkNum, bool allocateOrFree) +void* DebuggerHeapExecutableMemoryAllocator::ChangePageUsage(DebuggerHeapExecutableMemoryPage* page, int chunkNumber, ChangePageUsageAction action) { - uint64_t mask = (uint64_t)0x1 << (64 - chunkNum - 1); + ASSERT(action == ChangePageUsageAction::ALLOCATE || action == ChangePageUsageAction::FREE); + + uint64_t mask = (uint64_t)0x1 << (64 - chunkNumber - 1); uint64_t prevOccupancy = page->GetPageOccupancy(); - uint64_t newOccupancy = allocateOrFree ? (prevOccupancy | mask) : (prevOccupancy ^ mask); + uint64_t newOccupancy = (action == ChangePageUsageAction::ALLOCATE) ? (prevOccupancy | mask) : (prevOccupancy ^ mask); page->SetPageOccupancy(newOccupancy); - return page->GetPointerToChunk(chunkNum); + return page->GetPointerToChunk(chunkNumber); } /* ------------------------------------------------------------------------ * @@ -16795,7 +16801,7 @@ HRESULT DebuggerHeap::Init(BOOL fExecutable) #endif #ifdef FEATURE_PAL - m_execMemAllocator = new DebuggerHeapExecutableMemoryAllocator(); + m_execMemAllocator = new (nothrow) DebuggerHeapExecutableMemoryAllocator(); ASSERT(m_execMemAllocator != NULL); if (m_execMemAllocator == NULL) { diff --git a/src/debug/ee/debugger.h b/src/debug/ee/debugger.h index 71801e79a3..b471fba601 100644 --- a/src/debug/ee/debugger.h +++ b/src/debug/ee/debugger.h @@ -1098,14 +1098,14 @@ union DECLSPEC_ALIGN(64) DebuggerHeapExecutableMemoryChunk { struct DataChunk { - char data[48]; + char data[DBG_MAX_EXECUTABLE_ALLOC_SIZE]; DebuggerHeapExecutableMemoryPage *startOfPage; // The chunk number within the page. uint8_t chunkNumber; - } d; + } data; struct BookkeepingChunk { @@ -1113,11 +1113,11 @@ union DECLSPEC_ALIGN(64) DebuggerHeapExecutableMemoryChunk { uint64_t pageOccupancy; - char padding[48]; - - } b; + } bookkeeping; }; +static_assert(sizeof(DebuggerHeapExecutableMemoryChunk) == 64, "DebuggerHeapExecutableMemoryChunk is expect to be 64 bytes."); + // ------------------------------------------------------------------------ */ // DebuggerHeapExecutableMemoryPage // @@ -1131,17 +1131,17 @@ struct DECLSPEC_ALIGN(4096) DebuggerHeapExecutableMemoryPage { inline DebuggerHeapExecutableMemoryPage* GetNextPage() { - return chunks[0].b.nextPage; + return chunks[0].bookkeeping.nextPage; } inline void SetNextPage(DebuggerHeapExecutableMemoryPage* nextPage) { - chunks[0].b.nextPage = nextPage; + chunks[0].bookkeeping.nextPage = nextPage; } - inline uint64_t GetPageOccupancy() + inline uint64_t GetPageOccupancy() const { - return chunks[0].b.pageOccupancy; + return chunks[0].bookkeeping.pageOccupancy; } inline void SetPageOccupancy(uint64_t newOccupancy) @@ -1149,10 +1149,10 @@ struct DECLSPEC_ALIGN(4096) DebuggerHeapExecutableMemoryPage // Can't unset first bit of occupancy! ASSERT((newOccupancy & 0x8000000000000000) != 0); - chunks[0].b.pageOccupancy = newOccupancy; + chunks[0].bookkeeping.pageOccupancy = newOccupancy; } - inline void* GetPointerToChunk(int chunkNum) + inline void* GetPointerToChunk(int chunkNum) const { return (char*)this + chunkNum * sizeof(DebuggerHeapExecutableMemoryChunk); } @@ -1163,14 +1163,13 @@ struct DECLSPEC_ALIGN(4096) DebuggerHeapExecutableMemoryPage for (uint8_t i = 1; i < sizeof(chunks)/sizeof(chunks[0]); i++) { ASSERT(i != 0); - chunks[i].d.startOfPage = this; - chunks[i].d.chunkNumber = i; + chunks[i].data.startOfPage = this; + chunks[i].data.chunkNumber = i; } } private: DebuggerHeapExecutableMemoryChunk chunks[64]; - }; // ------------------------------------------------------------------------ */ @@ -1185,24 +1184,25 @@ class DebuggerHeapExecutableMemoryAllocator { public: DebuggerHeapExecutableMemoryAllocator() - : m_execMemAllocMutex(CrstDebuggerHeapExecMemLock, (CrstFlags)(CRST_UNSAFE_ANYMODE | CRST_REENTRANCY | CRST_DEBUGGER_THREAD)) - { - pages = NULL; - } + : m_pages(NULL) + , m_execMemAllocMutex(CrstDebuggerHeapExecMemLock, (CrstFlags)(CRST_UNSAFE_ANYMODE | CRST_REENTRANCY | CRST_DEBUGGER_THREAD)) + { } ~DebuggerHeapExecutableMemoryAllocator(); - void* Allocate(DWORD numBytes); + 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 chunkNum, bool allocateOrFree); + void* ChangePageUsage(DebuggerHeapExecutableMemoryPage* page, int chunkNumber, ChangePageUsageAction action); private: // Linked list of pages that have been allocated - DebuggerHeapExecutableMemoryPage* pages; + DebuggerHeapExecutableMemoryPage* m_pages; Crst m_execMemAllocMutex; }; @@ -3342,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 * @@ -3378,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); @@ -3418,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 // -- cgit v1.2.3