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

* [Bug libstdc++/105844] std::lcm(50000, 49999) is UB but accepted in a constexpr due to cast to unsigned
  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 ` goswin-v-b at web dot de
  2022-06-04  1:32 ` goswin-v-b at web dot de
                   ` (13 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: goswin-v-b at web dot de @ 2022-06-04  1:11 UTC (permalink / raw)
  To: gcc-bugs

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

Goswin von Brederlow <goswin-v-b at web dot de> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |goswin-v-b at web dot de

--- Comment #1 from Goswin von Brederlow <goswin-v-b at web dot de> ---
Created attachment 53081
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=53081&action=edit
Patch for numeric

Patch for the proposed changes

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

* [Bug libstdc++/105844] std::lcm(50000, 49999) is UB but accepted in a constexpr due to cast to unsigned
  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
                   ` (12 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: goswin-v-b at web dot de @ 2022-06-04  1:32 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #2 from Goswin von Brederlow <goswin-v-b at web dot de> ---
I know the patch doesn't work yet, the static_asserts aren't constexpr. But
hopefully it gives someone enough of an idea to fix it.

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

* [Bug libstdc++/105844] std::lcm(50000, 49999) is UB but accepted in a constexpr due to cast to unsigned
  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
                   ` (11 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: goswin-v-b at web dot de @ 2022-06-04  8:24 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #3 from Goswin von Brederlow <goswin-v-b at web dot de> ---
Created attachment 53082
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=53082&action=edit
Working patch for detecting UB

This will abort if the arguments are too large instead of static_assert, best I
could figure out that would work.

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

* [Bug libstdc++/105844] [10/11/12/13 Regression] std::lcm(50000, 49999) is UB but accepted in a constexpr due to cast to unsigned
  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
                   ` (2 preceding siblings ...)
  2022-06-04  8:24 ` goswin-v-b at web dot de
@ 2022-06-06 14:22 ` redi at gcc dot gnu.org
  2022-06-06 15:19 ` redi at gcc dot gnu.org
                   ` (10 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: redi at gcc dot gnu.org @ 2022-06-06 14:22 UTC (permalink / raw)
  To: gcc-bugs

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2022-06-06
           Priority|P3                          |P2
             Status|UNCONFIRMED                 |ASSIGNED
      Known to fail|                            |10.3.0, 11.1.0, 12.1.0,
                   |                            |13.0, 9.4.0
      Known to work|                            |10.2.0, 9.3.0
     Ever confirmed|0                           |1
            Summary|std::lcm(50000, 49999) is   |[10/11/12/13 Regression]
                   |UB but accepted in a        |std::lcm(50000, 49999) is
                   |constexpr due to cast to    |UB but accepted in a
                   |unsigned                    |constexpr due to cast to
                   |                            |unsigned
           Assignee|unassigned at gcc dot gnu.org      |redi at gcc dot gnu.org
           Keywords|                            |accepts-invalid
   Target Milestone|---                         |10.4

--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> ---
This is a regression introduced by the fix for PR 92978.

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

* [Bug libstdc++/105844] [10/11/12/13 Regression] std::lcm(50000, 49999) is UB but accepted in a constexpr due to cast to unsigned
  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
                   ` (3 preceding siblings ...)
  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
                   ` (9 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: redi at gcc dot gnu.org @ 2022-06-06 15:19 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #5 from Jonathan Wakely <redi at gcc dot gnu.org> ---
I think the fix we want is simply:

--- a/libstdc++-v3/include/std/numeric
+++ b/libstdc++-v3/include/std/numeric
@@ -68,6 +68,7 @@
 #if __cplusplus >= 201402L
 # include <type_traits>
 # include <bit>
+# include <ext/numeric_traits.h>
 #endif

 #if __cplusplus >= 201703L
@@ -102,7 +103,15 @@ namespace __detail
       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;
+
+      if (__val >= 0)
+       return static_cast<_Up>(__val);
+
+      if _GLIBCXX17_CONSTEXPR (sizeof(_Up) == sizeof(_Tp))
+       {
+         __glibcxx_assert(__val != __gnu_cxx::__int_traits<_Tp>::__min);
+       }
+      return -static_cast<_Up>(__val);
     }

   template<typename _Up> void __absu(bool) = delete;

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

* [Bug libstdc++/105844] [10/11/12/13 Regression] std::lcm(50000, 49999) is UB but accepted in a constexpr due to cast to unsigned
  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
                   ` (4 preceding siblings ...)
  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
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: redi at gcc dot gnu.org @ 2022-06-07 10:07 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #5)
> I think the fix we want is simply:

I meant to say the fix we want for __absu. We still need to detect overflow
separately.

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

* [Bug libstdc++/105844] [10/11/12/13 Regression] std::lcm(50000, 49999) is UB but accepted in a constexpr due to cast to unsigned
  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
                   ` (5 preceding siblings ...)
  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
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2022-06-10 14:24 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #7 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:671970a5621e18e7079b4ca113e56434c858db66

commit r13-1040-g671970a5621e18e7079b4ca113e56434c858db66
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Fri Jun 10 14:39:13 2022 +0100

    libstdc++: Make std::lcm and std::gcd detect overflow [PR105844]

    When I fixed PR libstdc++/92978 I introduced a regression whereby
    std::lcm(INT_MIN, 1) and std::lcm(50000, 49999) would no longer produce
    errors during constant evaluation. Those calls are undefined, because
    they violate the preconditions that |m| and the result can be
    represented in the return type (which is int in both those cases). The
    regression occurred because __absu<unsigned>(INT_MIN) is well-formed,
    due to the explicit casts to unsigned in that new helper function, and
    the out-of-range multiplication is well-formed, because unsigned
    arithmetic wraps instead of overflowing.

    To fix 92978 I made std::gcm and std::lcm calculate |m| and |n|
    immediately, yielding a common unsigned type that was used to calculate
    the result. That was partly correct, but there's no need to use an
    unsigned type. Doing so only suppresses the overflow errors so the
    compiler can't detect them. This change replaces __absu with __abs_r
    that returns the common type (not its corresponding unsigned type). This
    way we can detect overflow in __abs_r when required, while still
    supporting the most-negative value when it can be represented in the
    result type. To detect LCM results that are out of range of the result
    type we still need explicit checks, because neither constant evaluation
    nor UBsan will complain about unsigned wrapping for cases such as
    std::lcm(500000u, 499999u). We can detect those overflows efficiently by
    using __builtin_mul_overflow and asserting.

    libstdc++-v3/ChangeLog:

            PR libstdc++/105844
            * include/experimental/numeric (experimental::gcd): Simplify
            assertions. Use __abs_r instead of __absu.
            (experimental::lcm): Likewise. Remove use of __detail::__lcm so
            overflow can be detected.
            * include/std/numeric (__detail::__absu): Rename to __abs_r and
            change to allow signed result type, so overflow can be detected.
            (__detail::__lcm): Remove.
            (gcd): Simplify assertions. Use __abs_r instead of __absu.
            (lcm): Likewise. Remove use of __detail::__lcm so overflow can
            be detected.
            * testsuite/26_numerics/gcd/gcd_neg.cc: Adjust dg-error lines.
            * testsuite/26_numerics/lcm/lcm_neg.cc: Likewise.
            * testsuite/26_numerics/gcd/105844.cc: New test.
            * testsuite/26_numerics/lcm/105844.cc: New test.

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

* [Bug libstdc++/105844] [10/11/12/13 Regression] std::lcm(50000, 49999) is UB but accepted in a constexpr due to cast to unsigned
  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
                   ` (6 preceding siblings ...)
  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
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: jakub at gcc dot gnu.org @ 2022-06-28 10:49 UTC (permalink / raw)
  To: gcc-bugs

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|10.4                        |10.5

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 10.4 is being released, retargeting bugs to GCC 10.5.

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

* [Bug libstdc++/105844] [10/11/12 Regression] std::lcm(50000, 49999) is UB but accepted in a constexpr due to cast to unsigned
  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
                   ` (7 preceding siblings ...)
  2022-06-28 10:49 ` jakub at gcc dot gnu.org
@ 2022-08-03 13:46 ` cvs-commit at gcc dot gnu.org
  2022-08-03 13:49 ` [Bug libstdc++/105844] [10/11 " redi at gcc dot gnu.org
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2022-08-03 13:46 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #9 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-12 branch has been updated by Jonathan Wakely
<redi@gcc.gnu.org>:

https://gcc.gnu.org/g:8a57deb926cd660c2eae7ed621d61a301ae0d523

commit r12-8654-g8a57deb926cd660c2eae7ed621d61a301ae0d523
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Fri Jun 10 14:39:13 2022 +0100

    libstdc++: Make std::lcm and std::gcd detect overflow [PR105844]

    When I fixed PR libstdc++/92978 I introduced a regression whereby
    std::lcm(INT_MIN, 1) and std::lcm(50000, 49999) would no longer produce
    errors during constant evaluation. Those calls are undefined, because
    they violate the preconditions that |m| and the result can be
    represented in the return type (which is int in both those cases). The
    regression occurred because __absu<unsigned>(INT_MIN) is well-formed,
    due to the explicit casts to unsigned in that new helper function, and
    the out-of-range multiplication is well-formed, because unsigned
    arithmetic wraps instead of overflowing.

    To fix 92978 I made std::gcm and std::lcm calculate |m| and |n|
    immediately, yielding a common unsigned type that was used to calculate
    the result. That was partly correct, but there's no need to use an
    unsigned type. Doing so only suppresses the overflow errors so the
    compiler can't detect them. This change replaces __absu with __abs_r
    that returns the common type (not its corresponding unsigned type). This
    way we can detect overflow in __abs_r when required, while still
    supporting the most-negative value when it can be represented in the
    result type. To detect LCM results that are out of range of the result
    type we still need explicit checks, because neither constant evaluation
    nor UBsan will complain about unsigned wrapping for cases such as
    std::lcm(500000u, 499999u). We can detect those overflows efficiently by
    using __builtin_mul_overflow and asserting.

    libstdc++-v3/ChangeLog:

            PR libstdc++/105844
            * include/experimental/numeric (experimental::gcd): Simplify
            assertions. Use __abs_r instead of __absu.
            (experimental::lcm): Likewise. Remove use of __detail::__lcm so
            overflow can be detected.
            * include/std/numeric (__detail::__absu): Rename to __abs_r and
            change to allow signed result type, so overflow can be detected.
            (__detail::__lcm): Remove.
            (gcd): Simplify assertions. Use __abs_r instead of __absu.
            (lcm): Likewise. Remove use of __detail::__lcm so overflow can
            be detected.
            * testsuite/26_numerics/gcd/gcd_neg.cc: Adjust dg-error lines.
            * testsuite/26_numerics/lcm/lcm_neg.cc: Likewise.
            * testsuite/26_numerics/gcd/105844.cc: New test.
            * testsuite/26_numerics/lcm/105844.cc: New test.

    (cherry picked from commit 671970a5621e18e7079b4ca113e56434c858db66)

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

* [Bug libstdc++/105844] [10/11 Regression] std::lcm(50000, 49999) is UB but accepted in a constexpr due to cast to unsigned
  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
                   ` (8 preceding siblings ...)
  2022-08-03 13:46 ` [Bug libstdc++/105844] [10/11/12 " cvs-commit at gcc dot gnu.org
@ 2022-08-03 13:49 ` redi at gcc dot gnu.org
  2023-05-03 15:02 ` cvs-commit at gcc dot gnu.org
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: redi at gcc dot gnu.org @ 2022-08-03 13:49 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #10 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Backported for 12.2

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

* [Bug libstdc++/105844] [10/11 Regression] std::lcm(50000, 49999) is UB but accepted in a constexpr due to cast to unsigned
  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
                   ` (9 preceding siblings ...)
  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
                   ` (3 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2023-05-03 15:02 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #11 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by Jonathan Wakely
<redi@gcc.gnu.org>:

https://gcc.gnu.org/g:7c31a9c66269805d57591549edb031f84d84f9b0

commit r11-10739-g7c31a9c66269805d57591549edb031f84d84f9b0
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Fri Jun 10 14:39:13 2022 +0100

    libstdc++: Make std::lcm and std::gcd detect overflow [PR105844]

    When I fixed PR libstdc++/92978 I introduced a regression whereby
    std::lcm(INT_MIN, 1) and std::lcm(50000, 49999) would no longer produce
    errors during constant evaluation. Those calls are undefined, because
    they violate the preconditions that |m| and the result can be
    represented in the return type (which is int in both those cases). The
    regression occurred because __absu<unsigned>(INT_MIN) is well-formed,
    due to the explicit casts to unsigned in that new helper function, and
    the out-of-range multiplication is well-formed, because unsigned
    arithmetic wraps instead of overflowing.

    To fix 92978 I made std::gcm and std::lcm calculate |m| and |n|
    immediately, yielding a common unsigned type that was used to calculate
    the result. That was partly correct, but there's no need to use an
    unsigned type. Doing so only suppresses the overflow errors so the
    compiler can't detect them. This change replaces __absu with __abs_r
    that returns the common type (not its corresponding unsigned type). This
    way we can detect overflow in __abs_r when required, while still
    supporting the most-negative value when it can be represented in the
    result type. To detect LCM results that are out of range of the result
    type we still need explicit checks, because neither constant evaluation
    nor UBsan will complain about unsigned wrapping for cases such as
    std::lcm(500000u, 499999u). We can detect those overflows efficiently by
    using __builtin_mul_overflow and asserting.

    libstdc++-v3/ChangeLog:

            PR libstdc++/105844
            * include/experimental/numeric (experimental::gcd): Simplify
            assertions. Use __abs_r instead of __absu.
            (experimental::lcm): Likewise. Remove use of __detail::__lcm so
            overflow can be detected.
            * include/std/numeric (__detail::__absu): Rename to __abs_r and
            change to allow signed result type, so overflow can be detected.
            (__detail::__lcm): Remove.
            (gcd): Simplify assertions. Use __abs_r instead of __absu.
            (lcm): Likewise. Remove use of __detail::__lcm so overflow can
            be detected.
            * testsuite/26_numerics/gcd/gcd_neg.cc: Adjust dg-error lines.
            * testsuite/26_numerics/lcm/lcm_neg.cc: Likewise.
            * testsuite/26_numerics/gcd/105844.cc: New test.
            * testsuite/26_numerics/lcm/105844.cc: New test.

    (cherry picked from commit 671970a5621e18e7079b4ca113e56434c858db66)

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

* [Bug libstdc++/105844] [10/11 Regression] std::lcm(50000, 49999) is UB but accepted in a constexpr due to cast to unsigned
  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
                   ` (10 preceding siblings ...)
  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
                   ` (2 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2023-05-03 15:16 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #12 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by Jonathan Wakely
<redi@gcc.gnu.org>:

https://gcc.gnu.org/g:ad076469c00af1fbe2f848844ab5937587772a70

commit r11-10740-gad076469c00af1fbe2f848844ab5937587772a70
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed May 3 16:05:23 2023 +0100

    libstdc++: Fix broken backport for std::lcm [PR105844]

    The backport for gcc-11 was incomplete and should have replaced all uses
    of std::__is_constant_evaluated with __builtin_is_constant_evaluated.

    We also need to prune some additional output for the new tests, because
    the r12-4425-g1595fe44e11a96 change to always prune those lines is not
    present on the branch.

    libstdc++-v3/ChangeLog:

            PR libstdc++/105844
            * include/experimental/numeric (lcm): Use built-in instead of
            __is_constant_evaluated.
            * include/std/numeric (__abs_r, lcm): Likewise.
            * testsuite/26_numerics/gcd/105844.cc: Add dg-prune-output.
            * testsuite/26_numerics/lcm/105844.cc: Likewise.

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

* [Bug libstdc++/105844] [10/11 Regression] std::lcm(50000, 49999) is UB but accepted in a constexpr due to cast to unsigned
  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
                   ` (11 preceding siblings ...)
  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
  14 siblings, 0 replies; 16+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2023-05-03 16:34 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #13 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Jonathan Wakely
<redi@gcc.gnu.org>:

https://gcc.gnu.org/g:14175f2262c94868e26f01eef1e14418f2b5e6a9

commit r10-11388-g14175f2262c94868e26f01eef1e14418f2b5e6a9
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Fri Jun 10 14:39:13 2022 +0100

    libstdc++: Make std::lcm and std::gcd detect overflow [PR105844]

    When I fixed PR libstdc++/92978 I introduced a regression whereby
    std::lcm(INT_MIN, 1) and std::lcm(50000, 49999) would no longer produce
    errors during constant evaluation. Those calls are undefined, because
    they violate the preconditions that |m| and the result can be
    represented in the return type (which is int in both those cases). The
    regression occurred because __absu<unsigned>(INT_MIN) is well-formed,
    due to the explicit casts to unsigned in that new helper function, and
    the out-of-range multiplication is well-formed, because unsigned
    arithmetic wraps instead of overflowing.

    To fix 92978 I made std::gcm and std::lcm calculate |m| and |n|
    immediately, yielding a common unsigned type that was used to calculate
    the result. That was partly correct, but there's no need to use an
    unsigned type. Doing so only suppresses the overflow errors so the
    compiler can't detect them. This change replaces __absu with __abs_r
    that returns the common type (not its corresponding unsigned type). This
    way we can detect overflow in __abs_r when required, while still
    supporting the most-negative value when it can be represented in the
    result type. To detect LCM results that are out of range of the result
    type we still need explicit checks, because neither constant evaluation
    nor UBsan will complain about unsigned wrapping for cases such as
    std::lcm(500000u, 499999u). We can detect those overflows efficiently by
    using __builtin_mul_overflow and asserting.

    libstdc++-v3/ChangeLog:

            PR libstdc++/105844
            * include/experimental/numeric (experimental::gcd): Simplify
            assertions. Use __abs_r instead of __absu.
            (experimental::lcm): Likewise. Remove use of __detail::__lcm so
            overflow can be detected.
            * include/std/numeric (__detail::__absu): Rename to __abs_r and
            change to allow signed result type, so overflow can be detected.
            (__detail::__lcm): Remove.
            (gcd): Simplify assertions. Use __abs_r instead of __absu.
            (lcm): Likewise. Remove use of __detail::__lcm so overflow can
            be detected.
            * testsuite/26_numerics/gcd/gcd_neg.cc: Adjust dg-error lines.
            * testsuite/26_numerics/lcm/lcm_neg.cc: Likewise.
            * testsuite/26_numerics/gcd/105844.cc: New test.
            * testsuite/26_numerics/lcm/105844.cc: New test.

    (cherry picked from commit 671970a5621e18e7079b4ca113e56434c858db66)

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

* [Bug libstdc++/105844] [10/11 Regression] std::lcm(50000, 49999) is UB but accepted in a constexpr due to cast to unsigned
  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
                   ` (12 preceding siblings ...)
  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
  14 siblings, 0 replies; 16+ messages in thread
From: redi at gcc dot gnu.org @ 2023-05-03 16:35 UTC (permalink / raw)
  To: gcc-bugs

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #14 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Fixed for 11.4 and 10.5

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

* [Bug libstdc++/105844] [10/11 Regression] std::lcm(50000, 49999) is UB but accepted in a constexpr due to cast to unsigned
  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
                   ` (13 preceding siblings ...)
  2023-05-03 16:35 ` redi at gcc dot gnu.org
@ 2023-05-03 20:59 ` cvs-commit at gcc dot gnu.org
  14 siblings, 0 replies; 16+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2023-05-03 20:59 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #15 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Jonathan Wakely
<redi@gcc.gnu.org>:

https://gcc.gnu.org/g:b357af31b8d1e93f0f70133e25d3ad4045f7a32b

commit r10-11389-gb357af31b8d1e93f0f70133e25d3ad4045f7a32b
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed May 3 21:40:01 2023 +0100

    libstdc++: Ensure constexpr std::lcm detects out-of-range result [PR105844]

    On the gcc-10 branch, __glibcxx_assert does not unconditionally check
    the condition during constant evaluation. This means we need an explicit
    additional check for std::lcm results that cannot be represented in an
    unsigned result type.

    libstdc++-v3/ChangeLog:

            PR libstdc++/105844
            * include/std/numeric (lcm): Ensure out-of-range result is
            detected in constant evaluation.
            * testsuite/26_numerics/lcm/105844.cc: Adjust dg-error string.

^ 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).