Fix dex2oat failure when --zip-fd actually points to raw dexfile.
Installd always pass --zip-fd to dex2oat when perform dexopt,
however some apps use RawDexFile rather than ZipArchive as
secondary dex, so check whether it's a RawDexfile before try
it as ZipArchive. This make sure dex2oat success when secondary
dex is a raw dexfile.
Test: m test-art-host-gtest-oat_writer_test
Test: m test-art-host-gtest-dex2oat_test
Change-Id: I6da98845cdf64acee1865dd997616e8214417315
Signed-off-by: wangshumin <wangshumin@xiaomi.com>
diff --git a/dex2oat/dex2oat.cc b/dex2oat/dex2oat.cc
index 8713886..ccb89a6 100644
--- a/dex2oat/dex2oat.cc
+++ b/dex2oat/dex2oat.cc
@@ -2701,8 +2701,8 @@
}
} else if (zip_fd_ != -1) {
DCHECK_EQ(oat_writers_.size(), 1u);
- if (!oat_writers_[0]->AddZippedDexFilesSource(File(zip_fd_, /* check_usage */ false),
- zip_location_.c_str())) {
+ if (!oat_writers_[0]->AddDexFileSource(File(zip_fd_, /* check_usage */ false),
+ zip_location_.c_str())) {
return false;
}
} else if (oat_writers_.size() > 1u) {
diff --git a/dex2oat/dex2oat_test.cc b/dex2oat/dex2oat_test.cc
index 372ee35..45b7e4a 100644
--- a/dex2oat/dex2oat_test.cc
+++ b/dex2oat/dex2oat_test.cc
@@ -34,6 +34,7 @@
#include "base/mutex-inl.h"
#include "base/string_view_cpp20.h"
#include "base/utils.h"
+#include "base/zip_archive.h"
#include "dex/art_dex_file_loader.h"
#include "dex/base64_test_util.h"
#include "dex/bytecode_utils.h"
@@ -2249,6 +2250,40 @@
ASSERT_TRUE(odex_file != nullptr);
}
+TEST_F(Dex2oatTest, DexFileFd) {
+ std::string error_msg;
+ std::string zip_location = GetTestDexFileName("Main");
+ std::unique_ptr<File> zip_file(OS::OpenFileForReading(zip_location.c_str()));
+ ASSERT_NE(-1, zip_file->Fd());
+
+ std::unique_ptr<ZipArchive> zip_archive(
+ ZipArchive::OpenFromFd(zip_file->Release(), zip_location.c_str(), &error_msg));
+ ASSERT_TRUE(zip_archive != nullptr);
+
+ std::string entry_name = DexFileLoader::GetMultiDexClassesDexName(0);
+ std::unique_ptr<ZipEntry> entry(zip_archive->Find(entry_name.c_str(), &error_msg));
+ ASSERT_TRUE(entry != nullptr);
+
+ ScratchFile dex_file;
+ const std::string& dex_location = dex_file.GetFilename();
+ const std::string base_oat_name = GetScratchDir() + "/base.oat";
+
+ bool success = entry->ExtractToFile(*(dex_file.GetFile()), &error_msg);
+ ASSERT_TRUE(success);
+ ASSERT_EQ(0, lseek(dex_file.GetFd(), 0, SEEK_SET));
+
+ std::vector<std::string> extra_args{
+ StringPrintf("--zip-fd=%d", dex_file.GetFd()),
+ "--zip-location=" + dex_location,
+ };
+ ASSERT_TRUE(GenerateOdexForTest(dex_location,
+ base_oat_name,
+ CompilerFilter::Filter::kQuicken,
+ extra_args,
+ /*expect_success=*/ true,
+ /*use_fd=*/ false,
+ /*use_zip_fd=*/ true));
+}
TEST_F(Dex2oatTest, AppImageResolveStrings) {
using Hotness = ProfileCompilationInfo::MethodHotness;
diff --git a/dex2oat/linker/oat_writer.cc b/dex2oat/linker/oat_writer.cc
index 16e75d0..055f246 100644
--- a/dex2oat/linker/oat_writer.cc
+++ b/dex2oat/linker/oat_writer.cc
@@ -504,21 +504,38 @@
const char* location,
CreateTypeLookupTable create_type_lookup_table) {
DCHECK(write_state_ == WriteState::kAddingDexFileSources);
- uint32_t magic;
- std::string error_msg;
- File fd = OpenAndReadMagic(filename, &magic, &error_msg);
+ File fd(filename, O_RDONLY, /* check_usage= */ false);
if (fd.Fd() == -1) {
- PLOG(ERROR) << "Failed to read magic number from dex file: '" << filename << "'";
+ PLOG(ERROR) << "Failed to open dex file: '" << filename << "'";
return false;
- } else if (DexFileLoader::IsMagicValid(magic)) {
+ }
+
+ return AddDexFileSource(std::move(fd), location, create_type_lookup_table);
+}
+
+// Add dex file source(s) from a file specified by a file handle.
+// Note: The `dex_file_fd` specifies a plain dex file or a zip file.
+bool OatWriter::AddDexFileSource(File&& dex_file_fd,
+ const char* location,
+ CreateTypeLookupTable create_type_lookup_table) {
+ DCHECK(write_state_ == WriteState::kAddingDexFileSources);
+ std::string error_msg;
+ uint32_t magic;
+ if (!ReadMagicAndReset(dex_file_fd.Fd(), &magic, &error_msg)) {
+ LOG(ERROR) << "Failed to read magic number from dex file '" << location << "': " << error_msg;
+ return false;
+ }
+ if (DexFileLoader::IsMagicValid(magic)) {
uint8_t raw_header[sizeof(DexFile::Header)];
- const UnalignedDexFileHeader* header = GetDexFileHeader(&fd, raw_header, location);
+ const UnalignedDexFileHeader* header = GetDexFileHeader(&dex_file_fd, raw_header, location);
if (header == nullptr) {
+ LOG(ERROR) << "Failed to get DexFileHeader from file descriptor for '"
+ << location << "': " << error_msg;
return false;
}
// The file is open for reading, not writing, so it's OK to let the File destructor
// close it without checking for explicit Close(), so pass checkUsage = false.
- raw_dex_files_.emplace_back(new File(fd.Release(), location, /* checkUsage */ false));
+ raw_dex_files_.emplace_back(new File(dex_file_fd.Release(), location, /* checkUsage */ false));
oat_dex_files_.emplace_back(/* OatDexFile */
location,
DexFileSource(raw_dex_files_.back().get()),
@@ -526,48 +543,36 @@
header->checksum_,
header->file_size_);
} else if (IsZipMagic(magic)) {
- if (!AddZippedDexFilesSource(std::move(fd), location, create_type_lookup_table)) {
+ zip_archives_.emplace_back(ZipArchive::OpenFromFd(dex_file_fd.Release(), location, &error_msg));
+ ZipArchive* zip_archive = zip_archives_.back().get();
+ if (zip_archive == nullptr) {
+ LOG(ERROR) << "Failed to open zip from file descriptor for '" << location << "': "
+ << error_msg;
+ return false;
+ }
+ for (size_t i = 0; ; ++i) {
+ std::string entry_name = DexFileLoader::GetMultiDexClassesDexName(i);
+ std::unique_ptr<ZipEntry> entry(zip_archive->Find(entry_name.c_str(), &error_msg));
+ if (entry == nullptr) {
+ break;
+ }
+ zipped_dex_files_.push_back(std::move(entry));
+ zipped_dex_file_locations_.push_back(DexFileLoader::GetMultiDexLocation(i, location));
+ const char* full_location = zipped_dex_file_locations_.back().c_str();
+ // We override the checksum from header with the CRC from ZIP entry.
+ oat_dex_files_.emplace_back(/* OatDexFile */
+ full_location,
+ DexFileSource(zipped_dex_files_.back().get()),
+ create_type_lookup_table,
+ zipped_dex_files_.back()->GetCrc32(),
+ zipped_dex_files_.back()->GetUncompressedLength());
+ }
+ if (zipped_dex_file_locations_.empty()) {
+ LOG(ERROR) << "No dex files in zip file '" << location << "': " << error_msg;
return false;
}
} else {
- LOG(ERROR) << "Expected valid zip or dex file: '" << filename << "'";
- return false;
- }
- return true;
-}
-
-// Add dex file source(s) from a zip file specified by a file handle.
-bool OatWriter::AddZippedDexFilesSource(File&& zip_fd,
- const char* location,
- CreateTypeLookupTable create_type_lookup_table) {
- DCHECK(write_state_ == WriteState::kAddingDexFileSources);
- std::string error_msg;
- zip_archives_.emplace_back(ZipArchive::OpenFromFd(zip_fd.Release(), location, &error_msg));
- ZipArchive* zip_archive = zip_archives_.back().get();
- if (zip_archive == nullptr) {
- LOG(ERROR) << "Failed to open zip from file descriptor for '" << location << "': "
- << error_msg;
- return false;
- }
- for (size_t i = 0; ; ++i) {
- std::string entry_name = DexFileLoader::GetMultiDexClassesDexName(i);
- std::unique_ptr<ZipEntry> entry(zip_archive->Find(entry_name.c_str(), &error_msg));
- if (entry == nullptr) {
- break;
- }
- zipped_dex_files_.push_back(std::move(entry));
- zipped_dex_file_locations_.push_back(DexFileLoader::GetMultiDexLocation(i, location));
- const char* full_location = zipped_dex_file_locations_.back().c_str();
- // We override the checksum from header with the CRC from ZIP entry.
- oat_dex_files_.emplace_back(/* OatDexFile */
- full_location,
- DexFileSource(zipped_dex_files_.back().get()),
- create_type_lookup_table,
- zipped_dex_files_.back()->GetCrc32(),
- zipped_dex_files_.back()->GetUncompressedLength());
- }
- if (zipped_dex_file_locations_.empty()) {
- LOG(ERROR) << "No dex files in zip file '" << location << "': " << error_msg;
+ LOG(ERROR) << "Expected valid zip or dex file: '" << location << "'";
return false;
}
return true;
diff --git a/dex2oat/linker/oat_writer.h b/dex2oat/linker/oat_writer.h
index dd11b87..f83f077 100644
--- a/dex2oat/linker/oat_writer.h
+++ b/dex2oat/linker/oat_writer.h
@@ -127,7 +127,6 @@
// To produce a valid oat file, the user must first add sources with any combination of
// - AddDexFileSource(),
- // - AddZippedDexFilesSource(),
// - AddRawDexFileSource(),
// - AddVdexDexFilesSource().
// Then the user must call in order
@@ -148,9 +147,10 @@
const char* filename,
const char* location,
CreateTypeLookupTable create_type_lookup_table = CreateTypeLookupTable::kDefault);
- // Add dex file source(s) from a zip file specified by a file handle.
- bool AddZippedDexFilesSource(
- File&& zip_fd,
+ // Add dex file source(s) from a file specified by a file handle.
+ // Note: The `dex_file_fd` specifies a plain dex file or a zip file.
+ bool AddDexFileSource(
+ File&& dex_file_fd,
const char* location,
CreateTypeLookupTable create_type_lookup_table = CreateTypeLookupTable::kDefault);
// Add dex file source from raw memory.
diff --git a/dex2oat/linker/oat_writer_test.cc b/dex2oat/linker/oat_writer_test.cc
index 6d6d114..1b27bce 100644
--- a/dex2oat/linker/oat_writer_test.cc
+++ b/dex2oat/linker/oat_writer_test.cc
@@ -147,18 +147,19 @@
bool WriteElf(File* vdex_file,
File* oat_file,
- File&& zip_fd,
+ File&& dex_file_fd,
const char* location,
SafeMap<std::string, std::string>& key_value_store,
bool verify,
- CopyOption copy) {
+ CopyOption copy,
+ ProfileCompilationInfo* profile_compilation_info = nullptr) {
TimingLogger timings("WriteElf", false, false);
ClearBootImageOption();
OatWriter oat_writer(*compiler_options_,
&timings,
- /*profile_compilation_info*/nullptr,
+ profile_compilation_info,
CompactDexLevel::kCompactDexLevelNone);
- if (!oat_writer.AddZippedDexFilesSource(std::move(zip_fd), location)) {
+ if (!oat_writer.AddDexFileSource(std::move(dex_file_fd), location)) {
return false;
}
return DoWriteElf(vdex_file, oat_file, oat_writer, key_value_store, verify, copy);
@@ -262,6 +263,56 @@
return true;
}
+ void CheckOatWriteResult(ScratchFile& oat_file,
+ ScratchFile& vdex_file,
+ std::vector<std::unique_ptr<const DexFile>>& input_dexfiles,
+ const unsigned int expected_oat_dexfile_count,
+ bool low_4gb) {
+ ASSERT_EQ(expected_oat_dexfile_count, input_dexfiles.size());
+
+ std::string error_msg;
+ std::unique_ptr<OatFile> opened_oat_file(OatFile::Open(/*zip_fd=*/ -1,
+ oat_file.GetFilename(),
+ oat_file.GetFilename(),
+ /*executable=*/ false,
+ low_4gb,
+ &error_msg));
+ ASSERT_TRUE(opened_oat_file != nullptr) << error_msg;
+ ASSERT_EQ(expected_oat_dexfile_count, opened_oat_file->GetOatDexFiles().size());
+
+ if (low_4gb) {
+ uintptr_t begin = reinterpret_cast<uintptr_t>(opened_oat_file->Begin());
+ EXPECT_EQ(begin, static_cast<uint32_t>(begin));
+ }
+
+ for (uint32_t i = 0; i < input_dexfiles.size(); i++) {
+ const std::unique_ptr<const DexFile>& dex_file_data = input_dexfiles[i];
+ std::unique_ptr<const DexFile> opened_dex_file =
+ opened_oat_file->GetOatDexFiles()[i]->OpenDexFile(&error_msg);
+
+ ASSERT_EQ(opened_oat_file->GetOatDexFiles()[i]->GetDexFileLocationChecksum(),
+ dex_file_data->GetHeader().checksum_);
+
+ ASSERT_EQ(dex_file_data->GetHeader().file_size_, opened_dex_file->GetHeader().file_size_);
+ ASSERT_EQ(0, memcmp(&dex_file_data->GetHeader(),
+ &opened_dex_file->GetHeader(),
+ dex_file_data->GetHeader().file_size_));
+ ASSERT_EQ(dex_file_data->GetLocation(), opened_dex_file->GetLocation());
+ }
+ const VdexFile::DexSectionHeader &vdex_header =
+ opened_oat_file->GetVdexFile()->GetDexSectionHeader();
+ if (!compiler_driver_->GetCompilerOptions().IsQuickeningCompilationEnabled()) {
+ // If quickening is enabled we will always write the table since there is no special logic
+ // that checks for all methods not being quickened (not worth the complexity).
+ ASSERT_EQ(vdex_header.GetQuickeningInfoSize(), 0u);
+ }
+
+ int64_t actual_vdex_size = vdex_file.GetFile()->GetLength();
+ ASSERT_GE(actual_vdex_size, 0);
+ ASSERT_EQ(dchecked_integral_cast<uint64_t>(actual_vdex_size),
+ opened_oat_file->GetVdexFile()->GetComputedFileSize());
+ }
+
void TestDexFileInput(bool verify, bool low_4gb, bool use_profile);
void TestZipFileInput(bool verify, CopyOption copy);
void TestZipFileInputWithEmptyDex();
@@ -549,6 +600,8 @@
TimingLogger timings("OatTest::DexFileInput", false, false);
std::vector<const char*> input_filenames;
+ std::vector<std::unique_ptr<const DexFile>> input_dexfiles;
+ std::vector<const ScratchFile*> scratch_files;
ScratchFile dex_file1;
TestDexFileBuilder builder1;
@@ -564,6 +617,8 @@
success = dex_file1.GetFile()->Flush() == 0;
ASSERT_TRUE(success);
input_filenames.push_back(dex_file1.GetFilename().c_str());
+ input_dexfiles.push_back(std::move(dex_file1_data));
+ scratch_files.push_back(&dex_file1);
ScratchFile dex_file2;
TestDexFileBuilder builder2;
@@ -579,73 +634,75 @@
success = dex_file2.GetFile()->Flush() == 0;
ASSERT_TRUE(success);
input_filenames.push_back(dex_file2.GetFilename().c_str());
+ input_dexfiles.push_back(std::move(dex_file2_data));
+ scratch_files.push_back(&dex_file2);
- ScratchFile tmp_base, tmp_oat(tmp_base, ".oat"), tmp_vdex(tmp_base, ".vdex");
SafeMap<std::string, std::string> key_value_store;
- std::unique_ptr<ProfileCompilationInfo>
- profile_compilation_info(use_profile ? new ProfileCompilationInfo() : nullptr);
- success = WriteElf(tmp_vdex.GetFile(),
- tmp_oat.GetFile(),
- input_filenames,
- key_value_store,
- verify,
- CopyOption::kOnlyIfCompressed,
- profile_compilation_info.get());
+ {
+ // Test using the AddDexFileSource() interface with the dex files.
+ ScratchFile tmp_base, tmp_oat(tmp_base, ".oat"), tmp_vdex(tmp_base, ".vdex");
+ std::unique_ptr<ProfileCompilationInfo>
+ profile_compilation_info(use_profile ? new ProfileCompilationInfo() : nullptr);
+ success = WriteElf(tmp_vdex.GetFile(),
+ tmp_oat.GetFile(),
+ input_filenames,
+ key_value_store,
+ verify,
+ CopyOption::kOnlyIfCompressed,
+ profile_compilation_info.get());
- // In verify mode, we expect failure.
- if (verify) {
- ASSERT_FALSE(success);
- return;
+ // In verify mode, we expect failure.
+ if (verify) {
+ ASSERT_FALSE(success);
+ return;
+ }
+
+ ASSERT_TRUE(success);
+
+ CheckOatWriteResult(tmp_oat,
+ tmp_vdex,
+ input_dexfiles,
+ /* oat_dexfile_count */ 2,
+ low_4gb);
}
- ASSERT_TRUE(success);
+ {
+ // Test using the AddDexFileSource() interface with the dexfile1's fd.
+ // Only need one input dexfile.
+ std::vector<std::unique_ptr<const DexFile>> input_dexfiles2;
+ input_dexfiles2.push_back(std::move(input_dexfiles[0]));
+ const ScratchFile* dex_file = scratch_files[0];
+ File dex_file_fd(DupCloexec(dex_file->GetFd()), /*check_usage=*/ false);
- std::string error_msg;
- std::unique_ptr<OatFile> opened_oat_file(OatFile::Open(/*zip_fd=*/ -1,
- tmp_oat.GetFilename(),
- tmp_oat.GetFilename(),
- /*executable=*/ false,
- low_4gb,
- &error_msg));
- ASSERT_TRUE(opened_oat_file != nullptr) << error_msg;
- if (low_4gb) {
- uintptr_t begin = reinterpret_cast<uintptr_t>(opened_oat_file->Begin());
- EXPECT_EQ(begin, static_cast<uint32_t>(begin));
+ ASSERT_NE(-1, dex_file_fd.Fd());
+ ASSERT_EQ(0, lseek(dex_file_fd.Fd(), 0, SEEK_SET));
+
+ ScratchFile tmp_base, tmp_oat(tmp_base, ".oat"), tmp_vdex(tmp_base, ".vdex");
+ std::unique_ptr<ProfileCompilationInfo>
+ profile_compilation_info(use_profile ? new ProfileCompilationInfo() : nullptr);
+ success = WriteElf(tmp_vdex.GetFile(),
+ tmp_oat.GetFile(),
+ std::move(dex_file_fd),
+ dex_file->GetFilename().c_str(),
+ key_value_store,
+ verify,
+ CopyOption::kOnlyIfCompressed,
+ profile_compilation_info.get());
+
+ // In verify mode, we expect failure.
+ if (verify) {
+ ASSERT_FALSE(success);
+ return;
+ }
+
+ ASSERT_TRUE(success);
+
+ CheckOatWriteResult(tmp_oat,
+ tmp_vdex,
+ input_dexfiles2,
+ /* oat_dexfile_count */ 1,
+ low_4gb);
}
- ASSERT_EQ(2u, opened_oat_file->GetOatDexFiles().size());
- std::unique_ptr<const DexFile> opened_dex_file1 =
- opened_oat_file->GetOatDexFiles()[0]->OpenDexFile(&error_msg);
- std::unique_ptr<const DexFile> opened_dex_file2 =
- opened_oat_file->GetOatDexFiles()[1]->OpenDexFile(&error_msg);
-
- ASSERT_EQ(opened_oat_file->GetOatDexFiles()[0]->GetDexFileLocationChecksum(),
- dex_file1_data->GetHeader().checksum_);
- ASSERT_EQ(opened_oat_file->GetOatDexFiles()[1]->GetDexFileLocationChecksum(),
- dex_file2_data->GetHeader().checksum_);
-
- ASSERT_EQ(dex_file1_data->GetHeader().file_size_, opened_dex_file1->GetHeader().file_size_);
- ASSERT_EQ(0, memcmp(&dex_file1_data->GetHeader(),
- &opened_dex_file1->GetHeader(),
- dex_file1_data->GetHeader().file_size_));
- ASSERT_EQ(dex_file1_data->GetLocation(), opened_dex_file1->GetLocation());
-
- ASSERT_EQ(dex_file2_data->GetHeader().file_size_, opened_dex_file2->GetHeader().file_size_);
- ASSERT_EQ(0, memcmp(&dex_file2_data->GetHeader(),
- &opened_dex_file2->GetHeader(),
- dex_file2_data->GetHeader().file_size_));
- ASSERT_EQ(dex_file2_data->GetLocation(), opened_dex_file2->GetLocation());
-
- const VdexFile::DexSectionHeader &vdex_header =
- opened_oat_file->GetVdexFile()->GetDexSectionHeader();
- if (!compiler_driver_->GetCompilerOptions().IsQuickeningCompilationEnabled()) {
- // If quickening is enabled we will always write the table since there is no special logic that
- // checks for all methods not being quickened (not worth the complexity).
- ASSERT_EQ(vdex_header.GetQuickeningInfoSize(), 0u);
- }
-
- int64_t actual_vdex_size = tmp_vdex.GetFile()->GetLength();
- ASSERT_GE(actual_vdex_size, 0);
- ASSERT_EQ((uint64_t) actual_vdex_size, opened_oat_file->GetVdexFile()->GetComputedFileSize());
}
TEST_F(OatTest, DexFileInputCheckOutput) {
@@ -759,9 +816,10 @@
}
{
- // Test using the AddZipDexFileSource() interface with the zip file handle.
+ // Test using the AddDexFileSource() interface with the zip file handle.
File zip_fd(DupCloexec(zip_file.GetFd()), /*check_usage=*/ false);
ASSERT_NE(-1, zip_fd.Fd());
+ ASSERT_EQ(0, lseek(zip_fd.Fd(), 0, SEEK_SET));
ScratchFile tmp_base, tmp_oat(tmp_base, ".oat"), tmp_vdex(tmp_base, ".vdex");
success = WriteElf(tmp_vdex.GetFile(),
