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
diff --git a/compiler/optimizing/instruction_simplifier.cc b/compiler/optimizing/instruction_simplifier.cc
index 2bd2d5f..fbfee12 100644
--- a/compiler/optimizing/instruction_simplifier.cc
+++ b/compiler/optimizing/instruction_simplifier.cc
@@ -2024,6 +2024,20 @@
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 bdeb261..707ff34 100644
--- a/compiler/optimizing/intrinsics.h
+++ b/compiler/optimizing/intrinsics.h
@@ -203,6 +203,7 @@
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 ef85f9c..ca1b451 100644
--- a/compiler/optimizing/intrinsics_arm64.cc
+++ b/compiler/optimizing/intrinsics_arm64.cc
@@ -1519,6 +1519,13 @@
}
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 e0874d9..1d8ea09 100644
--- a/compiler/optimizing/intrinsics_arm_vixl.cc
+++ b/compiler/optimizing/intrinsics_arm_vixl.cc
@@ -1723,6 +1723,13 @@
}
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 4a8fbf2..140526a 100644
--- a/compiler/optimizing/intrinsics_mips.cc
+++ b/compiler/optimizing/intrinsics_mips.cc
@@ -2062,6 +2062,13 @@
// 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 512fb68..a58ff7c 100644
--- a/compiler/optimizing/intrinsics_mips64.cc
+++ b/compiler/optimizing/intrinsics_mips64.cc
@@ -1637,6 +1637,13 @@
// 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 8a0b6ae..baa410b 100644
--- a/compiler/optimizing/intrinsics_x86.cc
+++ b/compiler/optimizing/intrinsics_x86.cc
@@ -1345,6 +1345,13 @@
}
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 92ffda4..6dd8b8e 100644
--- a/compiler/optimizing/intrinsics_x86_64.cc
+++ b/compiler/optimizing/intrinsics_x86_64.cc
@@ -1520,6 +1520,13 @@
}
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 0000000..6a5618e
--- /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 0000000..1d3202ef
--- /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 0000000..d0ab6f8
--- /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 0000000..4badade
--- /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 60c7315..2b940ab 100644
--- a/test/common/runtime_state.cc
+++ b/test/common/runtime_state.cc
@@ -267,4 +267,13 @@
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