More changes for TSan v2 support:

More changes for TSan v2 support:
 -- add the ignores file
 -- set the appropriate definitions for compiling dynamic annotations
 -- add suppressions for existing data races

BUG=23244, 62694, 102327, 120808, 158718
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=165330

[Alexander Potapenko, 2012-10-23 12:50:22]

PTAL

[Nico, 2012-10-23 19:48:44]

What's the transition plan for the blacklist file? Will we switch from v1 to v2
in one step, or will we have two blacklists for a while, which manually need to
be kept in sync?

[Alexander Potapenko, 2012-10-24 10:18:09]

TSan v2 is quite different from v1, and the differences include different
suppression format (single line instead of a stack trace) and ignores format (v2
is better at source-level ignores, while v1 supports recursive ignoring of
functions).
Because v2 and v1 are going to co-exist for quite a long time (v2 is only
available on Linux x64 ATM), we'll need to manually support both formats.
Hopefully supporting v2 will require less effort, because it has better support
for atomics and will require less reports to be suppressed.

[Nico, 2012-10-24 18:04:09]

On 2012/10/24 10:18:09, Alexander Potapenko wrote:
> TSan v2 is quite different from v1, and the differences include different
> suppression format (single line instead of a stack trace) and ignores format
(v2
> is better at source-level ignores, while v1 supports recursive ignoring of
> functions).
> Because v2 and v1 are going to co-exist for quite a long time (v2 is only
> available on Linux x64 ATM), we'll need to manually support both formats.
> Hopefully supporting v2 will require less effort, because it has better
support
> for atomics and will require less reports to be suppressed.

Is it possible to automatically translate from v1 to v2 suppressions with some
script?

I'd really like to prevent having to maintain two mostly identical suppression
lists -- it's annoying for memcheck and heapcheck, and it'll be annoying here.
Maybe we should wait rolling out tsan v2 until it's in a state where all v1 bots
can be switched over?

[kcc1, 2012-10-25 05:23:47]

On Wed, Oct 24, 2012 at 10:04 PM, <thakis@chromium.org> wrote:

> On 2012/10/24 10:18:09, Alexander Potapenko wrote:
>
>> TSan v2 is quite different from v1, and the differences include different
>> suppression format (single line instead of a stack trace) and ignores
>> format
>>
> (v2
>
>> is better at source-level ignores, while v1 supports recursive ignoring of
>> functions).
>> Because v2 and v1 are going to co-exist for quite a long time (v2 is only
>> available on Linux x64 ATM), we'll need to manually support both formats.
>> Hopefully supporting v2 will require less effort, because it has better
>>
> support
>
>> for atomics and will require less reports to be suppressed.
>>
>
> Is it possible to automatically translate from v1 to v2 suppressions with
> some
> script?
>

It might be possible, but I'd prefer not to mix these things for a few
reasons.
1. tsan v2 natively support atomics and thus needs less suppressions
2. tsan v2 doesn't instrument libc and thus needs less ignores
3. (I guess) some of the tsan v1 suppressions are outdated anyway.
4. tsan v2 is only on linux and the suppression/ignore file should be
minimal (if it's not, Chrome code is dead broken)



>
> I'd really like to prevent having to maintain two mostly identical
> suppression
> lists -- it's annoying for memcheck and heapcheck, and it'll be annoying
> here.
> Maybe we should wait rolling out tsan v2 until it's in a state where all
> v1 bots
> can be switched over?
>

It'll make us wait for like 2 years, since tsan v2 is not going to appear
on Windows-32 any time soon (or ever)
(Windows-64 is possible after clang support is ready, but we are not using
Windows-64 yet).
Meanwhile, v2 provides 5x-10x better speed on Linux.

--kcc


>
>
https://codereview.chromium.**org/11227052/<https://codereview.chromium.org/1...
>

[Alexander Potapenko, 2012-10-25 16:24:06]

On 2012/10/24 18:04:09, Nico wrote:
> On 2012/10/24 10:18:09, Alexander Potapenko wrote:
> > TSan v2 is quite different from v1, and the differences include different
> > suppression format (single line instead of a stack trace) and ignores format
> (v2
> > is better at source-level ignores, while v1 supports recursive ignoring of
> > functions).
> > Because v2 and v1 are going to co-exist for quite a long time (v2 is only
> > available on Linux x64 ATM), we'll need to manually support both formats.
> > Hopefully supporting v2 will require less effort, because it has better
> support
> > for atomics and will require less reports to be suppressed.
> 
> Is it possible to automatically translate from v1 to v2 suppressions with some
> script?

This is quite challenging.
The current v1 suppressions contain many uninteresting frames, so picking either
the top or the bottom frame from the suppression stack may just mask other races
which aren't related.
I guess that before launching v2 we can try to somehow force the devs to fix the
existing races, because the suppressions shouldn't persist for years, this is
just a measure of urgent reaction to the error.
But anyway at the moment the suppressions are created by just copy-pasting the
racy stack, so we can't guess which frame to take.

> I'd really like to prevent having to maintain two mostly identical suppression
> lists -- it's annoying for memcheck and heapcheck, and it'll be annoying here.
> Maybe we should wait rolling out tsan v2 until it's in a state where all v1
bots
> can be switched over?

As you may guess, keeping a local diff required for the roll to happen slows the
development down.
First, we need to keep it up-to-date, while build/common.gypi is heavily
modified.
Second, the developers will have difficulties using the tool if it doesn't work
off the ToT.
Third, this approach won't let us to set up a FYI bot, which is a must before we
can switch to v2 (to some extent, because v2 doesn't work on Windows and Mac
(although v1 is also broken on Mac right now))

[Alexander Potapenko, 2012-10-29 15:40:18]

Ping.
Should we weigh in more arguments?

[Alexander Potapenko, 2012-10-29 15:49:43]

I've also decided to add my current diff for suppressions.txt here, because it's
somewhat related.

[Nico, 2012-10-29 23:51:38]

> It might be possible, but I'd prefer not to mix these things for a few
> reasons.
> 1. tsan v2 natively support atomics and thus needs less suppressions
> 2. tsan v2 doesn't instrument libc and thus needs less ignores
> 3. (I guess) some of the tsan v1 suppressions are outdated anyway.
> 4. tsan v2 is only on linux and the suppression/ignore file should be
> minimal (if it's not, Chrome code is dead broken)

Looking through tools/valgrind/tsan/suppressions.txt, many of them are for
benign races in chromium and 3rd-party code we build. We'd need those too.

> It'll make us wait for like 2 years, since tsan v2 is not going to appear
> on Windows-32 any time soon (or ever)
> (Windows-64 is possible after clang support is ready, but we are not using
> Windows-64 yet).
> Meanwhile, v2 provides 5x-10x better speed on Linux.

Is that fast enough to ship a canary with it?

If not,  the current bots are slow, but not unbearably slow:
http://build.chromium.org/p/chromium.memory.fyi/stats (ctrl-f 'tsan')

The last broken build (still red atm) had a single CL in the regression range:
http://build.chromium.org/p/chromium.memory.fyi/builders/Linux%20Tests%20%28t...




I'm fine with this going in for fyi bots. For real use though, I think I
personally prefer somewhat slower cycle times over redundant suppression files.
It's possible I'm the exception here, so you might want to poll the current
memory sheriff rotation for their opinion.

[kcc1, 2012-10-30 05:16:25]

On Tue, Oct 30, 2012 at 3:51 AM, <thakis@chromium.org> wrote:

> It might be possible, but I'd prefer not to mix these things for a few
>> reasons.
>> 1. tsan v2 natively support atomics and thus needs less suppressions
>> 2. tsan v2 doesn't instrument libc and thus needs less ignores
>> 3. (I guess) some of the tsan v1 suppressions are outdated anyway.
>> 4. tsan v2 is only on linux and the suppression/ignore file should be
>> minimal (if it's not, Chrome code is dead broken)
>>
>
> Looking through tools/valgrind/tsan/**suppressions.txt, many of them are
> for
> benign races in chromium and 3rd-party code we build. We'd need those too.
>
>
>  It'll make us wait for like 2 years, since tsan v2 is not going to appear
>> on Windows-32 any time soon (or ever)
>> (Windows-64 is possible after clang support is ready, but we are not using
>> Windows-64 yet).
>> Meanwhile, v2 provides 5x-10x better speed on Linux.
>>
>
> Is that fast enough to ship a canary with it?
>

Probably not.
With some more tircks (e.g. static sampling) -- yes.



>
> If not,  the current bots are slow, but not unbearably slow:
>
http://build.chromium.org/p/**chromium.memory.fyi/stats<http://build.chromium...
'tsan')
>
> The last broken build (still red atm) had a single CL in the regression
> range:
> http://build.chromium.org/p/**chromium.memory.fyi/builders/**
>
Linux%20Tests%20%28tsan%29%**281%29/builds/1812<http://build.chromium.org/p/chromium.memory.fyi/builders/Linux%20Tests%20%28tsan%29%281%29/builds/1812>



The bots are not the only way of using tsan.
We plan to be using tsan2 on ClusterFuzz for fuzzing, where tsan1 is
unapplicable due to its speed.

--kcc


>
> I'm fine with this going in for fyi bots. For real use though, I think I
> personally prefer somewhat slower cycle times over redundant suppression
> files.
> It's possible I'm the exception here, so you might want to poll the current
> memory sheriff rotation for their opinion.
>
>
https://codereview.chromium.**org/11227052/<https://codereview.chromium.org/1...
>

[Alexander Potapenko, 2012-10-30 07:49:38]

+Abhishek FHI

[Alexander Potapenko, 2012-10-30 07:50:10]

+Abhishek FHI

[Alexander Potapenko, 2012-10-31 17:21:51]

> 
> Looking through tools/valgrind/tsan/suppressions.txt, many of them are for
> benign races in chromium and 3rd-party code we build. We'd need those too.
Those occur in the libraries that we do not instrument yet, thus there'll be no
reports now.

> Is that fast enough to ship a canary with it?
> 
> If not,  the current bots are slow, but not unbearably slow:
> http://build.chromium.org/p/chromium.memory.fyi/stats (ctrl-f 'tsan')
> 
As Kostya said, we hope to have TSan running on ClusterFuzz.
> 
> I'm fine with this going in for fyi bots. For real use though, I think I
> personally prefer somewhat slower cycle times over redundant suppression
files.

Let us move forward with making this a FYI bot, so that we'll be able to clean
up the code and remove more suppressions.

[Nico, 2012-10-31 19:19:38]

lgtm for fyi bots

https://codereview.chromium.org/11227052/diff/34001/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/11227052/diff/34001/build/common.gypi#newcode255
build/common.gypi:255: 'tsan_blacklist%':
'<(PRODUCT_DIR)/../../tools/valgrind/tsan_v2/ignores.txt',
(It's a bit confusing that "tsan" means "clang's tsan" here, but clang's tsan is
called tsan_v2 in the other folder. You should probably s/tsan/tsan_v2/ in this
file, but it's not necessary in this patch.)

[Alexander Potapenko, 2012-11-01 08:41:03]

https://codereview.chromium.org/11227052/diff/34001/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/11227052/diff/34001/build/common.gypi#newcode255
build/common.gypi:255: 'tsan_blacklist%':
'<(PRODUCT_DIR)/../../tools/valgrind/tsan_v2/ignores.txt',
On the other hand, this is the first version from clang's point of view. But
you're right, I need to choose a better name.