ART: No JIT for native MethodHandle/VarHandle methods
MethodHandle invocations can record JIT samples. This can lead to
transitioning the JNI stubs for MH.invoke and MH.invokeExact from
AOT compiled to JIT compiled and then to generic when there is a JIT
code cache collection. The transitions and differences in stack frame
representations can cause a crash during stack walking.
This only affects native invocations of handles which we do to raise
UnsuppportedOperationExceptions when invoked reflectively. Not
performance critical and not likely to be exercised in real code, but
leads to flakes in gcstress testing of 956-methodhandles.
Bug: 78151261
Test: art/test/run-test --host --64 --compact-dex-level fast --jit --no-relocate --pic-test 956
Test: art/test.py --host -r -t 716
Change-Id: Ie3fdbcfc4decb12814290bcce8d25c5e2fde87f1
diff --git a/runtime/jit/jit.cc b/runtime/jit/jit.cc
index d20f760..2c0fbad 100644
--- a/runtime/jit/jit.cc
+++ b/runtime/jit/jit.cc
@@ -29,6 +29,8 @@
#include "interpreter/interpreter.h"
#include "java_vm_ext.h"
#include "jit_code_cache.h"
+#include "mirror/method_handle_impl.h"
+#include "mirror/var_handle.h"
#include "oat_file_manager.h"
#include "oat_quick_method_header.h"
#include "profile/profile_compilation_info.h"
@@ -638,15 +640,33 @@
DISALLOW_IMPLICIT_CONSTRUCTORS(JitCompileTask);
};
+static bool IgnoreSamplesForMethod(ArtMethod* method) REQUIRES_SHARED(Locks::mutator_lock_) {
+ if (method->IsClassInitializer() || !method->IsCompilable()) {
+ // We do not want to compile such methods.
+ return true;
+ }
+ if (method->IsNative()) {
+ ObjPtr<mirror::Class> klass = method->GetDeclaringClass();
+ if (klass == mirror::MethodHandle::StaticClass() || klass == mirror::VarHandle::StaticClass()) {
+ // MethodHandle and VarHandle invocation methods are required to throw an
+ // UnsupportedOperationException if invoked reflectively. We achieve this by having native
+ // implementations that arise the exception. We need to disable JIT compilation of these JNI
+ // methods as it can lead to transitioning between JIT compiled JNI stubs and generic JNI
+ // stubs. Since these stubs have different stack representations we can then crash in stack
+ // walking (b/78151261).
+ return true;
+ }
+ }
+ return false;
+}
+
void Jit::AddSamples(Thread* self, ArtMethod* method, uint16_t count, bool with_backedges) {
if (thread_pool_ == nullptr) {
// Should only see this when shutting down.
DCHECK(Runtime::Current()->IsShuttingDown(self));
return;
}
-
- if (method->IsClassInitializer() || !method->IsCompilable()) {
- // We do not want to compile such methods.
+ if (IgnoreSamplesForMethod(method)) {
return;
}
if (hot_method_threshold_ == 0) {
diff --git a/test/716-jli-jit-samples/build b/test/716-jli-jit-samples/build
new file mode 100755
index 0000000..730a8a1
--- /dev/null
+++ b/test/716-jli-jit-samples/build
@@ -0,0 +1,20 @@
+#!/bin/bash
+#
+# Copyright 2018 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
+
+./default-build "$@" --experimental var-handles
diff --git a/test/716-jli-jit-samples/expected.txt b/test/716-jli-jit-samples/expected.txt
new file mode 100644
index 0000000..dc533f3
--- /dev/null
+++ b/test/716-jli-jit-samples/expected.txt
@@ -0,0 +1,3 @@
+JNI_OnLoad called
+MethodHandle OK
+VarHandle OK
diff --git a/test/716-jli-jit-samples/info.txt b/test/716-jli-jit-samples/info.txt
new file mode 100644
index 0000000..81a76f6
--- /dev/null
+++ b/test/716-jli-jit-samples/info.txt
@@ -0,0 +1,2 @@
+Test MethodHandle and VarHandle invokes do not accumulate JIT samples
+(regression test for b/78151261).
diff --git a/test/716-jli-jit-samples/src-art/Main.java b/test/716-jli-jit-samples/src-art/Main.java
new file mode 100644
index 0000000..def6b9f
--- /dev/null
+++ b/test/716-jli-jit-samples/src-art/Main.java
@@ -0,0 +1,142 @@
+/*
+ * Copyright (C) 2018 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.invoke.MethodHandle;
+import java.lang.invoke.MethodHandles;
+import java.lang.invoke.MethodType;
+import java.lang.invoke.VarHandle;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+
+public class Main {
+ private static final int ITERATIONS = 100;
+
+ private static final VarHandle widgetIdVarHandle;
+
+ public static native int getHotnessCounter(Class<?> cls, String methodName);
+
+ public static class Widget {
+ public Widget(int id) {
+ this.id = id;
+ }
+
+ int getId() {
+ return id;
+ }
+
+ int id;
+ }
+
+ static {
+ try {
+ widgetIdVarHandle = MethodHandles.lookup().findVarHandle(Widget.class, "id", int.class);
+ } catch (Exception e) {
+ throw new Error(e);
+ }
+ }
+
+ private static void assertEquals(int i1, int i2) {
+ if (i1 == i2) {
+ return;
+ }
+ throw new AssertionError("assertEquals i1: " + i1 + ", i2: " + i2);
+ }
+
+ private static void assertEquals(Object o, Object p) {
+ if (o == p) {
+ return;
+ }
+ if (o != null && p != null && o.equals(p)) {
+ return;
+ }
+ throw new AssertionError("assertEquals: o1: " + o + ", o2: " + p);
+ }
+
+ private static void fail() {
+ System.out.println("fail");
+ Thread.dumpStack();
+ }
+
+ private static void fail(String message) {
+ System.out.println("fail: " + message);
+ Thread.dumpStack();
+ }
+
+ private static void testMethodHandleCounters() throws Throwable {
+ for (int i = 0; i < ITERATIONS; ++i) {
+ // Regular MethodHandle invocations
+ MethodHandle mh =
+ MethodHandles.lookup()
+ .findConstructor(
+ Widget.class, MethodType.methodType(void.class, int.class));
+ Widget w = (Widget) mh.invoke(3);
+ w = (Widget) mh.invokeExact(3);
+ assertEquals(0, getHotnessCounter(MethodHandle.class, "invoke"));
+ assertEquals(0, getHotnessCounter(MethodHandle.class, "invokeExact"));
+
+ // Reflective MethodHandle invocations
+ String[] methodNames = {"invoke", "invokeExact"};
+ for (String methodName : methodNames) {
+ Method invokeMethod = MethodHandle.class.getMethod(methodName, Object[].class);
+ MethodHandle instance =
+ MethodHandles.lookup()
+ .findVirtual(
+ Widget.class, "getId", MethodType.methodType(int.class));
+ try {
+ invokeMethod.invoke(instance, new Object[] {new Object[] {}});
+ fail();
+ } catch (InvocationTargetException ite) {
+ assertEquals(ite.getCause().getClass(), UnsupportedOperationException.class);
+ }
+ }
+ assertEquals(0, getHotnessCounter(MethodHandle.class, "invoke"));
+ assertEquals(0, getHotnessCounter(MethodHandle.class, "invokeExact"));
+ }
+
+ System.out.println("MethodHandle OK");
+ }
+
+ private static void testVarHandleCounters() throws Throwable {
+ Widget w = new Widget(0);
+ for (int i = 0; i < ITERATIONS; ++i) {
+ // Regular accessor invocations
+ widgetIdVarHandle.set(w, i);
+ assertEquals(i, widgetIdVarHandle.get(w));
+ assertEquals(0, getHotnessCounter(VarHandle.class, "set"));
+ assertEquals(0, getHotnessCounter(VarHandle.class, "get"));
+
+ // Reflective accessor invocations
+ for (String accessorName : new String[] {"get", "set"}) {
+ Method setMethod = VarHandle.class.getMethod(accessorName, Object[].class);
+ try {
+ setMethod.invoke(widgetIdVarHandle, new Object[] {new Object[0]});
+ fail();
+ } catch (InvocationTargetException ite) {
+ assertEquals(ite.getCause().getClass(), UnsupportedOperationException.class);
+ }
+ }
+ assertEquals(0, getHotnessCounter(VarHandle.class, "set"));
+ assertEquals(0, getHotnessCounter(VarHandle.class, "get"));
+ }
+ System.out.println("VarHandle OK");
+ }
+
+ public static void main(String[] args) throws Throwable {
+ System.loadLibrary(args[0]);
+ testMethodHandleCounters();
+ testVarHandleCounters();
+ }
+}
diff --git a/test/common/runtime_state.cc b/test/common/runtime_state.cc
index a55cc79..f89888b 100644
--- a/test/common/runtime_state.cc
+++ b/test/common/runtime_state.cc
@@ -258,17 +258,23 @@
jclass,
jclass cls,
jstring method_name) {
- ArtMethod* method = nullptr;
- {
- ScopedObjectAccess soa(Thread::Current());
-
- ScopedUtfChars chars(env, method_name);
- CHECK(chars.c_str() != nullptr);
- method = soa.Decode<mirror::Class>(cls)->FindDeclaredDirectMethodByName(
- chars.c_str(), kRuntimePointerSize);
+ ScopedObjectAccess soa(Thread::Current());
+ ScopedUtfChars chars(env, method_name);
+ CHECK(chars.c_str() != nullptr);
+ ArtMethod* method =
+ soa.Decode<mirror::Class>(cls)->FindDeclaredDirectMethodByName(chars.c_str(),
+ kRuntimePointerSize);
+ if (method != nullptr) {
+ return method->GetCounter();
}
- return method->GetCounter();
+ method = soa.Decode<mirror::Class>(cls)->FindDeclaredVirtualMethodByName(chars.c_str(),
+ kRuntimePointerSize);
+ if (method != nullptr) {
+ return method->GetCounter();
+ }
+
+ return std::numeric_limits<int32_t>::min();
}
extern "C" JNIEXPORT int JNICALL Java_Main_numberOfDeoptimizations(JNIEnv*, jclass) {