public inbox for libstdc++@gcc.gnu.org
 help / color / mirror / Atom feed
* [committed] libstdc++: Fix divide by zero in default template argument
@ 2020-10-07 23:45 Jonathan Wakely
  2020-10-08 13:50 ` Jonathan Wakely
  0 siblings, 1 reply; 2+ messages in thread
From: Jonathan Wakely @ 2020-10-07 23:45 UTC (permalink / raw)
  To: libstdc++, gcc-patches

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

libstdc++-v3/ChangeLog:

	* include/bits/random.h (__detail::_Mod): Avoid divide by zero.
	* testsuite/26_numerics/random/linear_congruential_engine/operators/call.cc:
	New test.

Tested powerpc64le-linux. Committed to trunk.


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

commit 6ae17a3b6835b30102607d45ac89c7a668e2c8d4
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Oct 8 00:34:56 2020

    libstdc++: Fix divide by zero in default template argument
    
    libstdc++-v3/ChangeLog:
    
            * include/bits/random.h (__detail::_Mod): Avoid divide by zero.
            * testsuite/26_numerics/random/linear_congruential_engine/operators/call.cc:
            New test.

diff --git a/libstdc++-v3/include/bits/random.h b/libstdc++-v3/include/bits/random.h
index 4a6558c966a..920f3d91513 100644
--- a/libstdc++-v3/include/bits/random.h
+++ b/libstdc++-v3/include/bits/random.h
@@ -109,7 +109,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     template<typename _Tp, _Tp __m, _Tp __a, _Tp __c,
 	     bool __big_enough = (!(__m & (__m - 1))
 				  || (_Tp(-1) - __c) / __a >= __m - 1),
-             bool __schrage_ok = __m % __a < __m / __a>
+             bool __schrage_ok = __a != 0 && __m % __a < __m / __a>
       struct _Mod
       {
 	typedef typename _Select_uint_least_t<std::__lg(__a)
diff --git a/libstdc++-v3/testsuite/26_numerics/random/linear_congruential_engine/operators/call.cc b/libstdc++-v3/testsuite/26_numerics/random/linear_congruential_engine/operators/call.cc
new file mode 100644
index 00000000000..d1fff6d0a5d
--- /dev/null
+++ b/libstdc++-v3/testsuite/26_numerics/random/linear_congruential_engine/operators/call.cc
@@ -0,0 +1,27 @@
+// 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-do compile { target c++11 } }
+
+#include <random>
+
+unsigned
+test01()
+{
+  std::linear_congruential_engine<unsigned, 0, 0, 0> l;
+  return l(); // this used to result in divide by zero
+}

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

* Re: [committed] libstdc++: Fix divide by zero in default template argument
  2020-10-07 23:45 [committed] libstdc++: Fix divide by zero in default template argument Jonathan Wakely
@ 2020-10-08 13:50 ` Jonathan Wakely
  0 siblings, 0 replies; 2+ messages in thread
From: Jonathan Wakely @ 2020-10-08 13:50 UTC (permalink / raw)
  To: libstdc++, gcc-patches

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

On 08/10/20 00:45 +0100, Jonathan Wakely wrote:
>libstdc++-v3/ChangeLog:
>
>	* include/bits/random.h (__detail::_Mod): Avoid divide by zero.
>	* testsuite/26_numerics/random/linear_congruential_engine/operators/call.cc:
>	New test.
>

That didn't work properly in all cases. Here's a better fix.

Tested powerpc64le-linux. Committed to trunk.



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

commit c06617a79b41da37d80d7e88a3dbc56818f3e706
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Oct 8 14:01:00 2020

    libstdc++: Avoid divide by zero in default template arguments
    
    My previous attempt to fix this only worked when m is a power of two.
    There is still a bug when a=00 and !has_single_bit(m).
    
    Instead of trying to make _Mod work for a==0 this change ensures that we
    never instantiate it with a==0. For C++17 we can use if-constexpr, but
    otherwise we need to use a different multipler. It doesn't matter what
    we use, as it won't actually be called, only instantiated.
    
    libstdc++-v3/ChangeLog:
    
            * include/bits/random.h (__detail::_Mod): Revert last change.
            (__detail::__mod): Do not use _Mod for a==0 case.
            * testsuite/26_numerics/random/linear_congruential_engine/operators/call.cc:
            Check other cases with a==0. Also check runtime results.
            * testsuite/26_numerics/random/pr60037-neg.cc: Adjust dg-error
            line.

diff --git a/libstdc++-v3/include/bits/random.h b/libstdc++-v3/include/bits/random.h
index 920f3d91513..0be1191e07d 100644
--- a/libstdc++-v3/include/bits/random.h
+++ b/libstdc++-v3/include/bits/random.h
@@ -109,7 +109,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     template<typename _Tp, _Tp __m, _Tp __a, _Tp __c,
 	     bool __big_enough = (!(__m & (__m - 1))
 				  || (_Tp(-1) - __c) / __a >= __m - 1),
-             bool __schrage_ok = __a != 0 && __m % __a < __m / __a>
+             bool __schrage_ok = __m % __a < __m / __a>
       struct _Mod
       {
 	typedef typename _Select_uint_least_t<std::__lg(__a)
@@ -146,7 +146,16 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     template<typename _Tp, _Tp __m, _Tp __a = 1, _Tp __c = 0>
       inline _Tp
       __mod(_Tp __x)
-      { return _Mod<_Tp, __m, __a, __c>::__calc(__x); }
+      {
+	if _GLIBCXX17_CONSTEXPR (__a == 0)
+	  return __c;
+	else
+	  {
+	    // _Mod must not be instantiated with a == 0
+	    constexpr _Tp __a1 = __a ? __a : 1;
+	    return _Mod<_Tp, __m, __a1, __c>::__calc(__x);
+	  }
+      }
 
     /*
      * An adaptor class for converting the output of any Generator into
diff --git a/libstdc++-v3/testsuite/26_numerics/random/linear_congruential_engine/operators/call.cc b/libstdc++-v3/testsuite/26_numerics/random/linear_congruential_engine/operators/call.cc
index d1fff6d0a5d..0000aa2402f 100644
--- a/libstdc++-v3/testsuite/26_numerics/random/linear_congruential_engine/operators/call.cc
+++ b/libstdc++-v3/testsuite/26_numerics/random/linear_congruential_engine/operators/call.cc
@@ -15,13 +15,50 @@
 // with this library; see the file COPYING3.  If not see
 // <http://www.gnu.org/licenses/>.
 
-// { dg-do compile { target c++11 } }
+// { dg-do run { target c++11 } }
 
 #include <random>
+#include <testsuite_hooks.h>
 
-unsigned
+void
 test01()
 {
   std::linear_congruential_engine<unsigned, 0, 0, 0> l;
-  return l(); // this used to result in divide by zero
+  auto r = l(); // this used to result in divide by zero
+  VERIFY( r == 0 );
+  l.seed(2);
+  r = l();
+  VERIFY( r == 0 );
+  VERIFY( l() == 0 );
+}
+
+void
+test02()
+{
+  std::linear_congruential_engine<unsigned, 0, 0, 3> l;
+  auto r = l(); // this used to result in a different divide by zero
+  VERIFY( r == 0 );
+  l.seed(2);
+  r = l();
+  VERIFY( r == 0 );
+  VERIFY( l() == 0 );
+}
+
+void
+test03()
+{
+  std::linear_congruential_engine<unsigned, 0, 2, 3> l;
+  auto r = l();
+  VERIFY( r == 2 );
+  l.seed(4);
+  r = l();
+  VERIFY( r == 2 );
+  VERIFY( l() == 2 );
+}
+
+int main()
+{
+  test01();
+  test02();
+  test03();
 }
diff --git a/libstdc++-v3/testsuite/26_numerics/random/pr60037-neg.cc b/libstdc++-v3/testsuite/26_numerics/random/pr60037-neg.cc
index f808132e9ea..139abbb3051 100644
--- a/libstdc++-v3/testsuite/26_numerics/random/pr60037-neg.cc
+++ b/libstdc++-v3/testsuite/26_numerics/random/pr60037-neg.cc
@@ -10,6 +10,6 @@ std::__detail::_Adaptor<std::mt19937, unsigned long> aurng(urng);
 auto x = std::generate_canonical<std::size_t,
 			std::numeric_limits<std::size_t>::digits>(urng);
 
-// { dg-error "static assertion failed: template argument must be a floating point type" "" { target *-*-* } 158 }
+// { dg-error "static assertion failed: template argument must be a floating point type" "" { target *-*-* } 167 }
 
 // { dg-error "static assertion failed: template argument must be a floating point type" "" { target *-*-* } 3281 }

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

end of thread, other threads:[~2020-10-08 13:50 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-07 23:45 [committed] libstdc++: Fix divide by zero in default template argument Jonathan Wakely
2020-10-08 13:50 ` Jonathan Wakely

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