[libcxx] Avoid destructor call for error_category singletons

[ldionne]

Summary

When a handle to an error_category singleton object is used during the termination phase of a program, the destruction of the error_category object may have occurred prior to execution of the current destructor or function registered with atexit, because the singleton object may have been constructed after the corresponding initialization or call to atexit. For example, the updated tests from this patch will fail if using a libc++ built using a compiler that updates the vtable of the object on destruction. This patch attempts to avoid the issue by causing the destructor to not be called in the style of ResourceInitHelper in src/experimental/memory_resource.cpp. This approach might not work if object lifetime is strictly enforced.

[EricWF]

I think a better fix would be to apply [[clang::no_destroy]] to the category variable definitions instead.

[EricWF]

Or alternatively, there's an example of how to fix this in memory_resource.cpp https://github.com/llvm-mirror/libcxx/blob/master/src/experimental/memory_resource.cpp#L67-L96

[mclow.lists]

@EricWF pointed out to me that it is UB to access standard library objects of static storage duration from an atexit handler: http://eel.is/c++draft/basic.start.term#6

[hubert.reinterpretcast]

In D65667#1613503, @mclow.lists wrote:

@EricWF pointed out to me that it is UB to access standard library objects of static storage duration from an atexit handler: http://eel.is/c++draft/basic.start.term#6

I don't read the paragraph that way. The access during evaluation of an atexit handler must, in the absence of multithreading, happen before "completion of destruction of objects with static storage duration and execution of std::atexit registered functions".

[mclow.lists]

When a handle to an error_category singleton object is used during the termination phase of a program, the destruction of the error_category object may have occurred prior to execution of the current destructor or function registered with atexit, because the singleton object may have been constructed after the corresponding initialization or call to atexit.

This sounds right, and that's exactly what http://eel.is/c++draft/basic.start.term#5 says (second sentence):

If a call to std::atexit strongly happens before the completion of the initialization of an object with static storage duration, the call to the destructor for the object is sequenced before the call to the function passed to std::atexit.

[mclow.lists]

The good news is that the constructor of the singleton object in libc++ is under your control. It can be constructed whenever you choose, just by calling generic_category().

[hubert.reinterpretcast]

In D65667#1615205, @mclow.lists wrote:

The good news is that the constructor of the singleton object in libc++ is under your control. It can be constructed whenever you choose, just by calling generic_category().

The lifetime of the object returned by generic_category() seems underspecified to me. Is there an objection to treating the behaviour as undesirable? It seems unfriendly to require users to collect possible error_category instances during initialization in order to keep error_codes around during program termination.

[mclow.lists]

Hubert - I'm not following what you're saying.
Where did error_code come from? As far as I can tell, your comment is the first mention of error_code in this conversation.

[hubert.reinterpretcast]

In D65667#1617183, @mclow.lists wrote:

Hubert - I'm not following what you're saying.
Where did error_code come from? As far as I can tell, your comment is the first mention of error_code in this conversation.

Yes, you're correct it is. We encountered this lifetime issue in an application where the reference to the out-of-lifetime object was the cat_ exposition-only member of error_code. The status quo with libc++ is that error_codes are likely unsafe to use during program termination.

[thakis]

To chime in from the sidelines: We strive to have zero code that runs before main in Chromium, so we're not excited about a patch that adds more such code to libc++. (In fact, we're hoping to contribute a build mode that removes the existing static initializer for cin/cout/cerr as well.)

[mclow.lists]

In D65667#1617952, @thakis wrote:

To chime in from the sidelines: We strive to have zero code that runs before main in Chromium, so we're not excited about a patch that adds more such code to libc++. (In fact, we're hoping to contribute a build mode that removes the existing static initializer for cin/cout/cerr as well.)

I'll pass along a comment from Howard (made to me privately, but he agreed that I could share it here):

There's a FILO guarantee on lifetime here: http://eel.is/c++draft/basic.start.term#3

A careful client would take advantage of that guarantee by ensuring that the lifetime of the object returned by std::generic_category() starts before the lifetime of a. One easy way to do this is in the default constructor for A:

A() {std::generic_category();}

Failure to go to such effort is where the UB lies. The suggested "fix" which attempts to remove this responsibility from this client imposes a cost on every other client of libc++, whether or not one uses <system_error>. The more namespace-scope static objects there are in libc++, the longer it takes every program that links to libc++ to launch and reach main(). Apple considered this such a large problem that they forbade namespace-scope static objects and I had to get special permission for the std streams: cin, cout, etc.

Not only would acceptance of this patch penalize all users, it would still not absolutely guarantee a fix. Platforms are free to initialize namespace-scope objects in main prior to initializing namespace-scope objects within libc++.

[mclow.lists]

In D65667#1617900, @hubert.reinterpretcast wrote:

In D65667#1617183, @mclow.lists wrote:

Hubert - I'm not following what you're saying.
Where did error_code come from? As far as I can tell, your comment is the first mention of error_code in this conversation.

Yes, you're correct it is. We encountered this lifetime issue in an application where the reference to the out-of-lifetime object was the cat_ exposition-only member of error_code. The status quo with libc++ is that error_codes are likely unsafe to use during program termination.

Thanks for the clarification.

So the situation is:

• You register an atexit handler.
• During program termination, your handler is called, and you do something that creates an error_code object.
• You try to use the error_category of that error code, and discover that it has been destroyed?

Is that correct?

[hubert.reinterpretcast]

In D65667#1619105, @mclow.lists wrote:

Thanks for the clarification.

So the situation is:

• You register an atexit handler.
• During program termination, your handler is called, and you do something that creates an error_code object.
• You try to use the error_category of that error code, and discover that it has been destroyed?

Is that correct?

We did not hit the atexit handler case. We had a static lifetime object (the one returned by llvm::errs()) that holds an error_code (llvm::raw_fd_ostream::EC) as a member. The error_code is assigned to during the course of the program. When the destructor for the llvm::raw_fd_ostream is called, we discover that we can't use the error_category of the error code (because it has been destroyed).

[andusy]

I've updated the patch based on @EricWF's earlier comment.

In D65667#1613180, @EricWF wrote:

Or alternatively, there's an example of how to fix this in memory_resource.cpp https://github.com/llvm-mirror/libcxx/blob/master/src/experimental/memory_resource.cpp#L67-L96

[hubert.reinterpretcast]

In D65667#1617952, @thakis wrote:

To chime in from the sidelines: We strive to have zero code that runs before main in Chromium, so we're not excited about a patch that adds more such code to libc++. (In fact, we're hoping to contribute a build mode that removes the existing static initializer for cin/cout/cerr as well.)

I believe the new patch no longer adds more code that runs before main.

[mclow.lists]

libcxx/src/future.cpp

63

Please don't write code like this. It's hard to read, and it's easy to have the attribute get disconnected from the thing it's referring to.

Instead, define a new macro _LIBCPP_SAFE_STATIC_AFTER_CXX11 and use that.
It's clear what the intent is.

You have five #ifdefs in this patch.
I'd prefer to see only one ifdef, in <__config> where no one will ever look at it.

That's why we have macros like _LIBCPP_CONSTEXPR_AFTER_CXX11

[mclow.lists]

libcxx/src/system_error.cpp

202

Why do you overlay both of these in a single structure?

Is your intent that a program only calls either generic_category or system_category in any particular run of the program? Otherwise, don't you get UB?

[hubert.reinterpretcast]

libcxx/src/system_error.cpp

202

These aren't overlaid, but it does mean that there would be an unused instance of one of the types for each helper we create. We should probably avoid that.

[mclow.lists]

libcxx/src/system_error.cpp

202

Sorry, I misread the union/struct nesting. You are correct.

[andusy]

• Added definition for _LIBCPP_SAFE_STATIC_AFTER_CXX11 in <__config>.
• Split up GenericAndSystemErrorHelper to GenericErrorHelper and SystemErrorHelper.

[hubert.reinterpretcast]

libcxx/src/future.cpp

62–68

@mclow.lists, not sure if you meant to have this on one line; otherwise, I think all of the comments have been addressed.

[hubert.reinterpretcast]

I'm not seeing unresolved comments. This patch LGTM.

[EricWF]

Sorry for being late to the party, I've looked at the codege of this and other approachs. what we want is a single global than initializes all of objects we need. Right now we have function local static then may may not have make guards and that's less than ideal.

I'm having to help the original author fix this patch, I also have a verified intestine patch of my own.

[hubert.reinterpretcast]

In D65667#1642281, @EricWF wrote:

Sorry for being late to the party, I've looked at the codege of this and other approachs. what we want is a single global than initializes all of objects we need. Right now we have function local static then may may not have make guards and that's less than ideal.

In other words, we only want to pay the cost of the destructor registration once for the entire library? I think this might need a new file and is a much larger scope than what this patch is attempting. @EricWF, is this patch being subsumed into a bigger effort?

[EricWF]

In D65667#1643059, @hubert.reinterpretcast wrote:

In D65667#1642281, @EricWF wrote:

Sorry for being late to the party, I've looked at the codege of this and other approachs. what we want is a single global than initializes all of objects we need. Right now we have function local static then may may not have make guards and that's less than ideal.

In other words, we only want to pay the cost of the destructor registration once for the entire library? I think this might need a new file and is a much larger scope than what this patch is attempting. @EricWF, is this patch being subsumed into a bigger effort?

That doesn't need to happen now. But I want to see these changed to be global statics not function local. At least that way we omit the guards entirely. and accessing the variable becomes a single instruction.

[hubert.reinterpretcast]

In D65667#1643305, @EricWF wrote:

That doesn't need to happen now. But I want to see these changed to be global statics not function local. At least that way we omit the guards entirely. and accessing the variable becomes a single instruction.

This goes back to the "paying for something you don't use" with regards to the up-front library loading cost. If people are actually agreed on this course of action, then we can create that version of the patch, but it does not seem clear to me.

[cebowleratibm]

This patch has sat idle for 2+ years but I think the current patch is an improvement. Currently, the error_category singletons are implemented as static locals for which there is no guarantee that the destructors won't run prematurely. The proposed patch fixes that problem.

Perhaps it is desirable to coalesce these singletons to static globals in a single module but there appears to be contention on the thread as to whether or not that is desirable. The static local solution has the advantage that it does not cause initialization if it is unused, but the accessor functions will be slower. My suggestion is to commit this patch and if anyone feels it's important to move to file scope globals then that can be addressed in a future patch.

[• Quuxplusone]

libcxx/src/system_error.cpp

199–209

After rebasing, I would expect this would end up looking like

const error_category&
system_category() _NOEXCEPT
{
    union U {
        __system_error_category s;
        _LIBCPP_CONSTEXPR explicit U() : s() {}
        ~U() {}  // Too bad this can't be trivial... or can it?
    };
    static _LIBCPP_CONSTINIT U u;
    return u.s;
}

i.e. a very small diff against the left-hand side.

[cebowleratibm]

libcxx/src/system_error.cpp

199–209

I think the constexpr version will hit this error:

error: constexpr variable cannot have non-literal type 'const U'

[• Quuxplusone]

libcxx/src/system_error.cpp

199–209

There's no constexpr variable here, but you're right, the constexpr constructor hits

src/system_error.cpp:212:37: error: constexpr constructor never produces a constant expression [-Winvalid-constexpr]
         _LIBCPP_CONSTEXPR explicit U() : s() {}
                                    ^

That's also a problem with the current PR. (Maybe that Clang diagnostic didn't exist when the PR was originally made; but it does exist now; I just tried it locally.)

[hubert.reinterpretcast]

libcxx/src/system_error.cpp

199–209

We probably want to apply [[no_destroy]] in conjunction with the above. That is, guarantee the functionality with the above and suppress the registration of the destructor for compilers that support it.

[cebowleratibm]

I'd like to take over this patch and rebase a revision if there are no objections.

[cebowleratibm]

I was writing a revision of this patch but hit a problem:

class _LIBCPP_TYPE_VIS error_category
{
public:
    virtual ~error_category() _NOEXCEPT;

#if defined(_LIBCPP_BUILDING_LIBRARY) && \
    defined(_LIBCPP_DEPRECATED_ABI_LEGACY_LIBRARY_DEFINITIONS_FOR_INLINE_FUNCTIONS)
    error_category() _NOEXCEPT;
#else
    _LIBCPP_INLINE_VISIBILITY
    _LIBCPP_CONSTEXPR_AFTER_CXX11 error_category() _NOEXCEPT = default;
#endif
...

When _LIBCPP_DEPRECATED_ABI_LEGACY_LIBRARY_DEFINITIONS_FOR_INLINE_FUNCTIONS is defined the non-constexpr error_category constructor causes a problem with the _LIBCPP_CONSTINIT singleton objects:

llvm-project/libcxx/src/ios.cpp:59:25: error: constexpr constructor never produces a constant expression [-Winvalid-constexpr]
     constexpr explicit IostreamErrorHelper()

It seems we would need a new CONSTINIT macro that doesn't get defined when _LIBCPP_DEPRECATED_ABI_LEGACY_LIBRARY_DEFINITIONS_FOR_INLINE_FUNCTIONS is defined and when libc++ is stuck on these legacy definitions then the singleton ctor calls can't be bypassed. It's a shame given that we know the constructor doesn't do anything meaningful.

[cebowleratibm]

I updated the patch to use the previous union pattern to bypass the singleton destructors but added constinit, constexpr and attribute((no_destroy)) as possible to avoid the runtime overhead of using a static local.

Note I had to introduce some new macros in __config to guard application of constinit and constexpr because one of the base classes error_category does not provide a constexpr constructor according to:

#if defined(_LIBCPP_BUILDING_LIBRARY) && \
    defined(_LIBCPP_DEPRECATED_ABI_LEGACY_LIBRARY_DEFINITIONS_FOR_INLINE_FUNCTIONS)
    error_category() _NOEXCEPT;
...

[ldionne]

Thanks for reviving this! I have a few comments and I also created D123519 to try and simplify this patch. I think i'm fine with this once it is rebased onto D123519.

libcxx/include/__config

760–764 ↗

(On Diff #418912)

I think we can get rid of those definitions once D123519 lands.

libcxx/test/std/input.output/iostreams.base/std.ios.manip/error.reporting/iostream_category.pass.cpp

25

Can you make this:

struct A { same-as-before };
static A a;

I initially missed the fact that you were defining A *and* defining a at the same time. I think it's a bit easier to read if you make it explicit. This applies to other patches as well.

[cebowleratibm]

Revised after D123519.

I think _LIBCPP_ERROR_CATEGORY_DEFINE_LEGACY_INLINE_FUNCTIONS belongs in __config now, though I just followed suit with what was done in D123519. I need to preprocess out the constexpr/constinit singleton pattern in each of the derived error_category compilation units.

I chose to keep the _LIBCPP_NO_DESTROY and the union pattern even with _LIBCPP_ERROR_CATEGORY_DEFINE_LEGACY_INLINE_FUNCTIONS as it's still useful to bypass calling the destructor and bypass registering the destructor when _LIBCPP_NO_DESTROY is supported. Unfortunately there's no way to bypass the initialization runtime guard with the old ABI, but an improvement nonetheless and the ideal solution moving forward.

[ldionne]

libcxx/src/future.cpp

65

Is there a reason why you can't simply use the #else branch all the time, and never define _LIBCPP_ERROR_CATEGORY_DEFINE_LEGACY_INLINE_FUNCTIONS in this translation unit (the same applies to other changes)?

My memory is blurry from being on vacation, but I think the intent of my D123519 was that you could always assume that error_category is constexpr friendly when building the library.

[cebowleratibm]

libcxx/src/future.cpp

65

If that is the case then I think I'm misunderstanding how things work.

From __config:

#elif _LIBCPP_ABI_VERSION == 1
#  if !defined(_LIBCPP_OBJECT_FORMAT_COFF)
// Enable compiling copies of now inline methods into the dylib to support
// applications compiled against older libraries. This is unnecessary with
// COFF dllexport semantics, since dllexport forces a non-inline definition
// of inline functions to be emitted anyway. Our own non-inline copy would
// conflict with the dllexport-emitted copy, so we disable it.
#    define _LIBCPP_DEPRECATED_ABI_LEGACY_LIBRARY_DEFINITIONS_FOR_INLINE_FUNCTIONS
#  endif

so there are at least some library builds that will define _LIBCPP_DEPRECATED_ABI_LEGACY_LIBRARY_DEFINITIONS_FOR_INLINE_FUNCTIONS, and when it is, the definition of error_category will not have a constexpr constructor:

class _LIBCPP_TYPE_VIS error_category
{
public:
    virtual ~error_category() _NOEXCEPT;

#if defined(_LIBCPP_ERROR_CATEGORY_DEFINE_LEGACY_INLINE_FUNCTIONS)
    error_category() noexcept;
#else
    _LIBCPP_INLINE_VISIBILITY
    _LIBCPP_CONSTEXPR_AFTER_CXX11 error_category() _NOEXCEPT = default;
#endif

So the #if / #else I added are meant to be symmetric to the #if / #else in the error_category definition.

[ldionne]

Commandeering to explain what I mean -- sorry, I figured it was going to be easier since I had something specific in mind.

[ldionne]

Rebase and implement my suggestion.

[ldionne]

libcxx/src/ios.cpp

62

Actually, since those are constinit, I think we could even define them at file scope and skip the function-local static. Thoughts?

[hubert.reinterpretcast]

libcxx/src/ios.cpp

62

That makes sense. It does have an advantage in cases where _LIBCPP_NO_DESTROY is not effective.

[cebowleratibm]

libcxx/src/ios.cpp

62

If _LIBCPP_NO_DESTROY is not effective, doesn't that mean that the user will pay the startup to register the atexit destruction, even if the category singleton is never used? I think the benefit of the static local remains: that you're not paying for it unless it's used. If _LIBCPP_NO_DESTROY is supported then there's no benefit to using static locals. Are we ok imposing the load time cost when the library is built without _LIBCPP_NO_DESTROY support?

[hubert.reinterpretcast]

libcxx/src/ios.cpp

62

It's a trade-off between one-time startup cost versus dealing with the guard for every call.

[ldionne]

libcxx/src/ios.cpp

62

Turns out there's no guard anyway since they are constinit.

[ldionne]

Rebase and remove the union trick which I don't think is necessary anymore.

[ldionne]

I'll ship this if CI is green.

[ldionne]

Run clang-format on new .cpp file.

[ldionne]

Re-introduce the union trick which is needed to implement nodestroy semantics on compilers that don't support the nodestroy attribute.

While we're at it, drop the nodestroy attribute since the union trick is enough.

libcxx/src/CMakeLists.txt

 set(LIBCXX_LIB_CMAKEFILES_DIR "${CMAKE_CURRENT_BINARY_DIR}${CMAKE_FILES_DIRECTORY}"  PARENT_SCOPE)
 
 # Get sources
 set(LIBCXX_SOURCES
   algorithm.cpp
   any.cpp
   atomic.cpp
   barrier.cpp
   bind.cpp
   charconv.cpp
   chrono.cpp
   condition_variable.cpp
   condition_variable_destructor.cpp
+  error_category.cpp
   exception.cpp
   filesystem/filesystem_clock.cpp
   filesystem/filesystem_error.cpp
   filesystem/path_parser.h
   filesystem/path.cpp
   functional.cpp
   future.cpp
   hash.cpp

libcxx/src/error_category.cpp

+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include <__config>
+
+#ifdef _LIBCPP_DEPRECATED_ABI_LEGACY_LIBRARY_DEFINITIONS_FOR_INLINE_FUNCTIONS
+#  define _LIBCPP_ERROR_CATEGORY_DEFINE_LEGACY_INLINE_FUNCTIONS
+#endif
+
+#include <system_error>
+
+_LIBCPP_BEGIN_NAMESPACE_STD
+
+// class error_category
+
+#if defined(_LIBCPP_ERROR_CATEGORY_DEFINE_LEGACY_INLINE_FUNCTIONS)
+error_category::error_category() noexcept {}
+#endif
+
+error_category::~error_category() noexcept {}
+
+error_condition error_category::default_error_condition(int ev) const noexcept { return error_condition(ev, *this); }
+
+bool error_category::equivalent(int code, const error_condition& condition) const noexcept {
+  return default_error_condition(code) == condition;
+}
+
+bool error_category::equivalent(const error_code& code, int condition) const noexcept {
+  return *this == code.category() && code.value() == condition;
+}
+
+_LIBCPP_END_NAMESPACE_STD

libcxx/src/future.cpp

__future_error_category::message(int ev) const
     return string("unspecified future_errc value\n");
 }
 
 _LIBCPP_DIAGNOSTIC_POP
 
 const error_category&
 future_category() noexcept
 {
-    static __future_error_category __f;
-    return __f;
+    union AvoidDestroyingFutureCategory {
+        __future_error_category future_error_category;
+        constexpr explicit AvoidDestroyingFutureCategory() : future_error_category() {}
+        ~AvoidDestroyingFutureCategory() {}
+    };
+    constinit static AvoidDestroyingFutureCategory helper;
+    return helper.future_error_category;
 }
 
 future_error::future_error(error_code __ec)
     : logic_error(__ec.message()),
       __ec_(__ec)
 {
 }
 

libcxx/src/ios.cpp

#endif // _LIBCPP_ELAST
         )
         return __do_message::message(ev);
     return string("unspecified iostream_category error");
 }
 
 const error_category&
 iostream_category() noexcept
 {
-    static __iostream_category s;
-    return s;
+    union AvoidDestroyingIostreamCategory {
+        __iostream_category iostream_error_category;
+        constexpr explicit AvoidDestroyingIostreamCategory() : iostream_error_category() {}
+        ~AvoidDestroyingIostreamCategory() {}
+    };
+    constinit static AvoidDestroyingIostreamCategory helper;
+    return helper.iostream_error_category;
 }
 
 // ios_base::failure
 
 ios_base::failure::failure(const string& msg, const error_code& ec)
     : system_error(ec, msg)
 {
 }
 

libcxx/src/system_error.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
-#include <__config>
-#ifdef _LIBCPP_DEPRECATED_ABI_LEGACY_LIBRARY_DEFINITIONS_FOR_INLINE_FUNCTIONS
-#   define _LIBCPP_ERROR_CATEGORY_DEFINE_LEGACY_INLINE_FUNCTIONS
-#endif
-
 #include <__assert>
+#include <__config>
 #include <__verbose_abort>
 #include <cerrno>
 #include <cstdio>
 #include <cstdlib>
 #include <cstring>
 #include <string>
 #include <string.h>
 #include <system_error>
 
 #include "include/config_elast.h"
 
 #if defined(__ANDROID__)
 #include <android/api-level.h>
 #endif
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
-// class error_category
-
-#if defined(_LIBCPP_ERROR_CATEGORY_DEFINE_LEGACY_INLINE_FUNCTIONS)
-error_category::error_category() noexcept
-{
-}
-#endif
-
-error_category::~error_category() noexcept
-{
-}
-
-error_condition
-error_category::default_error_condition(int ev) const noexcept
-{
-    return error_condition(ev, *this);
-}
-
-bool
-error_category::equivalent(int code, const error_condition& condition) const noexcept
-{
-    return default_error_condition(code) == condition;
-}
-
-bool
-error_category::equivalent(const error_code& code, int condition) const noexcept
-{
-    return *this == code.category() && code.value() == condition;
-}
-
 namespace {
 #if !defined(_LIBCPP_HAS_NO_THREADS)
 
 //  GLIBC also uses 1024 as the maximum buffer size internally.
 constexpr size_t strerror_buff_size = 1024;
 
 string do_strerror_r(int ev);
 
if (ev > _LIBCPP_ELAST)
       return string("unspecified generic_category error");
 #endif // _LIBCPP_ELAST
     return __do_message::message(ev);
 }
 
 const error_category&
 generic_category() noexcept
 {
-    static __generic_error_category s;
-    return s;
+    union AvoidDestroyingGenericCategory {
+        __generic_error_category generic_error_category;
+        constexpr explicit AvoidDestroyingGenericCategory() : generic_error_category() {}
+        ~AvoidDestroyingGenericCategory() {}
+    };
+    constinit static AvoidDestroyingGenericCategory helper;
+    return helper.generic_error_category;
 }
 
 class _LIBCPP_HIDDEN __system_error_category
     : public __do_message
 {
 public:
     virtual const char* name() const noexcept;
     virtual string message(int ev) const;

 {
 #ifdef _LIBCPP_ELAST
     if (ev > _LIBCPP_ELAST)
       return error_condition(ev, system_category());
 #endif // _LIBCPP_ELAST
     return error_condition(ev, generic_category());
 }
 
 const error_category&
 system_category() noexcept
 {
-    static __system_error_category s;
-    return s;
+    union AvoidDestroyingSystemCategory {
+        __system_error_category system_error_category;
+        constexpr explicit AvoidDestroyingSystemCategory() : system_error_category() {}
+        ~AvoidDestroyingSystemCategory() {}
+    };
+    constinit static AvoidDestroyingSystemCategory helper;
+    return helper.system_error_category;
 }
 
 // error_condition
 
 string
 error_condition::message() const
 {
     return __cat_->message(__val_);
 }

libcxx/test/std/diagnostics/syserr/syserr.errcat/syserr.errcat.objects/generic_category.pass.cpp

 
 #include <system_error>
 #include <cassert>
 #include <string>
 #include <cerrno>
 
 #include "test_macros.h"
 
-void test_message_for_bad_value() {
+// See https://llvm.org/D65667
+struct StaticInit {
+    const std::error_category* ec;
+    ~StaticInit() {
+        std::string str = ec->name();
+        assert(str == "generic") ;
+    }
+};
+static StaticInit foo;
+
+int main(int, char**)
+{
+    {
+        const std::error_category& e_cat1 = std::generic_category();
+        std::string m1 = e_cat1.name();
+        assert(m1 == "generic");
+    }
+
+    // Test the result of message(int cond) when given a bad error condition
+    {
         errno = E2BIG; // something that message will never generate
         const std::error_category& e_cat1 = std::generic_category();
         const std::string msg = e_cat1.message(-1);
         // Exact message format varies by platform.
 #if defined(_AIX)
         LIBCPP_ASSERT(msg.rfind("Error -1 occurred", 0) == 0);
 #else
         LIBCPP_ASSERT(msg.rfind("Unknown error", 0) == 0);
 #endif
         assert(errno == E2BIG);
     }
 
-int main(int, char**)
-{
-    const std::error_category& e_cat1 = std::generic_category();
-    std::string m1 = e_cat1.name();
-    assert(m1 == "generic");
     {
-        test_message_for_bad_value();
+        foo.ec = &std::generic_category();
+        std::string m2 = foo.ec->name();
+        assert(m2 == "generic");
     }
 
     return 0;
 }

libcxx/test/std/diagnostics/syserr/syserr.errcat/syserr.errcat.objects/system_category.pass.cpp

 
 #include <system_error>
 #include <cassert>
 #include <string>
 #include <cerrno>
 
 #include "test_macros.h"
 
-void test_message_for_bad_value() {
+// See https://llvm.org/D65667
+struct StaticInit {
+    const std::error_category* ec;
+    ~StaticInit() {
+        std::string str = ec->name();
+        assert(str == "system") ;
+    }
+};
+static StaticInit foo;
+
+int main(int, char**)
+{
+    {
+        const std::error_category& e_cat1 = std::system_category();
+        std::error_condition e_cond = e_cat1.default_error_condition(5);
+        assert(e_cond.value() == 5);
+        assert(e_cond.category() == std::generic_category());
+        e_cond = e_cat1.default_error_condition(5000);
+        assert(e_cond.value() == 5000);
+        assert(e_cond.category() == std::system_category());
+    }
+
+    // Test the result of message(int cond) when given a bad error condition
+    {
             errno = E2BIG; // something that message will never generate
             const std::error_category& e_cat1 = std::system_category();
             const std::string msg = e_cat1.message(-1);
             // Exact message format varies by platform.
 #if defined(_AIX)
             LIBCPP_ASSERT(msg.rfind("Error -1 occurred", 0) == 0);
 #else
             LIBCPP_ASSERT(msg.rfind("Unknown error", 0) == 0);
 #endif
             assert(errno == E2BIG);
     }
 
-int main(int, char**)
-{
-    const std::error_category& e_cat1 = std::system_category();
-    std::error_condition e_cond = e_cat1.default_error_condition(5);
-    assert(e_cond.value() == 5);
-    assert(e_cond.category() == std::generic_category());
-    e_cond = e_cat1.default_error_condition(5000);
-    assert(e_cond.value() == 5000);
-    assert(e_cond.category() == std::system_category());
     {
-        test_message_for_bad_value();
+        foo.ec = &std::system_category();
+        std::string m = foo.ec->name();
+        assert(m == "system");
     }
 
     return 0;
 }

libcxx/test/std/input.output/iostreams.base/std.ios.manip/error.reporting/iostream_category.pass.cpp

 // const error_category& iostream_category();
 
 #include <ios>
 #include <cassert>
 #include <string>
 
 #include "test_macros.h"
 
+// See https://llvm.org/D65667
+struct StaticInit {
+    const std::error_category* ec;
+    ~StaticInit() {
+        std::string str = ec->name();
+        assert(str == "iostream") ;
+    }
+};
+static StaticInit foo;
+
 int main(int, char**)
 {
+    {
         const std::error_category& e_cat1 = std::iostream_category();
         std::string m1 = e_cat1.name();
         assert(m1 == "iostream");
+    }
+
+    {
+        foo.ec = &std::iostream_category();
+        std::string m2 = foo.ec->name();
+        assert(m2 == "iostream");
+    }
 
     return 0;
 }

libcxx/test/std/thread/futures/futures.errors/future_category.pass.cpp

 // const error_category& future_category();
 
 #include <future>
 #include <cstring>
 #include <cassert>
 
 #include "test_macros.h"
 
+// See https://llvm.org/D65667
+struct StaticInit {
+    const std::error_category* ec;
+    ~StaticInit() {
+        assert(std::strcmp(ec->name(), "future") == 0);
+    }
+};
+static StaticInit foo;
+
 int main(int, char**)
 {
+    {
         const std::error_category& ec = std::future_category();
         assert(std::strcmp(ec.name(), "future") == 0);
+    }
+
+    {
+        foo.ec = &std::future_category();
+        assert(std::strcmp(foo.ec->name(), "future") == 0);
+    }
 
     return 0;
 }