Improve semantics

Core/CwCheat.cpp

@@ -919,7 +919,7 @@ void CWCheatEngine::ExecuteOp(const CheatOperation &op, const CheatCode &cheat,
 					float f;
 					uint32_t u;
 				} value;
-				value.u = Memory::Read_U32(op.addr);
+				value.u = Memory::ReadUnchecked_U32(op.addr);  // we check the range above
 				std::string shaderName = shaderChain[op.PostShaderUniform.shader]->section;
 				switch (op.PostShaderUniform.format) {
 				case 0:
@@ -1035,8 +1035,11 @@ void CWCheatEngine::ExecuteOp(const CheatOperation &op, const CheatCode &cheat,
 
 	case CheatOp::CwCheatPointerCommands:
 		{
+			if (!Memory::IsValidAddress(op.addr + op.pointerCommands.baseOffset)) {
+				break;
+			}
 			InvalidateICache(op.addr + op.pointerCommands.baseOffset, 4);  // See note at top of file
-			u32 base = Memory::Read_U32(op.addr + op.pointerCommands.baseOffset);
+			u32 base = Memory::ReadUnchecked_U32(op.addr + op.pointerCommands.baseOffset);
 			u32 val = op.val;
 			int type = op.pointerCommands.type;
 			for (int a = 0; a < op.pointerCommands.count; ++a) {

Core/Debugger/DisassemblyManager.cpp

@@ -790,7 +790,12 @@ void DisassemblyData::createLines()
 	lineAddresses.clear();
 
 	u32 pos = address;
-	const u32 end = address+size;
+	const u32 end = address + size;
+
+	if (!Memory::IsValidRange(address, size)) {
+		ERROR_LOG(Log::CPU, "DisassemblyData can't create lines for invalid range 0x%08X-0x%08X", address, end);
+	}
+
 	const u32 maxChars = g_disassemblyManager.getMaxParamChars();
 	
 	std::string currentLine;
@@ -802,7 +807,7 @@ void DisassemblyData::createLines()
 		bool inString = false;
 		while (pos < end)
 		{
-			u8 b = Memory::Read_U8(pos++);
+			u8 b = Memory::ReadUnchecked_U8(pos++);
 			if (b >= 0x20 && b <= 0x7F)
 			{
 				if (currentLine.size()+1 >= maxChars)
@@ -879,18 +884,18 @@ void DisassemblyData::createLines()
 			switch (type)
 			{
 			case DATATYPE_BYTE:
-				value = Memory::Read_U8(pos);
+				value = Memory::ReadUnchecked_U8(pos);
 				snprintf(buffer, sizeof(buffer), "0x%02X", value);
 				pos++;
 				break;
 			case DATATYPE_HALFWORD:
-				value = Memory::Read_U16(pos);
+				value = Memory::ReadUnchecked_U16(pos);
 				snprintf(buffer, sizeof(buffer), "0x%04X", value);
 				pos += 2;
 				break;
 			case DATATYPE_WORD:
 				{
-					value = Memory::Read_U32(pos);
+					value = Memory::ReadUnchecked_U32(pos);
 					const std::string label = g_symbolMap->GetLabelString(value);
 					if (!label.empty())
 						snprintf(buffer, sizeof(buffer), "%s", label.c_str());

Core/Debugger/WebSocket/MemorySubscriber.cpp

@@ -182,7 +182,7 @@ void WebSocketMemoryReadU32(DebuggerRequest &req) {
 	Core_RunOnCPUThread([&] {
 		AutoDisabledReplacements memLock = LockMemory(true);
 		JsonWriter &json = req.Respond();
-		json.writeUint("value", Memory::Read_U32(addr));
+		json.writeUint("value", Memory::ReadUnchecked_U32(addr));
 	});
 }
 
@@ -334,7 +334,7 @@ void WebSocketMemoryWriteU8(DebuggerRequest &req) {
 // Write two bytes to memory (memory.write_u16)
 //
 // Parameters:
-//  - address: unsigned integer
+//  - address: unsigned integer (can be unaligned! But not recommended. Should maybe disallow).
 //  - value: unsigned integer
 //
 // Response (same event name):
@@ -352,7 +352,7 @@ void WebSocketMemoryWriteU16(DebuggerRequest &req) {
 		return req.Fail("CPU not started");
 	// This only depends on addr, not on anything CPU-thread-owned, so fail fast here rather than
 	// making a round trip through the queue for a request we already know is invalid.
-	if (!Memory::IsValidAddress(addr))
+	if (!Memory::IsValidRange(addr, 2))
 		return req.Fail("Invalid address");
 
 	// Route the actual memory write to the CPU thread instead of poking at it directly
@@ -360,7 +360,7 @@ void WebSocketMemoryWriteU16(DebuggerRequest &req) {
 	Core_RunOnCPUThread([&] {
 		AutoDisabledReplacements memLock = LockMemory(true);
 		currentMIPS->InvalidateICache(addr, 2);
-		Memory::Write_U16(val, addr);
+		Memory::WriteUnchecked_U16(val, addr);
 		Reporting::NotifyDebugger();
 
 		JsonWriter &json = req.Respond();
@@ -371,7 +371,7 @@ void WebSocketMemoryWriteU16(DebuggerRequest &req) {
 // Write four bytes to memory (memory.write_u32)
 //
 // Parameters:
-//  - address: unsigned integer
+//  - address: unsigned integer (can be unaligned! But not recommended. Should maybe disallow).
 //  - value: unsigned integer
 //
 // Response (same event name):
@@ -389,7 +389,7 @@ void WebSocketMemoryWriteU32(DebuggerRequest &req) {
 		return req.Fail("CPU not started");
 	// This only depends on addr, not on anything CPU-thread-owned, so fail fast here rather than
 	// making a round trip through the queue for a request we already know is invalid.
-	if (!Memory::IsValidAddress(addr))
+	if (!Memory::IsValidRange(addr, 4))
 		return req.Fail("Invalid address");
 
 	// Route the actual memory write to the CPU thread instead of poking at it directly
@@ -397,11 +397,11 @@ void WebSocketMemoryWriteU32(DebuggerRequest &req) {
 	Core_RunOnCPUThread([&] {
 		AutoDisabledReplacements memLock = LockMemory(true);
 		currentMIPS->InvalidateICache(addr, 4);
-		Memory::Write_U32(val, addr);
+		Memory::WriteUnchecked_U32(val, addr);
 		Reporting::NotifyDebugger();
 
 		JsonWriter &json = req.Respond();
-		json.writeUint("value", Memory::Read_U32(addr));
+		json.writeUint("value", Memory::ReadUnchecked_U32(addr));
 	});
 }
 

Core/Dialog/PSPGamedataInstallDialog.cpp

@@ -43,25 +43,16 @@ const u32 PSP_UTILITY_GAMEDATA_MODE_SHOW_PROGRESS = 1;
 
 static const std::string SFO_FILENAME = "PARAM.SFO";
 
-namespace
-{
-	std::vector<std::string> GetPSPFileList (const std::string &dirpath) {
-		std::vector<std::string> FileList;
-		auto Fileinfos = pspFileSystem.GetDirListing(dirpath);
-		FileList.reserve(Fileinfos.size());
+static std::vector<std::string> GetPSPFileList(std::string_view dirpath) {
+	std::vector<std::string> FileList;
+	auto Fileinfos = pspFileSystem.GetDirListing(dirpath);
+	FileList.reserve(Fileinfos.size());
 
-		for (auto it = Fileinfos.begin(); it != Fileinfos.end(); ++it) {
-			std::string info = (*it).name;
-			FileList.push_back(info);
-		}
-		return FileList;
+	for (auto it = Fileinfos.begin(); it != Fileinfos.end(); ++it) {
+		std::string info = (*it).name;
+		FileList.push_back(info);
 	}
-}
-
-PSPGamedataInstallDialog::PSPGamedataInstallDialog(UtilityDialogType type) : PSPDialog(type) {
-}
-
-PSPGamedataInstallDialog::~PSPGamedataInstallDialog() {
+	return FileList;
 }
 
 int PSPGamedataInstallDialog::Init(u32 paramAddr) {
@@ -70,6 +61,12 @@ int PSPGamedataInstallDialog::Init(u32 paramAddr) {
 		return SCE_ERROR_UTILITY_INVALID_STATUS;
 	}
 
+	if (!Memory::IsValidRange(paramAddr, sizeof(SceUtilityGamedataInstallParam))) {
+		// This should probably crash
+		ERROR_LOG(Log::sceUtility, "sceGamedataInstallInitStart: invalid param address 0x%08X", paramAddr);
+		return SCE_KERNEL_ERROR_INVALID_POINTER;
+	}
+
 	param.ptr = paramAddr;
 	inFileNames = GetPSPFileList("disc0:/PSP_GAME/INSDIR");
 	numFiles = (int)inFileNames.size();
@@ -90,7 +87,7 @@ int PSPGamedataInstallDialog::Init(u32 paramAddr) {
 		return -1;
 	}
 
-	int size = Memory::Read_U32(paramAddr);
+	const int size = Memory::ReadUnchecked_U32(paramAddr);
 	if (size != 1424 && size != 1432) {
 		ERROR_LOG_REPORT(Log::sceUtility, "sceGamedataInstallInitStart: invalid param size %d", size);
 		return SCE_ERROR_UTILITY_INVALID_PARAM_SIZE;

Core/Dialog/PSPGamedataInstallDialog.h

@@ -35,8 +35,7 @@ struct SceUtilityGamedataInstallParam {
 
 class PSPGamedataInstallDialog: public PSPDialog {
 public:
-	PSPGamedataInstallDialog(UtilityDialogType type);
-	~PSPGamedataInstallDialog();
+	PSPGamedataInstallDialog(UtilityDialogType type) : PSPDialog(type) {}
 
 	int Init(u32 paramAddr);
 	int Update(int animSpeed) override;

Core/Dialog/PSPMsgDialog.cpp

@@ -57,12 +57,14 @@ int PSPMsgDialog::Init(unsigned int paramAddr) {
 	}
 
 	messageDialogAddr = paramAddr;
-	if (!Memory::IsValidAddress(messageDialogAddr))
-	{
-		return 0;
+
+	if (!Memory::IsValid4AlignedAddress(paramAddr)) {
+		// What to do?
+		return SCE_KERNEL_ERROR_BAD_ARGUMENT;
 	}
-	int size = Memory::Read_U32(paramAddr);
-	memset(&messageDialog,0,sizeof(messageDialog));
+
+	int size = Memory::ReadUnchecked_U32(paramAddr);
+	memset(&messageDialog, 0, sizeof(messageDialog));
 	// Only copy the right size to support different request format
 	Memory::Memcpy(&messageDialog,paramAddr,size);
 

Core/Dialog/PSPNetconfDialog.cpp

@@ -59,16 +59,21 @@ int PSPNetconfDialog::Init(u32 paramAddr) {
 	if (ReadStatus() != SCE_UTILITY_STATUS_NONE)
 		return SCE_ERROR_UTILITY_INVALID_STATUS;
 
+	if (!Memory::IsValid4AlignedRange(paramAddr, sizeof(request))) {
+		// What to do?
+		return SCE_KERNEL_ERROR_BAD_ARGUMENT;
+	}
+
 	NOTICE_LOG(Log::sceUtility, "PSPNetConfDialog Init");
 	jsonReady_ = false;
 	// Kick off a request to the infra-dns.json since we'll need it later.
 	StartInfraJsonDownload();
 
 	requestAddr = paramAddr;
-	int size = Memory::Read_U32(paramAddr);
+	const u32 size = Memory::ReadUnchecked_U32(paramAddr);
 	memset(&request, 0, sizeof(request));
-	// Only copy the right size to support different request format
-	Memory::Memcpy(&request, paramAddr, size);
+	// Only copy the right size (bounded by the struct) to support different request format
+	Memory::Memcpy(&request, paramAddr, std::min(size, (u32)sizeof(request)));
 
 	ChangeStatusInit(NET_INIT_DELAY_US);
 

Core/HLE/sceMpeg.cpp

@@ -1642,7 +1642,7 @@ static int sceMpegGetAvcAu(u32 mpeg, u32 streamId, u32 auAddr, u32 attrAddr)
 	avcAu.write(auAddr);
 
 	if (result == 0) {
-		// Jeanne d'Arc return 00000000 as attrAddr here and cause WriteToHardware error
+		// Jeanne d'Arc return 00000000 as attrAddr here and cause WriteMemoryOrRaiseException error
 		if (Memory::IsValidAddress(attrAddr)) {
 			Memory::Write_U32(1, attrAddr);
 		}
@@ -1742,7 +1742,7 @@ static int sceMpegGetAtracAu(u32 mpeg, u32 streamId, u32 auAddr, u32 attrAddr)
 	atracAu.write(auAddr);
 
 	if (result == 0) {
-		// 3rd birthday return 00000000 as attrAddr here and cause WriteToHardware error
+		// 3rd birthday return 00000000 as attrAddr here and cause WriteMemoryOrRaiseException error
 		if (Memory::IsValidAddress(attrAddr)) {
 			Memory::Write_U32(0, attrAddr);
 		}

Core/MemMapFunctions.cpp

@@ -86,7 +86,7 @@ const u8 *GetPointerRange(const u32 address, const u32 size) {
 }
 
 template <typename T>
-inline void ReadFromHardware(T &var, const u32 address) {
+inline void ReadMemoryOrRaiseException(T &var, const u32 address) {
 	if ((address & 0x3E000000) == 0x08000000 || // RAM
 		(address & 0xBF800000) == 0x04000000 || // VRAM
 		(address & 0xBFFFC000) == 0x00010000 || // Scratchpad
@@ -99,7 +99,7 @@ inline void ReadFromHardware(T &var, const u32 address) {
 }
 
 template <typename T>
-inline void WriteToHardware(u32 address, const T data) {
+inline void WriteMemoryOrRaiseException(u32 address, const T data) {
 	if ((address & 0x3E000000) == 0x08000000 || // RAM
 		(address & 0xBF800000) == 0x04000000 || // VRAM
 		(address & 0xBFFFC000) == 0x00010000 || // Scratchpad
@@ -126,25 +126,25 @@ bool IsScratchpadAddress(const u32 address) {
 
 u8 Read_U8(const u32 address) {
 	u8 value = 0;
-	ReadFromHardware<u8>(value, address);
+	ReadMemoryOrRaiseException<u8>(value, address);
 	return (u8)value;
 }
 
 u16 Read_U16(const u32 address) {
 	u16_le value = 0;
-	ReadFromHardware<u16_le>(value, address);
+	ReadMemoryOrRaiseException<u16_le>(value, address);
 	return (u16)value;
 }
 
 u32 Read_U32(const u32 address) {
 	u32_le value = 0;
-	ReadFromHardware<u32_le>(value, address);
+	ReadMemoryOrRaiseException<u32_le>(value, address);
 	return value;
 }
 
 u64 Read_U64(const u32 address) {
 	u64_le value = 0;
-	ReadFromHardware<u64_le>(value, address);
+	ReadMemoryOrRaiseException<u64_le>(value, address);
 	return value;
 }
 
@@ -157,19 +157,19 @@ u32 Read_U16_ZX(const u32 address) {
 }
 
 void Write_U8(const u8 _Data, const u32 address) {
-	WriteToHardware<u8>(address, _Data);
+	WriteMemoryOrRaiseException<u8>(address, _Data);
 }
 
 void Write_U16(const u16 _Data, const u32 address) {
-	WriteToHardware<u16_le>(address, _Data);
+	WriteMemoryOrRaiseException<u16_le>(address, _Data);
 }
 
 void Write_U32(const u32 _Data, const u32 address) {
-	WriteToHardware<u32_le>(address, _Data);
+	WriteMemoryOrRaiseException<u32_le>(address, _Data);
 }
 
 void Write_U64(const u64 _Data, const u32 address) {
-	WriteToHardware<u64_le>(address, _Data);
+	WriteMemoryOrRaiseException<u64_le>(address, _Data);
 }
 
 }	// namespace Memory