Am Mittwoch, dem 24.09.2025 um 09:55 +0200 schrieb Richard Biener:
> On Wed, 24 Sep 2025, Martin Uecker wrote:
>
> > Am Dienstag, dem 23.09.2025 um 08:57 +0200 schrieb Richard Biener:
> > > On Tue, 23 Sep 2025, Martin Uecker wrote:
> > >
> > > > Am Montag, dem 22.09.2025 um 11:00 +0200 schrieb Richard Biener:
> > > > > On Sat, 20 Sep 2025, Martin Uecker wrote:
> > > > >
> > > > > > Bootstrapped and regression tested on x86_64.
> > > > >
> > > > > How's it useful when all UBSAN generated traps get a compile-time
> > > > > diagnostic? Specifically for the testcase you add, how is this
> > > > > diagnostig useful?
> > > >
> > > > My intention was that this works for all traps the compiler
> > > > generates, so this would natually include UBsan generated
> > > > traps. I think it would be rather strange to exclude them. The
> > > > testcase is not mean to illustrate a use case.
> > > >
> > > > I personally find it useful to better understand what UBsan
> > > > does to the code. Clearly, if you simply apply it to a regular
> > > > C program compiled with UBsan this will give you a lot of warnings,
> > > > and this will not be too useful except maybe for statistics and
> > > > overall impact. But in more specific cases it is useful to
> > > > better understand whether some error scenario is instrumented and
> > > > whether it can actually occur or not.
> > >
> > > Hmm, but this then sounds like a debugging feature, not something
> > > for a diagnostic? What's wrong with notes in dump files? It
> > > _might_ also fit -fopt-info (though it's not really an "optimization").
> >
> > Often I am interested in what I need to fix in the source code to avoid
> > the trap (because this may not be acceptable, e.g. when controlling some
> > hardware or because I want to remove performance impact of having the
> > conditional check). So, I like a diagnostic because it ties it back to
> > the source code and existing tools can handle this nicely. For example,
> > consider this example:
> >
> > maybe(int) safe_divide(int a, int b)
> > {
> > if (b == 0)
> > return maybe_nothing(int);
> >
> > return maybe_just(int, a / b);
> > }
> >
> > I would expect that compiling with
> >
> > -fsanitize=undefined -fsanitize-trap=all -O2
> >
> > comes up clean because I tested for zero and I expect the later trap to
> > be removed by the optimizer. But with the warning it tells me:
> >
> >
> > div.c: In function ‘safe_divide’:
> > div.c:13:34: warning: trap generated: "builtin" [-Wtrap]
> > 13 | return maybe_just(int, a / b);
> > | ^
> >
>
> I think it's moot to argue from the point of single division testcases.
>
> Your argument is that it is a validation tool as well as a tool
> to validate optimization. The latter would be, in the embedded
> setting, equally valid for say the question whether there are
> any remaining calls to divdi3 or whether all divisions were
> emitted inline using non-divisons as a user might have expected.
I agree that this might also be useful, but if you do not
have a run-time you would already get the error during linking.
There seems no risk that you accidentally emit a call to "divdi3"
and then it randomly fails in production. But this is an issue
with traps.
Currently, if I want to make sure there is no trap, the only way
I can do this is screen in the assembly or dissect the binary
and there are scenarios where I need to treat every potential
trap as an error. And this is my argument why this should be
a diagnostic.
>
> I stand by the argument that a -W diagnostic does not seem to be
> a good fit (there's -Winline which IMO is a similary bad fit).
I am trying to understand why you think this is not a good fit.
Is it because it somehow depends on optimization?
But the key thing I want to test is whether there is a trap left
in the binary or not, and if it is then this is a problem I need
to investigate. The optimizer helps in the sense as it would
remove false positives (traps that are not a problem because they
are unreachable), but what I care about is that in the end there
is no trap left. It is similar to -Wtrampoline.
>
> I also think that in practice you want to have tooling around such
> "static analysis", wading throug a build log doesn't scale.
It does not scale if you apply it to regular C code where you
might get all lot of warnings. But I am not proposing to activate
it by default.
It scales if you already need to make sure you code can not
trap / have certain issues and then it beats any alternative
I know of.
Martin
>
> > When I also add the missing check for overflow there is no trap
> > anymore and the code then does what I expect it to do. This is
> > why I think a diagnostic is the right tool.
> >
> > https://godbolt.org/z/1cajbs481
> >
> >
> > Before even running the code, this would give me useful information
> > about potential issues. If I want to find such issues only using
> > run-time testing with the sanitizer, my best shot would be to use
> > a fuzzer, but this still misses many cases. People have been looking
> > at the assembly output instead, but this is not very nice and misses
> > too much context (it would be nice though to propagate the reason
> > string also into the assembly, but I do not know enough how this
> > can be done.)
> >
> >
> > One could consider this a misuse of the sanitizer with optimization,
> > but it in practice this works very well as GCC is very good in
> > removing unnecessary sanitzer checks. (Most instrumentation is
> > done early, so sometimes I also get better code with the sanitizer
> > because it injects additional information the optimizer can then
> > exploit.)
> >
> >
> >
> > >
> > > I'd find it equally useful, in the UBSAN context, when the compiler
> > > was able to (fully) elide a UBSAN instrumentation.
> >
> > You would get this with -O0, but I am not sure what your usecase is.
> >
> >
> > In WG14 we discuss having an optional safety mode where one would get
> > a warning about potentially unsafe code. But this would be a FE thing.
> >
> > Martin
> >
> >
> > >
> > > Richard.
> > >
> > > > >
> > > > > I'd instead have expected path isolation to diagnose inserted traps.
> > > >
> > > > It will be diagnosed by the patch, if I am not missing anything.
> > > > But yes, this should get its own reason string as well.
> > > >
> > > > Martin
> > > >
> > > >
> > > > >
> > > > > Richard.
> > > > >
> > > > > >
> > > > > > Add reason string to compiler emitted traps.
> > > > > >
> > > > > > For traps generated by the compiler, add a short string stating
> > > > > > the reason. This string will be shown with -Wtrap when the trap
> > > > > > is actually emitted.
> > > > > >
> > > > > > gcc/ChangeLog:
> > > > > > * builtins.cc (expand_builtin_trap): Add string
> > > > > > argument.
> > > > > > (expand_builtin): Add reasons for traps.
> > > > > > (expand_builtin_object_size): Don't emit message after
> > > > > > error.
> > > > > > * expr.cc (expand_assignment): Add reason for trap.
> > > > > > * stmt.cc (expand_sjlj_dispatch_table): Add reason for
> > > > > > trap.
> > > > > > * ubsan.cc (ubsan_build_string): New function.
> > > > > > (ubsan_build_string_ptr): New function.
> > > > > > (ubsan_trap): New functions.
> > > > > > (ubsan_source_location): Use ubsan_build_string_ptr.
> > > > > > (ubsan_type_descriptor): Use ubsan_build_string.
> > > > > > (ubsan_expand_bounds_ifn): Use ubsan_trap.
> > > > > > (ubsan_expand_bounds_null): Dito.
> > > > > > (ubsan_expand_objsize_ifn): Dito.
> > > > > > (ubsan_expand_ptr_ifn): Dito.
> > > > > > (instrument_bool_enum_load): Dito.
> > > > > > (instrument_nonnull_arg): Dito.
> > > > > > (instrument_nonnull_return): Dito.
> > > > > > (instrument_builtin): Dito.
> > > > > > (ubsan_build_overflow_builtin): Add message.
> > > > > > (ubsan_instrument_float_case): Dito.
> > > > > >
> > > > > > gcc/testsuite/ChangeLog:
> > > > > > * gcc.dg/Wtrap-2.c: New test.
> > > > > >
> > > > > > diff --git a/gcc/builtins.cc b/gcc/builtins.cc
> > > > > > index de5c96f3a25..2e3ac9df86d 100644
> > > > > > --- a/gcc/builtins.cc
> > > > > > +++ b/gcc/builtins.cc
> > > > > > @@ -5968,10 +5968,10 @@ expand_builtin_trap_no_msg (void)
> > > > > > /* Expand a call to __builtin_trap. */
> > > > > >
> > > > > > void
> > > > > > -expand_builtin_trap ()
> > > > > > +expand_builtin_trap (const char *reason)
> > > > > > {
> > > > > > if (warn_trap)
> > > > > > - warning_at (input_location, OPT_Wtrap, "trap generated");
> > > > > > + warning_at (input_location, OPT_Wtrap, "trap generated:
> > > > > > \"%.99s\"", reason);
> > > > > >
> > > > > > return expand_builtin_trap_no_msg ();
> > > > > > }
> > > > > > @@ -8448,8 +8448,11 @@ expand_builtin (tree exp, rtx target, rtx
> > > > > > subtarget, machine_mode mode,
> > > > > > return const0_rtx;
> > > > > >
> > > > > > case BUILT_IN_TRAP:
> > > > > > + expand_builtin_trap ("builtin");
> > > > > > + return const0_rtx;
> > > > > > +
> > > > > > case BUILT_IN_UNREACHABLE_TRAP:
> > > > > > - expand_builtin_trap ();
> > > > > > + expand_builtin_trap ("unreachable");
> > > > > > return const0_rtx;
> > > > > >
> > > > > > case BUILT_IN_UNREACHABLE:
> > > > > > @@ -11554,7 +11557,7 @@ expand_builtin_object_size (tree exp)
> > > > > > {
> > > > > > error ("first argument of %qD must be a pointer, second
> > > > > > integer constant",
> > > > > > fndecl);
> > > > > > - expand_builtin_trap ();
> > > > > > + expand_builtin_trap_no_msg ();
> > > > > > return const0_rtx;
> > > > > > }
> > > > > >
> > > > > > @@ -11567,7 +11570,7 @@ expand_builtin_object_size (tree exp)
> > > > > > {
> > > > > > error ("last argument of %qD is not integer constant between
> > > > > > 0 and 3",
> > > > > > fndecl);
> > > > > > - expand_builtin_trap ();
> > > > > > + expand_builtin_trap_no_msg ();
> > > > > > return const0_rtx;
> > > > > > }
> > > > > >
> > > > > > diff --git a/gcc/builtins.h b/gcc/builtins.h
> > > > > > index 5a553a9c836..ae4ba4a9b96 100644
> > > > > > --- a/gcc/builtins.h
> > > > > > +++ b/gcc/builtins.h
> > > > > > @@ -129,7 +129,7 @@ extern tree std_build_builtin_va_list (void);
> > > > > > extern tree std_fn_abi_va_list (tree);
> > > > > > extern tree std_canonical_va_list_type (tree);
> > > > > > extern void std_expand_builtin_va_start (tree, rtx);
> > > > > > -extern void expand_builtin_trap (void);
> > > > > > +extern void expand_builtin_trap (const char *);
> > > > > > extern void expand_ifn_atomic_bit_test_and (gcall *);
> > > > > > extern void expand_ifn_atomic_compare_exchange (gcall *);
> > > > > > extern void expand_ifn_atomic_op_fetch_cmp_0 (gcall *);
> > > > > > diff --git a/gcc/expr.cc b/gcc/expr.cc
> > > > > > index 4a699101bb5..c6fc3b25f42 100644
> > > > > > --- a/gcc/expr.cc
> > > > > > +++ b/gcc/expr.cc
> > > > > > @@ -6102,7 +6102,7 @@ expand_assignment (tree to, tree from, bool
> > > > > > nontemporal)
> > > > > > user code. Translate this to a trap instead of
> > > > > > ICEing. */
> > > > > > if (TREE_CODE (offset) == INTEGER_CST)
> > > > > > {
> > > > > > - expand_builtin_trap ();
> > > > > > + expand_builtin_trap ("invalid offset");
> > > > > > to_rtx = gen_rtx_MEM (BLKmode, const0_rtx);
> > > > > > }
> > > > > > /* Else spill for variable offset to the destination. We
> > > > > > expect
> > > > > > diff --git a/gcc/stmt.cc b/gcc/stmt.cc
> > > > > > index 7942aa3e484..2e95b207603 100644
> > > > > > --- a/gcc/stmt.cc
> > > > > > +++ b/gcc/stmt.cc
> > > > > > @@ -1393,7 +1393,7 @@ expand_sjlj_dispatch_table (rtx
> > > > > > dispatch_index,
> > > > > > }
> > > > > >
> > > > > > /* Dispatching something not handled? Trap! */
> > > > > > - expand_builtin_trap ();
> > > > > > + expand_builtin_trap ("dispatch not handled");
> > > > > >
> > > > > > reorder_insns (NEXT_INSN (before_case), get_last_insn (),
> > > > > > before_case);
> > > > > >
> > > > > > diff --git a/gcc/testsuite/gcc.dg/Wtrap-2.c
> > > > > > b/gcc/testsuite/gcc.dg/Wtrap-2.c
> > > > > > new file mode 100644
> > > > > > index 00000000000..ce2e596d947
> > > > > > --- /dev/null
> > > > > > +++ b/gcc/testsuite/gcc.dg/Wtrap-2.c
> > > > > > @@ -0,0 +1,13 @@
> > > > > > +/* { dg-do compile } */
> > > > > > +/* { dg-options "-Wtrap -fsanitize=bounds -fsanitize-trap=bounds"
> > > > > > } */
> > > > > > +
> > > > > > +void foo(int n, char (*buf)[n])
> > > > > > +{
> > > > > > + (*buf)[10] = 1; /* { dg-warning "trap
> > > > > > generated: \"bounds (sanitizer)\"" } */
> > > > > > +}
> > > > > > +
> > > > > > +void bar()
> > > > > > +{
> > > > > > + __builtin_unreachable(); /* { dg-warning "trap
> > > > > > generated: \"unreachable\"" } */
> > > > > > +}
> > > > > > +
> > > > > > diff --git a/gcc/ubsan.cc b/gcc/ubsan.cc
> > > > > > index 6d748258b1e..801f4c7830e 100644
> > > > > > --- a/gcc/ubsan.cc
> > > > > > +++ b/gcc/ubsan.cc
> > > > > > @@ -303,6 +303,35 @@ ubsan_get_source_location_type (void)
> > > > > > return ret;
> > > > > > }
> > > > > >
> > > > > > +
> > > > > > +static tree
> > > > > > +ubsan_build_string (const char *cstr)
> > > > > > +{
> > > > > > + size_t len = strlen (cstr) + 1;
> > > > > > + tree str = build_string (len, cstr);
> > > > > > + TREE_TYPE (str) = build_array_type_nelts (char_type_node, len);
> > > > > > + TREE_READONLY (str) = 1;
> > > > > > + TREE_STATIC (str) = 1;
> > > > > > + return str;
> > > > > > +}
> > > > > > +
> > > > > > +static tree
> > > > > > +ubsan_build_string_ptr (const char *cstr)
> > > > > > +{
> > > > > > + return build_fold_addr_expr (ubsan_build_string (cstr));
> > > > > > +}
> > > > > > +
> > > > > > +/* Helper routine that expands a trap with a message. */
> > > > > > +
> > > > > > +static gimple *
> > > > > > +ubsan_trap (const char *cstr)
> > > > > > +{
> > > > > > + return gimple_build_call (builtin_decl_explicit
> > > > > > (BUILT_IN_TRAP_MSG),
> > > > > > + 1, ubsan_build_string_ptr (cstr));
> > > > > > +}
> > > > > > +
> > > > > > +
> > > > > > +
> > > > > > /* Helper routine that returns a CONSTRUCTOR of
> > > > > > __ubsan_source_location
> > > > > > type with its fields filled from a location_t LOC. */
> > > > > >
> > > > > > @@ -323,12 +352,7 @@ ubsan_source_location (location_t loc)
> > > > > > else
> > > > > > {
> > > > > > /* Fill in the values from LOC. */
> > > > > > - size_t len = strlen (xloc.file) + 1;
> > > > > > - str = build_string (len, xloc.file);
> > > > > > - TREE_TYPE (str) = build_array_type_nelts (char_type_node,
> > > > > > len);
> > > > > > - TREE_READONLY (str) = 1;
> > > > > > - TREE_STATIC (str) = 1;
> > > > > > - str = build_fold_addr_expr (str);
> > > > > > + str = ubsan_build_string_ptr (xloc.file);
> > > > > > }
> > > > > > tree ctor = build_constructor_va (type, 3, NULL_TREE, str,
> > > > > > NULL_TREE,
> > > > > > build_int_cst (unsigned_type_node,
> > > > > > @@ -542,11 +566,7 @@ ubsan_type_descriptor (tree type, enum
> > > > > > ubsan_print_style pstyle)
> > > > > >
> > > > > > /* Create a new VAR_DECL of type descriptor. */
> > > > > > const char *tmp = pp_formatted_text (&pretty_name);
> > > > > > - size_t len = strlen (tmp) + 1;
> > > > > > - tree str = build_string (len, tmp);
> > > > > > - TREE_TYPE (str) = build_array_type_nelts (char_type_node, len);
> > > > > > - TREE_READONLY (str) = 1;
> > > > > > - TREE_STATIC (str) = 1;
> > > > > > + tree str = ubsan_build_string (tmp);
> > > > > >
> > > > > > decl = build_decl (UNKNOWN_LOCATION, VAR_DECL,
> > > > > > generate_internal_label ("Lubsan_type"), dtype);
> > > > > > @@ -786,7 +806,7 @@ ubsan_expand_bounds_ifn (gimple_stmt_iterator
> > > > > > *gsi)
> > > > > > /* Generate __ubsan_handle_out_of_bounds call. */
> > > > > > *gsi = gsi_after_labels (then_bb);
> > > > > > if (flag_sanitize_trap & SANITIZE_BOUNDS)
> > > > > > - g = gimple_build_call (builtin_decl_explicit (BUILT_IN_TRAP),
> > > > > > 0);
> > > > > > + g = ubsan_trap ("bounds (sanitizer)");
> > > > > > else
> > > > > > {
> > > > > > tree data
> > > > > > @@ -902,7 +922,7 @@ ubsan_expand_null_ifn (gimple_stmt_iterator
> > > > > > *gsip)
> > > > > > /* Put the ubsan builtin call into the newly created BB. */
> > > > > > 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);
> > > > > > + g = ubsan_trap ("alignment / null (sanitizer)");
> > > > > > else
> > > > > > {
> > > > > > enum built_in_function bcode
> > > > > > @@ -1072,7 +1092,7 @@ ubsan_expand_objsize_ifn
> > > > > > (gimple_stmt_iterator *gsi)
> > > > > >
> > > > > > /* Generate __ubsan_handle_type_mismatch call. */
> > > > > > if (flag_sanitize_trap & SANITIZE_OBJECT_SIZE)
> > > > > > - g = gimple_build_call (builtin_decl_explicit (BUILT_IN_TRAP),
> > > > > > 0);
> > > > > > + g = ubsan_trap ("object size (sanitizer)");
> > > > > > else
> > > > > > {
> > > > > > tree data
> > > > > > @@ -1218,7 +1238,7 @@ ubsan_expand_ptr_ifn (gimple_stmt_iterator
> > > > > > *gsip)
> > > > > >
> > > > > > /* Put the ubsan builtin call into the newly created BB. */
> > > > > > if (flag_sanitize_trap & SANITIZE_POINTER_OVERFLOW)
> > > > > > - g = gimple_build_call (builtin_decl_implicit (BUILT_IN_TRAP),
> > > > > > 0);
> > > > > > + g = ubsan_trap ("pointer overflow (sanitizer)");
> > > > > > else
> > > > > > {
> > > > > > enum built_in_function bcode
> > > > > > @@ -1601,8 +1621,11 @@ ubsan_build_overflow_builtin (tree_code
> > > > > > code, location_t loc, tree lhstype,
> > > > > > tree op0, tree op1, tree *datap)
> > > > > > {
> > > > > > if (flag_sanitize_trap & SANITIZE_SI_OVERFLOW)
> > > > > > - return build_call_expr_loc (loc, builtin_decl_explicit
> > > > > > (BUILT_IN_TRAP), 0);
> > > > > > -
> > > > > > + {
> > > > > > + tree reason = ubsan_build_string_ptr ("signed overflow
> > > > > > (sanitizer)");
> > > > > > + return build_call_expr_loc (loc, builtin_decl_explicit
> > > > > > (BUILT_IN_TRAP_MSG),
> > > > > > + 1, reason);
> > > > > > + }
> > > > > > tree data;
> > > > > > if (datap && *datap)
> > > > > > data = *datap;
> > > > > > @@ -1830,7 +1853,7 @@ instrument_bool_enum_load
> > > > > > (gimple_stmt_iterator *gsi)
> > > > > > gsi2 = gsi_after_labels (then_bb);
> > > > > > if (flag_sanitize_trap & (TREE_CODE (type) == BOOLEAN_TYPE
> > > > > > ? SANITIZE_BOOL : SANITIZE_ENUM))
> > > > > > - g = gimple_build_call (builtin_decl_explicit (BUILT_IN_TRAP),
> > > > > > 0);
> > > > > > + g = ubsan_trap ("bool / enum (sanitizer)");
> > > > > > else
> > > > > > {
> > > > > > tree data = ubsan_create_data ("__ubsan_invalid_value_data",
> > > > > > 1, &loc,
> > > > > > @@ -1991,7 +2014,11 @@ ubsan_instrument_float_cast (location_t loc,
> > > > > > tree type, tree expr)
> > > > > > return NULL_TREE;
> > > > > >
> > > > > > if (flag_sanitize_trap & SANITIZE_FLOAT_CAST)
> > > > > > - fn = build_call_expr_loc (loc, builtin_decl_explicit
> > > > > > (BUILT_IN_TRAP), 0);
> > > > > > + {
> > > > > > + tree reason = ubsan_build_string_ptr ("float cast
> > > > > > (sanitizer)");
> > > > > > + fn = build_call_expr_loc (loc, builtin_decl_explicit
> > > > > > (BUILT_IN_TRAP_MSG),
> > > > > > + 1, reason);
> > > > > > + }
> > > > > > else
> > > > > > {
> > > > > > location_t *loc_ptr = NULL;
> > > > > > @@ -2102,7 +2129,7 @@ instrument_nonnull_arg (gimple_stmt_iterator
> > > > > > *gsi)
> > > > > > *gsi = gsi_after_labels (then_bb);
> > > > > > }
> > > > > > if (flag_sanitize_trap & SANITIZE_NONNULL_ATTRIBUTE)
> > > > > > - g = gimple_build_call (builtin_decl_explicit
> > > > > > (BUILT_IN_TRAP), 0);
> > > > > > + g = ubsan_trap ("nonnull (sanitizer)");
> > > > > > else
> > > > > > {
> > > > > > tree data = ubsan_create_data ("__ubsan_nonnull_arg_data",
> > > > > > @@ -2158,7 +2185,7 @@ instrument_nonnull_return
> > > > > > (gimple_stmt_iterator *gsi)
> > > > > >
> > > > > > *gsi = gsi_after_labels (then_bb);
> > > > > > if (flag_sanitize_trap & SANITIZE_RETURNS_NONNULL_ATTRIBUTE)
> > > > > > - g = gimple_build_call (builtin_decl_explicit (BUILT_IN_TRAP),
> > > > > > 0);
> > > > > > + g = ubsan_trap ("returns nonnull (sanitizer)");
> > > > > > else
> > > > > > {
> > > > > > tree data = ubsan_create_data ("__ubsan_nonnull_return_data",
> > > > > > @@ -2404,7 +2431,7 @@ instrument_builtin (gimple_stmt_iterator *gsi)
> > > > > >
> > > > > > *gsi = gsi_after_labels (then_bb);
> > > > > > if (flag_sanitize_trap & SANITIZE_BUILTIN)
> > > > > > - g = gimple_build_call (builtin_decl_explicit
> > > > > > (BUILT_IN_TRAP), 0);
> > > > > > + g = ubsan_trap ("builtin (sanitizer)");
> > > > > > else
> > > > > > {
> > > > > > tree t = build_int_cst (unsigned_char_type_node, kind);
> > > > > >
> > > >
> >
--
Univ.-Prof. Dr. rer. nat. Martin Uecker
Graz University of Technology
Institute of Biomedical Imaging
