Re: [PATCH RFA] ubsan: default to trap on unreachable at -O0 and -Og [PR104642]

[Jakub Jelinek <jakub@redhat.com>]

On Mon, Jun 13, 2022 at 03:53:13PM -0400, Jason Merrill via Gcc-patches wrote:
> When not optimizing, we can't do anything useful with unreachability in
> terms of code performance, so we might as well improve debugging by turning
> __builtin_unreachable into a trap.  In the PR richi suggested introducing an
> -funreachable-traps flag for this, but this functionality is already
> implemented as -fsanitize=unreachable -fsanitize-undefined-trap-on-error, we
> just need to set those flags by default.
> 
> I think it also makes sense to do this when we're explicitly optimizing for
> the debugging experience.
> 
> I then needed to make options-save handle -fsanitize and
> -fsanitize-undefined-trap-on-error; since -fsanitize is has custom parsing,
> that meant handling it explicitly in the awk scripts.  I also noticed we
> weren't setting it in opts_set.
> 
> Do we still want -funreachable-traps as an alias (or separate flag) for this
> behavior that doesn't mention the sanitizer?

I do not like doing it this way, -fsanitize-undefined-trap-on-error is
(unfortunately for compatibility with llvm misdesign, users should have
ways to control which of the enabled sanitizers should be handled which way,
where the 3 ways are runtime diagnostics without abort, runtime diagnostics
with abort and __builtin_trap ()) an all or nothing option which affects all
the sanitizers.  Your patch will e.g. silently enable the sanitization
whenever just -fsanitize-undefined-trap-on-error is passed, but that can be
passed even when users don't really expect any sanitization, just making
sure that if it is enabled (say for selected TUs or functions), it doesn't
need a runtime library to report it.
Furthermore, handling it the UBSan way means we slow down the compiler
(enable a bunch of extra passes, like sanopt, ubsan), which is undesirable
e.g. for -O0 compilation speed.

So, I think -funreachable-traps should be a separate flag and not an alias,
enabled by default for -O0 and -Og, which would be handled elsewhere
(I'd say e.g. in fold_builtin_0 and perhaps gimple_fold_builtin too to
avoid allocating trees unnecessary) and would be done if
flag_unreachable_traps && !sanitize_flag_p (SANITIZE_UNREACHABLE),
just replacing that __builtin_unreachable call with __builtin_trap.
For the function ends in fact under those conditions we could emit
__builtin_trap right away instead of emitting __builtin_unreachable
and waiting on folding it later to __builtin_trap.

	Jakub