Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle exec addr errors better - don't let IgnoreBadMemoryAccesses skip dispatcher exceptions #14085

Merged
merged 4 commits into from
Feb 9, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 9 additions & 11 deletions Core/Core.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -460,17 +460,15 @@ void Core_ExecException(u32 address, u32 pc, ExecExceptionType type) {
const char *desc = ExecExceptionTypeAsString(type);
WARN_LOG(MEMMAP, "%s: Invalid destination %08x PC %08x LR %08x", desc, address, currentMIPS->pc, currentMIPS->r[MIPS_REG_RA]);

if (!g_Config.bIgnoreBadMemAccess) {
ExceptionInfo &e = g_exceptionInfo;
e = {};
e.type = ExceptionType::BAD_EXEC_ADDR;
e.info = "";
e.exec_type = type;
e.address = address;
e.pc = pc;
Core_EnableStepping(true);
host->SetDebugMode(true);
}
ExceptionInfo &e = g_exceptionInfo;
e = {};
e.type = ExceptionType::BAD_EXEC_ADDR;
e.info = "";
e.exec_type = type;
e.address = address;
e.pc = pc;
Core_EnableStepping(true);
host->SetDebugMode(true);
}

void Core_Break() {
Expand Down
1 change: 1 addition & 0 deletions Core/MIPS/ARM/ArmAsm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,7 @@ void ArmJit::GenerateFixedCode() {
LDR(R0, CTXREG, offsetof(MIPSState, pc));
// TODO: In practice, do we ever run code from uncached space (| 0x40000000)? If not, we can remove this BIC.
BIC(R0, R0, Operand2(0xC0, 4)); // &= 0x3FFFFFFF
dispatcherFetch = GetCodePtr();
LDR(R0, MEMBASEREG, R0);
AND(R1, R0, Operand2(0xFF, 4)); // rotation is to the right, in 2-bit increments.
BIC(R0, R0, Operand2(0xFF, 4));
Expand Down
4 changes: 4 additions & 0 deletions Core/MIPS/ARM/ArmJit.h
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,9 @@ class ArmJit : public ArmGen::ARMXCodeBlock, public JitInterface, public MIPSFro
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;
Expand Down Expand Up @@ -311,6 +314,7 @@ class ArmJit : public ArmGen::ARMXCodeBlock, public JitInterface, public MIPSFro
const u8 *dispatcherCheckCoreState;
const u8 *dispatcherPCInR0;
const u8 *dispatcher;
const u8 *dispatcherFetch;
const u8 *dispatcherNoCheck;

const u8 *restoreRoundingMode;
Expand Down
1 change: 1 addition & 0 deletions Core/MIPS/ARM64/Arm64Asm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 4 additions & 0 deletions Core/MIPS/ARM64/Arm64Jit.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
3 changes: 3 additions & 0 deletions Core/MIPS/JitCommon/JitCommon.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion Core/MIPS/x86/Asm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
5 changes: 5 additions & 0 deletions Core/MIPS/x86/Jit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,11 @@ void Jit::Compile(u32 em_address) {
ClearCache();
}

if (!Memory::IsValidAddress(em_address)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this in ArmJit and Arm64Jit?

-[Unknown]

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if we need it at all. I think when using the JIT, the exception handler will always catch it in the dispatcher.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe there's some other path we end up in Compile, I dunno...

Core_ExecException(em_address, em_address, ExecExceptionType::JUMP);
return;
}

BeginWrite();

int block_num = blocks.AllocateBlock(em_address);
Expand Down
6 changes: 6 additions & 0 deletions Core/MIPS/x86/Jit.h
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -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;
Expand Down
23 changes: 22 additions & 1 deletion Core/MemFault.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,14 @@ namespace Memory {

static int64_t g_numReportedBadAccesses = 0;
const uint8_t *g_lastCrashAddress;
MemoryExceptionType g_lastMemoryExceptionType;

std::unordered_set<const uint8_t *> g_ignoredAddresses;

void MemFault_Init() {
g_numReportedBadAccesses = 0;
g_lastCrashAddress = nullptr;
g_lastMemoryExceptionType = MemoryExceptionType::NONE;
g_ignoredAddresses.clear();
}

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -158,7 +165,19 @@ bool HandleFault(uintptr_t hostAddress, void *ctx) {
infoString += disassembly + "\n";
}

if (success) {
if (isAtDispatch) {
u32 targetAddr = currentMIPS->pc; // bad approximation
// TODO: Do the other archs and platforms.
#if PPSSPP_ARCH(AMD64) && PPSSPP_PLATFORM(WINDOWS)
// We know which register the address is in, look in Asm.cpp.
targetAddr = context->Rax;
#endif
Core_ExecException(targetAddr, currentMIPS->pc, ExecExceptionType::JUMP);
// Redirect execution to a crash handler that will switch to CoreState::CORE_RUNTIME_ERROR immediately.
context->CTX_PC = (uintptr_t)MIPSComp::jit->GetCrashHandler();
ERROR_LOG(MEMMAP, "Bad execution access detected, halting: %08x (last known pc %08x, host: %p)", targetAddr, currentMIPS->pc, (void *)hostAddress);
return true;
} else if (success) {
if (info.isMemoryWrite) {
type = MemoryExceptionType::WRITE_WORD;
} else {
Expand All @@ -168,6 +187,8 @@ bool HandleFault(uintptr_t hostAddress, void *ctx) {
type = MemoryExceptionType::UNKNOWN;
}

g_lastMemoryExceptionType = type;

if (success && (g_Config.bIgnoreBadMemAccess || g_ignoredAddresses.find(codePtr) != g_ignoredAddresses.end())) {
if (!info.isMemoryWrite) {
// It was a read. Fill the destination register with 0.
Expand Down