On Thu, 15 Nov 2018, Jakub Jelinek wrote:
> Hi!
>
> On the following testcase, we have a call (not marked noreturn), which can
> throw, followed immediately by __builtin_unreachable (), so it effectively
> is noreturn in this particular call site (i.e. if it returns, it is UB).
> In RTL this is represented by the corresponding bb having just EH edge
> successor, no fallthrough edge. If one of the callers of
> fixup_abnormal_edges (the stack pass in this case, or LRA or reload) adds
> some instructions after the call and calls fixup_abnormal_edges, it ICEs,
> as it tries to delete those insns and insert them on the NULL edge.
> Somebody has hit that issue already but solved it for USE insns only, this
> patch does that for all the cases where we don't have the fallthru edge.
> If there weren't any instructions after the call before one of these 3
> passes, I'd say it must be UB to return from such a call and thus it should
> be unimportant if those insns are executed or not.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK.
Richard.
> 2018-11-15 Jakub Jelinek <ja...@redhat.com>
>
> PR rtl-optimization/88018
> * cfgrtl.c (fixup_abnormal_edges): Guard moving insns to fallthru edge
> on the presence of fallthru edge, rather than if it is a USE or not.
>
> * g++.dg/tsan/pr88018.C: New test.
>
> --- gcc/cfgrtl.c.jj 2018-11-14 00:55:51.695333876 +0100
> +++ gcc/cfgrtl.c 2018-11-14 17:24:46.414915101 +0100
> @@ -3332,8 +3332,15 @@ fixup_abnormal_edges (void)
> If it's placed after a trapping call (i.e. that
> call is the last insn anyway), we have no fallthru
> edge. Simply delete this use and don't try to insert
> - on the non-existent edge. */
> - if (GET_CODE (PATTERN (insn)) != USE)
> + on the non-existent edge.
> + Similarly, sometimes a call that can throw is
> + followed in the source with __builtin_unreachable (),
> + meaning that there is UB if the call returns rather
> + than throws. If there weren't any instructions
> + following such calls before, supposedly even the ones
> + we've deleted aren't significant and can be
> + removed. */
> + if (e)
> {
> /* We're not deleting it, we're moving it. */
> insn->set_undeleted ();
> --- gcc/testsuite/g++.dg/tsan/pr88018.C.jj 2018-11-14 17:26:46.224944969
> +0100
> +++ gcc/testsuite/g++.dg/tsan/pr88018.C 2018-11-14 17:28:40.057073142
> +0100
> @@ -0,0 +1,6 @@
> +// PR rtl-optimization/88018
> +// { dg-do compile }
> +// { dg-skip-if "" { *-*-* } { "*" } { "-O0" } }
> +// { dg-options "-fsanitize=thread -fno-ipa-pure-const -O1
> -fno-inline-functions-called-once -w" }
> +
> +#include "../pr69667.C"
>
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
