rjmccall requested changes to this revision.
rjmccall added a comment.
This revision now requires changes to proceed.

I agree that coroutine resumption functions have a different formal type from 
the ramp function and so would need different treatment from 
`-fsanitize=functions` if it wants to sanitize the resumption calls, which I 
guess it currently doesn't.  So something here may be a necessary fix.

However, I don't think it's a sufficient fix for PR 50345, because the way that 
the frontend currently creates these prologue attributes is deeply problematic 
for any number of function transformations, not just coroutine splitting.  For 
example, any sort of function-cloning transformation will end up constructing 
an incorrect relative reference.  I expect that this self-reference will also 
interfere with DCE.  So in addition to whatever function-type fix we need for 
coroutines, we just need to change how we create this prologue.  I recommend 
the design I laid out in the PR:

- Have the frontend emit a more abstract attribute, like 
`sanitize_function_type(i8** @1)`
- Either lower this abstract attribute in a codegen pass by turning it into a 
`prologue` attribute or just handle it directly in the appropriate backend.

The coroutine part of the fix would then simply be to remove the 
`sanitize_function_type` attribute from the resumption function clones; or 
better yet, switch the coro.switch lowering to use the "prototype" design used 
by coro.retcon and coro.async, and then set the appropriate attribute (if any) 
on the prototype so that it will be cloned into the resumption functions.

In the meantime, this sanitizer should be disabled in 13.x.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114728

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to