Add atomics implementation for ThreadSanitizer v2 (https://sites.google.com/a/chromium.org/dev/deve…

Add atomics implementation for ThreadSanitizer v2 (https://sites.google.com/a/chromium.org/dev/developers/testing/threadsanitizer-tsan-v2)

BUG=128314
Committed: https://code.google.com/p/v8/source/detail?r=12953

[Alexander Potapenko, 2012-11-09 14:17:12]

Hi Daniel,

can you please take a look?
This change is required for TSan v2 to work correctly. We add calls to TSan
runtime that tell the tool about the atomic operations, thus removing a huge
number of false positives
(see
http://chromegw.corp.google.com/i/chromium.fyi/builders/Chromium%20Linux%20TS...
for an example of such false reports)

[Alexander Potapenko, 2012-11-09 14:36:13]

CC dvyukov@

[Alexander Potapenko, 2012-11-12 14:04:26]

+jkummerow
Jakob, any chance you can review this?

[danno, 2012-11-12 14:51:45]

The license and documentation seem to be taken directly from Chromium (e.g.
copyright and base/atomicops.h reference doesn't make much sense). Please add
this code with the V8 license like other files in this directory.

nit(s): please also fix the formatting to be Google code-style conform. In
addition to the stuff listed under "Lint" in codereview, a bunch of the
function's parameters have funky indenting on the follow-on line.

[Alexander Potapenko, 2012-11-12 16:50:24]

Fixed the license headers and most of the lint errors.

The remaining lint errors in atomicops.h are false positives caused by lint not
understanding the ifdefs. The added lines perfectly conform with the existing
code in atomicops.

Two "Use int16/int64/etc, rather than the C type short" errors in
atomicops_internal_tsan.h just ignore the NOLINT comment.

The "Could not find a newline character at the end of the file." is also
strange, because other files in this directory don't have any newline characters
either.

[danno, 2012-11-13 15:43:22]

Almost there...

https://codereview.chromium.org/10965035/diff/7002/src/atomicops_internals_ts...
File src/atomicops_internals_tsan.h (right):

https://codereview.chromium.org/10965035/diff/7002/src/atomicops_internals_ts...
src/atomicops_internals_tsan.h:164: Atomic32 old_value,
nit: parameter indentation? Here and below.

[Alexander Potapenko, 2012-11-13 16:45:00]

https://codereview.chromium.org/10965035/diff/7002/src/atomicops_internals_ts...
File src/atomicops_internals_tsan.h (right):

https://codereview.chromium.org/10965035/diff/7002/src/atomicops_internals_ts...
src/atomicops_internals_tsan.h:164: Atomic32 old_value,
On 2012/11/13 15:43:22, danno wrote:
> nit: parameter indentation? Here and below.

Done.

[danno, 2012-11-13 21:28:58]

lgtm with nit, I'll fix it and land it for you.

http://codereview.chromium.org/10965035/diff/12003/src/atomicops_internals_ts...
File src/atomicops_internals_tsan.h (right):

http://codereview.chromium.org/10965035/diff/12003/src/atomicops_internals_ts...
src/atomicops_internals_tsan.h:247: Atomic64 old_value,
nit: indentation

[Alexander Potapenko, 2012-11-14 10:08:43]

Is it fine to bump the V8 revision at Chrome, or you're only using stable
revisions?

[danno, 2012-11-14 10:11:06]

On 2012/11/14 10:08:43, Alexander Potapenko wrote:
> Is it fine to bump the V8 revision at Chrome, or you're only using stable
> revisions?

No, it's not okay to bump the version. You will have to wait until our next V8
push to trunk to complete (probably today).