[PATCH] D101980: [RFC] [Coroutines] Put the contents of byval argument to the frame

Wed May 12 19:18:07 PDT 2021

ChuanqiXu planned changes to this revision.
ChuanqiXu added a comment.

In D101980#2754257 <https://reviews.llvm.org/D101980#2754257>, @lxfind wrote:

> Sorry for being late in reviewing this patch. I have been out of work for a few days.
>
> The description of the problem is not accurate.
> Even for byval parameters, their content will still be copied into the local stack for coroutines correctly. This is handled by the front-end.
> So parameter copying is not the problem here.
> The problem is exactly what @rjmccall said about the assumptions optimizations typically make about parameters.
> The bug linked in this patch, is an example of such issue: for a byval parameter A, a memcpy from A to B, followed by another memcpy from B to C, will be optimized into a memcpy from A to C. And if the memcpy of B to C happens after a coroutine suspension, this leads to memory corruption because A would have died by that point.
> D101510 <https://reviews.llvm.org/D101510> is one attempt I tried to fix this, but it's too hacky.
> D100415 <https://reviews.llvm.org/D100415> is more long-term approach, but it's much harder than I thought and I don't know how long it will take.
> So the idea for this patch is to see if we can find a way to fix this problem within the coroutine passes only.

Thanks for the clarifying! It is sorry to make a mistake here to waste reviewers's time. I would take a closer look. It looks like the patch now is just a bad fix.

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

https://reviews.llvm.org/D101980