8134918: C2: Type speculation produces mismatched unsafe accesses
Reviewed-by: kvn
--- a/hotspot/src/share/vm/opto/compile.cpp Mon Apr 25 10:53:42 2016 +0200
+++ b/hotspot/src/share/vm/opto/compile.cpp Mon Apr 25 18:36:27 2016 +0300
@@ -1623,6 +1623,17 @@
}
}
+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) {
--- a/hotspot/src/share/vm/opto/compile.hpp Mon Apr 25 10:53:42 2016 +0200
+++ b/hotspot/src/share/vm/opto/compile.hpp Mon Apr 25 18:36:27 2016 +0300
@@ -213,6 +213,8 @@
_element = e;
}
+ BasicType basic_type() const;
+
void print_on(outputStream* st) PRODUCT_RETURN;
};
--- a/hotspot/src/share/vm/opto/library_call.cpp Mon Apr 25 10:53:42 2016 +0200
+++ b/hotspot/src/share/vm/opto/library_call.cpp Mon Apr 25 18:36:27 2016 +0300
@@ -2341,6 +2341,7 @@
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 @@
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 @@
// 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 @@
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.
+ assert(alias_type->index() != Compile::AliasIdxBot, "no bare pointers 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");
switch (kind) {
case LS_get_set:
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/hotspot/test/compiler/profiling/UnsafeAccess.java Mon Apr 25 18:36:27 2016 +0300
@@ -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");
+ }
+}