| Message ID | 70988952-dab7-3ba6-4694-2d90c035f80f@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Fix tests sensitive to internal library allocations | expand |
On Mon, 21 Aug 2023 at 18:05, François Dumont via Libstdc++ <libstdc++@gcc.gnu.org> wrote: > > Hi > > Here is a propocal to fix tests sensitive to libstdc++ internal allocations. Surely the enter() and exit() calls should be a constructor and destructor? The constructor could use count() to get the count, and then restore it in the destructor. Something like: --- a/libstdc++-v3/testsuite/util/replacement_memory_operators.h +++ b/libstdc++-v3/testsuite/util/replacement_memory_operators.h @@ -75,12 +75,30 @@ namespace __gnu_test counter& cntr = get(); cntr._M_increments = cntr._M_decrements = 0; } + + struct scope + { + scope() : _M_count(counter::count()) { } + ~scope() { counter::get()._M_count = _M_count; } + + private: + std::size_t _M_count; + +#if __cplusplus >= 201103L + scope(const scope&) = delete; + scope& operator=(const scope&) = delete; +#else + scope(const scope&); + scope& operator=(const scope&); +#endif + }; }; template<typename Alloc, bool uses_global_new> bool check_new(Alloc a = Alloc()) { + __gnu_test::counter::scope s; __gnu_test::counter::exceptions(false); __gnu_test::counter::reset(); (void) a.allocate(10); > > Tested by restoring allocation in tzdb.cc. > > As announced I'm also adding a test to detect such allocations. If it is > ok let me know if you prefer to see it in a different place. The test is a good idea. I think 17_intro/no_library_allocation.cc would be a better place for it. > > libstdc++: Fix tests relying on operator new/delete overload > > Fix tests that are checking for an allocation plan. They are failing if > an allocation is taking place outside the test. > > libstdc++-v3/ChangeLog > > * testsuite/util/replacement_memory_operators.h > (counter::_M_pre_enter_count): New. > (counter::enter, counter::exit): New static methods to call > on main() enter/exit. > * testsuite/23_containers/unordered_map/96088.cc (main): > Call __gnu_test::counter::enter/exit. > * testsuite/23_containers/unordered_multimap/96088.cc > (main): Likewise. > * testsuite/23_containers/unordered_multiset/96088.cc > (main): Likewise. > * testsuite/23_containers/unordered_set/96088.cc (main): > Likewise. > * testsuite/ext/malloc_allocator/deallocate_local.cc > (main): Likewise. > * testsuite/ext/new_allocator/deallocate_local.cc (main): > Likewise. > * testsuite/ext/throw_allocator/deallocate_local.cc (main): > Likewise. > * testsuite/ext/pool_allocator/allocate_chunk.cc (started): > New global. > (operator new(size_t)): Check started. > (main): Set/Unset started. > * testsuite/ext/no_library_allocation.cc: New test case. > > Ok to commit ? > > François
Here is the updated and tested patch. On 21/08/2023 20:07, Jonathan Wakely wrote: > On Mon, 21 Aug 2023 at 18:05, François Dumont via Libstdc++ > <libstdc++@gcc.gnu.org> wrote: >> Hi >> >> Here is a propocal to fix tests sensitive to libstdc++ internal allocations. > Surely the enter() and exit() calls should be a constructor and destructor? > > The constructor could use count() to get the count, and then restore > it in the destructor. Something like: > > --- a/libstdc++-v3/testsuite/util/replacement_memory_operators.h > +++ b/libstdc++-v3/testsuite/util/replacement_memory_operators.h > @@ -75,12 +75,30 @@ namespace __gnu_test > counter& cntr = get(); > cntr._M_increments = cntr._M_decrements = 0; > } > + > + struct scope > + { > + scope() : _M_count(counter::count()) { } > + ~scope() { counter::get()._M_count = _M_count; } > + > + private: > + std::size_t _M_count; > + > +#if __cplusplus >= 201103L > + scope(const scope&) = delete; > + scope& operator=(const scope&) = delete; > +#else > + scope(const scope&); > + scope& operator=(const scope&); > +#endif > + }; > }; > > template<typename Alloc, bool uses_global_new> > bool > check_new(Alloc a = Alloc()) > { > + __gnu_test::counter::scope s; > __gnu_test::counter::exceptions(false); > __gnu_test::counter::reset(); > (void) a.allocate(10); > > > > > > >> Tested by restoring allocation in tzdb.cc. >> >> As announced I'm also adding a test to detect such allocations. If it is >> ok let me know if you prefer to see it in a different place. > The test is a good idea. I think 17_intro/no_library_allocation.cc > would be a better place for it. > >> libstdc++: Fix tests relying on operator new/delete overload >> >> Fix tests that are checking for an allocation plan. They are failing if >> an allocation is taking place outside the test. >> >> libstdc++-v3/ChangeLog >> >> * testsuite/util/replacement_memory_operators.h >> (counter::_M_pre_enter_count): New. >> (counter::enter, counter::exit): New static methods to call >> on main() enter/exit. >> * testsuite/23_containers/unordered_map/96088.cc (main): >> Call __gnu_test::counter::enter/exit. >> * testsuite/23_containers/unordered_multimap/96088.cc >> (main): Likewise. >> * testsuite/23_containers/unordered_multiset/96088.cc >> (main): Likewise. >> * testsuite/23_containers/unordered_set/96088.cc (main): >> Likewise. >> * testsuite/ext/malloc_allocator/deallocate_local.cc >> (main): Likewise. >> * testsuite/ext/new_allocator/deallocate_local.cc (main): >> Likewise. >> * testsuite/ext/throw_allocator/deallocate_local.cc (main): >> Likewise. >> * testsuite/ext/pool_allocator/allocate_chunk.cc (started): >> New global. >> (operator new(size_t)): Check started. >> (main): Set/Unset started. >> * testsuite/ext/no_library_allocation.cc: New test case. >> >> Ok to commit ? >> >> François
On Mon, 21 Aug 2023 at 21:20, François Dumont <frs.dumont@gmail.com> wrote: > > Here is the updated and tested patch. OK for trunk, thanks. We could consider it for the branches too (I'm going to remove the global strings on the gcc-13 branch tomorrow). > > On 21/08/2023 20:07, Jonathan Wakely wrote: > > On Mon, 21 Aug 2023 at 18:05, François Dumont via Libstdc++ > > <libstdc++@gcc.gnu.org> wrote: > >> Hi > >> > >> Here is a propocal to fix tests sensitive to libstdc++ internal allocations. > > Surely the enter() and exit() calls should be a constructor and destructor? > > > > The constructor could use count() to get the count, and then restore > > it in the destructor. Something like: > > > > --- a/libstdc++-v3/testsuite/util/replacement_memory_operators.h > > +++ b/libstdc++-v3/testsuite/util/replacement_memory_operators.h > > @@ -75,12 +75,30 @@ namespace __gnu_test > > counter& cntr = get(); > > cntr._M_increments = cntr._M_decrements = 0; > > } > > + > > + struct scope > > + { > > + scope() : _M_count(counter::count()) { } > > + ~scope() { counter::get()._M_count = _M_count; } > > + > > + private: > > + std::size_t _M_count; > > + > > +#if __cplusplus >= 201103L > > + scope(const scope&) = delete; > > + scope& operator=(const scope&) = delete; > > +#else > > + scope(const scope&); > > + scope& operator=(const scope&); > > +#endif > > + }; > > }; > > > > template<typename Alloc, bool uses_global_new> > > bool > > check_new(Alloc a = Alloc()) > > { > > + __gnu_test::counter::scope s; > > __gnu_test::counter::exceptions(false); > > __gnu_test::counter::reset(); > > (void) a.allocate(10); > > > > > > > > > > > > > >> Tested by restoring allocation in tzdb.cc. > >> > >> As announced I'm also adding a test to detect such allocations. If it is > >> ok let me know if you prefer to see it in a different place. > > The test is a good idea. I think 17_intro/no_library_allocation.cc > > would be a better place for it. > > > >> libstdc++: Fix tests relying on operator new/delete overload > >> > >> Fix tests that are checking for an allocation plan. They are failing if > >> an allocation is taking place outside the test. > >> > >> libstdc++-v3/ChangeLog > >> > >> * testsuite/util/replacement_memory_operators.h > >> (counter::_M_pre_enter_count): New. > >> (counter::enter, counter::exit): New static methods to call > >> on main() enter/exit. > >> * testsuite/23_containers/unordered_map/96088.cc (main): > >> Call __gnu_test::counter::enter/exit. > >> * testsuite/23_containers/unordered_multimap/96088.cc > >> (main): Likewise. > >> * testsuite/23_containers/unordered_multiset/96088.cc > >> (main): Likewise. > >> * testsuite/23_containers/unordered_set/96088.cc (main): > >> Likewise. > >> * testsuite/ext/malloc_allocator/deallocate_local.cc > >> (main): Likewise. > >> * testsuite/ext/new_allocator/deallocate_local.cc (main): > >> Likewise. > >> * testsuite/ext/throw_allocator/deallocate_local.cc (main): > >> Likewise. > >> * testsuite/ext/pool_allocator/allocate_chunk.cc (started): > >> New global. > >> (operator new(size_t)): Check started. > >> (main): Set/Unset started. > >> * testsuite/ext/no_library_allocation.cc: New test case. > >> > >> Ok to commit ? > >> > >> François
On 21/08/2023 23:26, Jonathan Wakely wrote: > On Mon, 21 Aug 2023 at 21:20, François Dumont <frs.dumont@gmail.com> wrote: >> Here is the updated and tested patch. > OK for trunk, thanks. > > We could consider it for the branches too (I'm going to remove the > global strings on the gcc-13 branch tomorrow). It's not fixing anything so I don't think it worth it but let me know if you want me to do so.
diff --git a/libstdc++-v3/testsuite/23_containers/unordered_map/96088.cc b/libstdc++-v3/testsuite/23_containers/unordered_map/96088.cc index c6d50c20fbf..bcae891e5ec 100644 --- a/libstdc++-v3/testsuite/23_containers/unordered_map/96088.cc +++ b/libstdc++-v3/testsuite/23_containers/unordered_map/96088.cc @@ -268,6 +268,7 @@ test03() int main() { + __gnu_test::counter::enter(); test01(); test02(); test11(); @@ -275,5 +276,6 @@ main() test21(); test22(); test03(); + __gnu_test::counter::exit(); return 0; } diff --git a/libstdc++-v3/testsuite/23_containers/unordered_multimap/96088.cc b/libstdc++-v3/testsuite/23_containers/unordered_multimap/96088.cc index 214bc91a559..9f16ad68218 100644 --- a/libstdc++-v3/testsuite/23_containers/unordered_multimap/96088.cc +++ b/libstdc++-v3/testsuite/23_containers/unordered_multimap/96088.cc @@ -61,7 +61,9 @@ test02() int main() { + __gnu_test::counter::enter(); test01(); test02(); + __gnu_test::counter::exit(); return 0; } diff --git a/libstdc++-v3/testsuite/23_containers/unordered_multiset/96088.cc b/libstdc++-v3/testsuite/23_containers/unordered_multiset/96088.cc index 838ce8d5bc5..b34cfe67092 100644 --- a/libstdc++-v3/testsuite/23_containers/unordered_multiset/96088.cc +++ b/libstdc++-v3/testsuite/23_containers/unordered_multiset/96088.cc @@ -61,7 +61,9 @@ test02() int main() { + __gnu_test::counter::enter(); test01(); test02(); + __gnu_test::counter::exit(); return 0; } diff --git a/libstdc++-v3/testsuite/23_containers/unordered_set/96088.cc b/libstdc++-v3/testsuite/23_containers/unordered_set/96088.cc index 0f7dce2b38c..d5717fcec2b 100644 --- a/libstdc++-v3/testsuite/23_containers/unordered_set/96088.cc +++ b/libstdc++-v3/testsuite/23_containers/unordered_set/96088.cc @@ -269,6 +269,7 @@ test03() int main() { + __gnu_test::counter::enter(); test01(); test02(); test11(); @@ -277,5 +278,6 @@ main() test22(); test23(); test03(); + __gnu_test::counter::exit(); return 0; } diff --git a/libstdc++-v3/testsuite/ext/malloc_allocator/deallocate_local.cc b/libstdc++-v3/testsuite/ext/malloc_allocator/deallocate_local.cc index 79b583bd716..3aa65f298b1 100644 --- a/libstdc++-v3/testsuite/ext/malloc_allocator/deallocate_local.cc +++ b/libstdc++-v3/testsuite/ext/malloc_allocator/deallocate_local.cc @@ -27,6 +27,7 @@ typedef std::basic_string<char_t, traits_t, allocator_t> string_t; int main() { + __gnu_test::counter::enter(); { string_t s; s += "bayou bend"; @@ -34,5 +35,7 @@ int main() if (__gnu_test::counter::count() != 0) throw std::runtime_error("count not zero"); + + __gnu_test::counter::exit(); return 0; } diff --git a/libstdc++-v3/testsuite/ext/new_allocator/deallocate_local.cc b/libstdc++-v3/testsuite/ext/new_allocator/deallocate_local.cc index fcde46e6e10..ac4996698c7 100644 --- a/libstdc++-v3/testsuite/ext/new_allocator/deallocate_local.cc +++ b/libstdc++-v3/testsuite/ext/new_allocator/deallocate_local.cc @@ -27,6 +27,7 @@ typedef std::basic_string<char_t, traits_t, allocator_t> string_t; int main() { + __gnu_test::counter::enter(); { string_t s; s += "bayou bend"; @@ -34,5 +35,7 @@ int main() if (__gnu_test::counter::count() != 0) throw std::runtime_error("count not zero"); + + __gnu_test::counter::exit(); return 0; } diff --git a/libstdc++-v3/testsuite/ext/no_library_allocation.cc b/libstdc++-v3/testsuite/ext/no_library_allocation.cc new file mode 100644 index 00000000000..278d4757c93 --- /dev/null +++ b/libstdc++-v3/testsuite/ext/no_library_allocation.cc @@ -0,0 +1,8 @@ +#include <testsuite_hooks.h> +#include <replacement_memory_operators.h> + +int main() +{ + VERIFY( __gnu_test::counter::count() == 0 ); + return 0; +} diff --git a/libstdc++-v3/testsuite/ext/pool_allocator/allocate_chunk.cc b/libstdc++-v3/testsuite/ext/pool_allocator/allocate_chunk.cc index 17f8e3c7dcb..b11b450bf9e 100644 --- a/libstdc++-v3/testsuite/ext/pool_allocator/allocate_chunk.cc +++ b/libstdc++-v3/testsuite/ext/pool_allocator/allocate_chunk.cc @@ -32,16 +32,29 @@ struct big char c[64]; }; +bool started = false; + void* operator new(size_t n) THROW(std::bad_alloc) { - static bool first = true; - if (!first) - throw std::bad_alloc(); - first = false; + if (started) + { + static bool first = true; + if (!first) + throw std::bad_alloc(); + first = false; + } + return std::malloc(n); } +void +operator delete(void* p) throw() +{ + if (p) + std::free(p); +} + // http://gcc.gnu.org/ml/libstdc++/2004-10/msg00098.html void test01() { @@ -59,5 +72,7 @@ void test01() int main() { + started = true; test01(); + started = false; } diff --git a/libstdc++-v3/testsuite/ext/throw_allocator/deallocate_local.cc b/libstdc++-v3/testsuite/ext/throw_allocator/deallocate_local.cc index c6fd3538b82..268ae91f1ff 100644 --- a/libstdc++-v3/testsuite/ext/throw_allocator/deallocate_local.cc +++ b/libstdc++-v3/testsuite/ext/throw_allocator/deallocate_local.cc @@ -30,6 +30,7 @@ typedef std::basic_string<char_t, traits_t, allocator_t> string_t; int main() { + __gnu_test::counter::enter(); { string_t s; s += "bayou bend"; @@ -38,5 +39,6 @@ int main() if (__gnu_test::counter::count() != 0) throw std::runtime_error("count not zero"); + __gnu_test::counter::exit(); return 0; } diff --git a/libstdc++-v3/testsuite/util/replacement_memory_operators.h b/libstdc++-v3/testsuite/util/replacement_memory_operators.h index 6b1b3a82364..c2d1315691a 100644 --- a/libstdc++-v3/testsuite/util/replacement_memory_operators.h +++ b/libstdc++-v3/testsuite/util/replacement_memory_operators.h @@ -28,11 +28,11 @@ namespace __gnu_test struct counter { - std::size_t _M_count; + std::size_t _M_count, _M_pre_enter_count; std::size_t _M_increments, _M_decrements; bool _M_throw; - counter() : _M_count(0), _M_throw(true) { } + counter() : _M_count(0), _M_pre_enter_count(0), _M_throw(true) { } ~counter() THROW (counter_error) { @@ -75,17 +75,35 @@ namespace __gnu_test counter& cntr = get(); cntr._M_increments = cntr._M_decrements = 0; } + + static void + enter() + { + counter& cntr = get(); + cntr._M_pre_enter_count = cntr._M_count; + cntr._M_count = 0; + } + + static void + exit() + { + counter& cntr = get(); + cntr._M_count = cntr._M_pre_enter_count; + cntr._M_pre_enter_count = 0; + } }; template<typename Alloc, bool uses_global_new> bool check_new(Alloc a = Alloc()) { + __gnu_test::counter::enter(); __gnu_test::counter::exceptions(false); (void) a.allocate(10); const bool __b((__gnu_test::counter::count() > 0) == uses_global_new); if (!__b) throw std::logic_error("counter not incremented"); + __gnu_test::counter::exit(); return __b; }