Revert "Revert "Fix ArtMethod::GetInvokeType for static methods on interfaces.""
bug:29089267
bug:27521545
We were hitting a compiler DCHECK that a class would never require to
do access checks on itself. The reason was that the compiler driver
was not trying to resolve a type, but instead relied on the verifier
for pre-populating the dex cache. However, the verifier doesn't
necessarily run in JIT mode.
This reverts commit 12abcbd950bd0ff4528e2e0d27ca5e881c7b0467.
Change-Id: I59204c16927084f6605a2a3f999ca529f949e1ad
diff --git a/compiler/optimizing/optimizing_compiler.cc b/compiler/optimizing/optimizing_compiler.cc
index 37197af..c9a4bfe 100644
--- a/compiler/optimizing/optimizing_compiler.cc
+++ b/compiler/optimizing/optimizing_compiler.cc
@@ -321,6 +321,7 @@
jobject class_loader,
const DexFile& dex_file,
Handle<mirror::DexCache> dex_cache,
+ ArtMethod* method,
bool osr) const;
std::unique_ptr<OptimizingCompilerStats> compilation_stats_;
@@ -614,6 +615,7 @@
jobject class_loader,
const DexFile& dex_file,
Handle<mirror::DexCache> dex_cache,
+ ArtMethod* method,
bool osr) const {
MaybeRecordStat(MethodCompilationStat::kAttemptCompilation);
CompilerDriver* compiler_driver = GetCompilerDriver();
@@ -679,17 +681,30 @@
osr);
const uint8_t* interpreter_metadata = nullptr;
- {
+ if (method == nullptr) {
ScopedObjectAccess soa(Thread::Current());
StackHandleScope<1> hs(soa.Self());
Handle<mirror::ClassLoader> loader(hs.NewHandle(
soa.Decode<mirror::ClassLoader*>(class_loader)));
- ArtMethod* art_method = compiler_driver->ResolveMethod(
+ method = compiler_driver->ResolveMethod(
soa, dex_cache, loader, &dex_compilation_unit, method_idx, invoke_type);
- // We may not get a method, for example if its class is erroneous.
- if (art_method != nullptr) {
- graph->SetArtMethod(art_method);
- interpreter_metadata = art_method->GetQuickenedInfo();
+ }
+ // For AOT compilation, we may not get a method, for example if its class is erroneous.
+ // JIT should always have a method.
+ DCHECK(Runtime::Current()->IsAotCompiler() || method != nullptr);
+ if (method != nullptr) {
+ graph->SetArtMethod(method);
+ ScopedObjectAccess soa(Thread::Current());
+ interpreter_metadata = method->GetQuickenedInfo();
+ uint16_t type_index = method->GetDeclaringClass()->GetDexTypeIndex();
+
+ // Update the dex cache if the type is not in it yet. Note that under AOT,
+ // the verifier must have set it, but under JIT, there's no guarantee, as we
+ // don't necessarily run the verifier.
+ // The compiler and the compiler driver assume the compiling class is
+ // in the dex cache.
+ if (dex_cache->GetResolvedType(type_index) == nullptr) {
+ dex_cache->SetResolvedType(type_index, method->GetDeclaringClass());
}
}
@@ -798,6 +813,7 @@
jclass_loader,
dex_file,
dex_cache,
+ nullptr,
/* osr */ false));
if (codegen.get() != nullptr) {
MaybeRecordStat(MethodCompilationStat::kCompiled);
@@ -884,6 +900,7 @@
jclass_loader,
*dex_file,
dex_cache,
+ method,
osr));
if (codegen.get() == nullptr) {
return false;
diff --git a/runtime/art_method.cc b/runtime/art_method.cc
index 1790df6..f86cb13 100644
--- a/runtime/art_method.cc
+++ b/runtime/art_method.cc
@@ -79,10 +79,10 @@
InvokeType ArtMethod::GetInvokeType() {
// TODO: kSuper?
- if (GetDeclaringClass()->IsInterface()) {
- return kInterface;
- } else if (IsStatic()) {
+ if (IsStatic()) {
return kStatic;
+ } else if (GetDeclaringClass()->IsInterface()) {
+ return kInterface;
} else if (IsDirect()) {
return kDirect;
} else {
diff --git a/test/604-hot-static-interface/build b/test/604-hot-static-interface/build
new file mode 100755
index 0000000..1ca2daf
--- /dev/null
+++ b/test/604-hot-static-interface/build
@@ -0,0 +1,27 @@
+#!/bin/bash
+#
+# Copyright 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.
+
+# make us exit on a failure
+set -e
+
+if [[ $@ != *"--jvm"* ]]; then
+ # Don't do anything with jvm
+ # Hard-wired use of experimental jack.
+ # TODO: fix this temporary work-around for default-methods, see b/19467889
+ export USE_JACK=true
+fi
+
+./default-build "$@" --experimental default-methods
diff --git a/test/604-hot-static-interface/expected.txt b/test/604-hot-static-interface/expected.txt
new file mode 100644
index 0000000..6a5618e
--- /dev/null
+++ b/test/604-hot-static-interface/expected.txt
@@ -0,0 +1 @@
+JNI_OnLoad called
diff --git a/test/604-hot-static-interface/hot_static_interface.cc b/test/604-hot-static-interface/hot_static_interface.cc
new file mode 100644
index 0000000..9c51ca6
--- /dev/null
+++ b/test/604-hot-static-interface/hot_static_interface.cc
@@ -0,0 +1,57 @@
+/*
+ * 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/info.txt b/test/604-hot-static-interface/info.txt
new file mode 100644
index 0000000..bc00bda
--- /dev/null
+++ b/test/604-hot-static-interface/info.txt
@@ -0,0 +1,2 @@
+Regression test for the JIT that used to crash when compiling
+a static method of an interface.
diff --git a/test/604-hot-static-interface/src/Main.java b/test/604-hot-static-interface/src/Main.java
new file mode 100644
index 0000000..559f15d
--- /dev/null
+++ b/test/604-hot-static-interface/src/Main.java
@@ -0,0 +1,39 @@
+/*
+ * 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 Main {
+ public static void main(String[] args) throws Exception {
+ System.loadLibrary(args[0]);
+ // Loop enough to get Itf.foo JITed.
+ for (int i = 0; i < 100000; i++) {
+ Itf.foo(new Object());
+ }
+
+ waitUntilJitted(Itf.class, "foo");
+
+ if (!Itf.foo(new Object())) {
+ throw new Error("Unexpected result");
+ }
+ }
+
+ private static native void waitUntilJitted(Class itf, String method_name);
+}
+
+interface Itf {
+ public static boolean foo(Object o) {
+ return o.equals(o);
+ }
+}
diff --git a/test/Android.libarttest.mk b/test/Android.libarttest.mk
index 97204d3..2e6359b 100644
--- a/test/Android.libarttest.mk
+++ b/test/Android.libarttest.mk
@@ -45,7 +45,8 @@
570-checker-osr/osr.cc \
595-profile-saving/profile-saving.cc \
596-app-images/app_images.cc \
- 597-deopt-new-string/deopt.cc
+ 597-deopt-new-string/deopt.cc \
+ 604-hot-static-interface/hot_static_interface.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
