diff options
| -rw-r--r-- | runtime/interpreter/unstarted_runtime.cc | 45 | ||||
| -rw-r--r-- | runtime/interpreter/unstarted_runtime_test.cc | 231 | ||||
| -rw-r--r-- | runtime/mirror/object_array-inl.h | 7 | ||||
| -rw-r--r-- | runtime/mirror/object_array.h | 1 | ||||
| -rw-r--r-- | runtime/native/java_lang_System.cc | 4 |
5 files changed, 264 insertions, 24 deletions
diff --git a/runtime/interpreter/unstarted_runtime.cc b/runtime/interpreter/unstarted_runtime.cc index 78f34fff26..81be959cd5 100644 --- a/runtime/interpreter/unstarted_runtime.cc +++ b/runtime/interpreter/unstarted_runtime.cc @@ -353,28 +353,35 @@ void UnstartedRuntime::UnstartedSystemArraycopy( jint src_pos = shadow_frame->GetVReg(arg_offset + 1); jint dst_pos = shadow_frame->GetVReg(arg_offset + 3); jint length = shadow_frame->GetVReg(arg_offset + 4); - mirror::Array* src_array = shadow_frame->GetVRegReference(arg_offset)->AsArray(); - mirror::Array* dst_array = shadow_frame->GetVRegReference(arg_offset + 2)->AsArray(); - // Null checking. - if (src_array == nullptr) { + mirror::Object* src_obj = shadow_frame->GetVRegReference(arg_offset); + mirror::Object* dst_obj = shadow_frame->GetVRegReference(arg_offset + 2); + // Null checking. For simplicity, abort transaction. + if (src_obj == nullptr) { AbortTransactionOrFail(self, "src is null in arraycopy."); return; } - if (dst_array == nullptr) { + if (dst_obj == nullptr) { AbortTransactionOrFail(self, "dst is null in arraycopy."); return; } + // Test for arrayness. Throw ArrayStoreException. + if (!src_obj->IsArrayInstance() || !dst_obj->IsArrayInstance()) { + self->ThrowNewException("Ljava/lang/ArrayStoreException;", "src or trg is not an array"); + return; + } + + mirror::Array* src_array = src_obj->AsArray(); + mirror::Array* dst_array = dst_obj->AsArray(); - // Bounds checking. + // Bounds checking. Throw IndexOutOfBoundsException. if (UNLIKELY(src_pos < 0) || UNLIKELY(dst_pos < 0) || UNLIKELY(length < 0) || UNLIKELY(src_pos > src_array->GetLength() - length) || UNLIKELY(dst_pos > dst_array->GetLength() - length)) { - self->ThrowNewExceptionF("Ljava/lang/ArrayIndexOutOfBoundsException;", + self->ThrowNewExceptionF("Ljava/lang/IndexOutOfBoundsException;", "src.length=%d srcPos=%d dst.length=%d dstPos=%d length=%d", src_array->GetLength(), src_pos, dst_array->GetLength(), dst_pos, length); - AbortTransactionOrFail(self, "Index out of bounds."); return; } @@ -393,19 +400,11 @@ void UnstartedRuntime::UnstartedSystemArraycopy( return; } - // For simplicity only do this if the component types are the same. Otherwise we have to copy - // even more code from the object-array functions. - if (src_type != trg_type) { - AbortTransactionOrFail(self, "Types not the same in arraycopy: %s vs %s", - PrettyDescriptor(src_array->GetClass()->GetComponentType()).c_str(), - PrettyDescriptor(dst_array->GetClass()->GetComponentType()).c_str()); - return; - } - mirror::ObjectArray<mirror::Object>* src = src_array->AsObjectArray<mirror::Object>(); mirror::ObjectArray<mirror::Object>* dst = dst_array->AsObjectArray<mirror::Object>(); if (src == dst) { // Can overlap, but not have type mismatches. + // We cannot use ObjectArray::MemMove here, as it doesn't support transactions. const bool copy_forward = (dst_pos < src_pos) || (dst_pos - src_pos >= length); if (copy_forward) { for (int32_t i = 0; i < length; ++i) { @@ -417,9 +416,15 @@ void UnstartedRuntime::UnstartedSystemArraycopy( } } } else { - // Can't overlap. Would need type checks, but we abort above. - for (int32_t i = 0; i < length; ++i) { - dst->Set(dst_pos + i, src->Get(src_pos + i)); + // We're being lazy here. Optimally this could be a memcpy (if component types are + // assignable), but the ObjectArray implementation doesn't support transactions. The + // checking version, however, does. + if (Runtime::Current()->IsActiveTransaction()) { + dst->AssignableCheckingMemcpy<true>( + dst_pos, src, src_pos, length, true /* throw_exception */); + } else { + dst->AssignableCheckingMemcpy<false>( + dst_pos, src, src_pos, length, true /* throw_exception */); } } } else if (src_type->IsPrimitiveChar()) { diff --git a/runtime/interpreter/unstarted_runtime_test.cc b/runtime/interpreter/unstarted_runtime_test.cc index a1ae2aab9c..fb53b1d5ef 100644 --- a/runtime/interpreter/unstarted_runtime_test.cc +++ b/runtime/interpreter/unstarted_runtime_test.cc @@ -66,6 +66,94 @@ class UnstartedRuntimeTest : public CommonRuntimeTest { #undef UNSTARTED_RUNTIME_DIRECT_LIST #undef UNSTARTED_RUNTIME_JNI_LIST #undef UNSTARTED_JNI + + // Helpers for ArrayCopy. + // + // Note: as we have to use handles, we use StackHandleScope to transfer data. Hardcode a size + // of three everywhere. That is enough to test all cases. + + static mirror::ObjectArray<mirror::Object>* CreateObjectArray( + Thread* self, + mirror::Class* component_type, + const StackHandleScope<3>& data) + SHARED_REQUIRES(Locks::mutator_lock_) { + Runtime* runtime = Runtime::Current(); + mirror::Class* array_type = runtime->GetClassLinker()->FindArrayClass(self, &component_type); + CHECK(array_type != nullptr); + mirror::ObjectArray<mirror::Object>* result = + mirror::ObjectArray<mirror::Object>::Alloc(self, array_type, 3); + CHECK(result != nullptr); + for (size_t i = 0; i < 3; ++i) { + result->Set(static_cast<int32_t>(i), data.GetReference(i)); + CHECK(!self->IsExceptionPending()); + } + return result; + } + + static void CheckObjectArray(mirror::ObjectArray<mirror::Object>* array, + const StackHandleScope<3>& data) + SHARED_REQUIRES(Locks::mutator_lock_) { + CHECK_EQ(array->GetLength(), 3); + CHECK_EQ(data.NumberOfReferences(), 3U); + for (size_t i = 0; i < 3; ++i) { + EXPECT_EQ(data.GetReference(i), array->Get(static_cast<int32_t>(i))) << i; + } + } + + void RunArrayCopy(Thread* self, + ShadowFrame* tmp, + bool expect_exception, + mirror::ObjectArray<mirror::Object>* src, + int32_t src_pos, + mirror::ObjectArray<mirror::Object>* dst, + int32_t dst_pos, + int32_t length) + SHARED_REQUIRES(Locks::mutator_lock_) { + JValue result; + tmp->SetVRegReference(0, src); + tmp->SetVReg(1, src_pos); + tmp->SetVRegReference(2, dst); + tmp->SetVReg(3, dst_pos); + tmp->SetVReg(4, length); + UnstartedSystemArraycopy(self, tmp, &result, 0); + bool exception_pending = self->IsExceptionPending(); + EXPECT_EQ(exception_pending, expect_exception); + if (exception_pending) { + self->ClearException(); + } + } + + void RunArrayCopy(Thread* self, + ShadowFrame* tmp, + bool expect_exception, + mirror::Class* src_component_class, + mirror::Class* dst_component_class, + const StackHandleScope<3>& src_data, + int32_t src_pos, + const StackHandleScope<3>& dst_data, + int32_t dst_pos, + int32_t length, + const StackHandleScope<3>& expected_result) + SHARED_REQUIRES(Locks::mutator_lock_) { + StackHandleScope<3> hs_misc(self); + Handle<mirror::Class> dst_component_handle(hs_misc.NewHandle(dst_component_class)); + + Handle<mirror::ObjectArray<mirror::Object>> src_handle( + hs_misc.NewHandle(CreateObjectArray(self, src_component_class, src_data))); + + Handle<mirror::ObjectArray<mirror::Object>> dst_handle( + hs_misc.NewHandle(CreateObjectArray(self, dst_component_handle.Get(), dst_data))); + + RunArrayCopy(self, + tmp, + expect_exception, + src_handle.Get(), + src_pos, + dst_handle.Get(), + dst_pos, + length); + CheckObjectArray(dst_handle.Get(), expected_result); + } }; TEST_F(UnstartedRuntimeTest, MemoryPeekByte) { @@ -277,5 +365,148 @@ TEST_F(UnstartedRuntimeTest, StringInit) { ShadowFrame::DeleteDeoptimizedFrame(shadow_frame); } +// Tests the exceptions that should be checked before modifying the destination. +// (Doesn't check the object vs primitive case ATM.) +TEST_F(UnstartedRuntimeTest, SystemArrayCopyObjectArrayTestExceptions) { + Thread* self = Thread::Current(); + ScopedObjectAccess soa(self); + JValue result; + ShadowFrame* tmp = ShadowFrame::CreateDeoptimizedFrame(10, nullptr, nullptr, 0); + + // Note: all tests are not GC safe. Assume there's no GC running here with the few objects we + // allocate. + StackHandleScope<2> hs_misc(self); + Handle<mirror::Class> object_class( + hs_misc.NewHandle(mirror::Class::GetJavaLangClass()->GetSuperClass())); + + StackHandleScope<3> hs_data(self); + hs_data.NewHandle(mirror::String::AllocFromModifiedUtf8(self, "1")); + hs_data.NewHandle(mirror::String::AllocFromModifiedUtf8(self, "2")); + hs_data.NewHandle(mirror::String::AllocFromModifiedUtf8(self, "3")); + + Handle<mirror::ObjectArray<mirror::Object>> array( + hs_misc.NewHandle(CreateObjectArray(self, object_class.Get(), hs_data))); + + RunArrayCopy(self, tmp, true, array.Get(), -1, array.Get(), 0, 0); + RunArrayCopy(self, tmp, true, array.Get(), 0, array.Get(), -1, 0); + RunArrayCopy(self, tmp, true, array.Get(), 0, array.Get(), 0, -1); + RunArrayCopy(self, tmp, true, array.Get(), 0, array.Get(), 0, 4); + RunArrayCopy(self, tmp, true, array.Get(), 0, array.Get(), 1, 3); + RunArrayCopy(self, tmp, true, array.Get(), 1, array.Get(), 0, 3); + + mirror::ObjectArray<mirror::Object>* class_as_array = + reinterpret_cast<mirror::ObjectArray<mirror::Object>*>(object_class.Get()); + RunArrayCopy(self, tmp, true, class_as_array, 0, array.Get(), 0, 0); + RunArrayCopy(self, tmp, true, array.Get(), 0, class_as_array, 0, 0); + + ShadowFrame::DeleteDeoptimizedFrame(tmp); +} + +TEST_F(UnstartedRuntimeTest, SystemArrayCopyObjectArrayTest) { + Thread* self = Thread::Current(); + ScopedObjectAccess soa(self); + JValue result; + ShadowFrame* tmp = ShadowFrame::CreateDeoptimizedFrame(10, nullptr, nullptr, 0); + + StackHandleScope<1> hs_object(self); + Handle<mirror::Class> object_class( + hs_object.NewHandle(mirror::Class::GetJavaLangClass()->GetSuperClass())); + + // Simple test: + // [1,2,3]{1 @ 2} into [4,5,6] = [4,2,6] + { + StackHandleScope<3> hs_src(self); + hs_src.NewHandle(mirror::String::AllocFromModifiedUtf8(self, "1")); + hs_src.NewHandle(mirror::String::AllocFromModifiedUtf8(self, "2")); + hs_src.NewHandle(mirror::String::AllocFromModifiedUtf8(self, "3")); + + StackHandleScope<3> hs_dst(self); + hs_dst.NewHandle(mirror::String::AllocFromModifiedUtf8(self, "4")); + hs_dst.NewHandle(mirror::String::AllocFromModifiedUtf8(self, "5")); + hs_dst.NewHandle(mirror::String::AllocFromModifiedUtf8(self, "6")); + + StackHandleScope<3> hs_expected(self); + hs_expected.NewHandle(hs_dst.GetReference(0)); + hs_expected.NewHandle(hs_dst.GetReference(1)); + hs_expected.NewHandle(hs_src.GetReference(1)); + + RunArrayCopy(self, + tmp, + false, + object_class.Get(), + object_class.Get(), + hs_src, + 1, + hs_dst, + 2, + 1, + hs_expected); + } + + // Simple test: + // [1,2,3]{1 @ 1} into [4,5,6] = [4,2,6] (with dst String[]) + { + StackHandleScope<3> hs_src(self); + hs_src.NewHandle(mirror::String::AllocFromModifiedUtf8(self, "1")); + hs_src.NewHandle(mirror::String::AllocFromModifiedUtf8(self, "2")); + hs_src.NewHandle(mirror::String::AllocFromModifiedUtf8(self, "3")); + + StackHandleScope<3> hs_dst(self); + hs_dst.NewHandle(mirror::String::AllocFromModifiedUtf8(self, "4")); + hs_dst.NewHandle(mirror::String::AllocFromModifiedUtf8(self, "5")); + hs_dst.NewHandle(mirror::String::AllocFromModifiedUtf8(self, "6")); + + StackHandleScope<3> hs_expected(self); + hs_expected.NewHandle(hs_dst.GetReference(0)); + hs_expected.NewHandle(hs_src.GetReference(1)); + hs_expected.NewHandle(hs_dst.GetReference(2)); + + RunArrayCopy(self, + tmp, + false, + object_class.Get(), + mirror::String::GetJavaLangString(), + hs_src, + 1, + hs_dst, + 1, + 1, + hs_expected); + } + + // Simple test: + // [1,*,3] into [4,5,6] = [1,5,6] + exc + { + StackHandleScope<3> hs_src(self); + hs_src.NewHandle(mirror::String::AllocFromModifiedUtf8(self, "1")); + hs_src.NewHandle(mirror::String::GetJavaLangString()); + hs_src.NewHandle(mirror::String::AllocFromModifiedUtf8(self, "3")); + + StackHandleScope<3> hs_dst(self); + hs_dst.NewHandle(mirror::String::AllocFromModifiedUtf8(self, "4")); + hs_dst.NewHandle(mirror::String::AllocFromModifiedUtf8(self, "5")); + hs_dst.NewHandle(mirror::String::AllocFromModifiedUtf8(self, "6")); + + StackHandleScope<3> hs_expected(self); + hs_expected.NewHandle(hs_src.GetReference(0)); + hs_expected.NewHandle(hs_dst.GetReference(1)); + hs_expected.NewHandle(hs_dst.GetReference(2)); + + RunArrayCopy(self, + tmp, + true, + object_class.Get(), + mirror::String::GetJavaLangString(), + hs_src, + 0, + hs_dst, + 0, + 3, + hs_expected); + } + + ShadowFrame::DeleteDeoptimizedFrame(tmp); +} + } // namespace interpreter } // namespace art diff --git a/runtime/mirror/object_array-inl.h b/runtime/mirror/object_array-inl.h index 6f9d64297a..c3c523187d 100644 --- a/runtime/mirror/object_array-inl.h +++ b/runtime/mirror/object_array-inl.h @@ -197,6 +197,7 @@ inline void ObjectArray<T>::AssignableMemcpy(int32_t dst_pos, ObjectArray<T>* sr } template<class T> +template<bool kTransactionActive> inline void ObjectArray<T>::AssignableCheckingMemcpy(int32_t dst_pos, ObjectArray<T>* src, int32_t src_pos, int32_t count, bool throw_exception) { @@ -215,15 +216,15 @@ inline void ObjectArray<T>::AssignableCheckingMemcpy(int32_t dst_pos, ObjectArra o = src->GetWithoutChecks(src_pos + i); if (o == nullptr) { // Null is always assignable. - SetWithoutChecks<false>(dst_pos + i, nullptr); + SetWithoutChecks<kTransactionActive>(dst_pos + i, nullptr); } else { // TODO: use the underlying class reference to avoid uncompression when not necessary. Class* o_class = o->GetClass(); if (LIKELY(lastAssignableElementClass == o_class)) { - SetWithoutChecks<false>(dst_pos + i, o); + SetWithoutChecks<kTransactionActive>(dst_pos + i, o); } else if (LIKELY(dst_class->IsAssignableFrom(o_class))) { lastAssignableElementClass = o_class; - SetWithoutChecks<false>(dst_pos + i, o); + SetWithoutChecks<kTransactionActive>(dst_pos + i, o); } else { // Can't put this element into the array, break to perform write-barrier and throw // exception. diff --git a/runtime/mirror/object_array.h b/runtime/mirror/object_array.h index 1b1295cedb..42573968c9 100644 --- a/runtime/mirror/object_array.h +++ b/runtime/mirror/object_array.h @@ -78,6 +78,7 @@ class MANAGED ObjectArray: public Array { int32_t count) SHARED_REQUIRES(Locks::mutator_lock_); // Copy src into this array with assignability checks. + template<bool kTransactionActive> void AssignableCheckingMemcpy(int32_t dst_pos, ObjectArray<T>* src, int32_t src_pos, int32_t count, bool throw_exception) SHARED_REQUIRES(Locks::mutator_lock_); diff --git a/runtime/native/java_lang_System.cc b/runtime/native/java_lang_System.cc index d9863c579e..9e2d68d363 100644 --- a/runtime/native/java_lang_System.cc +++ b/runtime/native/java_lang_System.cc @@ -149,7 +149,9 @@ static void System_arraycopy(JNIEnv* env, jclass, jobject javaSrc, jint srcPos, dstObjArray->AssignableMemcpy(dstPos, srcObjArray, srcPos, count); return; } - dstObjArray->AssignableCheckingMemcpy(dstPos, srcObjArray, srcPos, count, true); + // This code is never run under a transaction. + DCHECK(!Runtime::Current()->IsActiveTransaction()); + dstObjArray->AssignableCheckingMemcpy<false>(dstPos, srcObjArray, srcPos, count, true); } // Template to convert general array to that of its specific primitive type. |