[PATCH] D114728: [Coroutine] Remove the prologue data of `-fsanitize=function` for split functions

Yuanfang Chen via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Nov 29 13:18:00 PST 2021


ychen added a comment.

In D114728#3159303 <https://reviews.llvm.org/D114728#3159303>, @rjmccall wrote:

> 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.

@rjmccall Yeah agreed that the attribute method is a better approach forward (I would work on a patch). I was intended to propose this as an alternative to disabling `-fsanitize=function` for 13.x.

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

@pcc does this sound good to you?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114728



More information about the cfe-commits mailing list