[PATCH] D56624: [Sanitizers] UBSan unreachable incompatible with ASan in the presence of `noreturn` calls

[Vedant Kumar via Phabricator via llvm-commits]

vsk added a comment.

In D56624#1370280 <https://reviews.llvm.org/D56624#1370280>, @eugenis wrote:

> > Wouldn’t it be preferable to unpoison the stack inside of maybe_longjmp, once the opaque condition can be checked?
>
> Sure, but that's not always possible. That's why we have interceptors.


Fair enough!

>>> One possible optimization that I can think of is splitting code after the call into a separate basic block and marking it as cold.
>>>  Admittedly, that's unlikely to have big impact in practice. I'd guess that [[expect_noreturn]] calls are typically not very hot in the first place.
>> 
>> The cold attribute is already used for this kind of splitting/reordering. I don't yet see how expect_noreturn creates new opportunities for the optimizer.
> 
> Strictly speaking, cold attribute on a function means that it is rarely called. It does not say anything about the code after the call being colder than the code before the call (within the same BB), which makes splitting the BB pointless.

That's true, but it's safe to assume that code which dominates the cold call (or is post-dominated by it) is at least as cold as the call.

> Anyway, I agree that the arguments [[expect_noreturn]] are not that strong and perhaps don't make the bar for the addition of a new IR attribute.
>  Should we go back to the intrinsic idea?

Sgtm, I think that'd be the simplest solution (something like inserting llvm.asan.stack_unpoison() where needed).


Repository:
  rL LLVM

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

https://reviews.llvm.org/D56624