From 0a112bbbcd761c749c346bfec0ec39c1ef37a590 Mon Sep 17 00:00:00 2001 From: Alex Light Date: Thu, 14 Aug 2014 14:16:26 -0700 Subject: [PATCH] Make apps able to run with a failing patchoat Bug: 17000769 (cherry picked from commit 9dcc4572949f6a8231a1b4ed859676ba6f411726) Change-Id: I0a1a4dc7f5d4bb268530840302ecfb1555231e05 --- runtime/class_linker.cc | 42 ++++++++++++++++++++++++++++++++-- runtime/class_linker.h | 5 ++++ runtime/oat_file.cc | 24 ++++++++++++-------- runtime/oat_file.h | 9 +++++++- test/117-nopatchoat/expected.txt | 9 ++++++++ test/117-nopatchoat/info.txt | 1 + test/117-nopatchoat/nopatchoat.cc | 41 +++++++++++++++++++++++++++++++++ test/117-nopatchoat/run | 36 +++++++++++++++++++++++++++++ test/117-nopatchoat/src/Main.java | 48 +++++++++++++++++++++++++++++++++++++++ test/Android.libarttest.mk | 3 ++- test/Android.run-test.mk | 36 +++++++++++++++++++++++++++++ 11 files changed, 241 insertions(+), 13 deletions(-) create mode 100644 test/117-nopatchoat/expected.txt create mode 100644 test/117-nopatchoat/info.txt create mode 100644 test/117-nopatchoat/nopatchoat.cc create mode 100755 test/117-nopatchoat/run create mode 100644 test/117-nopatchoat/src/Main.java diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc index 003e86880..749461579 100644 --- a/runtime/class_linker.cc +++ b/runtime/class_linker.cc @@ -854,7 +854,8 @@ bool ClassLinker::OpenDexFilesFromOat(const char* dex_location, const char* oat_ // We opened the oat file, so we must register it. RegisterOatFile(oat_file); } - return true; + // If the file isn't executable we failed patchoat but did manage to get the dex files. + return oat_file->IsExecutable(); } else { if (needs_registering) { // We opened it, delete it. @@ -1139,11 +1140,18 @@ const OatFile* ClassLinker::FindOatFileContainingDexFileFromDexLocation( error_msgs->push_back(StringPrintf("Failed to open oat file from dex location '%s'", dex_location)); return nullptr; - } else if (!VerifyOatWithDexFile(oat_file.get(), dex_location, &error_msg)) { + } else if (oat_file->IsExecutable() && + !VerifyOatWithDexFile(oat_file.get(), dex_location, &error_msg)) { error_msgs->push_back(StringPrintf("Failed to verify oat file '%s' found for dex location " "'%s': %s", oat_file->GetLocation().c_str(), dex_location, error_msg.c_str())); return nullptr; + } else if (!oat_file->IsExecutable() && + !VerifyOatImageChecksum(oat_file.get(), isa)) { + error_msgs->push_back(StringPrintf("Failed to verify non-executable oat file '%s' found for " + "dex location '%s'. Image checksum incorrect.", + oat_file->GetLocation().c_str(), dex_location)); + return nullptr; } else { return oat_file.release(); } @@ -1313,11 +1321,35 @@ const OatFile* ClassLinker::OpenOatFileFromDexLocation(const std::string& dex_lo return ret; } +const OatFile* ClassLinker::GetInterpretedOnlyOat(const std::string& oat_path, + InstructionSet isa, + std::string* error_msg) { + // We open it non-executable + std::unique_ptr output(OatFile::Open(oat_path, oat_path, NULL, false, error_msg)); + if (output.get() == nullptr) { + return nullptr; + } + if (VerifyOatImageChecksum(output.get(), isa)) { + return output.release(); + } else { + *error_msg = StringPrintf("Could not use oat file '%s', image checksum failed to verify.", + oat_path.c_str()); + return nullptr; + } +} + const OatFile* ClassLinker::PatchAndRetrieveOat(const std::string& input_oat, const std::string& output_oat, const std::string& image_location, InstructionSet isa, std::string* error_msg) { + if (!Runtime::Current()->IsDex2OatEnabled()) { + // We don't have dex2oat so we can assume we don't have patchoat either. We should just use the + // input_oat but make sure we only do interpretation on it's dex files. + LOG(WARNING) << "Patching of oat file '" << input_oat << "' not attempted due to dex2oat being " + << "disabled. Attempting to use oat file for interpretation"; + return GetInterpretedOnlyOat(input_oat, isa, error_msg); + } Locks::mutator_lock_->AssertNotHeld(Thread::Current()); // Avoid starving GC. std::string patchoat(Runtime::Current()->GetPatchoatExecutable()); @@ -1355,6 +1387,12 @@ const OatFile* ClassLinker::PatchAndRetrieveOat(const std::string& input_oat, "but was unable to open output file '%s': %s", input_oat.c_str(), output_oat.c_str(), error_msg->c_str()); } + } else if (!Runtime::Current()->IsCompiler()) { + // patchoat failed which means we probably don't have enough room to place the output oat file, + // instead of failing we should just run the interpreter from the dex files in the input oat. + LOG(WARNING) << "Patching of oat file '" << input_oat << "' failed. Attempting to use oat file " + << "for interpretation. patchoat failure was: " << *error_msg; + return GetInterpretedOnlyOat(input_oat, isa, error_msg); } else { *error_msg = StringPrintf("Patching of oat file '%s to '%s' " "failed: %s", input_oat.c_str(), output_oat.c_str(), diff --git a/runtime/class_linker.h b/runtime/class_linker.h index 6fc0f0e2f..bd4a17301 100644 --- a/runtime/class_linker.h +++ b/runtime/class_linker.h @@ -565,6 +565,10 @@ class ClassLinker { std::vector* error_msg) LOCKS_EXCLUDED(dex_lock_, Locks::mutator_lock_); + const OatFile* GetInterpretedOnlyOat(const std::string& oat_path, + InstructionSet isa, + std::string* error_msg); + const OatFile* PatchAndRetrieveOat(const std::string& input, const std::string& output, const std::string& image_location, InstructionSet isa, std::string* error_msg) @@ -744,6 +748,7 @@ class ClassLinker { friend class ImageDumper; // for FindOpenedOatFileFromOatLocation friend class ElfPatcher; // for FindOpenedOatFileForDexFile & FindOpenedOatFileFromOatLocation friend class NoDex2OatTest; // for FindOpenedOatFileForDexFile + friend class NoPatchoatTest; // for FindOpenedOatFileForDexFile FRIEND_TEST(ClassLinkerTest, ClassRootDescriptors); FRIEND_TEST(mirror::DexCacheTest, Open); FRIEND_TEST(ExceptionTest, FindExceptionHandler); diff --git a/runtime/oat_file.cc b/runtime/oat_file.cc index 971daf8bb..9afad2bab 100644 --- a/runtime/oat_file.cc +++ b/runtime/oat_file.cc @@ -45,7 +45,7 @@ OatFile* OatFile::OpenMemory(std::vector& oat_contents, std::string* error_msg) { CHECK(!oat_contents.empty()) << location; CheckLocation(location); - std::unique_ptr oat_file(new OatFile(location)); + std::unique_ptr oat_file(new OatFile(location, false)); oat_file->begin_ = &oat_contents[0]; oat_file->end_ = &oat_contents[oat_contents.size()]; return oat_file->Setup(error_msg) ? oat_file.release() : nullptr; @@ -97,7 +97,7 @@ OatFile* OatFile::OpenDlopen(const std::string& elf_filename, const std::string& location, byte* requested_base, std::string* error_msg) { - std::unique_ptr oat_file(new OatFile(location)); + std::unique_ptr oat_file(new OatFile(location, true)); bool success = oat_file->Dlopen(elf_filename, requested_base, error_msg); if (!success) { return nullptr; @@ -111,7 +111,7 @@ OatFile* OatFile::OpenElfFile(File* file, bool writable, bool executable, std::string* error_msg) { - std::unique_ptr oat_file(new OatFile(location)); + std::unique_ptr oat_file(new OatFile(location, executable)); bool success = oat_file->ElfFileOpen(file, requested_base, writable, executable, error_msg); if (!success) { CHECK(!error_msg->empty()); @@ -120,8 +120,9 @@ OatFile* OatFile::OpenElfFile(File* file, return oat_file.release(); } -OatFile::OatFile(const std::string& location) - : location_(location), begin_(NULL), end_(NULL), dlopen_handle_(NULL), +OatFile::OatFile(const std::string& location, bool is_executable) + : location_(location), begin_(NULL), end_(NULL), is_executable_(is_executable), + dlopen_handle_(NULL), secondary_lookup_lock_("OatFile secondary lookup lock", kOatFileSecondaryLookupLock) { CHECK(!location_.empty()); } @@ -530,10 +531,15 @@ const OatFile::OatMethod OatFile::OatClass::GetOatMethod(uint32_t method_index) methods_pointer_index = num_set_bits; } const OatMethodOffsets& oat_method_offsets = methods_pointer_[methods_pointer_index]; - return OatMethod( - oat_file_->Begin(), - oat_method_offsets.code_offset_, - oat_method_offsets.gc_map_offset_); + if (oat_file_->IsExecutable() || Runtime::Current()->IsCompiler()) { + return OatMethod( + oat_file_->Begin(), + oat_method_offsets.code_offset_, + oat_method_offsets.gc_map_offset_); + } else { + // We aren't allowed to use the compiled code. We just force it down the interpreted version. + return OatMethod(oat_file_->Begin(), 0, 0); + } } OatFile::OatMethod::OatMethod(const byte* base, diff --git a/runtime/oat_file.h b/runtime/oat_file.h index 810eccb2d..5997cc49a 100644 --- a/runtime/oat_file.h +++ b/runtime/oat_file.h @@ -64,6 +64,10 @@ class OatFile { ~OatFile(); + bool IsExecutable() const { + return is_executable_; + } + ElfFile* GetElfFile() const { CHECK_NE(reinterpret_cast(elf_file_.get()), reinterpret_cast(nullptr)) << "Cannot get an elf file from " << GetLocation(); @@ -260,7 +264,7 @@ class OatFile { bool executable, std::string* error_msg); - explicit OatFile(const std::string& filename); + explicit OatFile(const std::string& filename, bool executable); bool Dlopen(const std::string& elf_filename, byte* requested_base, std::string* error_msg); bool ElfFileOpen(File* file, byte* requested_base, bool writable, bool executable, std::string* error_msg); @@ -277,6 +281,9 @@ class OatFile { // Pointer to end of oat region for bounds checking. const byte* end_; + // Was this oat_file loaded executable? + const bool is_executable_; + // Backing memory map for oat file during when opened by ElfWriter during initial compilation. std::unique_ptr mem_map_; diff --git a/test/117-nopatchoat/expected.txt b/test/117-nopatchoat/expected.txt new file mode 100644 index 000000000..a1293aed6 --- /dev/null +++ b/test/117-nopatchoat/expected.txt @@ -0,0 +1,9 @@ +Run without dex2oat/patchoat +dex2oat & patchoat are disabled, has oat is true, has executable oat is false. +This is a function call +Run with dexoat/patchoat +dex2oat & patchoat are enabled, has oat is true, has executable oat is true. +This is a function call +Run default +dex2oat & patchoat are enabled, has oat is true, has executable oat is true. +This is a function call diff --git a/test/117-nopatchoat/info.txt b/test/117-nopatchoat/info.txt new file mode 100644 index 000000000..aa9f57cb0 --- /dev/null +++ b/test/117-nopatchoat/info.txt @@ -0,0 +1 @@ +Test that disables patchoat'ing the application. diff --git a/test/117-nopatchoat/nopatchoat.cc b/test/117-nopatchoat/nopatchoat.cc new file mode 100644 index 000000000..ced7f6ef4 --- /dev/null +++ b/test/117-nopatchoat/nopatchoat.cc @@ -0,0 +1,41 @@ +/* + * Copyright (C) 2014 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. + */ + +#include "class_linker.h" +#include "dex_file-inl.h" +#include "mirror/class-inl.h" +#include "scoped_thread_state_change.h" +#include "thread.h" + +namespace art { + +class NoPatchoatTest { + public: + static bool hasExecutableOat(jclass cls) { + ScopedObjectAccess soa(Thread::Current()); + mirror::Class* klass = soa.Decode(cls); + const DexFile& dex_file = klass->GetDexFile(); + const OatFile* oat_file = + Runtime::Current()->GetClassLinker()->FindOpenedOatFileForDexFile(dex_file); + return oat_file != nullptr && oat_file->IsExecutable(); + } +}; + +extern "C" JNIEXPORT jboolean JNICALL Java_Main_hasExecutableOat(JNIEnv*, jclass cls) { + return NoPatchoatTest::hasExecutableOat(cls); +} + +} // namespace art diff --git a/test/117-nopatchoat/run b/test/117-nopatchoat/run new file mode 100755 index 000000000..a7c96a0d3 --- /dev/null +++ b/test/117-nopatchoat/run @@ -0,0 +1,36 @@ +#!/bin/bash +# +# Copyright (C) 2014 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. + +# ensure flags includes prebuild and relocate. It doesn't make sense unless we +# have a oat file we want to relocate. +# TODO Unfortunately we have no way to force prebuild on for both host and target (or skip if not on). +flags="${@/--relocate/}" +flags="${flags/--no-relocate/}" +flags="${flags} --relocate" + +# Make sure we can run without relocation +echo "Run without dex2oat/patchoat" +# /bin/false is actually not even there for either, so the exec will fail. +# Unfortunately there is no equivalent to /bin/false in android. +${RUN} ${flags} --runtime-option -Xnodex2oat + +# Make sure we can run with the oat file. +echo "Run with dexoat/patchoat" +${RUN} ${flags} --runtime-option -Xdex2oat + +# Make sure we can run with the default settings. +echo "Run default" +${RUN} ${flags} diff --git a/test/117-nopatchoat/src/Main.java b/test/117-nopatchoat/src/Main.java new file mode 100644 index 000000000..f3f91ce1a --- /dev/null +++ b/test/117-nopatchoat/src/Main.java @@ -0,0 +1,48 @@ +/* + * Copyright (C) 2014 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 Main { + public static void main(String[] args) { + System.out.println( + "dex2oat & patchoat are " + ((isDex2OatEnabled()) ? "enabled" : "disabled") + + ", has oat is " + hasOat() + ", has executable oat is " + hasExecutableOat() + "."); + + if (!hasOat() && isDex2OatEnabled()) { + throw new Error("Application with dex2oat enabled runs without an oat file"); + } + + System.out.println(functionCall()); + } + + public static String functionCall() { + String arr[] = {"This", "is", "a", "function", "call"}; + String ret = ""; + for (int i = 0; i < arr.length; i++) { + ret = ret + arr[i] + " "; + } + return ret.substring(0, ret.length() - 1); + } + + static { + System.loadLibrary("arttest"); + } + + private native static boolean isDex2OatEnabled(); + + private native static boolean hasOat(); + + private native static boolean hasExecutableOat(); +} diff --git a/test/Android.libarttest.mk b/test/Android.libarttest.mk index 3871b28ec..caaf64924 100644 --- a/test/Android.libarttest.mk +++ b/test/Android.libarttest.mk @@ -24,7 +24,8 @@ LIBARTTEST_COMMON_SRC_FILES := \ 004-ReferenceMap/stack_walk_refmap_jni.cc \ 004-StackWalk/stack_walk_jni.cc \ 004-UnsafeTest/unsafe_test.cc \ - 116-nodex2oat/nodex2oat.cc + 116-nodex2oat/nodex2oat.cc \ + 117-nopatchoat/nopatchoat.cc ART_TARGET_LIBARTTEST_$(ART_PHONY_TEST_TARGET_SUFFIX) += $(ART_TARGET_TEST_OUT)/$(TARGET_ARCH)/libarttest.so ifdef TARGET_2ND_ARCH diff --git a/test/Android.run-test.mk b/test/Android.run-test.mk index d7ee383db..da94458bc 100644 --- a/test/Android.run-test.mk +++ b/test/Android.run-test.mk @@ -124,6 +124,42 @@ ART_TEST_KNOWN_BROKEN += $(call all-run-test-target-names,115-native-bridge,-tra ART_TEST_KNOWN_BROKEN += $(call all-run-test-target-names,115-native-bridge,-gcverify,-no-prebuild) ART_TEST_KNOWN_BROKEN += $(call all-run-test-target-names,115-native-bridge,-gcstress,-no-prebuild) +# NB 116-nodex2oat is not broken per-se it just doesn't (and isn't meant to) work with --prebuild. +# On host this is patched around by changing a run flag but we cannot do this on the target due to +# a different run-script. +TEST_ART_TARGET_BROKEN_PREBUILD_RUN_TESTS := \ + 116-nodex2oat + +ART_TEST_KNOWN_BROKEN += $(foreach test, $(TEST_ART_BROKEN_TARGET_PREBUILD_RUN_TESTS), $(call all-run-test-target-names,$(test),,-prebuild)) +ART_TEST_KNOWN_BROKEN += $(foreach test, $(TEST_ART_BROKEN_TARGET_PREBUILD_RUN_TESTS), $(call all-run-test-target-names,$(test),-trace,-prebuild)) +ART_TEST_KNOWN_BROKEN += $(foreach test, $(TEST_ART_BROKEN_TARGET_PREBUILD_RUN_TESTS), $(call all-run-test-target-names,$(test),-gcverify,-prebuild)) +ART_TEST_KNOWN_BROKEN += $(foreach test, $(TEST_ART_BROKEN_TARGET_PREBUILD_RUN_TESTS), $(call all-run-test-target-names,$(test),-gcstress,-prebuild)) + +# NB 117-nopatchoat is not broken per-se it just doesn't work (and isn't meant to) without --prebuild --relocate +TEST_ART_BROKEN_RELOCATE_TESTS := \ + 117-nopatchoat + +ART_TEST_KNOWN_BROKEN += $(foreach test, $(TEST_ART_BROKEN_RELOCATE_TESTS), $(call all-run-test-names,$(test),,-relocate)) +ART_TEST_KNOWN_BROKEN += $(foreach test, $(TEST_ART_BROKEN_RELOCATE_TESTS), $(call all-run-test-names,$(test),-trace,-relocate)) +ART_TEST_KNOWN_BROKEN += $(foreach test, $(TEST_ART_BROKEN_RELOCATE_TESTS), $(call all-run-test-names,$(test),-gcverify,-relocate)) +ART_TEST_KNOWN_BROKEN += $(foreach test, $(TEST_ART_BROKEN_RELOCATE_TESTS), $(call all-run-test-names,$(test),-gcstress,-relocate)) + +TEST_ART_BROKEN_NORELOCATE_TESTS := \ + 117-nopatchoat + +ART_TEST_KNOWN_BROKEN += $(foreach test, $(TEST_ART_BROKEN_NORELOCATE_TESTS), $(call all-run-test-names,$(test),,-norelocate)) +ART_TEST_KNOWN_BROKEN += $(foreach test, $(TEST_ART_BROKEN_NORELOCATE_TESTS), $(call all-run-test-names,$(test),-trace,-norelocate)) +ART_TEST_KNOWN_BROKEN += $(foreach test, $(TEST_ART_BROKEN_NORELOCATE_TESTS), $(call all-run-test-names,$(test),-gcverify,-norelocate)) +ART_TEST_KNOWN_BROKEN += $(foreach test, $(TEST_ART_BROKEN_NORELOCATE_TESTS), $(call all-run-test-names,$(test),-gcstress,-norelocate)) + +TEST_ART_BROKEN_NO_PREBUILD_TESTS := \ + 117-nopatchoat + +ART_TEST_KNOWN_BROKEN += $(foreach test, $(TEST_ART_BROKEN_NO_PREBUILD_TESTS), $(call all-run-test-names,$(test),,-no-prebuild)) +ART_TEST_KNOWN_BROKEN += $(foreach test, $(TEST_ART_BROKEN_NO_PREBUILD_TESTS), $(call all-run-test-names,$(test),-trace,-no-prebuild)) +ART_TEST_KNOWN_BROKEN += $(foreach test, $(TEST_ART_BROKEN_NO_PREBUILD_TESTS), $(call all-run-test-names,$(test),-gcverify,-no-prebuild)) +ART_TEST_KNOWN_BROKEN += $(foreach test, $(TEST_ART_BROKEN_NO_PREBUILD_TESTS), $(call all-run-test-names,$(test),-gcstress,-no-prebuild)) + # The path where build only targets will be output, e.g. # out/target/product/generic_x86_64/obj/PACKAGING/art-run-tests_intermediates/DATA art_run_tests_dir := $(call intermediates-dir-for,PACKAGING,art-run-tests)/DATA -- 2.11.0