--- old/src/share/vm/c1/c1_Canonicalizer.cpp 2014-09-19 19:56:31.000000000 -0700 +++ new/src/share/vm/c1/c1_Canonicalizer.cpp 2014-09-19 19:56:31.000000000 -0700 @@ -327,7 +327,7 @@ if (t2->is_constant()) { switch (t2->tag()) { case intTag : if (t2->as_IntConstant()->value() == 0) set_canonical(x->x()); return; - case longTag : if (t2->as_IntConstant()->value() == 0) set_canonical(x->x()); return; + case longTag : if (t2->as_LongConstant()->value() == (jlong)0) set_canonical(x->x()); return; default : ShouldNotReachHere(); } } @@ -808,28 +808,41 @@ static bool match_index_and_scale(Instruction* instr, Instruction** index, - int* log2_scale, - Instruction** instr_to_unpin) { - *instr_to_unpin = NULL; - - // Skip conversion ops + int* log2_scale) { + // Skip conversion ops. This works only on 32bit because of the implicit l2i that the + // unsafe performs. +#ifndef _LP64 Convert* convert = instr->as_Convert(); - if (convert != NULL) { + if (convert != NULL && convert->op() == Bytecodes::_i2l) { + assert(convert->value()->type() == intType, "invalid input type"); instr = convert->value(); } +#endif ShiftOp* shift = instr->as_ShiftOp(); if (shift != NULL) { - if (shift->is_pinned()) { - *instr_to_unpin = shift; + if (shift->op() == Bytecodes::_lshl) { + assert(shift->x()->type() == longType, "invalid input type"); + } else { +#ifndef _LP64 + if (shift->op() == Bytecodes::_ishl) { + assert(shift->x()->type() == intType, "invalid input type"); + } else { + return false; + } +#else + return false; +#endif } + + // Constant shift value? Constant* con = shift->y()->as_Constant(); if (con == NULL) return false; // Well-known type and value? IntConstant* val = con->type()->as_IntConstant(); - if (val == NULL) return false; - if (shift->x()->type() != intType) return false; + assert(val != NULL, "Should be an int constant"); + *index = shift->x(); int tmp_scale = val->value(); if (tmp_scale >= 0 && tmp_scale < 4) { @@ -842,31 +855,42 @@ ArithmeticOp* arith = instr->as_ArithmeticOp(); if (arith != NULL) { - if (arith->is_pinned()) { - *instr_to_unpin = arith; + // See if either arg is a known constant + Constant* con = arith->x()->as_Constant(); + if (con != NULL) { + *index = arith->y(); + } else { + con = arith->y()->as_Constant(); + if (con == NULL) return false; + *index = arith->x(); } + long const_value; // Check for integer multiply - if (arith->op() == Bytecodes::_imul) { - // See if either arg is a known constant - Constant* con = arith->x()->as_Constant(); - if (con != NULL) { - *index = arith->y(); + if (arith->op() == Bytecodes::_lmul) { + assert((*index)->type() == longType, "invalid input type"); + LongConstant* val = con->type()->as_LongConstant(); + assert(val != NULL, "expecting a long constant"); + const_value = val->value(); + } else { +#ifndef _LP64 + if (arith->op() == Bytecodes::_imul) { + assert((*index)->type() == intType, "invalid input type"); + IntConstant* val = con->type()->as_IntConstant(); + assert(val != NULL, "expecting an int constant"); + const_value = val->value(); } else { - con = arith->y()->as_Constant(); - if (con == NULL) return false; - *index = arith->x(); - } - if ((*index)->type() != intType) return false; - // Well-known type and value? - IntConstant* val = con->type()->as_IntConstant(); - if (val == NULL) return false; - switch (val->value()) { - case 1: *log2_scale = 0; return true; - case 2: *log2_scale = 1; return true; - case 4: *log2_scale = 2; return true; - case 8: *log2_scale = 3; return true; - default: return false; + return false; } +#else + return false; +#endif + } + switch (const_value) { + case 1: *log2_scale = 0; return true; + case 2: *log2_scale = 1; return true; + case 4: *log2_scale = 2; return true; + case 8: *log2_scale = 3; return true; + default: return false; } } @@ -879,29 +903,37 @@ Instruction** base, Instruction** index, int* log2_scale) { - Instruction* instr_to_unpin = NULL; ArithmeticOp* root = x->base()->as_ArithmeticOp(); if (root == NULL) return false; // Limit ourselves to addition for now if (root->op() != Bytecodes::_ladd) return false; + + bool match_found = false; // Try to find shift or scale op - if (match_index_and_scale(root->y(), index, log2_scale, &instr_to_unpin)) { + if (match_index_and_scale(root->y(), index, log2_scale)) { *base = root->x(); - } else if (match_index_and_scale(root->x(), index, log2_scale, &instr_to_unpin)) { + match_found = true; + } else if (match_index_and_scale(root->x(), index, log2_scale)) { *base = root->y(); - } else if (root->y()->as_Convert() != NULL) { + match_found = true; + } else if (NOT_LP64(root->y()->as_Convert() != NULL) LP64_ONLY(false)) { + // Skipping i2l works only on 32bit because of the implicit l2i that the unsafe performs. + // 64bit needs a real sign-extending conversion. Convert* convert = root->y()->as_Convert(); - if (convert->op() == Bytecodes::_i2l && convert->value()->type() == intType) { + if (convert->op() == Bytecodes::_i2l) { + assert(convert->value()->type() == intType, "should be an int"); // pick base and index, setting scale at 1 *base = root->x(); *index = convert->value(); *log2_scale = 0; - } else { - return false; + match_found = true; } - } else { - // doesn't match any expected sequences - return false; + } + // The default solution + if (!match_found) { + *base = root->x(); + *index = root->y(); + *log2_scale = 0; } // If the value is pinned then it will be always be computed so --- old/src/share/vm/c1/c1_LIRGenerator.cpp 2014-09-19 19:56:32.000000000 -0700 +++ new/src/share/vm/c1/c1_LIRGenerator.cpp 2014-09-19 19:56:32.000000000 -0700 @@ -2045,6 +2045,8 @@ } } +// Here UnsafeGetRaw may have x->base() and x->index() be int or long +// on both 64 and 32 bits. Expecting x->base() to be always long on 64bit. void LIRGenerator::do_UnsafeGetRaw(UnsafeGetRaw* x) { LIRItem base(x->base(), this); LIRItem idx(this); @@ -2059,50 +2061,70 @@ int log2_scale = 0; if (x->has_index()) { - assert(x->index()->type()->tag() == intTag, "should not find non-int index"); log2_scale = x->log2_scale(); } assert(!x->has_index() || idx.value() == x->index(), "should match"); LIR_Opr base_op = base.result(); + LIR_Opr index_op = idx.result(); #ifndef _LP64 if (x->base()->type()->tag() == longTag) { base_op = new_register(T_INT); __ convert(Bytecodes::_l2i, base.result(), base_op); - } else { - assert(x->base()->type()->tag() == intTag, "must be"); } + if (x->has_index()) { + if (x->index()->type()->tag() == longTag) { + LIR_Opr long_index_op = index_op; + if (x->index()->type()->is_constant()) { + long_index_op = new_register(T_LONG); + __ move(index_op, long_index_op); + } + index_op = new_register(T_INT); + __ convert(Bytecodes::_l2i, long_index_op, index_op); + } else { + assert(x->index()->type()->tag() == intTag, "must be"); + } + } + // At this point base and index should be all ints. + assert(base_op->type() == T_INT && !base_op->is_constant(), "base should be an non-constant int"); + assert(!x->has_index() || index_op->type() == T_INT, "index should be an int"); +#else + if (x->has_index()) { + if (x->index()->type()->tag() == intTag) { + if (!x->index()->type()->is_constant()) { + index_op = new_register(T_LONG); + __ convert(Bytecodes::_i2l, idx.result(), index_op); + } + } else { + assert(x->index()->type()->tag() == longTag, "must be"); + if (x->index()->type()->is_constant()) { + index_op = new_register(T_LONG); + __ move(idx.result(), index_op); + } + } + } + // At this point base is a long non-constant + // Index is a long register or a int constant + assert(base_op->type() == T_LONG && !base_op->is_constant(), "base must be a long non-constant"); + assert(!x->has_index() || (index_op->type() == T_INT && index_op->is_constant()) || + (index_op->type() == T_LONG && !index_op->is_constant()), "unexpected index type"); #endif BasicType dst_type = x->basic_type(); - LIR_Opr index_op = idx.result(); LIR_Address* addr; if (index_op->is_constant()) { assert(log2_scale == 0, "must not have a scale"); + assert(index_op->type() == T_INT, "only int constants supported"); addr = new LIR_Address(base_op, index_op->as_jint(), dst_type); } else { #ifdef X86 -#ifdef _LP64 - if (!index_op->is_illegal() && index_op->type() == T_INT) { - LIR_Opr tmp = new_pointer_register(); - __ convert(Bytecodes::_i2l, index_op, tmp); - index_op = tmp; - } -#endif addr = new LIR_Address(base_op, index_op, LIR_Address::Scale(log2_scale), 0, dst_type); #elif defined(ARM) addr = generate_address(base_op, index_op, log2_scale, 0, dst_type); #else if (index_op->is_illegal() || log2_scale == 0) { -#ifdef _LP64 - if (!index_op->is_illegal() && index_op->type() == T_INT) { - LIR_Opr tmp = new_pointer_register(); - __ convert(Bytecodes::_i2l, index_op, tmp); - index_op = tmp; - } -#endif addr = new LIR_Address(base_op, index_op, dst_type); } else { LIR_Opr tmp = new_pointer_register(); @@ -2129,7 +2151,6 @@ BasicType type = x->basic_type(); if (x->has_index()) { - assert(x->index()->type()->tag() == intTag, "should not find non-int index"); log2_scale = x->log2_scale(); } @@ -2152,38 +2173,39 @@ set_no_result(x); LIR_Opr base_op = base.result(); + LIR_Opr index_op = idx.result(); + #ifndef _LP64 if (x->base()->type()->tag() == longTag) { base_op = new_register(T_INT); __ convert(Bytecodes::_l2i, base.result(), base_op); - } else { - assert(x->base()->type()->tag() == intTag, "must be"); } + if (x->has_index()) { + if (x->index()->type()->tag() == longTag) { + index_op = new_register(T_INT); + __ convert(Bytecodes::_l2i, idx.result(), index_op); + } + } + // At this point base and index should be all ints and not constants + assert(base_op->type() == T_INT && !base_op->is_constant(), "base should be an non-constant int"); + assert(!x->has_index() || (index_op->type() == T_INT && !index_op->is_constant()), "index should be an non-constant int"); +#else + if (x->has_index()) { + if (x->index()->type()->tag() == intTag) { + index_op = new_register(T_LONG); + __ convert(Bytecodes::_i2l, idx.result(), index_op); + } + } + // At this point base and index are long and non-constant + assert(base_op->type() == T_LONG && !base_op->is_constant(), "base must be a non-constant long"); + assert(!x->has_index() || (index_op->type() == T_LONG && !index_op->is_constant()), "index must be a non-constant long"); #endif - LIR_Opr index_op = idx.result(); if (log2_scale != 0) { // temporary fix (platform dependent code without shift on Intel would be better) - index_op = new_pointer_register(); -#ifdef _LP64 - if(idx.result()->type() == T_INT) { - __ convert(Bytecodes::_i2l, idx.result(), index_op); - } else { -#endif - // TODO: ARM also allows embedded shift in the address - __ move(idx.result(), index_op); -#ifdef _LP64 - } -#endif + // TODO: ARM also allows embedded shift in the address __ shift_left(index_op, log2_scale, index_op); } -#ifdef _LP64 - else if(!index_op->is_illegal() && index_op->type() == T_INT) { - LIR_Opr tmp = new_pointer_register(); - __ convert(Bytecodes::_i2l, index_op, tmp); - index_op = tmp; - } -#endif LIR_Address* addr = new LIR_Address(base_op, index_op, x->basic_type()); __ move(value.result(), addr); --- /dev/null 2014-09-19 19:56:34.000000000 -0700 +++ new/test/compiler/unsafe/UnsafeRaw.java 2014-09-19 19:56:34.000000000 -0700 @@ -0,0 +1,123 @@ +/* + * Copyright (c) 2014, 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 8058744 + * @summary Invalid pattern-matching of address computations in raw unsafe + * @run main/othervm -Xbatch UnsafeRaw + * + */ + +import sun.misc.Unsafe; +import java.util.Random; + +public class UnsafeRaw { + private static sun.misc.Unsafe getUnsafe() throws NoSuchFieldException, IllegalAccessException { + java.lang.reflect.Field f = sun.misc + .Unsafe.class.getDeclaredField("theUnsafe"); + f.setAccessible(true); + sun.misc.Unsafe unsafe = (Unsafe) f.get(null); + return unsafe; + } + public static class Tests { + public static int int_index(sun.misc.Unsafe unsafe, long base, int index) throws Exception { + return unsafe.getInt(base + (index << 2)); + } + public static int long_index(sun.misc.Unsafe unsafe, long base, long index) throws Exception { + return unsafe.getInt(base + (index << 2)); + } + public static int int_index_back_ashift(sun.misc.Unsafe unsafe, long base, int index) throws Exception { + return unsafe.getInt(base + (index >> 2)); + } + public static int int_index_back_lshift(sun.misc.Unsafe unsafe, long base, int index) throws Exception { + return unsafe.getInt(base + (index >>> 2)); + } + public static int long_index_back_ashift(sun.misc.Unsafe unsafe, long base, long index) throws Exception { + return unsafe.getInt(base + (index >> 2)); + } + public static int long_index_back_lshift(sun.misc.Unsafe unsafe, long base, long index) throws Exception { + return unsafe.getInt(base + (index >>> 2)); + } + public static int int_const_12345678_index(sun.misc.Unsafe unsafe, long base) throws Exception { + int idx4 = 0x12345678; + return unsafe.getInt(base + idx4); + } + public static int long_const_1234567890abcdef_index(sun.misc.Unsafe unsafe, long base) throws Exception { + long idx5 = 0x1234567890abcdefL; + return unsafe.getInt(base + idx5); + } + } + + public static void main(String[] args) throws Exception { + sun.misc.Unsafe unsafe = getUnsafe(); + final int array_size = 128; + final int element_size = 4; + final int magic = 0x12345678; + + Random rnd = new Random(); + + long array = unsafe.allocateMemory(array_size * element_size); // 128 ints + long addr = array + array_size * element_size / 2; // something in the middle to work with + unsafe.putInt(addr, magic); + for (int j = 0; j < 100000; j++) { + if (Tests.int_index(unsafe, addr, 0) != magic) throw new Exception(); + if (Tests.long_index(unsafe, addr, 0) != magic) throw new Exception(); + { + long idx1 = rnd.nextLong(); + long addr1 = addr - (idx1 << 2); + if (Tests.long_index(unsafe, addr1, idx1) != magic) throw new Exception(); + } + { + long idx2 = rnd.nextLong(); + long addr2 = addr - (idx2 >> 2); + if (Tests.long_index_back_ashift(unsafe, addr2, idx2) != magic) throw new Exception(); + } + { + long idx3 = rnd.nextLong(); + long addr3 = addr - (idx3 >>> 2); + if (Tests.long_index_back_lshift(unsafe, addr3, idx3) != magic) throw new Exception(); + } + { + long idx4 = 0x12345678; + long addr4 = addr - idx4; + if (Tests.int_const_12345678_index(unsafe, addr4) != magic) throw new Exception(); + } + { + long idx5 = 0x1234567890abcdefL; + long addr5 = addr - idx5; + if (Tests.long_const_1234567890abcdef_index(unsafe, addr5) != magic) throw new Exception(); + } + { + int idx6 = rnd.nextInt(); + long addr6 = addr - (idx6 >> 2); + if (Tests.int_index_back_ashift(unsafe, addr6, idx6) != magic) throw new Exception(); + } + { + int idx7 = rnd.nextInt(); + long addr7 = addr - (idx7 >>> 2); + if (Tests.int_index_back_lshift(unsafe, addr7, idx7) != magic) throw new Exception(); + } + } + } +}