[Bug sanitizer/55561] TSAN: Fortran/OMP yields false positives

[dvyukov at google dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55561

--- Comment #31 from Dmitry Vyukov <dvyukov at google dot com> 2013-01-02 10:28:00 UTC ---
(In reply to comment #30)
> The formatting in the patch is wrong (multiple issues).
> 
> I don't see a point in the __atomic_load_n (addr, MEMMODEL_RELAXED), for
> aligned ints or pointers the loads are atomic on all architectures libgomp is
> supported on, after all kernel is also using just a normal load in the futex
> syscall, not __atomic_load_n (which expands to the normal load only anyway).

Do you agree about MEMMODEL_ACQUIRE?

Regarding MEMMODEL_RELAXED, there are 2 reasons to use it correctness aside.
First, it greatly contributes to code readability and self-documentation, and
allows readers to easily distinguish between plain loads and inter-thread
synchronization shared loads which are hideously different things. Seconds, it
allows tools like tsan to work properly on such code and point to more serious
issues (like that MEMMODEL_ACQUIRE above).

As for correctness, below is an example that I usually provide (and there is
also "How to miscompile programs with benign data races" paper by Boehm with
other good examples):

-----

Consider that you have an "atomic" (which is not actually marked as atomic for
compiler) store and some code w/o sync operations around it (potentially from
inlined functions):

...
*p = x;
...

C++ compiler assumes absence of data races. So if it sees a store to p, then it
is allowed to use it as a scratch storage for any garbage in the same
synchronization region. I.e. it can do:

...
*p = foo;
...
*p = x;
...

Note that it can't affect any correct race-free program.
This way, other threads will read random garbage from p.

Now imagine that foo is a function pointer:

...
*p = foo; // spill from register
...
foo = *p; // restore to register
if (bar) foo(); // and execute
...
*p = x;
...

Now imagine that this thread spills &ReadFile to p, and another thread spills
&LaunchNuclearMissle to p in between (but was not intended to execute it due to
bar==0).
Ooops, this "benign" race just caused accidental launch of nuclear missiles.