diff --git a/Benchmarks/Google/Core/CMakeLists.txt b/Benchmarks/Google/Core/CMakeLists.txt index d18c2ea05fa..329026c30ed 100644 --- a/Benchmarks/Google/Core/CMakeLists.txt +++ b/Benchmarks/Google/Core/CMakeLists.txt @@ -2,6 +2,7 @@ set(COREI_GOOGLEBENCHMARK_SRC GoogleBenchmarkMain.cpp GameEngine/Common/AsciiStringBenchmark.cpp + GameEngine/Common/INIBenchmark.cpp Libraries/Compression/CompressionManagerBenchmark.cpp ) diff --git a/Benchmarks/Google/Core/GameEngine/Common/INIBenchmark.cpp b/Benchmarks/Google/Core/GameEngine/Common/INIBenchmark.cpp new file mode 100644 index 00000000000..45f51fe072c --- /dev/null +++ b/Benchmarks/Google/Core/GameEngine/Common/INIBenchmark.cpp @@ -0,0 +1,99 @@ +/* +** Command & Conquer Generals Zero Hour(tm) +** Copyright 2026 TheSuperHackers +** +** This program is free software: you can redistribute it and/or modify +** it under the terms of the GNU General Public License as published by +** the Free Software Foundation, either version 3 of the License, or +** (at your option) any later version. +** +** This program is distributed in the hope that it will be useful, +** but WITHOUT ANY WARRANTY; without even the implied warranty of +** MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +** GNU General Public License for more details. +** +** You should have received a copy of the GNU General Public License +** along with this program. If not, see . +*/ + +#include +#include +#include + +#include "Common/INI.h" + +enum ScanParser +{ + ScanParser_Current, + ScanParser_Legacy, +}; + +// Ordinary inputs use the fast path. Overflow and underflow (Real only) use the range fallback. +enum ScanInput +{ + ScanInput_Ordinary, + ScanInput_Overflow, + ScanInput_Underflow, +}; + +// Keep the original conversion and failure check, with a call boundary like the production scanners. +// Out-of-range sscanf results characterize the local CRT, not portable C behavior. +template +NOINLINE static Type scanLegacy(const char* token) +{ + Type value; + const char* format = std::is_same_v ? "%f" : std::is_signed_v ? "%d" : "%u"; + if (std::sscanf(token, format, &value) != 1) + throw INI_INVALID_DATA; + return value; +} + +// Each iteration parses four fixed tokens. These are microbenchmarks, not a distribution of game INI inputs. +template +static void BM_INIScan(benchmark::State &state) +{ + const char* const integerTokens[][4] = { + {"0", "+42", "-1000", "2147483647"}, + {"9999999999999999999", "+9999999999999999999", "-9999999999999999999", "18446744073709551616"} + }; + const char* const realTokens[][4] = { + {"0.0", "+1.25", "-1000.5", "3.14159265"}, + {"1e100", "+1e100", "-1e100", "3.5e38"}, + {"1e-100", "+1e-100", "-1e-100", "1e-46"} + }; + const auto& tokens = std::is_same_v ? realTokens[Input] : integerTokens[Input]; + + for (auto _ : state) + { + for (const char* token : tokens) + { + benchmark::DoNotOptimize(token); + Type value; + if constexpr (Parser == ScanParser_Legacy) + value = scanLegacy(token); + else if constexpr (std::is_same_v) + value = INI::scanReal(token); + else if constexpr (std::is_signed_v) + value = INI::scanInt(token); + else + value = INI::scanUnsignedInt(token); + benchmark::DoNotOptimize(value); + } + } + state.SetItemsProcessed(state.iterations() * 4); +} + +BENCHMARK_TEMPLATE(BM_INIScan, Int, ScanParser_Current, ScanInput_Ordinary); +BENCHMARK_TEMPLATE(BM_INIScan, Int, ScanParser_Legacy, ScanInput_Ordinary); +BENCHMARK_TEMPLATE(BM_INIScan, Int, ScanParser_Current, ScanInput_Overflow); +BENCHMARK_TEMPLATE(BM_INIScan, Int, ScanParser_Legacy, ScanInput_Overflow); +BENCHMARK_TEMPLATE(BM_INIScan, UnsignedInt, ScanParser_Current, ScanInput_Ordinary); +BENCHMARK_TEMPLATE(BM_INIScan, UnsignedInt, ScanParser_Legacy, ScanInput_Ordinary); +BENCHMARK_TEMPLATE(BM_INIScan, UnsignedInt, ScanParser_Current, ScanInput_Overflow); +BENCHMARK_TEMPLATE(BM_INIScan, UnsignedInt, ScanParser_Legacy, ScanInput_Overflow); +BENCHMARK_TEMPLATE(BM_INIScan, Real, ScanParser_Current, ScanInput_Ordinary); +BENCHMARK_TEMPLATE(BM_INIScan, Real, ScanParser_Legacy, ScanInput_Ordinary); +BENCHMARK_TEMPLATE(BM_INIScan, Real, ScanParser_Current, ScanInput_Overflow); +BENCHMARK_TEMPLATE(BM_INIScan, Real, ScanParser_Legacy, ScanInput_Overflow); +BENCHMARK_TEMPLATE(BM_INIScan, Real, ScanParser_Current, ScanInput_Underflow); +BENCHMARK_TEMPLATE(BM_INIScan, Real, ScanParser_Legacy, ScanInput_Underflow); diff --git a/Core/GameEngine/Source/Common/INI/INI.cpp b/Core/GameEngine/Source/Common/INI/INI.cpp index a30d5b03cfe..4a7b68b86b3 100644 --- a/Core/GameEngine/Source/Common/INI/INI.cpp +++ b/Core/GameEngine/Source/Common/INI/INI.cpp @@ -67,6 +67,7 @@ #if USE_STD_FROM_CHARS_PARSING #include +#include #include #include #endif @@ -1611,20 +1612,38 @@ void INI::initFromINIMulti( void *what, const MultiIniFieldParse& parseTableList #if USE_STD_FROM_CHARS_PARSING template -Type scanType(std::string_view token) +Type scanType(const char* tokenString) { + std::string_view token(tokenString); DEBUG_ASSERTCRASH(!token.empty(), ("token is not expected to be empty")); // Unlike sscanf, std::from_chars cannot parse "+". - // Consume the plus symbol to accommodate custom ini files that have numbers prefixed with a plus. - if (token[0] == '+') - { - token.remove_prefix(1); - } + // Skip it for from_chars while preserving the original token for the fallback. + const size_t tokenOffset = (token[0] == '+'); + if (tokenOffset && token.size() > 1 && token[1] == '-') + throw INI_INVALID_DATA; // Unlike sscanf, std::from_chars cannot parse "-" as unsigned integer. std::conditional_t, Int64, Type> result{}; - const auto [ptr, ec] = std::from_chars(token.data(), token.data() + token.size(), result); + const auto [ptr, ec] = std::from_chars(token.data() + tokenOffset, token.data() + token.size(), result); + + // Preserve legacy range handling with defined strto* conversions. + if (ec == std::errc::result_out_of_range) + { + char* end; + Type value; + if constexpr (std::is_floating_point_v) + value = std::strtof(tokenString, &end); + else if constexpr (std::is_signed_v) + value = static_cast(std::strtoll(tokenString, &end, 10)); + else + value = static_cast(std::strtoull(tokenString, &end, 10)); + + if (end == tokenString) + throw INI_INVALID_DATA; + + return value; + } if (ec != std::errc{}) { diff --git a/TESTING.md b/TESTING.md index 8389081dbb9..bb58d2591c2 100644 --- a/TESTING.md +++ b/TESTING.md @@ -45,3 +45,13 @@ The test executables use the regular user data folder of the game, so a `RELEASE Benchmarks use [Google Benchmark](https://github.com/TheSuperHackers/google-benchmark). Enable them with `RTS_BUILD_OPTION_BENCHMARKS`. They are organized like the unit tests under Benchmarks/Google, with the executables g_googlebenchmark and z_googlebenchmark. CI builds them, but does not run them. Run benchmarks from a Release build. Select benchmarks with `--benchmark_filter=`, and use `--benchmark_out=.json` to compare the results before and after a change. + +The `BM_INIScan` benchmarks compare the production numeric scanners with the original `sscanf` conversion and failure check. Ordinary inputs exercise `from_chars`; overflow and underflow inputs exercise the range fallback. Each iteration parses four fixed tokens, so divide the reported CPU time by four for ns/token. `items_per_second` already counts individual tokens. Range-error `sscanf` results characterize the local CRT and are not portable C behavior. These measurements do not represent game startup time or the frequency of range errors in game data. + +For example, from an x86 developer shell: +``` +cmake --preset win32 -DRTS_BUILD_OPTION_BENCHMARKS=ON +cmake --build --preset win32 --target z_googlebenchmark g_googlebenchmark +build/win32/Benchmarks/Google/Release/z_googlebenchmark.exe --benchmark_filter=^BM_INIScan --benchmark_min_time=0.2s --benchmark_repetitions=5 --benchmark_enable_random_interleaving=true --benchmark_out=ini-benchmark.json --benchmark_out_format=json +``` +Use the median CPU time for each case when comparing repeated runs. The same filter also works with `g_googlebenchmark.exe`. Relative output paths are resolved from the executable's directory; use an absolute `--benchmark_out` path to save elsewhere. diff --git a/Tests/Google/Core/CMakeLists.txt b/Tests/Google/Core/CMakeLists.txt index f4a850622cf..45353c8a5c3 100644 --- a/Tests/Google/Core/CMakeLists.txt +++ b/Tests/Google/Core/CMakeLists.txt @@ -2,6 +2,7 @@ set(COREI_GOOGLETEST_SRC GoogleTestMain.cpp GameEngine/Common/AsciiStringTest.cpp + GameEngine/Common/INITest.cpp Libraries/Compression/CompressionManagerTest.cpp ) diff --git a/Tests/Google/Core/GameEngine/Common/INITest.cpp b/Tests/Google/Core/GameEngine/Common/INITest.cpp new file mode 100644 index 00000000000..2c507e33d07 --- /dev/null +++ b/Tests/Google/Core/GameEngine/Common/INITest.cpp @@ -0,0 +1,122 @@ +/* +** Command & Conquer Generals Zero Hour(tm) +** Copyright 2026 TheSuperHackers +** +** This program is free software: you can redistribute it and/or modify +** it under the terms of the GNU General Public License as published by +** the Free Software Foundation, either version 3 of the License, or +** (at your option) any later version. +** +** This program is distributed in the hope that it will be useful, +** but WITHOUT ANY WARRANTY; without even the implied warranty of +** MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +** GNU General Public License for more details. +** +** You should have received a copy of the GNU General Public License +** along with this program. If not, see . +*/ + +#include +#include +#include +#include + +#include "Common/INI.h" + +TEST(INI, OversizedIntegersMatchLegacyParser) +{ + const struct { + const char* token; + Int signedExpected; + UnsignedInt unsignedExpected; + } cases[] = { + {"9999999999999999999", -1, 2313682943u}, + {"+9999999999999999999", -1, 2313682943u}, + {"-9999999999999999999", 0, 1981284353u}, + {"9999999999999999999suffix", -1, 2313682943u}, + {"9223372036854775807", -1, 4294967295u}, + {"-9223372036854775808", 0, 0u}, + {"9223372036854775808", -1, 0u}, + {"-9223372036854775809", 0, 4294967295u}, + {"18446744073709551616", -1, 4294967295u}, + {"-18446744073709551616", 0, 4294967295u}, + {"99999999999999999999999999999999999999999999999999", -1, 4294967295u} + }; + // Windows legacy parsing saturates at 64 bits, then keeps the low 32 bits. + for (const auto& test : cases) + { + SCOPED_TRACE(test.token); + EXPECT_EQ(INI::scanInt(test.token), test.signedExpected); + EXPECT_EQ(INI::scanUnsignedInt(test.token), test.unsignedExpected); +#ifdef _WIN32 + // Characterize the Windows CRT; out-of-range scanf conversions are not portable. + Int legacySigned; + UnsignedInt legacyUnsigned; + ASSERT_EQ(std::sscanf(test.token, "%d", &legacySigned), 1); + ASSERT_EQ(std::sscanf(test.token, "%u", &legacyUnsigned), 1); + EXPECT_EQ(legacySigned, test.signedExpected); + EXPECT_EQ(legacyUnsigned, test.unsignedExpected); +#endif + } +} + +TEST(INI, InvalidIntegersAreRejected) +{ + for (const char* token : {"abc", "+", "-", "++1", "+-99999999", "+-9999999999999999999"}) + { + SCOPED_TRACE(token); + EXPECT_THROW(INI::scanInt(token), decltype(INI_INVALID_DATA)); + EXPECT_THROW(INI::scanUnsignedInt(token), decltype(INI_INVALID_DATA)); + Int legacySigned; + UnsignedInt legacyUnsigned; + EXPECT_NE(std::sscanf(token, "%d", &legacySigned), 1); + EXPECT_NE(std::sscanf(token, "%u", &legacyUnsigned), 1); + } +} + +TEST(INI, RealRangeErrorsMatchLegacyParser) +{ + const Real infinity = std::numeric_limits::infinity(); + const Real subnormal = std::numeric_limits::denorm_min(); + const struct { + const char* token; + Real expected; + } cases[] = { + {"1e100", infinity}, + {"+1e100", infinity}, + {"-1e100", -infinity}, + {"3.5e38", infinity}, + {"1e100suffix", infinity}, + {"1e-100", 0.0f}, + {"+1e-100", 0.0f}, + {"-1e-100", -0.0f}, + {"1e-45", subnormal}, + {"-1e-45", -subnormal}, + {"+1.25", 1.25f} + }; + for (const auto& test : cases) + { + SCOPED_TRACE(test.token); + const Real actual = INI::scanReal(test.token); + EXPECT_EQ(actual, test.expected); + EXPECT_EQ(std::signbit(actual), std::signbit(test.expected)); +#ifdef _WIN32 + // As above, scanf is a Windows compatibility check, not a portable overflow oracle. + Real legacy; + ASSERT_EQ(std::sscanf(test.token, "%f", &legacy), 1); + EXPECT_EQ(legacy, test.expected); + EXPECT_EQ(std::signbit(legacy), std::signbit(test.expected)); +#endif + } +} + +TEST(INI, InvalidRealsAreRejected) +{ + for (const char* token : {"abc", "+", "-", "++1.25", "+-1.25", "+-1e100", "+-1e-100"}) + { + SCOPED_TRACE(token); + EXPECT_THROW(INI::scanReal(token), decltype(INI_INVALID_DATA)); + Real legacy; + EXPECT_NE(std::sscanf(token, "%f", &legacy), 1); + } +}