In our build2 toolchain project we instantiate std::basic_regex class template with a custom character type (line_char) that, in particular, requires std::regex_traits<line_char> specialization and defining a locale class that installs the std::ctype<line_char> facet in constructor. Objects of the regex class that inherits from std::basic_regex<line_char> are created, used and destroyed concurrently in multiple threads but are not shared between threads. The problem is that while creating such an objects std::bad_cast is sporadically thrown. Unfortunately, I'm unable to reproduce the issue in my development environment to provide a full stack trace and, moreover, a simple test to replicate the issue. However, debugging through the creation of these regex objects makes me think that there is the following data race in the libstdc++'s locale implementation that may case the described behavior. The locale(const locale&, Facet*) constructor template (that is called from multiple threads in our case) calls _M_impl->_M_install_facet(&_Facet::id, __f); that in turn calls _M_id() for the passed facet id. On the first call the locale::id::_M_id() function sets/uses the locale::id::_M_index member in the thread-unsafe manner: size_t locale::id::_M_id() const throw() { if (!_M_index) { _M_index = ... } return _M_index - 1; } As a result, 2 locale objects created concurrently in 2 threads with the mentioned constructor may have a facet of the same type to be saved under different indexes in the _M_facets array. If that happens then use_facet() template function called for this facet type will end up badly for one of the objects, taking the facet pointer from the wrong index (may crash, throw bad_cast, etc).
Looks like we want a compare-and-swap there.
Author: redi Date: Wed Oct 9 15:59:56 2019 New Revision: 276762 URL: https://gcc.gnu.org/viewcvs?rev=276762&root=gcc&view=rev Log: PR libstdc++/91057 set locale::id::_M_index atomically If two threads see _M_index==0 concurrently they will both try to set it, potentially storing the facet at two different indices in the array. Either set the _M_index data member using an atomic compare-exchange operation or while holding a mutex. Also move the LONG_DOUBLE_COMPAT code into a separate function to remove the visual noise it creates. PR libstdc++/91057 * src/c++98/locale.cc (locale::id::_M_id()) [__GTHREADS]: Use atomic compare-exchange or double-checked lock to ensure only one thread sets the _M_index variable. [_GLIBCXX_LONG_DOUBLE_COMPAT]: Call find_ldbl_sync_facet to detect facets that share another facet's ID. [_GLIBCXX_LONG_DOUBLE_COMPAT] (find_ldbl_sync_facet): New function. Modified: trunk/libstdc++-v3/ChangeLog trunk/libstdc++-v3/src/c++98/locale.cc
The commit in comment #2 seems caused bootstrap to fail on powerpc64le. /home/linkw/gcc/gcc-svn/libstdc++-v3/src/c++98/locale.cc: In member function ‘std::size_t std::locale::id::_M_id() const’: /home/linkw/gcc/gcc-svn/libstdc++-v3/src/c++98/locale.cc:508:47: error: invalid conversion from ‘const std::locale::id*’ to ‘std::locale::id*’ [-fpermissive] 508 | if (locale::id* f = find_ldbl_sync_facet(this)) | ^ | | | const std::locale::id* /home/linkw/gcc/gcc-svn/libstdc++-v3/src/c++98/locale.cc:481:36: note: initializing argument 1 of ‘std::locale::id* std::{anonymous}::find_ldbl_sync_facet(std::locale::id*)’ 481 | find_ldbl_sync_facet(locale::id* __idp) | ~~~~~~~~~~~~^~~~~ One workaround could be: Index: libstdc++-v3/src/c++98/locale.cc =================================================================== --- libstdc++-v3/src/c++98/locale.cc (revision 276787) +++ libstdc++-v3/src/c++98/locale.cc (working copy) @@ -478,7 +478,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION #ifdef _GLIBCXX_LONG_DOUBLE_COMPAT namespace { inline locale::id* - find_ldbl_sync_facet(locale::id* __idp) + find_ldbl_sync_facet(const locale::id* __idp) { # define _GLIBCXX_SYNC_ID(facet, mangled) \ if (__idp == &::mangled) \
Sorry for not testing on a target that defines _GLIBCXX_LONG_DOUBLE_COMPAT. Your patch is approved for trunk, please go ahead and commit it.
There's another problem. I'm testing the fix now.
That should be fixed at r276840
Author: redi Date: Thu Oct 10 16:16:17 2019 New Revision: 276840 URL: https://gcc.gnu.org/viewcvs?rev=276840&root=gcc&view=rev Log: PR libstdc++/91057 fix bootstrap failure on powerpc PR libstdc++/91057 * src/c++98/locale.cc [_GLIBCXX_LONG_DOUBLE_COMPAT] (find_ldbl_sync_facet): Fix parameter type and missing return. Modified: trunk/libstdc++-v3/ChangeLog trunk/libstdc++-v3/src/c++98/locale.cc
Thanks for the prompt fix, it works well!
Fixed on trunk, but I think I want to backport it.
GCC 9.3.0 has been released, adjusting target milestone.
Not going to backport this.