Re: [committed] libstdc++: Use __libc_single_threaded for locale initialization

[Jonathan Wakely <jwakely@redhat.com>]

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.