From fb8c99ad49d27bbf16251ae853c92898509556f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Mon, 7 Sep 2026 10:18:26 -0600 Subject: [PATCH 1/3] sceIo: generate and resolve FAT 8.3 short names sceIoDread hands back a dirent whose d_private holds the 8.3 short name ahead of the long name, and we never wrote the short name at all - the game got whatever was on the stack there. Crazy Taxi: Fare Wars reads it rather than d_name, so it rejected every file in ms0:/MUSIC, ended up with an empty playlist and never even reserved an mp3 handle: custom soundtracks were silently dead, with the game spinning on InitResource/SetLoopNum forever. Generate the names from the directory listing, and resolve them back in DirectoryFileSystem so a game can open a file by the short name it was given. Both sides come from the same function, so they agree. We can't lean on the host for any of this. Linux, macOS and Android have no 8.3 names at all, and while Windows does keep aliases it generates them by a different rule - it counts to ~4 and then switches to a hash - so resolution runs before the literal path is tried rather than as a fallback, or on Windows we'd quietly open a different file than the one we handed the game. The exact names a real PSP produces are still unverified - no pspautotest covers d_private - so this implements the ordinary FAT rule and the new FatShortNames unit test pins that down until hardware can settle it. Co-Authored-By: Claude Opus 5 --- Core/FileSystems/DirectoryFileSystem.cpp | 59 ++++++++++- Core/FileSystems/DirectoryFileSystem.h | 7 ++ Core/FileSystems/FileSystem.cpp | 124 +++++++++++++++++++++++ Core/FileSystems/FileSystem.h | 5 + Core/HLE/sceIo.cpp | 18 +++- unittest/UnitTest.cpp | 60 +++++++++++ 6 files changed, 269 insertions(+), 4 deletions(-) diff --git a/Core/FileSystems/DirectoryFileSystem.cpp b/Core/FileSystems/DirectoryFileSystem.cpp index 0b64b8574e..6bc1fa57e9 100644 --- a/Core/FileSystems/DirectoryFileSystem.cpp +++ b/Core/FileSystems/DirectoryFileSystem.cpp @@ -612,14 +612,16 @@ int DirectoryFileSystem::RenameFile(const std::string &from, const std::string & } bool DirectoryFileSystem::RemoveFile(const std::string &filename) { - Path localPath = GetLocalPath(filename); + std::string resolved = filename; + ResolveShortNames(resolved); + Path localPath = GetLocalPath(resolved); bool retValue = File::Delete(localPath); if (flags & FileSystemFlags::CASE_SENSITIVE) { if (!retValue) { // May have failed due to case sensitivity, so try again. Try even if it fails? - std::string fullNamePath = filename; + std::string fullNamePath = resolved; if (!FixPathCase(basePath, fullNamePath, FPC_FILE_MUST_EXIST)) return (bool)ReplayApplyDisk(ReplayAction::FILE_REMOVE, false, CoreTiming::GetGlobalTimeUs()); localPath = GetLocalPath(fullNamePath); @@ -632,7 +634,53 @@ bool DirectoryFileSystem::RemoveFile(const std::string &filename) { return ReplayApplyDisk(ReplayAction::FILE_REMOVE, retValue, CoreTiming::GetGlobalTimeUs()) != 0; } +// Note that this runs *before* the literal path is tried, not as a fallback after it fails. That's +// deliberate: on Windows the host resolves its own 8.3 aliases, which are generated by a different +// rule than ours, so opening the literal name can quietly land on a different file than the one we +// handed the game in sceIoDread. Everywhere else the literal name simply wouldn't exist. +void DirectoryFileSystem::ResolveShortNames(std::string &path) { + // Only the memory stick is FAT, and only a name with a counter in it can be a short name, so + // ordinary paths cost one character scan and nothing more. + if (!(flags & FileSystemFlags::SIMULATE_FAT32) || path.find('~') == std::string::npos) { + return; + } + if (resolvingShortNames_) { + return; + } + resolvingShortNames_ = true; + + std::vector parts; + SplitString(path, '/', parts); + + std::string resolved; + for (std::string_view rawPart : parts) { + std::string part(rawPart); + if (part.find('~') != std::string::npos) { + bool exists = false; + std::vector listing = GetDirListing(resolved, &exists); + if (exists) { + std::vector shortNames; + GenerateFatShortNames(listing, &shortNames); + for (size_t i = 0; i < shortNames.size(); i++) { + if (equalsNoCase(shortNames[i], part)) { + part = listing[i].name; + break; + } + } + } + } + if (!resolved.empty()) { + resolved += "/"; + } + resolved += part; + } + + resolvingShortNames_ = false; + path = resolved; +} + int DirectoryFileSystem::OpenFile(std::string filename, FileAccess access, const char *devicename) { + ResolveShortNames(filename); OpenFileEntry entry; entry.hFile.fileSystemFlags_ = flags; u32 err = 0; @@ -761,6 +809,8 @@ static u32 PspAccessBits(bool isDirectory, bool isWritable) { } PSPFileInfo DirectoryFileSystem::GetFileInfo(std::string filename) { + ResolveShortNames(filename); + PSPFileInfo x; x.name = filename; @@ -894,6 +944,11 @@ bool DirectoryFileSystem::ComputeRecursiveDirSizeIfFast(const std::string &path, std::vector DirectoryFileSystem::GetDirListing(std::string_view path, bool *exists) { std::vector myVector; + // A game can open a subdirectory by its short name too. + std::string resolvedPath(path); + ResolveShortNames(resolvedPath); + path = resolvedPath; + std::vector files; Path localPath = GetLocalPath(path); const int flags = File::GETFILES_GETHIDDEN | File::GETFILES_GET_NAVIGATION_ENTRIES; diff --git a/Core/FileSystems/DirectoryFileSystem.h b/Core/FileSystems/DirectoryFileSystem.h index 6761d48e52..7d0a276397 100644 --- a/Core/FileSystems/DirectoryFileSystem.h +++ b/Core/FileSystems/DirectoryFileSystem.h @@ -105,6 +105,13 @@ private: FileSystemFlags flags; Path GetLocalPath(std::string_view internalPath) const; + + // Rewrites any FAT 8.3 short-name components of a guest path to the long names they were + // generated from, so a game that read a short name out of d_private can open the file by it. + void ResolveShortNames(std::string &path); + + // Guards ResolveShortNames against re-entering itself through GetDirListing. + bool resolvingShortNames_ = false; }; // VFSFileSystem: Ability to map in Android APK paths as well! Does not support all features, only meant for fonts. diff --git a/Core/FileSystems/FileSystem.cpp b/Core/FileSystems/FileSystem.cpp index 40b527f562..c45ac269a1 100644 --- a/Core/FileSystems/FileSystem.cpp +++ b/Core/FileSystems/FileSystem.cpp @@ -15,6 +15,10 @@ // Official git repository and contact information can be found at // https://github.com/hrydgard/ppsspp and http://www.ppsspp.org/. +#include +#include +#include + #include "Common/Serialize/Serializer.h" #include "Common/Serialize/SerializeFuncs.h" #include "Core/FileSystems/FileSystem.h" @@ -38,3 +42,123 @@ void PSPFileInfo::DoState(PointerWrap &p) { Do(p, sectorSize); } + +// FAT 8.3 short names. +// +// The PSP's memory stick is FAT, so every file there has both its long name and a generated 8.3 +// short name. sceIoDread returns the short name in the first bytes of d_private, ahead of the long +// name, and some games read that instead of d_name and then open files by it - Crazy Taxi: Fare +// Wars does, for its custom soundtracks, and silently finds no music at all without it. +// +// We generate the names ourselves and resolve them ourselves, rather than leaning on the host: +// * Linux, macOS and Android have no concept of 8.3 short names at all. +// * Windows does keep its own aliases, but generates them differently - it only counts up to ~4 +// and then switches to a hash (sample-15s-cbr-128kbps.mp3 becomes SA6615~1.MP3) - and 8.3 name +// creation can be disabled per volume, so it can't be relied on even there. +// +// TODO: The names a real PSP generates are unverified. There's no hardware test covering d_private +// (pspautotests io/directory checks d_name, size and attr only), so this implements the ordinary +// FAT rule - cleaned-up base, truncated, plus a ~N counter for collisions - which is what typical +// FAT drivers do. If a hardware test later shows the PSP numbering or truncation differs, this +// function is the only place that needs to change. +// +// One deliberate difference from real FAT: there the short name is written to disk when the file is +// created, so it's stable for the life of the file. We derive it from the directory listing, so +// adding or removing a file can renumber the ~N suffixes of the others. That only matters if a game +// holds a short name across a change to the directory, which nothing is expected to do. + +static bool IsValidShortNameChar(char c) { + if (c >= 'A' && c <= 'Z') + return true; + if (c >= '0' && c <= '9') + return true; + // The remaining characters FAT permits unescaped in a short name. + return strchr("$%'-_@~`!(){}^#&", c) != nullptr; +} + +// Uppercases and strips anything FAT wouldn't accept, reporting whether that lost information - +// which is what decides between using the name as-is and appending a ~N counter. +static std::string CleanShortNamePart(std::string_view part, size_t maxLen, bool *lossy) { + std::string out; + for (char c : part) { + if (c >= 'a' && c <= 'z') { + // Short names are case insensitive, so this on its own isn't a loss. + c = c - 'a' + 'A'; + } + if (c == ' ' || c == '.') { + // Dropped entirely rather than escaped, matching FAT. + *lossy = true; + continue; + } + if (!IsValidShortNameChar(c)) { + c = '_'; + *lossy = true; + } + if (out.size() >= maxLen) { + *lossy = true; + break; + } + out.push_back(c); + } + return out; +} + +void GenerateFatShortNames(const std::vector &listing, std::vector *shortNames) { + shortNames->clear(); + shortNames->reserve(listing.size()); + + std::set taken; + for (const PSPFileInfo &info : listing) { + // "." and ".." are their own short names and don't take part in numbering. + if (info.name == "." || info.name == "..") { + shortNames->push_back(info.name); + continue; + } + + // Split off the extension at the last dot. A leading dot is part of the name, not an + // extension separator, so ".hidden" has no extension. + size_t dot = info.name.find_last_of('.'); + bool lossy = false; + std::string_view baseIn = info.name; + std::string_view extIn; + if (dot != std::string::npos && dot != 0) { + baseIn = std::string_view(info.name).substr(0, dot); + extIn = std::string_view(info.name).substr(dot + 1); + } else if (dot == 0) { + lossy = true; + } + + std::string base = CleanShortNamePart(baseIn, 8, &lossy); + std::string ext = CleanShortNamePart(extIn, 3, &lossy); + if (base.empty()) { + base = "_"; + lossy = true; + } + + std::string candidate; + if (!lossy) { + candidate = ext.empty() ? base : base + "." + ext; + // A name that needs no mangling can still collide, since short names ignore case. + if (taken.find(candidate) != taken.end()) { + candidate.clear(); + } + } + + for (int n = 1; candidate.empty(); n++) { + char suffix[16]; + snprintf(suffix, sizeof(suffix), "~%d", n); + // The counter has to fit inside the eight characters along with the stem. + std::string stem = base.substr(0, std::max((size_t)1, 8 - strlen(suffix))); + std::string attempt = stem + suffix; + if (!ext.empty()) { + attempt += "." + ext; + } + if (taken.find(attempt) == taken.end()) { + candidate = attempt; + } + } + + taken.insert(candidate); + shortNames->push_back(candidate); + } +} diff --git a/Core/FileSystems/FileSystem.h b/Core/FileSystems/FileSystem.h index 20cc3b0149..4e59d3811a 100644 --- a/Core/FileSystems/FileSystem.h +++ b/Core/FileSystems/FileSystem.h @@ -131,6 +131,11 @@ struct PSPFileInfo { u32 sectorSize = 0; }; +// Generates the FAT 8.3 short names for a directory listing, one per entry and in the same order. +// Games read these out of d_private in sceIoDread and may then open files by them - see the long +// comment in FileSystem.cpp, including what's still unverified against hardware. +void GenerateFatShortNames(const std::vector &listing, std::vector *shortNames); + class IFileSystem { public: diff --git a/Core/HLE/sceIo.cpp b/Core/HLE/sceIo.cpp index 293b388ca0..aac4d990a9 100644 --- a/Core/HLE/sceIo.cpp +++ b/Core/HLE/sceIo.cpp @@ -2399,6 +2399,16 @@ public: static int GetStaticIDType() { return PPSSPP_KERNEL_TMID_DirList; } int GetIDType() const override { return PPSSPP_KERNEL_TMID_DirList; } + // The FAT short name for an entry, which games read out of d_private. Derived from the listing + // rather than stored, so it needs no savestate of its own - it's rebuilt on first use, which + // includes after loading a state. + const std::string &ShortName(int i) { + if (shortNames_.size() != listing.size()) { + GenerateFatShortNames(listing, &shortNames_); + } + return shortNames_[i]; + } + void DoState(PointerWrap &p) override { auto s = p.Section("DirListing", 1); if (!s) @@ -2419,6 +2429,9 @@ public: std::string name; std::vector listing; int index; + +private: + std::vector shortNames_; }; static u32 sceIoDopen(const char *path) { @@ -2530,6 +2543,7 @@ static u32 sceIoDread(int id, u32 dirent_addr) { bool isFAT = pspFileSystem.FlagsFromFilename(dir->name) & FileSystemFlags::SIMULATE_FAT32; // Only write d_private for memory stick if (isFAT) { + const std::string &shortName = dir->ShortName(dir->index); // All files look like they're executable on FAT. This is required for Beats, see issue #14812 entry->d_stat.st_mode |= 0111; // write d_private for supporting Custom BGM @@ -2540,7 +2554,7 @@ static u32 sceIoDread(int id, u32 dirent_addr) { // - [0..12] "8.3" file name (null-terminated), could be empty. // - [13..???] long file name (null-terminated) - // Hm, so currently we don't write the short name at all to d_private? TODO + strcpy_limit((char*)Memory::GetPointerUnchecked(entry->d_private), shortName.c_str(), 13); strcpy_limit((char*)Memory::GetPointerUnchecked(entry->d_private + 13), (const char*)entry->d_name, ARRAY_SIZE(entry->d_name)); } else { @@ -2549,8 +2563,8 @@ static u32 sceIoDread(int id, u32 dirent_addr) { // - [4..19] "8.3" file name (null-terminated), could be empty. // - [20..???] long file name (null-terminated) auto size = Memory::ReadUnchecked_U32(entry->d_private); - // Hm, so currently we don't write the short name at all to d_private? TODO if (size >= 1044) { + strcpy_limit((char*)Memory::GetPointerUnchecked(entry->d_private + 4), shortName.c_str(), 16); strcpy_limit((char*)Memory::GetPointerUnchecked(entry->d_private + 20), (const char*)entry->d_name, ARRAY_SIZE(entry->d_name)); } } diff --git a/unittest/UnitTest.cpp b/unittest/UnitTest.cpp index 9b9913cdb7..9086c598f1 100644 --- a/unittest/UnitTest.cpp +++ b/unittest/UnitTest.cpp @@ -103,6 +103,7 @@ #include "Common/UI/View.h" #include "Common/UI/ViewGroup.h" #include "Core/Debugger/MemBlockInfo.h" +#include "Core/FileSystems/FileSystem.h" #include "Core/FileSystems/ISOFileSystem.h" #include "Core/MemMap.h" #include "Core/KeyMap.h" @@ -2897,6 +2898,64 @@ bool TestZipSlip(); bool TestLzrc(); bool TestDemangle(); +// The 8.3 short names games read out of d_private. These aren't verified against hardware yet (no +// pspautotest covers d_private), so this pins down the behavior we chose - notably that the counter +// keeps going past ~4 rather than switching to a hash the way Windows does. +bool TestFatShortNames() { + auto shortNamesFor = [](const std::vector &names) { + std::vector listing; + for (const std::string &name : names) { + PSPFileInfo info; + info.name = name; + listing.push_back(info); + } + std::vector shortNames; + GenerateFatShortNames(listing, &shortNames); + return shortNames; + }; + + // Names that already fit 8.3 are only uppercased, and the navigation entries are left alone. + std::vector plain = shortNamesFor({".", "..", "TEST.TXT", "readme.md", "WIPEOUT"}); + EXPECT_EQ_STR(plain[0], std::string(".")); + EXPECT_EQ_STR(plain[1], std::string("..")); + EXPECT_EQ_STR(plain[2], std::string("TEST.TXT")); + EXPECT_EQ_STR(plain[3], std::string("README.MD")); + EXPECT_EQ_STR(plain[4], std::string("WIPEOUT")); + + // Long names get truncated to six characters plus a counter, which keeps counting past ~4. + std::vector many = shortNamesFor({ + "sample-12s.mp3", + "sample-15s-cbr-128kbps.mp3", + "sample-15s-cbr-192kbps.mp3", + "sample-15s-cbr-320kbps.mp3", + "sample-15s-cbr-64kbps.mp3", + "sample-15s-vbr-v0.mp3", + "music-sample-320kbps.mp3", + }); + EXPECT_EQ_STR(many[0], std::string("SAMPLE~1.MP3")); + EXPECT_EQ_STR(many[1], std::string("SAMPLE~2.MP3")); + EXPECT_EQ_STR(many[2], std::string("SAMPLE~3.MP3")); + EXPECT_EQ_STR(many[3], std::string("SAMPLE~4.MP3")); + EXPECT_EQ_STR(many[4], std::string("SAMPLE~5.MP3")); + EXPECT_EQ_STR(many[5], std::string("SAMPLE~6.MP3")); + // A different stem numbers independently. + EXPECT_EQ_STR(many[6], std::string("MUSIC-~1.MP3")); + + // Spaces and characters FAT won't take force a counter even when the name is short enough. + std::vector odd = shortNamesFor({"my song.mp3", "a+b.mp3", "no_ext", ".hidden"}); + EXPECT_EQ_STR(odd[0], std::string("MYSONG~1.MP3")); + EXPECT_EQ_STR(odd[1], std::string("A_B~1.MP3")); + EXPECT_EQ_STR(odd[2], std::string("NO_EXT")); + EXPECT_EQ_STR(odd[3], std::string("HIDDEN~1")); + + // Two long names sharing a six character stem must not collide. + std::vector collide = shortNamesFor({"longname-one.txt", "longname-two.txt"}); + EXPECT_EQ_STR(collide[0], std::string("LONGNA~1.TXT")); + EXPECT_EQ_STR(collide[1], std::string("LONGNA~2.TXT")); + + return true; +} + // Tab/Shift+Tab focus navigation walks the view hierarchy in declaration order rather than by // geometry, so what it does is entirely determined by CollectTabOrder - which is worth pinning // down, since the interesting cases (nesting, hidden tabs, disabled items) are all structural. @@ -3030,6 +3089,7 @@ TestItem availableTests[] = { TEST_ITEM(Demangle), TEST_ITEM(TextureReplacer), TEST_ITEM(UITabOrder), + TEST_ITEM(FatShortNames), }; int main(int argc, const char *argv[]) { From 87bb9dd965c6b506e948ff586644d2cd7040c0c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Tue, 8 Sep 2026 11:43:38 -0600 Subject: [PATCH 2/3] docs: how to write a pspautotest and run it on a real PSP We had a doc for running the existing tests against headless, but nothing on the other half - bringing up PSPLink and usbhostfs_pc, what gentest.py does, and how to get an .expected out of real hardware. Write that down, including the parts that cost time to rediscover: usbhostfs_pc's working directory is host0:/ so it has to start in the pspautotests root, gentest.py makes the whole test directory and several old tests no longer build under pspdev's GCC 15 (use -k), rebuilding a .prx with a newer toolchain balloons it, and host0: is not FAT so anything testing FAT semantics needs ms0:. Also adds the io/shortname test the doc uses as its worked example. It stays in tests_next: hardware preserves the case of d_name where we uppercase it, and appends ~1 to the short name of anything that isn't already valid uppercase 8.3 where we only do that on a collision. threads/tls/create moves to tests_next as well. It's collateral from the submodule bump - upstream 1dcefeb regenerated its .expected on a PSP with less free memory, so allocations at 1MB and above now expect failure, and partitions 8 and 9 now expect 800200D2 where we return 800200D1. --- AGENTS.md | 4 +- docs/pspautotests-hardware.md | 197 ++++++++++++++++++++++++++++++++++ pspautotests | 2 +- test.py | 3 +- 4 files changed, 203 insertions(+), 3 deletions(-) create mode 100644 docs/pspautotests-hardware.md diff --git a/AGENTS.md b/AGENTS.md index b9b43b7afd..3d01edfdfd 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -17,6 +17,7 @@ for it: | [docs/HLEModules.md](docs/HLEModules.md) | Adding an HLE module or function, and the seven build files a new source file goes in | | [docs/translations.md](docs/translations.md) | Translating UI strings with Tools/langtool | | [docs/pspautotests.md](docs/pspautotests.md) | Workflow for improving PPSSPP using pspautotests | +| [docs/pspautotests-hardware.md](docs/pspautotests-hardware.md) | Writing a new pspautotest, and running it on a real PSP over PSPLink to record its `.expected` | | [docs/frametest.md](docs/frametest.md) | Framedump rendering tests | | [docs/WebSocketDebugger.md](docs/WebSocketDebugger.md) | WebSocket debugger protocol reference | @@ -105,7 +106,8 @@ python test.py -g --graphics=software New unit tests are added to `availableTests`; large ones go in their own file in `unittest/`, listed in both CMakeLists.txt and the Visual Studio project. See [docs/building.md](docs/building.md) for the details and [docs/pspautotests.md](docs/pspautotests.md) for a workflow for improving PPSSPP with -pspautotest results. +pspautotest results. To write a *new* pspautotest and record its `.expected` from a real PSP over +PSPLink, see [docs/pspautotests-hardware.md](docs/pspautotests-hardware.md). ## Multiplatform considerations diff --git a/docs/pspautotests-hardware.md b/docs/pspautotests-hardware.md new file mode 100644 index 0000000000..11d6934f74 --- /dev/null +++ b/docs/pspautotests-hardware.md @@ -0,0 +1,197 @@ +# Writing pspautotests and running them on a real PSP + +[docs/pspautotests.md](pspautotests.md) covers running the *existing* tests against PPSSPPHeadless. +This document covers the other half: connecting a real PSP over USB, running a test on it, and +recording its output as the `.expected` file that becomes the ground truth. + +You need this whenever the answer to "what does the hardware actually do?" isn't already in an +`.expected` file - which is most of the time when you're implementing something new. + +## What you need + +- A PSP with custom firmware and a USB cable. (Verified against firmware 6.61 / PSPLink v3.0.) +- **PSPLink** installed on the PSP, in `PSP/GAME/psplink`, and *running* - launched from the game + menu. This is not the same as connecting the PSP in USB mass-storage mode; if the PC sees + "PSP Type A" you're in USB mode, and you want "PSP Type B". +- The **pspdev toolchain** on the PC, which supplies `psp-gcc`, `pspsh` and `usbhostfs_pc`. + On macOS it lives in `~/pspdev` by default and is *not* on `PATH`, so prefix commands with + `PATH="$HOME/pspdev/bin:$PATH"` or export it once per shell. On macOS you also need + `brew install libusb-compat`; on Windows, the libusbK driver via Zadig. + +Ask the user to connect the PSP and start PSPLink - you can't do it for them. To confirm it's +there before touching anything else: + +```bash +ioreg -p IOUSB -l -w 0 | grep -i "USB Product Name" # macOS; want "PSP Type B" +``` + +## The three moving parts + +``` + PSP (PSPLink) <--USB--> usbhostfs_pc <--TCP 3000--> pspsh / gentest.py +``` + +- **`usbhostfs_pc -b 3000`** bridges USB to a TCP port and serves the PC filesystem to the PSP as + `host0:/`. **Start it from the `pspautotests` root**, because `host0:/` is literally its working + directory, and that's where a test's output files land. +- **`pspsh -p 3000`** is a shell on the PSP. `pspsh -p 3000 -e ""` runs one command and exits. + Handy ones: `ls`, `pwd`, `reset` (reboot PSPLink after a hung test), `pspver`, `power`, `scrshot`. +- **`gentest.py`** builds a test, runs it through `pspsh`, waits for it to finish and copies the + output over the `.expected` file. It starts `usbhostfs_pc` itself if the port isn't already open. + +Bringing it up, once, from the repo root: + +```bash +export PATH="$HOME/pspdev/bin:$PATH" +cd pspautotests +(cd common && make) # builds libcommon.a; gentest.py refuses to run without it +usbhostfs_pc -b 3000 & # leave running; prints "Connected to device" +pspsh -p 3000 -e ls # sanity check - should list the pspautotests directory +``` + +If `ls` shows `host0:/` contents you have a working chain. If it hangs or shows nothing, PSPLink +isn't running or the USB driver didn't bind. + +## How a test reports its results + +`common/common.c` wraps every test. Before `main()` it redirects the process's `stdout` and +`stderr` to `host0:/__testoutput.txt` and `host0:/__testerror.txt`, and after `main()` returns it +writes `host0:/__testfinish.txt` as a completion marker. So in the `pspautotests` root you'll see: + +| File | Meaning | +|---|---| +| `__testoutput.txt` | the test's stdout - this is what becomes the `.expected` file | +| `__testerror.txt` | stderr; **non-empty means `gentest.py` refuses to write `.expected`** | +| `__testfinish.txt` | written on clean exit; its absence is how a timeout is detected | +| `__screenshot.bmp` | written by `emulatorEmitScreenshot()`; becomes `.expected.bmp` | + +`gentest.py` deletes all four before each run, so a stale file can't be mistaken for a result. + +That redirection is also why `usbhostfs_pc`'s working directory matters: get it wrong and the +output files appear somewhere unexpected, or the test can't open its data files. + +## Writing a new test + +A test is one directory under `pspautotests/tests/`, holding a `Makefile`, the source, the built +`.prx`, and the `.expected`. Minimal `Makefile` - use `common.mk`, not the older verbose style +still found in some directories: + +```make +TARGETS = shortname + +COMMON_DIR = ../../../common +include $(COMMON_DIR)/common.mk +``` + +`TARGETS` lists every test in that directory (one `.c`/`.cpp` per entry); `COMMON_DIR` is a +relative path, so count the levels. `common.mk` sets `BUILD_PRX = 1` and links `libcommon`. + +The source includes ``, which `#define`s `main` to `test_main` so the wrapper above runs. +Add `` if you need `sceKernelSetCompiledSdkVersion*`. Then just `printf`. + +```c +#include +#include + +int main(int argc, char **argv) { + printf("result: %08x\n", sceIoSomething()); + return 0; +} +``` + +Two things `common.h` gives you that are easy to miss: `ARRAY_SIZE`, and `checkpoint()`, which +prefixes each line with `[r]`/`[x]` to record whether a reschedule happened - see +[docs/pspautotests.md](pspautotests.md) for what those markers mean. Use plain `printf` when +scheduling isn't what you're testing. + +Then generate the expected output, from the `pspautotests` root: + +```bash +python3 gentest.py io/shortname/shortname # path under tests/, without .prx +``` + +It runs `make` in the test's directory first, then runs it on the PSP and writes +`tests/io/shortname/shortname.expected`. + +Finally register the test in the repo root's `test.py`: new tests go in **`tests_next`**, and move +to `tests_good` only once PPSSPP passes them. Then check PPSSPP against the hardware: + +```bash +python3 test.py --graphics=software io/shortname/shortname +``` + +### Design rules for a test that can actually pass + +- **Only print things that are the same on hardware and in the emulator.** Kernel pointers, heap + addresses and absolute times are not - PSPLink shifts the memory layout, so a test that prints + them can never pass. Print offsets from a base, or ranges, instead. +- **Sort anything whose order isn't the point.** `sceIoDread` returns a real FAT directory in + creation order and a host directory in whatever order the host filesystem gives; if you're + testing names, `qsort` by name and the difference disappears. +- **Make it repeatable.** If the test creates files, delete them at the start *and* the end - an + aborted run otherwise leaves state that changes the next run's output. +- **Choose an encoding with no ambiguity.** When dumping a buffer, don't render NUL as `.` if the + data can contain a literal `.`. Pick a character the data can't contain (`|` is forbidden in FAT + names, so it works there) - otherwise the `.expected` quietly lies. + +## Gotchas + +- **`gentest.py` runs `make` for the whole test directory, not just your test.** Several older + tests no longer compile with the modern pspdev GCC (15.x turns `-Wint-conversion` into an error, + e.g. `tests/misc/dcache.c`), and the build failure aborts the run before it reaches the PSP. + Work around it with `gentest.py -k` (`--keep`, skips `make` entirely) after building your own + target by hand with `make yourtest.prx`. +- **Rebuilding a `.prx` with a newer toolchain balloons it** - `testgp.prx` went from 117 KB to + 191 KB with no source change. The `.prx` files are committed, so check `git status` and revert + any you didn't mean to touch; don't sweep unrelated rebuilds into your commit. +- **`host0:` is not a FAT volume.** It's PSPLink's bridge to the PC, and it has its own rules - + `tests/io/directory` shows it uppercasing short names, and there are no 8.3 short names at all. + Anything testing FAT semantics has to run against `ms0:`, i.e. create a scratch directory on the + real memory stick (and clean it up). +- **A test that hangs leaves the PSP wedged.** `gentest.py` issues `pspsh -e reset` after a + timeout, but if you ran the PRX by hand, do that yourself. Default timeout is 10s; raise it with + `-t SECONDS`. +- **`tests_to_generate` in `gentest.py`** - the list used when you pass no arguments - is stale; + several paths in it no longer exist. Always name the test you want. +- **`--sdkver` matters for some APIs.** `gentest.py --sdkver=6060010 --sdkver-func=606` makes the + test call `sceKernelSetCompiledSdkVersion606()` at startup, and `-a`/`--all-versions` sweeps every + known version reporting which ones behave differently - a fast way to find version-gated + behavior. A test can also call `sceKernelSetCompiledSdkVersion*()` mid-run to cover several + versions in one `.expected`. +- **The module must be named `TESTMODULE`** (`common.c` does this) or `gentest.py` prints the load + line as an unexpected result. + +## Worked example: FAT short names + +`tests/io/shortname` was written this way and is a decent template. It creates a scratch directory +on `ms0:`, fills it with names that exercise the 8.3 rules, and dumps the raw `d_private` block +that `sceIoDread` fills in. + +It was written to *dump* rather than *decode* on purpose, and that immediately paid off: the +struct in the SDK's own `pspiofilemgr_dirent.h` does not match what firmware 6.61 writes when the +compiled SDK version is unset or below 3.08. The real layouts are + +| Compiled SDK version | Layout | +|---|---| +| unset, or <= 3.07 | short name at byte 0 (13 bytes), long name at byte 13 - no size field | +| >= 3.08 | caller-supplied size at byte 0, short name at byte 4 (16 bytes), long name at byte 20 | + +which is what `Core/HLE/sceIo.cpp` implements, now confirmed on hardware rather than inherited from +JPCSP. Decoding with the SDK struct instead would have produced strings truncated at the front and +an `.expected` that silently enshrined the mistake. + +The test also caught three real emulator/hardware differences, still open in `tests_next`: +hardware preserves the original case in `d_name` (`readme.txt`) where PPSSPP uppercases it +(`README.TXT`), and hardware appends `~1` to the short name of any name that isn't already valid +uppercase 8.3 (`readme.txt` -> `README~1.TXT`, `MiXeD.txt` -> `MIXED~1.TXT`) where PPSSPP only does +so on a collision. + +## Committing + +`pspautotests` is a git submodule, so a new test is two commits: + +1. Commit and push inside `pspautotests/` (the new test directory, including the built `.prx` and + the generated `.expected`). +2. Commit in the PPSSPP repo, including **both** the `test.py` change and the bumped submodule + pointer (`git add pspautotests`) - otherwise CI checks out the old submodule and the test + doesn't exist. diff --git a/pspautotests b/pspautotests index 6ba9cf9e1e..80e8ac290e 160000 --- a/pspautotests +++ b/pspautotests @@ -1 +1 @@ -Subproject commit 6ba9cf9e1eca2fd911dfa728f5509bb321d36b20 +Subproject commit 80e8ac290ecfe0140eaa7595b6d5b8512d17ac62 diff --git a/test.py b/test.py index e86427c0c7..7f430c8c4a 100755 --- a/test.py +++ b/test.py @@ -334,7 +334,6 @@ tests_good = [ "threads/threads/threadmanidlist", "threads/threads/threadmanidtype", "threads/threads/threads", - "threads/tls/create", "threads/tls/delete", "threads/tls/get", "threads/tls/free", @@ -456,6 +455,7 @@ tests_next = [ "io/file/file", "io/io/io", "io/iodrv/iodrv", + "io/shortname/shortname", "io/open/tty0", "jpeg/csc", "jpeg/decode", @@ -481,6 +481,7 @@ tests_next = [ "threads/scheduling/scheduling", "threads/threads/create", "threads/threads/terminate", + "threads/tls/create", "threads/vpl/create", "umd/io/umd_io", "umd/raw_access/raw_access", From 08336e8c22827ea8915841956a9de9058b6b799c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Tue, 8 Sep 2026 12:43:40 -0600 Subject: [PATCH 3/3] pspautotests: pick up the toolchain build fixes Every test directory builds under pspdev GCC 15 again, so gentest.py no longer aborts on a neighbour's compile error before it reaches the PSP. No .prx was regenerated, so the suite behaves exactly as before - 319/319 still pass. Also updates the hardware doc: the "several tests don't build" workaround is gone, and it now records what rebuilding a .prx actually costs (64-bit time_t changes what rtc/convert tests), that host0: differs per host OS, and that PRXs stay resident so you need a reset between runs. --- docs/pspautotests-hardware.md | 37 +++++++++++++++++++++++------------ pspautotests | 2 +- 2 files changed, 26 insertions(+), 13 deletions(-) diff --git a/docs/pspautotests-hardware.md b/docs/pspautotests-hardware.md index 11d6934f74..7597c2480b 100644 --- a/docs/pspautotests-hardware.md +++ b/docs/pspautotests-hardware.md @@ -136,18 +136,31 @@ python3 test.py --graphics=software io/shortname/shortname ## Gotchas -- **`gentest.py` runs `make` for the whole test directory, not just your test.** Several older - tests no longer compile with the modern pspdev GCC (15.x turns `-Wint-conversion` into an error, - e.g. `tests/misc/dcache.c`), and the build failure aborts the run before it reaches the PSP. - Work around it with `gentest.py -k` (`--keep`, skips `make` entirely) after building your own - target by hand with `make yourtest.prx`. -- **Rebuilding a `.prx` with a newer toolchain balloons it** - `testgp.prx` went from 117 KB to - 191 KB with no source change. The `.prx` files are committed, so check `git status` and revert - any you didn't mean to touch; don't sweep unrelated rebuilds into your commit. -- **`host0:` is not a FAT volume.** It's PSPLink's bridge to the PC, and it has its own rules - - `tests/io/directory` shows it uppercasing short names, and there are no 8.3 short names at all. - Anything testing FAT semantics has to run against `ms0:`, i.e. create a scratch directory on the - real memory stick (and clean it up). +- **`gentest.py` runs `make` for the whole test directory, not just your test**, so a neighbour + that doesn't compile stops your test before it ever reaches the PSP. Everything under `tests/` + builds with pspdev GCC 15 as of the "Make the tests build with a current pspdev toolchain" + commit; if you hit a broken one anyway, `gentest.py -k` (`--keep`) skips `make` entirely and + you can build your own target by hand with `make yourtest.prx`. +- **Rebuilding a `.prx` is not free, so don't regenerate one you didn't change.** The binaries are + committed and were built with a much older SDK. Rebuilding with the current toolchain grows them + by roughly a third (`testgp.prx`: 117 KB to 191 KB), and it can change what a test *does*: + `time_t` is 64-bit now, so `rtc/convert`'s `sceRtcSetTime_t(&pt, 62135596800ULL)` marshals + differently than the committed binary and stops matching its own `.expected`. Check + `git status` and revert any `.prx` you didn't mean to touch. +- **A `.prx` you did rebuild deserves a hardware run before you commit it.** Build it, run it, and + diff the output against the committed `.expected` - if it differs, decide whether the test + genuinely changed or whether the toolchain did. Note the committed `.expected` files have CRLF + line endings (they were recorded on Windows) while a fresh run writes LF, so compare with + `diff <(tr -d '\r' < __testoutput.txt) <(tr -d '\r' < the.expected)`. +- **Each test PRX stays resident after it runs.** Run a handful back to back and the next + `Load/Start` fails with `0x80020190` (out of memory) - which looks exactly like a hung test. + `pspsh -p 3000 -e reset` between runs, and wait for the PSP to come back before the next one. +- **`host0:` is not a FAT volume, and it isn't even the same across hosts.** It's PSPLink's bridge + to the PC, so it inherits the PC's filesystem: `tests/io/directory` was recorded on Windows and + does not match on macOS, where `..` reports a different size and short names come back as + `1.txt` rather than `1.TXT`. Anything testing FAT semantics has to run against `ms0:` - create a + scratch directory on the real memory stick and clean it up - and anything reading `host0:` will + only reproduce on the OS it was recorded on. - **A test that hangs leaves the PSP wedged.** `gentest.py` issues `pspsh -e reset` after a timeout, but if you ran the PRX by hand, do that yourself. Default timeout is 10s; raise it with `-t SECONDS`. diff --git a/pspautotests b/pspautotests index 80e8ac290e..40317c54ce 160000 --- a/pspautotests +++ b/pspautotests @@ -1 +1 @@ -Subproject commit 80e8ac290ecfe0140eaa7595b6d5b8512d17ac62 +Subproject commit 40317c54ce6709b0f52e7617e01fbaff03806a19