From 40e73b9134024aa70d95c8dccb59e22cae795b64 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Sun, 28 Jun 2020 01:32:43 +0300 Subject: [PATCH 1/3] Fix FPE, step 1 --- src/Functions/DivisionUtils.h | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/Functions/DivisionUtils.h b/src/Functions/DivisionUtils.h index 5a0c1b8232a..11adb5f4268 100644 --- a/src/Functions/DivisionUtils.h +++ b/src/Functions/DivisionUtils.h @@ -44,6 +44,13 @@ inline bool divisionLeadsToFPE(A a, B b) return false; } +template +inline auto checkedDivision(A a, B b) +{ + throwIfDivisionLeadsToFPE(a, b); + return a / b; +} + #pragma GCC diagnostic pop @@ -56,14 +63,12 @@ struct DivideIntegralImpl template static inline Result apply(A a, B b) { - throwIfDivisionLeadsToFPE(a, b); - /// Otherwise overflow may occur due to integer promotion. Example: int8_t(-1) / uint64_t(2). /// NOTE: overflow is still possible when dividing large signed number to large unsigned number or vice-versa. But it's less harmful. if constexpr (is_integral_v && is_integral_v && (is_signed_v || is_signed_v)) - return std::make_signed_t(a) / std::make_signed_t(b); + return checkedDivision(std::make_signed_t(a), std::make_signed_t(b)); else - return a / b; + return checkedDivision(a, b); } #if USE_EMBEDDED_COMPILER From ae67ea3ab373118ed1082a46c582d4358e5e4111 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Sun, 28 Jun 2020 01:37:46 +0300 Subject: [PATCH 2/3] Make it more correct --- src/Functions/DivisionUtils.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Functions/DivisionUtils.h b/src/Functions/DivisionUtils.h index 11adb5f4268..cab947b92a2 100644 --- a/src/Functions/DivisionUtils.h +++ b/src/Functions/DivisionUtils.h @@ -66,7 +66,8 @@ struct DivideIntegralImpl /// Otherwise overflow may occur due to integer promotion. Example: int8_t(-1) / uint64_t(2). /// NOTE: overflow is still possible when dividing large signed number to large unsigned number or vice-versa. But it's less harmful. if constexpr (is_integral_v && is_integral_v && (is_signed_v || is_signed_v)) - return checkedDivision(std::make_signed_t(a), std::make_signed_t(b)); + return checkedDivision(std::make_signed_t(a), + sizeof(A) > sizeof(B) ? std::make_signed_t(b) : std::make_signed_t(b)); else return checkedDivision(a, b); } From 58d52c37153a19b93f4731e91b3083c4abf81406 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Sun, 28 Jun 2020 01:37:52 +0300 Subject: [PATCH 3/3] Add a test --- .../0_stateless/01350_intdiv_nontrivial_fpe.reference | 3 +++ tests/queries/0_stateless/01350_intdiv_nontrivial_fpe.sql | 5 +++++ 2 files changed, 8 insertions(+) create mode 100644 tests/queries/0_stateless/01350_intdiv_nontrivial_fpe.reference create mode 100644 tests/queries/0_stateless/01350_intdiv_nontrivial_fpe.sql diff --git a/tests/queries/0_stateless/01350_intdiv_nontrivial_fpe.reference b/tests/queries/0_stateless/01350_intdiv_nontrivial_fpe.reference new file mode 100644 index 00000000000..ff5ca85eb55 --- /dev/null +++ b/tests/queries/0_stateless/01350_intdiv_nontrivial_fpe.reference @@ -0,0 +1,3 @@ +-36170086419038336 +-140739635871744 +-2147483648 diff --git a/tests/queries/0_stateless/01350_intdiv_nontrivial_fpe.sql b/tests/queries/0_stateless/01350_intdiv_nontrivial_fpe.sql new file mode 100644 index 00000000000..29dfb2c3fda --- /dev/null +++ b/tests/queries/0_stateless/01350_intdiv_nontrivial_fpe.sql @@ -0,0 +1,5 @@ +select intDiv(-9223372036854775808, 255); +select intDiv(-9223372036854775808, 65535); +select intDiv(-9223372036854775808, 4294967295); +select intDiv(-9223372036854775808, 18446744073709551615); -- { serverError 153 } +select intDiv(-9223372036854775808, -1); -- { serverError 153 }