public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug libstdc++/105844] New: std::lcm(50000, 49999) is UB but accepted in a constexpr due to cast to unsigned
@ 2022-06-04  1:08 goswin-v-b at web dot de
  2022-06-04  1:11 ` [Bug libstdc++/105844] " goswin-v-b at web dot de
                   ` (14 more replies)
  0 siblings, 15 replies; 16+ messages in thread
From: goswin-v-b at web dot de @ 2022-06-04  1:08 UTC (permalink / raw)
  To: gcc-bugs

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2023-05-03 20:59 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-04  1:08 [Bug libstdc++/105844] New: std::lcm(50000, 49999) is UB but accepted in a constexpr due to cast to unsigned goswin-v-b at web dot de
2022-06-04  1:11 ` [Bug libstdc++/105844] " goswin-v-b at web dot de
2022-06-04  1:32 ` goswin-v-b at web dot de
2022-06-04  8:24 ` goswin-v-b at web dot de
2022-06-06 14:22 ` [Bug libstdc++/105844] [10/11/12/13 Regression] " redi at gcc dot gnu.org
2022-06-06 15:19 ` redi at gcc dot gnu.org
2022-06-07 10:07 ` redi at gcc dot gnu.org
2022-06-10 14:24 ` cvs-commit at gcc dot gnu.org
2022-06-28 10:49 ` jakub at gcc dot gnu.org
2022-08-03 13:46 ` [Bug libstdc++/105844] [10/11/12 " cvs-commit at gcc dot gnu.org
2022-08-03 13:49 ` [Bug libstdc++/105844] [10/11 " redi at gcc dot gnu.org
2023-05-03 15:02 ` cvs-commit at gcc dot gnu.org
2023-05-03 15:16 ` cvs-commit at gcc dot gnu.org
2023-05-03 16:34 ` cvs-commit at gcc dot gnu.org
2023-05-03 16:35 ` redi at gcc dot gnu.org
2023-05-03 20:59 ` cvs-commit at gcc dot gnu.org

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).