diff options
author | 2025-02-03 18:29:23 +0000 | |
---|---|---|
committer | 2025-02-14 05:55:36 -0800 | |
commit | bde490d3c291724ff0512d4b6c1904f9d2bb5b5d (patch) | |
tree | 32abd9210ef4611c5d56c575fab7f9819338022e | |
parent | 153ccc814f6129c51304f874fcd8ae0512e24e71 (diff) |
Refactor OatFileAssistant - Step 2.
This change contains some behavioral changes. Particularly,
- Consolidate dm_for_oat_ and dm_for_odex_ into one: They are
duplicates.
- Only check IsUseable if the file exists: This reduces some logging.
- Propagate the error message of VdexFile::OpenFromDm to the parent.
Bug: 377474232
Bug: 345762752
Test: atest art_standalone_runtime_tests
Change-Id: Ia75b7f5b00ca06dda77204a562c6761ef2d5eecf
-rw-r--r-- | dex2oat/dex2oat.cc | 5 | ||||
-rw-r--r-- | runtime/oat/oat_file_assistant.cc | 74 | ||||
-rw-r--r-- | runtime/oat/oat_file_assistant.h | 6 | ||||
-rw-r--r-- | runtime/vdex_file.cc | 32 | ||||
-rw-r--r-- | runtime/vdex_file.h | 3 |
5 files changed, 63 insertions, 57 deletions
diff --git a/dex2oat/dex2oat.cc b/dex2oat/dex2oat.cc index 24bd112aef..0f377719a7 100644 --- a/dex2oat/dex2oat.cc +++ b/dex2oat/dex2oat.cc @@ -1373,9 +1373,12 @@ class Dex2Oat final { // In theory the files should be the same. if (dm_file_ != nullptr) { if (input_vdex_file_ == nullptr) { - input_vdex_file_ = VdexFile::OpenFromDm(dm_file_location_, *dm_file_); + std::string error_msg; + input_vdex_file_ = VdexFile::OpenFromDm(dm_file_location_, *dm_file_, &error_msg); if (input_vdex_file_ != nullptr) { VLOG(verifier) << "Doing fast verification with vdex from DexMetadata archive"; + } else { + LOG(WARNING) << error_msg; } } else { LOG(INFO) << "Ignoring vdex file in dex metadata due to vdex file already being passed"; diff --git a/runtime/oat/oat_file_assistant.cc b/runtime/oat/oat_file_assistant.cc index 8db02e5679..98e46644c8 100644 --- a/runtime/oat/oat_file_assistant.cc +++ b/runtime/oat/oat_file_assistant.cc @@ -117,8 +117,7 @@ OatFileAssistant::OatFileAssistant(const char* dex_location, oat_(this, /*is_oat_location=*/true), vdex_for_odex_(this, /*is_oat_location=*/false), vdex_for_oat_(this, /*is_oat_location=*/true), - dm_for_odex_(this, /*is_oat_location=*/false), - dm_for_oat_(this, /*is_oat_location=*/true), + dm_(this, /*is_oat_location=*/false), zip_fd_(zip_fd) { CHECK(dex_location != nullptr) << "OatFileAssistant: null dex location"; CHECK_IMPLIES(load_executable, context != nullptr) << "Loading executable without a context"; @@ -174,13 +173,6 @@ OatFileAssistant::OatFileAssistant(const char* dex_location, DupCloexec(zip_fd), DupCloexec(vdex_fd), DupCloexec(oat_fd)); - - std::string dm_file_name = GetDmFilename(dex_location_); - dm_for_odex_.Reset(dm_file_name, - UseFdToReadFiles(), - DupCloexec(zip_fd), - DupCloexec(vdex_fd), - DupCloexec(oat_fd)); } else { LOG(WARNING) << "Failed to determine odex file name: " << error_msg; } @@ -197,7 +189,7 @@ OatFileAssistant::OatFileAssistant(const char* dex_location, std::string vdex_file_name = GetVdexFilename(oat_file_name); vdex_for_oat_.Reset(vdex_file_name, UseFdToReadFiles(), zip_fd, vdex_fd, oat_fd); std::string dm_file_name = GetDmFilename(dex_location); - dm_for_oat_.Reset(dm_file_name, UseFdToReadFiles(), zip_fd, vdex_fd, oat_fd); + dm_.Reset(dm_file_name, UseFdToReadFiles(), zip_fd, vdex_fd, oat_fd); } else if (kIsTargetAndroid) { // No need to warn on host. We are probably in oatdump, where we only need OatFileAssistant to // validate BCP checksums. @@ -298,7 +290,7 @@ int OatFileAssistant::GetDexOptNeeded(CompilerFilter::Filter target_compiler_fil } DexOptNeeded dexopt_needed = info.GetDexOptNeeded( target_compiler_filter, GetDexOptTrigger(target_compiler_filter, profile_changed, downgrade)); - if (dexopt_needed != kNoDexOptNeeded && (&info == &dm_for_oat_ || &info == &dm_for_odex_)) { + if (dexopt_needed != kNoDexOptNeeded && (&info == &dm_)) { // The usable vdex file is in the DM file. This information cannot be encoded in the integer. // Return kDex2OatFromScratch so that neither the vdex in the "oat" location nor the vdex in the // "odex" location will be picked by installd. @@ -805,39 +797,45 @@ OatFileAssistant::OatFileInfo& OatFileAssistant::GetBestInfo() { // If the oat location is useable, take it. This must be an app on a readonly filesystem // (typically, a system app or an incremental app). This must be prioritized over the odex // location, because the odex location probably has the dexpreopt artifacts. - VLOG(oat) << ART_FORMAT("GetBestInfo checking odex in dalvik-cache ({})", oat_.DisplayFilename()); - if (oat_.IsUseable()) { - return oat_; + if (oat_.FileExists()) { + VLOG(oat) << ART_FORMAT("GetBestInfo checking odex in dalvik-cache ({})", + oat_.DisplayFilename()); + if (oat_.IsUseable()) { + return oat_; + } } // The odex location, which is the most common. - VLOG(oat) << ART_FORMAT("GetBestInfo checking odex next to the dex file ({})", - odex_.DisplayFilename()); - if (odex_.IsUseable()) { - return odex_; + if (odex_.FileExists()) { + VLOG(oat) << ART_FORMAT("GetBestInfo checking odex next to the dex file ({})", + odex_.DisplayFilename()); + if (odex_.IsUseable()) { + return odex_; + } } // No odex/oat available, look for a useable vdex file. - VLOG(oat) << ART_FORMAT("GetBestInfo checking vdex in dalvik-cache ({})", - vdex_for_oat_.DisplayFilename()); - if (vdex_for_oat_.IsUseable()) { - return vdex_for_oat_; + if (vdex_for_oat_.FileExists()) { + VLOG(oat) << ART_FORMAT("GetBestInfo checking vdex in dalvik-cache ({})", + vdex_for_oat_.DisplayFilename()); + if (vdex_for_oat_.IsUseable()) { + return vdex_for_oat_; + } } - VLOG(oat) << ART_FORMAT("GetBestInfo checking vdex next to the dex file ({})", - vdex_for_odex_.DisplayFilename()); - if (vdex_for_odex_.IsUseable()) { - return vdex_for_odex_; + if (vdex_for_odex_.FileExists()) { + VLOG(oat) << ART_FORMAT("GetBestInfo checking vdex next to the dex file ({})", + vdex_for_odex_.DisplayFilename()); + if (vdex_for_odex_.IsUseable()) { + return vdex_for_odex_; + } } // A .dm file may be available, look for it. - VLOG(oat) << ART_FORMAT("GetBestInfo checking dm ({})", dm_for_oat_.DisplayFilename()); - if (dm_for_oat_.IsUseable()) { - return dm_for_oat_; - } - // TODO(jiakaiz): Is this the same as above? - VLOG(oat) << ART_FORMAT("GetBestInfo checking dm ({})", dm_for_odex_.DisplayFilename()); - if (dm_for_odex_.IsUseable()) { - return dm_for_odex_; + if (dm_.FileExists()) { + VLOG(oat) << ART_FORMAT("GetBestInfo checking dm ({})", dm_.DisplayFilename()); + if (dm_.IsUseable()) { + return dm_; + } } // No usable artifact. Pick the odex if it exists, or the oat if not. @@ -937,6 +935,10 @@ OatFileAssistant::DexOptNeeded OatFileAssistant::OatFileInfo::GetDexOptNeeded( } } +bool OatFileAssistant::OatFileInfo::FileExists() const { + return use_fd_ || (!filename_.empty() && OS::FileExists(filename_.c_str())); +} + const OatFile* OatFileAssistant::OatFileInfo::GetFile() { CHECK(!file_released_) << "GetFile called after oat file released."; if (load_attempted_) { @@ -993,7 +995,7 @@ const OatFile* OatFileAssistant::OatFileInfo::GetFile() { // Check to see if there is a vdex file we can make use of. std::unique_ptr<ZipArchive> dm_file(ZipArchive::Open(filename_.c_str(), &error_msg)); if (dm_file != nullptr) { - std::unique_ptr<VdexFile> vdex(VdexFile::OpenFromDm(filename_, *dm_file)); + std::unique_ptr<VdexFile> vdex(VdexFile::OpenFromDm(filename_, *dm_file, &error_msg)); if (vdex != nullptr) { file_.reset(OatFile::OpenFromVdex(zip_fd_, std::move(vdex), @@ -1289,7 +1291,7 @@ bool OatFileAssistant::ZipFileOnlyContainsUncompressedDex() { OatFileAssistant::Location OatFileAssistant::GetLocation(OatFileInfo& info) { if (info.IsUseable()) { - if (&info == &dm_for_oat_ || &info == &dm_for_odex_) { + if (&info == &dm_) { return kLocationDm; } else if (info.IsOatLocation()) { return kLocationOat; diff --git a/runtime/oat/oat_file_assistant.h b/runtime/oat/oat_file_assistant.h index 9af1051362..4526843756 100644 --- a/runtime/oat/oat_file_assistant.h +++ b/runtime/oat/oat_file_assistant.h @@ -400,6 +400,9 @@ class OatFileAssistant { DexOptNeeded GetDexOptNeeded(CompilerFilter::Filter target_compiler_filter, const DexOptTrigger dexopt_trigger); + // Returns true if the file exists. + bool FileExists() const; + // Returns the loaded file. // Loads the file if needed. Returns null if the file failed to load. // The caller shouldn't clean up or free the returned pointer. @@ -567,8 +570,7 @@ class OatFileAssistant { OatFileInfo vdex_for_oat_; // The vdex-only file next to the apk. - OatFileInfo dm_for_odex_; - OatFileInfo dm_for_oat_; + OatFileInfo dm_; // File descriptor corresponding to apk, dex file, or zip. int zip_fd_; diff --git a/runtime/vdex_file.cc b/runtime/vdex_file.cc index 7090320fc4..1767d1f359 100644 --- a/runtime/vdex_file.cc +++ b/runtime/vdex_file.cc @@ -22,12 +22,11 @@ #include <memory> #include <unordered_set> -#include <android-base/logging.h> -#include <android-base/stringprintf.h> -#include <log/log.h> - +#include "android-base/logging.h" +#include "android-base/stringprintf.h" #include "base/bit_utils.h" #include "base/leb128.h" +#include "base/macros.h" #include "base/stl_util.h" #include "base/systrace.h" #include "base/unix_file/fd_file.h" @@ -39,8 +38,9 @@ #include "dex/dex_file_loader.h" #include "gc/heap.h" #include "gc/space/image_space.h" -#include "mirror/class-inl.h" #include "handle_scope-inl.h" +#include "log/log.h" +#include "mirror/class-inl.h" #include "runtime.h" #include "verifier/verifier_deps.h" @@ -140,30 +140,28 @@ std::unique_ptr<VdexFile> VdexFile::OpenAtAddress(uint8_t* mmap_addr, } std::unique_ptr<VdexFile> VdexFile::OpenFromDm(const std::string& filename, - const ZipArchive& archive) { - std::string error_msg; - std::unique_ptr<ZipEntry> zip_entry(archive.Find(VdexFile::kVdexNameInDmFile, &error_msg)); + const ZipArchive& archive, + std::string* error_msg) { + std::unique_ptr<ZipEntry> zip_entry(archive.Find(VdexFile::kVdexNameInDmFile, error_msg)); if (zip_entry == nullptr) { - LOG(INFO) << "No " << VdexFile::kVdexNameInDmFile << " file in DexMetadata archive. " - << "Not doing fast verification."; + *error_msg = ART_FORMAT("No {} file in DexMetadata archive. Not doing fast verification: {}", + VdexFile::kVdexNameInDmFile, + *error_msg); return nullptr; } MemMap input_file = zip_entry->MapDirectlyOrExtract( - filename.c_str(), - VdexFile::kVdexNameInDmFile, - &error_msg, - alignof(VdexFile)); + filename.c_str(), VdexFile::kVdexNameInDmFile, error_msg, alignof(VdexFile)); if (!input_file.IsValid()) { - LOG(WARNING) << "Could not open vdex file in DexMetadata archive: " << error_msg; + *error_msg = "Could not open vdex file in DexMetadata archive: " + *error_msg; return nullptr; } std::unique_ptr<VdexFile> vdex_file = std::make_unique<VdexFile>(std::move(input_file)); if (!vdex_file->IsValid()) { - LOG(WARNING) << "The dex metadata .vdex is not valid. Ignoring it."; + *error_msg = "The dex metadata .vdex is not valid. Ignoring it."; return nullptr; } if (vdex_file->HasDexSection()) { - LOG(ERROR) << "The dex metadata is not allowed to contain dex files"; + *error_msg = "The dex metadata is not allowed to contain dex files"; android_errorWriteLog(0x534e4554, "178055795"); // Report to SafetyNet. return nullptr; } diff --git a/runtime/vdex_file.h b/runtime/vdex_file.h index 7bfbbd169a..ab3ea1f3b3 100644 --- a/runtime/vdex_file.h +++ b/runtime/vdex_file.h @@ -223,7 +223,8 @@ class VdexFile { } EXPORT static std::unique_ptr<VdexFile> OpenFromDm(const std::string& filename, - const ZipArchive& archive); + const ZipArchive& archive, + std::string* error_msg); const uint8_t* Begin() const { return mmap_.Begin(); } const uint8_t* End() const { return mmap_.End(); } |