* [PATCH] Fix libgo breakage (PR tree-optimization/67284) @ 2015-08-21 11:00 Marek Polacek 2015-08-21 12:07 ` Richard Biener 0 siblings, 1 reply; 13+ messages in thread From: Marek Polacek @ 2015-08-21 11:00 UTC (permalink / raw) To: GCC Patches, Jeff Law, Richard Biener This fixes the libgo breakage. Seems I really should have removed the edge after we split the block with null dereference after __builtin_trap statement so that it's unreachable. Bootstrapped/regtested on x86_64-linux + ppc64-linux + bootstrapped on aarch64-linux, ok for trunk? 2015-08-21 Marek Polacek <polacek@redhat.com> PR tree-optimization/67284 * gimple-ssa-isolate-paths.c (insert_trap): Remove the edge after we split a block. diff --git gcc/gimple-ssa-isolate-paths.c gcc/gimple-ssa-isolate-paths.c index ca2322d..5ac61d3 100644 --- gcc/gimple-ssa-isolate-paths.c +++ gcc/gimple-ssa-isolate-paths.c @@ -115,7 +115,8 @@ insert_trap (gimple_stmt_iterator *si_p, tree op) else gsi_insert_before (si_p, seq, GSI_NEW_STMT); - split_block (gimple_bb (new_stmt), new_stmt); + edge e = split_block (gimple_bb (new_stmt), new_stmt); + remove_edge (e); *si_p = gsi_for_stmt (stmt); } Marek ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Fix libgo breakage (PR tree-optimization/67284) 2015-08-21 11:00 [PATCH] Fix libgo breakage (PR tree-optimization/67284) Marek Polacek @ 2015-08-21 12:07 ` Richard Biener 2015-08-21 12:50 ` Marek Polacek 0 siblings, 1 reply; 13+ messages in thread From: Richard Biener @ 2015-08-21 12:07 UTC (permalink / raw) To: Marek Polacek; +Cc: GCC Patches, Jeff Law, Richard Biener On Fri, Aug 21, 2015 at 12:52 PM, Marek Polacek <polacek@redhat.com> wrote: > This fixes the libgo breakage. Seems I really should have removed the > edge after we split the block with null dereference after __builtin_trap > statement so that it's unreachable. > > Bootstrapped/regtested on x86_64-linux + ppc64-linux + bootstrapped on > aarch64-linux, ok for trunk? Hum. I don't see why this is needed - CFG cleanup (which of course needs to run!) should do this for you. In fact stray unreachable blocks are usually more of a problem. Richard. > 2015-08-21 Marek Polacek <polacek@redhat.com> > > PR tree-optimization/67284 > * gimple-ssa-isolate-paths.c (insert_trap): Remove the edge after > we split a block. > > diff --git gcc/gimple-ssa-isolate-paths.c gcc/gimple-ssa-isolate-paths.c > index ca2322d..5ac61d3 100644 > --- gcc/gimple-ssa-isolate-paths.c > +++ gcc/gimple-ssa-isolate-paths.c > @@ -115,7 +115,8 @@ insert_trap (gimple_stmt_iterator *si_p, tree op) > else > gsi_insert_before (si_p, seq, GSI_NEW_STMT); > > - split_block (gimple_bb (new_stmt), new_stmt); > + edge e = split_block (gimple_bb (new_stmt), new_stmt); > + remove_edge (e); > *si_p = gsi_for_stmt (stmt); > } > > > Marek ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Fix libgo breakage (PR tree-optimization/67284) 2015-08-21 12:07 ` Richard Biener @ 2015-08-21 12:50 ` Marek Polacek 2015-08-21 13:40 ` Richard Biener 0 siblings, 1 reply; 13+ messages in thread From: Marek Polacek @ 2015-08-21 12:50 UTC (permalink / raw) To: Richard Biener; +Cc: GCC Patches, Jeff Law, Richard Biener On Fri, Aug 21, 2015 at 01:27:43PM +0200, Richard Biener wrote: > On Fri, Aug 21, 2015 at 12:52 PM, Marek Polacek <polacek@redhat.com> wrote: > > This fixes the libgo breakage. Seems I really should have removed the > > edge after we split the block with null dereference after __builtin_trap > > statement so that it's unreachable. > > > > Bootstrapped/regtested on x86_64-linux + ppc64-linux + bootstrapped on > > aarch64-linux, ok for trunk? > > Hum. I don't see why this is needed - CFG cleanup (which of course needs > to run!) should do this for you. In fact stray unreachable blocks are usually > more of a problem. Aha. It seems cleanup does that if I change the code to generate __builtin_unreachable instead of __builtin_trap. A hint maybe? ;) What's going on here is that we find a potential NULL dereference (PHI argument has NULL), we call isolate_path, that duplicates a bb and then cuts off the outgoing edges of this duplicated bb. And in some cases it leaves COND_EXPR at the end of the bb -> ICE. The we insert __builtin_trap on this isolated path. Marek ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Fix libgo breakage (PR tree-optimization/67284) 2015-08-21 12:50 ` Marek Polacek @ 2015-08-21 13:40 ` Richard Biener 2015-08-21 14:43 ` Marek Polacek 0 siblings, 1 reply; 13+ messages in thread From: Richard Biener @ 2015-08-21 13:40 UTC (permalink / raw) To: Marek Polacek; +Cc: GCC Patches, Jeff Law, Richard Biener On Fri, Aug 21, 2015 at 2:49 PM, Marek Polacek <polacek@redhat.com> wrote: > On Fri, Aug 21, 2015 at 01:27:43PM +0200, Richard Biener wrote: >> On Fri, Aug 21, 2015 at 12:52 PM, Marek Polacek <polacek@redhat.com> wrote: >> > This fixes the libgo breakage. Seems I really should have removed the >> > edge after we split the block with null dereference after __builtin_trap >> > statement so that it's unreachable. >> > >> > Bootstrapped/regtested on x86_64-linux + ppc64-linux + bootstrapped on >> > aarch64-linux, ok for trunk? >> >> Hum. I don't see why this is needed - CFG cleanup (which of course needs >> to run!) should do this for you. In fact stray unreachable blocks are usually >> more of a problem. > > Aha. It seems cleanup does that if I change the code to generate > __builtin_unreachable instead of __builtin_trap. A hint maybe? ;) Not really... static bool cleanup_control_flow_bb (basic_block bb) { ... /* Check for indirect calls that have been turned into noreturn calls. */ else if (is_gimple_call (stmt) && gimple_call_noreturn_p (stmt) && remove_fallthru_edge (bb->succs)) retval = true; and __builtin_trap is NORETURN. But there is the hint where to debug. Richard. > What's going on here is that we find a potential NULL dereference > (PHI argument has NULL), we call isolate_path, that duplicates a bb > and then cuts off the outgoing edges of this duplicated bb. And in > some cases it leaves COND_EXPR at the end of the bb -> ICE. > The we insert __builtin_trap on this isolated path. > > Marek ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Fix libgo breakage (PR tree-optimization/67284) 2015-08-21 13:40 ` Richard Biener @ 2015-08-21 14:43 ` Marek Polacek 2015-08-21 15:05 ` Jeff Law 2015-08-21 15:37 ` Jeff Law 0 siblings, 2 replies; 13+ messages in thread From: Marek Polacek @ 2015-08-21 14:43 UTC (permalink / raw) To: Richard Biener; +Cc: GCC Patches, Jeff Law, Richard Biener On Fri, Aug 21, 2015 at 03:37:38PM +0200, Richard Biener wrote: > On Fri, Aug 21, 2015 at 2:49 PM, Marek Polacek <polacek@redhat.com> wrote: > > On Fri, Aug 21, 2015 at 01:27:43PM +0200, Richard Biener wrote: > >> On Fri, Aug 21, 2015 at 12:52 PM, Marek Polacek <polacek@redhat.com> wrote: > >> > This fixes the libgo breakage. Seems I really should have removed the > >> > edge after we split the block with null dereference after __builtin_trap > >> > statement so that it's unreachable. > >> > > >> > Bootstrapped/regtested on x86_64-linux + ppc64-linux + bootstrapped on > >> > aarch64-linux, ok for trunk? > >> > >> Hum. I don't see why this is needed - CFG cleanup (which of course needs > >> to run!) should do this for you. In fact stray unreachable blocks are usually > >> more of a problem. > > > > Aha. It seems cleanup does that if I change the code to generate > > __builtin_unreachable instead of __builtin_trap. A hint maybe? ;) > > Not really... > > static bool > cleanup_control_flow_bb (basic_block bb) > { > ... > /* Check for indirect calls that have been turned into > noreturn calls. */ > else if (is_gimple_call (stmt) > && gimple_call_noreturn_p (stmt) > && remove_fallthru_edge (bb->succs)) > retval = true; > > and __builtin_trap is NORETURN. But there is the hint where to debug. Yes, but gimple_call_noreturn_p is false on __builtin_trap. That's quite confusing... but flags_from_decl_or_type really returns 0 for __builtin_trap. So can't we use __builtin_unreachable in isolate-path code? Marek ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Fix libgo breakage (PR tree-optimization/67284) 2015-08-21 14:43 ` Marek Polacek @ 2015-08-21 15:05 ` Jeff Law 2015-08-21 15:37 ` Jeff Law 1 sibling, 0 replies; 13+ messages in thread From: Jeff Law @ 2015-08-21 15:05 UTC (permalink / raw) To: Marek Polacek, Richard Biener; +Cc: GCC Patches, Richard Biener On 08/21/2015 08:41 AM, Marek Polacek wrote: > On Fri, Aug 21, 2015 at 03:37:38PM +0200, Richard Biener wrote: >> On Fri, Aug 21, 2015 at 2:49 PM, Marek Polacek <polacek@redhat.com> wrote: >>> On Fri, Aug 21, 2015 at 01:27:43PM +0200, Richard Biener wrote: >>>> On Fri, Aug 21, 2015 at 12:52 PM, Marek Polacek <polacek@redhat.com> wrote: >>>>> This fixes the libgo breakage. Seems I really should have removed the >>>>> edge after we split the block with null dereference after __builtin_trap >>>>> statement so that it's unreachable. >>>>> >>>>> Bootstrapped/regtested on x86_64-linux + ppc64-linux + bootstrapped on >>>>> aarch64-linux, ok for trunk? >>>> >>>> Hum. I don't see why this is needed - CFG cleanup (which of course needs >>>> to run!) should do this for you. In fact stray unreachable blocks are usually >>>> more of a problem. >>> >>> Aha. It seems cleanup does that if I change the code to generate >>> __builtin_unreachable instead of __builtin_trap. A hint maybe? ;) >> >> Not really... >> >> static bool >> cleanup_control_flow_bb (basic_block bb) >> { >> ... >> /* Check for indirect calls that have been turned into >> noreturn calls. */ >> else if (is_gimple_call (stmt) >> && gimple_call_noreturn_p (stmt) >> && remove_fallthru_edge (bb->succs)) >> retval = true; >> >> and __builtin_trap is NORETURN. But there is the hint where to debug. > > Yes, but gimple_call_noreturn_p is false on __builtin_trap. That's quite > confusing... but flags_from_decl_or_type really returns 0 for __builtin_trap. > > So can't we use __builtin_unreachable in isolate-path code? No, we really want the trap to halt execution rather than executing whatever code is following in the instruction stream. The latter is a significant security problem. jeff ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Fix libgo breakage (PR tree-optimization/67284) 2015-08-21 14:43 ` Marek Polacek 2015-08-21 15:05 ` Jeff Law @ 2015-08-21 15:37 ` Jeff Law 2015-08-21 16:23 ` Richard Biener 2015-08-21 16:30 ` Richard Biener 1 sibling, 2 replies; 13+ messages in thread From: Jeff Law @ 2015-08-21 15:37 UTC (permalink / raw) To: Marek Polacek, Richard Biener; +Cc: GCC Patches, Richard Biener On 08/21/2015 08:41 AM, Marek Polacek wrote: > On Fri, Aug 21, 2015 at 03:37:38PM +0200, Richard Biener wrote: >> On Fri, Aug 21, 2015 at 2:49 PM, Marek Polacek <polacek@redhat.com> wrote: >>> On Fri, Aug 21, 2015 at 01:27:43PM +0200, Richard Biener wrote: >>>> On Fri, Aug 21, 2015 at 12:52 PM, Marek Polacek <polacek@redhat.com> wrote: >>>>> This fixes the libgo breakage. Seems I really should have removed the >>>>> edge after we split the block with null dereference after __builtin_trap >>>>> statement so that it's unreachable. >>>>> >>>>> Bootstrapped/regtested on x86_64-linux + ppc64-linux + bootstrapped on >>>>> aarch64-linux, ok for trunk? >>>> >>>> Hum. I don't see why this is needed - CFG cleanup (which of course needs >>>> to run!) should do this for you. In fact stray unreachable blocks are usually >>>> more of a problem. >>> >>> Aha. It seems cleanup does that if I change the code to generate >>> __builtin_unreachable instead of __builtin_trap. A hint maybe? ;) >> >> Not really... >> >> static bool >> cleanup_control_flow_bb (basic_block bb) >> { >> ... >> /* Check for indirect calls that have been turned into >> noreturn calls. */ >> else if (is_gimple_call (stmt) >> && gimple_call_noreturn_p (stmt) >> && remove_fallthru_edge (bb->succs)) >> retval = true; >> >> and __builtin_trap is NORETURN. But there is the hint where to debug. > > Yes, but gimple_call_noreturn_p is false on __builtin_trap. That's quite > confusing... but flags_from_decl_or_type really returns 0 for __builtin_trap. Well, if that's intentional (and offhand I have no idea if it is), then you could emit a __builtin_trap followed by a __builtin_unreachable. jeff ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Fix libgo breakage (PR tree-optimization/67284) 2015-08-21 15:37 ` Jeff Law @ 2015-08-21 16:23 ` Richard Biener 2015-08-21 16:30 ` Richard Biener 1 sibling, 0 replies; 13+ messages in thread From: Richard Biener @ 2015-08-21 16:23 UTC (permalink / raw) To: Jeff Law, Marek Polacek, Richard Biener; +Cc: GCC Patches On August 21, 2015 5:09:46 PM GMT+02:00, Jeff Law <law@redhat.com> wrote: >On 08/21/2015 08:41 AM, Marek Polacek wrote: >> On Fri, Aug 21, 2015 at 03:37:38PM +0200, Richard Biener wrote: >>> On Fri, Aug 21, 2015 at 2:49 PM, Marek Polacek <polacek@redhat.com> >wrote: >>>> On Fri, Aug 21, 2015 at 01:27:43PM +0200, Richard Biener wrote: >>>>> On Fri, Aug 21, 2015 at 12:52 PM, Marek Polacek ><polacek@redhat.com> wrote: >>>>>> This fixes the libgo breakage. Seems I really should have >removed the >>>>>> edge after we split the block with null dereference after >__builtin_trap >>>>>> statement so that it's unreachable. >>>>>> >>>>>> Bootstrapped/regtested on x86_64-linux + ppc64-linux + >bootstrapped on >>>>>> aarch64-linux, ok for trunk? >>>>> >>>>> Hum. I don't see why this is needed - CFG cleanup (which of >course needs >>>>> to run!) should do this for you. In fact stray unreachable blocks >are usually >>>>> more of a problem. >>>> >>>> Aha. It seems cleanup does that if I change the code to generate >>>> __builtin_unreachable instead of __builtin_trap. A hint maybe? ;) >>> >>> Not really... >>> >>> static bool >>> cleanup_control_flow_bb (basic_block bb) >>> { >>> ... >>> /* Check for indirect calls that have been turned into >>> noreturn calls. */ >>> else if (is_gimple_call (stmt) >>> && gimple_call_noreturn_p (stmt) >>> && remove_fallthru_edge (bb->succs)) >>> retval = true; >>> >>> and __builtin_trap is NORETURN. But there is the hint where to >debug. >> >> Yes, but gimple_call_noreturn_p is false on __builtin_trap. That's >quite >> confusing... but flags_from_decl_or_type really returns 0 for >__builtin_trap. >Well, if that's intentional (and offhand I have no idea if it is), then > >you could emit a __builtin_trap followed by a __builtin_unreachable. Looking at builtins.def I'd say it should be no return. Richard. >jeff ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Fix libgo breakage (PR tree-optimization/67284) 2015-08-21 15:37 ` Jeff Law 2015-08-21 16:23 ` Richard Biener @ 2015-08-21 16:30 ` Richard Biener 2015-08-21 16:52 ` Marek Polacek 1 sibling, 1 reply; 13+ messages in thread From: Richard Biener @ 2015-08-21 16:30 UTC (permalink / raw) To: Jeff Law, Marek Polacek, Richard Biener; +Cc: GCC Patches On August 21, 2015 5:09:46 PM GMT+02:00, Jeff Law <law@redhat.com> wrote: >On 08/21/2015 08:41 AM, Marek Polacek wrote: >> On Fri, Aug 21, 2015 at 03:37:38PM +0200, Richard Biener wrote: >>> On Fri, Aug 21, 2015 at 2:49 PM, Marek Polacek <polacek@redhat.com> >wrote: >>>> On Fri, Aug 21, 2015 at 01:27:43PM +0200, Richard Biener wrote: >>>>> On Fri, Aug 21, 2015 at 12:52 PM, Marek Polacek ><polacek@redhat.com> wrote: >>>>>> This fixes the libgo breakage. Seems I really should have >removed the >>>>>> edge after we split the block with null dereference after >__builtin_trap >>>>>> statement so that it's unreachable. >>>>>> >>>>>> Bootstrapped/regtested on x86_64-linux + ppc64-linux + >bootstrapped on >>>>>> aarch64-linux, ok for trunk? >>>>> >>>>> Hum. I don't see why this is needed - CFG cleanup (which of >course needs >>>>> to run!) should do this for you. In fact stray unreachable blocks >are usually >>>>> more of a problem. >>>> >>>> Aha. It seems cleanup does that if I change the code to generate >>>> __builtin_unreachable instead of __builtin_trap. A hint maybe? ;) >>> >>> Not really... >>> >>> static bool >>> cleanup_control_flow_bb (basic_block bb) >>> { >>> ... >>> /* Check for indirect calls that have been turned into >>> noreturn calls. */ >>> else if (is_gimple_call (stmt) >>> && gimple_call_noreturn_p (stmt) >>> && remove_fallthru_edge (bb->succs)) >>> retval = true; >>> >>> and __builtin_trap is NORETURN. But there is the hint where to >debug. >> >> Yes, but gimple_call_noreturn_p is false on __builtin_trap. That's >quite >> confusing... but flags_from_decl_or_type really returns 0 for >__builtin_trap. >Well, if that's intentional (and offhand I have no idea if it is), then > >you could emit a __builtin_trap followed by a __builtin_unreachable. But maybe go is non-call-exceptions and that makes a difference? >jeff ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Fix libgo breakage (PR tree-optimization/67284) 2015-08-21 16:30 ` Richard Biener @ 2015-08-21 16:52 ` Marek Polacek 2015-08-21 17:31 ` Richard Biener 0 siblings, 1 reply; 13+ messages in thread From: Marek Polacek @ 2015-08-21 16:52 UTC (permalink / raw) To: Richard Biener; +Cc: Jeff Law, Richard Biener, GCC Patches On Fri, Aug 21, 2015 at 06:23:09PM +0200, Richard Biener wrote: > >> Yes, but gimple_call_noreturn_p is false on __builtin_trap. That's > >quite > >> confusing... but flags_from_decl_or_type really returns 0 for > >__builtin_trap. > >Well, if that's intentional (and offhand I have no idea if it is), then > > > >you could emit a __builtin_trap followed by a __builtin_unreachable. > > But maybe go is non-call-exceptions and that makes a difference? I suppose that's the case. In Makefile.am I see AM_CFLAGS = -fexceptions -fnon-call-exceptions -fplan9-extensions But I'm not clear on how this could make a difference wrt whether __builtin_trap is volatile (thus ECF_NORETURN). This is a patch to also generate __builtin_unreachable, just for the record. Bootstrapped/regtested on x86_64-linux. 2015-08-21 Marek Polacek <polacek@redhat.com> PR tree-optimization/67284 * gimple-ssa-isolate-paths.c (insert_trap): Also emit __builtin_unreachable. diff --git gcc/gimple-ssa-isolate-paths.c gcc/gimple-ssa-isolate-paths.c index ca2322d..06263d3 100644 --- gcc/gimple-ssa-isolate-paths.c +++ gcc/gimple-ssa-isolate-paths.c @@ -97,15 +97,18 @@ insert_trap (gimple_stmt_iterator *si_p, tree op) = gimple_build_call (builtin_decl_explicit (BUILT_IN_TRAP), 0); gimple_seq seq = NULL; gimple_seq_add_stmt (&seq, new_stmt); + new_stmt + = gimple_build_call (builtin_decl_explicit (BUILT_IN_UNREACHABLE), 0); + gimple_seq_add_stmt (&seq, new_stmt); /* If we had a NULL pointer dereference, then we want to insert the - __builtin_trap after the statement, for the other cases we want - to insert before the statement. */ + __builtin_trap/__builtin_unreachable pair after the statement, for + the other cases we want to insert before the statement. */ if (walk_stmt_load_store_ops (stmt, (void *)op, check_loadstore, check_loadstore)) { - gsi_insert_after (si_p, seq, GSI_NEW_STMT); + gsi_insert_seq_after (si_p, seq, GSI_NEW_STMT); if (stmt_ends_bb_p (stmt)) { split_block (gimple_bb (stmt), stmt); @@ -113,7 +116,7 @@ insert_trap (gimple_stmt_iterator *si_p, tree op) } } else - gsi_insert_before (si_p, seq, GSI_NEW_STMT); + gsi_insert_seq_before (si_p, seq, GSI_NEW_STMT); split_block (gimple_bb (new_stmt), new_stmt); *si_p = gsi_for_stmt (stmt); @@ -194,7 +197,7 @@ isolate_path (basic_block bb, basic_block duplicate, /* If we did not run to the end of DUPLICATE, then SI points to STMT and SI2 points to the duplicate of STMT in DUPLICATE. Insert a trap - before SI2 and remove SI2 and all trailing statements. */ + before SI2 and split the block. */ if (!gsi_end_p (si2)) { if (ret_zero) Marek ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Fix libgo breakage (PR tree-optimization/67284) 2015-08-21 16:52 ` Marek Polacek @ 2015-08-21 17:31 ` Richard Biener 2015-08-24 9:17 ` Jakub Jelinek 0 siblings, 1 reply; 13+ messages in thread From: Richard Biener @ 2015-08-21 17:31 UTC (permalink / raw) To: Marek Polacek; +Cc: Jeff Law, Richard Biener, GCC Patches On August 21, 2015 6:42:15 PM GMT+02:00, Marek Polacek <polacek@redhat.com> wrote: >On Fri, Aug 21, 2015 at 06:23:09PM +0200, Richard Biener wrote: >> >> Yes, but gimple_call_noreturn_p is false on __builtin_trap. >That's >> >quite >> >> confusing... but flags_from_decl_or_type really returns 0 for >> >__builtin_trap. >> >Well, if that's intentional (and offhand I have no idea if it is), >then >> > >> >you could emit a __builtin_trap followed by a __builtin_unreachable. >> >> But maybe go is non-call-exceptions and that makes a difference? > >I suppose that's the case. In Makefile.am I see >AM_CFLAGS = -fexceptions -fnon-call-exceptions -fplan9-extensions >But I'm not clear on how this could make a difference wrt whether >__builtin_trap is volatile (thus ECF_NORETURN). Not sure either. Needs to be investigated. BTW, built-in trap is also nothrow AFAIR. Richard. >This is a patch to also generate __builtin_unreachable, just for the >record. > >Bootstrapped/regtested on x86_64-linux. > >2015-08-21 Marek Polacek <polacek@redhat.com> > > PR tree-optimization/67284 > * gimple-ssa-isolate-paths.c (insert_trap): Also emit > __builtin_unreachable. > >diff --git gcc/gimple-ssa-isolate-paths.c >gcc/gimple-ssa-isolate-paths.c >index ca2322d..06263d3 100644 >--- gcc/gimple-ssa-isolate-paths.c >+++ gcc/gimple-ssa-isolate-paths.c >@@ -97,15 +97,18 @@ insert_trap (gimple_stmt_iterator *si_p, tree op) > = gimple_build_call (builtin_decl_explicit (BUILT_IN_TRAP), 0); > gimple_seq seq = NULL; > gimple_seq_add_stmt (&seq, new_stmt); >+ new_stmt >+ = gimple_build_call (builtin_decl_explicit >(BUILT_IN_UNREACHABLE), 0); >+ gimple_seq_add_stmt (&seq, new_stmt); > > /* If we had a NULL pointer dereference, then we want to insert the >- __builtin_trap after the statement, for the other cases we want >- to insert before the statement. */ >+ __builtin_trap/__builtin_unreachable pair after the statement, >for >+ the other cases we want to insert before the statement. */ > if (walk_stmt_load_store_ops (stmt, (void *)op, > check_loadstore, > check_loadstore)) > { >- gsi_insert_after (si_p, seq, GSI_NEW_STMT); >+ gsi_insert_seq_after (si_p, seq, GSI_NEW_STMT); > if (stmt_ends_bb_p (stmt)) > { > split_block (gimple_bb (stmt), stmt); >@@ -113,7 +116,7 @@ insert_trap (gimple_stmt_iterator *si_p, tree op) > } > } > else >- gsi_insert_before (si_p, seq, GSI_NEW_STMT); >+ gsi_insert_seq_before (si_p, seq, GSI_NEW_STMT); > > split_block (gimple_bb (new_stmt), new_stmt); > *si_p = gsi_for_stmt (stmt); >@@ -194,7 +197,7 @@ isolate_path (basic_block bb, basic_block >duplicate, > >/* If we did not run to the end of DUPLICATE, then SI points to STMT >and > SI2 points to the duplicate of STMT in DUPLICATE. Insert a trap >- before SI2 and remove SI2 and all trailing statements. */ >+ before SI2 and split the block. */ > if (!gsi_end_p (si2)) > { > if (ret_zero) > > Marek ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Fix libgo breakage (PR tree-optimization/67284) 2015-08-21 17:31 ` Richard Biener @ 2015-08-24 9:17 ` Jakub Jelinek 2015-08-24 9:23 ` Marek Polacek 0 siblings, 1 reply; 13+ messages in thread From: Jakub Jelinek @ 2015-08-24 9:17 UTC (permalink / raw) To: Richard Biener; +Cc: Marek Polacek, Jeff Law, Richard Biener, GCC Patches On Fri, Aug 21, 2015 at 07:22:24PM +0200, Richard Biener wrote: > On August 21, 2015 6:42:15 PM GMT+02:00, Marek Polacek <polacek@redhat.com> wrote: > >On Fri, Aug 21, 2015 at 06:23:09PM +0200, Richard Biener wrote: > >> >> Yes, but gimple_call_noreturn_p is false on __builtin_trap. > >That's > >> >quite > >> >> confusing... but flags_from_decl_or_type really returns 0 for > >> >__builtin_trap. > >> >Well, if that's intentional (and offhand I have no idea if it is), > >then > >> > > >> >you could emit a __builtin_trap followed by a __builtin_unreachable. > >> > >> But maybe go is non-call-exceptions and that makes a difference? > > > >I suppose that's the case. In Makefile.am I see > >AM_CFLAGS = -fexceptions -fnon-call-exceptions -fplan9-extensions > >But I'm not clear on how this could make a difference wrt whether > >__builtin_trap is volatile (thus ECF_NORETURN). > > Not sure either. Needs to be investigated. BTW, built-in trap is also nothrow AFAIR. I think the bug is in the Go FE which incorrectly doesn't set TREE_THIS_VOLATILE on the __builtin_trap decl. Jakub ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Fix libgo breakage (PR tree-optimization/67284) 2015-08-24 9:17 ` Jakub Jelinek @ 2015-08-24 9:23 ` Marek Polacek 0 siblings, 0 replies; 13+ messages in thread From: Marek Polacek @ 2015-08-24 9:23 UTC (permalink / raw) To: Jakub Jelinek; +Cc: Richard Biener, Jeff Law, Richard Biener, GCC Patches On Mon, Aug 24, 2015 at 11:07:41AM +0200, Jakub Jelinek wrote: > On Fri, Aug 21, 2015 at 07:22:24PM +0200, Richard Biener wrote: > > Not sure either. Needs to be investigated. BTW, built-in trap is also nothrow AFAIR. > > I think the bug is in the Go FE which incorrectly doesn't set > TREE_THIS_VOLATILE on the __builtin_trap decl. I have a patch that fixes this; and on a simple testcase I don't see the ICE anymore. Will post the patch after further testing. Thanks for the pointer Jakub. Marek ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2015-08-24 9:19 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-08-21 11:00 [PATCH] Fix libgo breakage (PR tree-optimization/67284) Marek Polacek 2015-08-21 12:07 ` Richard Biener 2015-08-21 12:50 ` Marek Polacek 2015-08-21 13:40 ` Richard Biener 2015-08-21 14:43 ` Marek Polacek 2015-08-21 15:05 ` Jeff Law 2015-08-21 15:37 ` Jeff Law 2015-08-21 16:23 ` Richard Biener 2015-08-21 16:30 ` Richard Biener 2015-08-21 16:52 ` Marek Polacek 2015-08-21 17:31 ` Richard Biener 2015-08-24 9:17 ` Jakub Jelinek 2015-08-24 9:23 ` Marek Polacek
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).