ART: Better support for arraycopy in unstarted runtime
Extend the System.arraycopy() cutout in the unstarted runtime
to support arrays with differing component types.
Add tests.
Bug: 27805718
(cherry picked from commit 85a098af5fc8d2dd0e39d61c9f93fc6257d631c5)
Change-Id: Iaacd95a372e9bfa26e9055a06b0d8f0335b8d6d1
diff --git a/runtime/interpreter/unstarted_runtime.cc b/runtime/interpreter/unstarted_runtime.cc
index eaea01d..49db49b 100644
--- a/runtime/interpreter/unstarted_runtime.cc
+++ b/runtime/interpreter/unstarted_runtime.cc
@@ -353,28 +353,35 @@
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;
+ }
- // Bounds checking.
+ mirror::Array* src_array = src_obj->AsArray();
+ mirror::Array* dst_array = dst_obj->AsArray();
+
+ // 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 @@
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 @@
}
}
} 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 a1ae2aa..fb53b1d 100644
--- a/runtime/interpreter/unstarted_runtime_test.cc
+++ b/runtime/interpreter/unstarted_runtime_test.cc
@@ -66,6 +66,94 @@
#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 @@
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 6f9d642..c3c5231 100644
--- a/runtime/mirror/object_array-inl.h
+++ b/runtime/mirror/object_array-inl.h
@@ -197,6 +197,7 @@
}
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 @@
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 1b1295c..4257396 100644
--- a/runtime/mirror/object_array.h
+++ b/runtime/mirror/object_array.h
@@ -78,6 +78,7 @@
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 d9863c5..9e2d68d 100644
--- a/runtime/native/java_lang_System.cc
+++ b/runtime/native/java_lang_System.cc
@@ -149,7 +149,9 @@
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.
