When testing a gcc patch, I ran into this failure ... -./gcc/testsuite/g++/g++.sum:PASS: g++.dg/tsan/cond_race.C -O2 output pattern test, ThreadSanitizer: data race.*pthread_cond_signal.* +./gcc/testsuite/g++/g++.sum:FAIL: g++.dg/tsan/cond_race.C -O2 output pattern test, is ================== ... I've run into the same failure before, here: https://gcc.gnu.org/ml/gcc-patches/2014-04/msg01758.html . Also, I've noticed it here: https://gcc.gnu.org/ml/gcc-testresults/2014-01/msg00127.html . The complete failure from the log is: ... FAIL: g++.dg/tsan/cond_race.C -O2 output pattern test, is ================== WARNING: ThreadSanitizer: heap-use-after-free (pid=5192) Read of size 8 at 0x7d180000efc8 by thread T1: #0 pthread_cond_signal src/libsanitizer/tsan/tsan_interceptors.cc:1011 (libtsan.so.0+0x000000027794) #1 thr(void*) src/gcc/testsuite/g++.dg/tsan/cond_race.C:20 (cond_race.exe+0x000000001033) Previous write of size 8 at 0x7d180000efc8 by main thread: #0 operator delete(void*) src/libsanitizer/tsan/tsan_interceptors.cc:583 (libtsan.so.0+0x000000025ab9) #1 main src/gcc/testsuite/g++.dg/tsan/cond_race.C:34 (cond_race.exe+0x000000000ea0) Location is heap block of size 96 at 0x7d180000efa0 allocated by main thread: #0 operator new(unsigned long) src/libsanitizer/tsan/tsan_interceptors.cc:551 (libtsan.so.0+0x000000025863) #1 main src/gcc/testsuite/g++.dg/tsan/cond_race.C:25 (cond_race.exe+0x000000000e12) Thread T1 (tid=5200, running) created by main thread at: #0 pthread_create src/libsanitizer/tsan/tsan_interceptors.cc:853 (libtsan.so.0+0x000000026f54) #1 main src/gcc/testsuite/g++.dg/tsan/cond_race.C:29 (cond_race.exe+0x000000000e5a) SUMMARY: ThreadSanitizer: heap-use-after-free src/gcc/testsuite/g++.dg/tsan/cond_race.C:20 thr(void*) ================== ThreadSanitizer: reported 1 warnings , should match ThreadSanitizer: data race.*pthread_cond_signal.* ... When compiling and running from the command line, the expected output is produced: ... WARNING: ThreadSanitizer: data race (pid=6294) Write of size 8 at 0x7d180000efc8 by main thread: #0 operator delete(void*) src/libsanitizer/tsan/tsan_interceptors.cc:583 (libtsan.so.0+0x000000025ab9) #1 main src/gcc/testsuite/g++.dg/tsan/cond_race.C:34 (cond_race.exe+0x000000000d00) Previous read of size 8 at 0x7d180000efc8 by thread T1: #0 pthread_cond_signal src/libsanitizer/tsan/tsan_interceptors.cc:1011 (libtsan.so.0+0x000000027794) #1 thr(void*) src/gcc/testsuite/g++.dg/tsan/cond_race.C:20 (cond_race.exe+0x000000000e93) Location is heap block of size 96 at 0x7d180000efa0 allocated by main thread: #0 operator new(unsigned long) src/libsanitizer/tsan/tsan_interceptors.cc:551 (libtsan.so.0+0x000000025863) #1 main src/gcc/testsuite/g++.dg/tsan/cond_race.C:25 (cond_race.exe+0x000000000c72) Thread T1 (tid=6296, running) created by main thread at: #0 pthread_create src/libsanitizer/tsan/tsan_interceptors.cc:853 (libtsan.so.0+0x000000026f54) #1 main src/gcc/testsuite/g++.dg/tsan/cond_race.C:29 (cond_race.exe+0x000000000cba) SUMMARY: ThreadSanitizer: data race src/gcc/testsuite/g++.dg/tsan/cond_race.C:34 main ... So, it seems there is a data race between: - the write from the delete at line 34, and - the read from the pthread_cond_signal at line 20. If the write comes first, we get the heap-use-after-free message. If the read comes first, we get the data race message. Tentatively setting component to testsuite.
Tentative patch: ... diff --git a/gcc/testsuite/g++.dg/tsan/cond_race.C b/gcc/testsuite/g++.dg/tsan/cond_race.C index a937614..90dfb19 100644 --- a/gcc/testsuite/g++.dg/tsan/cond_race.C +++ b/gcc/testsuite/g++.dg/tsan/cond_race.C @@ -1,5 +1,5 @@ /* { dg-shouldfail "tsan" } */ -/* { dg-output "ThreadSanitizer: data race.*" } */ +/* { dg-output "ThreadSanitizer: (data race|heap-use-after-free).*" } */ /* { dg-output "pthread_cond_signal.*" } */ #include <stdio.h> ...
(In reply to vries from comment #1) > Tentative patch: > ... > diff --git a/gcc/testsuite/g++.dg/tsan/cond_race.C > b/gcc/testsuite/g++.dg/tsan/cond_race.C > index a937614..90dfb19 100644 > --- a/gcc/testsuite/g++.dg/tsan/cond_race.C > +++ b/gcc/testsuite/g++.dg/tsan/cond_race.C > @@ -1,5 +1,5 @@ > /* { dg-shouldfail "tsan" } */ > -/* { dg-output "ThreadSanitizer: data race.*" } */ > +/* { dg-output "ThreadSanitizer: (data race|heap-use-after-free).*" } */ > /* { dg-output "pthread_cond_signal.*" } */ > > #include <stdio.h> > ... This test was copied from LLVM compiler-rt testsuite. I see that compiler-rt developers added sleep (1) right after pthread_mutex_unlock to avoid this problem. Perhaps we should do the same? diff --git a/gcc/testsuite/g++.dg/tsan/cond_race.C b/gcc/testsuite/g++.dg/tsan/cond_race.C index a937614..805465d 100644 --- a/gcc/testsuite/g++.dg/tsan/cond_race.C +++ b/gcc/testsuite/g++.dg/tsan/cond_race.C @@ -5,6 +5,7 @@ #include <stdio.h> #include <stdlib.h> #include <pthread.h> +#include <unistd.h> struct Ctx { pthread_mutex_t m; @@ -31,6 +32,8 @@ int main() { while (!c->done) pthread_cond_wait(&c->c, &c->m); pthread_mutex_unlock(&c->m); + // w/o this sleep, it can be reported as use-after-free + sleep(1); delete c; pthread_join(th, 0); }
Created attachment 33275 [details] script to find differences It seems we're missing more changes: ... $ ./find.sh --- compiler-rt/test/tsan/cond_race.cc 2014-08-08 14:23:28.907916207 +0200 +++ devel/src/gcc/testsuite/g++.dg/tsan/cond_race.C 2014-07-28 09:45:52.277091371 +0200 @@ -6,7 +5,6 @@ #include <stdio.h> #include <stdlib.h> #include <pthread.h> -#include <unistd.h> struct Ctx { pthread_mutex_t m; @@ -33,8 +31,6 @@ while (!c->done) pthread_cond_wait(&c->c, &c->m); pthread_mutex_unlock(&c->m); - // w/o this sleep, it can be reported as use-after-free - sleep(1); delete c; pthread_join(th, 0); } --- compiler-rt/test/tsan/thread_leak2.c 2014-08-08 14:23:28.927916207 +0200 +++ devel/src/gcc/testsuite/c-c++-common/tsan/thread_leak2.c 2014-07-28 09:45:52.185091367 +0200 @@ -1,16 +1,19 @@ + #include <pthread.h> -#include <stdio.h> +#include <unistd.h> void *Thread(void *x) { return 0; } int main() { + int i; + for (i = 0; i < 5; i++) { pthread_t t; pthread_create(&t, 0, Thread, 0); - pthread_detach(t); - printf("PASS\n"); + } + sleep(1); return 0; } --- compiler-rt/test/tsan/mutexset1.cc 2014-08-08 14:23:28.919916207 +0200 +++ devel/src/gcc/testsuite/c-c++-common/tsan/mutexset1.c 2014-07-28 16:48:55.546509594 +0200 @@ -16,7 +17,8 @@ void *Thread2(void *x) { Global--; - return NULL; + } int main() { @@ -34,4 +29,12 @@ pthread_join(t[0], NULL); pthread_join(t[1], NULL); pthread_mutex_destroy(&mtx); + return 0; } + --- compiler-rt/test/tsan/tls_race.cc 2014-08-08 14:23:28.907916207 +0200 +++ devel/src/gcc/testsuite/c-c++-common/tsan/tls_race.c 2014-07-28 09:45:52.185091367 +0200 @@ -1,10 +1,9 @@ + #include <pthread.h> #include <stddef.h> -#include <unistd.h> void *Thread(void *a) { - sleep(1); *(int*)a = 43; return 0; } --- compiler-rt/test/tsan/atomic_stack.cc 2014-08-08 14:23:28.995916208 +0200 +++ devel/src/gcc/testsuite/c-c++-common/tsan/atomic_stack.c 2014-07-28 09:45:52.185091367 +0200 @@ -21,9 +22,10 @@ pthread_create(&t[1], NULL, Thread2, NULL); pthread_join(t[0], NULL); pthread_join(t[1], NULL); + return 0; }
Adding kcc. It may make sense to detect testsuite changes during libsanitizer merges.
Also added Jakub. Looks like LLVM and GCC use orthogonal approaches to deflake TSan tests: GCC does this in source code (https://groups.google.com/forum/#!topic/thread-sanitizer/KIok3F_b1oI) whereas LLVM chose to go with retrying testrunner (http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20140526/218813.html).
should be fixed by r219367
Marking resolved, fixed