[PATCH] tsan: do not instrument not captured values

[Chandler Carruth]

On Tue, Jan 20, 2015 at 8:54 AM, Dmitry Vyukov <dvyukov at google.com> wrote:

> Hi kcc, samsonov, eugenis,
>
> Consider the following code:
>
> P p;
> p.x = 1;
> p.y = 2;
> foo(&p);
>
> P escapes and so tsan instruments accesses to it. However, before address
> of the variable is actually taken, it is not shared and so cannot
> participate in data races. So there is no point in instrumenting these
> initial memory accesses.
>

There can be races here.

Imagine:

foo(P *p) {
  racy_queue.relaxed_atomic_enqueue(p);
}

and then thread B:

bar() {
  P *p = racy_queue.relaxed_atomic_dequeue();
  read(p.x);
  ...
}

Here we don't have a race in my racy_queue because internally it uses
relaxed atomics. However it never establishes a happens-before edge, and so
the code in bar that reads this should be flagged as a race by TSan.

This isn't just a hypothetical, I have seen TSan catch this in more than
one project that had a bad lock-free queue implementation. This is
especially dangerous on x86 because the machine instructions used for
relaxed store and load are actually enough to establish the necessary
memory ordering guarantee.

While we should definitely coalesce the TSan instrumentation to be a single
instrumented write preceding the capture, I don't think we should remove
this entirely.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150121/d9d288e9/attachment.html>