[llvm-dev] Suggestions for how coroutines and UBSan codegen can play nice with one another?

[Vedant Kumar via llvm-dev]

> On Mar 19, 2018, at 3:44 PM, Brian Gesiak <modocache at gmail.com> wrote:
> 
> Hello all!
> (+cc Vedant Kumar, who I've been told knows a lot about UBSan!)
> 
> I am trying to fix an assert that occurs when the transforms in llvm/lib/Transforms/Coroutines are applied to LLVM IR that has been generated with UBSan enabled -- specifically, '-fsanitize=null'.
> 
> You can see an example of the assert in this 26-line C++ file here: https://godbolt.org/g/Gw9UZq <https://godbolt.org/g/Gw9UZq>
> 
> Note that without the '-fsanitize=null' option this compiles fine, but when that option is used, Clang/LLVM crashes due to "error in backend: cannot move instruction since its users are not dominated by CoroBegin".
> 
> The coroutine pass coro-split is responsible for transforming instructions within a coroutine function. Because a coroutine function can be suspended and then resumed much later in a program's execution, accesses to stack variables much be rewritten so that they access variables stored on the "coroutine frame," a heap-allocated piece of state that represents the execution frame of the coroutine. As part of this process, load instructions, such as the one generated for the '*this' expression in the source file above, are moved down past a call to the '@llvm.coro.begin' intrinsic.
> 
> However, when UBSan is enabled, the load of '*this' is then immediately followed by a null check and, in the null case, a conditional branch to a call to '@ __ubsan_handle_type_mismatch_v1'. The coro-split pass is not written to move these compare and branch instructions, and instead asserts.

It looks like there was a FIXME about this issue introduced circa r280678.


> You can see an example of the IR generated with and without '-fsanitize=null' here: https://gist.github.com/modocache/54a036c3bf9c06882fe85122e105d153 <https://gist.github.com/modocache/54a036c3bf9c06882fe85122e105d153> -- PR36578.ll lines 82 to 104 show the problematic UBSan branches. Note that PR36578.nosan.ll, which was compiled without '-fsanitize=null', does not include these branches.
> 
> I have several questions:
> 
> * What's the best way for the LLVM coroutines transform passes to play nicely with the code generated with UBSan's '-fsanitize=null'?

+ Gor, as he might be the best person to answer this.


> * Are there other LLVM passes I could look at, to see examples of how they were modified to handle UBSan?

No, no other llvm passes needed to be modified to handle ubsan instrumentation.


> * I've seen code in Clang's codebase that adds 'SanitizerKind::Null' to a set of skipped UBSan checks; should lib/CodeGen/CGCoroutine.cpp also do something like this, to avoid generating UBSan checks altogether? Would that be sufficient to avoid the error?

I don't think it would be sufficient. You might encounter the same issue with -fsanitize=alignment,object-size,vptr and possibly -fsanitize=address. Instead of blacklisting known bad sanitizers, it might be better to whitelist known good ones. Over time you could expand that set, adding tests as needed.


> Are there downsides to doing this?

I think a whitelisting approach is a reasonable way to prevent crashes. It at least allows you to enable sanitizers for coroutines later on. The alternative is to force users to partially disable sanitization (either with source annotations or by dropping the build configuration entirely). If that happens, it's much less likely that they'd re-enable it in the future.

vedant

> 
> Any and all advice is appreciated -- thanks!
> 
> - Brian Gesiak
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20180319/8a9ea5a3/attachment.html>