[PATCH] libstdc++: Fix iota_view::size() to avoid overflow

[PATCH] libstdc++: Fix iota_view::size() to avoid overflow

[Jonathan Wakely]

[-- Attachment #1: Type: text/plain, Size: 491 bytes --]

This avoids overfow that occurs when negating the most negative value of
an integral type.

Also prevent returning signed int when the values have lower rank and
promote to int.

libstdc++-v3/ChangeLog:

	* include/std/ranges (ranges::iota_view::size()): Perform all
	calculations in the right unsigned types.
	* testsuite/std/ranges/iota/size.cc: New test.

Tested powerpc64-linux. Not yet pushed.

Does anybody see any reason to stick with exactly what C++20 requires,
despite its bugs?



[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 5328 bytes --]

commit 6e7e3f742bf22139716712908efa83c74c734ed2
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Aug 20 19:44:43 2020

    libstdc++: Fix iota_view::size() to avoid overflow
    
    This avoids overfow that occurs when negating the most negative value of
    an integral type.
    
    Also prevent returning signed int when the values have lower rank and
    promote to int.
    
    libstdc++-v3/ChangeLog:
    
            * include/std/ranges (ranges::iota_view::size()): Perform all
            calculations in the right unsigned types.
            * testsuite/std/ranges/iota/size.cc: New test.

diff --git a/libstdc++-v3/include/std/ranges b/libstdc++-v3/include/std/ranges
index b8023e67c9f..22184006c08 100644
--- a/libstdc++-v3/include/std/ranges
+++ b/libstdc++-v3/include/std/ranges
@@ -889,12 +889,13 @@ namespace ranges
       {
 	using __detail::__is_integer_like;
 	using __detail::__to_unsigned_like;
-	if constexpr (__is_integer_like<_Winc> && __is_integer_like<_Bound>)
-	  return (_M_value < 0)
-	    ? ((_M_bound < 0)
-		? __to_unsigned_like(-_M_value) - __to_unsigned_like(-_M_bound)
-		: __to_unsigned_like(_M_bound) + __to_unsigned_like(-_M_value))
-	    : __to_unsigned_like(_M_bound) - __to_unsigned_like(_M_value);
+	if constexpr (integral<_Winc> && integral<_Bound>)
+	  {
+	    using _Up = make_unsigned_t<decltype(_M_bound - _M_value)>;
+	    return _Up(_M_bound) - _Up(_M_value);
+	  }
+	else if constexpr (__is_integer_like<_Winc>)
+	  return __to_unsigned_like(_M_bound) - __to_unsigned_like(_M_value);
 	else
 	  return __to_unsigned_like(_M_bound - _M_value);
       }
diff --git a/libstdc++-v3/testsuite/std/ranges/iota/size.cc b/libstdc++-v3/testsuite/std/ranges/iota/size.cc
new file mode 100644
index 00000000000..2a9d3870c5d
--- /dev/null
+++ b/libstdc++-v3/testsuite/std/ranges/iota/size.cc
@@ -0,0 +1,110 @@
+// Copyright (C) 2020 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-std=c++2a" }
+// { dg-do compile { target c++2a } }
+
+#include <ranges>
+#include <limits>
+
+template<typename T, typename U>
+constexpr bool
+equal(T t, U u) requires std::same_as<T, U>
+{
+  return t == u;
+}
+
+template<typename W, typename S = std::make_unsigned_t<W>>
+void
+test_integer_iota()
+{
+  using std::numeric_limits;
+
+  using V = std::ranges::iota_view<W, W>;
+  static_assert( std::ranges::sized_range<V> );
+
+  constexpr V zero(0, 0);
+  static_assert( equal(zero.size(), (S)0) );
+
+  constexpr V min(numeric_limits<W>::min(),
+		  numeric_limits<W>::min());
+  static_assert( equal(min.size(), (S)0) );
+
+  constexpr V max(numeric_limits<W>::max(),
+		  numeric_limits<W>::max());
+  static_assert( equal(max.size(), (S)0) );
+
+  constexpr V minmax(numeric_limits<W>::min(),
+		     numeric_limits<W>::max());
+  if constexpr (sizeof(W) < sizeof(S))
+  {
+    using S2 = std::make_unsigned_t<W>;
+    static_assert( equal(minmax.size(), (S)numeric_limits<S2>::max()) );
+  }
+  else
+    static_assert( equal(minmax.size(), numeric_limits<S>::max()) );
+
+  constexpr V pospos(20, 22);
+  static_assert( equal(pospos.size(), (S)2) );
+
+  if constexpr (std::numeric_limits<W>::is_signed)
+  {
+    constexpr V negneg(-20, -2);
+    static_assert( equal(negneg.size(), (S)18) );
+
+    constexpr V negpos(-20, 22);
+    static_assert( equal(negpos.size(), (S)42) );
+  }
+}
+
+void
+test01()
+{
+  test_integer_iota<signed char, unsigned int>();
+  test_integer_iota<signed short, unsigned int>();
+  test_integer_iota<signed int>();
+  test_integer_iota<signed long>();
+  test_integer_iota<signed long long>();
+  test_integer_iota<unsigned char, unsigned int>();
+  test_integer_iota<unsigned short, unsigned int>();
+  test_integer_iota<unsigned int>();
+  test_integer_iota<unsigned long>();
+  test_integer_iota<unsigned long long>();
+
+#ifdef __SIZEOF_INT128__
+  // When the target supports __int128 it can be used in iota_view
+  // even in strict mode where !integral<__int128>.
+  // Specify the size type explicitly, because make_unsigned_t<__int128>
+  // is undefined when !integral<__int128>.
+  test_integer_iota<__int128, unsigned __int128>();
+  test_integer_iota<unsigned __int128, unsigned __int128>();
+#endif
+}
+
+constexpr int arr[3] = { 1, 2, 3 };
+
+void
+test02()
+{
+  constexpr auto v = std::views::iota(std::begin(arr), std::end(arr));
+  static_assert( equal(v.size(), std::make_unsigned_t<std::ptrdiff_t>(3)) );
+
+  constexpr auto vv = std::views::iota(v.begin(), v.end());
+  constexpr auto vvsz = vv.size();
+  static_assert( ! std::numeric_limits<decltype(vvsz)>::is_signed );
+  static_assert( vvsz == 3 );
+}

Re: [PATCH] libstdc++: Fix iota_view::size() to avoid overflow

[Jonathan Wakely]

On 20/08/20 19:54 +0100, Jonathan Wakely wrote:
>This avoids overfow that occurs when negating the most negative value of
>an integral type.
>
>Also prevent returning signed int when the values have lower rank and
>promote to int.
>
>libstdc++-v3/ChangeLog:
>
>	* include/std/ranges (ranges::iota_view::size()): Perform all
>	calculations in the right unsigned types.
>	* testsuite/std/ranges/iota/size.cc: New test.
>
>Tested powerpc64-linux. Not yet pushed.
>
>Does anybody see any reason to stick with exactly what C++20 requires,
>despite its bugs?

I've pushed this now.


>commit 6e7e3f742bf22139716712908efa83c74c734ed2
>Author: Jonathan Wakely <jwakely@redhat.com>
>Date:   Thu Aug 20 19:44:43 2020
>
>    libstdc++: Fix iota_view::size() to avoid overflow
>
>    This avoids overfow that occurs when negating the most negative value of
>    an integral type.
>
>    Also prevent returning signed int when the values have lower rank and
>    promote to int.
>
>    libstdc++-v3/ChangeLog:
>
>            * include/std/ranges (ranges::iota_view::size()): Perform all
>            calculations in the right unsigned types.
>            * testsuite/std/ranges/iota/size.cc: New test.
>
>diff --git a/libstdc++-v3/include/std/ranges b/libstdc++-v3/include/std/ranges
>index b8023e67c9f..22184006c08 100644
>--- a/libstdc++-v3/include/std/ranges
>+++ b/libstdc++-v3/include/std/ranges
>@@ -889,12 +889,13 @@ namespace ranges
>       {
> 	using __detail::__is_integer_like;
> 	using __detail::__to_unsigned_like;
>-	if constexpr (__is_integer_like<_Winc> && __is_integer_like<_Bound>)
>-	  return (_M_value < 0)
>-	    ? ((_M_bound < 0)
>-		? __to_unsigned_like(-_M_value) - __to_unsigned_like(-_M_bound)
>-		: __to_unsigned_like(_M_bound) + __to_unsigned_like(-_M_value))
>-	    : __to_unsigned_like(_M_bound) - __to_unsigned_like(_M_value);
>+	if constexpr (integral<_Winc> && integral<_Bound>)
>+	  {
>+	    using _Up = make_unsigned_t<decltype(_M_bound - _M_value)>;
>+	    return _Up(_M_bound) - _Up(_M_value);
>+	  }
>+	else if constexpr (__is_integer_like<_Winc>)
>+	  return __to_unsigned_like(_M_bound) - __to_unsigned_like(_M_value);
> 	else
> 	  return __to_unsigned_like(_M_bound - _M_value);
>       }
>diff --git a/libstdc++-v3/testsuite/std/ranges/iota/size.cc b/libstdc++-v3/testsuite/std/ranges/iota/size.cc
>new file mode 100644
>index 00000000000..2a9d3870c5d
>--- /dev/null
>+++ b/libstdc++-v3/testsuite/std/ranges/iota/size.cc
>@@ -0,0 +1,110 @@
>+// Copyright (C) 2020 Free Software Foundation, Inc.
>+//
>+// This file is part of the GNU ISO C++ Library.  This library is free
>+// software; you can redistribute it and/or modify it under the
>+// terms of the GNU General Public License as published by the
>+// Free Software Foundation; either version 3, or (at your option)
>+// any later version.
>+
>+// This library is distributed in the hope that it will be useful,
>+// but WITHOUT ANY WARRANTY; without even the implied warranty of
>+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>+// GNU General Public License for more details.
>+
>+// You should have received a copy of the GNU General Public License along
>+// with this library; see the file COPYING3.  If not see
>+// <http://www.gnu.org/licenses/>.
>+
>+// { dg-options "-std=c++2a" }
>+// { dg-do compile { target c++2a } }
>+
>+#include <ranges>
>+#include <limits>
>+
>+template<typename T, typename U>
>+constexpr bool
>+equal(T t, U u) requires std::same_as<T, U>
>+{
>+  return t == u;
>+}
>+
>+template<typename W, typename S = std::make_unsigned_t<W>>
>+void
>+test_integer_iota()
>+{
>+  using std::numeric_limits;
>+
>+  using V = std::ranges::iota_view<W, W>;
>+  static_assert( std::ranges::sized_range<V> );
>+
>+  constexpr V zero(0, 0);
>+  static_assert( equal(zero.size(), (S)0) );
>+
>+  constexpr V min(numeric_limits<W>::min(),
>+		  numeric_limits<W>::min());
>+  static_assert( equal(min.size(), (S)0) );
>+
>+  constexpr V max(numeric_limits<W>::max(),
>+		  numeric_limits<W>::max());
>+  static_assert( equal(max.size(), (S)0) );
>+
>+  constexpr V minmax(numeric_limits<W>::min(),
>+		     numeric_limits<W>::max());
>+  if constexpr (sizeof(W) < sizeof(S))
>+  {
>+    using S2 = std::make_unsigned_t<W>;
>+    static_assert( equal(minmax.size(), (S)numeric_limits<S2>::max()) );
>+  }
>+  else
>+    static_assert( equal(minmax.size(), numeric_limits<S>::max()) );
>+
>+  constexpr V pospos(20, 22);
>+  static_assert( equal(pospos.size(), (S)2) );
>+
>+  if constexpr (std::numeric_limits<W>::is_signed)
>+  {
>+    constexpr V negneg(-20, -2);
>+    static_assert( equal(negneg.size(), (S)18) );
>+
>+    constexpr V negpos(-20, 22);
>+    static_assert( equal(negpos.size(), (S)42) );
>+  }
>+}
>+
>+void
>+test01()
>+{
>+  test_integer_iota<signed char, unsigned int>();
>+  test_integer_iota<signed short, unsigned int>();
>+  test_integer_iota<signed int>();
>+  test_integer_iota<signed long>();
>+  test_integer_iota<signed long long>();
>+  test_integer_iota<unsigned char, unsigned int>();
>+  test_integer_iota<unsigned short, unsigned int>();
>+  test_integer_iota<unsigned int>();
>+  test_integer_iota<unsigned long>();
>+  test_integer_iota<unsigned long long>();
>+
>+#ifdef __SIZEOF_INT128__
>+  // When the target supports __int128 it can be used in iota_view
>+  // even in strict mode where !integral<__int128>.
>+  // Specify the size type explicitly, because make_unsigned_t<__int128>
>+  // is undefined when !integral<__int128>.
>+  test_integer_iota<__int128, unsigned __int128>();
>+  test_integer_iota<unsigned __int128, unsigned __int128>();
>+#endif
>+}
>+
>+constexpr int arr[3] = { 1, 2, 3 };
>+
>+void
>+test02()
>+{
>+  constexpr auto v = std::views::iota(std::begin(arr), std::end(arr));
>+  static_assert( equal(v.size(), std::make_unsigned_t<std::ptrdiff_t>(3)) );
>+
>+  constexpr auto vv = std::views::iota(v.begin(), v.end());
>+  constexpr auto vvsz = vv.size();
>+  static_assert( ! std::numeric_limits<decltype(vvsz)>::is_signed );
>+  static_assert( vvsz == 3 );
>+}