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

Bad memory access handling improvements #13092

Merged
merged 7 commits into from
Jul 12, 2020
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
1 change: 1 addition & 0 deletions Core/Config.h
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ struct Config {

// Core
bool bIgnoreBadMemAccess;

bool bFastMemory;
int iCpuCore;
bool bCheckForNewVersion;
Expand Down
76 changes: 74 additions & 2 deletions Core/Core.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#include "profiler/profiler.h"

#include "Common/GraphicsContext.h"
#include "Common/Log.h"
#include "Core/Core.h"
#include "Core/Config.h"
#include "Core/Host.h"
Expand All @@ -44,7 +45,6 @@
#include "Windows/InputDevice.h"
#endif


// Time until we stop considering the core active without user input.
// Should this be configurable? 2 hours currently.
static const double ACTIVITY_IDLE_TIMEOUT = 2.0 * 3600.0;
Expand All @@ -63,6 +63,8 @@ static double lastKeepAwake = 0.0;
static GraphicsContext *graphicsContext;
static bool powerSaving = false;

static ExceptionInfo g_exceptionInfo;

void Core_SetGraphicsContext(GraphicsContext *ctx) {
graphicsContext = ctx;
PSP_CoreParameter().graphicsContext = graphicsContext;
Expand Down Expand Up @@ -339,7 +341,8 @@ void Core_Run(GraphicsContext *ctx) {

case CORE_POWERUP:
case CORE_POWERDOWN:
case CORE_ERROR:
case CORE_BOOT_ERROR:
case CORE_RUNTIME_ERROR:
// Exit loop!!
Core_StateProcessed();

Expand All @@ -364,6 +367,75 @@ void Core_EnableStepping(bool step) {
}
}

bool Core_NextFrame() {
if (coreState == CORE_RUNNING) {
coreState = CORE_NEXTFRAME;
return true;
} else {
return false;
}
}

int Core_GetSteppingCounter() {
return steppingCounter;
}

const char *ExceptionTypeAsString(ExceptionType type) {
switch (type) {
case ExceptionType::MEMORY: return "Invalid Memory Access";
case ExceptionType::BREAK: return "Break";
case ExceptionType::BAD_EXEC_ADDR: return "Bad Execution Address";
default: return "N/A";
}
}

const char *MemoryExceptionTypeAsString(MemoryExceptionType type) {
switch (type) {
case MemoryExceptionType::READ_WORD: return "Read Word";
case MemoryExceptionType::WRITE_WORD: return "Write Word";
case MemoryExceptionType::READ_BLOCK: return "Read Block";
case MemoryExceptionType::WRITE_BLOCK: return "Read/Write Block";
default:
return "N/A";
}
}

void Core_MemoryException(u32 address, u32 pc, MemoryExceptionType type) {
const char *desc = MemoryExceptionTypeAsString(type);
// In jit, we only flush PC when bIgnoreBadMemAccess is off.
if (g_Config.iCpuCore == (int)CPUCore::JIT && g_Config.bIgnoreBadMemAccess) {
WARN_LOG(MEMMAP, "%s: Invalid address %08x", desc, address);
} else {
WARN_LOG(MEMMAP, "%s: Invalid address %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::MEMORY;
e.info = "";
e.memory_type = type;
e.address = address;
e.pc = pc;
Core_EnableStepping(true);
host->SetDebugMode(true);
}
}

void Core_Break() {
ERROR_LOG(CPU, "BREAK!");

ExceptionInfo &e = g_exceptionInfo;
e = {};
e.type = ExceptionType::BREAK;
e.info = "";

if (!g_Config.bIgnoreBadMemAccess) {
Core_EnableStepping(true);
host->SetDebugMode(true);
}
}

const ExceptionInfo &Core_GetExceptionInfo() {
return g_exceptionInfo;
}
34 changes: 34 additions & 0 deletions Core/Core.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ void Core_SetGraphicsContext(GraphicsContext *ctx);

// called from gui
void Core_EnableStepping(bool step);

bool Core_NextFrame();
void Core_DoSingleStep();
void Core_UpdateSingleStep();
void Core_ProcessStepping();
Expand Down Expand Up @@ -76,3 +78,35 @@ void Core_NotifyActivity();

void Core_SetPowerSaving(bool mode);
bool Core_GetPowerSaving();

enum class MemoryExceptionType {
READ_WORD,
WRITE_WORD,
READ_BLOCK,
WRITE_BLOCK,
};

void Core_MemoryException(u32 address, u32 pc, MemoryExceptionType type);
void Core_Break();

enum class ExceptionType {
NONE,
MEMORY,
BREAK,
BAD_EXEC_ADDR,
};
hrydgard marked this conversation as resolved.
Show resolved Hide resolved

struct ExceptionInfo {
ExceptionType type;
std::string info;

// Memory exception info
MemoryExceptionType memory_type;
uint32_t pc;
uint32_t address;
};

const ExceptionInfo &Core_GetExceptionInfo();

const char *ExceptionTypeAsString(ExceptionType type);
const char *MemoryExceptionTypeAsString(MemoryExceptionType type);
5 changes: 3 additions & 2 deletions Core/HLE/HLE.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -469,7 +469,7 @@ void HLEReturnFromMipsCall() {

if ((stackData->nextOff & 0x0000000F) != 0 || !Memory::IsValidAddress(sp + stackData->nextOff)) {
ERROR_LOG(HLE, "Corrupt stack on HLE mips call return: %08x", stackData->nextOff);
Core_UpdateState(CORE_ERROR);
Core_UpdateState(CORE_RUNTIME_ERROR);
return;
}

Expand All @@ -482,9 +482,10 @@ void HLEReturnFromMipsCall() {
while ((finalMarker->nextOff & 0x0000000F) == 0 && Memory::IsValidAddress(finalMarker.ptr + finalMarker->nextOff)) {
finalMarker.ptr += finalMarker->nextOff;
}

if (finalMarker->nextOff != 0xFFFFFFFF) {
ERROR_LOG(HLE, "Corrupt stack on HLE mips call return action: %08x", finalMarker->nextOff);
Core_UpdateState(CORE_ERROR);
Core_UpdateState(CORE_RUNTIME_ERROR);
return;
}

Expand Down
1 change: 0 additions & 1 deletion Core/HLE/proAdhoc.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
#include "net/resolve.h"
#include "Common/ChunkFile.h"

#include "Core/Config.h"
#include "Core/CoreTiming.h"
#include "Core/MemMap.h"
#include "Core/HLE/HLE.h"
Expand Down
4 changes: 2 additions & 2 deletions Core/HLE/sceDisplay.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
#include "Core/CoreParameter.h"
#include "Core/Host.h"
#include "Core/Reporting.h"
#include "Core/Core.h"
#include "Core/System.h"
#include "Core/HLE/HLE.h"
#include "Core/HLE/FunctionWrappers.h"
Expand Down Expand Up @@ -791,8 +792,7 @@ void __DisplayFlip(int cyclesLate) {
const bool fbReallyDirty = gpu->FramebufferReallyDirty();
if (fbReallyDirty || noRecentFlip || postEffectRequiresFlip) {
// Check first though, might've just quit / been paused.
if (coreState == CORE_RUNNING) {
coreState = CORE_NEXTFRAME;
if (Core_NextFrame()) {
gpu->CopyDisplayToOutput(fbReallyDirty);
if (fbReallyDirty) {
actualFlips++;
Expand Down
2 changes: 1 addition & 1 deletion Core/HLE/sceIo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -555,7 +555,7 @@ static VFSFileSystem *flash0System = nullptr;

static void __IoManagerThread() {
setCurrentThreadName("IO");
while (ioManagerThreadEnabled && coreState != CORE_ERROR && coreState != CORE_POWERDOWN) {
while (ioManagerThreadEnabled && coreState != CORE_BOOT_ERROR && coreState != CORE_RUNTIME_ERROR && coreState != CORE_POWERDOWN) {
hrydgard marked this conversation as resolved.
Show resolved Hide resolved
ioManager.RunEventsUntil(CoreTiming::GetTicks() + msToCycles(1000));
}
}
Expand Down
6 changes: 6 additions & 0 deletions Core/HLE/sceKernel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,12 @@ bool __KernelIsRunning() {
return kernelRunning;
}

std::string __KernelStateSummary() {
std::string threadSummary = __KernelThreadingSummary();
return StringFromFormat("%s", threadSummary.c_str());
}


void sceKernelExitGame()
{
INFO_LOG(SCEKERNEL, "sceKernelExitGame");
Expand Down
4 changes: 4 additions & 0 deletions Core/HLE/sceKernel.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#pragma once

#include <map>
#include <string>

#include "Common/Common.h"
#include "Common/Swap.h"
Expand Down Expand Up @@ -374,6 +375,9 @@ void __KernelDoState(PointerWrap &p);
bool __KernelIsRunning();
bool __KernelLoadExec(const char *filename, SceKernelLoadExecParam *param);

// For crash reporting.
std::string __KernelStateSummary();

int sceKernelLoadExec(const char *filename, u32 paramPtr);

void sceKernelExitGame();
Expand Down
2 changes: 1 addition & 1 deletion Core/HLE/sceKernelModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1837,7 +1837,7 @@ int sceKernelLoadExec(const char *filename, u32 paramPtr)
std::string error_string;
if (!__KernelLoadExec(exec_filename.c_str(), paramPtr, &error_string)) {
ERROR_LOG(SCEMODULE, "sceKernelLoadExec failed: %s", error_string.c_str());
Core_UpdateState(CORE_ERROR);
Core_UpdateState(CORE_RUNTIME_ERROR);
return -1;
}
if (gpu) {
Expand Down
24 changes: 13 additions & 11 deletions Core/HLE/sceKernelThread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,8 @@ class ActionAfterCallback : public PSPAction

class PSPThread : public KernelObject {
public:
PSPThread() : debug(currentMIPS, context) {}

const char *GetName() override { return nt.name; }
const char *GetTypeName() override { return "Thread"; }
void GetQuickInfo(char *ptr, int size) override
Expand Down Expand Up @@ -491,10 +493,6 @@ class PSPThread : public KernelObject {
return true;
}

PSPThread() : debug(currentMIPS, context) {
currentStack.start = 0;
}

// Can't use a destructor since savestates will call that too.
void Cleanup()
{
Expand Down Expand Up @@ -573,14 +571,14 @@ class PSPThread : public KernelObject {
}
}

NativeThread nt;
NativeThread nt{};

ThreadWaitInfo waitInfo;
SceUID moduleId;
ThreadWaitInfo waitInfo{};
SceUID moduleId = -1;

bool isProcessingCallbacks;
u32 currentMipscallId;
SceUID currentCallbackId;
bool isProcessingCallbacks = false;
u32 currentMipscallId = -1;
SceUID currentCallbackId = -1;

PSPThreadContext context;
KernelThreadDebugInterface debug;
Expand All @@ -597,7 +595,7 @@ class PSPThread : public KernelObject {
// These are stacks that aren't "active" right now, but will pop off once the func returns.
std::vector<StackInfo> pushedStacks;

StackInfo currentStack;
StackInfo currentStack{};

// For thread end.
std::vector<SceUID> waitingThreads;
Expand Down Expand Up @@ -1184,6 +1182,10 @@ void __KernelThreadingShutdown() {
pendingDeleteThreads.clear();
}

std::string __KernelThreadingSummary() {
return StringFromFormat("Cur thread: %s", __GetCurrentThread()->GetName());
}

const char *__KernelGetThreadName(SceUID threadID)
{
u32 error;
Expand Down
3 changes: 3 additions & 0 deletions Core/HLE/sceKernelThread.h
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,9 @@ void __KernelThreadingInit();
void __KernelThreadingDoState(PointerWrap &p);
void __KernelThreadingDoStateLate(PointerWrap &p);
void __KernelThreadingShutdown();

std::string __KernelThreadingSummary();

KernelObject *__KernelThreadObject();
KernelObject *__KernelCallbackObject();

Expand Down
4 changes: 2 additions & 2 deletions Core/HW/AsyncIOManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ class AsyncIOManager : public IOThreadEventQueue {
protected:
void ProcessEvent(AsyncIOEvent ref) override;
bool ShouldExitEventLoop() override {
return coreState == CORE_ERROR || coreState == CORE_POWERDOWN;
return coreState == CORE_BOOT_ERROR || coreState == CORE_RUNTIME_ERROR || coreState == CORE_POWERDOWN;
}

private:
Expand All @@ -106,4 +106,4 @@ class AsyncIOManager : public IOThreadEventQueue {
std::condition_variable resultsWait_;
std::set<u32> resultsPending_;
std::map<u32, AsyncIOResult> results_;
};
};
1 change: 1 addition & 0 deletions Core/HW/Camera.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
// https://github.com/hrydgard/ppsspp and http://www.ppsspp.org/.

#include "Camera.h"
#include "Core/Config.h"

void convert_frame(int inw, int inh, unsigned char *inData, AVPixelFormat inFormat,
int outw, int outh, unsigned char **outData, int *outLen) {
Expand Down
1 change: 0 additions & 1 deletion Core/HW/Camera.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
#pragma once

#include "ppsspp_config.h"
#include "Core/Config.h"
#include "Core/HLE/sceUsbCam.h"
#include "Log.h"

Expand Down
6 changes: 3 additions & 3 deletions Core/Loaders.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ bool LoadFile(FileLoader **fileLoaderPtr, std::string *error_string) {
}
else if (ebootType == IdentifiedFileType::PSP_PS1_PBP) {
*error_string = "PS1 EBOOTs are not supported by PPSSPP.";
coreState = CORE_ERROR;
coreState = CORE_BOOT_ERROR;
return false;
}
std::string path = fileLoader->Path();
Expand All @@ -270,7 +270,7 @@ bool LoadFile(FileLoader **fileLoaderPtr, std::string *error_string) {
return Load_PSP_ELF_PBP(fileLoader, error_string);
} else {
*error_string = "No EBOOT.PBP, misidentified game";
coreState = CORE_ERROR;
coreState = CORE_BOOT_ERROR;
return false;
}
}
Expand Down Expand Up @@ -352,7 +352,7 @@ bool LoadFile(FileLoader **fileLoaderPtr, std::string *error_string) {
break;
}

coreState = CORE_ERROR;
coreState = CORE_BOOT_ERROR;
return false;
}

Expand Down
Loading