* [PATCH RFA] ubsan: default to trap on unreachable at -O0 and -Og [PR104642] @ 2022-06-13 19:53 Jason Merrill 2022-06-14 11:44 ` Jakub Jelinek 0 siblings, 1 reply; 12+ messages in thread From: Jason Merrill @ 2022-06-13 19:53 UTC (permalink / raw) To: gcc-patches, richard.guenther; +Cc: jwakely.gcc 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? Tested x86_64-pc-linux-gnu, OK for trunk? PR c++/104642 gcc/ChangeLog: * doc/invoke.texi (-fsanitize-undefined-trap-on-error): On by default at -O0, implying -fsanitize=unreachable,return * opts.cc (finish_options): At -O0 trap on unreachable code. (common_handle_option): Set opts_set->x_flag_sanitize. * common.opt (fsanitize-undefined-trap-on-error): Add Optimization tag. * optc-save-gen.awk, opth-gen.awk: Include flag_sanitize. gcc/testsuite/ChangeLog: * g++.dg/ubsan/return-8a.C: New test. --- gcc/doc/invoke.texi | 4 ++++ gcc/common.opt | 2 +- gcc/opts.cc | 10 ++++++++++ gcc/testsuite/g++.dg/ubsan/return-8a.C | 17 +++++++++++++++++ gcc/optc-save-gen.awk | 8 ++++++-- gcc/opth-gen.awk | 3 ++- 6 files changed, 40 insertions(+), 4 deletions(-) create mode 100644 gcc/testsuite/g++.dg/ubsan/return-8a.C diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 174bc09e5cf..446b0691305 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -16100,6 +16100,10 @@ a @code{libubsan} library routine. The advantage of this is that the @code{libubsan} library is not needed and is not linked in, so this is usable even in freestanding environments. +If @option{-fsanitize} has not been specified, this option implies +@option{-fsanitize=unreachable,return}, and is enabled by default at +@option{-O0} and @option{-Og}. + @item -fsanitize-coverage=trace-pc @opindex fsanitize-coverage=trace-pc Enable coverage-guided fuzzing code instrumentation. diff --git a/gcc/common.opt b/gcc/common.opt index 7ca0cceed82..90e3e84913b 100644 --- a/gcc/common.opt +++ b/gcc/common.opt @@ -1109,7 +1109,7 @@ fsanitize-address-use-after-scope Common Driver Var(flag_sanitize_address_use_after_scope) Init(0) fsanitize-undefined-trap-on-error -Common Driver Var(flag_sanitize_undefined_trap_on_error) Init(0) +Common Driver Optimization Var(flag_sanitize_undefined_trap_on_error) Init(0) Use trap instead of a library function for undefined behavior sanitization. fasynchronous-unwind-tables diff --git a/gcc/opts.cc b/gcc/opts.cc index bf06a55456a..3699eabb599 100644 --- a/gcc/opts.cc +++ b/gcc/opts.cc @@ -1122,6 +1122,15 @@ finish_options (struct gcc_options *opts, struct gcc_options *opts_set, opts->x_flag_no_inline = 1; } + /* At -O0 or -Og, turn __builtin_unreachable into a trap. */ + if ((!opts->x_optimize || opts->x_optimize_debug) + && !opts_set->x_flag_sanitize) + SET_OPTION_IF_UNSET (opts, opts_set, + flag_sanitize_undefined_trap_on_error, true); + if (opts->x_flag_sanitize_undefined_trap_on_error) + SET_OPTION_IF_UNSET (opts, opts_set, flag_sanitize, + SANITIZE_UNREACHABLE|SANITIZE_RETURN); + /* Pipelining of outer loops is only possible when general pipelining capabilities are requested. */ if (!opts->x_flag_sel_sched_pipelining) @@ -2613,6 +2622,7 @@ common_handle_option (struct gcc_options *opts, break; case OPT_fsanitize_: + opts_set->x_flag_sanitize = true; opts->x_flag_sanitize = parse_sanitizer_options (arg, loc, code, opts->x_flag_sanitize, value, true); diff --git a/gcc/testsuite/g++.dg/ubsan/return-8a.C b/gcc/testsuite/g++.dg/ubsan/return-8a.C new file mode 100644 index 00000000000..9b2265c4bb0 --- /dev/null +++ b/gcc/testsuite/g++.dg/ubsan/return-8a.C @@ -0,0 +1,17 @@ +// PR c++/104642 + +// At -O0 we default to +// -fsanitize=unreachable,return -fsanitize-undefined-trap-on-error +// so the below should abort at runtime. + +// { dg-do run } +// { dg-shouldfail { *-*-* } } +// { dg-additional-options "-O0" } + +bool b; + +int f() { + if (b) return 42; +} // { dg-warning "-Wreturn-type" } + +int main() { f(); } diff --git a/gcc/optc-save-gen.awk b/gcc/optc-save-gen.awk index 233d1fbb637..38c02bcc2cf 100644 --- a/gcc/optc-save-gen.awk +++ b/gcc/optc-save-gen.awk @@ -84,7 +84,7 @@ print "{"; n_opt_char = 4; n_opt_short = 0; -n_opt_int = 0; +n_opt_int = 1; n_opt_enum = 0; n_opt_string = 0; n_opt_other = 0; @@ -96,6 +96,7 @@ var_opt_range["optimize"] = "0, 255"; var_opt_range["optimize_size"] = "0, 2"; var_opt_range["optimize_debug"] = "0, 1"; var_opt_range["optimize_fast"] = "0, 1"; +var_opt_int[0] = "flag_sanitize"; # Sort by size to mimic how the structure is laid out to be friendlier to the # cache. @@ -1264,7 +1265,7 @@ for (i = 0; i < n_target_str; i++) { } print "}"; -n_opt_val = 4; +n_opt_val = 5; var_opt_val[0] = "x_optimize" var_opt_val_type[0] = "char " var_opt_hash[0] = 1; @@ -1277,6 +1278,9 @@ var_opt_hash[2] = 1; var_opt_val[3] = "x_optimize_fast" var_opt_val_type[3] = "char " var_opt_hash[3] = 1; +var_opt_val[4] = "x_flag_sanitize" +var_opt_val_type[4] = "unsigned int " +var_opt_hash[4] = 1; for (i = 0; i < n_opts; i++) { if (flag_set_p("(Optimization|PerFunction)", flags[i])) { name = var_name(flags[i]) diff --git a/gcc/opth-gen.awk b/gcc/opth-gen.awk index 8bba8ec4549..b3bedaa6da2 100644 --- a/gcc/opth-gen.awk +++ b/gcc/opth-gen.awk @@ -134,7 +134,7 @@ print "{"; n_opt_char = 4; n_opt_short = 0; -n_opt_int = 0; +n_opt_int = 1; n_opt_enum = 0; n_opt_other = 0; n_opt_explicit = 4; @@ -142,6 +142,7 @@ var_opt_char[0] = "unsigned char x_optimize"; var_opt_char[1] = "unsigned char x_optimize_size"; var_opt_char[2] = "unsigned char x_optimize_debug"; var_opt_char[3] = "unsigned char x_optimize_fast"; +var_opt_int[0] = "unsigned int x_flag_sanitize"; for (i = 0; i < n_opts; i++) { if (flag_set_p("(Optimization|PerFunction)", flags[i])) { base-commit: 13ea4a6e830da1f245136601e636dec62e74d1a7 prerequisite-patch-id: f75da3aa4e66d8b85562d5dd9ae35c5429c1ea74 -- 2.27.0 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH RFA] ubsan: default to trap on unreachable at -O0 and -Og [PR104642] 2022-06-13 19:53 [PATCH RFA] ubsan: default to trap on unreachable at -O0 and -Og [PR104642] Jason Merrill @ 2022-06-14 11:44 ` Jakub Jelinek 2022-06-15 20:38 ` Jason Merrill 2022-06-16 20:32 ` Jonathan Wakely 0 siblings, 2 replies; 12+ messages in thread From: Jakub Jelinek @ 2022-06-14 11:44 UTC (permalink / raw) To: Jason Merrill; +Cc: gcc-patches, richard.guenther, jwakely.gcc 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH RFA] ubsan: default to trap on unreachable at -O0 and -Og [PR104642] 2022-06-14 11:44 ` Jakub Jelinek @ 2022-06-15 20:38 ` Jason Merrill 2022-06-16 13:14 ` Jakub Jelinek 2022-06-16 20:32 ` Jonathan Wakely 1 sibling, 1 reply; 12+ messages in thread From: Jason Merrill @ 2022-06-15 20:38 UTC (permalink / raw) To: Jakub Jelinek; +Cc: gcc-patches, richard.guenther, jwakely.gcc On 6/14/22 07:44, Jakub Jelinek wrote: > 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. Makes sense. The first is -fsanitize-recover=, I think, the second is the default, and perhaps the third could be -fsanitize-trap=? > 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. I'm happy to drop that part. > 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. The ubsan pass is not enabled for unreachable|return. sanopt does a single pass over the function to rewrite __builtin_unreachable, but that doesn't seem like much overhead. > 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) I tried this approach, but it misses some __builtin_unreachable calls added by e.g. execute_fixup_cfg; it seems they never get folded by any subsequent pass. It seems to me that we want -funreachable-traps (or whatever spelling) to have exactly the same effect as the current -fsanitize=unreachable,return -fsanitize-undefined-trap-on-error, so using an entirely novel implementation strategy seems wrong. OTOH, maybe the sanitizer should also hook into fold_builtin_0 to rewrite many of the calls before any of the optimizers run, so we don't need to explicitly check SANITIZE_UNREACHABLE in e.g. optimize_unreachable. And I notice that we currently optimize away the call to f() in bool b; int f() { if (b) return 42; __builtin_unreachable (); } int main() { f(); } with -fsanitize=unreachable -O, so the test exits normally. > 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. Sure, but I generally prefer to change fewer places. Jason ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH RFA] ubsan: default to trap on unreachable at -O0 and -Og [PR104642] 2022-06-15 20:38 ` Jason Merrill @ 2022-06-16 13:14 ` Jakub Jelinek 2022-06-20 20:30 ` Jason Merrill 0 siblings, 1 reply; 12+ messages in thread From: Jakub Jelinek @ 2022-06-16 13:14 UTC (permalink / raw) To: Jason Merrill; +Cc: gcc-patches, richard.guenther, jwakely.gcc On Wed, Jun 15, 2022 at 04:38:49PM -0400, Jason Merrill wrote: > > 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. > > Makes sense. The first is -fsanitize-recover=, I think, the second is the > default, and perhaps the third could be -fsanitize-trap=? -f{,no-}sanitize-recover= is a bitmask, whether some sanitizer recovers or not. The default is that ubsan sanitizers except for unreachable and return default to recover on and similarly kasan and hwkasan, other sanitizers default to no recover. If we add -f{,no-}sanitize-trap= similar way, we'd need to figure out what it means if a both bits are set (i.e. we are asked to recover and trap at the same time). We could error, or silently prefer trap over recover, etc. And we'd need to define interaction with -fsanitize-undefined-trap-on-error, would that be effectively an alias for -fsanitize-trap=all (but if so, we'd need the silent trap takes priority over recover way)? > > 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. > > The ubsan pass is not enabled for unreachable|return. sanopt does a single You're right. > pass over the function to rewrite __builtin_unreachable, but that doesn't > seem like much overhead. But I think we are trying to avoid hard any kind of unnecessary whole IL extra walks, especially for -O0. > > 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) > > I tried this approach, but it misses some __builtin_unreachable calls added > by e.g. execute_fixup_cfg; it seems they never get folded by any subsequent > pass. We could also expand BUILT_IN_UNREACHABLE as BUILT_IN_TRAP during expansion to catch whatever isn't caught by folding. > > 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. > > Sure, but I generally prefer to change fewer places. I'd say this would be very small change and the fastest + most reliable. Simply replace all builtin_decl_implicit (BUILT_IN_UNREACHABLE) calls with builtin_decl_unreachable () (12 of them) and define tree builtin_decl_unreachable () { enum built_in_function fncode = BUILT_IN_UNREACHABLE; if (sanitize_flag_p (SANITIZE_UNREACHABLE)) { if (flag_sanitize_undefined_trap_on_error) fncode = BUILT_IN_TRAP; /* Otherwise we want __builtin_unreachable () later folded into __ubsan_handle_builtin_unreachable with extra args. */ } else if (flag_unreachable_traps) fncode = BUILT_IN_TRAP; return builtin_decl_implicit (fncode); } and that's it (well, also in build_common_builtin_nodes declare __builtin_trap for FEs that don't do that - like it is done for __builtin_unreachable). Jakub ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH RFA] ubsan: default to trap on unreachable at -O0 and -Og [PR104642] 2022-06-16 13:14 ` Jakub Jelinek @ 2022-06-20 20:30 ` Jason Merrill 2022-06-21 11:17 ` Jakub Jelinek 0 siblings, 1 reply; 12+ messages in thread From: Jason Merrill @ 2022-06-20 20:30 UTC (permalink / raw) To: Jakub Jelinek; +Cc: gcc-patches, richard.guenther, jwakely.gcc [-- Attachment #1: Type: text/plain, Size: 2739 bytes --] On 6/16/22 09:14, Jakub Jelinek wrote: > On Wed, Jun 15, 2022 at 04:38:49PM -0400, Jason Merrill wrote: >>> 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. >> >> The ubsan pass is not enabled for unreachable|return. sanopt does a single > > You're right. > >> pass over the function to rewrite __builtin_unreachable, but that doesn't >> seem like much overhead. > > But I think we are trying to avoid hard any kind of unnecessary whole IL > extra walks, especially for -O0. OK. >>> 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) >> >> I tried this approach, but it misses some __builtin_unreachable calls added >> by e.g. execute_fixup_cfg; it seems they never get folded by any subsequent >> pass. > > We could also expand BUILT_IN_UNREACHABLE as BUILT_IN_TRAP during expansion > to catch whatever isn't caught by folding. That was an early thing I tried, but that's too late to prevent it from being used for optimization. More recently I've put an assert in expand_builtin_unreachable to catch ones that slip past. >>> 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. >> >> Sure, but I generally prefer to change fewer places. > > I'd say this would be very small change and the fastest + most reliable. > Simply replace all builtin_decl_implicit (BUILT_IN_UNREACHABLE) calls > with builtin_decl_unreachable () (12 of them) and define > tree > builtin_decl_unreachable () > { > enum built_in_function fncode = BUILT_IN_UNREACHABLE; > > if (sanitize_flag_p (SANITIZE_UNREACHABLE)) > { > if (flag_sanitize_undefined_trap_on_error) > fncode = BUILT_IN_TRAP; > /* Otherwise we want __builtin_unreachable () later folded into > __ubsan_handle_builtin_unreachable with extra args. */ > } > else if (flag_unreachable_traps) > fncode = BUILT_IN_TRAP; > return builtin_decl_implicit (fncode); > } > and that's it (well, also in build_common_builtin_nodes > declare __builtin_trap for FEs that don't do that - like it is done > for __builtin_unreachable). OK, here's another version of the patch using that approach. [-- Attachment #2: 0001-ubsan-default-to-trap-on-unreachable-at-O0-and-Og-PR.patch --] [-- Type: text/x-patch, Size: 25185 bytes --] From 280713174b2bbda97ebd88cefa90d52df73813f5 Mon Sep 17 00:00:00 2001 From: Jason Merrill <jason@redhat.com> Date: Fri, 10 Jun 2022 16:35:21 -0400 Subject: [PATCH] ubsan: default to trap on unreachable at -O0 and -Og [PR104642] To: gcc-patches@gcc.gnu.org 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-trap=unreachable, 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; since it has custom parsing, that meant handling it explicitly in the awk scripts. Jakub observed that this would slow down -O0 by default from running the sanopt pass, so this revision avoids the need for sanopt by rewriting calls introduced by the compiler to __builtin_trap immediately, and calls written by the user at fold time. Many of the calls introduced by the compiler are also rewritten immediately to ubsan calls, which fixes ubsan-8b.C; previously the call to f() was optimized away before sanopt. But this early rewriting isn't practical for uses of __builtin_unreachable in devirtualization and such, so sanopt rewriting is still done for non-trapping sanitize. Do we still want -funreachable-traps as an alias for -fsanitize=unreachable,return -fsanitize-trap=unreachable,return? PR c++/104642 gcc/ChangeLog: * doc/invoke.texi (-fsanitize=unreachable): On by default at -O0. * opts.cc (finish_options): At -O0, trap on unreachable code. * optc-save-gen.awk, opth-gen.awk: Include flag_sanitize. * tree.cc (build_common_builtin_nodes): Add __builtin_trap. * sanopt.cc: Don't run for just SANITIZE_RETURN or SANITIZE_UNREACHABLE when trapping. * ubsan.cc (builtin_decl_unreachable): New. (unreachable_1): Factor out. (build_builtin_unreachable): Use it. (gimple_build_builtin_unreachable): Use it. (ubsan_instrument_unreachable): Use it. * builtins.cc (expand_builtin_unreachable): Add assert. (fold_builtin_0): Call build_builtin_unreachable. * tree.h (builtin_decl_unreachable) (gimple_build_builtin_unreachable) (build_builtin_unreachable): Declare. * cgraphunit.cc (walk_polymorphic_call_targets): Use them. * gimple-fold.cc (gimple_fold_call) (gimple_get_virt_method_for_vtable) * ipa-fnsummary.cc (redirect_to_unreachable) * ipa-prop.cc (ipa_make_edge_direct_to_target) (ipa_impossible_devirt_target) * ipa.cc (walk_polymorphic_call_targets) * tree-cfg.cc (pass_warn_function_return::execute) (execute_fixup_cfg) * tree-ssa-loop-ivcanon.cc (remove_exits_and_undefined_stmts) (unloop_loops) * tree-ssa-sccvn.cc (eliminate_dom_walker::eliminate_stmt): Likewise. gcc/cp/ChangeLog: * constexpr.cc (cxx_eval_builtin_function_call): Handle unreachable/trap earlier. * cp-gimplify.cc (cp_maybe_instrument_return): Use build_builtin_unreachable. gcc/testsuite/ChangeLog: * g++.dg/ubsan/return-8a.C: New test. * g++.dg/ubsan/return-8b.C: New test. * g++.dg/ubsan/return-8d.C: New test. * g++.dg/ubsan/return-8e.C: New test. --- gcc/doc/invoke.texi | 4 ++ gcc/tree.h | 5 ++ gcc/builtins.cc | 7 ++ gcc/cgraphunit.cc | 3 +- gcc/cp/constexpr.cc | 29 +++++---- gcc/cp/cp-gimplify.cc | 5 +- gcc/gimple-fold.cc | 7 +- gcc/ipa-fnsummary.cc | 4 +- gcc/ipa-prop.cc | 4 +- gcc/ipa.cc | 3 +- gcc/opts.cc | 11 ++++ gcc/sanopt.cc | 10 ++- gcc/testsuite/g++.dg/ubsan/return-8a.C | 17 +++++ gcc/testsuite/g++.dg/ubsan/return-8b.C | 17 +++++ gcc/testsuite/g++.dg/ubsan/return-8d.C | 17 +++++ gcc/testsuite/g++.dg/ubsan/return-8e.C | 18 ++++++ gcc/tree-cfg.cc | 7 +- gcc/tree-ssa-loop-ivcanon.cc | 7 +- gcc/tree-ssa-sccvn.cc | 2 +- gcc/tree.cc | 5 ++ gcc/ubsan.cc | 89 +++++++++++++++++++++----- gcc/optc-save-gen.awk | 8 ++- gcc/opth-gen.awk | 3 +- 23 files changed, 225 insertions(+), 57 deletions(-) create mode 100644 gcc/testsuite/g++.dg/ubsan/return-8a.C create mode 100644 gcc/testsuite/g++.dg/ubsan/return-8b.C create mode 100644 gcc/testsuite/g++.dg/ubsan/return-8d.C create mode 100644 gcc/testsuite/g++.dg/ubsan/return-8e.C diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 50f57877477..62b2f45ed64 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -15926,6 +15926,10 @@ With this option, the compiler turns the @code{__builtin_unreachable} call into a diagnostics message call instead. When reaching the @code{__builtin_unreachable} call, the behavior is undefined. +If @option{-fsanitize} has not been specified, this option is enabled +by default at @option{-O0} and @option{-Og}, along with +@option{-fsanitize=unreachable,return}. + @item -fsanitize=vla-bound @opindex fsanitize=vla-bound This option instructs the compiler to check that the size of a variable diff --git a/gcc/tree.h b/gcc/tree.h index 507ea252b95..b0aa6d961ee 100644 --- a/gcc/tree.h +++ b/gcc/tree.h @@ -5858,6 +5858,11 @@ builtin_decl_implicit (enum built_in_function fncode) return builtin_info[uns_fncode].decl; } +/* For BUILTIN_UNREACHABLE, use one of these instead of one of the above. */ +extern tree builtin_decl_unreachable (); +extern gcall *gimple_build_builtin_unreachable (location_t); +extern tree build_builtin_unreachable (location_t); + /* Set explicit builtin function nodes and whether it is an implicit function. */ diff --git a/gcc/builtins.cc b/gcc/builtins.cc index 971b18c3745..8d1002992f5 100644 --- a/gcc/builtins.cc +++ b/gcc/builtins.cc @@ -5184,6 +5184,7 @@ expand_builtin_trap (void) static void expand_builtin_unreachable (void) { + gcc_checking_assert (!sanitize_flags_p (SANITIZE_UNREACHABLE)); emit_barrier (); } @@ -9261,6 +9262,12 @@ fold_builtin_0 (location_t loc, tree fndecl) case BUILT_IN_CLASSIFY_TYPE: return fold_builtin_classify_type (NULL_TREE); + case BUILT_IN_UNREACHABLE: + /* Rewrite any explicit calls to __builtin_unreachable. */ + if (sanitize_flags_p (SANITIZE_UNREACHABLE)) + return build_builtin_unreachable (loc); + break; + default: break; } diff --git a/gcc/cgraphunit.cc b/gcc/cgraphunit.cc index e77bf97bea3..836e759cdf1 100644 --- a/gcc/cgraphunit.cc +++ b/gcc/cgraphunit.cc @@ -1033,8 +1033,7 @@ walk_polymorphic_call_targets (hash_set<void *> *reachable_call_targets, if (targets.length () == 1) target = targets[0]; else - target = cgraph_node::create - (builtin_decl_implicit (BUILT_IN_UNREACHABLE)); + target = cgraph_node::create (builtin_decl_unreachable ()); if (symtab->dump_file) { diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc index fd7f8c0fb88..0dc94d9445d 100644 --- a/gcc/cp/constexpr.cc +++ b/gcc/cp/constexpr.cc @@ -1438,6 +1438,20 @@ cxx_eval_builtin_function_call (const constexpr_ctx *ctx, tree t, tree fun, /* These builtins shall be ignored during constant expression evaluation. */ return void_node; + case BUILT_IN_UNREACHABLE: + case BUILT_IN_TRAP: + if (!*non_constant_p && !ctx->quiet) + { + /* Do not allow__builtin_unreachable in constexpr function. + The __builtin_unreachable call with BUILTINS_LOCATION + comes from cp_maybe_instrument_return. */ + if (EXPR_LOCATION (t) == BUILTINS_LOCATION) + error ("%<constexpr%> call flows off the end of the function"); + else + error ("%q+E is not a constant expression", t); + } + *non_constant_p = true; + return t; default: break; } @@ -1531,18 +1545,9 @@ cxx_eval_builtin_function_call (const constexpr_ctx *ctx, tree t, tree fun, { if (!*non_constant_p && !ctx->quiet) { - /* Do not allow__builtin_unreachable in constexpr function. - The __builtin_unreachable call with BUILTINS_LOCATION - comes from cp_maybe_instrument_return. */ - if (fndecl_built_in_p (fun, BUILT_IN_UNREACHABLE) - && EXPR_LOCATION (t) == BUILTINS_LOCATION) - error ("%<constexpr%> call flows off the end of the function"); - else - { - new_call = build_call_array_loc (EXPR_LOCATION (t), TREE_TYPE (t), - CALL_EXPR_FN (t), nargs, args); - error ("%q+E is not a constant expression", new_call); - } + new_call = build_call_array_loc (EXPR_LOCATION (t), TREE_TYPE (t), + CALL_EXPR_FN (t), nargs, args); + error ("%q+E is not a constant expression", new_call); } *non_constant_p = true; return t; diff --git a/gcc/cp/cp-gimplify.cc b/gcc/cp/cp-gimplify.cc index 6f84d157c98..7b0465729a3 100644 --- a/gcc/cp/cp-gimplify.cc +++ b/gcc/cp/cp-gimplify.cc @@ -1864,10 +1864,7 @@ cp_maybe_instrument_return (tree fndecl) if (sanitize_flags_p (SANITIZE_RETURN, fndecl)) t = ubsan_instrument_return (loc); else - { - tree fndecl = builtin_decl_explicit (BUILT_IN_UNREACHABLE); - t = build_call_expr_loc (BUILTINS_LOCATION, fndecl, 0); - } + t = build_builtin_unreachable (BUILTINS_LOCATION); append_to_statement_list (t, p); } diff --git a/gcc/gimple-fold.cc b/gcc/gimple-fold.cc index f61bc87da63..a1704784bc9 100644 --- a/gcc/gimple-fold.cc +++ b/gcc/gimple-fold.cc @@ -5510,9 +5510,8 @@ gimple_fold_call (gimple_stmt_iterator *gsi, bool inplace) } else { - tree fndecl = builtin_decl_implicit (BUILT_IN_UNREACHABLE); - gimple *new_stmt = gimple_build_call (fndecl, 0); - gimple_set_location (new_stmt, gimple_location (stmt)); + location_t loc = gimple_location (stmt); + gimple *new_stmt = gimple_build_builtin_unreachable (loc); /* If the call had a SSA name as lhs morph that into an uninitialized value. */ if (lhs && TREE_CODE (lhs) == SSA_NAME) @@ -8396,7 +8395,7 @@ gimple_get_virt_method_for_vtable (HOST_WIDE_INT token, if (!fn || (TREE_CODE (fn) != ADDR_EXPR && TREE_CODE (fn) != FDESC_EXPR) || TREE_CODE (TREE_OPERAND (fn, 0)) != FUNCTION_DECL) - fn = builtin_decl_implicit (BUILT_IN_UNREACHABLE); + fn = builtin_decl_unreachable (); else { fn = TREE_OPERAND (fn, 0); diff --git a/gcc/ipa-fnsummary.cc b/gcc/ipa-fnsummary.cc index b12e7a1124d..c9564451f26 100644 --- a/gcc/ipa-fnsummary.cc +++ b/gcc/ipa-fnsummary.cc @@ -250,8 +250,8 @@ static struct cgraph_edge * redirect_to_unreachable (struct cgraph_edge *e) { struct cgraph_node *callee = !e->inline_failed ? e->callee : NULL; - struct cgraph_node *target = cgraph_node::get_create - (builtin_decl_implicit (BUILT_IN_UNREACHABLE)); + struct cgraph_node *target + = cgraph_node::get_create (builtin_decl_unreachable ()); if (e->speculative) e = cgraph_edge::resolve_speculation (e, target->decl); diff --git a/gcc/ipa-prop.cc b/gcc/ipa-prop.cc index c037668e7d8..e1fc481423b 100644 --- a/gcc/ipa-prop.cc +++ b/gcc/ipa-prop.cc @@ -3410,7 +3410,7 @@ ipa_make_edge_direct_to_target (struct cgraph_edge *ie, tree target, ie->caller->dump_name ()); } - target = builtin_decl_implicit (BUILT_IN_UNREACHABLE); + target = builtin_decl_unreachable (); callee = cgraph_node::get_create (target); unreachable = true; } @@ -3821,7 +3821,7 @@ ipa_impossible_devirt_target (struct cgraph_edge *ie, tree target) "No devirtualization target in %s\n", ie->caller->dump_name ()); } - tree new_target = builtin_decl_implicit (BUILT_IN_UNREACHABLE); + tree new_target = builtin_decl_unreachable (); cgraph_node::get_create (new_target); return new_target; } diff --git a/gcc/ipa.cc b/gcc/ipa.cc index f53f15f5f0a..4d5729f8370 100644 --- a/gcc/ipa.cc +++ b/gcc/ipa.cc @@ -232,8 +232,7 @@ walk_polymorphic_call_targets (hash_set<void *> *reachable_call_targets, if (targets.length () == 1) target = targets[0]; else - target = cgraph_node::get_create - (builtin_decl_implicit (BUILT_IN_UNREACHABLE)); + target = cgraph_node::get_create (builtin_decl_unreachable ()); if (dump_enabled_p ()) { diff --git a/gcc/opts.cc b/gcc/opts.cc index 959d48d173f..d92699a1bc9 100644 --- a/gcc/opts.cc +++ b/gcc/opts.cc @@ -1122,6 +1122,17 @@ finish_options (struct gcc_options *opts, struct gcc_options *opts_set, opts->x_flag_no_inline = 1; } + /* At -O0 or -Og, turn __builtin_unreachable into a trap. */ + if (!opts_set->x_flag_sanitize) + { + if (!opts->x_optimize || opts->x_optimize_debug) + opts->x_flag_sanitize = SANITIZE_UNREACHABLE|SANITIZE_RETURN; + + /* Change this without regard to optimization level so we don't need to + deal with it in optc-save-gen.awk. */ + opts->x_flag_sanitize_trap = SANITIZE_UNREACHABLE|SANITIZE_RETURN; + } + /* Pipelining of outer loops is only possible when general pipelining capabilities are requested. */ if (!opts->x_flag_sel_sched_pipelining) diff --git a/gcc/sanopt.cc b/gcc/sanopt.cc index c3187631153..2b05553baeb 100644 --- a/gcc/sanopt.cc +++ b/gcc/sanopt.cc @@ -942,7 +942,15 @@ public: {} /* opt_pass methods: */ - virtual bool gate (function *) { return flag_sanitize; } + virtual bool gate (function *) + { + /* SANITIZE_RETURN is handled in the front-end. When trapping, + SANITIZE_UNREACHABLE is handled by builtin_decl_unreachable. */ + unsigned int mask = SANITIZE_RETURN; + if (flag_sanitize_trap & SANITIZE_UNREACHABLE) + mask |= SANITIZE_UNREACHABLE; + return flag_sanitize & ~mask; + } virtual unsigned int execute (function *); }; // class pass_sanopt diff --git a/gcc/testsuite/g++.dg/ubsan/return-8a.C b/gcc/testsuite/g++.dg/ubsan/return-8a.C new file mode 100644 index 00000000000..5ecb6f4189e --- /dev/null +++ b/gcc/testsuite/g++.dg/ubsan/return-8a.C @@ -0,0 +1,17 @@ +// PR c++/104642 + +// At -O0 and -Og we default to +// -fsanitize=unreachable,return -fsanitize-trap=unreachable,return +// so the below should abort at runtime. + +// { dg-do run } +// { dg-shouldfail { *-*-* } } +// { dg-additional-options "-O0" } + +bool b; + +int f() { + if (b) return 42; +} // { dg-warning "-Wreturn-type" } + +int main() { f(); } diff --git a/gcc/testsuite/g++.dg/ubsan/return-8b.C b/gcc/testsuite/g++.dg/ubsan/return-8b.C new file mode 100644 index 00000000000..bdaea60f809 --- /dev/null +++ b/gcc/testsuite/g++.dg/ubsan/return-8b.C @@ -0,0 +1,17 @@ +// PR c++/104642 + +// With -fsanitize=unreachable we shouldn't optimize away the call to f. + +// { dg-do run } +// { dg-shouldfail { *-*-* } } +// { dg-additional-options "-O -fsanitize=unreachable" } + +bool b; + +int f() { + if (b) return 42; + __builtin_unreachable (); + return 24; +} + +int main() { f(); } diff --git a/gcc/testsuite/g++.dg/ubsan/return-8d.C b/gcc/testsuite/g++.dg/ubsan/return-8d.C new file mode 100644 index 00000000000..74d86ce337c --- /dev/null +++ b/gcc/testsuite/g++.dg/ubsan/return-8d.C @@ -0,0 +1,17 @@ +// PR c++/104642 + +// At -O0 and -Og we default to +// -fsanitize=unreachable,return -fsanitize-trap=unreachable,return +// so the below should abort at runtime. + +// { dg-do run } +// { dg-shouldfail { *-*-* } } +// { dg-additional-options "-Og" } + +bool b; + +int f() { + if (b) return 42; +} // { dg-warning "-Wreturn-type" } + +int main() { f(); } diff --git a/gcc/testsuite/g++.dg/ubsan/return-8e.C b/gcc/testsuite/g++.dg/ubsan/return-8e.C new file mode 100644 index 00000000000..d32e2c1d518 --- /dev/null +++ b/gcc/testsuite/g++.dg/ubsan/return-8e.C @@ -0,0 +1,18 @@ +// PR c++/104642 + +// At -O0 and -Og we default to +// -fsanitize=unreachable,return -fsanitize-trap=unreachable,return +// so the below should abort at runtime. + +// { dg-do run } +// { dg-shouldfail { *-*-* } } +// { dg-additional-options "-O2" } + +bool b; + +__attribute__ ((optimize ("Og"))) +int f() { + if (b) return 42; +} // { dg-warning "-Wreturn-type" } + +int main() { f(); } diff --git a/gcc/tree-cfg.cc b/gcc/tree-cfg.cc index c67c278dad0..734fdddbf97 100644 --- a/gcc/tree-cfg.cc +++ b/gcc/tree-cfg.cc @@ -9503,9 +9503,8 @@ pass_warn_function_return::execute (function *fun) with __builtin_unreachable () call. */ if (optimize && gimple_code (last) == GIMPLE_RETURN) { - tree fndecl = builtin_decl_implicit (BUILT_IN_UNREACHABLE); - gimple *new_stmt = gimple_build_call (fndecl, 0); - gimple_set_location (new_stmt, gimple_location (last)); + location_t loc = gimple_location (last); + gimple *new_stmt = gimple_build_builtin_unreachable (loc); gimple_stmt_iterator gsi = gsi_for_stmt (last); gsi_replace (&gsi, new_stmt, true); remove_edge (e); @@ -9834,7 +9833,7 @@ execute_fixup_cfg (void) { if (stmt && is_gimple_call (stmt)) gimple_call_set_ctrl_altering (stmt, false); - tree fndecl = builtin_decl_implicit (BUILT_IN_UNREACHABLE); + tree fndecl = builtin_decl_unreachable (); stmt = gimple_build_call (fndecl, 0); gimple_stmt_iterator gsi = gsi_last_bb (bb); gsi_insert_after (&gsi, stmt, GSI_NEW_STMT); diff --git a/gcc/tree-ssa-loop-ivcanon.cc b/gcc/tree-ssa-loop-ivcanon.cc index 2ee00a3f843..6a38b77ab7a 100644 --- a/gcc/tree-ssa-loop-ivcanon.cc +++ b/gcc/tree-ssa-loop-ivcanon.cc @@ -505,9 +505,8 @@ remove_exits_and_undefined_stmts (class loop *loop, unsigned int npeeled) && wi::ltu_p (elt->bound, npeeled)) { gimple_stmt_iterator gsi = gsi_for_stmt (elt->stmt); - gcall *stmt = gimple_build_call - (builtin_decl_implicit (BUILT_IN_UNREACHABLE), 0); - gimple_set_location (stmt, gimple_location (elt->stmt)); + location_t loc = gimple_location (elt->stmt); + gcall *stmt = gimple_build_builtin_unreachable (loc); gsi_insert_before (&gsi, stmt, GSI_NEW_STMT); split_block (gimple_bb (stmt), stmt); changed = true; @@ -641,7 +640,7 @@ unloop_loops (bitmap loop_closed_ssa_invalidated, /* Create new basic block for the latch edge destination and wire it in. */ - stmt = gimple_build_call (builtin_decl_implicit (BUILT_IN_UNREACHABLE), 0); + stmt = gimple_build_builtin_unreachable (locus); latch_edge = make_edge (latch, create_basic_block (NULL, NULL, latch), flags); latch_edge->probability = profile_probability::never (); latch_edge->flags |= flags; diff --git a/gcc/tree-ssa-sccvn.cc b/gcc/tree-ssa-sccvn.cc index ed68557f0b2..776dccbbf5a 100644 --- a/gcc/tree-ssa-sccvn.cc +++ b/gcc/tree-ssa-sccvn.cc @@ -6807,7 +6807,7 @@ eliminate_dom_walker::eliminate_stmt (basic_block b, gimple_stmt_iterator *gsi) if (targets.length () == 1) fn = targets[0]->decl; else - fn = builtin_decl_implicit (BUILT_IN_UNREACHABLE); + fn = builtin_decl_unreachable (); if (dump_enabled_p ()) { dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, stmt, diff --git a/gcc/tree.cc b/gcc/tree.cc index 2bfb67489c6..51831f80512 100644 --- a/gcc/tree.cc +++ b/gcc/tree.cc @@ -9649,6 +9649,7 @@ build_common_builtin_nodes (void) } if (!builtin_decl_explicit_p (BUILT_IN_UNREACHABLE) + || !builtin_decl_explicit_p (BUILT_IN_TRAP) || !builtin_decl_explicit_p (BUILT_IN_ABORT)) { ftype = build_function_type (void_type_node, void_list_node); @@ -9662,6 +9663,10 @@ build_common_builtin_nodes (void) local_define_builtin ("__builtin_abort", ftype, BUILT_IN_ABORT, "abort", ECF_LEAF | ECF_NORETURN | ECF_CONST | ECF_COLD); + if (!builtin_decl_explicit_p (BUILT_IN_TRAP)) + local_define_builtin ("__builtin_trap", ftype, BUILT_IN_TRAP, + "__builtin_trap", + ECF_NORETURN | ECF_NOTHROW | ECF_LEAF | ECF_COLD); } if (!builtin_decl_explicit_p (BUILT_IN_MEMCPY) diff --git a/gcc/ubsan.cc b/gcc/ubsan.cc index 3aa25b534bb..4a34b8fee58 100644 --- a/gcc/ubsan.cc +++ b/gcc/ubsan.cc @@ -638,27 +638,84 @@ ubsan_create_data (const char *name, int loccnt, const location_t *ploc, ...) return var; } -/* Instrument the __builtin_unreachable call. We just call the libubsan - routine instead. */ +/* The built-in decl to use to mark code points believed to be unreachable. + Typically __builtin_unreachable, but __builtin_trap if + -fsanitize=unreachable -fsanitize-trap=unreachable. If only + -fsanitize=unreachable, we rely on sanopt to replace any calls with the + appropriate ubsan function. When building a call directly, use + {gimple_},build_builtin_unreachable instead. */ + +tree +builtin_decl_unreachable () +{ + enum built_in_function fncode = BUILT_IN_UNREACHABLE; + + if (sanitize_flags_p (SANITIZE_UNREACHABLE)) + { + if (flag_sanitize_trap & SANITIZE_UNREACHABLE) + fncode = BUILT_IN_TRAP; + /* Otherwise we want __builtin_unreachable () later folded into + __ubsan_handle_builtin_unreachable with extra args. */ + } + return builtin_decl_explicit (fncode); +} + +/* Shared between *build_builtin_unreachable. */ + +static void +unreachable_1 (tree &fn, tree &data, location_t loc) +{ + if (!sanitize_flags_p (SANITIZE_UNREACHABLE)) + { + fn = builtin_decl_explicit (BUILT_IN_UNREACHABLE); + data = NULL_TREE; + } + else if (flag_sanitize_trap & SANITIZE_UNREACHABLE) + { + fn = builtin_decl_explicit (BUILT_IN_TRAP); + data = NULL_TREE; + } + else + { + fn = builtin_decl_explicit (BUILT_IN_UBSAN_HANDLE_BUILTIN_UNREACHABLE); + data = ubsan_create_data ("__ubsan_unreachable_data", 1, &loc, + NULL_TREE, NULL_TREE); + data = build_fold_addr_expr_loc (loc, data); + } +} + +/* Build a call to __builtin_unreachable as rewritten by + -fsanitize=unreachable. */ + +tree +build_builtin_unreachable (location_t loc) +{ + tree fn, data; + unreachable_1 (fn, data, loc); + return build_call_expr_loc (loc, fn, data != NULL_TREE, data); +} + +/* Build a gcall to __builtin_unreachable as rewritten by + -fsanitize=unreachable. */ + +gcall * +gimple_build_builtin_unreachable (location_t loc) +{ + tree fn, data; + unreachable_1 (fn, data, loc); + gcall *g = gimple_build_call (fn, data != NULL_TREE, data); + gimple_set_location (g, loc); + return g; +} + +/* Rewrite a gcall to __builtin_unreachable for -fsanitize=unreachable. Called + by the sanopt pass. */ bool ubsan_instrument_unreachable (gimple_stmt_iterator *gsi) { - gimple *g; location_t loc = gimple_location (gsi_stmt (*gsi)); - - if (flag_sanitize_trap & SANITIZE_UNREACHABLE) - g = gimple_build_call (builtin_decl_explicit (BUILT_IN_TRAP), 0); - else - { - tree data = ubsan_create_data ("__ubsan_unreachable_data", 1, &loc, - NULL_TREE, NULL_TREE); - data = build_fold_addr_expr_loc (loc, data); - tree fn - = builtin_decl_explicit (BUILT_IN_UBSAN_HANDLE_BUILTIN_UNREACHABLE); - g = gimple_build_call (fn, 1, data); - } - gimple_set_location (g, loc); + gimple *g = gimple_build_builtin_unreachable (loc); gsi_replace (gsi, g, false); return false; } diff --git a/gcc/optc-save-gen.awk b/gcc/optc-save-gen.awk index 233d1fbb637..38c02bcc2cf 100644 --- a/gcc/optc-save-gen.awk +++ b/gcc/optc-save-gen.awk @@ -84,7 +84,7 @@ print "{"; n_opt_char = 4; n_opt_short = 0; -n_opt_int = 0; +n_opt_int = 1; n_opt_enum = 0; n_opt_string = 0; n_opt_other = 0; @@ -96,6 +96,7 @@ var_opt_range["optimize"] = "0, 255"; var_opt_range["optimize_size"] = "0, 2"; var_opt_range["optimize_debug"] = "0, 1"; var_opt_range["optimize_fast"] = "0, 1"; +var_opt_int[0] = "flag_sanitize"; # Sort by size to mimic how the structure is laid out to be friendlier to the # cache. @@ -1264,7 +1265,7 @@ for (i = 0; i < n_target_str; i++) { } print "}"; -n_opt_val = 4; +n_opt_val = 5; var_opt_val[0] = "x_optimize" var_opt_val_type[0] = "char " var_opt_hash[0] = 1; @@ -1277,6 +1278,9 @@ var_opt_hash[2] = 1; var_opt_val[3] = "x_optimize_fast" var_opt_val_type[3] = "char " var_opt_hash[3] = 1; +var_opt_val[4] = "x_flag_sanitize" +var_opt_val_type[4] = "unsigned int " +var_opt_hash[4] = 1; for (i = 0; i < n_opts; i++) { if (flag_set_p("(Optimization|PerFunction)", flags[i])) { name = var_name(flags[i]) diff --git a/gcc/opth-gen.awk b/gcc/opth-gen.awk index 8bba8ec4549..b3bedaa6da2 100644 --- a/gcc/opth-gen.awk +++ b/gcc/opth-gen.awk @@ -134,7 +134,7 @@ print "{"; n_opt_char = 4; n_opt_short = 0; -n_opt_int = 0; +n_opt_int = 1; n_opt_enum = 0; n_opt_other = 0; n_opt_explicit = 4; @@ -142,6 +142,7 @@ var_opt_char[0] = "unsigned char x_optimize"; var_opt_char[1] = "unsigned char x_optimize_size"; var_opt_char[2] = "unsigned char x_optimize_debug"; var_opt_char[3] = "unsigned char x_optimize_fast"; +var_opt_int[0] = "unsigned int x_flag_sanitize"; for (i = 0; i < n_opts; i++) { if (flag_set_p("(Optimization|PerFunction)", flags[i])) { -- 2.27.0 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH RFA] ubsan: default to trap on unreachable at -O0 and -Og [PR104642] 2022-06-20 20:30 ` Jason Merrill @ 2022-06-21 11:17 ` Jakub Jelinek 2022-06-22 3:59 ` Jason Merrill 0 siblings, 1 reply; 12+ messages in thread From: Jakub Jelinek @ 2022-06-21 11:17 UTC (permalink / raw) To: Jason Merrill; +Cc: gcc-patches, richard.guenther, jwakely.gcc On Mon, Jun 20, 2022 at 04:30:51PM -0400, Jason Merrill wrote: I'd still prefer to see a separate -funreachable-traps. The thing is that -fsanitize{,-recover,-trap}= are global options, not per function (and only tweaked by no_sanitize attribute), while something that needs to depend on the per-function -O0/-Og setting is necessarily per function. The *.awk changes I understand make -fsanitize= kind of per function but -fsanitize-{recover,trap}= remain global, that is going to be a nightmare especially with LTO which saves/restores the per function flags and for the global ones merges them across TUs. By separating sanitizers (which would remain global with no_sanitize overrides) from -funreachable-traps which would be Optimization option (with default set if unset in default_options_optimization or so) saved/restored upon function changes that issue is gone. > --- a/gcc/tree.h > +++ b/gcc/tree.h > @@ -5858,6 +5858,11 @@ builtin_decl_implicit (enum built_in_function fncode) > return builtin_info[uns_fncode].decl; > } > > +/* For BUILTIN_UNREACHABLE, use one of these instead of one of the above. */ > +extern tree builtin_decl_unreachable (); > +extern gcall *gimple_build_builtin_unreachable (location_t); > +extern tree build_builtin_unreachable (location_t); I think we generally try to declare functions in the header with same basename as the source file in which they are defined. So, the question is if builtin_decl_unreachable and build_builtin_unreachable shouldn't be defined in tree.cc and declared in tree.h and gimple_build_builtin_unreachable in gimple.cc and declared in gimple.h, using a helper defined in ubsan.cc and declared in ubsan.h (your current unreachable_1). > + > /* Set explicit builtin function nodes and whether it is an implicit > function. */ > > --- a/gcc/builtins.cc > +++ b/gcc/builtins.cc > --- a/gcc/cgraphunit.cc > +++ b/gcc/cgraphunit.cc > --- a/gcc/cp/constexpr.cc > +++ b/gcc/cp/constexpr.cc > --- a/gcc/cp/cp-gimplify.cc > +++ b/gcc/cp/cp-gimplify.cc > --- a/gcc/gimple-fold.cc > +++ b/gcc/gimple-fold.cc > --- a/gcc/ipa-fnsummary.cc > +++ b/gcc/ipa-fnsummary.cc > --- a/gcc/ipa-prop.cc > +++ b/gcc/ipa-prop.cc > --- a/gcc/ipa.cc > +++ b/gcc/ipa.cc The above changes LGTM. > if (dump_enabled_p ()) > { > diff --git a/gcc/opts.cc b/gcc/opts.cc > index 959d48d173f..d92699a1bc9 100644 > --- a/gcc/opts.cc > +++ b/gcc/opts.cc > @@ -1122,6 +1122,17 @@ finish_options (struct gcc_options *opts, struct gcc_options *opts_set, > opts->x_flag_no_inline = 1; > } > > + /* At -O0 or -Og, turn __builtin_unreachable into a trap. */ > + if (!opts_set->x_flag_sanitize) > + { > + if (!opts->x_optimize || opts->x_optimize_debug) > + opts->x_flag_sanitize = SANITIZE_UNREACHABLE|SANITIZE_RETURN; > + > + /* Change this without regard to optimization level so we don't need to > + deal with it in optc-save-gen.awk. */ > + opts->x_flag_sanitize_trap = SANITIZE_UNREACHABLE|SANITIZE_RETURN; > + } > + > /* Pipelining of outer loops is only possible when general pipelining > capabilities are requested. */ > if (!opts->x_flag_sel_sched_pipelining) See above. > --- a/gcc/sanopt.cc > +++ b/gcc/sanopt.cc > @@ -942,7 +942,15 @@ public: > {} > > /* opt_pass methods: */ > - virtual bool gate (function *) { return flag_sanitize; } > + virtual bool gate (function *) > + { > + /* SANITIZE_RETURN is handled in the front-end. When trapping, > + SANITIZE_UNREACHABLE is handled by builtin_decl_unreachable. */ > + unsigned int mask = SANITIZE_RETURN; There are other sanitizers purely handled in the FEs, guess as a follow-up we should look at which of them don't really need any sanopt handling. > + if (flag_sanitize_trap & SANITIZE_UNREACHABLE) > + mask |= SANITIZE_UNREACHABLE; > + return flag_sanitize & ~mask; > + } > --- a/gcc/tree-cfg.cc > +++ b/gcc/tree-cfg.cc > --- a/gcc/tree-ssa-loop-ivcanon.cc > +++ b/gcc/tree-ssa-loop-ivcanon.cc > --- a/gcc/tree-ssa-sccvn.cc > +++ b/gcc/tree-ssa-sccvn.cc > --- a/gcc/tree.cc > +++ b/gcc/tree.cc LGTM. > --- a/gcc/ubsan.cc > +++ b/gcc/ubsan.cc > @@ -638,27 +638,84 @@ ubsan_create_data (const char *name, int loccnt, const location_t *ploc, ...) > return var; > } > > -/* Instrument the __builtin_unreachable call. We just call the libubsan > - routine instead. */ > +/* The built-in decl to use to mark code points believed to be unreachable. > + Typically __builtin_unreachable, but __builtin_trap if > + -fsanitize=unreachable -fsanitize-trap=unreachable. If only > + -fsanitize=unreachable, we rely on sanopt to replace any calls with the > + appropriate ubsan function. When building a call directly, use > + {gimple_},build_builtin_unreachable instead. */ > + > +tree > +builtin_decl_unreachable () > +{ > + enum built_in_function fncode = BUILT_IN_UNREACHABLE; > + > + if (sanitize_flags_p (SANITIZE_UNREACHABLE)) > + { > + if (flag_sanitize_trap & SANITIZE_UNREACHABLE) > + fncode = BUILT_IN_TRAP; > + /* Otherwise we want __builtin_unreachable () later folded into > + __ubsan_handle_builtin_unreachable with extra args. */ > + } I'd add the flag_unreachable_traps stuff here as else > +/* Shared between *build_builtin_unreachable. */ > + > +static void > +unreachable_1 (tree &fn, tree &data, location_t loc) Besides the potential moving, I think the coding guidelines don't recommend using references that way. But even if it is used, wouldn't it be better to return fn instead of void and just set data (either using reference or taking address of data)? Jakub ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH RFA] ubsan: default to trap on unreachable at -O0 and -Og [PR104642] 2022-06-21 11:17 ` Jakub Jelinek @ 2022-06-22 3:59 ` Jason Merrill 2022-06-22 8:05 ` Jakub Jelinek 0 siblings, 1 reply; 12+ messages in thread From: Jason Merrill @ 2022-06-22 3:59 UTC (permalink / raw) To: Jakub Jelinek; +Cc: gcc-patches, richard.guenther, jwakely.gcc [-- Attachment #1: Type: text/plain, Size: 5897 bytes --] On 6/21/22 07:17, Jakub Jelinek wrote: > On Mon, Jun 20, 2022 at 04:30:51PM -0400, Jason Merrill wrote: > I'd still prefer to see a separate -funreachable-traps. > The thing is that -fsanitize{,-recover,-trap}= are global options, not per > function (and only tweaked by no_sanitize attribute), while something > that needs to depend on the per-function -O0/-Og setting is necessarily per > function. The *.awk changes I understand make -fsanitize= kind of per > function but -fsanitize-{recover,trap}= remain global, that is going to be a > nightmare especially with LTO which saves/restores the per function flags > and for the global ones merges them across TUs. > By separating sanitizers (which would remain global with no_sanitize > overrides) from -funreachable-traps which would be Optimization option > (with default set if unset in default_options_optimization or so) > saved/restored upon function changes that issue is gone. Done. >> --- a/gcc/tree.h >> +++ b/gcc/tree.h >> @@ -5858,6 +5858,11 @@ builtin_decl_implicit (enum built_in_function fncode) >> return builtin_info[uns_fncode].decl; >> } >> >> +/* For BUILTIN_UNREACHABLE, use one of these instead of one of the above. */ >> +extern tree builtin_decl_unreachable (); >> +extern gcall *gimple_build_builtin_unreachable (location_t); >> +extern tree build_builtin_unreachable (location_t); > > I think we generally try to declare functions in the header with same > basename as the source file in which they are defined. > So, the question is if builtin_decl_unreachable and build_builtin_unreachable > shouldn't be defined in tree.cc and declared in tree.h and > gimple_build_builtin_unreachable in gimple.cc and declared in gimple.h, > using a helper defined in ubsan.cc and declared in ubsan.h (your current > unreachable_1). Done. >> + >> /* Set explicit builtin function nodes and whether it is an implicit >> function. */ >> >> --- a/gcc/builtins.cc >> +++ b/gcc/builtins.cc >> --- a/gcc/cgraphunit.cc >> +++ b/gcc/cgraphunit.cc >> --- a/gcc/cp/constexpr.cc >> +++ b/gcc/cp/constexpr.cc >> --- a/gcc/cp/cp-gimplify.cc >> +++ b/gcc/cp/cp-gimplify.cc >> --- a/gcc/gimple-fold.cc >> +++ b/gcc/gimple-fold.cc >> --- a/gcc/ipa-fnsummary.cc >> +++ b/gcc/ipa-fnsummary.cc >> --- a/gcc/ipa-prop.cc >> +++ b/gcc/ipa-prop.cc >> --- a/gcc/ipa.cc >> +++ b/gcc/ipa.cc > > The above changes LGTM. >> if (dump_enabled_p ()) >> { >> diff --git a/gcc/opts.cc b/gcc/opts.cc >> index 959d48d173f..d92699a1bc9 100644 >> --- a/gcc/opts.cc >> +++ b/gcc/opts.cc >> @@ -1122,6 +1122,17 @@ finish_options (struct gcc_options *opts, struct gcc_options *opts_set, >> opts->x_flag_no_inline = 1; >> } >> >> + /* At -O0 or -Og, turn __builtin_unreachable into a trap. */ >> + if (!opts_set->x_flag_sanitize) >> + { >> + if (!opts->x_optimize || opts->x_optimize_debug) >> + opts->x_flag_sanitize = SANITIZE_UNREACHABLE|SANITIZE_RETURN; >> + >> + /* Change this without regard to optimization level so we don't need to >> + deal with it in optc-save-gen.awk. */ >> + opts->x_flag_sanitize_trap = SANITIZE_UNREACHABLE|SANITIZE_RETURN; >> + } >> + >> /* Pipelining of outer loops is only possible when general pipelining >> capabilities are requested. */ >> if (!opts->x_flag_sel_sched_pipelining) > > See above. > >> --- a/gcc/sanopt.cc >> +++ b/gcc/sanopt.cc >> @@ -942,7 +942,15 @@ public: >> {} >> >> /* opt_pass methods: */ >> - virtual bool gate (function *) { return flag_sanitize; } >> + virtual bool gate (function *) >> + { >> + /* SANITIZE_RETURN is handled in the front-end. When trapping, >> + SANITIZE_UNREACHABLE is handled by builtin_decl_unreachable. */ >> + unsigned int mask = SANITIZE_RETURN; > > There are other sanitizers purely handled in the FEs, guess as a follow-up > we should look at which of them don't really need any sanopt handling. > >> + if (flag_sanitize_trap & SANITIZE_UNREACHABLE) >> + mask |= SANITIZE_UNREACHABLE; >> + return flag_sanitize & ~mask; >> + } >> --- a/gcc/tree-cfg.cc >> +++ b/gcc/tree-cfg.cc >> --- a/gcc/tree-ssa-loop-ivcanon.cc >> +++ b/gcc/tree-ssa-loop-ivcanon.cc >> --- a/gcc/tree-ssa-sccvn.cc >> +++ b/gcc/tree-ssa-sccvn.cc >> --- a/gcc/tree.cc >> +++ b/gcc/tree.cc > > LGTM. > >> --- a/gcc/ubsan.cc >> +++ b/gcc/ubsan.cc >> @@ -638,27 +638,84 @@ ubsan_create_data (const char *name, int loccnt, const location_t *ploc, ...) >> return var; >> } >> >> -/* Instrument the __builtin_unreachable call. We just call the libubsan >> - routine instead. */ >> +/* The built-in decl to use to mark code points believed to be unreachable. >> + Typically __builtin_unreachable, but __builtin_trap if >> + -fsanitize=unreachable -fsanitize-trap=unreachable. If only >> + -fsanitize=unreachable, we rely on sanopt to replace any calls with the >> + appropriate ubsan function. When building a call directly, use >> + {gimple_},build_builtin_unreachable instead. */ >> + >> +tree >> +builtin_decl_unreachable () >> +{ >> + enum built_in_function fncode = BUILT_IN_UNREACHABLE; >> + >> + if (sanitize_flags_p (SANITIZE_UNREACHABLE)) >> + { >> + if (flag_sanitize_trap & SANITIZE_UNREACHABLE) >> + fncode = BUILT_IN_TRAP; >> + /* Otherwise we want __builtin_unreachable () later folded into >> + __ubsan_handle_builtin_unreachable with extra args. */ >> + } > > I'd add the flag_unreachable_traps stuff here as else > >> +/* Shared between *build_builtin_unreachable. */ >> + >> +static void >> +unreachable_1 (tree &fn, tree &data, location_t loc) > > Besides the potential moving, I think the coding guidelines don't recommend > using references that way. But even if it is used, wouldn't it be better > to return fn instead of void and just set data (either using reference > or taking address of data)? Done. [-- Attachment #2: 0001-ubsan-default-to-trap-on-unreachable-at-O0-and-Og-PR.patch --] [-- Type: text/x-patch, Size: 26659 bytes --] From 5eef7f71ee8acd4929e1c4b121c4f8c58251a591 Mon Sep 17 00:00:00 2001 From: Jason Merrill <jason@redhat.com> Date: Fri, 10 Jun 2022 16:35:21 -0400 Subject: [PATCH] ubsan: default to trap on unreachable at -O0 and -Og [PR104642] To: gcc-patches@gcc.gnu.org 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. I think it also makes sense to do this when we're explicitly optimizing for the debugging experience. In the PR richi suggested introducing an -funreachable-traps flag for this. This functionality is already implemented as -fsanitize=unreachable -fsanitize-trap=unreachable, and we want to share the implementation, but it does seem useful to have a separate flag that isn't affected by the various sanitization controls. Jakub observed that this would slow down -O0 by default from running the sanopt pass, so this revision avoids the need for sanopt by rewriting calls introduced by the compiler immediately, and calls written by the user at fold time. Many of the calls introduced by the compiler are also rewritten immediately to ubsan calls when not trapping, which fixes ubsan-8b.C; previously the call to f() was optimized away before sanopt. But this early rewriting isn't practical for uses of __builtin_unreachable in devirtualization and such, so sanopt rewriting is still done for non-trapping sanitize. In this patch -fsanitize=unreachable overrides -funreachable-traps; we could also choose to go the other way. PR c++/104642 gcc/ChangeLog: * common.opt: Add -funreachable-traps. * doc/invoke.texi (-funreachable-traps): Document it. * opts.cc (finish_options): Enable at -O0 or -Og. * tree.cc (build_common_builtin_nodes): Add __builtin_trap. (builtin_decl_unreachable, build_builtin_unreachable): New. * tree.h: Declare them. * ubsan.cc (sanitize_unreachable_fn): Factor out. (ubsan_instrument_unreachable): Use gimple_build_builtin_unreachable. * ubsan.h (sanitize_unreachable_fn): Declare. * gimple.cc (gimple_build_builtin_unreachable): New. * gimple.h: Declare it. * builtins.cc (expand_builtin_unreachable): Add assert. (fold_builtin_0): Call build_builtin_unreachable. * sanopt.cc: Don't run for just SANITIZE_RETURN or SANITIZE_UNREACHABLE when trapping. * cgraphunit.cc (walk_polymorphic_call_targets): Use new unreachable functions. * gimple-fold.cc (gimple_fold_call) (gimple_get_virt_method_for_vtable) * ipa-fnsummary.cc (redirect_to_unreachable) * ipa-prop.cc (ipa_make_edge_direct_to_target) (ipa_impossible_devirt_target) * ipa.cc (walk_polymorphic_call_targets) * tree-cfg.cc (pass_warn_function_return::execute) (execute_fixup_cfg) * tree-ssa-loop-ivcanon.cc (remove_exits_and_undefined_stmts) (unloop_loops) * tree-ssa-sccvn.cc (eliminate_dom_walker::eliminate_stmt): Likewise. gcc/cp/ChangeLog: * constexpr.cc (cxx_eval_builtin_function_call): Handle unreachable/trap earlier. * cp-gimplify.cc (cp_maybe_instrument_return): Use build_builtin_unreachable. gcc/testsuite/ChangeLog: * g++.dg/ubsan/return-8a.C: New test. * g++.dg/ubsan/return-8b.C: New test. * g++.dg/ubsan/return-8d.C: New test. * g++.dg/ubsan/return-8e.C: New test. --- gcc/doc/invoke.texi | 14 ++++++++ gcc/common.opt | 4 +++ gcc/gimple.h | 1 + gcc/tree.h | 5 +++ gcc/ubsan.h | 1 + gcc/builtins.cc | 9 +++++ gcc/cgraphunit.cc | 3 +- gcc/cp/constexpr.cc | 29 +++++++++------- gcc/cp/cp-gimplify.cc | 7 ++-- gcc/gimple-fold.cc | 7 ++-- gcc/gimple.cc | 13 +++++++ gcc/ipa-fnsummary.cc | 4 +-- gcc/ipa-prop.cc | 4 +-- gcc/ipa.cc | 3 +- gcc/opts.cc | 4 +++ gcc/sanopt.cc | 10 +++++- gcc/testsuite/g++.dg/ubsan/return-8a.C | 16 +++++++++ gcc/testsuite/g++.dg/ubsan/return-8b.C | 17 +++++++++ gcc/testsuite/g++.dg/ubsan/return-8d.C | 16 +++++++++ gcc/testsuite/g++.dg/ubsan/return-8e.C | 17 +++++++++ gcc/tree-cfg.cc | 7 ++-- gcc/tree-ssa-loop-ivcanon.cc | 7 ++-- gcc/tree-ssa-sccvn.cc | 2 +- gcc/tree.cc | 40 +++++++++++++++++++++ gcc/ubsan.cc | 48 +++++++++++++++++--------- 25 files changed, 233 insertions(+), 55 deletions(-) create mode 100644 gcc/testsuite/g++.dg/ubsan/return-8a.C create mode 100644 gcc/testsuite/g++.dg/ubsan/return-8b.C create mode 100644 gcc/testsuite/g++.dg/ubsan/return-8d.C create mode 100644 gcc/testsuite/g++.dg/ubsan/return-8e.C diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 81d13f4e78e..e077eaa773c 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -12939,6 +12939,20 @@ also at @option{-O0} if @option{-fsection-anchors} is explicitly requested. Additionally @option{-fno-toplevel-reorder} implies @option{-fno-section-anchors}. +@item -funreachable-traps +@opindex funreachable-traps +With this option, the compiler turns calls to +@code{__builtin_unreachable} into traps, instead of using them for +optimization. This also affects any such calls implicitly generated +by the compiler. + +This option has the same effect as @option{-fsanitize=unreachable +-fsanitize-trap=unreachable}, but does not affect the values of those +options. If @option{-fsanitize=unreachable} is enabled, that option +takes priority over this one. + +This option is enabled by default at @option{-O0} and @option{-Og}. + @item -fweb @opindex fweb Constructs webs as commonly used for register allocation purposes and assign diff --git a/gcc/common.opt b/gcc/common.opt index 32917aafcae..da5d23d1de8 100644 --- a/gcc/common.opt +++ b/gcc/common.opt @@ -3017,6 +3017,10 @@ funit-at-a-time Common Var(flag_unit_at_a_time) Init(1) Compile whole compilation unit at a time. +funreachable-traps +Common Var(flag_unreachable_traps) Optimization +Trap on __builtin_unreachable instead of using it for optimization. + funroll-loops Common Var(flag_unroll_loops) Optimization EnabledBy(funroll-all-loops) Perform loop unrolling when iteration count is known. diff --git a/gcc/gimple.h b/gcc/gimple.h index 870629cd562..1d15ff98ac2 100644 --- a/gcc/gimple.h +++ b/gcc/gimple.h @@ -1638,6 +1638,7 @@ extern void maybe_remove_unused_call_args (struct function *, gimple *); extern bool gimple_inexpensive_call_p (gcall *); extern bool stmt_can_terminate_bb_p (gimple *); extern location_t gimple_or_expr_nonartificial_location (gimple *, tree); +gcall *gimple_build_builtin_unreachable (location_t); /* Return the disposition for a warning (or all warnings by default) for a statement. */ diff --git a/gcc/tree.h b/gcc/tree.h index 507ea252b95..6f6ad5a3a5f 100644 --- a/gcc/tree.h +++ b/gcc/tree.h @@ -5858,6 +5858,11 @@ builtin_decl_implicit (enum built_in_function fncode) return builtin_info[uns_fncode].decl; } +/* For BUILTIN_UNREACHABLE, use one of these or + gimple_build_builtin_unreachable instead of one of the above. */ +extern tree builtin_decl_unreachable (); +extern tree build_builtin_unreachable (location_t); + /* Set explicit builtin function nodes and whether it is an implicit function. */ diff --git a/gcc/ubsan.h b/gcc/ubsan.h index 17c5254c17e..71a40a399e4 100644 --- a/gcc/ubsan.h +++ b/gcc/ubsan.h @@ -65,5 +65,6 @@ extern tree ubsan_build_overflow_builtin (tree_code, location_t, tree, tree, tree, tree *); extern tree ubsan_instrument_float_cast (location_t, tree, tree); extern tree ubsan_get_source_location_type (void); +extern tree sanitize_unreachable_fn (tree *data, location_t loc); #endif /* GCC_UBSAN_H */ diff --git a/gcc/builtins.cc b/gcc/builtins.cc index 971b18c3745..e6816d5c865 100644 --- a/gcc/builtins.cc +++ b/gcc/builtins.cc @@ -5184,6 +5184,9 @@ expand_builtin_trap (void) static void expand_builtin_unreachable (void) { + /* Use gimple_build_builtin_unreachable or builtin_decl_unreachable + to avoid this. */ + gcc_checking_assert (!sanitize_flags_p (SANITIZE_UNREACHABLE)); emit_barrier (); } @@ -9261,6 +9264,12 @@ fold_builtin_0 (location_t loc, tree fndecl) case BUILT_IN_CLASSIFY_TYPE: return fold_builtin_classify_type (NULL_TREE); + case BUILT_IN_UNREACHABLE: + /* Rewrite any explicit calls to __builtin_unreachable. */ + if (sanitize_flags_p (SANITIZE_UNREACHABLE)) + return build_builtin_unreachable (loc); + break; + default: break; } diff --git a/gcc/cgraphunit.cc b/gcc/cgraphunit.cc index e77bf97bea3..836e759cdf1 100644 --- a/gcc/cgraphunit.cc +++ b/gcc/cgraphunit.cc @@ -1033,8 +1033,7 @@ walk_polymorphic_call_targets (hash_set<void *> *reachable_call_targets, if (targets.length () == 1) target = targets[0]; else - target = cgraph_node::create - (builtin_decl_implicit (BUILT_IN_UNREACHABLE)); + target = cgraph_node::create (builtin_decl_unreachable ()); if (symtab->dump_file) { diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc index fd7f8c0fb88..0dc94d9445d 100644 --- a/gcc/cp/constexpr.cc +++ b/gcc/cp/constexpr.cc @@ -1438,6 +1438,20 @@ cxx_eval_builtin_function_call (const constexpr_ctx *ctx, tree t, tree fun, /* These builtins shall be ignored during constant expression evaluation. */ return void_node; + case BUILT_IN_UNREACHABLE: + case BUILT_IN_TRAP: + if (!*non_constant_p && !ctx->quiet) + { + /* Do not allow__builtin_unreachable in constexpr function. + The __builtin_unreachable call with BUILTINS_LOCATION + comes from cp_maybe_instrument_return. */ + if (EXPR_LOCATION (t) == BUILTINS_LOCATION) + error ("%<constexpr%> call flows off the end of the function"); + else + error ("%q+E is not a constant expression", t); + } + *non_constant_p = true; + return t; default: break; } @@ -1531,18 +1545,9 @@ cxx_eval_builtin_function_call (const constexpr_ctx *ctx, tree t, tree fun, { if (!*non_constant_p && !ctx->quiet) { - /* Do not allow__builtin_unreachable in constexpr function. - The __builtin_unreachable call with BUILTINS_LOCATION - comes from cp_maybe_instrument_return. */ - if (fndecl_built_in_p (fun, BUILT_IN_UNREACHABLE) - && EXPR_LOCATION (t) == BUILTINS_LOCATION) - error ("%<constexpr%> call flows off the end of the function"); - else - { - new_call = build_call_array_loc (EXPR_LOCATION (t), TREE_TYPE (t), - CALL_EXPR_FN (t), nargs, args); - error ("%q+E is not a constant expression", new_call); - } + new_call = build_call_array_loc (EXPR_LOCATION (t), TREE_TYPE (t), + CALL_EXPR_FN (t), nargs, args); + error ("%q+E is not a constant expression", new_call); } *non_constant_p = true; return t; diff --git a/gcc/cp/cp-gimplify.cc b/gcc/cp/cp-gimplify.cc index 6f84d157c98..c05be833357 100644 --- a/gcc/cp/cp-gimplify.cc +++ b/gcc/cp/cp-gimplify.cc @@ -1814,7 +1814,7 @@ cp_maybe_instrument_return (tree fndecl) information is provided, while the __builtin_unreachable () below if return sanitization is disabled will just result in hard to understand runtime error without location. */ - && (!optimize + && ((!optimize && !flag_unreachable_traps) || sanitize_flags_p (SANITIZE_UNREACHABLE, fndecl))) return; @@ -1864,10 +1864,7 @@ cp_maybe_instrument_return (tree fndecl) if (sanitize_flags_p (SANITIZE_RETURN, fndecl)) t = ubsan_instrument_return (loc); else - { - tree fndecl = builtin_decl_explicit (BUILT_IN_UNREACHABLE); - t = build_call_expr_loc (BUILTINS_LOCATION, fndecl, 0); - } + t = build_builtin_unreachable (BUILTINS_LOCATION); append_to_statement_list (t, p); } diff --git a/gcc/gimple-fold.cc b/gcc/gimple-fold.cc index f61bc87da63..a1704784bc9 100644 --- a/gcc/gimple-fold.cc +++ b/gcc/gimple-fold.cc @@ -5510,9 +5510,8 @@ gimple_fold_call (gimple_stmt_iterator *gsi, bool inplace) } else { - tree fndecl = builtin_decl_implicit (BUILT_IN_UNREACHABLE); - gimple *new_stmt = gimple_build_call (fndecl, 0); - gimple_set_location (new_stmt, gimple_location (stmt)); + location_t loc = gimple_location (stmt); + gimple *new_stmt = gimple_build_builtin_unreachable (loc); /* If the call had a SSA name as lhs morph that into an uninitialized value. */ if (lhs && TREE_CODE (lhs) == SSA_NAME) @@ -8396,7 +8395,7 @@ gimple_get_virt_method_for_vtable (HOST_WIDE_INT token, if (!fn || (TREE_CODE (fn) != ADDR_EXPR && TREE_CODE (fn) != FDESC_EXPR) || TREE_CODE (TREE_OPERAND (fn, 0)) != FUNCTION_DECL) - fn = builtin_decl_implicit (BUILT_IN_UNREACHABLE); + fn = builtin_decl_unreachable (); else { fn = TREE_OPERAND (fn, 0); diff --git a/gcc/gimple.cc b/gcc/gimple.cc index b70ab4d2523..9b156399ba1 100644 --- a/gcc/gimple.cc +++ b/gcc/gimple.cc @@ -44,6 +44,7 @@ along with GCC; see the file COPYING3. If not see #include "stringpool.h" #include "attribs.h" #include "asan.h" +#include "ubsan.h" #include "langhooks.h" #include "attr-fnspec.h" #include "ipa-modref-tree.h" @@ -421,6 +422,18 @@ gimple_build_call_from_tree (tree t, tree fnptrtype) return call; } +/* Build a gcall to __builtin_unreachable as rewritten by + -fsanitize=unreachable. */ + +gcall * +gimple_build_builtin_unreachable (location_t loc) +{ + tree data = NULL_TREE; + tree fn = sanitize_unreachable_fn (&data, loc); + gcall *g = gimple_build_call (fn, data != NULL_TREE, data); + gimple_set_location (g, loc); + return g; +} /* Build a GIMPLE_ASSIGN statement. diff --git a/gcc/ipa-fnsummary.cc b/gcc/ipa-fnsummary.cc index b12e7a1124d..c9564451f26 100644 --- a/gcc/ipa-fnsummary.cc +++ b/gcc/ipa-fnsummary.cc @@ -250,8 +250,8 @@ static struct cgraph_edge * redirect_to_unreachable (struct cgraph_edge *e) { struct cgraph_node *callee = !e->inline_failed ? e->callee : NULL; - struct cgraph_node *target = cgraph_node::get_create - (builtin_decl_implicit (BUILT_IN_UNREACHABLE)); + struct cgraph_node *target + = cgraph_node::get_create (builtin_decl_unreachable ()); if (e->speculative) e = cgraph_edge::resolve_speculation (e, target->decl); diff --git a/gcc/ipa-prop.cc b/gcc/ipa-prop.cc index c037668e7d8..e1fc481423b 100644 --- a/gcc/ipa-prop.cc +++ b/gcc/ipa-prop.cc @@ -3410,7 +3410,7 @@ ipa_make_edge_direct_to_target (struct cgraph_edge *ie, tree target, ie->caller->dump_name ()); } - target = builtin_decl_implicit (BUILT_IN_UNREACHABLE); + target = builtin_decl_unreachable (); callee = cgraph_node::get_create (target); unreachable = true; } @@ -3821,7 +3821,7 @@ ipa_impossible_devirt_target (struct cgraph_edge *ie, tree target) "No devirtualization target in %s\n", ie->caller->dump_name ()); } - tree new_target = builtin_decl_implicit (BUILT_IN_UNREACHABLE); + tree new_target = builtin_decl_unreachable (); cgraph_node::get_create (new_target); return new_target; } diff --git a/gcc/ipa.cc b/gcc/ipa.cc index f53f15f5f0a..4d5729f8370 100644 --- a/gcc/ipa.cc +++ b/gcc/ipa.cc @@ -232,8 +232,7 @@ walk_polymorphic_call_targets (hash_set<void *> *reachable_call_targets, if (targets.length () == 1) target = targets[0]; else - target = cgraph_node::get_create - (builtin_decl_implicit (BUILT_IN_UNREACHABLE)); + target = cgraph_node::get_create (builtin_decl_unreachable ()); if (dump_enabled_p ()) { diff --git a/gcc/opts.cc b/gcc/opts.cc index 959d48d173f..9982974c8b5 100644 --- a/gcc/opts.cc +++ b/gcc/opts.cc @@ -1122,6 +1122,10 @@ finish_options (struct gcc_options *opts, struct gcc_options *opts_set, opts->x_flag_no_inline = 1; } + /* At -O0 or -Og, turn __builtin_unreachable into a trap. */ + if (!opts->x_optimize || opts->x_optimize_debug) + SET_OPTION_IF_UNSET (opts, opts_set, flag_unreachable_traps, true); + /* Pipelining of outer loops is only possible when general pipelining capabilities are requested. */ if (!opts->x_flag_sel_sched_pipelining) diff --git a/gcc/sanopt.cc b/gcc/sanopt.cc index c3187631153..2b05553baeb 100644 --- a/gcc/sanopt.cc +++ b/gcc/sanopt.cc @@ -942,7 +942,15 @@ public: {} /* opt_pass methods: */ - virtual bool gate (function *) { return flag_sanitize; } + virtual bool gate (function *) + { + /* SANITIZE_RETURN is handled in the front-end. When trapping, + SANITIZE_UNREACHABLE is handled by builtin_decl_unreachable. */ + unsigned int mask = SANITIZE_RETURN; + if (flag_sanitize_trap & SANITIZE_UNREACHABLE) + mask |= SANITIZE_UNREACHABLE; + return flag_sanitize & ~mask; + } virtual unsigned int execute (function *); }; // class pass_sanopt diff --git a/gcc/testsuite/g++.dg/ubsan/return-8a.C b/gcc/testsuite/g++.dg/ubsan/return-8a.C new file mode 100644 index 00000000000..54f1c7b5326 --- /dev/null +++ b/gcc/testsuite/g++.dg/ubsan/return-8a.C @@ -0,0 +1,16 @@ +// PR c++/104642 + +// At -O0 and -Og we default to -funreachable-traps +// so the below should abort at runtime. + +// { dg-do run } +// { dg-shouldfail { *-*-* } } +// { dg-additional-options "-O0" } + +bool b; + +int f() { + if (b) return 42; +} // { dg-warning "-Wreturn-type" } + +int main() { f(); } diff --git a/gcc/testsuite/g++.dg/ubsan/return-8b.C b/gcc/testsuite/g++.dg/ubsan/return-8b.C new file mode 100644 index 00000000000..bdaea60f809 --- /dev/null +++ b/gcc/testsuite/g++.dg/ubsan/return-8b.C @@ -0,0 +1,17 @@ +// PR c++/104642 + +// With -fsanitize=unreachable we shouldn't optimize away the call to f. + +// { dg-do run } +// { dg-shouldfail { *-*-* } } +// { dg-additional-options "-O -fsanitize=unreachable" } + +bool b; + +int f() { + if (b) return 42; + __builtin_unreachable (); + return 24; +} + +int main() { f(); } diff --git a/gcc/testsuite/g++.dg/ubsan/return-8d.C b/gcc/testsuite/g++.dg/ubsan/return-8d.C new file mode 100644 index 00000000000..7eaded035ae --- /dev/null +++ b/gcc/testsuite/g++.dg/ubsan/return-8d.C @@ -0,0 +1,16 @@ +// PR c++/104642 + +// At -O0 and -Og we default to -funreachable-traps +// so the below should abort at runtime. + +// { dg-do run } +// { dg-shouldfail { *-*-* } } +// { dg-additional-options "-Og" } + +bool b; + +int f() { + if (b) return 42; +} // { dg-warning "-Wreturn-type" } + +int main() { f(); } diff --git a/gcc/testsuite/g++.dg/ubsan/return-8e.C b/gcc/testsuite/g++.dg/ubsan/return-8e.C new file mode 100644 index 00000000000..fba402b35f2 --- /dev/null +++ b/gcc/testsuite/g++.dg/ubsan/return-8e.C @@ -0,0 +1,17 @@ +// PR c++/104642 + +// At -O0 and -Og we default to -funreachable-traps +// so the below should abort at runtime. + +// { dg-do run } +// { dg-shouldfail { *-*-* } } +// { dg-additional-options "-O2" } + +bool b; + +__attribute__ ((optimize ("Og"))) +int f() { + if (b) return 42; +} // { dg-warning "-Wreturn-type" } + +int main() { f(); } diff --git a/gcc/tree-cfg.cc b/gcc/tree-cfg.cc index c67c278dad0..734fdddbf97 100644 --- a/gcc/tree-cfg.cc +++ b/gcc/tree-cfg.cc @@ -9503,9 +9503,8 @@ pass_warn_function_return::execute (function *fun) with __builtin_unreachable () call. */ if (optimize && gimple_code (last) == GIMPLE_RETURN) { - tree fndecl = builtin_decl_implicit (BUILT_IN_UNREACHABLE); - gimple *new_stmt = gimple_build_call (fndecl, 0); - gimple_set_location (new_stmt, gimple_location (last)); + location_t loc = gimple_location (last); + gimple *new_stmt = gimple_build_builtin_unreachable (loc); gimple_stmt_iterator gsi = gsi_for_stmt (last); gsi_replace (&gsi, new_stmt, true); remove_edge (e); @@ -9834,7 +9833,7 @@ execute_fixup_cfg (void) { if (stmt && is_gimple_call (stmt)) gimple_call_set_ctrl_altering (stmt, false); - tree fndecl = builtin_decl_implicit (BUILT_IN_UNREACHABLE); + tree fndecl = builtin_decl_unreachable (); stmt = gimple_build_call (fndecl, 0); gimple_stmt_iterator gsi = gsi_last_bb (bb); gsi_insert_after (&gsi, stmt, GSI_NEW_STMT); diff --git a/gcc/tree-ssa-loop-ivcanon.cc b/gcc/tree-ssa-loop-ivcanon.cc index 2ee00a3f843..6a38b77ab7a 100644 --- a/gcc/tree-ssa-loop-ivcanon.cc +++ b/gcc/tree-ssa-loop-ivcanon.cc @@ -505,9 +505,8 @@ remove_exits_and_undefined_stmts (class loop *loop, unsigned int npeeled) && wi::ltu_p (elt->bound, npeeled)) { gimple_stmt_iterator gsi = gsi_for_stmt (elt->stmt); - gcall *stmt = gimple_build_call - (builtin_decl_implicit (BUILT_IN_UNREACHABLE), 0); - gimple_set_location (stmt, gimple_location (elt->stmt)); + location_t loc = gimple_location (elt->stmt); + gcall *stmt = gimple_build_builtin_unreachable (loc); gsi_insert_before (&gsi, stmt, GSI_NEW_STMT); split_block (gimple_bb (stmt), stmt); changed = true; @@ -641,7 +640,7 @@ unloop_loops (bitmap loop_closed_ssa_invalidated, /* Create new basic block for the latch edge destination and wire it in. */ - stmt = gimple_build_call (builtin_decl_implicit (BUILT_IN_UNREACHABLE), 0); + stmt = gimple_build_builtin_unreachable (locus); latch_edge = make_edge (latch, create_basic_block (NULL, NULL, latch), flags); latch_edge->probability = profile_probability::never (); latch_edge->flags |= flags; diff --git a/gcc/tree-ssa-sccvn.cc b/gcc/tree-ssa-sccvn.cc index ed68557f0b2..776dccbbf5a 100644 --- a/gcc/tree-ssa-sccvn.cc +++ b/gcc/tree-ssa-sccvn.cc @@ -6807,7 +6807,7 @@ eliminate_dom_walker::eliminate_stmt (basic_block b, gimple_stmt_iterator *gsi) if (targets.length () == 1) fn = targets[0]->decl; else - fn = builtin_decl_implicit (BUILT_IN_UNREACHABLE); + fn = builtin_decl_unreachable (); if (dump_enabled_p ()) { dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, stmt, diff --git a/gcc/tree.cc b/gcc/tree.cc index 2bfb67489c6..84000dd8b69 100644 --- a/gcc/tree.cc +++ b/gcc/tree.cc @@ -71,6 +71,8 @@ along with GCC; see the file COPYING3. If not see #include "gimple-range.h" #include "gomp-constants.h" #include "dfp.h" +#include "asan.h" +#include "ubsan.h" /* Tree code classes. */ @@ -9649,6 +9651,7 @@ build_common_builtin_nodes (void) } if (!builtin_decl_explicit_p (BUILT_IN_UNREACHABLE) + || !builtin_decl_explicit_p (BUILT_IN_TRAP) || !builtin_decl_explicit_p (BUILT_IN_ABORT)) { ftype = build_function_type (void_type_node, void_list_node); @@ -9662,6 +9665,10 @@ build_common_builtin_nodes (void) local_define_builtin ("__builtin_abort", ftype, BUILT_IN_ABORT, "abort", ECF_LEAF | ECF_NORETURN | ECF_CONST | ECF_COLD); + if (!builtin_decl_explicit_p (BUILT_IN_TRAP)) + local_define_builtin ("__builtin_trap", ftype, BUILT_IN_TRAP, + "__builtin_trap", + ECF_NORETURN | ECF_NOTHROW | ECF_LEAF | ECF_COLD); } if (!builtin_decl_explicit_p (BUILT_IN_MEMCPY) @@ -10779,6 +10786,39 @@ build_alloca_call_expr (tree size, unsigned int align, HOST_WIDE_INT max_size) } } +/* The built-in decl to use to mark code points believed to be unreachable. + Typically __builtin_unreachable, but __builtin_trap if + -fsanitize=unreachable -fsanitize-trap=unreachable. If only + -fsanitize=unreachable, we rely on sanopt to replace calls with the + appropriate ubsan function. When building a call directly, use + {gimple_},build_builtin_unreachable instead. */ + +tree +builtin_decl_unreachable () +{ + enum built_in_function fncode = BUILT_IN_UNREACHABLE; + + if (sanitize_flags_p (SANITIZE_UNREACHABLE) + ? (flag_sanitize_trap & SANITIZE_UNREACHABLE) + : flag_unreachable_traps) + fncode = BUILT_IN_TRAP; + /* For non-trapping sanitize, we will rewrite __builtin_unreachable () later, + in the sanopt pass. */ + + return builtin_decl_explicit (fncode); +} + +/* Build a call to __builtin_unreachable, possibly rewritten by + -fsanitize=unreachable. Use this rather than the above when practical. */ + +tree +build_builtin_unreachable (location_t loc) +{ + tree data = NULL_TREE; + tree fn = sanitize_unreachable_fn (&data, loc); + return build_call_expr_loc (loc, fn, data != NULL_TREE, data); +} + /* Create a new constant string literal of type ELTYPE[SIZE] (or LEN if SIZE == -1) and return a tree node representing char* pointer to it as an ADDR_EXPR (ARRAY_REF (ELTYPE, ...)). When STR is nonnull diff --git a/gcc/ubsan.cc b/gcc/ubsan.cc index 3aa25b534bb..f15026872a1 100644 --- a/gcc/ubsan.cc +++ b/gcc/ubsan.cc @@ -638,27 +638,43 @@ ubsan_create_data (const char *name, int loccnt, const location_t *ploc, ...) return var; } -/* Instrument the __builtin_unreachable call. We just call the libubsan - routine instead. */ +/* Shared between *build_builtin_unreachable. */ + +tree +sanitize_unreachable_fn (tree *data, location_t loc) +{ + tree fn = NULL_TREE; + bool san = sanitize_flags_p (SANITIZE_UNREACHABLE); + if (san + ? (flag_sanitize_trap & SANITIZE_UNREACHABLE) + : flag_unreachable_traps) + { + fn = builtin_decl_explicit (BUILT_IN_TRAP); + *data = NULL_TREE; + } + else if (san) + { + fn = builtin_decl_explicit (BUILT_IN_UBSAN_HANDLE_BUILTIN_UNREACHABLE); + *data = ubsan_create_data ("__ubsan_unreachable_data", 1, &loc, + NULL_TREE, NULL_TREE); + *data = build_fold_addr_expr_loc (loc, *data); + } + else + { + fn = builtin_decl_explicit (BUILT_IN_UNREACHABLE); + *data = NULL_TREE; + } + return fn; +} + +/* Rewrite a gcall to __builtin_unreachable for -fsanitize=unreachable. Called + by the sanopt pass. */ bool ubsan_instrument_unreachable (gimple_stmt_iterator *gsi) { - gimple *g; location_t loc = gimple_location (gsi_stmt (*gsi)); - - if (flag_sanitize_trap & SANITIZE_UNREACHABLE) - g = gimple_build_call (builtin_decl_explicit (BUILT_IN_TRAP), 0); - else - { - tree data = ubsan_create_data ("__ubsan_unreachable_data", 1, &loc, - NULL_TREE, NULL_TREE); - data = build_fold_addr_expr_loc (loc, data); - tree fn - = builtin_decl_explicit (BUILT_IN_UBSAN_HANDLE_BUILTIN_UNREACHABLE); - g = gimple_build_call (fn, 1, data); - } - gimple_set_location (g, loc); + gimple *g = gimple_build_builtin_unreachable (loc); gsi_replace (gsi, g, false); return false; } -- 2.27.0 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH RFA] ubsan: default to trap on unreachable at -O0 and -Og [PR104642] 2022-06-22 3:59 ` Jason Merrill @ 2022-06-22 8:05 ` Jakub Jelinek 0 siblings, 0 replies; 12+ messages in thread From: Jakub Jelinek @ 2022-06-22 8:05 UTC (permalink / raw) To: Jason Merrill; +Cc: gcc-patches, richard.guenther, jwakely.gcc On Tue, Jun 21, 2022 at 11:59:56PM -0400, Jason Merrill wrote: > PR c++/104642 > > gcc/ChangeLog: > > * common.opt: Add -funreachable-traps. > * doc/invoke.texi (-funreachable-traps): Document it. > * opts.cc (finish_options): Enable at -O0 or -Og. > * tree.cc (build_common_builtin_nodes): Add __builtin_trap. > (builtin_decl_unreachable, build_builtin_unreachable): New. > * tree.h: Declare them. > * ubsan.cc (sanitize_unreachable_fn): Factor out. > (ubsan_instrument_unreachable): Use > gimple_build_builtin_unreachable. > * ubsan.h (sanitize_unreachable_fn): Declare. > * gimple.cc (gimple_build_builtin_unreachable): New. > * gimple.h: Declare it. > * builtins.cc (expand_builtin_unreachable): Add assert. > (fold_builtin_0): Call build_builtin_unreachable. > * sanopt.cc: Don't run for just SANITIZE_RETURN > or SANITIZE_UNREACHABLE when trapping. > * cgraphunit.cc (walk_polymorphic_call_targets): Use new > unreachable functions. > * gimple-fold.cc (gimple_fold_call) > (gimple_get_virt_method_for_vtable) > * ipa-fnsummary.cc (redirect_to_unreachable) > * ipa-prop.cc (ipa_make_edge_direct_to_target) > (ipa_impossible_devirt_target) > * ipa.cc (walk_polymorphic_call_targets) > * tree-cfg.cc (pass_warn_function_return::execute) > (execute_fixup_cfg) > * tree-ssa-loop-ivcanon.cc (remove_exits_and_undefined_stmts) > (unloop_loops) > * tree-ssa-sccvn.cc (eliminate_dom_walker::eliminate_stmt): > Likewise. > > gcc/cp/ChangeLog: > > * constexpr.cc (cxx_eval_builtin_function_call): Handle > unreachable/trap earlier. > * cp-gimplify.cc (cp_maybe_instrument_return): Use > build_builtin_unreachable. > > gcc/testsuite/ChangeLog: > > * g++.dg/ubsan/return-8a.C: New test. > * g++.dg/ubsan/return-8b.C: New test. > * g++.dg/ubsan/return-8d.C: New test. > * g++.dg/ubsan/return-8e.C: New test. LGTM, thanks. Jakub ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH RFA] ubsan: default to trap on unreachable at -O0 and -Og [PR104642] 2022-06-14 11:44 ` Jakub Jelinek 2022-06-15 20:38 ` Jason Merrill @ 2022-06-16 20:32 ` Jonathan Wakely 2022-06-16 20:53 ` Jakub Jelinek 2022-06-17 15:34 ` [PATCH] ubsan: Add -fsanitize-trap= support Jakub Jelinek 1 sibling, 2 replies; 12+ messages in thread From: Jonathan Wakely @ 2022-06-16 20:32 UTC (permalink / raw) To: Jakub Jelinek; +Cc: Jason Merrill, gcc-patches, Richard Biener On Tue, 14 Jun 2022, 12:44 Jakub Jelinek, <jakub@redhat.com> wrote: > 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. It looks like clang has addressed this deficiency now: https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html#usage You can choose a different outcome for different checks. They also have a smaller, intended-for-production runtime now: https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html#minimal-runtime ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH RFA] ubsan: default to trap on unreachable at -O0 and -Og [PR104642] 2022-06-16 20:32 ` Jonathan Wakely @ 2022-06-16 20:53 ` Jakub Jelinek 2022-06-17 15:34 ` [PATCH] ubsan: Add -fsanitize-trap= support Jakub Jelinek 1 sibling, 0 replies; 12+ messages in thread From: Jakub Jelinek @ 2022-06-16 20:53 UTC (permalink / raw) To: Jonathan Wakely; +Cc: Jason Merrill, gcc-patches, Richard Biener On Thu, Jun 16, 2022 at 09:32:02PM +0100, Jonathan Wakely wrote: > It looks like clang has addressed this deficiency now: > > https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html#usage Thanks, will study how it works tomorrow. Jakub ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] ubsan: Add -fsanitize-trap= support 2022-06-16 20:32 ` Jonathan Wakely 2022-06-16 20:53 ` Jakub Jelinek @ 2022-06-17 15:34 ` Jakub Jelinek 2022-06-17 18:21 ` Jason Merrill 1 sibling, 1 reply; 12+ messages in thread From: Jakub Jelinek @ 2022-06-17 15:34 UTC (permalink / raw) To: Richard Biener, Jeff Law, Jonathan Wakely Cc: gcc-patches, Martin Liška, Marek Polacek On Thu, Jun 16, 2022 at 09:32:02PM +0100, Jonathan Wakely wrote: > It looks like clang has addressed this deficiency now: > > https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html#usage Thanks, that is roughly what I'd implement anyway and apparently they have it already since 2015, we've added the -fsanitize-undefined-trap-on-error support back in 2014 and didn't change it since then. As a small divergence from clang, I chose -fsanitize-undefined-trap-on-error to be a (deprecated) alias for -fsanitize-trap aka -fsanitize-trap=all rather thn -fsanitize-trap=undefined which seems to be what clang does, because for a deprecated option it is IMHO more important backwards compatibility with what gcc did over the past 8 years rather than clang compatibility. Some sanitizers (e.g. asan, lsan, tsan) don't support traps, -fsanitize-trap=address etc. will be rejected (if enabled at the end of command line), -fno-sanitize-trap= can be specified even for them. This is similar behavior to -fsanitize-recover=. One complication is vptr sanitization, which can't easily trap, as the whole slow path of the checking is inside of libubsan. Previously, -fsanitize=vptr -fsanitize-undefined-trap-on-error silently ignored vptr sanitization. This patch similarly to what clang does will accept -fsanitize-trap=all or -fsanitize-trap=undefined which enable the vptr bit as trapping and again that causes silent disabling of vptr sanitization, while -fsanitize-trap=vptr is rejected (already during option processing). So far quickly tested with make check-gcc check-g++ RUNTESTFLAGS=ubsan.exp, ok for trunk if it passes full bootstrap/regtest? 2022-06-17 Jakub Jelinek <jakub@redhat.com> gcc/ * common.opt (flag_sanitize_trap): New variable. (fsanitize-trap=, fsanitize-trap): New options. (fsanitize-undefined-trap-on-error): Change into deprecated alias for -fsanitize-trap=all. * opts.h (struct sanitizer_opts_s): Add can_trap member. * opts.cc (finish_options): Complain about unsupported -fsanitize-trap= options. (sanitizer_opts): Add can_trap values to all entries. (get_closest_sanitizer_option): Ignore -fsanitize-trap= options which have can_trap false. (parse_sanitizer_options): Add support for -fsanitize-trap=. For -fsanitize-trap=all, enable SANITIZE_UNDEFINED | SANITIZE_UNDEFINED_NONDEFAULT. Disallow -fsanitize-trap=vptr here. (common_handle_option): Handle OPT_fsanitize_trap_ and OPT_fsanitize_trap. * sanopt.cc (maybe_optimize_ubsan_null_ifn): Check flag_sanitize_trap & SANITIZE_{NULL,ALIGNMENT} instead of flag_sanitize_undefined_trap_on_error. * gcc.cc (sanitize_spec_function): Use flag_sanitize & ~flag_sanitize_trap instead of flag_sanitize and drop use of flag_sanitize_undefined_trap_on_error in "undefined" handling. * ubsan.cc (ubsan_instrument_unreachable): Use flag_sanitize_trap & SANITIZE_??? instead of flag_sanitize_undefined_trap_on_error. (ubsan_expand_bounds_ifn, ubsan_expand_null_ifn, ubsan_expand_objsize_ifn, ubsan_expand_ptr_ifn, ubsan_build_overflow_builtin, instrument_bool_enum_load, ubsan_instrument_float_cast, instrument_nonnull_arg, instrument_nonnull_return, instrument_builtin): Likewise. * doc/invoke.texi (-fsanitize-trap=, -fsanitize-trap): Document. (-fsanitize-undefined-trap-on-error): Document as deprecated alias of -fsanitize-trap. gcc/c-family/ * c-ubsan.cc (ubsan_instrument_division, ubsan_instrument_shift): Use flag_sanitize_trap & SANITIZE_??? instead of flag_sanitize_undefined_trap_on_error. If 2 sanitizers are involved and flag_sanitize_trap differs for them, emit __builtin_trap only for the comparison where trap is requested. (ubsan_instrument_vla, ubsan_instrument_return): Use lag_sanitize_trap & SANITIZE_??? instead of flag_sanitize_undefined_trap_on_error. gcc/cp/ * cp-ubsan.cc (cp_ubsan_instrument_vptr_p): Use flag_sanitize_trap & SANITIZE_VPTR instead of flag_sanitize_undefined_trap_on_error. gcc/testsuite/ * c-c++-common/ubsan/nonnull-4.c: Use -fsanitize-trap=all instead of -fsanitize-undefined-trap-on-error. * c-c++-common/ubsan/div-by-zero-4.c: Use -fsanitize-trap=signed-integer-overflow instead of -fsanitize-undefined-trap-on-error. * c-c++-common/ubsan/overflow-add-4.c: Use -fsanitize-trap=undefined instead of -fsanitize-undefined-trap-on-error. * c-c++-common/ubsan/pr56956.c: Likewise. * c-c++-common/ubsan/pr68142.c: Likewise. * c-c++-common/ubsan/pr80932.c: Use -fno-sanitize-trap=all -fsanitize-trap=shift,undefined instead of -fsanitize-undefined-trap-on-error. * c-c++-common/ubsan/align-8.c: Use -fsanitize-trap=alignment instead of -fsanitize-undefined-trap-on-error. --- gcc/common.opt.jj 2022-06-13 14:02:29.532643844 +0200 +++ gcc/common.opt 2022-06-17 15:24:18.950562715 +0200 @@ -223,6 +223,10 @@ unsigned int flag_sanitize Variable unsigned int flag_sanitize_recover = (SANITIZE_UNDEFINED | SANITIZE_UNDEFINED_NONDEFAULT | SANITIZE_KERNEL_ADDRESS | SANITIZE_KERNEL_HWADDRESS) & ~(SANITIZE_UNREACHABLE | SANITIZE_RETURN) +; What sanitizers should use __builtin_trap () instead of runtime diagnostics +Variable +unsigned int flag_sanitize_trap + ; Flag whether a prefix has been added to dump_base_name Variable bool dump_base_name_prefixed = false @@ -1105,12 +1109,19 @@ fsanitize-recover Common This switch is deprecated; use -fsanitize-recover= instead. +fsanitize-trap= +Common Driver Joined +Use traps instead of diagnostics of undefined behavior sanitizers. + +fsanitize-trap +Common Driver + fsanitize-address-use-after-scope Common Driver Var(flag_sanitize_address_use_after_scope) Init(0) fsanitize-undefined-trap-on-error -Common Driver Var(flag_sanitize_undefined_trap_on_error) Init(0) -Use trap instead of a library function for undefined behavior sanitization. +Common Driver Alias(fsanitize-trap) +This switch is deprecated; use -fsanitize-trap= instead. fasynchronous-unwind-tables Common Var(flag_asynchronous_unwind_tables) Optimization --- gcc/opts.h.jj 2022-02-04 14:36:55.456599000 +0100 +++ gcc/opts.h 2022-06-17 11:43:22.131670515 +0200 @@ -473,6 +473,7 @@ extern const struct sanitizer_opts_s unsigned int flag; size_t len; bool can_recover; + bool can_trap; } sanitizer_opts[]; extern const struct zero_call_used_regs_opts_s --- gcc/opts.cc.jj 2022-06-17 11:08:11.875492903 +0200 +++ gcc/opts.cc 2022-06-17 15:43:20.424343087 +0200 @@ -1232,6 +1232,18 @@ finish_options (struct gcc_options *opts error_at (loc, "%<-fsanitize-recover=%s%> is not supported", sanitizer_opts[i].name); + /* Check -fsanitize-trap option. */ + for (int i = 0; sanitizer_opts[i].name != NULL; ++i) + if ((opts->x_flag_sanitize_trap & sanitizer_opts[i].flag) + && !sanitizer_opts[i].can_trap + /* Allow -fsanitize-trap=all or -fsanitize-trap=undefined + to set flag_sanitize_trap & SANITIZE_VPTR bit which will + effectively disable -fsanitize=vptr, just disallow + explicit -fsanitize-trap=vptr. */ + && sanitizer_opts[i].flag != SANITIZE_VPTR) + error_at (loc, "%<-fsanitize-trap=%s%> is not supported", + sanitizer_opts[i].name); + /* When instrumenting the pointers, we don't want to remove the null pointer checks. */ if (opts->x_flag_sanitize & (SANITIZE_NULL | SANITIZE_NONNULL_ATTRIBUTE @@ -2020,48 +2032,50 @@ enable_fdo_optimizations (struct gcc_opt /* -f{,no-}sanitize{,-recover}= suboptions. */ const struct sanitizer_opts_s sanitizer_opts[] = { -#define SANITIZER_OPT(name, flags, recover) \ - { #name, flags, sizeof #name - 1, recover } - SANITIZER_OPT (address, (SANITIZE_ADDRESS | SANITIZE_USER_ADDRESS), true), +#define SANITIZER_OPT(name, flags, recover, trap) \ + { #name, flags, sizeof #name - 1, recover, trap } + SANITIZER_OPT (address, (SANITIZE_ADDRESS | SANITIZE_USER_ADDRESS), true, + false), SANITIZER_OPT (hwaddress, (SANITIZE_HWADDRESS | SANITIZE_USER_HWADDRESS), - true), + true, false), SANITIZER_OPT (kernel-address, (SANITIZE_ADDRESS | SANITIZE_KERNEL_ADDRESS), - true), + true, false), SANITIZER_OPT (kernel-hwaddress, (SANITIZE_HWADDRESS | SANITIZE_KERNEL_HWADDRESS), + true, false), + SANITIZER_OPT (pointer-compare, SANITIZE_POINTER_COMPARE, true, false), + SANITIZER_OPT (pointer-subtract, SANITIZE_POINTER_SUBTRACT, true, false), + SANITIZER_OPT (thread, SANITIZE_THREAD, false, false), + SANITIZER_OPT (leak, SANITIZE_LEAK, false, false), + SANITIZER_OPT (shift, SANITIZE_SHIFT, true, true), + SANITIZER_OPT (shift-base, SANITIZE_SHIFT_BASE, true, true), + SANITIZER_OPT (shift-exponent, SANITIZE_SHIFT_EXPONENT, true, true), + SANITIZER_OPT (integer-divide-by-zero, SANITIZE_DIVIDE, true, true), + SANITIZER_OPT (undefined, SANITIZE_UNDEFINED, true, true), + SANITIZER_OPT (unreachable, SANITIZE_UNREACHABLE, false, true), + SANITIZER_OPT (vla-bound, SANITIZE_VLA, true, true), + SANITIZER_OPT (return, SANITIZE_RETURN, false, true), + SANITIZER_OPT (null, SANITIZE_NULL, true, true), + SANITIZER_OPT (signed-integer-overflow, SANITIZE_SI_OVERFLOW, true, true), + SANITIZER_OPT (bool, SANITIZE_BOOL, true, true), + SANITIZER_OPT (enum, SANITIZE_ENUM, true, true), + SANITIZER_OPT (float-divide-by-zero, SANITIZE_FLOAT_DIVIDE, true, true), + SANITIZER_OPT (float-cast-overflow, SANITIZE_FLOAT_CAST, true, true), + SANITIZER_OPT (bounds, SANITIZE_BOUNDS, true, true), + SANITIZER_OPT (bounds-strict, SANITIZE_BOUNDS | SANITIZE_BOUNDS_STRICT, true, true), - SANITIZER_OPT (pointer-compare, SANITIZE_POINTER_COMPARE, true), - SANITIZER_OPT (pointer-subtract, SANITIZE_POINTER_SUBTRACT, true), - SANITIZER_OPT (thread, SANITIZE_THREAD, false), - SANITIZER_OPT (leak, SANITIZE_LEAK, false), - SANITIZER_OPT (shift, SANITIZE_SHIFT, true), - SANITIZER_OPT (shift-base, SANITIZE_SHIFT_BASE, true), - SANITIZER_OPT (shift-exponent, SANITIZE_SHIFT_EXPONENT, true), - SANITIZER_OPT (integer-divide-by-zero, SANITIZE_DIVIDE, true), - SANITIZER_OPT (undefined, SANITIZE_UNDEFINED, true), - SANITIZER_OPT (unreachable, SANITIZE_UNREACHABLE, false), - SANITIZER_OPT (vla-bound, SANITIZE_VLA, true), - SANITIZER_OPT (return, SANITIZE_RETURN, false), - SANITIZER_OPT (null, SANITIZE_NULL, true), - SANITIZER_OPT (signed-integer-overflow, SANITIZE_SI_OVERFLOW, true), - SANITIZER_OPT (bool, SANITIZE_BOOL, true), - SANITIZER_OPT (enum, SANITIZE_ENUM, true), - SANITIZER_OPT (float-divide-by-zero, SANITIZE_FLOAT_DIVIDE, true), - SANITIZER_OPT (float-cast-overflow, SANITIZE_FLOAT_CAST, true), - SANITIZER_OPT (bounds, SANITIZE_BOUNDS, true), - SANITIZER_OPT (bounds-strict, SANITIZE_BOUNDS | SANITIZE_BOUNDS_STRICT, true), - SANITIZER_OPT (alignment, SANITIZE_ALIGNMENT, true), - SANITIZER_OPT (nonnull-attribute, SANITIZE_NONNULL_ATTRIBUTE, true), + SANITIZER_OPT (alignment, SANITIZE_ALIGNMENT, true, true), + SANITIZER_OPT (nonnull-attribute, SANITIZE_NONNULL_ATTRIBUTE, true, true), SANITIZER_OPT (returns-nonnull-attribute, SANITIZE_RETURNS_NONNULL_ATTRIBUTE, - true), - SANITIZER_OPT (object-size, SANITIZE_OBJECT_SIZE, true), - SANITIZER_OPT (vptr, SANITIZE_VPTR, true), - SANITIZER_OPT (pointer-overflow, SANITIZE_POINTER_OVERFLOW, true), - SANITIZER_OPT (builtin, SANITIZE_BUILTIN, true), - SANITIZER_OPT (shadow-call-stack, SANITIZE_SHADOW_CALL_STACK, false), - SANITIZER_OPT (all, ~0U, true), + true, true), + SANITIZER_OPT (object-size, SANITIZE_OBJECT_SIZE, true, true), + SANITIZER_OPT (vptr, SANITIZE_VPTR, true, false), + SANITIZER_OPT (pointer-overflow, SANITIZE_POINTER_OVERFLOW, true, true), + SANITIZER_OPT (builtin, SANITIZE_BUILTIN, true, true), + SANITIZER_OPT (shadow-call-stack, SANITIZE_SHADOW_CALL_STACK, false, false), + SANITIZER_OPT (all, ~0U, true, true), #undef SANITIZER_OPT - { NULL, 0U, 0UL, false } + { NULL, 0U, 0UL, false, false } }; /* -fzero-call-used-regs= suboptions. */ @@ -2114,7 +2128,7 @@ struct edit_distance_traits<const string /* Given ARG, an unrecognized sanitizer option, return the best matching sanitizer option, or NULL if there isn't one. OPTS is array of candidate sanitizer options. - CODE is OPT_fsanitize_ or OPT_fsanitize_recover_. + CODE is OPT_fsanitize_, OPT_fsanitize_recover_ or OPT_fsanitize_trap_. VALUE is non-zero for the regular form of the option, zero for the "no-" form (e.g. "-fno-sanitize-recover="). */ @@ -2139,6 +2153,13 @@ get_closest_sanitizer_option (const stri && value) continue; + /* For -fsanitize-trap= (and not -fno-sanitize-trap=), + don't offer the non-trapping options. */ + if (code == OPT_fsanitize_trap_ + && !opts[i].can_trap + && value) + continue; + bm.consider (opts[i].name); } return bm.get_best_meaningful_candidate (); @@ -2183,10 +2204,13 @@ parse_sanitizer_options (const char *p, if (complain) error_at (loc, "%<-fsanitize=all%> option is not valid"); } - else + else if (code == OPT_fsanitize_recover_) flags |= ~(SANITIZE_THREAD | SANITIZE_LEAK | SANITIZE_UNREACHABLE | SANITIZE_RETURN | SANITIZE_SHADOW_CALL_STACK); + else + flags |= (SANITIZE_UNDEFINED + | SANITIZE_UNDEFINED_NONDEFAULT); } else if (value) { @@ -2197,6 +2221,10 @@ parse_sanitizer_options (const char *p, && sanitizer_opts[i].flag == SANITIZE_UNDEFINED) flags |= (SANITIZE_UNDEFINED & ~(SANITIZE_UNREACHABLE | SANITIZE_RETURN)); + else if (code == OPT_fsanitize_trap_ + && sanitizer_opts[i].flag == SANITIZE_VPTR) + error_at (loc, "%<-fsanitize-trap=%s%> is not supported", + sanitizer_opts[i].name); else flags |= sanitizer_opts[i].flag; } @@ -2215,6 +2243,8 @@ parse_sanitizer_options (const char *p, const char *suffix; if (code == OPT_fsanitize_recover_) suffix = "-recover"; + else if (code == OPT_fsanitize_trap_) + suffix = "-trap"; else suffix = ""; @@ -2647,6 +2677,12 @@ common_handle_option (struct gcc_options opts->x_flag_sanitize_recover, value, true); break; + case OPT_fsanitize_trap_: + opts->x_flag_sanitize_trap + = parse_sanitizer_options (arg, loc, code, + opts->x_flag_sanitize_trap, value, true); + break; + case OPT_fasan_shadow_offset_: /* Deferred. */ break; @@ -2665,6 +2701,15 @@ common_handle_option (struct gcc_options &= ~(SANITIZE_UNDEFINED | SANITIZE_UNDEFINED_NONDEFAULT); break; + case OPT_fsanitize_trap: + if (value) + opts->x_flag_sanitize_trap + |= (SANITIZE_UNDEFINED | SANITIZE_UNDEFINED_NONDEFAULT); + else + opts->x_flag_sanitize_trap + &= ~(SANITIZE_UNDEFINED | SANITIZE_UNDEFINED_NONDEFAULT); + break; + case OPT_O: case OPT_Os: case OPT_Ofast: --- gcc/sanopt.cc.jj 2022-01-18 11:58:59.886977714 +0100 +++ gcc/sanopt.cc 2022-06-17 14:07:18.021155983 +0200 @@ -392,11 +392,11 @@ maybe_optimize_ubsan_null_ifn (class san stmts have same location. */ else if (integer_zerop (align)) remove = (flag_sanitize_recover & SANITIZE_NULL) == 0 - || flag_sanitize_undefined_trap_on_error + || (flag_sanitize_trap & SANITIZE_NULL) != 0 || gimple_location (g) == gimple_location (stmt); else if (tree_int_cst_le (cur_align, align)) remove = (flag_sanitize_recover & SANITIZE_ALIGNMENT) == 0 - || flag_sanitize_undefined_trap_on_error + || (flag_sanitize_trap & SANITIZE_ALIGNMENT) != 0 || gimple_location (g) == gimple_location (stmt); if (!remove && gimple_bb (g) == gimple_bb (stmt) --- gcc/gcc.cc.jj 2022-06-03 11:20:13.231070818 +0200 +++ gcc/gcc.cc 2022-06-17 13:34:16.482830857 +0200 @@ -10313,8 +10313,9 @@ sanitize_spec_function (int argc, const return (flag_sanitize & SANITIZE_THREAD) ? "" : NULL; if (strcmp (argv[0], "undefined") == 0) return ((flag_sanitize - & (SANITIZE_UNDEFINED | SANITIZE_UNDEFINED_NONDEFAULT)) - && !flag_sanitize_undefined_trap_on_error) ? "" : NULL; + & ~flag_sanitize_trap + & (SANITIZE_UNDEFINED | SANITIZE_UNDEFINED_NONDEFAULT))) + ? "" : NULL; if (strcmp (argv[0], "leak") == 0) return ((flag_sanitize & (SANITIZE_ADDRESS | SANITIZE_LEAK | SANITIZE_THREAD)) --- gcc/ubsan.cc.jj 2022-05-13 09:34:24.043146671 +0200 +++ gcc/ubsan.cc 2022-06-17 14:14:42.164217896 +0200 @@ -647,7 +647,7 @@ ubsan_instrument_unreachable (gimple_stm gimple *g; location_t loc = gimple_location (gsi_stmt (*gsi)); - if (flag_sanitize_undefined_trap_on_error) + if (flag_sanitize_trap & SANITIZE_UNREACHABLE) g = gimple_build_call (builtin_decl_explicit (BUILT_IN_TRAP), 0); else { @@ -719,7 +719,7 @@ ubsan_expand_bounds_ifn (gimple_stmt_ite /* Generate __ubsan_handle_out_of_bounds call. */ *gsi = gsi_after_labels (then_bb); - if (flag_sanitize_undefined_trap_on_error) + if (flag_sanitize_trap & SANITIZE_BOUNDS) g = gimple_build_call (builtin_decl_explicit (BUILT_IN_TRAP), 0); else { @@ -827,7 +827,8 @@ ubsan_expand_null_ifn (gimple_stmt_itera set_immediate_dominator (CDI_DOMINATORS, then_bb, cond_bb); /* Put the ubsan builtin call into the newly created BB. */ - if (flag_sanitize_undefined_trap_on_error) + if (flag_sanitize_trap & ((check_align ? SANITIZE_ALIGNMENT + 0 : 0) + | (check_null ? SANITIZE_NULL + 0 : 0))) g = gimple_build_call (builtin_decl_implicit (BUILT_IN_TRAP), 0); else { @@ -997,7 +998,7 @@ ubsan_expand_objsize_ifn (gimple_stmt_it } /* Generate __ubsan_handle_type_mismatch call. */ - if (flag_sanitize_undefined_trap_on_error) + if (flag_sanitize_trap & SANITIZE_OBJECT_SIZE) g = gimple_build_call (builtin_decl_explicit (BUILT_IN_TRAP), 0); else { @@ -1143,7 +1144,7 @@ ubsan_expand_ptr_ifn (gimple_stmt_iterat } /* Put the ubsan builtin call into the newly created BB. */ - if (flag_sanitize_undefined_trap_on_error) + if (flag_sanitize_trap & SANITIZE_POINTER_OVERFLOW) g = gimple_build_call (builtin_decl_implicit (BUILT_IN_TRAP), 0); else { @@ -1518,7 +1519,7 @@ tree ubsan_build_overflow_builtin (tree_code code, location_t loc, tree lhstype, tree op0, tree op1, tree *datap) { - if (flag_sanitize_undefined_trap_on_error) + if (flag_sanitize_trap & SANITIZE_SI_OVERFLOW) return build_call_expr_loc (loc, builtin_decl_explicit (BUILT_IN_TRAP), 0); tree data; @@ -1741,7 +1742,8 @@ instrument_bool_enum_load (gimple_stmt_i } gsi2 = gsi_after_labels (then_bb); - if (flag_sanitize_undefined_trap_on_error) + if (flag_sanitize_trap & (TREE_CODE (type) == BOOLEAN_TYPE + ? SANITIZE_BOOL : SANITIZE_ENUM)) g = gimple_build_call (builtin_decl_explicit (BUILT_IN_TRAP), 0); else { @@ -1904,7 +1906,7 @@ ubsan_instrument_float_cast (location_t if (integer_zerop (t)) return NULL_TREE; - if (flag_sanitize_undefined_trap_on_error) + if (flag_sanitize_trap & SANITIZE_FLOAT_CAST) fn = build_call_expr_loc (loc, builtin_decl_explicit (BUILT_IN_TRAP), 0); else { @@ -1974,7 +1976,7 @@ instrument_nonnull_arg (gimple_stmt_iter gsi_insert_after (gsi, g, GSI_NEW_STMT); *gsi = gsi_after_labels (then_bb); - if (flag_sanitize_undefined_trap_on_error) + if (flag_sanitize_trap & SANITIZE_NONNULL_ATTRIBUTE) g = gimple_build_call (builtin_decl_explicit (BUILT_IN_TRAP), 0); else { @@ -2030,7 +2032,7 @@ instrument_nonnull_return (gimple_stmt_i gsi_insert_after (gsi, g, GSI_NEW_STMT); *gsi = gsi_after_labels (then_bb); - if (flag_sanitize_undefined_trap_on_error) + if (flag_sanitize_trap & SANITIZE_RETURNS_NONNULL_ATTRIBUTE) g = gimple_build_call (builtin_decl_explicit (BUILT_IN_TRAP), 0); else { @@ -2279,7 +2281,7 @@ instrument_builtin (gimple_stmt_iterator gsi_insert_after (gsi, g, GSI_NEW_STMT); *gsi = gsi_after_labels (then_bb); - if (flag_sanitize_undefined_trap_on_error) + if (flag_sanitize_trap & SANITIZE_BUILTIN) g = gimple_build_call (builtin_decl_explicit (BUILT_IN_TRAP), 0); else { --- gcc/doc/invoke.texi.jj 2022-06-16 10:56:28.826386840 +0200 +++ gcc/doc/invoke.texi 2022-06-17 16:42:59.370670655 +0200 @@ -609,6 +609,7 @@ Objective-C and Objective-C++ Dialects}. -fprofile-exclude-files=@var{regex} @gol -fprofile-reproducible=@r{[}multithreaded@r{|}parallel-runs@r{|}serial@r{]} @gol -fsanitize=@var{style} -fsanitize-recover -fsanitize-recover=@var{style} @gol +-fsanitize-trap -fsanitize-trap=@var{style} @gol -fasan-shadow-offset=@var{number} -fsanitize-sections=@var{s1},@var{s2},... @gol -fsanitize-undefined-trap-on-error -fbounds-check @gol -fcf-protection=@r{[}full@r{|}branch@r{|}return@r{|}none@r{|}check@r{]} @gol @@ -16116,13 +16117,37 @@ undefined,float-cast-overflow,float-divi Enable sanitization of local variables to detect use-after-scope bugs. The option sets @option{-fstack-reuse} to @samp{none}. +@item -fsanitize-trap@r{[}=@var{opts}@r{]} +@opindex fsanitize-trap +@opindex fno-sanitize-trap +The @option{-fsanitize-trap=} option instructs the compiler to +report for sanitizers mentioned in comma-separated list of @var{opts} +undefined behavior using @code{__builtin_trap} rather than a @code{libubsan} +library routine. If this option is enabled for certain sanitizer, +it takes precedence over the @option{-fsanitizer-recover=} for that +sanitizer, @code{__builtin_trap} will be emitted and be fatal regardless +of whether recovery is enabled or disabled using @option{-fsanitize-recover=}. + +The advantage of this is that the @code{libubsan} library is not needed +and is not linked in, so this is usable even in freestanding environments. + +Currently this feature works with @option{-fsanitize=undefined} (and its suboptions +except for @option{-fsanitize=vptr}), @option{-fsanitize=float-cast-overflow}, +@option{-fsanitize=float-divide-by-zero} and +@option{-fsanitize=bounds-strict}. @code{-fsanitize-trap=all} can be also +specified, which enables it for @code{undefined} suboptions, +@option{-fsanitize=float-cast-overflow}, +@option{-fsanitize=float-divide-by-zero} and +@option{-fsanitize=bounds-strict}. +If @code{-fsanitize-trap=undefined} or @code{-fsanitize-trap=all} is used +and @code{-fsanitize=vptr} is enabled on the command line, the +instrumentation is silently ignored as the instrumentation always needs +@code{libubsan} support, @option{-fsanitize-trap=vptr} is not allowed. + @item -fsanitize-undefined-trap-on-error @opindex fsanitize-undefined-trap-on-error -The @option{-fsanitize-undefined-trap-on-error} option instructs the compiler to -report undefined behavior using @code{__builtin_trap} rather than -a @code{libubsan} library routine. The advantage of this is that the -@code{libubsan} library is not needed and is not linked in, so this -is usable even in freestanding environments. +The @option{-fsanitize-undefined-trap-on-error} option is deprecated +equivalent of @option{-fsanitize-trap=all}. @item -fsanitize-coverage=trace-pc @opindex fsanitize-coverage=trace-pc --- gcc/c-family/c-ubsan.cc.jj 2022-06-14 13:17:01.559722905 +0200 +++ gcc/c-family/c-ubsan.cc 2022-06-17 15:23:16.899389988 +0200 @@ -83,8 +83,9 @@ ubsan_instrument_division (location_t lo x = NULL_TREE; flag = SANITIZE_SI_OVERFLOW; } - else if (flag_sanitize_undefined_trap_on_error - || (((flag_sanitize_recover & SANITIZE_DIVIDE) == 0) + else if ((((flag_sanitize_trap & SANITIZE_DIVIDE) == 0) + == ((flag_sanitize_trap & SANITIZE_SI_OVERFLOW) == 0)) + && (((flag_sanitize_recover & SANITIZE_DIVIDE) == 0) == ((flag_sanitize_recover & SANITIZE_SI_OVERFLOW) == 0))) { t = fold_build2 (TRUTH_OR_EXPR, boolean_type_node, t, x); @@ -105,7 +106,7 @@ ubsan_instrument_division (location_t lo make sure it gets evaluated before the condition. */ t = fold_build2 (COMPOUND_EXPR, TREE_TYPE (t), unshare_expr (op0), t); t = fold_build2 (COMPOUND_EXPR, TREE_TYPE (t), unshare_expr (op1), t); - if (flag_sanitize_undefined_trap_on_error) + if ((flag_sanitize_trap & flag) && x == NULL_TREE) tt = build_call_expr_loc (loc, builtin_decl_explicit (BUILT_IN_TRAP), 0); else { @@ -113,25 +114,41 @@ ubsan_instrument_division (location_t lo ubsan_type_descriptor (type), NULL_TREE, NULL_TREE); data = build_fold_addr_expr_loc (loc, data); - enum built_in_function bcode - = (flag_sanitize_recover & flag) - ? BUILT_IN_UBSAN_HANDLE_DIVREM_OVERFLOW - : BUILT_IN_UBSAN_HANDLE_DIVREM_OVERFLOW_ABORT; - tt = builtin_decl_explicit (bcode); - op0 = unshare_expr (op0); - op1 = unshare_expr (op1); - tt = build_call_expr_loc (loc, tt, 3, data, ubsan_encode_value (op0), - ubsan_encode_value (op1)); - if (x) + if (flag_sanitize_trap & flag) + tt = build_call_expr_loc (loc, builtin_decl_explicit (BUILT_IN_TRAP), + 0); + else { - bcode = (flag_sanitize_recover & SANITIZE_SI_OVERFLOW) - ? BUILT_IN_UBSAN_HANDLE_DIVREM_OVERFLOW - : BUILT_IN_UBSAN_HANDLE_DIVREM_OVERFLOW_ABORT; - tree xt = builtin_decl_explicit (bcode); + enum built_in_function bcode + = (flag_sanitize_recover & flag) + ? BUILT_IN_UBSAN_HANDLE_DIVREM_OVERFLOW + : BUILT_IN_UBSAN_HANDLE_DIVREM_OVERFLOW_ABORT; + tt = builtin_decl_explicit (bcode); op0 = unshare_expr (op0); op1 = unshare_expr (op1); - xt = build_call_expr_loc (loc, xt, 3, data, ubsan_encode_value (op0), + tt = build_call_expr_loc (loc, tt, 3, data, ubsan_encode_value (op0), ubsan_encode_value (op1)); + } + if (x) + { + tree xt; + if (flag_sanitize_trap & SANITIZE_SI_OVERFLOW) + xt = build_call_expr_loc (loc, + builtin_decl_explicit (BUILT_IN_TRAP), + 0); + else + { + enum built_in_function bcode + = (flag_sanitize_recover & SANITIZE_SI_OVERFLOW) + ? BUILT_IN_UBSAN_HANDLE_DIVREM_OVERFLOW + : BUILT_IN_UBSAN_HANDLE_DIVREM_OVERFLOW_ABORT; + xt = builtin_decl_explicit (bcode); + op0 = unshare_expr (op0); + op1 = unshare_expr (op1); + xt = build_call_expr_loc (loc, xt, 3, data, + ubsan_encode_value (op0), + ubsan_encode_value (op1)); + } x = fold_build3 (COND_EXPR, void_type_node, x, xt, void_node); } } @@ -225,8 +242,9 @@ ubsan_instrument_shift (location_t loc, } else { - if (flag_sanitize_undefined_trap_on_error - || ((!(flag_sanitize_recover & SANITIZE_SHIFT_EXPONENT)) + if (((!(flag_sanitize_trap & SANITIZE_SHIFT_EXPONENT)) + == (!(flag_sanitize_trap & SANITIZE_SHIFT_BASE))) + && ((!(flag_sanitize_recover & SANITIZE_SHIFT_EXPONENT)) == (!(flag_sanitize_recover & SANITIZE_SHIFT_BASE)))) t = fold_build2 (TRUTH_OR_EXPR, boolean_type_node, t, tt); else @@ -234,7 +252,7 @@ ubsan_instrument_shift (location_t loc, } } - if (flag_sanitize_undefined_trap_on_error) + if ((flag_sanitize_trap & recover_kind) && else_t == void_node) tt = build_call_expr_loc (loc, builtin_decl_explicit (BUILT_IN_TRAP), 0); else { @@ -244,26 +262,40 @@ ubsan_instrument_shift (location_t loc, NULL_TREE); data = build_fold_addr_expr_loc (loc, data); - enum built_in_function bcode - = (flag_sanitize_recover & recover_kind) - ? BUILT_IN_UBSAN_HANDLE_SHIFT_OUT_OF_BOUNDS - : BUILT_IN_UBSAN_HANDLE_SHIFT_OUT_OF_BOUNDS_ABORT; - tt = builtin_decl_explicit (bcode); - op0 = unshare_expr (op0); - op1 = unshare_expr (op1); - tt = build_call_expr_loc (loc, tt, 3, data, ubsan_encode_value (op0), - ubsan_encode_value (op1)); + if (flag_sanitize_trap & recover_kind) + tt = build_call_expr_loc (loc, builtin_decl_explicit (BUILT_IN_TRAP), 0); + else + { + enum built_in_function bcode + = (flag_sanitize_recover & recover_kind) + ? BUILT_IN_UBSAN_HANDLE_SHIFT_OUT_OF_BOUNDS + : BUILT_IN_UBSAN_HANDLE_SHIFT_OUT_OF_BOUNDS_ABORT; + tt = builtin_decl_explicit (bcode); + op0 = unshare_expr (op0); + op1 = unshare_expr (op1); + tt = build_call_expr_loc (loc, tt, 3, data, ubsan_encode_value (op0), + ubsan_encode_value (op1)); + } if (else_t != void_node) { - bcode = (flag_sanitize_recover & SANITIZE_SHIFT_BASE) + tree else_tt; + if (flag_sanitize_trap & SANITIZE_SHIFT_BASE) + else_tt + = build_call_expr_loc (loc, + builtin_decl_explicit (BUILT_IN_TRAP), 0); + else + { + enum built_in_function bcode + = (flag_sanitize_recover & SANITIZE_SHIFT_BASE) ? BUILT_IN_UBSAN_HANDLE_SHIFT_OUT_OF_BOUNDS : BUILT_IN_UBSAN_HANDLE_SHIFT_OUT_OF_BOUNDS_ABORT; - tree else_tt = builtin_decl_explicit (bcode); - op0 = unshare_expr (op0); - op1 = unshare_expr (op1); - else_tt = build_call_expr_loc (loc, else_tt, 3, data, - ubsan_encode_value (op0), - ubsan_encode_value (op1)); + else_tt = builtin_decl_explicit (bcode); + op0 = unshare_expr (op0); + op1 = unshare_expr (op1); + else_tt = build_call_expr_loc (loc, else_tt, 3, data, + ubsan_encode_value (op0), + ubsan_encode_value (op1)); + } else_t = fold_build3 (COND_EXPR, void_type_node, else_t, else_tt, void_node); } @@ -282,7 +314,7 @@ ubsan_instrument_vla (location_t loc, tr tree t, tt; t = fold_build2 (LE_EXPR, boolean_type_node, size, build_int_cst (type, 0)); - if (flag_sanitize_undefined_trap_on_error) + if (flag_sanitize_trap & SANITIZE_VLA) tt = build_call_expr_loc (loc, builtin_decl_explicit (BUILT_IN_TRAP), 0); else { @@ -307,10 +339,10 @@ ubsan_instrument_vla (location_t loc, tr tree ubsan_instrument_return (location_t loc) { - if (flag_sanitize_undefined_trap_on_error) - return build_call_expr_loc - /* pass_warn_function_return checks for BUILTINS_LOCATION. */ - (BUILTINS_LOCATION, builtin_decl_explicit (BUILT_IN_TRAP), 0); + if (flag_sanitize_trap & SANITIZE_RETURN) + /* pass_warn_function_return checks for BUILTINS_LOCATION. */ + return build_call_expr_loc (BUILTINS_LOCATION, + builtin_decl_explicit (BUILT_IN_TRAP), 0); tree data = ubsan_create_data ("__ubsan_missing_return_data", 1, &loc, NULL_TREE, NULL_TREE); --- gcc/cp/cp-ubsan.cc.jj 2022-01-18 11:58:59.299986100 +0100 +++ gcc/cp/cp-ubsan.cc 2022-06-17 15:14:01.760791859 +0200 @@ -32,7 +32,7 @@ along with GCC; see the file COPYING3. static bool cp_ubsan_instrument_vptr_p (tree type) { - if (!flag_rtti || flag_sanitize_undefined_trap_on_error) + if (!flag_rtti || (flag_sanitize_trap & SANITIZE_VPTR)) return false; if (!sanitize_flags_p (SANITIZE_VPTR)) --- gcc/testsuite/c-c++-common/ubsan/nonnull-4.c.jj 2020-01-12 11:54:37.029404115 +0100 +++ gcc/testsuite/c-c++-common/ubsan/nonnull-4.c 2022-06-17 16:53:38.976152460 +0200 @@ -1,6 +1,6 @@ /* { dg-do run } */ /* { dg-shouldfail "ubsan" } */ -/* { dg-options "-fsanitize=undefined -fsanitize-undefined-trap-on-error" } */ +/* { dg-options "-fsanitize=undefined -fsanitize-trap=all" } */ int q, r; void *a, *b, *c = (void *) &q, *d, *e, *f = (void *) &q, *g, *h; --- gcc/testsuite/c-c++-common/ubsan/div-by-zero-4.c.jj 2021-12-30 15:12:43.248150079 +0100 +++ gcc/testsuite/c-c++-common/ubsan/div-by-zero-4.c 2022-06-17 16:53:02.139643041 +0200 @@ -1,5 +1,5 @@ /* { dg-do run } */ -/* { dg-options "-fsanitize=signed-integer-overflow -fsanitize-undefined-trap-on-error -Wno-overflow" } */ +/* { dg-options "-fsanitize=signed-integer-overflow -fsanitize-trap=signed-integer-overflow -Wno-overflow" } */ #define INT_MIN (-__INT_MAX__ - 1) --- gcc/testsuite/c-c++-common/ubsan/overflow-add-4.c.jj 2020-01-12 11:54:37.030404100 +0100 +++ gcc/testsuite/c-c++-common/ubsan/overflow-add-4.c 2022-06-17 16:54:12.544705407 +0200 @@ -1,5 +1,5 @@ /* { dg-do run } */ -/* { dg-options "-fsanitize=signed-integer-overflow -Wno-unused-variable -fsanitize-undefined-trap-on-error" } */ +/* { dg-options "-fsanitize=signed-integer-overflow -Wno-unused-variable -fsanitize-trap=undefined" } */ /* { dg-shouldfail "ubsan" } */ #define INT_MAX __INT_MAX__ --- gcc/testsuite/c-c++-common/ubsan/pr56956.c.jj 2020-01-12 11:54:37.031404085 +0100 +++ gcc/testsuite/c-c++-common/ubsan/pr56956.c 2022-06-17 16:54:35.557398933 +0200 @@ -1,5 +1,5 @@ /* { dg-do run } */ -/* { dg-options "-fsanitize=undefined -fsanitize-undefined-trap-on-error" } */ +/* { dg-options "-fsanitize=undefined -fsanitize-trap=undefined" } */ unsigned int __attribute__((noinline,noclone)) foo (unsigned int x) --- gcc/testsuite/c-c++-common/ubsan/pr68142.c.jj 2020-01-12 11:54:37.031404085 +0100 +++ gcc/testsuite/c-c++-common/ubsan/pr68142.c 2022-06-17 16:55:02.426041091 +0200 @@ -1,5 +1,5 @@ /* { dg-do run } */ -/* { dg-options "-fsanitize=undefined -fsanitize-undefined-trap-on-error" } */ +/* { dg-options "-fsanitize=undefined -fsanitize-trap=undefined" } */ int __attribute__((noinline,noclone)) h(int a) --- gcc/testsuite/c-c++-common/ubsan/pr80932.c.jj 2020-01-12 11:54:37.032404070 +0100 +++ gcc/testsuite/c-c++-common/ubsan/pr80932.c 2022-06-17 16:55:59.401282322 +0200 @@ -1,6 +1,6 @@ /* PR sanitizer/80932 */ /* { dg-do run } */ -/* { dg-options "-fsanitize=undefined -fsanitize-undefined-trap-on-error" } */ +/* { dg-options "-fsanitize=undefined -fno-sanitize-trap=all -fsanitize-trap=shift,undefined" } */ int x = 1; --- gcc/testsuite/c-c++-common/ubsan/align-8.c.jj 2020-01-12 11:54:37.028404130 +0100 +++ gcc/testsuite/c-c++-common/ubsan/align-8.c 2022-06-17 16:50:57.827298591 +0200 @@ -1,6 +1,6 @@ /* Limit this to known non-strict alignment targets. */ /* { dg-do run { target { i?86-*-linux* x86_64-*-linux* } } } */ -/* { dg-options "-O -fsanitize=alignment -fsanitize-undefined-trap-on-error -Wno-address-of-packed-member -fdump-tree-sanopt-details" } */ +/* { dg-options "-O -fsanitize=alignment -fsanitize-trap=alignment -Wno-address-of-packed-member -fdump-tree-sanopt-details" } */ /* { dg-skip-if "" { *-*-* } { "-flto -fno-fat-lto-objects" } } */ /* { dg-shouldfail "ubsan" } */ Jakub ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] ubsan: Add -fsanitize-trap= support 2022-06-17 15:34 ` [PATCH] ubsan: Add -fsanitize-trap= support Jakub Jelinek @ 2022-06-17 18:21 ` Jason Merrill 0 siblings, 0 replies; 12+ messages in thread From: Jason Merrill @ 2022-06-17 18:21 UTC (permalink / raw) To: Jakub Jelinek, Richard Biener, Jeff Law, Jonathan Wakely Cc: Marek Polacek, gcc-patches On 6/17/22 11:34, Jakub Jelinek via Gcc-patches wrote: > On Thu, Jun 16, 2022 at 09:32:02PM +0100, Jonathan Wakely wrote: >> It looks like clang has addressed this deficiency now: >> >> https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html#usage > > Thanks, that is roughly what I'd implement anyway and apparently they have > it already since 2015, we've added the -fsanitize-undefined-trap-on-error > support back in 2014 and didn't change it since then. > > As a small divergence from clang, I chose -fsanitize-undefined-trap-on-error > to be a (deprecated) alias for -fsanitize-trap aka -fsanitize-trap=all > rather thn -fsanitize-trap=undefined which seems to be what clang does, > because for a deprecated option it is IMHO more important backwards > compatibility with what gcc did over the past 8 years rather than clang > compatibility. > Some sanitizers (e.g. asan, lsan, tsan) don't support traps, > -fsanitize-trap=address etc. will be rejected (if enabled at the end of > command line), -fno-sanitize-trap= can be specified even for them. > This is similar behavior to -fsanitize-recover=. > One complication is vptr sanitization, which can't easily trap, > as the whole slow path of the checking is inside of libubsan. > Previously, -fsanitize=vptr -fsanitize-undefined-trap-on-error > silently ignored vptr sanitization. > This patch similarly to what clang does will accept > -fsanitize-trap=all or -fsanitize-trap=undefined which enable > the vptr bit as trapping and again that causes silent disabling > of vptr sanitization, while -fsanitize-trap=vptr is rejected > (already during option processing). > > So far quickly tested with make check-gcc check-g++ RUNTESTFLAGS=ubsan.exp, > ok for trunk if it passes full bootstrap/regtest? > > 2022-06-17 Jakub Jelinek <jakub@redhat.com> > > gcc/ > * common.opt (flag_sanitize_trap): New variable. > (fsanitize-trap=, fsanitize-trap): New options. > (fsanitize-undefined-trap-on-error): Change into deprecated alias > for -fsanitize-trap=all. > * opts.h (struct sanitizer_opts_s): Add can_trap member. > * opts.cc (finish_options): Complain about unsupported > -fsanitize-trap= options. > (sanitizer_opts): Add can_trap values to all entries. > (get_closest_sanitizer_option): Ignore -fsanitize-trap= > options which have can_trap false. > (parse_sanitizer_options): Add support for -fsanitize-trap=. > For -fsanitize-trap=all, enable > SANITIZE_UNDEFINED | SANITIZE_UNDEFINED_NONDEFAULT. Disallow > -fsanitize-trap=vptr here. > (common_handle_option): Handle OPT_fsanitize_trap_ and > OPT_fsanitize_trap. > * sanopt.cc (maybe_optimize_ubsan_null_ifn): Check > flag_sanitize_trap & SANITIZE_{NULL,ALIGNMENT} instead of > flag_sanitize_undefined_trap_on_error. > * gcc.cc (sanitize_spec_function): Use > flag_sanitize & ~flag_sanitize_trap instead of flag_sanitize > and drop use of flag_sanitize_undefined_trap_on_error in > "undefined" handling. > * ubsan.cc (ubsan_instrument_unreachable): Use > flag_sanitize_trap & SANITIZE_??? instead of > flag_sanitize_undefined_trap_on_error. > (ubsan_expand_bounds_ifn, ubsan_expand_null_ifn, > ubsan_expand_objsize_ifn, ubsan_expand_ptr_ifn, > ubsan_build_overflow_builtin, instrument_bool_enum_load, > ubsan_instrument_float_cast, instrument_nonnull_arg, > instrument_nonnull_return, instrument_builtin): Likewise. > * doc/invoke.texi (-fsanitize-trap=, -fsanitize-trap): Document. > (-fsanitize-undefined-trap-on-error): Document as deprecated > alias of -fsanitize-trap. > gcc/c-family/ > * c-ubsan.cc (ubsan_instrument_division, ubsan_instrument_shift): > Use flag_sanitize_trap & SANITIZE_??? instead of > flag_sanitize_undefined_trap_on_error. If 2 sanitizers are involved > and flag_sanitize_trap differs for them, emit __builtin_trap only > for the comparison where trap is requested. > (ubsan_instrument_vla, ubsan_instrument_return): Use > lag_sanitize_trap & SANITIZE_??? instead of > flag_sanitize_undefined_trap_on_error. > gcc/cp/ > * cp-ubsan.cc (cp_ubsan_instrument_vptr_p): Use > flag_sanitize_trap & SANITIZE_VPTR instead of > flag_sanitize_undefined_trap_on_error. > gcc/testsuite/ > * c-c++-common/ubsan/nonnull-4.c: Use -fsanitize-trap=all > instead of -fsanitize-undefined-trap-on-error. > * c-c++-common/ubsan/div-by-zero-4.c: Use > -fsanitize-trap=signed-integer-overflow instead of > -fsanitize-undefined-trap-on-error. > * c-c++-common/ubsan/overflow-add-4.c: Use -fsanitize-trap=undefined > instead of -fsanitize-undefined-trap-on-error. > * c-c++-common/ubsan/pr56956.c: Likewise. > * c-c++-common/ubsan/pr68142.c: Likewise. > * c-c++-common/ubsan/pr80932.c: Use > -fno-sanitize-trap=all -fsanitize-trap=shift,undefined > instead of -fsanitize-undefined-trap-on-error. > * c-c++-common/ubsan/align-8.c: Use -fsanitize-trap=alignment > instead of -fsanitize-undefined-trap-on-error. > > --- gcc/common.opt.jj 2022-06-13 14:02:29.532643844 +0200 > +++ gcc/common.opt 2022-06-17 15:24:18.950562715 +0200 > @@ -223,6 +223,10 @@ unsigned int flag_sanitize > Variable > unsigned int flag_sanitize_recover = (SANITIZE_UNDEFINED | SANITIZE_UNDEFINED_NONDEFAULT | SANITIZE_KERNEL_ADDRESS | SANITIZE_KERNEL_HWADDRESS) & ~(SANITIZE_UNREACHABLE | SANITIZE_RETURN) > > +; What sanitizers should use __builtin_trap () instead of runtime diagnostics > +Variable > +unsigned int flag_sanitize_trap > + > ; Flag whether a prefix has been added to dump_base_name > Variable > bool dump_base_name_prefixed = false > @@ -1105,12 +1109,19 @@ fsanitize-recover > Common > This switch is deprecated; use -fsanitize-recover= instead. > > +fsanitize-trap= > +Common Driver Joined > +Use traps instead of diagnostics of undefined behavior sanitizers. > + > +fsanitize-trap > +Common Driver > + > fsanitize-address-use-after-scope > Common Driver Var(flag_sanitize_address_use_after_scope) Init(0) > > fsanitize-undefined-trap-on-error > -Common Driver Var(flag_sanitize_undefined_trap_on_error) Init(0) > -Use trap instead of a library function for undefined behavior sanitization. > +Common Driver Alias(fsanitize-trap) > +This switch is deprecated; use -fsanitize-trap= instead. > > fasynchronous-unwind-tables > Common Var(flag_asynchronous_unwind_tables) Optimization > --- gcc/opts.h.jj 2022-02-04 14:36:55.456599000 +0100 > +++ gcc/opts.h 2022-06-17 11:43:22.131670515 +0200 > @@ -473,6 +473,7 @@ extern const struct sanitizer_opts_s > unsigned int flag; > size_t len; > bool can_recover; > + bool can_trap; > } sanitizer_opts[]; > > extern const struct zero_call_used_regs_opts_s > --- gcc/opts.cc.jj 2022-06-17 11:08:11.875492903 +0200 > +++ gcc/opts.cc 2022-06-17 15:43:20.424343087 +0200 > @@ -1232,6 +1232,18 @@ finish_options (struct gcc_options *opts > error_at (loc, "%<-fsanitize-recover=%s%> is not supported", > sanitizer_opts[i].name); > > + /* Check -fsanitize-trap option. */ > + for (int i = 0; sanitizer_opts[i].name != NULL; ++i) > + if ((opts->x_flag_sanitize_trap & sanitizer_opts[i].flag) > + && !sanitizer_opts[i].can_trap > + /* Allow -fsanitize-trap=all or -fsanitize-trap=undefined > + to set flag_sanitize_trap & SANITIZE_VPTR bit which will > + effectively disable -fsanitize=vptr, just disallow > + explicit -fsanitize-trap=vptr. */ > + && sanitizer_opts[i].flag != SANITIZE_VPTR) > + error_at (loc, "%<-fsanitize-trap=%s%> is not supported", > + sanitizer_opts[i].name); > + > /* When instrumenting the pointers, we don't want to remove > the null pointer checks. */ > if (opts->x_flag_sanitize & (SANITIZE_NULL | SANITIZE_NONNULL_ATTRIBUTE > @@ -2020,48 +2032,50 @@ enable_fdo_optimizations (struct gcc_opt > /* -f{,no-}sanitize{,-recover}= suboptions. */ > const struct sanitizer_opts_s sanitizer_opts[] = > { > -#define SANITIZER_OPT(name, flags, recover) \ > - { #name, flags, sizeof #name - 1, recover } > - SANITIZER_OPT (address, (SANITIZE_ADDRESS | SANITIZE_USER_ADDRESS), true), > +#define SANITIZER_OPT(name, flags, recover, trap) \ > + { #name, flags, sizeof #name - 1, recover, trap } > + SANITIZER_OPT (address, (SANITIZE_ADDRESS | SANITIZE_USER_ADDRESS), true, > + false), > SANITIZER_OPT (hwaddress, (SANITIZE_HWADDRESS | SANITIZE_USER_HWADDRESS), > - true), > + true, false), > SANITIZER_OPT (kernel-address, (SANITIZE_ADDRESS | SANITIZE_KERNEL_ADDRESS), > - true), > + true, false), > SANITIZER_OPT (kernel-hwaddress, > (SANITIZE_HWADDRESS | SANITIZE_KERNEL_HWADDRESS), > + true, false), > + SANITIZER_OPT (pointer-compare, SANITIZE_POINTER_COMPARE, true, false), > + SANITIZER_OPT (pointer-subtract, SANITIZE_POINTER_SUBTRACT, true, false), > + SANITIZER_OPT (thread, SANITIZE_THREAD, false, false), > + SANITIZER_OPT (leak, SANITIZE_LEAK, false, false), > + SANITIZER_OPT (shift, SANITIZE_SHIFT, true, true), > + SANITIZER_OPT (shift-base, SANITIZE_SHIFT_BASE, true, true), > + SANITIZER_OPT (shift-exponent, SANITIZE_SHIFT_EXPONENT, true, true), > + SANITIZER_OPT (integer-divide-by-zero, SANITIZE_DIVIDE, true, true), > + SANITIZER_OPT (undefined, SANITIZE_UNDEFINED, true, true), > + SANITIZER_OPT (unreachable, SANITIZE_UNREACHABLE, false, true), > + SANITIZER_OPT (vla-bound, SANITIZE_VLA, true, true), > + SANITIZER_OPT (return, SANITIZE_RETURN, false, true), > + SANITIZER_OPT (null, SANITIZE_NULL, true, true), > + SANITIZER_OPT (signed-integer-overflow, SANITIZE_SI_OVERFLOW, true, true), > + SANITIZER_OPT (bool, SANITIZE_BOOL, true, true), > + SANITIZER_OPT (enum, SANITIZE_ENUM, true, true), > + SANITIZER_OPT (float-divide-by-zero, SANITIZE_FLOAT_DIVIDE, true, true), > + SANITIZER_OPT (float-cast-overflow, SANITIZE_FLOAT_CAST, true, true), > + SANITIZER_OPT (bounds, SANITIZE_BOUNDS, true, true), > + SANITIZER_OPT (bounds-strict, SANITIZE_BOUNDS | SANITIZE_BOUNDS_STRICT, true, > true), > - SANITIZER_OPT (pointer-compare, SANITIZE_POINTER_COMPARE, true), > - SANITIZER_OPT (pointer-subtract, SANITIZE_POINTER_SUBTRACT, true), > - SANITIZER_OPT (thread, SANITIZE_THREAD, false), > - SANITIZER_OPT (leak, SANITIZE_LEAK, false), > - SANITIZER_OPT (shift, SANITIZE_SHIFT, true), > - SANITIZER_OPT (shift-base, SANITIZE_SHIFT_BASE, true), > - SANITIZER_OPT (shift-exponent, SANITIZE_SHIFT_EXPONENT, true), > - SANITIZER_OPT (integer-divide-by-zero, SANITIZE_DIVIDE, true), > - SANITIZER_OPT (undefined, SANITIZE_UNDEFINED, true), > - SANITIZER_OPT (unreachable, SANITIZE_UNREACHABLE, false), > - SANITIZER_OPT (vla-bound, SANITIZE_VLA, true), > - SANITIZER_OPT (return, SANITIZE_RETURN, false), > - SANITIZER_OPT (null, SANITIZE_NULL, true), > - SANITIZER_OPT (signed-integer-overflow, SANITIZE_SI_OVERFLOW, true), > - SANITIZER_OPT (bool, SANITIZE_BOOL, true), > - SANITIZER_OPT (enum, SANITIZE_ENUM, true), > - SANITIZER_OPT (float-divide-by-zero, SANITIZE_FLOAT_DIVIDE, true), > - SANITIZER_OPT (float-cast-overflow, SANITIZE_FLOAT_CAST, true), > - SANITIZER_OPT (bounds, SANITIZE_BOUNDS, true), > - SANITIZER_OPT (bounds-strict, SANITIZE_BOUNDS | SANITIZE_BOUNDS_STRICT, true), > - SANITIZER_OPT (alignment, SANITIZE_ALIGNMENT, true), > - SANITIZER_OPT (nonnull-attribute, SANITIZE_NONNULL_ATTRIBUTE, true), > + SANITIZER_OPT (alignment, SANITIZE_ALIGNMENT, true, true), > + SANITIZER_OPT (nonnull-attribute, SANITIZE_NONNULL_ATTRIBUTE, true, true), > SANITIZER_OPT (returns-nonnull-attribute, SANITIZE_RETURNS_NONNULL_ATTRIBUTE, > - true), > - SANITIZER_OPT (object-size, SANITIZE_OBJECT_SIZE, true), > - SANITIZER_OPT (vptr, SANITIZE_VPTR, true), > - SANITIZER_OPT (pointer-overflow, SANITIZE_POINTER_OVERFLOW, true), > - SANITIZER_OPT (builtin, SANITIZE_BUILTIN, true), > - SANITIZER_OPT (shadow-call-stack, SANITIZE_SHADOW_CALL_STACK, false), > - SANITIZER_OPT (all, ~0U, true), > + true, true), > + SANITIZER_OPT (object-size, SANITIZE_OBJECT_SIZE, true, true), > + SANITIZER_OPT (vptr, SANITIZE_VPTR, true, false), > + SANITIZER_OPT (pointer-overflow, SANITIZE_POINTER_OVERFLOW, true, true), > + SANITIZER_OPT (builtin, SANITIZE_BUILTIN, true, true), > + SANITIZER_OPT (shadow-call-stack, SANITIZE_SHADOW_CALL_STACK, false, false), > + SANITIZER_OPT (all, ~0U, true, true), > #undef SANITIZER_OPT > - { NULL, 0U, 0UL, false } > + { NULL, 0U, 0UL, false, false } > }; > > /* -fzero-call-used-regs= suboptions. */ > @@ -2114,7 +2128,7 @@ struct edit_distance_traits<const string > /* Given ARG, an unrecognized sanitizer option, return the best > matching sanitizer option, or NULL if there isn't one. > OPTS is array of candidate sanitizer options. > - CODE is OPT_fsanitize_ or OPT_fsanitize_recover_. > + CODE is OPT_fsanitize_, OPT_fsanitize_recover_ or OPT_fsanitize_trap_. > VALUE is non-zero for the regular form of the option, zero > for the "no-" form (e.g. "-fno-sanitize-recover="). */ > > @@ -2139,6 +2153,13 @@ get_closest_sanitizer_option (const stri > && value) > continue; > > + /* For -fsanitize-trap= (and not -fno-sanitize-trap=), > + don't offer the non-trapping options. */ > + if (code == OPT_fsanitize_trap_ > + && !opts[i].can_trap > + && value) > + continue; > + > bm.consider (opts[i].name); > } > return bm.get_best_meaningful_candidate (); > @@ -2183,10 +2204,13 @@ parse_sanitizer_options (const char *p, > if (complain) > error_at (loc, "%<-fsanitize=all%> option is not valid"); > } > - else > + else if (code == OPT_fsanitize_recover_) > flags |= ~(SANITIZE_THREAD | SANITIZE_LEAK > | SANITIZE_UNREACHABLE | SANITIZE_RETURN > | SANITIZE_SHADOW_CALL_STACK); > + else This could use a comment that this is about -fsanitize-trap. Otherwise LGTM. > + flags |= (SANITIZE_UNDEFINED > + | SANITIZE_UNDEFINED_NONDEFAULT); > } > else if (value) > { > @@ -2197,6 +2221,10 @@ parse_sanitizer_options (const char *p, > && sanitizer_opts[i].flag == SANITIZE_UNDEFINED) > flags |= (SANITIZE_UNDEFINED > & ~(SANITIZE_UNREACHABLE | SANITIZE_RETURN)); > + else if (code == OPT_fsanitize_trap_ > + && sanitizer_opts[i].flag == SANITIZE_VPTR) > + error_at (loc, "%<-fsanitize-trap=%s%> is not supported", > + sanitizer_opts[i].name); > else > flags |= sanitizer_opts[i].flag; > } > @@ -2215,6 +2243,8 @@ parse_sanitizer_options (const char *p, > const char *suffix; > if (code == OPT_fsanitize_recover_) > suffix = "-recover"; > + else if (code == OPT_fsanitize_trap_) > + suffix = "-trap"; > else > suffix = ""; > > @@ -2647,6 +2677,12 @@ common_handle_option (struct gcc_options > opts->x_flag_sanitize_recover, value, true); > break; > > + case OPT_fsanitize_trap_: > + opts->x_flag_sanitize_trap > + = parse_sanitizer_options (arg, loc, code, > + opts->x_flag_sanitize_trap, value, true); > + break; > + > case OPT_fasan_shadow_offset_: > /* Deferred. */ > break; > @@ -2665,6 +2701,15 @@ common_handle_option (struct gcc_options > &= ~(SANITIZE_UNDEFINED | SANITIZE_UNDEFINED_NONDEFAULT); > break; > > + case OPT_fsanitize_trap: > + if (value) > + opts->x_flag_sanitize_trap > + |= (SANITIZE_UNDEFINED | SANITIZE_UNDEFINED_NONDEFAULT); > + else > + opts->x_flag_sanitize_trap > + &= ~(SANITIZE_UNDEFINED | SANITIZE_UNDEFINED_NONDEFAULT); > + break; > + > case OPT_O: > case OPT_Os: > case OPT_Ofast: > --- gcc/sanopt.cc.jj 2022-01-18 11:58:59.886977714 +0100 > +++ gcc/sanopt.cc 2022-06-17 14:07:18.021155983 +0200 > @@ -392,11 +392,11 @@ maybe_optimize_ubsan_null_ifn (class san > stmts have same location. */ > else if (integer_zerop (align)) > remove = (flag_sanitize_recover & SANITIZE_NULL) == 0 > - || flag_sanitize_undefined_trap_on_error > + || (flag_sanitize_trap & SANITIZE_NULL) != 0 > || gimple_location (g) == gimple_location (stmt); > else if (tree_int_cst_le (cur_align, align)) > remove = (flag_sanitize_recover & SANITIZE_ALIGNMENT) == 0 > - || flag_sanitize_undefined_trap_on_error > + || (flag_sanitize_trap & SANITIZE_ALIGNMENT) != 0 > || gimple_location (g) == gimple_location (stmt); > > if (!remove && gimple_bb (g) == gimple_bb (stmt) > --- gcc/gcc.cc.jj 2022-06-03 11:20:13.231070818 +0200 > +++ gcc/gcc.cc 2022-06-17 13:34:16.482830857 +0200 > @@ -10313,8 +10313,9 @@ sanitize_spec_function (int argc, const > return (flag_sanitize & SANITIZE_THREAD) ? "" : NULL; > if (strcmp (argv[0], "undefined") == 0) > return ((flag_sanitize > - & (SANITIZE_UNDEFINED | SANITIZE_UNDEFINED_NONDEFAULT)) > - && !flag_sanitize_undefined_trap_on_error) ? "" : NULL; > + & ~flag_sanitize_trap > + & (SANITIZE_UNDEFINED | SANITIZE_UNDEFINED_NONDEFAULT))) > + ? "" : NULL; > if (strcmp (argv[0], "leak") == 0) > return ((flag_sanitize > & (SANITIZE_ADDRESS | SANITIZE_LEAK | SANITIZE_THREAD)) > --- gcc/ubsan.cc.jj 2022-05-13 09:34:24.043146671 +0200 > +++ gcc/ubsan.cc 2022-06-17 14:14:42.164217896 +0200 > @@ -647,7 +647,7 @@ ubsan_instrument_unreachable (gimple_stm > gimple *g; > location_t loc = gimple_location (gsi_stmt (*gsi)); > > - if (flag_sanitize_undefined_trap_on_error) > + if (flag_sanitize_trap & SANITIZE_UNREACHABLE) > g = gimple_build_call (builtin_decl_explicit (BUILT_IN_TRAP), 0); > else > { > @@ -719,7 +719,7 @@ ubsan_expand_bounds_ifn (gimple_stmt_ite > > /* Generate __ubsan_handle_out_of_bounds call. */ > *gsi = gsi_after_labels (then_bb); > - if (flag_sanitize_undefined_trap_on_error) > + if (flag_sanitize_trap & SANITIZE_BOUNDS) > g = gimple_build_call (builtin_decl_explicit (BUILT_IN_TRAP), 0); > else > { > @@ -827,7 +827,8 @@ ubsan_expand_null_ifn (gimple_stmt_itera > set_immediate_dominator (CDI_DOMINATORS, then_bb, cond_bb); > > /* Put the ubsan builtin call into the newly created BB. */ > - if (flag_sanitize_undefined_trap_on_error) > + if (flag_sanitize_trap & ((check_align ? SANITIZE_ALIGNMENT + 0 : 0) > + | (check_null ? SANITIZE_NULL + 0 : 0))) > g = gimple_build_call (builtin_decl_implicit (BUILT_IN_TRAP), 0); > else > { > @@ -997,7 +998,7 @@ ubsan_expand_objsize_ifn (gimple_stmt_it > } > > /* Generate __ubsan_handle_type_mismatch call. */ > - if (flag_sanitize_undefined_trap_on_error) > + if (flag_sanitize_trap & SANITIZE_OBJECT_SIZE) > g = gimple_build_call (builtin_decl_explicit (BUILT_IN_TRAP), 0); > else > { > @@ -1143,7 +1144,7 @@ ubsan_expand_ptr_ifn (gimple_stmt_iterat > } > > /* Put the ubsan builtin call into the newly created BB. */ > - if (flag_sanitize_undefined_trap_on_error) > + if (flag_sanitize_trap & SANITIZE_POINTER_OVERFLOW) > g = gimple_build_call (builtin_decl_implicit (BUILT_IN_TRAP), 0); > else > { > @@ -1518,7 +1519,7 @@ tree > ubsan_build_overflow_builtin (tree_code code, location_t loc, tree lhstype, > tree op0, tree op1, tree *datap) > { > - if (flag_sanitize_undefined_trap_on_error) > + if (flag_sanitize_trap & SANITIZE_SI_OVERFLOW) > return build_call_expr_loc (loc, builtin_decl_explicit (BUILT_IN_TRAP), 0); > > tree data; > @@ -1741,7 +1742,8 @@ instrument_bool_enum_load (gimple_stmt_i > } > > gsi2 = gsi_after_labels (then_bb); > - if (flag_sanitize_undefined_trap_on_error) > + if (flag_sanitize_trap & (TREE_CODE (type) == BOOLEAN_TYPE > + ? SANITIZE_BOOL : SANITIZE_ENUM)) > g = gimple_build_call (builtin_decl_explicit (BUILT_IN_TRAP), 0); > else > { > @@ -1904,7 +1906,7 @@ ubsan_instrument_float_cast (location_t > if (integer_zerop (t)) > return NULL_TREE; > > - if (flag_sanitize_undefined_trap_on_error) > + if (flag_sanitize_trap & SANITIZE_FLOAT_CAST) > fn = build_call_expr_loc (loc, builtin_decl_explicit (BUILT_IN_TRAP), 0); > else > { > @@ -1974,7 +1976,7 @@ instrument_nonnull_arg (gimple_stmt_iter > gsi_insert_after (gsi, g, GSI_NEW_STMT); > > *gsi = gsi_after_labels (then_bb); > - if (flag_sanitize_undefined_trap_on_error) > + if (flag_sanitize_trap & SANITIZE_NONNULL_ATTRIBUTE) > g = gimple_build_call (builtin_decl_explicit (BUILT_IN_TRAP), 0); > else > { > @@ -2030,7 +2032,7 @@ instrument_nonnull_return (gimple_stmt_i > gsi_insert_after (gsi, g, GSI_NEW_STMT); > > *gsi = gsi_after_labels (then_bb); > - if (flag_sanitize_undefined_trap_on_error) > + if (flag_sanitize_trap & SANITIZE_RETURNS_NONNULL_ATTRIBUTE) > g = gimple_build_call (builtin_decl_explicit (BUILT_IN_TRAP), 0); > else > { > @@ -2279,7 +2281,7 @@ instrument_builtin (gimple_stmt_iterator > gsi_insert_after (gsi, g, GSI_NEW_STMT); > > *gsi = gsi_after_labels (then_bb); > - if (flag_sanitize_undefined_trap_on_error) > + if (flag_sanitize_trap & SANITIZE_BUILTIN) > g = gimple_build_call (builtin_decl_explicit (BUILT_IN_TRAP), 0); > else > { > --- gcc/doc/invoke.texi.jj 2022-06-16 10:56:28.826386840 +0200 > +++ gcc/doc/invoke.texi 2022-06-17 16:42:59.370670655 +0200 > @@ -609,6 +609,7 @@ Objective-C and Objective-C++ Dialects}. > -fprofile-exclude-files=@var{regex} @gol > -fprofile-reproducible=@r{[}multithreaded@r{|}parallel-runs@r{|}serial@r{]} @gol > -fsanitize=@var{style} -fsanitize-recover -fsanitize-recover=@var{style} @gol > +-fsanitize-trap -fsanitize-trap=@var{style} @gol > -fasan-shadow-offset=@var{number} -fsanitize-sections=@var{s1},@var{s2},... @gol > -fsanitize-undefined-trap-on-error -fbounds-check @gol > -fcf-protection=@r{[}full@r{|}branch@r{|}return@r{|}none@r{|}check@r{]} @gol > @@ -16116,13 +16117,37 @@ undefined,float-cast-overflow,float-divi > Enable sanitization of local variables to detect use-after-scope bugs. > The option sets @option{-fstack-reuse} to @samp{none}. > > +@item -fsanitize-trap@r{[}=@var{opts}@r{]} > +@opindex fsanitize-trap > +@opindex fno-sanitize-trap > +The @option{-fsanitize-trap=} option instructs the compiler to > +report for sanitizers mentioned in comma-separated list of @var{opts} > +undefined behavior using @code{__builtin_trap} rather than a @code{libubsan} > +library routine. If this option is enabled for certain sanitizer, > +it takes precedence over the @option{-fsanitizer-recover=} for that > +sanitizer, @code{__builtin_trap} will be emitted and be fatal regardless > +of whether recovery is enabled or disabled using @option{-fsanitize-recover=}. > + > +The advantage of this is that the @code{libubsan} library is not needed > +and is not linked in, so this is usable even in freestanding environments. > + > +Currently this feature works with @option{-fsanitize=undefined} (and its suboptions > +except for @option{-fsanitize=vptr}), @option{-fsanitize=float-cast-overflow}, > +@option{-fsanitize=float-divide-by-zero} and > +@option{-fsanitize=bounds-strict}. @code{-fsanitize-trap=all} can be also > +specified, which enables it for @code{undefined} suboptions, > +@option{-fsanitize=float-cast-overflow}, > +@option{-fsanitize=float-divide-by-zero} and > +@option{-fsanitize=bounds-strict}. > +If @code{-fsanitize-trap=undefined} or @code{-fsanitize-trap=all} is used > +and @code{-fsanitize=vptr} is enabled on the command line, the > +instrumentation is silently ignored as the instrumentation always needs > +@code{libubsan} support, @option{-fsanitize-trap=vptr} is not allowed. > + > @item -fsanitize-undefined-trap-on-error > @opindex fsanitize-undefined-trap-on-error > -The @option{-fsanitize-undefined-trap-on-error} option instructs the compiler to > -report undefined behavior using @code{__builtin_trap} rather than > -a @code{libubsan} library routine. The advantage of this is that the > -@code{libubsan} library is not needed and is not linked in, so this > -is usable even in freestanding environments. > +The @option{-fsanitize-undefined-trap-on-error} option is deprecated > +equivalent of @option{-fsanitize-trap=all}. > > @item -fsanitize-coverage=trace-pc > @opindex fsanitize-coverage=trace-pc > --- gcc/c-family/c-ubsan.cc.jj 2022-06-14 13:17:01.559722905 +0200 > +++ gcc/c-family/c-ubsan.cc 2022-06-17 15:23:16.899389988 +0200 > @@ -83,8 +83,9 @@ ubsan_instrument_division (location_t lo > x = NULL_TREE; > flag = SANITIZE_SI_OVERFLOW; > } > - else if (flag_sanitize_undefined_trap_on_error > - || (((flag_sanitize_recover & SANITIZE_DIVIDE) == 0) > + else if ((((flag_sanitize_trap & SANITIZE_DIVIDE) == 0) > + == ((flag_sanitize_trap & SANITIZE_SI_OVERFLOW) == 0)) > + && (((flag_sanitize_recover & SANITIZE_DIVIDE) == 0) > == ((flag_sanitize_recover & SANITIZE_SI_OVERFLOW) == 0))) > { > t = fold_build2 (TRUTH_OR_EXPR, boolean_type_node, t, x); > @@ -105,7 +106,7 @@ ubsan_instrument_division (location_t lo > make sure it gets evaluated before the condition. */ > t = fold_build2 (COMPOUND_EXPR, TREE_TYPE (t), unshare_expr (op0), t); > t = fold_build2 (COMPOUND_EXPR, TREE_TYPE (t), unshare_expr (op1), t); > - if (flag_sanitize_undefined_trap_on_error) > + if ((flag_sanitize_trap & flag) && x == NULL_TREE) > tt = build_call_expr_loc (loc, builtin_decl_explicit (BUILT_IN_TRAP), 0); > else > { > @@ -113,25 +114,41 @@ ubsan_instrument_division (location_t lo > ubsan_type_descriptor (type), NULL_TREE, > NULL_TREE); > data = build_fold_addr_expr_loc (loc, data); > - enum built_in_function bcode > - = (flag_sanitize_recover & flag) > - ? BUILT_IN_UBSAN_HANDLE_DIVREM_OVERFLOW > - : BUILT_IN_UBSAN_HANDLE_DIVREM_OVERFLOW_ABORT; > - tt = builtin_decl_explicit (bcode); > - op0 = unshare_expr (op0); > - op1 = unshare_expr (op1); > - tt = build_call_expr_loc (loc, tt, 3, data, ubsan_encode_value (op0), > - ubsan_encode_value (op1)); > - if (x) > + if (flag_sanitize_trap & flag) > + tt = build_call_expr_loc (loc, builtin_decl_explicit (BUILT_IN_TRAP), > + 0); > + else > { > - bcode = (flag_sanitize_recover & SANITIZE_SI_OVERFLOW) > - ? BUILT_IN_UBSAN_HANDLE_DIVREM_OVERFLOW > - : BUILT_IN_UBSAN_HANDLE_DIVREM_OVERFLOW_ABORT; > - tree xt = builtin_decl_explicit (bcode); > + enum built_in_function bcode > + = (flag_sanitize_recover & flag) > + ? BUILT_IN_UBSAN_HANDLE_DIVREM_OVERFLOW > + : BUILT_IN_UBSAN_HANDLE_DIVREM_OVERFLOW_ABORT; > + tt = builtin_decl_explicit (bcode); > op0 = unshare_expr (op0); > op1 = unshare_expr (op1); > - xt = build_call_expr_loc (loc, xt, 3, data, ubsan_encode_value (op0), > + tt = build_call_expr_loc (loc, tt, 3, data, ubsan_encode_value (op0), > ubsan_encode_value (op1)); > + } > + if (x) > + { > + tree xt; > + if (flag_sanitize_trap & SANITIZE_SI_OVERFLOW) > + xt = build_call_expr_loc (loc, > + builtin_decl_explicit (BUILT_IN_TRAP), > + 0); > + else > + { > + enum built_in_function bcode > + = (flag_sanitize_recover & SANITIZE_SI_OVERFLOW) > + ? BUILT_IN_UBSAN_HANDLE_DIVREM_OVERFLOW > + : BUILT_IN_UBSAN_HANDLE_DIVREM_OVERFLOW_ABORT; > + xt = builtin_decl_explicit (bcode); > + op0 = unshare_expr (op0); > + op1 = unshare_expr (op1); > + xt = build_call_expr_loc (loc, xt, 3, data, > + ubsan_encode_value (op0), > + ubsan_encode_value (op1)); > + } > x = fold_build3 (COND_EXPR, void_type_node, x, xt, void_node); > } > } > @@ -225,8 +242,9 @@ ubsan_instrument_shift (location_t loc, > } > else > { > - if (flag_sanitize_undefined_trap_on_error > - || ((!(flag_sanitize_recover & SANITIZE_SHIFT_EXPONENT)) > + if (((!(flag_sanitize_trap & SANITIZE_SHIFT_EXPONENT)) > + == (!(flag_sanitize_trap & SANITIZE_SHIFT_BASE))) > + && ((!(flag_sanitize_recover & SANITIZE_SHIFT_EXPONENT)) > == (!(flag_sanitize_recover & SANITIZE_SHIFT_BASE)))) > t = fold_build2 (TRUTH_OR_EXPR, boolean_type_node, t, tt); > else > @@ -234,7 +252,7 @@ ubsan_instrument_shift (location_t loc, > } > } > > - if (flag_sanitize_undefined_trap_on_error) > + if ((flag_sanitize_trap & recover_kind) && else_t == void_node) > tt = build_call_expr_loc (loc, builtin_decl_explicit (BUILT_IN_TRAP), 0); > else > { > @@ -244,26 +262,40 @@ ubsan_instrument_shift (location_t loc, > NULL_TREE); > data = build_fold_addr_expr_loc (loc, data); > > - enum built_in_function bcode > - = (flag_sanitize_recover & recover_kind) > - ? BUILT_IN_UBSAN_HANDLE_SHIFT_OUT_OF_BOUNDS > - : BUILT_IN_UBSAN_HANDLE_SHIFT_OUT_OF_BOUNDS_ABORT; > - tt = builtin_decl_explicit (bcode); > - op0 = unshare_expr (op0); > - op1 = unshare_expr (op1); > - tt = build_call_expr_loc (loc, tt, 3, data, ubsan_encode_value (op0), > - ubsan_encode_value (op1)); > + if (flag_sanitize_trap & recover_kind) > + tt = build_call_expr_loc (loc, builtin_decl_explicit (BUILT_IN_TRAP), 0); > + else > + { > + enum built_in_function bcode > + = (flag_sanitize_recover & recover_kind) > + ? BUILT_IN_UBSAN_HANDLE_SHIFT_OUT_OF_BOUNDS > + : BUILT_IN_UBSAN_HANDLE_SHIFT_OUT_OF_BOUNDS_ABORT; > + tt = builtin_decl_explicit (bcode); > + op0 = unshare_expr (op0); > + op1 = unshare_expr (op1); > + tt = build_call_expr_loc (loc, tt, 3, data, ubsan_encode_value (op0), > + ubsan_encode_value (op1)); > + } > if (else_t != void_node) > { > - bcode = (flag_sanitize_recover & SANITIZE_SHIFT_BASE) > + tree else_tt; > + if (flag_sanitize_trap & SANITIZE_SHIFT_BASE) > + else_tt > + = build_call_expr_loc (loc, > + builtin_decl_explicit (BUILT_IN_TRAP), 0); > + else > + { > + enum built_in_function bcode > + = (flag_sanitize_recover & SANITIZE_SHIFT_BASE) > ? BUILT_IN_UBSAN_HANDLE_SHIFT_OUT_OF_BOUNDS > : BUILT_IN_UBSAN_HANDLE_SHIFT_OUT_OF_BOUNDS_ABORT; > - tree else_tt = builtin_decl_explicit (bcode); > - op0 = unshare_expr (op0); > - op1 = unshare_expr (op1); > - else_tt = build_call_expr_loc (loc, else_tt, 3, data, > - ubsan_encode_value (op0), > - ubsan_encode_value (op1)); > + else_tt = builtin_decl_explicit (bcode); > + op0 = unshare_expr (op0); > + op1 = unshare_expr (op1); > + else_tt = build_call_expr_loc (loc, else_tt, 3, data, > + ubsan_encode_value (op0), > + ubsan_encode_value (op1)); > + } > else_t = fold_build3 (COND_EXPR, void_type_node, else_t, > else_tt, void_node); > } > @@ -282,7 +314,7 @@ ubsan_instrument_vla (location_t loc, tr > tree t, tt; > > t = fold_build2 (LE_EXPR, boolean_type_node, size, build_int_cst (type, 0)); > - if (flag_sanitize_undefined_trap_on_error) > + if (flag_sanitize_trap & SANITIZE_VLA) > tt = build_call_expr_loc (loc, builtin_decl_explicit (BUILT_IN_TRAP), 0); > else > { > @@ -307,10 +339,10 @@ ubsan_instrument_vla (location_t loc, tr > tree > ubsan_instrument_return (location_t loc) > { > - if (flag_sanitize_undefined_trap_on_error) > - return build_call_expr_loc > - /* pass_warn_function_return checks for BUILTINS_LOCATION. */ > - (BUILTINS_LOCATION, builtin_decl_explicit (BUILT_IN_TRAP), 0); > + if (flag_sanitize_trap & SANITIZE_RETURN) > + /* pass_warn_function_return checks for BUILTINS_LOCATION. */ > + return build_call_expr_loc (BUILTINS_LOCATION, > + builtin_decl_explicit (BUILT_IN_TRAP), 0); > > tree data = ubsan_create_data ("__ubsan_missing_return_data", 1, &loc, > NULL_TREE, NULL_TREE); > --- gcc/cp/cp-ubsan.cc.jj 2022-01-18 11:58:59.299986100 +0100 > +++ gcc/cp/cp-ubsan.cc 2022-06-17 15:14:01.760791859 +0200 > @@ -32,7 +32,7 @@ along with GCC; see the file COPYING3. > static bool > cp_ubsan_instrument_vptr_p (tree type) > { > - if (!flag_rtti || flag_sanitize_undefined_trap_on_error) > + if (!flag_rtti || (flag_sanitize_trap & SANITIZE_VPTR)) > return false; > > if (!sanitize_flags_p (SANITIZE_VPTR)) > --- gcc/testsuite/c-c++-common/ubsan/nonnull-4.c.jj 2020-01-12 11:54:37.029404115 +0100 > +++ gcc/testsuite/c-c++-common/ubsan/nonnull-4.c 2022-06-17 16:53:38.976152460 +0200 > @@ -1,6 +1,6 @@ > /* { dg-do run } */ > /* { dg-shouldfail "ubsan" } */ > -/* { dg-options "-fsanitize=undefined -fsanitize-undefined-trap-on-error" } */ > +/* { dg-options "-fsanitize=undefined -fsanitize-trap=all" } */ > > int q, r; > void *a, *b, *c = (void *) &q, *d, *e, *f = (void *) &q, *g, *h; > --- gcc/testsuite/c-c++-common/ubsan/div-by-zero-4.c.jj 2021-12-30 15:12:43.248150079 +0100 > +++ gcc/testsuite/c-c++-common/ubsan/div-by-zero-4.c 2022-06-17 16:53:02.139643041 +0200 > @@ -1,5 +1,5 @@ > /* { dg-do run } */ > -/* { dg-options "-fsanitize=signed-integer-overflow -fsanitize-undefined-trap-on-error -Wno-overflow" } */ > +/* { dg-options "-fsanitize=signed-integer-overflow -fsanitize-trap=signed-integer-overflow -Wno-overflow" } */ > > #define INT_MIN (-__INT_MAX__ - 1) > > --- gcc/testsuite/c-c++-common/ubsan/overflow-add-4.c.jj 2020-01-12 11:54:37.030404100 +0100 > +++ gcc/testsuite/c-c++-common/ubsan/overflow-add-4.c 2022-06-17 16:54:12.544705407 +0200 > @@ -1,5 +1,5 @@ > /* { dg-do run } */ > -/* { dg-options "-fsanitize=signed-integer-overflow -Wno-unused-variable -fsanitize-undefined-trap-on-error" } */ > +/* { dg-options "-fsanitize=signed-integer-overflow -Wno-unused-variable -fsanitize-trap=undefined" } */ > /* { dg-shouldfail "ubsan" } */ > > #define INT_MAX __INT_MAX__ > --- gcc/testsuite/c-c++-common/ubsan/pr56956.c.jj 2020-01-12 11:54:37.031404085 +0100 > +++ gcc/testsuite/c-c++-common/ubsan/pr56956.c 2022-06-17 16:54:35.557398933 +0200 > @@ -1,5 +1,5 @@ > /* { dg-do run } */ > -/* { dg-options "-fsanitize=undefined -fsanitize-undefined-trap-on-error" } */ > +/* { dg-options "-fsanitize=undefined -fsanitize-trap=undefined" } */ > > unsigned int __attribute__((noinline,noclone)) > foo (unsigned int x) > --- gcc/testsuite/c-c++-common/ubsan/pr68142.c.jj 2020-01-12 11:54:37.031404085 +0100 > +++ gcc/testsuite/c-c++-common/ubsan/pr68142.c 2022-06-17 16:55:02.426041091 +0200 > @@ -1,5 +1,5 @@ > /* { dg-do run } */ > -/* { dg-options "-fsanitize=undefined -fsanitize-undefined-trap-on-error" } */ > +/* { dg-options "-fsanitize=undefined -fsanitize-trap=undefined" } */ > > int __attribute__((noinline,noclone)) > h(int a) > --- gcc/testsuite/c-c++-common/ubsan/pr80932.c.jj 2020-01-12 11:54:37.032404070 +0100 > +++ gcc/testsuite/c-c++-common/ubsan/pr80932.c 2022-06-17 16:55:59.401282322 +0200 > @@ -1,6 +1,6 @@ > /* PR sanitizer/80932 */ > /* { dg-do run } */ > -/* { dg-options "-fsanitize=undefined -fsanitize-undefined-trap-on-error" } */ > +/* { dg-options "-fsanitize=undefined -fno-sanitize-trap=all -fsanitize-trap=shift,undefined" } */ > > int x = 1; > > --- gcc/testsuite/c-c++-common/ubsan/align-8.c.jj 2020-01-12 11:54:37.028404130 +0100 > +++ gcc/testsuite/c-c++-common/ubsan/align-8.c 2022-06-17 16:50:57.827298591 +0200 > @@ -1,6 +1,6 @@ > /* Limit this to known non-strict alignment targets. */ > /* { dg-do run { target { i?86-*-linux* x86_64-*-linux* } } } */ > -/* { dg-options "-O -fsanitize=alignment -fsanitize-undefined-trap-on-error -Wno-address-of-packed-member -fdump-tree-sanopt-details" } */ > +/* { dg-options "-O -fsanitize=alignment -fsanitize-trap=alignment -Wno-address-of-packed-member -fdump-tree-sanopt-details" } */ > /* { dg-skip-if "" { *-*-* } { "-flto -fno-fat-lto-objects" } } */ > /* { dg-shouldfail "ubsan" } */ > > > > Jakub > ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2022-06-22 8:06 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-06-13 19:53 [PATCH RFA] ubsan: default to trap on unreachable at -O0 and -Og [PR104642] Jason Merrill 2022-06-14 11:44 ` Jakub Jelinek 2022-06-15 20:38 ` Jason Merrill 2022-06-16 13:14 ` Jakub Jelinek 2022-06-20 20:30 ` Jason Merrill 2022-06-21 11:17 ` Jakub Jelinek 2022-06-22 3:59 ` Jason Merrill 2022-06-22 8:05 ` Jakub Jelinek 2022-06-16 20:32 ` Jonathan Wakely 2022-06-16 20:53 ` Jakub Jelinek 2022-06-17 15:34 ` [PATCH] ubsan: Add -fsanitize-trap= support Jakub Jelinek 2022-06-17 18:21 ` Jason Merrill
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).