[Bug testsuite/62060] New: g++.dg/tsan/cond_race.C triggers heap-use-after-free

[Bug testsuite/62060] New: g++.dg/tsan/cond_race.C triggers heap-use-after-free

[vries at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62060

            Bug ID: 62060
           Summary: g++.dg/tsan/cond_race.C triggers heap-use-after-free
           Product: gcc
           Version: 4.10.0
            Status: UNCONFIRMED
          Severity: minor
          Priority: P3
         Component: testsuite
          Assignee: unassigned at gcc dot gnu.org
          Reporter: vries at gcc dot gnu.org

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.

[Bug testsuite/62060] g++.dg/tsan/cond_race.C triggers heap-use-after-free

[vries at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62060

vries at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |chefmax at gcc dot gnu.org

--- Comment #1 from vries at gcc dot gnu.org ---
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>
...

[Bug testsuite/62060] g++.dg/tsan/cond_race.C triggers heap-use-after-free

[chefmax at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62060

--- Comment #2 from Maxim Ostapenko <chefmax at gcc dot gnu.org> ---
(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);
 }

[Bug testsuite/62060] g++.dg/tsan/cond_race.C triggers heap-use-after-free

[vries at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62060

--- Comment #3 from vries at gcc dot gnu.org ---
Created attachment 33275
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=33275&action=edit
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;
 }

[Bug testsuite/62060] g++.dg/tsan/cond_race.C triggers heap-use-after-free

[tetra2005 at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62060

Yuri Gribov <tetra2005 at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |kcc at gcc dot gnu.org

--- Comment #4 from Yuri Gribov <tetra2005 at gmail dot com> ---
Adding kcc. It may make sense to detect testsuite changes during libsanitizer
merges.

[Bug testsuite/62060] g++.dg/tsan/cond_race.C triggers heap-use-after-free

[tetra2005 at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62060

Yuri Gribov <tetra2005 at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #5 from Yuri Gribov <tetra2005 at gmail dot com> ---
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).

[Bug testsuite/62060] g++.dg/tsan/cond_race.C triggers heap-use-after-free

[bernd.edlinger at hotmail dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62060

Bernd Edlinger <bernd.edlinger at hotmail dot de> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bernd.edlinger at hotmail dot de

--- Comment #6 from Bernd Edlinger <bernd.edlinger at hotmail dot de> ---
should be fixed by r219367

[Bug testsuite/62060] g++.dg/tsan/cond_race.C triggers heap-use-after-free

[vries at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62060

vries at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |FIXED

--- Comment #7 from vries at gcc dot gnu.org ---
Marking resolved, fixed