|
From: Konstantin S. <kon...@gm...> - 2009-05-29 10:58:34
|
[changed subject] Julian, Bart, 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. Thanks, --kcc On Thu, Feb 12, 2009 at 12:00 AM, Konstantin Serebryany <kon...@gm...> wrote: > On Wed, Feb 11, 2009 at 8:53 PM, Bart Van Assche > <bar...@gm...> wrote: >> On Tue, Feb 10, 2009 at 7:42 AM, Konstantin Serebryany >> <kon...@gm...> wrote: >>> On Mon, Feb 9, 2009 at 9:48 PM, Bart Van Assche >>> <bar...@gm...> wrote: >>>> IMHO it would be a significant advantage for Valgrind users if there >>>> would be a single set of source code annotations that is understood by >>>> all data race detectors built on top of Valgrind (Helgrind, DRD and >>>> ThreadSanitizer). >>> >>> Yes. >>> Another question: should these client requests be binary compatible >>> with each other, or only source compatible? >>> (source compatible == macro definitions with different >>> implementations for different tools) >>> If we push for binary compatibility, we may loose the ability to be >>> compatible with non-valgrind-based tools. >> >> Do you think it is possible to define the annotations such that >> recompilation would only be needed when switching between >> Valgrind-tools and non-Valgrind tools ? > > Yes, sure. > either define the same client requests in all three tools or intercept > the same functions (like AnnotateBlahBlahBlah()) > --kcc > > --kcc >> >> Bart. >> > |
|
From: Bart V. A. <bar...@gm...> - 2009-05-29 11:19:49
|
On Fri, May 29, 2009 at 12:58 PM, Konstantin Serebryany <kon...@gm...> wrote: > [changed subject] > > 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. Regarding the dynamic_annotations.h header file: which parts are generic and which parts are ThreadSanitizer-specific ? I assume that client programs should use the ANNOTATE_*() macro's and not the Annotate*() functions ? I assume that you have already developed unit tests for these macro's in ThreadSanitizer. Are available under the GPL ? One aspect of the dynamic_annotations.h header file that I do not like is that enabling / disabling the ANNOTATE*() macro's is controlled by the NDEBUG macro. IMHO a new macro should be defined that allows to enable / disable the ANNOTATE*() macro's. Adding support for the ANNOTATE_*() macro's in drd/drd.h is probably not too hard. I'll have a look at it. Bart. |
|
From: Konstantin S. <kon...@gm...> - 2009-05-29 11:50:26
|
On Fri, May 29, 2009 at 3:19 PM, Bart Van Assche <bar...@gm...> wrote: > On Fri, May 29, 2009 at 12:58 PM, Konstantin Serebryany > <kon...@gm...> wrote: >> [changed subject] >> >> 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. > > Regarding the dynamic_annotations.h header file: which parts are > generic and which parts are ThreadSanitizer-specific ? Most are generic. But some of the annotations are usually required only with hybrid detectors. E.g. ANNOTATE_PUBLISH_MEMORY will be needed with a pure-h-b detector only if you publish your objects w/o locks. > I assume that > client programs should use the ANNOTATE_*() macro's and not the > Annotate*() functions ? Correct. Annotate*() functions are implementation details and may change. (comment at line 267) > > I assume that you have already developed unit tests for these macro's > in ThreadSanitizer. http://code.google.com/p/data-race-test/source/browse/trunk/unittest/racecheck_unittest.cc is a set of unittests for ThreadSanitizer. Some of the tests specifically test the annotations. > Are available under the GPL ? Yes, GPL v2+ As for annotations, I think they should go under a BSD-like license (as valgrind.h does) > > One aspect of the dynamic_annotations.h header file that I do not like > is that enabling / disabling the ANNOTATE*() macro's is controlled by > the NDEBUG macro. IMHO a new macro should be defined that allows to > enable / disable the ANNOTATE*() macro's. I don't have a strong opinion here. > > Adding support for the ANNOTATE_*() macro's in drd/drd.h is probably > not too hard. I'll have a look at it. Implementation-wise you need to support Annotate*() functions, or rewrite ANNOTATE*() macro's using client requests and support those client requests. I find the approach with Annotate*() functions simpler to maintain. --kcc > > Bart. > |
|
From: Julian S. <js...@ac...> - 2009-05-30 11:00:27
|
Ok in principle, but some comments: > > Adding support for the ANNOTATE_*() macro's in drd/drd.h is probably > > not too hard. I'll have a look at it. > > Implementation-wise you need to support Annotate*() functions, or > rewrite ANNOTATE*() macro's using client requests and support those > client requests. > I find the approach with Annotate*() functions simpler to maintain. I prefer that the basic thing that is implemented and documented to be the ANNOTATE_* macros, not the Annotate*() functions. The macros can be implemented with just a header file, like the Memcheck macros. The functions require shared objects etc and therefore create a dependency on a new library (IIUC), which isn't desirable. There are a lot of these requests, which is a big overhead w.r.t. verifying that the implementation is correct and keeping it correct. Are they all really necessary? I wonder if you can implement these using a smaller subset, based on the idea of sending an abstract message between threads. Suppose you had a ANNOTATE_SENDTO_SYNC_OBJECT(address) and ANNOTATE_RECVFROM_SYNC_OBJECT(address) which create a h-b edge through the sync object. How many of these requests could be implemented using just those two? J |
|
From: Bart V. A. <bar...@gm...> - 2009-05-30 11:15:26
|
On Sat, May 30, 2009 at 1:02 PM, Julian Seward <js...@ac...> wrote: > > Ok in principle, but some comments: > >> > Adding support for the ANNOTATE_*() macro's in drd/drd.h is probably >> > not too hard. I'll have a look at it. >> >> Implementation-wise you need to support Annotate*() functions, or >> rewrite ANNOTATE*() macro's using client requests and support those >> client requests. >> I find the approach with Annotate*() functions simpler to maintain. > > I prefer that the basic thing that is implemented and documented to > be the ANNOTATE_* macros, not the Annotate*() functions. The macros > can be implemented with just a header file, like the Memcheck macros. > The functions require shared objects etc and therefore create a > dependency on a new library (IIUC), which isn't desirable. > > There are a lot of these requests, which is a big overhead w.r.t. > verifying that the implementation is correct and keeping it correct. > Are they all really necessary? I wonder if you can implement these > using a smaller subset, based on the idea of sending an abstract message > between threads. > > Suppose you had a > > ANNOTATE_SENDTO_SYNC_OBJECT(address) and > ANNOTATE_RECVFROM_SYNC_OBJECT(address) > > which create a h-b edge through the sync object. How many of these > requests could be implemented using just those two? Implementing some of the ANNOTATE_* macro's using the above two macro's would limit the usefulness of a thread checker tool because no information is passed to the tool about the type of synchronization object that resides at 'address'. Hence the tool cannot check whether the synchronization object has been used properly. Furthermore, implementing the ANNOTATE_RWLOCK_REQUIRED() / ANNOTATE_RWLOCK_RELEASED() macro's using ANNOTATE_SENDTO_SYNC_OBJECT() / ANNOTATE_RECVFROM_SYNC_OBJECT() would require that the client tracks the set of threads that hold a reader lock. This is something that should not be done by the client but by the tool. Bart. |
|
From: Konstantin S. <kon...@gm...> - 2009-06-01 07:30:04
|
On Sat, May 30, 2009 at 3:02 PM, Julian Seward <js...@ac...> wrote: > > Ok in principle, but some comments: > >> > Adding support for the ANNOTATE_*() macro's in drd/drd.h is probably >> > not too hard. I'll have a look at it. >> >> Implementation-wise you need to support Annotate*() functions, or >> rewrite ANNOTATE*() macro's using client requests and support those >> client requests. >> I find the approach with Annotate*() functions simpler to maintain. > > I prefer that the basic thing that is implemented and documented to > be the ANNOTATE_* macros, not the Annotate*() functions. The macros > can be implemented with just a header file, like the Memcheck macros. > The functions require shared objects etc and therefore create a > dependency on a new library (IIUC), which isn't desirable. This will give us source-level compatibility. If we also want binary-level compatibility between Helgrind/DRD/ThreadSanitizer we will have to define the implementation of these macros. > > There are a lot of these requests, which is a big overhead w.r.t. > verifying that the implementation is correct and keeping it correct. > Are they all really necessary? I wonder if you can implement these > using a smaller subset, based on the idea of sending an abstract message > between threads. > > Suppose you had a > > ANNOTATE_SENDTO_SYNC_OBJECT(address) and > ANNOTATE_RECVFROM_SYNC_OBJECT(address) > > which create a h-b edge through the sync object. How many of these > requests could be implemented using just those two? Just those two. For pure happens-before detector you will probably need only BENIGN_RACE and IGNORE_* in addition to SENDTO/RECVFROM. For hybrid you need much more. Pure h-b is easy to use due to low false-positive rate, but it misses ~half of the races (and is also less predictable). So, ThreadSanitizer supports both modes and needs the full set of annotations. --kcc > > J > |
|
From: Bart V. A. <bar...@gm...> - 2009-05-30 11:35:41
|
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. * Are the ANNOTATE_PCQ_* macro's really needed ? I could not find any test code for these annotations inside ThreadSanitizer. * A size argument is missing for the ANNOTATE_BENIGN_RACE*(), ANNOTATE_EXPECT_RACE() and ANNOTATE_TRACE_MEMORY() macro's. 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. Bart. |
|
From: Bart V. A. <bar...@gm...> - 2009-05-30 19:13:03
|
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. Another remark: I suggest to remove the macro's ANNOTATE_CONDVAR_WAIT() and ANNOTATE_CONDVAR_SIGNAL() but to keep their aliases ANNOTATE_HAPPENS_BEFORE() and ANNOTATE_HAPPENS_AFTER(). The names of the first two macro's are really confusing: these two macro's are a.o. used to annotate ordering constraints between mutexes(!) in racecheck_unittest.cc. Bart. |
|
From: Bart V. A. <bar...@gm...> - 2009-05-31 19:14:35
|
On Sat, May 30, 2009 at 9:12 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. > > Another remark: I suggest to remove the macro's > ANNOTATE_CONDVAR_WAIT() and ANNOTATE_CONDVAR_SIGNAL() but to keep > their aliases ANNOTATE_HAPPENS_BEFORE() and ANNOTATE_HAPPENS_AFTER(). > The names of the first two macro's are really confusing: these two > macro's are a.o. used to annotate ordering constraints between > mutexes(!) in racecheck_unittest.cc. Please ignore the above comment -- I was misled by the statement ANNOTATE_CONDVAR_SIGNAL(&mu) in racecheck_unittest.cc. By this time I figured out that &mu is not the address of a mutex but the address of a condition variable. Bart. |
|
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. > |
|
From: Konstantin S. <kon...@gm...> - 2009-06-01 08:29:14
|
On Sun, May 31, 2009 at 11:14 PM, Bart Van Assche <bar...@gm...> wrote: > On Sat, May 30, 2009 at 9:12 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. >> >> Another remark: I suggest to remove the macro's >> ANNOTATE_CONDVAR_WAIT() and ANNOTATE_CONDVAR_SIGNAL() but to keep >> their aliases ANNOTATE_HAPPENS_BEFORE() and ANNOTATE_HAPPENS_AFTER(). >> The names of the first two macro's are really confusing: these two >> macro's are a.o. used to annotate ordering constraints between >> mutexes(!) in racecheck_unittest.cc. > > Please ignore the above comment -- I was misled by the statement > ANNOTATE_CONDVAR_SIGNAL(&mu) in racecheck_unittest.cc. By this time I > figured out that &mu is not the address of a mutex but the address of > a condition variable. The parameter of ANNOTATE_CONDVAR_ is any pointer. Since I introduced the HAPPENS_BEFORE/AFTER aliases, CONDVAR annotations are supposed to be used only with cond vars (which is only required in hybrid mode). The aliases are there just to avoid confusion. --kcc > > Bart. > |
|
From: Bart V. A. <bar...@gm...> - 2009-06-01 16:19:45
|
On Mon, Jun 1, 2009 at 10:28 AM, Konstantin Serebryany <kon...@gm...> wrote: > On Sun, May 31, 2009 at 11:14 PM, Bart Van Assche > <bar...@gm...> wrote: >> On Sat, May 30, 2009 at 9:12 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. >>> >>> Another remark: I suggest to remove the macro's >>> ANNOTATE_CONDVAR_WAIT() and ANNOTATE_CONDVAR_SIGNAL() but to keep >>> their aliases ANNOTATE_HAPPENS_BEFORE() and ANNOTATE_HAPPENS_AFTER(). >>> The names of the first two macro's are really confusing: these two >>> macro's are a.o. used to annotate ordering constraints between >>> mutexes(!) in racecheck_unittest.cc. >> >> Please ignore the above comment -- I was misled by the statement >> ANNOTATE_CONDVAR_SIGNAL(&mu) in racecheck_unittest.cc. By this time I >> figured out that &mu is not the address of a mutex but the address of >> a condition variable. > > The parameter of ANNOTATE_CONDVAR_ is any pointer. > Since I introduced the HAPPENS_BEFORE/AFTER aliases, CONDVAR > annotations are supposed to be used only with cond vars (which is only > required in hybrid mode). > The aliases are there just to avoid confusion. IMHO it would be an improvement if it would be specified explicitly in the header file dynamic_annotations.h that the ANNOTATE_CONDVAR_* macro's should be used only with condition variables (pthread_condvar_t*) and that the ANNOTATE_HAPPENS_* macro's should be used only with other objects than POSIX synchronization objects. That would allow pure happens-before data race detectors to ignore the ANNOTATE_CONDVAR_* annotations and only handle the ANNOTATE_HAPPENS_* annotations. Can you please also update regression tests 30 and 31 ? These tests currently use the ANNOTATE_CONDVAR_* macro's while according to what you wrote these should use the ANNOTATE_HAPPENS_* macro's. One additional remark regarding the ANNOTATE_HAPPENS_* macro's: a data race detection tool has to allocate some memory to keep track of the inter-thread ordering imposed by these annotations. Since ANNOTATE_HAPPENS_AFTER may be invoked multiple times with the same argument, a tool cannot know when it can free the memory allocated to implement such an annotation. I have added the ANNOTATE_HAPPENS_AFTER_DONE() annotation in drd.h for this purpose. Bart. |
|
From: Bart V. A. <bar...@gm...> - 2009-06-01 17:54:27
|
On Mon, Jun 1, 2009 at 10:24 AM, Konstantin Serebryany <kon...@gm...> wrote: > On Sat, May 30, 2009 at 3:35 PM, Bart Van Assche > <bar...@gm...> wrote: >> 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. The annotations should be general enough such that these are useful for any modern memory architecture. It's not entirely clear to me what the intended semantics of PUBLISH_MEMORY_RANGE() is. How does it e.g. map on the memory barrier instructions as defined by the Alpha architecture or the acquire/release labels as defined by the Itanium architecture ? On these two architectures making sure that all the store operations performed on one CPU are visible on another CPU requires the following: * First CPU modifies an object as necessary. * First CPU issues a memory barrier and sets a flag (Alpha) or updates a flag via a store operation that has release semantics. * Second CPU observes that the flag has been set and issues a memory barrier (Alpha) or observes that the flag has been modified through a load with acquire semantics (Itanium). * Second CPU loads object data. The update of the flag is necessary to make sure that all store operations performed by the first CPU will be observed by the second CPU: many memory consistency models allow stores to be reordered if not explicitly prevented. My point is that on a multiprocessor with sufficiently weak ordering guarantees, you can't just publish memory modifications. Cooperation of the consumer is needed to make sure that the intended semantics are realized. Looking at unit test 92 I get the impression that the semantics of PUBLISH_MEMORY_RANGE() is similar to that of the happens before / happens after annotations but only for an address range instead of all memory locations ? >> * 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. OK, I'll have a look at these annotations and unit tests. Bart. |
|
From: Konstantin S. <kon...@gm...> - 2009-06-02 07:50:06
|
On Mon, Jun 1, 2009 at 9:54 PM, Bart Van Assche
<bar...@gm...> wrote:
> On Mon, Jun 1, 2009 at 10:24 AM, Konstantin Serebryany
> <kon...@gm...> wrote:
>> On Sat, May 30, 2009 at 3:35 PM, Bart Van Assche
>> <bar...@gm...> wrote:
>>> 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.
>
> The annotations should be general enough such that these are useful
> for any modern memory architecture. It's not entirely clear to me what
> the intended semantics of PUBLISH_MEMORY_RANGE() is. How does it e.g.
> map on the memory barrier instructions as defined by the Alpha
> architecture or the acquire/release labels as defined by the Itanium
> architecture ? On these two architectures making sure that all the
> store operations performed on one CPU are visible on another CPU
> requires the following:
> * First CPU modifies an object as necessary.
> * First CPU issues a memory barrier and sets a flag (Alpha) or updates
> a flag via a store operation that has release semantics.
> * Second CPU observes that the flag has been set and issues a memory
> barrier (Alpha) or observes that the flag has been modified through a
> load with acquire semantics (Itanium).
> * Second CPU loads object data.
> The update of the flag is necessary to make sure that all store
> operations performed by the first CPU will be observed by the second
> CPU: many memory consistency models allow stores to be reordered if
> not explicitly prevented. My point is that on a multiprocessor with
> sufficiently weak ordering guarantees, you can't just publish memory
> modifications. Cooperation of the consumer is needed to make sure that
> the intended semantics are realized.
Let's not argue about lock-less synchronization for now.
Just think about hybrid detectors:
Object *o = NULL;
void Thread1() {
Object *t = new Object;
ScopedMutexLock lock(&mu);
o = t;
ANNOTATE_PUBLISH_MEMORY_RANGE(o, sizeof(*o)) ;
}
void Thread2() {
ScopedMutexLock lock(&mu);
if (o) o->UseMe();
}
...
// we have 999 different places where 'o' is used.
void Thread999() {
ScopedMutexLock lock(&mu);
if (o) o->UseMeInSomeOtherWay();
}
Here, w/o the annotations, a hybrid detector will report a false
positive because the object was constructed outside the mutex.
The same thing could be done with ANNOTATE_HAPPENS_*, but it will
require 1000 annotations instead of just one.
>
> Looking at unit test 92 I get the impression that the semantics of
> PUBLISH_MEMORY_RANGE() is similar to that of the happens before /
> happens after annotations but only for an address range instead of all
> memory locations ?
>
PUBLISH_MEMORY_RANGE creates the same h-b arch as any other h-b
annotation or as e.g. sem_post/sem_wait.
The difference is that there is just one annotation.
The h-b edge is created between the call to PUBLISH_MEMORY_RANGE(mem,
size) and subsequent accesses to memory within the range [mem,
mem+size).
Once the memory [mem, mem+size) is freed, this stops.
--kcc
|
|
From: Konstantin S. <kon...@gm...> - 2009-06-02 07:37:24
|
On Mon, Jun 1, 2009 at 8:19 PM, Bart Van Assche <bar...@gm...> wrote: > On Mon, Jun 1, 2009 at 10:28 AM, Konstantin Serebryany > <kon...@gm...> wrote: >> On Sun, May 31, 2009 at 11:14 PM, Bart Van Assche >> <bar...@gm...> wrote: >>> On Sat, May 30, 2009 at 9:12 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. >>>> >>>> Another remark: I suggest to remove the macro's >>>> ANNOTATE_CONDVAR_WAIT() and ANNOTATE_CONDVAR_SIGNAL() but to keep >>>> their aliases ANNOTATE_HAPPENS_BEFORE() and ANNOTATE_HAPPENS_AFTER(). >>>> The names of the first two macro's are really confusing: these two >>>> macro's are a.o. used to annotate ordering constraints between >>>> mutexes(!) in racecheck_unittest.cc. >>> >>> Please ignore the above comment -- I was misled by the statement >>> ANNOTATE_CONDVAR_SIGNAL(&mu) in racecheck_unittest.cc. By this time I >>> figured out that &mu is not the address of a mutex but the address of >>> a condition variable. >> >> The parameter of ANNOTATE_CONDVAR_ is any pointer. >> Since I introduced the HAPPENS_BEFORE/AFTER aliases, CONDVAR >> annotations are supposed to be used only with cond vars (which is only >> required in hybrid mode). >> The aliases are there just to avoid confusion. > > IMHO it would be an improvement if it would be specified explicitly in > the header file dynamic_annotations.h that the ANNOTATE_CONDVAR_* > macro's should be used only with condition variables > (pthread_condvar_t*) and that the ANNOTATE_HAPPENS_* macro's should be > used only with other objects than POSIX synchronization objects. That > would allow pure happens-before data race detectors to ignore the > ANNOTATE_CONDVAR_* annotations and only handle the ANNOTATE_HAPPENS_* > annotations. Hm. Dunno. Maybe. BTW, please don't forget that there are other synchronization primitives in the world, not just POSIX objects. > > Can you please also update regression tests 30 and 31 ? These tests > currently use the ANNOTATE_CONDVAR_* macro's while according to what > you wrote these should use the ANNOTATE_HAPPENS_* macro's. Done, thanks! > > One additional remark regarding the ANNOTATE_HAPPENS_* macro's: a data > race detection tool has to allocate some memory to keep track of the > inter-thread ordering imposed by these annotations. True. > Since > ANNOTATE_HAPPENS_AFTER may be invoked multiple times with the same > argument, a tool cannot know when it can free the memory allocated to > implement such an annotation. I have added the > ANNOTATE_HAPPENS_AFTER_DONE() annotation in drd.h for this purpose. Grr. I would avoid doing this. Once the program frees the memory which was used as an argument to ANNOTATE_HAPPENS_*, the detector may release the resources. (ThreadSanitizer doesn't do this now. I haven't seen this as a problem, but maybe it is...) > > Bart. > |
|
From: Bart V. A. <bar...@gm...> - 2009-06-02 07:54:21
|
On Tue, Jun 2, 2009 at 9:36 AM, Konstantin Serebryany <kon...@gm...> wrote: > On Mon, Jun 1, 2009 at 8:19 PM, Bart Van Assche >> One additional remark regarding the ANNOTATE_HAPPENS_* macro's: a data >> race detection tool has to allocate some memory to keep track of the >> inter-thread ordering imposed by these annotations. > > True. > >> Since >> ANNOTATE_HAPPENS_AFTER may be invoked multiple times with the same >> argument, a tool cannot know when it can free the memory allocated to >> implement such an annotation. I have added the >> ANNOTATE_HAPPENS_AFTER_DONE() annotation in drd.h for this purpose. > > I would avoid doing this. > Once the program frees the memory which was used as an argument to > ANNOTATE_HAPPENS_*, the detector may release the resources. > (ThreadSanitizer doesn't do this now. I haven't seen this as a > problem, but maybe it is...) I agree that the tool should free the allocated resources at the time the client object is freed. But what should a tool do when the argument passed to ANNOTATE_HAPPENS_* is not a valid client address, such as in unit tests 30 and 31 ? Bart. |
|
From: Konstantin S. <kon...@gm...> - 2009-06-02 08:05:30
|
On Tue, Jun 2, 2009 at 11:54 AM, Bart Van Assche <bar...@gm...> wrote: > On Tue, Jun 2, 2009 at 9:36 AM, Konstantin Serebryany > <kon...@gm...> wrote: >> On Mon, Jun 1, 2009 at 8:19 PM, Bart Van Assche >>> One additional remark regarding the ANNOTATE_HAPPENS_* macro's: a data >>> race detection tool has to allocate some memory to keep track of the >>> inter-thread ordering imposed by these annotations. >> >> True. >> >>> Since >>> ANNOTATE_HAPPENS_AFTER may be invoked multiple times with the same >>> argument, a tool cannot know when it can free the memory allocated to >>> implement such an annotation. I have added the >>> ANNOTATE_HAPPENS_AFTER_DONE() annotation in drd.h for this purpose. >> >> I would avoid doing this. >> Once the program frees the memory which was used as an argument to >> ANNOTATE_HAPPENS_*, the detector may release the resources. >> (ThreadSanitizer doesn't do this now. I haven't seen this as a >> problem, but maybe it is...) > > I agree that the tool should free the allocated resources at the time > the client object is freed. But what should a tool do when the > argument passed to ANNOTATE_HAPPENS_* is not a valid client address, > such as in unit tests 30 and 31 ? Hm. Maybe just leak the resources? I hate to have annotations that do not represent any synchronization and are need just for bookkeeping. In ThreadSanitizer I have to cleanup the whole state from time to time (usually, once per 10-20 minutes). This is done because of other reasons, but the leaked objects are freed as well. --kcc > > Bart. > |
|
From: Bart V. A. <bar...@gm...> - 2009-06-24 19:13:34
|
On Fri, May 29, 2009 at 12:58 PM, Konstantin Serebryany<kon...@gm...> wrote: > [changed subject] > > Julian, Bart, > > 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. My last set of remarks on the annotations defined in dynamic_annotations.h: * Currently there are two macro's defined for annotating acquiring and releasing an rwlock, namely ANNOTATE_RWLOCK_ACQUIRED(lock, is_w) ANNOTATE_RWLOCK_RELEASED(lock, is_w). The second argument, is_w, is a boolean. Having to specify 1 for a writer lock and 0 for a reader lock makes readability of the annotations subobtimal. Has it already been considered to replace these two macro's by e.g. the following four macro's: ANNOTATE_ACQUIRED_READERLOCK(lock), ANNOTATE_ACQUIRED_WRITERLOCK(lock), ANNOTATE_RELEASED_READERLOCK(lock) and ANNOTATE_RELEASED_WRITERLOCK(lock) ? * The source file thread_wrappers_pthread.h makes it clear that the ANNOTATE_RWLOCK_* macro's are used both for annotating reader-writer locks and for annotating spinlocks. It might be a good idea to define separate annotations for reader-writer locks and for spinlocks. This would make it easier to implement these macro's in Helgrind and DRD at least. Bart. |
|
From: Konstantin S. <kon...@gm...> - 2009-06-25 09:15:49
|
On Wed, Jun 24, 2009 at 11:11 PM, Bart Van Assche<bar...@gm...> wrote: > On Fri, May 29, 2009 at 12:58 PM, Konstantin > Serebryany<kon...@gm...> wrote: >> [changed subject] >> >> Julian, Bart, >> >> 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. > > My last set of remarks on the annotations defined in dynamic_annotations.h: > * Currently there are two macro's defined for annotating acquiring and > releasing an rwlock, namely ANNOTATE_RWLOCK_ACQUIRED(lock, is_w) > ANNOTATE_RWLOCK_RELEASED(lock, is_w). The second argument, is_w, is a > boolean. Having to specify 1 for a writer lock and 0 for a reader lock > makes readability of the annotations subobtimal. Has it already been > considered to replace these two macro's by e.g. the following four > macro's: ANNOTATE_ACQUIRED_READERLOCK(lock), > ANNOTATE_ACQUIRED_WRITERLOCK(lock), ANNOTATE_RELEASED_READERLOCK(lock) > and ANNOTATE_RELEASED_WRITERLOCK(lock) ? Instead of replacing, we can create aliases, e.g. #define ANNOTATE_READERLOCK_ACQUIRED(lock) ANNOTATE_RWLOCK_ACQUIRED(lock, 0) The original macros have to remain there since there are cases when the second argument is not a constant, but a parameter that comes from somewhere else. > * The source file thread_wrappers_pthread.h makes it clear that the > ANNOTATE_RWLOCK_* macro's are used both for annotating reader-writer > locks and for annotating spinlocks. ... or any other kind of mutex lock. > It might be a good idea to define > separate annotations for reader-writer locks and for spinlocks. Why? I don't really see a difference from a race detector's point of view. > This would make it easier to implement these macro's in Helgrind and DRD at least. All these annotations were implemented in helgrind (HGDEV branch) w/o any problem. --kcc > > Bart. > |
|
From: Bart V. A. <bar...@gm...> - 2009-06-25 09:29:33
|
On Thu, Jun 25, 2009 at 11:15 AM, Konstantin Serebryany <kon...@gm...> wrote: > > On Wed, Jun 24, 2009 at 11:11 PM, Bart Van > Assche<bar...@gm...> wrote: > > On Fri, May 29, 2009 at 12:58 PM, Konstantin > > Serebryany<kon...@gm...> wrote: > >> [changed subject] > >> > >> Julian, Bart, > >> > >> 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. > > > > My last set of remarks on the annotations defined in dynamic_annotations.h: > > * Currently there are two macro's defined for annotating acquiring and > > releasing an rwlock, namely ANNOTATE_RWLOCK_ACQUIRED(lock, is_w) > > ANNOTATE_RWLOCK_RELEASED(lock, is_w). The second argument, is_w, is a > > boolean. Having to specify 1 for a writer lock and 0 for a reader lock > > makes readability of the annotations subobtimal. Has it already been > > considered to replace these two macro's by e.g. the following four > > macro's: ANNOTATE_ACQUIRED_READERLOCK(lock), > > ANNOTATE_ACQUIRED_WRITERLOCK(lock), ANNOTATE_RELEASED_READERLOCK(lock) > > and ANNOTATE_RELEASED_WRITERLOCK(lock) ? > > Instead of replacing, we can create aliases, e.g. > #define ANNOTATE_READERLOCK_ACQUIRED(lock) ANNOTATE_RWLOCK_ACQUIRED(lock, 0) That's also fine for me. > > * The source file thread_wrappers_pthread.h makes it clear that the > > ANNOTATE_RWLOCK_* macro's are used both for annotating reader-writer > > locks and for annotating spinlocks. > ... or any other kind of mutex lock. > > > It might be a good idea to define > > separate annotations for reader-writer locks and for spinlocks. > > Why? I don't really see a difference from a race detector's point of view. >From a data race detector's point of view this indeed doesn't make a big difference. But defining separate annotations for spinlocks makes the intention more clear in the annotated source code -- seeing ANNOTATE_RWLOCK_...() around spinlocks looks a little bit strange to me. And defining separate annotations would also slightly simplify the implementation of the rwlock annotations in data race detectors that already support spinlocks (currently only DRD). Bart. |
|
From: Konstantin S. <kon...@gm...> - 2009-06-25 10:19:13
|
On Thu, Jun 25, 2009 at 1:29 PM, Bart Van Assche<bar...@gm...> wrote: > On Thu, Jun 25, 2009 at 11:15 AM, Konstantin Serebryany > <kon...@gm...> wrote: >> >> On Wed, Jun 24, 2009 at 11:11 PM, Bart Van >> Assche<bar...@gm...> wrote: >> > On Fri, May 29, 2009 at 12:58 PM, Konstantin >> > Serebryany<kon...@gm...> wrote: >> >> [changed subject] >> >> >> >> Julian, Bart, >> >> >> >> 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. >> > >> > My last set of remarks on the annotations defined in dynamic_annotations.h: >> > * Currently there are two macro's defined for annotating acquiring and >> > releasing an rwlock, namely ANNOTATE_RWLOCK_ACQUIRED(lock, is_w) >> > ANNOTATE_RWLOCK_RELEASED(lock, is_w). The second argument, is_w, is a >> > boolean. Having to specify 1 for a writer lock and 0 for a reader lock >> > makes readability of the annotations subobtimal. Has it already been >> > considered to replace these two macro's by e.g. the following four >> > macro's: ANNOTATE_ACQUIRED_READERLOCK(lock), >> > ANNOTATE_ACQUIRED_WRITERLOCK(lock), ANNOTATE_RELEASED_READERLOCK(lock) >> > and ANNOTATE_RELEASED_WRITERLOCK(lock) ? >> >> Instead of replacing, we can create aliases, e.g. >> #define ANNOTATE_READERLOCK_ACQUIRED(lock) ANNOTATE_RWLOCK_ACQUIRED(lock, 0) > > That's also fine for me. > >> > * The source file thread_wrappers_pthread.h makes it clear that the >> > ANNOTATE_RWLOCK_* macro's are used both for annotating reader-writer >> > locks and for annotating spinlocks. >> ... or any other kind of mutex lock. >> >> > It might be a good idea to define >> > separate annotations for reader-writer locks and for spinlocks. >> >> Why? I don't really see a difference from a race detector's point of view. > > From a data race detector's point of view this indeed doesn't make a > big difference. But defining separate annotations for spinlocks makes > the intention more clear in the annotated source code -- seeing > ANNOTATE_RWLOCK_...() around spinlocks looks a little bit strange to > me. We can do this with aliases, no? > And defining separate annotations would also slightly simplify the > implementation of the rwlock annotations in data race detectors that > already support spinlocks (currently only DRD). You mean, the pthread_spin_lock()? It should be supported directly, not annotated. There is a problem however: pthread_spin_unlock and pthread_spin_init are the same symbol in some libc implementations. That's another story... --kcc > > Bart. > |
|
From: Konstantin S. <kon...@gm...> - 2009-06-26 13:06:21
|
Just FYI. We've added dynamic annotations to Chromium: http://src.chromium.org/viewvc/chrome?view=rev&revision=19353 In Chromium there are few classes/functions that use atomic instructions (e.g. Singleton, atomic refcount), so that w/o the annotations every race detector will have false positives. I wonder if other large opensource projects (e.g. Firefox, OpenOffice, etc) use atomics for synchronization. Any experience detecting races there? --kcc On Thu, Jun 25, 2009 at 2:18 PM, Konstantin Serebryany<kon...@gm...> wrote: > On Thu, Jun 25, 2009 at 1:29 PM, Bart Van > Assche<bar...@gm...> wrote: >> On Thu, Jun 25, 2009 at 11:15 AM, Konstantin Serebryany >> <kon...@gm...> wrote: >>> >>> On Wed, Jun 24, 2009 at 11:11 PM, Bart Van >>> Assche<bar...@gm...> wrote: >>> > On Fri, May 29, 2009 at 12:58 PM, Konstantin >>> > Serebryany<kon...@gm...> wrote: >>> >> [changed subject] >>> >> >>> >> Julian, Bart, >>> >> >>> >> 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. >>> > >>> > My last set of remarks on the annotations defined in dynamic_annotations.h: >>> > * Currently there are two macro's defined for annotating acquiring and >>> > releasing an rwlock, namely ANNOTATE_RWLOCK_ACQUIRED(lock, is_w) >>> > ANNOTATE_RWLOCK_RELEASED(lock, is_w). The second argument, is_w, is a >>> > boolean. Having to specify 1 for a writer lock and 0 for a reader lock >>> > makes readability of the annotations subobtimal. Has it already been >>> > considered to replace these two macro's by e.g. the following four >>> > macro's: ANNOTATE_ACQUIRED_READERLOCK(lock), >>> > ANNOTATE_ACQUIRED_WRITERLOCK(lock), ANNOTATE_RELEASED_READERLOCK(lock) >>> > and ANNOTATE_RELEASED_WRITERLOCK(lock) ? >>> >>> Instead of replacing, we can create aliases, e.g. >>> #define ANNOTATE_READERLOCK_ACQUIRED(lock) ANNOTATE_RWLOCK_ACQUIRED(lock, 0) >> >> That's also fine for me. >> >>> > * The source file thread_wrappers_pthread.h makes it clear that the >>> > ANNOTATE_RWLOCK_* macro's are used both for annotating reader-writer >>> > locks and for annotating spinlocks. >>> ... or any other kind of mutex lock. >>> >>> > It might be a good idea to define >>> > separate annotations for reader-writer locks and for spinlocks. >>> >>> Why? I don't really see a difference from a race detector's point of view. >> >> From a data race detector's point of view this indeed doesn't make a >> big difference. But defining separate annotations for spinlocks makes >> the intention more clear in the annotated source code -- seeing >> ANNOTATE_RWLOCK_...() around spinlocks looks a little bit strange to >> me. > > We can do this with aliases, no? > >> And defining separate annotations would also slightly simplify the >> implementation of the rwlock annotations in data race detectors that >> already support spinlocks (currently only DRD). > > You mean, the pthread_spin_lock()? > It should be supported directly, not annotated. > There is a problem however: pthread_spin_unlock and pthread_spin_init > are the same symbol in some libc implementations. That's another > story... > > --kcc >> >> Bart. >> > |
|
From: Bart V. A. <bar...@gm...> - 2009-06-28 09:55:30
|
On Fri, Jun 26, 2009 at 2:41 PM, Konstantin Serebryany<kon...@gm...> wrote: > We've added dynamic annotations to Chromium: > http://src.chromium.org/viewvc/chrome?view=rev&revision=19353 > In Chromium there are few classes/functions that use atomic > instructions (e.g. Singleton, atomic refcount), so that w/o the > annotations every race detector will have false positives. Interesting. > I wonder if other large opensource projects (e.g. Firefox, OpenOffice, > etc) use atomics for synchronization. > Any experience detecting races there? Regarding Firefox: some time ago I tried to run Firefox under DRD. You can find the feedback I received about using atomics for the debug counters in this bugzilla entry: https://bugzilla.mozilla.org/show_bug.cgi?id=425923. Another open source project that uses multithreading extensively (through Open/MP) is GraphicsMagick. The maintainers of this project have added support in their makefiles to analyze all regression tests with memcheck, ptrcheck, helgrind and DRD. Bart. |
|
From: Stefan K. <en...@ho...> - 2009-07-06 22:52:03
|
Bart Van Assche schrieb: > On Fri, Jun 26, 2009 at 2:41 PM, Konstantin > Serebryany<kon...@gm...> wrote: > >> We've added dynamic annotations to Chromium: >> http://src.chromium.org/viewvc/chrome?view=rev&revision=19353 >> In Chromium there are few classes/functions that use atomic >> instructions (e.g. Singleton, atomic refcount), so that w/o the >> annotations every race detector will have false positives. >> > > Interesting. > > >> I wonder if other large opensource projects (e.g. Firefox, OpenOffice, >> etc) use atomics for synchronization. >> Any experience detecting races there? >> > > Regarding Firefox: some time ago I tried to run Firefox under DRD. You > can find the feedback I received about using atomics for the debug > counters in this bugzilla entry: > https://bugzilla.mozilla.org/show_bug.cgi?id=425923. > > Another open source project that uses multithreading extensively > (through Open/MP) is GraphicsMagick. The maintainers of this project > have added support in their makefiles to analyze all regression tests > with memcheck, ptrcheck, helgrind and DRD. > GStreamer is heavily multithreaded too and uses both mutexes and glibs atomic operations. The unit tests are already using valgrind's memcheck in a 2nd run. I've tried drd and helgrind once, but got a lot of warnings and had no idea where to start. Stefan > Bart. > > ------------------------------------------------------------------------------ > _______________________________________________ > Valgrind-developers mailing list > Val...@li... > https://lists.sourceforge.net/lists/listinfo/valgrind-developers > |
|
From: Konstantin S. <kon...@gm...> - 2009-07-07 07:18:58
|
> GStreamer is heavily multithreaded too and uses both mutexes and glibs > atomic operations. The unit tests are already using valgrind's memcheck > in a 2nd run. I've tried drd and helgrind once, but got a lot of > warnings and had no idea where to start. Dynamic annotations (supported by ThreadSanitizer and, afaik, by fresh DRD) can help you explain lock-less synchronization to the tool. --kcc |