[libcxx-commits] [libcxx] [libc++] Work around benign initialization order fiasco in locale (PR #73533)

[Louis Dionne via libcxx-commits]

https://github.com/ldionne updated https://github.com/llvm/llvm-project/pull/73533

>From 8484ff20ad032812ffbc46bfe6d769ddba0a5acf Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Mon, 27 Nov 2023 11:01:35 -0500
Subject: [PATCH 1/3] [libc++] Work around benign initialization order fiasco
 in locale

Technically, it is possible for the ios initializers to run before
the initializer of classic_locale_imp_, which would cause `.emplace`
to be called on a not-yet-initialized __no_destroy variable.

This patch fixes that by making the variable trivially constinit,
since we're not doing anything in its constructor anyway.
---
 libcxx/include/__utility/no_destroy.h | 13 +++++++------
 libcxx/src/locale.cpp                 |  2 +-
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/libcxx/include/__utility/no_destroy.h b/libcxx/include/__utility/no_destroy.h
index c8581cf9606b571..fc936d2d287316a 100644
--- a/libcxx/include/__utility/no_destroy.h
+++ b/libcxx/include/__utility/no_destroy.h
@@ -28,22 +28,23 @@ struct __uninitialized_tag {};
 // initialization using __emplace.
 template <class _Tp>
 struct __no_destroy {
-  _LIBCPP_HIDE_FROM_ABI explicit __no_destroy(__uninitialized_tag) {}
-  _LIBCPP_HIDE_FROM_ABI ~__no_destroy() {
+  _LIBCPP_CONSTEXPR _LIBCPP_HIDE_FROM_ABI explicit __no_destroy(__uninitialized_tag) {}
+  _LIBCPP_CONSTEXPR _LIBCPP_HIDE_FROM_ABI ~__no_destroy() {
     // nothing
   }
 
   template <class... _Args>
-  _LIBCPP_HIDE_FROM_ABI explicit __no_destroy(_Args&&... __args) : __obj_(std::forward<_Args>(__args)...) {}
+  _LIBCPP_CONSTEXPR _LIBCPP_HIDE_FROM_ABI explicit __no_destroy(_Args&&... __args)
+      : __obj_(std::forward<_Args>(__args)...) {}
 
   template <class... _Args>
-  _LIBCPP_HIDE_FROM_ABI _Tp& __emplace(_Args&&... __args) {
+  _LIBCPP_CONSTEXPR _LIBCPP_HIDE_FROM_ABI _Tp& __emplace(_Args&&... __args) {
     new (&__obj_) _Tp(std::forward<_Args>(__args)...);
     return __obj_;
   }
 
-  _LIBCPP_HIDE_FROM_ABI _Tp& __get() { return __obj_; }
-  _LIBCPP_HIDE_FROM_ABI _Tp const& __get() const { return __obj_; }
+  _LIBCPP_CONSTEXPR _LIBCPP_HIDE_FROM_ABI _Tp& __get() { return __obj_; }
+  _LIBCPP_CONSTEXPR _LIBCPP_HIDE_FROM_ABI _Tp const& __get() const { return __obj_; }
 
 private:
   union {
diff --git a/libcxx/src/locale.cpp b/libcxx/src/locale.cpp
index 2cb75f81b36da33..d6ba490a00a1109 100644
--- a/libcxx/src/locale.cpp
+++ b/libcxx/src/locale.cpp
@@ -546,7 +546,7 @@ locale::__imp::use_facet(long id) const
 // expensive in parallel applications. The classic locale is used by default
 // in all streams. Note: if a new global locale is installed, then we lose
 // the benefit of no reference counting.
-__no_destroy<locale::__imp> locale::__imp::classic_locale_imp_(__uninitialized_tag{}); // initialized below in classic()
+constinit __no_destroy<locale::__imp> locale::__imp::classic_locale_imp_(__uninitialized_tag{}); // initialized below in classic()
 
 const locale& locale::classic() {
   static const __no_destroy<locale> classic_locale(__private_tag{}, [] {

>From 918dc9bce13ee2f9524f3db65f733b2a383a24b5 Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Mon, 27 Nov 2023 14:05:59 -0500
Subject: [PATCH 2/3] Format

---
 libcxx/src/locale.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/libcxx/src/locale.cpp b/libcxx/src/locale.cpp
index d6ba490a00a1109..5bd53d99ca41a48 100644
--- a/libcxx/src/locale.cpp
+++ b/libcxx/src/locale.cpp
@@ -546,7 +546,8 @@ locale::__imp::use_facet(long id) const
 // expensive in parallel applications. The classic locale is used by default
 // in all streams. Note: if a new global locale is installed, then we lose
 // the benefit of no reference counting.
-constinit __no_destroy<locale::__imp> locale::__imp::classic_locale_imp_(__uninitialized_tag{}); // initialized below in classic()
+constinit __no_destroy<locale::__imp>
+    locale::__imp::classic_locale_imp_(__uninitialized_tag{}); // initialized below in classic()
 
 const locale& locale::classic() {
   static const __no_destroy<locale> classic_locale(__private_tag{}, [] {

>From 1e35a468323e99d381c8fe3b18bf91e908058e01 Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Mon, 27 Nov 2023 14:27:26 -0500
Subject: [PATCH 3/3] Initialize dummy member when constant evaluated

---
 libcxx/include/__utility/no_destroy.h | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/libcxx/include/__utility/no_destroy.h b/libcxx/include/__utility/no_destroy.h
index fc936d2d287316a..3563774301efba0 100644
--- a/libcxx/include/__utility/no_destroy.h
+++ b/libcxx/include/__utility/no_destroy.h
@@ -10,6 +10,7 @@
 #define _LIBCPP___UTILITY_NO_DESTROY_H
 
 #include <__config>
+#include <__type_traits/is_constant_evaluated.h>
 #include <__utility/forward.h>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
@@ -28,8 +29,12 @@ struct __uninitialized_tag {};
 // initialization using __emplace.
 template <class _Tp>
 struct __no_destroy {
-  _LIBCPP_CONSTEXPR _LIBCPP_HIDE_FROM_ABI explicit __no_destroy(__uninitialized_tag) {}
-  _LIBCPP_CONSTEXPR _LIBCPP_HIDE_FROM_ABI ~__no_destroy() {
+  _LIBCPP_CONSTEXPR _LIBCPP_HIDE_FROM_ABI explicit __no_destroy(__uninitialized_tag) : __dummy_() {
+    if (__libcpp_is_constant_evaluated()) {
+      __dummy_ = char();
+    }
+  }
+  _LIBCPP_HIDE_FROM_ABI ~__no_destroy() {
     // nothing
   }
 
@@ -49,6 +54,7 @@ struct __no_destroy {
 private:
   union {
     _Tp __obj_;
+    char __dummy_; // so we can initialize a member even with __uninitialized_tag for constexpr-friendliness
   };
 };