diff mbox

PR libstdc++/60037 - SIGFPE in std::generate_canonical<unsigned int...>

Message ID 53D7B6B6.6010408@verizon.net
State New
Headers show

Commit Message

Ed Smith-Rowland July 29, 2014, 2:59 p.m. UTC 
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  <3dw4rd@verizon.net>

	PR libstdc++/60037 - SIGFPE in std::generate_canonical<unsigned 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.
diff mbox

Patch

Index: include/bits/random.h
===================================================================
--- include/bits/random.h	(revision 213145)
+++ include/bits/random.h	(working copy)
@@ -164,6 +164,8 @@ 
     template<typename _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_cast<size_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::_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 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
+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);
+}