On Wed, Jul 27, 2022 at 09:33:47AM +0000, Richard Biener wrote:
> > __builtin_unreachable and __ubsan_handle_builtin_unreachable don't
> > use vops, they are marked const/leaf/noreturn/nothrow/cold.
> > But __builtin_trap uses vops, isn't const, just leaf/noreturn/nothrow/cold.
> > This is I believe so that when users explicitly use __builtin_trap in their
> > sources they get stores visible at the trap side.
> > -fsanitize=unreachable -fsanitize-undefined-trap-on-error used to transform
> > __builtin_unreachable to __builtin_trap even in the past, but the sanopt
> > pass
> > has TODO_update_ssa, so it worked fine.
> >
> > Now that gimple_build_builtin_unreachable can build a __builtin_trap call
> > right away, we can run into problems that whenever we need it we would need
> > to either manually or through TODO_update* ensure the vops being updated.
> >
> > Though, as it is originally __builtin_unreachable which is just implemented
> > as trap, I think for this case it is fine to avoid vops. For this the
> > patch introduces IFN_TRAP, which has ECF_* flags like __builtin_unreachable
> > and is expanded as __builtin_trap.
> >
> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> I think for the sake of sanitizing unreachable as trap this is OK
> but it seems this isn't actually what is done.
We chose to sanitize not just explicit user __builtin_unreachable calls,
but also the internally generated __builtin_unreachable calls (with the
one exception of fall through to end of C++ function returning non-void,
which had before a separate sanitizer) and we've been doing it since 2013
when ubsan was added.
Even for the internally generated unreachable calls like those from
devirtualization or other reasons like ivcanon/unrolling, having the
possibility to get some runtime diagnostics or trap can be useful over
just falling through to random following code.
Previously we'd always emit __builtin_unreachable, then perhaps in some
cases could e.g. optimize it away (say if there is a guarding condition
around the implicitly added unreachable turning the condition into VRP
info and optimizing the conditional away), otherwise the sanopt pass
would turn those __builtin_unreachable calls into __builtin_trap.
With the recent changes, we don't run the sanopt pass when only
doing -fsanitize=unreachable (or -funrechable-traps) though, so we need
to emit the trap/__ubsan_handle_unreachable/__builtin_unreachable right
away.
Jakub
