Merge "Fix to prevent a dex file from being verified multiple times." into dalvik-dev

8 files changed, 83 insertions, 51 deletions

diff --git a/src/class_linker.cc b/src/class_linker.cc
index 5687453448..2f1dc9ab2a 100644
--- a/src/class_linker.cc
+++ b/src/class_linker.cc
@@ -749,7 +749,6 @@ const OatFile* ClassLinker::FindOatFileForDexFile(const DexFile& dex_file) {
     if (oat_file.get() != NULL) {
       const OatFile::OatDexFile* oat_dex_file = oat_file->GetOatDexFile(dex_file.GetLocation());
       if (dex_file.GetHeader().checksum_ == oat_dex_file->GetDexFileChecksum()) {
-        RegisterOatFileLocked(*oat_file.get());
         return oat_file.release();
       }
       LOG(WARNING) << ".oat file " << oat_file->GetLocation()
@@ -817,7 +816,13 @@ const OatFile* ClassLinker::FindOpenedOatFileFromOatLocation(const std::string&
 }
 
 const OatFile* ClassLinker::FindOatFileFromOatLocation(const std::string& oat_location) {
-  const OatFile* oat_file = OatFile::Open(oat_location, "", NULL);
+  MutexLock mu(dex_lock_);
+  const OatFile* oat_file = FindOpenedOatFileFromOatLocation(oat_location);
+  if (oat_file != NULL) {
+    return oat_file;
+  }
+
+  oat_file = OatFile::Open(oat_location, "", NULL);
   if (oat_file == NULL) {
     if (oat_location.empty() || oat_location[0] != '/') {
       LOG(ERROR) << "Failed to open oat file from " << oat_location;
@@ -838,9 +843,30 @@ const OatFile* ClassLinker::FindOatFileFromOatLocation(const std::string& oat_lo
   }
 
   CHECK(oat_file != NULL) << oat_location;
+  RegisterOatFileLocked(*oat_file);
   return oat_file;
 }
 
+const DexFile* ClassLinker::FindDexFileFromDexLocation(const std::string& location) {
+  std::string oat_location(OatFile::DexFilenameToOatFilename(location));
+  const OatFile* oat_file = FindOatFileFromOatLocation(oat_location);
+  if (oat_file == NULL) {
+    return NULL;
+  }
+  const OatFile::OatDexFile* oat_dex_file = oat_file->GetOatDexFile(location);
+  if (oat_dex_file == NULL) {
+    return NULL;
+  }
+  const DexFile* dex_file = oat_dex_file->OpenDexFile();
+  if (dex_file == NULL) {
+    return NULL;
+  }
+  if (oat_dex_file->GetDexFileChecksum() != dex_file->GetHeader().checksum_) {
+    return NULL;
+  }
+  return dex_file;
+}
+
 void ClassLinker::InitFromImage() {
   VLOG(startup) << "ClassLinker::InitFromImage entering";
   CHECK(!init_done_);
diff --git a/src/class_linker.h b/src/class_linker.h
index 8471cbc0cb..10d830523c 100644
--- a/src/class_linker.h
+++ b/src/class_linker.h
@@ -251,6 +251,9 @@ class ClassLinker {
   const OatFile* FindOatFileForDexFile(const DexFile& dex_file);
   const OatFile* FindOatFileFromOatLocation(const std::string& location);
 
+  // Find a DexFile within an OatFile given a DexFile location
+  const DexFile* FindDexFileFromDexLocation(const std::string& location);
+
   // TODO: replace this with multiple methods that allocate the correct managed type.
   template <class T>
   ObjectArray<T>* AllocObjectArray(size_t length) {
diff --git a/src/dalvik_system_DexFile.cc b/src/dalvik_system_DexFile.cc
index dd91eacf4b..67cb49bc9d 100644
--- a/src/dalvik_system_DexFile.cc
+++ b/src/dalvik_system_DexFile.cc
@@ -90,7 +90,10 @@ static jint DexFile_openDexFile(JNIEnv* env, jclass, jstring javaSourceName, jst
   }
   const DexFile* dex_file;
   if (outputName.c_str() == NULL) {
-    dex_file = DexFile::Open(sourceName.c_str(), "");
+    dex_file = Runtime::Current()->GetClassLinker()->FindDexFileFromDexLocation(sourceName.c_str());
+    if (dex_file == NULL) {
+      dex_file = DexFile::Open(sourceName.c_str(), "");
+    }
   } else {
     // Sanity check the arguments.
     if (!IsValidZipFilename(sourceName.c_str()) || !IsValidDexFilename(outputName.c_str())) {
@@ -233,20 +236,8 @@ jboolean DexFile_isDexOptNeeded(JNIEnv* env, jclass, jstring javaFilename) {
     }
   }
 
-  UniquePtr<const DexFile> dex_file(DexFile::Open(filename.c_str(), ""));
-  if (dex_file.get() == NULL) {
-    return JNI_TRUE;
-  }
-
-  const OatFile* oat_file = class_linker->FindOatFileForDexFile(*dex_file.get());
-  if (oat_file == NULL) {
-    return JNI_TRUE;
-  }
-  const OatFile::OatDexFile* oat_dex_file = oat_file->GetOatDexFile(dex_file->GetLocation());
-  if (oat_dex_file == NULL) {
-    return JNI_TRUE;
-  }
-  if (oat_dex_file->GetDexFileChecksum() != dex_file->GetHeader().checksum_) {
+  const DexFile* dex_file = class_linker->FindDexFileFromDexLocation(filename.c_str());
+  if (dex_file == NULL) {
     return JNI_TRUE;
   }
   return JNI_FALSE;
diff --git a/src/dex_file.cc b/src/dex_file.cc
index 14413e8b1f..974f6e5201 100644
--- a/src/dex_file.cc
+++ b/src/dex_file.cc
@@ -134,7 +134,11 @@ const DexFile* DexFile::OpenFile(const std::string& filename,
     return NULL;
   }
   close(fd);
-  return OpenMemory(location, map.release());
+  const DexFile* dex_file = OpenMemory(location, map.release());
+  if (dex_file != NULL) {
+    DexFileVerifier::Verify(dex_file, dex_file->Begin(), dex_file->Size());
+  }
+  return dex_file;
 }
 
 const char* DexFile::kClassesDex = "classes.dex";
@@ -178,15 +182,19 @@ const DexFile* DexFile::Open(const ZipArchive& zip_archive, const std::string& l
     return NULL;
   }
 
-  return OpenMemory(location, map.release());
+  const DexFile* dex_file = OpenMemory(location, map.release());
+  if (dex_file != NULL) {
+    DexFileVerifier::Verify(dex_file, dex_file->Begin(), dex_file->Size());
+  }
+  return dex_file;
 }
 
 const DexFile* DexFile::OpenMemory(const byte* base,
-                                   size_t length,
+                                   size_t size,
                                    const std::string& location,
                                    MemMap* mem_map) {
   CHECK_ALIGNED(base, 4); // various dex file structures must be word aligned
-  UniquePtr<DexFile> dex_file(new DexFile(base, length, location, mem_map));
+  UniquePtr<DexFile> dex_file(new DexFile(base, size, location, mem_map));
   if (!dex_file->Init()) {
     return NULL;
   } else {
@@ -208,7 +216,7 @@ jobject DexFile::GetDexObject(JNIEnv* env) const {
   }
 
   void* address = const_cast<void*>(reinterpret_cast<const void*>(begin_));
-  jobject byte_buffer = env->NewDirectByteBuffer(address, length_);
+  jobject byte_buffer = env->NewDirectByteBuffer(address, size_);
   if (byte_buffer == NULL) {
     return NULL;
   }
@@ -240,9 +248,6 @@ bool DexFile::Init() {
     return false;
   }
   InitIndex();
-  if (!DexFileVerifier::Verify(this, begin_, length_)) {
-    return false;
-  }
   return true;
 }
 
@@ -256,7 +261,7 @@ void DexFile::InitMembers() {
   method_ids_ = reinterpret_cast<const MethodId*>(b + h->method_ids_off_);
   proto_ids_ = reinterpret_cast<const ProtoId*>(b + h->proto_ids_off_);
   class_defs_ = reinterpret_cast<const ClassDef*>(b + h->class_defs_off_);
-  DCHECK_EQ(length_, header_->file_size_);
+  DCHECK_EQ(size_, header_->file_size_);
 }
 
 bool DexFile::CheckMagicAndVersion() const {
diff --git a/src/dex_file.h b/src/dex_file.h
index 4c42b9b592..d883f983c4 100644
--- a/src/dex_file.h
+++ b/src/dex_file.h
@@ -56,7 +56,7 @@ class DexFile {
     uint8_t magic_[8];
     uint32_t checksum_;
     uint8_t signature_[kSha1DigestSize];
-    uint32_t file_size_;  // length of entire file
+    uint32_t file_size_;  // size of entire file
     uint32_t header_size_;  // offset to start of next section
     uint32_t endian_tag_;
     uint32_t link_size_;  // unused
@@ -324,8 +324,8 @@ class DexFile {
                              const std::string& strip_location_prefix);
 
   // Opens .dex file, backed by existing memory
-  static const DexFile* Open(const uint8_t* base, size_t length, const std::string& location) {
-    return OpenMemory(base, length, location, NULL);
+  static const DexFile* Open(const uint8_t* base, size_t size, const std::string& location) {
+    return OpenMemory(base, size, location, NULL);
   }
 
   // Opens .dex file from the classes.dex in a zip archive
@@ -783,13 +783,13 @@ class DexFile {
 
   // Opens a .dex file at the given address, optionally backed by a MemMap
   static const DexFile* OpenMemory(const byte* dex_file,
-                                   size_t length,
+                                   size_t size,
                                    const std::string& location,
                                    MemMap* mem_map);
 
-  DexFile(const byte* base, size_t length, const std::string& location, MemMap* mem_map)
+  DexFile(const byte* base, size_t size, const std::string& location, MemMap* mem_map)
       : begin_(base),
-        length_(length),
+        size_(size),
         location_(location),
         mem_map_(mem_map),
         dex_object_lock_("a dex_object_lock_"),
@@ -802,7 +802,15 @@ class DexFile {
         proto_ids_(0),
         class_defs_(0) {
     CHECK(begin_ != NULL) << GetLocation();
-    CHECK_GT(length_, 0U) << GetLocation();
+    CHECK_GT(size_, 0U) << GetLocation();
+  }
+
+  const byte* Begin() const {
+    return begin_;
+  }
+
+  size_t Size() const {
+    return size_;
   }
 
   // Top-level initializer that calls other Init methods.
@@ -829,7 +837,7 @@ class DexFile {
   const byte* begin_;
 
   // The size of the underlying memory allocation in bytes.
-  size_t length_;
+  size_t size_;
 
   // Typically the dex file name when available, alternatively some identifying string.
   //
diff --git a/src/dex_file_verifier.cc b/src/dex_file_verifier.cc
index e731c1bcd7..eb06e2a50c 100644
--- a/src/dex_file_verifier.cc
+++ b/src/dex_file_verifier.cc
@@ -99,8 +99,8 @@ static bool CheckShortyDescriptorMatch(char shorty_char, const char* descriptor,
   return true;
 }
 
-bool DexFileVerifier::Verify(DexFile* dex_file, const byte* begin, size_t length) {
-  UniquePtr<DexFileVerifier> verifier(new DexFileVerifier(dex_file, begin, length));
+bool DexFileVerifier::Verify(const DexFile* dex_file, const byte* begin, size_t size) {
+  UniquePtr<DexFileVerifier> verifier(new DexFileVerifier(dex_file, begin, size));
   return verifier->Verify();
 }
 
@@ -108,7 +108,7 @@ bool DexFileVerifier::CheckPointerRange(const void* start, const void* end, cons
   uint32_t range_start = reinterpret_cast<uint32_t>(start);
   uint32_t range_end = reinterpret_cast<uint32_t>(end);
   uint32_t file_start = reinterpret_cast<uint32_t>(begin_);
-  uint32_t file_end = file_start + length_;
+  uint32_t file_end = file_start + size_;
   if ((range_start < file_start) || (range_start > file_end) ||
       (range_end < file_start) || (range_end > file_end)) {
     LOG(ERROR) << StringPrintf("Bad range for %s: %x to %x", label,
@@ -133,10 +133,10 @@ bool DexFileVerifier::CheckIndex(uint32_t field, uint32_t limit, const char* lab
 }
 
 bool DexFileVerifier::CheckHeader() const {
-  // Check file length from the header.
-  uint32_t expected_length = header_->file_size_;
-  if (length_ != expected_length) {
-    LOG(ERROR) << "Bad file length (" << length_ << ", expected " << expected_length << ")";
+  // Check file size from the header.
+  uint32_t expected_size = header_->file_size_;
+  if (size_ != expected_size) {
+    LOG(ERROR) << "Bad file size (" << size_ << ", expected " << expected_size << ")";
     return false;
   }
 
@@ -144,7 +144,7 @@ bool DexFileVerifier::CheckHeader() const {
   uint32_t adler_checksum = adler32(0L, Z_NULL, 0);
   const uint32_t non_sum = sizeof(header_->magic_) + sizeof(header_->checksum_);
   const byte* non_sum_ptr = reinterpret_cast<const byte*>(header_) + non_sum;
-  adler_checksum = adler32(adler_checksum, non_sum_ptr, expected_length - non_sum);
+  adler_checksum = adler32(adler_checksum, non_sum_ptr, expected_size - non_sum);
   if (adler_checksum != header_->checksum_) {
     LOG(ERROR) << StringPrintf("Bad checksum (%08x, expected %08x)", adler_checksum, header_->checksum_);
     return false;
@@ -157,7 +157,7 @@ bool DexFileVerifier::CheckHeader() const {
   }
 
   if (header_->header_size_ != sizeof(DexFile::Header)) {
-    LOG(ERROR) << "Bad header length: " << header_->header_size_;
+    LOG(ERROR) << "Bad header size: " << header_->header_size_;
     return false;
   }
 
@@ -685,7 +685,7 @@ bool DexFileVerifier::CheckIntraCodeItem() {
 
 bool DexFileVerifier::CheckIntraStringDataItem() {
   uint32_t size = DecodeUnsignedLeb128(&ptr_);
-  const byte* file_end = begin_ + length_;
+  const byte* file_end = begin_ + size_;
 
   for (uint32_t i = 0; i < size; i++) {
     if (ptr_ >= file_end) {
@@ -1121,7 +1121,7 @@ bool DexFileVerifier::CheckIntraSectionIterate(uint32_t offset, uint32_t count,
     }
 
     aligned_offset = reinterpret_cast<uint32_t>(ptr_) - reinterpret_cast<uint32_t>(begin_);
-    if (aligned_offset > length_) {
+    if (aligned_offset > size_) {
       LOG(ERROR) << StringPrintf("Item %d at ends out of bounds", i);
       return false;
     }
diff --git a/src/dex_file_verifier.h b/src/dex_file_verifier.h
index e6aa32997c..58b598437d 100644
--- a/src/dex_file_verifier.h
+++ b/src/dex_file_verifier.h
@@ -25,11 +25,11 @@ namespace art {
 
 class DexFileVerifier {
  public:
-  static bool Verify(DexFile* dex_file, const byte* begin, size_t length);
+  static bool Verify(const DexFile* dex_file, const byte* begin, size_t size);
 
  private:
-  DexFileVerifier(DexFile* dex_file, const byte* begin, size_t length)
-      : dex_file_(dex_file), begin_(begin), length_(length),
+  DexFileVerifier(const DexFile* dex_file, const byte* begin, size_t size)
+      : dex_file_(dex_file), begin_(begin), size_(size),
         header_(&dex_file->GetHeader()), ptr_(NULL), previous_item_(NULL)  {
   }
 
@@ -83,9 +83,9 @@ class DexFileVerifier {
   bool CheckInterSectionIterate(uint32_t offset, uint32_t count, uint16_t type);
   bool CheckInterSection();
 
-  DexFile* dex_file_;
+  const DexFile* dex_file_;
   const byte* begin_;
-  size_t length_;
+  size_t size_;
   const DexFile::Header* header_;
 
   std::map<uint32_t, uint16_t> offset_to_type_map_;
diff --git a/src/oatdump.cc b/src/oatdump.cc
index a70bcd3b0b..60b1a3baef 100644
--- a/src/oatdump.cc
+++ b/src/oatdump.cc
@@ -291,7 +291,6 @@ class ImageDump {
     os << "\n";
     os << std::flush;
 
-    class_linker->RegisterOatFile(*oat_file);
     OatDump::Dump(oat_location, host_prefix, os, *oat_file);
   }