[clang] [llvm] [Coroutines] Mark parameter allocas with coro.outside.frame metadata (PR #127653)

[Hans Wennborg via llvm-commits]

zmodem wrote:

> In the issue (#127499) you pointed out that this issue applies to the MSVC ABI where all parameters are destroyed in the callee. Is it reasonable to construct the analogous test case in that environment?

Yes, in fact the reproducer on that issue will trigger use-after-free on x86-64 Windows also without the `trivial_abi` attribute.

I've updated coro-params.cpp to cover this as well.

> My instinct reaction is, it may block some optimizations to optimize the alloca out.

The intrinsic gets dropped by CoroSplit, so after that it's not a problem. Before that, I'm not sure much interesting optimization can happen to allocas anyway? Especially since it's not known if they will be part of the coro frame or not at that point.

> I believe we shouldn't access the coroutine frame after coro.end. If the doc doesn't make it clear, let's try to make it.

How would we motivate that change though?

Also, would that solve the GRO issue (#49843) which led to adding the existing metadata?

>> In fact, my change broke a test which seems to be doing exactly that. Just because it's old doesn't mean it was wrong. So the patch didn't feel solid.

> I do feel it is wrong since it accesses the frame after it ends. It makes no sense.

(To make it easier to follow, we're talking about [this access](https://github.com/llvm/llvm-project/blob/llvmorg-20.1.0-rc2/llvm/test/Transforms/Coroutines/coro-debug.ll#L75).) 

Looking closer, I think that code might be fine. The code after `coro.end` will go at the end of the ramp function. At that point the coro frame is still alive. And since that alloca is also [accessed in the resume function](https://github.com/llvm/llvm-project/blob/llvmorg-20.1.0-rc2/llvm/test/Transforms/Coroutines/coro-debug.ll#L67) (after a suspend), it *needs* to be in the frame.

https://github.com/llvm/llvm-project/pull/127653