OSDN Git Service

Pass the right class loader when inlining.
authorNicolas Geoffray <ngeoffray@google.com>
Thu, 28 Jul 2016 02:49:14 +0000 (03:49 +0100)
committerBrian Carlstrom <bdc@google.com>
Sat, 27 Aug 2016 01:31:36 +0000 (01:31 +0000)
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
Change-Id: I23f94486f51c65e0a1328c6185b36084627e09b3
test:./art/test/run-test --host --jit --dev --no-prebuild 613
(cherry picked from commit 0a210d9b108c87c0e7c1d430a92ce6fc89790c95)

compiler/optimizing/inliner.cc
test/613-inlining-dex-cache/expected.txt [new file with mode: 0644]
test/613-inlining-dex-cache/info.txt [new file with mode: 0644]
test/613-inlining-dex-cache/run [new file with mode: 0644]
test/613-inlining-dex-cache/src-ex/B.java [new file with mode: 0644]
test/613-inlining-dex-cache/src-ex/LoadedByAppClassLoader.java [new file with mode: 0644]
test/613-inlining-dex-cache/src/B.java [new file with mode: 0644]
test/613-inlining-dex-cache/src/Main.java [new file with mode: 0644]

index 834d981..547c9aa 100644 (file)
@@ -1035,8 +1035,11 @@ bool HInliner::TryBuildAndInlineHelper(HInvoke* invoke_instruction,
   uint32_t method_index = resolved_method->GetDexMethodIndex();
   ClassLinker* class_linker = caller_compilation_unit_.GetClassLinker();
   Handle<mirror::DexCache> dex_cache(handles_->NewHandle(resolved_method->GetDexCache()));
+  Handle<mirror::ClassLoader> 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 (file)
index 0000000..6a5618e
--- /dev/null
@@ -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 (file)
index 0000000..e80f642
--- /dev/null
@@ -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 (file)
index 0000000..9c1e7aa
--- /dev/null
@@ -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 (file)
index 0000000..4da9a1d
--- /dev/null
@@ -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 (file)
index 0000000..f4e0f10
--- /dev/null
@@ -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 (file)
index 0000000..6e7e55d
--- /dev/null
@@ -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 (file)
index 0000000..31ab1d2
--- /dev/null
@@ -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;
+  }
+}