PPU: Cleanup MacOs executable write protection guards

Utilities/JIT.h

@@ -434,13 +434,35 @@ namespace asmjit
 #endif
 }
 
+#ifdef __APPLE__
+struct jit_write_guard
+{
+	jit_write_guard() noexcept
+	{
+		pthread_jit_write_protect_np(false);
+
+		// Ensure stores are not reordered by the compiler
+		atomic_fence_acq_rel();
+	}
+
+	~jit_write_guard() noexcept
+	{
+		// Ensure stores are not reordered by the compiler
+		atomic_fence_seq_cst();
+
+		pthread_jit_write_protect_np(true);
+	}
+};
+#else
+#define jit_write_guard [[maybe_unused]] int
+#endif
+
 // Build runtime function with asmjit::X86Assembler
 template <typename FT, typename Asm = native_asm, typename F>
 inline FT build_function_asm(std::string_view name, F&& builder, ::jit_runtime* custom_runtime = nullptr, bool reduced_size = false)
 {
-#ifdef __APPLE__
+	jit_write_guard jit_guard;
-	pthread_jit_write_protect_np(false);
+
-#endif
 	using namespace asmjit;
 
 	auto& rt = custom_runtime ? *custom_runtime : get_global_runtime();

rpcs3/Emu/Cell/PPUThread.cpp

@@ -2175,18 +2175,12 @@ void ppu_thread::cpu_task()
 		}
 		case ppu_cmd::lle_call:
 		{
-#ifdef __APPLE__
-			pthread_jit_write_protect_np(true);
-#endif
 			const vm::ptr<u32> opd(arg < 32 ? vm::cast(gpr[arg]) : vm::cast(arg));
 			cmd_pop(), fast_call(opd[0], opd[1]);
 			break;
 		}
 		case ppu_cmd::entry_call:
 		{
-#ifdef __APPLE__
-			pthread_jit_write_protect_np(true);
-#endif
 			cmd_pop(), fast_call(entry_func.addr, entry_func.rtoc, true);
 			break;
 		}
@@ -2197,9 +2191,6 @@ void ppu_thread::cpu_task()
 		}
 		case ppu_cmd::opd_call:
 		{
-#ifdef __APPLE__
-			pthread_jit_write_protect_np(true);
-#endif
 			const ppu_func_opd_t opd = cmd_get(1).as<ppu_func_opd_t>();
 			cmd_pop(1), fast_call(opd.addr, opd.rtoc);
 			break;
@@ -2218,9 +2209,6 @@ void ppu_thread::cpu_task()
 		}
 		case ppu_cmd::initialize:
 		{
-#ifdef __APPLE__
-			pthread_jit_write_protect_np(false);
-#endif
 			cmd_pop();
 
 			ppu_initialize();
@@ -2232,9 +2220,6 @@ void ppu_thread::cpu_task()
 
 			spu_cache::initialize();
 
-#ifdef __APPLE__
-			pthread_jit_write_protect_np(true);
-#endif
 #ifdef ARCH_ARM64
 			// Flush all cache lines after potentially writing executable code
 			asm("ISB");
@@ -2381,6 +2366,11 @@ void ppu_thread::cpu_wait(bs_t<cpu_flag> old)
 
 void ppu_thread::exec_task()
 {
+#ifdef __APPLE__
+	// Ensure correct state before executing JIT code
+	pthread_jit_write_protect_np(true);
+#endif
+
 	if (g_cfg.core.ppu_decoder != ppu_decoder_type::_static)
 	{
 		// HVContext push to allow recursion. This happens with guest callback invocations.
@@ -2462,9 +2452,6 @@ ppu_thread::ppu_thread(const ppu_thread_params& param, std::string_view name, u3
 	syscall_history.data.resize(g_cfg.core.ppu_call_history ? syscall_history_max_size : 1);
 	syscall_history.count_debug_arguments = static_cast<u32>(g_cfg.core.ppu_call_history ? std::size(syscall_history.data[0].args) : 0);
 
-#ifdef __APPLE__
-	pthread_jit_write_protect_np(true);
-#endif
 #ifdef ARCH_ARM64
 	// Flush all cache lines after potentially writing executable code
 	asm("ISB");
@@ -4008,9 +3995,8 @@ extern void ppu_precompile(std::vector<std::string>& dir_queue, std::vector<ppu_
 
 	named_thread_group workers("SPRX Worker ", std::min<u32>(software_thread_limit, cpu_thread_limit), [&]
 	{
-#ifdef __APPLE__
+		jit_write_guard jit_guard;
-		pthread_jit_write_protect_np(false);
+
-#endif
 		// Set low priority
 		thread_ctrl::scoped_priority low_prio(-1);
 		u32 inc_fdone = 1;
@@ -4236,9 +4222,6 @@ extern void ppu_precompile(std::vector<std::string>& dir_queue, std::vector<ppu_
 			return;
 		}
 
-#ifdef __APPLE__
-		pthread_jit_write_protect_np(false);
-#endif
 		// Set low priority
 		thread_ctrl::scoped_priority low_prio(-1);
 
@@ -4617,6 +4600,8 @@ bool ppu_initialize(const ppu_module<lv2_obj>& info, bool check_only, u64 file_s
 		progress_dialog.emplace(get_localized_string(localized_string_id::PROGRESS_DIALOG_LOADING_PPU_MODULES));
 	}
 
+	jit_write_guard jit_guard;
+
 	// Permanently loaded compiled PPU modules (name -> data)
 	jit_module& jit_mod = g_fxo->get<jit_module_manager>().get(cache_path + "_" + std::to_string(std::bit_cast<usz>(info.segs[0].ptr)));
 
@@ -4797,9 +4782,6 @@ bool ppu_initialize(const ppu_module<lv2_obj>& info, bool check_only, u64 file_s
 		// Try to make the code fit in 16 bytes, may fail and fallback
 		if (*full_sample && abs_diff(*full_sample, reinterpret_cast<u64>(jit_runtime::peek(true) + 3 * 4)) < (128u << 20))
 		{
-#ifdef __APPLE__
-			pthread_jit_write_protect_np(false);
-#endif
 			u8* code = jit_runtime::alloc(12, 4, true);
 			code_ptr = reinterpret_cast<u64>(code);
 
@@ -4914,6 +4896,11 @@ bool ppu_initialize(const ppu_module<lv2_obj>& info, bool check_only, u64 file_s
 #endif
 		}, runtime.get(), true);
 
+#ifdef __APPLE__
+		// Restore write-protection state (modified by build_function_asm)
+		pthread_jit_write_protect_np(false);
+#endif
+
 		// Full sample may exist already, but is very far away
 		// So in this case, a new sample is written
 		ensure(code_size_until_jump != umax);
@@ -5307,9 +5294,8 @@ bool ppu_initialize(const ppu_module<lv2_obj>& info, bool check_only, u64 file_s
 				// Set low priority
 				thread_ctrl::scoped_priority low_prio(-1);
 
-	#ifdef __APPLE__
+				jit_write_guard jit_guard;
-				pthread_jit_write_protect_np(false);
+
-	#endif
 				for (u32 i = (*work_cv)++; i < workload.size(); i = (*work_cv)++, (*work_done)++, g_progr_pdone++)
 				{
 					if (cpu ? cpu->state.all_of(cpu_flag::exit) : Emu.IsStopped())
@@ -5332,10 +5318,6 @@ bool ppu_initialize(const ppu_module<lv2_obj>& info, bool check_only, u64 file_s
 				}
 
 				core_lock.unlock();
-
-	#ifdef __APPLE__
-				pthread_jit_write_protect_np(true);
-	#endif
 			}
 		};
 
@@ -5487,10 +5469,6 @@ bool ppu_initialize(const ppu_module<lv2_obj>& info, bool check_only, u64 file_s
 		}
 	}
 
-#ifdef __APPLE__
-	// Symbol resolver is in JIT mem, so we must enable execution
-	pthread_jit_write_protect_np(true);
-#endif
 	{
 		usz index = umax;
 
@@ -5503,11 +5481,6 @@ bool ppu_initialize(const ppu_module<lv2_obj>& info, bool check_only, u64 file_s
 		}
 	}
 
-#ifdef __APPLE__
-	// Symbol resolver is in JIT mem, so we must enable execution
-	pthread_jit_write_protect_np(false);
-#endif
-
 	// Find a BLR-only function in order to copy it to all BLRs (some games need it)
 	for (const auto& func : info.get_funcs())
 	{

rpcs3/Emu/Cell/SPUCommonRecompiler.cpp

@@ -207,9 +207,7 @@ static void ghc_cpp_trampoline(u64 fn_target, native_asm& c, auto& args)
 
 DECLARE(spu_runtime::tr_dispatch) = []
 {
-#ifdef __APPLE__
+	jit_write_guard jit_guard;
-	pthread_jit_write_protect_np(false);
-#endif
 #if defined(ARCH_X64)
 	// Generate a special trampoline to spu_recompiler_base::dispatch with pause instruction
 	u8* const trptr = jit_runtime::alloc(32, 16);
@@ -237,6 +235,7 @@ DECLARE(spu_runtime::tr_dispatch) = []
 
 DECLARE(spu_runtime::tr_branch) = []
 {
+	jit_write_guard jit_guard;
 #if defined(ARCH_X64)
 	// Generate a trampoline to spu_recompiler_base::branch
 	u8* const trptr = jit_runtime::alloc(32, 16);
@@ -262,6 +261,7 @@ DECLARE(spu_runtime::tr_branch) = []
 
 DECLARE(spu_runtime::tr_interpreter) = []
 {
+	jit_write_guard jit_guard;
 #if defined(ARCH_X64)
 	u8* const trptr = jit_runtime::alloc(32, 16);
 	u8* raw = move_args_ghc_to_native(trptr);
@@ -283,6 +283,8 @@ DECLARE(spu_runtime::tr_interpreter) = []
 
 DECLARE(spu_runtime::g_dispatcher) = []
 {
+	jit_write_guard jit_guard;
+
 	// Allocate 2^20 positions in data area
 	const auto ptr = reinterpret_cast<std::remove_const_t<decltype(spu_runtime::g_dispatcher)>>(jit_runtime::alloc(sizeof(*g_dispatcher), 64, false));
 
@@ -296,6 +298,8 @@ DECLARE(spu_runtime::g_dispatcher) = []
 
 DECLARE(spu_runtime::tr_all) = []
 {
+	jit_write_guard jit_guard;
+
 #if defined(ARCH_X64)
 	u8* const trptr = jit_runtime::alloc(32, 16);
 	u8* raw = trptr;
@@ -834,6 +838,8 @@ void spu_cache::add(const spu_program& func)
 
 void spu_cache::initialize(bool build_existing_cache)
 {
+	jit_write_guard jit_guard;
+
 	spu_runtime::g_interpreter = spu_runtime::g_gateway;
 
 	if (g_cfg.core.spu_decoder == spu_decoder_type::_static || g_cfg.core.spu_decoder == spu_decoder_type::dynamic)
@@ -964,15 +970,7 @@ void spu_cache::initialize(bool build_existing_cache)
 		// every exit path (return, exception, etc.). Leaving a worker
 		// in write mode at teardown can leave per-thread state
 		// inconsistent on AArch64.
-		pthread_jit_write_protect_np(false);
+		jit_write_guard jit_guard;
-
-		struct jit_write_guard
-		{
-			~jit_write_guard()
-			{
-				pthread_jit_write_protect_np(true);
-			}
-		} _jit_guard;
 #endif
 		// Set low priority
 		thread_ctrl::scoped_priority low_prio(-1);
@@ -2207,6 +2205,9 @@ void spu_recompiler_base::dispatch(spu_thread& spu, void*, u8* rip)
 		return;
 	}
 
+#if defined(__APPLE__)
+	pthread_jit_write_protect_np(false);
+#endif
 	auto program = spu.jit->analyse(spu._ptr<u32>(0), spu.pc);
 #ifdef ARCH_ARM64
 	const auto func = compile_spu_llvm_with_retry(spu.jit, program);

rpcs3/Emu/Cell/lv2/lv2.cpp

@@ -1252,16 +1252,8 @@ extern void ppu_execute_syscall(ppu_thread& ppu, u64 code)
 
 		if (const auto func = g_ppu_syscall_table[code].first)
 		{
-#ifdef __APPLE__
-			pthread_jit_write_protect_np(false);
-#endif
 			func(ppu, {}, vm::_ptr<u32>(ppu.cia), nullptr);
 			ppu_log.trace("Syscall '%s' (%llu) finished, r3=0x%llx", ppu_syscall_code(code), code, ppu.gpr[3]);
-
-#ifdef __APPLE__
-			pthread_jit_write_protect_np(true);
-			// No need to flush cache lines after a syscall, since we didn't generate any code.
-#endif
 			return;
 		}
 	}

rpcs3/Emu/System.cpp

@@ -1893,7 +1893,7 @@ game_boot_result Emulator::Load(const std::string& title_id, bool is_disc_patch,
 
 				struct jit_write_guard
 				{
-					~jit_write_guard()
+					~jit_write_guard() noexcept
 					{
 						pthread_jit_write_protect_np(true);
 					}