Add command line parameters for controlling the behavior on memory/instruction exceptions

Core/CmdLine.cpp

@@ -23,6 +23,7 @@ enum class CmdParamType {
 	Int,
 	Double,
 	String,
+	Enum,
 };
 
 struct CommandLineParam {
@@ -32,6 +33,8 @@ struct CommandLineParam {
 	char shortName;  // can be 0 for no short name
 	const char *docString;
 	CmdLineMode mode;
+	const char **enumValues;
+	int enumCount;
 };
 
 enum class ParseParamResult {
@@ -41,6 +44,7 @@ enum class ParseParamResult {
 };
 
 ParseParamResult SetValue(CommandLineOptions *options, const CommandLineParam &param, const std::string &value) {
+	uint8_t *optionsPtr = reinterpret_cast<uint8_t *>(options);
 	switch (param.type) {
 	case CmdParamType::Bool:
 	case CmdParamType::BoolInverse:
@@ -58,18 +62,37 @@ ParseParamResult SetValue(CommandLineOptions *options, const CommandLineParam &p
 		if (param.type == CmdParamType::BoolInverse) {
 			v = !v;
 		}
-		*reinterpret_cast<std::optional<bool> *>(reinterpret_cast<uint8_t *>(options) + param.offsetInStruct) = v;
+		*reinterpret_cast<std::optional<bool> *>(optionsPtr + param.offsetInStruct) = v;
 		break;
 	}
 	case CmdParamType::Int:
-		*reinterpret_cast<std::optional<int> *>(reinterpret_cast<uint8_t *>(options) + param.offsetInStruct) = std::stoi(value);
+		*reinterpret_cast<std::optional<int> *>(optionsPtr + param.offsetInStruct) = std::stoi(value);
 		break;
 	case CmdParamType::Double:
-		*reinterpret_cast<std::optional<double> *>(reinterpret_cast<uint8_t *>(options) + param.offsetInStruct) = std::stod(value);
+		*reinterpret_cast<std::optional<double> *>(optionsPtr + param.offsetInStruct) = std::stod(value);
 		break;
 	case CmdParamType::String:
-		*reinterpret_cast<std::optional<std::string> *>(reinterpret_cast<uint8_t *>(options) + param.offsetInStruct) = value;
+		*reinterpret_cast<std::optional<std::string> *>(optionsPtr + param.offsetInStruct) = value;
 		break;
+	case CmdParamType::Enum:
+	{
+		int enumIndex = -1;
+		for (int i = 0; i < param.enumCount; ++i) {
+			if (value == param.enumValues[i]) {
+				enumIndex = i;
+				break;
+			}
+		}
+		if (enumIndex == -1) {
+			PRINT_STDERR("Error: Invalid value for parameter --%s: '%s'. Expected one of: ", param.longName, value.c_str());
+			for (int i = 0; i < param.enumCount; ++i) {
+				PRINT_STDERR("%s%s", param.enumValues[i], (i + 1 < param.enumCount) ? ", " : "\n");
+			}
+			return ParseParamResult::BadValue;
+		}
+		*reinterpret_cast<std::optional<int> *>(optionsPtr + param.offsetInStruct) = enumIndex;
+		break;
+	}
 	default:
 		break;
 	}
@@ -132,6 +155,14 @@ static ParseParamResult ParseParameterStr(int argc, const char *argv[], size_t &
 	return ParseParamResult::NoMatch;
 }
 
+// Enum ExceptionAction
+const char *g_ExceptionActionValues[] = {
+	"default",
+	"ignore",
+	"log",
+	"break",
+};
+
 #define POFF(member) offsetof(CommandLineOptions, member)
 static const CommandLineParam g_autoParams[] = {
 	{POFF(fullscreen), CmdParamType::Bool, "fullscreen", '\0', "Force full screen mode", CmdLineMode::Application},
@@ -152,28 +183,35 @@ static const CommandLineParam g_autoParams[] = {
 	{POFF(resolutionScale), CmdParamType::Int, "resolution-scale", '\0', "Set the resolution scale factor"},
 	{POFF(debuggerPort), CmdParamType::Int, "debugger", '\0', "Enable the WebSocket debugger on this port (0 = pick automatically); see docs/WebSocketDebugger.md"},
 	{POFF(bootVSH), CmdParamType::Bool, "vsh", '\0', "Boot the VSH (requires files dumped from a PSP in the flash0 directory)"},
+	{POFF(memReadAction), CmdParamType::Enum, "memread", '\0', "Set the action for memory read exceptions", CmdLineMode::Both, g_ExceptionActionValues, ARRAY_SIZE(g_ExceptionActionValues)},
+	{POFF(memWriteAction), CmdParamType::Enum, "memwrite", '\0', "Set the action for memory write exceptions", CmdLineMode::Both, g_ExceptionActionValues, ARRAY_SIZE(g_ExceptionActionValues)},
+	{POFF(breakAction), CmdParamType::Enum, "break", '\0', "Set the action for break exceptions", CmdLineMode::Both, g_ExceptionActionValues, ARRAY_SIZE(g_ExceptionActionValues)},
+
 	// TODO: At some point we should maybe simply expose all config settings to be set directly from the command line automatically?
 };
 
-// NOTE: On Windows this prints nothing unfortunately, since PPSSPP is not a "console app".
+// NOTE: On Windows this prints nothing unfortunately, since PPSSPP is not a "console app" (PPSSPPHeadless is though, there it works).
 // A fun trick we could do is AttachConsole(ATTACH_PARENT_PROCESS) which works at least from git bash and powershell, if we also use WriteConsole.
 // However it's not exactly ideal.
-static int printUsage(int argc, const char *argv[], CmdLineMode mode) {
+int CommandLineOptions::PrintUsage(const char *progname, const char *situationText) const {
 	// NOTE: by convention, --help outputs to stdout,
 	// not to stderr, since it is intended output in this
 	// case (usage printed under different circumstances,
 	// say in response to error during parsing commandline,
 	// may go to stderr).
-	const char *progname = argc > 0 ? argv[0] : "ppsspp";
 	// NOTE: wording largely taken from
 	// https://www.ppsspp.org/docs/reference/command-line/
-	if (mode == CmdLineMode::Application) {
-		PRINT_STDOUT("PPSSPP - a PSP emulator\n");
+	if (situationText) {
+		PRINT_STDOUT("%s\n\n", situationText);
 	} else {
-		PRINT_STDOUT("PPSSPP Headless\n");
-		PRINT_STDOUT("This is primarily meant as a non-interactive test tool.\n\n");
+		if (mode == CmdLineMode::Application) {
+			PRINT_STDOUT("PPSSPP - a PSP emulator\n");
+		} else {
+			PRINT_STDOUT("PPSSPP Headless\n");
+			PRINT_STDOUT("This is primarily meant as a non-interactive test tool.\n\n");
+		}
 	}
-	PRINT_STDOUT("PPSSPP - a PSP emulator\n");
+
 	PRINT_STDOUT("Usage: %s [options] [FILE]\n\n", progname);
 	PRINT_STDOUT("Launches FILE (e.g. ISO image) if present.\n");
 	PRINT_STDOUT("Options (some of these are specific to SDL backend):\n");
@@ -199,6 +237,13 @@ static int printUsage(int argc, const char *argv[], CmdLineMode mode) {
 		PRINT_STDOUT("  --%s%s%s\n", param.longName,
 			param.shortName ? ", -" : "",
 			param.shortName ? std::string(1, param.shortName).c_str() : "");
+		if (param.type == CmdParamType::Enum) {
+			PRINT_STDOUT("                        options: ");
+			for (int i = 0; i < param.enumCount; ++i) {
+				PRINT_STDOUT("%s%s", param.enumValues[i], i + 1 < param.enumCount ? ", " : "");
+			}
+			PRINT_STDOUT("\n");
+		}
 		PRINT_STDOUT("                        %s\n", param.docString);
 	}
 
@@ -218,6 +263,7 @@ static int printUsage(int argc, const char *argv[], CmdLineMode mode) {
 // Error reporting is done with PRINT_STDERR(....).
 // Actually might want to reconsider given Android...
 CommandLineParseResult CommandLineOptions::Parse(int argc, const char *argv[], CmdLineMode mode) {
+	this->mode = mode;
 	constexpr std::string_view gpuBackendStr = "--graphics=";
 	constexpr std::string_view configOption = "--config=";
 	constexpr std::string_view controlsOption = "--controlconfig=";
@@ -260,7 +306,7 @@ CommandLineParseResult CommandLineOptions::Parse(int argc, const char *argv[], C
 				logLevel = LogLevel::LVERBOSE;
 				break;
 			case 'h':
-				printUsage(argc, argv, mode);
+				PrintUsage(argv[0], mode, nullptr);
 				return CommandLineParseResult::Exit;
 			// Legacy cpucore options.
 			case 'j':
@@ -306,7 +352,7 @@ CommandLineParseResult CommandLineOptions::Parse(int argc, const char *argv[], C
 			// We already incremented i.
 			continue;
 		} else if (equals(argv[i], "--help")) {
-			printUsage(argc, argv, mode);
+			PrintUsage(argv[0], mode, nullptr);
 			return CommandLineParseResult::Exit;
 		} else if (equals(argv[i], "--version")) {
 			printf("%s\n", PPSSPP_GIT_VERSION);
@@ -423,7 +469,22 @@ void CommandLineOptions::ApplyToConfig() const {
 		g_Config.DoNotSaveSetting(&g_Config.iInternalResolution);
 	}
 
-	// --vsh is applied by the caller.
+	if (memReadAction.has_value()) {
+		g_Config.iExceptionActionMemRead = memReadAction.value();
+		g_Config.DoNotSaveSetting(&g_Config.iExceptionActionMemRead);
+	}
+
+	if (memWriteAction.has_value()) {
+		g_Config.iExceptionActionMemWrite = memWriteAction.value();
+		g_Config.DoNotSaveSetting(&g_Config.iExceptionActionMemWrite);
+	}
+
+	if (breakAction.has_value()) {
+		g_Config.iExceptionActionBreak = breakAction.value();
+		g_Config.DoNotSaveSetting(&g_Config.iExceptionActionBreak);
+	}
+
+	// --vsh is applied by the caller (by setting their boot file name variable).
 
 	// Note: dpi is not applied here - it's platform-specific.
 	// Platforms should check cmdLineOptions.dpi.has_value() and handle accordingly.

Core/CmdLine.h

@@ -24,6 +24,9 @@ struct CommandLineOptions {
 	// If returns CommandLineParseResult::Exit or ::Error, the program should exit immediately (with an error return code if Error).
 	CommandLineParseResult Parse(int argc, const char *argv[], CmdLineMode mode = CmdLineMode::Application);
 	void ApplyToConfig() const;
+	int PrintUsage(const char *progname, const char *situationText) const;
+
+	CmdLineMode mode;
 
 	std::optional<bool> fullscreen;
 	std::optional<GPUBackend> gpuBackend;
@@ -52,6 +55,10 @@ struct CommandLineOptions {
 	std::optional<std::string> root;  // mount root, needs more explanation
 	std::optional<std::string> stateToLoad;
 
+	std::optional<int> memReadAction;
+	std::optional<int> memWriteAction;
+	std::optional<int> breakAction;
+
 	// SDL only: Option to force a specific OpenGL version (42="4.2",
 	// etc.; -1 means "try them all").
 	// Implemented as a workaround for https://github.com/hrydgard/ppsspp/issues/20687

Core/Config.h

@@ -213,6 +213,11 @@ public:
 	// Core
 	bool bIgnoreBadMemAccess;
 
+	// ExceptionAction enum: 0 = default (obey bIgnoreBadMemAccess), 1 = log, 2 = break, 3 = exit
+	int iExceptionActionMemRead;  // this also includes alignment and other odd memory exceptions.
+	int iExceptionActionMemWrite;
+	int iExceptionActionBreak;
+
 	bool bFastMemory;
 	int iCpuCore;
 	bool bCheckForNewVersion;

Core/ConfigValues.h

@@ -180,6 +180,13 @@ enum IOTimingMethods {
 	IOTIMING_UMDSLOWREALISTIC = 3,
 };
 
+enum class ExceptionAction {
+	Default = 0,  // Obey the value of "Ignore bad memory accesses" (that corresponds to Ignore vs Crash)
+	Ignore = 1,
+	Log = 2,  // Ignore, but log details
+	Break = 3, // Break execution in the PPSSPP debuggers
+};
+
 enum class AutoLoadSaveState {
 	OFF = 0,
 	OLDEST = 1,

Core/Core.cpp

@@ -550,21 +550,46 @@ const char *ExecExceptionTypeAsString(ExecExceptionType type) {
 	}
 }
 
-void Core_MemoryException(u32 address, u32 accessSize, u32 pc, MemoryExceptionType type) {
+static ExceptionAction ResolveExceptionAction(ExceptionAction action) {
+	if (action == ExceptionAction::Default) {
+		return g_Config.bIgnoreBadMemAccess ? ExceptionAction::Ignore : ExceptionAction::Break;
+	}
+	return action;
+}
+
+void Core_MemoryException(u32 address, u32 accessSize, u32 pc, MemoryExceptionType type, std::string_view additionalInfo, bool forceReport) {
 	const char *desc = MemoryExceptionTypeAsString(type);
 	// In jit, we only flush PC when bIgnoreBadMemAccess is off.
-	if ((g_Config.iCpuCore == (int)CPUCore::JIT || g_Config.iCpuCore == (int)CPUCore::JIT_IR) && g_Config.bIgnoreBadMemAccess) {
-		WARN_LOG(Log::MemMap, "%s: Invalid access at %08x (size %08x)", desc, address, accessSize);
-	} else {
-		WARN_LOG(Log::MemMap, "%s: Invalid access at %08x (size %08x) PC %08x LR %08x", desc, address, accessSize, currentMIPS->pc, currentMIPS->r[MIPS_REG_RA]);
+
+	char pcDetails[64];
+	pcDetails[0] = 0;
+	if ((CPUCore)g_Config.iCpuCore == CPUCore::INTERPRETER) {
+		snprintf(pcDetails, sizeof(pcDetails), " PC %08x LR %08x", currentMIPS->pc, currentMIPS->r[MIPS_REG_RA]);
 	}
 
-	if (!g_Config.bIgnoreBadMemAccess) {
-		// Try to fetch a call stack, to start with.
-		std::vector<MIPSStackWalk::StackFrame> stackFrames = WalkCurrentStack(-1);
-		std::string stackTrace = FormatStackTrace(stackFrames);
-		WARN_LOG(Log::MemMap, "\n%s", stackTrace.c_str());
+	ExceptionAction action;
+	switch (type) {
+	case MemoryExceptionType::WRITE_WORD:
+	case MemoryExceptionType::WRITE_BLOCK:
+		action = ResolveExceptionAction((ExceptionAction)g_Config.iExceptionActionMemWrite);
+		break;
+	case MemoryExceptionType::READ_WORD:
+	case MemoryExceptionType::READ_BLOCK:
+	default:
+		action = ResolveExceptionAction((ExceptionAction)g_Config.iExceptionActionMemRead);
+		break;
+	}
 
+	if (action == ExceptionAction::Ignore) {
+		// Simplest logging and continue.
+		WARN_LOG(Log::MemMap, "%s: Invalid access at %08x (size %08x) %s%.*s", desc, address, accessSize, pcDetails, (int)additionalInfo.length(), additionalInfo.data());
+		return;
+	}
+
+	const std::string stackTrace = FormatStackTrace(WalkCurrentStack(-1));
+	// Do the most detailed logging we can.
+	ERROR_LOG(Log::MemMap, "%s: Invalid access at %08x (size %08x) %s%.*s\n%s", desc, address, accessSize, pcDetails, (int)additionalInfo.length(), additionalInfo.data(), stackTrace.c_str());
+	if (action == ExceptionAction::Break) {
 		MIPSExceptionInfo &e = g_exceptionInfo;
 		e = {};
 		e.type = MIPSExceptionType::MEMORY;
@@ -578,35 +603,7 @@ void Core_MemoryException(u32 address, u32 accessSize, u32 pc, MemoryExceptionTy
 	}
 }
 
-void Core_MemoryExceptionInfo(u32 address, u32 accessSize, u32 pc, MemoryExceptionType type, std::string_view additionalInfo, bool forceReport) {
-	const char *desc = MemoryExceptionTypeAsString(type);
-	// In jit, we only flush PC when bIgnoreBadMemAccess is off.
-	if ((g_Config.iCpuCore == (int)CPUCore::JIT || g_Config.iCpuCore == (int)CPUCore::JIT_IR) && g_Config.bIgnoreBadMemAccess) {
-		WARN_LOG(Log::MemMap, "%s: Invalid access at %08x (size %08x). %.*s", desc, address, accessSize, (int)additionalInfo.length(), additionalInfo.data());
-	} else {
-		WARN_LOG(Log::MemMap, "%s: Invalid access at %08x (size %08x) PC %08x LR %08x %.*s", desc, address, accessSize, currentMIPS->pc, currentMIPS->r[MIPS_REG_RA], (int)additionalInfo.length(), additionalInfo.data());
-	}
-
-	if (!g_Config.bIgnoreBadMemAccess || forceReport) {
-		// Try to fetch a call stack, to start with.
-		std::vector<MIPSStackWalk::StackFrame> stackFrames = WalkCurrentStack(-1);
-		std::string stackTrace = FormatStackTrace(stackFrames);
-		WARN_LOG(Log::MemMap, "\n%s", stackTrace.c_str());
-
-		MIPSExceptionInfo &e = g_exceptionInfo;
-		e = {};
-		e.type = MIPSExceptionType::MEMORY;
-		e.info = additionalInfo;
-		e.memory_type = type;
-		e.address = address;
-		e.accessSize = accessSize;
-		e.stackTrace = stackTrace;
-		e.pc = pc;
-		Core_Break(BreakReason::MemoryException, address);
-	}
-}
-
-// Can't be ignored
+// Can't be ignored, must break. Not sure we can get a meaningful stack trace here (since the PC is invalid).
 void Core_ExecException(u32 address, u32 pc, ExecExceptionType type) {
 	const char *desc = ExecExceptionTypeAsString(type);
 	WARN_LOG(Log::MemMap, "%s: Invalid exec address %08x pc=%08x ra=%08x", desc, address, pc, currentMIPS->r[MIPS_REG_RA]);
@@ -625,15 +622,22 @@ void Core_ExecException(u32 address, u32 pc, ExecExceptionType type) {
 }
 
 void Core_BreakException(u32 pc) {
-	ERROR_LOG(Log::CPU, "CPU exception: break instruction hit at %08x", pc);
-
 	MIPSExceptionInfo &e = g_exceptionInfo;
 	e = {};
 	e.type = MIPSExceptionType::BREAK;
 	e.info.clear();
 	e.pc = pc;
 
-	if (!g_Config.bIgnoreBadMemAccess) {
+	const ExceptionAction action = ResolveExceptionAction((ExceptionAction)g_Config.iExceptionActionBreak);
+	if (action == ExceptionAction::Ignore) {
+		// Simplest logging and continue.
+		WARN_LOG(Log::CPU, "CPU exception: break instruction hit at %08x. Ignoring (use --break=log for more details or --break=break to break)", pc);
+		return;
+	}
+
+	const std::string stackTrace = FormatStackTrace(WalkCurrentStack(-1));
+	ERROR_LOG(Log::CPU, "CPU exception: break instruction hit at %08x (ra=%08x)\n%s", pc, currentMIPS->r[MIPS_REG_RA], stackTrace.c_str());
+	if (action == ExceptionAction::Break) {
 		Core_Break(BreakReason::BreakInstruction, currentMIPS->pc);
 	}
 }

Core/Core.h

@@ -22,6 +22,7 @@
 #include <string_view>
 
 #include "Common/CommonTypes.h"
+#include "Core/ConfigValues.h"
 
 class GraphicsContext;
 
@@ -166,11 +167,7 @@ enum class ExecExceptionType {
 	THREAD,
 };
 
-// Separate one for without info, to avoid having to allocate a string
-void Core_MemoryException(u32 address, u32 accessSize, u32 pc, MemoryExceptionType type);
-
-void Core_MemoryExceptionInfo(u32 address, u32 accessSize, u32 pc, MemoryExceptionType type, std::string_view additionalInfo, bool forceReport);
-
+void Core_MemoryException(u32 address, u32 accessSize, u32 pc, MemoryExceptionType type, std::string_view additionalInfo = "", bool forceReport = false);
 void Core_ExecException(u32 address, u32 pc, ExecExceptionType type);
 void Core_BreakException(u32 pc);
 // Call when loading save states, etc.

Core/MemFault.cpp

@@ -319,7 +319,7 @@ bool HandleFault(uintptr_t hostAddress, void *ctx) {
 		uint32_t approximatePC = currentMIPS->pc;
 		// TODO: Determine access size from the disassembled native instruction. We have some partial info already,
 		// just need to clean it up.
-		Core_MemoryExceptionInfo(guestAddress, 0, approximatePC, type, infoString, true);
+		Core_MemoryException(guestAddress, 0, approximatePC, type, infoString, true);
 
 		// There's a small chance we can resume from this type of crash.
 		g_lastCrashAddress = codePtr;