From f1aedb126ad1cb56dc7a38c8e07113295b123f58 Mon Sep 17 00:00:00 2001 From: Nicolas Geoffray Date: Thu, 28 Jul 2016 03:49:14 +0100 Subject: [PATCH] Pass the right class loader when inlining. Otherwise, method and type resolution can resolve to the wrong things and as a side effect update the dex cache with wrong data. bug:30403437 test:./art/test/run-test --host --jit --dev --no-prebuild 613 (cherry picked from commit 0a210d9b108c87c0e7c1d430a92ce6fc89790c95) Change-Id: I8fdca96fccae079c8e99b8e86e7b6935acfce89d --- compiler/optimizing/inliner.cc | 5 +- test/613-inlining-dex-cache/expected.txt | 1 + test/613-inlining-dex-cache/info.txt | 2 + test/613-inlining-dex-cache/run | 20 +++++ test/613-inlining-dex-cache/src-ex/B.java | 18 +++++ .../src-ex/LoadedByAppClassLoader.java | 22 ++++++ test/613-inlining-dex-cache/src/B.java | 20 +++++ test/613-inlining-dex-cache/src/Main.java | 85 ++++++++++++++++++++++ 8 files changed, 172 insertions(+), 1 deletion(-) create mode 100644 test/613-inlining-dex-cache/expected.txt create mode 100644 test/613-inlining-dex-cache/info.txt create mode 100644 test/613-inlining-dex-cache/run create mode 100644 test/613-inlining-dex-cache/src-ex/B.java create mode 100644 test/613-inlining-dex-cache/src-ex/LoadedByAppClassLoader.java create mode 100644 test/613-inlining-dex-cache/src/B.java create mode 100644 test/613-inlining-dex-cache/src/Main.java diff --git a/compiler/optimizing/inliner.cc b/compiler/optimizing/inliner.cc index a592162eb..aab24ebb2 100644 --- a/compiler/optimizing/inliner.cc +++ b/compiler/optimizing/inliner.cc @@ -1087,8 +1087,11 @@ bool HInliner::TryBuildAndInlineHelper(HInvoke* invoke_instruction, uint32_t method_index = resolved_method->GetDexMethodIndex(); ClassLinker* class_linker = caller_compilation_unit_.GetClassLinker(); Handle dex_cache(handles_->NewHandle(resolved_method->GetDexCache())); + Handle class_loader(handles_->NewHandle( + resolved_method->GetDeclaringClass()->GetClassLoader())); + DexCompilationUnit dex_compilation_unit( - caller_compilation_unit_.GetClassLoader(), + class_loader.ToJObject(), class_linker, callee_dex_file, code_item, diff --git a/test/613-inlining-dex-cache/expected.txt b/test/613-inlining-dex-cache/expected.txt new file mode 100644 index 000000000..6a5618ebc --- /dev/null +++ b/test/613-inlining-dex-cache/expected.txt @@ -0,0 +1 @@ +JNI_OnLoad called diff --git a/test/613-inlining-dex-cache/info.txt b/test/613-inlining-dex-cache/info.txt new file mode 100644 index 000000000..e80f642f3 --- /dev/null +++ b/test/613-inlining-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/613-inlining-dex-cache/run b/test/613-inlining-dex-cache/run new file mode 100644 index 000000000..9c1e7aa95 --- /dev/null +++ b/test/613-inlining-dex-cache/run @@ -0,0 +1,20 @@ +#!/bin/bash +# +# 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. + +flags="$@" +# We need the dex files pre-verified to avoid running the verifier +# at runtime which will update the dex cache. +exec ${RUN} ${flags/verify-at-runtime/interpret-only} diff --git a/test/613-inlining-dex-cache/src-ex/B.java b/test/613-inlining-dex-cache/src-ex/B.java new file mode 100644 index 000000000..4da9a1da6 --- /dev/null +++ b/test/613-inlining-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/613-inlining-dex-cache/src-ex/LoadedByAppClassLoader.java b/test/613-inlining-dex-cache/src-ex/LoadedByAppClassLoader.java new file mode 100644 index 000000000..f4e0f1019 --- /dev/null +++ b/test/613-inlining-dex-cache/src-ex/LoadedByAppClassLoader.java @@ -0,0 +1,22 @@ +/* + * 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() { + // We used to pass the wrong class loader when trying to inline 'Main.foo'. + Main.foo(null); + } +} diff --git a/test/613-inlining-dex-cache/src/B.java b/test/613-inlining-dex-cache/src/B.java new file mode 100644 index 000000000..6e7e55d43 --- /dev/null +++ b/test/613-inlining-dex-cache/src/B.java @@ -0,0 +1,20 @@ +/* + * 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 { + public void foo() { + } +} diff --git a/test/613-inlining-dex-cache/src/Main.java b/test/613-inlining-dex-cache/src/Main.java new file mode 100644 index 000000000..31ab1d2bf --- /dev/null +++ b/test/613-inlining-dex-cache/src/Main.java @@ -0,0 +1,85 @@ +/* + * 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 { + + public static void main(String[] args) throws Exception { + System.loadLibrary(args[0]); + final String DEX_FILE = System.getenv("DEX_LOCATION") + "/613-inlining-dex-cache-ex.jar"; + ClassLoader loader = new DelegateLastPathClassLoader(DEX_FILE, Main.class.getClassLoader()); + Class cls = loader.loadClass("LoadedByAppClassLoader"); + Method m = cls.getDeclaredMethod("letMeInlineYou"); + // Invoke the method enough times to get JITted. + for (int i = 0; i < 10000; ++i) { + m.invoke(null); + } + ensureJitCompiled(cls, "letMeInlineYou"); + ClassLoader bLoader = areYouB(); + if (bLoader != Main.class.getClassLoader()) { + throw new Error("Wrong class loader"); + } + } + + public static void foo(Main o) { + // LoadedByAppClassLoader.letMeInlineYou will try to inline this + // method but used to pass the wrong class loader. As a result, + // the lookup of B.foo was updating the dex cache with the other + // class loader's B class. + if (o != null) { + o.myField.foo(); + } + } + + public B myField; + + public static ClassLoader areYouB() { + return OtherClass.getB().getClassLoader(); + } + + public static native void ensureJitCompiled(Class cls, String method_name); +} + +class OtherClass { + public static Class getB() { + // This used to return the B class of another class loader. + return B.class; + } +} -- 2.11.0