This is a revised version of a patch Giovanni submitted some years ago,
which has been unreviewed until recently.

Tested x86_64-linux. I would like to push this to trunk.

-- >8 --

When RAND_MAX is small and the number of elements being shuffled is
close to it, we get very uneven distributions in std::random_shuffle.

This uses a simple xorshift generator seeded by std::rand if we can't
rely on std::rand itself.

libstdc++-v3/ChangeLog:

        PR libstdc++/88935
        * include/bits/stl_algo.h (random_shuffle) [RAND_MAX < INT_MAX]:
        Use xorshift instead of rand().
        * testsuite/25_algorithms/random_shuffle/88935.cc: New test.

Co-authored-by: Jonathan Wakely <jwak...@redhat.com>
Signed-off-by: Giovanni Bajo <ra...@develer.com>
---
 libstdc++-v3/include/bits/stl_algo.h          | 42 +++++++++++++++----
 .../25_algorithms/random_shuffle/88935.cc     | 24 +++++++++++
 2 files changed, 57 insertions(+), 9 deletions(-)
 create mode 100644 libstdc++-v3/testsuite/25_algorithms/random_shuffle/88935.cc

diff --git a/libstdc++-v3/include/bits/stl_algo.h 
b/libstdc++-v3/include/bits/stl_algo.h
index 541f588883b..778a37ac46f 100644
--- a/libstdc++-v3/include/bits/stl_algo.h
+++ b/libstdc++-v3/include/bits/stl_algo.h
@@ -4521,15 +4521,39 @@ _GLIBCXX_BEGIN_NAMESPACE_ALGO
            _RandomAccessIterator>)
       __glibcxx_requires_valid_range(__first, __last);
 
-      if (__first != __last)
-       for (_RandomAccessIterator __i = __first + 1; __i != __last; ++__i)
-         {
-           // XXX rand() % N is not uniformly distributed
-           _RandomAccessIterator __j = __first
-                                       + std::rand() % ((__i - __first) + 1);
-           if (__i != __j)
-             std::iter_swap(__i, __j);
-         }
+      if (__first == __last)
+       return;
+
+#if RAND_MAX < __INT_MAX__
+      if (__builtin_expect((__last - __first) >= RAND_MAX / 4, 0))
+       {
+         // Use a xorshift implementation seeded by two calls to rand()
+         // instead of using rand() for all the random numbers needed.
+         unsigned __xss
+           = (unsigned)std::rand() ^ ((unsigned)std::rand() << 15);
+         for (_RandomAccessIterator __i = __first + 1; __i != __last; ++__i)
+           {
+             __xss += !__xss;
+             __xss ^= __xss << 13;
+             __xss ^= __xss >> 17;
+             __xss ^= __xss << 5;
+             _RandomAccessIterator __j = __first
+                                           + (__xss % ((__i - __first) + 1));
+             if (__i != __j)
+               std::iter_swap(__i, __j);
+           }
+         return;
+       }
+#endif
+
+      for (_RandomAccessIterator __i = __first + 1; __i != __last; ++__i)
+       {
+         // XXX rand() % N is not uniformly distributed
+         _RandomAccessIterator __j = __first
+                                       + (std::rand() % ((__i - __first) + 1));
+         if (__i != __j)
+           std::iter_swap(__i, __j);
+       }
     }
 
   /**
diff --git a/libstdc++-v3/testsuite/25_algorithms/random_shuffle/88935.cc 
b/libstdc++-v3/testsuite/25_algorithms/random_shuffle/88935.cc
new file mode 100644
index 00000000000..30dca2a897a
--- /dev/null
+++ b/libstdc++-v3/testsuite/25_algorithms/random_shuffle/88935.cc
@@ -0,0 +1,24 @@
+// { dg-do run }
+// { dg-options "-Wno-deprecated-declarations" }
+
+// Bug 88935 std::random_shuffle does not work if the sequence
+// is longer than RAND_MAX elements
+
+#include <algorithm>
+#include <vector>
+#include <testsuite_hooks.h>
+
+int main()
+{
+  const std::size_t N = 30000;
+  std::vector<unsigned char> v(N, (unsigned char)0);
+  std::fill(v.begin() + (N / 5 * 4), v.end(), (unsigned char)1);
+  std::random_shuffle(v.begin(), v.end());
+  double sum = 0;
+  for (std::size_t i = 0; i < v.size(); ++i)
+  {
+    sum += v[i];
+    if (i > 0 && i % (N / 100) == 0)
+      VERIFY( (sum / i) < 0.3 );
+  }
+}
-- 
2.46.0

Reply via email to