Skip to content

Commit fd62906

Browse files
authored
[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
1 parent c69ea04 commit fd62906

File tree

2 files changed

+31
-4
lines changed

2 files changed

+31
-4
lines changed

libcxx/include/__numeric/gcd_lcm.h

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -117,11 +117,13 @@ constexpr _LIBCPP_HIDE_FROM_ABI common_type_t<_Tp, _Up> lcm(_Tp __m, _Up __n) {
117117
using _Rp = common_type_t<_Tp, _Up>;
118118
_Rp __val1 = __ct_abs<_Rp, _Tp>()(__m) / std::gcd(__m, __n);
119119
_Rp __val2 = __ct_abs<_Rp, _Up>()(__n);
120-
_LIBCPP_ASSERT_ARGUMENT_WITHIN_DOMAIN((numeric_limits<_Rp>::max() / __val1 > __val2), "Overflow in lcm");
121-
return __val1 * __val2;
120+
_Rp __res;
121+
[[maybe_unused]] bool __overflow = __builtin_mul_overflow(__val1, __val2, &__res);
122+
_LIBCPP_ASSERT_ARGUMENT_WITHIN_DOMAIN(!__overflow, "Overflow in lcm");
123+
return __res;
122124
}
123125

124-
#endif // _LIBCPP_STD_VER
126+
#endif // _LIBCPP_STD_VER >= 17
125127

126128
_LIBCPP_END_NAMESPACE_STD
127129

libcxx/test/std/numerics/numeric.ops/numeric.ops.lcm/lcm.pass.cpp

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#include <cassert>
1717
#include <climits>
1818
#include <cstdint>
19+
#include <limits>
1920
#include <type_traits>
2021
#include "test_macros.h"
2122

@@ -89,6 +90,13 @@ constexpr bool do_test(int = 0)
8990
return accumulate;
9091
}
9192

93+
template <class T>
94+
constexpr bool test_limits() {
95+
assert(std::lcm(std::numeric_limits<T>::max() - 1, std::numeric_limits<T>::max() - 1) == std::numeric_limits<T>::max() - 1);
96+
assert(std::lcm(std::numeric_limits<T>::max(), std::numeric_limits<T>::max()) == std::numeric_limits<T>::max());
97+
return true;
98+
}
99+
92100
int main(int argc, char**)
93101
{
94102
int non_cce = argc; // a value that can't possibly be constexpr
@@ -141,5 +149,22 @@ int main(int argc, char**)
141149
assert(res1 == 1324997410816LL);
142150
}
143151

144-
return 0;
152+
// https://github.com/llvm/llvm-project/issues/96196
153+
{
154+
assert(test_limits<unsigned int>());
155+
assert(test_limits<std::uint32_t>());
156+
assert(test_limits<std::uint64_t>());
157+
assert(test_limits<int>());
158+
assert(test_limits<std::int32_t>());
159+
assert(test_limits<std::int64_t>());
160+
161+
static_assert(test_limits<unsigned int>(), "");
162+
static_assert(test_limits<std::uint32_t>(), "");
163+
static_assert(test_limits<std::uint64_t>(), "");
164+
static_assert(test_limits<int>(), "");
165+
static_assert(test_limits<std::int32_t>(), "");
166+
static_assert(test_limits<std::int64_t>(), "");
167+
}
168+
169+
return 0;
145170
}

0 commit comments

Comments
 (0)