Don't use memcpy when copying mirror::Object.
Instead use existing code doing a raw copy of a mirror::Object,
correctly using atomic instructions.
Test: test.py
Bug: 260557058
Change-Id: Ie5b0cd9421d537e1693ddc5c8db3a201c0dd5ab8
diff --git a/runtime/mirror/object.cc b/runtime/mirror/object.cc
index bb9e85d..5016c20 100644
--- a/runtime/mirror/object.cc
+++ b/runtime/mirror/object.cc
@@ -74,46 +74,50 @@
const ObjPtr<Object> dest_obj_;
};
+void Object::CopyRawObjectData(uint8_t* dst_bytes,
+ ObjPtr<mirror::Object> src,
+ size_t num_bytes) {
+ // Copy instance data. Don't assume memcpy copies by words (b/32012820).
+ const size_t offset = sizeof(Object);
+ uint8_t* src_bytes = reinterpret_cast<uint8_t*>(src.Ptr()) + offset;
+ dst_bytes += offset;
+ DCHECK_ALIGNED(src_bytes, sizeof(uintptr_t));
+ DCHECK_ALIGNED(dst_bytes, sizeof(uintptr_t));
+ // Use word sized copies to begin.
+ while (num_bytes >= sizeof(uintptr_t)) {
+ reinterpret_cast<Atomic<uintptr_t>*>(dst_bytes)->store(
+ reinterpret_cast<Atomic<uintptr_t>*>(src_bytes)->load(std::memory_order_relaxed),
+ std::memory_order_relaxed);
+ src_bytes += sizeof(uintptr_t);
+ dst_bytes += sizeof(uintptr_t);
+ num_bytes -= sizeof(uintptr_t);
+ }
+ // Copy possible 32 bit word.
+ if (sizeof(uintptr_t) != sizeof(uint32_t) && num_bytes >= sizeof(uint32_t)) {
+ reinterpret_cast<Atomic<uint32_t>*>(dst_bytes)->store(
+ reinterpret_cast<Atomic<uint32_t>*>(src_bytes)->load(std::memory_order_relaxed),
+ std::memory_order_relaxed);
+ src_bytes += sizeof(uint32_t);
+ dst_bytes += sizeof(uint32_t);
+ num_bytes -= sizeof(uint32_t);
+ }
+ // Copy remaining bytes, avoid going past the end of num_bytes since there may be a redzone
+ // there.
+ while (num_bytes > 0) {
+ reinterpret_cast<Atomic<uint8_t>*>(dst_bytes)->store(
+ reinterpret_cast<Atomic<uint8_t>*>(src_bytes)->load(std::memory_order_relaxed),
+ std::memory_order_relaxed);
+ src_bytes += sizeof(uint8_t);
+ dst_bytes += sizeof(uint8_t);
+ num_bytes -= sizeof(uint8_t);
+ }
+}
+
ObjPtr<Object> Object::CopyObject(ObjPtr<mirror::Object> dest,
ObjPtr<mirror::Object> src,
size_t num_bytes) {
- // Copy instance data. Don't assume memcpy copies by words (b/32012820).
- {
- const size_t offset = sizeof(Object);
- uint8_t* src_bytes = reinterpret_cast<uint8_t*>(src.Ptr()) + offset;
- uint8_t* dst_bytes = reinterpret_cast<uint8_t*>(dest.Ptr()) + offset;
- num_bytes -= offset;
- DCHECK_ALIGNED(src_bytes, sizeof(uintptr_t));
- DCHECK_ALIGNED(dst_bytes, sizeof(uintptr_t));
- // Use word sized copies to begin.
- while (num_bytes >= sizeof(uintptr_t)) {
- reinterpret_cast<Atomic<uintptr_t>*>(dst_bytes)->store(
- reinterpret_cast<Atomic<uintptr_t>*>(src_bytes)->load(std::memory_order_relaxed),
- std::memory_order_relaxed);
- src_bytes += sizeof(uintptr_t);
- dst_bytes += sizeof(uintptr_t);
- num_bytes -= sizeof(uintptr_t);
- }
- // Copy possible 32 bit word.
- if (sizeof(uintptr_t) != sizeof(uint32_t) && num_bytes >= sizeof(uint32_t)) {
- reinterpret_cast<Atomic<uint32_t>*>(dst_bytes)->store(
- reinterpret_cast<Atomic<uint32_t>*>(src_bytes)->load(std::memory_order_relaxed),
- std::memory_order_relaxed);
- src_bytes += sizeof(uint32_t);
- dst_bytes += sizeof(uint32_t);
- num_bytes -= sizeof(uint32_t);
- }
- // Copy remaining bytes, avoid going past the end of num_bytes since there may be a redzone
- // there.
- while (num_bytes > 0) {
- reinterpret_cast<Atomic<uint8_t>*>(dst_bytes)->store(
- reinterpret_cast<Atomic<uint8_t>*>(src_bytes)->load(std::memory_order_relaxed),
- std::memory_order_relaxed);
- src_bytes += sizeof(uint8_t);
- dst_bytes += sizeof(uint8_t);
- num_bytes -= sizeof(uint8_t);
- }
- }
+ // Copy everything but the header.
+ CopyRawObjectData(reinterpret_cast<uint8_t*>(dest.Ptr()), src, num_bytes - sizeof(Object));
if (gUseReadBarrier) {
// We need a RB here. After copying the whole object above, copy references fields one by one
diff --git a/runtime/mirror/object.h b/runtime/mirror/object.h
index 76de550..95b9f86 100644
--- a/runtime/mirror/object.h
+++ b/runtime/mirror/object.h
@@ -675,6 +675,13 @@
std::string PrettyTypeOf()
REQUIRES_SHARED(Locks::mutator_lock_);
+ // A utility function that does a raw copy of `src`'s data into the buffer `dst_bytes`.
+ // Skips the object header.
+ static void CopyRawObjectData(uint8_t* dst_bytes,
+ ObjPtr<mirror::Object> src,
+ size_t num_bytes)
+ REQUIRES_SHARED(Locks::mutator_lock_);
+
protected:
// Accessors for non-Java type fields
template<class T, VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags, bool kIsVolatile = false>
diff --git a/runtime/runtime_image.cc b/runtime/runtime_image.cc
index 3b0f6ad..f685dd6 100644
--- a/runtime/runtime_image.cc
+++ b/runtime/runtime_image.cc
@@ -1383,23 +1383,25 @@
DCHECK(IsAligned<kObjectAlignment>(offset));
object_offsets_.push_back(offset);
objects_.resize(RoundUp(offset + object_size, kObjectAlignment));
- memcpy(objects_.data() + offset, obj.Ptr(), object_size);
- object_section_size_ += RoundUp(object_size, kObjectAlignment);
+
+ mirror::Object* copy = reinterpret_cast<mirror::Object*>(objects_.data() + offset);
+ mirror::Object::CopyRawObjectData(
+ reinterpret_cast<uint8_t*>(copy), obj, object_size - sizeof(mirror::Object));
+ // Clear any lockword data.
+ copy->SetLockWord(LockWord::Default(), /* as_volatile= */ false);
+ copy->SetClass(obj->GetClass());
// Fixup reference pointers.
FixupVisitor visitor(this, offset);
obj->VisitReferences</*kVisitNativeRoots=*/ false>(visitor, visitor);
- mirror::Object* copy = reinterpret_cast<mirror::Object*>(objects_.data() + offset);
-
- // Clear any lockword data.
- copy->SetLockWord(LockWord::Default(), /* as_volatile= */ false);
-
if (obj->IsString()) {
// Ensure a string always has a hashcode stored. This is checked at
// runtime because boot images don't want strings dirtied due to hashcode.
reinterpret_cast<mirror::String*>(copy)->GetHashCode();
}
+
+ object_section_size_ += RoundUp(object_size, kObjectAlignment);
return offset;
}