Merge "Installd: don't generate app image with empty reference profile"

2 files changed, 57 insertions, 36 deletions

diff --git a/cmds/installd/dexopt.cpp b/cmds/installd/dexopt.cpp
index ce3d669a63..794750ff81 100644
--- a/cmds/installd/dexopt.cpp
+++ b/cmds/installd/dexopt.cpp
@@ -1917,10 +1917,11 @@ int dexopt(const char* dex_path, uid_t uid, const char* pkgname, const char* ins
     // Open the reference profile if needed.
     UniqueFile reference_profile = maybe_open_reference_profile(
             pkgname, dex_path, profile_name, profile_guided, is_public, uid, is_secondary_dex);
-
-    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.
+    struct stat sbuf;
+    if (reference_profile.fd() == -1 ||
+        (fstat(reference_profile.fd(), &sbuf) != -1 && sbuf.st_size == 0)) {
+        // We don't create an app image with empty or non existing reference profile since there
+        // is no speedup from loading it in that case and instead will be a small overhead.
         generate_app_image = false;
     }
 
diff --git a/cmds/installd/tests/installd_dexopt_test.cpp b/cmds/installd/tests/installd_dexopt_test.cpp
index 3b589dc581..5c4e1a45fc 100644
--- a/cmds/installd/tests/installd_dexopt_test.cpp
+++ b/cmds/installd/tests/installd_dexopt_test.cpp
@@ -185,7 +185,7 @@ protected:
     std::optional<std::string> volume_uuid_;
     std::string package_name_;
     std::string apk_path_;
-    std::string empty_dm_file_;
+    std::string dm_file_;
     std::string app_apk_dir_;
     std::string app_private_dir_ce_;
     std::string app_private_dir_de_;
@@ -248,26 +248,6 @@ protected:
                                                  << " : " << error_msg;
         }
 
-        // Create an empty dm file.
-        empty_dm_file_ = apk_path_ + ".dm";
-        {
-            int fd = open(empty_dm_file_.c_str(), O_WRONLY | O_CREAT, S_IRUSR | S_IWUSR);
-            if (fd < 0) {
-                return ::testing::AssertionFailure() << "Could not open " << empty_dm_file_;
-            }
-            FILE* file = fdopen(fd, "wb");
-            if (file == nullptr) {
-                return ::testing::AssertionFailure() << "Null file for " << empty_dm_file_
-                         << " fd=" << fd;
-            }
-            ZipWriter writer(file);
-            // Add vdex to zip.
-            writer.StartEntry("primary.prof", ZipWriter::kCompress);
-            writer.FinishEntry();
-            writer.Finish();
-            fclose(file);
-          }
-
         // Create the app user data.
         binder::Status status = service_->createAppData(
                 volume_uuid_,
@@ -316,6 +296,46 @@ protected:
                                                  << secondary_dex_de_ << " : " << error_msg;
         }
 
+        // Create a non-empty dm file.
+        dm_file_ = apk_path_ + ".dm";
+        {
+            android::base::unique_fd fd(open(dm_file_.c_str(),
+                                          O_WRONLY | O_CREAT, S_IRUSR | S_IWUSR));
+            if (fd.get() < 0) {
+                return ::testing::AssertionFailure() << "Could not open " << dm_file_;
+            }
+            FILE* file = fdopen(fd.release(), "wb");
+            if (file == nullptr) {
+                return ::testing::AssertionFailure() << "Null file for " << dm_file_
+                                  << " fd=" << fd.get();
+            }
+
+            // Create a profile file.
+            std::string profile_file = app_private_dir_ce_ + "/primary.prof";
+            run_cmd("profman --generate-test-profile=" + profile_file);
+
+            // Add profile to zip.
+            ZipWriter writer(file);
+            writer.StartEntry("primary.prof", ZipWriter::kCompress);
+            android::base::unique_fd profile_fd(open(profile_file.c_str(), O_RDONLY));
+            if (profile_fd.get() < 0) {
+                return ::testing::AssertionFailure() << "Failed to open profile '"
+                                  << profile_file << "'";
+            }
+            std::string profile_content;
+            if (!android::base::ReadFdToString(profile_fd, &profile_content)) {
+                return ::testing::AssertionFailure() << "Failed to read profile "
+                                  << profile_file << "'";
+            }
+            writer.WriteBytes(profile_content.c_str(), profile_content.length());
+            writer.FinishEntry();
+            writer.Finish();
+            fclose(file);
+
+            // Delete the temp file.
+            unlink(profile_file.c_str());
+        }
+
         // Fix app data uid.
         status = service_->fixupAppData(volume_uuid_, kTestUserId);
         if (!status.isOk()) {
@@ -608,7 +628,7 @@ protected:
                 kTestAppGid,
                 DEX2OAT_FROM_SCRATCH,
                 /*binder_result=*/nullptr,
-                empty_dm_file_.c_str());
+                dm_file_.c_str());
 
 
         int64_t odex_size = GetSize(GetPrimaryDexArtifact(oat_dir, apk_path_,
@@ -657,13 +677,13 @@ protected:
                             DEXOPT_BOOTCOMPLETE | DEXOPT_PROFILE_GUIDED | DEXOPT_PUBLIC |
                                     DEXOPT_GENERATE_APP_IMAGE,
                             oat_dir, kTestAppGid, DEX2OAT_FROM_SCRATCH,
-                            /*binder_result=*/nullptr, empty_dm_file_.c_str());
+                            /*binder_result=*/nullptr, dm_file_.c_str());
         checkVisibility(in_dalvik_cache, ODEX_IS_PUBLIC);
 
         CompilePrimaryDexOk("speed-profile",
                             DEXOPT_BOOTCOMPLETE | DEXOPT_PROFILE_GUIDED | DEXOPT_GENERATE_APP_IMAGE,
                             oat_dir, kTestAppGid, DEX2OAT_FROM_SCRATCH,
-                            /*binder_result=*/nullptr, empty_dm_file_.c_str());
+                            /*binder_result=*/nullptr, dm_file_.c_str());
         checkVisibility(in_dalvik_cache, ODEX_IS_PRIVATE);
     }
 };
@@ -787,7 +807,7 @@ TEST_F(DexoptTest, DexoptPrimaryProfileNonPublic) {
                         kTestAppGid,
                         DEX2OAT_FROM_SCRATCH,
                         /*binder_result=*/nullptr,
-                        empty_dm_file_.c_str());
+                        dm_file_.c_str());
 }
 
 TEST_F(DexoptTest, DexoptPrimaryProfilePublic) {
@@ -799,7 +819,7 @@ TEST_F(DexoptTest, DexoptPrimaryProfilePublic) {
                         kTestAppGid,
                         DEX2OAT_FROM_SCRATCH,
                         /*binder_result=*/nullptr,
-                        empty_dm_file_.c_str());
+                        dm_file_.c_str());
 }
 
 TEST_F(DexoptTest, DexoptPrimaryBackgroundOk) {
@@ -811,7 +831,7 @@ TEST_F(DexoptTest, DexoptPrimaryBackgroundOk) {
                         kTestAppGid,
                         DEX2OAT_FROM_SCRATCH,
                         /*binder_result=*/nullptr,
-                        empty_dm_file_.c_str());
+                        dm_file_.c_str());
 }
 
 TEST_F(DexoptTest, DexoptBlockPrimary) {
@@ -874,7 +894,7 @@ TEST_F(DexoptTest, ResolveStartupConstStrings) {
                         kTestAppGid,
                         DEX2OAT_FROM_SCRATCH,
                         /*binder_result=*/nullptr,
-                        empty_dm_file_.c_str());
+                        dm_file_.c_str());
     run_cmd_and_process_output(
             "oatdump --header-only --oat-file=" + odex,
             [&](const std::string& line) {
@@ -893,7 +913,7 @@ TEST_F(DexoptTest, ResolveStartupConstStrings) {
                         kTestAppGid,
                         DEX2OAT_FROM_SCRATCH,
                         /*binder_result=*/nullptr,
-                        empty_dm_file_.c_str());
+                        dm_file_.c_str());
     run_cmd_and_process_output(
             "oatdump --header-only --oat-file=" + odex,
             [&](const std::string& line) {
@@ -926,7 +946,7 @@ TEST_F(DexoptTest, DexoptDex2oat64Enabled) {
                         kTestAppGid,
                         DEX2OAT_FROM_SCRATCH,
                         /*binder_result=*/nullptr,
-                        empty_dm_file_.c_str());
+                        dm_file_.c_str());
     // Enable the property and use dex2oat64.
     ASSERT_TRUE(android::base::SetProperty(property, "true")) << property;
     CompilePrimaryDexOk("speed-profile",
@@ -936,7 +956,7 @@ TEST_F(DexoptTest, DexoptDex2oat64Enabled) {
                         kTestAppGid,
                         DEX2OAT_FROM_SCRATCH,
                         /*binder_result=*/nullptr,
-                        empty_dm_file_.c_str());
+                        dm_file_.c_str());
 }
 
 class PrimaryDexReCompilationTest : public DexoptTest {
@@ -1143,7 +1163,7 @@ class ProfileTest : public DexoptTest {
                 service_->prepareAppProfile(package_name, has_user_id ? kTestUserId : USER_NULL,
                                             kTestAppId, profile_name, apk_path_,
                                             has_dex_metadata ? std::make_optional<std::string>(
-                                                                       empty_dm_file_)
+                                                                       dm_file_)
                                                              : std::nullopt,
                                             &result));
         ASSERT_EQ(expected_result, result);