Installd: Separate profile merging and compilation

Create a new merge_profiles command to check and merge profiles.

(Almost) always pass a profile to a dexopt pass.

Require compilation filter input to dexopt and use it for dex2oat
(except for hard overrides like safe-mode).

Bug: 27689078
Change-Id: I1257857cc15c17e2271d1261ea4cc80752270fcb

5 files changed, 82 insertions, 60 deletions

diff --git a/cmds/installd/commands.cpp b/cmds/installd/commands.cpp
index 47c78c3c13..c0e0214030 100644
--- a/cmds/installd/commands.cpp
+++ b/cmds/installd/commands.cpp
@@ -714,7 +714,7 @@ static void run_patchoat(int input_fd, int oat_fd, const char* input_file_name,
 
 static void run_dex2oat(int zip_fd, int oat_fd, int image_fd, const char* input_file_name,
         const char* output_file_name, int swap_fd, const char *instruction_set,
-        bool vm_safe_mode, bool debuggable, bool post_bootcomplete, bool extract_only,
+        const char* compiler_filter, bool vm_safe_mode, bool debuggable, bool post_bootcomplete,
         int profile_fd) {
     static const unsigned int MAX_INSTRUCTION_SET_LEN = 7;
 
@@ -730,10 +730,6 @@ static void run_dex2oat(int zip_fd, int oat_fd, int image_fd, const char* input_
     char dex2oat_Xmx_flag[kPropertyValueMax];
     bool have_dex2oat_Xmx_flag = get_property("dalvik.vm.dex2oat-Xmx", dex2oat_Xmx_flag, NULL) > 0;
 
-    char dex2oat_compiler_filter_flag[kPropertyValueMax];
-    bool have_dex2oat_compiler_filter_flag = get_property("dalvik.vm.dex2oat-filter",
-                                                          dex2oat_compiler_filter_flag, NULL) > 0;
-
     char dex2oat_threads_buf[kPropertyValueMax];
     bool have_dex2oat_threads_flag = get_property(post_bootcomplete
                                                       ? "dalvik.vm.dex2oat-threads"
@@ -825,6 +821,10 @@ static void run_dex2oat(int zip_fd, int oat_fd, int image_fd, const char* input_
     if (have_dex2oat_Xmx_flag) {
         sprintf(dex2oat_Xmx_arg, "-Xmx%s", dex2oat_Xmx_flag);
     }
+
+    // Compute compiler filter.
+
+    bool have_dex2oat_compiler_filter_flag;
     if (skip_compilation) {
         strcpy(dex2oat_compiler_filter_arg, "--compiler-filter=verify-none");
         have_dex2oat_compiler_filter_flag = true;
@@ -832,13 +832,20 @@ static void run_dex2oat(int zip_fd, int oat_fd, int image_fd, const char* input_
     } else if (vm_safe_mode) {
         strcpy(dex2oat_compiler_filter_arg, "--compiler-filter=interpret-only");
         have_dex2oat_compiler_filter_flag = true;
-    } else if (extract_only) {
-        // Temporarily make extract-only mean interpret-only, so extracted files will be verified.
-        // b/26833007
-        strcpy(dex2oat_compiler_filter_arg, "--compiler-filter=interpret-only");
+    } else if (compiler_filter != nullptr &&
+            strlen(compiler_filter) + strlen("--compiler-filter=") <
+                    arraysize(dex2oat_compiler_filter_arg)) {
+        sprintf(dex2oat_compiler_filter_arg, "--compiler-filter=%s", compiler_filter);
         have_dex2oat_compiler_filter_flag = true;
-    } else if (have_dex2oat_compiler_filter_flag) {
-        sprintf(dex2oat_compiler_filter_arg, "--compiler-filter=%s", dex2oat_compiler_filter_flag);
+    } else {
+        char dex2oat_compiler_filter_flag[kPropertyValueMax];
+        have_dex2oat_compiler_filter_flag = get_property("dalvik.vm.dex2oat-filter",
+                                                         dex2oat_compiler_filter_flag, NULL) > 0;
+        if (have_dex2oat_compiler_filter_flag) {
+            sprintf(dex2oat_compiler_filter_arg,
+                    "--compiler-filter=%s",
+                    dex2oat_compiler_filter_flag);
+        }
     }
 
     // Check whether all apps should be compiled debuggable.
@@ -1283,9 +1290,14 @@ static bool create_oat_out_path(const char* apk_path, const char* instruction_se
     return true;
 }
 
+// TODO: Consider returning error codes.
+bool merge_profiles(uid_t uid, const char *pkgname) {
+    return analyse_profiles(uid, pkgname);
+}
+
 int dexopt(const char* apk_path, uid_t uid, const char* pkgname, const char* instruction_set,
-           int dexopt_needed, const char* oat_dir, int dexopt_flags,
-           const char* volume_uuid ATTRIBUTE_UNUSED, bool use_profiles)
+           int dexopt_needed, const char* oat_dir, int dexopt_flags, const char* compiler_filter,
+           const char* volume_uuid ATTRIBUTE_UNUSED)
 {
     struct utimbuf ut;
     struct stat input_stat;
@@ -1300,29 +1312,18 @@ int dexopt(const char* apk_path, uid_t uid, const char* pkgname, const char* ins
     bool vm_safe_mode = (dexopt_flags & DEXOPT_SAFEMODE) != 0;
     bool debuggable = (dexopt_flags & DEXOPT_DEBUGGABLE) != 0;
     bool boot_complete = (dexopt_flags & DEXOPT_BOOTCOMPLETE) != 0;
-    bool extract_only = (dexopt_flags & DEXOPT_EXTRACTONLY) != 0;
-    fd_t reference_profile_fd = -1;
+    bool profile_guided = (dexopt_flags & DEXOPT_PROFILE_GUIDED) != 0;
 
-    if (is_public && use_profiles) {
-        // We should not give public access to apks compiled with profile information.
-        // Log an error and return early if are asked to do so.
-        ALOGE("use_profiles should not be used with is_public.");
-        return -1;
-    }
+    CHECK(pkgname != nullptr);
+    CHECK(pkgname[0] != 0);
 
-    if (use_profiles) {
-        if (analyse_profiles(uid, pkgname)) {
-            // Open again reference profile in read only mode as dex2oat does not get write
-            // permissions.
-            reference_profile_fd = open_reference_profile(uid, pkgname, /*read_write*/ false);
-            if (reference_profile_fd == -1) {
-                PLOG(WARNING) << "Couldn't open reference profile in read only mode " << pkgname;
-                exit(72);
-            }
-        } else {
-            // No need to (re)compile. Return early.
-            return 0;
-        }
+    fd_t reference_profile_fd = -1;
+    // Public apps should not be compiled with profile information ever. Same goes for the special
+    // package '*' used for the system server.
+    if (!is_public && pkgname[0] != '*') {
+        // Open reference profile in read only mode as dex2oat does not get write permissions.
+        reference_profile_fd = open_reference_profile(uid, pkgname, /*read_write*/ false);
+        // Note: it's OK to not find a profile here.
     }
 
     if ((dexopt_flags & ~DEXOPT_MASK) != 0) {
@@ -1397,7 +1398,9 @@ int dexopt(const char* apk_path, uid_t uid, const char* pkgname, const char* ins
       char app_image_format[kPropertyValueMax];
       bool have_app_image_format =
               get_property("dalvik.vm.appimageformat", app_image_format, NULL) > 0;
-      if (!extract_only && have_app_image_format) {
+      // Use app images only if it is enabled (by a set image format) and we are compiling
+      // profile-guided (so the app image doesn't conservatively contain all classes).
+      if (profile_guided && have_app_image_format) {
           // Recreate is false since we want to avoid deleting the image in case dex2oat decides to
           // not compile anything.
           image_fd = open_output_file(image_path, /*recreate*/false);
@@ -1440,7 +1443,7 @@ int dexopt(const char* apk_path, uid_t uid, const char* pkgname, const char* ins
                 input_file_name++;
             }
             run_dex2oat(input_fd, out_fd, image_fd, input_file_name, out_path, swap_fd,
-                        instruction_set, vm_safe_mode, debuggable, boot_complete, extract_only,
+                        instruction_set, compiler_filter, vm_safe_mode, debuggable, boot_complete,
                         reference_profile_fd);
         } else {
             ALOGE("Invalid dexopt needed: %d\n", dexopt_needed);
diff --git a/cmds/installd/commands.h b/cmds/installd/commands.h
index b473e3eca5..70cb410907 100644
--- a/cmds/installd/commands.h
+++ b/cmds/installd/commands.h
@@ -48,9 +48,12 @@ int make_user_config(userid_t userid);
 int delete_user(const char *uuid, userid_t userid);
 int rm_dex(const char *path, const char *instruction_set);
 int free_cache(const char *uuid, int64_t free_size);
+
+bool merge_profiles(uid_t uid, const char *pkgname);
+
 int dexopt(const char *apk_path, uid_t uid, const char *pkgName, const char *instruction_set,
-           int dexopt_needed, const char* oat_dir, int dexopt_flags,
-           const char* volume_uuid, bool use_profiles);
+           int dexopt_needed, const char* oat_dir, int dexopt_flags, const char* compiler_filter,
+           const char* volume_uuid);
 int mark_boot_complete(const char *instruction_set);
 int linklib(const char* uuid, const char* pkgname, const char* asecLibDir, int userId);
 int idmap(const char *target_path, const char *overlay_path, uid_t uid);
diff --git a/cmds/installd/installd.cpp b/cmds/installd/installd.cpp
index 2bf27a2f04..dc3418a6a3 100644
--- a/cmds/installd/installd.cpp
+++ b/cmds/installd/installd.cpp
@@ -258,10 +258,27 @@ static int do_dexopt(char **arg, char reply[REPLY_MAX])
     if ((dexopt_flags & DEXOPT_OTA) != 0) {
       return do_ota_dexopt(arg, reply);
     }
-    /* apk_path, uid, pkgname, instruction_set, dexopt_needed, oat_dir, dexopt_flags, volume_uuid,
-            use_profiles */
-    return dexopt(arg[0], atoi(arg[1]), arg[2], arg[3], atoi(arg[4]),
-                  arg[5], dexopt_flags, parse_null(arg[7]), (atoi(arg[8]) == 0 ? false : true));
+    return dexopt(arg[0],                      // apk_path
+                  atoi(arg[1]),                // uid
+                  arg[2],                      // pkgname
+                  arg[3],                      // instruction_set
+                  atoi(arg[4]),                // dexopt_needed
+                  arg[5],                      // oat_dir
+                  dexopt_flags,
+                  arg[7],                      // compiler_filter
+                  parse_null(arg[8]));         // volume_uuid
+}
+
+static int do_merge_profiles(char **arg, char reply[REPLY_MAX])
+{
+    uid_t uid = static_cast<uid_t>(atoi(arg[0]));
+    const char* pkgname = arg[1];
+    if (merge_profiles(uid, pkgname)) {
+        strncpy(reply, "true", REPLY_MAX);
+    } else {
+        strncpy(reply, "false", REPLY_MAX);
+    }
+    return 0;
 }
 
 static int do_mark_boot_complete(char **arg, char reply[REPLY_MAX] ATTRIBUTE_UNUSED)
@@ -388,6 +405,7 @@ struct cmdinfo cmds[] = {
     { "rmprofiles",           1, do_rm_profiles },
     { "linkfile",             3, do_link_file },
     { "move_ab",              3, do_move_ab },
+    { "merge_profiles",       2, do_merge_profiles },
 };
 
 static int readx(int s, void *_buf, int count)
diff --git a/cmds/installd/installd_constants.h b/cmds/installd/installd_constants.h
index 8f6e928a9d..8513695ef3 100644
--- a/cmds/installd/installd_constants.h
+++ b/cmds/installd/installd_constants.h
@@ -75,12 +75,12 @@ constexpr int DEXOPT_SELF_PATCHOAT_NEEDED = 3;
  * IMPORTANT: These values are passed from Java code. Keep them in sync with
  * frameworks/base/services/core/java/com/android/server/pm/Installer.java
  ***************************************************************************/
-constexpr int DEXOPT_PUBLIC       = 1 << 1;
-constexpr int DEXOPT_SAFEMODE     = 1 << 2;
-constexpr int DEXOPT_DEBUGGABLE   = 1 << 3;
-constexpr int DEXOPT_BOOTCOMPLETE = 1 << 4;
-constexpr int DEXOPT_EXTRACTONLY  = 1 << 5;
-constexpr int DEXOPT_OTA          = 1 << 6;
+constexpr int DEXOPT_PUBLIC         = 1 << 1;
+constexpr int DEXOPT_SAFEMODE       = 1 << 2;
+constexpr int DEXOPT_DEBUGGABLE     = 1 << 3;
+constexpr int DEXOPT_BOOTCOMPLETE   = 1 << 4;
+constexpr int DEXOPT_PROFILE_GUIDED = 1 << 5;
+constexpr int DEXOPT_OTA            = 1 << 6;
 
 /* all known values for dexopt flags */
 constexpr int DEXOPT_MASK =
@@ -88,7 +88,7 @@ constexpr int DEXOPT_MASK =
     | DEXOPT_SAFEMODE
     | DEXOPT_DEBUGGABLE
     | DEXOPT_BOOTCOMPLETE
-    | DEXOPT_EXTRACTONLY
+    | DEXOPT_PROFILE_GUIDED
     | DEXOPT_OTA;
 
 #define ARRAY_SIZE(a) (sizeof(a) / sizeof(*(a)))
diff --git a/cmds/installd/otapreopt.cpp b/cmds/installd/otapreopt.cpp
index 89a4225ed2..245694a1f4 100644
--- a/cmds/installd/otapreopt.cpp
+++ b/cmds/installd/otapreopt.cpp
@@ -357,17 +357,15 @@ private:
     }
 
     int RunPreopt() {
-        /* apk_path, uid, pkgname, instruction_set, dexopt_needed, oat_dir, dexopt_flags,
-           volume_uuid, use_profiles */
-        int ret = dexopt(package_parameters_[0],
-                atoi(package_parameters_[1]),
-                package_parameters_[2],
-                package_parameters_[3],
-                atoi(package_parameters_[4]),
-                package_parameters_[5],
-                atoi(package_parameters_[6]),
-                ParseNull(package_parameters_[7]),
-                (atoi(package_parameters_[8]) == 0 ? false : true));
+        int ret = dexopt(package_parameters_[0],          // apk_path
+                atoi(package_parameters_[1]),             // uid
+                package_parameters_[2],                   // pkgname
+                package_parameters_[3],                   // instruction_set
+                atoi(package_parameters_[4]),             // dexopt_needed
+                package_parameters_[5],                   // oat_dir
+                atoi(package_parameters_[6]),             // dexopt_flags
+                package_parameters_[7],                   // compiler_filter
+                ParseNull(package_parameters_[8]));       // volume_uuid
         return ret;
     }