[Bug sanitizer/68260] false positive with tsan

[jakub at gcc dot gnu.org]

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

Jakub Jelinek  changed:

   What|Removed |Added

 Status|ASSIGNED|RESOLVED
 Resolution|--- |FIXED

--- Comment #13 from Jakub Jelinek  ---
Fixed.

[Bug sanitizer/68260] false positive with tsan

[jakub at gcc dot gnu.org]

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

--- Comment #12 from Jakub Jelinek  ---
Author: jakub
Date: Tue May 30 07:22:25 2017
New Revision: 248606

URL: https://gcc.gnu.org/viewcvs?rev=248606=gcc=rev
Log:
Backported from mainline
2016-09-14  Jakub Jelinek  

PR sanitizer/68260
* tsan.c: Include target.h.
(enum tsan_atomic_action): Add bool_clear and bool_test_and_set.
(BOOL_CLEAR, BOOL_TEST_AND_SET): Define.
(tsan_atomic_table): Add BUILT_IN_ATOMIC_CLEAR and
BUILT_IN_ATOMIC_TEST_AND_SET entries.
(instrument_builtin_call): Handle bool_clear and bool_test_and_set.

* c-c++-common/tsan/pr68260.c: New test.

Added:
branches/gcc-5-branch/gcc/testsuite/c-c++-common/tsan/pr68260.c
Modified:
branches/gcc-5-branch/gcc/ChangeLog
branches/gcc-5-branch/gcc/testsuite/ChangeLog
branches/gcc-5-branch/gcc/tsan.c

[Bug sanitizer/68260] false positive with tsan

[jakub at gcc dot gnu.org]

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

--- Comment #11 from Jakub Jelinek  ---
Fixed for 6.3+ so far.

[Bug sanitizer/68260] false positive with tsan

[jakub at gcc dot gnu.org]

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

--- Comment #10 from Jakub Jelinek  ---
Author: jakub
Date: Fri Sep 16 09:37:50 2016
New Revision: 240182

URL: https://gcc.gnu.org/viewcvs?rev=240182=gcc=rev
Log:
Backported from mainline
2016-09-14  Jakub Jelinek  

PR sanitizer/68260
* tsan.c: Include target.h.
(enum tsan_atomic_action): Add bool_clear and bool_test_and_set.
(BOOL_CLEAR, BOOL_TEST_AND_SET): Define.
(tsan_atomic_table): Add BUILT_IN_ATOMIC_CLEAR and
BUILT_IN_ATOMIC_TEST_AND_SET entries.
(instrument_builtin_call): Handle bool_clear and bool_test_and_set.

* c-c++-common/tsan/pr68260.c: New test.

Added:
branches/gcc-6-branch/gcc/testsuite/c-c++-common/tsan/pr68260.c
Modified:
branches/gcc-6-branch/gcc/ChangeLog
branches/gcc-6-branch/gcc/testsuite/ChangeLog
branches/gcc-6-branch/gcc/tsan.c

[Bug sanitizer/68260] false positive with tsan

[jakub at gcc dot gnu.org]

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

--- Comment #9 from Jakub Jelinek  ---
Author: jakub
Date: Wed Sep 14 09:01:49 2016
New Revision: 240129

URL: https://gcc.gnu.org/viewcvs?rev=240129=gcc=rev
Log:
PR sanitizer/68260
* tsan.c: Include target.h.
(enum tsan_atomic_action): Add bool_clear and bool_test_and_set.
(BOOL_CLEAR, BOOL_TEST_AND_SET): Define.
(tsan_atomic_table): Add BUILT_IN_ATOMIC_CLEAR and
BUILT_IN_ATOMIC_TEST_AND_SET entries.
(instrument_builtin_call): Handle bool_clear and bool_test_and_set.

* c-c++-common/tsan/pr68260.c: New test.

Added:
trunk/gcc/testsuite/c-c++-common/tsan/pr68260.c
Modified:
trunk/gcc/ChangeLog
trunk/gcc/testsuite/ChangeLog
trunk/gcc/tsan.c

[Bug sanitizer/68260] false positive with tsan

[pinskia at gcc dot gnu.org]

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

Andrew Pinski  changed:

   What|Removed |Added

 CC||rogero at howzatt dot 
demon.co.uk

--- Comment #8 from Andrew Pinski  ---
*** Bug 63627 has been marked as a duplicate of this bug. ***

[Bug sanitizer/68260] false positive with tsan

[dvyukov at google dot com]

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

--- Comment #7 from Dmitry Vyukov  ---
I looked at the patch, but I am unqualified to review it. The test looks good
to me. +Yuri

[Bug sanitizer/68260] false positive with tsan

[jakub at gcc dot gnu.org]

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

--- Comment #6 from Jakub Jelinek  ---
Created attachment 39573
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=39573=edit
gcc7-pr68260.patch

Untested fix.

[Bug sanitizer/68260] false positive with tsan

[jakub at gcc dot gnu.org]

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

Jakub Jelinek  changed:

   What|Removed |Added

 Status|UNCONFIRMED |ASSIGNED
   Last reconfirmed||2016-09-06
   Assignee|unassigned at gcc dot gnu.org  |jakub at gcc dot gnu.org
 Ever confirmed|0   |1

--- Comment #5 from Jakub Jelinek  ---
No, it is a tsan.c bug, which doesn't instrument __atomic_clear and
__atomic_test_and_set builtins (i.e. those that operate solely on bool). 
Trying to implement that now.

[Bug sanitizer/68260] false positive with tsan

[dvyukov at google dot com]

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

--- Comment #4 from Dmitry Vyukov  ---
Good point. I wonder if using -O2 fixes this.
We tend to always use tsan with -O2 for performance reasons. Tsan already
considerably slows down execution, and additional unnecessary memory accesses
and non-inlined trivial function calls (which there can be zillions in "modern
C++") with -O0 slow down execution even more.

[Bug sanitizer/68260] false positive with tsan

[pinskia at gcc dot gnu.org]

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

--- Comment #3 from Andrew Pinski  ---
(In reply to Dmitry Vyukov from comment #1)
> gcc does not instrument atomic operations:
> 
> 004a73d0 <_ZN4spin6unlockEv>:
> 
> void unlock() { flag.clear(std::memory_order_release); }
>   4a73d0:   55  push   %rbp
>   4a73d1:   48 89 e5mov%rsp,%rbp
>   4a73d4:   48 83 ec 10 sub$0x10,%rsp
>   4a73d8:   48 8b 45 08 mov0x8(%rbp),%rax
>   4a73dc:   48 89 7d f0 mov%rdi,-0x10(%rbp)
>   4a73e0:   48 89 c7mov%rax,%rdi
>   4a73e3:   e8 68 4f fd ff  callq  47c350 <__tsan_func_entry>
>   4a73e8:   be 03 00 00 00  mov$0x3,%esi
>   4a73ed:   48 8b 45 f0 mov-0x10(%rbp),%rax
>   4a73f1:   48 89 45 f8 mov%rax,-0x8(%rbp)
>   4a73f5:   48 8b 7d f8 mov-0x8(%rbp),%rdi
>   4a73f9:   e8 62 02 00 00  callq  4a7660
> <_ZNSt11atomic_flag5clearESt12memory_order>
>   4a73fe:   e8 fd 4f fd ff  callq  47c400 <__tsan_func_exit>
>   4a7403:   48 83 c4 10 add$0x10,%rsp
>   4a7407:   5d  pop%rbp
>   4a7408:   c3  retq   
> 
> 
> 
> FWIW this programs works fine with clang.


Actually I wonder if _ZNSt11atomic_flag5clearESt12memory_order is instrumented
but the one being chose by the linker is not.  ODR issues :)

[Bug sanitizer/68260] false positive with tsan

[pinskia at gcc dot gnu.org]

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

--- Comment #2 from Andrew Pinski  ---
#include 

struct spin {
  void lock() {
while (flag.test_and_set(std::memory_order_acquire))
  ;
  }

  void unlock() { flag.clear(std::memory_order_release); }

  std::atomic_flag flag = ATOMIC_FLAG_INIT;
};

int main() {
  auto counter = 0;
  spin lock;

  auto t = std::thread([&]() {
lock.lock();
++counter;
lock.unlock();
  });

  lock.lock();
  ++counter;
  lock.unlock();

  t.join();
  return 0;
}

/*
g++ -std=c++14 -fsanitize=thread -g test.cc -lpthread && ./a.out
==
WARNING: ThreadSanitizer: data race (pid=5795)
  Read of size 4 at 0x7ffe877260dc by thread T1:
#0 main::{lambda()#1}::operator()() const  (a.out+0x004012ff)
#1 _M_invoke<> /usr/include/c++/5/functional:1531 (a.out+0x00402b87)
#2 operator() /usr/include/c++/5/functional:1520 (a.out+0x00402a7c)
#3 _M_run /usr/include/c++/5/thread:115 (a.out+0x0040299a)
#4   (libstdc++.so.6+0x000b902f)
  Previous write of size 4 at 0x7ffe877260dc by main thread:
#0 main /home/kevg/Desktop/test.cc:25 (a.out+0x004013ea)
  Location is stack of main thread.
  Thread T1 (tid=5797, running) created by main thread at:
#0 pthread_create  (libtsan.so.0+0x00027aa7)
#1 std::thread::_M_start_thread(std::shared_ptr,
void (*)())  (libstdc++.so.6+0x000b9172)
#2 main /home/kevg/Desktop/test.cc:22 (a.out+0x004013c0)
SUMMARY: ThreadSanitizer: data race ??:0 main::{lambda()#1}::operator()() const
==
ThreadSanitizer: reported 1 warnings
*/

[Bug sanitizer/68260] false positive with tsan

[dvyukov at google dot com]

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

Dmitry Vyukov  changed:

   What|Removed |Added

 CC||dvyukov at google dot com

--- Comment #1 from Dmitry Vyukov  ---
gcc does not instrument atomic operations:

004a73d0 <_ZN4spin6unlockEv>:

void unlock() { flag.clear(std::memory_order_release); }
  4a73d0:   55  push   %rbp
  4a73d1:   48 89 e5mov%rsp,%rbp
  4a73d4:   48 83 ec 10 sub$0x10,%rsp
  4a73d8:   48 8b 45 08 mov0x8(%rbp),%rax
  4a73dc:   48 89 7d f0 mov%rdi,-0x10(%rbp)
  4a73e0:   48 89 c7mov%rax,%rdi
  4a73e3:   e8 68 4f fd ff  callq  47c350 <__tsan_func_entry>
  4a73e8:   be 03 00 00 00  mov$0x3,%esi
  4a73ed:   48 8b 45 f0 mov-0x10(%rbp),%rax
  4a73f1:   48 89 45 f8 mov%rax,-0x8(%rbp)
  4a73f5:   48 8b 7d f8 mov-0x8(%rbp),%rdi
  4a73f9:   e8 62 02 00 00  callq  4a7660
<_ZNSt11atomic_flag5clearESt12memory_order>
  4a73fe:   e8 fd 4f fd ff  callq  47c400 <__tsan_func_exit>
  4a7403:   48 83 c4 10 add$0x10,%rsp
  4a7407:   5d  pop%rbp
  4a7408:   c3  retq   



FWIW this programs works fine with clang.