On 3/2/20 4:37 AM, Iain Sandoe wrote:
Hi,this corrects a thinko that seemed initially to be a missed optimisation, but turns out to lead to wrong code in some cases. tested on x86_64 darwin, linux and powerpc linux OK for trunk? thanks Iain In general, we need to manage the lifetime of compiler- generated awaitable instances in the coroutine frame, since these must persist across suspension points. However, it is quite possible that the user might provide the awaitable instances, either as function params or as a local variable. We will already generate a frame entry for these as required. At present, under this circumstance, we are duplicating these, awaitable, initialising a second frame copy for them (which we then subsequently destroy manually after the suspension point). That's not efficient - so an undesirable thinko in the first place. However, there is also an actual bug; if the compiler elects to elide the copy (which is perfectly legal), it does not have visibility of the manual management of the post-suspend destruction - this subsequently leads to double-free errors. The solution is not to make the second copy (as noted, params and local vars already have frame copies with managed lifetimes). gcc/cp/ChangeLog: 2020-03-02 Iain Sandoe <i...@sandoe.co.uk> * coroutines.cc (build_co_await): Do not build frame awaitable proxy vars when the co_await expression is a function parameter or local var. (co_await_expander): Do not initialise a frame var with itself. (transform_await_expr): Only substitute the awaitable frame var if it's needed. (register_awaits): Do not make frame copies for param or local vars that are awaitables.
ok -- Nathan Sidwell
