diff options
author | Louis Dionne <ldionne.2@gmail.com> | 2024-06-25 08:35:23 -0500 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-06-25 08:35:23 -0500 |
commit | fd62906ddb252298f6ed63fe85e146d477acdaed (patch) | |
tree | 05b78066b849caae13338d2dd2a0cae5877884df | |
parent | c69ea04fb9738db283263eb350669e00b77ee4fd (diff) | |
download | llvm-fd62906ddb252298f6ed63fe85e146d477acdaed.zip llvm-fd62906ddb252298f6ed63fe85e146d477acdaed.tar.gz llvm-fd62906ddb252298f6ed63fe85e146d477acdaed.tar.bz2 |
[libc++] Fix incorrect overflow checking in std::lcm (#96310)
We should have been using __builtin_mul_overflow from the start instead
of adding a manual (and error-prone) check for overflow.
Fixes #96196
-rw-r--r-- | libcxx/include/__numeric/gcd_lcm.h | 8 | ||||
-rw-r--r-- | libcxx/test/std/numerics/numeric.ops/numeric.ops.lcm/lcm.pass.cpp | 27 |
2 files changed, 31 insertions, 4 deletions
diff --git a/libcxx/include/__numeric/gcd_lcm.h b/libcxx/include/__numeric/gcd_lcm.h index 3317076..9be6cf8 100644 --- a/libcxx/include/__numeric/gcd_lcm.h +++ b/libcxx/include/__numeric/gcd_lcm.h @@ -117,11 +117,13 @@ constexpr _LIBCPP_HIDE_FROM_ABI common_type_t<_Tp, _Up> lcm(_Tp __m, _Up __n) { using _Rp = common_type_t<_Tp, _Up>; _Rp __val1 = __ct_abs<_Rp, _Tp>()(__m) / std::gcd(__m, __n); _Rp __val2 = __ct_abs<_Rp, _Up>()(__n); - _LIBCPP_ASSERT_ARGUMENT_WITHIN_DOMAIN((numeric_limits<_Rp>::max() / __val1 > __val2), "Overflow in lcm"); - return __val1 * __val2; + _Rp __res; + [[maybe_unused]] bool __overflow = __builtin_mul_overflow(__val1, __val2, &__res); + _LIBCPP_ASSERT_ARGUMENT_WITHIN_DOMAIN(!__overflow, "Overflow in lcm"); + return __res; } -#endif // _LIBCPP_STD_VER +#endif // _LIBCPP_STD_VER >= 17 _LIBCPP_END_NAMESPACE_STD diff --git a/libcxx/test/std/numerics/numeric.ops/numeric.ops.lcm/lcm.pass.cpp b/libcxx/test/std/numerics/numeric.ops/numeric.ops.lcm/lcm.pass.cpp index 16bfbbe..edd6d9f 100644 --- a/libcxx/test/std/numerics/numeric.ops/numeric.ops.lcm/lcm.pass.cpp +++ b/libcxx/test/std/numerics/numeric.ops/numeric.ops.lcm/lcm.pass.cpp @@ -16,6 +16,7 @@ #include <cassert> #include <climits> #include <cstdint> +#include <limits> #include <type_traits> #include "test_macros.h" @@ -89,6 +90,13 @@ constexpr bool do_test(int = 0) return accumulate; } +template <class T> +constexpr bool test_limits() { + assert(std::lcm(std::numeric_limits<T>::max() - 1, std::numeric_limits<T>::max() - 1) == std::numeric_limits<T>::max() - 1); + assert(std::lcm(std::numeric_limits<T>::max(), std::numeric_limits<T>::max()) == std::numeric_limits<T>::max()); + return true; +} + int main(int argc, char**) { int non_cce = argc; // a value that can't possibly be constexpr @@ -141,5 +149,22 @@ int main(int argc, char**) assert(res1 == 1324997410816LL); } - return 0; + // https://github.com/llvm/llvm-project/issues/96196 + { + assert(test_limits<unsigned int>()); + assert(test_limits<std::uint32_t>()); + assert(test_limits<std::uint64_t>()); + assert(test_limits<int>()); + assert(test_limits<std::int32_t>()); + assert(test_limits<std::int64_t>()); + + static_assert(test_limits<unsigned int>(), ""); + static_assert(test_limits<std::uint32_t>(), ""); + static_assert(test_limits<std::uint64_t>(), ""); + static_assert(test_limits<int>(), ""); + static_assert(test_limits<std::int32_t>(), ""); + static_assert(test_limits<std::int64_t>(), ""); + } + + return 0; } |