* [PATCH] PR libstdc++/91057 set locale::id::_M_index atomically
@ 2019-10-09 15:59 Jonathan Wakely
2019-10-10 16:16 ` Jonathan Wakely
0 siblings, 1 reply; 2+ messages in thread
From: Jonathan Wakely @ 2019-10-09 15:59 UTC (permalink / raw)
To: libstdc++, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 758 bytes --]
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.
Tested x86_64-linux, committed to trunk.
[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 4753 bytes --]
commit 9868e2cb77b226e7aeaeec4c04637a161d7f05fc
Author: Jonathan Wakely <jwakely@redhat.com>
Date: Fri Jul 5 14:03:30 2019 +0100
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.
diff --git a/libstdc++-v3/src/c++98/locale.cc b/libstdc++-v3/src/c++98/locale.cc
index 8652f8559c2..1d00edc6f51 100644
--- a/libstdc++-v3/src/c++98/locale.cc
+++ b/libstdc++-v3/src/c++98/locale.cc
@@ -474,6 +474,30 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
// Definitions for static const data members of locale::id
_Atomic_word locale::id::_S_refcount; // init'd to 0 by linker
+ // XXX GLIBCXX_ABI Deprecated
+#ifdef _GLIBCXX_LONG_DOUBLE_COMPAT
+namespace {
+ inline locale::id*
+ find_ldbl_sync_facet(locale::id* __idp)
+ {
+# define _GLIBCXX_SYNC_ID(facet, mangled) \
+ if (__idp == &::mangled) \
+ return &facet::id
+
+ _GLIBCXX_SYNC_ID (num_get<char>, _ZNSt7num_getIcSt19istreambuf_iteratorIcSt11char_traitsIcEEE2idE);
+ _GLIBCXX_SYNC_ID (num_put<char>, _ZNSt7num_putIcSt19ostreambuf_iteratorIcSt11char_traitsIcEEE2idE);
+ _GLIBCXX_SYNC_ID (money_get<char>, _ZNSt9money_getIcSt19istreambuf_iteratorIcSt11char_traitsIcEEE2idE);
+ _GLIBCXX_SYNC_ID (money_put<char>, _ZNSt9money_putIcSt19ostreambuf_iteratorIcSt11char_traitsIcEEE2idE);
+# ifdef _GLIBCXX_USE_WCHAR_T
+ _GLIBCXX_SYNC_ID (num_get<wchar_t>, _ZNSt7num_getIwSt19istreambuf_iteratorIwSt11char_traitsIwEEE2idE);
+ _GLIBCXX_SYNC_ID (num_put<wchar_t>, _ZNSt7num_putIwSt19ostreambuf_iteratorIwSt11char_traitsIwEEE2idE);
+ _GLIBCXX_SYNC_ID (money_get<wchar_t>, _ZNSt9money_getIwSt19istreambuf_iteratorIwSt11char_traitsIwEEE2idE);
+ _GLIBCXX_SYNC_ID (money_put<wchar_t>, _ZNSt9money_putIwSt19ostreambuf_iteratorIwSt11char_traitsIwEEE2idE);
+# endif
+ }
+} // namespace
+#endif
+
size_t
locale::id::_M_id() const throw()
{
@@ -481,26 +505,38 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
{
// XXX GLIBCXX_ABI Deprecated
#ifdef _GLIBCXX_LONG_DOUBLE_COMPAT
- locale::id *f = 0;
-# define _GLIBCXX_SYNC_ID(facet, mangled) \
- if (this == &::mangled) \
- f = &facet::id
- _GLIBCXX_SYNC_ID (num_get<char>, _ZNSt7num_getIcSt19istreambuf_iteratorIcSt11char_traitsIcEEE2idE);
- _GLIBCXX_SYNC_ID (num_put<char>, _ZNSt7num_putIcSt19ostreambuf_iteratorIcSt11char_traitsIcEEE2idE);
- _GLIBCXX_SYNC_ID (money_get<char>, _ZNSt9money_getIcSt19istreambuf_iteratorIcSt11char_traitsIcEEE2idE);
- _GLIBCXX_SYNC_ID (money_put<char>, _ZNSt9money_putIcSt19ostreambuf_iteratorIcSt11char_traitsIcEEE2idE);
-# ifdef _GLIBCXX_USE_WCHAR_T
- _GLIBCXX_SYNC_ID (num_get<wchar_t>, _ZNSt7num_getIwSt19istreambuf_iteratorIwSt11char_traitsIwEEE2idE);
- _GLIBCXX_SYNC_ID (num_put<wchar_t>, _ZNSt7num_putIwSt19ostreambuf_iteratorIwSt11char_traitsIwEEE2idE);
- _GLIBCXX_SYNC_ID (money_get<wchar_t>, _ZNSt9money_getIwSt19istreambuf_iteratorIwSt11char_traitsIwEEE2idE);
- _GLIBCXX_SYNC_ID (money_put<wchar_t>, _ZNSt9money_putIwSt19ostreambuf_iteratorIwSt11char_traitsIwEEE2idE);
-# endif
- if (f)
- _M_index = 1 + f->_M_id();
+ if (locale::id* f = find_ldbl_sync_facet(this))
+ {
+ const size_t sync_id = f->_M_id();
+ _M_index = 1 + sync_id;
+ return sync_id;
+ }
+#endif
+
+#ifdef __GTHREADS
+ if (__gthread_active_p())
+ {
+ if (__atomic_always_lock_free(sizeof(_M_index), &_M_index))
+ {
+ const _Atomic_word next
+ = 1 + __gnu_cxx::__exchange_and_add(&_S_refcount, 1);
+ size_t expected = 0;
+ __atomic_compare_exchange_n(&_M_index, &expected, next,
+ /* weak = */ false,
+ /* success = */ __ATOMIC_ACQ_REL,
+ /* failure = */ __ATOMIC_ACQUIRE);
+ }
+ else
+ {
+ static __gnu_cxx::__mutex m;
+ __gnu_cxx::__scoped_lock l(m);
+ if (!_M_index)
+ _M_index = ++_S_refcount;
+ }
+ }
else
#endif
- _M_index = 1 + __gnu_cxx::__exchange_and_add_dispatch(&_S_refcount,
- 1);
+ _M_index = ++_S_refcount; // single-threaded case
}
return _M_index - 1;
}
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] PR libstdc++/91057 set locale::id::_M_index atomically
2019-10-09 15:59 [PATCH] PR libstdc++/91057 set locale::id::_M_index atomically Jonathan Wakely
@ 2019-10-10 16:16 ` Jonathan Wakely
0 siblings, 0 replies; 2+ messages in thread
From: Jonathan Wakely @ 2019-10-10 16:16 UTC (permalink / raw)
To: libstdc++, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 957 bytes --]
On 09/10/19 16:59 +0100, Jonathan Wakely wrote:
>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.
>
>Tested x86_64-linux, committed to trunk.
That patch was broken on powerpc and other targets with long double
compat symbols.
Tested powerpc64le-linux, committed to trunk.
[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 1172 bytes --]
commit 0964424db7ff62b3b1f561af60b1cc6560b4a742
Author: Jonathan Wakely <jwakely@redhat.com>
Date: Thu Oct 10 16:42:08 2019 +0100
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.
diff --git a/libstdc++-v3/src/c++98/locale.cc b/libstdc++-v3/src/c++98/locale.cc
index 1d00edc6f51..74a800c9c15 100644
--- a/libstdc++-v3/src/c++98/locale.cc
+++ b/libstdc++-v3/src/c++98/locale.cc
@@ -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) \
@@ -494,6 +494,7 @@ namespace {
_GLIBCXX_SYNC_ID (money_get<wchar_t>, _ZNSt9money_getIwSt19istreambuf_iteratorIwSt11char_traitsIwEEE2idE);
_GLIBCXX_SYNC_ID (money_put<wchar_t>, _ZNSt9money_putIwSt19ostreambuf_iteratorIwSt11char_traitsIwEEE2idE);
# endif
+ return 0;
}
} // namespace
#endif
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2019-10-10 16:16 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-09 15:59 [PATCH] PR libstdc++/91057 set locale::id::_M_index atomically Jonathan Wakely
2019-10-10 16:16 ` Jonathan Wakely
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).