From ac657a1c4afd7e4b156029126d11f7b3b36e3ae4 Mon Sep 17 00:00:00 2001 From: Ghabry Date: Thu, 24 Feb 2022 19:59:56 +0100 Subject: [PATCH 1/7] Implement Overflow/Underflow handling of variables Fix #2650 --- src/game_variables.cpp | 62 ++++++++++++++++++++++++++++++++++++++++-- tests/variables.cpp | 42 ++++++++++++++++++++++++++++ 2 files changed, 101 insertions(+), 3 deletions(-) diff --git a/src/game_variables.cpp b/src/game_variables.cpp index 72de2660ac..c8d21c7b78 100644 --- a/src/game_variables.cpp +++ b/src/game_variables.cpp @@ -40,15 +40,71 @@ constexpr Var_t VarSet(Var_t o, Var_t n) { } constexpr Var_t VarAdd(Var_t l, Var_t r) { - return l + r; + Var_t res = 0; + +#ifdef _MSC_VER + res = l + r; + if (res < 0 && l > 0 && r > 0) { + return std::numeric_limits::max(); + } else if (res > 0 && l < 0 && r < 0) { + return std::numeric_limits::min(); + } +#else + if (EP_UNLIKELY(__builtin_add_overflow(l, r, &res))) { + if (l >= 0 && r >= 0) { + return std::numeric_limits::max(); + } + return std::numeric_limits::min(); + } +#endif + + return res; } constexpr Var_t VarSub(Var_t l, Var_t r) { - return l - r; + Var_t res = 0; + +#ifdef _MSC_VER + res = l - r; + if (res < 0 && l > 0 && r < 0) { + return std::numeric_limits::max(); + } else if (res > 0 && l < 0 && r > 0) { + return std::numeric_limits::min(); + } +#else + if (EP_UNLIKELY(__builtin_sub_overflow(l, r, &res))) { + if (r < 0) { + return std::numeric_limits::max(); + } + return std::numeric_limits::min(); + } +#endif + + return res; } constexpr Var_t VarMult(Var_t l, Var_t r) { - return l * r; + Var_t res = 0; + +#ifdef _MSC_VER + res = l * r; + if (l != 0 && res / l != r) { + if ((l > 0 && r > 0) || (l < 0 && r < 0)) { + return std::numeric_limits::max(); + } else { + return std::numeric_limits::min(); + } + } +#else + if (EP_UNLIKELY(__builtin_mul_overflow(l, r, &res))) { + if ((l > 0 && r > 0) || (l < 0 && r < 0)) { + return std::numeric_limits::max(); + } + return std::numeric_limits::min(); + } +#endif + + return res; } constexpr Var_t VarDiv(Var_t n, Var_t d) { diff --git a/tests/variables.cpp b/tests/variables.cpp index 5b9c3ac9ee..4d5fa28e75 100644 --- a/tests/variables.cpp +++ b/tests/variables.cpp @@ -494,6 +494,48 @@ TEST_CASE("RangeRandom") { REQUIRE_NE(first_diff, 0); } +TEST_CASE("Overflow/Underflow") { + lcf::Data::variables.resize(max_vars); + + auto _min = std::numeric_limits::min(); + auto _max = std::numeric_limits::max(); + + Game_Variables v(_min, _max); + v.SetWarning(0); + + v.Set(1, _max); + v.Add(1, 1); + REQUIRE(v.Get(1) == _max); + + v.Set(1, _min); + v.Add(1, -1); + REQUIRE(v.Get(1) == _min); + + v.Set(1, _max); + v.Sub(1, -1); + REQUIRE(v.Get(1) == _max); + + v.Set(1, _min); + v.Sub(1, 1); + REQUIRE(v.Get(1) == _min); + + v.Set(1, _max); + v.Mult(1, 2); + REQUIRE(v.Get(1) == _max); + + v.Set(1, _min); + v.Mult(1, -2); + REQUIRE(v.Get(1) == _max); + + v.Set(1, _max); + v.Mult(1, -2); + REQUIRE(v.Get(1) == _min); + + v.Set(1, _min); + v.Mult(1, 2); + REQUIRE(v.Get(1) == _min); +} + TEST_CASE("Enumerate") { auto s = make(); From 74e3329079e4dedcefb102b088433f2f97391c24 Mon Sep 17 00:00:00 2001 From: Ghabry Date: Thu, 24 Feb 2022 20:00:42 +0100 Subject: [PATCH 2/7] Maniac: Set proper variable limit --- src/player.cpp | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/player.cpp b/src/player.cpp index 9623712e8e..5c587af818 100644 --- a/src/player.cpp +++ b/src/player.cpp @@ -885,11 +885,19 @@ void Player::ResetGameObjects() { auto min_var = lcf::Data::system.easyrpg_variable_min_value; if (min_var == 0) { - min_var = Player::IsRPG2k3() ? Game_Variables::min_2k3 : Game_Variables::min_2k; + if (Player::IsPatchManiac()) { + min_var = std::numeric_limits::min(); + } else { + min_var = Player::IsRPG2k3() ? Game_Variables::min_2k3 : Game_Variables::min_2k; + } } auto max_var = lcf::Data::system.easyrpg_variable_max_value; if (max_var == 0) { - max_var = Player::IsRPG2k3() ? Game_Variables::max_2k3 : Game_Variables::max_2k; + if (Player::IsPatchManiac()) { + max_var = std::numeric_limits::max(); + } else { + max_var = Player::IsRPG2k3() ? Game_Variables::max_2k3 : Game_Variables::max_2k; + } } Main_Data::game_variables = std::make_unique(min_var, max_var); From 7184e29d4aef6dfbc825a6da272d95dd754cbf3b Mon Sep 17 00:00:00 2001 From: Ghabry Date: Thu, 24 Feb 2022 20:25:32 +0100 Subject: [PATCH 3/7] Number Input: Support numbers up to 32bit --- src/game_variables.cpp | 2 +- src/window_numberinput.cpp | 23 +++++++++++++++++------ src/window_numberinput.h | 2 +- 3 files changed, 19 insertions(+), 8 deletions(-) diff --git a/src/game_variables.cpp b/src/game_variables.cpp index c8d21c7b78..af697272e8 100644 --- a/src/game_variables.cpp +++ b/src/game_variables.cpp @@ -617,5 +617,5 @@ StringView Game_Variables::GetName(int _id) const { int Game_Variables::GetMaxDigits() const { auto val = std::max(std::abs(_max), std::abs(_min)); - return std::log10(val) + 1; + return static_cast(std::log10(val) + 1); } diff --git a/src/window_numberinput.cpp b/src/window_numberinput.cpp index ef28f61dc0..c5c0a33abe 100644 --- a/src/window_numberinput.cpp +++ b/src/window_numberinput.cpp @@ -62,8 +62,20 @@ void Window_NumberInput::Refresh() { } } -int Window_NumberInput::GetNumber() { - return number * (plus ? 1 : -1); +int Window_NumberInput::GetNumber() const { + if (plus) { + if (number > std::numeric_limits::max()) { + return std::numeric_limits::max(); + } else { + return static_cast(number); + } + } else { + if (number * -1 < std::numeric_limits::min()) { + return std::numeric_limits::min(); + } else { + return static_cast(-number); + } + } } void Window_NumberInput::SetNumber(int inumber) { @@ -85,9 +97,8 @@ int Window_NumberInput::GetMaxDigits() { } void Window_NumberInput::SetMaxDigits(int idigits_max) { - // At least 7 digits because of gold input in debug scene - // (free space and 6 digits for the gold value) - int top = std::max(7, idigits_max); + // Up to 10 digits (highest 32 bit number) + int top = std::max(10, idigits_max); digits_max = (idigits_max > top) ? top : (idigits_max <= 0) ? 1 : @@ -122,7 +133,7 @@ void Window_NumberInput::Update() { for (int i = 0; i < (digits_max - 1 - (int)index + (int)show_operator); ++i) { place *= 10; } - int n = number / place % 10; + int64_t n = number / place % 10; number -= n * place; if (Input::IsRepeated(Input::UP)) { n = (n + 1) % 10; diff --git a/src/window_numberinput.h b/src/window_numberinput.h index 714c74b729..9916a73712 100644 --- a/src/window_numberinput.h +++ b/src/window_numberinput.h @@ -97,7 +97,7 @@ class Window_NumberInput : public Window_Selectable { void Update() override; protected: - int number; + int64_t number; int digits_max; int cursor_width; int index; From 85e7edf08939c28bd7b65e3d6e07a69e5cb0fc1b Mon Sep 17 00:00:00 2001 From: Ghabry Date: Thu, 24 Feb 2022 20:25:58 +0100 Subject: [PATCH 4/7] Number Input: Make some functions const --- src/window_numberinput.cpp | 4 ++-- src/window_numberinput.h | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/window_numberinput.cpp b/src/window_numberinput.cpp index c5c0a33abe..c3542d845a 100644 --- a/src/window_numberinput.cpp +++ b/src/window_numberinput.cpp @@ -92,7 +92,7 @@ void Window_NumberInput::SetNumber(int inumber) { Refresh(); } -int Window_NumberInput::GetMaxDigits() { +int Window_NumberInput::GetMaxDigits() const { return digits_max; } @@ -108,7 +108,7 @@ void Window_NumberInput::SetMaxDigits(int idigits_max) { Refresh(); } -bool Window_NumberInput::GetShowOperator() { +bool Window_NumberInput::GetShowOperator() const { return show_operator; } diff --git a/src/window_numberinput.h b/src/window_numberinput.h index 9916a73712..d8b94b9d91 100644 --- a/src/window_numberinput.h +++ b/src/window_numberinput.h @@ -47,7 +47,7 @@ class Window_NumberInput : public Window_Selectable { * * @return the currently input number. */ - int GetNumber(); + int GetNumber() const; /** * Sets a new number value. @@ -61,7 +61,7 @@ class Window_NumberInput : public Window_Selectable { * * @return number of displayed digits. */ - int GetMaxDigits(); + int GetMaxDigits() const; /** * Sets the maximal displayed digits. @@ -76,7 +76,7 @@ class Window_NumberInput : public Window_Selectable { * * @return the current operator state */ - bool GetShowOperator(); + bool GetShowOperator() const; /** * Enables or Disables the +- operator before the numbers. From 1aa08e544156b8fda8c9b9bd1f30b7d1717ec310 Mon Sep 17 00:00:00 2001 From: Ghabry Date: Thu, 24 Feb 2022 20:37:21 +0100 Subject: [PATCH 5/7] Number Input: Fix another overflow in SetNumber --- src/window_numberinput.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/window_numberinput.cpp b/src/window_numberinput.cpp index c3542d845a..509b54c7b6 100644 --- a/src/window_numberinput.cpp +++ b/src/window_numberinput.cpp @@ -79,11 +79,11 @@ int Window_NumberInput::GetNumber() const { } void Window_NumberInput::SetNumber(int inumber) { - int num = 1; + int64_t num = 1; for (int i = 0; i < digits_max; ++i) { num *= 10; } - number = min(max(abs(inumber), 0), num - 1); + number = Utils::Clamp(std::llabs(inumber), 0, num - 1); ResetIndex(); plus = inumber >= 0; From 4c2d1626fc58f7aa102d4e0ff193c15382efb296 Mon Sep 17 00:00:00 2001 From: Ghabry Date: Thu, 24 Feb 2022 20:54:52 +0100 Subject: [PATCH 6/7] Number Input: Allow numpad input (extension) --- src/window_numberinput.cpp | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/window_numberinput.cpp b/src/window_numberinput.cpp index 509b54c7b6..d103a8df2c 100644 --- a/src/window_numberinput.cpp +++ b/src/window_numberinput.cpp @@ -161,6 +161,26 @@ void Window_NumberInput::Update() { index = (index + digits_max - 1 + (int)show_operator) % (digits_max + (int)show_operator); } + // Extension: Allow number input through numpad + if (!show_operator || index > 0) { + for (int btn = static_cast(Input::N0); btn <= static_cast(Input::N9); ++btn) { + if (Input::IsTriggered(static_cast(btn))) { + Main_Data::game_system->SePlay(Main_Data::game_system->GetSystemSE(Main_Data::game_system->SFX_Cursor)); + + int place = 1; + for (int i = 0; i < (digits_max - 1 - (int)index + (int)show_operator); ++i) { + place *= 10; + } + int64_t n = number / place % 10; + number -= n * place; + number += (btn - static_cast(Input::N0)) * static_cast(place); + index = (index + 1) % (digits_max + (int)show_operator); + Refresh(); + break; + } + } + } + UpdateCursorRect(); } } From 47f2e8122ec58ba3856d09fef60c6450ca0df563 Mon Sep 17 00:00:00 2001 From: Ghabry Date: Sat, 26 Feb 2022 02:21:49 +0100 Subject: [PATCH 7/7] GetMaxDigits: Fix overflow in Release build --- src/game_variables.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/game_variables.cpp b/src/game_variables.cpp index af697272e8..ba89616524 100644 --- a/src/game_variables.cpp +++ b/src/game_variables.cpp @@ -616,6 +616,6 @@ StringView Game_Variables::GetName(int _id) const { } int Game_Variables::GetMaxDigits() const { - auto val = std::max(std::abs(_max), std::abs(_min)); + auto val = std::max(std::llabs(_max), std::llabs(_min)); return static_cast(std::log10(val) + 1); }