Fix String.equals() for moveable String.class.

If the String.class is moveable (i.e. running without boot
image), the instanceof check emitted by the JIT in the
String.equals() intrinsic would require read barriers.
As we do not really care about the performance of running
without the boot image, disable the intrinsic in this case.

Test: 669-moveable-string-class-equals (--jit)
Bug: 68181300
Change-Id: I39c9f9935e0482b3b30f1ae5cd23515cbda0603b

13 files changed, 166 insertions, 0 deletions

diff --git a/compiler/optimizing/instruction_simplifier.cc b/compiler/optimizing/instruction_simplifier.cc
index 2bd2d5f0a1..fbfee12be9 100644
--- a/compiler/optimizing/instruction_simplifier.cc
+++ b/compiler/optimizing/instruction_simplifier.cc
@@ -2024,6 +2024,20 @@ void InstructionSimplifierVisitor::SimplifyStringEquals(HInvoke* instruction) {
     ReferenceTypeInfo argument_rti = argument->GetReferenceTypeInfo();
     if (argument_rti.IsValid() && argument_rti.IsStringClass()) {
       optimizations.SetArgumentIsString();
+    } else if (kUseReadBarrier) {
+      DCHECK(instruction->GetResolvedMethod() != nullptr);
+      DCHECK(instruction->GetResolvedMethod()->GetDeclaringClass()->IsStringClass());
+      Runtime* runtime = Runtime::Current();
+      // For AOT, we always assume that the boot image shall contain the String.class and
+      // we do not need a read barrier for boot image classes as they are non-moveable.
+      // For JIT, check if we actually have a boot image; if we do, the String.class
+      // should also be non-moveable.
+      if (runtime->IsAotCompiler() || runtime->GetHeap()->HasBootImageSpace()) {
+        DCHECK(runtime->IsAotCompiler() ||
+               !runtime->GetHeap()->IsMovableObject(
+                   instruction->GetResolvedMethod()->GetDeclaringClass()));
+        optimizations.SetNoReadBarrierForStringClass();
+      }
     }
   }
 }
diff --git a/compiler/optimizing/intrinsics.h b/compiler/optimizing/intrinsics.h
index bdeb261dbe..707ff3408e 100644
--- a/compiler/optimizing/intrinsics.h
+++ b/compiler/optimizing/intrinsics.h
@@ -203,6 +203,7 @@ class StringEqualsOptimizations : public IntrinsicOptimizations {
 
   INTRINSIC_OPTIMIZATION(ArgumentNotNull, 0);
   INTRINSIC_OPTIMIZATION(ArgumentIsString, 1);
+  INTRINSIC_OPTIMIZATION(NoReadBarrierForStringClass, 2);
 
  private:
   DISALLOW_COPY_AND_ASSIGN(StringEqualsOptimizations);
diff --git a/compiler/optimizing/intrinsics_arm64.cc b/compiler/optimizing/intrinsics_arm64.cc
index ef85f9ccc4..ca1b451e6b 100644
--- a/compiler/optimizing/intrinsics_arm64.cc
+++ b/compiler/optimizing/intrinsics_arm64.cc
@@ -1519,6 +1519,13 @@ static const char* GetConstString(HInstruction* candidate, uint32_t* utf16_lengt
 }
 
 void IntrinsicLocationsBuilderARM64::VisitStringEquals(HInvoke* invoke) {
+  if (kEmitCompilerReadBarrier &&
+      !StringEqualsOptimizations(invoke).GetArgumentIsString() &&
+      !StringEqualsOptimizations(invoke).GetNoReadBarrierForStringClass()) {
+    // No support for this odd case (String class is moveable, not in the boot image).
+    return;
+  }
+
   LocationSummary* locations =
       new (allocator_) LocationSummary(invoke, LocationSummary::kNoCall, kIntrinsified);
   locations->SetInAt(0, Location::RequiresRegister());
diff --git a/compiler/optimizing/intrinsics_arm_vixl.cc b/compiler/optimizing/intrinsics_arm_vixl.cc
index e0874d9549..1d8ea092a4 100644
--- a/compiler/optimizing/intrinsics_arm_vixl.cc
+++ b/compiler/optimizing/intrinsics_arm_vixl.cc
@@ -1723,6 +1723,13 @@ static const char* GetConstString(HInstruction* candidate, uint32_t* utf16_lengt
 }
 
 void IntrinsicLocationsBuilderARMVIXL::VisitStringEquals(HInvoke* invoke) {
+  if (kEmitCompilerReadBarrier &&
+      !StringEqualsOptimizations(invoke).GetArgumentIsString() &&
+      !StringEqualsOptimizations(invoke).GetNoReadBarrierForStringClass()) {
+    // No support for this odd case (String class is moveable, not in the boot image).
+    return;
+  }
+
   LocationSummary* locations =
       new (allocator_) LocationSummary(invoke, LocationSummary::kNoCall, kIntrinsified);
   InvokeRuntimeCallingConventionARMVIXL calling_convention;
diff --git a/compiler/optimizing/intrinsics_mips.cc b/compiler/optimizing/intrinsics_mips.cc
index 4a8fbf26ce..140526a018 100644
--- a/compiler/optimizing/intrinsics_mips.cc
+++ b/compiler/optimizing/intrinsics_mips.cc
@@ -2062,6 +2062,13 @@ void IntrinsicCodeGeneratorMIPS::VisitStringCompareTo(HInvoke* invoke) {
 
 // boolean java.lang.String.equals(Object anObject)
 void IntrinsicLocationsBuilderMIPS::VisitStringEquals(HInvoke* invoke) {
+  if (kEmitCompilerReadBarrier &&
+      !StringEqualsOptimizations(invoke).GetArgumentIsString() &&
+      !StringEqualsOptimizations(invoke).GetNoReadBarrierForStringClass()) {
+    // No support for this odd case (String class is moveable, not in the boot image).
+    return;
+  }
+
   LocationSummary* locations =
       new (allocator_) LocationSummary(invoke, LocationSummary::kNoCall, kIntrinsified);
   locations->SetInAt(0, Location::RequiresRegister());
diff --git a/compiler/optimizing/intrinsics_mips64.cc b/compiler/optimizing/intrinsics_mips64.cc
index 512fb68fad..a58ff7c7f2 100644
--- a/compiler/optimizing/intrinsics_mips64.cc
+++ b/compiler/optimizing/intrinsics_mips64.cc
@@ -1637,6 +1637,13 @@ void IntrinsicCodeGeneratorMIPS64::VisitStringCompareTo(HInvoke* invoke) {
 
 // boolean java.lang.String.equals(Object anObject)
 void IntrinsicLocationsBuilderMIPS64::VisitStringEquals(HInvoke* invoke) {
+  if (kEmitCompilerReadBarrier &&
+      !StringEqualsOptimizations(invoke).GetArgumentIsString() &&
+      !StringEqualsOptimizations(invoke).GetNoReadBarrierForStringClass()) {
+    // No support for this odd case (String class is moveable, not in the boot image).
+    return;
+  }
+
   LocationSummary* locations =
       new (allocator_) LocationSummary(invoke, LocationSummary::kNoCall, kIntrinsified);
   locations->SetInAt(0, Location::RequiresRegister());
diff --git a/compiler/optimizing/intrinsics_x86.cc b/compiler/optimizing/intrinsics_x86.cc
index 8a0b6aeb0e..baa410b884 100644
--- a/compiler/optimizing/intrinsics_x86.cc
+++ b/compiler/optimizing/intrinsics_x86.cc
@@ -1345,6 +1345,13 @@ void IntrinsicCodeGeneratorX86::VisitStringCompareTo(HInvoke* invoke) {
 }
 
 void IntrinsicLocationsBuilderX86::VisitStringEquals(HInvoke* invoke) {
+  if (kEmitCompilerReadBarrier &&
+      !StringEqualsOptimizations(invoke).GetArgumentIsString() &&
+      !StringEqualsOptimizations(invoke).GetNoReadBarrierForStringClass()) {
+    // No support for this odd case (String class is moveable, not in the boot image).
+    return;
+  }
+
   LocationSummary* locations =
       new (allocator_) LocationSummary(invoke, LocationSummary::kNoCall, kIntrinsified);
   locations->SetInAt(0, Location::RequiresRegister());
diff --git a/compiler/optimizing/intrinsics_x86_64.cc b/compiler/optimizing/intrinsics_x86_64.cc
index 92ffda427b..6dd8b8e1f5 100644
--- a/compiler/optimizing/intrinsics_x86_64.cc
+++ b/compiler/optimizing/intrinsics_x86_64.cc
@@ -1520,6 +1520,13 @@ void IntrinsicCodeGeneratorX86_64::VisitStringCompareTo(HInvoke* invoke) {
 }
 
 void IntrinsicLocationsBuilderX86_64::VisitStringEquals(HInvoke* invoke) {
+  if (kEmitCompilerReadBarrier &&
+      !StringEqualsOptimizations(invoke).GetArgumentIsString() &&
+      !StringEqualsOptimizations(invoke).GetNoReadBarrierForStringClass()) {
+    // No support for this odd case (String class is moveable, not in the boot image).
+    return;
+  }
+
   LocationSummary* locations =
       new (allocator_) LocationSummary(invoke, LocationSummary::kNoCall, kIntrinsified);
   locations->SetInAt(0, Location::RequiresRegister());
diff --git a/test/669-moveable-string-class-equals/expected.txt b/test/669-moveable-string-class-equals/expected.txt
new file mode 100644
index 0000000000..6a5618ebc6
--- /dev/null
+++ b/test/669-moveable-string-class-equals/expected.txt
@@ -0,0 +1 @@
+JNI_OnLoad called
diff --git a/test/669-moveable-string-class-equals/info.txt b/test/669-moveable-string-class-equals/info.txt
new file mode 100644
index 0000000000..1d3202ef46
--- /dev/null
+++ b/test/669-moveable-string-class-equals/info.txt
@@ -0,0 +1,2 @@
+Regression test for String.equals() intrinsic instanceof check
+when the String.class is moveable.
diff --git a/test/669-moveable-string-class-equals/run b/test/669-moveable-string-class-equals/run
new file mode 100755
index 0000000000..d0ab6f8d55
--- /dev/null
+++ b/test/669-moveable-string-class-equals/run
@@ -0,0 +1,19 @@
+#!/bin/bash
+#
+# Copyright (C) 2017 The Android Open Source Project
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# Run without image, so that String.class is moveable.
+# Reduce heap size to force more frequent GCs.
+${RUN} --no-image --no-dex2oat --runtime-option -Xmx16m "$@"
diff --git a/test/669-moveable-string-class-equals/src/Main.java b/test/669-moveable-string-class-equals/src/Main.java
new file mode 100644
index 0000000000..4badadeae4
--- /dev/null
+++ b/test/669-moveable-string-class-equals/src/Main.java
@@ -0,0 +1,78 @@
+/*
+ * Copyright (C) 2017 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+public class Main {
+  public static void main(String[] args) {
+    System.loadLibrary(args[0]);
+    if (!hasJit()) {
+      // Make the test pass if not using JIT.
+      return;
+    }
+    if (hasImage()) {
+      throw new Error("The `run` script should prevent this test from running with an image!");
+    }
+    if (!isClassMoveable(String.class)) {
+      throw new Error("String.class not moveable despite running without image!");
+    }
+
+    // Make sure the Main.test() is JIT-compiled and then call it.
+    ensureJitCompiled(Main.class, "test");
+    test();
+  }
+
+  public static void test() {
+    int length = 5;
+
+    // Hide the type of these strings in an Object array,
+    // so that we treat them as Object for the String.equals() below.
+    Object[] array = new Object[length];
+    for (int i = 0; i != length; ++i) {
+      array[i] = "V" + i;
+    }
+
+    for (int count = 0; count != 128 * 1024; ++count) {
+      for (int i = 0; i != length; ++i) {
+        allocateAtLeast1KiB();
+        assertTrue(("V" + i).equals(array[i]));
+      }
+    }
+  }
+
+  public static void allocateAtLeast1KiB() {
+    // Give GC more work by allocating Object arrays.
+    memory[allocationIndex] = new Object[1024 / 4];
+    ++allocationIndex;
+    if (allocationIndex == memory.length) {
+      allocationIndex = 0;
+    }
+  }
+
+  public static void assertTrue(boolean value) {
+    if (!value) {
+      throw new Error("Assertion failed!");
+    }
+  }
+
+  private native static boolean hasJit();
+  private native static boolean hasImage();
+  private native static boolean isClassMoveable(Class<?> cls);
+  private static native void ensureJitCompiled(Class<?> itf, String method_name);
+
+  // We shall retain some allocated memory and release old allocations
+  // so that the GC has something to do.
+  public static Object[] memory = new Object[4096];
+  public static int allocationIndex = 0;
+}
diff --git a/test/common/runtime_state.cc b/test/common/runtime_state.cc
index 60c7315b6f..2b940ab7c7 100644
--- a/test/common/runtime_state.cc
+++ b/test/common/runtime_state.cc
@@ -267,4 +267,13 @@ extern "C" JNIEXPORT void JNICALL Java_Main_fetchProfiles(JNIEnv*, jclass) {
   code_cache->GetProfiledMethods(unused_locations, unused_vector);
 }
 
+extern "C" JNIEXPORT jboolean JNICALL Java_Main_isClassMoveable(JNIEnv*,
+                                                                jclass,
+                                                                jclass cls) {
+  Runtime* runtime = Runtime::Current();
+  ScopedObjectAccess soa(Thread::Current());
+  ObjPtr<mirror::Class> klass = soa.Decode<mirror::Class>(cls);
+  return runtime->GetHeap()->IsMovableObject(klass);
+}
+
 }  // namespace art