[PATCH ] PR libstdc++/60037 - SIGFPE in std::generate_canonicalunsigned int...
As discussed in the audit trail both _Adaptor and generate_canonical are
both meant to use floating point values. Both are here given
static_asserts to that effect.
This would have prevented this error and might help future users.
The main issue is the use of value_type in _Adaptor and thus in
generate_canonical in hypergeometric_distribution::operator(). This
distribution is a discreet distribution and thus value_type is an
unsigned integer which caused overflow in generate_canonical. In
keeping with practice in all other discreet distributions a double type
will now be used in _Adaptor.
Someday, it might be beneficial to discuss an _IntegralAdaptor and a
corresponding __generate_canonical for use in our discreet distributions
but I want to close this bug with this patch.
Built and tested on x86_64-linux.
OK?
2014-07-29 Ed Smith-Rowland 3dw...@verizon.net
* include/bits/random.h (_Adaptor): static_assert for non floating-point
result type.
* include/bits/random.tcc (generate_canonical): Ditto.
* include/ext/random.tcc (hypergeometric_distribution::operator()):
Use double as a rng result type.
* testsuite/26_numerics/random/pr60037-neg.cc: New.
* testsuite/ext/random/hypergeometric_distribution/pr60037.cc: New.
Index: include/bits/random.h
===
--- include/bits/random.h (revision 213145)
+++ include/bits/random.h (working copy)
@@ -164,6 +164,8 @@
templatetypename _Engine, typename _DInputType
struct _Adaptor
{
+ static_assert(std::is_floating_point_DInputType::value,
+ template argument not a floating point type);
public:
_Adaptor(_Engine __g)
Index: include/bits/random.tcc
===
--- include/bits/random.tcc (revision 213145)
+++ include/bits/random.tcc (working copy)
@@ -3463,6 +3463,9 @@
_RealType
generate_canonical(_UniformRandomNumberGenerator __urng)
{
+ static_assert(std::is_floating_point_RealType::value,
+ template argument not a floating point type);
+
const size_t __b
= std::min(static_castsize_t(std::numeric_limits_RealType::digits),
__bits);
Index: include/ext/random.tcc
===
--- include/ext/random.tcc (revision 213145)
+++ include/ext/random.tcc (working copy)
@@ -1355,7 +1355,7 @@
operator()(_UniformRandomNumberGenerator __urng,
const param_type __param)
{
- std::__detail::_Adaptor_UniformRandomNumberGenerator, result_type
+ std::__detail::_Adaptor_UniformRandomNumberGenerator, double
__aurng(__urng);
result_type __a = __param.successful_size();
Index: testsuite/26_numerics/random/pr60037-neg.cc
===
--- testsuite/26_numerics/random/pr60037-neg.cc (revision 0)
+++ testsuite/26_numerics/random/pr60037-neg.cc (working copy)
@@ -0,0 +1,9 @@
+
+#include random
+
+std::mt19937 urng;
+
+std::__detail::_Adaptorstd::mt19937, unsigned long aurng(urng); // {
dg-error template argument not a floating point type }
+
+auto x = std::generate_canonicalstd::size_t,
+ std::numeric_limitsstd::size_t::digits(urng); // {
dg-error template argument not a floating point type }
Index: testsuite/ext/random/hypergeometric_distribution/pr60037.cc
===
--- testsuite/ext/random/hypergeometric_distribution/pr60037.cc (revision 0)
+++ testsuite/ext/random/hypergeometric_distribution/pr60037.cc (working copy)
@@ -0,0 +1,24 @@
+// { dg-options -std=gnu++11 }
+// { dg-require-cstdint }
+// { dg-require-cmath }
+// { dg-options -O0 }
+
+#include ext/random
+#include functional
+
+void
+hyperplot(unsigned int N, unsigned int K, unsigned int n)
+{
+ std::mt19937 re; // the default engine
+ __gnu_cxx::hypergeometric_distribution hd(N, K, n);
+ auto gen = std::bind(hd, re);
+ gen();
+}
+
+int
+main()
+{
+ hyperplot(15, 3, 2);
+ hyperplot(500, 50, 30);
+ hyperplot(100, 20, 5);
+}
Re: [PATCH ] PR libstdc++/60037 - SIGFPE in std::generate_canonicalunsigned int...
On 29/07/14 04:11 -0400, Ed Smith-Rowland wrote:
As discussed in the audit trail both _Adaptor and generate_canonical
are both meant to use floating point values. Both are here given
static_asserts to that effect.
This would have prevented this error and might help future users.
The main issue is the use of value_type in _Adaptor and thus in
generate_canonical in hypergeometric_distribution::operator(). This
distribution is a discreet distribution and thus value_type is an
unsigned integer which caused overflow in generate_canonical. In
keeping with practice in all other discreet distributions a double
type will now be used in _Adaptor.
Someday, it might be beneficial to discuss an _IntegralAdaptor and a
corresponding __generate_canonical for use in our discreet
distributions but I want to close this bug with this patch.
Makes sense, thanks for the fix.
Built and tested on x86_64-linux.
OK?
One question ...
Index: testsuite/ext/random/hypergeometric_distribution/pr60037.cc
===
--- testsuite/ext/random/hypergeometric_distribution/pr60037.cc (revision 0)
+++ testsuite/ext/random/hypergeometric_distribution/pr60037.cc (working copy)
@@ -0,0 +1,24 @@
+// { dg-options -std=gnu++11 }
+// { dg-require-cstdint }
+// { dg-require-cmath }
+// { dg-options -O0 }
Did you mean to have two separate dg-options directives here?
OK to commit, after either combining them into one or removing the
second one.
Re: [PATCH ] PR libstdc++/60037 - SIGFPE in std::generate_canonicalunsigned int...
On 07/29/2014 04:29 AM, Jonathan Wakely wrote:
On 29/07/14 04:11 -0400, Ed Smith-Rowland wrote:
As discussed in the audit trail both _Adaptor and generate_canonical
are both meant to use floating point values. Both are here given
static_asserts to that effect.
This would have prevented this error and might help future users.
The main issue is the use of value_type in _Adaptor and thus in
generate_canonical in hypergeometric_distribution::operator(). This
distribution is a discreet distribution and thus value_type is an
unsigned integer which caused overflow in generate_canonical. In
keeping with practice in all other discreet distributions a double
type will now be used in _Adaptor.
Someday, it might be beneficial to discuss an _IntegralAdaptor and a
corresponding __generate_canonical for use in our discreet
distributions but I want to close this bug with this patch.
Makes sense, thanks for the fix.
Built and tested on x86_64-linux.
OK?
One question ...
Index: testsuite/ext/random/hypergeometric_distribution/pr60037.cc
===
--- testsuite/ext/random/hypergeometric_distribution/pr60037.cc
(revision 0)
+++ testsuite/ext/random/hypergeometric_distribution/pr60037.cc
(working copy)
@@ -0,0 +1,24 @@
+// { dg-options -std=gnu++11 }
+// { dg-require-cstdint }
+// { dg-require-cmath }
+// { dg-options -O0 }
Did you mean to have two separate dg-options directives here?
OK to commit, after either combining them into one or removing the
second one.
Committed (with small changes to test cases).
Attached new CLand patch.
2014-07-29 Ed Smith-Rowland 3dw...@verizon.net
PR libstdc++/60037 - SIGFPE in std::generate_canonicalunsigned int...
* include/bits/random.h (_Adaptor): static_assert for non floating-point
result type.
* include/bits/random.tcc (generate_canonical): Ditto.
* include/ext/random.tcc (hypergeometric_distribution::operator()):
Use double as a rng result type.
* testsuite/26_numerics/random/pr60037-neg.cc: New.
* testsuite/ext/random/hypergeometric_distribution/pr60037.cc: New.
Index: include/bits/random.h
===
--- include/bits/random.h (revision 213145)
+++ include/bits/random.h (working copy)
@@ -164,6 +164,8 @@
templatetypename _Engine, typename _DInputType
struct _Adaptor
{
+ static_assert(std::is_floating_point_DInputType::value,
+ template argument not a floating point type);
public:
_Adaptor(_Engine __g)
Index: include/bits/random.tcc
===
--- include/bits/random.tcc (revision 213145)
+++ include/bits/random.tcc (working copy)
@@ -3463,6 +3463,9 @@
_RealType
generate_canonical(_UniformRandomNumberGenerator __urng)
{
+ static_assert(std::is_floating_point_RealType::value,
+ template argument not a floating point type);
+
const size_t __b
= std::min(static_castsize_t(std::numeric_limits_RealType::digits),
__bits);
Index: include/ext/random.tcc
===
--- include/ext/random.tcc (revision 213145)
+++ include/ext/random.tcc (working copy)
@@ -1355,7 +1355,7 @@
operator()(_UniformRandomNumberGenerator __urng,
const param_type __param)
{
- std::__detail::_Adaptor_UniformRandomNumberGenerator, result_type
+ std::__detail::_Adaptor_UniformRandomNumberGenerator, double
__aurng(__urng);
result_type __a = __param.successful_size();
Index: testsuite/26_numerics/random/pr60037-neg.cc
===
--- testsuite/26_numerics/random/pr60037-neg.cc (revision 0)
+++ testsuite/26_numerics/random/pr60037-neg.cc (working copy)
@@ -0,0 +1,15 @@
+// { dg-do compile }
+// { dg-options -std=gnu++11 }
+
+#include random
+
+std::mt19937 urng;
+
+std::__detail::_Adaptorstd::mt19937, unsigned long aurng(urng);
+
+auto x = std::generate_canonicalstd::size_t,
+ std::numeric_limitsstd::size_t::digits(urng);
+
+// { dg-error static assertion failed: template argument not a floating point
type { target *-*-* } 167 }
+
+// { dg-error static assertion failed: template argument not a floating point
type { target *-*-* } 3466 }
Index: testsuite/ext/random/hypergeometric_distribution/pr60037.cc
===
--- testsuite/ext/random/hypergeometric_distribution/pr60037.cc (revision 0)
+++ testsuite/ext/random/hypergeometric_distribution/pr60037.cc (working copy)
@@ -0,0 +1,23 @@
+// { dg-options -std=gnu++11 -O0 }
+// { dg-require-cstdint }
+// { dg-require-cmath }
+
+#include ext/random
+#include functional
+
+void
