From 6e6f1b2ffb243b3e5ae112bba3cd52031deb31ba Mon Sep 17 00:00:00 2001 From: Calin Juravle Date: Tue, 15 Dec 2020 19:13:19 -0800 Subject: Do not open dex files in CLC if we only need to get dexopt status The verifying the class loader context when calling GetDexOptNeeded we only need the dex locations and the checksums. Opening the full dex files may lead to in memory extraction which is expensive and unnecessary. Add a special path in ClassLoaderContext::OpenDexFiles which will extract the locations and the checksums from the apk instead of actually opening the dex files. We re-uses the same logic in OpenDexFiles in order to avoid implementing the opening algorithm twice (which, given all the edge cases is not trivial). Bug: 169869944 Test: test-art-host Change-Id: Ic327889677ce697cd60c5c688281515b932a2a76 --- runtime/class_loader_context_test.cc | 305 ++++++++++++++++++++++------------- 1 file changed, 193 insertions(+), 112 deletions(-) (limited to 'runtime/class_loader_context_test.cc') diff --git a/runtime/class_loader_context_test.cc b/runtime/class_loader_context_test.cc index 987f0d14fd..ec51dee06c 100644 --- a/runtime/class_loader_context_test.cc +++ b/runtime/class_loader_context_test.cc @@ -142,22 +142,38 @@ class ClassLoaderContextTest : public CommonRuntimeTest { ClassLoaderContext* context, size_t index, std::vector>* all_dex_files, - bool classpath_matches_dex_location = true) { + bool classpath_matches_dex_location = true, + bool only_read_checksums = false) { ASSERT_TRUE(context != nullptr); - ASSERT_TRUE(context->dex_files_open_attempted_); - ASSERT_TRUE(context->dex_files_open_result_); + if (only_read_checksums) { + ASSERT_EQ(context->dex_files_state_, + ClassLoaderContext::ContextDexFilesState::kDexFilesChecksumsRead); + } else { + ASSERT_EQ(context->dex_files_state_, + ClassLoaderContext::ContextDexFilesState::kDexFilesOpened); + } ClassLoaderContext::ClassLoaderInfo& info = *context->GetParent(index); ASSERT_EQ(all_dex_files->size(), info.classpath.size()); - ASSERT_EQ(all_dex_files->size(), info.opened_dex_files.size()); - size_t cur_open_dex_index = 0; - for (size_t k = 0; k < all_dex_files->size(); k++) { - std::unique_ptr& opened_dex_file = - info.opened_dex_files[cur_open_dex_index++]; - std::unique_ptr& expected_dex_file = (*all_dex_files)[k]; + ASSERT_EQ(all_dex_files->size(), info.checksums.size()); + if (only_read_checksums) { + ASSERT_EQ(0u, info.opened_dex_files.size()); + } else { + ASSERT_EQ(all_dex_files->size(), info.opened_dex_files.size()); + } + for (size_t k = 0, cur_open_dex_index = 0; + k < all_dex_files->size(); + k++, cur_open_dex_index++) { + const std::string& opened_location = only_read_checksums + ? info.classpath[cur_open_dex_index] + : info.opened_dex_files[cur_open_dex_index]->GetLocation(); + uint32_t opened_checksum = only_read_checksums + ? info.checksums[cur_open_dex_index] + : info.opened_dex_files[cur_open_dex_index]->GetLocationChecksum(); + + std::unique_ptr& expected_dex_file = (*all_dex_files)[k]; std::string expected_location = expected_dex_file->GetLocation(); - const std::string& opened_location = opened_dex_file->GetLocation(); if (!IsAbsoluteLocation(opened_location)) { // If the opened location is relative (it was open from a relative path without a // classpath_dir) it might not match the expected location which is absolute in tests). @@ -169,7 +185,7 @@ class ClassLoaderContextTest : public CommonRuntimeTest { } else { ASSERT_EQ(expected_location, opened_location); } - ASSERT_EQ(expected_dex_file->GetLocationChecksum(), opened_dex_file->GetLocationChecksum()); + ASSERT_EQ(expected_dex_file->GetLocationChecksum(), opened_checksum); if (classpath_matches_dex_location) { ASSERT_EQ(info.classpath[k], opened_location); } @@ -190,8 +206,7 @@ class ClassLoaderContextTest : public CommonRuntimeTest { void VerifyContextForClassLoader(ClassLoaderContext* context) { ASSERT_TRUE(context != nullptr); - ASSERT_TRUE(context->dex_files_open_attempted_); - ASSERT_TRUE(context->dex_files_open_result_); + ASSERT_EQ(context->dex_files_state_, ClassLoaderContext::ContextDexFilesState::kDexFilesOpened); ASSERT_FALSE(context->owns_the_dex_files_); ASSERT_FALSE(context->special_shared_library_); } @@ -215,8 +230,106 @@ class ClassLoaderContextTest : public CommonRuntimeTest { } void PretendContextOpenedDexFiles(ClassLoaderContext* context) { - context->dex_files_open_attempted_ = true; - context->dex_files_open_result_ = true; + context->dex_files_state_ = ClassLoaderContext::ContextDexFilesState::kDexFilesOpened; + } + + void PretendContextOpenedDexFilesForChecksums(ClassLoaderContext* context) { + context->dex_files_state_ = ClassLoaderContext::ContextDexFilesState::kDexFilesChecksumsRead; + } + + void TestOpenDexFiles(bool only_read_checksums) { + std::string multidex_name = GetTestDexFileName("MultiDex"); + std::string myclass_dex_name = GetTestDexFileName("MyClass"); + std::string dex_name = GetTestDexFileName("Main"); + + std::unique_ptr context = + ClassLoaderContext::Create( + "PCL[" + multidex_name + ":" + myclass_dex_name + "];" + + "DLC[" + dex_name + "]"); + + ASSERT_TRUE(context->OpenDexFiles( + /*classpath_dir=*/ "", + /*context_fds=*/ std::vector(), + only_read_checksums)); + + VerifyContextSize(context.get(), 2); + + std::vector> all_dex_files0 = OpenTestDexFiles("MultiDex"); + std::vector> myclass_dex_files = OpenTestDexFiles("MyClass"); + for (size_t i = 0; i < myclass_dex_files.size(); i++) { + all_dex_files0.emplace_back(myclass_dex_files[i].release()); + } + VerifyOpenDexFiles(context.get(), + /*index=*/ 0, + &all_dex_files0, + /*classpath_matches_dex_location=*/ false, + only_read_checksums); + std::vector> all_dex_files1 = OpenTestDexFiles("Main"); + VerifyOpenDexFiles(context.get(), + /*index=*/ 1, + &all_dex_files1, + /*classpath_matches_dex_location=*/ false, + only_read_checksums); + } + + void TestOpenValidDexFilesRelative(bool use_classpath_dir, bool only_read_checksums) { + char cwd_buf[4096]; + if (getcwd(cwd_buf, arraysize(cwd_buf)) == nullptr) { + PLOG(FATAL) << "Could not get working directory"; + } + std::string multidex_name; + std::string myclass_dex_name; + std::string dex_name; + if (!CreateRelativeString(GetTestDexFileName("MultiDex"), cwd_buf, &multidex_name) || + !CreateRelativeString(GetTestDexFileName("MyClass"), cwd_buf, &myclass_dex_name) || + !CreateRelativeString(GetTestDexFileName("Main"), cwd_buf, &dex_name)) { + LOG(ERROR) << "Test OpenValidDexFilesRelative cannot be run because target dex files have no " + << "relative path."; + SUCCEED(); + return; + } + + std::unique_ptr context = + ClassLoaderContext::Create( + "PCL[" + multidex_name + ":" + myclass_dex_name + "];" + + "DLC[" + dex_name + "]"); + + ASSERT_TRUE(context->OpenDexFiles( + /*classpath_dir=*/ use_classpath_dir ? cwd_buf : "", + /*context_fds=*/ std::vector(), + only_read_checksums)); + VerifyContextSize(context.get(), 2); + + std::vector> all_dex_files0 = OpenTestDexFiles("MultiDex"); + std::vector> myclass_dex_files = OpenTestDexFiles("MyClass"); + for (size_t i = 0; i < myclass_dex_files.size(); i++) { + all_dex_files0.emplace_back(myclass_dex_files[i].release()); + } + VerifyOpenDexFiles(context.get(), + /*index=*/ 0, + &all_dex_files0, + /*classpath_matches_dex_location=*/ false, + only_read_checksums); + + std::vector> all_dex_files1 = OpenTestDexFiles("Main"); + VerifyOpenDexFiles(context.get(), + /*index=*/ 1, + &all_dex_files1, + /*classpath_matches_dex_location=*/ false, + only_read_checksums); + } + + // Creates a relative path from cwd to 'in'. Returns false if it cannot be done. + // TODO We should somehow support this in all situations. b/72042237. + bool CreateRelativeString(const std::string& in, const char* cwd, std::string* out) { + int cwd_len = strlen(cwd); + if (!android::base::StartsWith(in, cwd) || (cwd_len < 1)) { + return false; + } + bool contains_trailing_slash = (cwd[cwd_len - 1] == '/'); + int start_position = cwd_len + (contains_trailing_slash ? 0 : 1); + *out = in.substr(start_position); + return true; } private: @@ -404,113 +517,38 @@ TEST_F(ClassLoaderContextTest, OpenInvalidDexFiles) { ASSERT_FALSE(context->OpenDexFiles(".")); } -TEST_F(ClassLoaderContextTest, OpenValidDexFiles) { - std::string multidex_name = GetTestDexFileName("MultiDex"); - std::string myclass_dex_name = GetTestDexFileName("MyClass"); - std::string dex_name = GetTestDexFileName("Main"); - - +TEST_F(ClassLoaderContextTest, ReadChecksumsInvalidDexFiles) { std::unique_ptr context = - ClassLoaderContext::Create( - "PCL[" + multidex_name + ":" + myclass_dex_name + "];" + - "DLC[" + dex_name + "]"); - - ASSERT_TRUE(context->OpenDexFiles()); - - VerifyContextSize(context.get(), 2); - - std::vector> all_dex_files0 = OpenTestDexFiles("MultiDex"); - std::vector> myclass_dex_files = OpenTestDexFiles("MyClass"); - for (size_t i = 0; i < myclass_dex_files.size(); i++) { - all_dex_files0.emplace_back(myclass_dex_files[i].release()); - } - VerifyOpenDexFiles(context.get(), 0, &all_dex_files0); + ClassLoaderContext::Create("PCL[does_not_exist.dex]"); + VerifyContextSize(context.get(), 1); + ASSERT_FALSE(context->OpenDexFiles( + /*classpath_dir=*/ ".", + /*context_fds=*/ std::vector(), + /*only_read_checksums=*/ true)); +} - std::vector> all_dex_files1 = OpenTestDexFiles("Main"); - VerifyOpenDexFiles(context.get(), 1, &all_dex_files1); +TEST_F(ClassLoaderContextTest, OpenValidDexFiles) { + TestOpenDexFiles(/*only_read_checksums=*/ false); } -// Creates a relative path from cwd to 'in'. Returns false if it cannot be done. -// TODO We should somehow support this in all situations. b/72042237. -static bool CreateRelativeString(const std::string& in, const char* cwd, std::string* out) { - int cwd_len = strlen(cwd); - if (!android::base::StartsWith(in, cwd) || (cwd_len < 1)) { - return false; - } - bool contains_trailing_slash = (cwd[cwd_len - 1] == '/'); - int start_position = cwd_len + (contains_trailing_slash ? 0 : 1); - *out = in.substr(start_position); - return true; +TEST_F(ClassLoaderContextTest, ReadDexFileChecksums) { + TestOpenDexFiles(/*only_read_checksums=*/ true); } TEST_F(ClassLoaderContextTest, OpenValidDexFilesRelative) { - char cwd_buf[4096]; - if (getcwd(cwd_buf, arraysize(cwd_buf)) == nullptr) { - PLOG(FATAL) << "Could not get working directory"; - } - std::string multidex_name; - std::string myclass_dex_name; - std::string dex_name; - if (!CreateRelativeString(GetTestDexFileName("MultiDex"), cwd_buf, &multidex_name) || - !CreateRelativeString(GetTestDexFileName("MyClass"), cwd_buf, &myclass_dex_name) || - !CreateRelativeString(GetTestDexFileName("Main"), cwd_buf, &dex_name)) { - LOG(ERROR) << "Test OpenValidDexFilesRelative cannot be run because target dex files have no " - << "relative path."; - SUCCEED(); - return; - } - - std::unique_ptr context = - ClassLoaderContext::Create( - "PCL[" + multidex_name + ":" + myclass_dex_name + "];" + - "DLC[" + dex_name + "]"); - - ASSERT_TRUE(context->OpenDexFiles()); - - std::vector> all_dex_files0 = OpenTestDexFiles("MultiDex"); - std::vector> myclass_dex_files = OpenTestDexFiles("MyClass"); - for (size_t i = 0; i < myclass_dex_files.size(); i++) { - all_dex_files0.emplace_back(myclass_dex_files[i].release()); - } - VerifyOpenDexFiles(context.get(), 0, &all_dex_files0); + TestOpenValidDexFilesRelative(/*use_classpath_dir=*/ false, /*only_read_checksums=*/ false); +} - std::vector> all_dex_files1 = OpenTestDexFiles("Main"); - VerifyOpenDexFiles(context.get(), 1, &all_dex_files1); +TEST_F(ClassLoaderContextTest, ReadChecksumsValidDexFilesRelative) { + TestOpenValidDexFilesRelative(/*use_classpath_dir=*/ false, /*only_read_checksums=*/ true); } TEST_F(ClassLoaderContextTest, OpenValidDexFilesClasspathDir) { - char cwd_buf[4096]; - if (getcwd(cwd_buf, arraysize(cwd_buf)) == nullptr) { - PLOG(FATAL) << "Could not get working directory"; - } - std::string multidex_name; - std::string myclass_dex_name; - std::string dex_name; - if (!CreateRelativeString(GetTestDexFileName("MultiDex"), cwd_buf, &multidex_name) || - !CreateRelativeString(GetTestDexFileName("MyClass"), cwd_buf, &myclass_dex_name) || - !CreateRelativeString(GetTestDexFileName("Main"), cwd_buf, &dex_name)) { - LOG(ERROR) << "Test OpenValidDexFilesClasspathDir cannot be run because target dex files have " - << "no relative path."; - SUCCEED(); - return; - } - std::unique_ptr context = - ClassLoaderContext::Create( - "PCL[" + multidex_name + ":" + myclass_dex_name + "];" + - "DLC[" + dex_name + "]"); - - ASSERT_TRUE(context->OpenDexFiles(cwd_buf)); - - VerifyContextSize(context.get(), 2); - std::vector> all_dex_files0 = OpenTestDexFiles("MultiDex"); - std::vector> myclass_dex_files = OpenTestDexFiles("MyClass"); - for (size_t i = 0; i < myclass_dex_files.size(); i++) { - all_dex_files0.emplace_back(myclass_dex_files[i].release()); - } - VerifyOpenDexFiles(context.get(), 0, &all_dex_files0); + TestOpenValidDexFilesRelative(/*use_classpath_dir=*/ true, /*only_read_checksums=*/ false); +} - std::vector> all_dex_files1 = OpenTestDexFiles("Main"); - VerifyOpenDexFiles(context.get(), 1, &all_dex_files1); +TEST_F(ClassLoaderContextTest, ReadChecksumsValidDexFilesClasspathDir) { + TestOpenValidDexFilesRelative(/*use_classpath_dir=*/ true, /*only_read_checksums=*/ true); } TEST_F(ClassLoaderContextTest, OpenInvalidDexFilesMix) { @@ -520,6 +558,16 @@ TEST_F(ClassLoaderContextTest, OpenInvalidDexFilesMix) { ASSERT_FALSE(context->OpenDexFiles()); } +TEST_F(ClassLoaderContextTest, ReadChecksumsInvalidDexFilesMix) { + std::string dex_name = GetTestDexFileName("Main"); + std::unique_ptr context = + ClassLoaderContext::Create("PCL[does_not_exist.dex];DLC[" + dex_name + "]"); + ASSERT_FALSE(context->OpenDexFiles( + /*classpath_dir=*/ "", + /*context_fds=*/ std::vector(), + /*only_read_checksums=*/ true)); +} + TEST_F(ClassLoaderContextTest, OpenDexFilesForIMCFails) { std::unique_ptr context; std::string dex_name = GetTestDexFileName("Main"); @@ -529,6 +577,39 @@ TEST_F(ClassLoaderContextTest, OpenDexFilesForIMCFails) { ASSERT_FALSE(context->OpenDexFiles(".")); } +// Verify that we can fully open the dex files after only reading their checksums. +TEST_F(ClassLoaderContextTest, SubsequentOpenDexFilesOperations) { + std::string dex_name = GetTestDexFileName("Main"); + std::unique_ptr context = + ClassLoaderContext::Create("PCL[" + dex_name + "]"); + + std::vector> all_dex_files0 = OpenTestDexFiles("Main"); + + ASSERT_TRUE(context->OpenDexFiles( + /*classpath_dir=*/ "", + /*context_fds=*/ std::vector(), + /*only_read_checksums=*/ true)); + + VerifyOpenDexFiles( + context.get(), + /*index=*/ 0, + &all_dex_files0, + /*classpath_matches_dex_location=*/ false, + /*only_read_checksums=*/ true); + + ASSERT_TRUE(context->OpenDexFiles( + /*classpath_dir=*/ "", + /*context_fds=*/ std::vector(), + /*only_read_checksums=*/ false)); + + VerifyOpenDexFiles( + context.get(), + /*index=*/ 0, + &all_dex_files0, + /*classpath_matches_dex_location=*/ false, + /*only_read_checksums=*/ false); +} + TEST_F(ClassLoaderContextTest, CreateClassLoader) { std::string dex_name = GetTestDexFileName("Main"); std::unique_ptr context = @@ -1393,7 +1474,7 @@ TEST_F(ClassLoaderContextTest, VerifyClassLoaderContextFirstElement) { std::string context_spec = "PCL[]"; std::unique_ptr context = ParseContextWithChecksums(context_spec); ASSERT_TRUE(context != nullptr); - PretendContextOpenedDexFiles(context.get()); + PretendContextOpenedDexFilesForChecksums(context.get()); // Ensure that the special shared library marks as verified for the first thing in the class path. ASSERT_EQ(context->VerifyClassLoaderContextMatch(OatFile::kSpecialSharedLibrary), ClassLoaderContext::VerificationResult::kVerifies); @@ -1404,7 +1485,7 @@ TEST_F(ClassLoaderContextTest, VerifyClassLoaderContextMatch) { std::unique_ptr context = ParseContextWithChecksums(context_spec); // Pretend that we successfully open the dex files to pass the DCHECKS. // (as it's much easier to test all the corner cases without relying on actual dex files). - PretendContextOpenedDexFiles(context.get()); + PretendContextOpenedDexFilesForChecksums(context.get()); VerifyContextSize(context.get(), 2); VerifyClassLoaderPCL(context.get(), 0, "a.dex:b.dex"); -- cgit v1.2.3-59-g8ed1b