diff options
author | Richard Uhler <ruhler@google.com> | 2015-01-14 16:00:55 +0000 |
---|---|---|
committer | Gerrit Code Review <noreply-gerritcodereview@google.com> | 2015-01-14 16:00:55 +0000 |
commit | 127d933868b767eca1ec5446e91a3e8df65c659e (patch) | |
tree | 929a8d16f6899a5331c2443fbb0f5f5cf90c9196 | |
parent | c5573c29b7d7af0815e0e7b0ae241aafb52817a5 (diff) | |
parent | fbef44de596d298dc6430f482dffc933a046dd28 (diff) | |
download | art-127d933868b767eca1ec5446e91a3e8df65c659e.zip art-127d933868b767eca1ec5446e91a3e8df65c659e.tar.gz art-127d933868b767eca1ec5446e91a3e8df65c659e.tar.bz2 |
Merge "Use unique_ptr to track ownership of dex files."
-rw-r--r-- | compiler/driver/compiler_driver_test.cc | 27 | ||||
-rw-r--r-- | compiler/oat_test.cc | 23 | ||||
-rw-r--r-- | dex2oat/dex2oat.cc | 47 | ||||
-rw-r--r-- | oatdump/oatdump.cc | 12 | ||||
-rw-r--r-- | runtime/class_linker.cc | 40 | ||||
-rw-r--r-- | runtime/class_linker.h | 5 | ||||
-rw-r--r-- | runtime/class_linker_test.cc | 23 | ||||
-rw-r--r-- | runtime/common_runtime_test.cc | 41 | ||||
-rw-r--r-- | runtime/common_runtime_test.h | 18 | ||||
-rw-r--r-- | runtime/dex_file.cc | 53 | ||||
-rw-r--r-- | runtime/dex_file.h | 44 | ||||
-rw-r--r-- | runtime/dex_file_test.cc | 26 | ||||
-rw-r--r-- | runtime/dex_file_verifier_test.cc | 18 | ||||
-rw-r--r-- | runtime/mirror/dex_cache_test.cc | 1 | ||||
-rw-r--r-- | runtime/native/dalvik_system_DexFile.cc | 37 | ||||
-rw-r--r-- | runtime/oat_file.cc | 2 | ||||
-rw-r--r-- | runtime/oat_file.h | 2 | ||||
-rw-r--r-- | runtime/runtime.cc | 20 | ||||
-rw-r--r-- | runtime/runtime.h | 3 | ||||
-rw-r--r-- | runtime/verifier/method_verifier_test.cc | 13 |
20 files changed, 254 insertions, 201 deletions
diff --git a/compiler/driver/compiler_driver_test.cc b/compiler/driver/compiler_driver_test.cc index c30cc04..a02e25e 100644 --- a/compiler/driver/compiler_driver_test.cc +++ b/compiler/driver/compiler_driver_test.cc @@ -106,36 +106,37 @@ TEST_F(CompilerDriverTest, DISABLED_LARGE_CompileDexLibCore) { // All libcore references should resolve ScopedObjectAccess soa(Thread::Current()); - const DexFile* dex = java_lang_dex_file_; - mirror::DexCache* dex_cache = class_linker_->FindDexCache(*dex); - EXPECT_EQ(dex->NumStringIds(), dex_cache->NumStrings()); + ASSERT_TRUE(java_lang_dex_file_ != NULL); + const DexFile& dex = *java_lang_dex_file_; + mirror::DexCache* dex_cache = class_linker_->FindDexCache(dex); + EXPECT_EQ(dex.NumStringIds(), dex_cache->NumStrings()); for (size_t i = 0; i < dex_cache->NumStrings(); i++) { const mirror::String* string = dex_cache->GetResolvedString(i); EXPECT_TRUE(string != NULL) << "string_idx=" << i; } - EXPECT_EQ(dex->NumTypeIds(), dex_cache->NumResolvedTypes()); + EXPECT_EQ(dex.NumTypeIds(), dex_cache->NumResolvedTypes()); for (size_t i = 0; i < dex_cache->NumResolvedTypes(); i++) { mirror::Class* type = dex_cache->GetResolvedType(i); EXPECT_TRUE(type != NULL) << "type_idx=" << i - << " " << dex->GetTypeDescriptor(dex->GetTypeId(i)); + << " " << dex.GetTypeDescriptor(dex.GetTypeId(i)); } - EXPECT_EQ(dex->NumMethodIds(), dex_cache->NumResolvedMethods()); + EXPECT_EQ(dex.NumMethodIds(), dex_cache->NumResolvedMethods()); for (size_t i = 0; i < dex_cache->NumResolvedMethods(); i++) { mirror::ArtMethod* method = dex_cache->GetResolvedMethod(i); EXPECT_TRUE(method != NULL) << "method_idx=" << i - << " " << dex->GetMethodDeclaringClassDescriptor(dex->GetMethodId(i)) - << " " << dex->GetMethodName(dex->GetMethodId(i)); + << " " << dex.GetMethodDeclaringClassDescriptor(dex.GetMethodId(i)) + << " " << dex.GetMethodName(dex.GetMethodId(i)); EXPECT_TRUE(method->GetEntryPointFromQuickCompiledCode() != NULL) << "method_idx=" << i << " " - << dex->GetMethodDeclaringClassDescriptor(dex->GetMethodId(i)) - << " " << dex->GetMethodName(dex->GetMethodId(i)); + << dex.GetMethodDeclaringClassDescriptor(dex.GetMethodId(i)) + << " " << dex.GetMethodName(dex.GetMethodId(i)); } - EXPECT_EQ(dex->NumFieldIds(), dex_cache->NumResolvedFields()); + EXPECT_EQ(dex.NumFieldIds(), dex_cache->NumResolvedFields()); for (size_t i = 0; i < dex_cache->NumResolvedFields(); i++) { mirror::ArtField* field = dex_cache->GetResolvedField(i); EXPECT_TRUE(field != NULL) << "field_idx=" << i - << " " << dex->GetFieldDeclaringClassDescriptor(dex->GetFieldId(i)) - << " " << dex->GetFieldName(dex->GetFieldId(i)); + << " " << dex.GetFieldDeclaringClassDescriptor(dex.GetFieldId(i)) + << " " << dex.GetFieldName(dex.GetFieldId(i)); } // TODO check Class::IsVerified for all classes diff --git a/compiler/oat_test.cc b/compiler/oat_test.cc index d141538..b3ab370 100644 --- a/compiler/oat_test.cc +++ b/compiler/oat_test.cc @@ -39,10 +39,10 @@ class OatTest : public CommonCompilerTest { void CheckMethod(mirror::ArtMethod* method, const OatFile::OatMethod& oat_method, - const DexFile* dex_file) + const DexFile& dex_file) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) { const CompiledMethod* compiled_method = - compiler_driver_->GetCompiledMethod(MethodReference(dex_file, + compiler_driver_->GetCompiledMethod(MethodReference(&dex_file, method->GetDexMethodIndex())); if (compiled_method == nullptr) { @@ -130,22 +130,23 @@ TEST_F(OatTest, WriteRead) { ASSERT_EQ(4096U, oat_header.GetImageFileLocationOatDataBegin()); ASSERT_EQ("lue.art", std::string(oat_header.GetStoreValueByKey(OatHeader::kImageLocationKey))); - const DexFile* dex_file = java_lang_dex_file_; - uint32_t dex_file_checksum = dex_file->GetLocationChecksum(); - const OatFile::OatDexFile* oat_dex_file = oat_file->GetOatDexFile(dex_file->GetLocation().c_str(), + ASSERT_TRUE(java_lang_dex_file_ != nullptr); + const DexFile& dex_file = *java_lang_dex_file_; + uint32_t dex_file_checksum = dex_file.GetLocationChecksum(); + const OatFile::OatDexFile* oat_dex_file = oat_file->GetOatDexFile(dex_file.GetLocation().c_str(), &dex_file_checksum); ASSERT_TRUE(oat_dex_file != nullptr); - CHECK_EQ(dex_file->GetLocationChecksum(), oat_dex_file->GetDexFileLocationChecksum()); + CHECK_EQ(dex_file.GetLocationChecksum(), oat_dex_file->GetDexFileLocationChecksum()); ScopedObjectAccess soa(Thread::Current()); - for (size_t i = 0; i < dex_file->NumClassDefs(); i++) { - const DexFile::ClassDef& class_def = dex_file->GetClassDef(i); - const uint8_t* class_data = dex_file->GetClassData(class_def); + for (size_t i = 0; i < dex_file.NumClassDefs(); i++) { + const DexFile::ClassDef& class_def = dex_file.GetClassDef(i); + const uint8_t* class_data = dex_file.GetClassData(class_def); size_t num_virtual_methods = 0; if (class_data != nullptr) { - ClassDataItemIterator it(*dex_file, class_data); + ClassDataItemIterator it(dex_file, class_data); num_virtual_methods = it.NumVirtualMethods(); } - const char* descriptor = dex_file->GetClassDescriptor(class_def); + const char* descriptor = dex_file.GetClassDescriptor(class_def); StackHandleScope<1> hs(soa.Self()); mirror::Class* klass = class_linker->FindClass(soa.Self(), descriptor, NullHandle<mirror::ClassLoader>()); diff --git a/dex2oat/dex2oat.cc b/dex2oat/dex2oat.cc index 4b4675b..56d4582 100644 --- a/dex2oat/dex2oat.cc +++ b/dex2oat/dex2oat.cc @@ -446,7 +446,15 @@ class Dex2Oat FINAL { timings_(timings) {} ~Dex2Oat() { - LogCompletionTime(); // Needs to be before since it accesses the runtime. + // Free opened dex files before deleting the runtime_, because ~DexFile + // uses MemMap, which is shut down by ~Runtime. + class_path_files_.clear(); + opened_dex_files_.clear(); + + // Log completion time before deleting the runtime_, because this accesses + // the runtime. + LogCompletionTime(); + if (kIsDebugBuild || (RUNNING_ON_VALGRIND != 0)) { delete runtime_; // See field declaration for why this is manual. } @@ -1108,18 +1116,24 @@ class Dex2Oat FINAL { << error_msg; return false; } - if (!DexFile::OpenFromZip(*zip_archive.get(), zip_location_, &error_msg, &dex_files_)) { + if (!DexFile::OpenFromZip(*zip_archive.get(), zip_location_, &error_msg, &opened_dex_files_)) { LOG(ERROR) << "Failed to open dex from file descriptor for zip file '" << zip_location_ << "': " << error_msg; return false; } + for (auto& dex_file : opened_dex_files_) { + dex_files_.push_back(dex_file.get()); + } ATRACE_END(); } else { - size_t failure_count = OpenDexFiles(dex_filenames_, dex_locations_, dex_files_); + size_t failure_count = OpenDexFiles(dex_filenames_, dex_locations_, &opened_dex_files_); if (failure_count > 0) { LOG(ERROR) << "Failed to open some dex files: " << failure_count; return false; } + for (auto& dex_file : opened_dex_files_) { + dex_files_.push_back(dex_file.get()); + } } constexpr bool kSaveDexInput = false; @@ -1193,9 +1207,13 @@ class Dex2Oat FINAL { Thread* self = Thread::Current(); if (!boot_image_option_.empty()) { ClassLinker* class_linker = Runtime::Current()->GetClassLinker(); - std::vector<const DexFile*> class_path_files(dex_files_); - OpenClassPathFiles(runtime_->GetClassPathString(), class_path_files); + OpenClassPathFiles(runtime_->GetClassPathString(), dex_files_, &class_path_files_); ScopedObjectAccess soa(self); + std::vector<const DexFile*> class_path_files(dex_files_); + for (auto& class_path_file : class_path_files_) { + class_path_files.push_back(class_path_file.get()); + } + for (size_t i = 0; i < class_path_files.size(); i++) { class_linker->RegisterDexFile(*class_path_files[i]); } @@ -1446,7 +1464,8 @@ class Dex2Oat FINAL { private: static size_t OpenDexFiles(const std::vector<const char*>& dex_filenames, const std::vector<const char*>& dex_locations, - std::vector<const DexFile*>& dex_files) { + std::vector<std::unique_ptr<const DexFile>>* dex_files) { + DCHECK(dex_files != nullptr) << "OpenDexFiles out-param is NULL"; size_t failure_count = 0; for (size_t i = 0; i < dex_filenames.size(); i++) { const char* dex_filename = dex_filenames[i]; @@ -1457,7 +1476,7 @@ class Dex2Oat FINAL { LOG(WARNING) << "Skipping non-existent dex file '" << dex_filename << "'"; continue; } - if (!DexFile::Open(dex_filename, dex_location, &error_msg, &dex_files)) { + if (!DexFile::Open(dex_filename, dex_location, &error_msg, dex_files)) { LOG(WARNING) << "Failed to open .dex from file '" << dex_filename << "': " << error_msg; ++failure_count; } @@ -1477,10 +1496,12 @@ class Dex2Oat FINAL { return false; } - // Appends to dex_files any elements of class_path that it doesn't already - // contain. This will open those dex files as necessary. + // Appends to opened_dex_files any elements of class_path that dex_files + // doesn't already contain. This will open those dex files as necessary. static void OpenClassPathFiles(const std::string& class_path, - std::vector<const DexFile*>& dex_files) { + std::vector<const DexFile*> dex_files, + std::vector<std::unique_ptr<const DexFile>>* opened_dex_files) { + DCHECK(opened_dex_files != nullptr) << "OpenClassPathFiles out-param is NULL"; std::vector<std::string> parsed; Split(class_path, ':', &parsed); // Take Locks::mutator_lock_ so that lock ordering on the ClassLinker::dex_lock_ is maintained. @@ -1490,7 +1511,7 @@ class Dex2Oat FINAL { continue; } std::string error_msg; - if (!DexFile::Open(parsed[i].c_str(), parsed[i].c_str(), &error_msg, &dex_files)) { + if (!DexFile::Open(parsed[i].c_str(), parsed[i].c_str(), &error_msg, opened_dex_files)) { LOG(WARNING) << "Failed to open dex file '" << parsed[i] << "': " << error_msg; } } @@ -1630,6 +1651,9 @@ class Dex2Oat FINAL { DexFileToMethodInlinerMap method_inliner_map_; std::unique_ptr<QuickCompilerCallbacks> callbacks_; + // Ownership for the class path files. + std::vector<std::unique_ptr<const DexFile>> class_path_files_; + // Not a unique_ptr as we want to just exit on non-debug builds, not bringing the runtime down // in an orderly fashion. The destructor takes care of deleting this. Runtime* runtime_; @@ -1662,6 +1686,7 @@ class Dex2Oat FINAL { bool is_host_; std::string android_root_; std::vector<const DexFile*> dex_files_; + std::vector<std::unique_ptr<const DexFile>> opened_dex_files_; std::unique_ptr<CompilerDriver> driver_; std::vector<std::string> verbose_methods_; bool dump_stats_; diff --git a/oatdump/oatdump.cc b/oatdump/oatdump.cc index de4ea36..931cca7 100644 --- a/oatdump/oatdump.cc +++ b/oatdump/oatdump.cc @@ -1968,13 +1968,13 @@ static int DumpOatWithRuntime(Runtime* runtime, OatFile* oat_file, OatDumperOpti ScopedObjectAccess soa(self); ClassLinker* class_linker = runtime->GetClassLinker(); class_linker->RegisterOatFile(oat_file); - std::vector<const DexFile*> dex_files; + std::vector<std::unique_ptr<const DexFile>> dex_files; for (const OatFile::OatDexFile* odf : oat_file->GetOatDexFiles()) { std::string error_msg; - const DexFile* dex_file = odf->OpenDexFile(&error_msg); + std::unique_ptr<const DexFile> dex_file = odf->OpenDexFile(&error_msg); CHECK(dex_file != nullptr) << error_msg; class_linker->RegisterDexFile(*dex_file); - dex_files.push_back(dex_file); + dex_files.push_back(std::move(dex_file)); } // Need a class loader. @@ -1983,7 +1983,11 @@ static int DumpOatWithRuntime(Runtime* runtime, OatFile* oat_file, OatDumperOpti soa.Env()->AllocObject(WellKnownClasses::dalvik_system_PathClassLoader)); jobject class_loader = soa.Env()->NewGlobalRef(class_loader_local.get()); // Fake that we're a compiler. - runtime->SetCompileTimeClassPath(class_loader, dex_files); + std::vector<const DexFile*> class_path; + for (auto& dex_file : dex_files) { + class_path.push_back(dex_file.get()); + } + runtime->SetCompileTimeClassPath(class_loader, class_path); // Use the class loader while dumping. StackHandleScope<1> scope(self); diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc index d119a56..438cebf 100644 --- a/runtime/class_linker.cc +++ b/runtime/class_linker.cc @@ -246,7 +246,7 @@ ClassLinker::ClassLinker(InternTable* intern_table) memset(find_array_class_cache_, 0, kFindArrayCacheSize * sizeof(mirror::Class*)); } -void ClassLinker::InitWithoutImage(const std::vector<const DexFile*>& boot_class_path) { +void ClassLinker::InitWithoutImage(std::vector<std::unique_ptr<const DexFile>> boot_class_path) { VLOG(startup) << "ClassLinker::Init"; CHECK(!Runtime::Current()->GetHeap()->HasImageSpace()) << "Runtime has image. We should use it."; @@ -405,9 +405,10 @@ void ClassLinker::InitWithoutImage(const std::vector<const DexFile*>& boot_class // DexCache instances. Needs to be after String, Field, Method arrays since AllocDexCache uses // these roots. CHECK_NE(0U, boot_class_path.size()); - for (const DexFile* dex_file : boot_class_path) { - CHECK(dex_file != nullptr); + for (auto& dex_file : boot_class_path) { + CHECK(dex_file.get() != nullptr); AppendToBootClassPath(self, *dex_file); + opened_dex_files_.push_back(std::move(dex_file)); } // now we can use FindSystemClass @@ -794,7 +795,7 @@ static bool LoadMultiDexFilesFromOatFile(const OatFile* oat_file, const uint32_t* dex_location_checksum, bool generated, std::vector<std::string>* error_msgs, - std::vector<const DexFile*>* dex_files) { + std::vector<std::unique_ptr<const DexFile>>* dex_files) { if (oat_file == nullptr) { return false; } @@ -841,12 +842,12 @@ static bool LoadMultiDexFilesFromOatFile(const OatFile* oat_file, } if (success) { - const DexFile* dex_file = oat_dex_file->OpenDexFile(&error_msg); - if (dex_file == nullptr) { + std::unique_ptr<const DexFile> dex_file = oat_dex_file->OpenDexFile(&error_msg); + if (dex_file.get() == nullptr) { success = false; error_msgs->push_back(error_msg); } else { - dex_files->push_back(dex_file); + dex_files->push_back(std::move(dex_file)); } } @@ -864,14 +865,7 @@ static bool LoadMultiDexFilesFromOatFile(const OatFile* oat_file, if (success) { return true; } else { - // Free all the dex files we have loaded. - auto it = dex_files->begin() + old_size; - auto it_end = dex_files->end(); - for (; it != it_end; it++) { - delete *it; - } - dex_files->erase(dex_files->begin() + old_size, it_end); - + dex_files->erase(dex_files->begin() + old_size, dex_files->end()); return false; } } @@ -882,7 +876,7 @@ static bool LoadMultiDexFilesFromOatFile(const OatFile* oat_file, // multidex ahead of time. bool ClassLinker::OpenDexFilesFromOat(const char* dex_location, const char* oat_location, std::vector<std::string>* error_msgs, - std::vector<const DexFile*>* dex_files) { + std::vector<std::unique_ptr<const DexFile>>* dex_files) { // 1) Check whether we have an open oat file. // This requires a dex checksum, use the "primary" one. uint32_t dex_location_checksum; @@ -1232,15 +1226,15 @@ bool ClassLinker::VerifyOatWithDexFile(const OatFile* oat_file, error_msg->c_str()); return false; } - dex_file.reset(oat_dex_file->OpenDexFile(error_msg)); + dex_file = oat_dex_file->OpenDexFile(error_msg); } else { bool verified = VerifyOatAndDexFileChecksums(oat_file, dex_location, *dex_location_checksum, kRuntimeISA, error_msg); if (!verified) { return false; } - dex_file.reset(oat_file->GetOatDexFile(dex_location, - dex_location_checksum)->OpenDexFile(error_msg)); + dex_file = oat_file->GetOatDexFile(dex_location, + dex_location_checksum)->OpenDexFile(error_msg); } return dex_file.get() != nullptr; } @@ -1685,8 +1679,8 @@ void ClassLinker::InitFromImage() { nullptr); CHECK(oat_dex_file != nullptr) << oat_file.GetLocation() << " " << dex_file_location; std::string error_msg; - const DexFile* dex_file = oat_dex_file->OpenDexFile(&error_msg); - if (dex_file == nullptr) { + std::unique_ptr<const DexFile> dex_file = oat_dex_file->OpenDexFile(&error_msg); + if (dex_file.get() == nullptr) { LOG(FATAL) << "Failed to open dex file " << dex_file_location << " from within oat file " << oat_file.GetLocation() << " error '" << error_msg << "'"; @@ -1695,7 +1689,8 @@ void ClassLinker::InitFromImage() { CHECK_EQ(dex_file->GetLocationChecksum(), oat_dex_file->GetDexFileLocationChecksum()); - AppendToBootClassPath(*dex_file, dex_cache); + AppendToBootClassPath(*dex_file.get(), dex_cache); + opened_dex_files_.push_back(std::move(dex_file)); } // Set classes on AbstractMethod early so that IsMethod tests can be performed during the live @@ -1928,7 +1923,6 @@ ClassLinker::~ClassLinker() { mirror::ShortArray::ResetArrayClass(); mirror::Throwable::ResetClass(); mirror::StackTraceElement::ResetClass(); - STLDeleteElements(&boot_class_path_); STLDeleteElements(&oat_files_); } diff --git a/runtime/class_linker.h b/runtime/class_linker.h index 6461835..6570c5f 100644 --- a/runtime/class_linker.h +++ b/runtime/class_linker.h @@ -105,7 +105,7 @@ class ClassLinker { ~ClassLinker(); // Initialize class linker by bootstraping from dex files. - void InitWithoutImage(const std::vector<const DexFile*>& boot_class_path) + void InitWithoutImage(std::vector<std::unique_ptr<const DexFile>> boot_class_path) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); // Initialize class linker from one or more images. @@ -324,7 +324,7 @@ class ClassLinker { // (if multidex) into the given vector. bool OpenDexFilesFromOat(const char* dex_location, const char* oat_location, std::vector<std::string>* error_msgs, - std::vector<const DexFile*>* dex_files) + std::vector<std::unique_ptr<const DexFile>>* dex_files) LOCKS_EXCLUDED(dex_lock_, Locks::mutator_lock_); // Returns true if the given oat file has the same image checksum as the image it is paired with. @@ -722,6 +722,7 @@ class ClassLinker { const void* GetRuntimeQuickGenericJniStub() const; std::vector<const DexFile*> boot_class_path_; + std::vector<std::unique_ptr<const DexFile>> opened_dex_files_; mutable ReaderWriterMutex dex_lock_ DEFAULT_MUTEX_ACQUIRED_AFTER; std::vector<size_t> new_dex_cache_roots_ GUARDED_BY(dex_lock_); diff --git a/runtime/class_linker_test.cc b/runtime/class_linker_test.cc index 4f09460..6c7c1e2 100644 --- a/runtime/class_linker_test.cc +++ b/runtime/class_linker_test.cc @@ -342,28 +342,26 @@ class ClassLinkerTest : public CommonRuntimeTest { } } - void AssertDexFile(const DexFile* dex, mirror::ClassLoader* class_loader) + void AssertDexFile(const DexFile& dex, mirror::ClassLoader* class_loader) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) { - ASSERT_TRUE(dex != nullptr); - // Verify all the classes defined in this file - for (size_t i = 0; i < dex->NumClassDefs(); i++) { - const DexFile::ClassDef& class_def = dex->GetClassDef(i); - const char* descriptor = dex->GetClassDescriptor(class_def); + for (size_t i = 0; i < dex.NumClassDefs(); i++) { + const DexFile::ClassDef& class_def = dex.GetClassDef(i); + const char* descriptor = dex.GetClassDescriptor(class_def); AssertDexFileClass(class_loader, descriptor); } // Verify all the types referenced by this file - for (size_t i = 0; i < dex->NumTypeIds(); i++) { - const DexFile::TypeId& type_id = dex->GetTypeId(i); - const char* descriptor = dex->GetTypeDescriptor(type_id); + for (size_t i = 0; i < dex.NumTypeIds(); i++) { + const DexFile::TypeId& type_id = dex.GetTypeId(i); + const char* descriptor = dex.GetTypeDescriptor(type_id); AssertDexFileClass(class_loader, descriptor); } class_linker_->VisitRoots(TestRootVisitor, nullptr, kVisitRootFlagAllRoots); // Verify the dex cache has resolution methods in all resolved method slots - mirror::DexCache* dex_cache = class_linker_->FindDexCache(*dex); + mirror::DexCache* dex_cache = class_linker_->FindDexCache(dex); mirror::ObjectArray<mirror::ArtMethod>* resolved_methods = dex_cache->GetResolvedMethods(); for (size_t i = 0; i < static_cast<size_t>(resolved_methods->GetLength()); i++) { - EXPECT_TRUE(resolved_methods->Get(i) != nullptr) << dex->GetLocation() << " i=" << i; + EXPECT_TRUE(resolved_methods->Get(i) != nullptr) << dex.GetLocation() << " i=" << i; } } @@ -744,7 +742,8 @@ TEST_F(ClassLinkerTest, FindClass) { TEST_F(ClassLinkerTest, LibCore) { ScopedObjectAccess soa(Thread::Current()); - AssertDexFile(java_lang_dex_file_, nullptr); + ASSERT_TRUE(java_lang_dex_file_ != nullptr); + AssertDexFile(*java_lang_dex_file_, nullptr); } // The first reference array element must be a multiple of 4 bytes from the diff --git a/runtime/common_runtime_test.cc b/runtime/common_runtime_test.cc index 75ba9dd..e017699 100644 --- a/runtime/common_runtime_test.cc +++ b/runtime/common_runtime_test.cc @@ -102,7 +102,11 @@ void ScratchFile::Unlink() { } CommonRuntimeTest::CommonRuntimeTest() {} -CommonRuntimeTest::~CommonRuntimeTest() {} +CommonRuntimeTest::~CommonRuntimeTest() { + // Ensure the dex files are cleaned up before the runtime. + loaded_dex_files_.clear(); + runtime_.reset(); +} void CommonRuntimeTest::SetUpAndroidRoot() { if (IsHost()) { @@ -181,15 +185,15 @@ std::string CommonRuntimeTest::GetCoreOatLocation() { return GetCoreFileLocation("oat"); } -const DexFile* CommonRuntimeTest::LoadExpectSingleDexFile(const char* location) { - std::vector<const DexFile*> dex_files; +std::unique_ptr<const DexFile> CommonRuntimeTest::LoadExpectSingleDexFile(const char* location) { + std::vector<std::unique_ptr<const DexFile>> dex_files; std::string error_msg; if (!DexFile::Open(location, location, &error_msg, &dex_files)) { LOG(FATAL) << "Could not open .dex file '" << location << "': " << error_msg << "\n"; - return nullptr; + UNREACHABLE(); } else { CHECK_EQ(1U, dex_files.size()) << "Expected only one dex file in " << location; - return dex_files[0]; + return std::move(dex_files[0]); } } @@ -222,6 +226,9 @@ void CommonRuntimeTest::SetUp() { class_linker_ = runtime_->GetClassLinker(); class_linker_->FixupDexCaches(runtime_->GetResolutionMethod()); class_linker_->RunRootClinits(); + boot_class_path_ = class_linker_->GetBootClassPath(); + java_lang_dex_file_ = boot_class_path_[0]; + // Runtime::Create acquired the mutator_lock_ that is normally given away when we // Runtime::Start, give it away now and then switch to a more managable ScopedObjectAccess. @@ -285,8 +292,6 @@ void CommonRuntimeTest::TearDown() { IcuCleanupFn icu_cleanup_fn = reinterpret_cast<IcuCleanupFn>(sym); (*icu_cleanup_fn)(); - STLDeleteElements(&opened_dex_files_); - Runtime::Current()->GetHeap()->VerifyHeap(); // Check for heap corruption after the test } @@ -323,7 +328,7 @@ std::string CommonRuntimeTest::GetTestAndroidRoot() { #define ART_TARGET_NATIVETEST_DIR_STRING "" #endif -std::vector<const DexFile*> CommonRuntimeTest::OpenTestDexFiles(const char* name) { +std::vector<std::unique_ptr<const DexFile>> CommonRuntimeTest::OpenTestDexFiles(const char* name) { CHECK(name != nullptr); std::string filename; if (IsHost()) { @@ -336,28 +341,30 @@ std::vector<const DexFile*> CommonRuntimeTest::OpenTestDexFiles(const char* name filename += name; filename += ".jar"; std::string error_msg; - std::vector<const DexFile*> dex_files; + std::vector<std::unique_ptr<const DexFile>> dex_files; bool success = DexFile::Open(filename.c_str(), filename.c_str(), &error_msg, &dex_files); CHECK(success) << "Failed to open '" << filename << "': " << error_msg; - for (const DexFile* dex_file : dex_files) { + for (auto& dex_file : dex_files) { CHECK_EQ(PROT_READ, dex_file->GetPermissions()); CHECK(dex_file->IsReadOnly()); } - opened_dex_files_.insert(opened_dex_files_.end(), dex_files.begin(), dex_files.end()); return dex_files; } -const DexFile* CommonRuntimeTest::OpenTestDexFile(const char* name) { - std::vector<const DexFile*> vector = OpenTestDexFiles(name); +std::unique_ptr<const DexFile> CommonRuntimeTest::OpenTestDexFile(const char* name) { + std::vector<std::unique_ptr<const DexFile>> vector = OpenTestDexFiles(name); EXPECT_EQ(1U, vector.size()); - return vector[0]; + return std::move(vector[0]); } jobject CommonRuntimeTest::LoadDex(const char* dex_name) { - std::vector<const DexFile*> dex_files = OpenTestDexFiles(dex_name); + std::vector<std::unique_ptr<const DexFile>> dex_files = OpenTestDexFiles(dex_name); + std::vector<const DexFile*> class_path; CHECK_NE(0U, dex_files.size()); - for (const DexFile* dex_file : dex_files) { + for (auto& dex_file : dex_files) { + class_path.push_back(dex_file.get()); class_linker_->RegisterDexFile(*dex_file); + loaded_dex_files_.push_back(std::move(dex_file)); } Thread* self = Thread::Current(); JNIEnvExt* env = self->GetJniEnv(); @@ -365,7 +372,7 @@ jobject CommonRuntimeTest::LoadDex(const char* dex_name) { env->AllocObject(WellKnownClasses::dalvik_system_PathClassLoader)); jobject class_loader = env->NewGlobalRef(class_loader_local.get()); self->SetClassLoaderOverride(class_loader_local.get()); - Runtime::Current()->SetCompileTimeClassPath(class_loader, dex_files); + Runtime::Current()->SetCompileTimeClassPath(class_loader, class_path); return class_loader; } diff --git a/runtime/common_runtime_test.h b/runtime/common_runtime_test.h index 35dc30f..38a9733 100644 --- a/runtime/common_runtime_test.h +++ b/runtime/common_runtime_test.h @@ -87,7 +87,7 @@ class CommonRuntimeTest : public testing::Test { // File location to core.oat, e.g. $ANDROID_HOST_OUT/system/framework/core.oat static std::string GetCoreOatLocation(); - const DexFile* LoadExpectSingleDexFile(const char* location); + std::unique_ptr<const DexFile> LoadExpectSingleDexFile(const char* location); virtual void SetUp(); @@ -106,26 +106,30 @@ class CommonRuntimeTest : public testing::Test { std::string GetTestAndroidRoot(); - std::vector<const DexFile*> OpenTestDexFiles(const char* name) + std::vector<std::unique_ptr<const DexFile>> OpenTestDexFiles(const char* name) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); - const DexFile* OpenTestDexFile(const char* name) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); + std::unique_ptr<const DexFile> OpenTestDexFile(const char* name) + SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); jobject LoadDex(const char* dex_name) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); std::string android_data_; std::string dalvik_cache_; - const DexFile* java_lang_dex_file_; // owned by runtime_ - std::vector<const DexFile*> boot_class_path_; // owned by runtime_ + std::unique_ptr<Runtime> runtime_; - // Owned by the runtime + + // The class_linker_, java_lang_dex_file_, and boot_class_path_ are all + // owned by the runtime. ClassLinker* class_linker_; + const DexFile* java_lang_dex_file_; + std::vector<const DexFile*> boot_class_path_; private: static std::string GetCoreFileLocation(const char* suffix); std::unique_ptr<CompilerCallbacks> callbacks_; - std::vector<const DexFile*> opened_dex_files_; + std::vector<std::unique_ptr<const DexFile>> loaded_dex_files_; }; // Sets a CheckJni abort hook to catch failures. Note that this will cause CheckJNI to carry on diff --git a/runtime/dex_file.cc b/runtime/dex_file.cc index 3d4184b..3f6175f 100644 --- a/runtime/dex_file.cc +++ b/runtime/dex_file.cc @@ -125,7 +125,8 @@ bool DexFile::GetChecksum(const char* filename, uint32_t* checksum, std::string* } bool DexFile::Open(const char* filename, const char* location, std::string* error_msg, - std::vector<const DexFile*>* dex_files) { + std::vector<std::unique_ptr<const DexFile>>* dex_files) { + DCHECK(dex_files != nullptr) << "DexFile::Open: out-param is NULL"; uint32_t magic; ScopedFd fd(OpenAndReadMagic(filename, &magic, error_msg)); if (fd.get() == -1) { @@ -139,7 +140,7 @@ bool DexFile::Open(const char* filename, const char* location, std::string* erro std::unique_ptr<const DexFile> dex_file(DexFile::OpenFile(fd.release(), location, true, error_msg)); if (dex_file.get() != nullptr) { - dex_files->push_back(dex_file.release()); + dex_files->push_back(std::move(dex_file)); return true; } else { return false; @@ -179,8 +180,8 @@ bool DexFile::DisableWrite() const { } } -const DexFile* DexFile::OpenFile(int fd, const char* location, bool verify, - std::string* error_msg) { +std::unique_ptr<const DexFile> DexFile::OpenFile(int fd, const char* location, bool verify, + std::string* error_msg) { CHECK(location != nullptr); std::unique_ptr<MemMap> map; { @@ -224,13 +225,14 @@ const DexFile* DexFile::OpenFile(int fd, const char* location, bool verify, return nullptr; } - return dex_file.release(); + return dex_file; } const char* DexFile::kClassesDex = "classes.dex"; bool DexFile::OpenZip(int fd, const std::string& location, std::string* error_msg, - std::vector<const DexFile*>* dex_files) { + std::vector<std::unique_ptr<const DexFile>>* dex_files) { + DCHECK(dex_files != nullptr) << "DexFile::OpenZip: out-param is NULL"; std::unique_ptr<ZipArchive> zip_archive(ZipArchive::OpenFromFd(fd, location.c_str(), error_msg)); if (zip_archive.get() == nullptr) { DCHECK(!error_msg->empty()); @@ -239,10 +241,10 @@ bool DexFile::OpenZip(int fd, const std::string& location, std::string* error_ms return DexFile::OpenFromZip(*zip_archive, location, error_msg, dex_files); } -const DexFile* DexFile::OpenMemory(const std::string& location, - uint32_t location_checksum, - MemMap* mem_map, - std::string* error_msg) { +std::unique_ptr<const DexFile> DexFile::OpenMemory(const std::string& location, + uint32_t location_checksum, + MemMap* mem_map, + std::string* error_msg) { return OpenMemory(mem_map->Begin(), mem_map->Size(), location, @@ -251,9 +253,9 @@ const DexFile* DexFile::OpenMemory(const std::string& location, error_msg); } -const DexFile* DexFile::Open(const ZipArchive& zip_archive, const char* entry_name, - const std::string& location, std::string* error_msg, - ZipOpenErrorCode* error_code) { +std::unique_ptr<const DexFile> DexFile::Open(const ZipArchive& zip_archive, const char* entry_name, + const std::string& location, std::string* error_msg, + ZipOpenErrorCode* error_code) { CHECK(!location.empty()); std::unique_ptr<ZipEntry> zip_entry(zip_archive.Find(entry_name, error_msg)); if (zip_entry.get() == NULL) { @@ -287,11 +289,13 @@ const DexFile* DexFile::Open(const ZipArchive& zip_archive, const char* entry_na return nullptr; } *error_code = ZipOpenErrorCode::kNoError; - return dex_file.release(); + return dex_file; } bool DexFile::OpenFromZip(const ZipArchive& zip_archive, const std::string& location, - std::string* error_msg, std::vector<const DexFile*>* dex_files) { + std::string* error_msg, + std::vector<std::unique_ptr<const DexFile>>* dex_files) { + DCHECK(dex_files != nullptr) << "DexFile::OpenFromZip: out-param is NULL"; ZipOpenErrorCode error_code; std::unique_ptr<const DexFile> dex_file(Open(zip_archive, kClassesDex, location, error_msg, &error_code)); @@ -299,7 +303,7 @@ bool DexFile::OpenFromZip(const ZipArchive& zip_archive, const std::string& loca return false; } else { // Had at least classes.dex. - dex_files->push_back(dex_file.release()); + dex_files->push_back(std::move(dex_file)); // Now try some more. size_t i = 2; @@ -318,7 +322,7 @@ bool DexFile::OpenFromZip(const ZipArchive& zip_archive, const std::string& loca } break; } else { - dex_files->push_back(next_dex_file.release()); + dex_files->push_back(std::move(next_dex_file)); } i++; @@ -329,18 +333,17 @@ bool DexFile::OpenFromZip(const ZipArchive& zip_archive, const std::string& loca } -const DexFile* DexFile::OpenMemory(const uint8_t* base, - size_t size, - const std::string& location, - uint32_t location_checksum, - MemMap* mem_map, std::string* error_msg) { +std::unique_ptr<const DexFile> DexFile::OpenMemory(const uint8_t* base, + size_t size, + const std::string& location, + uint32_t location_checksum, + MemMap* mem_map, std::string* error_msg) { CHECK_ALIGNED(base, 4); // various dex file structures must be word aligned std::unique_ptr<DexFile> dex_file(new DexFile(base, size, location, location_checksum, mem_map)); if (!dex_file->Init(error_msg)) { - return nullptr; - } else { - return dex_file.release(); + dex_file.reset(); } + return std::unique_ptr<const DexFile>(dex_file.release()); } DexFile::DexFile(const uint8_t* base, size_t size, diff --git a/runtime/dex_file.h b/runtime/dex_file.h index a71ca42..019c8e6 100644 --- a/runtime/dex_file.h +++ b/runtime/dex_file.h @@ -385,19 +385,20 @@ class DexFile { // Opens .dex files found in the container, guessing the container format based on file extension. static bool Open(const char* filename, const char* location, std::string* error_msg, - std::vector<const DexFile*>* dex_files); + std::vector<std::unique_ptr<const DexFile>>* dex_files); // Opens .dex file, backed by existing memory - static const DexFile* Open(const uint8_t* base, size_t size, - const std::string& location, - uint32_t location_checksum, - std::string* error_msg) { + static std::unique_ptr<const DexFile> Open(const uint8_t* base, size_t size, + const std::string& location, + uint32_t location_checksum, + std::string* error_msg) { return OpenMemory(base, size, location, location_checksum, NULL, error_msg); } // Open all classesXXX.dex files from a zip archive. static bool OpenFromZip(const ZipArchive& zip_archive, const std::string& location, - std::string* error_msg, std::vector<const DexFile*>* dex_files); + std::string* error_msg, + std::vector<std::unique_ptr<const DexFile>>* dex_files); // Closes a .dex file. virtual ~DexFile(); @@ -892,11 +893,12 @@ class DexFile { private: // Opens a .dex file - static const DexFile* OpenFile(int fd, const char* location, bool verify, std::string* error_msg); + static std::unique_ptr<const DexFile> OpenFile(int fd, const char* location, + bool verify, std::string* error_msg); // Opens dex files from within a .jar, .zip, or .apk file static bool OpenZip(int fd, const std::string& location, std::string* error_msg, - std::vector<const DexFile*>* dex_files); + std::vector<std::unique_ptr<const DexFile>>* dex_files); enum class ZipOpenErrorCode { // private kNoError, @@ -909,23 +911,23 @@ class DexFile { // Opens .dex file from the entry_name in a zip archive. error_code is undefined when non-nullptr // return. - static const DexFile* Open(const ZipArchive& zip_archive, const char* entry_name, - const std::string& location, std::string* error_msg, - ZipOpenErrorCode* error_code); + static std::unique_ptr<const DexFile> Open(const ZipArchive& zip_archive, const char* entry_name, + const std::string& location, std::string* error_msg, + ZipOpenErrorCode* error_code); // Opens a .dex file at the given address backed by a MemMap - static const DexFile* OpenMemory(const std::string& location, - uint32_t location_checksum, - MemMap* mem_map, - std::string* error_msg); + static std::unique_ptr<const DexFile> OpenMemory(const std::string& location, + uint32_t location_checksum, + MemMap* mem_map, + std::string* error_msg); // Opens a .dex file at the given address, optionally backed by a MemMap - static const DexFile* OpenMemory(const uint8_t* dex_file, - size_t size, - const std::string& location, - uint32_t location_checksum, - MemMap* mem_map, - std::string* error_msg); + static std::unique_ptr<const DexFile> OpenMemory(const uint8_t* dex_file, + size_t size, + const std::string& location, + uint32_t location_checksum, + MemMap* mem_map, + std::string* error_msg); DexFile(const uint8_t* base, size_t size, const std::string& location, diff --git a/runtime/dex_file_test.cc b/runtime/dex_file_test.cc index 0b54d47..7f5a181 100644 --- a/runtime/dex_file_test.cc +++ b/runtime/dex_file_test.cc @@ -32,8 +32,8 @@ class DexFileTest : public CommonRuntimeTest {}; TEST_F(DexFileTest, Open) { ScopedObjectAccess soa(Thread::Current()); - const DexFile* dex(OpenTestDexFile("Nested")); - ASSERT_TRUE(dex != NULL); + std::unique_ptr<const DexFile> dex(OpenTestDexFile("Nested")); + ASSERT_TRUE(dex.get() != NULL); } static const uint8_t kBase64Map[256] = { @@ -133,8 +133,8 @@ static const char kRawDex[] = "AAACAAAAQAEAAAEgAAACAAAAVAEAAAYgAAACAAAAiAEAAAEQAAABAAAAqAEAAAIgAAAPAAAArgEA" "AAMgAAACAAAAiAIAAAQgAAADAAAAlAIAAAAgAAACAAAAqwIAAAAQAAABAAAAxAIAAA=="; -static const DexFile* OpenDexFileBase64(const char* base64, - const char* location) { +static std::unique_ptr<const DexFile> OpenDexFileBase64(const char* base64, + const char* location) { // decode base64 CHECK(base64 != NULL); size_t length; @@ -155,11 +155,11 @@ static const DexFile* OpenDexFileBase64(const char* base64, // read dex file ScopedObjectAccess soa(Thread::Current()); std::string error_msg; - std::vector<const DexFile*> tmp; + std::vector<std::unique_ptr<const DexFile>> tmp; bool success = DexFile::Open(location, location, &error_msg, &tmp); CHECK(success) << error_msg; EXPECT_EQ(1U, tmp.size()); - const DexFile* dex_file = tmp[0]; + std::unique_ptr<const DexFile> dex_file = std::move(tmp[0]); EXPECT_EQ(PROT_READ, dex_file->GetPermissions()); EXPECT_TRUE(dex_file->IsReadOnly()); return dex_file; @@ -198,7 +198,7 @@ TEST_F(DexFileTest, Header) { TEST_F(DexFileTest, GetLocationChecksum) { ScopedObjectAccess soa(Thread::Current()); - const DexFile* raw(OpenTestDexFile("Main")); + std::unique_ptr<const DexFile> raw(OpenTestDexFile("Main")); EXPECT_NE(raw->GetHeader().checksum_, raw->GetLocationChecksum()); } @@ -213,8 +213,8 @@ TEST_F(DexFileTest, GetChecksum) { TEST_F(DexFileTest, ClassDefs) { ScopedObjectAccess soa(Thread::Current()); - const DexFile* raw(OpenTestDexFile("Nested")); - ASSERT_TRUE(raw != NULL); + std::unique_ptr<const DexFile> raw(OpenTestDexFile("Nested")); + ASSERT_TRUE(raw.get() != nullptr); EXPECT_EQ(2U, raw->NumClassDefs()); const DexFile::ClassDef& c0 = raw->GetClassDef(0); @@ -226,8 +226,8 @@ TEST_F(DexFileTest, ClassDefs) { TEST_F(DexFileTest, GetMethodSignature) { ScopedObjectAccess soa(Thread::Current()); - const DexFile* raw(OpenTestDexFile("GetMethodSignature")); - ASSERT_TRUE(raw != NULL); + std::unique_ptr<const DexFile> raw(OpenTestDexFile("GetMethodSignature")); + ASSERT_TRUE(raw.get() != nullptr); EXPECT_EQ(1U, raw->NumClassDefs()); const DexFile::ClassDef& class_def = raw->GetClassDef(0); @@ -276,8 +276,8 @@ TEST_F(DexFileTest, GetMethodSignature) { TEST_F(DexFileTest, FindStringId) { ScopedObjectAccess soa(Thread::Current()); - const DexFile* raw(OpenTestDexFile("GetMethodSignature")); - ASSERT_TRUE(raw != NULL); + std::unique_ptr<const DexFile> raw(OpenTestDexFile("GetMethodSignature")); + ASSERT_TRUE(raw.get() != nullptr); EXPECT_EQ(1U, raw->NumClassDefs()); const char* strings[] = { "LGetMethodSignature;", "Ljava/lang/Float;", "Ljava/lang/Object;", diff --git a/runtime/dex_file_verifier_test.cc b/runtime/dex_file_verifier_test.cc index ec1e5f0..00ca8a9 100644 --- a/runtime/dex_file_verifier_test.cc +++ b/runtime/dex_file_verifier_test.cc @@ -101,8 +101,9 @@ static inline uint8_t* DecodeBase64(const char* src, size_t* dst_size) { return dst.release(); } -static const DexFile* OpenDexFileBase64(const char* base64, const char* location, - std::string* error_msg) { +static std::unique_ptr<const DexFile> OpenDexFileBase64(const char* base64, + const char* location, + std::string* error_msg) { // decode base64 CHECK(base64 != NULL); size_t length; @@ -122,11 +123,11 @@ static const DexFile* OpenDexFileBase64(const char* base64, const char* location // read dex file ScopedObjectAccess soa(Thread::Current()); - std::vector<const DexFile*> tmp; + std::vector<std::unique_ptr<const DexFile>> tmp; bool success = DexFile::Open(location, location, error_msg, &tmp); CHECK(success) << error_msg; EXPECT_EQ(1U, tmp.size()); - const DexFile* dex_file = tmp[0]; + std::unique_ptr<const DexFile> dex_file = std::move(tmp[0]); EXPECT_EQ(PROT_READ, dex_file->GetPermissions()); EXPECT_TRUE(dex_file->IsReadOnly()); return dex_file; @@ -166,8 +167,9 @@ static void FixUpChecksum(uint8_t* dex_file) { header->checksum_ = adler_checksum; } -static const DexFile* FixChecksumAndOpen(uint8_t* bytes, size_t length, const char* location, - std::string* error_msg) { +static std::unique_ptr<const DexFile> FixChecksumAndOpen(uint8_t* bytes, size_t length, + const char* location, + std::string* error_msg) { // Check data. CHECK(bytes != nullptr); @@ -187,12 +189,12 @@ static const DexFile* FixChecksumAndOpen(uint8_t* bytes, size_t length, const ch // read dex file ScopedObjectAccess soa(Thread::Current()); - std::vector<const DexFile*> tmp; + std::vector<std::unique_ptr<const DexFile>> tmp; if (!DexFile::Open(location, location, error_msg, &tmp)) { return nullptr; } EXPECT_EQ(1U, tmp.size()); - const DexFile* dex_file = tmp[0]; + std::unique_ptr<const DexFile> dex_file = std::move(tmp[0]); EXPECT_EQ(PROT_READ, dex_file->GetPermissions()); EXPECT_TRUE(dex_file->IsReadOnly()); return dex_file; diff --git a/runtime/mirror/dex_cache_test.cc b/runtime/mirror/dex_cache_test.cc index ef6fc67..53e5534 100644 --- a/runtime/mirror/dex_cache_test.cc +++ b/runtime/mirror/dex_cache_test.cc @@ -34,6 +34,7 @@ class DexCacheTest : public CommonRuntimeTest {}; TEST_F(DexCacheTest, Open) { ScopedObjectAccess soa(Thread::Current()); StackHandleScope<1> hs(soa.Self()); + ASSERT_TRUE(java_lang_dex_file_ != NULL); Handle<DexCache> dex_cache( hs.NewHandle(class_linker_->AllocDexCache(soa.Self(), *java_lang_dex_file_))); ASSERT_TRUE(dex_cache.Get() != NULL); diff --git a/runtime/native/dalvik_system_DexFile.cc b/runtime/native/dalvik_system_DexFile.cc index 44c6d87..037072d 100644 --- a/runtime/native/dalvik_system_DexFile.cc +++ b/runtime/native/dalvik_system_DexFile.cc @@ -115,7 +115,8 @@ static jlong DexFile_openDexFileNative(JNIEnv* env, jclass, jstring javaSourceNa } ClassLinker* linker = Runtime::Current()->GetClassLinker(); - std::unique_ptr<std::vector<const DexFile*>> dex_files(new std::vector<const DexFile*>()); + std::unique_ptr<std::vector<std::unique_ptr<const DexFile>>> dex_files( + new std::vector<std::unique_ptr<const DexFile>>()); std::vector<std::string> error_msgs; bool success = linker->OpenDexFilesFromOat(sourceName.c_str(), outputName.c_str(), &error_msgs, @@ -143,9 +144,11 @@ static jlong DexFile_openDexFileNative(JNIEnv* env, jclass, jstring javaSourceNa } } -static std::vector<const DexFile*>* toDexFiles(jlong dex_file_address, JNIEnv* env) { - std::vector<const DexFile*>* dex_files = reinterpret_cast<std::vector<const DexFile*>*>( - static_cast<uintptr_t>(dex_file_address)); +static std::vector<std::unique_ptr<const DexFile>>* +toDexFiles(jlong dex_file_address, JNIEnv* env) { + std::vector<std::unique_ptr<const DexFile>>* dex_files + = reinterpret_cast<std::vector<std::unique_ptr<const DexFile>>*>( + static_cast<uintptr_t>(dex_file_address)); if (UNLIKELY(dex_files == nullptr)) { ScopedObjectAccess soa(env); ThrowNullPointerException(NULL, "dex_file == null"); @@ -154,27 +157,29 @@ static std::vector<const DexFile*>* toDexFiles(jlong dex_file_address, JNIEnv* e } static void DexFile_closeDexFile(JNIEnv* env, jclass, jlong cookie) { - std::unique_ptr<std::vector<const DexFile*>> dex_files(toDexFiles(cookie, env)); + std::unique_ptr<std::vector<std::unique_ptr<const DexFile>>> dex_files(toDexFiles(cookie, env)); if (dex_files.get() == nullptr) { return; } ScopedObjectAccess soa(env); - size_t index = 0; - for (const DexFile* dex_file : *dex_files) { + // The Runtime currently never unloads classes, which means any registered + // dex files must be kept around forever in case they are used. We + // accomplish this here by explicitly leaking those dex files that are + // registered. + // + // TODO: The Runtime should support unloading of classes and freeing of the + // dex files for those unloaded classes rather than leaking dex files here. + for (auto& dex_file : *dex_files) { if (Runtime::Current()->GetClassLinker()->IsDexFileRegistered(*dex_file)) { - (*dex_files)[index] = nullptr; + dex_file.release(); } - index++; } - - STLDeleteElements(dex_files.get()); - // Unique_ptr will delete the vector itself. } static jclass DexFile_defineClassNative(JNIEnv* env, jclass, jstring javaName, jobject javaLoader, jlong cookie) { - std::vector<const DexFile*>* dex_files = toDexFiles(cookie, env); + std::vector<std::unique_ptr<const DexFile>>* dex_files = toDexFiles(cookie, env); if (dex_files == NULL) { VLOG(class_linker) << "Failed to find dex_file"; return NULL; @@ -186,7 +191,7 @@ static jclass DexFile_defineClassNative(JNIEnv* env, jclass, jstring javaName, j } const std::string descriptor(DotToDescriptor(class_name.c_str())); const size_t hash(ComputeModifiedUtf8Hash(descriptor.c_str())); - for (const DexFile* dex_file : *dex_files) { + for (auto& dex_file : *dex_files) { const DexFile::ClassDef* dex_class_def = dex_file->FindClassDef(descriptor.c_str(), hash); if (dex_class_def != nullptr) { ScopedObjectAccess soa(env); @@ -218,13 +223,13 @@ struct CharPointerComparator { // Note: this can be an expensive call, as we sort out duplicates in MultiDex files. static jobjectArray DexFile_getClassNameList(JNIEnv* env, jclass, jlong cookie) { jobjectArray result = nullptr; - std::vector<const DexFile*>* dex_files = toDexFiles(cookie, env); + std::vector<std::unique_ptr<const DexFile>>* dex_files = toDexFiles(cookie, env); if (dex_files != nullptr) { // Push all class descriptors into a set. Use set instead of unordered_set as we want to // retrieve all in the end. std::set<const char*, CharPointerComparator> descriptors; - for (const DexFile* dex_file : *dex_files) { + for (auto& dex_file : *dex_files) { for (size_t i = 0; i < dex_file->NumClassDefs(); ++i) { const DexFile::ClassDef& class_def = dex_file->GetClassDef(i); const char* descriptor = dex_file->GetClassDescriptor(class_def); diff --git a/runtime/oat_file.cc b/runtime/oat_file.cc index 1c6cc8b..358519b 100644 --- a/runtime/oat_file.cc +++ b/runtime/oat_file.cc @@ -454,7 +454,7 @@ size_t OatFile::OatDexFile::FileSize() const { return reinterpret_cast<const DexFile::Header*>(dex_file_pointer_)->file_size_; } -const DexFile* OatFile::OatDexFile::OpenDexFile(std::string* error_msg) const { +std::unique_ptr<const DexFile> OatFile::OatDexFile::OpenDexFile(std::string* error_msg) const { return DexFile::Open(dex_file_pointer_, FileSize(), dex_file_location_, dex_file_location_checksum_, error_msg); } diff --git a/runtime/oat_file.h b/runtime/oat_file.h index 831ba1e..6ae3c3e 100644 --- a/runtime/oat_file.h +++ b/runtime/oat_file.h @@ -210,7 +210,7 @@ class OatFile { class OatDexFile { public: // Opens the DexFile referred to by this OatDexFile from within the containing OatFile. - const DexFile* OpenDexFile(std::string* error_msg) const; + std::unique_ptr<const DexFile> OpenDexFile(std::string* error_msg) const; const OatFile* GetOatFile() const { return oat_file_; diff --git a/runtime/runtime.cc b/runtime/runtime.cc index c2e814b..fabbbfb 100644 --- a/runtime/runtime.cc +++ b/runtime/runtime.cc @@ -625,8 +625,9 @@ void Runtime::StartDaemonThreads() { } static bool OpenDexFilesFromImage(const std::string& image_location, - std::vector<const DexFile*>& dex_files, + std::vector<std::unique_ptr<const DexFile>>* dex_files, size_t* failures) { + DCHECK(dex_files != nullptr) << "OpenDexFilesFromImage: out-param is NULL"; std::string system_filename; bool has_system = false; std::string cache_filename_unused; @@ -670,11 +671,11 @@ static bool OpenDexFilesFromImage(const std::string& image_location, *failures += 1; continue; } - const DexFile* dex_file = oat_dex_file->OpenDexFile(&error_msg); - if (dex_file == nullptr) { + std::unique_ptr<const DexFile> dex_file = oat_dex_file->OpenDexFile(&error_msg); + if (dex_file.get() == nullptr) { *failures += 1; } else { - dex_files.push_back(dex_file); + dex_files->push_back(std::move(dex_file)); } } Runtime::Current()->GetClassLinker()->RegisterOatFile(oat_file.release()); @@ -685,7 +686,8 @@ static bool OpenDexFilesFromImage(const std::string& image_location, static size_t OpenDexFiles(const std::vector<std::string>& dex_filenames, const std::vector<std::string>& dex_locations, const std::string& image_location, - std::vector<const DexFile*>& dex_files) { + std::vector<std::unique_ptr<const DexFile>>* dex_files) { + DCHECK(dex_files != nullptr) << "OpenDexFiles: out-param is NULL"; size_t failure_count = 0; if (!image_location.empty() && OpenDexFilesFromImage(image_location, dex_files, &failure_count)) { return failure_count; @@ -699,7 +701,7 @@ static size_t OpenDexFiles(const std::vector<std::string>& dex_filenames, LOG(WARNING) << "Skipping non-existent dex file '" << dex_filename << "'"; continue; } - if (!DexFile::Open(dex_filename, dex_location, &error_msg, &dex_files)) { + if (!DexFile::Open(dex_filename, dex_location, &error_msg, dex_files)) { LOG(WARNING) << "Failed to open .dex from file '" << dex_filename << "': " << error_msg; ++failure_count; } @@ -887,9 +889,9 @@ bool Runtime::Init(const RuntimeOptions& raw_options, bool ignore_unrecognized) CHECK_EQ(dex_filenames.size(), dex_locations.size()); } - std::vector<const DexFile*> boot_class_path; - OpenDexFiles(dex_filenames, dex_locations, options->image_, boot_class_path); - class_linker_->InitWithoutImage(boot_class_path); + std::vector<std::unique_ptr<const DexFile>> boot_class_path; + OpenDexFiles(dex_filenames, dex_locations, options->image_, &boot_class_path); + class_linker_->InitWithoutImage(std::move(boot_class_path)); // TODO: Should we move the following to InitWithoutImage? SetInstructionSet(kRuntimeISA); for (int i = 0; i < Runtime::kLastCalleeSaveType; i++) { diff --git a/runtime/runtime.h b/runtime/runtime.h index e319963..d58fe3c 100644 --- a/runtime/runtime.h +++ b/runtime/runtime.h @@ -425,6 +425,9 @@ class Runtime { LOCKS_EXCLUDED(method_verifier_lock_); const std::vector<const DexFile*>& GetCompileTimeClassPath(jobject class_loader); + + // The caller is responsible for ensuring the class_path DexFiles remain + // valid as long as the Runtime object remains valid. void SetCompileTimeClassPath(jobject class_loader, std::vector<const DexFile*>& class_path); void StartProfiler(const char* profile_output_filename); diff --git a/runtime/verifier/method_verifier_test.cc b/runtime/verifier/method_verifier_test.cc index 770ca7e..f67adc1 100644 --- a/runtime/verifier/method_verifier_test.cc +++ b/runtime/verifier/method_verifier_test.cc @@ -41,14 +41,12 @@ class MethodVerifierTest : public CommonRuntimeTest { << error_msg; } - void VerifyDexFile(const DexFile* dex) + void VerifyDexFile(const DexFile& dex) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) { - ASSERT_TRUE(dex != NULL); - // Verify all the classes defined in this file - for (size_t i = 0; i < dex->NumClassDefs(); i++) { - const DexFile::ClassDef& class_def = dex->GetClassDef(i); - const char* descriptor = dex->GetClassDescriptor(class_def); + for (size_t i = 0; i < dex.NumClassDefs(); i++) { + const DexFile::ClassDef& class_def = dex.GetClassDef(i); + const char* descriptor = dex.GetClassDescriptor(class_def); VerifyClass(descriptor); } } @@ -56,7 +54,8 @@ class MethodVerifierTest : public CommonRuntimeTest { TEST_F(MethodVerifierTest, LibCore) { ScopedObjectAccess soa(Thread::Current()); - VerifyDexFile(java_lang_dex_file_); + ASSERT_TRUE(java_lang_dex_file_ != nullptr); + VerifyDexFile(*java_lang_dex_file_); } } // namespace verifier |