changeset 56734:c63b9b87c28a

8224658: Unsafe access C2 compile fails with assert(flat != TypePtr::BOTTOM) failed: cannot alias-analyze an untyped ptr: adr_type = NULL Summary: Handle unsafe off-heap access with constant zero address. Reviewed-by: vlivanov, roland
author thartmann
date Wed, 12 Jun 2019 14:06:16 +0200
parents ef577fa0dd10
children 596ae6c3ef6f
files src/hotspot/share/opto/graphKit.hpp src/hotspot/share/opto/library_call.cpp src/hotspot/share/opto/memnode.cpp src/hotspot/share/opto/output.cpp test/hotspot/jtreg/compiler/unsafe/TestUnsafeLoadWithZeroAddress.java
diffstat 5 files changed, 111 insertions(+), 8 deletions(-) [+]
line wrap: on
line diff
--- a/src/hotspot/share/opto/graphKit.hpp	Wed Jun 12 13:49:43 2019 +0200
+++ b/src/hotspot/share/opto/graphKit.hpp	Wed Jun 12 14:06:16 2019 +0200
@@ -565,7 +565,7 @@
     return store_to_memory(ctl, adr, val, bt,
                            C->get_alias_index(adr_type),
                            mo, require_atomic_access,
-                           unaligned, mismatched);
+                           unaligned, mismatched, unsafe);
   }
   // This is the base version which is given alias index
   // Return the new StoreXNode
--- a/src/hotspot/share/opto/library_call.cpp	Wed Jun 12 13:49:43 2019 +0200
+++ b/src/hotspot/share/opto/library_call.cpp	Wed Jun 12 14:06:16 2019 +0200
@@ -2439,7 +2439,10 @@
 
   val = is_store ? argument(4) : NULL;
 
-  const TypePtr *adr_type = _gvn.type(adr)->isa_ptr();
+  const TypePtr* adr_type = _gvn.type(adr)->isa_ptr();
+  if (adr_type == TypePtr::NULL_PTR) {
+    return false; // off-heap access with zero address
+  }
 
   // Try to categorize the address.
   Compile::AliasType* alias_type = C->alias_type(adr_type);
--- a/src/hotspot/share/opto/memnode.cpp	Wed Jun 12 13:49:43 2019 +0200
+++ b/src/hotspot/share/opto/memnode.cpp	Wed Jun 12 14:06:16 2019 +0200
@@ -44,6 +44,7 @@
 #include "opto/narrowptrnode.hpp"
 #include "opto/phaseX.hpp"
 #include "opto/regmask.hpp"
+#include "opto/rootnode.hpp"
 #include "utilities/align.hpp"
 #include "utilities/copy.hpp"
 #include "utilities/macros.hpp"
@@ -328,6 +329,24 @@
   const Type *t_adr = phase->type(address);
   if (t_adr == Type::TOP)              return NodeSentinel; // caller will return NULL
 
+  if (can_reshape && is_unsafe_access() && (t_adr == TypePtr::NULL_PTR)) {
+    // Unsafe off-heap access with zero address. Remove access and other control users
+    // to not confuse optimizations and add a HaltNode to fail if this is ever executed.
+    assert(ctl != NULL, "unsafe accesses should be control dependent");
+    for (DUIterator_Fast imax, i = ctl->fast_outs(imax); i < imax; i++) {
+      Node* u = ctl->fast_out(i);
+      if (u != ctl) {
+        igvn->rehash_node_delayed(u);
+        int nb = u->replace_edge(ctl, phase->C->top());
+        --i, imax -= nb;
+      }
+    }
+    Node* frame = igvn->transform(new ParmNode(phase->C->start(), TypeFunc::FramePtr));
+    Node* halt = igvn->transform(new HaltNode(ctl, frame));
+    phase->C->root()->add_req(halt);
+    return this;
+  }
+
   if (can_reshape && igvn != NULL &&
       (igvn->_worklist.member(address) ||
        (igvn->_worklist.size() > 0 && t_adr != adr_type())) ) {
--- a/src/hotspot/share/opto/output.cpp	Wed Jun 12 13:49:43 2019 +0200
+++ b/src/hotspot/share/opto/output.cpp	Wed Jun 12 14:06:16 2019 +0200
@@ -2412,12 +2412,9 @@
     }
     assert(!last->is_Mach() || last->as_Mach()->ideal_Opcode() != Op_Con, "");
     if( last->is_Catch() ||
-       // Exclude unreachable path case when Halt node is in a separate block.
-       (_bb_end > 1 && last->is_Mach() && last->as_Mach()->ideal_Opcode() == Op_Halt) ) {
-      // There must be a prior call.  Skip it.
-      while( !bb->get_node(--_bb_end)->is_MachCall() ) {
-        assert( bb->get_node(_bb_end)->is_MachProj(), "skipping projections after expected call" );
-      }
+       (last->is_Mach() && last->as_Mach()->ideal_Opcode() == Op_Halt) ) {
+      // There might be a prior call.  Skip it.
+      while (_bb_start < _bb_end && bb->get_node(--_bb_end)->is_MachProj());
     } else if( last->is_MachNullCheck() ) {
       // Backup so the last null-checked memory instruction is
       // outside the schedulable range. Skip over the nullcheck,
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/hotspot/jtreg/compiler/unsafe/TestUnsafeLoadWithZeroAddress.java	Wed Jun 12 14:06:16 2019 +0200
@@ -0,0 +1,84 @@
+/*
+ * Copyright (c) 2019, Oracle and/or its affiliates. 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 8224658
+ * @summary Test compilation of unsafe access with zero address.
+ * @modules java.base/jdk.internal.misc:+open
+ * @run main/othervm -Xcomp
+ *                   -XX:CompileCommand=compileonly,compiler.unsafe.TestUnsafeLoadWithZeroAddress::*
+ *                   compiler.unsafe.TestUnsafeLoadWithZeroAddress
+ * @run main/othervm -Xcomp -XX:+IgnoreUnrecognizedVMOptions -XX:+AlwaysIncrementalInline
+ *                   -XX:CompileCommand=compileonly,compiler.unsafe.TestUnsafeLoadWithZeroAddress::*
+ *                   compiler.unsafe.TestUnsafeLoadWithZeroAddress
+ */
+
+package compiler.unsafe;
+
+import jdk.internal.misc.Unsafe;
+
+public class TestUnsafeLoadWithZeroAddress {
+    static final Unsafe UNSAFE = Unsafe.getUnsafe();
+    static boolean f;
+
+    public static void test1() {
+        if (f) {
+            // This branch is never executed but compiled due to -Xcomp
+            UNSAFE.getInt(0);
+        }
+    }
+
+    public static void test2() {
+        if (f) {
+            // This branch is never executed but compiled due to -Xcomp
+            UNSAFE.putInt(0, 0);
+        }
+    }
+
+    private static int getAddress() {
+        return 0;
+    }
+
+    public static void test3() {
+        if (f) {
+            // This branch is never executed but compiled due to -Xcomp
+            UNSAFE.getInt(getAddress());
+        }
+    }
+
+    public static void test4() {
+        if (f) {
+            // This branch is never executed but compiled due to -Xcomp
+            UNSAFE.putInt(getAddress(), 0);
+        }
+    }
+
+    static public void main(String[] args) {
+        test1();
+        test2();
+        test3();
+        test4();
+    }
+}
+