From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 2DA903854801 for ; Thu, 17 Aug 2023 19:45:03 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 2DA903854801 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1692301502; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=697DVOzWaHLzkuWVLRPQw01kTdoi9lE+XMEXmu0TJHY=; b=gjDlamlD+sUB+b4y5CQ//FjzwzeRL1/cMnBD8Y+lF6Wo+ieck54YONRR/VYZhH6NtdnSCD 9ZEKfeixFcv/0aRRJP7w2d7TbGyJ8DpVD1On37w5Ljg5qzRcb1rq+WAoSHK7ge1UH+8vfi f2RMTRfqwoYHpf7Bc3icJRMKFxXK9Ms= Received: from mail-lj1-f200.google.com (mail-lj1-f200.google.com [209.85.208.200]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-445-ApMZhoheNW2uuDiLhzYUbA-1; Thu, 17 Aug 2023 15:45:00 -0400 X-MC-Unique: ApMZhoheNW2uuDiLhzYUbA-1 Received: by mail-lj1-f200.google.com with SMTP id 38308e7fff4ca-2b9ba3d6191so1338521fa.2 for ; Thu, 17 Aug 2023 12:45:00 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1692301499; x=1692906299; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=697DVOzWaHLzkuWVLRPQw01kTdoi9lE+XMEXmu0TJHY=; b=GrKleq7AD9frHDxWVU3pMYQGwebN0NE9swpE+Jgo0bDYTpuPBSCzHaomvWbpvQuv0O Iz8wtUql5iVPGUs+KCiB6ld5SLRg7oZG9PtUe7IMW28QpKSN3a0UHThW7Ai+jBqQWgh9 GWMhkLgQcZ982lQ/lA0lgeqCxoQFG0N45NSIdRiw1GapvXO5W3b06Z4n3lHtlrEwh+vY J+QcT6H1zMWNge2PBQVzHc1XjP5YolAdBvW1seDZ8HA9P4f79E0Bj1wXhRu5sUK0UHTH XahhZ6LtUFzPoOOX5yBe9FNoNgeoBLZtP9FraY0AlZW0Rb8W9+yjDkzdGYwG1QuV/XXP mn7Q== X-Gm-Message-State: AOJu0YxW2z8lQovflv9qZzCnFQ9B0QMfd7XxAcWCKx5+URWJlspbWhzH xZfomgoDlRf70ETBvW5ZjwjtPQqUveULaw3tM6nhIqYi+EYCSH0Uxx0XOMvkrWQo5XYTOi1pQVT R0QgnyiVzM7h7HEexrzuYTSp3LwqVCYtBWg== X-Received: by 2002:a2e:a0d8:0:b0:2b8:4079:fd9d with SMTP id f24-20020a2ea0d8000000b002b84079fd9dmr219929ljm.29.1692301499221; Thu, 17 Aug 2023 12:44:59 -0700 (PDT) X-Google-Smtp-Source: AGHT+IGEp8BcyPXbPE94QfijousfjArNYAUt8xSctnwIR0IwqOaIiVRAMX6yo0KxPU1eyLo1UvAlk3jklsR9qQYaII4= X-Received: by 2002:a2e:a0d8:0:b0:2b8:4079:fd9d with SMTP id f24-20020a2ea0d8000000b002b84079fd9dmr219917ljm.29.1692301498846; Thu, 17 Aug 2023 12:44:58 -0700 (PDT) MIME-Version: 1.0 References: <1dc681f4-41b7-d171-02ac-b0194617bdee@gmail.com> <91dc6383-6bff-ce6c-b24d-81cd2ab2dce8@gmail.com> <698fb340-2457-585f-f375-709491faf43e@gmail.com> In-Reply-To: From: Jonathan Wakely Date: Thu, 17 Aug 2023 20:44:48 +0100 Message-ID: Subject: Re: [PATCH] sso-string@gnu-versioned-namespace [PR83077] To: =?UTF-8?Q?Fran=C3=A7ois_Dumont?= Cc: Jonathan Wakely , "libstdc++" , gcc-patches X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-6.2 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: 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=C3=A7ois Dumont wrote: > > > > > > > > > On 17/08/2023 19:22, Jonathan Wakely wrote: > > > > On Sun, 13 Aug 2023 at 14:27, Fran=C3=A7ois 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 want= ed to > > > >> let you know that I tried to put COW std::basic_string into a nest= ed > > > >> __cow namespace when _GLIBCXX_USE_CXX11_ABI. But it had more impac= t on > > > >> string-inst.cc so I preferred the macro substitution approach. > > > > I was thinking of implementing the necessary special members functi= ons > > > > of __cow_string directly, so they are ABI compatible with the COW > > > > std::basic_string but don't actually reuse the code. That would mea= n > > > > 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 =3D=3D 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=3D0 several times per day and I'm not seei= ng > > > > 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/9= 6088. > > cc:44: void test01(): Assertion '__gnu_test::counter::count() =3D=3D 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 =3D "/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 =3D 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 =3D false; > int times_to_fail =3D 0; > > void* allocate(std::size_t n) > { > - if (!times_to_fail--) > + if (tests_started && !times_to_fail--) > return 0; > > void* ret =3D 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 =3D true; > + > for (int j =3D 0; j < iters; ++j) > { > for (int i =3D 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.