[Bug libstdc++/111726] New: Data race in std::poisson_distribution

[Bug libstdc++/111726] New: Data race in std::poisson_distribution

[rugasikrisztian at gmail dot com]

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

            Bug ID: 111726
           Summary: Data race in std::poisson_distribution
           Product: gcc
           Version: 13.2.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: rugasikrisztian at gmail dot com
  Target Milestone: ---

The following program, which uses separate std::poisson_distribution instances
on multiple threads, has a data race in both the constructor and operator() of
the distribution:

https://godbolt.org/z/r1ojKdYna


#include <random>
#include <thread>

int rand_poisson() {
    thread_local std::mt19937_64 gen{ 0x123456789 };
    std::poisson_distribution<int> dist{ 15.0 };
    return dist(gen);
}

int main() {
    std::jthread T1{ rand_poisson };
    std::jthread T2{ rand_poisson };
}


Compiled with gcc 13.2 and the options -std=c++20 -O3 -fsanitize=thread, the
output is:

==================
WARNING: ThreadSanitizer: data race (pid=1)
  Write of size 4 at 0x7fed5e87a108 by thread T2:
    #0 lgamma <null> (libtsan.so.2+0x536da) (BuildId:
dd14181575b14129bf3264ad0047089fc45e39d4)
    #1 std::poisson_distribution<int>::param_type::_M_initialize()
/opt/compiler-explorer/gcc-13.2.0/include/c++/13.2.0/bits/random.tcc:1275
(output.s+0x4014a0) (BuildId: 835919b751a9e2f5ac4bdd461e8f650240b0a6eb)
    #2 std::poisson_distribution<int>::param_type::param_type(double)
/opt/compiler-explorer/gcc-13.2.0/include/c++/13.2.0/bits/random.h:4573
(output.s+0x4014a0)
    #3 std::poisson_distribution<int>::poisson_distribution(double)
/opt/compiler-explorer/gcc-13.2.0/include/c++/13.2.0/bits/random.h:4609
(output.s+0x4014a0)
    #4 rand_poisson() /app/example.cpp:6 (output.s+0x4014a0)
    #5 int std::__invoke_impl<int, int (*)()>(std::__invoke_other, int (*&&)())
/opt/compiler-explorer/gcc-13.2.0/include/c++/13.2.0/bits/invoke.h:61
(output.s+0x401559) (BuildId: 835919b751a9e2f5ac4bdd461e8f650240b0a6eb)
    #6 std::__invoke_result<int (*)()>::type std::__invoke<int (*)()>(int
(*&&)()) /opt/compiler-explorer/gcc-13.2.0/include/c++/13.2.0/bits/invoke.h:96
(output.s+0x401559)
    #7 int std::thread::_Invoker<std::tuple<int (*)()>
>::_M_invoke<0ul>(std::_Index_tuple<0ul>)
/opt/compiler-explorer/gcc-13.2.0/include/c++/13.2.0/bits/std_thread.h:292
(output.s+0x401559)
    #8 std::thread::_Invoker<std::tuple<int (*)()> >::operator()()
/opt/compiler-explorer/gcc-13.2.0/include/c++/13.2.0/bits/std_thread.h:299
(output.s+0x401559)
    #9 std::thread::_State_impl<std::thread::_Invoker<std::tuple<int (*)()> >
>::_M_run()
/opt/compiler-explorer/gcc-13.2.0/include/c++/13.2.0/bits/std_thread.h:244
(output.s+0x401559)
    #10 <null> <null> (libstdc++.so.6+0xe8ad2) (BuildId:
9f3caf39aab289429a6a218ddc8d4c9ff5ef08a4)

  Previous write of size 4 at 0x7fed5e87a108 by thread T1:
    #0 lgamma <null> (libtsan.so.2+0x536da) (BuildId:
dd14181575b14129bf3264ad0047089fc45e39d4)
    #1 int
std::poisson_distribution<int>::operator()<std::mersenne_twister_engine<unsigned
long, 64ul, 312ul, 156ul, 31ul, 13043109905998158313ul, 29ul,
6148914691236517205ul, 17ul, 8202884508482404352ul, 37ul,
18444473444759240704ul, 43ul, 6364136223846793005ul>
>(std::mersenne_twister_engine<unsigned long, 64ul, 312ul, 156ul, 31ul,
13043109905998158313ul, 29ul, 6148914691236517205ul, 17ul,
8202884508482404352ul, 37ul, 18444473444759240704ul, 43ul,
6364136223846793005ul>&, std::poisson_distribution<int>::param_type const&)
/opt/compiler-explorer/gcc-13.2.0/include/c++/13.2.0/bits/random.tcc:1388
(output.s+0x40255c) (BuildId: 835919b751a9e2f5ac4bdd461e8f650240b0a6eb)
    #2 int
std::poisson_distribution<int>::operator()<std::mersenne_twister_engine<unsigned
long, 64ul, 312ul, 156ul, 31ul, 13043109905998158313ul, 29ul,
6148914691236517205ul, 17ul, 8202884508482404352ul, 37ul,
18444473444759240704ul, 43ul, 6364136223846793005ul>
>(std::mersenne_twister_engine<unsigned long, 64ul, 312ul, 156ul, 31ul,
13043109905998158313ul, 29ul, 6148914691236517205ul, 17ul,
8202884508482404352ul, 37ul, 18444473444759240704ul, 43ul,
6364136223846793005ul>&)
/opt/compiler-explorer/gcc-13.2.0/include/c++/13.2.0/bits/random.h:4666
(output.s+0x401524) (BuildId: 835919b751a9e2f5ac4bdd461e8f650240b0a6eb)
    #3 rand_poisson() /app/example.cpp:7 (output.s+0x401524)
    #4 int std::__invoke_impl<int, int (*)()>(std::__invoke_other, int (*&&)())
/opt/compiler-explorer/gcc-13.2.0/include/c++/13.2.0/bits/invoke.h:61
(output.s+0x401559) (BuildId: 835919b751a9e2f5ac4bdd461e8f650240b0a6eb)
    #5 std::__invoke_result<int (*)()>::type std::__invoke<int (*)()>(int
(*&&)()) /opt/compiler-explorer/gcc-13.2.0/include/c++/13.2.0/bits/invoke.h:96
(output.s+0x401559)
    #6 int std::thread::_Invoker<std::tuple<int (*)()>
>::_M_invoke<0ul>(std::_Index_tuple<0ul>)
/opt/compiler-explorer/gcc-13.2.0/include/c++/13.2.0/bits/std_thread.h:292
(output.s+0x401559)
    #7 std::thread::_Invoker<std::tuple<int (*)()> >::operator()()
/opt/compiler-explorer/gcc-13.2.0/include/c++/13.2.0/bits/std_thread.h:299
(output.s+0x401559)
    #8 std::thread::_State_impl<std::thread::_Invoker<std::tuple<int (*)()> >
>::_M_run()
/opt/compiler-explorer/gcc-13.2.0/include/c++/13.2.0/bits/std_thread.h:244
(output.s+0x401559)
    #9 <null> <null> (libstdc++.so.6+0xe8ad2) (BuildId:
9f3caf39aab289429a6a218ddc8d4c9ff5ef08a4)

  Location is global '__signgam' of size 4 at 0x7fed5e87a108
(libm.so.6+0x14e108)

  Thread T2 (tid=4, running) created by main thread at:
    #0 pthread_create <null> (libtsan.so.2+0x40cd6) (BuildId:
dd14181575b14129bf3264ad0047089fc45e39d4)
    #1 std::thread::_M_start_thread(std::unique_ptr<std::thread::_State,
std::default_delete<std::thread::_State> >, void (*)()) <null>
(libstdc++.so.6+0xe8e4b) (BuildId: 9f3caf39aab289429a6a218ddc8d4c9ff5ef08a4)
    #2 main /app/example.cpp:12 (output.s+0x40126a) (BuildId:
835919b751a9e2f5ac4bdd461e8f650240b0a6eb)

  Thread T1 (tid=3, finished) created by main thread at:
    #0 pthread_create <null> (libtsan.so.2+0x40cd6) (BuildId:
dd14181575b14129bf3264ad0047089fc45e39d4)
    #1 std::thread::_M_start_thread(std::unique_ptr<std::thread::_State,
std::default_delete<std::thread::_State> >, void (*)()) <null>
(libstdc++.so.6+0xe8e4b) (BuildId: 9f3caf39aab289429a6a218ddc8d4c9ff5ef08a4)
    #2 main /app/example.cpp:11 (output.s+0x40125b) (BuildId:
835919b751a9e2f5ac4bdd461e8f650240b0a6eb)

SUMMARY: ThreadSanitizer: data race
(/opt/compiler-explorer/gcc-13.2.0/lib64/libtsan.so.2+0x536da) (BuildId:
dd14181575b14129bf3264ad0047089fc45e39d4) in lgamma
==================
ThreadSanitizer: reported 1 warnings


As far as I can tell this issue has been present since the initial
implementation of std::poisson_distribution.
The cause of it is that the implementation uses the lgamma function, which
modifies a global variable.

[Bug libstdc++/111726] lgamma usage in std::poisson_distribution could cause a Data race

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|Data race in                |lgamma usage in
                   |std::poisson_distribution   |std::poisson_distribution
                   |                            |could cause a Data race

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Krisztián Rugási from comment #0)
> As far as I can tell this issue has been present since the initial
> implementation of std::poisson_distribution.
> The cause of it is that the implementation uses the lgamma function, which
> modifies a global variable.

I am not 100% sure but since _M_initialize does not use signgam, this is just 2
writes to a global variable that will not be read so the data write race is
100% ok.

[Bug libstdc++/111726] lgamma usage in std::poisson_distribution could cause a Data race

[redi at gcc dot gnu.org]

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2023-10-16
     Ever confirmed|0                           |1

--- Comment #2 from Jonathan Wakely <redi at gcc dot gnu.org> ---
We should use the non-standard but thread-safe lgamma_r if available.

[Bug libstdc++/111726] lgamma usage in std::poisson_distribution could cause a Data race

[redi at gcc dot gnu.org]

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

--- Comment #3 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #1)
> I am not 100% sure but since _M_initialize does not use signgam, this is
> just 2 writes to a global variable that will not be read so the data write
> race is 100% ok.

This example might not misbehave (even though it's technically UB) but if
somebody constructs a std::poisson_distribution in one thread and use lgamma in
another thread, the lgamma call might get the wrong result.

So in the general case, this data race can cause incorrect results (as well as
being UB).

[Bug libstdc++/111726] lgamma usage in std::poisson_distribution could cause a Data race

[redi at gcc dot gnu.org]

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

--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #2)
> We should use the non-standard but thread-safe lgamma_r if available.

And if not available, we can use std::log(std::abs(std::tgamma(x))).

[Bug libstdc++/111726] lgamma usage in std::poisson_distribution could cause a Data race

[redi at gcc dot gnu.org]

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

--- Comment #5 from Jonathan Wakely <redi at gcc dot gnu.org> ---
I think the abs is not needed for some of the lgamma calls, as the argument is
guaranteed to be non-negative. Unfortunately, that's not the case for all of
them.

lgamma_r, lgammaf_r and lgammal_r are present in glibc, musl, newlib, Solaris,
FreeBSD, NetBSD at least.

Not present in AIX or mingw-w64. Not present in darwin unless you compile with
_REENTRANT, which makes it unusable for libstdc++ (we need it to be declared by
<math.h> unconditionally).

[Bug libstdc++/111726] lgamma usage in std::poisson_distribution could cause a Data race

[redi at gcc dot gnu.org]

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

--- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #5)
> lgamma_r, lgammaf_r and lgammal_r are present in glibc, musl, newlib,
> Solaris, FreeBSD, NetBSD at least.

And OpenBSD has lgamma_r and lgammaf_r (but not lgammal_r).