SIGSEGV in C2 compiler

[Denis Lila]

Hi Tom.

> So, as long as it's possible for the data to die, but that data's
> control to stay alive, I think this will be a problem, regardless
> of ConvI2L's behaviour.

I should have probably asked this before I sent that huge e-mail:

Is it a legal state for a data input of a phi to be dead but that data's
control to be alive?

To me it doesn't seem so. In that case, can this happen with any node
other than a range checked ConvI2L whose input is always out of range?

Thank you,
Denis.

----- Original Message -----
> > Ping me directly if you'd like help restructuring the loop to avoid
> > the
> > problem.
> 
> Thank you, but I already know how to do that. The reason I worked on
> this
> is because I am interested in compilers, and I would like to work on
> hotspot. This seemed like a good place to start.
> 
> > I'm sorry I didn't follow up on this with you, I lost track of it
> > over
> > the Christmas holiday. This appears to be the same issue as 6675699
> > where a ConvI2L with a constrained type on it has it's input
> > replaced
> > with a constant outside that range. The identity transformation
> > converts that to top which causes a phi collapse and the control
> > flow
> > to be killed. We've kind of band aided this in a few other places
> > but
> > it appears that loop unswitching can create similar issues.
> 
> I've spent a lot of time thinking about this and staring at graphs,
> and
> here's what I think: (now, before this, I had never even read anything
> about
> compilers, let alone worked on one, so please, if anything doesn't
> make
> sense, don't go easy on me ;-) )
> 
> (Before I begin, I refer to a lot of node indexes below. Here's the
> graphs
> they were taken from:
> http://icedtea.classpath.org/~dlila/ecjGraph.xml. I used
> IdealGraphVisualizer from src/share/tools)
> 
> That bug's evaluation says "We need to remove the band aid fixes for
> 4781451 and 4799512 and disallow ConvI2L from being initialized with
> a narrower type than bottom". I'm not sure I understand why. I mean,
> in this case it made sense to me that node 147 should be initialized
> with type 1..maxint because, like the comment where this is done says,
> we can simply assume that our input is a valid array index because if
> it's not a runtime check will take care of that case and the index
> won't
> be used.
> 
> What I think is going wrong is that when we do subsume_node(k, i) (in
> phaseX.cpp:1108 - in this case that would be subsume_node(357, 296))
> we
> are losing information about one of the uses of 296. Before the
> subsume
> the uses of 296 are 357 (the phi) and 295. After the subsume they are
> 259 and 295. When compute_lca_of_uses(296, 314) is called, Node *c in
> the
> loop becomes 259 in the 2nd iteration, and when we call get_ctrl(259)
> we
> get 226. 226 is dominated by 350, and the idom of 350 is 286 (the if
> that
> is executed before the loops - the one generated by the loop
> splitting)
> <i>because the controls from both split loops to 350 are still
> alive</i>.
> 286 dominates the early control of 296 (which is 314), so it thinks
> this
> is a bad graph.
> 
> If 357 had not been removed, when c.idx==357 in
> compute_lca_of_uses(296...),
> because 357 is a phi node, we know to "skip" over the region and set
> the
> "use" variable to the control input to the region that corresponds to
> the
> data input in question to the phi. In this case 296 is the data input,
> the phi
> is 357, and the corresponding control input to the region is 349,
> which is
> the control from the split loop where 296 is computed. So "use" would
> be set
> to node 349, and nothing would go wrong because 314 dominates both 349
> and 303.
> 
> So, what I meant when I said "we are losing information about one of
> the uses
> of 296" is that we are losing the information that, if the value
> produced by
> node 296 is used by node 259, the control path taken _must_ have gone
> through
> 349, and it _cannot_ have gone through 217 (which is control path from
> the i!=8
> unswitched loop (the one that never runs)).
> 
> It seems to me like this problem isn't specific to ConvI2L, but can
> happen
> any time that a data input to a phi node dies, but the control input
> to that
> phi's region that corresponds to the dead data doesn't die. I'm not
> sure if
> this state of affairs can be caused by anything other than a ConvI2L,
> but even
> so, I think this should get its own fix, because in
> PhiNode::unique_input, when
> we do
> if (un == NULL || un == this || phase->type(un) == Type::TOP) {
> continue; // ignore if top, or in(i) and "this" are in a data cycle
> }
> we're not stating an invariant such as "a phi's data input dies if and
> only if
> that data's corresponding control dies". So, as long as it's possible
> for the
> data to die, but that data's control to stay alive, I think this will
> be a
> problem, regardless of ConvI2L's behaviour.
> 
> I have two potential fixes in mind:
> 1. Don't subsume a phi node with its unique input if it has dead
> inputs but
> the dead inputs' controls are not dead.
> 2. For every node N that is used by a phi node P that is killed, save
> a pointer
> to P->in(0) and a number j such that P->in(j) == N. That will allow us
> to correctly
> compute "use" in compute_lca_of_uses even if c isn't a phi.
> 
> Like I said above, I'm not sure if any of this makes any sense at all
> so I
> would very much appreciate any criticism.
> 
> Thank you,
> Denis.
> 
> ----- Original Message -----
> 
> >
> > Pop 147 ConvI2L === _ 20 [[ 153 ]] Type:long:1..maxint:www !jvms:
> > Test::main @ bci:32
> > &lt; long:1..maxint:www &lt; 147 ConvI2L === _ _ [[]] [3000147]
> ...