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

Crash: Ensure we never handle faults in faults #16636

Merged
merged 2 commits into from
Dec 21, 2022
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
18 changes: 13 additions & 5 deletions Core/MIPS/ARM64/Arm64Jit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -427,12 +427,20 @@ bool Arm64Jit::DescribeCodePtr(const u8 *ptr, std::string &name) {
name = "loadStaticRegisters";
else {
u32 addr = blocks.GetAddressFromBlockPtr(ptr);
std::vector<int> numbers;
blocks.GetBlockNumbersFromAddress(addr, &numbers);
if (!numbers.empty()) {
const JitBlock *block = blocks.GetBlock(numbers[0]);
// Returns 0 when it's valid, but unknown.
if (addr == 0) {
name = "(unknown or deleted block)";
return true;
} else if (addr != (u32)-1) {
name = "(outside space)";
return true;
}

int number = blocks.GetBlockNumberFromAddress(addr);
if (number != -1) {
const JitBlock *block = blocks.GetBlock(number);
if (block) {
name = StringFromFormat("(block %d at %08x)", numbers[0], block->originalAddress);
name = StringFromFormat("(block %d at %08x)", number, block->originalAddress);
return true;
}
}
Expand Down
9 changes: 9 additions & 0 deletions Core/MIPS/JitCommon/JitBlockCache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,15 @@ void JitBlockCache::GetBlockNumbersFromAddress(u32 em_address, std::vector<int>
block_numbers->push_back(i);
}

int JitBlockCache::GetBlockNumberFromAddress(u32 em_address) {
for (int i = 0; i < num_blocks_; i++) {
if (blocks_[i].ContainsAddress(em_address))
return i;
}

return -1;
}

u32 JitBlockCache::GetAddressFromBlockPtr(const u8 *ptr) const {
if (!codeBlock_->IsInSpace(ptr))
return (u32)-1;
Expand Down
2 changes: 2 additions & 0 deletions Core/MIPS/JitCommon/JitBlockCache.h
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,8 @@ class JitBlockCache : public JitBlockCacheDebugInterface {
// Returns a list of block numbers - only one block can start at a particular address, but they CAN overlap.
// This one is slow so should only be used for one-shots from the debugger UI, not for anything during runtime.
void GetBlockNumbersFromAddress(u32 em_address, std::vector<int> *block_numbers);
// Similar to above, but only the first matching address.
int GetBlockNumberFromAddress(u32 em_address);
int GetBlockNumberFromEmuHackOp(MIPSOpcode inst, bool ignoreBad = false) const;

u32 GetAddressFromBlockPtr(const u8 *ptr) const;
Expand Down
13 changes: 12 additions & 1 deletion Core/MemFault.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ namespace Memory {
static int64_t g_numReportedBadAccesses = 0;
const uint8_t *g_lastCrashAddress;
MemoryExceptionType g_lastMemoryExceptionType;
static bool inCrashHandler = false;

std::unordered_set<const uint8_t *> g_ignoredAddresses;

Expand Down Expand Up @@ -88,6 +89,10 @@ static bool DisassembleNativeAt(const uint8_t *codePtr, int instructionSize, std
}

bool HandleFault(uintptr_t hostAddress, void *ctx) {
if (inCrashHandler)
return false;
inCrashHandler = true;

SContext *context = (SContext *)ctx;
const uint8_t *codePtr = (uint8_t *)(context->CTX_PC);

Expand All @@ -100,6 +105,7 @@ bool HandleFault(uintptr_t hostAddress, void *ctx) {
bool inJitSpace = MIPSComp::jit && MIPSComp::jit->CodeInRange(codePtr);
if (!inJitSpace) {
// This is a crash in non-jitted code. Not something we want to handle here, ignore.
inCrashHandler = false;
return false;
}

Expand All @@ -114,8 +120,10 @@ bool HandleFault(uintptr_t hostAddress, void *ctx) {
bool invalidHostAddress = hostAddress == (uintptr_t)0xFFFFFFFFFFFFFFFFULL;
if (hostAddress < baseAddress || hostAddress >= baseAddress + addressSpaceSize) {
// Host address outside - this was a different kind of crash.
if (!invalidHostAddress)
if (!invalidHostAddress) {
inCrashHandler = false;
return false;
}
}


Expand Down Expand Up @@ -182,6 +190,7 @@ bool HandleFault(uintptr_t hostAddress, void *ctx) {
// 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);
inCrashHandler = false;
return true;
} else if (success) {
if (info.isMemoryWrite) {
Expand Down Expand Up @@ -218,6 +227,8 @@ bool HandleFault(uintptr_t hostAddress, void *ctx) {
context->CTX_PC = (uintptr_t)MIPSComp::jit->GetCrashHandler();
ERROR_LOG(MEMMAP, "Bad memory access detected! %08x (%p) Stopping emulation. Info:\n%s", guestAddress, (void *)hostAddress, infoString.c_str());
}

inCrashHandler = false;
return true;
}

Expand Down
8 changes: 1 addition & 7 deletions UI/DevScreens.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1145,13 +1145,7 @@ UI::EventReturn JitCompareScreen::OnCurrentBlock(UI::EventParams &e) {
JitBlockCache *blockCache = MIPSComp::jit->GetBlockCache();
if (!blockCache)
return UI::EVENT_DONE;
std::vector<int> blockNum;
blockCache->GetBlockNumbersFromAddress(currentMIPS->pc, &blockNum);
if (blockNum.size() > 0) {
currentBlock_ = blockNum[0];
} else {
currentBlock_ = -1;
}
currentBlock_ = blockCache->GetBlockNumberFromAddress(currentMIPS->pc);
UpdateDisasm();
return UI::EVENT_DONE;
}
Expand Down