RFR: 8274074: SIGFPE with C2 compiled code with -XX:+StressGCM [v3]
Christian Hagedorn
chagedorn at openjdk.java.net
Mon Sep 27 07:44:13 UTC 2021
> In the testcase, the divisor input node of a `DivI` node is sunk out of a loop to a div by zero UCT and is pinned with a `CastII` node to ensure it's not floating back into the loop. The divisor is optimized by taking into account that it's only executed on the uncommon path. The `CastII`, however, is removed later and the division floats back into the loop which results in a SIGFPE crash.
>
> The relevent lines in the testcase are the following two divisions:
>
> static int iFld = 1;
> static int q = 0;
> ...
> y = iFld - q; // divisor
> y = (iArrFld[2] / y); // division 1
> y = (5 / iFld); // division 2
>
> After sinking the `divisor` of `division 1` in the testcase to the div by zero UCT of `division 2`, the graph looks like this:
>
> 
>
> - `201 If` is the zero check of `division 2` (will always succeed because `iFld = 1`, i.e. UCT is never taken).
> - `193 DivI` (`division 1`) is not sunk because its `get_ctrl()` is `203 IfFalse` (outside the loop already because there is no use inside the loop since the local `y` is directly overwritten again).
> - `275 SubI` (`divisor`) was sunk out of the loop and is pinned by `276 CastII` (unconditional dependency).
>
> In IGVN, `CastII::Value()` is called for `276 CastII`. It sees an `If/Cmp` (zero check of `division 2`) with the same `137 LoadI` input as for the `276 CastII`. Therefore, we set its type to [0,0] here:
> https://github.com/openjdk/jdk/blob/d0987513665def1b6b2981ab5932b6f1b8b310d8/src/hotspot/share/opto/castnode.cpp#L248-L252
>
> As a result, we replace `276 CastII` with a constant zero in IGVN. But now we lost the pin to the uncommon path of the zero check of `division 2` for `275 SubI` and `193 DivI`. `193 DivI` is only used on the uncommon path but can now float around again, also inside the loop itself, which happens in the testcase. Inside the loop, we execute the division with the now optimized divisor `0 - q = 0` which is a division by zero and we crash.
>
> In summary, it's not a problem that a `Div` node floats above its zero check here but rather that we optimize an input node used as divisor by assuming that we only execute the division on the uncommon path when the zero check of `division 2` failed (which never happens). This divisor optimization would be wrong when the division is executed inside the loop. But due to losing the pin, we end up doing exactly that which results in a SIGFPE crash.
>
> The suggested fix is to extend the sinking algorithm to rewire data nodes with a control input inside a loop whose `get_ctrl()` is actually completely outside loops on uncommon paths. The control input is set to `get_ctrl()` to force the nodes out of loops. In the example above, the control input of `193 DivI` is set to `203 IfFalse`, ensuring that it is still pinned to the uncommon path after `276 CastII` is removed. This fix is also beneficial if we do not sink any nodes at all later.
>
> Thanks,
> Christian
Christian Hagedorn has updated the pull request incrementally with one additional commit since the last revision:
Fix typo
-------------
Changes:
- all: https://git.openjdk.java.net/jdk/pull/5651/files
- new: https://git.openjdk.java.net/jdk/pull/5651/files/941420c1..0238b730
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5651&range=02
- incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5651&range=01-02
Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod
Patch: https://git.openjdk.java.net/jdk/pull/5651.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/5651/head:pull/5651
PR: https://git.openjdk.java.net/jdk/pull/5651
More information about the hotspot-compiler-dev
mailing list