On 25/03/21 18:45 +0100, François Dumont via Libstdc++ wrote:
On 25/03/21 4:29 pm, Jonathan Wakely wrote:
On 25/03/21 14:48 +0000, Jonathan Wakely wrote:
On 25/03/21 13:05 +0000, Jonathan Wakely wrote:
On 24/03/21 22:48 +0100, François Dumont wrote:
I still need to find out why, when running test on
__gnu_debug::basic_string after the std::basic_string one, the
generate(sz) call always returns sz.
The "random" generator will always return the same sequence of numbers
every time you run the test. It uses a default-constructed
std::mt19937 without a seed, so the sequence of random numbers is 100%
reproducable.
This patch allows those random engines to be seeded, so that we can test
with different random numbers.
It's already found a bug:
GLIBCXX_SEED_TEST_RNG=-941908610 make check
RUNTESTFLAGS=conformance.exp=23_containers/forward_list/requirements/exception/generation_prohibited.cc
Using random seed 3353058686
FAIL: 23_containers/forward_list/requirements/exception/generation_prohibited.cc
execution test
We need to investigate that.
Oh, it's the same generate(sz) bug as you already found. But I've
found other bugs, e.g. with GLIBCXX_SEED_TEST_RNG=1908970375).
I think we should also add a check for non-empty containers to those
test functions, and ensure we don't try to erase from empty
containers (see attached).
Yes, I also realized this empty container potential issue.
Please go ahead with any of your patches, I'll just adapt if you push first.
OK, thanks. I've pushed the attached patch. You should only need to
undo the generate(sz) changes in your patch.
I will commit in a couple of hours.
Great, thanks.
commit c7fc73ee459045edabb99816658f14f32d23bf92
Author: Jonathan Wakely <jwak...@redhat.com>
Date: Thu Mar 25 13:51:08 2021
libstdc++: Allow seeding random engines in testsuite
The testsuite utilities that use random numbers use a
default-constructed mersenne_twister_engine, meaning the values are
reproducable. This adds support for seeding them, controlledby an
environment variable. Defining GLIBCXX_SEED_TEST_RNG=val in the
environment will cause the engines to be seeded with atoi(val) if that
is non-zero, or with a value read from std::random_device otherwise.
Running with different seeds revealed some bugs in the tests, where a
randomly selected iterator was past-the-end (which can't be erased), or
where the randomly populated container was empty, and then we tried to
remove elements from it unconditionally.
libstdc++-v3/ChangeLog:
* testsuite/util/exception/safety.h (setup_base::generate):
Support seeding random engine.
(erase_point, erase_range): Adjust range of random numbers to
ensure dereferenceable iterators are used where required.
(generation_prohibited::run): Do not try to erase from empty
containers.
* testsuite/util/testsuite_containergen.h (test_containers):
Support seeding random engine.
diff --git a/libstdc++-v3/testsuite/util/exception/safety.h b/libstdc++-v3/testsuite/util/exception/safety.h
index 6c91e740e0d..2e5d8acae00 100644
--- a/libstdc++-v3/testsuite/util/exception/safety.h
+++ b/libstdc++-v3/testsuite/util/exception/safety.h
@@ -22,6 +22,8 @@
#include <testsuite_container_traits.h>
#include <ext/throw_allocator.h>
+#include <cstdlib> // getenv, atoi
+#include <cstdio> // printf, fflush
// Container requirement testing.
namespace __gnu_test
@@ -33,27 +35,34 @@ namespace __gnu_test
typedef std::uniform_int_distribution<size_type> distribution_type;
typedef std::mt19937 engine_type;
+ static engine_type
+ get_engine()
+ {
+ engine_type engine;
+ if (const char* v = std::getenv("GLIBCXX_SEED_TEST_RNG"))
+ {
+ // A single seed value is much smaller than the mt19937 state size,
+ // but we're not trying to be cryptographically secure here.
+ int s = std::atoi(v);
+ if (s == 0)
+ s = (int)std::random_device{}();
+ std::printf("Using random seed %d\n", s);
+ std::fflush(stdout);
+ engine.seed((unsigned)s);
+ }
+ return engine;
+ }
+
// Return randomly generated integer on range [0, __max_size].
static size_type
generate(size_type __max_size)
{
- // Make the generator static...
- const engine_type engine;
- const distribution_type distribution;
- static auto generator = std::bind(distribution, engine,
- std::placeholders::_1);
+ using param_type = typename distribution_type::param_type;
- // ... but set the range for this particular invocation here.
- const typename distribution_type::param_type p(0, __max_size);
- size_type random = generator(p);
- if (random < distribution.min() || random > distribution.max())
- std::__throw_out_of_range_fmt(__N("setup_base::generate\n"
- "random number generated is: %zu "
- "out of range [%zu, %zu]\n"),
- (size_t)random,
- (size_t)distribution.min(),
- (size_t)distribution.max());
- return random;
+ // Make the engine and distribution static...
+ static engine_type engine = get_engine();
+ static distribution_type distribution;
+ return distribution(engine, param_type{0, __max_size});
}
// Given an instantiating type, return a unique value.
@@ -309,10 +318,13 @@ namespace __gnu_test
// computed with begin() and end().
const size_type sz = std::distance(__container.begin(),
__container.end());
+ // Container::erase(pos) requires dereferenceable pos.
+ if (sz == 0)
+ throw std::logic_error("erase_point: empty container");
// NB: Lowest common denominator: use forward iterator operations.
auto i = __container.begin();
- std::advance(i, generate(sz));
+ std::advance(i, generate(sz - 1));
// Makes it easier to think of this as __container.erase(i)
(__container.*_F_erase_point)(i);
@@ -337,12 +349,15 @@ namespace __gnu_test
// computed with begin() and end().
const size_type sz = std::distance(__container.begin(),
__container.end());
+ // forward_list::erase_after(pos) requires dereferenceable pos.
+ if (sz == 0)
+ throw std::logic_error("erase_point: empty container");
// NB: Lowest common denominator: use forward iterator operations.
auto i = __container.before_begin();
- std::advance(i, generate(sz));
+ std::advance(i, generate(sz - 1));
- // Makes it easier to think of this as __container.erase(i)
+ // Makes it easier to think of this as __container.erase_after(i)
(__container.*_F_erase_point)(i);
}
catch(const __gnu_cxx::forced_error&)
@@ -405,14 +420,19 @@ namespace __gnu_test
{
const size_type sz = std::distance(__container.begin(),
__container.end());
- size_type s1 = generate(sz);
- size_type s2 = generate(sz);
+ // forward_list::erase_after(pos, last) requires a pos != last
+ if (sz == 0)
+ return; // Caller doesn't check for this, not a logic error.
+
+ size_type s1 = generate(sz - 1);
+ size_type s2 = generate(sz - 1);
auto i1 = __container.before_begin();
auto i2 = __container.before_begin();
std::advance(i1, std::min(s1, s2));
- std::advance(i2, std::max(s1, s2));
+ std::advance(i2, std::max(s1, s2) + 1);
- // Makes it easier to think of this as __container.erase(i1, i2).
+ // Makes it easier to think of this as
+ // __container.erase_after(i1, i2).
(__container.*_F_erase_range)(i1, i2);
}
catch(const __gnu_cxx::forced_error&)
@@ -1454,16 +1474,25 @@ namespace __gnu_test
// constructor or assignment operator of value_type throws.
if (!traits<container_type>::has_throwing_erase::value)
{
- typename base_type::erase_point erasep;
- erasep(container);
+ if (!container.empty())
+ {
+ typename base_type::erase_point erasep;
+ erasep(container);
+ }
typename base_type::erase_range eraser;
eraser(container);
}
- typename base_type::pop_front popf;
- popf(container);
- typename base_type::pop_back popb;
- popb(container);
+ if (!container.empty())
+ {
+ typename base_type::pop_front popf;
+ popf(container);
+ }
+ if (!container.empty())
+ {
+ typename base_type::pop_back popb;
+ popb(container);
+ }
typename base_type::iterator_ops iops;
iops(container);
diff --git a/libstdc++-v3/testsuite/util/testsuite_containergen.h b/libstdc++-v3/testsuite/util/testsuite_containergen.h
index a2156733ec6..c468c7f4415 100644
--- a/libstdc++-v3/testsuite/util/testsuite_containergen.h
+++ b/libstdc++-v3/testsuite/util/testsuite_containergen.h
@@ -20,6 +20,8 @@
#include <testsuite_container_traits.h>
#include <random>
+#include <cstdlib> // getenv, atoi
+#include <cstdio> // printf, fflush
namespace __gnu_test
{
@@ -63,6 +65,18 @@ namespace __gnu_test
{
std::mt19937_64 random_gen;
+ if (const char* v = std::getenv("GLIBCXX_SEED_TEST_RNG"))
+ {
+ // A single seed value is much smaller than the mt19937 state size,
+ // but we're not trying to be cryptographically secure here.
+ int s = std::atoi(v);
+ if (s == 0)
+ s = (int)std::random_device{}();
+ std::printf("Using random seed %d\n", s);
+ std::fflush(stdout);
+ random_gen.seed((unsigned)s);
+ }
+
#ifdef SIMULATOR_TEST
int loops = 10;
#else
