On 27/11/20 11:15 +0000, Jonathan Wakely wrote:
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.
Fixed with this patch, tested powerp64le-linux (glibc 2.32).
Pushed to trunk.
Thanks for the testing and bug report, as always.
commit 0d7d69ca4a8c05d883e07ee42058c9c6b0c72370
Author: Jonathan Wakely <jwak...@redhat.com>
Date: Fri Nov 27 11:00:15 2020
libstdc++: Partially revert r11-5314
The changes in r11-5314 are broken, because it means we don't use
__gthread_once for the first few initializations, but after the program
becomes multi-threaded we will repeat the initialization, using
__gthread_once once this time. This leads to memory errors.
The use of __is_single_threaded() in locale::id::_M_id() is OK, because
the side effects are the same either way.
libstdc++-v3/ChangeLog:
* src/c++98/locale.cc (locale::facet::_S_get_c_locale()):
Revert change to use __is_single_threaded.
* src/c++98/locale_init.cc (locale::_S_initialize()):
Likewise.
diff --git a/libstdc++-v3/src/c++98/locale.cc b/libstdc++-v3/src/c++98/locale.cc
index 9b3fc3515152..4c1612cc5dca 100644
--- a/libstdc++-v3/src/c++98/locale.cc
+++ b/libstdc++-v3/src/c++98/locale.cc
@@ -214,7 +214,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
locale::facet::_S_get_c_locale()
{
#ifdef __GTHREADS
- if (!__gnu_cxx::__is_single_threaded())
+ if (__gthread_active_p())
__gthread_once(&_S_once, _S_initialize_once);
else
#endif
diff --git a/libstdc++-v3/src/c++98/locale_init.cc b/libstdc++-v3/src/c++98/locale_init.cc
index fc8416ba01a6..c3841ccbd3c9 100644
--- a/libstdc++-v3/src/c++98/locale_init.cc
+++ b/libstdc++-v3/src/c++98/locale_init.cc
@@ -320,7 +320,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
locale::_S_initialize()
{
#ifdef __GTHREADS
- if (!__gnu_cxx::__is_single_threaded())
+ if (__gthread_active_p())
__gthread_once(&_S_once, _S_initialize_once);
#endif
if (!_S_classic)
