Handle quickened opcodes in instrumentation deopt.
Also update 602-deoptimizeable by removing frame assertions. These
are covered by 685-deoptimizeable.
Test: 602-deoptimizeable, 685-deoptimizeable, 687-deopt
bug: 115849764
Change-Id: Ibca3b49b22fa77541be5b972149618ce19842af9
diff --git a/runtime/instrumentation.cc b/runtime/instrumentation.cc
index 5c7b0ae..ee6f479 100644
--- a/runtime/instrumentation.cc
+++ b/runtime/instrumentation.cc
@@ -1360,38 +1360,56 @@
: StackVisitor(thread, nullptr, StackVisitor::StackWalkKind::kIncludeInlinedFrames),
shorty('V') {}
+ static uint16_t GetMethodIndexOfInvoke(ArtMethod* caller,
+ const Instruction& inst,
+ uint32_t dex_pc)
+ REQUIRES_SHARED(Locks::mutator_lock_) {
+ switch (inst.Opcode()) {
+ case Instruction::INVOKE_VIRTUAL_RANGE_QUICK:
+ case Instruction::INVOKE_VIRTUAL_QUICK: {
+ uint16_t method_idx = caller->GetIndexFromQuickening(dex_pc);
+ CHECK_NE(method_idx, DexFile::kDexNoIndex16);
+ return method_idx;
+ }
+ default: {
+ return inst.VRegB();
+ }
+ }
+ }
+
bool VisitFrame() override REQUIRES_SHARED(Locks::mutator_lock_) {
ArtMethod* m = GetMethod();
- if (m != nullptr && !m->IsRuntimeMethod()) {
- // The first Java method.
- if (m->IsNative()) {
- // Use JNI method's shorty for the jni stub.
- shorty = m->GetShorty()[0];
- return false;
- }
- if (m->IsProxyMethod()) {
- // Proxy method just invokes its proxied method via
- // art_quick_proxy_invoke_handler.
- shorty = m->GetInterfaceMethodIfProxy(kRuntimePointerSize)->GetShorty()[0];
- return false;
- }
+ if (m == nullptr || m->IsRuntimeMethod()) {
+ return true;
+ }
+ // The first Java method.
+ if (m->IsNative()) {
+ // Use JNI method's shorty for the jni stub.
+ shorty = m->GetShorty()[0];
+ } else if (m->IsProxyMethod()) {
+ // Proxy method just invokes its proxied method via
+ // art_quick_proxy_invoke_handler.
+ shorty = m->GetInterfaceMethodIfProxy(kRuntimePointerSize)->GetShorty()[0];
+ } else {
const Instruction& instr = m->DexInstructions().InstructionAt(GetDexPc());
if (instr.IsInvoke()) {
+ uint16_t method_index = GetMethodIndexOfInvoke(m, instr, GetDexPc());
const DexFile* dex_file = m->GetDexFile();
- if (interpreter::IsStringInit(dex_file, instr.VRegB())) {
+ if (interpreter::IsStringInit(dex_file, method_index)) {
// Invoking string init constructor is turned into invoking
// StringFactory.newStringFromChars() which returns a string.
shorty = 'L';
- return false;
+ } else {
+ shorty = dex_file->GetMethodShorty(method_index)[0];
}
- // A regular invoke, use callee's shorty.
- uint32_t method_idx = instr.VRegB();
- shorty = dex_file->GetMethodShorty(method_idx)[0];
+ } else {
+ // It could be that a non-invoke opcode invokes a stub, which in turn
+ // invokes Java code. In such cases, we should never expect a return
+ // value from the stub.
}
- // Stop stack walking since we've seen a Java frame.
- return false;
}
- return true;
+ // Stop stack walking since we've seen a Java frame.
+ return false;
}
char shorty;
diff --git a/test/602-deoptimizeable/info.txt b/test/602-deoptimizeable/info.txt
index 4b6147f..d0952f9 100644
--- a/test/602-deoptimizeable/info.txt
+++ b/test/602-deoptimizeable/info.txt
@@ -1,8 +1 @@
Test various cases for full/partial-fragment deoptimization.
-
-TODO: we should remove this test as its expectations at point of
-writing was that debuggable apps could run un-deoptimizeable frames
-from the boot image. Today, we deoptimize the boot image as soon as
-we see the app being debuggable. Test 685-deoptimizeable is the proper
-version of this test, but we currently keep the 602 version around to
-try diagnosing a gcstress issue.
diff --git a/test/602-deoptimizeable/src/Main.java b/test/602-deoptimizeable/src/Main.java
index 7a3285d..46584b0 100644
--- a/test/602-deoptimizeable/src/Main.java
+++ b/test/602-deoptimizeable/src/Main.java
@@ -33,10 +33,7 @@
public int hashCode() {
sHashCodeInvoked = true;
- Main.assertIsManaged();
Main.deoptimizeAll();
- Main.assertIsInterpreted();
- Main.assertCallerIsManaged(); // Caller is from framework code HashMap.
return i % 64;
}
}
@@ -46,13 +43,6 @@
public static native void deoptimizeAll();
public static native void undeoptimizeAll();
- public static native void assertIsInterpreted();
- public static native void assertIsManaged();
- public static native void assertCallerIsInterpreted();
- public static native void assertCallerIsManaged();
- public static native void disableStackFrameAsserts();
- public static native boolean hasOatFile();
- public static native boolean isInterpreted();
public static void execute(Runnable runnable) throws Exception {
Thread t = new Thread(runnable);
@@ -62,19 +52,13 @@
public static void main(String[] args) throws Exception {
System.loadLibrary(args[0]);
- // TODO: Stack frame assertions are irrelevant in this test as we now
- // always run JIT with debuggable. 685-deoptimizeable is the proper version
- // of this test, but we keep this version around to diagnose a gcstress issue.
- disableStackFrameAsserts();
final HashMap<DummyObject, Long> map = new HashMap<DummyObject, Long>();
// Single-frame deoptimization that covers partial fragment.
execute(new Runnable() {
public void run() {
int[] arr = new int[3];
- assertIsManaged();
int res = $noinline$run1(arr);
- assertIsManaged(); // Only single frame is deoptimized.
if (res != 79) {
System.out.println("Failure 1!");
System.exit(0);
@@ -87,13 +71,11 @@
public void run() {
try {
int[] arr = new int[3];
- assertIsManaged();
// Use reflection to call $noinline$run2 so that it does
// full-fragment deoptimization since that is an upcall.
Class<?> cls = Class.forName("Main");
Method method = cls.getDeclaredMethod("$noinline$run2", int[].class);
double res = (double)method.invoke(Main.class, arr);
- assertIsManaged(); // Only single frame is deoptimized.
if (res != 79.3d) {
System.out.println("Failure 2!");
System.exit(0);
@@ -107,9 +89,7 @@
// Full-fragment deoptimization.
execute(new Runnable() {
public void run() {
- assertIsManaged();
float res = $noinline$run3B();
- assertIsInterpreted(); // Every deoptimizeable method is deoptimized.
if (res != 0.034f) {
System.out.println("Failure 3!");
System.exit(0);
@@ -123,9 +103,7 @@
execute(new Runnable() {
public void run() {
try {
- assertIsManaged();
map.put(new DummyObject(10), Long.valueOf(100));
- assertIsInterpreted(); // Every deoptimizeable method is deoptimized.
if (map.get(new DummyObject(10)) == null) {
System.out.println("Expected map to contain DummyObject(10)");
}
@@ -147,7 +125,6 @@
}
public static int $noinline$run1(int[] arr) {
- assertIsManaged();
// Prevent inlining.
if (sFlag) {
throw new Error();
@@ -161,18 +138,15 @@
// This causes AIOOBE and triggers deoptimization from compiled code.
arr[3] = 1;
} catch (ArrayIndexOutOfBoundsException e) {
- assertIsInterpreted(); // Single-frame deoptimization triggered.
caught = true;
}
if (!caught) {
System.out.println("Expected exception");
}
- assertIsInterpreted();
return 79;
}
public static double $noinline$run2(int[] arr) {
- assertIsManaged();
// Prevent inlining.
if (sFlag) {
throw new Error();
@@ -186,37 +160,30 @@
// This causes AIOOBE and triggers deoptimization from compiled code.
arr[3] = 1;
} catch (ArrayIndexOutOfBoundsException e) {
- assertIsInterpreted(); // Single-frame deoptimization triggered.
caught = true;
}
if (!caught) {
System.out.println("Expected exception");
}
- assertIsInterpreted();
return 79.3d;
}
public static float $noinline$run3A() {
- assertIsManaged();
// Prevent inlining.
if (sFlag) {
throw new Error();
}
// Deoptimize callers.
deoptimizeAll();
- assertIsInterpreted();
- assertCallerIsInterpreted(); // $noinline$run3B is deoptimizeable.
return 0.034f;
}
public static float $noinline$run3B() {
- assertIsManaged();
// Prevent inlining.
if (sFlag) {
throw new Error();
}
float res = $noinline$run3A();
- assertIsInterpreted();
return res;
}
}
diff --git a/test/687-deopt/expected.txt b/test/687-deopt/expected.txt
new file mode 100644
index 0000000..6a5618e
--- /dev/null
+++ b/test/687-deopt/expected.txt
@@ -0,0 +1 @@
+JNI_OnLoad called
diff --git a/test/687-deopt/info.txt b/test/687-deopt/info.txt
new file mode 100644
index 0000000..ef56f51
--- /dev/null
+++ b/test/687-deopt/info.txt
@@ -0,0 +1,2 @@
+Regression test for instrumentation deopt, which previously did not expect a
+quickened instruction when returning from instrumentation stub.
diff --git a/test/687-deopt/src/Main.java b/test/687-deopt/src/Main.java
new file mode 100644
index 0000000..afe90d6
--- /dev/null
+++ b/test/687-deopt/src/Main.java
@@ -0,0 +1,53 @@
+/*
+ * 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.util.HashMap;
+
+public class Main {
+ public static void main(String[] args) {
+ System.loadLibrary(args[0]);
+
+ // Jit compile HashMap.hash method, so that instrumentation stubs
+ // will deoptimize it.
+ ensureJitCompiled(HashMap.class, "hash");
+
+ Main key = new Main();
+ Integer value = new Integer(10);
+ HashMap<Main, Integer> map = new HashMap<>();
+ map.put(key, value);
+ Integer res = map.get(key);
+ if (!value.equals(res)) {
+ throw new Error("Expected 10, got " + res);
+ }
+ }
+
+ public int hashCode() {
+ // The call stack at this point is:
+ // Main.main
+ // HashMap.put
+ // HashMap.hash
+ // Main.hashCode
+ //
+ // The opcode at HashMap.hash is invoke-virtual-quick which the
+ // instrumentation code did not expect and used to fetch the wrong
+ // method index for it.
+ deoptimizeAll();
+ return 42;
+ }
+
+ public static native void deoptimizeAll();
+ public static native void ensureJitCompiled(Class<?> cls, String methodName);
+}
diff --git a/test/knownfailures.json b/test/knownfailures.json
index d831993..7b40160 100644
--- a/test/knownfailures.json
+++ b/test/knownfailures.json
@@ -948,6 +948,7 @@
"685-deoptimizeable",
"685-shifts",
"686-get-this",
+ "687-deopt",
"706-checker-scheduler",
"707-checker-invalid-profile",
"714-invoke-custom-lambda-metafactory",