From 76ed9310f290075073d1b7c49de3cae1f59113db Mon Sep 17 00:00:00 2001 From: Skyler Saleh Date: Sat, 15 May 2021 07:10:10 -0700 Subject: [PATCH] Apple M1: RAII Wrapper for JITPageWrite*Execute*() Added RAII wrapper around the the JITPageWriteEnableExecuteDisable() and JITPageWriteDisableExecuteEnable() to make it so that it is harder to forget to pair the calls in all code branches as suggested by leoetlino. --- Source/Core/Common/MemoryUtil.h | 13 +++++++++++++ Source/Core/Core/PowerPC/JitArm64/Jit.cpp | 7 ++----- Source/Core/Core/PowerPC/JitArm64/JitArm64Cache.cpp | 6 ++---- .../Core/PowerPC/JitArm64/JitArm64_BackPatch.cpp | 3 +-- Source/Core/Core/PowerPC/JitArm64/JitAsm.cpp | 3 +-- Source/Core/VideoCommon/VertexLoaderARM64.cpp | 3 +-- 6 files changed, 20 insertions(+), 15 deletions(-) diff --git a/Source/Core/Common/MemoryUtil.h b/Source/Core/Common/MemoryUtil.h index b64d5cd8de..f13c55dab4 100644 --- a/Source/Core/Common/MemoryUtil.h +++ b/Source/Core/Common/MemoryUtil.h @@ -10,10 +10,23 @@ namespace Common { void* AllocateExecutableMemory(size_t size); + +// These two functions control the executable/writable state of the W^X memory +// allocations. More detailed documentation about them is in the .cpp file. +// In general where applicable the ScopedJITPageWriteAndNoExecute wrapper +// should be used to prevent bugs from not pairing up the calls properly. + // Allows a thread to write to executable memory, but not execute the data. void JITPageWriteEnableExecuteDisable(); // Allows a thread to execute memory allocated for execution, but not write to it. void JITPageWriteDisableExecuteEnable(); +// RAII Wrapper around JITPageWrite*Execute*(). When this is in scope the thread can +// write to executable memory but not execute it. +struct ScopedJITPageWriteAndNoExecute +{ + ScopedJITPageWriteAndNoExecute() { JITPageWriteEnableExecuteDisable(); } + ~ScopedJITPageWriteAndNoExecute() { JITPageWriteDisableExecuteEnable(); } +}; void* AllocateMemoryPages(size_t size); void FreeMemoryPages(void* ptr, size_t size); void* AllocateAlignedMemory(size_t size, size_t alignment); diff --git a/Source/Core/Core/PowerPC/JitArm64/Jit.cpp b/Source/Core/Core/PowerPC/JitArm64/Jit.cpp index 0a7caf1e3e..b606b071ea 100644 --- a/Source/Core/Core/PowerPC/JitArm64/Jit.cpp +++ b/Source/Core/Core/PowerPC/JitArm64/Jit.cpp @@ -127,13 +127,12 @@ void JitArm64::ClearCache() m_handler_to_loc.clear(); blocks.Clear(); - Common::JITPageWriteEnableExecuteDisable(); + const Common::ScopedJITPageWriteAndNoExecute enable_jit_page_writes; ClearCodeSpace(); farcode.ClearCodeSpace(); UpdateMemoryOptions(); GenerateAsm(); - Common::JITPageWriteDisableExecuteEnable(); } void JitArm64::Shutdown() @@ -601,7 +600,7 @@ void JitArm64::Jit(u32) { ClearCache(); } - Common::JITPageWriteEnableExecuteDisable(); + const Common::ScopedJITPageWriteAndNoExecute enable_jit_page_writes; std::size_t block_size = m_code_buffer.size(); const u32 em_address = PowerPC::ppcState.pc; @@ -623,7 +622,6 @@ void JitArm64::Jit(u32) NPC = nextPC; PowerPC::ppcState.Exceptions |= EXCEPTION_ISI; PowerPC::CheckExceptions(); - Common::JITPageWriteDisableExecuteEnable(); WARN_LOG_FMT(POWERPC, "ISI exception at {:#010x}", nextPC); return; } @@ -631,7 +629,6 @@ void JitArm64::Jit(u32) JitBlock* b = blocks.AllocateBlock(em_address); DoJit(em_address, b, nextPC); blocks.FinalizeBlock(*b, jo.enableBlocklink, code_block.m_physical_addresses); - Common::JITPageWriteDisableExecuteEnable(); } void JitArm64::DoJit(u32 em_address, JitBlock* b, u32 nextPC) diff --git a/Source/Core/Core/PowerPC/JitArm64/JitArm64Cache.cpp b/Source/Core/Core/PowerPC/JitArm64/JitArm64Cache.cpp index b02ba0eb58..73dc1af127 100644 --- a/Source/Core/Core/PowerPC/JitArm64/JitArm64Cache.cpp +++ b/Source/Core/Core/PowerPC/JitArm64/JitArm64Cache.cpp @@ -59,12 +59,11 @@ void JitArm64BlockCache::WriteLinkBlock(Arm64Gen::ARM64XEmitter& emit, void JitArm64BlockCache::WriteLinkBlock(const JitBlock::LinkData& source, const JitBlock* dest) { - Common::JITPageWriteEnableExecuteDisable(); + const Common::ScopedJITPageWriteAndNoExecute enable_jit_page_writes; u8* location = source.exitPtrs; ARM64XEmitter emit(location); WriteLinkBlock(emit, source, dest); - Common::JITPageWriteDisableExecuteEnable(); emit.FlushIcache(); } @@ -72,9 +71,8 @@ void JitArm64BlockCache::WriteDestroyBlock(const JitBlock& block) { // Only clear the entry points as we might still be within this block. ARM64XEmitter emit(block.checkedEntry); - Common::JITPageWriteEnableExecuteDisable(); + const Common::ScopedJITPageWriteAndNoExecute enable_jit_page_writes; while (emit.GetWritableCodePtr() <= block.normalEntry) emit.BRK(0x123); - Common::JITPageWriteDisableExecuteEnable(); emit.FlushIcache(); } diff --git a/Source/Core/Core/PowerPC/JitArm64/JitArm64_BackPatch.cpp b/Source/Core/Core/PowerPC/JitArm64/JitArm64_BackPatch.cpp index a82d52b4b3..23236a524e 100644 --- a/Source/Core/Core/PowerPC/JitArm64/JitArm64_BackPatch.cpp +++ b/Source/Core/Core/PowerPC/JitArm64/JitArm64_BackPatch.cpp @@ -289,7 +289,7 @@ bool JitArm64::HandleFastmemFault(uintptr_t access_address, SContext* ctx) if ((const u8*)ctx->CTX_PC - fault_location > fastmem_area_length) return false; - Common::JITPageWriteEnableExecuteDisable(); + const Common::ScopedJITPageWriteAndNoExecute enable_jit_page_writes; ARM64XEmitter emitter((u8*)fault_location); emitter.BL(slow_handler_iter->second.slowmem_code); @@ -301,7 +301,6 @@ bool JitArm64::HandleFastmemFault(uintptr_t access_address, SContext* ctx) m_fault_to_handler.erase(slow_handler_iter); emitter.FlushIcache(); - Common::JITPageWriteDisableExecuteEnable(); ctx->CTX_PC = reinterpret_cast(fault_location); return true; diff --git a/Source/Core/Core/PowerPC/JitArm64/JitAsm.cpp b/Source/Core/Core/PowerPC/JitArm64/JitAsm.cpp index 8d28cdcb4f..d61280fe8b 100644 --- a/Source/Core/Core/PowerPC/JitArm64/JitAsm.cpp +++ b/Source/Core/Core/PowerPC/JitArm64/JitAsm.cpp @@ -25,7 +25,7 @@ using namespace Arm64Gen; void JitArm64::GenerateAsm() { - Common::JITPageWriteEnableExecuteDisable(); + const Common::ScopedJITPageWriteAndNoExecute enable_jit_page_writes; // This value is all of the callee saved registers that we are required to save. // According to the AACPS64 we need to save R19 ~ R30 and Q8 ~ Q15. @@ -199,7 +199,6 @@ void JitArm64::GenerateAsm() GenerateCommonAsm(); FlushIcache(); - Common::JITPageWriteDisableExecuteEnable(); } void JitArm64::GenerateCommonAsm() diff --git a/Source/Core/VideoCommon/VertexLoaderARM64.cpp b/Source/Core/VideoCommon/VertexLoaderARM64.cpp index bb4245523c..afd2f0f21d 100644 --- a/Source/Core/VideoCommon/VertexLoaderARM64.cpp +++ b/Source/Core/VideoCommon/VertexLoaderARM64.cpp @@ -54,10 +54,9 @@ VertexLoaderARM64::VertexLoaderARM64(const TVtxDesc& vtx_desc, const VAT& vtx_at : VertexLoaderBase(vtx_desc, vtx_att), m_float_emit(this) { AllocCodeSpace(4096); - Common::JITPageWriteEnableExecuteDisable(); + const Common::ScopedJITPageWriteAndNoExecute enable_jit_page_writes; ClearCodeSpace(); GenerateVertexLoader(); - Common::JITPageWriteDisableExecuteEnable(); WriteProtect(); }