This is the mail archive of the
libstdc++@gcc.gnu.org
mailing list for the libstdc++ project.
Re: [PATCH] Make it possible to annotate the shared pointer operations in the std::thread implementation
- From: Jonathan Wakely <jwakely dot gcc at gmail dot com>
- To: Bart Van Assche <bvanassche at acm dot org>
- Cc: libstdc++ at gcc dot gnu dot org, Konstantin Serebryany <konstantin dot s dot serebryany at gmail dot com>, gcc-patches <gcc-patches at gcc dot gnu dot org>
- Date: Sat, 24 Dec 2011 11:11:33 +0000
- Subject: Re: [PATCH] Make it possible to annotate the shared pointer operations in the std::thread implementation
- References: <33987586.92vJibue4B@linux> <1459889.aDstdfhJvV@linux>
On 24 December 2011 09:53, Bart Van Assche wrote:
> As documented in the libstdc++ manual, the shared pointer operations in
> libstdc++ headers can be instrumented by defining the macros
> _GLIBCXX_SYNCHRONIZATION_HAPPENS_BEFORE()/AFTER() and either libstdc++ or
> thread.cc has to be rebuilt in order to instrument the remaining shared
> pointer operations. However, rebuilding is inconvenient. So let's move the
> thread wrapper code from thread.cc into <thread>.
The patch is not OK because the function passed to pthread_create must
be extern "C" and a static member function is not extern "C"
(G++ fails to diagnose that error, but that's a long-standing bug:
http://gcc.gnu.org/PR2316 )
Apart from that, I prefer to keep the thread-launching logic in the
library rather than headers, as it allows us to change it more easily
and for users to benefit just by linking to a newer library rather
than recompiling. I don't think the data race detection macros are a
compelling reason to change that.
As I noted yesterday on PR 51504, you don't need to recompile the
whole library, you only need to recompile thread.cc with the data race
detection macros defined and link that object earlier in your linker
search path than libstdc++ to allow symbol interposition to replace
std::thread::_M_start_thread from the library with your instrument
version of that symbol. I'll update the manual to say that.