[PATCH] D89066: [Coroutine][Sema] Only tighten the suspend call temp lifetime for final awaiter

[JunMa via Phabricator via cfe-commits]

junparser added a comment.

In D89066#2325358 <https://reviews.llvm.org/D89066#2325358>, @lxfind wrote:

> In D89066#2324291 <https://reviews.llvm.org/D89066#2324291>, @junparser wrote:
>
>> In D89066#2324151 <https://reviews.llvm.org/D89066#2324151>, @lxfind wrote:
>>
>>> In D89066#2324115 <https://reviews.llvm.org/D89066#2324115>, @junparser wrote:
>>>
>>>> why we should not do this with normal await call?
>>>
>>> To be honest, I don't know yet. My understanding of how expression cleanup and temp lifetime management is insufficient at the moment.
>>> But first of all, without adding any cleanup expression here, I saw ASAN failures due to heap-use-after-free, because sometimes the frame have already been destroyed after the await_suspend call, and yet we are still writing into the frame due to unnecessarily cross-suspend lifetime. However, if I apply the cleanup to all await_suepend calls, it also causes ASAN failures as it's cleaning up data that's still alive.
>>> So this patch is more of a temporary walkaround to stop bleeding without causing any trouble.
>>> I plan to get back to this latter after I am done with the spilling/alloca issues.
>>
>> I'm not familiar with ASAN instrumentation. Do you have any testcases to explain this?
>
> Unfortunately I don't.  But this is not related to ASAN. Basically, this is causing destructing of objects that should still be alive. I suspect that it's because ExprWithCleanups always clean up temps that belongs to the full expression, not just the sub-expression in it.

should we also need to add ExprWithCleanups when expand co_await to await_ready & await_suspend and await_resume expression which may fix this issue？


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89066/new/

https://reviews.llvm.org/D89066