diff options
author | Treehugger Robot <treehugger-gerrit@google.com> | 2020-08-27 14:59:48 +0000 |
---|---|---|
committer | Gerrit Code Review <noreply-gerritcodereview@google.com> | 2020-08-27 14:59:48 +0000 |
commit | c8fd7faf2b61370390e88e5f482a479f0ed90610 (patch) | |
tree | 7cf5e3bd3fb1571e6b776d277b4146c1923439b0 | |
parent | d1e17720adb32c8934d945a02fc1283a5b6fda37 (diff) | |
parent | cb35a06ff45d43b2dcf73f183a899da3a2084f3d (diff) | |
download | native-c8fd7faf2b61370390e88e5f482a479f0ed90610.tar.gz |
Merge "installd: Replace Dex2oatFileWrapper with UniqueFile"
-rw-r--r-- | cmds/installd/Android.bp | 6 | ||||
-rw-r--r-- | cmds/installd/dexopt.cpp | 297 | ||||
-rw-r--r-- | cmds/installd/run_dex2oat.cpp | 84 | ||||
-rw-r--r-- | cmds/installd/run_dex2oat.h | 42 | ||||
-rw-r--r-- | cmds/installd/run_dex2oat_test.cpp | 77 | ||||
-rw-r--r-- | cmds/installd/unique_file.cpp | 80 | ||||
-rw-r--r-- | cmds/installd/unique_file.h | 101 |
7 files changed, 386 insertions, 301 deletions
diff --git a/cmds/installd/Android.bp b/cmds/installd/Android.bp index 5fa59a5f40..2754571b08 100644 --- a/cmds/installd/Android.bp +++ b/cmds/installd/Android.bp @@ -19,6 +19,7 @@ cc_defaults { "execv_helper.cpp", "globals.cpp", "run_dex2oat.cpp", + "unique_file.cpp", "utils.cpp", "utils_default.cpp", "view_compiler.cpp", @@ -111,6 +112,7 @@ cc_test_host { srcs: [ "run_dex2oat_test.cpp", "run_dex2oat.cpp", + "unique_file.cpp", "execv_helper.cpp", ], cflags: ["-Wall", "-Werror"], @@ -118,9 +120,6 @@ cc_test_host { "libbase", "server_configurable_flags", ], - static_libs: [ - //"libinstalld", - ], test_config: "run_dex2oat_test.xml", } @@ -231,6 +230,7 @@ cc_binary { "otapreopt.cpp", "otapreopt_utils.cpp", "run_dex2oat.cpp", + "unique_file.cpp", "utils.cpp", "utils_default.cpp", "view_compiler.cpp", diff --git a/cmds/installd/dexopt.cpp b/cmds/installd/dexopt.cpp index 82ea746475..5076ae6e56 100644 --- a/cmds/installd/dexopt.cpp +++ b/cmds/installd/dexopt.cpp @@ -54,6 +54,7 @@ #include "installd_deps.h" #include "otapreopt_utils.h" #include "run_dex2oat.h" +#include "unique_file.h" #include "utils.h" using android::base::Basename; @@ -230,6 +231,12 @@ static bool IsBootClassPathProfilingEnable() { return profile_boot_class_path == "true"; } +static void UnlinkIgnoreResult(const std::string& path) { + if (unlink(path.c_str()) < 0) { + PLOG(ERROR) << "Failed to unlink " << path; + } +} + /* * Whether dexopt should use a swap file when compiling an APK. * @@ -346,6 +353,16 @@ static unique_fd open_reference_profile(uid_t uid, const std::string& package_na return open_profile(uid, profile, read_write ? (O_CREAT | O_RDWR) : O_RDONLY); } +static UniqueFile open_reference_profile_as_unique_file(uid_t uid, const std::string& package_name, + const std::string& location, bool read_write, bool is_secondary_dex) { + std::string profile_path = create_reference_profile_path(package_name, location, + is_secondary_dex); + unique_fd ufd = open_profile(uid, profile_path, read_write ? (O_CREAT | O_RDWR) : O_RDONLY); + return UniqueFile(ufd.release(), profile_path, [](const std::string& path) { + clear_profile(path); + }); +} + static unique_fd open_spnashot_profile(uid_t uid, const std::string& package_name, const std::string& location) { std::string profile = create_snapshot_profile_path(package_name, location); @@ -836,118 +853,14 @@ static bool create_oat_out_path(const char* apk_path, const char* instruction_se return true; } -// Helper for fd management. This is similar to a unique_fd in that it closes the file descriptor -// on destruction. It will also run the given cleanup (unless told not to) after closing. -// -// Usage example: -// -// Dex2oatFileWrapper file(open(...), -// [name]() { -// unlink(name.c_str()); -// }); -// // Note: care needs to be taken about name, as it needs to have a lifetime longer than the -// wrapper if captured as a reference. -// -// if (file.get() == -1) { -// // Error opening... -// } -// -// ... -// if (error) { -// // At this point, when the Dex2oatFileWrapper is destructed, the cleanup function will run -// // and delete the file (after the fd is closed). -// return -1; -// } -// -// (Success case) -// file.SetCleanup(false); -// // At this point, when the Dex2oatFileWrapper is destructed, the cleanup function will not run -// // (leaving the file around; after the fd is closed). -// -class Dex2oatFileWrapper { - public: - Dex2oatFileWrapper() : value_(-1), cleanup_(), do_cleanup_(true), auto_close_(true) { - } - - Dex2oatFileWrapper(int value, std::function<void ()> cleanup) - : value_(value), cleanup_(cleanup), do_cleanup_(true), auto_close_(true) {} - - Dex2oatFileWrapper(Dex2oatFileWrapper&& other) { - value_ = other.value_; - cleanup_ = other.cleanup_; - do_cleanup_ = other.do_cleanup_; - auto_close_ = other.auto_close_; - other.release(); - } - - Dex2oatFileWrapper& operator=(Dex2oatFileWrapper&& other) { - value_ = other.value_; - cleanup_ = other.cleanup_; - do_cleanup_ = other.do_cleanup_; - auto_close_ = other.auto_close_; - other.release(); - return *this; - } - - ~Dex2oatFileWrapper() { - reset(-1); - } - - int get() { - return value_; - } - - void SetCleanup(bool cleanup) { - do_cleanup_ = cleanup; - } - - void reset(int new_value) { - if (auto_close_ && value_ >= 0) { - close(value_); - } - if (do_cleanup_ && cleanup_ != nullptr) { - cleanup_(); - } - - value_ = new_value; - } - - void reset(int new_value, std::function<void ()> new_cleanup) { - if (auto_close_ && value_ >= 0) { - close(value_); - } - if (do_cleanup_ && cleanup_ != nullptr) { - cleanup_(); - } - - value_ = new_value; - cleanup_ = new_cleanup; - } - - void DisableAutoClose() { - auto_close_ = false; - } - - private: - void release() { - value_ = -1; - do_cleanup_ = false; - cleanup_ = nullptr; - } - int value_; - std::function<void ()> cleanup_; - bool do_cleanup_; - bool auto_close_; -}; - // (re)Creates the app image if needed. -Dex2oatFileWrapper maybe_open_app_image(const char* out_oat_path, +UniqueFile maybe_open_app_image(const std::string& out_oat_path, bool generate_app_image, bool is_public, int uid, bool is_secondary_dex) { const std::string image_path = create_image_filename(out_oat_path); if (image_path.empty()) { // Happens when the out_oat_path has an unknown extension. - return Dex2oatFileWrapper(); + return UniqueFile(); } // In case there is a stale image, remove it now. Ignore any error. @@ -955,18 +868,19 @@ Dex2oatFileWrapper maybe_open_app_image(const char* out_oat_path, // Not enabled, exit. if (!generate_app_image) { - return Dex2oatFileWrapper(); + return UniqueFile(); } std::string app_image_format = GetProperty("dalvik.vm.appimageformat", ""); if (app_image_format.empty()) { - return Dex2oatFileWrapper(); + return UniqueFile(); } // Recreate is true since we do not want to modify a mapped image. If the app is // already running and we modify the image file, it can cause crashes (b/27493510). - Dex2oatFileWrapper wrapper_fd( + UniqueFile image_file( open_output_file(image_path.c_str(), true /*recreate*/, 0600 /*permissions*/), - [image_path]() { unlink(image_path.c_str()); }); - if (wrapper_fd.get() < 0) { + image_path, + UnlinkIgnoreResult); + if (image_file.fd() < 0) { // Could not create application image file. Go on since we can compile without it. LOG(ERROR) << "installd could not create '" << image_path << "' for image file during dexopt"; @@ -977,21 +891,21 @@ Dex2oatFileWrapper maybe_open_app_image(const char* out_oat_path, } } } else if (!set_permissions_and_ownership( - wrapper_fd.get(), is_public, uid, image_path.c_str(), is_secondary_dex)) { + image_file.fd(), is_public, uid, image_path.c_str(), is_secondary_dex)) { ALOGE("installd cannot set owner '%s' for image during dexopt\n", image_path.c_str()); - wrapper_fd.reset(-1); + image_file.reset(); } - return wrapper_fd; + return image_file; } // Creates the dexopt swap file if necessary and return its fd. // Returns -1 if there's no need for a swap or in case of errors. -unique_fd maybe_open_dexopt_swap_file(const char* out_oat_path) { +unique_fd maybe_open_dexopt_swap_file(const std::string& out_oat_path) { if (!ShouldUseSwapFileForDexopt()) { return invalid_unique_fd(); } - auto swap_file_name = std::string(out_oat_path) + ".swap"; + auto swap_file_name = out_oat_path + ".swap"; unique_fd swap_fd(open_output_file( swap_file_name.c_str(), /*recreate*/true, /*permissions*/0600)); if (swap_fd.get() < 0) { @@ -1009,13 +923,13 @@ unique_fd maybe_open_dexopt_swap_file(const char* out_oat_path) { // Opens the reference profiles if needed. // Note that the reference profile might not exist so it's OK if the fd will be -1. -Dex2oatFileWrapper maybe_open_reference_profile(const std::string& pkgname, +UniqueFile maybe_open_reference_profile(const std::string& pkgname, const std::string& dex_path, const char* profile_name, bool profile_guided, bool is_public, int uid, bool is_secondary_dex) { // If we are not profile guided compilation, or we are compiling system server // do not bother to open the profiles; we won't be using them. if (!profile_guided || (pkgname[0] == '*')) { - return Dex2oatFileWrapper(); + return UniqueFile(); } // If this is a secondary dex path which is public do not open the profile. @@ -1027,7 +941,7 @@ Dex2oatFileWrapper maybe_open_reference_profile(const std::string& pkgname, // compiling with a public profile from the .dm file the PackageManager will // set is_public toghether with the profile guided compilation. if (is_secondary_dex && is_public) { - return Dex2oatFileWrapper(); + return UniqueFile(); } // Open reference profile in read only mode as dex2oat does not get write permissions. @@ -1037,33 +951,28 @@ Dex2oatFileWrapper maybe_open_reference_profile(const std::string& pkgname, } else { if (profile_name == nullptr) { // This path is taken for system server re-compilation lunched from ZygoteInit. - return Dex2oatFileWrapper(); + return UniqueFile(); } else { location = profile_name; } } - unique_fd ufd = open_reference_profile(uid, pkgname, location, /*read_write*/false, - is_secondary_dex); - const auto& cleanup = [pkgname, location, is_secondary_dex]() { - clear_reference_profile(pkgname, location, is_secondary_dex); - }; - return Dex2oatFileWrapper(ufd.release(), cleanup); + return open_reference_profile_as_unique_file(uid, pkgname, location, /*read_write*/false, + is_secondary_dex); } -// Opens the vdex files and assigns the input fd to in_vdex_wrapper_fd and the output fd to -// out_vdex_wrapper_fd. Returns true for success or false in case of errors. +// Opens the vdex files and assigns the input fd to in_vdex_wrapper and the output fd to +// out_vdex_wrapper. Returns true for success or false in case of errors. bool open_vdex_files_for_dex2oat(const char* apk_path, const char* out_oat_path, int dexopt_needed, const char* instruction_set, bool is_public, int uid, bool is_secondary_dex, - bool profile_guided, Dex2oatFileWrapper* in_vdex_wrapper_fd, - Dex2oatFileWrapper* out_vdex_wrapper_fd) { - CHECK(in_vdex_wrapper_fd != nullptr); - CHECK(out_vdex_wrapper_fd != nullptr); + bool profile_guided, UniqueFile* in_vdex_wrapper, + UniqueFile* out_vdex_wrapper) { + CHECK(in_vdex_wrapper != nullptr); + CHECK(out_vdex_wrapper != nullptr); // Open the existing VDEX. We do this before creating the new output VDEX, which will // unlink the old one. char in_odex_path[PKG_PATH_MAX]; int dexopt_action = abs(dexopt_needed); bool is_odex_location = dexopt_needed < 0; - std::string in_vdex_path_str; // Infer the name of the output VDEX. const std::string out_vdex_path_str = create_vdex_filename(out_oat_path); @@ -1085,7 +994,7 @@ bool open_vdex_files_for_dex2oat(const char* apk_path, const char* out_oat_path, } else { path = out_oat_path; } - in_vdex_path_str = create_vdex_filename(path); + std::string in_vdex_path_str = create_vdex_filename(path); if (in_vdex_path_str.empty()) { ALOGE("installd cannot compute input vdex location for '%s'\n", path); return false; @@ -1103,13 +1012,15 @@ bool open_vdex_files_for_dex2oat(const char* apk_path, const char* out_oat_path, !profile_guided; if (update_vdex_in_place) { // Open the file read-write to be able to update it. - in_vdex_wrapper_fd->reset(open(in_vdex_path_str.c_str(), O_RDWR, 0)); - if (in_vdex_wrapper_fd->get() == -1) { + in_vdex_wrapper->reset(open(in_vdex_path_str.c_str(), O_RDWR, 0), + in_vdex_path_str); + if (in_vdex_wrapper->fd() == -1) { // If we failed to open the file, we cannot update it in place. update_vdex_in_place = false; } } else { - in_vdex_wrapper_fd->reset(open(in_vdex_path_str.c_str(), O_RDONLY, 0)); + in_vdex_wrapper->reset(open(in_vdex_path_str.c_str(), O_RDONLY, 0), + in_vdex_path_str); } } @@ -1118,22 +1029,24 @@ bool open_vdex_files_for_dex2oat(const char* apk_path, const char* out_oat_path, if (update_vdex_in_place) { // We unlink the file in case the invocation of dex2oat fails, to ensure we don't // have bogus stale vdex files. - out_vdex_wrapper_fd->reset( - in_vdex_wrapper_fd->get(), - [out_vdex_path_str]() { unlink(out_vdex_path_str.c_str()); }); + out_vdex_wrapper->reset( + in_vdex_wrapper->fd(), + out_vdex_path_str, + UnlinkIgnoreResult); // Disable auto close for the in wrapper fd (it will be done when destructing the out // wrapper). - in_vdex_wrapper_fd->DisableAutoClose(); + in_vdex_wrapper->DisableAutoClose(); } else { - out_vdex_wrapper_fd->reset( + out_vdex_wrapper->reset( open_output_file(out_vdex_path_str.c_str(), /*recreate*/true, /*permissions*/0644), - [out_vdex_path_str]() { unlink(out_vdex_path_str.c_str()); }); - if (out_vdex_wrapper_fd->get() < 0) { + out_vdex_path_str, + UnlinkIgnoreResult); + if (out_vdex_wrapper->fd() < 0) { ALOGE("installd cannot open vdex'%s' during dexopt\n", out_vdex_path_str.c_str()); return false; } } - if (!set_permissions_and_ownership(out_vdex_wrapper_fd->get(), is_public, uid, + if (!set_permissions_and_ownership(out_vdex_wrapper->fd(), is_public, uid, out_vdex_path_str.c_str(), is_secondary_dex)) { ALOGE("installd cannot set owner '%s' for vdex during dexopt\n", out_vdex_path_str.c_str()); return false; @@ -1144,25 +1057,24 @@ bool open_vdex_files_for_dex2oat(const char* apk_path, const char* out_oat_path, } // Opens the output oat file for the given apk. -// If successful it stores the output path into out_oat_path and returns true. -Dex2oatFileWrapper open_oat_out_file(const char* apk_path, const char* oat_dir, - bool is_public, int uid, const char* instruction_set, bool is_secondary_dex, - char* out_oat_path) { +UniqueFile open_oat_out_file(const char* apk_path, const char* oat_dir, + bool is_public, int uid, const char* instruction_set, bool is_secondary_dex) { + char out_oat_path[PKG_PATH_MAX]; if (!create_oat_out_path(apk_path, instruction_set, oat_dir, is_secondary_dex, out_oat_path)) { - return Dex2oatFileWrapper(); + return UniqueFile(); } - const std::string out_oat_path_str(out_oat_path); - Dex2oatFileWrapper wrapper_fd( + UniqueFile oat( open_output_file(out_oat_path, /*recreate*/true, /*permissions*/0644), - [out_oat_path_str]() { unlink(out_oat_path_str.c_str()); }); - if (wrapper_fd.get() < 0) { + out_oat_path, + UnlinkIgnoreResult); + if (oat.fd() < 0) { PLOG(ERROR) << "installd cannot open output during dexopt" << out_oat_path; } else if (!set_permissions_and_ownership( - wrapper_fd.get(), is_public, uid, out_oat_path, is_secondary_dex)) { + oat.fd(), is_public, uid, out_oat_path, is_secondary_dex)) { ALOGE("installd cannot set owner '%s' for output during dexopt\n", out_oat_path); - wrapper_fd.reset(-1); + oat.reset(); } - return wrapper_fd; + return oat; } // Creates RDONLY fds for oat and vdex files, if exist. @@ -1769,8 +1681,8 @@ int dexopt(const char* dex_path, uid_t uid, const char* pkgname, const char* ins } // Open the input file. - unique_fd input_fd(open(dex_path, O_RDONLY, 0)); - if (input_fd.get() < 0) { + UniqueFile in_dex(open(dex_path, O_RDONLY, 0), dex_path); + if (in_dex.fd() < 0) { *error_msg = StringPrintf("installd cannot open '%s' for input during dexopt", dex_path); LOG(ERROR) << *error_msg; return -1; @@ -1784,19 +1696,19 @@ int dexopt(const char* dex_path, uid_t uid, const char* pkgname, const char* ins } // Create the output OAT file. - char out_oat_path[PKG_PATH_MAX]; - Dex2oatFileWrapper out_oat_fd = open_oat_out_file(dex_path, oat_dir, is_public, uid, - instruction_set, is_secondary_dex, out_oat_path); - if (out_oat_fd.get() < 0) { + UniqueFile out_oat = open_oat_out_file(dex_path, oat_dir, is_public, uid, + instruction_set, is_secondary_dex); + if (out_oat.fd() < 0) { *error_msg = "Could not open out oat file."; return -1; } // Open vdex files. - Dex2oatFileWrapper in_vdex_fd; - Dex2oatFileWrapper out_vdex_fd; - if (!open_vdex_files_for_dex2oat(dex_path, out_oat_path, dexopt_needed, instruction_set, - is_public, uid, is_secondary_dex, profile_guided, &in_vdex_fd, &out_vdex_fd)) { + UniqueFile in_vdex; + UniqueFile out_vdex; + if (!open_vdex_files_for_dex2oat(dex_path, out_oat.path().c_str(), dexopt_needed, + instruction_set, is_public, uid, is_secondary_dex, profile_guided, &in_vdex, + &out_vdex)) { *error_msg = "Could not open vdex files."; return -1; } @@ -1816,26 +1728,27 @@ int dexopt(const char* dex_path, uid_t uid, const char* pkgname, const char* ins } // Create a swap file if necessary. - unique_fd swap_fd = maybe_open_dexopt_swap_file(out_oat_path); + unique_fd swap_fd = maybe_open_dexopt_swap_file(out_oat.path()); // Open the reference profile if needed. - Dex2oatFileWrapper reference_profile_fd = maybe_open_reference_profile( + UniqueFile reference_profile = maybe_open_reference_profile( pkgname, dex_path, profile_name, profile_guided, is_public, uid, is_secondary_dex); - if (reference_profile_fd.get() == -1) { + if (reference_profile.fd() == -1) { // We don't create an app image without reference profile since there is no speedup from // loading it in that case and instead will be a small overhead. generate_app_image = false; } // Create the app image file if needed. - Dex2oatFileWrapper image_fd = maybe_open_app_image( - out_oat_path, generate_app_image, is_public, uid, is_secondary_dex); + UniqueFile out_image = maybe_open_app_image( + out_oat.path(), generate_app_image, is_public, uid, is_secondary_dex); - unique_fd dex_metadata_fd; + UniqueFile dex_metadata; if (dex_metadata_path != nullptr) { - dex_metadata_fd.reset(TEMP_FAILURE_RETRY(open(dex_metadata_path, O_RDONLY | O_NOFOLLOW))); - if (dex_metadata_fd.get() < 0) { + dex_metadata.reset(TEMP_FAILURE_RETRY(open(dex_metadata_path, O_RDONLY | O_NOFOLLOW)), + dex_metadata_path); + if (dex_metadata.fd() < 0) { PLOG(ERROR) << "Failed to open dex metadata file " << dex_metadata_path; } } @@ -1862,26 +1775,24 @@ int dexopt(const char* dex_path, uid_t uid, const char* pkgname, const char* ins LOG(VERBOSE) << "DexInv: --- BEGIN '" << dex_path << "' ---"; RunDex2Oat runner(dex2oat_bin, execv_helper.get()); - runner.Initialize(input_fd.get(), - out_oat_fd.get(), - in_vdex_fd.get(), - out_vdex_fd.get(), - image_fd.get(), - dex_path, - out_oat_path, + runner.Initialize(out_oat, + out_vdex, + out_image, + in_dex, + in_vdex, + dex_metadata, + reference_profile, + class_loader_context, + join_fds(context_input_fds), swap_fd.get(), instruction_set, compiler_filter, debuggable, boot_complete, for_restore, - reference_profile_fd.get(), - class_loader_context, - join_fds(context_input_fds), target_sdk_version, enable_hidden_api_checks, generate_compact_dex, - dex_metadata_fd.get(), use_jitzygote_image, compilation_reason); @@ -1891,8 +1802,8 @@ int dexopt(const char* dex_path, uid_t uid, const char* pkgname, const char* ins drop_capabilities(uid); SetDex2OatScheduling(boot_complete); - if (flock(out_oat_fd.get(), LOCK_EX | LOCK_NB) != 0) { - PLOG(ERROR) << "flock(" << out_oat_path << ") failed"; + if (flock(out_oat.fd(), LOCK_EX | LOCK_NB) != 0) { + PLOG(ERROR) << "flock(" << out_oat.path() << ") failed"; _exit(DexoptReturnCodes::kFlock); } @@ -1909,13 +1820,13 @@ int dexopt(const char* dex_path, uid_t uid, const char* pkgname, const char* ins } } - update_out_oat_access_times(dex_path, out_oat_path); + update_out_oat_access_times(dex_path, out_oat.path().c_str()); // We've been successful, don't delete output. - out_oat_fd.SetCleanup(false); - out_vdex_fd.SetCleanup(false); - image_fd.SetCleanup(false); - reference_profile_fd.SetCleanup(false); + out_oat.DisableCleanup(); + out_vdex.DisableCleanup(); + out_image.DisableCleanup(); + reference_profile.DisableCleanup(); return 0; } diff --git a/cmds/installd/run_dex2oat.cpp b/cmds/installd/run_dex2oat.cpp index 1572e3b2a4..17ea90359c 100644 --- a/cmds/installd/run_dex2oat.cpp +++ b/cmds/installd/run_dex2oat.cpp @@ -30,6 +30,8 @@ #include <log/log.h> #include <server_configurable_flags/get_flags.h> +#include "unique_file.h" + using android::base::Basename; using android::base::StringPrintf; @@ -64,35 +66,33 @@ std::vector<std::string> SplitBySpaces(const std::string& str) { RunDex2Oat::RunDex2Oat(const char* dex2oat_bin, ExecVHelper* execv_helper) : dex2oat_bin_(dex2oat_bin), execv_helper_(execv_helper) {} -void RunDex2Oat::Initialize(int zip_fd, - int oat_fd, - int input_vdex_fd, - int output_vdex_fd, - int image_fd, - const char* input_file_name, - const char* output_file_name, +void RunDex2Oat::Initialize(const UniqueFile& output_oat, + const UniqueFile& output_vdex, + const UniqueFile& output_image, + const UniqueFile& input_dex, + const UniqueFile& input_vdex, + const UniqueFile& dex_metadata, + const UniqueFile& profile, + const char* class_loader_context, + const std::string& class_loader_context_fds, int swap_fd, const char* instruction_set, const char* compiler_filter, bool debuggable, bool post_bootcomplete, bool for_restore, - int profile_fd, - const char* class_loader_context, - const std::string& class_loader_context_fds, int target_sdk_version, bool enable_hidden_api_checks, bool generate_compact_dex, - int dex_metadata_fd, bool use_jitzygote_image, const char* compilation_reason) { PrepareBootImageAndBootClasspathFlags(use_jitzygote_image); - PrepareInputFileFlags(zip_fd, oat_fd, input_vdex_fd, output_vdex_fd, image_fd, input_file_name, - output_file_name, profile_fd, dex_metadata_fd, swap_fd, - class_loader_context, class_loader_context_fds); + PrepareInputFileFlags(output_oat, output_vdex, output_image, input_dex, input_vdex, + dex_metadata, profile, swap_fd, class_loader_context, + class_loader_context_fds); - PrepareCompilerConfigFlags(input_vdex_fd, output_vdex_fd, instruction_set, compiler_filter, + PrepareCompilerConfigFlags(input_vdex, output_vdex, instruction_set, compiler_filter, debuggable, target_sdk_version, enable_hidden_api_checks, generate_compact_dex, compilation_reason); @@ -139,38 +139,36 @@ void RunDex2Oat::PrepareBootImageAndBootClasspathFlags(bool use_jitzygote_image) AddArg(updatable_bcp_packages); } -void RunDex2Oat::PrepareInputFileFlags(int zip_fd, - int oat_fd, - int input_vdex_fd, - int output_vdex_fd, - int image_fd, - const char* input_file_name, - const char* output_file_name, - int profile_fd, - int dex_metadata_fd, +void RunDex2Oat::PrepareInputFileFlags(const UniqueFile& output_oat, + const UniqueFile& output_vdex, + const UniqueFile& output_image, + const UniqueFile& input_dex, + const UniqueFile& input_vdex, + const UniqueFile& dex_metadata, + const UniqueFile& profile, int swap_fd, const char* class_loader_context, const std::string& class_loader_context_fds) { - std::string input_basename = Basename(input_file_name); - ALOGV("Running %s in=%s out=%s\n", dex2oat_bin_.c_str(), input_basename.c_str(), - output_file_name); + std::string input_basename = Basename(input_dex.path()); + LOG(VERBOSE) << "Running " << dex2oat_bin_ << " in=" << input_basename << " out=" + << output_oat.path(); - AddArg(StringPrintf("--zip-fd=%d", zip_fd)); + AddArg(StringPrintf("--zip-fd=%d", input_dex.fd())); AddArg(StringPrintf("--zip-location=%s", input_basename.c_str())); - AddArg(StringPrintf("--oat-fd=%d", oat_fd)); - AddArg(StringPrintf("--oat-location=%s", output_file_name)); - AddArg(StringPrintf("--input-vdex-fd=%d", input_vdex_fd)); - AddArg(StringPrintf("--output-vdex-fd=%d", output_vdex_fd)); + AddArg(StringPrintf("--oat-fd=%d", output_oat.fd())); + AddArg(StringPrintf("--oat-location=%s", output_oat.path().c_str())); + AddArg(StringPrintf("--input-vdex-fd=%d", input_vdex.fd())); + AddArg(StringPrintf("--output-vdex-fd=%d", output_vdex.fd())); - if (image_fd >= 0) { - AddArg(StringPrintf("--app-image-fd=%d", image_fd)); + if (output_image.fd() >= 0) { + AddArg(StringPrintf("--app-image-fd=%d", output_image.fd())); AddArg(MapPropertyToArg("dalvik.vm.appimageformat", "--image-format=%s")); } - if (dex_metadata_fd > -1) { - AddArg("--dm-fd=" + std::to_string(dex_metadata_fd)); + if (dex_metadata.fd() > -1) { + AddArg("--dm-fd=" + std::to_string(dex_metadata.fd())); } - if (profile_fd != -1) { - AddArg(StringPrintf("--profile-file-fd=%d", profile_fd)); + if (profile.fd() != -1) { + AddArg(StringPrintf("--profile-file-fd=%d", profile.fd())); } if (swap_fd >= 0) { AddArg(StringPrintf("--swap-fd=%d", swap_fd)); @@ -178,7 +176,7 @@ void RunDex2Oat::PrepareInputFileFlags(int zip_fd, // Get the directory of the apk to pass as a base classpath directory. { - std::string apk_dir(input_file_name); + std::string apk_dir(input_dex.path()); size_t dir_index = apk_dir.rfind('/'); if (dir_index != std::string::npos) { apk_dir = apk_dir.substr(0, dir_index); @@ -195,8 +193,8 @@ void RunDex2Oat::PrepareInputFileFlags(int zip_fd, } } -void RunDex2Oat::PrepareCompilerConfigFlags(int input_vdex_fd, - int output_vdex_fd, +void RunDex2Oat::PrepareCompilerConfigFlags(const UniqueFile& input_vdex, + const UniqueFile& output_vdex, const char* instruction_set, const char* compiler_filter, bool debuggable, @@ -206,11 +204,11 @@ void RunDex2Oat::PrepareCompilerConfigFlags(int input_vdex_fd, const char* compilation_reason) { // Disable cdex if update input vdex is true since this combination of options is not // supported. - const bool disable_cdex = !generate_compact_dex || (input_vdex_fd == output_vdex_fd); + const bool disable_cdex = !generate_compact_dex || (input_vdex.fd() == output_vdex.fd()); if (disable_cdex) { AddArg(kDisableCompactDexFlag); } - + // ISA related { AddArg(StringPrintf("--instruction-set=%s", instruction_set)); diff --git a/cmds/installd/run_dex2oat.h b/cmds/installd/run_dex2oat.h index 5453272153..325a3a2599 100644 --- a/cmds/installd/run_dex2oat.h +++ b/cmds/installd/run_dex2oat.h @@ -25,31 +25,31 @@ namespace android { namespace installd { +class UniqueFile; + class RunDex2Oat { public: explicit RunDex2Oat(const char* dex2oat_bin, ExecVHelper* execv_helper); virtual ~RunDex2Oat(); - void Initialize(int zip_fd, - int oat_fd, - int input_vdex_fd, - int output_vdex_fd, - int image_fd, - const char* input_file_name, - const char* output_file_name, + void Initialize(const UniqueFile& output_oat, + const UniqueFile& output_vdex, + const UniqueFile& output_image, + const UniqueFile& input_dex, + const UniqueFile& input_vdex, + const UniqueFile& dex_metadata, + const UniqueFile& profile, + const char* class_loader_context, + const std::string& class_loader_context_fds, int swap_fd, const char* instruction_set, const char* compiler_filter, bool debuggable, bool post_bootcomplete, bool for_restore, - int profile_fd, - const char* class_loader_context, - const std::string& class_loader_context_fds, int target_sdk_version, bool enable_hidden_api_checks, bool generate_compact_dex, - int dex_metadata_fd, bool use_jitzygote_image, const char* compilation_reason); @@ -57,20 +57,18 @@ class RunDex2Oat { protected: void PrepareBootImageAndBootClasspathFlags(bool use_jitzygote_image); - void PrepareInputFileFlags(int zip_fd, - int oat_fd, - int input_vdex_fd, - int output_vdex_fd, - int image_fd, - const char* input_file_name, - const char* output_file_name, - int profile_fd, - int dex_metadata_fd, + void PrepareInputFileFlags(const UniqueFile& output_oat, + const UniqueFile& output_vdex, + const UniqueFile& output_image, + const UniqueFile& input_dex, + const UniqueFile& input_vdex, + const UniqueFile& dex_metadata, + const UniqueFile& profile, int swap_fd, const char* class_loader_context, const std::string& class_loader_context_fds); - void PrepareCompilerConfigFlags(int input_vdex_fd, - int output_vdex_fd, + void PrepareCompilerConfigFlags(const UniqueFile& input_vdex, + const UniqueFile& output_vdex, const char* instruction_set, const char* compiler_filter, bool debuggable, diff --git a/cmds/installd/run_dex2oat_test.cpp b/cmds/installd/run_dex2oat_test.cpp index b1f429d01f..3813cf7c09 100644 --- a/cmds/installd/run_dex2oat_test.cpp +++ b/cmds/installd/run_dex2oat_test.cpp @@ -24,6 +24,7 @@ #include "execv_helper.h" #include "run_dex2oat.h" +#include "unique_file.h" namespace android { namespace installd { @@ -34,14 +35,16 @@ class RunDex2OatTest : public testing::Test { static constexpr const char* OUTPUT_PATH = "/dir/output/basename.oat"; static constexpr const char* FLAG_UNUSED = "{{FLAG_UNUSED}}"; - static constexpr int ZIP_FD = 1; - static constexpr int OAT_FD = 2; - static constexpr int INPUT_VDEX_FD = 3; - static constexpr int OUTPUT_VDEX_FD = 4; - static constexpr int IMAGE_FD = 5; - static constexpr int PROFILE_FD = 6; - static constexpr int DEX_METADATA_FD = 7; - static constexpr int SWAP_FD = 8; + // UniqueFile closes FD. Avoid using standard I/O since the test is expected to print gtest + // results. Alternatively, mock out UniqueFile to avoid the side effect of close(2). + static constexpr int ZIP_FD = 3; + static constexpr int OAT_FD = 4; + static constexpr int INPUT_VDEX_FD = 5; + static constexpr int OUTPUT_VDEX_FD = 6; + static constexpr int IMAGE_FD = 7; + static constexpr int PROFILE_FD = 8; + static constexpr int DEX_METADATA_FD = 9; + static constexpr int SWAP_FD = 10; using FakeSystemProperties = std::map<std::string, std::string>; @@ -84,37 +87,33 @@ class RunDex2OatTest : public testing::Test { struct RunDex2OatArgs { static std::unique_ptr<RunDex2OatArgs> MakeDefaultTestArgs() { auto args = std::make_unique<RunDex2OatArgs>(); - args->input_file_name = INPUT_PATH; - args->zip_fd = ZIP_FD; - args->output_file_name = OUTPUT_PATH; - args->oat_fd = OAT_FD; - args->input_vdex_fd = INPUT_VDEX_FD; - args->output_vdex_fd = OUTPUT_VDEX_FD; + args->input_dex.reset(ZIP_FD, INPUT_PATH); + args->output_oat.reset(OAT_FD, OUTPUT_PATH); + args->input_vdex.reset(INPUT_VDEX_FD, "UNUSED_PATH"); + args->output_vdex.reset(OUTPUT_VDEX_FD, "UNUSED_PATH"); args->instruction_set = "arm64"; args->compilation_reason = "rundex2oattest"; return args; } - int zip_fd = -1; - int oat_fd = -1; - int input_vdex_fd = -1; - int output_vdex_fd = -1; - int image_fd = -1; - const char* input_file_name = nullptr; - const char* output_file_name = nullptr; + UniqueFile output_oat; + UniqueFile output_vdex; + UniqueFile output_image; + UniqueFile input_dex; + UniqueFile input_vdex; + UniqueFile dex_metadata; + UniqueFile profile; int swap_fd = -1; const char* instruction_set = nullptr; const char* compiler_filter = "extract"; bool debuggable = false; bool post_bootcomplete = false; bool for_restore = false; - int profile_fd = -1; const char* class_loader_context = nullptr; std::string class_loader_context_fds; int target_sdk_version = 0; bool enable_hidden_api_checks = false; bool generate_compact_dex = true; - int dex_metadata_fd = -1; bool use_jitzygote_image = false; const char* compilation_reason = nullptr; }; @@ -143,7 +142,7 @@ class RunDex2OatTest : public testing::Test { cmd += arg; cmd += " "; } - LOG(DEBUG) << "FakeExecVHelper exit_code: " << exit_code << " cmd: " << cmd; + LOG(DEBUG) << "FakeExecVHelper exit_code: " << exit_code << " cmd: " << cmd << "\n"; } }; @@ -230,7 +229,7 @@ class RunDex2OatTest : public testing::Test { << "Flag " << flag << " should be specified without value, but got " << value; } else { EXPECT_TRUE(execv_helper_->HasArg(flag + value)) - << "Flag " << flag << "=" << value << " is not specificed"; + << "Flag " << flag << value << " is not specificed"; } } } @@ -241,26 +240,24 @@ class RunDex2OatTest : public testing::Test { void CallRunDex2Oat(std::unique_ptr<RunDex2OatArgs> args) { FakeRunDex2Oat runner(execv_helper_.get(), &system_properties_); - runner.Initialize(args->zip_fd, - args->oat_fd, - args->input_vdex_fd, - args->output_vdex_fd, - args->image_fd, - args->input_file_name, - args->output_file_name, + runner.Initialize(args->output_oat, + args->output_vdex, + args->output_image, + args->input_dex, + args->input_vdex, + args->dex_metadata, + args->profile, + args->class_loader_context, + args->class_loader_context_fds, args->swap_fd, args->instruction_set, args->compiler_filter, args->debuggable, args->post_bootcomplete, args->for_restore, - args->profile_fd, - args->class_loader_context, - args->class_loader_context_fds, args->target_sdk_version, args->enable_hidden_api_checks, args->generate_compact_dex, - args->dex_metadata_fd, args->use_jitzygote_image, args->compilation_reason); runner.Exec(/*exit_code=*/ 0); @@ -281,8 +278,8 @@ TEST_F(RunDex2OatTest, BasicInputOutput) { TEST_F(RunDex2OatTest, WithAllOtherInputFds) { auto args = RunDex2OatArgs::MakeDefaultTestArgs(); - args->image_fd = IMAGE_FD; - args->profile_fd = PROFILE_FD; + args->output_image.reset(IMAGE_FD, "UNUSED_PATH"); + args->profile.reset(PROFILE_FD, "UNUSED_PATH"); args->swap_fd = SWAP_FD; CallRunDex2Oat(std::move(args)); @@ -357,8 +354,8 @@ TEST_F(RunDex2OatTest, DoNotGenerateCompactDex) { TEST_F(RunDex2OatTest, DoNotGenerateCompactDexWithVdexInPlaceUpdate) { auto args = RunDex2OatArgs::MakeDefaultTestArgs(); args->generate_compact_dex = true; - args->input_vdex_fd = INPUT_VDEX_FD; - args->output_vdex_fd = INPUT_VDEX_FD; + args->input_vdex.reset(INPUT_VDEX_FD, "UNUSED_PATH"); + args->output_vdex.reset(INPUT_VDEX_FD, "UNUSED_PATH"); CallRunDex2Oat(std::move(args)); SetExpectedFlagUsed("--compact-dex-level", "=none"); diff --git a/cmds/installd/unique_file.cpp b/cmds/installd/unique_file.cpp new file mode 100644 index 0000000000..e99ce1eeac --- /dev/null +++ b/cmds/installd/unique_file.cpp @@ -0,0 +1,80 @@ +/* + * Copyright (C) 2020 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "unique_file.h" + +#include <string> + +#include <unistd.h> + +#include <android-base/logging.h> + +namespace android { +namespace installd { + +UniqueFile::UniqueFile() : UniqueFile(-1, "") {} + +UniqueFile::UniqueFile(int value, std::string path) : UniqueFile(value, path, nullptr) {} + +UniqueFile::UniqueFile(int value, std::string path, CleanUpFunction cleanup) + : value_(value), path_(path), cleanup_(cleanup), do_cleanup_(true), auto_close_(true) {} + +UniqueFile::UniqueFile(UniqueFile&& other) { + *this = std::move(other); +} + +UniqueFile::~UniqueFile() { + reset(); +} + +UniqueFile& UniqueFile::operator=(UniqueFile&& other) { + value_ = other.value_; + path_ = other.path_; + cleanup_ = other.cleanup_; + do_cleanup_ = other.do_cleanup_; + auto_close_ = other.auto_close_; + other.release(); + return *this; +} + +void UniqueFile::reset() { + reset(-1, ""); +} + +void UniqueFile::reset(int new_value, std::string path, CleanUpFunction new_cleanup) { + if (auto_close_ && value_ >= 0) { + if (close(value_) < 0) { + PLOG(ERROR) << "Failed to close fd " << value_ << ", with path " << path; + } + } + if (do_cleanup_ && cleanup_ != nullptr) { + cleanup_(path_); + } + + value_ = new_value; + path_ = path; + cleanup_ = new_cleanup; +} + +void UniqueFile::release() { + value_ = -1; + path_ = ""; + do_cleanup_ = false; + cleanup_ = nullptr; +} + +} // namespace installd +} // namespace android diff --git a/cmds/installd/unique_file.h b/cmds/installd/unique_file.h new file mode 100644 index 0000000000..e85e23b3a1 --- /dev/null +++ b/cmds/installd/unique_file.h @@ -0,0 +1,101 @@ +/* + * Copyright (C) 2020 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef ANDROID_INSTALLD_UNIQUE_FILE_H +#define ANDROID_INSTALLD_UNIQUE_FILE_H + +#include <functional> +#include <string> + +namespace android { +namespace installd { + +// A file management helper that serves two purposes: +// +// 1. Closes the file description on destruction, similar unique_fd. +// 2. Runs a cleanup function on after close, if not cancelled. +// +// The class does not assume the relationship between the given fd and file path. +// +// Example: +// +// UniqueFile file(open(...), +// filepath, +// [](const std::string& path) { +// unlink(path.c_str()); +// }); +// if (file.fd() == -1) { +// // Error opening... +// } +// +// ... +// if (error) { +// // At this point, when the UniqueFile is destructed, the cleanup function will run +// // (e.g. to delete the file) after the fd is closed. +// return -1; +// } +// +// (Success case) +// file.DisableCleanup(); +// // At this point, when the UniqueFile is destructed, the cleanup function will not run +// // (e.g. leaving the file around) after the fd is closed. +// +class UniqueFile { + private: + using CleanUpFunction = std::function<void (const std::string&)>; + + public: + UniqueFile(); + UniqueFile(int value, std::string path); + UniqueFile(int value, std::string path, CleanUpFunction cleanup); + UniqueFile(UniqueFile&& other); + ~UniqueFile(); + + UniqueFile& operator=(UniqueFile&& other); + + int fd() const { + return value_; + } + + const std::string& path() const { + return path_; + } + + void DisableAutoClose() { + auto_close_ = false; + } + + void DisableCleanup() { + do_cleanup_ = false; + } + + void reset(); + void reset(int new_value, std::string path, CleanUpFunction new_cleanup = nullptr); + + private: + void release(); + + int value_; + std::string path_; + CleanUpFunction cleanup_; + bool do_cleanup_; + bool auto_close_; +}; + +} // namespace installd +} // namespace android + +#endif // ANDROID_INSTALLD_UNIQUE_FILE_H |