--- old/src/hotspot/share/gc/shared/barrierSet.cpp 2019-08-29 16:29:05.238559238 +0200 +++ new/src/hotspot/share/gc/shared/barrierSet.cpp 2019-08-29 16:29:04.878552935 +0200 @@ -25,6 +25,8 @@ #include "precompiled.hpp" #include "gc/shared/barrierSet.hpp" #include "gc/shared/barrierSetAssembler.hpp" +#include "memory/resourceArea.hpp" +#include "oops/objArrayKlass.inline.hpp" #include "runtime/thread.hpp" #include "utilities/debug.hpp" #include "utilities/macros.hpp" @@ -49,6 +51,32 @@ _barrier_set->on_thread_create(Thread::current()); } +void BarrierSet::throw_array_null_pointer_store_exception(arrayOop src, arrayOop dst, TRAPS) { + Klass* bound = ObjArrayKlass::cast(dst->klass())->element_klass(); + stringStream ss; + ss.print("arraycopy: can not copy null values into %s[]", + bound->external_name()); + THROW_MSG(vmSymbols::java_lang_NullPointerException(), ss.as_string()); +} + +void BarrierSet::throw_array_store_exception(arrayOop src, arrayOop dst, TRAPS) { + ResourceMark rm(THREAD); + Klass* bound = ObjArrayKlass::cast(dst->klass())->element_klass(); + Klass* stype = ObjArrayKlass::cast(src->klass())->element_klass(); + stringStream ss; + if (!bound->is_subtype_of(stype)) { + ss.print("arraycopy: type mismatch: can not copy %s[] into %s[]", + stype->external_name(), bound->external_name()); + } else { + // oop_arraycopy should return the index in the source array that + // contains the problematic oop. + ss.print("arraycopy: element type mismatch: can not cast one of the elements" + " of %s[] to the type of the destination array, %s", + stype->external_name(), bound->external_name()); + } + THROW_MSG(vmSymbols::java_lang_ArrayStoreException(), ss.as_string()); +} + // Called from init.cpp void gc_barrier_stubs_init() { BarrierSet* bs = BarrierSet::barrier_set(); --- old/src/hotspot/share/gc/shared/barrierSet.hpp 2019-08-29 16:29:05.974572126 +0200 +++ new/src/hotspot/share/gc/shared/barrierSet.hpp 2019-08-29 16:29:05.618565892 +0200 @@ -30,6 +30,7 @@ #include "oops/access.hpp" #include "oops/accessBackend.hpp" #include "oops/oopsHierarchy.hpp" +#include "utilities/exceptions.hpp" #include "utilities/fakeRttiSupport.hpp" #include "utilities/macros.hpp" @@ -121,6 +122,9 @@ return COMPILER2_PRESENT(new BarrierSetC2T()) NOT_COMPILER2(NULL); } + static void throw_array_null_pointer_store_exception(arrayOop src, arrayOop dst, TRAPS); + static void throw_array_store_exception(arrayOop src, arrayOop dst, TRAPS); + public: // Support for optimizing compilers to call the barrier set on slow path allocations // that did not enter a TLAB. Used for e.g. ReduceInitialCardMarks. @@ -283,7 +287,7 @@ } template - static bool oop_arraycopy_in_heap(arrayOop src_obj, size_t src_offset_in_bytes, T* src_raw, + static void oop_arraycopy_in_heap(arrayOop src_obj, size_t src_offset_in_bytes, T* src_raw, arrayOop dst_obj, size_t dst_offset_in_bytes, T* dst_raw, size_t length); --- old/src/hotspot/share/gc/shared/barrierSet.inline.hpp 2019-08-29 16:29:06.698584804 +0200 +++ new/src/hotspot/share/gc/shared/barrierSet.inline.hpp 2019-08-29 16:29:06.310578009 +0200 @@ -33,28 +33,34 @@ template template -inline bool BarrierSet::AccessBarrier::oop_arraycopy_in_heap(arrayOop src_obj, size_t src_offset_in_bytes, T* src_raw, +inline void BarrierSet::AccessBarrier::oop_arraycopy_in_heap(arrayOop src_obj, size_t src_offset_in_bytes, T* src_raw, arrayOop dst_obj, size_t dst_offset_in_bytes, T* dst_raw, size_t length) { T* src = arrayOopDesc::obj_offset_to_raw(src_obj, src_offset_in_bytes, src_raw); T* dst = arrayOopDesc::obj_offset_to_raw(dst_obj, dst_offset_in_bytes, dst_raw); - if (!HasDecorator::value) { + if ((!HasDecorator::value) && + (!HasDecorator::value)) { // Covariant, copy without checks - return Raw::oop_arraycopy(NULL, 0, src, NULL, 0, dst, length); + Raw::oop_arraycopy(NULL, 0, src, NULL, 0, dst, length); + return; } // Copy each element with checking casts Klass* const dst_klass = objArrayOop(dst_obj)->element_klass(); for (const T* const end = src + length; src < end; src++, dst++) { const T elem = *src; - if (!oopDesc::is_instanceof_or_null(CompressedOops::decode(elem), dst_klass)) { - return false; + if (HasDecorator::value && CompressedOops::is_null(elem)) { + throw_array_null_pointer_store_exception(src_obj, dst_obj, Thread::current()); + return; + } + if (HasDecorator::value && + (!oopDesc::is_instanceof_or_null(CompressedOops::decode(elem), dst_klass))) { + throw_array_store_exception(src_obj, dst_obj, Thread::current()); + return; } *dst = elem; } - - return true; } #endif // SHARE_GC_SHARED_BARRIERSET_INLINE_HPP --- old/src/hotspot/share/gc/shared/modRefBarrierSet.hpp 2019-08-29 16:29:07.478598461 +0200 +++ new/src/hotspot/share/gc/shared/modRefBarrierSet.hpp 2019-08-29 16:29:07.082591527 +0200 @@ -68,7 +68,6 @@ protected: virtual void write_ref_array_work(MemRegion mr) = 0; - public: // The ModRef abstraction introduces pre and post barriers template @@ -84,10 +83,14 @@ static oop oop_atomic_xchg_in_heap(oop new_value, T* addr); template - static bool oop_arraycopy_in_heap(arrayOop src_obj, size_t src_offset_in_bytes, T* src_raw, + static void oop_arraycopy_in_heap(arrayOop src_obj, size_t src_offset_in_bytes, T* src_raw, arrayOop dst_obj, size_t dst_offset_in_bytes, T* dst_raw, size_t length); - + private: + // Failing checkcast or check null during copy, still needs barrier + template + static inline void oop_arraycopy_partial_barrier(BarrierSetT *bs, T* dst_raw, T* p); + public: static void clone_in_heap(oop src, oop dst, size_t size); static void oop_store_in_heap_at(oop base, ptrdiff_t offset, oop value) { --- old/src/hotspot/share/gc/shared/modRefBarrierSet.inline.hpp 2019-08-29 16:29:08.262612190 +0200 +++ new/src/hotspot/share/gc/shared/modRefBarrierSet.inline.hpp 2019-08-29 16:29:07.878605466 +0200 @@ -90,7 +90,18 @@ template template -inline bool ModRefBarrierSet::AccessBarrier:: +inline void ModRefBarrierSet::AccessBarrier:: +oop_arraycopy_partial_barrier(BarrierSetT *bs, T* dst_raw, T* p) { + const size_t pd = pointer_delta(p, dst_raw, (size_t)heapOopSize); + // pointer delta is scaled to number of elements (length field in + // objArrayOop) which we assume is 32 bit. + assert(pd == (size_t)(int)pd, "length field overflow"); + bs->write_ref_array((HeapWord*)dst_raw, pd); +} + +template +template +inline void ModRefBarrierSet::AccessBarrier:: oop_arraycopy_in_heap(arrayOop src_obj, size_t src_offset_in_bytes, T* src_raw, arrayOop dst_obj, size_t dst_offset_in_bytes, T* dst_raw, size_t length) { @@ -99,7 +110,8 @@ src_raw = arrayOopDesc::obj_offset_to_raw(src_obj, src_offset_in_bytes, src_raw); dst_raw = arrayOopDesc::obj_offset_to_raw(dst_obj, dst_offset_in_bytes, dst_raw); - if (!HasDecorator::value) { + if ((!HasDecorator::value) && + (!HasDecorator::value)) { // Optimized covariant case bs->write_ref_array_pre(dst_raw, length, HasDecorator::value); @@ -112,22 +124,24 @@ T* end = from + length; for (T* p = dst_raw; from < end; from++, p++) { T element = *from; - if (oopDesc::is_instanceof_or_null(CompressedOops::decode(element), bound)) { - bs->template write_ref_field_pre(p); - *p = element; - } else { - // We must do a barrier to cover the partial copy. - const size_t pd = pointer_delta(p, dst_raw, (size_t)heapOopSize); - // pointer delta is scaled to number of elements (length field in - // objArrayOop) which we assume is 32 bit. - assert(pd == (size_t)(int)pd, "length field overflow"); - bs->write_ref_array((HeapWord*)dst_raw, pd); - return false; + // Apply any required checks + if (HasDecorator::value && CompressedOops::is_null(element)) { + oop_arraycopy_partial_barrier(bs, dst_raw, p); + throw_array_null_pointer_store_exception(src_obj, dst_obj, Thread::current()); + return; + } + if (HasDecorator::value && + (!oopDesc::is_instanceof_or_null(CompressedOops::decode(element), bound))) { + oop_arraycopy_partial_barrier(bs, dst_raw, p); + throw_array_store_exception(src_obj, dst_obj, Thread::current()); + return; } + // write + bs->template write_ref_field_pre(p); + *p = element; } bs->write_ref_array((HeapWord*)dst_raw, length); } - return true; } template --- old/src/hotspot/share/gc/z/zBarrierSet.hpp 2019-08-29 16:29:09.314630610 +0200 +++ new/src/hotspot/share/gc/z/zBarrierSet.hpp 2019-08-29 16:29:08.950624237 +0200 @@ -78,7 +78,7 @@ static oop oop_atomic_xchg_in_heap_at(oop new_value, oop base, ptrdiff_t offset); template - static bool oop_arraycopy_in_heap(arrayOop src_obj, size_t src_offset_in_bytes, T* src_raw, + static void oop_arraycopy_in_heap(arrayOop src_obj, size_t src_offset_in_bytes, T* src_raw, arrayOop dst_obj, size_t dst_offset_in_bytes, T* dst_raw, size_t length); --- old/src/hotspot/share/gc/z/zBarrierSet.inline.hpp 2019-08-29 16:29:10.338648541 +0200 +++ new/src/hotspot/share/gc/z/zBarrierSet.inline.hpp 2019-08-29 16:29:09.978642237 +0200 @@ -170,32 +170,38 @@ template template -inline bool ZBarrierSet::AccessBarrier::oop_arraycopy_in_heap(arrayOop src_obj, size_t src_offset_in_bytes, T* src_raw, +inline void ZBarrierSet::AccessBarrier::oop_arraycopy_in_heap(arrayOop src_obj, size_t src_offset_in_bytes, T* src_raw, arrayOop dst_obj, size_t dst_offset_in_bytes, T* dst_raw, size_t length) { T* src = arrayOopDesc::obj_offset_to_raw(src_obj, src_offset_in_bytes, src_raw); T* dst = arrayOopDesc::obj_offset_to_raw(dst_obj, dst_offset_in_bytes, dst_raw); - if (!HasDecorator::value) { + if ((!HasDecorator::value) && + (!HasDecorator::value)) { // No check cast, bulk barrier and bulk copy ZBarrier::load_barrier_on_oop_array(src, length); - return Raw::oop_arraycopy_in_heap(NULL, 0, src, NULL, 0, dst, length); + Raw::oop_arraycopy_in_heap(NULL, 0, src, NULL, 0, dst, length); + return; } // Check cast and copy each elements Klass* const dst_klass = objArrayOop(dst_obj)->element_klass(); for (const T* const end = src + length; src < end; src++, dst++) { const oop elem = ZBarrier::load_barrier_on_oop_field(src); - if (!oopDesc::is_instanceof_or_null(elem, dst_klass)) { + if (HasDecorator::value && elem == NULL) { + throw_array_null_pointer_store_exception(src_obj, dst_obj, Thread::current()); + return; + } + if (HasDecorator::value && + (!oopDesc::is_instanceof_or_null(elem, dst_klass))) { // Check cast failed - return false; + throw_array_store_exception(src_obj, dst_obj, Thread::current()); + return; } // Cast is safe, since we know it's never a narrowOop *(oop*)dst = elem; } - - return true; } template --- old/src/hotspot/share/oops/access.hpp 2019-08-29 16:29:11.066661288 +0200 +++ new/src/hotspot/share/oops/access.hpp 2019-08-29 16:29:10.706654984 +0200 @@ -131,14 +131,14 @@ protected: template - static inline bool oop_arraycopy(arrayOop src_obj, size_t src_offset_in_bytes, const T* src_raw, + static inline void oop_arraycopy(arrayOop src_obj, size_t src_offset_in_bytes, const T* src_raw, arrayOop dst_obj, size_t dst_offset_in_bytes, T* dst_raw, size_t length) { verify_decorators(); - return AccessInternal::arraycopy(src_obj, src_offset_in_bytes, src_raw, - dst_obj, dst_offset_in_bytes, dst_raw, - length); + AccessInternal::arraycopy(src_obj, src_offset_in_bytes, src_raw, + dst_obj, dst_offset_in_bytes, dst_raw, + length); } template @@ -329,19 +329,19 @@ length); } - static inline bool oop_arraycopy(arrayOop src_obj, size_t src_offset_in_bytes, + static inline void oop_arraycopy(arrayOop src_obj, size_t src_offset_in_bytes, arrayOop dst_obj, size_t dst_offset_in_bytes, size_t length) { - return AccessT::oop_arraycopy(src_obj, src_offset_in_bytes, reinterpret_cast(NULL), - dst_obj, dst_offset_in_bytes, reinterpret_cast(NULL), - length); + AccessT::oop_arraycopy(src_obj, src_offset_in_bytes, reinterpret_cast(NULL), + dst_obj, dst_offset_in_bytes, reinterpret_cast(NULL), + length); } template - static inline bool oop_arraycopy_raw(T* src, T* dst, size_t length) { - return AccessT::oop_arraycopy(NULL, 0, src, - NULL, 0, dst, - length); + static inline void oop_arraycopy_raw(T* src, T* dst, size_t length) { + AccessT::oop_arraycopy(NULL, 0, src, + NULL, 0, dst, + length); } }; --- old/src/hotspot/share/oops/access.inline.hpp 2019-08-29 16:29:11.834674736 +0200 +++ new/src/hotspot/share/oops/access.inline.hpp 2019-08-29 16:29:11.438667802 +0200 @@ -124,23 +124,22 @@ template struct PostRuntimeDispatch: public AllStatic { template - static bool access_barrier(arrayOop src_obj, size_t src_offset_in_bytes, T* src_raw, + static void access_barrier(arrayOop src_obj, size_t src_offset_in_bytes, T* src_raw, arrayOop dst_obj, size_t dst_offset_in_bytes, T* dst_raw, size_t length) { GCBarrierType::arraycopy_in_heap(src_obj, src_offset_in_bytes, src_raw, dst_obj, dst_offset_in_bytes, dst_raw, length); - return true; } template - static bool oop_access_barrier(arrayOop src_obj, size_t src_offset_in_bytes, T* src_raw, + static void oop_access_barrier(arrayOop src_obj, size_t src_offset_in_bytes, T* src_raw, arrayOop dst_obj, size_t dst_offset_in_bytes, T* dst_raw, size_t length) { typedef typename HeapOopType::type OopType; - return GCBarrierType::oop_arraycopy_in_heap(src_obj, src_offset_in_bytes, reinterpret_cast(src_raw), - dst_obj, dst_offset_in_bytes, reinterpret_cast(dst_raw), - length); + GCBarrierType::oop_arraycopy_in_heap(src_obj, src_offset_in_bytes, reinterpret_cast(src_raw), + dst_obj, dst_offset_in_bytes, reinterpret_cast(dst_raw), + length); } }; @@ -344,14 +343,14 @@ } template - bool RuntimeDispatch::arraycopy_init(arrayOop src_obj, size_t src_offset_in_bytes, T* src_raw, + void RuntimeDispatch::arraycopy_init(arrayOop src_obj, size_t src_offset_in_bytes, T* src_raw, arrayOop dst_obj, size_t dst_offset_in_bytes, T* dst_raw, size_t length) { func_t function = BarrierResolver::resolve_barrier(); _arraycopy_func = function; - return function(src_obj, src_offset_in_bytes, src_raw, - dst_obj, dst_offset_in_bytes, dst_raw, - length); + function(src_obj, src_offset_in_bytes, src_raw, + dst_obj, dst_offset_in_bytes, dst_raw, + length); } template --- old/src/hotspot/share/oops/accessBackend.hpp 2019-08-29 16:29:12.538687063 +0200 +++ new/src/hotspot/share/oops/accessBackend.hpp 2019-08-29 16:29:12.182680830 +0200 @@ -110,7 +110,7 @@ typedef T (*atomic_cmpxchg_func_t)(T new_value, void* addr, T compare_value); typedef T (*atomic_xchg_func_t)(T new_value, void* addr); - typedef bool (*arraycopy_func_t)(arrayOop src_obj, size_t src_offset_in_bytes, T* src_raw, + typedef void (*arraycopy_func_t)(arrayOop src_obj, size_t src_offset_in_bytes, T* src_raw, arrayOop dst_obj, size_t dst_offset_in_bytes, T* dst_raw, size_t length); typedef void (*clone_func_t)(oop src, oop dst, size_t size); @@ -120,7 +120,7 @@ template struct AccessFunctionTypes { - typedef bool (*arraycopy_func_t)(arrayOop src_obj, size_t src_offset_in_bytes, void* src, + typedef void (*arraycopy_func_t)(arrayOop src_obj, size_t src_offset_in_bytes, void* src, arrayOop dst_obj, size_t dst_offset_in_bytes, void* dst, size_t length); }; @@ -357,7 +357,7 @@ } template - static bool arraycopy(arrayOop src_obj, size_t src_offset_in_bytes, T* src_raw, + static void arraycopy(arrayOop src_obj, size_t src_offset_in_bytes, T* src_raw, arrayOop dst_obj, size_t dst_offset_in_bytes, T* dst_raw, size_t length); @@ -402,7 +402,7 @@ } template - static bool oop_arraycopy(arrayOop src_obj, size_t src_offset_in_bytes, T* src_raw, + static void oop_arraycopy(arrayOop src_obj, size_t src_offset_in_bytes, T* src_raw, arrayOop dst_obj, size_t dst_offset_in_bytes, T* dst_raw, size_t length); @@ -567,11 +567,11 @@ typedef typename AccessFunction::type func_t; static func_t _arraycopy_func; - static bool arraycopy_init(arrayOop src_obj, size_t src_offset_in_bytes, T* src_raw, + static void arraycopy_init(arrayOop src_obj, size_t src_offset_in_bytes, T* src_raw, arrayOop dst_obj, size_t dst_offset_in_bytes, T* dst_raw, size_t length); - static inline bool arraycopy(arrayOop src_obj, size_t src_offset_in_bytes, T* src_raw, + static inline void arraycopy(arrayOop src_obj, size_t src_offset_in_bytes, T* src_raw, arrayOop dst_obj, size_t dst_offset_in_bytes, T* dst_raw, size_t length) { return _arraycopy_func(src_obj, src_offset_in_bytes, src_raw, @@ -913,56 +913,56 @@ template inline static typename EnableIf< - HasDecorator::value && CanHardwireRaw::value, bool>::type + HasDecorator::value && CanHardwireRaw::value, void>::type arraycopy(arrayOop src_obj, size_t src_offset_in_bytes, T* src_raw, arrayOop dst_obj, size_t dst_offset_in_bytes, T* dst_raw, size_t length) { typedef RawAccessBarrier Raw; if (HasDecorator::value) { - return Raw::oop_arraycopy(src_obj, src_offset_in_bytes, src_raw, - dst_obj, dst_offset_in_bytes, dst_raw, - length); + Raw::oop_arraycopy(src_obj, src_offset_in_bytes, src_raw, + dst_obj, dst_offset_in_bytes, dst_raw, + length); } else { - return Raw::arraycopy(src_obj, src_offset_in_bytes, src_raw, - dst_obj, dst_offset_in_bytes, dst_raw, - length); + Raw::arraycopy(src_obj, src_offset_in_bytes, src_raw, + dst_obj, dst_offset_in_bytes, dst_raw, + length); } } template inline static typename EnableIf< - HasDecorator::value && !CanHardwireRaw::value, bool>::type + HasDecorator::value && !CanHardwireRaw::value, void>::type arraycopy(arrayOop src_obj, size_t src_offset_in_bytes, T* src_raw, arrayOop dst_obj, size_t dst_offset_in_bytes, T* dst_raw, size_t length) { if (UseCompressedOops) { const DecoratorSet expanded_decorators = decorators | convert_compressed_oops; - return PreRuntimeDispatch::arraycopy(src_obj, src_offset_in_bytes, src_raw, - dst_obj, dst_offset_in_bytes, dst_raw, - length); + PreRuntimeDispatch::arraycopy(src_obj, src_offset_in_bytes, src_raw, + dst_obj, dst_offset_in_bytes, dst_raw, + length); } else { const DecoratorSet expanded_decorators = decorators & ~convert_compressed_oops; - return PreRuntimeDispatch::arraycopy(src_obj, src_offset_in_bytes, src_raw, - dst_obj, dst_offset_in_bytes, dst_raw, - length); + PreRuntimeDispatch::arraycopy(src_obj, src_offset_in_bytes, src_raw, + dst_obj, dst_offset_in_bytes, dst_raw, + length); } } template inline static typename EnableIf< - !HasDecorator::value, bool>::type + !HasDecorator::value, void>::type arraycopy(arrayOop src_obj, size_t src_offset_in_bytes, T* src_raw, arrayOop dst_obj, size_t dst_offset_in_bytes, T* dst_raw, size_t length) { if (is_hardwired_primitive()) { const DecoratorSet expanded_decorators = decorators | AS_RAW; - return PreRuntimeDispatch::arraycopy(src_obj, src_offset_in_bytes, src_raw, - dst_obj, dst_offset_in_bytes, dst_raw, - length); + PreRuntimeDispatch::arraycopy(src_obj, src_offset_in_bytes, src_raw, + dst_obj, dst_offset_in_bytes, dst_raw, + length); } else { - return RuntimeDispatch::arraycopy(src_obj, src_offset_in_bytes, src_raw, - dst_obj, dst_offset_in_bytes, dst_raw, - length); + RuntimeDispatch::arraycopy(src_obj, src_offset_in_bytes, src_raw, + dst_obj, dst_offset_in_bytes, dst_raw, + length); } } @@ -1124,33 +1124,33 @@ } template - inline bool arraycopy_reduce_types(arrayOop src_obj, size_t src_offset_in_bytes, T* src_raw, + inline void arraycopy_reduce_types(arrayOop src_obj, size_t src_offset_in_bytes, T* src_raw, arrayOop dst_obj, size_t dst_offset_in_bytes, T* dst_raw, size_t length) { - return PreRuntimeDispatch::arraycopy(src_obj, src_offset_in_bytes, src_raw, - dst_obj, dst_offset_in_bytes, dst_raw, - length); + PreRuntimeDispatch::arraycopy(src_obj, src_offset_in_bytes, src_raw, + dst_obj, dst_offset_in_bytes, dst_raw, + length); } template - inline bool arraycopy_reduce_types(arrayOop src_obj, size_t src_offset_in_bytes, HeapWord* src_raw, + inline void arraycopy_reduce_types(arrayOop src_obj, size_t src_offset_in_bytes, HeapWord* src_raw, arrayOop dst_obj, size_t dst_offset_in_bytes, HeapWord* dst_raw, size_t length) { const DecoratorSet expanded_decorators = decorators | INTERNAL_CONVERT_COMPRESSED_OOP; - return PreRuntimeDispatch::arraycopy(src_obj, src_offset_in_bytes, src_raw, - dst_obj, dst_offset_in_bytes, dst_raw, - length); + PreRuntimeDispatch::arraycopy(src_obj, src_offset_in_bytes, src_raw, + dst_obj, dst_offset_in_bytes, dst_raw, + length); } template - inline bool arraycopy_reduce_types(arrayOop src_obj, size_t src_offset_in_bytes, narrowOop* src_raw, + inline void arraycopy_reduce_types(arrayOop src_obj, size_t src_offset_in_bytes, narrowOop* src_raw, arrayOop dst_obj, size_t dst_offset_in_bytes, narrowOop* dst_raw, size_t length) { const DecoratorSet expanded_decorators = decorators | INTERNAL_CONVERT_COMPRESSED_OOP | INTERNAL_RT_USE_COMPRESSED_OOPS; - return PreRuntimeDispatch::arraycopy(src_obj, src_offset_in_bytes, src_raw, - dst_obj, dst_offset_in_bytes, dst_raw, - length); + PreRuntimeDispatch::arraycopy(src_obj, src_offset_in_bytes, src_raw, + dst_obj, dst_offset_in_bytes, dst_raw, + length); } // Step 1: Set default decorators. This step remembers if a type was volatile @@ -1283,7 +1283,7 @@ } template - inline bool arraycopy(arrayOop src_obj, size_t src_offset_in_bytes, const T* src_raw, + inline void arraycopy(arrayOop src_obj, size_t src_offset_in_bytes, const T* src_raw, arrayOop dst_obj, size_t dst_offset_in_bytes, T* dst_raw, size_t length) { STATIC_ASSERT((HasDecorator::value || @@ -1291,9 +1291,9 @@ IsFloatingPoint::value)); // arraycopy allows type erased void elements typedef typename Decay::type DecayedT; const DecoratorSet expanded_decorators = DecoratorFixup::value; - return arraycopy_reduce_types(src_obj, src_offset_in_bytes, const_cast(src_raw), - dst_obj, dst_offset_in_bytes, const_cast(dst_raw), - length); + arraycopy_reduce_types(src_obj, src_offset_in_bytes, const_cast(src_raw), + dst_obj, dst_offset_in_bytes, const_cast(dst_raw), + length); } template --- old/src/hotspot/share/oops/accessBackend.inline.hpp 2019-08-29 16:29:13.346701211 +0200 +++ new/src/hotspot/share/oops/accessBackend.inline.hpp 2019-08-29 16:29:12.950694277 +0200 @@ -118,12 +118,12 @@ template template -inline bool RawAccessBarrier::oop_arraycopy(arrayOop src_obj, size_t src_offset_in_bytes, T* src_raw, +inline void RawAccessBarrier::oop_arraycopy(arrayOop src_obj, size_t src_offset_in_bytes, T* src_raw, arrayOop dst_obj, size_t dst_offset_in_bytes, T* dst_raw, size_t length) { - return arraycopy(src_obj, src_offset_in_bytes, src_raw, - dst_obj, dst_offset_in_bytes, dst_raw, - length); + arraycopy(src_obj, src_offset_in_bytes, src_raw, + dst_obj, dst_offset_in_bytes, dst_raw, + length); } template @@ -334,13 +334,12 @@ template template -inline bool RawAccessBarrier::arraycopy(arrayOop src_obj, size_t src_offset_in_bytes, T* src_raw, +inline void RawAccessBarrier::arraycopy(arrayOop src_obj, size_t src_offset_in_bytes, T* src_raw, arrayOop dst_obj, size_t dst_offset_in_bytes, T* dst_raw, size_t length) { RawAccessBarrierArrayCopy::arraycopy(src_obj, src_offset_in_bytes, src_raw, dst_obj, dst_offset_in_bytes, dst_raw, length); - return true; } template --- old/src/hotspot/share/oops/accessDecorators.hpp 2019-08-29 16:29:14.146715219 +0200 +++ new/src/hotspot/share/oops/accessDecorators.hpp 2019-08-29 16:29:13.758708425 +0200 @@ -200,29 +200,33 @@ // are not guaranteed to be subclasses of the class of the destination array. This requires // a check-cast barrier during the copying operation. If this is not set, it is assumed // that the array is covariant: (the source array type is-a destination array type) +// * ARRAYCOPY_NOTNULL: This property means that the source array may contain null elements +// but the destination does not allow null elements (i.e. throw NPE) // * ARRAYCOPY_DISJOINT: This property means that it is known that the two array ranges // are disjoint. // * ARRAYCOPY_ARRAYOF: The copy is in the arrayof form. // * ARRAYCOPY_ATOMIC: The accesses have to be atomic over the size of its elements. // * ARRAYCOPY_ALIGNED: The accesses have to be aligned on a HeapWord. const DecoratorSet ARRAYCOPY_CHECKCAST = UCONST64(1) << 24; -const DecoratorSet ARRAYCOPY_DISJOINT = UCONST64(1) << 25; -const DecoratorSet ARRAYCOPY_ARRAYOF = UCONST64(1) << 26; -const DecoratorSet ARRAYCOPY_ATOMIC = UCONST64(1) << 27; -const DecoratorSet ARRAYCOPY_ALIGNED = UCONST64(1) << 28; -const DecoratorSet ARRAYCOPY_DECORATOR_MASK = ARRAYCOPY_CHECKCAST | ARRAYCOPY_DISJOINT | - ARRAYCOPY_DISJOINT | ARRAYCOPY_ARRAYOF | - ARRAYCOPY_ATOMIC | ARRAYCOPY_ALIGNED; +const DecoratorSet ARRAYCOPY_NOTNULL = UCONST64(1) << 25; +const DecoratorSet ARRAYCOPY_DISJOINT = UCONST64(1) << 26; +const DecoratorSet ARRAYCOPY_ARRAYOF = UCONST64(1) << 27; +const DecoratorSet ARRAYCOPY_ATOMIC = UCONST64(1) << 28; +const DecoratorSet ARRAYCOPY_ALIGNED = UCONST64(1) << 29; +const DecoratorSet ARRAYCOPY_DECORATOR_MASK = ARRAYCOPY_CHECKCAST | ARRAYCOPY_NOTNULL | + ARRAYCOPY_DISJOINT | ARRAYCOPY_DISJOINT | + ARRAYCOPY_ARRAYOF | ARRAYCOPY_ATOMIC | + ARRAYCOPY_ALIGNED; // == Resolve barrier decorators == // * ACCESS_READ: Indicate that the resolved object is accessed read-only. This allows the GC // backend to use weaker and more efficient barriers. // * ACCESS_WRITE: Indicate that the resolved object is used for write access. -const DecoratorSet ACCESS_READ = UCONST64(1) << 29; -const DecoratorSet ACCESS_WRITE = UCONST64(1) << 30; +const DecoratorSet ACCESS_READ = UCONST64(1) << 30; +const DecoratorSet ACCESS_WRITE = UCONST64(1) << 31; // Keep track of the last decorator. -const DecoratorSet DECORATOR_LAST = UCONST64(1) << 30; +const DecoratorSet DECORATOR_LAST = UCONST64(1) << 31; namespace AccessInternal { // This class adds implied decorators that follow according to decorator rules. --- old/src/hotspot/share/oops/objArrayKlass.cpp 2019-08-29 16:29:15.050731048 +0200 +++ new/src/hotspot/share/oops/objArrayKlass.cpp 2019-08-29 16:29:14.674724464 +0200 @@ -233,26 +233,20 @@ // We have to make sure all elements conform to the destination array Klass* bound = ObjArrayKlass::cast(d->klass())->element_klass(); Klass* stype = ObjArrayKlass::cast(s->klass())->element_klass(); + // Perform null check if dst is null-free but src has no such guarantee + bool null_check = ((!ArrayKlass::cast(s->klass())->storage_properties().is_null_free()) && + ArrayKlass::cast(d->klass())->storage_properties().is_null_free()); if (stype == bound || stype->is_subtype_of(bound)) { - // elements are guaranteed to be subtypes, so no check necessary - ArrayAccess::oop_arraycopy(s, src_offset, d, dst_offset, length); + if (null_check) { + ArrayAccess::oop_arraycopy(s, src_offset, d, dst_offset, length); + } else { + ArrayAccess::oop_arraycopy(s, src_offset, d, dst_offset, length); + } } else { - // slow case: need individual subtype checks - // note: don't use obj_at_put below because it includes a redundant store check - if (!ArrayAccess::oop_arraycopy(s, src_offset, d, dst_offset, length)) { - ResourceMark rm(THREAD); - stringStream ss; - if (!bound->is_subtype_of(stype)) { - ss.print("arraycopy: type mismatch: can not copy %s[] into %s[]", - stype->external_name(), bound->external_name()); - } else { - // oop_arraycopy should return the index in the source array that - // contains the problematic oop. - ss.print("arraycopy: element type mismatch: can not cast one of the elements" - " of %s[] to the type of the destination array, %s", - stype->external_name(), bound->external_name()); - } - THROW_MSG(vmSymbols::java_lang_ArrayStoreException(), ss.as_string()); + if (null_check) { + ArrayAccess::oop_arraycopy(s, src_offset, d, dst_offset, length); + } else { + ArrayAccess::oop_arraycopy(s, src_offset, d, dst_offset, length); } } } @@ -320,28 +314,7 @@ if (length==0) { return; } - if (EnableValhalla && ArrayKlass::cast(d->klass())->element_klass()->is_value()) { - assert(d->is_objArray(), "Expected objArray"); - ValueKlass* d_elem_vklass = ValueKlass::cast(ArrayKlass::cast(d->klass())->element_klass()); - objArrayOop da = objArrayOop(d); - objArrayOop sa = objArrayOop(s); - int src_end = src_pos + length; - bool null_free = ArrayKlass::cast(s->klass())->storage_properties().is_null_free() || - ArrayKlass::cast(d->klass())->storage_properties().is_null_free(); - while (src_pos < src_end) { - oop se = sa->obj_at(src_pos); - if (null_free && se == NULL) { - THROW(vmSymbols::java_lang_NullPointerException()); - } - // Check exact type per element - if (se != NULL && se->klass() != d_elem_vklass) { - THROW(vmSymbols::java_lang_ArrayStoreException()); - } - da->obj_at_put(dst_pos, se); // TODO: review with ValueArrayKlass::copy_array and Access API - dst_pos++; - src_pos++; - } - } else if (UseCompressedOops) { + if (UseCompressedOops) { size_t src_offset = (size_t) objArrayOopDesc::obj_at_offset(src_pos); size_t dst_offset = (size_t) objArrayOopDesc::obj_at_offset(dst_pos); assert(arrayOopDesc::obj_offset_to_raw(s, src_offset, NULL) == --- old/src/hotspot/share/oops/valueArrayKlass.cpp 2019-08-29 16:29:15.830744705 +0200 +++ new/src/hotspot/share/oops/valueArrayKlass.cpp 2019-08-29 16:29:15.434737772 +0200 @@ -184,6 +184,12 @@ return element_klass()->protection_domain(); } +// Temp hack having this here: need to move towards Access API +static bool needs_backwards_copy(arrayOop s, int src_pos, + arrayOop d, int dst_pos, int length) { + return oopDesc::equals(s, d) && (dst_pos > src_pos) && (dst_pos - src_pos) < length; +} + void ValueArrayKlass::copy_array(arrayOop s, int src_pos, arrayOop d, int dst_pos, int length, TRAPS) { @@ -236,10 +242,21 @@ if (contains_oops()) { int elem_incr = 1 << log2_element_size(); address src_end = src + (length << log2_element_size()); - while (src < src_end) { - s_elem_vklass->value_store(src, dst, element_byte_size(), true, false); - src += elem_incr; - dst += elem_incr; + if (needs_backwards_copy(s, src_pos, d, dst_pos, length)) { + swap(src, src_end); + dst = dst + (length << log2_element_size()); + do { + src -= elem_incr; + dst -= elem_incr; + s_elem_vklass->value_store(src, dst, element_byte_size(), true, false); + } while (src > src_end); + } else { + address src_end = src + (length << log2_element_size()); + while (src < src_end) { + s_elem_vklass->value_store(src, dst, element_byte_size(), true, false); + src += elem_incr; + dst += elem_incr; + } } } else { // we are basically a type array...don't bother limiting element copy --- old/test/hotspot/jtreg/runtime/valhalla/valuetypes/Long8Value.java 2019-08-29 16:29:16.598758152 +0200 +++ new/test/hotspot/jtreg/runtime/valhalla/valuetypes/Long8Value.java 2019-08-29 16:29:16.206751289 +0200 @@ -36,15 +36,15 @@ final long longField7; final long longField8; - private Long8Value() { - longField1 = 0; - longField2 = 0; - longField3 = 0; - longField4 = 0; - longField5 = 0; - longField6 = 0; - longField7 = 0; - longField8 = 0; + private Long8Value(long l1, long l2, long l3, long l4, long l5, long l6, long l7, long l8) { + longField1 = l1; + longField2 = l2; + longField3 = l3; + longField4 = l4; + longField5 = l5; + longField6 = l6; + longField7 = l7; + longField8 = l8; } public long getLongField1() { return longField1; } @@ -64,16 +64,7 @@ long long6, long long7, long long8) { - Long8Value l8v = Long8Value.default; - l8v = __WithField(l8v.longField1, long1); - l8v = __WithField(l8v.longField2, long2); - l8v = __WithField(l8v.longField3, long3); - l8v = __WithField(l8v.longField4, long4); - l8v = __WithField(l8v.longField5, long5); - l8v = __WithField(l8v.longField6, long6); - l8v = __WithField(l8v.longField7, long7); - l8v = __WithField(l8v.longField8, long8); - return l8v; + return new Long8Value(long1, long2, long3, long4, long5, long6, long7, long8); } static void check(Long8Value value, --- old/test/hotspot/jtreg/runtime/valhalla/valuetypes/Person.java 2019-08-29 16:29:17.330770970 +0200 +++ new/test/hotspot/jtreg/runtime/valhalla/valuetypes/Person.java 2019-08-29 16:29:16.970764666 +0200 @@ -29,10 +29,10 @@ final String firstName; final String lastName; - private Person() { - id = 0; - firstName = null; - lastName = null; + private Person(int id, String firstName, String lastName) { + this.id = id; + this.firstName = firstName; + this.lastName = lastName; } public int getId() { return id; } @@ -44,10 +44,6 @@ } static Person create(int id, String firstName, String lastName) { - Person p = Person.default; - p = __WithField(p.id, id); - p = __WithField(p.firstName, firstName); - p = __WithField(p.lastName, lastName); - return p; + return new Person(id, firstName, lastName); } } --- old/test/hotspot/jtreg/runtime/valhalla/valuetypes/Point.java 2019-08-29 16:29:18.114784697 +0200 +++ new/test/hotspot/jtreg/runtime/valhalla/valuetypes/Point.java 2019-08-29 16:29:17.722777834 +0200 @@ -26,9 +26,9 @@ final int x; final int y; - private Point() { - x = 0; - y = 0; + private Point(int x, int y) { + this.x = x; + this.y = y; } public int getX() { return x; } @@ -51,9 +51,6 @@ } public static Point createPoint(int x, int y) { - Point p = Point.default; - p = __WithField(p.x, x); - p = __WithField(p.y, y); - return p; + return new Point(x, y); } } --- old/test/hotspot/jtreg/runtime/valhalla/valuetypes/ValueTypeArray.java 2019-08-29 16:29:18.842797444 +0200 +++ new/test/hotspot/jtreg/runtime/valhalla/valuetypes/ValueTypeArray.java 2019-08-29 16:29:18.454790651 +0200 @@ -34,7 +34,7 @@ * @test ValueTypeArray * @summary Plain array test for Inline Types * @library /test/lib - * @compile -XDemitQtypes -XDenableValueTypes -XDallowWithFieldOperator -XDallowFlattenabilityModifiers -XDallowGenericsOverValues ValueTypeArray.java Point.java Long8Value.java Person.java + * @compile -XDallowGenericsOverValues ValueTypeArray.java Point.java Long8Value.java Person.java * @run main/othervm -Xint -XX:ValueArrayElemMaxFlatSize=-1 runtime.valhalla.valuetypes.ValueTypeArray * @run main/othervm -Xint -XX:ValueArrayElemMaxFlatSize=0 runtime.valhalla.valuetypes.ValueTypeArray * @run main/othervm -Xcomp -XX:ValueArrayElemMaxFlatSize=-1 runtime.valhalla.valuetypes.ValueTypeArray @@ -109,28 +109,43 @@ assertTrue(gotNpe, "Expected NullPointerException"); Point[] points = createSimplePointArray(); - checkSimplePointArray(points); System.gc(); // check that VTs survive GC + checkSimplePointArray(points); assertTrue(points instanceof Point[], "Instance of"); + testSimplePointArrayCopy(); + } + + void testSimplePointArrayCopy() { + Point[] points = createSimplePointArray(); Point[] pointsCopy = new Point[points.length]; System.arraycopy(points, 0, pointsCopy, 0, points.length); checkSimplePointArray(pointsCopy); + + // Conjoint, overlap...left + System.arraycopy(points, 0, points, 1, 2); + checkArrayElementsEqual(points, new Point[] { pointsCopy[0], pointsCopy[0], pointsCopy[1], pointsCopy[3] }); + + // Conjoint, overlap...right + points = createSimplePointArray(); + System.arraycopy(points, 2, points, 1, 2); + checkArrayElementsEqual(points, new Point[] { pointsCopy[0], pointsCopy[2], pointsCopy[3], pointsCopy[3] }); } static Point[] createSimplePointArray() { - Point[] ps = new Point[2]; - assertEquals(ps.length, 2, "Length"); + Point[] ps = new Point[4]; + assertEquals(ps.length, 4, "Length"); ps.toString(); ps[0] = Point.createPoint(1, 2); ps[1] = Point.createPoint(3, 4); + ps[2] = Point.createPoint(5, 6); + ps[3] = Point.createPoint(7, 8); boolean sawOob = false; try { - ps[2] = Point.createPoint(0, 0); + ps[ps.length] = Point.createPoint(0, 0); } catch (ArrayIndexOutOfBoundsException aioobe) { sawOob = true; } assertTrue(sawOob, "Didn't see AIOOBE"); - System.gc(); // check that VTs survive GC return ps; } @@ -139,6 +154,10 @@ assertEquals(points[0].y, 2, "invalid 0 point y value"); assertEquals(points[1].x, 3, "invalid 1 point x value"); assertEquals(points[1].y, 4, "invalid 1 point y value"); + assertEquals(points[2].x, 5, "invalid 2 point x value"); + assertEquals(points[2].y, 6, "invalid 2 point y value"); + assertEquals(points[3].x, 7, "invalid 3 point x value"); + assertEquals(points[3].y, 8, "invalid 3 point y value"); } void testLong8Array() { @@ -213,7 +232,8 @@ static final inline class MyInt implements Comparable { final int value; - private MyInt() { value = 0; } + private MyInt() { this(0); } + private MyInt(int v) { value = v; } public int getValue() { return value; } public String toString() { return "MyInt: " + getValue(); } public int compareTo(MyInt? that) { return Integer.compare(this.getValue(), that.getValue()); } @@ -225,9 +245,7 @@ } public static MyInt create(int v) { - MyInt mi = MyInt.default; - mi = __WithField(mi.value, v); - return mi; + return new MyInt(v); } // Null-able fields here are a temp hack to avoid ClassCircularityError @@ -426,9 +444,10 @@ final MyInt x; final MyInt y; - private MyPoint() { - x = (MyInt) MyInt.ZERO; - y = x; + private MyPoint() { this(0, 0); } + private MyPoint(int x, int y) { + this.x = new MyInt(x); + this.y = new MyInt(y); } public boolean equals(Object that) { if (that instanceof MyPoint) { @@ -438,15 +457,10 @@ return false; } static MyPoint create(int x) { - MyPoint mp = MyPoint.default; - mp = __WithField(mp.x, MyInt.create(x)); - return mp; + return new MyPoint(x, x); } static MyPoint create(int x, int y) { - MyPoint mp = MyPoint.default; - mp = __WithField(mp.x, MyInt.create(x)); - mp = __WithField(mp.y, MyInt.create(y)); - return mp; + return new MyPoint(x, y); } static final MyPoint? ORIGIN = create(0); }