Fix to prevent a dex file from being verified multiple times.
Instead of verifying a dex file whenever one is initialized, they're now
verified when not opened from memory. Also, the way dalvik_system_DexFile
opens dex files has been changed to check for an existing oat file and
get the corresponding dex file from there instead.
Change-Id: I75fc26247150107d628e2c4e364ef8a53fbf9481
diff --git a/src/class_linker.cc b/src/class_linker.cc
index 5687453..2f1dc9a 100644
--- a/src/class_linker.cc
+++ b/src/class_linker.cc
@@ -749,7 +749,6 @@
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::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 @@
}
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 8471cbc..10d8305 100644
--- a/src/class_linker.h
+++ b/src/class_linker.h
@@ -251,6 +251,9 @@
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 dd91eac..67cb49b 100644
--- a/src/dalvik_system_DexFile.cc
+++ b/src/dalvik_system_DexFile.cc
@@ -90,7 +90,10 @@
}
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 @@
}
}
- 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 14413e8..974f6e5 100644
--- a/src/dex_file.cc
+++ b/src/dex_file.cc
@@ -134,7 +134,11 @@
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 @@
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 @@
}
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 @@
return false;
}
InitIndex();
- if (!DexFileVerifier::Verify(this, begin_, length_)) {
- return false;
- }
return true;
}
@@ -256,7 +261,7 @@
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 4c42b9b..d883f98 100644
--- a/src/dex_file.h
+++ b/src/dex_file.h
@@ -56,7 +56,7 @@
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 @@
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 @@
// 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 @@
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 @@
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 e731c1b..eb06e2a 100644
--- a/src/dex_file_verifier.cc
+++ b/src/dex_file_verifier.cc
@@ -99,8 +99,8 @@
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 @@
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::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 @@
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 @@
}
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::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 @@
}
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 e6aa329..58b5984 100644
--- a/src/dex_file_verifier.h
+++ b/src/dex_file_verifier.h
@@ -25,11 +25,11 @@
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 @@
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 a70bcd3..60b1a3b 100644
--- a/src/oatdump.cc
+++ b/src/oatdump.cc
@@ -291,7 +291,6 @@
os << "\n";
os << std::flush;
- class_linker->RegisterOatFile(*oat_file);
OatDump::Dump(oat_location, host_prefix, os, *oat_file);
}