From 6ea3c6a7e3fe2388d112071d0773a6d4b1220137 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Thu, 26 Jan 2017 08:51:48 +0100 Subject: [PATCH 1/5] Delete unused code --- Common/MemArenaAndroid.cpp | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/Common/MemArenaAndroid.cpp b/Common/MemArenaAndroid.cpp index dbcde0584c..a320d1d243 100644 --- a/Common/MemArenaAndroid.cpp +++ b/Common/MemArenaAndroid.cpp @@ -67,21 +67,6 @@ error: return ret; } -static int ashmem_set_prot_region(int fd, int prot) { - return ioctl(fd, ASHMEM_SET_PROT_MASK, prot); -} - -static int ashmem_pin_region(int fd, size_t offset, size_t len) { - // Even on 64-bit, it seems these arguments are 32-bit and thus need a cast to avoid warnings. - struct ashmem_pin pin = { (uint32_t)offset, (uint32_t)len }; - return ioctl(fd, ASHMEM_PIN, &pin); -} - -static int ashmem_unpin_region(int fd, size_t offset, size_t len) { - struct ashmem_pin pin = { (uint32_t)offset, (uint32_t)len }; - return ioctl(fd, ASHMEM_UNPIN, &pin); -} - // Windows mappings need to be on 64K boundaries, due to Alpha legacy. size_t MemArena::roundup(size_t x) { return x; From 69b35a35becbd74fa9b2e9a0a410c5d4f7f0f771 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Thu, 26 Jan 2017 09:20:05 +0100 Subject: [PATCH 2/5] Show page size and WX Exclusive status in system info --- UI/DevScreens.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/UI/DevScreens.cpp b/UI/DevScreens.cpp index 75a2878873..97af7a724b 100644 --- a/UI/DevScreens.cpp +++ b/UI/DevScreens.cpp @@ -363,6 +363,9 @@ void SystemInfoScreen::CreateViews() { deviceSpecs->Add(new InfoItem("D3DX Version", StringFromFormat("%d", GetD3DXVersion()))); } #endif + deviceSpecs->Add(new ItemHeader("OS Information")); + deviceSpecs->Add(new InfoItem("Memory Page Size", StringFromFormat("%d bytes", GetMemoryProtectPageSize()))); + deviceSpecs->Add(new InfoItem("RW/RX exclusive: ", PlatformIsWXExclusive() ? "Yes" : "No")); #ifdef __ANDROID__ deviceSpecs->Add(new ItemHeader("Audio Information")); From 1503f00885895b8e22fbe81f6a1308603f81c450 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Thu, 26 Jan 2017 09:49:26 +0100 Subject: [PATCH 3/5] Memory function cleanups. Better simulation of RW exclusive mode on Windows (if desired) --- Common/CodeBlock.h | 8 +++----- Common/MemoryUtil.cpp | 23 +++++++++++++++-------- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/Common/CodeBlock.h b/Common/CodeBlock.h index f8e5b6be67..41a02e70d2 100644 --- a/Common/CodeBlock.h +++ b/Common/CodeBlock.h @@ -51,12 +51,9 @@ public: // Call this before you generate any code. void AllocCodeSpace(int size) { region_size = size; + // The protection will be set to RW if PlatformIsWXExclusive. region = (u8*)AllocateExecutableMemory(region_size); T::SetCodePointer(region); - // On W^X platforms, we start with writable but not executable pages. - if (PlatformIsWXExclusive()) { - ProtectMemoryPages(region, region_size, MEM_PROT_READ | MEM_PROT_WRITE); - } } // Always clear code space with breakpoints, so that if someone accidentally executes @@ -71,7 +68,8 @@ public: ResetCodePtr(); } - // BeginWrite/EndWrite assume that we keep appending. If you don't specify a size and we encounter later executable block, we're screwed. + // BeginWrite/EndWrite assume that we keep appending. + // If you don't specify a size and we later encounter an executable non-writable block, we're screwed. // These CANNOT be nested. We rely on the memory protection starting at READ|WRITE after start and reset. void BeginWrite(size_t sizeEstimate = 1) { #ifdef _DEBUG diff --git a/Common/MemoryUtil.cpp b/Common/MemoryUtil.cpp index 8e6c76cef2..b127a26b2b 100644 --- a/Common/MemoryUtil.cpp +++ b/Common/MemoryUtil.cpp @@ -50,7 +50,7 @@ static SYSTEM_INFO sys_info; #ifdef _WIN32 // Win32 flags are odd... -uint32_t ConvertProtFlagsWin32(uint32_t flags) { +static uint32_t ConvertProtFlagsWin32(uint32_t flags) { uint32_t protect = 0; switch (flags) { case 0: protect = PAGE_NOACCESS; break; @@ -67,7 +67,7 @@ uint32_t ConvertProtFlagsWin32(uint32_t flags) { #else -uint32_t ConvertProtFlagsUnix(uint32_t flags) { +static uint32_t ConvertProtFlagsUnix(uint32_t flags) { uint32_t protect = 0; if (flags & MEM_PROT_READ) protect |= PROT_READ; @@ -117,6 +117,9 @@ static void *SearchForFreeMem(size_t size) { void *AllocateExecutableMemory(size_t size) { #if defined(_WIN32) void *ptr; + DWORD prot = PAGE_EXECUTE_READWRITE; + if (PlatformIsWXExclusive()) + prot = PAGE_READWRITE; #if defined(_M_X64) if ((uintptr_t)&hint_location > 0xFFFFFFFFULL) { if (sys_info.dwPageSize == 0) @@ -131,14 +134,16 @@ void *AllocateExecutableMemory(size_t size) { ptr = SearchForFreeMem(aligned_size); } if (ptr) { - ptr = VirtualAlloc(ptr, aligned_size, MEM_RESERVE | MEM_COMMIT, PAGE_EXECUTE_READWRITE); + ptr = VirtualAlloc(ptr, aligned_size, MEM_RESERVE | MEM_COMMIT, prot); } else { ERROR_LOG(COMMON, "Unable to find nearby executable memory for jit"); } } else #endif - ptr = VirtualAlloc(0, size, MEM_RESERVE | MEM_COMMIT, PAGE_EXECUTE_READWRITE); + { + ptr = VirtualAlloc(0, size, MEM_RESERVE | MEM_COMMIT, prot); + } #else static char *map_hint = 0; #if defined(_M_X64) @@ -192,7 +197,6 @@ void *AllocateExecutableMemory(size_t size) { } } #endif - return ptr; } @@ -205,7 +209,7 @@ void *AllocateMemoryPages(size_t size, uint32_t memProtFlags) { PanicAlert("Failed to allocate raw memory"); #else uint32_t protect = ConvertProtFlagsUnix(memProtFlags); - void* ptr = mmap(0, size, protect, MAP_ANON | MAP_PRIVATE, -1, 0); + void *ptr = mmap(0, size, protect, MAP_ANON | MAP_PRIVATE, -1, 0); if (ptr == MAP_FAILED) { ERROR_LOG(MEMMAP, "Failed to allocate memory pages: errno=%d", errno); return nullptr; @@ -274,11 +278,14 @@ bool PlatformIsWXExclusive() { } bool ProtectMemoryPages(const void* ptr, size_t size, uint32_t memProtFlags) { - VERBOSE_LOG(JIT, "ProtectMemoryPages: %p (%d) : r%d w%d x%d", ptr, (int)size, (memProtFlags & MEM_PROT_READ) != 0, (memProtFlags & MEM_PROT_WRITE) != 0, (memProtFlags & MEM_PROT_EXEC) != 0); + VERBOSE_LOG(JIT, "ProtectMemoryPages: %p (%d) : r%d w%d x%d", ptr, (int)size, + (memProtFlags & MEM_PROT_READ) != 0, (memProtFlags & MEM_PROT_WRITE) != 0, (memProtFlags & MEM_PROT_EXEC) != 0); if (PlatformIsWXExclusive()) { - if ((memProtFlags & (MEM_PROT_WRITE | MEM_PROT_EXEC)) == (MEM_PROT_WRITE | MEM_PROT_EXEC)) + if ((memProtFlags & (MEM_PROT_WRITE | MEM_PROT_EXEC)) == (MEM_PROT_WRITE | MEM_PROT_EXEC)) { + ERROR_LOG(MEMMAP, "Bad memory protection %d!", memProtFlags); PanicAlert("Bad memory protect : W^X is in effect, can't both write and exec"); + } } // Note - VirtualProtect will affect the full pages containing the requested range. // mprotect does not seem to, at least not on Android unless I made a mistake somewhere, so we manually round. From 635b2ada4318585023b379964cfa10f1aa86b1c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Thu, 26 Jan 2017 09:50:16 +0100 Subject: [PATCH 4/5] Remove a function that didn't make a lot of sense. --- Core/Debugger/Breakpoints.cpp | 2 +- Core/MIPS/ARM/ArmJit.cpp | 5 ----- Core/MIPS/ARM/ArmJit.h | 1 - Core/MIPS/ARM64/Arm64Jit.cpp | 14 +++++--------- Core/MIPS/ARM64/Arm64Jit.h | 1 - Core/MIPS/IR/IRJit.cpp | 4 ---- Core/MIPS/IR/IRJit.h | 1 - Core/MIPS/JitCommon/JitBlockCache.cpp | 2 +- Core/MIPS/JitCommon/JitCommon.h | 1 - Core/MIPS/MIPS/MipsJit.cpp | 5 ----- Core/MIPS/MIPS/MipsJit.h | 1 - Core/MIPS/fake/FakeJit.cpp | 5 ----- Core/MIPS/fake/FakeJit.h | 1 - Core/MIPS/x86/Jit.cpp | 5 ----- Core/MIPS/x86/Jit.h | 1 - 15 files changed, 7 insertions(+), 42 deletions(-) diff --git a/Core/Debugger/Breakpoints.cpp b/Core/Debugger/Breakpoints.cpp index a850df55db..9f7a27d39f 100644 --- a/Core/Debugger/Breakpoints.cpp +++ b/Core/Debugger/Breakpoints.cpp @@ -540,7 +540,7 @@ void CBreakPoints::Update(u32 addr) if (addr != 0) MIPSComp::jit->InvalidateCacheAt(addr - 4, 8); else - MIPSComp::jit->InvalidateCache(); + MIPSComp::jit->ClearCache(); if (resume) Core_EnableStepping(false); diff --git a/Core/MIPS/ARM/ArmJit.cpp b/Core/MIPS/ARM/ArmJit.cpp index e89aa3bf95..33f17bfca7 100644 --- a/Core/MIPS/ARM/ArmJit.cpp +++ b/Core/MIPS/ARM/ArmJit.cpp @@ -154,11 +154,6 @@ void ArmJit::ClearCache() GenerateFixedCode(); } -void ArmJit::InvalidateCache() -{ - blocks.Clear(); -} - void ArmJit::InvalidateCacheAt(u32 em_address, int length) { blocks.InvalidateICache(em_address, length); diff --git a/Core/MIPS/ARM/ArmJit.h b/Core/MIPS/ARM/ArmJit.h index 3f55e87a66..511ce5fce1 100644 --- a/Core/MIPS/ARM/ArmJit.h +++ b/Core/MIPS/ARM/ArmJit.h @@ -174,7 +174,6 @@ public: void RestoreSavedEmuHackOps(std::vector saved) override { blocks.RestoreSavedEmuHackOps(saved); } void ClearCache() override; - void InvalidateCache() override; void InvalidateCacheAt(u32 em_address, int length = 4) override; void EatPrefix() override { js.EatPrefix(); } diff --git a/Core/MIPS/ARM64/Arm64Jit.cpp b/Core/MIPS/ARM64/Arm64Jit.cpp index 4184ca83d3..4f9e59252a 100644 --- a/Core/MIPS/ARM64/Arm64Jit.cpp +++ b/Core/MIPS/ARM64/Arm64Jit.cpp @@ -146,10 +146,6 @@ void Arm64Jit::ClearCache() { GenerateFixedCode(jo); } -void Arm64Jit::InvalidateCache() { - blocks.Clear(); -} - void Arm64Jit::InvalidateCacheAt(u32 em_address, int length) { blocks.InvalidateICache(em_address, length); } @@ -194,7 +190,7 @@ void Arm64Jit::Compile(u32 em_address) { ClearCache(); } - BeginWrite(); + BeginWrite(4); int block_num = blocks.AllocateBlock(em_address); JitBlock *b = blocks.GetBlock(block_num); @@ -203,6 +199,9 @@ void Arm64Jit::Compile(u32 em_address) { EndWrite(); + // Don't forget to zap the newly written instructions in the instruction cache! + FlushIcache(); + bool cleanSlate = false; if (js.hasSetRounding && !js.lastSetRounding) { @@ -336,9 +335,6 @@ const u8 *Arm64Jit::DoJit(u32 em_address, JitBlock *b) { if (dontLogBlocks > 0) dontLogBlocks--; - // Don't forget to zap the newly written instructions in the instruction cache! - FlushIcache(); - if (js.lastContinuedPC == 0) { b->originalSize = js.numInstructions; } else { @@ -406,7 +402,7 @@ void Arm64Jit::LinkBlock(u8 *exitPoint, const u8 *checkedEntry) { } ARM64XEmitter emit(exitPoint); emit.B(checkedEntry); - // TODO: Write stuff after. + // TODO: Write stuff after, convering up the now-unused instructions. emit.FlushIcache(); if (PlatformIsWXExclusive()) { ProtectMemoryPages(exitPoint, 32, MEM_PROT_READ | MEM_PROT_EXEC); diff --git a/Core/MIPS/ARM64/Arm64Jit.h b/Core/MIPS/ARM64/Arm64Jit.h index b4491e5384..2eed09ee0c 100644 --- a/Core/MIPS/ARM64/Arm64Jit.h +++ b/Core/MIPS/ARM64/Arm64Jit.h @@ -175,7 +175,6 @@ public: void RestoreSavedEmuHackOps(std::vector saved) override { blocks.RestoreSavedEmuHackOps(saved); } void ClearCache() override; - void InvalidateCache() override; void InvalidateCacheAt(u32 em_address, int length = 4) override; void EatPrefix() override { js.EatPrefix(); } diff --git a/Core/MIPS/IR/IRJit.cpp b/Core/MIPS/IR/IRJit.cpp index 7832456ab5..26f66bc6ad 100644 --- a/Core/MIPS/IR/IRJit.cpp +++ b/Core/MIPS/IR/IRJit.cpp @@ -69,10 +69,6 @@ void IRJit::ClearCache() { blocks_.Clear(); } -void IRJit::InvalidateCache() { - blocks_.Clear(); -} - void IRJit::InvalidateCacheAt(u32 em_address, int length) { blocks_.InvalidateICache(em_address, length); } diff --git a/Core/MIPS/IR/IRJit.h b/Core/MIPS/IR/IRJit.h index 0d085b86c3..84851df7f3 100644 --- a/Core/MIPS/IR/IRJit.h +++ b/Core/MIPS/IR/IRJit.h @@ -144,7 +144,6 @@ public: void RestoreSavedEmuHackOps(std::vector saved) override { blocks_.RestoreSavedEmuHackOps(saved); } void ClearCache() override; - void InvalidateCache() override; void InvalidateCacheAt(u32 em_address, int length = 4) override; const u8 *GetDispatcher() const override { return nullptr; } diff --git a/Core/MIPS/JitCommon/JitBlockCache.cpp b/Core/MIPS/JitCommon/JitBlockCache.cpp index 196140dcd0..ad6466e8d1 100644 --- a/Core/MIPS/JitCommon/JitBlockCache.cpp +++ b/Core/MIPS/JitCommon/JitBlockCache.cpp @@ -60,7 +60,7 @@ op_agent_t agent; const u32 INVALID_EXIT = 0xFFFFFFFF; JitBlockCache::JitBlockCache(MIPSState *mips, CodeBlockCommon *codeBlock) : - codeBlock_(codeBlock), blocks_(0), num_blocks_(0) { + codeBlock_(codeBlock), blocks_(nullptr), num_blocks_(0) { } JitBlockCache::~JitBlockCache() { diff --git a/Core/MIPS/JitCommon/JitCommon.h b/Core/MIPS/JitCommon/JitCommon.h index 004c7990f0..9e91664713 100644 --- a/Core/MIPS/JitCommon/JitCommon.h +++ b/Core/MIPS/JitCommon/JitCommon.h @@ -123,7 +123,6 @@ namespace MIPSComp { virtual bool DescribeCodePtr(const u8 *ptr, std::string &name) = 0; virtual const u8 *GetDispatcher() const = 0; virtual JitBlockCache *GetBlockCache() = 0; - virtual void InvalidateCache() = 0; virtual void InvalidateCacheAt(u32 em_address, int length = 4) = 0; virtual void DoState(PointerWrap &p) = 0; virtual void DoDummyState(PointerWrap &p) = 0; diff --git a/Core/MIPS/MIPS/MipsJit.cpp b/Core/MIPS/MIPS/MipsJit.cpp index 88b8594094..b669109899 100644 --- a/Core/MIPS/MIPS/MipsJit.cpp +++ b/Core/MIPS/MIPS/MipsJit.cpp @@ -99,11 +99,6 @@ void MipsJit::ClearCache() //GenerateFixedCode(); } -void MipsJit::InvalidateCache() -{ - blocks.Clear(); -} - void MipsJit::InvalidateCacheAt(u32 em_address, int length) { blocks.InvalidateICache(em_address, length); diff --git a/Core/MIPS/MIPS/MipsJit.h b/Core/MIPS/MIPS/MipsJit.h index 3afea13852..ad158f5de0 100644 --- a/Core/MIPS/MIPS/MipsJit.h +++ b/Core/MIPS/MIPS/MipsJit.h @@ -135,7 +135,6 @@ public: void RestoreSavedEmuHackOps(std::vector saved) override { blocks.RestoreSavedEmuHackOps(saved); } void ClearCache() override; - void InvalidateCache() override; void InvalidateCacheAt(u32 em_address, int length = 4) override; void EatPrefix() override { js.EatPrefix(); } diff --git a/Core/MIPS/fake/FakeJit.cpp b/Core/MIPS/fake/FakeJit.cpp index beb0d37188..1a521ee68b 100644 --- a/Core/MIPS/fake/FakeJit.cpp +++ b/Core/MIPS/fake/FakeJit.cpp @@ -94,11 +94,6 @@ void FakeJit::ClearCache() //GenerateFixedCode(); } -void FakeJit::InvalidateCache() -{ - blocks.Clear(); -} - void FakeJit::InvalidateCacheAt(u32 em_address, int length) { blocks.InvalidateICache(em_address, length); diff --git a/Core/MIPS/fake/FakeJit.h b/Core/MIPS/fake/FakeJit.h index cfb09a8a56..d23303a654 100644 --- a/Core/MIPS/fake/FakeJit.h +++ b/Core/MIPS/fake/FakeJit.h @@ -128,7 +128,6 @@ public: JitBlockCache *GetBlockCache() { return &blocks; } void ClearCache(); - void InvalidateCache(); void InvalidateCacheAt(u32 em_address, int length = 4); void EatPrefix() { js.EatPrefix(); } diff --git a/Core/MIPS/x86/Jit.cpp b/Core/MIPS/x86/Jit.cpp index 9e087bfd8b..2c73633cc3 100644 --- a/Core/MIPS/x86/Jit.cpp +++ b/Core/MIPS/x86/Jit.cpp @@ -245,11 +245,6 @@ void Jit::ClearCache() GenerateFixedCode(jo); } -void Jit::InvalidateCache() -{ - blocks.Clear(); -} - void Jit::CompileDelaySlot(int flags, RegCacheState *state) { // Need to offset the downcount which was already incremented for the branch + delay slot. diff --git a/Core/MIPS/x86/Jit.h b/Core/MIPS/x86/Jit.h index 51e8791d9b..72ca6d87b5 100644 --- a/Core/MIPS/x86/Jit.h +++ b/Core/MIPS/x86/Jit.h @@ -163,7 +163,6 @@ public: void RestoreSavedEmuHackOps(std::vector saved) override { blocks.RestoreSavedEmuHackOps(saved); } void ClearCache() override; - void InvalidateCache() override; void InvalidateCacheAt(u32 em_address, int length = 4) override { if (blocks.RangeMayHaveEmuHacks(em_address, em_address + length)) { blocks.InvalidateICache(em_address, length); From 4a0437d0ed6d89c21a9dd7933c1a07a84a9bbe67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Thu, 26 Jan 2017 10:01:14 +0100 Subject: [PATCH 5/5] Remove a useless wrapper function --- Common/CodeBlock.h | 4 ++-- Common/MemoryUtil.h | 3 --- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/Common/CodeBlock.h b/Common/CodeBlock.h index 41a02e70d2..a9911fcdd5 100644 --- a/Common/CodeBlock.h +++ b/Common/CodeBlock.h @@ -86,9 +86,9 @@ public: void EndWrite() { // OK, we're done. Re-protect the memory we touched. - if (PlatformIsWXExclusive()) { + if (PlatformIsWXExclusive() && writeStart_ != nullptr) { const uint8_t *end = GetCodePtr(); - ProtectMemoryPages(writeStart_, end, MEM_PROT_READ | MEM_PROT_EXEC); + ProtectMemoryPages(writeStart_, end - writeStart_, MEM_PROT_READ | MEM_PROT_EXEC); writeStart_ = nullptr; } } diff --git a/Common/MemoryUtil.h b/Common/MemoryUtil.h index 27e6fe6eae..53c917b7b8 100644 --- a/Common/MemoryUtil.h +++ b/Common/MemoryUtil.h @@ -37,9 +37,6 @@ void* AllocateExecutableMemory(size_t size); void* AllocateMemoryPages(size_t size, uint32_t memProtFlags); // Note that on platforms returning PlatformIsWXExclusive, you cannot set a page to be both readable and writable at the same time. bool ProtectMemoryPages(const void* ptr, size_t size, uint32_t memProtFlags); -inline void ProtectMemoryPages(const void *start, const void *end, uint32_t memProtFlags) { - ProtectMemoryPages((void *)start, (const uint8_t *)end - (const uint8_t *)start, memProtFlags); -} void FreeMemoryPages(void* ptr, size_t size); // Regular aligned memory. Don't try to apply memory protection willy-nilly to memory allocated this way as in-page alignment is unknown (though could be checked).