This is the mail archive of the
libstdc++@gcc.gnu.org
mailing list for the libstdc++ project.
Re: libstdc++ and race detectors
> You know, my idea would be, adding the macros for consistency
> *everywhere* an atomic counter is decreased (I gather this is the
> technical core of the problem). And then, in the docs, point out which
> are effective (that is, able to actually suppress the warnings) with a
> normally built library, which require the extern template trick on the
> command line (I think only basic_string), which require a library
> rebuild. I'm not saying you have to provide the whole package at once,
> for instance you can split out the documentation work, but please commit
> to something consistent for 4.6.x,
Which is when?
I do commit if there is enough time.
> we don't want to deliver half cooked
> stuff, in particular nothing missing a serious documentation, because
> these issues can be tricky for the ""average"" user.
Here is one more iteration (only two places are annotated for now, I'd
prefer to finalize the docs and c++config parts first).
http://codereview.appspot.com/download/issue1800042_16001.diff (also,
inlined below)
We will probably need tests to verify that the annotations work, but
let this be a separate issue.
A related question: does libstdc++ have other kinds of atomic
synchronization (e.g. lock free queues, locks free hash maps, etc)?
Index: tr1_impl/boost_sp_counted_base.h
===================================================================
--- tr1_impl/boost_sp_counted_base.h (revision 162071)
+++ tr1_impl/boost_sp_counted_base.h (working copy)
@@ -139,8 +139,11 @@
void
_M_release() // nothrow
{
+ // Be race-detector-friendly. For more info see bits/c++config.
+ _GLIBCXX_SYNCHRONIZATION_HAPPENS_BEFORE(&_M_use_count);
if (__gnu_cxx::__exchange_and_add_dispatch(&_M_use_count, -1) == 1)
{
+ _GLIBCXX_SYNCHRONIZATION_HAPPENS_AFTER(&_M_use_count);
_M_dispose();
// There must be a memory barrier between dispose() and destroy()
// to ensure that the effects of dispose() are observed in the
@@ -152,9 +155,14 @@
_GLIBCXX_WRITE_MEM_BARRIER;
}
+ // Be race-detector-friendly. For more info see bits/c++config.
+ _GLIBCXX_SYNCHRONIZATION_HAPPENS_BEFORE(&_M_weak_count);
if (__gnu_cxx::__exchange_and_add_dispatch(&_M_weak_count,
-1) == 1)
+ {
+ _GLIBCXX_SYNCHRONIZATION_HAPPENS_AFTER(&_M_weak_count);
_M_destroy();
+ }
}
}
@@ -165,8 +173,11 @@
void
_M_weak_release() // nothrow
{
+ // Be race-detector-friendly. For more info see bits/c++config.
+ _GLIBCXX_SYNCHRONIZATION_HAPPENS_BEFORE(&_M_weak_count);
if (__gnu_cxx::__exchange_and_add_dispatch(&_M_weak_count, -1) == 1)
{
+ _GLIBCXX_SYNCHRONIZATION_HAPPENS_AFTER(&_M_weak_count);
if (_Mutex_base<_Lp>::_S_need_barriers)
{
// See _M_release(),
Index: bits/basic_string.h
===================================================================
--- bits/basic_string.h (revision 162071)
+++ bits/basic_string.h (working copy)
@@ -232,9 +232,14 @@
#ifndef _GLIBCXX_FULLY_DYNAMIC_STRING
if (__builtin_expect(this != &_S_empty_rep(), false))
#endif
+ // Be race-detector-friendly. For more info see bits/c++config.
+ _GLIBCXX_SYNCHRONIZATION_HAPPENS_BEFORE(&this->_M_refcount);
if (__gnu_cxx::__exchange_and_add_dispatch(&this->_M_refcount,
-1) <= 0)
+ {
+ _GLIBCXX_SYNCHRONIZATION_HAPPENS_AFTER(&this->_M_refcount);
_M_destroy(__a);
+ }
} // XXX MT
void
Index: bits/c++config
===================================================================
--- bits/c++config (revision 162071)
+++ bits/c++config (working copy)
@@ -60,6 +60,93 @@
# define _GLIBCXX_DEPRECATED_ATTR
#endif
+// Macros for race detectors.
+// _GLIBCXX_SYNCHRONIZATION_HAPPENS_BEFORE(x) and
+// _GLIBCXX_SYNCHRONIZATION_HAPPENS_AFTER(x) should be used to explain
+// atomic (lock-free) synchronization to race detectors:
+// the race detector will infer a happens-before arc from the former to the
+// latter when they share the same argument pointer.
+//
+// The most frequent use case for these macros (and the only case in the
+// current implementation of the library) is atomic reference counting:
+// void _M_remove_reference() {
+// _GLIBCXX_SYNCHRONIZATION_HAPPENS_BEFORE(&this->_M_refcount);
+// if (__gnu_cxx::__exchange_and_add_dispatch(&this->_M_refcount, -1) <= 0)
+// {
+// _GLIBCXX_SYNCHRONIZATION_HAPPENS_AFTER(&this->_M_refcount);
+// _M_destroy(__a);
+// }
+// }
+// The annotations in this example tell the race detector that all memory
+// accesses occurred when the refcount was positive do not race with
+// memory accesses which occurred after the refcount became zero.
+//
+// For more details refer to <TODO: link to DOCS (below)>
+
+/* move to DOCS:
+Support for race detectors.
+
+All synchronization primitives used in the library internals should be
+understood by race detectors so that the race detectors do not produce
+false reports.
+
+We use two annotations (macros) to explain low-level
+synchronization to race detectors:
+ - _GLIBCXX_SYNCHRONIZATION_HAPPENS_BEFORE() and
+ - _GLIBCXX_SYNCHRONIZATION_HAPPENS_AFTER().
+
+By default, these two macros are defined empty -- anyone who wants
+to use a race detector will need to redefine these macros to call
+appropriate race detector API.
+
+Since these macros are empty by default, redefining them in the user code
+will affect only the inline template code, i.e.
+ - shared_ptr
+ - foo
+ - bar
+
+In order to redefine the macros in basic_string one will need to disable
+extern templates (by defining _GLIBCXX_EXTERN_TEMPLATE=-1) or rebuild the .so.
+
+The rest of the cases will require to rebuild the .so:
+ - ios_base::Init::~Init
+ - bazz
+ - zoo
+
+The approach described above works at least with the following
+race detection tools:
+ - DRD, http://valgrind.org/docs/manual/drd-manual.html
+ - Helgrind, http://valgrind.org/docs/manual/hg-manual.html
+ - Intel Parallel Inspector,
http://software.intel.com/en-us/intel-parallel-inspector
+ - ThreadSanitizer, http://code.google.com/p/data-race-test
+
+With DRD, Helgrind and ThreadSanitizer you will need to define
+the macros like this:
+#define _GLIBCXX_SYNCHRONIZATION_HAPPENS_BEFORE(a) ANNOTATE_HAPPENS_BEFORE(a)
+#define _GLIBCXX_SYNCHRONIZATION_HAPPENS_AFTER(a) ANNOTATE_HAPPENS_AFTER(a)
+
+For Intel Parallel Inspector:
+#define _GLIBCXX_SYNCHRONIZATION_HAPPENS_BEFORE(a) itt_notify_sync_releasing(a)
+#define _GLIBCXX_SYNCHRONIZATION_HAPPENS_AFTER(a) itt_notify_sync_acquired(a)
+
+(refer to documentation of a particular tool for the details).
+
+*/
+
+
+
+// -------------- end docs ----------
+
+
+#ifndef _GLIBCXX_SYNCHRONIZATION_HAPPENS_BEFORE
+# define _GLIBCXX_SYNCHRONIZATION_HAPPENS_BEFORE(a)
+#endif
+#ifndef _GLIBCXX_SYNCHRONIZATION_HAPPENS_AFTER
+# define _GLIBCXX_SYNCHRONIZATION_HAPPENS_AFTER(a)
+#endif
+
+
+
// Macros for activating various namespace association modes.
// _GLIBCXX_NAMESPACE_ASSOCIATION_DEBUG
// _GLIBCXX_NAMESPACE_ASSOCIATION_PARALLEL