Fix Object::Clone()'s pre-fence barrier.

Pass in a pre-fence barrier object that sets in the array length
instead of setting it after returning from AllocObject().

Fix another potential bug due to the wrong default pre-fence barrier
parameter value. Since this appears error-prone, removed the default
parameter value and make it an explicit parameter.

Fix another potential moving GC bug due to a lack of a SirtRef.

Bug: 13097759
Change-Id: I466aa0e50f9e1a5dbf20be5a195edee619c7514e
diff --git a/runtime/mirror/class-inl.h b/runtime/mirror/class-inl.h
index 89d9241..025e62a 100644
--- a/runtime/mirror/class-inl.h
+++ b/runtime/mirror/class-inl.h
@@ -442,7 +442,14 @@
 }
 
 inline void Class::CheckObjectAlloc() {
-  DCHECK(!IsArrayClass()) << PrettyClass(this);
+  DCHECK(!IsArrayClass())
+      << PrettyClass(this)
+      << "A array shouldn't be allocated through this "
+      << "as it requires a pre-fence visitor that sets the class size.";
+  DCHECK(!IsClassClass())
+      << PrettyClass(this)
+      << "A class object shouldn't be allocated through this "
+      << "as it requires a pre-fence visitor that sets the class size.";
   DCHECK(IsInstantiable()) << PrettyClass(this);
   // TODO: decide whether we want this check. It currently fails during bootstrap.
   // DCHECK(!Runtime::Current()->IsStarted() || IsInitializing()) << PrettyClass(this);
@@ -454,7 +461,7 @@
   CheckObjectAlloc();
   gc::Heap* heap = Runtime::Current()->GetHeap();
   return heap->AllocObjectWithAllocator<kIsInstrumented, false>(self, this, this->object_size_,
-                                                                allocator_type);
+                                                                allocator_type, VoidFunctor());
 }
 
 inline Object* Class::AllocObject(Thread* self) {
diff --git a/runtime/mirror/object.cc b/runtime/mirror/object.cc
index f1485e5..d9155f5 100644
--- a/runtime/mirror/object.cc
+++ b/runtime/mirror/object.cc
@@ -66,6 +66,26 @@
   return dest;
 }
 
+// An allocation pre-fence visitor that copies the object.
+class CopyObjectVisitor {
+ public:
+  explicit CopyObjectVisitor(Thread* self, SirtRef<Object>* orig, size_t num_bytes)
+      : self_(self), orig_(orig), num_bytes_(num_bytes) {
+  }
+
+  void operator()(Object* obj, size_t usable_size) const
+      SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
+    UNUSED(usable_size);
+    CopyObject(self_, obj, orig_->get(), num_bytes_);
+  }
+
+ private:
+  Thread* const self_;
+  SirtRef<Object>* const orig_;
+  const size_t num_bytes_;
+  DISALLOW_COPY_AND_ASSIGN(CopyObjectVisitor);
+};
+
 Object* Object::Clone(Thread* self) {
   CHECK(!IsClass()) << "Can't clone classes.";
   // Object::SizeOf gets the right size even if we're an array. Using c->AllocObject() here would
@@ -74,13 +94,11 @@
   size_t num_bytes = SizeOf();
   SirtRef<Object> this_object(self, this);
   Object* copy;
+  CopyObjectVisitor visitor(self, &this_object, num_bytes);
   if (heap->IsMovableObject(this)) {
-    copy = heap->AllocObject<true>(self, GetClass(), num_bytes);
+    copy = heap->AllocObject<true>(self, GetClass(), num_bytes, visitor);
   } else {
-    copy = heap->AllocNonMovableObject<true>(self, GetClass(), num_bytes);
-  }
-  if (LIKELY(copy != nullptr)) {
-    return CopyObject(self, copy, this_object.get(), num_bytes);
+    copy = heap->AllocNonMovableObject<true>(self, GetClass(), num_bytes, visitor);
   }
   return copy;
 }