libstdc++: fix data races in basic_string implementation

        * include/bits/basic_string.h: Fix data races on _M_refcount.

From-SVN: r227403

diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog
index d9c9d49353f7db8601187ea9c82be882832ff452..9feeb55b91a78ad2391f7c8c335e2d2048c02004 100644 (file)
--- a/libstdc++-v3/ChangeLog
+++ b/libstdc++-v3/ChangeLog
@@ -1,3 +1,7 @@
+2015-09-02  Dmitry Vyukov  <dvyukov@google.com>
+
+       * include/bits/basic_string.h: Fix data races on _M_refcount.
+
 2015-09-02  Sebastian Huber  <sebastian.huber@embedded-brains.de>
 
        PR libstdc++/67408

diff --git a/libstdc++-v3/include/bits/basic_string.h b/libstdc++-v3/include/bits/basic_string.h
index 923fb83937cc9f3d348b05c30bad4eca88b036d0..3ba6fc8fde545ecc9e618413234bd6073edde140 100644 (file)
--- a/libstdc++-v3/include/bits/basic_string.h
+++ b/libstdc++-v3/include/bits/basic_string.h
@@ -2601,11 +2601,32 @@ _GLIBCXX_END_NAMESPACE_CXX11
 
         bool
        _M_is_leaked() const _GLIBCXX_NOEXCEPT
-        { return this->_M_refcount < 0; }
+        {
+#if defined(__GTHREADS)
+          // _M_refcount is mutated concurrently by _M_refcopy/_M_dispose,
+          // so we need to use an atomic load. However, _M_is_leaked
+          // predicate does not change concurrently (i.e. the string is either
+          // leaked or not), so a relaxed load is enough.
+          return __atomic_load_n(&this->_M_refcount, __ATOMIC_RELAXED) < 0;
+#else
+          return this->_M_refcount < 0;
+#endif
+        }
 
         bool
        _M_is_shared() const _GLIBCXX_NOEXCEPT
-        { return this->_M_refcount > 0; }
+       {
+#if defined(__GTHREADS)
+          // _M_refcount is mutated concurrently by _M_refcopy/_M_dispose,
+          // so we need to use an atomic load. Another thread can drop last
+          // but one reference concurrently with this check, so we need this
+          // load to be acquire to synchronize with release fetch_and_add in
+          // _M_dispose.
+          return __atomic_load_n(&this->_M_refcount, __ATOMIC_ACQUIRE) > 0;
+#else
+          return this->_M_refcount > 0;
+#endif
+        }
 
         void
        _M_set_leaked() _GLIBCXX_NOEXCEPT
@@ -2654,6 +2675,14 @@ _GLIBCXX_END_NAMESPACE_CXX11
            {
              // Be race-detector-friendly.  For more info see bits/c++config.
              _GLIBCXX_SYNCHRONIZATION_HAPPENS_BEFORE(&this->_M_refcount);
+              // Decrement of _M_refcount is acq_rel, because:
+              // - all but last decrements need to release to synchronize with
+              //   the last decrement that will delete the object.
+              // - the last decrement needs to acquire to synchronize with
+              //   all the previous decrements.
+              // - last but one decrement needs to release to synchronize with
+              //   the acquire load in _M_is_shared that will conclude that
+              //   the object is not shared anymore.
              if (__gnu_cxx::__exchange_and_add_dispatch(&this->_M_refcount,
                                                         -1) <= 0)
                {