Merge "ART: Allow oat files with duplicates classes in corner case"
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index 962e821..b099088 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -82,10 +82,6 @@
static constexpr bool kSanityCheckObjects = kIsDebugBuild;
-// Do a simple class redefinition check in OpenDexFilesFromOat. This is a conservative check to
-// avoid problems with compile-time class-path != runtime class-path.
-static constexpr bool kCheckForDexCollisions = true;
-
static void ThrowNoClassDefFoundError(const char* fmt, ...)
__attribute__((__format__(__printf__, 1, 2)))
SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
@@ -743,6 +739,8 @@
const char* rhsDescriptor = rhs.cached_descriptor_;
int cmp = strcmp(lhsDescriptor, rhsDescriptor);
if (cmp != 0) {
+ // Note that the order must be reversed. We want to iterate over the classes in dex files.
+ // They are sorted lexicographically. Thus, the priority-queue must be a min-queue.
return cmp > 0;
}
return dex_file_ < rhs.dex_file_;
@@ -768,6 +766,11 @@
return dex_file_;
}
+ void DeleteDexFile() {
+ delete dex_file_;
+ dex_file_ = nullptr;
+ }
+
private:
static const char* GetClassDescriptor(const DexFile* dex_file, size_t index) {
const DexFile::ClassDef& class_def = dex_file->GetClassDef(static_cast<uint16_t>(index));
@@ -799,13 +802,13 @@
}
}
-static void AddNext(const DexFileAndClassPair& original,
+static void AddNext(DexFileAndClassPair* original,
std::priority_queue<DexFileAndClassPair>* heap) {
- if (original.DexFileHasMoreClasses()) {
- heap->push(original.GetNext());
+ if (original->DexFileHasMoreClasses()) {
+ heap->push(original->GetNext());
} else {
// Need to delete the dex file.
- delete original.GetDexFile();
+ original->DeleteDexFile();
}
}
@@ -824,19 +827,17 @@
// the two elements agree on whether their dex file was from an already-loaded oat-file or the
// new oat file. Any disagreement indicates a collision.
bool ClassLinker::HasCollisions(const OatFile* oat_file, std::string* error_msg) {
- if (!kCheckForDexCollisions) {
- return false;
- }
-
// Dex files are registered late - once a class is actually being loaded. We have to compare
- // against the open oat files.
+ // against the open oat files. Take the dex_lock_ that protects oat_files_ accesses.
ReaderMutexLock mu(Thread::Current(), dex_lock_);
- std::priority_queue<DexFileAndClassPair> heap;
+ std::priority_queue<DexFileAndClassPair> queue;
// Add dex files from already loaded oat files, but skip boot.
{
- // To grab the boot oat, look at the dex files in the boot classpath.
+ // To grab the boot oat, look at the dex files in the boot classpath. Any of those is fine, as
+ // they were all compiled into the same oat file. So grab the first one, which is guaranteed to
+ // exist if the boot class-path isn't empty.
const OatFile* boot_oat = nullptr;
if (!boot_class_path_.empty()) {
const DexFile* boot_dex_file = boot_class_path_[0];
@@ -850,26 +851,26 @@
if (loaded_oat_file == boot_oat) {
continue;
}
- AddDexFilesFromOat(loaded_oat_file, true, &heap);
+ AddDexFilesFromOat(loaded_oat_file, true, &queue);
}
}
- if (heap.empty()) {
+ if (queue.empty()) {
// No other oat files, return early.
return false;
}
// Add dex files from the oat file to check.
- AddDexFilesFromOat(oat_file, false, &heap);
+ AddDexFilesFromOat(oat_file, false, &queue);
- // Now drain the heap.
- while (!heap.empty()) {
- DexFileAndClassPair compare_pop = heap.top();
- heap.pop();
+ // Now drain the queue.
+ while (!queue.empty()) {
+ DexFileAndClassPair compare_pop = queue.top();
+ queue.pop();
// Compare against the following elements.
- while (!heap.empty()) {
- DexFileAndClassPair top = heap.top();
+ while (!queue.empty()) {
+ DexFileAndClassPair top = queue.top();
if (strcmp(compare_pop.GetCachedDescriptor(), top.GetCachedDescriptor()) == 0) {
// Same descriptor. Check whether it's crossing old-oat-files to new-oat-files.
@@ -879,18 +880,18 @@
compare_pop.GetCachedDescriptor(),
compare_pop.GetDexFile()->GetLocation().c_str(),
top.GetDexFile()->GetLocation().c_str());
- FreeDexFilesInHeap(&heap);
+ FreeDexFilesInHeap(&queue);
return true;
}
// Pop it.
- heap.pop();
- AddNext(top, &heap);
+ queue.pop();
+ AddNext(&top, &queue);
} else {
// Something else. Done here.
break;
}
}
- AddNext(compare_pop, &heap);
+ AddNext(&compare_pop, &queue);
}
return false;
@@ -941,11 +942,10 @@
// Get the oat file on disk.
std::unique_ptr<OatFile> oat_file = oat_file_assistant.GetBestOatFile();
if (oat_file.get() != nullptr) {
- // Take the file only if it has no collisions.
- if (!HasCollisions(oat_file.get(), &error_msg)) {
- source_oat_file = oat_file.release();
- RegisterOatFile(source_oat_file);
- } else {
+ // Take the file only if it has no collisions, or we must take it because of preopting.
+ bool accept_oat_file = !HasCollisions(oat_file.get(), &error_msg);
+ if (!accept_oat_file) {
+ // Failed the collision check. Print warning.
if (Runtime::Current()->IsDexFileFallbackEnabled()) {
LOG(WARNING) << "Found duplicate classes, falling back to interpreter mode for "
<< dex_location;
@@ -954,6 +954,19 @@
" load classes for " << dex_location;
}
LOG(WARNING) << error_msg;
+
+ // However, if the app was part of /system and preopted, there is no original dex file
+ // available. In that case grudgingly accept the oat file.
+ if (!DexFile::MaybeDex(dex_location)) {
+ accept_oat_file = true;
+ LOG(WARNING) << "Dex location " << dex_location << " does not seem to include dex file. "
+ << "Allow oat file use. This is potentially dangerous.";
+ }
+ }
+
+ if (accept_oat_file) {
+ source_oat_file = oat_file.release();
+ RegisterOatFile(source_oat_file);
}
}
}
@@ -975,8 +988,7 @@
if (Runtime::Current()->IsDexFileFallbackEnabled()) {
if (!DexFile::Open(dex_location, dex_location, &error_msg, &dex_files)) {
LOG(WARNING) << error_msg;
- error_msgs->push_back("Failed to open dex files from "
- + std::string(dex_location));
+ error_msgs->push_back("Failed to open dex files from " + std::string(dex_location));
}
} else {
error_msgs->push_back("Fallback mode disabled, skipping dex files.");
diff --git a/runtime/dex_file.cc b/runtime/dex_file.cc
index 20098e7..dfe5a04 100644
--- a/runtime/dex_file.cc
+++ b/runtime/dex_file.cc
@@ -153,6 +153,31 @@
return false;
}
+static bool ContainsClassesDex(int fd, const char* filename) {
+ std::string error_msg;
+ std::unique_ptr<ZipArchive> zip_archive(ZipArchive::OpenFromFd(fd, filename, &error_msg));
+ if (zip_archive.get() == nullptr) {
+ return false;
+ }
+ std::unique_ptr<ZipEntry> zip_entry(zip_archive->Find(DexFile::kClassesDex, &error_msg));
+ return (zip_entry.get() != nullptr);
+}
+
+bool DexFile::MaybeDex(const char* filename) {
+ uint32_t magic;
+ std::string error_msg;
+ ScopedFd fd(OpenAndReadMagic(filename, &magic, &error_msg));
+ if (fd.get() == -1) {
+ return false;
+ }
+ if (IsZipMagic(magic)) {
+ return ContainsClassesDex(fd.release(), filename);
+ } else if (IsDexMagic(magic)) {
+ return true;
+ }
+ return false;
+}
+
int DexFile::GetPermissions() const {
if (mem_map_.get() == nullptr) {
return 0;
diff --git a/runtime/dex_file.h b/runtime/dex_file.h
index 6b3f883..84eaa4a 100644
--- a/runtime/dex_file.h
+++ b/runtime/dex_file.h
@@ -388,6 +388,10 @@
static bool Open(const char* filename, const char* location, std::string* error_msg,
std::vector<std::unique_ptr<const DexFile>>* dex_files);
+ // Checks whether the given file has the dex magic, or is a zip file with a classes.dex entry.
+ // If this function returns false, Open will not succeed. The inverse is not true, however.
+ static bool MaybeDex(const char* filename);
+
// Opens .dex file, backed by existing memory
static std::unique_ptr<const DexFile> Open(const uint8_t* base, size_t size,
const std::string& location,
diff --git a/test/138-duplicate-classes-check/build b/test/138-duplicate-classes-check/build
deleted file mode 100755
index 7ddc81d..0000000
--- a/test/138-duplicate-classes-check/build
+++ /dev/null
@@ -1,31 +0,0 @@
-#!/bin/bash
-#
-# Copyright (C) 2015 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.
-
-# Stop if something fails.
-set -e
-
-mkdir classes
-${JAVAC} -d classes `find src -name '*.java'`
-
-mkdir classes-ex
-${JAVAC} -d classes-ex `find src-ex -name '*.java'`
-
-if [ ${NEED_DEX} = "true" ]; then
- ${DX} -JXmx256m --debug --dex --dump-to=classes.lst --output=classes.dex --dump-width=1000 classes
- zip $TEST_NAME.jar classes.dex
- ${DX} -JXmx256m --debug --dex --dump-to=classes-ex.lst --output=classes.dex --dump-width=1000 classes-ex
- zip ${TEST_NAME}-ex.jar classes.dex
-fi