Remove read barriers in Class::IsClassClass and Object::IsClass

Remove unnecessary read barriers since Class.class is not movable.

Test: test-art-host
Bug: 114413743

Change-Id: Id68562606bab75bf5ba99a7e8a38e7db9d46df1d
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index 511d468..44c73fb 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -431,6 +431,8 @@
   heap->IncrementDisableMovingGC(self);
   StackHandleScope<64> hs(self);  // 64 is picked arbitrarily.
   auto class_class_size = mirror::Class::ClassClassSize(image_pointer_size_);
+  // Allocate the object as non-movable so that there are no cases where Object::IsClass returns
+  // the incorrect result when comparing to-space vs from-space.
   Handle<mirror::Class> java_lang_Class(hs.NewHandle(ObjPtr<mirror::Class>::DownCast(MakeObjPtr(
       heap->AllocNonMovableObject<true>(self, nullptr, class_class_size, VoidFunctor())))));
   CHECK(java_lang_Class != nullptr);
diff --git a/runtime/gc/space/image_space.cc b/runtime/gc/space/image_space.cc
index 3999e27..18e656c 100644
--- a/runtime/gc/space/image_space.cc
+++ b/runtime/gc/space/image_space.cc
@@ -1008,7 +1008,7 @@
       }
 
       if (obj->IsClass()) {
-        mirror::Class* klass = obj->AsClass<kVerifyNone, kWithoutReadBarrier>();
+        mirror::Class* klass = obj->AsClass<kVerifyNone>();
         // Fixup super class before visiting instance fields which require
         // information from their super class to calculate offsets.
         mirror::Class* super_class = klass->GetSuperClass<kVerifyNone, kWithoutReadBarrier>();
@@ -1026,8 +1026,8 @@
           *this);
       // Note that this code relies on no circular dependencies.
       // We want to use our own class loader and not the one in the image.
-      if (obj->IsClass<kVerifyNone, kWithoutReadBarrier>()) {
-        mirror::Class* as_klass = obj->AsClass<kVerifyNone, kWithoutReadBarrier>();
+      if (obj->IsClass<kVerifyNone>()) {
+        mirror::Class* as_klass = obj->AsClass<kVerifyNone>();
         FixupObjectAdapter visitor(boot_image_, boot_oat_, app_image_, app_oat_);
         as_klass->FixupNativePointers<kVerifyNone, kWithoutReadBarrier>(as_klass,
                                                                         pointer_size_,
diff --git a/runtime/jit/jit_code_cache.cc b/runtime/jit/jit_code_cache.cc
index 184aba8..bcbdc3b 100644
--- a/runtime/jit/jit_code_cache.cc
+++ b/runtime/jit/jit_code_cache.cc
@@ -530,7 +530,7 @@
   // This does not need a read barrier because this is called by GC.
   mirror::Class* cls = root_ptr->Read<kWithoutReadBarrier>();
   if (cls != nullptr && cls != weak_sentinel) {
-    DCHECK((cls->IsClass<kDefaultVerifyFlags, kWithoutReadBarrier>()));
+    DCHECK((cls->IsClass<kDefaultVerifyFlags>()));
     // Look at the classloader of the class to know if it has been unloaded.
     // This does not need a read barrier because this is called by GC.
     mirror::Object* class_loader =
diff --git a/runtime/mirror/class-inl.h b/runtime/mirror/class-inl.h
index 51dc1a4..d3f8921 100644
--- a/runtime/mirror/class-inl.h
+++ b/runtime/mirror/class-inl.h
@@ -378,7 +378,7 @@
 inline bool Class::IsVariableSize() {
   // Classes, arrays, and strings vary in size, and so the object_size_ field cannot
   // be used to Get their instance size
-  return IsClassClass<kVerifyFlags, kReadBarrierOption>() ||
+  return IsClassClass<kVerifyFlags>() ||
          IsArrayClass<kVerifyFlags, kReadBarrierOption>() ||
          IsStringClass();
 }
@@ -853,10 +853,12 @@
   return size;
 }
 
-template<VerifyObjectFlags kVerifyFlags, ReadBarrierOption kReadBarrierOption>
+template<VerifyObjectFlags kVerifyFlags>
 inline bool Class::IsClassClass() {
-  ObjPtr<Class> java_lang_Class = GetClass<kVerifyFlags, kReadBarrierOption>()->
-      template GetClass<kVerifyFlags, kReadBarrierOption>();
+  // OK to look at from-space copies since java.lang.Class.class is not movable.
+  // See b/114413743
+  ObjPtr<Class> java_lang_Class = GetClass<kVerifyFlags, kWithoutReadBarrier>()->
+      template GetClass<kVerifyFlags, kWithoutReadBarrier>();
   return this == java_lang_Class;
 }
 
diff --git a/runtime/mirror/class.h b/runtime/mirror/class.h
index 811ee51..4015bd2 100644
--- a/runtime/mirror/class.h
+++ b/runtime/mirror/class.h
@@ -430,8 +430,7 @@
            ReadBarrierOption kReadBarrierOption = kWithReadBarrier>
   bool IsArrayClass() REQUIRES_SHARED(Locks::mutator_lock_);
 
-  template<VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags,
-           ReadBarrierOption kReadBarrierOption = kWithReadBarrier>
+  template<VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags>
   bool IsClassClass() REQUIRES_SHARED(Locks::mutator_lock_);
 
   bool IsThrowableClass() REQUIRES_SHARED(Locks::mutator_lock_);
diff --git a/runtime/mirror/object-inl.h b/runtime/mirror/object-inl.h
index 40832bc..1b03956 100644
--- a/runtime/mirror/object-inl.h
+++ b/runtime/mirror/object-inl.h
@@ -137,17 +137,18 @@
   return klass->IsAssignableFrom(GetClass<kVerifyFlags>());
 }
 
-template<VerifyObjectFlags kVerifyFlags, ReadBarrierOption kReadBarrierOption>
+template<VerifyObjectFlags kVerifyFlags>
 inline bool Object::IsClass() {
-  constexpr auto kNewFlags = RemoveThisFlags(kVerifyFlags);
-  Class* java_lang_Class = GetClass<kVerifyFlags, kReadBarrierOption>()->
-      template GetClass<kVerifyFlags, kReadBarrierOption>();
-  return GetClass<kNewFlags, kReadBarrierOption>() == java_lang_Class;
+  // OK to look at from-space copies since java.lang.Class.class is not movable.
+  // See b/114413743
+  ObjPtr<Class> klass = GetClass<kVerifyFlags, kWithoutReadBarrier>();
+  ObjPtr<Class> java_lang_Class = klass->template GetClass<kVerifyFlags, kWithoutReadBarrier>();
+  return klass == java_lang_Class;
 }
 
-template<VerifyObjectFlags kVerifyFlags, ReadBarrierOption kReadBarrierOption>
+template<VerifyObjectFlags kVerifyFlags>
 inline Class* Object::AsClass() {
-  DCHECK((IsClass<kVerifyFlags, kReadBarrierOption>()));
+  DCHECK((IsClass<kVerifyFlags>()));
   return down_cast<Class*>(this);
 }
 
@@ -350,8 +351,8 @@
   constexpr auto kNewFlags = RemoveThisFlags(kVerifyFlags);
   if (IsArrayInstance<kVerifyFlags, kRBO>()) {
     result = AsArray<kNewFlags, kRBO>()->template SizeOf<kNewFlags, kRBO>();
-  } else if (IsClass<kNewFlags, kRBO>()) {
-    result = AsClass<kNewFlags, kRBO>()->template SizeOf<kNewFlags, kRBO>();
+  } else if (IsClass<kNewFlags>()) {
+    result = AsClass<kNewFlags>()->template SizeOf<kNewFlags, kRBO>();
   } else if (GetClass<kNewFlags, kRBO>()->IsStringClass()) {
     result = AsString<kNewFlags, kRBO>()->template SizeOf<kNewFlags>();
   } else {
@@ -867,7 +868,7 @@
     // inheritance hierarchy and find reference offsets the hard way. In the static case, just
     // consider this class.
     for (ObjPtr<Class> klass = kIsStatic
-            ? AsClass<kVerifyFlags, kReadBarrierOption>()
+            ? AsClass<kVerifyFlags>()
             : GetClass<kVerifyFlags, kReadBarrierOption>();
         klass != nullptr;
         klass = kIsStatic ? nullptr : klass->GetSuperClass<kVerifyFlags, kReadBarrierOption>()) {
diff --git a/runtime/mirror/object-refvisitor-inl.h b/runtime/mirror/object-refvisitor-inl.h
index 39e32bf..bd23971 100644
--- a/runtime/mirror/object-refvisitor-inl.h
+++ b/runtime/mirror/object-refvisitor-inl.h
@@ -39,7 +39,7 @@
   if (LIKELY(class_flags == kClassFlagNormal)) {
     DCHECK((!klass->IsVariableSize<kVerifyFlags, kReadBarrierOption>()));
     VisitInstanceFieldsReferences<kVerifyFlags, kReadBarrierOption>(klass, visitor);
-    DCHECK((!klass->IsClassClass<kVerifyFlags, kReadBarrierOption>()));
+    DCHECK((!klass->IsClassClass<kVerifyFlags>()));
     DCHECK(!klass->IsStringClass());
     DCHECK(!klass->IsClassLoaderClass());
     DCHECK((!klass->IsArrayClass<kVerifyFlags, kReadBarrierOption>()));
@@ -47,8 +47,8 @@
     if ((class_flags & kClassFlagNoReferenceFields) == 0) {
       DCHECK(!klass->IsStringClass());
       if (class_flags == kClassFlagClass) {
-        DCHECK((klass->IsClassClass<kVerifyFlags, kReadBarrierOption>()));
-        ObjPtr<Class> as_klass = AsClass<kVerifyNone, kReadBarrierOption>();
+        DCHECK((klass->IsClassClass<kVerifyFlags>()));
+        ObjPtr<Class> as_klass = AsClass<kVerifyNone>();
         as_klass->VisitReferences<kVisitNativeRoots, kVerifyFlags, kReadBarrierOption>(klass,
                                                                                        visitor);
       } else if (class_flags == kClassFlagObjectArray) {
@@ -69,7 +69,7 @@
                                       kReadBarrierOption>(klass, visitor);
       }
     } else if (kIsDebugBuild) {
-      CHECK((!klass->IsClassClass<kVerifyFlags, kReadBarrierOption>()));
+      CHECK((!klass->IsClassClass<kVerifyFlags>()));
       CHECK((!klass->IsObjectArrayClass<kVerifyFlags, kReadBarrierOption>()));
       // String still has instance fields for reflection purposes but these don't exist in
       // actual string instances.
diff --git a/runtime/mirror/object.h b/runtime/mirror/object.h
index 35946d7..48ce5c1 100644
--- a/runtime/mirror/object.h
+++ b/runtime/mirror/object.h
@@ -169,11 +169,9 @@
   void NotifyAll(Thread* self) REQUIRES_SHARED(Locks::mutator_lock_);
   void Wait(Thread* self, int64_t timeout, int32_t nanos) REQUIRES_SHARED(Locks::mutator_lock_);
 
-  template<VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags,
-           ReadBarrierOption kReadBarrierOption = kWithReadBarrier>
+  template<VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags>
   bool IsClass() REQUIRES_SHARED(Locks::mutator_lock_);
-  template<VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags,
-           ReadBarrierOption kReadBarrierOption = kWithReadBarrier>
+  template<VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags>
   Class* AsClass() REQUIRES_SHARED(Locks::mutator_lock_);
 
   template<VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags,