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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to