Re: [PATCH/RFC] On the use of -funreachable-traps to deal with PR 109627

[Richard Biener <richard.guenther@gmail.com>]

On Tue, Apr 9, 2024 at 9:11 AM Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Tue, Apr 09, 2024 at 09:03:59AM +0200, Richard Biener wrote:
> > > With the possibility of sounding like a broken record, I think
> > > __builtin_unreachable is fundamentally flawed.   It generates no code
> > > and just lets the program continue if ever "reached".  This is a
> > > security risk and (IMHO) just plain silly.  We're in a situation that is
> > > never supposed to happen, so continuing to execute code is just asking
> > > for problems.
> > >
> > > If it were up to me, I'd have __builtin_unreachable emit a trap or
> > > similar construct that should (in general) halt execution.
> >
> > __builtin_unreachable tells the compiler it's OK to omit a path to it
> > while __builtin_trap doesn't.  So once we replace the former with the
> > latter we have to keep the path.  Maybe that's OK.  I do agree that
> > the RTL representation of expanding __builtin_unreachable () to
> > "nothing" is bad.  Expanding to a trap always would be OK with me.
>
> Even that would prevent tons of needed optimizations, especially the
> reason why __builtin_unreachable () has been added in the first place
> - for asm goto which always branches and so the kernel can put
> __builtin_unreachable () after it to say that it won't fall through.
> I think the kernel folks would be upset if we change that.
>
> So, can't we instead just emit a trap when in the last cfglayout -> cfgrtl
> switch we see that the last bb in the function doesn't have any successors?

That's probably a good middle-ground if we can identify that "last" switch
easily (why not do it at each such switch?)

Richard.

>         Jakub
>