diff --git a/include/boost/capy/detail/await_suspend_helper.hpp b/include/boost/capy/detail/await_suspend_helper.hpp index c2d704ccb..7b88065d8 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,62 @@ 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. + + On x64 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. + + On ARM64 the workaround does not retire at 19.50, because + that target has a second, unrelated defect. With a real + symmetric transfer, MSVC 19.51 release loses the handler for + a `try` region that spans the suspend point: after the + coroutine is resumed from another call stack, a `throw` + inside that region is not caught by the `catch` beside it and + escapes to the promise's `unhandled_exception`. A `catch(...)` + misses it too, so the region is not found at all rather than + the handler failing to match. Debug builds are unaffected, as + is x64 at the same toolset. `testCatchAfterDeferredResume` in + test/unit/task.cpp covers this. + + `_M_ARM64EC` is included conservatively. It generates ARM64 + code and has not been tested here; keeping the workaround on + is the safe direction, since it costs stack depth rather than + correctness. + + 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(_M_ARM64) || defined(_M_ARM64EC)) && \ + !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(); { diff --git a/test/unit/task.cpp b/test/unit/task.cpp index a2200f03f..30006b898 100644 --- a/test/unit/task.cpp +++ b/test/unit/task.cpp @@ -1277,9 +1277,100 @@ struct task_test BOOST_TEST_EQ(result, 7); } + // Suspends by parking the handle and returning noop_coroutine, so + // the coroutine resumes later from whatever stack calls resume(), + // rather than being transferred into inline. This is the shape of + // an async read that completes on the io_context thread. + struct parking_awaitable + { + std::coroutine_handle<>& slot; + + bool await_ready() const noexcept { return false; } + + std::coroutine_handle<> + await_suspend( + std::coroutine_handle<> h, io_env const*) const noexcept + { + slot = h; + return std::noop_coroutine(); + } + + // Returning a value the body tests before throwing mirrors + // `auto [ec, n] = co_await sock.read_some(buf); if(ec) throw;`. + bool await_resume() const noexcept { return true; } + }; + + // The body whose catch must run. Kept as a coroutine taking + // references rather than a capturing lambda so the shape matches + // the corosio session function this reproduces. + static task + deferred_throw_session( + std::coroutine_handle<>& slot, + bool& caught) + { + try + { + if(co_await parking_awaitable{slot}) + throw_test_exception("deferred resume"); + } + catch(test_exception const&) + { + caught = true; + } + } + + // Regression test for an MSVC 14.51 ARM64 release codegen bug. + // + // When a co_await whose await_suspend returns a coroutine_handle + // suspends, and the coroutine is later resumed from a different + // call stack, a throw in the same try block was not caught by the + // catch beside it. The exception escaped to the promise's + // unhandled_exception and reached run_async's error handler; in + // corosio's 3a_echo_server documentation test it terminated the + // process. + // + // detail::symmetric_transfer hides the bug wherever its workaround + // is enabled, because it resumes on the current stack and returns + // void instead of performing a real symmetric transfer. So this + // test passes on every compiler the workaround covers, and is only + // meaningful where it does not. + // + // The escape is checked through run_async's error handler rather + // than allowed to propagate: an uncaught exception here would + // terminate the whole test binary and hide every other result. + void + testCatchAfterDeferredResume() + { + int dispatch_count = 0; + test_executor ex(dispatch_count); + + std::coroutine_handle<> parked; + bool caught = false; + bool escaped = false; + bool completed = false; + + run_async(ex, + [&]() { completed = true; }, + [&](std::exception_ptr) { escaped = true; })( + deferred_throw_session(parked, caught)); + + // The awaitable parked the handle and nothing has thrown yet. + BOOST_TEST(static_cast(parked)); + BOOST_TEST(!caught); + + // Resume from here, not from inside await_suspend: arriving on + // an unrelated call stack is what the bug needs. + parked.resume(); + + BOOST_TEST(caught); + BOOST_TEST(!escaped); + BOOST_TEST(completed); + } + void run() { + testCatchAfterDeferredResume(); testBoolSuspendAwaitable(); testReturnValue(); testException();