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 [63.128.21.124]) by sourceware.org (Postfix) with ESMTP id 59ED5385802B for ; Fri, 27 Nov 2020 11:15:14 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 59ED5385802B Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-318-uB4QFDrMNsGpj4Ec4HwGMQ-1; Fri, 27 Nov 2020 06:15:12 -0500 X-MC-Unique: uB4QFDrMNsGpj4Ec4HwGMQ-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 28E33809DD1; Fri, 27 Nov 2020 11:15:11 +0000 (UTC) Received: from localhost (unknown [10.33.37.15]) by smtp.corp.redhat.com (Postfix) with ESMTP id BB1DE60861; Fri, 27 Nov 2020 11:15:10 +0000 (UTC) Date: Fri, 27 Nov 2020 11:15:10 +0000 From: Jonathan Wakely To: Stephan Bergmann Cc: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org Subject: Re: [committed] libstdc++: Use __libc_single_threaded for locale initialization Message-ID: <20201127111510.GR1312820@redhat.com> References: <20201124145952.GA1857606@redhat.com> MIME-Version: 1.0 In-Reply-To: X-Clacks-Overhead: GNU Terry Pratchett X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii; format=flowed Content-Disposition: inline X-Spam-Status: No, score=-8.0 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_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libstdc++@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libstdc++ mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 27 Nov 2020 11:15:24 -0000 On 27/11/20 09:43 +0100, Stephan Bergmann via Libstdc++ wrote: >On 24/11/2020 15:59, Jonathan Wakely via Gcc-patches wrote: >>Most initialization of locales and facets happens before main() during >>startup, when the program is likely to only have one thread. By using >>the new __gnu_cxx::__is_single_threaded() function instead of checking >>__gthread_active_p() we can avoid using pthread_once or atomics for the >>common case. >> >>That said, I'm not sure why we don't just use a local static variable >>instead, as __cxa_guard_acquire() already optimizes for the >>single-threaded case: >> >> static const bool init = (_S_initialize_once(), true); >> >>I'll revisit that for GCC 12. >> >>libstdc++-v3/ChangeLog: >> >> * src/c++98/locale.cc (locale::facet::_S_get_c_locale()) >> (locale::id::_M_id() const): Use __is_single_threaded. >> * src/c++98/locale_init.cc (locale::_S_initialize()): >> Likewise. >> >>Tested powerpc64le-linux. Committed to trunk. > >I now started to get weird crashes when running LibreOffice test code >at least with Clang -fsanitize=address and latest libstdc++, where the >Clang ASan machinery SEGVs while it wants to report some malloc/free >issue. That goes away when reverting this commit, and I think the >root cause is that locale::facet::_S_initialize_once() now gets called >twice: First during __cxx_global_var_init when the process is still >single threaded (so > > if (!__gnu_cxx::__is_single_threaded()) > >in locale::facet::_S_get_c_locale, reading __libc_single_threaded, is >false, whereas > > if (__gthread_active_p()) > >would have been true even if the process still only had a single >thread). And again after the process has spawned further threads via >pthread_create (flipping __libc_single_threaded) and calls >std::ostringstream() -> ... std::locale() -> ..., at which point > > if (!__gnu_cxx::__is_single_threaded()) > >in locale::facet::_S_get_c_locale is true now. Gah, yes, that's broken. Sorry. I'm reverting that part and testing it now. I still think we should just use local static variables here and let the runtime do the initialization and optimize it for single-threaded, but I'll leave that for GCC 12. There might be some reason that wasn't done originally, maybe so the library could be built with -fno-threadsafe-statics (which I don't think we should support) or maybe just because it predates the threadsafe-static code working well.