diff options
author | Jan Vorlicek <jan.vorlicek@volny.cz> | 2021-06-12 10:54:58 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-06-12 10:54:58 +0300 |
commit | cc6d3147ff1f10d2ee2b0751cf799fc4cc1abbae (patch) | |
tree | 6f152bfc4ba7880faff9b2189e25d6b85743771a /src/coreclr/debug/ee | |
parent | 2a011f802b83051d1381f7800131ca5734b4c59e (diff) |
Unify macOS ARM64 write protection holders with W^X ones (#54067)
* Unify macOS ARM64 write protection holders with W^X ones
This change removes the original holders that were added for changing
memory protection for executable code and moves the actual switching to
the recently added W^X holders.
The unixexports files don't support target specific symbols. So I needed
to export a dummy version of the PAL_JitWriteProtect for macOS x64.
Diffstat (limited to 'src/coreclr/debug/ee')
-rw-r--r-- | src/coreclr/debug/ee/controller.cpp | 29 | ||||
-rw-r--r-- | src/coreclr/debug/ee/debugger.cpp | 12 | ||||
-rw-r--r-- | src/coreclr/debug/ee/debugger.h | 81 |
3 files changed, 28 insertions, 94 deletions
diff --git a/src/coreclr/debug/ee/controller.cpp b/src/coreclr/debug/ee/controller.cpp index 01aedf586e1..eed3c453e78 100644 --- a/src/coreclr/debug/ee/controller.cpp +++ b/src/coreclr/debug/ee/controller.cpp @@ -83,7 +83,11 @@ SharedPatchBypassBuffer* DebuggerControllerPatch::GetOrCreateSharedPatchBypassBu if (m_pSharedPatchBypassBuffer == NULL) { - m_pSharedPatchBypassBuffer = new (interopsafeEXEC) SharedPatchBypassBuffer(); + void *pSharedPatchBypassBufferRX = g_pDebugger->GetInteropSafeExecutableHeap()->Alloc(sizeof(SharedPatchBypassBuffer)); + ExecutableWriterHolder<SharedPatchBypassBuffer> sharedPatchBypassBufferWriterHolder((SharedPatchBypassBuffer*)pSharedPatchBypassBufferRX, sizeof(SharedPatchBypassBuffer)); + new (sharedPatchBypassBufferWriterHolder.GetRW()) SharedPatchBypassBuffer(); + m_pSharedPatchBypassBuffer = (SharedPatchBypassBuffer*)pSharedPatchBypassBufferRX; + _ASSERTE(m_pSharedPatchBypassBuffer); TRACE_ALLOC(m_pSharedPatchBypassBuffer); } @@ -1364,9 +1368,7 @@ bool DebuggerController::ApplyPatch(DebuggerControllerPatch *patch) LPVOID baseAddress = (LPVOID)(patch->address); -#if defined(HOST_OSX) && defined(HOST_ARM64) - auto jitWriteEnableHolder = PAL_JITWriteEnable(true); -#else // defined(HOST_OSX) && defined(HOST_ARM64) +#if !defined(HOST_OSX) || !defined(HOST_ARM64) DWORD oldProt; if (!VirtualProtect(baseAddress, @@ -1376,7 +1378,7 @@ bool DebuggerController::ApplyPatch(DebuggerControllerPatch *patch) _ASSERTE(!"VirtualProtect of code page failed"); return false; } -#endif // defined(HOST_OSX) && defined(HOST_ARM64) +#endif // !defined(HOST_OSX) || !defined(HOST_ARM64) patch->opcode = CORDbgGetInstruction(patch->address); @@ -1391,7 +1393,7 @@ bool DebuggerController::ApplyPatch(DebuggerControllerPatch *patch) _ASSERTE(!"VirtualProtect of code page failed"); return false; } -#endif // !defined(HOST_OSX) || !defined(HOST_ARM64) +#endif // !defined(HOST_OSX) || !defined(HOST_ARM64) } // TODO: : determine if this is needed for AMD64 #if defined(TARGET_X86) //REVISIT_TODO what is this?! @@ -1408,12 +1410,14 @@ bool DebuggerController::ApplyPatch(DebuggerControllerPatch *patch) _ASSERTE(!"VirtualProtect of code page failed"); return false; } + patch->opcode = (unsigned int) *(unsigned short*)(patch->address+1); _ASSERTE(patch->opcode != CEE_BREAK); - *(unsigned short *) (patch->address+1) = CEE_BREAK; + ExecutableWriterHolder<BYTE> breakpointWriterHolder((BYTE*)patch->address, 2); + *(unsigned short *) (breakpointWriterHolder.GetRW()+1) = CEE_BREAK; if (!VirtualProtect((void *) patch->address, 2, oldProt, &oldProt)) { @@ -1460,9 +1464,7 @@ bool DebuggerController::UnapplyPatch(DebuggerControllerPatch *patch) LPVOID baseAddress = (LPVOID)(patch->address); -#if defined(HOST_OSX) && defined(HOST_ARM64) - auto jitWriteEnableHolder = PAL_JITWriteEnable(true); -#else // defined(HOST_OSX) && defined(HOST_ARM64) +#if !defined(HOST_OSX) || !defined(HOST_ARM64) DWORD oldProt; if (!VirtualProtect(baseAddress, @@ -1477,7 +1479,7 @@ bool DebuggerController::UnapplyPatch(DebuggerControllerPatch *patch) InitializePRD(&(patch->opcode)); return false; } -#endif // defined(HOST_OSX) && defined(HOST_ARM64) +#endif // !defined(HOST_OSX) || !defined(HOST_ARM64) CORDbgSetInstruction((CORDB_ADDRESS_TYPE *)patch->address, patch->opcode); @@ -1494,7 +1496,7 @@ bool DebuggerController::UnapplyPatch(DebuggerControllerPatch *patch) _ASSERTE(!"VirtualProtect of code page failed"); return false; } -#endif // !defined(HOST_OSX) || !defined(HOST_ARM64) +#endif // !defined(HOST_OSX) || !defined(HOST_ARM64) } else { @@ -1519,7 +1521,8 @@ bool DebuggerController::UnapplyPatch(DebuggerControllerPatch *patch) #if defined(TARGET_X86) _ASSERTE(*(unsigned short*)(patch->address+1) == CEE_BREAK); - *(unsigned short *) (patch->address+1) + ExecutableWriterHolder<BYTE> breakpointWriterHolder((BYTE*)patch->address, 2); + *(unsigned short *) (breakpointWriterHolder.GetRW()+1) = (unsigned short) patch->opcode; #endif //this makes no sense on anything but X86 //VERY IMPORTANT to zero out opcode, else we might mistake diff --git a/src/coreclr/debug/ee/debugger.cpp b/src/coreclr/debug/ee/debugger.cpp index d8791362ca7..4706790dd3d 100644 --- a/src/coreclr/debug/ee/debugger.cpp +++ b/src/coreclr/debug/ee/debugger.cpp @@ -67,7 +67,6 @@ bool g_EnableSIS = false; // The following instances are used for invoking overloaded new/delete InteropSafe interopsafe; -InteropSafeExecutable interopsafeEXEC; #ifndef DACCESS_COMPILE @@ -1316,16 +1315,15 @@ DebuggerEval::DebuggerEval(CONTEXT * pContext, DebuggerIPCE_FuncEvalInfo * pEval { WRAPPER_NO_CONTRACT; -#if defined(HOST_OSX) && defined(HOST_ARM64) - auto jitWriteEnableHolder = PAL_JITWriteEnable(true); -#endif // defined(HOST_OSX) && defined(HOST_ARM64) - // Allocate the breakpoint instruction info in executable memory. - m_bpInfoSegment = new (interopsafeEXEC, nothrow) DebuggerEvalBreakpointInfoSegment(this); + void *bpInfoSegmentRX = g_pDebugger->GetInteropSafeExecutableHeap()->Alloc(sizeof(DebuggerEvalBreakpointInfoSegment)); + ExecutableWriterHolder<DebuggerEvalBreakpointInfoSegment> bpInfoSegmentWriterHolder((DebuggerEvalBreakpointInfoSegment*)bpInfoSegmentRX, sizeof(DebuggerEvalBreakpointInfoSegment)); + new (bpInfoSegmentWriterHolder.GetRW()) DebuggerEvalBreakpointInfoSegment(this); + m_bpInfoSegment = (DebuggerEvalBreakpointInfoSegment*)bpInfoSegmentRX; // 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_bpInfoSegment->m_breakpointInstruction[0] = 0x16; + bpInfoSegmentWriterHolder.GetRW()->m_breakpointInstruction[0] = 0x16; #if defined(TARGET_ARM) USHORT *bp = (USHORT*)&m_bpInfoSegment->m_breakpointInstruction; *bp = CORDbg_BREAK_INSTRUCTION; diff --git a/src/coreclr/debug/ee/debugger.h b/src/coreclr/debug/ee/debugger.h index d66198607e5..9d80fb3eec7 100644 --- a/src/coreclr/debug/ee/debugger.h +++ b/src/coreclr/debug/ee/debugger.h @@ -1104,11 +1104,8 @@ struct DECLSPEC_ALIGN(4096) DebuggerHeapExecutableMemoryPage inline void SetNextPage(DebuggerHeapExecutableMemoryPage* nextPage) { -#if defined(HOST_OSX) && defined(HOST_ARM64) - auto jitWriteEnableHolder = PAL_JITWriteEnable(true); -#endif // defined(HOST_OSX) && defined(HOST_ARM64) - - chunks[0].bookkeeping.nextPage = nextPage; + ExecutableWriterHolder<DebuggerHeapExecutableMemoryPage> debuggerHeapPageWriterHolder(this, sizeof(DebuggerHeapExecutableMemoryPage)); + debuggerHeapPageWriterHolder.GetRW()->chunks[0].bookkeeping.nextPage = nextPage; } inline uint64_t GetPageOccupancy() const @@ -1118,14 +1115,11 @@ struct DECLSPEC_ALIGN(4096) DebuggerHeapExecutableMemoryPage inline void SetPageOccupancy(uint64_t newOccupancy) { -#if defined(HOST_OSX) && defined(HOST_ARM64) - auto jitWriteEnableHolder = PAL_JITWriteEnable(true); -#endif // defined(HOST_OSX) && defined(HOST_ARM64) - // Can't unset first bit of occupancy! ASSERT((newOccupancy & 0x8000000000000000) != 0); - chunks[0].bookkeeping.pageOccupancy = newOccupancy; + ExecutableWriterHolder<DebuggerHeapExecutableMemoryPage> debuggerHeapPageWriterHolder(this, sizeof(DebuggerHeapExecutableMemoryPage)); + debuggerHeapPageWriterHolder.GetRW()->chunks[0].bookkeeping.pageOccupancy = newOccupancy; } inline void* GetPointerToChunk(int chunkNum) const @@ -1135,16 +1129,14 @@ struct DECLSPEC_ALIGN(4096) DebuggerHeapExecutableMemoryPage DebuggerHeapExecutableMemoryPage() { -#if defined(HOST_OSX) && defined(HOST_ARM64) - auto jitWriteEnableHolder = PAL_JITWriteEnable(true); -#endif // defined(HOST_OSX) && defined(HOST_ARM64) + ExecutableWriterHolder<DebuggerHeapExecutableMemoryPage> debuggerHeapPageWriterHolder(this, sizeof(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; + debuggerHeapPageWriterHolder.GetRW()->chunks[i].data.startOfPage = this; + debuggerHeapPageWriterHolder.GetRW()->chunks[i].data.chunkNumber = i; } } @@ -3486,9 +3478,6 @@ public: class InteropSafe {}; extern InteropSafe interopsafe; -class InteropSafeExecutable {}; -extern InteropSafeExecutable interopsafeEXEC; - #ifndef DACCESS_COMPILE inline void * __cdecl operator new(size_t n, const InteropSafe&) { @@ -3631,62 +3620,6 @@ template<class T> void DeleteInteropSafe(T *p) } } -inline void * __cdecl operator new(size_t n, const InteropSafeExecutable&) -{ - CONTRACTL - { - THROWS; // throw on OOM - GC_NOTRIGGER; - } - CONTRACTL_END; - - _ASSERTE(g_pDebugger != NULL); - void *result = g_pDebugger->GetInteropSafeExecutableHeap()->Alloc((DWORD)n); - if (result == NULL) { - ThrowOutOfMemory(); - } - return result; -} - -inline void * __cdecl operator new(size_t n, const InteropSafeExecutable&, const NoThrow&) throw() -{ - CONTRACTL - { - NOTHROW; - GC_NOTRIGGER; - } - CONTRACTL_END; - - _ASSERTE(g_pDebugger != NULL); - DebuggerHeap * pHeap = g_pDebugger->GetInteropSafeExecutableHeap_NoThrow(); - if (pHeap == NULL) - { - return NULL; - } - void *result = pHeap->Alloc((DWORD)n); - return result; -} - -// Note: there is no C++ syntax for manually invoking this, but if a constructor throws an exception I understand that -// this delete operator will be invoked automatically to destroy the object. -inline void __cdecl operator delete(void *p, const InteropSafeExecutable&) -{ - CONTRACTL - { - NOTHROW; - GC_NOTRIGGER; - } - CONTRACTL_END; - - if (p != NULL) - { - _ASSERTE(g_pDebugger != NULL); - DebuggerHeap * pHeap = g_pDebugger->GetInteropSafeExecutableHeap_NoThrow(); - _ASSERTE(pHeap != NULL); // should have had heap around if we're deleting - pHeap->Free(p); - } -} - // // Interop safe delete to match the interop safe new's above. There is no C++ syntax for actually invoking those interop // safe delete operators above, so we use this method to accomplish the same thing. |