[libcxx-commits] [libcxx] fbdf684 - [libc++] Avoid destructor call for error_category singletons

Louis Dionne via libcxx-commits libcxx-commits at lists.llvm.org
Tue Sep 5 10:45:04 PDT 2023


Author: Chris Bowler
Date: 2023-09-05T13:44:10-04:00
New Revision: fbdf684fae5243e7a9ff50dd4abdc5b55e6aa895

URL: https://github.com/llvm/llvm-project/commit/fbdf684fae5243e7a9ff50dd4abdc5b55e6aa895
DIFF: https://github.com/llvm/llvm-project/commit/fbdf684fae5243e7a9ff50dd4abdc5b55e6aa895.diff

LOG: [libc++] Avoid destructor call for error_category singletons

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.

Differential Revision: https://reviews.llvm.org/D65667

Co-authored-by: Louis Dionne <ldionne.2 at gmail.com>

Added: 
    libcxx/src/error_category.cpp

Modified: 
    libcxx/src/CMakeLists.txt
    libcxx/src/future.cpp
    libcxx/src/ios.cpp
    libcxx/src/system_error.cpp
    libcxx/test/std/diagnostics/syserr/syserr.errcat/syserr.errcat.objects/generic_category.pass.cpp
    libcxx/test/std/diagnostics/syserr/syserr.errcat/syserr.errcat.objects/system_category.pass.cpp
    libcxx/test/std/input.output/iostreams.base/std.ios.manip/error.reporting/iostream_category.pass.cpp
    libcxx/test/std/thread/futures/futures.errors/future_category.pass.cpp

Removed: 
    


################################################################################
diff  --git a/libcxx/src/CMakeLists.txt b/libcxx/src/CMakeLists.txt
index 35b4665270960d9..daaa9df2600b29f 100644
--- a/libcxx/src/CMakeLists.txt
+++ b/libcxx/src/CMakeLists.txt
@@ -11,6 +11,7 @@ set(LIBCXX_SOURCES
   chrono.cpp
   condition_variable.cpp
   condition_variable_destructor.cpp
+  error_category.cpp
   exception.cpp
   filesystem/filesystem_clock.cpp
   filesystem/filesystem_error.cpp

diff  --git a/libcxx/src/error_category.cpp b/libcxx/src/error_category.cpp
new file mode 100644
index 000000000000000..8ae460fb5f1f4f2
--- /dev/null
+++ b/libcxx/src/error_category.cpp
@@ -0,0 +1,37 @@
+//===----------------------------------------------------------------------===//
+//
+// 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

diff  --git a/libcxx/src/future.cpp b/libcxx/src/future.cpp
index f8f466bd5a21cb2..fa1004e13571287 100644
--- a/libcxx/src/future.cpp
+++ b/libcxx/src/future.cpp
@@ -59,8 +59,13 @@ _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)

diff  --git a/libcxx/src/ios.cpp b/libcxx/src/ios.cpp
index bb62eb9948758a2..f17cd6e1dbbb14e 100644
--- a/libcxx/src/ios.cpp
+++ b/libcxx/src/ios.cpp
@@ -52,8 +52,13 @@ __iostream_category::message(int ev) const
 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

diff  --git a/libcxx/src/system_error.cpp b/libcxx/src/system_error.cpp
index 670375c73ca8acd..f187090f75b6ce9 100644
--- a/libcxx/src/system_error.cpp
+++ b/libcxx/src/system_error.cpp
@@ -6,12 +6,8 @@
 //
 //===----------------------------------------------------------------------===//
 
-#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>
@@ -29,36 +25,6 @@
 
 _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)
 
@@ -186,8 +152,13 @@ __generic_error_category::message(int ev) const
 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
@@ -228,8 +199,13 @@ __system_error_category::default_error_condition(int ev) const noexcept
 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

diff  --git a/libcxx/test/std/diagnostics/syserr/syserr.errcat/syserr.errcat.objects/generic_category.pass.cpp b/libcxx/test/std/diagnostics/syserr/syserr.errcat/syserr.errcat.objects/generic_category.pass.cpp
index 84f209c8ce87953..90e5bd39e5b016d 100644
--- a/libcxx/test/std/diagnostics/syserr/syserr.errcat/syserr.errcat.objects/generic_category.pass.cpp
+++ b/libcxx/test/std/diagnostics/syserr/syserr.errcat/syserr.errcat.objects/generic_category.pass.cpp
@@ -22,27 +22,43 @@
 
 #include "test_macros.h"
 
-void test_message_for_bad_value() {
-    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.
+// 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);
+        LIBCPP_ASSERT(msg.rfind("Error -1 occurred", 0) == 0);
 #else
-    LIBCPP_ASSERT(msg.rfind("Unknown error", 0) == 0);
+        LIBCPP_ASSERT(msg.rfind("Unknown error", 0) == 0);
 #endif
-    assert(errno == E2BIG);
-}
+        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;
+    return 0;
 }

diff  --git a/libcxx/test/std/diagnostics/syserr/syserr.errcat/syserr.errcat.objects/system_category.pass.cpp b/libcxx/test/std/diagnostics/syserr/syserr.errcat/syserr.errcat.objects/system_category.pass.cpp
index 7b15da3cd6d17b0..1de1b59e5c6c3d6 100644
--- a/libcxx/test/std/diagnostics/syserr/syserr.errcat/syserr.errcat.objects/system_category.pass.cpp
+++ b/libcxx/test/std/diagnostics/syserr/syserr.errcat/syserr.errcat.objects/system_category.pass.cpp
@@ -22,31 +22,47 @@
 
 #include "test_macros.h"
 
-void test_message_for_bad_value() {
-    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.
+// 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);
+            LIBCPP_ASSERT(msg.rfind("Error -1 occurred", 0) == 0);
 #else
-    LIBCPP_ASSERT(msg.rfind("Unknown error", 0) == 0);
+            LIBCPP_ASSERT(msg.rfind("Unknown error", 0) == 0);
 #endif
-    assert(errno == E2BIG);
-}
+            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;
+    return 0;
 }

diff  --git a/libcxx/test/std/input.output/iostreams.base/std.ios.manip/error.reporting/iostream_category.pass.cpp b/libcxx/test/std/input.output/iostreams.base/std.ios.manip/error.reporting/iostream_category.pass.cpp
index 62e52ab3f9b55e5..f2612dea16db070 100644
--- a/libcxx/test/std/input.output/iostreams.base/std.ios.manip/error.reporting/iostream_category.pass.cpp
+++ b/libcxx/test/std/input.output/iostreams.base/std.ios.manip/error.reporting/iostream_category.pass.cpp
@@ -16,11 +16,29 @@
 
 #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");
+    {
+        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;
+    return 0;
 }

diff  --git a/libcxx/test/std/thread/futures/futures.errors/future_category.pass.cpp b/libcxx/test/std/thread/futures/futures.errors/future_category.pass.cpp
index 4c8f9da6c2bfc36..bb86837afb69408 100644
--- a/libcxx/test/std/thread/futures/futures.errors/future_category.pass.cpp
+++ b/libcxx/test/std/thread/futures/futures.errors/future_category.pass.cpp
@@ -18,10 +18,26 @@
 
 #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);
+    {
+        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;
+    return 0;
 }


        


More information about the libcxx-commits mailing list