|
From: Bart V. A. <bva...@ac...> - 2010-02-15 13:23:44
|
On Mon, Feb 15, 2010 at 12:52 PM, Konstantin Serebryany <kon...@gm...> wrote: > On Mon, Feb 15, 2010 at 2:45 PM, Bart Van Assche <bva...@ac...> wrote: >> On Mon, Feb 15, 2010 at 12:11 PM, <dat...@go...> wrote: >>> Revision: 1655 >>> Author: kon...@gm... >>> Date: Mon Feb 15 03:10:36 2010 >>> Log: added ANNOTATE_BENIGN_RACE_SIZED; ANNOTATE_BENIGN_RACE_STATIC uses >>> *_SIZED variant now >>> http://code.google.com/p/data-race-test/source/detail?r=1655 >> [ ... ] >> >> Hello Konstantin, >> >> Have you already considered deprecating ANNOTATE_BENIGN_RACE(), since >> this annotation doesn't have precisely defined semantics ? > > Well, it does have precisely defined semantics now. :) > It is equivalent to ANNOTATE_BENIGN_RACE_SIZED(mem, 1, descr); As far as I know this is the first time it is specified how many bytes ANNOTATE_BENIGN_RACE() applies to. I'm not sure it is a good idea to choose a range of one byte, given that e.g. the following code is present in the TSAN unittests: int GLOB = 0; ... ANNOTATE_BENIGN_RACE(&GLOB, "test56. Use of ANNOTATE_BENIGN_RACE."); Bart. |
|
From: Konstantin S. <kon...@gm...> - 2010-02-15 13:23:50
|
On Mon, Feb 15, 2010 at 3:17 PM, Bart Van Assche <bva...@ac...> wrote: > On Mon, Feb 15, 2010 at 12:52 PM, Konstantin Serebryany > <kon...@gm...> wrote: >> On Mon, Feb 15, 2010 at 2:45 PM, Bart Van Assche <bva...@ac...> wrote: >>> On Mon, Feb 15, 2010 at 12:11 PM, <dat...@go...> wrote: >>>> Revision: 1655 >>>> Author: kon...@gm... >>>> Date: Mon Feb 15 03:10:36 2010 >>>> Log: added ANNOTATE_BENIGN_RACE_SIZED; ANNOTATE_BENIGN_RACE_STATIC uses >>>> *_SIZED variant now >>>> http://code.google.com/p/data-race-test/source/detail?r=1655 >>> [ ... ] >>> >>> Hello Konstantin, >>> >>> Have you already considered deprecating ANNOTATE_BENIGN_RACE(), since >>> this annotation doesn't have precisely defined semantics ? >> >> Well, it does have precisely defined semantics now. :) >> It is equivalent to ANNOTATE_BENIGN_RACE_SIZED(mem, 1, descr); > > As far as I know this is the first time it is specified how many bytes > ANNOTATE_BENIGN_RACE() applies to. I'm not sure it is a good idea to > choose a range of one byte, given that e.g. the following code is > present in the TSAN unittests: > > int GLOB = 0; > ... > ANNOTATE_BENIGN_RACE(&GLOB, "test56. Use of ANNOTATE_BENIGN_RACE."); Yes, I agree. In ThreadSanitizer, when 2, 4 or 8 bytes are accessed by a single instruction and all previous accesses had the same size, the tool will only deal with the first byte. So, the annotation in this test still works correctly with ThreadSanitizer. This does not have to be a part of the definition of this annotation though. What do you think? The lack of 'size' argument in this annotation is my mistake from almost 2 years ago, but I can't just drop the current code now... :( --kcc > > Bart. > > -- > You received this message because you are subscribed to the Google Groups "data-race-test" group. > To post to this group, send email to dat...@go.... > To unsubscribe from this group, send email to dat...@go.... > For more options, visit this group at http://groups.google.com/group/data-race-test?hl=en. > > |
|
From: Konstantin S. <kon...@gm...> - 2010-02-16 18:54:56
|
Here is another idea. make ANNOTATE_BENIGN_RACE(ptr, descr) mark the bytes in range [ptr, sizeof(*ptr)). This will add a specific requirment that sizeof(*ptr) is a valid expression (i.e. ptr is not void*). This: - will have well defined semantics - will match the current use of this macro - will break rare cases when void* is passsed as a parameter. ? --kcc On Mon, Feb 15, 2010 at 3:26 PM, Konstantin Serebryany <kon...@gm...> wrote: > On Mon, Feb 15, 2010 at 3:17 PM, Bart Van Assche <bva...@ac...> wrote: >> On Mon, Feb 15, 2010 at 12:52 PM, Konstantin Serebryany >> <kon...@gm...> wrote: >>> On Mon, Feb 15, 2010 at 2:45 PM, Bart Van Assche <bva...@ac...> wrote: >>>> On Mon, Feb 15, 2010 at 12:11 PM, <dat...@go...> wrote: >>>>> Revision: 1655 >>>>> Author: kon...@gm... >>>>> Date: Mon Feb 15 03:10:36 2010 >>>>> Log: added ANNOTATE_BENIGN_RACE_SIZED; ANNOTATE_BENIGN_RACE_STATIC uses >>>>> *_SIZED variant now >>>>> http://code.google.com/p/data-race-test/source/detail?r=1655 >>>> [ ... ] >>>> >>>> Hello Konstantin, >>>> >>>> Have you already considered deprecating ANNOTATE_BENIGN_RACE(), since >>>> this annotation doesn't have precisely defined semantics ? >>> >>> Well, it does have precisely defined semantics now. :) >>> It is equivalent to ANNOTATE_BENIGN_RACE_SIZED(mem, 1, descr); >> >> As far as I know this is the first time it is specified how many bytes >> ANNOTATE_BENIGN_RACE() applies to. I'm not sure it is a good idea to >> choose a range of one byte, given that e.g. the following code is >> present in the TSAN unittests: >> >> int GLOB = 0; >> ... >> ANNOTATE_BENIGN_RACE(&GLOB, "test56. Use of ANNOTATE_BENIGN_RACE."); > > Yes, I agree. > In ThreadSanitizer, when 2, 4 or 8 bytes are accessed by a single > instruction and all previous accesses had the same size, the tool will > only deal with the first byte. > So, the annotation in this test still works correctly with ThreadSanitizer. > This does not have to be a part of the definition of this annotation > though. What do you think? > > The lack of 'size' argument in this annotation is my mistake from > almost 2 years ago, but I can't just drop the current code now... :( > > --kcc > > > > > > >> >> Bart. >> >> -- >> You received this message because you are subscribed to the Google Groups "data-race-test" group. >> To post to this group, send email to dat...@go.... >> To unsubscribe from this group, send email to dat...@go.... >> For more options, visit this group at http://groups.google.com/group/data-race-test?hl=en. >> >> > |
|
From: Bart V. A. <bva...@ac...> - 2010-02-17 07:10:47
|
On Tue, Feb 16, 2010 at 7:54 PM, Konstantin Serebryany < kon...@gm...> wrote: > Here is another idea. > > make ANNOTATE_BENIGN_RACE(ptr, descr) mark the bytes in range [ptr, > sizeof(*ptr)). > This will add a specific requirment that sizeof(*ptr) is a valid > expression (i.e. ptr is not void*). > This: > - will have well defined semantics > - will match the current use of this macro > - will break rare cases when void* is passed as a parameter. > Sounds like a good idea to me. And when ptr has type void*, inserting a cast to a pointer of the proper type is sufficient to get the old semantics back. Bart. |