JIT: Don't update the dex cache of another class loader.
This only works for properly delegating class loaders. But Java allows
non-delegating ones :(
bug:29964720
test:612-jit-dex-cache
(cherry picked from commit 491617a612c8a1c890e72d8ba780a151ddee8e11)
(cherry picked from commit a2d7cbb44e570ec0a9064dc93f57441e6ab9e45a)
(cherry picked from commit ae093d69f58c2b6257c3e5b82a32c135a1f33641)
Change-Id: I73df55286a9bdb3f96963e0756e3a8b784aa9077
diff --git a/compiler/optimizing/inliner.cc b/compiler/optimizing/inliner.cc
index 59de895..834d981 100644
--- a/compiler/optimizing/inliner.cc
+++ b/compiler/optimizing/inliner.cc
@@ -185,17 +185,6 @@
}
}
-static uint32_t FindMethodIndexIn(ArtMethod* method,
- const DexFile& dex_file,
- uint32_t referrer_index)
- SHARED_REQUIRES(Locks::mutator_lock_) {
- if (IsSameDexFile(*method->GetDexFile(), dex_file)) {
- return method->GetDexMethodIndex();
- } else {
- return method->FindDexMethodIndexInOtherDexFile(dex_file, referrer_index);
- }
-}
-
static uint32_t FindClassIndexIn(mirror::Class* cls,
const DexFile& dex_file,
Handle<mirror::DexCache> dex_cache)
@@ -208,12 +197,8 @@
DCHECK(cls->IsProxyClass()) << PrettyClass(cls);
// TODO: deal with proxy classes.
} else if (IsSameDexFile(cls->GetDexFile(), dex_file)) {
+ DCHECK_EQ(cls->GetDexCache(), dex_cache.Get());
index = cls->GetDexTypeIndex();
- } else {
- index = cls->FindTypeIndexInOtherDexFile(dex_file);
- }
-
- if (index != DexFile::kDexNoIndex) {
// Update the dex cache to ensure the class is in. The generated code will
// consider it is. We make it safe by updating the dex cache, as other
// dex files might also load the class, and there is no guarantee the dex
@@ -221,6 +206,14 @@
if (dex_cache->GetResolvedType(index) == nullptr) {
dex_cache->SetResolvedType(index, cls);
}
+ } else {
+ index = cls->FindTypeIndexInOtherDexFile(dex_file);
+ // We cannot guarantee the entry in the dex cache will resolve to the same class,
+ // as there may be different class loaders. So only return the index if it's
+ // the right class in the dex cache already.
+ if (index != DexFile::kDexNoIndex && dex_cache->GetResolvedType(index) != cls) {
+ index = DexFile::kDexNoIndex;
+ }
}
return index;
@@ -273,7 +266,7 @@
return false;
}
MethodReference ref = invoke_instruction->AsInvokeStaticOrDirect()->GetTargetMethod();
- mirror::DexCache* const dex_cache = (&caller_dex_file == ref.dex_file)
+ mirror::DexCache* const dex_cache = IsSameDexFile(caller_dex_file, *ref.dex_file)
? caller_compilation_unit_.GetDexCache().Get()
: class_linker->FindDexCache(soa.Self(), *ref.dex_file);
resolved_method = dex_cache->GetResolvedMethod(
@@ -763,8 +756,6 @@
bool HInliner::TryBuildAndInline(HInvoke* invoke_instruction,
ArtMethod* method,
HInstruction** return_replacement) {
- const DexFile& caller_dex_file = *caller_compilation_unit_.GetDexFile();
-
if (method->IsProxyMethod()) {
VLOG(compiler) << "Method " << PrettyMethod(method)
<< " is not inlined because of unimplemented inline support for proxy methods.";
@@ -787,15 +778,6 @@
return false;
}
- uint32_t method_index = FindMethodIndexIn(
- method, caller_dex_file, invoke_instruction->GetDexMethodIndex());
- if (method_index == DexFile::kDexNoIndex) {
- VLOG(compiler) << "Call to "
- << PrettyMethod(method)
- << " cannot be inlined because unaccessible to caller";
- return false;
- }
-
bool same_dex_file = IsSameDexFile(*outer_compilation_unit_.GetDexFile(), *method->GetDexFile());
const DexFile::CodeItem* code_item = method->GetCodeItem();
@@ -832,7 +814,7 @@
if (Runtime::Current()->UseJitCompilation() ||
!compiler_driver_->IsMethodVerifiedWithoutFailures(
method->GetDexMethodIndex(), class_def_idx, *method->GetDexFile())) {
- VLOG(compiler) << "Method " << PrettyMethod(method_index, caller_dex_file)
+ VLOG(compiler) << "Method " << PrettyMethod(method)
<< " couldn't be verified, so it cannot be inlined";
return false;
}
@@ -842,7 +824,7 @@
invoke_instruction->AsInvokeStaticOrDirect()->IsStaticWithImplicitClinitCheck()) {
// Case of a static method that cannot be inlined because it implicitly
// requires an initialization check of its declaring class.
- VLOG(compiler) << "Method " << PrettyMethod(method_index, caller_dex_file)
+ VLOG(compiler) << "Method " << PrettyMethod(method)
<< " is not inlined because it is static and requires a clinit"
<< " check that cannot be emitted due to Dex cache limitations";
return false;
@@ -852,7 +834,7 @@
return false;
}
- VLOG(compiler) << "Successfully inlined " << PrettyMethod(method_index, caller_dex_file);
+ VLOG(compiler) << "Successfully inlined " << PrettyMethod(method);
MaybeRecordStat(kInlinedInvoke);
return true;
}
diff --git a/test/604-hot-static-interface/hot_static_interface.cc b/test/604-hot-static-interface/hot_static_interface.cc
deleted file mode 100644
index 9c51ca6..0000000
--- a/test/604-hot-static-interface/hot_static_interface.cc
+++ /dev/null
@@ -1,57 +0,0 @@
-/*
- * Copyright (C) 2016 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.
- */
-
-#include "art_method.h"
-#include "jit/jit.h"
-#include "jit/jit_code_cache.h"
-#include "oat_quick_method_header.h"
-#include "scoped_thread_state_change.h"
-#include "ScopedUtfChars.h"
-#include "stack_map.h"
-
-namespace art {
-
-extern "C" JNIEXPORT void JNICALL Java_Main_waitUntilJitted(JNIEnv* env,
- jclass,
- jclass itf,
- jstring method_name) {
- jit::Jit* jit = Runtime::Current()->GetJit();
- if (jit == nullptr) {
- return;
- }
-
- ScopedObjectAccess soa(Thread::Current());
-
- ScopedUtfChars chars(env, method_name);
- CHECK(chars.c_str() != nullptr);
-
- mirror::Class* klass = soa.Decode<mirror::Class*>(itf);
- ArtMethod* method = klass->FindDeclaredDirectMethodByName(chars.c_str(), sizeof(void*));
-
- jit::JitCodeCache* code_cache = jit->GetCodeCache();
- OatQuickMethodHeader* header = nullptr;
- while (true) {
- header = OatQuickMethodHeader::FromEntryPoint(method->GetEntryPointFromQuickCompiledCode());
- if (code_cache->ContainsPc(header->GetCode())) {
- break;
- } else {
- // yield to scheduler to give time to the JIT compiler.
- sched_yield();
- }
- }
-}
-
-} // namespace art
diff --git a/test/604-hot-static-interface/src/Main.java b/test/604-hot-static-interface/src/Main.java
index 559f15d..04d7cd6 100644
--- a/test/604-hot-static-interface/src/Main.java
+++ b/test/604-hot-static-interface/src/Main.java
@@ -22,14 +22,14 @@
Itf.foo(new Object());
}
- waitUntilJitted(Itf.class, "foo");
+ ensureJitCompiled(Itf.class, "foo");
if (!Itf.foo(new Object())) {
throw new Error("Unexpected result");
}
}
- private static native void waitUntilJitted(Class itf, String method_name);
+ private static native void ensureJitCompiled(Class itf, String method_name);
}
interface Itf {
diff --git a/test/612-jit-dex-cache/expected.txt b/test/612-jit-dex-cache/expected.txt
new file mode 100644
index 0000000..6a5618e
--- /dev/null
+++ b/test/612-jit-dex-cache/expected.txt
@@ -0,0 +1 @@
+JNI_OnLoad called
diff --git a/test/612-jit-dex-cache/info.txt b/test/612-jit-dex-cache/info.txt
new file mode 100644
index 0000000..e80f642
--- /dev/null
+++ b/test/612-jit-dex-cache/info.txt
@@ -0,0 +1,2 @@
+Regression test for the JIT compiler which used to
+wrongly update the dex cache of a class loader.
diff --git a/test/612-jit-dex-cache/src-ex/B.java b/test/612-jit-dex-cache/src-ex/B.java
new file mode 100644
index 0000000..4da9a1d
--- /dev/null
+++ b/test/612-jit-dex-cache/src-ex/B.java
@@ -0,0 +1,18 @@
+/*
+ * Copyright (C) 2016 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 B {
+}
diff --git a/test/612-jit-dex-cache/src-ex/LoadedByAppClassLoader.java b/test/612-jit-dex-cache/src-ex/LoadedByAppClassLoader.java
new file mode 100644
index 0000000..1d6158a
--- /dev/null
+++ b/test/612-jit-dex-cache/src-ex/LoadedByAppClassLoader.java
@@ -0,0 +1,36 @@
+/*
+ * Copyright (C) 2016 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 LoadedByAppClassLoader {
+ public static void letMeInlineYou(A a) {
+ a.foo();
+ }
+
+ public static ClassLoader areYouB() {
+ // Ensure letMeInlineYou is JITted and tries to do inlining of A.foo.
+ // The compiler used to wrongly update the dex cache of letMeInlineYou's
+ // class loader.
+ Main.ensureJitCompiled(LoadedByAppClassLoader.class, "letMeInlineYou");
+ return OtherClass.getB().getClassLoader();
+ }
+}
+
+class OtherClass {
+ public static Class getB() {
+ // This used to return the B class of another class loader.
+ return B.class;
+ }
+}
diff --git a/test/612-jit-dex-cache/src/A.java b/test/612-jit-dex-cache/src/A.java
new file mode 100644
index 0000000..415c712
--- /dev/null
+++ b/test/612-jit-dex-cache/src/A.java
@@ -0,0 +1,21 @@
+/*
+ * Copyright (C) 2016 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 A {
+ public int foo() {
+ return 42;
+ }
+}
diff --git a/test/612-jit-dex-cache/src/B.java b/test/612-jit-dex-cache/src/B.java
new file mode 100644
index 0000000..46c878b
--- /dev/null
+++ b/test/612-jit-dex-cache/src/B.java
@@ -0,0 +1,18 @@
+/*
+ * Copyright (C) 2016 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 B extends A {
+}
diff --git a/test/612-jit-dex-cache/src/Main.java b/test/612-jit-dex-cache/src/Main.java
new file mode 100644
index 0000000..0e4bd22
--- /dev/null
+++ b/test/612-jit-dex-cache/src/Main.java
@@ -0,0 +1,67 @@
+/*
+ * Copyright (C) 2016 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.
+ */
+
+import java.lang.reflect.Method;
+import java.lang.reflect.InvocationTargetException;
+
+import dalvik.system.PathClassLoader;
+
+// ClassLoader not delegating for non java. packages.
+class DelegateLastPathClassLoader extends PathClassLoader {
+
+ public DelegateLastPathClassLoader(String dexPath, ClassLoader parent) {
+ super(dexPath, parent);
+ }
+
+ @Override
+ protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundException {
+ if (!name.startsWith("java.")) {
+ try {
+ return findClass(name);
+ } catch (ClassNotFoundException ignore) {
+ // Ignore and fall through to parent class loader.
+ }
+ }
+ return super.loadClass(name, resolve);
+ }
+}
+
+public class Main {
+
+ private static Class classFromDifferentLoader() throws Exception {
+ final String DEX_FILE = System.getenv("DEX_LOCATION") + "/612-jit-dex-cache-ex.jar";
+ ClassLoader loader = new DelegateLastPathClassLoader(DEX_FILE, Main.class.getClassLoader());
+ return loader.loadClass("LoadedByAppClassLoader");
+ }
+
+ public static void main(String[] args) throws Exception {
+ System.loadLibrary(args[0]);
+ Class cls = classFromDifferentLoader();
+ Method m = cls.getDeclaredMethod("letMeInlineYou", A.class);
+ B b = new B();
+ // Invoke the method enough times to get an inline cache and get JITted.
+ for (int i = 0; i < 10000; ++i) {
+ m.invoke(null, b);
+ }
+ m = cls.getDeclaredMethod("areYouB", null);
+ ClassLoader loader = (ClassLoader) m.invoke(null);
+ if (loader != cls.getClassLoader()) {
+ throw new Error("Wrong class loader");
+ }
+ }
+
+ public static native void ensureJitCompiled(Class cls, String method_name);
+}
diff --git a/test/Android.libarttest.mk b/test/Android.libarttest.mk
index 6d40cdf..5b1af27 100644
--- a/test/Android.libarttest.mk
+++ b/test/Android.libarttest.mk
@@ -44,8 +44,7 @@
566-polymorphic-inlining/polymorphic_inline.cc \
570-checker-osr/osr.cc \
595-profile-saving/profile-saving.cc \
- 597-deopt-new-string/deopt.cc \
- 604-hot-static-interface/hot_static_interface.cc
+ 597-deopt-new-string/deopt.cc
ART_TARGET_LIBARTTEST_$(ART_PHONY_TEST_TARGET_SUFFIX) += $(ART_TARGET_TEST_OUT)/$(TARGET_ARCH)/libarttest.so
ART_TARGET_LIBARTTEST_$(ART_PHONY_TEST_TARGET_SUFFIX) += $(ART_TARGET_TEST_OUT)/$(TARGET_ARCH)/libarttestd.so
diff --git a/test/common/runtime_state.cc b/test/common/runtime_state.cc
index fd41fd2..e70a95c 100644
--- a/test/common/runtime_state.cc
+++ b/test/common/runtime_state.cc
@@ -18,10 +18,14 @@
#include "base/logging.h"
#include "dex_file-inl.h"
+#include "jit/jit.h"
+#include "jit/jit_code_cache.h"
#include "mirror/class-inl.h"
#include "nth_caller_visitor.h"
+#include "oat_quick_method_header.h"
#include "runtime.h"
#include "scoped_thread_state_change.h"
+#include "ScopedUtfChars.h"
#include "stack.h"
#include "thread-inl.h"
@@ -116,4 +120,38 @@
return JNI_TRUE;
}
+extern "C" JNIEXPORT void JNICALL Java_Main_ensureJitCompiled(JNIEnv* env,
+ jclass,
+ jclass cls,
+ jstring method_name) {
+ jit::Jit* jit = Runtime::Current()->GetJit();
+ if (jit == nullptr) {
+ return;
+ }
+
+ ScopedObjectAccess soa(Thread::Current());
+
+ ScopedUtfChars chars(env, method_name);
+ CHECK(chars.c_str() != nullptr);
+
+ mirror::Class* klass = soa.Decode<mirror::Class*>(cls);
+ ArtMethod* method = klass->FindDeclaredDirectMethodByName(chars.c_str(), sizeof(void*));
+
+ jit::JitCodeCache* code_cache = jit->GetCodeCache();
+ OatQuickMethodHeader* header = nullptr;
+ // Make sure there is a profiling info, required by the compiler.
+ ProfilingInfo::Create(soa.Self(), method, /* retry_allocation */ true);
+ while (true) {
+ header = OatQuickMethodHeader::FromEntryPoint(method->GetEntryPointFromQuickCompiledCode());
+ if (code_cache->ContainsPc(header->GetCode())) {
+ break;
+ } else {
+ // Sleep to yield to the compiler thread.
+ usleep(1000);
+ // Will either ensure it's compiled or do the compilation itself.
+ jit->CompileMethod(method, soa.Self(), /* osr */ false);
+ }
+ }
+}
+
} // namespace art
