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.133.124]) by sourceware.org (Postfix) with ESMTPS id 5326A385C426 for ; Thu, 17 Aug 2023 23:31:36 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 5326A385C426 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=1692315096; 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=QZLSGYj8l+yuYlavrmxYahxkrsZL1zCfiQmIiLX9tco=; b=ck4VGQjc0MNOECVCmRsS3CIZNzyvJWTdwuppUELqJNM7o47SWkM/ofF0yZAeRV/yVKMsWQ TCSvdZI3c1QWY2ibeJbvHy6PTpBM+bjdobFQSTUMPRV+Uqw9Z5OK6cNiCsFAj3bljnqXWp F5TqPpjx+uYZJN3ZiBRMXI9HHYEHeF8= Received: from mail-lj1-f198.google.com (mail-lj1-f198.google.com [209.85.208.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-672-cToC8AKzNSa55Dm6rQ-aFQ-1; Thu, 17 Aug 2023 19:31:34 -0400 X-MC-Unique: cToC8AKzNSa55Dm6rQ-aFQ-1 Received: by mail-lj1-f198.google.com with SMTP id 38308e7fff4ca-2bb9a5f7f18so3389481fa.2 for ; Thu, 17 Aug 2023 16:31:34 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1692315093; x=1692919893; 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=QZLSGYj8l+yuYlavrmxYahxkrsZL1zCfiQmIiLX9tco=; b=B7wYKYTcW9ToQGHsyHt6/F+8AaYVNqfdRDv+yiKmNmrHMu+m2lXqqxliSwcGX3I5sk QcWnbGGyqcHjO3zxB/eX1dBvA2qIN4N4REI0xBCjrOhLNuc/VXdEkVYbvqYnUmGyUUjT hXudfxsp9PciDg/9tw7XZXqOKHhN1zg02Wghjkj7h6qas/2OUsZvETzQZJlyIlZOlHkS cYroMtb0IHK840Onb1KbMU0/9rBsfvKjVO1RN2cgYSkkU135P9Ra7K9eGos2PUhD9ojZ UHpt8lscbsTwf4OVL3WI/y/5t+vticaizZixpicmznGt69Z5HOYDzWvRclEQk1MlZAqa bPOg== X-Gm-Message-State: AOJu0YxnjQVlhllK3+oqHVChUf/pIWSw2AV66+CagsWkkAzDvgS55fpQ 2EGJaFL7FYslguPl4McAf/+NoutFfChv/ALMvWvIr9rQBRGwmoaHc2C2nxU2WQJxSKSH20yc0Z/ dFSkwedSJMQLasAYdiqvNf2bIArT+M58= X-Received: by 2002:a2e:8812:0:b0:2b6:9da9:2884 with SMTP id x18-20020a2e8812000000b002b69da92884mr485631ljh.40.1692315093074; Thu, 17 Aug 2023 16:31:33 -0700 (PDT) X-Google-Smtp-Source: AGHT+IHb10t5wSaYrVkFH35UefLrdO0yYz39DtBwl9eUx26dkicbCsejgczyaPgvnL2OLcZHrs0ruXi4muWvh/tBBvg= X-Received: by 2002:a2e:8812:0:b0:2b6:9da9:2884 with SMTP id x18-20020a2e8812000000b002b69da92884mr485618ljh.40.1692315092676; Thu, 17 Aug 2023 16:31:32 -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: Fri, 18 Aug 2023 00:31:22 +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=-3.4 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: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 wrot= e: > > > > > > 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 wa= nted to > > > > >> let you know that I tried to put COW std::basic_string into a ne= sted > > > > >> __cow namespace when _GLIBCXX_USE_CXX11_ABI. But it had more imp= act on > > > > >> string-inst.cc so I preferred the macro substitution approach. > > > > > I was thinking of implementing the necessary special members func= tions > > > > > of __cow_string directly, so they are ABI compatible with the COW > > > > > std::basic_string but don't actually reuse the code. That would m= ean > > > > > we don't need to compile and instantiate the whole COW string jus= t 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 inclu= de > > > > basic_string.tcc but it is also a lot of useless code indeed. > > > > > > > > > > > > > > > > > >> There are some test failing when !_GLIBCXX_USE_CXX11_ABI that ar= e > > > > >> 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 se= eing > > > > > 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_AB= I && > > > > !_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() =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. Done at r14-3309-gd82a85b6161cbe > > We should still fix those tests though.