Here is a rebased patch following the resize_and_overwrite change. I confirm that tests are now fixed after the change in tzdb.cc. I'll prepare a fix for those tests still but preparing also a test to detect allocations in the lib. François On 17/08/2023 21:44, Jonathan Wakely wrote: > On Thu, 17 Aug 2023 at 20:37, Jonathan Wakely wrote: >> On Thu, 17 Aug 2023 at 19:59, Jonathan Wakely wrote: >>> On Thu, 17 Aug 2023 at 18:40, François Dumont wrote: >>>> >>>> On 17/08/2023 19:22, Jonathan Wakely wrote: >>>>> On Sun, 13 Aug 2023 at 14:27, François Dumont via Libstdc++ >>>>> wrote: >>>>>> Here is the fixed patch tested in all 3 modes: >>>>>> >>>>>> - _GLIBCXX_USE_DUAL_ABI >>>>>> >>>>>> - !_GLIBCXX_USE_DUAL_ABI && !_GLIBCXX_USE_CXX11_ABI >>>>>> >>>>>> - !_GLIBCXX_USE_DUAL_ABI && _GLIBCXX_USE_CXX11_ABI >>>>>> >>>>>> I don't know what you have in mind for the change below but I wanted to >>>>>> let you know that I tried to put COW std::basic_string into a nested >>>>>> __cow namespace when _GLIBCXX_USE_CXX11_ABI. But it had more impact on >>>>>> string-inst.cc so I preferred the macro substitution approach. >>>>> I was thinking of implementing the necessary special members functions >>>>> of __cow_string directly, so they are ABI compatible with the COW >>>>> std::basic_string but don't actually reuse the code. That would mean >>>>> we don't need to compile and instantiate the whole COW string just to >>>>> use a few members from it. But that can be done later, the macro >>>>> approach seems OK for now. >>>> You'll see that when cow_string.h is included while >>>> _GLIBCXX_USE_CXX11_ABI == 1 then I am hiding a big part of the >>>> basic_string definition. Initially it was to avoid to have to include >>>> basic_string.tcc but it is also a lot of useless code indeed. >>>> >>>> >>>>>> There are some test failing when !_GLIBCXX_USE_CXX11_ABI that are >>>>>> unrelated with my changes. I'll propose fixes in coming days. >>>>> Which tests? I run the entire testsuite with >>>>> -D_GLIBCXX_USE_CXX11_ABI=0 several times per day and I'm not seeing >>>>> failures. >>>>> >>>>> I'll review the patch ASAP, thanks for working on it. >>>>> >>>> So far the only issue I found are in the mode !_GLIBCXX_USE_DUAL_ABI && >>>> !_GLIBCXX_USE_CXX11_ABI. They are: >>>> >>>> 23_containers/unordered_map/96088.cc >>>> 23_containers/unordered_multimap/96088.cc >>>> 23_containers/unordered_multiset/96088.cc >>>> 23_containers/unordered_set/96088.cc >>>> ext/debug_allocator/check_new.cc >>>> ext/malloc_allocator/check_new.cc >>>> ext/malloc_allocator/deallocate_local.cc >>>> ext/new_allocator/deallocate_local.cc >>>> ext/pool_allocator/allocate_chunk.cc >>>> ext/throw_allocator/deallocate_local.cc >>> Ah yes, they fail for !USE_DUAL_ABI builds, I wonder why. >>> >>> /home/test/src/gcc/libstdc++-v3/testsuite/23_containers/unordered_map/96088. >>> cc:44: void test01(): Assertion '__gnu_test::counter::count() == 3' failed. >>> FAIL: 23_containers/unordered_map/96088.cc execution test >> It's due to this global object in src/c++20/tzdb.cc: >> 1081 const string tzdata_file = "/tzdata.zi"; >> >> When the library uses COW strings that requires an allocation before >> main, which uses the replacement operator new in the tests, which >> fails to allocate. For example, in 22_locale/locale/cons/12352.cc we >> have this function used by operator new: >> >> int times_to_fail = 0; >> >> void* allocate(std::size_t n) >> { >> if (!times_to_fail--) >> return 0; >> >> The counter is initially zero, so if we try to allocate before it gets >> set to a non-zero value in test01() then we fail. >> >> The test should not assume no allocations before main() begins. The >> simplest way to do that is with another global that says "we have >> started testing" e.g. >> >> --- a/libstdc++-v3/testsuite/22_locale/locale/cons/12352.cc >> +++ b/libstdc++-v3/testsuite/22_locale/locale/cons/12352.cc >> @@ -26,11 +26,12 @@ >> #include >> #include >> >> +bool tests_started = false; >> int times_to_fail = 0; >> >> void* allocate(std::size_t n) >> { >> - if (!times_to_fail--) >> + if (tests_started && !times_to_fail--) >> return 0; >> >> void* ret = std::malloc(n ? n : 1); >> @@ -106,6 +107,8 @@ void operator delete[](void* p, const >> std::nothrow_t&) throw() >> // libstdc++/12352 >> void test01(int iters) >> { >> + tests_started = true; >> + >> for (int j = 0; j < iters; ++j) >> { >> for (int i = 0; i < 100; ++i) >> >> >> This way the replacement operator new doesn't start intentionally >> failing until we ask it to do so. > I'll replace the global std::string objects with std::string_view > objects, so that they don't allocate even if the library only uses COW > strings. > > We should still fix those tests though. >