Merge "Check that the String class is not movable in String.equals intrinsics."

8 files changed, 60 insertions, 0 deletions

diff --git a/compiler/optimizing/intrinsics.cc b/compiler/optimizing/intrinsics.cc
index 2721cb5a0f..2de0f0c737 100644
--- a/compiler/optimizing/intrinsics.cc
+++ b/compiler/optimizing/intrinsics.cc
@@ -20,6 +20,7 @@
 #include "art_method-inl.h"
 #include "base/utils.h"
 #include "class_linker.h"
+#include "class_root.h"
 #include "dex/invoke_type.h"
 #include "driver/compiler_options.h"
 #include "gc/space/image_space.h"
@@ -362,4 +363,13 @@ IntrinsicVisitor::IntegerValueOfInfo IntrinsicVisitor::ComputeIntegerValueOfInfo
   return info;
 }
 
+void IntrinsicVisitor::AssertNonMovableStringClass() {
+  if (kIsDebugBuild) {
+    Thread* const self = Thread::Current();
+    ReaderMutexLock mu(self, *Locks::mutator_lock_);
+    ObjPtr<mirror::Class> string_class = GetClassRoot<art::mirror::String>();
+    CHECK(!art::Runtime::Current()->GetHeap()->IsMovableObject(string_class));
+  }
+}
+
 }  // namespace art
diff --git a/compiler/optimizing/intrinsics.h b/compiler/optimizing/intrinsics.h
index 50b13c842b..ab68cce304 100644
--- a/compiler/optimizing/intrinsics.h
+++ b/compiler/optimizing/intrinsics.h
@@ -142,6 +142,8 @@ class IntrinsicVisitor : public ValueObject {
  protected:
   IntrinsicVisitor() {}
 
+  static void AssertNonMovableStringClass();
+
  private:
   DISALLOW_COPY_AND_ASSIGN(IntrinsicVisitor);
 };
diff --git a/compiler/optimizing/intrinsics_arm64.cc b/compiler/optimizing/intrinsics_arm64.cc
index ca790f655a..ec5d17a443 100644
--- a/compiler/optimizing/intrinsics_arm64.cc
+++ b/compiler/optimizing/intrinsics_arm64.cc
@@ -1464,8 +1464,16 @@ void IntrinsicCodeGeneratorARM64::VisitStringEquals(HInvoke* invoke) {
     // All string objects must have the same type since String cannot be subclassed.
     // Receiver must be a string object, so its class field is equal to all strings' class fields.
     // If the argument is a string object, its class field must be equal to receiver's class field.
+    //
+    // As the String class is expected to be non-movable, we can read the class
+    // field from String.equals' arguments without read barriers.
+    AssertNonMovableStringClass();
+    // /* HeapReference<Class> */ temp = str->klass_
     __ Ldr(temp, MemOperand(str.X(), class_offset));
+    // /* HeapReference<Class> */ temp1 = arg->klass_
     __ Ldr(temp1, MemOperand(arg.X(), class_offset));
+    // Also, because we use the previously loaded class references only in the
+    // following comparison, we don't need to unpoison them.
     __ Cmp(temp, temp1);
     __ B(&return_false, ne);
   }
diff --git a/compiler/optimizing/intrinsics_arm_vixl.cc b/compiler/optimizing/intrinsics_arm_vixl.cc
index 396ff62a16..f0aa92e981 100644
--- a/compiler/optimizing/intrinsics_arm_vixl.cc
+++ b/compiler/optimizing/intrinsics_arm_vixl.cc
@@ -1529,8 +1529,16 @@ void IntrinsicCodeGeneratorARMVIXL::VisitStringEquals(HInvoke* invoke) {
     // All string objects must have the same type since String cannot be subclassed.
     // Receiver must be a string object, so its class field is equal to all strings' class fields.
     // If the argument is a string object, its class field must be equal to receiver's class field.
+    //
+    // As the String class is expected to be non-movable, we can read the class
+    // field from String.equals' arguments without read barriers.
+    AssertNonMovableStringClass();
+    // /* HeapReference<Class> */ temp = str->klass_
     __ Ldr(temp, MemOperand(str, class_offset));
+    // /* HeapReference<Class> */ out = arg->klass_
     __ Ldr(out, MemOperand(arg, class_offset));
+    // Also, because we use the previously loaded class references only in the
+    // following comparison, we don't need to unpoison them.
     __ Cmp(temp, out);
     __ B(ne, &return_false, /* is_far_target= */ false);
   }
diff --git a/compiler/optimizing/intrinsics_mips.cc b/compiler/optimizing/intrinsics_mips.cc
index 5b359741d9..3da0e578bf 100644
--- a/compiler/optimizing/intrinsics_mips.cc
+++ b/compiler/optimizing/intrinsics_mips.cc
@@ -1575,8 +1575,16 @@ void IntrinsicCodeGeneratorMIPS::VisitStringEquals(HInvoke* invoke) {
     // All string objects must have the same type since String cannot be subclassed.
     // Receiver must be a string object, so its class field is equal to all strings' class fields.
     // If the argument is a string object, its class field must be equal to receiver's class field.
+    //
+    // As the String class is expected to be non-movable, we can read the class
+    // field from String.equals' arguments without read barriers.
+    AssertNonMovableStringClass();
+    // /* HeapReference<Class> */ temp1 = str->klass_
     __ Lw(temp1, str, class_offset);
+    // /* HeapReference<Class> */ temp2 = arg->klass_
     __ Lw(temp2, arg, class_offset);
+    // Also, because we use the previously loaded class references only in the
+    // following comparison, we don't need to unpoison them.
     __ Bne(temp1, temp2, &return_false);
   }
 
diff --git a/compiler/optimizing/intrinsics_mips64.cc b/compiler/optimizing/intrinsics_mips64.cc
index afaa4ca4c8..3e687652d3 100644
--- a/compiler/optimizing/intrinsics_mips64.cc
+++ b/compiler/optimizing/intrinsics_mips64.cc
@@ -1429,8 +1429,16 @@ void IntrinsicCodeGeneratorMIPS64::VisitStringEquals(HInvoke* invoke) {
     // All string objects must have the same type since String cannot be subclassed.
     // Receiver must be a string object, so its class field is equal to all strings' class fields.
     // If the argument is a string object, its class field must be equal to receiver's class field.
+    //
+    // As the String class is expected to be non-movable, we can read the class
+    // field from String.equals' arguments without read barriers.
+    AssertNonMovableStringClass();
+    // /* HeapReference<Class> */ temp1 = str->klass_
     __ Lw(temp1, str, class_offset);
+    // /* HeapReference<Class> */ temp2 = arg->klass_
     __ Lw(temp2, arg, class_offset);
+    // Also, because we use the previously loaded class references only in the
+    // following comparison, we don't need to unpoison them.
     __ Bnec(temp1, temp2, &return_false);
   }
 
diff --git a/compiler/optimizing/intrinsics_x86.cc b/compiler/optimizing/intrinsics_x86.cc
index 8747f061d3..de697f0f96 100644
--- a/compiler/optimizing/intrinsics_x86.cc
+++ b/compiler/optimizing/intrinsics_x86.cc
@@ -1072,7 +1072,15 @@ void IntrinsicCodeGeneratorX86::VisitStringEquals(HInvoke* invoke) {
     // All string objects must have the same type since String cannot be subclassed.
     // Receiver must be a string object, so its class field is equal to all strings' class fields.
     // If the argument is a string object, its class field must be equal to receiver's class field.
+    //
+    // As the String class is expected to be non-movable, we can read the class
+    // field from String.equals' arguments without read barriers.
+    AssertNonMovableStringClass();
+    // Also, because we use the loaded class references only to compare them, we
+    // don't need to unpoison them.
+    // /* HeapReference<Class> */ ecx = str->klass_
     __ movl(ecx, Address(str, class_offset));
+    // if (ecx != /* HeapReference<Class> */ arg->klass_) return false
     __ cmpl(ecx, Address(arg, class_offset));
     __ j(kNotEqual, &return_false);
   }
diff --git a/compiler/optimizing/intrinsics_x86_64.cc b/compiler/optimizing/intrinsics_x86_64.cc
index 167c1d8632..e79c0c9adf 100644
--- a/compiler/optimizing/intrinsics_x86_64.cc
+++ b/compiler/optimizing/intrinsics_x86_64.cc
@@ -1275,7 +1275,15 @@ void IntrinsicCodeGeneratorX86_64::VisitStringEquals(HInvoke* invoke) {
     // All string objects must have the same type since String cannot be subclassed.
     // Receiver must be a string object, so its class field is equal to all strings' class fields.
     // If the argument is a string object, its class field must be equal to receiver's class field.
+    //
+    // As the String class is expected to be non-movable, we can read the class
+    // field from String.equals' arguments without read barriers.
+    AssertNonMovableStringClass();
+    // Also, because we use the loaded class references only to compare them, we
+    // don't need to unpoison them.
+    // /* HeapReference<Class> */ rcx = str->klass_
     __ movl(rcx, Address(str, class_offset));
+    // if (rcx != /* HeapReference<Class> */ arg->klass_) return false
     __ cmpl(rcx, Address(arg, class_offset));
     __ j(kNotEqual, &return_false);
   }