Do not fail when loading an APK without dex files.
There are some packages that claims to have dex code but actually
doesn't. There isn't a way to filter them out using Package Manager
APIs. Before this change, when ART Service tries to dexopt them,
profman prints annoying errors like "OpenZip failed for ... Entry not
found". See details in b/263245950.
This CL updates ArtDexFileLoader to just return an empty vector of dex
files if the APK doesn't have any dex file.
Bug: 263245950
Test: atest art_standalone_libdexfile_tests
Test: -
1. adb shell pm art dexopt-package -m speed-profile com.google.android.storagemanager.auto_generated_rro_product__
2. See no error in the logs.
Change-Id: If2c4c923f254fb4c2ae7a579543cc1c429b98ce1
Merged-In: If2c4c923f254fb4c2ae7a579543cc1c429b98ce1
(cherry picked from commit db9a49c27e9ab07b3c851a93165db2695c985841)
diff --git a/libdexfile/dex/art_dex_file_loader.cc b/libdexfile/dex/art_dex_file_loader.cc
index 654668e..8c11479 100644
--- a/libdexfile/dex/art_dex_file_loader.cc
+++ b/libdexfile/dex/art_dex_file_loader.cc
@@ -290,7 +290,8 @@
ScopedTrace trace(std::string("Open dex file ") + std::string(location));
DCHECK(dex_files != nullptr) << "DexFile::Open: out-param is nullptr";
if (IsZipMagic(magic)) {
- return OpenZip(fd, location, verify, verify_checksum, error_msg, dex_files);
+ return OpenZip(
+ fd, location, verify, verify_checksum, /*allow_no_dex_files=*/false, error_msg, dex_files);
}
if (IsMagicValid(magic)) {
std::unique_ptr<const DexFile> dex_file(OpenFile(fd,
@@ -324,6 +325,7 @@
const std::string& location,
bool verify,
bool verify_checksum,
+ bool allow_no_dex_files,
std::string* error_msg,
std::vector<std::unique_ptr<const DexFile>>* dex_files) const {
ScopedTrace trace("Dex file open Zip " + std::string(location));
@@ -331,6 +333,7 @@
location,
verify,
verify_checksum,
+ allow_no_dex_files,
error_msg,
dex_files);
}
@@ -340,6 +343,7 @@
const std::string& location,
bool verify,
bool verify_checksum,
+ bool allow_no_dex_files,
std::string* error_msg,
std::vector<std::unique_ptr<const DexFile>>* dex_files) const {
ScopedTrace trace("Dex file open Zip " + std::string(location) + " (owned fd)");
@@ -347,6 +351,7 @@
location,
verify,
verify_checksum,
+ allow_no_dex_files,
error_msg,
dex_files);
}
@@ -356,6 +361,7 @@
const std::string& location,
bool verify,
bool verify_checksum,
+ bool allow_no_dex_files,
std::string* error_msg,
std::vector<std::unique_ptr<const DexFile>>* dex_files) const {
DCHECK(dex_files != nullptr) << "DexFile::OpenZip: out-param is nullptr";
@@ -365,7 +371,7 @@
return false;
}
return OpenAllDexFilesFromZip(
- *zip_archive, location, verify, verify_checksum, error_msg, dex_files);
+ *zip_archive, location, verify, verify_checksum, allow_no_dex_files, error_msg, dex_files);
}
std::unique_ptr<const DexFile> ArtDexFileLoader::OpenFile(int fd,
@@ -537,19 +543,20 @@
const std::string& location,
bool verify,
bool verify_checksum,
+ bool allow_no_dex_files,
std::string* error_msg,
std::vector<std::unique_ptr<const DexFile>>* dex_files) const {
ScopedTrace trace("Dex file open from Zip " + std::string(location));
DCHECK(dex_files != nullptr) << "DexFile::OpenFromZip: out-param is nullptr";
DexFileLoaderErrorCode error_code;
- std::unique_ptr<const DexFile> dex_file(OpenOneDexFileFromZip(zip_archive,
- kClassesDex,
- location,
- verify,
- verify_checksum,
- error_msg,
- &error_code));
+ std::string local_error_msg;
+ std::unique_ptr<const DexFile> dex_file(OpenOneDexFileFromZip(
+ zip_archive, kClassesDex, location, verify, verify_checksum, &local_error_msg, &error_code));
if (dex_file.get() == nullptr) {
+ if (allow_no_dex_files && error_code == DexFileLoaderErrorCode::kEntryNotFound) {
+ return true;
+ }
+ *error_msg = std::move(local_error_msg);
return false;
} else {
// Had at least classes.dex.
@@ -560,7 +567,7 @@
// We could try to avoid std::string allocations by working on a char array directly. As we
// do not expect a lot of iterations, this seems too involved and brittle.
- for (size_t i = 1; ; ++i) {
+ for (size_t i = 1;; ++i) {
std::string name = GetMultiDexClassesDexName(i);
std::string fake_location = GetMultiDexLocation(i, location.c_str());
std::unique_ptr<const DexFile> next_dex_file(OpenOneDexFileFromZip(zip_archive,
diff --git a/libdexfile/dex/art_dex_file_loader.h b/libdexfile/dex/art_dex_file_loader.h
index acf7e28..6f8b3c6 100644
--- a/libdexfile/dex/art_dex_file_loader.h
+++ b/libdexfile/dex/art_dex_file_loader.h
@@ -119,19 +119,27 @@
// Opens dex files from within a .jar, .zip, or .apk file using its file descriptor. The file
// descriptor ownership is taken over, i.e. will be closed by this class.
+ // If the zip file doesn't contain any dex code and `allow_no_dex_files` is true, returns true and
+ // keeps `dex_files` to be an empty vector; if the zip file doesn't contain any dex code and
+ // `allow_no_dex_files` is false, returns false and sets the error message.
bool OpenZip(int fd,
const std::string& location,
bool verify,
bool verify_checksum,
+ bool allow_no_dex_files,
std::string* error_msg,
std::vector<std::unique_ptr<const DexFile>>* dex_files) const;
// Opens dex files from within a .jar, .zip, or .apk file using its file descriptor. The file
// descriptor is assumed owned by the caller.
+ // If the zip file doesn't contain any dex code and `allow_no_dex_files` is true, returns true and
+ // keeps `dex_files` to be an empty vector; if the zip file doesn't contain any dex code and
+ // `allow_no_dex_files` is false, returns false and sets the error message.
bool OpenZipFromOwnedFd(int fd,
const std::string& location,
bool verify,
bool verify_checksum,
+ bool allow_no_dex_files,
std::string* error_msg,
std::vector<std::unique_ptr<const DexFile>>* dex_files) const;
@@ -156,6 +164,7 @@
const std::string& location,
bool verify,
bool verify_checksum,
+ bool allow_no_dex_files,
std::string* error_msg,
std::vector<std::unique_ptr<const DexFile>>* dex_files) const;
@@ -173,6 +182,7 @@
const std::string& location,
bool verify,
bool verify_checksum,
+ bool allow_no_dex_files,
std::string* error_msg,
std::vector<std::unique_ptr<const DexFile>>* dex_files) const;
};
diff --git a/libdexfile/dex/art_dex_file_loader_test.cc b/libdexfile/dex/art_dex_file_loader_test.cc
index bf1d306..477d8c5 100644
--- a/libdexfile/dex/art_dex_file_loader_test.cc
+++ b/libdexfile/dex/art_dex_file_loader_test.cc
@@ -57,6 +57,42 @@
ASSERT_TRUE(dex.get() != nullptr);
}
+TEST_F(ArtDexFileLoaderTest, OpenZipMultiDex) {
+ std::string zip_file = GetTestDexFileName("MultiDex");
+ File file(zip_file, O_RDONLY, /*check_usage=*/false);
+ ASSERT_GE(file.Fd(), 0);
+ ArtDexFileLoader dex_file_loader;
+ std::vector<std::unique_ptr<const DexFile>> dex_files;
+ std::string error_msg;
+ ASSERT_TRUE(dex_file_loader.OpenZip(file.Release(),
+ zip_file,
+ /*verify=*/false,
+ /*verify_checksum=*/true,
+ /*allow_no_dex_files=*/true,
+ &error_msg,
+ &dex_files))
+ << error_msg;
+ EXPECT_GT(dex_files.size(), 1);
+}
+
+TEST_F(ArtDexFileLoaderTest, OpenZipEmpty) {
+ std::string zip_file = GetTestDexFileName("MainEmptyUncompressed");
+ File file(zip_file, O_RDONLY, /*check_usage=*/false);
+ ASSERT_GE(file.Fd(), 0);
+ ArtDexFileLoader dex_file_loader;
+ std::vector<std::unique_ptr<const DexFile>> dex_files;
+ std::string error_msg;
+ ASSERT_TRUE(dex_file_loader.OpenZip(file.Release(),
+ zip_file,
+ /*verify=*/false,
+ /*verify_checksum=*/true,
+ /*allow_no_dex_files=*/true,
+ &error_msg,
+ &dex_files))
+ << error_msg;
+ EXPECT_EQ(dex_files.size(), 0);
+}
+
TEST_F(ArtDexFileLoaderTest, GetLocationChecksum) {
std::unique_ptr<const DexFile> raw(OpenTestDexFile("Main"));
EXPECT_NE(raw->GetHeader().checksum_, raw->GetLocationChecksum());
diff --git a/profman/profman.cc b/profman/profman.cc
index c2b98a2..55c8555 100644
--- a/profman/profman.cc
+++ b/profman/profman.cc
@@ -613,8 +613,9 @@
if (use_apk_fd_list) {
if (dex_file_loader.OpenZip(apks_fd_[i],
dex_locations_[i],
- /* verify= */ false,
+ /*verify=*/false,
kVerifyChecksum,
+ /*allow_no_dex_files=*/true,
&error_msg,
&dex_files_for_location)) {
} else {
@@ -622,12 +623,18 @@
return false;
}
} else {
- if (dex_file_loader.Open(apk_files_[i].c_str(),
- dex_locations_[i],
- /* verify= */ false,
- kVerifyChecksum,
- &error_msg,
- &dex_files_for_location)) {
+ File file(apk_files_[i], O_RDONLY, /*check_usage=*/false);
+ if (file.Fd() < 0) {
+ PLOG(ERROR) << "Unable to open '" << apk_files_[i] << "'";
+ return false;
+ }
+ if (dex_file_loader.OpenZip(file.Release(),
+ dex_locations_[i],
+ /*verify=*/false,
+ kVerifyChecksum,
+ /*allow_no_dex_files=*/true,
+ &error_msg,
+ &dex_files_for_location)) {
} else {
LOG(ERROR) << "Open failed for '" << dex_locations_[i] << "' " << error_msg;
return false;
diff --git a/runtime/oat_file.cc b/runtime/oat_file.cc
index ec8f067..de14835 100644
--- a/runtime/oat_file.cc
+++ b/runtime/oat_file.cc
@@ -283,7 +283,7 @@
std::string* error_msg) {
vdex_ = VdexFile::OpenAtAddress(vdex_begin_,
vdex_end_ - vdex_begin_,
- /*mmap_reuse=*/ vdex_begin_ != nullptr,
+ /*mmap_reuse=*/vdex_begin_ != nullptr,
vdex_filename,
writable,
low_4gb,
@@ -308,16 +308,15 @@
if (rc == -1) {
PLOG(WARNING) << "Failed getting length of vdex file";
} else {
- vdex_ = VdexFile::OpenAtAddress(
- vdex_begin_,
- vdex_end_ - vdex_begin_,
- /*mmap_reuse=*/ vdex_begin_ != nullptr,
- vdex_fd,
- s.st_size,
- vdex_filename,
- writable,
- low_4gb,
- error_msg);
+ vdex_ = VdexFile::OpenAtAddress(vdex_begin_,
+ vdex_end_ - vdex_begin_,
+ /*mmap_reuse=*/vdex_begin_ != nullptr,
+ vdex_fd,
+ s.st_size,
+ vdex_filename,
+ writable,
+ low_4gb,
+ error_msg);
if (vdex_.get() == nullptr) {
*error_msg = "Failed opening vdex file.";
return false;
@@ -794,23 +793,25 @@
if (zip_fd != -1) {
loaded = dex_file_loader.OpenZip(zip_fd,
dex_file_location,
- /*verify=*/ false,
- /*verify_checksum=*/ false,
+ /*verify=*/false,
+ /*verify_checksum=*/false,
+ /*allow_no_dex_files=*/false,
error_msg,
&new_dex_files);
} else if (dex_fd != -1) {
// Note that we assume dex_fds are backing by jars.
loaded = dex_file_loader.OpenZipFromOwnedFd(dex_fd,
dex_file_location,
- /*verify=*/ false,
- /*verify_checksum=*/ false,
+ /*verify=*/false,
+ /*verify_checksum=*/false,
+ /*allow_no_dex_files=*/false,
error_msg,
&new_dex_files);
} else {
loaded = dex_file_loader.Open(dex_file_name.c_str(),
dex_file_location,
- /*verify=*/ false,
- /*verify_checksum=*/ false,
+ /*verify=*/false,
+ /*verify_checksum=*/false,
error_msg,
&new_dex_files);
}
@@ -1695,7 +1696,7 @@
ScopedTrace trace(__PRETTY_FUNCTION__);
elf_file_.reset(ElfFile::Open(file,
writable,
- /*program_header_only=*/ true,
+ /*program_header_only=*/true,
low_4gb,
error_msg));
if (elf_file_ == nullptr) {
@@ -1710,7 +1711,7 @@
class OatFileBackedByVdex final : public OatFileBase {
public:
explicit OatFileBackedByVdex(const std::string& filename)
- : OatFileBase(filename, /*executable=*/ false) {}
+ : OatFileBase(filename, /*executable=*/false) {}
static OatFileBackedByVdex* Open(const std::vector<const DexFile*>& dex_files,
std::unique_ptr<VdexFile>&& vdex_file,
@@ -1810,15 +1811,16 @@
if (zip_fd != -1) {
loaded = dex_file_loader.OpenZip(zip_fd,
dex_location,
- /*verify=*/ false,
- /*verify_checksum=*/ false,
+ /*verify=*/false,
+ /*verify_checksum=*/false,
+ /*allow_no_dex_files=*/false,
error_msg,
&oat_file->external_dex_files_);
} else {
loaded = dex_file_loader.Open(dex_location.c_str(),
dex_location,
- /*verify=*/ false,
- /*verify_checksum=*/ false,
+ /*verify=*/false,
+ /*verify_checksum=*/false,
error_msg,
&oat_file->external_dex_files_);
}
@@ -1932,7 +1934,7 @@
vdex_filename,
oat_filename,
oat_location,
- /*writable=*/ false,
+ /*writable=*/false,
executable,
low_4gb,
dex_filenames,
@@ -1962,7 +1964,7 @@
vdex_filename,
oat_filename,
oat_location,
- /*writable=*/ false,
+ /*writable=*/false,
executable,
low_4gb,
dex_filenames,
@@ -1991,7 +1993,7 @@
oat_fd,
vdex_location,
oat_location,
- /*writable=*/ false,
+ /*writable=*/false,
executable,
low_4gb,
dex_filenames,