Created attachment 147408 [details] semtest.c that shows the potential false positive SUMMARY Helgrind 3.18.1 appears to flag what I believe to be a false positive when a stack-allocated semaphore is signaled to and the stack space on which it is allocated is then reused. STEPS TO REPRODUCE 1. Compile the attached program with gcc -g -pthread semtest.c -o semtest (gcc 9.4.0 on Ubuntu 20.04 with GNU libc 2.2.5 I believe) 2. Run it with valgrind --tool=helgrind ./semtest OBSERVED RESULT ==2965601== Helgrind, a thread error detector ==2965601== Copyright (C) 2007-2017, and GNU GPL'd, by OpenWorks LLP et al. ==2965601== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright info ==2965601== Command: ./semtest ==2965601== ==2965601== ---Thread-Announcement------------------------------------------ ==2965601== ==2965601== Thread #1 is the program's root thread ==2965601== ==2965601== ---Thread-Announcement------------------------------------------ ==2965601== ==2965601== Thread #2 was created ==2965601== at 0x49BC152: clone (clone.S:71) ==2965601== by 0x48812EB: create_thread (createthread.c:101) ==2965601== by 0x4882E0F: pthread_create@@GLIBC_2.2.5 (pthread_create.c:817) ==2965601== by 0x4846FBA: pthread_create_WRK (hg_intercepts.c:445) ==2965601== by 0x48480BD: pthread_create@* (hg_intercepts.c:478) ==2965601== by 0x1092C9: use_stack_allocated_semaphore (in /home/gback/cs3214/dutchblitz.sem/tests/semtest) ==2965601== by 0x10931C: main (in /home/gback/cs3214/dutchblitz.sem/tests/semtest) ==2965601== ==2965601== ---------------------------------------------------------------- ==2965601== ==2965601== Possible data race during write of size 8 at 0x1FFEFFFDE0 by thread #1 ==2965601== Locks held: none ==2965601== at 0x484BCD9: memset (vg_replace_strmem.c:1358) ==2965601== by 0x10923F: touch_the_stack (in /home/gback/cs3214/dutchblitz.sem/tests/semtest) ==2965601== by 0x109317: main (in /home/gback/cs3214/dutchblitz.sem/tests/semtest) ==2965601== ==2965601== This conflicts with a previous read of size 8 by thread #2 ==2965601== Locks held: none ==2965601== at 0x488CA37: sem_post@@GLIBC_2.2.5 (sem_post.c:44) ==2965601== by 0x4847ED1: sem_post_WRK (hg_intercepts.c:2993) ==2965601== by 0x4848AB6: sem_post@* (hg_intercepts.c:3013) ==2965601== by 0x109272: post_thread (in /home/gback/cs3214/dutchblitz.sem/tests/semtest) ==2965601== by 0x48471B2: mythread_wrapper (hg_intercepts.c:406) ==2965601== by 0x4882608: start_thread (pthread_create.c:477) ==2965601== by 0x49BC162: clone (clone.S:95) ==2965601== Address 0x1ffefffde0 is on thread #1's stack ==2965601== in frame #1, created by touch_the_stack (???:) ==2965601== ==2965601== ---------------------------------------------------------------- ==2965601== ==2965601== Possible data race during write of size 4 at 0x1FFEFFFDE8 by thread #1 ==2965601== Locks held: none ==2965601== at 0x484BCDC: memset (vg_replace_strmem.c:1358) ==2965601== by 0x10923F: touch_the_stack (in /home/gback/cs3214/dutchblitz.sem/tests/semtest) ==2965601== by 0x109317: main (in /home/gback/cs3214/dutchblitz.sem/tests/semtest) ==2965601== ==2965601== This conflicts with a previous read of size 4 by thread #2 ==2965601== Locks held: none ==2965601== at 0x488CA34: sem_post@@GLIBC_2.2.5 (sem_post.c:36) ==2965601== by 0x4847ED1: sem_post_WRK (hg_intercepts.c:2993) ==2965601== by 0x4848AB6: sem_post@* (hg_intercepts.c:3013) ==2965601== by 0x109272: post_thread (in /home/gback/cs3214/dutchblitz.sem/tests/semtest) ==2965601== by 0x48471B2: mythread_wrapper (hg_intercepts.c:406) ==2965601== by 0x4882608: start_thread (pthread_create.c:477) ==2965601== by 0x49BC162: clone (clone.S:95) ==2965601== Address 0x1ffefffde8 is on thread #1's stack ==2965601== in frame #1, created by touch_the_stack (???:) ==2965601== ==2965601== ==2965601== Use --history-level=approx or =none to gain increased speed, at ==2965601== the cost of reduced accuracy of conflicting-access information ==2965601== For lists of detected and suppressed errors, rerun with: -s ==2965601== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0) EXPECTED RESULT There should be no errors shown. SOFTWARE/OS VERSIONS - Valgrind 3.18.1 (built from source) - gcc 9.4.0 - Ubuntu 20.04.4 ADDITIONAL INFORMATION The error is not shown if the semaphore is dynamically allocated.
I'll see if I can reproduce this with clang. Does joining the thread change anything? I'm not a threading expert, but I would expect the thread to be created detached or joined.
Answering my own question. On Linux, if I add pthread_join: static void use_stack_allocated_semaphore(void) { sem_t sem; sem_init(&sem, 0, 0); pthread_t t; pthread_create(&t, NULL, post_thread, &sem); sem_wait(&sem); sem_destroy(&sem); pthread_join(t, NULL); } then I get no error ==17867== Helgrind, a thread error detector ==17867== Copyright (C) 2007-2017, and GNU GPL'd, by OpenWorks LLP et al. ==17867== Using Valgrind-3.20.0.GIT and LibVEX; rerun with -h for copyright info ==17867== Command: ./451327 ==17867== ==17867== ==17867== Use --history-level=approx or =none to gain increased speed, at ==17867== the cost of reduced accuracy of conflicting-access information ==17867== For lists of detected and suppressed errors, rerun with: -s ==17867== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
And the manual says https://valgrind.org/docs/manual/hg-manual.html 7.5. Hints and Tips for Effective Use of Helgrind 6. Round up all finished threads using pthread_join. Avoid detaching threads: don't create threads in the detached state, and don't call pthread_detach on existing threads.
The actual application in which the false positive occurs uses pthread_join properly. I created a minimum reproducible test case here for you to see the issue. Adding pthread_join introduces additional edges in the happens-before graph, which influences Helgrind's race detection. I'll prepare a larger example that uses pthread_join.
Created attachment 148434 [details] test program that shows spurious warning even when threads are joined I've attached a larger example from the actual code to show the potential issue.
OK, I've reopened this. The error is intermittent for me so I assume that there is some race.
It's more reliable if you `NTHREADS` and `n`
(In reply to Godmar Back from comment #7) > It's more reliable if you `NTHREADS` and `n` if you *increase*