From 05f92b64229f933b5ed9af9bd3b7800e64883843 Mon Sep 17 00:00:00 2001 From: Emil Date: Mon, 28 Sep 2026 16:42:22 +0200 Subject: [PATCH] libCes CP932 fix for Windows/macOS, fix SaveDataDialog lockups (#878) * libs: fix CP932 iconv conversion on Windows and macOS SDL3's SDL_SYSTEM_ICONV build option defaults off on Windows and macOS, so SDL falls back to its own bundled iconv there, which only implements ASCII/Latin-1/UTF/UCS. SDL_iconv_open("UTF-8", "CP932") always failed as a result, crashing games that convert Shift-JIS strings via sceCesMbcsStrToUtf8Str (e.g. save-data dialogs). Add a small CesIconv shim selected at compile time: Windows converts one character at a time via the Win32 codepage-932 APIs, replicating SDL_iconv's chunked semantics and error codes; macOS calls real POSIX iconv directly, since libiconv is always present there and does support CP932, unlike SDL's fallback. Linux is untouched, since SDL already links real system iconv by default there. Co-Authored-By: Claude Sonnet 5 * libs: fix SaveDataDialog infinite loop on save SaveDataDialogOpen() jumped straight from SAVE_STATUS_INITIALIZED (1) to SAVE_STATUS_FINISHED (3), skipping SAVE_STATUS_RUNNING (2) entirely. At least one title's save-write dialog stage only accepts a FINISHED status after observing a RUNNING poll first; without it, the title assumed the Open() request never took effect and re-issued it forever, hanging the save flow (this only became visible once the CP932 crash above stopped masking it). Open() now starts the dialog as RUNNING. UpdateStatus() advances the state machine, reporting RUNNING once before flipping to FINISHED for subsequent polls; GetStatus() is untouched, staying a pure peek with no side effects, matching the real SDK's pump-vs-peek split between the two calls. Co-Authored-By: Claude Sonnet 5 * Add iconv linking to macOS * Fix missing macOS iconv link for tests * Fix CP932 edge cases and save dialog status consistency --------- Co-authored-by: Claude Sonnet 5 Co-authored-by: nmzik --- 3rdparty/CMakeLists.txt | 4 + CMakeLists.txt | 15 +++ src/libs/dialog.cpp | 43 ++++++--- src/libs/libCes.cpp | 70 +++++++++++++- tests/CesTests.cpp | 127 ++++++++++++++++++++++++++ tests/SaveDataDialogTests.cpp | 166 ++++++++++++++++++++++++++++++++++ 6 files changed, 409 insertions(+), 16 deletions(-) create mode 100644 tests/CesTests.cpp create mode 100644 tests/SaveDataDialogTests.cpp diff --git a/3rdparty/CMakeLists.txt b/3rdparty/CMakeLists.txt index 19533be1..4960bda5 100644 --- a/3rdparty/CMakeLists.txt +++ b/3rdparty/CMakeLists.txt @@ -29,6 +29,10 @@ FetchContent_Declare( FetchContent_MakeAvailable(xbyak zydis) set(SDL_SHARED OFF CACHE BOOL "" FORCE) +if(APPLE) + # SDL's built-in iconv fallback does not support CP932. + set(SDL_SYSTEM_ICONV ON CACHE BOOL "Use system iconv for CP932 conversion" FORCE) +endif() add_subdirectory(SDL3) add_library(Vulkan-Headers INTERFACE) diff --git a/CMakeLists.txt b/CMakeLists.txt index b9609b1d..f3847c1b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -490,6 +490,17 @@ add_executable(save_data_memory_tests EXCLUDE_FROM_ALL target_link_libraries(save_data_memory_tests common fmt::fmt) target_include_directories(save_data_memory_tests PRIVATE ${inc_headers}) +add_executable(ces_tests EXCLUDE_FROM_ALL "${KYTY_TESTS_DIR}/CesTests.cpp") +target_link_libraries(ces_tests common fmt::fmt SDL3::SDL3-static) +target_include_directories(ces_tests PRIVATE ${inc_headers}) + +add_executable(save_data_dialog_tests EXCLUDE_FROM_ALL + "${KYTY_TESTS_DIR}/SaveDataDialogTests.cpp" + "${KYTY_SOURCE_DIR}/libs/dialog.cpp" +) +target_link_libraries(save_data_dialog_tests common fmt::fmt) +target_include_directories(save_data_dialog_tests PRIVATE ${inc_headers}) + add_kyty_full_emulator_test(http_uri_parse_tests "${KYTY_TESTS_DIR}/HttpUriParseTests.cpp") @@ -581,6 +592,8 @@ if(BUILD_TESTING) add_test(NAME ngs2_sampler COMMAND $) add_test(NAME pad_haptics COMMAND $) add_test(NAME save_data_memory COMMAND $) + add_test(NAME ces COMMAND $) + add_test(NAME save_data_dialog COMMAND $) add_test(NAME http_uri_parse COMMAND $) add_test(NAME shader_recompiler_compute COMMAND $) add_test(NAME shader_recompiler_alignbyte @@ -650,6 +663,8 @@ if(BUILD_TESTING) ngs2_sampler_tests pad_haptics_tests save_data_memory_tests + ces_tests + save_data_dialog_tests http_uri_parse_tests ) endif() diff --git a/src/libs/dialog.cpp b/src/libs/dialog.cpp index 54ae845f..cec494bf 100644 --- a/src/libs/dialog.cpp +++ b/src/libs/dialog.cpp @@ -6,6 +6,7 @@ #include "libs/errno.h" #include "libs/libs.h" +#include #include #include #include @@ -306,6 +307,7 @@ LIB_NAME("SaveDataDialog", "SaveDataDialog"); constexpr int SAVE_STATUS_NONE = 0; constexpr int SAVE_STATUS_INITIALIZED = 1; +constexpr int SAVE_STATUS_RUNNING = 2; constexpr int SAVE_STATUS_FINISHED = 3; constexpr int SAVE_RESULT_OK = 0; constexpr int SAVE_BUTTON_ID_OK = 1; @@ -351,18 +353,20 @@ struct SaveDataDialogResult { uint8_t reserved[32]; }; -static int g_save_status = SAVE_STATUS_NONE; -static int g_save_mode = 0; -static void* g_save_user_data = nullptr; -static char g_save_dir_name[sizeof(SaveDataDirName::data)] {}; +static std::atomic g_save_status = SAVE_STATUS_NONE; +static bool g_save_running_polled = false; +static int g_save_mode = 0; +static void* g_save_user_data = nullptr; +static char g_save_dir_name[sizeof(SaveDataDirName::data)] {}; int KYTY_SYSV_ABI SaveDataDialogInitialize() { PRINT_NAME(); - g_save_status = SAVE_STATUS_INITIALIZED; - g_save_mode = 0; - g_save_user_data = nullptr; - g_save_dir_name[0] = '\0'; + g_save_status = SAVE_STATUS_INITIALIZED; + g_save_running_polled = false; + g_save_mode = 0; + g_save_user_data = nullptr; + g_save_dir_name[0] = '\0'; return OK; } @@ -376,6 +380,14 @@ int KYTY_SYSV_ABI SaveDataDialogGetStatus() { int KYTY_SYSV_ABI SaveDataDialogUpdateStatus() { PRINT_NAME(); + // Some games require a RUNNING update before FINISHED. + if (g_save_status == SAVE_STATUS_RUNNING) { + if (g_save_running_polled) { + g_save_status = SAVE_STATUS_FINISHED; + } + g_save_running_polled = true; + } + return g_save_status; } @@ -431,7 +443,8 @@ int KYTY_SYSV_ABI SaveDataDialogOpen(const void* param) { } } - g_save_status = SAVE_STATUS_FINISHED; + g_save_status = SAVE_STATUS_RUNNING; + g_save_running_polled = false; return OK; } @@ -441,7 +454,8 @@ int KYTY_SYSV_ABI SaveDataDialogClose(const void* close_param) { LOGF("\t close_param = 0x%016" PRIx64 "\n", reinterpret_cast(close_param)); - g_save_status = SAVE_STATUS_FINISHED; + g_save_status = SAVE_STATUS_FINISHED; + g_save_running_polled = false; return OK; } @@ -455,10 +469,11 @@ int KYTY_SYSV_ABI SaveDataDialogIsReadyToDisplay() { int KYTY_SYSV_ABI SaveDataDialogTerminate() { PRINT_NAME(); - g_save_status = SAVE_STATUS_NONE; - g_save_mode = 0; - g_save_user_data = nullptr; - g_save_dir_name[0] = '\0'; + g_save_status = SAVE_STATUS_NONE; + g_save_running_polled = false; + g_save_mode = 0; + g_save_user_data = nullptr; + g_save_dir_name[0] = '\0'; return 0; } diff --git a/src/libs/libCes.cpp b/src/libs/libCes.cpp index f7d984fb..8739d34c 100644 --- a/src/libs/libCes.cpp +++ b/src/libs/libCes.cpp @@ -7,6 +7,16 @@ #include #include +#if defined(_WIN32) +#ifndef WIN32_LEAN_AND_MEAN +#define WIN32_LEAN_AND_MEAN +#endif +#ifndef NOMINMAX +#define NOMINMAX +#endif +#include +#endif + namespace Libs { namespace LibCes { @@ -213,6 +223,54 @@ static int KYTY_SYSV_ABI CesMbcsUcsContextInit(CesContext* context, const CesPro return 0; } +#if defined(_WIN32) + +static size_t ConvertCp932ToUtf8(const char** inbuf, size_t* inbytesleft, char** outbuf, + size_t* outbytesleft) { + const auto* in = reinterpret_cast(*inbuf); + while (*inbytesleft != 0) { + const uint8_t lead = in[0]; + // Win32 accepts 0x80 as U+0080; the SJis1997 profile rejects this byte. + if (lead == 0x80) { + *inbuf = reinterpret_cast(in); + return SDL_ICONV_EILSEQ; + } + const bool is_lead_byte = (lead >= 0x81 && lead <= 0x9f) || (lead >= 0xe0 && lead <= 0xfc); + const size_t char_len = is_lead_byte ? 2 : 1; + if (*inbytesleft < char_len) { + *inbuf = reinterpret_cast(in); + return SDL_ICONV_EINVAL; + } + + wchar_t wide[2]; + const int wide_len = + MultiByteToWideChar(932, MB_ERR_INVALID_CHARS, reinterpret_cast(in), + static_cast(char_len), wide, 2); + if (wide_len <= 0) { + *inbuf = reinterpret_cast(in); + return SDL_ICONV_EILSEQ; + } + + char utf8[8]; + const int utf8_len = + WideCharToMultiByte(CP_UTF8, 0, wide, wide_len, utf8, sizeof(utf8), nullptr, nullptr); + if (utf8_len <= 0 || static_cast(utf8_len) > *outbytesleft) { + *inbuf = reinterpret_cast(in); + return SDL_ICONV_E2BIG; + } + + std::memcpy(*outbuf, utf8, static_cast(utf8_len)); + *outbuf += utf8_len; + *outbytesleft -= static_cast(utf8_len); + in += char_len; + *inbytesleft -= char_len; + } + *inbuf = reinterpret_cast(in); + return 0; +} + +#endif + static int ConvertMbcsToUtf8(CesContext* context, const uint8_t* source, uint32_t source_max, uint32_t* source_len, uint8_t* destination, uint32_t destination_max, uint32_t* destination_len, bool measure) { @@ -246,8 +304,10 @@ static int ConvertMbcsToUtf8(CesContext* context, const uint8_t* source, uint32_ ++source_size; } const bool bounded_end = source_max != 0 && source_size == source_max; - const auto converter = SDL_iconv_open("UTF-8", context->profile->encoding); +#if !defined(_WIN32) + const auto converter = SDL_iconv_open("UTF-8", context->profile->encoding); EXIT_IF(converter == reinterpret_cast(-1)); +#endif const char* input = reinterpret_cast(source); size_t input_left = source_size; @@ -258,7 +318,11 @@ static int ConvertMbcsToUtf8(CesContext* context, const uint8_t* source, uint32_ char* output = measure ? scratch.data() : reinterpret_cast(destination) + produced; const size_t capacity = measure ? scratch.size() : destination_max - 1 - produced; size_t output_left = capacity; - const auto status = SDL_iconv(converter, &input, &input_left, &output, &output_left); +#if defined(_WIN32) + const auto status = ConvertCp932ToUtf8(&input, &input_left, &output, &output_left); +#else + const auto status = SDL_iconv(converter, &input, &input_left, &output, &output_left); +#endif produced += capacity - output_left; if (status == SDL_ICONV_E2BIG) { if (measure) { @@ -277,7 +341,9 @@ static int ConvertMbcsToUtf8(CesContext* context, const uint8_t* source, uint32_ } break; } while (input_left != 0); +#if !defined(_WIN32) SDL_iconv_close(converter); +#endif if (!measure) { destination[produced] = 0; diff --git a/tests/CesTests.cpp b/tests/CesTests.cpp new file mode 100644 index 00000000..5d59f7f7 --- /dev/null +++ b/tests/CesTests.cpp @@ -0,0 +1,127 @@ +#include "libs/libCes.cpp" + +#include +#include + +namespace Loader { +void SymbolDatabase::Add(const SymbolResolve&, uint64_t, const std::string&) {} +namespace Timer { +double GetTimeMs() { + return 0.0; +} +} // namespace Timer +} // namespace Loader + +namespace { +using namespace Libs::LibCes; + +int failures = 0; + +#define CHECK(condition) \ + do { \ + if (!(condition)) { \ + std::fprintf(stderr, "CesTests:%d: %s\n", __LINE__, #condition); \ + ++failures; \ + } \ + } while (false) + +void CheckConversion(CesContext* context, const std::vector& source, + const std::string& expected) { + std::vector output(expected.size() + 2, 0xcc); + uint32_t consumed = 0; + uint32_t produced = 0; + CHECK(CesMbcsStrToUtf8Str(context, source.data(), 0, &consumed, output.data(), + static_cast(expected.size() + 1), &produced) == 0); + CHECK(consumed == source.size() - 1); + CHECK(produced == expected.size()); + CHECK(std::memcmp(output.data(), expected.data(), expected.size()) == 0); + CHECK(output[expected.size()] == 0); + CHECK(output.back() == 0xcc); + CHECK(CesMbcsStrGetUtf8Len(context, source.data(), 0, &consumed, &produced) == 0); + CHECK(consumed == source.size() - 1); + CHECK(produced == expected.size()); +} + +void TestMappingsAndMeasurement(CesContext* context) { + CheckConversion(context, {0}, ""); + CheckConversion(context, {'S', 'a', 'v', 'e', ' ', 0x93, 0xfa, 0x96, 0x7b, 0xb6, 0}, + "Save \xe6\x97\xa5\xe6\x9c\xac\xef\xbd\xb6"); + // CP932 maps this character to U+FF5E, unlike strict Shift-JIS (U+301C). + CheckConversion(context, {0x81, 0x60, 0}, "\xef\xbd\x9e"); + std::vector source; + std::string expected; + for (int i = 0; i < 200; ++i) { + source.insert(source.end(), {0x82, 0xa0}); + expected += "\xe3\x81\x82"; + } + source.push_back(0); + CheckConversion(context, source, expected); +} + +void TestPartialOutput(CesContext* context) { + const uint8_t source[] = {'A', 0x82, 0xa0, 'B', 0}; + for (uint32_t capacity = 1; capacity <= 7; ++capacity) { + std::array output; + output.fill(0xcc); + uint32_t consumed = 99; + uint32_t produced = 99; + const int status = + CesMbcsStrToUtf8Str(context, source, 0, &consumed, output.data(), capacity, &produced); + const uint32_t expected_consumed = capacity == 1 ? 0 + : capacity < 5 ? 1 + : capacity == 5 ? 3 + : 4; + const uint32_t expected_produced = capacity == 1 ? 0 + : capacity < 5 ? 1 + : capacity == 5 ? 4 + : 5; + CHECK(status == (capacity < 6 ? CES_ERROR_DST_BUFFER_END : 0)); + CHECK(consumed == expected_consumed); + CHECK(produced == expected_produced); + CHECK(std::memcmp(output.data(), + "A\xe3\x81\x82" + "B", + expected_produced) == 0); + CHECK(output[expected_produced] == 0); + CHECK(output[capacity] == 0xcc); + } +} + +void TestErrors(CesContext* context) { + struct Case { + std::vector source; + uint32_t limit; + int error; + }; + const Case cases[] = { + {{'A', 0x80, 0}, 0, CES_ERROR_INVALID_ENCODE}, + {{'A', 0x82, 0}, 0, CES_ERROR_INVALID_ENCODE}, + {{'A', 0x82, 0xa0, 0}, 2, CES_ERROR_SRC_BUFFER_END}, + {{'A', 0x82, 0x20, 0}, 0, CES_ERROR_INVALID_ENCODE}, + {{'A', 0x81, 0xad, 0}, 0, CES_ERROR_UNASSIGNED_CODE}, + }; + for (const auto& test: cases) { + std::array output {}; + uint32_t consumed = 99; + uint32_t produced = 99; + CHECK(CesMbcsStrToUtf8Str(context, test.source.data(), test.limit, &consumed, output.data(), + output.size(), &produced) == test.error); + CHECK(consumed == 1 && produced == 1); + CHECK(output[0] == 'A' && output[1] == 0); + CHECK(CesMbcsStrGetUtf8Len(context, test.source.data(), test.limit, &consumed, &produced) == + test.error); + CHECK(consumed == 1 && produced == 1); + } +} +} // namespace + +int main() { + CesProfile profile {}; + CesContext context {}; + CHECK(CesUcsProfileInitSJis1997Cp932(&profile) == &profile); + CHECK(CesMbcsUcsContextInit(&context, &profile) == 0); + TestMappingsAndMeasurement(&context); + TestPartialOutput(&context); + TestErrors(&context); + return failures == 0 ? 0 : 1; +} diff --git a/tests/SaveDataDialogTests.cpp b/tests/SaveDataDialogTests.cpp new file mode 100644 index 00000000..77c8f556 --- /dev/null +++ b/tests/SaveDataDialogTests.cpp @@ -0,0 +1,166 @@ +#include "libs/dialog.h" + +#include +#include +#include +#include + +namespace Loader::Timer { +double GetTimeMs() { + return 0.0; +} +} // namespace Loader::Timer + +namespace { +namespace Save = Libs::Dialog::SaveDataDialog; + +struct BaseParam { + uint64_t size; + uint8_t reserved[36]; + uint32_t magic; +}; + +struct Items { + int32_t user_id; + uint32_t pad0; + const void* title_id; + const void* dir_names; + uint32_t dir_names_num; + uint32_t pad1; + const void* new_item; + int32_t focus_pos; + uint32_t pad2; + const void* focus_dir_name; + int32_t item_style; + uint8_t reserved[36]; +}; + +struct UserMessage { + int32_t button_type; + int32_t message_type; + const char* message; + uint8_t reserved[32]; +}; + +struct Param { + BaseParam base; + int32_t size; + int32_t mode; + int32_t display_type; + uint32_t pad0; + void* animation; + Items* items; + UserMessage* user_message; + void* system_message; + void* error_code; + void* progress_bar; + void* user_data; + void* option; + void* wizard; + uint8_t reserved[16]; +}; + +static_assert(sizeof(BaseParam) == 48); +static_assert(sizeof(Items) == 96); +static_assert(sizeof(Param) == 152); +static_assert(offsetof(Param, items) == 72); +static_assert(offsetof(Param, user_data) == 112); + +bool CheckLatestStatus(const char* stage) { + const int updated = Save::SaveDataDialogUpdateStatus(); + const int current = Save::SaveDataDialogGetStatus(); + if (updated != current) { + std::fprintf(stderr, + "SaveDataDialogTests: %s: UpdateStatus returned %d but the current " + "status is %d\n", + stage, updated, current); + return false; + } + return true; +} + +bool CheckStatus(int actual, int expected, const char* stage) { + if (actual != expected) { + std::fprintf(stderr, "SaveDataDialogTests: %s: expected %d, got %d\n", stage, expected, + actual); + return false; + } + return true; +} + +bool TestRunningThenFinished(Param& param) { + bool passed = CheckStatus(Save::SaveDataDialogOpen(¶m), 0, "Open"); + for (int poll = 0; poll < 3; ++poll) { + passed = + CheckStatus(Save::SaveDataDialogGetStatus(), 2, "peek before first update") && passed; + } + passed = CheckStatus(Save::SaveDataDialogUpdateStatus(), 2, "first update") && passed; + for (int poll = 0; poll < 3; ++poll) { + passed = + CheckStatus(Save::SaveDataDialogGetStatus(), 2, "peek after first update") && passed; + } + passed = CheckStatus(Save::SaveDataDialogUpdateStatus(), 3, "second update") && passed; + passed = CheckStatus(Save::SaveDataDialogGetStatus(), 3, "finished peek") && passed; + passed = CheckStatus(Save::SaveDataDialogUpdateStatus(), 3, "finished update") && passed; + return passed; +} + +bool TestCloseAndReset(Param& param) { + bool passed = CheckStatus(Save::SaveDataDialogOpen(¶m), 0, "Open before Close"); + passed = + CheckStatus(Save::SaveDataDialogClose(nullptr), 0, "Close before first poll") && passed; + passed = CheckStatus(Save::SaveDataDialogUpdateStatus(), 3, "poll after Close") && passed; + passed = CheckStatus(Save::SaveDataDialogGetStatus(), 3, "peek after Close") && passed; + passed = TestRunningThenFinished(param) && passed; + + passed = CheckStatus(Save::SaveDataDialogOpen(¶m), 0, "Open before Terminate") && passed; + passed = CheckStatus(Save::SaveDataDialogUpdateStatus(), 2, "poll before Terminate") && passed; + passed = CheckStatus(Save::SaveDataDialogTerminate(), 0, "Terminate while running") && passed; + passed = CheckStatus(Save::SaveDataDialogGetStatus(), 0, "peek after Terminate") && passed; + passed = CheckStatus(Save::SaveDataDialogUpdateStatus(), 0, "poll after Terminate") && passed; + passed = CheckStatus(Save::SaveDataDialogInitialize(), 0, "reinitialize") && passed; + passed = CheckStatus(Save::SaveDataDialogUpdateStatus(), 1, "initialized update") && passed; + passed = TestRunningThenFinished(param) && passed; + return passed; +} +} // namespace + +int main(int argc, char** argv) { + const bool consistency_only = argc == 2 && std::strcmp(argv[1], "--consistency-only") == 0; + if (argc != 1 && !consistency_only) { + std::fprintf(stderr, "usage: SaveDataDialogTests [--consistency-only]\n"); + return 2; + } + Items items {}; + items.user_id = 1000; + items.dir_names_num = 1; // No save-data information is displayed with a null dir_names. + UserMessage message {}; + message.message = "Status polling"; + Param param {}; + param.base.size = sizeof(BaseParam); + param.base.magic = + static_cast(0xc0d1a109u + reinterpret_cast(¶m.base)); + param.size = sizeof(Param); + param.mode = 2; // USER_MSG + param.display_type = 1; // SAVE + param.items = &items; + param.user_message = &message; + + if (Libs::Dialog::CommonDialog::CommonDialogInitialize() != 0 || + Save::SaveDataDialogInitialize() != 0) { + return 2; + } + bool passed = CheckLatestStatus("initialized"); + if (Save::SaveDataDialogOpen(¶m) != 0) { + return 2; + } + passed = CheckLatestStatus("first poll after Open") && passed; + passed = CheckLatestStatus("second poll after Open") && passed; + if (!consistency_only) { + passed = TestRunningThenFinished(param) && passed; + passed = TestCloseAndReset(param) && passed; + } + Save::SaveDataDialogTerminate(); + passed = CheckLatestStatus("terminated") && passed; + return passed ? 0 : 1; +}