From 2ba099a8133ad39048f168104195fb3f406abee7 Mon Sep 17 00:00:00 2001 From: Michael Vandeberg Date: Tue, 22 Sep 2026 09:20:15 -0700 Subject: [PATCH] fix: narrow the symmetric-transfer workaround to MSVC < 19.50, exclude Clang detail::symmetric_transfer substituted h.resume() on the current stack for any compiler defining _MSC_VER, trading O(1) tail-calls for O(n) stack growth. That over-matched on two axes. MSVC 19.34 through 19.44 place await_suspend's hidden return slot on the coroutine frame, at __coro_frame_ptr$ + 0xC0, so an await_suspend that destroys its own frame and then transfers leaves the write and read-back of the returned handle pointing at freed memory. MSVC 19.50 makes that slot an rsp-relative stack temporary which outlives the frame, so the workaround no longer applies. clang-cl and clang++ targeting Windows define _MSC_VER for ABI compatibility but emit a correct tail-call, so they never needed the workaround. A version-only gate would not exclude them, since Clang continues to emulate a sub-1950 _MSC_VER. The gate becomes: BOOST_CAPY_WORKAROUND(_MSC_VER, < 1950) && !defined(__clang__) Adds a regression test for the shape when_all_runner and when_any_runner use in final_suspend: destroy the frame inside await_suspend, then transfer through symmetric_transfer. Frames are allocated with VirtualAlloc/mmap and unmapped on destroy so that any post-destroy access faults. A poisoning allocator cannot detect this case, because routing the return through symmetric_transfer moves the compiler's write into the frame slot to after destroy, which repairs the poison and hides the defect. Adds MSVC 14.51 (VS 2026) to the CI matrix on windows-2025-vs2026, which is the only configuration that exercises the newly enabled tail-call path. It builds with Ninja because the CMake that cmake-workflow selects on that image predates the Visual Studio 18 2026 generator. is_latest moves from 14.44 to 14.51, taking the MSVC CMake and ASan variants with it; Visual Studio generator coverage remains on 14.34. Includes detail/config.hpp explicitly rather than relying on it arriving through ex/io_env.hpp. Refs #378 --- .github/compilers.json | 10 +- .../capy/detail/await_suspend_helper.hpp | 39 ++++- test/unit/detail/await_suspend_helper.cpp | 141 ++++++++++++++++++ 3 files changed, 182 insertions(+), 8 deletions(-) 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(); {