Don't access component_type_ for obj-array's SizeOf

For object arrays the component-type class could be allocated in the
higher address than the object array. This causes problems in
userfaultfd GC as it frees from-space pages as the compaction
progresses.
Fortunately, for object arrays we recognize them using the class-flags
and therefore when calling SizeOf() on it in VisitRefsForCompaction(),
we can calculate component-size-shift without accessing component-type
class. For primitive object arrays it's not a problem as the
component-type class is either in boot/zygote images or in lower address
in the moving space.

Bug: 160737021
Bug: 272272332
Bug: 274327217
Test: install module and check of tombstones/ANR due to NPE in GC thread
Change-Id: Ic657ec95aed8b3642c62b82945ffabf947ee5ad7

4 files changed, 17 insertions, 15 deletions

diff --git a/runtime/gc/collector/mark_compact.cc b/runtime/gc/collector/mark_compact.cc
index 1751994b08..f9a4e72a50 100644
--- a/runtime/gc/collector/mark_compact.cc
+++ b/runtime/gc/collector/mark_compact.cc
@@ -1434,7 +1434,6 @@ void MarkCompact::CompactPage(mirror::Object* obj,
                               uint32_t offset,
                               uint8_t* addr,
                               bool needs_memset_zero) {
-  mirror::Object* first_obj = obj;
   DCHECK(moving_space_bitmap_->Test(obj)
          && live_words_bitmap_->Test(obj));
   DCHECK(live_words_bitmap_->Test(offset)) << "obj=" << obj
@@ -1526,12 +1525,11 @@ void MarkCompact::CompactPage(mirror::Object* obj,
                                                                      + kPageSize));
     }
     obj_size = RoundUp(obj_size, kAlignment);
-    CHECK_GT(obj_size, offset_within_obj)
+    DCHECK_GT(obj_size, offset_within_obj)
         << "obj:" << obj
         << " class:"
         << obj->GetClass<kDefaultVerifyFlags, kWithFromSpaceBarrier>()
         << " to_addr:" << to_ref
-        << " first-obj:" << first_obj
         << " black-allocation-begin:" << reinterpret_cast<void*>(black_allocations_begin_)
         << " post-compact-end:" << reinterpret_cast<void*>(post_compact_end_)
         << " offset:" << offset * kAlignment
@@ -1590,12 +1588,11 @@ void MarkCompact::CompactPage(mirror::Object* obj,
                                            MemberOffset(0),
                                            MemberOffset(end_addr - (addr + bytes_done)));
     obj_size = RoundUp(obj_size, kAlignment);
-    CHECK_GT(obj_size, 0u)
+    DCHECK_GT(obj_size, 0u)
         << "from_addr:" << obj
         << " from-space-class:"
         << obj->GetClass<kDefaultVerifyFlags, kWithFromSpaceBarrier>()
         << " to_addr:" << ref
-        << " first-obj:" << first_obj
         << " black-allocation-begin:" << reinterpret_cast<void*>(black_allocations_begin_)
         << " post-compact-end:" << reinterpret_cast<void*>(post_compact_end_)
         << " offset:" << offset * kAlignment
@@ -2000,9 +1997,7 @@ void MarkCompact::FreeFromSpacePages(size_t cur_page_idx, int mode) {
   }
   DCHECK_LE(idx, last_checked_reclaim_page_idx_);
   if (idx == last_checked_reclaim_page_idx_) {
-    // Nothing to do. Also, this possibly avoids freeing from-space pages too
-    // soon. TODO: Update the comment if returning here indeed fixed NPE like
-    // in b/272272332.
+    // Nothing to do.
     return;
   }
 
diff --git a/runtime/mirror/array-inl.h b/runtime/mirror/array-inl.h
index 2bdf8277cd..8f81ae5c1a 100644
--- a/runtime/mirror/array-inl.h
+++ b/runtime/mirror/array-inl.h
@@ -36,11 +36,15 @@ inline uint32_t Array::ClassSize(PointerSize pointer_size) {
   return Class::ComputeClassSize(true, vtable_entries, 0, 0, 0, 0, 0, pointer_size);
 }
 
-template<VerifyObjectFlags kVerifyFlags, ReadBarrierOption kReadBarrierOption>
+template <VerifyObjectFlags kVerifyFlags, ReadBarrierOption kReadBarrierOption, bool kIsObjArray>
 inline size_t Array::SizeOf() {
-  size_t component_size_shift =
-      GetClass<kVerifyFlags, kReadBarrierOption>()
-      ->template GetComponentSizeShift<kReadBarrierOption>();
+  // When we are certain that this is a object array, then don't fetch shift
+  // from component_type_ as that doesn't work well with userfaultfd GC as the
+  // component-type class may be allocated at a higher address than the array.
+  size_t component_size_shift = kIsObjArray ?
+                                    Primitive::ComponentSizeShift(Primitive::kPrimNot) :
+                                    GetClass<kVerifyFlags, kReadBarrierOption>()
+                                        ->template GetComponentSizeShift<kReadBarrierOption>();
   // Don't need to check this since we already check this in GetClass.
   int32_t component_count =
       GetLength<static_cast<VerifyObjectFlags>(kVerifyFlags & ~kVerifyThis)>();
diff --git a/runtime/mirror/array.h b/runtime/mirror/array.h
index 0116fdee6f..5d64167566 100644
--- a/runtime/mirror/array.h
+++ b/runtime/mirror/array.h
@@ -58,8 +58,9 @@ class MANAGED Array : public Object {
       REQUIRES_SHARED(Locks::mutator_lock_)
       REQUIRES(!Roles::uninterruptible_);
 
-  template<VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags,
-           ReadBarrierOption kReadBarrierOption = kWithoutReadBarrier>
+  template <VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags,
+            ReadBarrierOption kReadBarrierOption = kWithoutReadBarrier,
+            bool kIsObjArray = false>
   size_t SizeOf() REQUIRES_SHARED(Locks::mutator_lock_);
   template<VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags>
   ALWAYS_INLINE int32_t GetLength() REQUIRES_SHARED(Locks::mutator_lock_) {
diff --git a/runtime/mirror/object-refvisitor-inl.h b/runtime/mirror/object-refvisitor-inl.h
index 5251953859..4c72cd58c3 100644
--- a/runtime/mirror/object-refvisitor-inl.h
+++ b/runtime/mirror/object-refvisitor-inl.h
@@ -127,7 +127,9 @@ inline size_t Object::VisitRefsForCompaction(const Visitor& visitor,
         DCHECK((klass->IsObjectArrayClass<kVerifyFlags, kReadBarrierOption>()));
         ObjPtr<ObjectArray<Object>> obj_arr = ObjPtr<ObjectArray<Object>>::DownCast(this);
         obj_arr->VisitReferences(visitor, begin, end);
-        size = kFetchObjSize ? obj_arr->SizeOf<kSizeOfFlags, kReadBarrierOption>() : 0;
+        size = kFetchObjSize ?
+                   obj_arr->SizeOf<kSizeOfFlags, kReadBarrierOption, /*kIsObjArray*/ true>() :
+                   0;
       } else if ((class_flags & kClassFlagReference) != 0) {
         VisitInstanceFieldsReferences<kVerifyFlags, kReadBarrierOption>(klass, visitor);
         // Visit referent also as this is about updating the reference only.