Summary: | Data race in locale(const locale&, Facet*) constructor | ||
---|---|---|---|
Product: | gcc | Reporter: | Karen <karen.arutyunov> |
Component: | libstdc++ | Assignee: | Jonathan Wakely <redi> |
Status: | RESOLVED FIXED | ||
Severity: | normal | CC: | dimhen, webrown.cpp |
Priority: | P3 | ||
Version: | 9.1.0 | ||
Target Milestone: | 10.0 | ||
Host: | Target: | ||
Build: | Known to work: | ||
Known to fail: | Last reconfirmed: | 2019-07-02 00:00:00 |
Description
Karen
2019-07-02 13:40:53 UTC
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. 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. |