https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105844

            Bug ID: 105844
           Summary: std::lcm(50000, 49999) is UB but accepted in a
                    constexpr due to cast to unsigned
           Product: gcc
           Version: 12.1.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: goswin-v-b at web dot de
  Target Milestone: ---

Running "gcc-12.1 -std=c++20 -O2 -W -Wall" on

    #include <numeric>
    constinit int t = std::lcm(50000, 49999);

produces

    t:
        .long   -1795017296

The standard says:

> The behavior is undefined if |m|, |n|, or the least common multiple of |m|
> and |n| is not representable as a value of type std::common_type_t<M, N>. 

Which is the case here, the lvm overflows and is undefined. The negative number
produced is not correct and the compile should fail.

The problem is the __absu function in <numeric> casting the arguments to an
unsigned type:

  // std::abs is not constexpr, doesn't support unsigned integers,
  // and std::abs(std::numeric_limits<T>::min()) is undefined.
  template<typename _Up, typename _Tp>
    constexpr _Up
    __absu(_Tp __val)
    {
      static_assert(is_unsigned<_Up>::value, "result type must be unsigned");
      static_assert(sizeof(_Up) >= sizeof(_Tp),
          "result type must be at least as wide as the input type");
      return __val < 0 ? -(_Up)__val : (_Up)__val;
    }

  /// Least common multiple
  template<typename _Mn, typename _Nn>
    constexpr common_type_t<_Mn, _Nn>
    lcm(_Mn __m, _Nn __n) noexcept
    {
      static_assert(is_integral_v<_Mn>, "std::lcm arguments must be integers");
      static_assert(is_integral_v<_Nn>, "std::lcm arguments must be integers");
      static_assert(_Mn(2) == 2, "std::lcm arguments must not be bool");
      static_assert(_Nn(2) == 2, "std::lcm arguments must not be bool");
      using _Up = make_unsigned_t<common_type_t<_Mn, _Nn>>;
      return __detail::__lcm(__detail::__absu<_Up>(__m),
                             __detail::__absu<_Up>(__n));
    }

__lvm is called with unsigned arguments which do not overflow for the given
numbers. And any unsigned overflow would not be undefined behavior. The result
of __lcm is then converted back to the signed type, which is not UB.

I suggest the following changes:

  // LCM implementation
  template<typename _Tp>
    constexpr _Tp
    __lcm(_Tp __m, _Tp __n)
    {
      static_assert(__m == 0 || __n == 0 || __m / __detail::__gcd(__m, __n) <=
std::numeric_limits<_Tp>::max() / __n, "std::lcm not representable in commont
type");
      return (__m != 0 && __n != 0)
        ? (__m / __detail::__gcd(__m, __n)) * __n
        : 0;
    }


  /// Least common multiple
  template<typename _Mn, typename _Nn>
    constexpr common_type_t<_Mn, _Nn>
    lcm(_Mn __m, _Nn __n) noexcept
    {
      static_assert(is_integral_v<_Mn>, "std::lcm arguments must be integers");
      static_assert(is_integral_v<_Nn>, "std::lcm arguments must be integers");
      static_assert(_Mn(2) == 2, "std::lcm arguments must not be bool");
      static_assert(_Nn(2) == 2, "std::lcm arguments must not be bool");
      using _Cp = common_type_t<_Mn, _Nn>;
      using _Up = make_unsigned_t<common_type_t<_Mn, _Nn>>;
      _Up t = __detail::__lcm(__detail::__absu<_Up>(__m),
                              __detail::__absu<_Up>(__n));
      static_assert(t <= (_Up)std::numeric_limits<_Cp>::max(), "std::lcm not
representable in commont type");
      return t;
    }

Reply via email to