Re: [PATCH] Improve DSE to handle stores before __builtin_unreachable ()

[Jan Hubicka <hubicka@ucw.cz>]

> 
> 
> On 6/22/23 00:31, Richard Biener wrote:
> > I think there's a difference in that __builtin_trap () is observable
> > while __builtin_unreachable () is not and reaching __builtin_unreachable
> > () invokes undefined behavior while reaching __builtin_trap () does not.
> > 
> > So the isolation code marking the trapping code volatile should be
> > enough and the trap () is just there to end the basic block
> > (and maybe be on the safe side to really trap).
> Agreed WRT observability -- but that's not really the point of the trap and
> if we wanted we could change that behavior.
> 
> The trap is there to halt execution immediately rather than letting it keep
> running.  That was a design decision from a security standpoint. If we've
> detected that we're executing undefined behavior, stop rather than
> potentially letting a malicious actor turn a bug into an exploit.

Also as discussed some time ago, the volatile loads between traps has
effect of turning previously pure/const functions into non-const which
is somewhat sad, so it is still on my todo list to change it this stage1
to something more careful.   We discussed internal functions trap_store
and trap_load which will expand to load/store + trap but will make it
clear that side effect does not count to modref.

I wanted to give it some time if I can come with something better, but
didn't so far.

Honza