From 7b9a83f327805defbdefcbea47be20da31c662db Mon Sep 17 00:00:00 2001 From: Mingyao Yang Date: Tue, 13 Dec 2016 12:28:31 -0800 Subject: [PATCH] Don't visit proxy methods in CHAStackVisitor::VisitFrame Proxy methods do not have an OatQuickMethodHeader. Test: test-art-host, launch com.azarlive.android Bug: 33471784 Change-Id: Idb660c78a8263501d068d8467476b0477d910393 --- runtime/cha.cc | 7 +- test/616-cha-regression-proxy-method/expected.txt | 1 + test/616-cha-regression-proxy-method/info.txt | 1 + test/616-cha-regression-proxy-method/src/Main.java | 131 +++++++++++++++++++++ test/616-cha/src/Main.java | 9 +- 5 files changed, 141 insertions(+), 8 deletions(-) create mode 100644 test/616-cha-regression-proxy-method/expected.txt create mode 100644 test/616-cha-regression-proxy-method/info.txt create mode 100644 test/616-cha-regression-proxy-method/src/Main.java diff --git a/runtime/cha.cc b/runtime/cha.cc index be675a82c..d94b091eb 100644 --- a/runtime/cha.cc +++ b/runtime/cha.cc @@ -100,7 +100,11 @@ class CHAStackVisitor FINAL : public StackVisitor { bool VisitFrame() OVERRIDE REQUIRES_SHARED(Locks::mutator_lock_) { ArtMethod* method = GetMethod(); - if (method == nullptr || method->IsRuntimeMethod() || method->IsNative()) { + // Avoid types of methods that do not have an oat quick method header. + if (method == nullptr || + method->IsRuntimeMethod() || + method->IsNative() || + method->IsProxyMethod()) { return true; } if (GetCurrentQuickFrame() == nullptr) { @@ -110,6 +114,7 @@ class CHAStackVisitor FINAL : public StackVisitor { // Method may have multiple versions of compiled code. Check // the method header to see if it has should_deoptimize flag. const OatQuickMethodHeader* method_header = GetCurrentOatQuickMethodHeader(); + DCHECK(method_header != nullptr); if (!method_header->HasShouldDeoptimizeFlag()) { // This compiled version doesn't have should_deoptimize flag. Skip. return true; diff --git a/test/616-cha-regression-proxy-method/expected.txt b/test/616-cha-regression-proxy-method/expected.txt new file mode 100644 index 000000000..6a5618ebc --- /dev/null +++ b/test/616-cha-regression-proxy-method/expected.txt @@ -0,0 +1 @@ +JNI_OnLoad called diff --git a/test/616-cha-regression-proxy-method/info.txt b/test/616-cha-regression-proxy-method/info.txt new file mode 100644 index 000000000..386a07f39 --- /dev/null +++ b/test/616-cha-regression-proxy-method/info.txt @@ -0,0 +1 @@ +Regression test for Class Hierarchy Analysis (CHA) on visiting proxy method frame. diff --git a/test/616-cha-regression-proxy-method/src/Main.java b/test/616-cha-regression-proxy-method/src/Main.java new file mode 100644 index 000000000..19c92be00 --- /dev/null +++ b/test/616-cha-regression-proxy-method/src/Main.java @@ -0,0 +1,131 @@ +/* + * 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.InvocationHandler; +import java.lang.reflect.Method; +import java.lang.reflect.Proxy; + +class Main1 { + void foo(int i) { + if (i != 1) { + printError("error1"); + } + } + + void printError(String msg) { + System.out.println(msg); + } +} + +class Main2 extends Main1 { + void foo(int i) { + if (i != 2) { + printError("error2"); + } + } +} + +class Proxied implements Runnable { + public void run() { + synchronized(Main.class) { + Main.sOtherThreadStarted = true; + // Wait for Main2 to be linked and deoptimization is triggered. + try { + Main.class.wait(); + } catch (Exception e) { + } + } + } +} + +class MyInvocationHandler implements InvocationHandler { + private final Proxied proxied; + + public MyInvocationHandler(Proxied proxied) { + this.proxied = proxied; + } + + public Object invoke(Object proxy, Method method, Object[] args) throws Throwable { + return method.invoke(proxied, args); + } +} + +public class Main { + static Main1 sMain1; + static Main1 sMain2; + static volatile boolean sOtherThreadStarted; + + // sMain1.foo() will be always be Main1.foo() before Main2 is loaded/linked. + // So sMain1.foo() can be devirtualized to Main1.foo() and be inlined. + // After Dummy.createMain2() which links in Main2, live testOverride() on stack + // should be deoptimized. + static void testOverride() { + sMain1.foo(sMain1.getClass() == Main1.class ? 1 : 2); + + // Wait for the other thread to start. + while (!sOtherThreadStarted); + // Create an Main2 instance and assign it to sMain2. + // sMain1 is kept the same. + sMain2 = Dummy.createMain2(); + // Wake up the other thread. + synchronized(Main.class) { + Main.class.notify(); + } + + // There should be a deoptimization here right after Main2 is linked by + // calling Dummy.createMain2(), even though sMain1 didn't change. + // The behavior here would be different if inline-cache is used, which + // doesn't deoptimize since sMain1 still hits the type cache. + sMain1.foo(sMain1.getClass() == Main1.class ? 1 : 2); + if (sMain2 != null) { + sMain2.foo(sMain2.getClass() == Main1.class ? 1 : 2); + } + } + + // Test scenarios under which CHA-based devirtualization happens, + // and class loading that overrides a method can invalidate compiled code. + // Also create a proxy method such that a proxy method's frame is visited + // during stack walking. + public static void main(String[] args) { + System.loadLibrary(args[0]); + // sMain1 is an instance of Main1. Main2 hasn't bee loaded yet. + sMain1 = new Main1(); + + // Create another thread that calls a proxy method. + new Thread() { + public void run() { + Runnable proxy = (Runnable)Proxy.newProxyInstance( + Proxied.class.getClassLoader(), + new Class[] { Runnable.class }, + new MyInvocationHandler(new Proxied())); + proxy.run(); + } + }.start(); + + ensureJitCompiled(Main.class, "testOverride"); + // This will create Main2 instance in the middle of testOverride(). + testOverride(); + } + + private static native void ensureJitCompiled(Class itf, String method_name); +} + +// Put createMain2() in another class to avoid class loading due to verifier. +class Dummy { + static Main1 createMain2() { + return new Main2(); + } +} diff --git a/test/616-cha/src/Main.java b/test/616-cha/src/Main.java index 787318dd6..b6179449d 100644 --- a/test/616-cha/src/Main.java +++ b/test/616-cha/src/Main.java @@ -179,7 +179,7 @@ public class Main { } } - // Test scanerios under which CHA-based devirtualization happens, + // Test scenarios under which CHA-based devirtualization happens, // and class loading that overrides a method can invalidate compiled code. // Also test pure non-overriding case, which is more for checking generated // code form. @@ -206,11 +206,6 @@ public class Main { // sMain1 is an instance of Main1. Main2 hasn't bee loaded yet. sMain1 = new Main1(); - // Loop enough to get testOverride() JITed. - for (int i=0; i<100; i++) { - testOverride(false, false, false); - } - ensureJitCompiled(Main.class, "testOverride"); testOverride(false, false, true); @@ -244,7 +239,7 @@ public class Main { private static native boolean hasSingleImplementation(Class clazz, String method_name); } -// Do it in another class to avoid class loading due to verifier. +// Put createMain2() in another class to avoid class loading due to verifier. class Dummy { static Main1 createMain2() { return new Main2(); -- 2.11.0