[RFR] 8217359: C2 compiler triggers SIGSEGV after tranformation in ConvI2LNode::Ideal

[Tobias Hartmann]

Hi Felix,

Could you please add the regression test as jtreg test?

Otherwise, the fix looks reasonable to me. Nice analysis!

Thanks,
Tobias


On 18.01.19 06:36, Yangfei (Felix) wrote:
> Hi,
> 
>     Can someone help review this change to the C2 compiler? 
> 
>     Bug: https://bugs.openjdk.java.net/browse/JDK-8217359
>     Webrev: http://cr.openjdk.java.net/~fyang/8217359/webrev.00/
> 
>     The bug triggers when C2 compiler does the following transformation in function ConvI2LNode::Ideal: 
>     // Convert ConvI2L(AddI(x, y)) to AddL(ConvI2L(x), ConvI2L(y))
>     ......
>     395     Node* cx = phase->C->constrained_convI2L(phase, x, TypeInt::make(rxlo, rxhi, widen), NULL);
>     396     Node* cy = phase->C->constrained_convI2L(phase, y, TypeInt::make(rylo, ryhi, widen), NULL);
>     ....
> 
>     Here is the process of how it triggers:
> 
> // =========================================================
> // Before line 395, x is an AddINode (id: 202). y is also an AddINode (id: 553) and x is a subtree of y.
> // The ideal graph looks like:
> //
> //       ...  ...  ...  ...
> //         \   |  /     |
> //          86_Phi   33_ConI
> //             |     /
> //         \   |    /
> //          202_AddI
> //
> //            ...  ... ... ...
> //             |     \  |  /
> //         27_ConI  202_AddI --------- (node x)
> //             |    /
> //        \    |   /
> //         549_SubI
> //             |    ...
> //         \   |    /
> //          553_AddI ---------- (node y)
> //
> // ==========================================================
> // After line 395, x is converted to cx and cx is an AddLNode (id: 1274).
> // At this point, everything looks fine.
> //         ...     ...  ...
> //           \     /    /
> //      1271_ConvI2L  1273_ConL
> //              |     /
> //         \    |    /
> //          1274_AddL
> //
> // ==========================================================
> // In line 396, y will be converted to cy.  In this progress, y
> // and its subnode will all be converted recursively.  This is
> // a rather long progress.  The convertion of y is like this:
> //
> // Node 27_ConI will be converted to node 1278_ConL.
> //
> // Since x(202) is the input edge of node 549, it will be
> // converted again.  And the result cx_2 is node 1282_AddL.
> // The structure of cx_2 is the same as cx.  After GVN(hash_find_insert()),
> // 1282_AddL is replaced with 1274_AddL.
> //
> // Then 549_SubI will be converted to 1283_SubL and the ideal graph looks like: 
> //                    ...    ...  ...
> //                      \    /    /
> //                1271_ConvI2L  1273_ConL
> //            ...         |     /
> //             |     \    |    /
> //         1278_ConL  1274_AddL
> //             |     /
> //        \    |    /
> //         1283_SubL
> //
> // After that, C2 will do the following transformation to node 1283_SubL: 
> //      x - (y + cons) ==> (x - y) - cons
> //
> // When this is done, node 1283_SubL is converted to node 1286_AddL: 
> //                        ...      ...   ...
> //                         |        |    /
> //                   1278_ConL  1271_ConI2L
> //                         |    /     ...
> //                   \     |   /      /
> //                    1284_SubL   1285_ConL
> //                         |     /
> //                    \    |    /
> //                     1286_AddL
> //
> // Then in function subsume_node(), 1283_SubL is replaced with 1286_AddL. 
> // During this progress, following operations will be carried out:
> //  | In function subsume_node(), 1283_SubL will be regarded as a
> //  | dead_node since it is replaced by 1286_AddL. The same inspection
> //  | of dead node will be carried out to the subnodes of 1283
> //  | recursively.  And remove_dead_node() function will be called
> //  | by subsume_node() to replace the input edges of dead node to NULL.
> //  | 1274_AddL is node cx. At this moment, 1274_AddL has only one
> //  | output edge, that is 1283. Since 1283 is a dead node, 1274 will
> //  | also be regarded as a dead node. Then input edges of 1274_AddL
> //  | will be set to NULL. After that, cx will be an isolated node which
> //  | has neither input edge nor output edge.
> //
> // ==========================================
> // After all of this, program continues and cx->in(2) is used in addnode.cpp:163. 
> // Since now cx has no input edges, the program crashes.
> 
>     The proposed solution is fairly straight-forward: 
>     After the conversion of x, build a hook node add a use to cx to prevent it from dying. 
>     When conversion of y is finished, this new output of cx is removed. 
> 
>     JTreg tested with both x86_64 fastdebug & release build.  Is it OK? 
> 
> Thanks,
> Felix
>