|
From: Konstantin S. <kon...@gm...> - 2010-07-01 09:20:40
|
On Wed, Jun 30, 2010 at 11:16 PM, Bart Van Assche <bva...@ac...> wrote: > On Tue, Jun 29, 2010 at 4:33 PM, Konstantin Serebryany > <kon...@gm...> wrote: >> >> Helgrind and DRD does not seem to detect races between a memory access >> and free(). >> (memcheck will find such bug only if free() happens before the access >> during a particular run). >> Do you plan to implement such functionality? (basically, free() should >> be treated as a write) >> >> I implemented such feature in ThreadSanitizer but was hit by >> libstdc++: the string implementation is unfriendly to race detectors >> because it uses atomic reference counting in the destructor. >> >> http://gcc.gnu.org/onlinedocs/libstdc++/latest-doxygen/a00762_source.html#l00228 >> Have you observed any issues around reference counting in string<>? >> Any thoughts? > > Detecting races between memory accesses and freeing memory is certainly > useful. But since the functions that manipulate reference counts are > typically implemented as inline functions, it looks hard to me to let a data > race detection tool recognize automatically the dependencies that are the > result of a reference counting scheme. Not only std::string<> in libstdc++ > is using this technique, but also std::tr1::shared_ptr<> in libstdc++, > boost::shared_ptr<> in the Boost libraries, QSharedPointer in Qt and > probably several other class implementations. Oh, yes! > I'm not sure it will be > possible to convince the maintainers of all these libraries to add > ANNOTATE_HAPPENS_BEFORE() / AFTER() annotations such that Valgrind-based > race detectors can recognize the dependencies resulting from reference > counting schemes. I don't think we need to use ANONTATE_* in standard libraries. Instead, I want to ask the library writers to arrange their code to make it easier to intercept by valgrind tools. In particular, for atomic ref counting I want to ask them to have a new function which simply means "atomically decrement reference count and return whether count is zero". That function could itself simply call __exchange_and_add_dispatch or whatever. The function should not be inlined when compiling in debug mode (or we need some way to intercept it). What do you think, Bart? Julian? Before reaching to the library folks I want to make sure that we agree with each other... --kcc > > Bart. > |