Re: [PATCH] Fix PR68067

[Richard Biener <rguenther@suse.de>]

On Fri, 27 Nov 2015, Alan Lawrence wrote:

> On 27/11/15 15:07, Alan Lawrence wrote:
> > On 23/11/15 09:43, Richard Biener wrote:
> > > On Fri, 20 Nov 2015, Alan Lawrence wrote:
> > > 
> > > > ...the asserts
> > > > you suggested in
> > > > (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=3D68117#c27)...
> >  >>
> > > > So I have to ask, how sure are you that those assertions are(/should
> > > > be!) "correct"? :)
> > > 
> > > Ideally they should be correct but they happen to be not (and I think
> > > the intent was that this should be harmless).  Basically I tried
> > > to assert that nobody creates stale edge redirect data that is not
> > > later consumed or cleared.  Happens to be too optimistic :/
> > 
> > Maybe so, but it looks like the edge_var_redirect_map is still suspect here.
> > On
> > the ~~28th call to loop_version, from tree_unswitch_loop, the call to
> > lv_flush_pending_stmts executes (tree-cfg.c flush_pending_stmts):
> > 
> >             def = redirect_edge_var_map_def (vm);
> >             add_phi_arg (phi, def, e, redirect_edge_var_map_location(vm));
> > 
> > and BLOCK_LOCATION (redirect_edge_var_map_location(vm)) is
> > 
> > <<invalid tree code> 0x7fb7704a80 side-effects addressable asm_written used
> > protected static visited tree_0 tree_2 tree_5>
> > 
> > so yeah, next question, how'd that get there...
> > 
> > A.
> 
> Well, pass_dominator::execute calls redirect_edge_var_map with that edge
> pointer, at which time the edge is from from 32 (0x7fb79cc6e8) to block 20
> (0x7fb7485e38), and locus is 2147483884; and then again, with locus 0.
> 
> With no intervening calls to redirect_edge_var_map_clear for that edge,
> loop_version's call to flush_pending_statements then reads
> redirect_edge_var_map_vector for that edge pointer - which is now an edge from
> block 126 (0x7fb7485af8) to 117 (0x7fb74856e8). It sees those locations
> (2147483884 and 0)...
> 
> Clearing the edge redirect map at the end of pass_dominator fixes the ICE (as
> would clearing it at the end of each stage, or clearing it at the beginning of
> loop_unswitch, I guess).
> 
> I'll post a patch after more testing, but obviously I'm keen to hear if there
> are obvious problems with the approach?
> 
> And coming up with a testcase, well, heh - this broke because of identical
> pointers to structures allocated at different times, with intervening
> free...ideas welcome of course!

Yeah.  I've pondered with clearing the hashmap after each pass
(and hope no IPA pass would redirect edges).  Or even more aggressive,
clear the hashmap as well when we do set_cfun ().

Maybe you can try that?

And no, I don't think any pass expects this stuff to be live across
passes.

Richard.