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/debug') 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