Author: marshall Date: Fri Feb 10 14:49:08 2017 New Revision: 294779 URL: http://llvm.org/viewvc/llvm-project?rev=294779&view=rev Log: Make lcm/gcd work better in edge cases. Fixes a UBSAN failure.
Modified: libcxx/trunk/include/experimental/numeric libcxx/trunk/include/numeric libcxx/trunk/test/std/experimental/numeric/numeric.ops/numeric.ops.gcd/gcd.pass.cpp libcxx/trunk/test/std/experimental/numeric/numeric.ops/numeric.ops.lcm/lcm.pass.cpp libcxx/trunk/test/std/numerics/numeric.ops/numeric.ops.gcd/gcd.pass.cpp libcxx/trunk/test/std/numerics/numeric.ops/numeric.ops.lcm/lcm.pass.cpp Modified: libcxx/trunk/include/experimental/numeric URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/experimental/numeric?rev=294779&r1=294778&r2=294779&view=diff ============================================================================== --- libcxx/trunk/include/experimental/numeric (original) +++ libcxx/trunk/include/experimental/numeric Fri Feb 10 14:49:08 2017 @@ -45,18 +45,23 @@ inline namespace fundamentals_v2 { _LIBCPP_BEGIN_NAMESPACE_LFTS_V2 -template <typename _Tp, bool _IsSigned = is_signed<_Tp>::value> struct __abs; +template <typename _Result, typename _Source, bool _IsSigned = is_signed<_Source>::value> struct __abs; -template <typename _Tp> -struct __abs<_Tp, true> { +template <typename _Result, typename _Source> +struct __abs<_Result, _Source, true> { _LIBCPP_CONSTEXPR _LIBCPP_INLINE_VISIBILITY - _Tp operator()(_Tp __t) const noexcept { return __t >= 0 ? __t : -__t; } + _Result operator()(_Source __t) const noexcept + { + if (__t >= 0) return __t; + if (__t == numeric_limits<_Source>::min()) return -static_cast<_Result>(__t); + return -__t; + } }; -template <typename _Tp> -struct __abs<_Tp, false> { +template <typename _Result, typename _Source> +struct __abs<_Result, _Source, false> { _LIBCPP_CONSTEXPR _LIBCPP_INLINE_VISIBILITY - _Tp operator()(_Tp __t) const noexcept { return __t; } + _Result operator()(_Source __t) const noexcept { return __t; } }; @@ -79,8 +84,8 @@ gcd(_Tp __m, _Up __n) static_assert((!is_same<typename remove_cv<_Up>::type, bool>::value), "Second argument to gcd cannot be bool" ); using _Rp = common_type_t<_Tp,_Up>; using _Wp = make_unsigned_t<_Rp>; - return static_cast<_Rp>(__gcd(static_cast<_Wp>(__abs<_Tp>()(__m)), - static_cast<_Wp>(__abs<_Up>()(__n)))); + return static_cast<_Rp>(__gcd(static_cast<_Wp>(__abs<_Rp, _Tp>()(__m)), + static_cast<_Wp>(__abs<_Rp, _Up>()(__n)))); } template<class _Tp, class _Up> @@ -95,8 +100,8 @@ lcm(_Tp __m, _Up __n) return 0; using _Rp = common_type_t<_Tp,_Up>; - _Rp __val1 = __abs<_Tp>()(__m) / gcd(__m,__n); - _Up __val2 = __abs<_Up>()(__n); + _Rp __val1 = __abs<_Rp, _Tp>()(__m) / gcd(__m, __n); + _Rp __val2 = __abs<_Rp, _Up>()(__n); _LIBCPP_ASSERT((numeric_limits<_Rp>::max() / __val1 > __val2), "Overflow in lcm"); return __val1 * __val2; } Modified: libcxx/trunk/include/numeric URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/numeric?rev=294779&r1=294778&r2=294779&view=diff ============================================================================== --- libcxx/trunk/include/numeric (original) +++ libcxx/trunk/include/numeric Fri Feb 10 14:49:08 2017 @@ -201,18 +201,23 @@ iota(_ForwardIterator __first, _ForwardI #if _LIBCPP_STD_VER > 14 -template <typename _Tp, bool _IsSigned = is_signed<_Tp>::value> struct __abs; +template <typename _Result, typename _Source, bool _IsSigned = is_signed<_Source>::value> struct __abs; -template <typename _Tp> -struct __abs<_Tp, true> { +template <typename _Result, typename _Source> +struct __abs<_Result, _Source, true> { _LIBCPP_CONSTEXPR _LIBCPP_INLINE_VISIBILITY - _Tp operator()(_Tp __t) const noexcept { return __t >= 0 ? __t : -__t; } + _Result operator()(_Source __t) const noexcept + { + if (__t >= 0) return __t; + if (__t == numeric_limits<_Source>::min()) return -static_cast<_Result>(__t); + return -__t; + } }; -template <typename _Tp> -struct __abs<_Tp, false> { +template <typename _Result, typename _Source> +struct __abs<_Result, _Source, false> { _LIBCPP_CONSTEXPR _LIBCPP_INLINE_VISIBILITY - _Tp operator()(_Tp __t) const noexcept { return __t; } + _Result operator()(_Source __t) const noexcept { return __t; } }; @@ -220,7 +225,7 @@ template<class _Tp> _LIBCPP_CONSTEXPR _LIBCPP_INLINE_VISIBILITY _Tp __gcd(_Tp __m, _Tp __n) { - static_assert((!is_signed<_Tp>::value), "" ); + static_assert((!is_signed<_Tp>::value), ""); return __n == 0 ? __m : __gcd<_Tp>(__n, __m % __n); } @@ -235,8 +240,8 @@ gcd(_Tp __m, _Up __n) static_assert((!is_same<typename remove_cv<_Up>::type, bool>::value), "Second argument to gcd cannot be bool" ); using _Rp = common_type_t<_Tp,_Up>; using _Wp = make_unsigned_t<_Rp>; - return static_cast<_Rp>(__gcd(static_cast<_Wp>(__abs<_Tp>()(__m)), - static_cast<_Wp>(__abs<_Up>()(__n)))); + return static_cast<_Rp>(__gcd(static_cast<_Wp>(__abs<_Rp, _Tp>()(__m)), + static_cast<_Wp>(__abs<_Rp, _Up>()(__n)))); } template<class _Tp, class _Up> @@ -251,8 +256,8 @@ lcm(_Tp __m, _Up __n) return 0; using _Rp = common_type_t<_Tp,_Up>; - _Rp __val1 = __abs<_Tp>()(__m) / gcd(__m,__n); - _Up __val2 = __abs<_Up>()(__n); + _Rp __val1 = __abs<_Rp, _Tp>()(__m) / gcd(__m, __n); + _Rp __val2 = __abs<_Rp, _Up>()(__n); _LIBCPP_ASSERT((numeric_limits<_Rp>::max() / __val1 > __val2), "Overflow in lcm"); return __val1 * __val2; } Modified: libcxx/trunk/test/std/experimental/numeric/numeric.ops/numeric.ops.gcd/gcd.pass.cpp URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/experimental/numeric/numeric.ops/numeric.ops.gcd/gcd.pass.cpp?rev=294779&r1=294778&r2=294779&view=diff ============================================================================== --- libcxx/trunk/test/std/experimental/numeric/numeric.ops/numeric.ops.gcd/gcd.pass.cpp (original) +++ libcxx/trunk/test/std/experimental/numeric/numeric.ops/numeric.ops.gcd/gcd.pass.cpp Fri Feb 10 14:49:08 2017 @@ -129,4 +129,11 @@ int main() assert((do_test<long, int>(non_cce))); assert((do_test<int, long long>(non_cce))); assert((do_test<long long, int>(non_cce))); + +// LWG#2792 + { + auto res = std::gcd((int64_t)1234, (int32_t)-2147483648); + static_assert( std::is_same<decltype(res), std::common_type<int64_t, int32_t>::type>::value, ""); + assert(res == 2); + } } Modified: libcxx/trunk/test/std/experimental/numeric/numeric.ops/numeric.ops.lcm/lcm.pass.cpp URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/experimental/numeric/numeric.ops/numeric.ops.lcm/lcm.pass.cpp?rev=294779&r1=294778&r2=294779&view=diff ============================================================================== --- libcxx/trunk/test/std/experimental/numeric/numeric.ops/numeric.ops.lcm/lcm.pass.cpp (original) +++ libcxx/trunk/test/std/experimental/numeric/numeric.ops/numeric.ops.lcm/lcm.pass.cpp Fri Feb 10 14:49:08 2017 @@ -128,4 +128,12 @@ int main() assert((do_test<long, int>(non_cce))); assert((do_test<int, long long>(non_cce))); assert((do_test<long long, int>(non_cce))); + +// LWG#2792 + { + auto res1 = std::lcm((int64_t)1234, (int32_t)-2147483648); + (void) std::lcm<int, unsigned long>(INT_MIN, 2); // this used to trigger UBSAN + static_assert( std::is_same<decltype(res1), std::common_type<int64_t, int32_t>::type>::value, ""); + assert(res1 == 1324997410816LL); + } } Modified: libcxx/trunk/test/std/numerics/numeric.ops/numeric.ops.gcd/gcd.pass.cpp URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/numerics/numeric.ops/numeric.ops.gcd/gcd.pass.cpp?rev=294779&r1=294778&r2=294779&view=diff ============================================================================== --- libcxx/trunk/test/std/numerics/numeric.ops/numeric.ops.gcd/gcd.pass.cpp (original) +++ libcxx/trunk/test/std/numerics/numeric.ops/numeric.ops.gcd/gcd.pass.cpp Fri Feb 10 14:49:08 2017 @@ -8,7 +8,6 @@ //===----------------------------------------------------------------------===// // // UNSUPPORTED: c++98, c++03, c++11, c++14 -// XFAIL: ubsan // <numeric> Modified: libcxx/trunk/test/std/numerics/numeric.ops/numeric.ops.lcm/lcm.pass.cpp URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/numerics/numeric.ops/numeric.ops.lcm/lcm.pass.cpp?rev=294779&r1=294778&r2=294779&view=diff ============================================================================== --- libcxx/trunk/test/std/numerics/numeric.ops/numeric.ops.lcm/lcm.pass.cpp (original) +++ libcxx/trunk/test/std/numerics/numeric.ops/numeric.ops.lcm/lcm.pass.cpp Fri Feb 10 14:49:08 2017 @@ -8,7 +8,6 @@ //===----------------------------------------------------------------------===// // // UNSUPPORTED: c++98, c++03, c++11, c++14 -// XFAIL: ubsan // <numeric> // template<class _M, class _N> @@ -132,8 +131,9 @@ int main() // LWG#2837 { - auto res = std::lcm((int64_t)1234, (int32_t)-2147483648); - static_assert( std::is_same<decltype(res), std::common_type<int64_t, int32_t>::type>::value, ""); - assert(res == -1324997410816LL); + auto res1 = std::lcm((int64_t)1234, (int32_t)-2147483648); + (void) std::lcm<int, unsigned long>(INT_MIN, 2); // this used to trigger UBSAN + static_assert( std::is_same<decltype(res1), std::common_type<int64_t, int32_t>::type>::value, ""); + assert(res1 == 1324997410816LL); } } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits