[Bug libstdc++/77704] Data race on std::ctype<char>

[Bug libstdc++/77704] Data race on std::ctype<char>

[dboles.src at gmail dot com]

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

Daniel Boles <dboles.src at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dboles.src at gmail dot com

--- Comment #7 from Daniel Boles <dboles.src at gmail dot com> ---
Yup, I too was just bitten by this, with libstdc++ 9.4.0-1ubuntu1~20.04.1

Luckily the Thread Sanitizer finally led me to the diagnosis, and many thanks
to Boris for posting his workaround, which I can confirm seems to fix it for me
too

[Bug libstdc++/77704] Data race on std::ctype<char>

[rustamabd at gmail dot com]

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

Rustam Abdullaev <rustamabd at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rustamabd at gmail dot com

--- Comment #8 from Rustam Abdullaev <rustamabd at gmail dot com> ---
Still an issue in GCC 12.3.0.

[Bug libstdc++/77704] Data race on std::ctype<char>

[redi at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |redi at gcc dot gnu.org

--- Comment #9 from Jonathan Wakely <redi at gcc dot gnu.org> ---
There's a related data race in std::basic_ios::fill() because of these mutable
members:

      mutable char_type                              _M_fill;
      mutable bool                                   _M_fill_init;

      char_type
      fill() const
      {
        if (!_M_fill_init)
          {
            _M_fill = this->widen(' ');
            _M_fill_init = true;
          }
        return _M_fill;
      }

That one's easier to fix.

[Bug libstdc++/77704] Data race on std::ctype<char>

[redi at gcc dot gnu.org]

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

--- Comment #10 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Boris Kolpackov from comment #5)
> For anyone interested, here is the workaround we came up with:
> 
> // A data race happens in the libstdc++ (as of GCC 7.2) implementation of the
> // ctype<ctype>::narrow() function (bug #77704). The issue is easily
> triggered
> // by the testscript runner that indirectly (via regex) uses ctype<char>
> facet
> // of the global locale (and can potentially be triggered by other locale-
> // aware code). We work around this by pre-initializing the global locale
> // facet internal cache.
> //
> #ifdef _GLIBCXX_
>   {
>     const ctype<char>& ct (use_facet<ctype<char>> (locale ()));
> 
>     for (size_t i (0); i != 256; ++i)
>       ct.narrow (static_cast<char> (i), '\0');
>   }
> #endif

It would be better to call ct.narrow(0, 0, 0, 0) as that will populate the
_M_narrow array and also set the _M_narrow_ok flag. Otherwise you can still get
later races on the flag.

[Bug libstdc++/77704] Data race on std::ctype<char>

[cvs-commit at gcc dot gnu.org]

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

--- Comment #11 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:23ef0f68ad5fca1fd7027caaaa5f6cb9f6d27b28

commit r15-510-g23ef0f68ad5fca1fd7027caaaa5f6cb9f6d27b28
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Fri May 3 20:00:08 2024 +0100

    libstdc++: Fix data race in std::basic_ios::fill() [PR77704]

    The lazy caching in std::basic_ios::fill() updates a mutable member
    without synchronization, which can cause a data race if two threads both
    call fill() on the same stream object when _M_fill_init is false.

    To avoid this we can just cache the _M_fill member and set _M_fill_init
    early in std::basic_ios::init, instead of doing it lazily. As explained
    by the comment in init, there's a good reason for doing it lazily. When
    char_type is neither char nor wchar_t, the locale might not have a
    std::ctype<char_type> facet, so getting the fill character would throw
    an exception. The current lazy init allows using unformatted I/O with
    such a stream, because the fill character is never needed and so it
    doesn't matter if the locale doesn't have a ctype<char_type> facet. We
    can maintain this property by only setting the fill character in
    std::basic_ios::init if the ctype facet is present at that time. If
    fill() is called later and the fill character wasn't set by init, we can
    get it from the stream's current locale at the point when fill() is
    called (and not try to cache it without synchronization). If the stream
    hasn't been imbued with a locale that includes the facet when we need
    the fill() character, then throw bad_cast at that point.

    This causes a change in behaviour for the following program:

      std::ostringstream out;
      out.imbue(loc);
      auto fill = out.fill();

    Previously the fill character would have been set when fill() is called,
    and so would have used the new locale. This commit changes it so that
    the fill character is set on construction and isn't affected by the new
    locale being imbued later. This new behaviour seems to be what the
    standard requires, and matches MSVC.

    The new 27_io/basic_ios/fill/char/fill.cc test verifies that it's still
    possible to use a std::basic_ios without the ctype<char_type> facet
    being present at construction.

    libstdc++-v3/ChangeLog:

            PR libstdc++/77704
            * include/bits/basic_ios.h (basic_ios::fill()): Do not modify
            _M_fill and _M_fill_init in a const member function.
            (basic_ios::fill(char_type)): Use _M_fill directly instead of
            calling fill(). Set _M_fill_init to true.
            * include/bits/basic_ios.tcc (basic_ios::init): Set _M_fill and
            _M_fill_init here instead.
            * testsuite/27_io/basic_ios/fill/char/1.cc: New test.
            * testsuite/27_io/basic_ios/fill/wchar_t/1.cc: New test.