public inbox for libstdc++@gcc.gnu.org
 help / color / mirror / Atom feed
* [GCC 12] libstdc++: Fix lifetime bugs for non-TLS eh_globals [PR105880]
@ 2022-07-21  7:16 Sebastian Huber
  2022-07-21  7:48 ` Jonathan Wakely
  0 siblings, 1 reply; 3+ messages in thread
From: Sebastian Huber @ 2022-07-21  7:16 UTC (permalink / raw)
  To: libstdc++; +Cc: Jonathan Wakely

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?

 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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [GCC 12] libstdc++: Fix lifetime bugs for non-TLS eh_globals [PR105880]
  2022-07-21  7:16 [GCC 12] libstdc++: Fix lifetime bugs for non-TLS eh_globals [PR105880] Sebastian Huber
@ 2022-07-21  7:48 ` Jonathan Wakely
  2022-10-12 10:55   ` Jonathan Wakely
  0 siblings, 1 reply; 3+ messages in thread
From: Jonathan Wakely @ 2022-07-21  7:48 UTC (permalink / raw)
  To: Sebastian Huber; +Cc: libstdc++, Jonathan Wakely

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




>  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
>
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [GCC 12] libstdc++: Fix lifetime bugs for non-TLS eh_globals [PR105880]
  2022-07-21  7:48 ` Jonathan Wakely
@ 2022-10-12 10:55   ` Jonathan Wakely
  0 siblings, 0 replies; 3+ messages in thread
From: Jonathan Wakely @ 2022-10-12 10:55 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: Sebastian Huber, libstdc++

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
>>


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-10-12 10:55 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-21  7:16 [GCC 12] libstdc++: Fix lifetime bugs for non-TLS eh_globals [PR105880] Sebastian Huber
2022-07-21  7:48 ` Jonathan Wakely
2022-10-12 10:55   ` Jonathan Wakely

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).