[cfe-dev] libc++: Race condition in facets initialization?

[Howard Hinnant]

On Jul 8, 2015, at 5:26 AM, Andrew Parker <andrew.j.c.parker at gmail.com> wrote:
> 
> I'm currently porting libc++ to work with MSVC.  I'm seeing a crash when I call the insertion operator on std::err.  I've traced the problem down to static initialization order of the static locale::id member of the ctype<char> facet.
> 
> I've looked over the code several times and am convinced that there's a genuine issue here.  It's entirely possible that the changes I've made for MSVC (or just the use of MSVC itself) may be causing unexpected problems.  Hence the need for a second opinion.
> 
> Here's a rough overview of the chain of events:
> 
> - static constructors for my binary are called.
> - ios_base::Init::Init() called to initialize std::cout, std::cerr etc..
> - const locale& locale::__imp::make_classic() called during initialization of first basic_streambuf.
> - Enter locale::__imp::__imp(size_t refs) to start constructing and installing facets into the locale.
> 
> The cause of my particular crash is when we install ctype<char>, i.e. install(&make<_VSTD::ctype<char> >(nullptr, false, 1u));
> 
> The install member of locale::__imp looks like:
> 
> template <class F> void install(F* f) {install(f, f->id.__get());}
> 
> The thing to note here is that the id member of *f is actually a static member of ctype<char> (the template param F is resolving to ctype<char> here). The call to get() looks at the once flag member of ctype<char>::id, which is zero as the id variable is static an zero initialized.  This means the member __id_ of id is set to the next available id (__next_id) and installed at that index in the locale.
> 
> Things go wrong later when the static ctr for locale::id ctype<char>::id is called.  This effectively zero initializes the id again.  Later on when use_facet is called (during my call to the std::cerr insertion operator) the id gets set again (to __next_id).  This index is invalid and causes a crash when looked up in the locale.
> 
> It seems to me that this issue would affect all of the static id members of the various facets.  Any thoughts anyone?  How could this have never been seen before?  Is it possible GCC/clang somehow skirt around this bug?
> 
> I want to be sure it's not me stuffing things up before I start writing patches.

This is tricky (and non-portable) code.

The intent is that the locale::id default constructor:

    _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR id() :__id_(0) {}

is constexpr.  Meaning that it is zero-initialized statically.  And what you are seeing is that the compiler is initializing it dynamically, likely because constexpr is not yet implemented.

There is a similar (but unrelated) issue with the mutex constructor, which must be statically initializable:

     constexpr mutex() _NOEXCEPT : __m_(PTHREAD_MUTEX_INITIALIZER) {}

Howard