From 49de5a21881be28f01a639fa4e773eb6beb6f8d3 Mon Sep 17 00:00:00 2001 From: Jeff Hao Date: Mon, 23 May 2016 19:17:04 -0700 Subject: Don't update checksum if data ptr is null. Passing a nullptr to adler32 resets it to its initial value. Bug: 28874264 Change-Id: I863fa4d489618c1c6fa579cc89fb050f4cd23760 --- compiler/oat_test.cc | 58 +++++++++++++++++++++++++++++++++++++--------------- runtime/oat.cc | 8 ++++++-- 2 files changed, 47 insertions(+), 19 deletions(-) diff --git a/compiler/oat_test.cc b/compiler/oat_test.cc index 5b192846ba..21e198c12f 100644 --- a/compiler/oat_test.cc +++ b/compiler/oat_test.cc @@ -448,23 +448,23 @@ TEST_F(OatTest, OatHeaderSizeCheck) { } TEST_F(OatTest, OatHeaderIsValid) { - InstructionSet insn_set = kX86; - std::string error_msg; - std::unique_ptr insn_features( - InstructionSetFeatures::FromVariant(insn_set, "default", &error_msg)); - ASSERT_TRUE(insn_features.get() != nullptr) << error_msg; - std::unique_ptr oat_header(OatHeader::Create(insn_set, - insn_features.get(), - 0u, - nullptr)); - ASSERT_NE(oat_header.get(), nullptr); - ASSERT_TRUE(oat_header->IsValid()); - - char* magic = const_cast(oat_header->GetMagic()); - strcpy(magic, ""); // bad magic - ASSERT_FALSE(oat_header->IsValid()); - strcpy(magic, "oat\n000"); // bad version - ASSERT_FALSE(oat_header->IsValid()); + InstructionSet insn_set = kX86; + std::string error_msg; + std::unique_ptr insn_features( + InstructionSetFeatures::FromVariant(insn_set, "default", &error_msg)); + ASSERT_TRUE(insn_features.get() != nullptr) << error_msg; + std::unique_ptr oat_header(OatHeader::Create(insn_set, + insn_features.get(), + 0u, + nullptr)); + ASSERT_NE(oat_header.get(), nullptr); + ASSERT_TRUE(oat_header->IsValid()); + + char* magic = const_cast(oat_header->GetMagic()); + strcpy(magic, ""); // bad magic + ASSERT_FALSE(oat_header->IsValid()); + strcpy(magic, "oat\n000"); // bad version + ASSERT_FALSE(oat_header->IsValid()); } TEST_F(OatTest, EmptyTextSection) { @@ -766,4 +766,28 @@ TEST_F(OatTest, ZipFileInputCheckVerifier) { TestZipFileInput(true); } +TEST_F(OatTest, UpdateChecksum) { + InstructionSet insn_set = kX86; + std::string error_msg; + std::unique_ptr insn_features( + InstructionSetFeatures::FromVariant(insn_set, "default", &error_msg)); + ASSERT_TRUE(insn_features.get() != nullptr) << error_msg; + std::unique_ptr oat_header(OatHeader::Create(insn_set, + insn_features.get(), + 0u, + nullptr)); + // The starting adler32 value is 1. + EXPECT_EQ(1U, oat_header->GetChecksum()); + + oat_header->UpdateChecksum(OatHeader::kOatMagic, sizeof(OatHeader::kOatMagic)); + EXPECT_EQ(64291151U, oat_header->GetChecksum()); + + // Make sure that null data does not reset the checksum. + oat_header->UpdateChecksum(nullptr, 0); + EXPECT_EQ(64291151U, oat_header->GetChecksum()); + + oat_header->UpdateChecksum(OatHeader::kOatMagic, sizeof(OatHeader::kOatMagic)); + EXPECT_EQ(216138397U, oat_header->GetChecksum()); +} + } // namespace art diff --git a/runtime/oat.cc b/runtime/oat.cc index 80231f3890..aab0e81047 100644 --- a/runtime/oat.cc +++ b/runtime/oat.cc @@ -182,8 +182,12 @@ void OatHeader::UpdateChecksumWithHeaderData() { void OatHeader::UpdateChecksum(const void* data, size_t length) { DCHECK(IsValid()); - const uint8_t* bytes = reinterpret_cast(data); - adler32_checksum_ = adler32(adler32_checksum_, bytes, length); + if (data != nullptr) { + const uint8_t* bytes = reinterpret_cast(data); + adler32_checksum_ = adler32(adler32_checksum_, bytes, length); + } else { + DCHECK_EQ(0U, length); + } } InstructionSet OatHeader::GetInstructionSet() const { -- cgit v1.2.3 From 71b768168908052484e1aea9b076e939e422dac8 Mon Sep 17 00:00:00 2001 From: Philip Cuadra Date: Tue, 12 Jul 2016 16:37:40 -0700 Subject: Add API for getting location of odex or oat file Add an API for getting the file path of odex or oat file given a dex path. Bug 28251566 Change-Id: Ibebaa20f15d8135b25d9eb5927b7979801ebf0b2 --- runtime/native/dalvik_system_DexFile.cc | 38 +++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/runtime/native/dalvik_system_DexFile.cc b/runtime/native/dalvik_system_DexFile.cc index 8c7c966102..4bb83b62a0 100644 --- a/runtime/native/dalvik_system_DexFile.cc +++ b/runtime/native/dalvik_system_DexFile.cc @@ -554,6 +554,42 @@ static jboolean DexFile_isBackedByOatFile(JNIEnv* env, jclass, jobject cookie) { return oat_file != nullptr; } +static jstring DexFile_getDexFileOutputPath(JNIEnv* env, + jclass, + jstring javaFilename, + jstring javaInstructionSet) { + ScopedUtfChars filename(env, javaFilename); + if (env->ExceptionCheck()) { + return nullptr; + } + + ScopedUtfChars instruction_set(env, javaInstructionSet); + if (env->ExceptionCheck()) { + return nullptr; + } + + const InstructionSet target_instruction_set = GetInstructionSetFromString( + instruction_set.c_str()); + if (target_instruction_set == kNone) { + ScopedLocalRef iae(env, env->FindClass("java/lang/IllegalArgumentException")); + std::string message(StringPrintf("Instruction set %s is invalid.", instruction_set.c_str())); + env->ThrowNew(iae.get(), message.c_str()); + return nullptr; + } + + OatFileAssistant oat_file_assistant(filename.c_str(), + target_instruction_set, + false /* profile_changed */, + false /* load_executable */); + + std::unique_ptr best_oat_file = oat_file_assistant.GetBestOatFile(); + if (best_oat_file == nullptr) { + return nullptr; + } + + return env->NewStringUTF(best_oat_file->GetLocation().c_str()); +} + static JNINativeMethod gMethods[] = { NATIVE_METHOD(DexFile, closeDexFile, "(Ljava/lang/Object;)Z"), NATIVE_METHOD(DexFile, @@ -581,6 +617,8 @@ static JNINativeMethod gMethods[] = { "(Ljava/lang/String;)Ljava/lang/String;"), NATIVE_METHOD(DexFile, isBackedByOatFile, "(Ljava/lang/Object;)Z"), NATIVE_METHOD(DexFile, getDexFileStatus, + "(Ljava/lang/String;Ljava/lang/String;)Ljava/lang/String;"), + NATIVE_METHOD(DexFile, getDexFileOutputPath, "(Ljava/lang/String;Ljava/lang/String;)Ljava/lang/String;") }; -- cgit v1.2.3 From 64f25fddcf9361064acfabb05bcd26e6a260dd51 Mon Sep 17 00:00:00 2001 From: Nicolas Geoffray Date: Thu, 28 Jul 2016 03:49:14 +0100 Subject: 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 Change-Id: I23f94486f51c65e0a1328c6185b36084627e09b3 test:./art/test/run-test --host --jit --dev --no-prebuild 613 --- 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 932fad5085..c8a983b2bd 100644 --- a/compiler/optimizing/inliner.cc +++ b/compiler/optimizing/inliner.cc @@ -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 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 0000000000..6a5618ebc6 --- /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 0000000000..e80f642f3e --- /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 0000000000..9c1e7aa95c --- /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 0000000000..4da9a1da6b --- /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 0000000000..f4e0f1019a --- /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 0000000000..6e7e55d430 --- /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 0000000000..31ab1d2bfa --- /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; + } +} -- cgit v1.2.3