diff options
-rw-r--r-- | compiler/dex/dex_to_dex_compiler.cc | 53 | ||||
-rw-r--r-- | dex2oat/dex2oat.cc | 18 | ||||
-rw-r--r-- | runtime/common_test.h | 2 | ||||
-rw-r--r-- | runtime/dex_file.cc | 28 | ||||
-rw-r--r-- | runtime/dex_file.h | 4 | ||||
-rw-r--r-- | runtime/dex_file_test.cc | 2 | ||||
-rw-r--r-- | runtime/mem_map.cc | 23 | ||||
-rw-r--r-- | runtime/mem_map.h | 2 |
8 files changed, 33 insertions, 99 deletions
diff --git a/compiler/dex/dex_to_dex_compiler.cc b/compiler/dex/dex_to_dex_compiler.cc index a0e2c1e..a392f82 100644 --- a/compiler/dex/dex_to_dex_compiler.cc +++ b/compiler/dex/dex_to_dex_compiler.cc @@ -95,55 +95,6 @@ class DexCompiler { DISALLOW_COPY_AND_ASSIGN(DexCompiler); }; -// Ensures write access to a part of DEX file. -// -// If a DEX file is read-only, it modifies its protection (mprotect) so it allows -// write access to the part of DEX file defined by an address and a length. -// In this case, it also takes the DexFile::modification_lock to prevent from -// concurrent protection modification from a parallel DEX-to-DEX compilation on -// the same DEX file. -// When the instance is destroyed, it recovers original protection and releases -// the lock. -// TODO: as this scoped class is similar to a MutexLock we should use annotalysis -// to capture the locking behavior. -class ScopedDexWriteAccess { - public: - ScopedDexWriteAccess(DexFile& dex_file, Instruction* inst, - size_t length) - : dex_file_(dex_file), - address_(reinterpret_cast<uint8_t*>(inst)), - length_(length), - is_read_only_(dex_file_.IsReadOnly()) { - if (is_read_only_) { - // We need to enable DEX write access. To avoid concurrent DEX write access - // modification, we take the DexFile::modification_lock before. - dex_file_.GetModificationLock().ExclusiveLock(Thread::Current()); - bool success = dex_file_.EnableWrite(address_, length_); - DCHECK(success) << "Failed to enable DEX write access"; - } - } - - ~ScopedDexWriteAccess() { - DCHECK_EQ(is_read_only_, dex_file_.IsReadOnly()); - if (is_read_only_) { - bool success = dex_file_.DisableWrite(address_, length_); - DCHECK(success) << "Failed to disable DEX write access"; - // Now we recovered original read-only protection, we can release the - // DexFile::modification_lock. - dex_file_.GetModificationLock().ExclusiveUnlock(Thread::Current()); - } - } - - private: - DexFile& dex_file_; - // TODO: make address_ const. - uint8_t* address_; - const size_t length_; - const bool is_read_only_; - - DISALLOW_COPY_AND_ASSIGN(ScopedDexWriteAccess); -}; - void DexCompiler::Compile() { DCHECK_GE(dex_to_dex_compilation_level_, kRequired); const DexFile::CodeItem* code_item = unit_.GetCodeItem(); @@ -223,7 +174,6 @@ void DexCompiler::CompileReturnVoid(Instruction* inst, uint32_t dex_pc) { << " by " << Instruction::Name(Instruction::RETURN_VOID_BARRIER) << " at dex pc " << StringPrintf("0x%x", dex_pc) << " in method " << PrettyMethod(unit_.GetDexMethodIndex(), GetDexFile(), true); - ScopedDexWriteAccess sdwa(GetModifiableDexFile(), inst, 2u); inst->SetOpcode(Instruction::RETURN_VOID_BARRIER); } @@ -246,7 +196,6 @@ Instruction* DexCompiler::CompileCheckCast(Instruction* inst, uint32_t dex_pc) { << StringPrintf("0x%x", dex_pc) << " in method " << PrettyMethod(unit_.GetDexMethodIndex(), GetDexFile(), true); // We are modifying 4 consecutive bytes. - ScopedDexWriteAccess sdwa(GetModifiableDexFile(), inst, 4u); inst->SetOpcode(Instruction::NOP); inst->SetVRegA_10x(0u); // keep compliant with verifier. // Get to next instruction which is the second half of check-cast and replace @@ -277,7 +226,6 @@ void DexCompiler::CompileInstanceFieldAccess(Instruction* inst, << " at dex pc " << StringPrintf("0x%x", dex_pc) << " in method " << PrettyMethod(unit_.GetDexMethodIndex(), GetDexFile(), true); // We are modifying 4 consecutive bytes. - ScopedDexWriteAccess sdwa(GetModifiableDexFile(), inst, 4u); inst->SetOpcode(new_opcode); // Replace field index by field offset. inst->SetVRegC_22c(static_cast<uint16_t>(field_offset)); @@ -313,7 +261,6 @@ void DexCompiler::CompileInvokeVirtual(Instruction* inst, << " at dex pc " << StringPrintf("0x%x", dex_pc) << " in method " << PrettyMethod(unit_.GetDexMethodIndex(), GetDexFile(), true); // We are modifying 4 consecutive bytes. - ScopedDexWriteAccess sdwa(GetModifiableDexFile(), inst, 4u); inst->SetOpcode(new_opcode); // Replace method index by vtable index. if (is_range) { diff --git a/dex2oat/dex2oat.cc b/dex2oat/dex2oat.cc index 5c96d74..b6a7683 100644 --- a/dex2oat/dex2oat.cc +++ b/dex2oat/dex2oat.cc @@ -417,9 +417,13 @@ static size_t OpenDexFiles(const std::vector<const char*>& dex_filenames, const char* dex_location = dex_locations[i]; const DexFile* dex_file = DexFile::Open(dex_filename, dex_location); if (dex_file == NULL) { - LOG(WARNING) << "Could not open .dex from file '" << dex_filename << "'\n"; + LOG(WARNING) << "Failed to open .dex from file '" << dex_filename << "'\n"; ++failure_count; } else { + // Ensure writable for dex-to-dex transformations. + if (!dex_file->EnableWrite()) { + PLOG(ERROR) << "Failed to make .dex file writeable '" << dex_filename << "'\n"; + } dex_files.push_back(dex_file); } } @@ -567,7 +571,7 @@ static int dex2oat(int argc, char** argv) { argc--; if (argc == 0) { - Usage("no arguments specified"); + Usage("No arguments specified"); } std::vector<const char*> dex_filenames; @@ -622,7 +626,7 @@ static int dex2oat(int argc, char** argv) { } else if (option.starts_with("--zip-fd=")) { const char* zip_fd_str = option.substr(strlen("--zip-fd=")).data(); if (!ParseInt(zip_fd_str, &zip_fd)) { - Usage("could not parse --zip-fd argument '%s' as an integer", zip_fd_str); + Usage("Failed to parse --zip-fd argument '%s' as an integer", zip_fd_str); } } else if (option.starts_with("--zip-location=")) { zip_location = option.substr(strlen("--zip-location=")).data(); @@ -633,7 +637,7 @@ static int dex2oat(int argc, char** argv) { } else if (option.starts_with("--oat-fd=")) { const char* oat_fd_str = option.substr(strlen("--oat-fd=")).data(); if (!ParseInt(oat_fd_str, &oat_fd)) { - Usage("could not parse --oat-fd argument '%s' as an integer", oat_fd_str); + Usage("Failed to parse --oat-fd argument '%s' as an integer", oat_fd_str); } } else if (option == "--watch-dog") { watch_dog_enabled = true; @@ -642,7 +646,7 @@ static int dex2oat(int argc, char** argv) { } else if (option.starts_with("-j")) { const char* thread_count_str = option.substr(strlen("-j")).data(); if (!ParseInt(thread_count_str, &thread_count)) { - Usage("could not parse -j argument '%s' as an integer", thread_count_str); + Usage("Failed to parse -j argument '%s' as an integer", thread_count_str); } } else if (option.starts_with("--oat-location=")) { oat_location = option.substr(strlen("--oat-location=")).data(); @@ -696,7 +700,7 @@ static int dex2oat(int argc, char** argv) { } else if (option == "--dump-timing") { dump_timing = true; } else { - Usage("unknown argument %s", option.data()); + Usage("Unknown argument %s", option.data()); } } @@ -789,7 +793,7 @@ static int dex2oat(int argc, char** argv) { if (boot_image_option.empty()) { if (image_base == 0) { - Usage("non-zero --base not specified"); + Usage("Non-zero --base not specified"); } } diff --git a/runtime/common_test.h b/runtime/common_test.h index e2dda86..dc1f592 100644 --- a/runtime/common_test.h +++ b/runtime/common_test.h @@ -434,6 +434,8 @@ class CommonTest : public testing::Test { filename += ".jar"; const DexFile* dex_file = DexFile::Open(filename, filename); CHECK(dex_file != NULL) << "Failed to open " << filename; + CHECK_EQ(PROT_READ, dex_file->GetPermissions()); + CHECK(dex_file->IsReadOnly()); opened_dex_files_.push_back(dex_file); return dex_file; } diff --git a/runtime/dex_file.cc b/runtime/dex_file.cc index 45b3427..4fd9a60 100644 --- a/runtime/dex_file.cc +++ b/runtime/dex_file.cc @@ -111,21 +111,21 @@ bool DexFile::IsReadOnly() const { return GetPermissions() == PROT_READ; } -bool DexFile::EnableWrite(uint8_t* addr, size_t length) const { +bool DexFile::EnableWrite() const { CHECK(IsReadOnly()); if (mem_map_.get() == NULL) { return false; } else { - return mem_map_->ProtectRegion(addr, length, PROT_READ | PROT_WRITE); + return mem_map_->Protect(PROT_READ | PROT_WRITE); } } -bool DexFile::DisableWrite(uint8_t* addr, size_t length) const { +bool DexFile::DisableWrite() const { CHECK(!IsReadOnly()); if (mem_map_.get() == NULL) { return false; } else { - return mem_map_->ProtectRegion(addr, length, PROT_READ); + return mem_map_->Protect(PROT_READ); } } @@ -208,24 +208,26 @@ const DexFile* DexFile::Open(const ZipArchive& zip_archive, const std::string& l LOG(ERROR) << "Failed to find classes.dex within '" << location << "'"; return NULL; } - UniquePtr<MemMap> map(zip_entry->ExtractToMemMap(kClassesDex)); if (map.get() == NULL) { LOG(ERROR) << "Failed to extract '" << kClassesDex << "' from '" << location << "'"; return NULL; } - const DexFile* dex_file = OpenMemory(location, zip_entry->GetCrc32(), map.release()); - if (dex_file == NULL) { + UniquePtr<const DexFile> dex_file(OpenMemory(location, zip_entry->GetCrc32(), map.release())); + if (dex_file.get() == NULL) { LOG(ERROR) << "Failed to open dex file '" << location << "' from memory"; return NULL; } - - if (!DexFileVerifier::Verify(dex_file, dex_file->Begin(), dex_file->Size())) { + if (!DexFileVerifier::Verify(dex_file.get(), dex_file->Begin(), dex_file->Size())) { LOG(ERROR) << "Failed to verify dex file '" << location << "'"; return NULL; } - - return dex_file; + if (!dex_file->DisableWrite()) { + LOG(ERROR) << "Failed to make dex file read only '" << location << "'"; + return NULL; + } + CHECK(dex_file->IsReadOnly()) << location; + return dex_file.release(); } const DexFile* DexFile::OpenMemory(const byte* base, @@ -839,7 +841,9 @@ void DexFile::DecodeDebugInfo(const CodeItem* code_item, bool is_static, uint32_ DexDebugNewPositionCb position_cb, DexDebugNewLocalCb local_cb, void* context) const { const byte* stream = GetDebugInfoStream(code_item); - UniquePtr<LocalInfo[]> local_in_reg(local_cb != NULL ? new LocalInfo[code_item->registers_size_] : NULL); + UniquePtr<LocalInfo[]> local_in_reg(local_cb != NULL ? + new LocalInfo[code_item->registers_size_] : + NULL); if (stream != NULL) { DecodeDebugInfo0(code_item, is_static, method_idx, position_cb, local_cb, context, stream, &local_in_reg[0]); } diff --git a/runtime/dex_file.h b/runtime/dex_file.h index 006b692..26635ae 100644 --- a/runtime/dex_file.h +++ b/runtime/dex_file.h @@ -805,9 +805,9 @@ class DexFile { bool IsReadOnly() const; - bool EnableWrite(uint8_t* addr, size_t size) const; + bool EnableWrite() const; - bool DisableWrite(uint8_t* addr, size_t size) const; + bool DisableWrite() const; private: // Opens a .dex file diff --git a/runtime/dex_file_test.cc b/runtime/dex_file_test.cc index 6449493..32a8354 100644 --- a/runtime/dex_file_test.cc +++ b/runtime/dex_file_test.cc @@ -75,6 +75,8 @@ static const DexFile* OpenDexFileBase64(const char* base64, ScopedObjectAccess soa(Thread::Current()); const DexFile* dex_file = DexFile::Open(location, location); CHECK(dex_file != NULL); + EXPECT_EQ(PROT_READ, dex_file->GetPermissions()); + EXPECT_TRUE(dex_file->IsReadOnly()); return dex_file; } diff --git a/runtime/mem_map.cc b/runtime/mem_map.cc index 9a34610..6451d5c 100644 --- a/runtime/mem_map.cc +++ b/runtime/mem_map.cc @@ -192,27 +192,4 @@ bool MemMap::Protect(int prot) { return false; } -bool MemMap::ProtectRegion(uint8_t* addr, size_t length, int prot) { - CHECK_GE(addr, base_begin_); - CHECK_LT(addr + length, reinterpret_cast<const uint8_t*>(base_begin_) + base_size_); - - /* - * Align "addr" to a page boundary and adjust "length" appropriately. - * (The address must be page-aligned, the length doesn't need to be, - * but we do need to ensure we cover the same range.) - */ - uint8_t* alignAddr = reinterpret_cast<uint8_t*>(RoundDown(reinterpret_cast<uintptr_t>(addr), - kPageSize)); - size_t alignLength = length + (addr - alignAddr); - - if (mprotect(alignAddr, alignLength, prot) == 0) { - prot_ = prot; - return true; - } - - PLOG(ERROR) << "mprotect(" << reinterpret_cast<void*>(alignAddr) << ", " << alignLength << ", " - << prot << ") failed"; - return false; -} - } // namespace art diff --git a/runtime/mem_map.h b/runtime/mem_map.h index 7d418a5..e294824 100644 --- a/runtime/mem_map.h +++ b/runtime/mem_map.h @@ -61,8 +61,6 @@ class MemMap { bool Protect(int prot); - bool ProtectRegion(uint8_t* addr, size_t length, int prot); - int GetProtect() const { return prot_; } |