Hi Mike,
On Mon, 5 Jan 2015 14:01:42, Mike Stump wrote:
>
> On Jan 5, 2015, at 12:58 PM, Mike Stump <mikest...@comcast.net> wrote:
>> So, to help you out, I tried my hand at wiring up the extra library code:
>
> So, my tsan build finally finished, and I could try this with a real test
> case. I ran it 20 times, and got 11 fails with:
>
> $ i=20; while let i--; do make
> RUNTESTFLAGS=tsan.exp=aligned_vs_unaligned_race.C check-g++ | grep FAIL; done
> FAIL: g++.dg/tsan/aligned_vs_unaligned_race.C -O2 execution test
> FAIL: g++.dg/tsan/aligned_vs_unaligned_race.C -O0 execution test
> FAIL: g++.dg/tsan/aligned_vs_unaligned_race.C -O2 execution test
> FAIL: g++.dg/tsan/aligned_vs_unaligned_race.C -O2 execution test
> FAIL: g++.dg/tsan/aligned_vs_unaligned_race.C -O2 execution test
> FAIL: g++.dg/tsan/aligned_vs_unaligned_race.C -O0 execution test
> FAIL: g++.dg/tsan/aligned_vs_unaligned_race.C -O0 execution test
> FAIL: g++.dg/tsan/aligned_vs_unaligned_race.C -O2 execution test
> FAIL: g++.dg/tsan/aligned_vs_unaligned_race.C -O2 execution test
> FAIL: g++.dg/tsan/aligned_vs_unaligned_race.C -O2 execution test
> FAIL: g++.dg/tsan/aligned_vs_unaligned_race.C -O0 execution test
> $
>
> When I fixed the problem, I ran it 20 times:
>
> $ i=20; while let i--; do make
> RUNTESTFLAGS=tsan.exp=aligned_vs_unaligned_race.C check-g++ | grep FAIL; done
> $
>
> and got 0 failures.
>
> So, it seems to work. I’d like a tsan person to review it and we can go from
> there.
>
> The only thing I would add to it would be a huge comment that explains that
> the tsan run time isn’t thread safe and they should use a lock free update to
> the shadow bits, but, they don’t. We introduce the step primitive to work
> around that bug. Ideally, the the problem should be filed into a bug database
> for the tsan code gen and when closed as not to be fixed, we can then change
> the word bug to design, but leave the bug reference so that others that want
> to completely understand the issue can go read up on it. If they actually fix
> the codegen to be thread safe, then we can simply remove all the step code.
>
> To make this clang friendly, if one turns off inlining and obscures the
> semantics with weak from the optimizer and puts it into a header files and
> then #includes that header files, I think it would work. I’ll leave this to
> someone that might want to do that. If not, I’m fine with #ifdef clang/gcc
> and have the gcc test cases use step and the clang test cases, well, be
> unreliable.
>
> $ svn diff
> Index: g++.dg/tsan/aligned_vs_unaligned_race.C
> ===================================================================
> --- g++.dg/tsan/aligned_vs_unaligned_race.C (revision 219198)
> +++ g++.dg/tsan/aligned_vs_unaligned_race.C (working copy)
> @@ -1,11 +1,17 @@
> +/* { dg-additional-sources "lib.c" } */
> /* { dg-shouldfail "tsan" } */
> +
> #include <pthread.h>
> #include <stdio.h>
> #include <stdint.h>
> +#include <unistd.h>
> +
> +void step(int);
>
> uint64_t Global[2];
>
> void *Thread1(void *x) {
> + step (2);
> Global[1]++;
> return NULL;
> }
> @@ -15,6 +21,7 @@ void *Thread2(void *x) {
> struct __attribute__((packed, aligned(1))) u_uint64_t { uint64_t val; };
> u_uint64_t *p4 = reinterpret_cast<u_uint64_t *>(p1 + 1);
> (*p4).val++;
> + step (1);
> return NULL;
> }
>
> $ cat g++.dg/tsan/lib.c
> /* { dg-do compile } */
> #include <unistd.h>
>
> volatile int serial;
>
> __attribute__((no_sanitize_address))
> void step(int i) {
> while (serial != i-1) ;
> while (1) {
> if (++serial == i)
> return;
> --serial;
> sleep (1);
> }
> }
I tried to elaborate your idea a bit, and used proper atomics.
It is necessary that the logic of the step function is completely invisible to
tsan.
Therefore it should not call sleep, because that can be intercepted, even
if the code is not instrumented. And if this is the right solution for
aligned_vs_unaligned.C it should be the used in all other tests as well.
Unfortunately there is no __attribute__((no_sanitize_thread)) just
no_sanitize_address, and no_sanitize_undefined. So I need to call
the gcc compiler driver a second time with different options to compile
lib.c and link that together with the test case. If done by hand, the tests
work very reliable, even without any sleep.
I did not know about the dg-additional-sources, that was new to me.
But it does not quite do what I need. Unfortunately it just adds lib.c
to the gcc command line, and both sources use the same options.
But that instruments the atomics in lib.c and thus tsan figures out,
that there is really no race condition at all.
So some tests start to fail.
make check-gcc-c++ RUNTESTFLAGS="tsan.exp=*"
...
FAIL: g++.dg/tsan/aligned_vs_unaligned_race.C -O0 execution test
FAIL: g++.dg/tsan/aligned_vs_unaligned_race.C -O2 execution test
FAIL: g++.dg/tsan/atomic_free.C -O0 execution test
FAIL: g++.dg/tsan/atomic_free.C -O2 execution test
That is of course only a technical problem, but one that is beyond my scope.
And OTOH I doubt that these race-free tests look very realistic.
Bernd.
tsan-racefree.diff
Description: Binary data
