Am Sonntag, dem 13.10.2024 um 10:56 +0200 schrieb Richard Biener:
> On Sat, 12 Oct 2024, Martin Uecker wrote:
>
> > Am Samstag, dem 12.10.2024 um 18:44 +0200 schrieb Richard Biener:
> > >
> > > > Am 12.10.2024 um 16:43 schrieb Martin Uecker <uec...@tugraz.at>:
> > > >
> > > >
> > > > There is code which should not fail at run-time. For this,
> > > > it is helpful to get a warning when a compiler inserts traps
> > > > (e.g. sanitizers, hardbools, __builtin_trap(), etc.).
> > > >
> > > > Having a warning for this also has many other use cases, e.g.
> > > > one can use it with some sanitizer to rule out that some
> > > > piece of code has certain undefined behavior such as
> > > > signed overflow or undefined behavior in left-shifts
> > > > (one gets a warning if the optimizer does not prove the
> > > > trap is dead and it is emitted).
> > > >
> > > > Another potential use case could be writing tests.
> > > >
> > > >
> > > > Bootstrapped and regression tested on x64_84.
> > > >
> > > >
> > > > Add warning option that warns when a trap is generated.
> > > >
> > > > This adds a warning option -Wtrap that is emitted in
> > > > expand_builtin_trap. It can be used to verify that traps
> > > > are generated or - more importantly - are not generated
> > > > under various conditions, e.g. for UBSan with -fsanitize-trap,
> > > > hardbools, etc.
> > >
> > > Isn’t it better to diagnose with more context from the callers that
> > > insert the trap?
> >
> > More context would be better. Should there be additional
> > arguments when creating the call to the builtin?
>
> Why not diagnose when we create the call?
I agree that having optional warnings for all situation where there
could be run-time UB (or a trap) would be useful. But having a
generic warning for all such situations would produce many warnings
and also cover cases where we already have more specific warnings.
Doing it when the trap is generated directly gives me somewhat
different information that I sometimes need: Is there a trap left
in the generated binary?
We have a similar warning already for generating trampolines.
Before adding the warning to my local tree, I often looked at the
generated assembly to look for generated "ud2" instructions. But this
is painful and gives me even less context.
A practical example from failing to properly take integer
promotions into account (adapted from a old bug in a crypto
library) is:
uint32_t bar(int N, unsigned char key)
{
unsigned int kappa = key << 24;
return kappa;
}
which has UB that the warning tells me about and
where adding a cast is required to eliminate it:
https://godbolt.org/z/osvEsdcqc
> But sure, adding a diagnostic
> argument would work, it might also work to distinguish calls we want to
> diagnose from those we don't.
Would it be reasonable to approve this patch now and I try
to improve this later?
Martin
