8134918: C2: Type speculation produces mismatched unsafe accesses

Reviewed-by: kvn

hotspot/src/share/vm/opto/compile.cpp

@@ -1623,6 +1623,17 @@ void Compile::AliasType::Init(int i, const TypePtr* at) {
   }
 }
 
+BasicType Compile::AliasType::basic_type() const {
+  if (element() != NULL) {
+    const Type* element = adr_type()->is_aryptr()->elem();
+    return element->isa_narrowoop() ? T_OBJECT : element->array_element_basic_type();
+  } if (field() != NULL) {
+    return field()->layout_type();
+  } else {
+    return T_ILLEGAL; // unknown
+  }
+}
+
 //---------------------------------print_on------------------------------------
 #ifndef PRODUCT
 void Compile::AliasType::print_on(outputStream* st) {

hotspot/src/share/vm/opto/compile.hpp

@@ -213,6 +213,8 @@ class Compile : public Phase {
       _element = e;
     }
 
+    BasicType basic_type() const;
+
     void print_on(outputStream* st) PRODUCT_RETURN;
   };
 

hotspot/src/share/vm/opto/library_call.cpp

@@ -2341,6 +2341,7 @@ bool LibraryCallKit::inline_unsafe_access(const bool is_native_ptr, bool is_stor
   if (callee()->is_static())  return false;  // caller must have the capability!
   guarantee(!is_store || kind != Acquire, "Acquire accesses can be produced only for loads");
   guarantee( is_store || kind != Release, "Release accesses can be produced only for stores");
+  assert(type != T_OBJECT || !unaligned, "unaligned access not supported with object type");
 
 #ifndef PRODUCT
   {
@@ -2416,14 +2417,35 @@ bool LibraryCallKit::inline_unsafe_access(const bool is_native_ptr, bool is_stor
 
   const TypePtr *adr_type = _gvn.type(adr)->isa_ptr();
 
-  // First guess at the value type.
-  const Type *value_type = Type::get_const_basic_type(type);
-
   // Try to categorize the address.  If it comes up as TypeJavaPtr::BOTTOM,
   // there was not enough information to nail it down.
   Compile::AliasType* alias_type = C->alias_type(adr_type);
   assert(alias_type->index() != Compile::AliasIdxBot, "no bare pointers here");
 
+  assert(alias_type->adr_type() == TypeRawPtr::BOTTOM || alias_type->adr_type() == TypeOopPtr::BOTTOM ||
+         alias_type->basic_type() != T_ILLEGAL, "field, array element or unknown");
+  bool mismatched = false;
+  BasicType bt = alias_type->basic_type();
+  if (bt != T_ILLEGAL) {
+    if (bt == T_BYTE && adr_type->isa_aryptr()) {
+      // Alias type doesn't differentiate between byte[] and boolean[]).
+      // Use address type to get the element type.
+      bt = adr_type->is_aryptr()->elem()->array_element_basic_type();
+    }
+    if (bt == T_ARRAY || bt == T_NARROWOOP) {
+      // accessing an array field with getObject is not a mismatch
+      bt = T_OBJECT;
+    }
+    if ((bt == T_OBJECT) != (type == T_OBJECT)) {
+      // Don't intrinsify mismatched object accesses
+      return false;
+    }
+    mismatched = (bt != type);
+  }
+
+  // First guess at the value type.
+  const Type *value_type = Type::get_const_basic_type(type);
+
   // We will need memory barriers unless we can determine a unique
   // alias category for this reference.  (Note:  If for some reason
   // the barriers get omitted and the unsafe reference begins to "pollute"
@@ -2524,29 +2546,6 @@ bool LibraryCallKit::inline_unsafe_access(const bool is_native_ptr, bool is_stor
   // of safe & unsafe memory.
   if (need_mem_bar) insert_mem_bar(Op_MemBarCPUOrder);
 
-  assert(alias_type->adr_type() == TypeRawPtr::BOTTOM || alias_type->adr_type() == TypeOopPtr::BOTTOM ||
-         alias_type->field() != NULL || alias_type->element() != NULL, "field, array element or unknown");
-  bool mismatched = false;
-  if (alias_type->element() != NULL || alias_type->field() != NULL) {
-    BasicType bt;
-    if (alias_type->element() != NULL) {
-      // Use address type to get the element type. Alias type doesn't provide
-      // enough information (e.g., doesn't differentiate between byte[] and boolean[]).
-      const Type* element = adr_type->is_aryptr()->elem();
-      bt = element->isa_narrowoop() ? T_OBJECT : element->array_element_basic_type();
-    } else {
-      bt = alias_type->field()->layout_type();
-    }
-    if (bt == T_ARRAY) {
-      // accessing an array field with getObject is not a mismatch
-      bt = T_OBJECT;
-    }
-    if (bt != type) {
-      mismatched = true;
-    }
-  }
-  assert(type != T_OBJECT || !unaligned, "unaligned access not supported with object type");
-
   if (!is_store) {
     Node* p = NULL;
     // Try to constant fold a load from a constant field
@@ -2814,11 +2813,20 @@ bool LibraryCallKit::inline_unsafe_load_store(const BasicType type, const LoadSt
   Node* adr = make_unsafe_address(base, offset);
   const TypePtr *adr_type = _gvn.type(adr)->isa_ptr();
 
+  Compile::AliasType* alias_type = C->alias_type(adr_type);
+  assert(alias_type->adr_type() == TypeRawPtr::BOTTOM || alias_type->adr_type() == TypeOopPtr::BOTTOM ||
+         alias_type->basic_type() != T_ILLEGAL, "field, array element or unknown");
+  BasicType bt = alias_type->basic_type();
+  if (bt != T_ILLEGAL &&
+      ((bt == T_OBJECT || bt == T_ARRAY) != (type == T_OBJECT))) {
+    // Don't intrinsify mismatched object accesses.
+    return false;
+  }
+
   // For CAS, unlike inline_unsafe_access, there seems no point in
   // trying to refine types. Just use the coarse types here.
-  const Type *value_type = Type::get_const_basic_type(type);
-  Compile::AliasType* alias_type = C->alias_type(adr_type);
   assert(alias_type->index() != Compile::AliasIdxBot, "no bare pointers here");
+  const Type *value_type = Type::get_const_basic_type(type);
 
   switch (kind) {
     case LS_get_set:

hotspot/test/compiler/profiling/UnsafeAccess.java

@@ -0,0 +1,88 @@
+/*
+ * Copyright (c) 2016, 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 8134918
+ * @modules java.base/jdk.internal.misc
+ * @run main/bootclasspath -XX:+IgnoreUnrecognizedVMOptions -XX:TypeProfileLevel=222 -XX:+UseTypeSpeculation -Xbatch
+ *                         -XX:CompileCommand=dontinline,UnsafeAccess::test*
+ *                         UnsafeAccess
+ */
+import jdk.internal.misc.Unsafe;
+
+public class UnsafeAccess {
+    private static final Unsafe U = Unsafe.getUnsafe();
+
+    static Class cls = Object.class;
+    static long off = U.ARRAY_OBJECT_BASE_OFFSET;
+
+    static Object testUnsafeAccess(Object o, boolean isObjArray) {
+        if (o != null && cls.isInstance(o)) { // speculates "o" type to int[]
+            return helperUnsafeAccess(o, isObjArray);
+        }
+        return null;
+    }
+
+    static Object helperUnsafeAccess(Object o, boolean isObjArray) {
+        if (isObjArray) {
+            U.putObject(o, off, new Object());
+        }
+        return o;
+    }
+
+    static Object testUnsafeLoadStore(Object o, boolean isObjArray) {
+        if (o != null && cls.isInstance(o)) { // speculates "o" type to int[]
+            return helperUnsafeLoadStore(o, isObjArray);
+        }
+        return null;
+    }
+
+    static Object helperUnsafeLoadStore(Object o, boolean isObjArray) {
+        if (isObjArray) {
+            Object o1 = U.getObject(o, off);
+            U.compareAndSwapObject(o, off, o1, new Object());
+        }
+        return o;
+    }
+
+    public static void main(String[] args) {
+        Object[] objArray = new Object[10];
+        int[]    intArray = new    int[10];
+
+        for (int i = 0; i < 20_000; i++) {
+            helperUnsafeAccess(objArray, true);
+        }
+        for (int i = 0; i < 20_000; i++) {
+            testUnsafeAccess(intArray, false);
+        }
+
+        for (int i = 0; i < 20_000; i++) {
+            helperUnsafeLoadStore(objArray, true);
+        }
+        for (int i = 0; i < 20_000; i++) {
+            testUnsafeLoadStore(intArray, false);
+        }
+
+        System.out.println("TEST PASSED");
+    }
+}