From: Jason Merrill <jason@redhat.com>
To: Jakub Jelinek <jakub@redhat.com>,
Richard Biener <rguenther@suse.de>,
Jeff Law <jeffreyalaw@gmail.com>,
Jonathan Wakely <jwakely.gcc@gmail.com>
Cc: Marek Polacek <polacek@redhat.com>, gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] ubsan: Add -fsanitize-trap= support
Date: Fri, 17 Jun 2022 14:21:33 -0400 [thread overview]
Message-ID: <1d8d9982-764d-92c8-eafb-1c6ff4d4c747@redhat.com> (raw)
In-Reply-To: <YqyfE7v03ecNZQkM@tucnak>
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
>
prev parent reply other threads:[~2022-06-17 18:21 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
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 message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1d8d9982-764d-92c8-eafb-1c6ff4d4c747@redhat.com \
--to=jason@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=jakub@redhat.com \
--cc=jeffreyalaw@gmail.com \
--cc=jwakely.gcc@gmail.com \
--cc=polacek@redhat.com \
--cc=rguenther@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).