diff --git a/.github/workflows/gh-actions.yml b/.github/workflows/gh-actions.yml index 512d9e0..844f08e 100644 --- a/.github/workflows/gh-actions.yml +++ b/.github/workflows/gh-actions.yml @@ -10,19 +10,31 @@ jobs: build: name: ${{ matrix.config.name }} — ${{ matrix.build_type }} runs-on: ${{ matrix.config.os }} - timeout-minutes: 20 + container: ${{ matrix.config.container || '' }} + timeout-minutes: 25 strategy: fail-fast: false matrix: config: - - { os: ubuntu-latest, name: Clang, cc: clang, cxx: clang++, gen: '-G Ninja' } - - { os: macos-26, name: Apple-Clang, cc: clang, cxx: clang++, gen: '-G Ninja' } + - { os: ubuntu-latest, name: Clang, cc: clang, cxx: clang++, gen: '-G Ninja' } + - { os: ubuntu-latest, name: GCC, cc: gcc, cxx: g++, gen: '-G Ninja', container: 'gcc:16' } + - { os: macos-26, name: Apple-Clang, cc: clang, cxx: clang++, gen: '-G Ninja' } + # windows-2025 carries Visual Studio 2026 (runner-images migration, + # June 2026), so the default generator is the VS 2026 one. + - { os: windows-2025, name: MSVC, gen: '-A x64' } + - { os: windows-2025, name: Clang-CL, gen: '-A x64 -T ClangCL' } build_type: [Debug, Release] steps: - uses: actions/checkout@v4 + - name: Install build tools (GCC container) + if: matrix.config.name == 'GCC' + run: | + apt-get update + apt-get install -y --no-install-recommends cmake ninja-build git ca-certificates + - name: Install LLVM/Clang 22 (Linux) if: matrix.config.name == 'Clang' run: | @@ -32,7 +44,12 @@ jobs: echo "CC=/usr/lib/llvm-22/bin/clang" >> $GITHUB_ENV echo "CXX=/usr/lib/llvm-22/bin/clang++" >> $GITHUB_ENV - - name: Configure + # The Windows generators are multi-config and pick their own toolset + # (-T), so they take neither an explicit compiler nor CMAKE_BUILD_TYPE — + # the configuration is chosen at build/test time via --config / -C. + - name: Configure (Unix) + if: runner.os != 'Windows' + shell: bash run: | cmake -B build ${{ matrix.config.gen }} \ -DCMAKE_CXX_COMPILER="${CXX:-${{ matrix.config.cxx }}}" \ @@ -40,7 +57,12 @@ jobs: -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} \ -S . + - name: Configure (Windows) + if: runner.os == 'Windows' + run: cmake -B build ${{ matrix.config.gen }} -S . + - name: Build & test + shell: bash run: | cmake --build build --config ${{ matrix.build_type }} -j cd build/test diff --git a/CMakeLists.txt b/CMakeLists.txt index 399d996..09d3e78 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -47,6 +47,15 @@ endif() include( ${build_SOURCE_DIR}/build_options.cmake ) set( CMAKE_CXX_STANDARD 26 ) +if ( CMAKE_CXX_COMPILER_ID STREQUAL "MSVC" ) + # CMake has no cxx_std_26 mapping for MSVC: CMAKE_CXX_STANDARD alone is + # silently DROPPED (the compile then fails far away, complaining that + # nested namespace definitions need /std:c++17), and asking for + # CMAKE_CXX_STANDARD_REQUIRED turns that into a hard configure error + # instead. Request the dialect explicitly. Clang-CL reports "Clang" and + # needs none of this - CMake knows its flags. + add_compile_options( /std:c++latest ) +endif() add_compile_definitions( BOOST_ALL_NO_LIB ) if ( WIN32 ) add_compile_definitions( WIN32_LEAN_AND_MEAN NOMINMAX ) diff --git a/include/psi/functionoid/detail/callable_base.hpp b/include/psi/functionoid/detail/callable_base.hpp index b8f908b..c57396b 100644 --- a/include/psi/functionoid/detail/callable_base.hpp +++ b/include/psi/functionoid/detail/callable_base.hpp @@ -38,6 +38,21 @@ namespace boost template class reference_wrapper; } //------------------------------------------------------------------------------ + +//------------------------------------------------------------------------------ +// Scoped to GCC 16, the only release observed to need it, so the workaround +// expires by itself: a later GCC compiles the annotation away, and if the +// miscompile is still there the GCC lane says so loudly rather than the +// library quietly carrying a pessimization forever. (clang reports +// __GNUC__ == 4, so it is excluded by the version test alone - the explicit +// guard is for compilers that impersonate a newer GCC.) +#if defined( __GNUC__ ) && ( __GNUC__ == 16 ) && !defined( __clang__ ) + // GCC's `optimize` attribute resets the function's option set, so it is + // applied to as little code as possible - see the note at its use site. +# define PSI_FUNCTIONOID_GCC16_MODREF_WORKAROUND [[ gnu::optimize( "no-ipa-modref" ) ]] +#else +# define PSI_FUNCTIONOID_GCC16_MODREF_WORKAROUND +#endif namespace psi::functionoid { //------------------------------------------------------------------------------ @@ -162,6 +177,17 @@ auto const invalid_ptr( reinterpret_cast( static_cast void debug_clear( T & ) {} #endif // _DEBUG +// gcc-16 (only) miscompiles the small-object store below at -O2 -DNDEBUG: +// interprocedural mod/ref analysis drops the placement-new into the function +// buffer, so a later invoke reads a garbage target and segfaults. Pinned here, +// on the STORE, rather than on invoke_impl - annotating either end cures it, +// and this one is a single placement-new on the assignment path while +// invoke_impl is the hot path taken by every call. +// Reduced to ~20 lines (a move-only callable assigned a reference-capturing +// lambda, then invoked); clang is unaffected and plain -O2 without NDEBUG is +// fine. Reported upstream: https://gcc.gnu.org/bugzilla/ - PR number to be +// filled in here once the report is filed; drop the workaround when it is +// fixed. /// Manager for trivial objects that fit into sizeof( void * ). struct manager_ptr { @@ -171,7 +197,7 @@ struct manager_ptr static auto functor_ptr( function_buffer_base const & buffer ) { BOOST_ASSUME( buffer.obj_ptr ); return &buffer.obj_ptr; } template - static void assign( Functor const functor, function_buffer_base & out_buffer, Allocator ) noexcept + PSI_FUNCTIONOID_GCC16_MODREF_WORKAROUND static void assign( Functor const functor, function_buffer_base & out_buffer, Allocator ) noexcept { static_assert( functor_traits::allowsPtrObjectOptimization ); # ifdef _MSC_VER @@ -181,14 +207,14 @@ struct manager_ptr new ( functor_ptr( out_buffer ) ) Functor( functor ); } - static void clone( function_buffer_base const & in_buffer, function_buffer_base & out_buffer ) noexcept + PSI_FUNCTIONOID_GCC16_MODREF_WORKAROUND static void clone( function_buffer_base const & in_buffer, function_buffer_base & out_buffer ) noexcept { //...zzz...even with __assume MSVC still generates branching code... //assign( *functor_ptr( in_buffer ), out_buffer, dummy_allocator() ); out_buffer.obj_ptr = in_buffer.obj_ptr; } - static void move( function_buffer_base && in_buffer, function_buffer_base & out_buffer ) noexcept + PSI_FUNCTIONOID_GCC16_MODREF_WORKAROUND static void move( function_buffer_base && in_buffer, function_buffer_base & out_buffer ) noexcept { clone( in_buffer, out_buffer ); destroy( in_buffer ); @@ -209,7 +235,7 @@ struct manager_trivial_small static void * functor_ptr( function_buffer_base & buffer ) { return &buffer; } template - static void assign( Functor const & functor, Buffer & out_buffer, Allocator ) noexcept + PSI_FUNCTIONOID_GCC16_MODREF_WORKAROUND static void assign( Functor const & functor, Buffer & out_buffer, Allocator ) noexcept { static_assert ( @@ -223,12 +249,12 @@ struct manager_trivial_small new ( functor_ptr( out_buffer ) ) Functor( functor ); } - static void clone( function_buffer_base const & __restrict in_buffer, function_buffer_base & __restrict out_buffer ) noexcept + PSI_FUNCTIONOID_GCC16_MODREF_WORKAROUND static void clone( function_buffer_base const & __restrict in_buffer, function_buffer_base & __restrict out_buffer ) noexcept { assign( Buffer::from_base( in_buffer ), Buffer::from_base( out_buffer ), dummy_allocator{} ); } - static void move( function_buffer_base && __restrict in_buffer, function_buffer_base & __restrict out_buffer ) noexcept + PSI_FUNCTIONOID_GCC16_MODREF_WORKAROUND static void move( function_buffer_base && __restrict in_buffer, function_buffer_base & __restrict out_buffer ) noexcept { clone( in_buffer, out_buffer ); destroy( in_buffer ); @@ -255,7 +281,7 @@ struct manager_trivial_heap static void const * functor_ptr( function_buffer_base const & buffer ) { return functor_ptr( const_cast( buffer ) ); } template - static void assign( Functor const & functor, function_buffer_base & out_buffer, [[ maybe_unused ]] Allocator const a ) + PSI_FUNCTIONOID_GCC16_MODREF_WORKAROUND static void assign( Functor const & functor, function_buffer_base & out_buffer, [[ maybe_unused ]] Allocator const a ) { static_assert ( @@ -271,7 +297,7 @@ struct manager_trivial_heap clone( in_buffer, out_buffer ); } - static void clone( function_buffer_base const & __restrict in_buffer, function_buffer_base & __restrict out_buffer ) + PSI_FUNCTIONOID_GCC16_MODREF_WORKAROUND static void clone( function_buffer_base const & __restrict in_buffer, function_buffer_base & __restrict out_buffer ) { BOOST_ASSERT( ( out_buffer.trivial_heap_obj.ptr == 0 ) || ( out_buffer.trivial_heap_obj.ptr == reinterpret_cast( -1 ) ) ); BOOST_ASSERT( ( out_buffer.trivial_heap_obj.size == 0 ) || ( out_buffer.trivial_heap_obj.size == static_cast ( -1 ) ) ); @@ -282,7 +308,7 @@ struct manager_trivial_heap std::memcpy( functor_ptr( out_buffer ), functor_ptr( in_buffer ), storage_size ); } - static void move( function_buffer_base && __restrict in_buffer, function_buffer_base & __restrict out_buffer ) noexcept + PSI_FUNCTIONOID_GCC16_MODREF_WORKAROUND static void move( function_buffer_base && __restrict in_buffer, function_buffer_base & __restrict out_buffer ) noexcept { out_buffer.trivial_heap_obj = in_buffer.trivial_heap_obj; debug_clear( in_buffer.trivial_heap_obj ); @@ -311,7 +337,7 @@ struct manager_small static Functor const * functor_ptr( function_buffer_base const & buffer ) { return functor_ptr( const_cast( buffer ) ); } template - static void assign( F && functor, Buffer & out_buffer, Allocator ) noexcept( noexcept( Functor( std::forward( functor ) ) ) ) + PSI_FUNCTIONOID_GCC16_MODREF_WORKAROUND static void assign( F && functor, Buffer & out_buffer, Allocator ) noexcept( noexcept( Functor( std::forward( functor ) ) ) ) { # ifdef _MSC_VER // MSVC14u3 still generates a branch w/o this (GCC issues a warning that it knows that &out_buffer cannot be null so we have to ifdef guard this). @@ -320,13 +346,13 @@ struct manager_small new ( functor_ptr( out_buffer ) ) Functor( std::forward( functor ) ); } - static void clone( function_buffer_base const & in_buffer, function_buffer_base & out_buffer ) noexcept( std::is_nothrow_copy_constructible_v ) + PSI_FUNCTIONOID_GCC16_MODREF_WORKAROUND static void clone( function_buffer_base const & in_buffer, function_buffer_base & out_buffer ) noexcept( std::is_nothrow_copy_constructible_v ) { auto const & __restrict in_functor( *functor_ptr( in_buffer ) ); assign( in_functor, Buffer::from_base( out_buffer ), dummy_allocator() ); } - static void move( function_buffer_base && __restrict in_buffer, function_buffer_base & __restrict out_buffer ) noexcept( std::is_nothrow_move_constructible_v ) + PSI_FUNCTIONOID_GCC16_MODREF_WORKAROUND static void move( function_buffer_base && __restrict in_buffer, function_buffer_base & __restrict out_buffer ) noexcept( std::is_nothrow_move_constructible_v ) { auto & __restrict in_functor( *functor_ptr( in_buffer ) ); assign( std::move( in_functor ), Buffer::from_base( out_buffer ), dummy_allocator{} ); @@ -370,7 +396,7 @@ struct manager_generic } template - static void assign( F && functor, function_buffer_base & out_buffer, OriginalAllocator source_allocator ) + PSI_FUNCTIONOID_GCC16_MODREF_WORKAROUND static void assign( F && functor, function_buffer_base & out_buffer, OriginalAllocator source_allocator ) { auto constexpr does_not_need_guard { @@ -389,13 +415,13 @@ struct manager_generic #if BOOST_MSVC // Bogus heap-overflow failure w/ VS 16.10 in implicit memcpy of OriginalAllocator{ in_functor_and_allocator.allocator() } __declspec( no_sanitize_address ) #endif // BOOST_MSVC - static void clone( function_buffer_base const & __restrict in_buffer, function_buffer_base & __restrict out_buffer ) + PSI_FUNCTIONOID_GCC16_MODREF_WORKAROUND static void clone( function_buffer_base const & __restrict in_buffer, function_buffer_base & __restrict out_buffer ) { functor_and_allocator_t const & in_functor_and_allocator{ *functor_ptr( in_buffer ) }; assign( in_functor_and_allocator.functor(), out_buffer, in_functor_and_allocator.allocator() ); } - static void move( function_buffer_base && __restrict in_buffer, function_buffer_base & __restrict out_buffer ) noexcept + PSI_FUNCTIONOID_GCC16_MODREF_WORKAROUND static void move( function_buffer_base && __restrict in_buffer, function_buffer_base & __restrict out_buffer ) noexcept { manager_trivial_heap::move( std::move( in_buffer ), out_buffer ); } @@ -583,7 +609,7 @@ struct cloner { constexpr cloner( void const * ) noexcept {} template - static void clone( Buffer const & __restrict in_buffer, Buffer & __restrict out_buffer ) noexcept { out_buffer = in_buffer; } + PSI_FUNCTIONOID_GCC16_MODREF_WORKAROUND static void clone( Buffer const & __restrict in_buffer, Buffer & __restrict out_buffer ) noexcept { out_buffer = in_buffer; } }; template <> struct cloner { constexpr cloner( void const * ) noexcept {} }; @@ -599,7 +625,7 @@ struct mover { constexpr mover( void const * ) noexcept {} template - static void move( Buffer && __restrict in_buffer, Buffer & __restrict out_buffer ) noexcept { cloner::clone( in_buffer, out_buffer ); } + PSI_FUNCTIONOID_GCC16_MODREF_WORKAROUND static void move( Buffer && __restrict in_buffer, Buffer & __restrict out_buffer ) noexcept { cloner::clone( in_buffer, out_buffer ); } }; template <> struct mover { constexpr mover( void const * ) noexcept {} }; @@ -1270,7 +1296,7 @@ class callable_base::safe_mover_base public: void cancel() noexcept { BOOST_ASSERT( p_function_to_restore_to_ ); p_function_to_restore_to_ = 0; } - static void move( callable_base & source, callable_base & destination, vtable const & empty_handler_vtable ) noexcept + PSI_FUNCTIONOID_GCC16_MODREF_WORKAROUND static void move( callable_base & source, callable_base & destination, vtable const & empty_handler_vtable ) noexcept { source.move_to( destination ); destination.store_vtable( source.load_vtable( std::memory_order_relaxed ) ); diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 7d9e66c..df07632 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -11,10 +11,26 @@ add_executable( functionoid_smoke callable_move_only_test.cpp callable_invoke_test.cpp nested_callable_test.cpp - vtable_attrs_test.cpp concurrent_reads_test.cpp ) target_link_libraries( functionoid_smoke PRIVATE GTest::gtest_main Psi::Functionoid ) set_target_properties( functionoid_smoke PROPERTIES RUNTIME_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}/test" ) add_test( NAME functionoid_smoke COMMAND functionoid_smoke ) + +# Its own binary, and it must stay that way: the TU defines +# PSI_FUNCTIONOID_DETAIL_INVOKE_FN_ATTR, which changes the DECLARATION of the +# vtable's invoke slot. Linking it beside TUs compiled without that macro makes +# the same class template two different types - an ODR violation whose symptom +# lands in the OTHER tests, not in this one: GCC propagates the surviving +# `pure` declaration to callers that were compiled believing the call writes +# memory, and dead-store elimination then removes stores it may not remove +# (segfaults under gcc-16 -O2 -DNDEBUG; -fno-ipa-modref / -fno-tree-dse / +# -fno-inline each hide it). +# Keeping it separate also keeps the attribute honest: `pure` holds for this +# test's stateless target, not for callables in general. +add_executable( functionoid_vtable_attrs vtable_attrs_test.cpp ) +target_link_libraries( functionoid_vtable_attrs PRIVATE GTest::gtest_main Psi::Functionoid ) + +set_target_properties( functionoid_vtable_attrs PROPERTIES RUNTIME_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}/test" ) +add_test( NAME functionoid_vtable_attrs COMMAND functionoid_vtable_attrs )