Fix longstanding JIT bug in interaction with class initialization.
The entrypoint of a method of a class being initialized was
wrongly updated to pointing to compiled code when it should stay
the resolution stub.
Test: 694-clinit-jit
Bug: 119800099
Bug: 130337355
Change-Id: I1bd5769c950b62330d8ae5c34cc08111b2fb4c04
diff --git a/runtime/jit/jit_code_cache.cc b/runtime/jit/jit_code_cache.cc
index d3e4582..2052f14 100644
--- a/runtime/jit/jit_code_cache.cc
+++ b/runtime/jit/jit_code_cache.cc
@@ -1977,6 +1977,18 @@
return false;
}
+ ClassLinker* class_linker = Runtime::Current()->GetClassLinker();
+ if (class_linker->IsQuickResolutionStub(method->GetEntryPointFromQuickCompiledCode())) {
+ // We currently don't save the JIT compiled code if we cannot update the entrypoint due
+ // to having the resolution stub.
+ VLOG(jit) << "Not compiling "
+ << method->PrettyMethod()
+ << " because it has the resolution stub";
+ // Give it a new chance to be hot.
+ ClearMethodCounter(method, /*was_warm=*/ false);
+ return false;
+ }
+
MutexLock mu(self, lock_);
if (osr && (osr_code_map_.find(method) != osr_code_map_.end())) {
return false;
diff --git a/test/655-jit-clinit/src/Main.java b/test/655-jit-clinit/src/Main.java
index 7fe5084..2d43a39 100644
--- a/test/655-jit-clinit/src/Main.java
+++ b/test/655-jit-clinit/src/Main.java
@@ -20,7 +20,8 @@
if (!hasJit()) {
return;
}
- Foo.$noinline$hotMethod();
+ // Force initialization.
+ Foo.initialize();
}
public native static boolean hasJitCompiledEntrypoint(Class<?> cls, String methodName);
@@ -28,7 +29,9 @@
}
class Foo {
- static void $noinline$hotMethod() {
+ // This method needs to be virtual for the test. Otherwise if it's a static method,
+ // the JIT compiler won't compile while its entrypoint is the resolution stub.
+ void $noinline$hotMethod() {
for (int i = 0; i < array.length; ++i) {
array[i] = array;
}
@@ -37,7 +40,7 @@
static {
array = new Object[10000];
while (!Main.hasJitCompiledEntrypoint(Foo.class, "$noinline$hotMethod")) {
- Foo.$noinline$hotMethod();
+ new Foo().$noinline$hotMethod();
try {
// Sleep to give a chance for the JIT to compile `hotMethod`.
Thread.sleep(100);
@@ -47,5 +50,8 @@
}
}
+ static void initialize() {
+ }
+
static Object[] array;
}
diff --git a/test/694-clinit-jit/expected.txt b/test/694-clinit-jit/expected.txt
new file mode 100644
index 0000000..d81cc07
--- /dev/null
+++ b/test/694-clinit-jit/expected.txt
@@ -0,0 +1 @@
+42
diff --git a/test/694-clinit-jit/info.txt b/test/694-clinit-jit/info.txt
new file mode 100644
index 0000000..2b42644
--- /dev/null
+++ b/test/694-clinit-jit/info.txt
@@ -0,0 +1,3 @@
+Regression test for the JIT compiler which used to wrongly
+update the entrypoint of a method of class still being
+initialized.
diff --git a/test/694-clinit-jit/src/Main.java b/test/694-clinit-jit/src/Main.java
new file mode 100644
index 0000000..31435d4
--- /dev/null
+++ b/test/694-clinit-jit/src/Main.java
@@ -0,0 +1,89 @@
+/*
+ * Copyright (C) 2019 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 class InnerInitialized {
+ static int staticValue1 = 0;
+ static int staticValue2 = 1;
+
+ static int $noinline$runHotMethod(boolean doComputation) {
+ if (doComputation) {
+ for (int i = 0; i < 100000; i++) {
+ staticValue1 += staticValue2;
+ }
+ }
+ return staticValue1;
+ }
+
+ static {
+ // Make $noinline$runHotMethod hot so it gets compiled. The
+ // regression used to be that the JIT would incorrectly update the
+ // resolution entrypoint of the method. The entrypoint needs to stay
+ // the resolution entrypoint otherwise other threads may incorrectly
+ // execute static methods of the class while the class is still being
+ // initialized.
+ for (int i = 0; i < 10; i++) {
+ $noinline$runHotMethod(true);
+ }
+
+ // Start another thread that will invoke a static method of InnerInitialized.
+ new Thread(new Runnable() {
+ public void run() {
+ for (int i = 0; i < 100000; i++) {
+ $noinline$runInternalHotMethod(false);
+ }
+ // Give some time for the JIT compiler to compile $noinline$runInternalHotMethod.
+ // Only compiled code invoke the entrypoint of an ArtMethod.
+ try {
+ Thread.sleep(1000);
+ } catch (Exception e) {
+ }
+ int value = $noinline$runInternalHotMethod(true);
+ if (value != 42) {
+ throw new Error("Expected 42, got " + value);
+ }
+ }
+
+ public int $noinline$runInternalHotMethod(boolean invokeStaticMethod) {
+ if (invokeStaticMethod) {
+ // The bug used to be here: the compiled code of $noinline$runInternalHotMethod
+ // would invoke the entrypoint of $noinline$runHotMethod, which was incorrectly
+ // updated to the JIT entrypoint and therefore not hitting the resolution
+ // trampoline which would have waited for the class to be initialized.
+ return $noinline$runHotMethod(false);
+ }
+ return 0;
+ }
+
+ }).start();
+ // Give some time for the JIT compiler to compile runHotMethod, and also for the
+ // other thread to invoke $noinline$runHotMethod.
+ // This wait should be longer than the other thread's wait to make sure the other
+ // thread hits the $noinline$runHotMethod call before the initialization of
+ // InnerInitialized is finished.
+ try {
+ Thread.sleep(5000);
+ } catch (Exception e) {
+ }
+ staticValue1 = 42;
+ }
+ }
+
+ public static void main(String[] args) throws Exception {
+ System.out.println(InnerInitialized.staticValue1);
+ }
+}
diff --git a/test/common/runtime_state.cc b/test/common/runtime_state.cc
index 7bd5828..0031e14 100644
--- a/test/common/runtime_state.cc
+++ b/test/common/runtime_state.cc
@@ -218,6 +218,14 @@
ThrowIllegalStateException(msg.c_str());
return;
}
+ // We force initialization of the declaring class to make sure the method doesn't keep
+ // the resolution stub as entrypoint.
+ StackHandleScope<1> hs(self);
+ Handle<mirror::Class> h_klass(hs.NewHandle(method->GetDeclaringClass()));
+ if (!Runtime::Current()->GetClassLinker()->EnsureInitialized(self, h_klass, true, true)) {
+ self->AssertPendingException();
+ return;
+ }
}
jit::Jit* jit = GetJitIfEnabled();
jit::JitCodeCache* code_cache = jit->GetCodeCache();