From 21fae17b2073bfb71f863c058b809a7104764e92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Tue, 9 Feb 2021 01:30:31 +0100 Subject: [PATCH] Handle exec addr errors better - don't let IgnoreBadMemoryAccesses skip dispatcher exceptions. It would then just fall through into the compiler and die. Should remove one of the "mystery" crashes from #14082. --- Core/Core.cpp | 1 + Core/Core.h | 1 + Core/MIPS/ARM64/Arm64Asm.cpp | 1 + Core/MIPS/ARM64/Arm64Jit.h | 4 ++++ Core/MIPS/JitCommon/JitCommon.h | 3 +++ Core/MIPS/x86/Asm.cpp | 2 +- Core/MIPS/x86/Jit.cpp | 5 +++++ Core/MIPS/x86/Jit.h | 6 ++++++ Core/MemFault.cpp | 17 ++++++++++++++--- pspautotests | 2 +- 10 files changed, 37 insertions(+), 5 deletions(-) diff --git a/Core/Core.cpp b/Core/Core.cpp index f3cdd9b62421..362f3a44e31a 100644 --- a/Core/Core.cpp +++ b/Core/Core.cpp @@ -398,6 +398,7 @@ const char *MemoryExceptionTypeAsString(MemoryExceptionType type) { case MemoryExceptionType::WRITE_WORD: return "Write Word"; case MemoryExceptionType::READ_BLOCK: return "Read Block"; case MemoryExceptionType::WRITE_BLOCK: return "Read/Write Block"; + case MemoryExceptionType::EXEC_ADDR: return "Bad Exec Addr"; default: return "N/A"; } diff --git a/Core/Core.h b/Core/Core.h index 21facdb735d5..735352347057 100644 --- a/Core/Core.h +++ b/Core/Core.h @@ -88,6 +88,7 @@ enum class MemoryExceptionType { WRITE_WORD, READ_BLOCK, WRITE_BLOCK, + EXEC_ADDR, }; enum class ExecExceptionType { JUMP, diff --git a/Core/MIPS/ARM64/Arm64Asm.cpp b/Core/MIPS/ARM64/Arm64Asm.cpp index b95b8e54b4d9..bca3f535e179 100644 --- a/Core/MIPS/ARM64/Arm64Asm.cpp +++ b/Core/MIPS/ARM64/Arm64Asm.cpp @@ -250,6 +250,7 @@ void Arm64Jit::GenerateFixedCode(const JitOptions &jo) { #ifdef MASKED_PSP_MEMORY ANDI2R(SCRATCH1, SCRATCH1, 0x3FFFFFFF); #endif + dispatcherFetch = GetCodePtr(); LDR(SCRATCH1, MEMBASEREG, SCRATCH1_64); LSR(SCRATCH2, SCRATCH1, 24); // or UBFX(SCRATCH2, SCRATCH1, 24, 8) ANDI2R(SCRATCH1, SCRATCH1, 0x00FFFFFF); diff --git a/Core/MIPS/ARM64/Arm64Jit.h b/Core/MIPS/ARM64/Arm64Jit.h index 17734ade594a..95affe2515b4 100644 --- a/Core/MIPS/ARM64/Arm64Jit.h +++ b/Core/MIPS/ARM64/Arm64Jit.h @@ -185,6 +185,9 @@ class Arm64Jit : public Arm64Gen::ARM64CodeBlock, public JitInterface, public MI const u8 *GetDispatcher() const override { return dispatcher; } + bool IsAtDispatchFetch(const u8 *ptr) const override { + return ptr == dispatcherFetch; + } void LinkBlock(u8 *exitPoint, const u8 *checkedEntry) override; void UnlinkBlock(u8 *checkedEntry, u32 originalAddress) override; @@ -275,6 +278,7 @@ class Arm64Jit : public Arm64Gen::ARM64CodeBlock, public JitInterface, public MI const u8 *dispatcherPCInSCRATCH1; const u8 *dispatcher; const u8 *dispatcherNoCheck; + const u8 *dispatcherFetch; const u8 *saveStaticRegisters; const u8 *loadStaticRegisters; diff --git a/Core/MIPS/JitCommon/JitCommon.h b/Core/MIPS/JitCommon/JitCommon.h index 08063f866e49..e39a8bab217b 100644 --- a/Core/MIPS/JitCommon/JitCommon.h +++ b/Core/MIPS/JitCommon/JitCommon.h @@ -123,6 +123,9 @@ namespace MIPSComp { virtual bool CodeInRange(const u8 *ptr) const = 0; virtual bool DescribeCodePtr(const u8 *ptr, std::string &name) = 0; + virtual bool IsAtDispatchFetch(const u8 *ptr) const { + return false; + } virtual const u8 *GetDispatcher() const = 0; virtual const u8 *GetCrashHandler() const = 0; virtual JitBlockCache *GetBlockCache() = 0; diff --git a/Core/MIPS/x86/Asm.cpp b/Core/MIPS/x86/Asm.cpp index a099e1044b64..84fa1e9cccfe 100644 --- a/Core/MIPS/x86/Asm.cpp +++ b/Core/MIPS/x86/Asm.cpp @@ -161,7 +161,7 @@ void Jit::GenerateFixedCode(JitOptions &jo) { #ifdef MASKED_PSP_MEMORY AND(32, R(EAX), Imm32(Memory::MEMVIEW32_MASK)); #endif - + dispatcherFetch = GetCodePtr(); #ifdef _M_IX86 _assert_msg_( Memory::base != 0, "Memory base bogus"); MOV(32, R(EAX), MDisp(EAX, (u32)Memory::base)); diff --git a/Core/MIPS/x86/Jit.cpp b/Core/MIPS/x86/Jit.cpp index 97682d5f7db2..6e77fbade4e0 100644 --- a/Core/MIPS/x86/Jit.cpp +++ b/Core/MIPS/x86/Jit.cpp @@ -274,6 +274,11 @@ void Jit::Compile(u32 em_address) { ClearCache(); } + if (!Memory::IsValidAddress(em_address)) { + Core_ExecException(em_address, em_address, ExecExceptionType::JUMP); + return; + } + BeginWrite(); int block_num = blocks.AllocateBlock(em_address); diff --git a/Core/MIPS/x86/Jit.h b/Core/MIPS/x86/Jit.h index b065d9052216..6c679547bafa 100644 --- a/Core/MIPS/x86/Jit.h +++ b/Core/MIPS/x86/Jit.h @@ -298,6 +298,11 @@ class Jit : public Gen::XCodeBlock, public JitInterface, public MIPSFrontendInte } return true; } + + bool IsAtDispatchFetch(const u8 *codePtr) const override { + return codePtr == dispatcherFetch; + } + void SaveFlags(); void LoadFlags(); @@ -321,6 +326,7 @@ class Jit : public Gen::XCodeBlock, public JitInterface, public MIPSFrontendInte const u8 *dispatcherCheckCoreState; const u8 *dispatcherNoCheck; const u8 *dispatcherInEAXNoCheck; + const u8 *dispatcherFetch; const u8 *restoreRoundingMode; const u8 *applyRoundingMode; diff --git a/Core/MemFault.cpp b/Core/MemFault.cpp index 38944bf98ba4..4db12f0ab1f3 100644 --- a/Core/MemFault.cpp +++ b/Core/MemFault.cpp @@ -41,16 +41,19 @@ namespace Memory { static int64_t g_numReportedBadAccesses = 0; const uint8_t *g_lastCrashAddress; +MemoryExceptionType g_lastMemoryExceptionType; + std::unordered_set g_ignoredAddresses; void MemFault_Init() { g_numReportedBadAccesses = 0; g_lastCrashAddress = nullptr; + g_lastMemoryExceptionType = MemoryExceptionType::NONE; g_ignoredAddresses.clear(); } bool MemFault_MayBeResumable() { - return g_lastCrashAddress != nullptr; + return g_lastCrashAddress != nullptr && g_lastMemoryExceptionType != MemoryExceptionType::EXEC_ADDR; } void MemFault_IgnoreLastCrash() { @@ -122,11 +125,15 @@ bool HandleFault(uintptr_t hostAddress, void *ctx) { std::string infoString = ""; + bool isAtDispatch = false; if (MIPSComp::jit) { std::string desc; if (MIPSComp::jit->DescribeCodePtr(codePtr, desc)) { infoString += desc + "\n"; } + if (MIPSComp::jit->IsAtDispatchFetch(codePtr)) { + isAtDispatch = true; + } } int instructionSize = 4; @@ -158,7 +165,9 @@ bool HandleFault(uintptr_t hostAddress, void *ctx) { infoString += disassembly + "\n"; } - if (success) { + if (isAtDispatch) { + type = MemoryExceptionType::EXEC_ADDR; + } else if (success) { if (info.isMemoryWrite) { type = MemoryExceptionType::WRITE_WORD; } else { @@ -168,7 +177,9 @@ bool HandleFault(uintptr_t hostAddress, void *ctx) { type = MemoryExceptionType::UNKNOWN; } - if (success && (g_Config.bIgnoreBadMemAccess || g_ignoredAddresses.find(codePtr) != g_ignoredAddresses.end())) { + g_lastMemoryExceptionType = type; + + if (success && !isAtDispatch && (g_Config.bIgnoreBadMemAccess || g_ignoredAddresses.find(codePtr) != g_ignoredAddresses.end())) { if (!info.isMemoryWrite) { // It was a read. Fill the destination register with 0. // TODO diff --git a/pspautotests b/pspautotests index 0a5453fb9f8e..1047400eaec6 160000 --- a/pspautotests +++ b/pspautotests @@ -1 +1 @@ -Subproject commit 0a5453fb9f8ef77bf81252488c218cfee3d6546a +Subproject commit 1047400eaec6bcbdb2a64d326375ef6a6617c4ac