Bug 327881 - False Positive Warning on std::atomic_bool
Summary: False Positive Warning on std::atomic_bool
Status: CONFIRMED
Alias: None
Product: valgrind
Classification: Developer tools
Component: helgrind (show other bugs)
Version: 3.9.0
Platform: Compiled Sources Linux
: NOR normal
Target Milestone: ---
Assignee: Julian Seward
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-11-20 21:04 UTC by Дилян Палаузов
Modified: 2023-03-06 12:56 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Дилян Палаузов 2013-11-20 21:04:46 UTC
I have valgrind svn revision 13715 and gcc 4.7.4 20131116 (prerelease) on x86_64 and ld --version 2.23.2 .

When I compile this programm:

#include <atomic>
#include <cstdio>
#include <thread>
int main() {
  std::atomic_bool a ( false );
  std::thread t {[&a]() {
      a = true;
      std::printf("%i\n", a.load());
    }};
  a = false;
  std::printf("%i\n", a.load());
  t.join();
}

with "gcc -g -std=c++11 test.cpp -o test -O0 -lstdc++ -lpthread" and run helgrind on it, as you can see from the report below, a = true and a = false are considered not-synchronized accesses on the same variable from different threads.

However, as the a-variable is std::atomic_bool , it cannot be "partially" assigned, so the reported data race is false positive.

Reproducible: Always

Steps to Reproduce:
see above
Actual Results:  
---Thread-Announcement------------------------------------------
Thread #1 is the program's root thread

---Thread-Announcement------------------------------------------
Thread #2 was created
   at 0x56545DE: clone (in /lib64/libc-2.17.so)
   by 0x513FFC4: do_clone.constprop.4 (in /lib64/libpthread-2.17.so)
   by 0x5141438: pthread_create@@GLIBC_2.2.5 (in /lib64/libpthread-2.17.so)
   by 0x4C2F588: pthread_create_WRK (in /usr/lib64/valgrind/vgpreload_helgrind-amd64-linux.so)
   by 0x4EE676E: std::thread::_M_start_thread(std::shared_ptr<std::thread::_Impl_base>) (gthr-default.h:662)
   by 0x400F43: std::thread::thread<main::{lambda()#1}>(main::{lambda()#1}&&) (thread:133)
   by 0x400E0D: main (test.cpp:10)

----------------------------------------------------------------
Possible data race during write of size 1 at 0xFFF00051F by thread #1
Locks held: none
   at 0x401F88: std::__atomic_base<bool>::store(bool, std::memory_order) (atomic_base.h:444)
   by 0x401F4F: std::__atomic_base<bool>::operator=(bool) (atomic_base.h:346)
   by 0x401D4C: std::atomic_bool::operator=(bool) (atomic:70)
   by 0x400E1E: main (test.cpp:11)

This conflicts with a previous write of size 1 by thread #2
Locks held: none
   at 0x401F88: std::__atomic_base<bool>::store(bool, std::memory_order) (atomic_base.h:444)
   by 0x401F4F: std::__atomic_base<bool>::operator=(bool) (atomic_base.h:346)
   by 0x401D4C: std::atomic_bool::operator=(bool) (atomic:70)
   by 0x400DBB: main::{lambda()#1}::operator()() const (test.cpp:8)
   by 0x401CE3: void std::_Bind_simple<main::{lambda()#1} ()>::_M_invoke<>(std::_Index_tuple<>) (functional:1598)
   by 0x401C30: std::_Bind_simple<main::{lambda()#1} ()>::operator()() (functional:1586)
   by 0x401BC9: std::thread::_Impl<std::_Bind_simple<main::{lambda()#1} ()> >::_M_run() (thread:115)
   by 0x4EE651F: execute_native_thread_routine (thread.cc:84)

Expected Results:  
No data races on std::atomic_bool upon operator=  / ::load ().
Comment 1 Дилян Палаузов 2013-11-23 00:22:31 UTC
Another example in pure C.  According the documentation of libc,, the type sig_atomic_t is defined as "Reading and writing this data type is guaranteed to happen in a single instruction, so there's no way for a handler to run “in the middle” of an access. " (http://www.gnu.org/software/libc/manual/html_node/Atomic-Types.html)

I understand this in a way, that reading and writing on sig_atomic_t is guaranteed to be atomic and causes no data races..

Running this program

#include <pthread.h>
#include <signal.h>

sig_atomic_t a = 3;

void* run() {
  a = 4;
  return NULL;
}

int main() {
  pthread_t p;
  pthread_create (&p, NULL, run, NULL);
  a = 5;
  pthread_join (p, NULL);
  return 0;
}

with the mentioned compilers (gcc -O0 -g -o test test.c -lpthread) , produces the warning
---Thread-Announcement------------------------------------------
Thread #1 is the program's root thread

---Thread-Announcement------------------------------------------
Thread #2 was created
   at 0x513C5DE: clone (in /lib64/libc-2.17.so)
   by 0x4E3CFC4: do_clone.constprop.4 (in /lib64/libpthread-2.17.so)
   by 0x4E3E438: pthread_create@@GLIBC_2.2.5 (in /lib64/libpthread-2.17.so)
   by 0x4C2F588: pthread_create_WRK (in /usr/lib64/valgrind/vgpreload_helgrind-amd64-linux.so)
   by 0x4006A3: main (test.c:13)

----------------------------------------------------------------
Possible data race during write of size 4 at 0x600B08 by thread #1
Locks held: none
   at 0x4006A4: main (test.c:14)

This conflicts with a previous write of size 4 by thread #2
Locks held: none
   at 0x400670: run (test.c:7)
   by 0x4C2F72D: mythread_wrapper (in /usr/lib64/valgrind/vgpreload_helgrind-amd64-linux.so)
   by 0x4E3DDDE: start_thread (in /lib64/libpthread-2.17.so)

However there is no data race at all, as reading or writing to sig_action_t variables is atomic.  Thus the expected output of helgrind is no data races et al.
Comment 2 Milian Wolff 2016-09-22 10:58:08 UTC
I can confirm this behavior, seems like the suppression files must be extended to ignore anything happening within std::atomic* ?
Comment 3 David Faure 2018-04-22 21:28:22 UTC
At least on x86, yes, there suppression files are the only solution since the CPU takes care of cache synchronization so non-atomic read/writes lead to the same assembly as atomic read/writes.

On other architectures like ARM we probably don't want such suppressions to be enabled.