Added dynamic annotation files into base/...,...

Added dynamic annotation files into base/.
Added annotations for atomic reference counting, LazyInstance and Singleton classes.

This changelist is a part of an effort of adding ThreadSanitizer support for Chromium.
See http://code.google.com/p/data-race-test/wiki/ThreadSanitizer

Patch by Timur Iskhodzhanov.

[Dean McNamee, 2009-06-23 12:58:15]

This looks like a good start.  Some nits.  Also, most people reading this code
won't understand how data race detectors work, so if you could expand the
comments it would be really helpful.

http://codereview.chromium.org/147008/diff/1022/40
File base/atomic_ref_count.h (right):

http://codereview.chromium.org/147008/diff/1022/40#newcode8
Line 8: // The implementation includes annotations to avoid some false alarms
false arms should be false positives

http://codereview.chromium.org/147008/diff/1022/40#newcode9
Line 9: // when using data race detectors.
data race detection tools?

http://codereview.chromium.org/147008/diff/1022/38
File base/dynamic_annotations.cc (right):

http://codereview.chromium.org/147008/diff/1022/38#newcode12
Line 12: const volatile void *lock){}
space after )

http://codereview.chromium.org/147008/diff/1022/38#newcode65
Line 65: #endif
Will we use this?

http://codereview.chromium.org/147008/diff/1022/42
File base/dynamic_annotations.h (right):

http://codereview.chromium.org/147008/diff/1022/42#newcode22
Line 22: // - ThreadSanitizer (NDEBUG is not defined).
Shouldn't this only happen on non-windows?

http://codereview.chromium.org/147008/diff/1022/42#newcode30
Line 30: #ifndef NDEBUG
  // Debug build

http://codereview.chromium.org/147008/diff/1022/42#newcode32
Line 32: // -------------------------------------------------------------
we don't indent within an ifdef

http://codereview.chromium.org/147008/diff/1022/42#newcode233
Line 233: #else  // NDEBUG is defined
Release build, empty macros.

http://codereview.chromium.org/147008/diff/1022/42#newcode314
Line 314: const volatile void *arg);
Do we care about these declarations if we're in release?

http://codereview.chromium.org/147008/diff/1022/42#newcode316
Line 316: #ifndef NDEBUG
  // Debug build

http://codereview.chromium.org/147008/diff/1022/42#newcode346
Line 346: #else // !NDEBUG
Make these all either Release or Debug build, it's confusing otherwise.

http://codereview.chromium.org/147008/diff/1022/42#newcode354
Line 354: extern "C" int RunningOnValgrind();
Do we need / want this?

http://codereview.chromium.org/147008/diff/1022/39
File base/lazy_instance.cc (right):

http://codereview.chromium.org/147008/diff/1022/39#newcode25
Line 25: // See the corresponding HAPPENS_AFTER in Pointer().
Can you expand on these comments?  I understand the idea, but I'm not that
familiar with the tools, and the goes for a lot of other people.

[the_wrong_timurrrr, 2009-06-23 14:55:39]

http://codereview.chromium.org/147008/diff/1022/40
File base/atomic_ref_count.h (right):

http://codereview.chromium.org/147008/diff/1022/40#newcode8
Line 8: // The implementation includes annotations to avoid some false alarms
On 2009/06/23 12:58:15, Dean McNamee wrote:
> false arms should be false positives

Done.

http://codereview.chromium.org/147008/diff/1022/40#newcode9
Line 9: // when using data race detectors.
On 2009/06/23 12:58:15, Dean McNamee wrote:
> data race detection tools?

Done.

http://codereview.chromium.org/147008/diff/1022/38
File base/dynamic_annotations.cc (right):

http://codereview.chromium.org/147008/diff/1022/38#newcode12
Line 12: const volatile void *lock){}
On 2009/06/23 12:58:15, Dean McNamee wrote:
> space after )

Done.

http://codereview.chromium.org/147008/diff/1022/38#newcode65
Line 65: #endif
On 2009/06/23 12:58:15, Dean McNamee wrote:
> Will we use this?
See the comment in base/dynamic_annotations.h

http://codereview.chromium.org/147008/diff/1022/42
File base/dynamic_annotations.h (right):

http://codereview.chromium.org/147008/diff/1022/42#newcode22
Line 22: // - ThreadSanitizer (NDEBUG is not defined).
On 2009/06/23 12:58:15, Dean McNamee wrote:
> Shouldn't this only happen on non-windows?
At the moment ThreadSanitizer works on Linux-only.
However, one may run Windows binaries under Wine under ThreadSanitizer.

http://codereview.chromium.org/147008/diff/1022/42#newcode30
Line 30: #ifndef NDEBUG
On 2009/06/23 12:58:15, Dean McNamee wrote:
>   // Debug build

Done.

http://codereview.chromium.org/147008/diff/1022/42#newcode32
Line 32: // -------------------------------------------------------------
On 2009/06/23 12:58:15, Dean McNamee wrote:
> we don't indent within an ifdef

Done.

http://codereview.chromium.org/147008/diff/1022/42#newcode233
Line 233: #else  // NDEBUG is defined
On 2009/06/23 12:58:15, Dean McNamee wrote:
> Release build, empty macros.

Done.

http://codereview.chromium.org/147008/diff/1022/42#newcode314
Line 314: const volatile void *arg);
On 2009/06/23 12:58:15, Dean McNamee wrote:
> Do we care about these declarations if we're in release?

Done.

http://codereview.chromium.org/147008/diff/1022/42#newcode316
Line 316: #ifndef NDEBUG
On 2009/06/23 12:58:15, Dean McNamee wrote:
>   // Debug build

Done.

http://codereview.chromium.org/147008/diff/1022/42#newcode346
Line 346: #else // !NDEBUG
On 2009/06/23 12:58:15, Dean McNamee wrote:
> Make these all either Release or Debug build, it's confusing otherwise.

Done.

http://codereview.chromium.org/147008/diff/1022/42#newcode354
Line 354: extern "C" int RunningOnValgrind();
On 2009/06/23 12:58:15, Dean McNamee wrote:
> Do we need / want this?
In this changelist - no.
In general - yes. For example, you can increase test timeouts if they are run
under Valgrind tools.
Also, be using RunningOnValgrind() it is possible to fix http://crbug.com/9194

http://codereview.chromium.org/147008/diff/1022/39
File base/lazy_instance.cc (right):

http://codereview.chromium.org/147008/diff/1022/39#newcode25
Line 25: // See the corresponding HAPPENS_AFTER in Pointer().
On 2009/06/23 12:58:15, Dean McNamee wrote:
> Can you expand on these comments?  I understand the idea, but I'm not that
> familiar with the tools, and the goes for a lot of other people.

Done.

[the_wrong_timurrrr, 2009-06-25 13:54:49]

On 2009/06/23 14:55:39, timurrrr wrote:
> http://codereview.chromium.org/147008/diff/1022/40
> File base/atomic_ref_count.h (right):
> 
> http://codereview.chromium.org/147008/diff/1022/40#newcode8
> Line 8: // The implementation includes annotations to avoid some false alarms
> On 2009/06/23 12:58:15, Dean McNamee wrote:
> > false arms should be false positives
> 
> Done.
> 
> http://codereview.chromium.org/147008/diff/1022/40#newcode9
> Line 9: // when using data race detectors.
> On 2009/06/23 12:58:15, Dean McNamee wrote:
> > data race detection tools?
> 
> Done.
> 
> http://codereview.chromium.org/147008/diff/1022/38
> File base/dynamic_annotations.cc (right):
> 
> http://codereview.chromium.org/147008/diff/1022/38#newcode12
> Line 12: const volatile void *lock){}
> On 2009/06/23 12:58:15, Dean McNamee wrote:
> > space after )
> 
> Done.
> 
> http://codereview.chromium.org/147008/diff/1022/38#newcode65
> Line 65: #endif
> On 2009/06/23 12:58:15, Dean McNamee wrote:
> > Will we use this?
> See the comment in base/dynamic_annotations.h
> 
> http://codereview.chromium.org/147008/diff/1022/42
> File base/dynamic_annotations.h (right):
> 
> http://codereview.chromium.org/147008/diff/1022/42#newcode22
> Line 22: // - ThreadSanitizer (NDEBUG is not defined).
> On 2009/06/23 12:58:15, Dean McNamee wrote:
> > Shouldn't this only happen on non-windows?
> At the moment ThreadSanitizer works on Linux-only.
> However, one may run Windows binaries under Wine under ThreadSanitizer.
> 
> http://codereview.chromium.org/147008/diff/1022/42#newcode30
> Line 30: #ifndef NDEBUG
> On 2009/06/23 12:58:15, Dean McNamee wrote:
> >   // Debug build
> 
> Done.
> 
> http://codereview.chromium.org/147008/diff/1022/42#newcode32
> Line 32: // -------------------------------------------------------------
> On 2009/06/23 12:58:15, Dean McNamee wrote:
> > we don't indent within an ifdef
> 
> Done.
> 
> http://codereview.chromium.org/147008/diff/1022/42#newcode233
> Line 233: #else  // NDEBUG is defined
> On 2009/06/23 12:58:15, Dean McNamee wrote:
> > Release build, empty macros.
> 
> Done.
> 
> http://codereview.chromium.org/147008/diff/1022/42#newcode314
> Line 314: const volatile void *arg);
> On 2009/06/23 12:58:15, Dean McNamee wrote:
> > Do we care about these declarations if we're in release?
> 
> Done.
> 
> http://codereview.chromium.org/147008/diff/1022/42#newcode316
> Line 316: #ifndef NDEBUG
> On 2009/06/23 12:58:15, Dean McNamee wrote:
> >   // Debug build
> 
> Done.
> 
> http://codereview.chromium.org/147008/diff/1022/42#newcode346
> Line 346: #else // !NDEBUG
> On 2009/06/23 12:58:15, Dean McNamee wrote:
> > Make these all either Release or Debug build, it's confusing otherwise.
> 
> Done.
> 
> http://codereview.chromium.org/147008/diff/1022/42#newcode354
> Line 354: extern "C" int RunningOnValgrind();
> On 2009/06/23 12:58:15, Dean McNamee wrote:
> > Do we need / want this?
> In this changelist - no.
> In general - yes. For example, you can increase test timeouts if they are run
> under Valgrind tools.
> Also, be using RunningOnValgrind() it is possible to fix http://crbug.com/9194
> 
> http://codereview.chromium.org/147008/diff/1022/39
> File base/lazy_instance.cc (right):
> 
> http://codereview.chromium.org/147008/diff/1022/39#newcode25
> Line 25: // See the corresponding HAPPENS_AFTER in Pointer().
> On 2009/06/23 12:58:15, Dean McNamee wrote:
> > Can you expand on these comments?  I understand the idea, but I'm not that
> > familiar with the tools, and the goes for a lot of other people.
> 
> Done.

Is there anything else I should do?

[Dean McNamee, 2009-06-26 10:07:04]

I will commit it, just needs testing on Mac and Windows, etc.

On 2009/06/25 13:54:49, timurrrr wrote:
> On 2009/06/23 14:55:39, timurrrr wrote:
> > http://codereview.chromium.org/147008/diff/1022/40
> > File base/atomic_ref_count.h (right):
> > 
> > http://codereview.chromium.org/147008/diff/1022/40#newcode8
> > Line 8: // The implementation includes annotations to avoid some false
alarms
> > On 2009/06/23 12:58:15, Dean McNamee wrote:
> > > false arms should be false positives
> > 
> > Done.
> > 
> > http://codereview.chromium.org/147008/diff/1022/40#newcode9
> > Line 9: // when using data race detectors.
> > On 2009/06/23 12:58:15, Dean McNamee wrote:
> > > data race detection tools?
> > 
> > Done.
> > 
> > http://codereview.chromium.org/147008/diff/1022/38
> > File base/dynamic_annotations.cc (right):
> > 
> > http://codereview.chromium.org/147008/diff/1022/38#newcode12
> > Line 12: const volatile void *lock){}
> > On 2009/06/23 12:58:15, Dean McNamee wrote:
> > > space after )
> > 
> > Done.
> > 
> > http://codereview.chromium.org/147008/diff/1022/38#newcode65
> > Line 65: #endif
> > On 2009/06/23 12:58:15, Dean McNamee wrote:
> > > Will we use this?
> > See the comment in base/dynamic_annotations.h
> > 
> > http://codereview.chromium.org/147008/diff/1022/42
> > File base/dynamic_annotations.h (right):
> > 
> > http://codereview.chromium.org/147008/diff/1022/42#newcode22
> > Line 22: // - ThreadSanitizer (NDEBUG is not defined).
> > On 2009/06/23 12:58:15, Dean McNamee wrote:
> > > Shouldn't this only happen on non-windows?
> > At the moment ThreadSanitizer works on Linux-only.
> > However, one may run Windows binaries under Wine under ThreadSanitizer.
> > 
> > http://codereview.chromium.org/147008/diff/1022/42#newcode30
> > Line 30: #ifndef NDEBUG
> > On 2009/06/23 12:58:15, Dean McNamee wrote:
> > >   // Debug build
> > 
> > Done.
> > 
> > http://codereview.chromium.org/147008/diff/1022/42#newcode32
> > Line 32: // -------------------------------------------------------------
> > On 2009/06/23 12:58:15, Dean McNamee wrote:
> > > we don't indent within an ifdef
> > 
> > Done.
> > 
> > http://codereview.chromium.org/147008/diff/1022/42#newcode233
> > Line 233: #else  // NDEBUG is defined
> > On 2009/06/23 12:58:15, Dean McNamee wrote:
> > > Release build, empty macros.
> > 
> > Done.
> > 
> > http://codereview.chromium.org/147008/diff/1022/42#newcode314
> > Line 314: const volatile void *arg);
> > On 2009/06/23 12:58:15, Dean McNamee wrote:
> > > Do we care about these declarations if we're in release?
> > 
> > Done.
> > 
> > http://codereview.chromium.org/147008/diff/1022/42#newcode316
> > Line 316: #ifndef NDEBUG
> > On 2009/06/23 12:58:15, Dean McNamee wrote:
> > >   // Debug build
> > 
> > Done.
> > 
> > http://codereview.chromium.org/147008/diff/1022/42#newcode346
> > Line 346: #else // !NDEBUG
> > On 2009/06/23 12:58:15, Dean McNamee wrote:
> > > Make these all either Release or Debug build, it's confusing otherwise.
> > 
> > Done.
> > 
> > http://codereview.chromium.org/147008/diff/1022/42#newcode354
> > Line 354: extern "C" int RunningOnValgrind();
> > On 2009/06/23 12:58:15, Dean McNamee wrote:
> > > Do we need / want this?
> > In this changelist - no.
> > In general - yes. For example, you can increase test timeouts if they are
run
> > under Valgrind tools.
> > Also, be using RunningOnValgrind() it is possible to fix
http://crbug.com/9194
> > 
> > http://codereview.chromium.org/147008/diff/1022/39
> > File base/lazy_instance.cc (right):
> > 
> > http://codereview.chromium.org/147008/diff/1022/39#newcode25
> > Line 25: // See the corresponding HAPPENS_AFTER in Pointer().
> > On 2009/06/23 12:58:15, Dean McNamee wrote:
> > > Can you expand on these comments?  I understand the idea, but I'm not that
> > > familiar with the tools, and the goes for a lot of other people.
> > 
> > Done.
> 
> Is there anything else I should do?

[Dean McNamee, 2009-06-26 12:00:44]

Committed, and everything seemed ok.

Thanks again