|
From: Konstantin S. <kon...@gm...> - 2009-06-01 08:25:13
|
On Sat, May 30, 2009 at 3:35 PM, Bart Van Assche <bar...@gm...> wrote: > On Fri, May 29, 2009 at 12:58 PM, Konstantin Serebryany > <kon...@gm...> wrote: >> Do you plan to support annotations (aka client requests) in Helgrind >> and DRD in a compatible way (and possibly, in a way compatible with >> ThreadSanitizer)? >> Something like http://code.google.com/p/google-perftools/source/browse/trunk/src/base/dynamic_annotations.h, >> or completely different. >> Our experience shows that even a pure-happens-before race detector is >> completely useless w/o annotations if your code has lock-less >> synchronization and hundreds of benign races. > > A few remarks about the semantics of the ANNOTATE_* macro's: > * I do not really like ANNOTATE_PUBLISH_MEMORY_RANGE. The comment > above this macro says more or less that any other thread may access > the published memory range safely after it has been published. > However, no matter which synchronization instructions have been issued > by the publishing thread, a consumer thread may only access the > published memory safely after proper synchronization with the > publishing thread. So my proposal is to remove this annotation and to > use ANNOTATE_MUTEX_IS_USED_AS_CONDVAR instead. ANNOTATE_MUTEX_IS_USED_AS_CONDVAR is a big hammer as it essentially makes the detection to be pure h-b. PUBLISH_MEMORY_RANGE() is needed to hybrid mode. I am not a great expert in lock-less synchronization but I believe that an object could be published safely in a way that does not require any action by a consumer. You can publish an object with just one CAS (at least on x86?). No? So, you can use this annotation in a situation were you don't have locks at all. > > * Are the ANNOTATE_PCQ_* macro's really needed ? Yes. They are much more gentle than general h-b annotations. The name is bad, it should probably be renamed to ANNOTATE_FIFO_* > I could not find any > test code for these annotations inside ThreadSanitizer. Sorry. Just added test142 and test143. > > * A size argument is missing for the ANNOTATE_BENIGN_RACE*(), > ANNOTATE_EXPECT_RACE() and ANNOTATE_TRACE_MEMORY() macro's. Yes, true. I wanted to add an argument for a long time. I just never really needed it. Still I agree with you. > Do these > macro's apply to a single address, a range of four bytes or a range of > eight bytes ? Choosing any of these three range sizes for all these > macro's won't give the expected semantics for at least one of these > macro's. Just 1 byte. --kcc > > Bart. > |