--- old/src/share/vm/opto/graphKit.hpp 2017-06-16 14:34:20.529928428 +0200 +++ new/src/share/vm/opto/graphKit.hpp 2017-06-16 14:34:20.209928443 +0200 @@ -866,6 +866,7 @@ // (Caller is responsible for doing replace_in_map.) Node* type_check_receiver(Node* receiver, ciKlass* klass, float prob, Node* *casted_receiver); + Node* type_check(Node* recv_klass, const TypeKlassPtr* tklass, float prob); // implementation of object creation Node* set_output_for_allocation(AllocateNode* alloc, --- old/src/share/vm/opto/parse2.cpp 2017-06-16 14:34:20.681928421 +0200 +++ new/src/share/vm/opto/parse2.cpp 2017-06-16 14:34:20.325928438 +0200 @@ -1748,8 +1748,7 @@ case Bytecodes::_vastore: { d = array_addressing(T_OBJECT, 1); if (stopped()) return; // guaranteed null or range check - // TODO fix this - // array_store_check(); + array_store_check(true); c = pop(); // Oop to store b = pop(); // index (already used) a = pop(); // the array itself --- old/src/share/vm/oops/arrayKlass.hpp 2017-06-16 14:34:20.665928422 +0200 +++ new/src/share/vm/oops/arrayKlass.hpp 2017-06-16 14:34:20.281928440 +0200 @@ -41,6 +41,13 @@ Klass* volatile _lower_dimension; // Refers the (n-1)'th-dimensional array (if present). protected: + Klass* _element_klass; // The klass of the elements of this array type + // The element type must be registered for both object arrays + // (incl. object arrays with value type elements) and value type + // arrays containing flattened value types. However, the element + // type must not be registered for arrays of primitive types. + // TODO: Update the class hierarchy so that element klass appears + // only in array that contain non-primitive types. // Constructors // The constructor with the Symbol argument does the real array // initialization, the other is a dummy @@ -51,6 +58,13 @@ static Symbol* create_element_klass_array_name(Klass* element_klass, TRAPS); public: + // Instance variables + virtual Klass* element_klass() const { return _element_klass; } + virtual void set_element_klass(Klass* k) { _element_klass = k; } + + // Compiler/Interpreter offset + static ByteSize element_klass_offset() { return in_ByteSize(offset_of(ArrayKlass, _element_klass)); } + // Testing operation DEBUG_ONLY(bool is_array_klass_slow() const { return true; }) --- old/src/share/vm/opto/parseHelper.cpp 2017-06-16 14:34:20.681928421 +0200 +++ new/src/share/vm/opto/parseHelper.cpp 2017-06-16 14:34:20.317928438 +0200 @@ -27,6 +27,7 @@ #include "classfile/systemDictionary.hpp" #include "compiler/compileLog.hpp" #include "oops/objArrayKlass.hpp" +#include "oops/valueArrayKlass.hpp" #include "opto/addnode.hpp" #include "opto/memnode.hpp" #include "opto/mulnode.hpp" @@ -139,7 +140,7 @@ //------------------------------array_store_check------------------------------ // pull array from stack and check that the store is valid -void Parse::array_store_check() { +void Parse::array_store_check(bool target_is_valuetypearray) { // Shorthand access to array store elements without popping them. Node *obj = peek(0); @@ -223,7 +224,8 @@ // Come here for polymorphic array klasses // Extract the array element class - int element_klass_offset = in_bytes(ObjArrayKlass::element_klass_offset()); + int element_klass_offset = in_bytes(ArrayKlass::element_klass_offset()); + Node *p2 = basic_plus_adr(array_klass, array_klass, element_klass_offset); // We are allowed to use the constant type only if cast succeeded. If always_see_exact_class is true, // we must set a control edge from the IfTrue node created by the uncommon_trap above to the @@ -231,9 +233,22 @@ Node* a_e_klass = _gvn.transform(LoadKlassNode::make(_gvn, always_see_exact_class ? control() : NULL, immutable_memory(), p2, tak)); - // Check (the hard way) and throw if not a subklass. - // Result is ignored, we just need the CFG effects. - gen_checkcast(obj, a_e_klass); + if (target_is_valuetypearray) { + ciKlass* target_elem_klass = gvn().type(a_e_klass)->is_klassptr()->klass(); + ciKlass* source_klass = gvn().type(obj)->is_valuetype()->value_klass(); + if (!target_elem_klass->equals(source_klass)) { + Node* slow_ctl = type_check(a_e_klass, TypeKlassPtr::make(source_klass), 1.0); + { + PreserveJVMState pjvms(this); + set_control(slow_ctl); + builtin_throw(Deoptimization::Reason_class_check); + } + } + } else { + // Check (the hard way) and throw if not a subklass. + // Result is ignored, we just need the CFG effects. + gen_checkcast(obj, a_e_klass); + } } --- old/src/share/vm/oops/valueArrayKlass.cpp 2017-06-16 14:34:20.665928422 +0200 +++ new/src/share/vm/oops/valueArrayKlass.cpp 2017-06-16 14:34:20.277928440 +0200 @@ -69,14 +69,18 @@ #endif } -void ValueArrayKlass::set_element_klass(ValueKlass* k) { +ValueKlass* ValueArrayKlass::element_klass() const { + return ValueKlass::cast(_element_klass); +} + +void ValueArrayKlass::set_element_klass(Klass* k) { _element_klass = k; } ValueArrayKlass* ValueArrayKlass::allocate_klass(Klass* element_klass, Symbol* name, TRAPS) { - assert(ValueArrayFlatten, "Flatten array not allowed"); + assert(ValueArrayFlatten, "Flatten array required"); assert(ValueKlass::cast(element_klass)->is_atomic() || (!ValueArrayAtomicAccess), "Atomic by-default"); ClassLoaderData* loader_data = element_klass->class_loader_data(); --- old/src/share/vm/ci/bcEscapeAnalyzer.cpp 2017-06-16 14:34:20.673928422 +0200 +++ new/src/share/vm/ci/bcEscapeAnalyzer.cpp 2017-06-16 14:34:20.301928439 +0200 @@ -553,7 +553,6 @@ set_modified(arr, OFFSET_ANY, type2size[T_LONG]*HeapWordSize); break; } - case Bytecodes::_vastore: case Bytecodes::_aastore: { set_global_escape(state.apop()); @@ -562,6 +561,17 @@ set_modified(arr, OFFSET_ANY, type2size[T_OBJECT]*HeapWordSize); break; } + case Bytecodes::_vastore: + { + set_global_escape(state.apop()); + state.spop(); + ArgumentMap arr = state.apop(); + // If the array is flattened, a larger part of it is modified than + // the size of a reference. However, if OFFSET_ANY is given as + // parameter to set_modified(), size is not taken into account. + set_modified(arr, OFFSET_ANY, type2size[T_VALUETYPE]*HeapWordSize); + break; + } case Bytecodes::_pop: state.raw_pop(); break; --- old/src/share/vm/oops/valueArrayKlass.hpp 2017-06-16 14:34:20.717928419 +0200 +++ new/src/share/vm/oops/valueArrayKlass.hpp 2017-06-16 14:34:20.317928438 +0200 @@ -36,11 +36,8 @@ class ValueArrayKlass : public ArrayKlass { friend class VMStructs; private: - ValueKlass* _element_klass; // The klass of the elements of this array type - // Constructor ValueArrayKlass(Klass* element_klass, Symbol* name); - void set_element_klass(ValueKlass* k); static ValueArrayKlass* allocate_klass(Klass* element_klass, Symbol* name, TRAPS); protected: @@ -54,6 +51,9 @@ ValueArrayKlass() {} + virtual ValueKlass* element_klass() const; + virtual void set_element_klass(Klass* k); + // Casting from Klass* static ValueArrayKlass* cast(Klass* k) { assert(k->is_valueArray_klass(), "cast to ValueArrayKlass"); @@ -65,10 +65,6 @@ void initialize(TRAPS); - // Instance variables - ValueKlass* element_klass() const { return _element_klass; } - ValueKlass** element_klass_addr() { return &_element_klass; } - ModuleEntry* module() const; PackageEntry* package() const; @@ -76,8 +72,13 @@ bool is_valueArray_klass_slow() const { return true; } - bool contains_oops() { return element_klass()->contains_oops(); } - bool is_atomic() { return element_klass()->is_atomic(); } + bool contains_oops() { + return element_klass()->contains_oops(); + } + + bool is_atomic() { + return element_klass()->is_atomic(); + } oop protection_domain() const; @@ -101,9 +102,6 @@ // Copying void copy_array(arrayOop s, int src_pos, arrayOop d, int dst_pos, int length, TRAPS); - // Compiler/Interpreter offset - static ByteSize element_klass_offset() { return in_ByteSize(offset_of(ValueArrayKlass, _element_klass)); } - // GC specific object visitors // // Mark Sweep --- old/src/share/vm/oops/objArrayKlass.hpp 2017-06-16 14:34:20.753928418 +0200 +++ new/src/share/vm/oops/objArrayKlass.hpp 2017-06-16 14:34:20.333928437 +0200 @@ -35,7 +35,6 @@ friend class VMStructs; friend class JVMCIVMStructs; private: - Klass* _element_klass; // The klass of the elements of this array type Klass* _bottom_klass; // The one-dimensional type (InstanceKlass or TypeArrayKlass) // Constructor @@ -45,11 +44,6 @@ // For dummy objects ObjArrayKlass() {} - // Instance variables - Klass* element_klass() const { return _element_klass; } - void set_element_klass(Klass* k) { _element_klass = k; } - Klass** element_klass_addr() { return &_element_klass; } - Klass* bottom_klass() const { return _bottom_klass; } void set_bottom_klass(Klass* k) { _bottom_klass = k; } Klass** bottom_klass_addr() { return &_bottom_klass; } @@ -57,9 +51,6 @@ ModuleEntry* module() const; PackageEntry* package() const; - // Compiler/Interpreter offset - static ByteSize element_klass_offset() { return in_ByteSize(offset_of(ObjArrayKlass, _element_klass)); } - // Dispatched operation bool can_be_primary_super_slow() const; GrowableArray* compute_secondary_supers(int num_extra_slots); --- old/src/share/vm/opto/parse.hpp 2017-06-16 14:34:20.789928416 +0200 +++ new/src/share/vm/opto/parse.hpp 2017-06-16 14:34:20.389928435 +0200 @@ -464,7 +464,7 @@ void do_one_bytecode(); // helper function to generate array store check - void array_store_check(); + void array_store_check(bool target_is_valuetypearray = false); // Helper function to generate array load void array_load(BasicType etype); // Helper function to generate array store --- old/test/compiler/valhalla/valuetypes/ValueTypeTestBench.java 2017-06-16 14:34:20.793928416 +0200 +++ new/test/compiler/valhalla/valuetypes/ValueTypeTestBench.java 2017-06-16 14:34:20.413928434 +0200 @@ -566,6 +566,8 @@ private static final MethodHandle objectUnboxLoadLongMH = generateObjectUnboxLoadLongMH(); private static final MethodHandle objectBoxMH = generateObjectBoxMH(); private static final MethodHandle checkedvccUnboxLoadLongMH = generateCheckedVCCUnboxLoadLongMH(); + private static final MethodHandle vastoreMH = generateVastore(); + private static final MethodHandle invalidVastoreMH = generateInvalidVastore(); private static final ValueCapableClass1 vcc = ValueCapableClass1.create(rL, rI, (short)rI, (short)rI); private static final ValueCapableClass2 vcc2 = ValueCapableClass2.create(rL + 1); @@ -769,7 +771,7 @@ } // Test loop with uncommon trap referencing a value type - @Test(match = {TRAP, SCOBJ}, matchCount = {1, -1 /* at least 1 */}, failOn = LOAD) + @Test(match = {SCOBJ}, matchCount = {-1 /* at least 1 */}, failOn = LOAD) public long test12(boolean b) { MyValue1 v = MyValue1.createWithFieldsInline(rI, rL); MyValue1[] va = new MyValue1[Math.abs(rI) % 10]; @@ -799,7 +801,7 @@ } // Test loop with uncommon trap referencing a value type - @Test(match = {TRAP}, matchCount = {1}) + @Test public long test13(boolean b) { MyValue1 v = MyValue1.createWithFieldsDontInline(rI, rL); MyValue1[] va = new MyValue1[Math.abs(rI) % 10]; @@ -1437,7 +1439,7 @@ } // Merge value type arrays created from two branches - @Test(failOn = (TRAP)) + @Test public MyValue1[] test45(boolean b) { MyValue1[] va; if (b) { @@ -2161,6 +2163,208 @@ } } + /* Array load out of bounds (upper bound) at compile time.*/ + @Test + public int test77() { + int arraySize = Math.abs(rI) % 10;; + MyValue1[] va = new MyValue1[arraySize]; + + for (int i = 0; i < arraySize; i++) { + va[i] = MyValue1.createWithFieldsDontInline(rI + 1, rL); + } + + try { + return va[arraySize + 1].x; + } catch (ArrayIndexOutOfBoundsException e) { + return rI; + } + } + + public void test77_verifier(boolean warmup) { + Asserts.assertEQ(test77(), rI); + } + + /* Array load out of bounds (lower bound) at compile time.*/ + @Test + public int test78() { + int arraySize = Math.abs(rI) % 10;; + MyValue1[] va = new MyValue1[arraySize]; + + for (int i = 0; i < arraySize; i++) { + va[i] = MyValue1.createWithFieldsDontInline(rI + i, rL); + } + + try { + return va[-arraySize].x; + } catch (ArrayIndexOutOfBoundsException e) { + return rI; + } + } + + public void test78_verifier(boolean warmup) { + Asserts.assertEQ(test78(), rI); + } + + /* Array load out of bound not known to compiler (both lower and upper bound). */ + @Test + public int test79(MyValue1[] va, int index) { + return va[index].x; + } + + public void test79_verifier(boolean warmup) { + int arraySize = Math.abs(rI) % 10; + MyValue1[] va = new MyValue1[arraySize]; + + for (int i = 0; i < arraySize; i++) { + va[i] = MyValue1.createWithFieldsDontInline(rI, rL); + } + + int result; + for (int i = -20; i < 20; i++) { + try { + result = test79(va, i); + } catch (ArrayIndexOutOfBoundsException e) { + result = rI; + } + Asserts.assertEQ(result, rI); + } + } + + /* Array store out of bounds (upper bound) at compile time.*/ + @Test + public int test80() { + int arraySize = Math.abs(rI) % 10;; + MyValue1[] va = new MyValue1[arraySize]; + + try { + for (int i = 0; i <= arraySize; i++) { + va[i] = MyValue1.createWithFieldsDontInline(rI + 1, rL); + } + return rI - 1; + } catch (ArrayIndexOutOfBoundsException e) { + return rI; + } + } + + public void test80_verifier(boolean warmup) { + Asserts.assertEQ(test80(), rI); + } + + /* Array store out of bounds (lower bound) at compile time.*/ + @Test + public int test81() { + int arraySize = Math.abs(rI) % 10;; + MyValue1[] va = new MyValue1[arraySize]; + + try { + for (int i = -1; i <= arraySize; i++) { + va[i] = MyValue1.createWithFieldsDontInline(rI + 1, rL); + } + return rI - 1; + } catch (ArrayIndexOutOfBoundsException e) { + return rI; + } + } + + public void test81_verifier(boolean warmup) { + Asserts.assertEQ(test81(), rI); + } + + /* Array store out of bound not known to compiler (both lower and upper bound). */ + @Test + public int test82(MyValue1[] va, int index, MyValue1 vt) { + va[index] = vt; + return va[index].x; + } + + @DontCompile + public void test82_verifier(boolean warmup) { + int arraySize = Math.abs(rI) % 10; + MyValue1[] va = new MyValue1[arraySize]; + + for (int i = 0; i < arraySize; i++) { + va[i] = MyValue1.createWithFieldsDontInline(rI, rL); + } + + MyValue1 vt = MyValue1.createWithFieldsDontInline(rI + 1, rL); + int result; + for (int i = -20; i < 20; i++) { + try { + result = test82(va, i, vt); + } catch (ArrayIndexOutOfBoundsException e) { + result = rI + 1; + } + Asserts.assertEQ(result, rI + 1); + } + + for (int i = 0; i < arraySize; i++) { + Asserts.assertEQ(va[i].x, rI + 1); + } + } + + /* Create a new value type array and store a value type into + * it. The test should pass without throwing an exception. */ + @Test + public void test83() throws Throwable { + vastoreMH.invokeExact(vcc); + } + + public void test83_verifier(boolean warmup) throws Throwable { + test83(); + } + + private static MethodHandle generateVastore() { + return MethodHandleBuilder.loadCode(MethodHandles.lookup(), + "Vastore", + MethodType.methodType(void.class, ValueCapableClass1.class), + CODE -> { + CODE. + iconst_1(). + anewarray(ValueType.forClass(ValueCapableClass1.class).valueClass()). + iconst_0(). + aload_0(). + vunbox(ValueType.forClass(ValueCapableClass1.class).valueClass()). + vastore(). + return_(); + } + ); + } + + /* Create a new value type array with element type + * ValueCapableClass1 and attempt to store a value type of type + * ValueCapableClass2 into it. */ + @Test + public void test84() throws Throwable { + invalidVastoreMH.invokeExact(vcc2); + } + + public void test84_verifier(boolean warmup) throws Throwable { + boolean exceptionThrown = false; + try { + test84(); + } catch (ArrayStoreException e) { + exceptionThrown = true; + } + Asserts.assertTrue(exceptionThrown, "ArrayStoreException must be thrown"); + } + + private static MethodHandle generateInvalidVastore() { + return MethodHandleBuilder.loadCode(MethodHandles.lookup(), + "Vastore", + MethodType.methodType(void.class, ValueCapableClass2.class), + CODE -> { + CODE. + iconst_1(). + anewarray(ValueType.forClass(ValueCapableClass1.class).valueClass()). + iconst_0(). + aload_0(). + vunbox(ValueType.forClass(ValueCapableClass2.class).valueClass()). + vastore(). + return_(); + } + ); + } + // ========== Test infrastructure ========== private static final WhiteBox WHITE_BOX = WhiteBox.getWhiteBox(); @@ -2235,7 +2439,7 @@ } public static void main(String[] args) throws Throwable { - //tests.values().removeIf(p -> !p.getName().equals("test74")); // Run single test + //tests.values().removeIf(p -> !p.getName().equals("test85")); // Run single test if (args.length == 0) { execute_vm("-XX:+IgnoreUnrecognizedVMOptions", "-XX:-BackgroundCompilation", "-XX:+PrintCompilation", "-XX:+PrintInlining", "-XX:+PrintIdeal", "-XX:+PrintOptoAssembly", --- old/src/share/vm/opto/graphKit.cpp 2017-06-16 14:34:20.801928416 +0200 +++ new/src/share/vm/opto/graphKit.cpp 2017-06-16 14:34:20.453928432 +0200 @@ -580,7 +580,7 @@ ex_obj = env()->ArrayIndexOutOfBoundsException_instance(); break; case Deoptimization::Reason_class_check: - if (java_bc() == Bytecodes::_aastore) { + if (java_bc() == Bytecodes::_aastore || java_bc() == Bytecodes::_vastore) { ex_obj = env()->ArrayStoreException_instance(); } else { ex_obj = env()->ClassCastException_instance(); @@ -2641,13 +2641,7 @@ Node* *casted_receiver) { const TypeKlassPtr* tklass = TypeKlassPtr::make(klass); Node* recv_klass = load_object_klass(receiver); - Node* want_klass = makecon(tklass); - Node* cmp = _gvn.transform( new CmpPNode(recv_klass, want_klass) ); - Node* bol = _gvn.transform( new BoolNode(cmp, BoolTest::eq) ); - IfNode* iff = create_and_xform_if(control(), bol, prob, COUNT_UNKNOWN); - set_control( _gvn.transform( new IfTrueNode (iff) )); - Node* fail = _gvn.transform( new IfFalseNode(iff) ); - + Node* fail = type_check(recv_klass, tklass, prob); const TypeOopPtr* recv_xtype = tklass->as_instance_type(); assert(recv_xtype->klass_is_exact(), ""); @@ -2660,6 +2654,18 @@ return fail; } +Node* GraphKit::type_check(Node* recv_klass, const TypeKlassPtr* tklass, + float prob) { + //const TypeKlassPtr* tklass = TypeKlassPtr::make(klass); + Node* want_klass = makecon(tklass); + Node* cmp = _gvn.transform( new CmpPNode(recv_klass, want_klass)); + Node* bol = _gvn.transform( new BoolNode(cmp, BoolTest::eq) ); + IfNode* iff = create_and_xform_if(control(), bol, prob, COUNT_UNKNOWN); + set_control( _gvn.transform( new IfTrueNode (iff))); + Node* fail = _gvn.transform( new IfFalseNode(iff)); + return fail; +} + //------------------------------seems_never_null------------------------------- // Use null_seen information if it is available from the profile. @@ -3397,7 +3403,7 @@ } //-------------------------------new_array------------------------------------- -// helper for newarray, anewarray and vnewarray +// helper for newarray and anewarray // The 'length' parameter is (obviously) the length of the array. // See comments on new_instance for the meaning of the other arguments. Node* GraphKit::new_array(Node* klass_node, // array klass (maybe variable) --- old/src/share/vm/interpreter/interpreterRuntime.cpp 2017-06-16 14:34:20.849928413 +0200 +++ new/src/share/vm/interpreter/interpreterRuntime.cpp 2017-06-16 14:34:20.525928428 +0200 @@ -396,6 +396,10 @@ IRT_ENTRY(void, InterpreterRuntime::value_array_store(JavaThread* thread, arrayOopDesc* array, int index, void* val)) Klass* klass = array->klass(); assert(klass->is_valueArray_klass() || klass->is_objArray_klass(), "expected value or object array oop"); + + if (ArrayKlass::cast(klass)->element_klass() != ((oop)val)->klass()) { + THROW(vmSymbols::java_lang_ArrayStoreException()); + } if (klass->is_objArray_klass()) { ((objArrayOop) array)->obj_at_put(index, (oop)val); }