Re: [GCC 12] libstdc++: Fix lifetime bugs for non-TLS eh_globals [PR105880]

[Jonathan Wakely <jwakely@redhat.com>]

On Thu, 21 Jul 2022 at 08:48, Jonathan Wakely wrote:
>
>
>
> On Thu, 21 Jul 2022, 08:16 Sebastian Huber, <sebastian.huber@embedded-brains.de> wrote:
>>
>> From: Jonathan Wakely <jwakely@redhat.com>
>>
>> This ensures that the single-threaded fallback buffer eh_globals is not
>> destroyed during program termination, using the same immortalization
>> technique used for error category objects.
>>
>> Also ensure that init._M_init can still be read after init has been
>> destroyed, by making it a static data member.
>>
>> libstdc++-v3/ChangeLog:
>>
>>         PR libstdc++/105880
>>         * libsupc++/eh_globals.cc (eh_globals): Ensure constant init and
>>         prevent destruction during termination.
>>         (__eh_globals_init::_M_init): Replace with static member _S_init.
>>         (__cxxabiv1::__cxa_get_globals_fast): Update.
>>         (__cxxabiv1::__cxa_get_globals): Likewise.
>>
>> (cherry picked from commit 1e65f2ed99024f23c56f7b6a961898bcaa882a92)
>> ---
>>
>> Would it be acceptable to back port this fix to GCC 12?
>
>
> Yes, it's ok for gcc-12

But the a33dda016e5acf9c6325ce8a72a1b0238130374e follow-up is also
needed on the branch. I'll take care of that.


>
>
>
>>
>>  libstdc++-v3/libsupc++/eh_globals.cc | 51 ++++++++++++++++++++--------
>>  1 file changed, 37 insertions(+), 14 deletions(-)
>>
>> diff --git a/libstdc++-v3/libsupc++/eh_globals.cc b/libstdc++-v3/libsupc++/eh_globals.cc
>> index 3a003b89edf..768425c0f40 100644
>> --- a/libstdc++-v3/libsupc++/eh_globals.cc
>> +++ b/libstdc++-v3/libsupc++/eh_globals.cc
>> @@ -64,8 +64,26 @@ __cxxabiv1::__cxa_get_globals() _GLIBCXX_NOTHROW
>>
>>  #else
>>
>> -// Single-threaded fallback buffer.
>> -static __cxa_eh_globals eh_globals;
>> +#if __has_cpp_attribute(clang::require_constant_initialization)
>> +#  define __constinit [[clang::require_constant_initialization]]
>> +#endif
>> +
>> +namespace
>> +{
>> +  struct constant_init
>> +  {
>> +    union {
>> +      unsigned char unused;
>> +      __cxa_eh_globals obj;
>> +    };
>> +    constexpr constant_init() : obj() { }
>> +
>> +    ~constant_init() { /* do nothing, union member is not destroyed */ }
>> +  };
>> +
>> +  // Single-threaded fallback buffer.
>> +  __constinit constant_init eh_globals;
>> +}
>>
>>  #if __GTHREADS
>>
>> @@ -90,32 +108,37 @@ eh_globals_dtor(void* ptr)
>>  struct __eh_globals_init
>>  {
>>    __gthread_key_t      _M_key;
>> -  bool                         _M_init;
>> +  static bool          _S_init;
>>
>> -  __eh_globals_init() : _M_init(false)
>> -  {
>> +  __eh_globals_init()
>> +  {
>>      if (__gthread_active_p())
>> -      _M_init = __gthread_key_create(&_M_key, eh_globals_dtor) == 0;
>> +      _S_init = __gthread_key_create(&_M_key, eh_globals_dtor) == 0;
>>    }
>>
>>    ~__eh_globals_init()
>>    {
>> -    if (_M_init)
>> +    if (_S_init)
>>        __gthread_key_delete(_M_key);
>> -    _M_init = false;
>> +    _S_init = false;
>>    }
>> +
>> +  __eh_globals_init(const __eh_globals_init&) = delete;
>> +  __eh_globals_init& operator=(const __eh_globals_init&) = delete;
>>  };
>>
>> +bool __eh_globals_init::_S_init = false;
>> +
>>  static __eh_globals_init init;
>>
>>  extern "C" __cxa_eh_globals*
>>  __cxxabiv1::__cxa_get_globals_fast() _GLIBCXX_NOTHROW
>>  {
>>    __cxa_eh_globals* g;
>> -  if (init._M_init)
>> +  if (init._S_init)
>>      g = static_cast<__cxa_eh_globals*>(__gthread_getspecific(init._M_key));
>>    else
>> -    g = &eh_globals;
>> +    g = &eh_globals.obj;
>>    return g;
>>  }
>>
>> @@ -123,7 +146,7 @@ extern "C" __cxa_eh_globals*
>>  __cxxabiv1::__cxa_get_globals() _GLIBCXX_NOTHROW
>>  {
>>    __cxa_eh_globals* g;
>> -  if (init._M_init)
>> +  if (init._S_init)
>>      {
>>        g = static_cast<__cxa_eh_globals*>(__gthread_getspecific(init._M_key));
>>        if (!g)
>> @@ -140,7 +163,7 @@ __cxxabiv1::__cxa_get_globals() _GLIBCXX_NOTHROW
>>         }
>>      }
>>    else
>> -    g = &eh_globals;
>> +    g = &eh_globals.obj;
>>    return g;
>>  }
>>
>> @@ -148,11 +171,11 @@ __cxxabiv1::__cxa_get_globals() _GLIBCXX_NOTHROW
>>
>>  extern "C" __cxa_eh_globals*
>>  __cxxabiv1::__cxa_get_globals_fast() _GLIBCXX_NOTHROW
>> -{ return &eh_globals; }
>> +{ return &eh_globals.obj; }
>>
>>  extern "C" __cxa_eh_globals*
>>  __cxxabiv1::__cxa_get_globals() _GLIBCXX_NOTHROW
>> -{ return &eh_globals; }
>> +{ return &eh_globals.obj; }
>>
>>  #endif
>>
>> --
>> 2.35.3
>>