
<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class=""><br class=""></div><div class=""><div><blockquote type="cite" class=""><div class="">On Mar 19, 2018, at 3:44 PM, Brian Gesiak <<a href="mailto:modocache@gmail.com" class="">modocache@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class="">Hello all!<div class="">(+cc Vedant Kumar, who I've been told knows a lot about UBSan!)<br class=""></div><div class=""><br class=""></div><div class="">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'.</div><div class=""><br class=""></div><div class="">You can see an example of the assert in this 26-line C++ file here: <a href="https://godbolt.org/g/Gw9UZq" class="">https://godbolt.org/g/Gw9UZq</a></div><div class=""><br class=""></div><div class="">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".</div><div class=""><br class=""></div><div class="">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.</div><div class=""><br class=""></div><div class="">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.</div></div></div></blockquote><div><br class=""></div><span class="">It looks like there was a FIXME about this issue introduced circa r280678.<br class=""></span><br class=""><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="">You can see an example of the IR generated with and without '-fsanitize=null' here: <a href="https://gist.github.com/modocache/54a036c3bf9c06882fe85122e105d153" class="">https://gist.github.com/modocache/54a036c3bf9c06882fe85122e105d153</a> -- 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.</div><div class=""><br class=""></div><div class="">I have several questions:</div><div class=""><br class=""></div><div class="">* What's the best way for the LLVM coroutines transform passes to play nicely with the code generated with UBSan's '-fsanitize=null'?</div></div></div></blockquote><div><br class=""></div>+ Gor, as he might be the best person to answer this.</div><div><br class=""></div><div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="">* Are there other LLVM passes I could look at, to see examples of how they were modified to handle UBSan?</div></div></div></blockquote><div><br class=""></div><div>No, no other llvm passes needed to be modified to handle ubsan instrumentation.</div><div><br class=""></div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="">* 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?</div></div></div></blockquote><div><br class=""></div><div>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.</div><div><br class=""></div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class=""> Are there downsides to doing this?</div></div></div></blockquote><div><br class=""></div><div>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.</div><div><br class=""></div><div>vedant</div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class=""><br class=""></div><div class="">Any and all advice is appreciated -- thanks!</div><div class=""><br class=""></div><div class="">- Brian Gesiak</div><div class=""><br class=""></div></div>

</div></blockquote></div><br class=""></div></body></html>