diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index a156710..86e22f7 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -46,3 +46,30 @@ jobs: - name: Test run: dotnet test translator/Translator.sln -c Release --no-build --verbosity normal + + nand: + name: NAND regression tests (${{ matrix.os }}) + runs-on: ${{ matrix.os }} + timeout-minutes: 10 + strategy: + fail-fast: false + matrix: + os: [windows-latest, ubuntu-latest, macos-14] + steps: + - uses: actions/checkout@v7 + with: + persist-credentials: false + - name: Compile and run NAND tests + shell: bash + run: | + flags=(-std=c++17 -Wall -Wextra -Werror) + if [[ "$RUNNER_OS" != Windows ]]; then flags+=(-pthread); fi + clang++ "${flags[@]}" -Iruntime/src/hle/storage \ + runtime/tests/nand_move_tests.cpp runtime/src/hle/storage/nand_file_ops.cpp \ + -o "$RUNNER_TEMP/nand_move_tests.exe" + "$RUNNER_TEMP/nand_move_tests.exe" + for test in nand_save nand_settings; do + clang++ "${flags[@]}" -Iruntime/include "runtime/tests/${test}_tests.cpp" \ + -o "$RUNNER_TEMP/${test}_tests.exe" + "$RUNNER_TEMP/${test}_tests.exe" + done diff --git a/runtime/CMakeLists.txt b/runtime/CMakeLists.txt index 90c322d..f722181 100644 --- a/runtime/CMakeLists.txt +++ b/runtime/CMakeLists.txt @@ -282,8 +282,12 @@ target_include_directories(mkw_nand_save_tests PRIVATE "${CMAKE_CURRENT_LIST_DIR target_compile_features(mkw_nand_save_tests PRIVATE cxx_std_17) add_test(NAME mkw_nand_save_tests COMMAND mkw_nand_save_tests) -add_executable(mkw_nand_move_tests "${CMAKE_CURRENT_LIST_DIR}/tests/nand_move_tests.cpp") -target_include_directories(mkw_nand_move_tests PRIVATE "${CMAKE_CURRENT_LIST_DIR}/include") +add_executable(mkw_nand_move_tests + "${CMAKE_CURRENT_LIST_DIR}/tests/nand_move_tests.cpp" + "${CMAKE_CURRENT_LIST_DIR}/src/hle/storage/nand_file_ops.cpp") +find_package(Threads REQUIRED) +target_link_libraries(mkw_nand_move_tests PRIVATE Threads::Threads) +target_include_directories(mkw_nand_move_tests PRIVATE "${CMAKE_CURRENT_LIST_DIR}/src/hle/storage") target_compile_features(mkw_nand_move_tests PRIVATE cxx_std_17) add_test(NAME mkw_nand_move_tests COMMAND mkw_nand_move_tests) diff --git a/runtime/include/nand_move.h b/runtime/include/nand_move.h deleted file mode 100644 index d0a4102..0000000 --- a/runtime/include/nand_move.h +++ /dev/null @@ -1,69 +0,0 @@ -#pragma once - -#include -#include -#include -#include -#include - -namespace RuntimeNandMove { - -// NANDMove does not replace an existing entry. Riivolution save redirects can -// put the destination on a different filesystem from the guest's /tmp files. -inline void Move(const std::filesystem::path& source, const std::filesystem::path& destination, - std::error_code& ec) { - namespace fs = std::filesystem; - const auto status = fs::symlink_status(destination, ec); - if (ec && ec != std::errc::no_such_file_or_directory) return; - ec.clear(); - if (fs::exists(status)) { - ec = std::make_error_code(std::errc::file_exists); - return; - } - fs::rename(source, destination, ec); - if (ec != std::errc::cross_device_link) return; - - // Do not turn a directory move into a partially completed recursive copy, - // or follow a symlink and delete the link after copying its target. - const auto sourceStatus = fs::symlink_status(source, ec); - if (ec) return; - if (!fs::is_regular_file(sourceStatus)) { - ec = std::make_error_code(std::errc::cross_device_link); - return; - } - - static std::atomic sequence{0}; - const auto stamp = std::chrono::steady_clock::now().time_since_epoch().count(); - fs::path stagingDirectory; - bool created = false; - for (int attempt = 0; attempt < 64; ++attempt) { - stagingDirectory = destination.parent_path() / - (".nand-move-" + std::to_string(stamp) + "-" + std::to_string(sequence++)); - created = fs::create_directory(stagingDirectory, ec); - if (created) break; - if (ec && ec != std::errc::file_exists) return; - } - if (!created) { - ec = std::make_error_code(std::errc::file_exists); - return; - } - - const auto staged = stagingDirectory / "data"; - fs::copy_file(source, staged, fs::copy_options::none, ec); - if (!ec) { - // Publication is a same-filesystem rename: readers never see a partial - // copy. Keep the source until the complete destination is in place. - const auto current = fs::symlink_status(destination, ec); - if (ec == std::errc::no_such_file_or_directory) ec.clear(); - if (!ec && fs::exists(current)) ec = std::make_error_code(std::errc::file_exists); - if (!ec) fs::rename(staged, destination, ec); - if (!ec) fs::remove(source, ec); - } - // On failure the source remains available; after publication a failed - // source removal leaves both complete copies. Preserve the original error. - std::error_code cleanupError; - fs::remove(staged, cleanupError); - fs::remove(stagingDirectory, cleanupError); -} - -} // namespace RuntimeNandMove diff --git a/runtime/src/hle/storage/nand_api.cpp b/runtime/src/hle/storage/nand_api.cpp index 5864107..ca8eee7 100644 --- a/runtime/src/hle/storage/nand_api.cpp +++ b/runtime/src/hle/storage/nand_api.cpp @@ -3,7 +3,7 @@ // Shared state and helpers live in nand_internal.h. #include "nand_internal.h" -#include "nand_move.h" +#include "nand_file_ops.h" // ============================================================================ // Local helpers @@ -379,7 +379,7 @@ extern "C" int32_t NANDMove_HLE(uint32_t srcPathPtr, uint32_t dstPathPtr) { } std::error_code ec; - RuntimeNandMove::Move(srcHost, dstHost, ec); + NandMove(srcHost, dstHost, ec); if (!ec) { return NAND_RESULT_OK; } diff --git a/runtime/src/hle/storage/nand_file_ops.cpp b/runtime/src/hle/storage/nand_file_ops.cpp new file mode 100644 index 0000000..a97ba6a --- /dev/null +++ b/runtime/src/hle/storage/nand_file_ops.cpp @@ -0,0 +1,115 @@ +#include "nand_file_ops.h" + +#include +#include +#include +#include +#include +#include + +#ifdef _WIN32 +#include +#elif defined(__linux__) +#include +#include +#include +#include +#elif defined(__APPLE__) +#include +#endif + +namespace { + +// Unlike std::filesystem::rename on POSIX, this cannot overwrite a destination +// created by another writer between checking it and publishing the move. +void RenameNoReplace(const std::filesystem::path& source, + const std::filesystem::path& destination, std::error_code& ec) { +#ifdef _WIN32 + if (MoveFileExW(source.c_str(), destination.c_str(), 0)) ec.clear(); + else ec = std::error_code(GetLastError(), std::system_category()); +#else +#ifdef __linux__ + const auto result = syscall(SYS_renameat2, AT_FDCWD, source.c_str(), + AT_FDCWD, destination.c_str(), RENAME_NOREPLACE); +#else + const auto result = renamex_np(source.c_str(), destination.c_str(), RENAME_EXCL); +#endif + if (result == 0) { + ec.clear(); + return; + } + ec = std::error_code(errno, std::generic_category()); + if (ec != std::errc::function_not_supported && ec != std::errc::invalid_argument && + ec != std::errc::operation_not_supported) return; + + // Older filesystems may lack exclusive rename. Linking also publishes + // without replacement; never fall back to an overwriting rename. + std::filesystem::create_hard_link(source, destination, ec); + if (!ec) std::filesystem::remove(source, ec); +#endif +} + +struct StagedMove { + std::filesystem::path directory; + std::filesystem::path file; + ~StagedMove() { + std::error_code ignored; + std::filesystem::remove(file, ignored); + std::filesystem::remove(directory, ignored); + } +}; + +} // namespace + +// NANDMove does not replace an existing entry. Riivolution save redirects can +// put the destination on a different filesystem from the guest's /tmp files. +void NandMove(const std::filesystem::path& source, const std::filesystem::path& destination, + std::error_code& ec) { + namespace fs = std::filesystem; + const auto status = fs::symlink_status(destination, ec); + if (ec && ec != std::errc::no_such_file_or_directory) return; + ec.clear(); + if (fs::exists(status)) { + ec = std::make_error_code(std::errc::file_exists); + return; + } + RenameNoReplace(source, destination, ec); + if (ec != std::errc::cross_device_link) return; + + // Do not turn a directory move into a partially completed recursive copy, + // or follow a symlink and delete the link after copying its target. + const auto sourceStatus = fs::symlink_status(source, ec); + if (ec) return; + if (!fs::is_regular_file(sourceStatus)) { + ec = std::make_error_code(std::errc::cross_device_link); + return; + } + + static std::atomic sequence{0}; + const auto stamp = std::chrono::steady_clock::now().time_since_epoch().count(); + fs::path stagingDirectory; + bool created = false; + for (int attempt = 0; attempt < 64; ++attempt) { + stagingDirectory = destination.parent_path() / + (".nand-move-" + std::to_string(stamp) + "-" + std::to_string(sequence++)); + created = fs::create_directory(stagingDirectory, ec); + if (created) break; + if (ec && ec != std::errc::file_exists) return; + } + if (!created) { + ec = std::make_error_code(std::errc::file_exists); + return; + } + + const StagedMove staging{stagingDirectory, stagingDirectory / "data"}; + const auto& staged = staging.file; + fs::copy_file(source, staged, fs::copy_options::none, ec); + if (!ec) { + // Publication is a same-filesystem rename: readers never see a partial + // copy. Keep the source until the complete destination is in place. + RenameNoReplace(staged, destination, ec); + if (!ec) fs::remove(source, ec); + } + // A failed source removal leaves both complete copies and reports failure. + // Staging cleanup preserves ec, including on exceptions. +} diff --git a/runtime/src/hle/storage/nand_file_ops.h b/runtime/src/hle/storage/nand_file_ops.h new file mode 100644 index 0000000..7f149b7 --- /dev/null +++ b/runtime/src/hle/storage/nand_file_ops.h @@ -0,0 +1,10 @@ +#pragma once + +#include +#include + +// Move without replacing an existing entry. Cross-filesystem regular files are +// staged at the destination before removing the source. On failure the source +// remains; a failed source removal can leave both complete copies. +void NandMove(const std::filesystem::path& source, const std::filesystem::path& destination, + std::error_code& ec); diff --git a/runtime/tests/nand_move_tests.cpp b/runtime/tests/nand_move_tests.cpp index 61b8a4e..1370214 100644 --- a/runtime/tests/nand_move_tests.cpp +++ b/runtime/tests/nand_move_tests.cpp @@ -1,6 +1,12 @@ -#include "nand_move.h" +#include "nand_file_ops.h" +#include +#include #include +#include +#include +#include +#include #include #include @@ -30,6 +36,40 @@ static std::string Read(const fs::path& path) { return {std::istreambuf_iterator(input), std::istreambuf_iterator()}; } +// Competing moves must have exactly one winner, retain the losing source, and +// publish the winner's complete contents. Exercise both direct and staged moves. +static void CheckCompetingMoves(const fs::path& sourceRoot, const fs::path& destinationRoot) { + const auto left = sourceRoot / "race-left"; + const auto right = sourceRoot / "race-right"; + const auto target = destinationRoot / "race-target"; + const std::string leftBytes(65536, 'L'), rightBytes(65536, 'R'); + for (int attempt = 0; attempt < 128; ++attempt) { + Write(left, leftBytes); + Write(right, rightBytes); + std::atomic ready{0}; + std::error_code leftError, rightError; + auto move = [&](const fs::path& source, std::error_code& ec) { + ++ready; + while (ready.load() != 2) std::this_thread::yield(); + NandMove(source, target, ec); + }; + std::thread first(move, std::cref(left), std::ref(leftError)); + std::thread second(move, std::cref(right), std::ref(rightError)); + first.join(); + second.join(); + Require(bool(leftError) != bool(rightError), "Competing moves must have exactly one winner"); + const bool leftWon = !leftError; + Require((leftWon ? rightError : leftError) == std::errc::file_exists, + "Losing move must report an existing destination"); + Require(Read(target) == (leftWon ? leftBytes : rightBytes), "Winner's contents were overwritten"); + Require(!fs::exists(leftWon ? left : right), "Winning source was not removed"); + Require(Read(leftWon ? right : left) == (leftWon ? rightBytes : leftBytes), + "Losing source must remain intact"); + fs::remove(leftWon ? right : left); + fs::remove(target); + } +} + int main() { const auto name = "wiicomp-nand-move-" + std::to_string(std::chrono::steady_clock::now().time_since_epoch().count()); @@ -41,19 +81,29 @@ int main() { const auto destination = root / "moved.bin"; std::error_code ec; Write(source, "banner"); - RuntimeNandMove::Move(source, destination, ec); + NandMove(source, destination, ec); Require(!ec && !fs::exists(source) && Read(destination) == "banner", "Same-device move failed"); + CheckCompetingMoves(root, root); Write(source, "keep source"); - RuntimeNandMove::Move(source, destination, ec); + NandMove(source, destination, ec); Require(ec == std::errc::file_exists && Read(source) == "keep source" && Read(destination) == "banner", "Existing destination must not be overwritten"); - RuntimeNandMove::Move(root / "missing", root / "absent", ec); + NandMove(root / "missing", root / "absent", ec); Require(bool(ec) && !fs::exists(root / "absent"), "Missing source must fail"); + NandMove(source, root / "missing-parent/file", ec); + Require(bool(ec) && Read(source) == "keep source", "Missing parent must preserve source"); fs::create_directory(root / "directory"); Write(root / "directory/child", "child"); - RuntimeNandMove::Move(root / "directory", root / "renamed-directory", ec); + NandMove(root / "directory", root / "renamed-directory", ec); Require(!ec && Read(root / "renamed-directory/child") == "child", "Same-device directory move regressed"); +#ifndef _WIN32 + fs::create_symlink(root / "missing", root / "same-link"); + NandMove(root / "same-link", root / "moved-link", ec); + Require(!ec && fs::is_symlink(fs::symlink_status(root / "moved-link")) && + !fs::is_symlink(fs::symlink_status(root / "same-link")), "Same-device symlink move regressed"); +#endif + #ifdef __linux__ // /dev/shm is a separate tmpfs on ordinary Linux systems, including CI // and WSL. Fail rather than silently passing without exercising EXDEV. @@ -62,30 +112,32 @@ int main() { struct stat left{}, right{}; Require(::stat(root.c_str(), &left) == 0 && ::stat(other.c_str(), &right) == 0 && left.st_dev != right.st_dev, "Cross-device test requires /tmp and /dev/shm on separate filesystems"); + CheckCompetingMoves(root, other); + Require(fs::is_empty(other), "Competing moves left staging files behind"); const auto target = other / "banner.bin"; const std::string bytes = std::string(8192, '\0') + "banner payload"; Write(source, bytes); fs::rename(source, target, ec); Require(ec == std::errc::cross_device_link, "Fixture must reproduce the original EXDEV failure"); - RuntimeNandMove::Move(source, target, ec); + NandMove(source, target, ec); Require(!ec && !fs::exists(source) && Read(target) == bytes, "Cross-device move must preserve every byte"); Write(source, "do not overwrite"); - RuntimeNandMove::Move(source, target, ec); + NandMove(source, target, ec); Require(ec == std::errc::file_exists && Read(source) == "do not overwrite" && Read(target) == bytes, "Cross-device move must preserve an existing destination"); fs::remove(target); fs::create_symlink(other / "missing", target); - RuntimeNandMove::Move(source, target, ec); + NandMove(source, target, ec); Require(ec == std::errc::file_exists && fs::is_symlink(target) && Read(source) == "do not overwrite", "Dangling destination symlink must not be replaced"); fs::remove(target); - RuntimeNandMove::Move(root / "renamed-directory", other / "directory", ec); + NandMove(root / "renamed-directory", other / "directory", ec); Require(ec == std::errc::cross_device_link && Read(root / "renamed-directory/child") == "child" && !fs::exists(other / "directory"), "Unsupported directory move must leave source intact"); fs::create_symlink(source, root / "link"); - RuntimeNandMove::Move(root / "link", other / "link", ec); + NandMove(root / "link", other / "link", ec); Require(ec == std::errc::cross_device_link && fs::is_symlink(root / "link") && !fs::exists(other / "link"), "Cross-device source symlinks must not be dereferenced"); @@ -97,7 +149,7 @@ int main() { limited.rlim_cur = 1024; const auto oldHandler = std::signal(SIGXFSZ, SIG_IGN); Require(setrlimit(RLIMIT_FSIZE, &limited) == 0, "Cannot set file-size limit"); - RuntimeNandMove::Move(source, target, ec); + NandMove(source, target, ec); const auto copyError = ec; const auto restored = setrlimit(RLIMIT_FSIZE, &saved); std::signal(SIGXFSZ, oldHandler); @@ -107,7 +159,7 @@ int main() { Require(geteuid() != 0, "Run permission tests as an unprivileged user"); fs::permissions(root, fs::perms::owner_read | fs::perms::owner_exec); - RuntimeNandMove::Move(source, target, ec); + NandMove(source, target, ec); fs::permissions(root, fs::perms::owner_all); Require(bool(ec) && Read(source) == bytes && Read(target) == bytes, "Failed source deletion must leave both complete copies");