8186125: "DU iteration must converge quickly" assert in split if with unsafe accesses
authorroland
Thu, 09 Nov 2017 12:43:13 -0800
changeset 47820 1bc021ddeae0
parent 47819 ee36a8e36561
child 47821 0cd18aa4f7b6
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
src/hotspot/share/opto/split_if.cpp
test/hotspot/jtreg/compiler/unsafe/TestSplitIf.java
--- 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;
         }
       }
--- /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);
+        }
+    }
+
+}