diff --git a/.github/compilers.json b/.github/compilers.json index 950eeb1b2..afa954ae9 100644 --- a/.github/compilers.json +++ b/.github/compilers.json @@ -95,7 +95,15 @@ "latest_cxxstd": "20", "runs_on": "windows-2022", "b2_toolset": "msvc-14.4", - "generator": "Visual Studio 17 2022", + "generator": "Visual Studio 17 2022" + }, + { + "version": "14.51", + "cxxstd": "20", + "latest_cxxstd": "20", + "runs_on": "windows-2025-vs2026", + "b2_toolset": "msvc-14.5", + "generator": "Ninja", "is_latest": true } ], diff --git a/include/boost/capy/detail/await_suspend_helper.hpp b/include/boost/capy/detail/await_suspend_helper.hpp index c2d704ccb..e609fbe66 100644 --- a/include/boost/capy/detail/await_suspend_helper.hpp +++ b/include/boost/capy/detail/await_suspend_helper.hpp @@ -12,6 +12,7 @@ #define BOOST_CAPY_DETAIL_AWAIT_SUSPEND_HELPER_HPP #include +#include #include #include @@ -40,19 +41,43 @@ namespace detail { frame before the runtime reads `__$ReturnUdt$` (e.g. `boundary_trampoline` final_suspend). - On MSVC this function calls `h.resume()` on the current stack - and returns `void`, causing unconditional suspension. The - trade-off is O(n) stack growth instead of O(1) tail-calls. - - On other compilers the handle is returned directly for proper - symmetric transfer. + On affected compilers this function calls `h.resume()` on the + current stack and returns `void`, causing unconditional + suspension. The trade-off is O(n) stack growth instead of + O(1) tail-calls. + + The workaround applies to MSVC 19.34 through 19.44 and + self-retires on MSVC 19.50 (VS 2026 / 18.0). Measured on + 19.44 the caller builds the hidden return slot at + `__coro_frame_ptr$ + 0xC0`, on the coroutine frame; on 19.51 + it is an `rsp`-relative stack temporary, so destroying the + frame no longer invalidates it. + + Do not widen this gate on the basis of Developer Community + ticket 10251975, tagged "Fixed in VS 2022 17.9 Preview 2"; + 19.39 reproduces the fault identically to 19.34. + + The gate deliberately excludes Clang. Both `clang-cl` and + `clang++` targeting Windows define `_MSC_VER` for ABI + compatibility, but generate a correct tail-call. + + Note that a probe which merely poisons the destroyed frame + cannot validate this gate. Routing the return through this + function moves the frame write to after `destroy()`, which + repairs the poison pattern and hides the defect. The + regression test in + test/unit/detail/await_suspend_helper.cpp unmaps the frame + instead, so any post-destroy access faults. + + On unaffected compilers the handle is returned directly for + proper symmetric transfer. Callers must use `auto` return type on their `await_suspend` so the return type adapts per platform. @param h The coroutine handle to transfer to. */ -#if BOOST_CAPY_WORKAROUND(_MSC_VER, >= 1) +#if BOOST_CAPY_WORKAROUND(_MSC_VER, < 1950) && !defined(__clang__) inline void symmetric_transfer(std::coroutine_handle<> h) noexcept { // safe_resume is not needed here: the calling coroutine is diff --git a/test/unit/detail/await_suspend_helper.cpp b/test/unit/detail/await_suspend_helper.cpp index 70b6eec96..45cb95fc7 100644 --- a/test/unit/detail/await_suspend_helper.cpp +++ b/test/unit/detail/await_suspend_helper.cpp @@ -13,6 +13,14 @@ #include #include +#include +#include + +#ifdef _WIN32 +# include +#else +# include +#endif #include "test_suite.hpp" @@ -20,6 +28,111 @@ namespace boost { namespace capy { namespace detail { +namespace { + +bool probe_continuation_ran = false; + +// Coroutine frames for the destroy-then-transfer probe, allocated so +// that destroying a frame UNMAPS it. Any access to the frame after +// destroy - read or write - then faults. +// +// A poisoning allocator is not sufficient. Routing the return through +// symmetric_transfer moves the compiler's write of the handle into the +// frame slot to after the frame is destroyed, which repairs a poison +// pattern and hides the defect. Unmapping cannot be repaired, so this +// probe sees the frame access that poisoning misses. +struct probe_frame +{ + static void* allocate(std::size_t n) noexcept + { + #ifdef _WIN32 + return ::VirtualAlloc( + nullptr, n, MEM_COMMIT | MEM_RESERVE, PAGE_READWRITE); + #else + void* const p = ::mmap(nullptr, n, PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); + return p == MAP_FAILED ? nullptr : p; + #endif + } + + static void release(void* p, std::size_t n) noexcept + { + #ifdef _WIN32 + (void)n; + ::VirtualFree(p, 0, MEM_RELEASE); + #else + ::munmap(p, n); + #endif + } +}; + +struct probe_task +{ + struct promise_type + { + void* operator new(std::size_t n) + { + void* const p = probe_frame::allocate(n); + if(! p) + throw std::bad_alloc(); + return p; + } + + void operator delete(void* p, std::size_t n) noexcept + { + probe_frame::release(p, n); + } + + probe_task get_return_object() noexcept + { + return { std::coroutine_handle< + promise_type>::from_promise(*this) }; + } + + std::suspend_always initial_suspend() noexcept { return {}; } + std::suspend_always final_suspend() noexcept { return {}; } + void return_void() noexcept {} + void unhandled_exception() noexcept { BOOST_TEST(false); } + }; + + std::coroutine_handle h; +}; + +// Mirrors the final_suspend awaiters in when_all_runner and +// when_any_runner: destroy our own frame, then transfer to the +// continuation. auto return type, because symmetric_transfer +// returns void on the workaround path. +struct destroy_then_transfer +{ + std::coroutine_handle<> next; + + bool await_ready() const noexcept { return false; } + + auto await_suspend(std::coroutine_handle<> self) noexcept + { + // Copy to the stack first: this awaiter lives on the frame + // about to be destroyed. + auto const continuation = next; + self.destroy(); + return symmetric_transfer(continuation); + } + + void await_resume() const noexcept {} +}; + +probe_task probe_continuation() +{ + probe_continuation_ran = true; + co_return; +} + +probe_task probe_victim(std::coroutine_handle<> next) +{ + co_await destroy_then_transfer{ next }; +} + +} // (anon) + class await_suspend_helper_test { // await_suspend returning void: caller suspends unconditionally. @@ -55,9 +168,37 @@ class await_suspend_helper_test }; public: + // capy#378: transferring out of an await_suspend that has already + // destroyed its own frame must not touch that frame afterwards. + // On affected MSVC toolsets symmetric_transfer resumes on the + // current stack to avoid the frame round-trip; elsewhere it + // performs a real tail-call. Either way the continuation must run + // and the process must survive. + // + // When the gate is wrong for the compiler in use this fails as a + // hard access violation rather than a BOOST_TEST failure. Each + // test runs in its own process under CTest, so the crash is + // reported as this test failing. + void + testDestroyThenTransfer() + { + probe_continuation_ran = false; + + auto cont = probe_continuation(); + auto victim = probe_victim(cont.h); + + victim.h.resume(); + + BOOST_TEST(probe_continuation_ran); + + cont.h.destroy(); + } + void run() { + testDestroyThenTransfer(); + auto const h = std::noop_coroutine(); {