Am Dienstag, dem 15.10.2024 um 12:15 +0200 schrieb Jakub Jelinek:
> On Tue, Oct 15, 2024 at 11:50:21AM +0200, Richard Biener wrote:
> > > Would it be reasonable to approve this patch now and I try
> > > to improve this later?
> >
> > On the patch itself:
> >
> > void
> > expand_builtin_trap (void)
> > {
> > + if (warn_trap)
> > + {
> > + location_t current_location =
>
> Formatting-wise, = shouldn't be at the end of line.
>
> > + linemap_unwind_to_first_non_reserved_loc (line_table,
> > input_location,
> > + NULL);
> > + warning_at (current_location, OPT_Wtrap, "trap generated");
> > + }
> > +
> > if (targetm.have_trap ())
> >
> > this also diagnoses calls the user puts in by calling __builtin_trap (),
> > the documentation should probably mention this. I see the only testcase
> > exercises only this path. I have doubts -fsanitize-trap with any
> > sanitizer will ever yield a clean binary, so I wonder about practical
> > uses besides very small testcases?
>
> Given that even simple
> int foo (int x, int y) { return x + y; }
> calls it for -fsanitize=undefined -fsanitize-trap=undefined, and more
> importantly, we try not to optimize away sanitizer checks based on VRP and
> other optimizations at least to some extent, because VRP and other
> optimizations optimize on UB not happening while sanitizers try to catch
> the UB, I have serious doubts about the warning.
> One would need completely different approach, where we try as much as
> possible to prove UB can't happen and only warn if we couldn't prove it
> can't. Still, there would be tons of false positives where things just
> aren't inlined and we can't prove UB won't happen, or warnings on dead
> code...
>
It would not be practical with some sanitizers (and sanitizers are also not
the only usecase anyhow). But attached below is the list of emitted warnings
for different sanitizers depending on the optimization level for
BART (https://github.com/mrirecon/bart). The first number is optimization,
then the number of warnings, and then the number with different warnings
from the same source location (e.g. macros) combined. The project has
>400 C source files.
For some sanitizers the number is rather low and e.g. shift is something
I was looking at specifically and where I already found this useful. But
also for some the others the numbers are low enough to investigate each case.
For new code, an average number of warnings per file of 10 would also
still seem entirely reasonable, so I think one could eliminate all
integer overflow cases if one wanted to. But also doing this only for
criticial pieces of code (e.g. using #pragma GCC diagnostic ...) would
seem useful.
One also sees that the optimizer actually does remove a lot of traps. That
the total number sometimes goes up with -O2 is mostly likely due to code
duplication, but the unique cases always go down with more optimization.
(except for object-size which does not run fully for -O1 I believe)
I have to admit I did not understand your comment about VRP, but
in my experience we remove UB sanitization based on value ranges, e.g.
in this example for signed overflow.
https://godbolt.org/z/zeMvas3nq
Martin
shift 0 525 31
shift 1 298 19
shift 2 221 19
signed-integer-overflow 0 12212 11350
signed-integer-overflow 1 6310 5913
signed-integer-overflow 2 3803 3172
integer-divide-by-zero 0 332 332
integer-divide-by-zero 1 135 135
integer-divide-by-zero 2 95 95
unreachable 0 0 0
unreachable 1 0 0
unreachable 2 4 4
vla-bound 0 2671 2426
vla-bound 1 1206 1143
vla-bound 2 1024 906
null 0 32041 26739
null 1 8926 7666
null 2 8622 6769
return 0 0 0
return 1 0 0
return 2 4 4
bounds 0 5031 4830
bounds 1 4776 4551
bounds 2 4986 4547
bounds-strict 0 5031 4830
bounds-strict 1 4776 4551
bounds-strict 2 4986 4547
alignment 0 31818 26524
alignment 1 9848 8441
alignment 2 10031 7855
object-size 0 465 453
object-size 1 5928 5491
object-size 2 6013 5366
float-divide-by-zero 0 532 532
float-divide-by-zero 1 430 429
float-divide-by-zero 2 250 245
float-cast-overflow 0 107 98
float-cast-overflow 1 99 91
float-cast-overflow 2 65 59
nonnull-attribute 0 13433 7672
nonnull-attribute 1 5199 3466
nonnull-attribute 2 1085 1005
return-nonnull-attribute 0 0 0
return-nonnull-attribute 1 0 0
return-nonnull-attribute 2 0 0
bool 0 1053 1036
bool 1 988 972
bool 2 371 363
enum 0 0 0
enum 1 0 0
enum 2 4 4
pointer-overflow 0 29095 25298
pointer-overflow 1 17284 16254
pointer-overflow 2 18598 16110
builtin 0 0 0
builtin 1 0 0
builtin 2 4 4
