Merge pull request #21976 from hrydgard/misc-changes

Better error messages for unresolved syscalls, etc.

AGENTS.md

@@ -45,6 +45,30 @@ when making cross platform changes.
 New unit tests are added by listing them in availableTests in unittest.cpp. If they are large, put them in
 separate files in the unittest subdirectory. Remember to update both CMakeLists.txt and the visual studio project.
 
+## Adding HLE modules
+
+HLE module implementations live in `Core/HLE/sce<ModuleName>.cpp` / `.h` (e.g. `sceOpenPSID.cpp`, `scePauth.cpp` are good
+small examples to copy from). A module is a `const HLEFunction <name>[]` table of
+`{nid, &WrapX_YYY<func>, "funcName", retChar, argString}` entries, registered via
+`RegisterHLEModule("<name>", ARRAY_SIZE(table), table)` inside a `Register_<name>()` function declared in the header.
+
+- `FunctionWrappers.h` has generic `WrapX_YYY<func>` templates for common signatures (return type X, args YYY) - check
+  there before writing a manual wrapper.
+- Format string legend for the `retmask`/argmask chars: `x` = u32 (shown as hex), `i` = int/s32, `f` = float, `X` = u64,
+  `I` = s64, `v` = void.
+- For functions of genuinely unknown purpose (only known by NID), name them `<moduleName>_<NID>` and stub them with
+  `return hleLogError(Log::HLE, 0, "UNIMPL");` - an established pattern (see `scePauth.cpp`, `sceOpenPSID.cpp`).
+- **New modules must be registered at the very end** of the registration function in `Core/HLE/HLETables.cpp` (look for
+  the `// add new modules here.` comment near the end of that function) - not inserted alphabetically/logically among
+  the existing `Register_*()` calls. Module registration order affects numeric IDs used in savestates, so inserting a
+  new module earlier in that list would break save-state compatibility for saves made with older builds.
+- Remember to add any new `.cpp`/`.c` file to **five** places: `Core/CMakeLists.txt`, `Core/Core.vcxproj`,
+  `Core/Core.vcxproj.filters`, `android/jni/Android.mk`, and `libretro/Makefile.common`. New `.h` files only need the
+  first three (`Android.mk`/`Makefile.common` are plain compiled-source lists, not project generators, so headers
+  don't go in them). Only the CMakeLists.txt change can be verified from a Linux/Mac build - the rest can't be
+  build-tested here, so double check them by hand against how an existing neighboring file (e.g. `sceVaudio.cpp`) is
+  listed in each.
+
 ## Quick rebuild on Linux
 
 You don't need to do ./b.sh --debug to verify every single little change, instead use this shortcut:

Core/HLE/HLE.cpp

@@ -39,6 +39,7 @@
 #include "Core/HLE/ErrorCodes.h"
 #include "Core/HLE/sceKernelThread.h"
 #include "Core/HLE/sceKernelInterrupt.h"
+#include "Core/HLE/sceKernelModule.h"
 #include "Core/HLE/HLE.h"
 
 enum {
@@ -906,7 +907,16 @@ const HLEFunction *GetSyscallFuncPointer(MIPSOpcode op) {
 	int modulenum = (callno & 0xFF000) >> 12;
 	if (funcnum == 0xfff) {
 		std::string_view modName = modulenum >= (int)moduleDB.size() ? "(unknown)" : moduleDB[modulenum].name;
-		ERROR_LOG(Log::HLE, "Unknown syscall: Module: '%.*s' (module: %d func: %d)", (int)modName.size(), modName.data(), modulenum, funcnum);
+		// This is what a still-unresolved import looks like once written as a syscall opcode -
+		// the original module name/NID aren't recoverable from the opcode itself (see
+		// WriteFuncMissingStub), but the calling address is a stub we may still be tracking.
+		std::string importModuleName, importingModuleName;
+		u32 nid = 0;
+		if (currentMIPS->pc >= 8 && KernelFindImportByStubAddr(currentMIPS->pc - 8, &importModuleName, &nid, &importingModuleName)) {
+			ERROR_LOG(Log::HLE, "Unknown syscall: unresolved import %s/%08x (%s), called from '%s'", importModuleName.c_str(), nid, GetHLEFuncName(importModuleName, nid), importingModuleName.c_str());
+		} else {
+			ERROR_LOG(Log::HLE, "Unknown syscall: Module: '%.*s' (module: %d func: %d)", (int)modName.size(), modName.data(), modulenum, funcnum);
+		}
 		return NULL;
 	}
 	if (modulenum >= (int)moduleDB.size()) {

Core/HLE/sceKernelMemory.cpp

@@ -391,14 +391,14 @@ void __KernelMemoryShutdown()
 
 BlockAllocator *BlockAllocatorFromID(int id) {
 	switch (id) {
-	case 1:
+	case KERNEL_PARTITION_ID:
 	case 3:
 	case 4:
 		if (hleIsKernelMode())
 			return &kernelMemory;
 		return nullptr;
 
-	case 2:
+	case USER_PARTITION_ID:
 	case 6:
 		return &userMemory;
 
@@ -408,7 +408,7 @@ BlockAllocator *BlockAllocatorFromID(int id) {
 			return &userMemory;
 		return nullptr;
 
-	case 5:
+	case VSHELL_PARTITION_ID:
 		return &volatileMemory;
 
 	default:
@@ -420,11 +420,11 @@ BlockAllocator *BlockAllocatorFromID(int id) {
 
 int BlockAllocatorToID(const BlockAllocator *alloc) {
 	if (alloc == &kernelMemory)
-		return 1;
+		return KERNEL_PARTITION_ID;
 	if (alloc == &userMemory)
-		return 2;
+		return USER_PARTITION_ID;
 	if (alloc == &volatileMemory)
-		return 5;
+		return VSHELL_PARTITION_ID;
 	return 0;
 }
 

Core/HLE/sceKernelMemory.h

@@ -32,6 +32,16 @@ enum MemblockType {
 	PSP_SMEM_HighAligned = 4,
 };
 
+// Memory partition IDs, as passed to sceKernelAllocPartitionMemory() and friends.
+// Per TyRaNiD: 1 = kernel, 2 = user, 3 = me, 4 = kernel mirror. 5 is the "volatile"
+// partition also used by sceKernelVolatileMemLock() - the VSH uses this partition
+// for its own private allocations while it's running.
+enum {
+	KERNEL_PARTITION_ID = 1,
+	USER_PARTITION_ID = 2,
+	VSHELL_PARTITION_ID = 5,
+};
+
 extern BlockAllocator userMemory;
 extern BlockAllocator kernelMemory;
 

Core/HLE/sceKernelModule.cpp

@@ -766,6 +766,30 @@ void UnexportFuncSymbol(const FuncSymbolExport &func) {
 	}
 }
 
+// Used to add detail to the "Unknown syscall" log in HLE.cpp's GetSyscallFuncPointer - a call
+// through a still-unresolved import ends up as a generic "invalid syscall" opcode that no
+// longer carries the original module name/NID, but the (fixed, unique) address of the syscall
+// instruction itself does - it's exactly the stubAddr every pending FuncSymbolImport recorded
+// when it was written by WriteFuncMissingStub.
+bool KernelFindImportByStubAddr(u32 stubAddr, std::string *importModuleName, u32 *nid, std::string *importingModuleName) {
+	u32 error;
+	for (SceUID moduleId : loadedModules) {
+		PSPModule *module = kernelObjects.Get<PSPModule>(moduleId, error);
+		if (!module) {
+			continue;
+		}
+		for (const auto &func : module->importedFuncs) {
+			if (func.stubAddr == stubAddr) {
+				*importModuleName = func.moduleName;
+				*nid = func.nid;
+				*importingModuleName = module->GetName();
+				return true;
+			}
+		}
+	}
+	return false;
+}
+
 void PSPModule::Cleanup() {
 	MIPSAnalyst::ForgetFunctions(textStart, textEnd);
 
@@ -1111,10 +1135,17 @@ static PSPModule *__KernelLoadELFFromPtr(const u8 *ptr, size_t elfSize, u32 load
 			_assert_msg_(temp != nullptr, "Failed to allocate gzip decompression buffer (decryptedSize: %d)", decryptedSize);
 			memcpy(temp, ptr, decryptedSize);
 			int outBytes = gzipDecompress((u8 *)ptr, maxElfSize, temp);
-			if (outBytes < 0) {
-				ERROR_LOG(Log::sceModule, "Module gzip decompression failed!");
-			}
 			free(temp);
+			if (outBytes < 0) {
+				// Not necessarily actually gzip - some kd/ system modules (and possibly VSH
+				// modules) use KL4E compression instead, which we don't support decompressing.
+				// Bail out cleanly here rather than falling through to parse whatever's left
+				// in the buffer (still compressed, not a valid ELF) as if it were real code.
+				*error_string = StringFromFormat("Module '%s' decompression failed", head->modname);
+				// TODO: Might be the wrong error code.
+				error = SCE_KERNEL_ERROR_FILEERR;
+				return nullptr;
+			}
 			INFO_LOG(Log::sceModule, "gzip is enabled in '%s', decompressing (%d -> %d bytes, bufmax=%d).", head->modname, decryptedSize, outBytes, maxElfSize);
 		}
 

Core/HLE/sceKernelModule.h

@@ -228,6 +228,7 @@ u32 __KernelGetModuleGP(SceUID module);
 bool KernelModuleIsKernelMode(SceUID module);
 bool __KernelLoadGEDump(std::string_view base_filename, std::string *error_string);
 bool __KernelLoadExec(const char *filename, u32 paramPtr, std::string *error_string);
+bool KernelFindImportByStubAddr(u32 stubAddr, std::string *importModuleName, u32 *nid, std::string *importingModuleName);
 int __KernelGPUReplay();
 void __KernelReturnFromModuleFunc();
 SceUID KernelLoadModule(const std::string &filename, std::string *error_string);

Core/HLE/sceNetAdhocMatching.cpp

@@ -1907,10 +1907,8 @@ int NetAdhocMatching_Start(int matchingId, int evthPri, int evthPartitionId, int
 	return hleLogDebug(Log::sceNet, 0);
 }
 
-#define KERNEL_PARTITION_ID  1
-#define USER_PARTITION_ID  2
-#define VSHELL_PARTITION_ID  5
 // This should be similar with sceNetAdhocMatchingStart2 but using USER_PARTITION_ID (2) for PartitionId params
+// (KERNEL_PARTITION_ID/USER_PARTITION_ID/VSHELL_PARTITION_ID come from Core/HLE/sceKernelMemory.h)
 static int sceNetAdhocMatchingStart(int matchingId, int evthPri, int evthStack, int inthPri, int inthStack, int optLen, u32 optDataAddr) {
 	WARN_LOG(Log::sceNet, "UNTESTED sceNetAdhocMatchingStart(%i, %i, %i, %i, %i, %i, %08x) at %08x", matchingId, evthPri, evthStack, inthPri, inthStack, optLen, optDataAddr, currentMIPS->pc);
 	if (!g_Config.bEnableWlan) {

Core/MemFault.cpp

@@ -296,7 +296,12 @@ bool HandleFault(uintptr_t hostAddress, void *ctx) {
 		context->CTX_PC += info.instructionSize;
 		g_numReportedBadAccesses++;
 		if (g_numReportedBadAccesses < 100) {
-			ERROR_LOG(Log::MemMap, "Bad memory access detected and ignored: %08x (%p)", guestAddress, (void *)hostAddress);
+			std::string temp;
+			if (MIPSComp::jit && MIPSComp::jit->DescribeCodePtr(codePtr, temp)) {
+				ERROR_LOG(Log::MemMap, "Bad memory access detected and ignored: %08x (%p) at %s", guestAddress, (void *)hostAddress, temp.c_str());
+			} else {
+				ERROR_LOG(Log::MemMap, "Bad memory access detected and ignored: %08x (%p)", guestAddress, (void *)hostAddress);
+			}
 		}
 	} else {
 		std::string infoString = "";