|
From: Konstantin S. <kon...@gm...> - 2009-02-06 20:22:00
|
Hi Bart, On Fri, Feb 6, 2009 at 10:29 PM, Bart Van Assche <bar...@gm...> wrote: > On Mon, Jan 26, 2009 at 4:45 PM, Konstantin Serebryany > <kon...@gm...> wrote: >> Did you ever think that two data race detectors (Helgrind and Drd) is >> too much for the Valgrind project? >> In fact, I think that two is too few. :) >> >> Please welcome ThreadSanitizer, yet another data race detector based >> on Valgrind. >> http://code.google.com/p/data-race-test/wiki/ThreadSanitizer >> >> I'd appreciate your feedback, > > Hello Konstantin, > > I had a quick look at the command line options supported by > ThreadSanitizer, and found this: > > --ignore-in-dtor=yes|no, default yes, Ignore reports with a C++ class > Destructor in stack trace. > > Can you explain why this option defaults to yes, and why it even > exists ? Oh well. This is probably one of the most painful questions: to choose between false positives and false negatives. So, tsan has 3 flags that control noise vs signal: --pure-happens-before (default = no): Even though I really love the default hybrid state machine, I start thinking that the default should change to --pure-happens-before=yes. Many, many developers are not used to race detectors, false positives may frighten them... More on hybrid vs pure happens-before is here: http://code.google.com/p/data-race-test/wiki/ThreadSanitizerAlgorithm#Pure_happens-before_vs_Hybrid --fast-mode (default=yes): makes sense only for the hybrid mode. A close analog of Eraser's initialization mode. Misses roughly half of the races. But also cuts the number of false positives of the hybrid machine by something like 90%. http://code.google.com/p/data-race-test/wiki/ThreadSanitizerAlgorithm#Fast_mode --ignore-in-dtor (default=yes): One of the most frequent cause of false positives is reference counting. Ref counting implemented via atomics is not understood even by pure happens-before detectotrs. And the reports are almost always in DTORs. See test401: http://code.google.com/p/data-race-test/source/browse/trunk/unittest/racecheck_unittest.cc#6468 In hybrid mode there are more reasons for this flag. Well, maybe in pure-happens-before mode the default for this should be 'no'. Dunno. Note, that even if you use the most aggressive mode (--pure-happens-before=no --fast-mode=no --ignore-in-dtor=no), it is possible to avoid all false positives by annotating the code with http://code.google.com/p/data-race-test/wiki/DynamicAnnotations But it requires some skill. IMHO, the default values should be set in a way that will not frighten novice users too much. Those who will be brave enough to read the docs and will understand the value of the tool, will likely start using more aggressive flags. Yet I have no clear vision on which settings should be default. >Any race I have seen in C++ destructors until now indicated a > real problem. A very common cause for data races triggered inside a > C++ destructor is the following: > - A class has been defined that wraps thread creation and destruction, > e.g. CThread. > - One of the methods in that class is declared virtual and is run > inside the created thread, e.g. CThread::Run(). > - There exists methods for starting the thread and waiting until the > thread stopped, e.g. CThread::StartRunning() and > CThread::WaitUntilStopped(). This last method notifies CThread::Run() > that it should stop and then waits until that thread stopped, e.g. via > pthread_join(). > - Calling CThread::WaitUntilStopped() from the CThread destructor will > trigger a race condition on the vtbl pointer of class CThread. This is > a real bug and a bug that can cause severe trouble, not something that > should be hidden. Hehe, yea... Something similar: test86 and test87 http://code.google.com/p/data-race-test/source/browse/trunk/unittest/racecheck_unittest.cc#4060 --kcc > > Bart. > |