OSDN Git Service

ART: Allow oat files with duplicates classes in corner case
authorAndreas Gampe <agampe@google.com>
Thu, 30 Apr 2015 03:47:16 +0000 (20:47 -0700)
committerAndreas Gampe <agampe@google.com>
Thu, 30 Apr 2015 03:47:16 +0000 (20:47 -0700)
When the oat file is actually an odex file, that is, a preopted
/system app, then it is impossible to fall back to the original
APK, as that has been stripped.

When it looks like it will be impossible to successfully open the
original dex location, grudgingly allow to open the found oat file,
even if it has duplicate classes, but warn accordingly.

Bug: 20697582
Change-Id: I1dd459563d977a2e77806eacd03e49334d5b1f14

runtime/class_linker.cc
runtime/dex_file.cc
runtime/dex_file.h
test/138-duplicate-classes-check/build [deleted file]

index 962e821..b099088 100644 (file)
@@ -82,10 +82,6 @@ namespace art {
 
 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 @@ class DexFileAndClassPair : ValueObject {
     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 @@ class DexFileAndClassPair : ValueObject {
     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 AddDexFilesFromOat(const OatFile* oat_file, bool already_loaded,
   }
 }
 
-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 @@ static void FreeDexFilesInHeap(std::priority_queue<DexFileAndClassPair>* heap) {
 // 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 @@ bool ClassLinker::HasCollisions(const OatFile* oat_file, std::string* error_msg)
       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 @@ bool ClassLinker::HasCollisions(const OatFile* oat_file, std::string* error_msg)
                            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 @@ std::vector<std::unique_ptr<const DexFile>> ClassLinker::OpenDexFilesFromOat(
     // 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 @@ std::vector<std::unique_ptr<const DexFile>> ClassLinker::OpenDexFilesFromOat(
                           " 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 @@ std::vector<std::unique_ptr<const DexFile>> ClassLinker::OpenDexFilesFromOat(
     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.");
index 20098e7..dfe5a04 100644 (file)
@@ -153,6 +153,31 @@ bool DexFile::Open(const char* filename, const char* location, std::string* erro
   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;
index 6b3f883..84eaa4a 100644 (file)
@@ -388,6 +388,10 @@ class DexFile {
   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 (executable)
index 7ddc81d..0000000
+++ /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