Reworked function pointers use and definitions.

Instead of using atomic<> to access global function pointers, use raw pointers and atomic_ref to access them safely in multi-threaded builds. This allows to ensure constant initialization of the function pointers, even in C++03. This also solves the problem of undefined dynamic initialization order that we previously tried to solve with the init_priority attribute. The attribute turns out to not work if the pointers were raw pointers (in single-threaded builds). It is also not supported by Intel Compiler and possibly other, which required us to avoid using the function pointer for fill_random. The resulting code should be simpler and more portable. In order to check for C++20 std::atomic_ref availability, the older check for <atomic> header was replaced with a check for std::atomic_ref. If not available, we're using Boost.Atomic, as before.
2026-01-19 04:12:09 +00:00 · 2021-06-14 22:09:15 +03:00
parent 08e7a20785
commit 2dda038306
9 changed files with 98 additions and 60 deletions
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -22,6 +22,7 @@ set(BOOST_FILESYSTEM_DISABLE_BCRYPT OFF CACHE BOOL "Disable usage of BCrypt API

 set(CMAKE_REQUIRED_INCLUDES "${CMAKE_CURRENT_SOURCE_DIR}/src" "${CMAKE_CURRENT_SOURCE_DIR}/../..")

+check_cxx_source_compiles("#include <${CMAKE_CURRENT_SOURCE_DIR}/config/has_cxx20_atomic_ref.cpp>" BOOST_FILESYSTEM_HAS_CXX20_ATOMIC_REF)
 check_cxx_source_compiles("#include <${CMAKE_CURRENT_SOURCE_DIR}/config/has_stat_st_blksize.cpp>" BOOST_FILESYSTEM_HAS_STAT_ST_BLKSIZE)
 check_cxx_source_compiles("#include <${CMAKE_CURRENT_SOURCE_DIR}/config/has_stat_st_mtim.cpp>" BOOST_FILESYSTEM_HAS_STAT_ST_MTIM)
 check_cxx_source_compiles("#include <${CMAKE_CURRENT_SOURCE_DIR}/config/has_stat_st_mtimensec.cpp>" BOOST_FILESYSTEM_HAS_STAT_ST_MTIMENSEC)
@@ -43,12 +44,6 @@ endif()

 unset(CMAKE_REQUIRED_INCLUDES)

-set(CMAKE_REQUIRED_INCLUDES "${CMAKE_CURRENT_SOURCE_DIR}/../..")
-set(CMAKE_REQUIRED_DEFINITIONS "-DTEST_BOOST_NO_CXX11_HDR_ATOMIC")
-check_cxx_source_compiles("#include <${CMAKE_CURRENT_SOURCE_DIR}/../config/checks/test_case.cpp>" BOOST_FILESYSTEM_HAS_CXX11_HDR_ATOMIC)
-unset(CMAKE_REQUIRED_DEFINITIONS)
-unset(CMAKE_REQUIRED_INCLUDES)
-
 set(BOOST_FILESYSTEM_SOURCES
    src/codecvt_error_category.cpp
    src/exception.cpp
@@ -153,7 +148,8 @@ target_link_libraries(boost_filesystem
        Boost::predef
 )

-if(NOT BOOST_FILESYSTEM_HAS_CXX11_HDR_ATOMIC)
+if(NOT BOOST_FILESYSTEM_HAS_CXX20_ATOMIC_REF)
+    target_compile_definitions(boost_filesystem PRIVATE BOOST_FILESYSTEM_NO_CXX20_ATOMIC_REF)
    target_link_libraries(boost_filesystem PRIVATE Boost::atomic)
 endif()

--- a/build/Jamfile.v2
+++ b/build/Jamfile.v2
@@ -79,16 +79,17 @@ rule check-statx ( properties * )
    return $(result) ;
 }

-# The rule checks if the standard <atomic> header is supported
-rule check-cxx11-hdr-atomic ( properties * )
+# The rule checks if std::atomic_ref is supported
+rule check-cxx20-atomic-ref ( properties * )
 {
    local result ;

    if ! <threading>single in $(properties)
    {
-        if ! [ configure.builds ../../config/checks//cxx11_hdr_atomic : $(properties) : "has <atomic> header" ]
+        if ! [ configure.builds ../config//has_cxx20_atomic_ref : $(properties) : "has std::atomic_ref" ]
        {
-            result = <library>/boost/atomic//boost_atomic ;
+            result = <define>BOOST_FILESYSTEM_NO_CXX20_ATOMIC_REF ;
+            result += <library>/boost/atomic//boost_atomic ;
        }
    }
    else
@@ -112,7 +113,7 @@ project boost/filesystem
      [ check-target-builds ../config//has_stat_st_birthtimespec "has stat::st_birthtimespec" : <define>BOOST_FILESYSTEM_HAS_STAT_ST_BIRTHTIMESPEC ]
      <conditional>@check-statx
      <conditional>@select-windows-crypto-api
-      <conditional>@check-cxx11-hdr-atomic
+      <conditional>@check-cxx20-atomic-ref
      <target-os>windows:<define>BOOST_USE_WINDOWS_H
      <target-os>windows:<define>WIN32_LEAN_AND_MEAN
      <target-os>windows:<define>NOMINMAX
--- a/config/Jamfile.v2
+++ b/config/Jamfile.v2
@@ -5,6 +5,8 @@
 #          http://www.boost.org/LICENSE_1_0.txt)
 #

+obj has_cxx20_atomic_ref : has_cxx20_atomic_ref.cpp : <include>../src ;
+explicit has_cxx20_atomic_ref ;
 obj has_stat_st_blksize : has_stat_st_blksize.cpp : <include>../src ;
 explicit has_stat_st_blksize ;
 obj has_stat_st_mtim : has_stat_st_mtim.cpp : <include>../src ;
--- a/config/has_cxx20_atomic_ref.cpp
+++ b/config/has_cxx20_atomic_ref.cpp
@@ -0,0 +1,19 @@
+//  Copyright 2021 Andrey Semashev
+
+//  Distributed under the Boost Software License, Version 1.0.
+//  See http://www.boost.org/LICENSE_1_0.txt
+
+//  See library home page at http://www.boost.org/libs/filesystem
+
+#include <atomic>
+
+typedef void func_t();
+
+int main()
+{
+    func_t* func = 0;
+    std::atomic_ref< func_t* > ref(func);
+    ref.load(std::memory_order_relaxed);
+    ref.store((func_t*)0, std::memory_order_relaxed);
+    return 0;
+}
--- a/src/atomic_ref.hpp
+++ b/src/atomic_ref.hpp
@@ -9,24 +9,24 @@

 //--------------------------------------------------------------------------------------//

-#ifndef BOOST_FILESYSTEM_SRC_ATOMIC_HPP_
-#define BOOST_FILESYSTEM_SRC_ATOMIC_HPP_
+#ifndef BOOST_FILESYSTEM_SRC_ATOMIC_REF_HPP_
+#define BOOST_FILESYSTEM_SRC_ATOMIC_REF_HPP_

 #include <boost/filesystem/config.hpp>

-#if !defined(BOOST_NO_CXX11_HDR_ATOMIC)
+#if !defined(BOOST_FILESYSTEM_NO_CXX20_ATOMIC_REF)

 #include <atomic>

 namespace atomic_ns = std;

-#else // !defined(BOOST_NO_CXX11_HDR_ATOMIC)
+#else // !defined(BOOST_FILESYSTEM_NO_CXX20_ATOMIC_REF)

 #include <boost/memory_order.hpp>
-#include <boost/atomic/atomic.hpp>
+#include <boost/atomic/atomic_ref.hpp>

 namespace atomic_ns = boost;

-#endif // !defined(BOOST_NO_CXX11_HDR_ATOMIC)
+#endif // !defined(BOOST_FILESYSTEM_NO_CXX20_ATOMIC_REF)

-#endif // BOOST_FILESYSTEM_SRC_ATOMIC_HPP_
+#endif // BOOST_FILESYSTEM_SRC_ATOMIC_REF_HPP_
--- a/src/atomic_tools.hpp
+++ b/src/atomic_tools.hpp
@@ -16,17 +16,53 @@

 #if !defined(BOOST_FILESYSTEM_SINGLE_THREADED)

-#include "atomic.hpp"
+#include "atomic_ref.hpp"

-#define BOOST_FILESYSTEM_ATOMIC_VAR(type, name, init) atomic_ns::atomic< type > name(init)
-#define BOOST_FILESYSTEM_ATOMIC_LOAD_RELAXED(a) a.load(atomic_ns::memory_order_relaxed)
-#define BOOST_FILESYSTEM_ATOMIC_STORE_RELAXED(a, x) a.store(x, atomic_ns::memory_order_relaxed)
+namespace boost {
+namespace filesystem {
+namespace detail {
+
+//! Atomically loads the value
+template< typename T >
+BOOST_FORCEINLINE T atomic_load_relaxed(T& a)
+{
+    return atomic_ns::atomic_ref< T >(a).load(atomic_ns::memory_order_relaxed);
+}
+
+//! Atomically stores the value
+template< typename T >
+BOOST_FORCEINLINE void atomic_store_relaxed(T& a, T val)
+{
+    atomic_ns::atomic_ref< T >(a).store(val, atomic_ns::memory_order_relaxed);
+}
+
+} // namespace detail
+} // namespace filesystem
+} // namespace boost

 #else // !defined(BOOST_FILESYSTEM_SINGLE_THREADED)

-#define BOOST_FILESYSTEM_ATOMIC_VAR(type, name, init) type name = init
-#define BOOST_FILESYSTEM_ATOMIC_LOAD_RELAXED(a) a
-#define BOOST_FILESYSTEM_ATOMIC_STORE_RELAXED(a, x) a = x
+namespace boost {
+namespace filesystem {
+namespace detail {
+
+//! Atomically loads the value
+template< typename T >
+BOOST_FORCEINLINE T atomic_load_relaxed(T const& a)
+{
+    return a;
+}
+
+//! Atomically stores the value
+template< typename T >
+BOOST_FORCEINLINE void atomic_store_relaxed(T& a, T val)
+{
+    a = val;
+}
+
+} // namespace detail
+} // namespace filesystem
+} // namespace boost

 #endif // !defined(BOOST_FILESYSTEM_SINGLE_THREADED)

--- a/src/operations.cpp
+++ b/src/operations.cpp
@@ -506,12 +506,11 @@ int statx_fstatat(int dirfd, const char* path, int flags, unsigned int mask, str
 typedef int statx_t(int dirfd, const char* path, int flags, unsigned int mask, struct ::statx* stx);

 //! Pointer to the actual implementation of the statx implementation
-BOOST_FILESYSTEM_INIT_PRIORITY(BOOST_FILESYSTEM_FUNC_PTR_INIT_PRIORITY)
-BOOST_FILESYSTEM_ATOMIC_VAR(statx_t*, statx_ptr, &statx_fstatat);
+statx_t* statx_ptr = &statx_fstatat;

 inline int invoke_statx(int dirfd, const char* path, int flags, unsigned int mask, struct ::statx* stx) BOOST_NOEXCEPT
 {
-    return BOOST_FILESYSTEM_ATOMIC_LOAD_RELAXED(statx_ptr)(dirfd, path, flags, mask, stx);
+    return filesystem::detail::atomic_load_relaxed(statx_ptr)(dirfd, path, flags, mask, stx);
 }

 //! A wrapper for the statx syscall. Disable MSAN since at least on clang 10 it doesn't
@@ -525,7 +524,7 @@ int statx_syscall(int dirfd, const char* path, int flags, unsigned int mask, str
        const int err = errno;
        if (BOOST_UNLIKELY(err == ENOSYS))
        {
-            BOOST_FILESYSTEM_ATOMIC_STORE_RELAXED(statx_ptr, &statx_fstatat);
+            filesystem::detail::atomic_store_relaxed(statx_ptr, &statx_fstatat);
            return statx_fstatat(dirfd, path, flags, mask, stx);
        }
    }
@@ -545,7 +544,7 @@ inline void init_statx_impl(unsigned int major_ver, unsigned int minor_ver, unsi
    if (major_ver > 4u || (major_ver == 4u && minor_ver >= 11u))
        stx = &statx_syscall;

-    BOOST_FILESYSTEM_ATOMIC_STORE_RELAXED(statx_ptr, stx);
+    filesystem::detail::atomic_store_relaxed(statx_ptr, stx);
 #endif // !defined(BOOST_FILESYSTEM_HAS_STATX) && defined(BOOST_FILESYSTEM_HAS_STATX_SYSCALL)
 }

@@ -748,8 +747,7 @@ int copy_file_data_read_write(int infile, int outfile, uintmax_t size, std::size
 typedef int copy_file_data_t(int infile, int outfile, uintmax_t size, std::size_t blksize);

 //! Pointer to the actual implementation of the copy_file_data implementation
-BOOST_FILESYSTEM_INIT_PRIORITY(BOOST_FILESYSTEM_FUNC_PTR_INIT_PRIORITY)
-BOOST_FILESYSTEM_ATOMIC_VAR(copy_file_data_t*, copy_file_data, &copy_file_data_read_write);
+copy_file_data_t* copy_file_data = &copy_file_data_read_write;

 #if defined(BOOST_FILESYSTEM_USE_SENDFILE)

@@ -783,7 +781,7 @@ int copy_file_data_sendfile(int infile, int outfile, uintmax_t size, std::size_t

                if (err == ENOSYS)
                {
-                    BOOST_FILESYSTEM_ATOMIC_STORE_RELAXED(copy_file_data, &copy_file_data_read_write);
+                    filesystem::detail::atomic_store_relaxed(copy_file_data, &copy_file_data_read_write);
                    goto fallback_to_read_write;
                }
            }
@@ -851,10 +849,10 @@ int copy_file_data_copy_file_range(int infile, int outfile, uintmax_t size, std:
                if (err == ENOSYS)
                {
 #if defined(BOOST_FILESYSTEM_USE_SENDFILE)
-                    BOOST_FILESYSTEM_ATOMIC_STORE_RELAXED(copy_file_data, &copy_file_data_sendfile);
+                    filesystem::detail::atomic_store_relaxed(copy_file_data, &copy_file_data_sendfile);
                    goto fallback_to_sendfile;
 #else
-                    BOOST_FILESYSTEM_ATOMIC_STORE_RELAXED(copy_file_data, &copy_file_data_read_write);
+                    filesystem::detail::atomic_store_relaxed(copy_file_data, &copy_file_data_read_write);
                    goto fallback_to_read_write;
 #endif
                }
@@ -934,7 +932,7 @@ inline void init_copy_file_data_impl(unsigned int major_ver, unsigned int minor_
        cfd = &check_fs_type< &copy_file_data_copy_file_range >;
 #endif

-    BOOST_FILESYSTEM_ATOMIC_STORE_RELAXED(copy_file_data, cfd);
+    filesystem::detail::atomic_store_relaxed(copy_file_data, cfd);
 #endif // defined(BOOST_FILESYSTEM_USE_SENDFILE) || defined(BOOST_FILESYSTEM_USE_COPY_FILE_RANGE)
 }

@@ -961,7 +959,7 @@ struct syscall_initializer
    }
 };

-BOOST_FILESYSTEM_INIT_PRIORITY(BOOST_FILESYSTEM_FUNC_PTR_INIT_INIT_PRIORITY)
+BOOST_FILESYSTEM_INIT_PRIORITY(BOOST_FILESYSTEM_FUNC_PTR_INIT_PRIORITY)
 const syscall_initializer syscall_init;

 #endif // defined(linux) || defined(__linux) || defined(__linux__)
@@ -1901,7 +1899,7 @@ bool copy_file(path const& from, path const& to, unsigned int options, error_cod
    }

    // Note: Use block size of the target file since it is most important for writing performance.
-    err = BOOST_FILESYSTEM_ATOMIC_LOAD_RELAXED(detail::copy_file_data)(infile.fd, outfile.fd, get_size(from_stat), get_blksize(to_stat));
+    err = filesystem::detail::atomic_load_relaxed(filesystem::detail::copy_file_data)(infile.fd, outfile.fd, get_size(from_stat), get_blksize(to_stat));
    if (BOOST_UNLIKELY(err != 0))
        goto fail; // err already contains the error code

--- a/src/private_config.hpp
+++ b/src/private_config.hpp
@@ -29,13 +29,11 @@
 #endif

 // According to https://gcc.gnu.org/bugzilla//show_bug.cgi?id=65115,
-// the default C++ object initializers priority is 65535. We want to
+// the default C++ object initializers priority is 65535. We would like to
 // initialize function pointers earlier than that (with lower priority values),
-// and construct the function pointers initializer after that but preferably
-// before the other global objects initializers. Other than this,
+// before the other global objects initializers are run. Other than this,
 // these priority values are arbitrary.
-#define BOOST_FILESYSTEM_FUNC_PTR_INIT_PRIORITY 32766
-#define BOOST_FILESYSTEM_FUNC_PTR_INIT_INIT_PRIORITY 32767
+#define BOOST_FILESYSTEM_FUNC_PTR_INIT_PRIORITY 32767

 #if defined(__has_feature) && defined(__has_attribute)
 #if __has_feature(memory_sanitizer) && __has_attribute(no_sanitize)
--- a/src/unique_path.cpp
+++ b/src/unique_path.cpp
@@ -166,14 +166,8 @@ int fill_random_dev_random(void* buf, std::size_t len)

 typedef int fill_random_t(void* buf, std::size_t len);

-#if defined(BOOST_FILESYSTEM_HAS_INIT_PRIORITY)
 //! Pointer to the implementation of fill_random.
-//! Since the variable may be a boost::atomic with a non-constexpr constructor, dynamic initialization may be happening.
-//! To avoid dynamic initialization order issues, initialization priority must be supported by the compiler so that
-//! this pointer is initialized before the syscall initializer in operations.cpp.
-BOOST_FILESYSTEM_INIT_PRIORITY(BOOST_FILESYSTEM_FUNC_PTR_INIT_PRIORITY)
-BOOST_FILESYSTEM_ATOMIC_VAR(fill_random_t*, fill_random, &fill_random_dev_random);
-#endif // defined(BOOST_FILESYSTEM_HAS_INIT_PRIORITY)
+fill_random_t* fill_random = &fill_random_dev_random;

 //! Fills buffer with cryptographically random data obtained from getrandom()
 int fill_random_getrandom(void* buf, std::size_t len)
@@ -194,9 +188,7 @@ int fill_random_getrandom(void* buf, std::size_t len)

            if (err == ENOSYS && bytes_read == 0u)
            {
-#if defined(BOOST_FILESYSTEM_HAS_INIT_PRIORITY)
-                BOOST_FILESYSTEM_ATOMIC_STORE_RELAXED(fill_random, &fill_random_dev_random);
-#endif
+                filesystem::detail::atomic_store_relaxed(fill_random, &fill_random_dev_random);
                return fill_random_dev_random(buf, len);
            }

@@ -220,11 +212,7 @@ void system_crypt_random(void* buf, std::size_t len, boost::system::error_code*

 #if defined(BOOST_FILESYSTEM_HAS_GETRANDOM) || defined(BOOST_FILESYSTEM_HAS_GETRANDOM_SYSCALL)

-#if defined(BOOST_FILESYSTEM_HAS_INIT_PRIORITY)
-    int err = BOOST_FILESYSTEM_ATOMIC_LOAD_RELAXED(fill_random)(buf, len);
-#else
-    int err = fill_random_getrandom(buf, len);
-#endif
+    int err = filesystem::detail::atomic_load_relaxed(fill_random)(buf, len);
    if (BOOST_UNLIKELY(err != 0))
        emit_error(err, ec, "boost::filesystem::unique_path");

@@ -310,7 +298,7 @@ void init_fill_random_impl(unsigned int major_ver, unsigned int minor_ver, unsig
    if (major_ver > 3u || (major_ver == 3u && minor_ver >= 17u))
        fr = &fill_random_getrandom;

-    BOOST_FILESYSTEM_ATOMIC_STORE_RELAXED(fill_random, fr);
+    filesystem::detail::atomic_store_relaxed(fill_random, fr);
 #endif
 }