bug #1584: Improve random (avoid undefined behavior).

This commit is contained in:
Alexey Frunze 2018-08-08 20:19:32 -07:00
parent caf7e6e7a7
commit e6c8d0b72d
2 changed files with 41 additions and 24 deletions

View File

@ -617,20 +617,27 @@ struct random_default_impl<Scalar, false, true>
{ {
static inline Scalar run(const Scalar& x, const Scalar& y) static inline Scalar run(const Scalar& x, const Scalar& y)
{ {
typedef typename conditional<NumTraits<Scalar>::IsSigned,std::ptrdiff_t,std::size_t>::type ScalarX; if (y <= x)
if(y<x)
return x; return x;
// the following difference might overflow on a 32 bits system, // ScalarU is the unsigned counterpart of Scalar, possibly Scalar itself.
// but since y>=x the result converted to an unsigned long is still correct. typedef typename make_unsigned<Scalar>::type ScalarU;
std::size_t range = ScalarX(y)-ScalarX(x); // ScalarX is the widest of ScalarU and unsigned int.
std::size_t offset = 0; // We'll deal only with ScalarX and unsigned int below thus avoiding signed
// rejection sampling // types and arithmetic and signed overflows (which are undefined behavior).
std::size_t divisor = 1; typedef typename conditional<(ScalarU(-1) > unsigned(-1)), ScalarU, unsigned>::type ScalarX;
std::size_t multiplier = 1; // The following difference doesn't overflow, provided our integer types are two's
if(range<RAND_MAX) divisor = (std::size_t(RAND_MAX)+1)/(range+1); // complement and have the same number of padding bits in signed and unsigned variants.
else multiplier = 1 + range/(std::size_t(RAND_MAX)+1); // This is the case in most modern implementations of C++.
ScalarX range = ScalarX(y) - ScalarX(x);
ScalarX offset = 0;
ScalarX divisor = 1;
ScalarX multiplier = 1;
const unsigned rand_max = RAND_MAX;
if (range <= rand_max) divisor = (rand_max + 1) / (range + 1);
else multiplier = 1 + range / (rand_max + 1);
// Rejection sampling.
do { do {
offset = (std::size_t(std::rand()) * multiplier) / divisor; offset = (unsigned(std::rand()) * multiplier) / divisor;
} while (offset > range); } while (offset > range);
return Scalar(ScalarX(x) + offset); return Scalar(ScalarX(x) + offset);
} }

View File

@ -97,17 +97,27 @@ template<> struct is_arithmetic<unsigned int> { enum { value = true }; };
template<> struct is_arithmetic<signed long> { enum { value = true }; }; template<> struct is_arithmetic<signed long> { enum { value = true }; };
template<> struct is_arithmetic<unsigned long> { enum { value = true }; }; template<> struct is_arithmetic<unsigned long> { enum { value = true }; };
template<typename T> struct is_integral { enum { value = false }; }; #if EIGEN_HAS_CXX11
template<> struct is_integral<bool> { enum { value = true }; }; using std::make_unsigned;
template<> struct is_integral<char> { enum { value = true }; }; #else
template<> struct is_integral<signed char> { enum { value = true }; }; // TODO: Possibly improve this implementation of make_unsigned.
template<> struct is_integral<unsigned char> { enum { value = true }; }; // It is currently used only by
template<> struct is_integral<signed short> { enum { value = true }; }; // template<typename Scalar> struct random_default_impl<Scalar, false, true>.
template<> struct is_integral<unsigned short> { enum { value = true }; }; template<typename> struct make_unsigned;
template<> struct is_integral<signed int> { enum { value = true }; }; template<> struct make_unsigned<char> { typedef unsigned char type; };
template<> struct is_integral<unsigned int> { enum { value = true }; }; template<> struct make_unsigned<signed char> { typedef unsigned char type; };
template<> struct is_integral<signed long> { enum { value = true }; }; template<> struct make_unsigned<unsigned char> { typedef unsigned char type; };
template<> struct is_integral<unsigned long> { enum { value = true }; }; template<> struct make_unsigned<signed short> { typedef unsigned short type; };
template<> struct make_unsigned<unsigned short> { typedef unsigned short type; };
template<> struct make_unsigned<signed int> { typedef unsigned int type; };
template<> struct make_unsigned<unsigned int> { typedef unsigned int type; };
template<> struct make_unsigned<signed long> { typedef unsigned long type; };
template<> struct make_unsigned<unsigned long> { typedef unsigned long type; };
#if EIGEN_COMP_MSVC
template<> struct make_unsigned<signed __int64> { typedef unsigned __int64 type; };
template<> struct make_unsigned<unsigned __int64> { typedef unsigned __int64 type; };
#endif
#endif
template <typename T> struct add_const { typedef const T type; }; template <typename T> struct add_const { typedef const T type; };
template <typename T> struct add_const<T&> { typedef T& type; }; template <typename T> struct add_const<T&> { typedef T& type; };