[libcxx] r294779 - Make lcm/gcd work better in edge cases. Fixes a UBSAN failure.

Marshall Clow via cfe-commits cfe-commits at lists.llvm.org
Fri Feb 10 12:49:09 PST 2017


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);
     }
 }




More information about the cfe-commits mailing list