# HG changeset patch # User roland # Date 1510260193 28800 # Node ID 1bc021ddeae02d07b2424564ee0027cbab8f1f1c # Parent ee36a8e365610fcc54407d5182a06b9977f97e0e 8186125: "DU iteration must converge quickly" assert in split if with unsafe accesses Summary: added missing checks for Opaque4Node in split-if code Reviewed-by: kvn diff -r ee36a8e36561 -r 1bc021ddeae0 src/hotspot/share/opto/split_if.cpp --- a/src/hotspot/share/opto/split_if.cpp Wed Oct 25 10:05:17 2017 +0200 +++ b/src/hotspot/share/opto/split_if.cpp Thu Nov 09 12:43:13 2017 -0800 @@ -116,9 +116,23 @@ assert( bol->is_Bool(), "" ); if (bol->outcnt() == 1) { Node* use = bol->unique_out(); - Node *use_c = use->is_If() ? use->in(0) : get_ctrl(use); - if (use_c == blk1 || use_c == blk2) { - continue; + if (use->Opcode() == Op_Opaque4) { + if (use->outcnt() == 1) { + Node* iff = use->unique_out(); + assert(iff->is_If(), "unexpected node type"); + Node *use_c = iff->in(0); + if (use_c == blk1 || use_c == blk2) { + continue; + } + } + } else { + // We might see an Opaque1 from a loop limit check here + assert(use->is_If() || use->is_CMove() || use->Opcode() == Op_Opaque1, "unexpected node type"); + Node *use_c = use->is_If() ? use->in(0) : get_ctrl(use); + if (use_c == blk1 || use_c == blk2) { + assert(use->is_CMove(), "unexpected node type"); + continue; + } } } if (get_ctrl(bol) == blk1 || get_ctrl(bol) == blk2) { @@ -129,17 +143,40 @@ bol->dump(); } #endif - for (DUIterator_Last jmin, j = bol->last_outs(jmin); j >= jmin; --j) { - // Uses are either IfNodes or CMoves - Node* iff = bol->last_out(j); - assert( iff->in(1) == bol, "" ); - // Get control block of either the CMove or the If input - Node *iff_ctrl = iff->is_If() ? iff->in(0) : get_ctrl(iff); - Node *x = bol->clone(); - register_new_node(x, iff_ctrl); - _igvn.replace_input_of(iff, 1, x); + for (DUIterator j = bol->outs(); bol->has_out(j); j++) { + Node* u = bol->out(j); + // Uses are either IfNodes, CMoves or Opaque4 + if (u->Opcode() == Op_Opaque4) { + assert(u->in(1) == bol, "bad input"); + for (DUIterator_Last kmin, k = u->last_outs(kmin); k >= kmin; --k) { + Node* iff = u->last_out(k); + assert(iff->is_If() || iff->is_CMove(), "unexpected node type"); + assert( iff->in(1) == u, "" ); + // Get control block of either the CMove or the If input + Node *iff_ctrl = iff->is_If() ? iff->in(0) : get_ctrl(iff); + Node *x1 = bol->clone(); + Node *x2 = u->clone(); + register_new_node(x1, iff_ctrl); + register_new_node(x2, iff_ctrl); + _igvn.replace_input_of(x2, 1, x1); + _igvn.replace_input_of(iff, 1, x2); + } + _igvn.remove_dead_node(u); + --j; + } else { + // We might see an Opaque1 from a loop limit check here + assert(u->is_If() || u->is_CMove() || u->Opcode() == Op_Opaque1, "unexpected node type"); + assert(u->in(1) == bol, ""); + // Get control block of either the CMove or the If input + Node *u_ctrl = u->is_If() ? u->in(0) : get_ctrl(u); + assert(u_ctrl != blk1 && u_ctrl != blk2, "won't converge"); + Node *x = bol->clone(); + register_new_node(x, u_ctrl); + _igvn.replace_input_of(u, 1, x); + --j; + } } - _igvn.remove_dead_node( bol ); + _igvn.remove_dead_node(bol); --i; } } diff -r ee36a8e36561 -r 1bc021ddeae0 test/hotspot/jtreg/compiler/unsafe/TestSplitIf.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/hotspot/jtreg/compiler/unsafe/TestSplitIf.java Thu Nov 09 12:43:13 2017 -0800 @@ -0,0 +1,84 @@ +/* + * Copyright (c) 2017, Red Hat, Inc. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/** + * @test + * @bug 8186125 + * @summary cast before unsafe access moved in dominating null check null path causes crash + * @modules java.base/jdk.internal.misc:+open + * + * @run main/othervm -XX:-BackgroundCompilation TestSplitIf + * + */ + +import jdk.internal.misc.Unsafe; +import java.lang.reflect.Field; + +public class TestSplitIf { + + static final jdk.internal.misc.Unsafe UNSAFE = Unsafe.getUnsafe(); + static final long F_OFFSET; + + static class A { + int f; + A(int f) { + this.f = f; + } + } + + static { + try { + Field fField = A.class.getDeclaredField("f"); + F_OFFSET = UNSAFE.objectFieldOffset(fField); + } catch (Exception e) { + throw new RuntimeException(e); + } + } + + static int test(A a1, A a2, boolean flag1) { + boolean flag2; + int f = 0; + A a = null; + if (flag1) { + flag2 = true; + a = a1; + } else { + flag2 = false; + a = a2; + } + if (flag2) { + f = UNSAFE.getInt(a, F_OFFSET); + } else { + f = UNSAFE.getInt(a, F_OFFSET); + } + return f; + } + + static public void main(String[] args) { + A a = new A(0x42); + for (int i = 0; i < 20000; i++) { + test(a, a, (i % 2) == 0); + } + } + +}