Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 023b2241 authored by Andreas Gampe's avatar Andreas Gampe
Browse files

Installd: Add error message to dexopt 

To give better messages to the binder call, add an out parameter.
This is only the start, threading things further into the calls
dexopt makes, and formatting according to exit codes, will be in
a follow-up CL.

Test: installd_dexopt_test
Change-Id: I0271212c5cddf3a6317d6b597e97537865723fdc
parent 1330e553

cmds/installd/InstalldNativeService.cpp

+3 −2
Original line number Diff line number Diff line
@@ -1909,10 +1909,11 @@ binder::Status InstalldNativeService::dexopt(const std::string& apkPath, int32_t 
    const char* profile_name = getCStr(profileName);

    const char* dm_path = getCStr(dexMetadataPath);

    const char* compilation_reason = getCStr(compilationReason);

    std::string error_msg;

    int res = android::installd::dexopt(apk_path, uid, pkgname, instruction_set, dexoptNeeded,

            oat_dir, dexFlags, compiler_filter, volume_uuid, class_loader_context, se_info,

            downgrade, targetSdkVersion, profile_name, dm_path, compilation_reason);

    return res ? error(res, "Failed to dexopt") : ok();

            downgrade, targetSdkVersion, profile_name, dm_path, compilation_reason, &error_msg);

    return res ? error(res, error_msg) : ok();

}



binder::Status InstalldNativeService::markBootComplete(const std::string& instructionSet) {

cmds/installd/dexopt.cpp

+25 −8
Original line number Diff line number Diff line
@@ -28,6 +28,8 @@ 
#include <sys/wait.h>

#include <unistd.h>



#include <iomanip>



#include <android-base/file.h>

#include <android-base/logging.h>

#include <android-base/properties.h>
@@ -1870,22 +1872,30 @@ static bool process_secondary_dex_dexopt(const std::string& dex_path, const char 
    return success;

}



static std::string format_dexopt_error(int retcode, const char* dex_path) {

  return StringPrintf("Dex2oat invocation for %s failed with 0x%04x", dex_path, retcode);

}



int dexopt(const char* dex_path, uid_t uid, const char* pkgname, const char* instruction_set,

        int dexopt_needed, const char* oat_dir, int dexopt_flags, const char* compiler_filter,

        const char* volume_uuid, const char* class_loader_context, const char* se_info,

        bool downgrade, int target_sdk_version, const char* profile_name,

        const char* dex_metadata_path, const char* compilation_reason) {

        const char* dex_metadata_path, const char* compilation_reason, std::string* error_msg) {

    CHECK(pkgname != nullptr);

    CHECK(pkgname[0] != 0);

    CHECK(error_msg != nullptr);

    CHECK_EQ(dexopt_flags & ~DEXOPT_MASK, 0)

        << "dexopt flags contains unknown fields: " << dexopt_flags;



    if (!validate_dex_path_size(dex_path)) {

        *error_msg = StringPrintf("Failed to validate %s", dex_path);

        return -1;

    }



    if (class_loader_context != nullptr && strlen(class_loader_context) > PKG_PATH_MAX) {

        LOG(ERROR) << "Class loader context exceeds the allowed size: " << class_loader_context;

        *error_msg = StringPrintf("Class loader context exceeds the allowed size: %s",

                                  class_loader_context);

        LOG(ERROR) << *error_msg;

        return -1;

    }
@@ -1908,6 +1918,7 @@ int dexopt(const char* dex_path, uid_t uid, const char* pkgname, const char* ins 
                return 0;  // Nothing to do, report success.

            }

        } else {

            *error_msg = "Failed processing secondary.";

            return -1;  // We had an error, logged in the process method.

        }

    } else {
@@ -1920,7 +1931,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) {

        ALOGE("installd cannot open '%s' for input during dexopt\n", dex_path);

        *error_msg = StringPrintf("installd cannot open '%s' for input during dexopt", dex_path);

        LOG(ERROR) << *error_msg;

        return -1;

    }
@@ -1929,6 +1941,7 @@ int dexopt(const char* dex_path, uid_t uid, const char* pkgname, const char* ins 
    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) {

        *error_msg = "Could not open out oat file.";

        return -1;

    }
@@ -1937,6 +1950,7 @@ int dexopt(const char* dex_path, uid_t uid, const char* pkgname, const char* ins 
    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)) {

        *error_msg = "Could not open vdex files.";

        return -1;

    }
@@ -1948,7 +1962,8 @@ int dexopt(const char* dex_path, uid_t uid, const char* pkgname, const char* ins 
    if (is_secondary_dex) {

        if (selinux_android_restorecon_pkgdir(oat_dir, se_info, uid,

                SELINUX_ANDROID_RESTORECON_RECURSE)) {

            LOG(ERROR) << "Failed to restorecon " << oat_dir;

            *error_msg = std::string("Failed to restorecon ").append(oat_dir);

            LOG(ERROR) << *error_msg;

            return -1;

        }

    }
@@ -1972,7 +1987,7 @@ int dexopt(const char* dex_path, uid_t uid, const char* pkgname, const char* ins 
        }

    }



    ALOGV("DexInv: --- BEGIN '%s' ---\n", dex_path);

    LOG(VERBOSE) << "DexInv: --- BEGIN '" << dex_path << "' ---";



    pid_t pid = fork();

    if (pid == 0) {
@@ -1981,7 +1996,7 @@ int dexopt(const char* dex_path, uid_t uid, const char* pkgname, const char* ins 


        SetDex2OatScheduling(boot_complete);

        if (flock(out_oat_fd.get(), LOCK_EX | LOCK_NB) != 0) {

            ALOGE("flock(%s) failed: %s\n", out_oat_path, strerror(errno));

            PLOG(ERROR) << "flock(" << out_oat_path << ") failed";

            _exit(67);

        }
@@ -2008,9 +2023,11 @@ int dexopt(const char* dex_path, uid_t uid, const char* pkgname, const char* ins 
    } else {

        int res = wait_child(pid);

        if (res == 0) {

            ALOGV("DexInv: --- END '%s' (success) ---\n", dex_path);

            LOG(VERBOSE) << "DexInv: --- END '" << dex_path << "' (success) ---";

        } else {

            ALOGE("DexInv: --- END '%s' --- status=0x%04x, process failed\n", dex_path, res);

            LOG(VERBOSE) << "DexInv: --- END '" << dex_path << "' --- status=0x"

                         << std::hex << std::setw(4) << res << ", process failed";

            *error_msg = format_dexopt_error(res, dex_path);

            return res;

        }

    }

cmds/installd/dexopt.h

+1 −1
Original line number Diff line number Diff line
@@ -106,7 +106,7 @@ int dexopt(const char *apk_path, uid_t uid, const char *pkgName, const char *ins 
        int dexopt_needed, const char* oat_dir, int dexopt_flags, const char* compiler_filter,

        const char* volume_uuid, const char* class_loader_context, const char* se_info,

        bool downgrade, int target_sdk_version, const char* profile_name,

        const char* dexMetadataPath, const char* compilation_reason);

        const char* dexMetadataPath, const char* compilation_reason, std::string* error_msg);



bool calculate_oat_file_path_default(char path[PKG_PATH_MAX], const char *oat_dir,

        const char *apk_path, const char *instruction_set);

cmds/installd/otapreopt.cpp

+3 −1
Original line number Diff line number Diff line
@@ -568,6 +568,7 @@ private: 
    // Run dexopt with the parameters of parameters_.

    // TODO(calin): embed the profile name in the parameters.

    int Dexopt() {

        std::string dummy;

        return dexopt(parameters_.apk_path,

                      parameters_.uid,

                      parameters_.pkgName,
@@ -583,7 +584,8 @@ private: 
                      parameters_.target_sdk_version,

                      parameters_.profile_name,

                      parameters_.dex_metadata_path,

                      parameters_.compilation_reason);

                      parameters_.compilation_reason,

                      &dummy);

    }



    int RunPreopt() {

cmds/installd/tests/installd_dexopt_test.cpp

+50 −11
Original line number Diff line number Diff line
@@ -249,7 +249,7 @@ protected: 


    void CompileSecondaryDex(const std::string& path, int32_t dex_storage_flag,

            bool should_binder_call_succeed, bool should_dex_be_compiled = true,

            int32_t uid = -1) {

            /*out */ binder::Status* binder_result = nullptr, int32_t uid = -1) {

        if (uid == -1) {

            uid = kTestAppUid;

        }
@@ -290,6 +290,9 @@ protected: 
        ASSERT_EQ(expected_access, access(odex.c_str(), R_OK));

        ASSERT_EQ(expected_access, access(vdex.c_str(), R_OK));

        ASSERT_EQ(-1, access(art.c_str(), R_OK));  // empty profiles do not generate an image.

        if (binder_result != nullptr) {

            *binder_result = result;

        }

    }



    void reconcile_secondary_dex(const std::string& path, int32_t storage_flag,
@@ -335,10 +338,18 @@ protected: 
                             const char* oat_dir,

                             int32_t uid,

                             int32_t dexopt_needed,

                             binder::Status* binder_result = nullptr,

                             const char* dm_path = nullptr,

                             bool downgrade = false) {

        return CompilePrimaryDex(

                compiler_filter, dex_flags, oat_dir, uid, dexopt_needed, dm_path, downgrade, true);

        CompilePrimaryDex(compiler_filter,

                          dex_flags,

                          oat_dir,

                          uid,

                          dexopt_needed,

                          dm_path,

                          downgrade,

                          true,

                          binder_result);

    }



    void CompilePrimaryDexFail(std::string compiler_filter,
@@ -346,10 +357,18 @@ protected: 
                               const char* oat_dir,

                               int32_t uid,

                               int32_t dexopt_needed,

                               binder::Status* binder_result = nullptr,

                               const char* dm_path = nullptr,

                               bool downgrade = false) {

        return CompilePrimaryDex(

                compiler_filter, dex_flags, oat_dir, uid, dexopt_needed, dm_path, downgrade, false);

        CompilePrimaryDex(compiler_filter,

                          dex_flags,

                          oat_dir,

                          uid,

                          dexopt_needed,

                          dm_path,

                          downgrade,

                          false,

                          binder_result);

    }



    void CompilePrimaryDex(std::string compiler_filter,
@@ -359,7 +378,8 @@ protected: 
                           int32_t dexopt_needed,

                           const char* dm_path,

                           bool downgrade,

                           bool should_binder_call_succeed) {

                           bool should_binder_call_succeed,

                           /*out */ binder::Status* binder_result) {

        std::unique_ptr<std::string> package_name_ptr(new std::string(package_name_));

        std::unique_ptr<std::string> out_path(

                oat_dir == nullptr ? nullptr : new std::string(oat_dir));
@@ -400,6 +420,9 @@ protected: 
        ASSERT_EQ(should_binder_call_succeed, result.isOk()) << result.toString8().c_str();



        if (!should_binder_call_succeed) {

            if (binder_result != nullptr) {

                *binder_result = result;

            }

            return;

        }

        // Check the access to the compiler output.
@@ -417,6 +440,9 @@ protected: 
        if (compiler_filter == "speed-profile") {

            CheckFileAccess(art, kSystemUid, uid, mode);

        }

        if (binder_result != nullptr) {

            *binder_result = result;

        }

    }



    std::string GetPrimaryDexArtifact(const char* oat_dir,
@@ -462,26 +488,34 @@ TEST_F(DexoptTest, DexoptSecondaryDe) { 
TEST_F(DexoptTest, DexoptSecondaryDoesNotExist) {

    LOG(INFO) << "DexoptSecondaryDoesNotExist";

    // If the file validates but does not exist we do not treat it as an error.

    binder::Status status;

    CompileSecondaryDex(secondary_dex_ce_ + "not.there", DEXOPT_STORAGE_CE,

        /*binder_ok*/ true,  /*compile_ok*/ false);

        /*binder_ok*/ true,  /*compile_ok*/ false, &status);

    EXPECT_STREQ(status.toString8().c_str(), "No error");

}



TEST_F(DexoptTest, DexoptSecondaryStorageValidationError) {

    LOG(INFO) << "DexoptSecondaryStorageValidationError";

    binder::Status status;

    CompileSecondaryDex(secondary_dex_ce_, DEXOPT_STORAGE_DE,

        /*binder_ok*/ false,  /*compile_ok*/ false);

        /*binder_ok*/ false,  /*compile_ok*/ false, &status);

    EXPECT_STREQ(status.toString8().c_str(), "Status(-8): '-1: Failed processing secondary.'");

}



TEST_F(DexoptTest, DexoptSecondaryAppOwnershipValidationError) {

    LOG(INFO) << "DexoptSecondaryAppOwnershipValidationError";

    binder::Status status;

    CompileSecondaryDex("/data/data/random.app/secondary.jar", DEXOPT_STORAGE_CE,

        /*binder_ok*/ false,  /*compile_ok*/ false);

        /*binder_ok*/ false,  /*compile_ok*/ false, &status);

    EXPECT_STREQ(status.toString8().c_str(), "Status(-8): '-1: Failed processing secondary.'");

}



TEST_F(DexoptTest, DexoptSecondaryAcessViaDifferentUidError) {

    LOG(INFO) << "DexoptSecondaryAcessViaDifferentUidError";

    binder::Status status;

    CompileSecondaryDex(secondary_dex_ce_, DEXOPT_STORAGE_CE,

        /*binder_ok*/ false,  /*compile_ok*/ false, kSystemUid);

        /*binder_ok*/ false,  /*compile_ok*/ false, &status, kSystemUid);

    EXPECT_STREQ(status.toString8().c_str(), "Status(-8): '-1: Failed processing secondary.'");

}



TEST_F(DexoptTest, DexoptPrimaryPublic) {
@@ -495,11 +529,16 @@ TEST_F(DexoptTest, DexoptPrimaryPublic) { 


TEST_F(DexoptTest, DexoptPrimaryFailedInvalidFilter) {

    LOG(INFO) << "DexoptPrimaryFailedInvalidFilter";

    binder::Status status;

    CompilePrimaryDexFail("awesome-filter",

                          DEXOPT_IDLE_BACKGROUND_JOB | DEXOPT_PUBLIC,

                          app_oat_dir_.c_str(),

                          kTestAppGid,

                          DEX2OAT_FROM_SCRATCH);

                          DEX2OAT_FROM_SCRATCH,

                          &status);

    EXPECT_STREQ(status.toString8().c_str(),

                 "Status(-8): \'256: Dex2oat invocation for "

                 "/data/app/com.installd.test.dexopt/base.jar failed with 0x0100\'");

}



TEST_F(DexoptTest, DexoptPrimaryProfileNonPublic) {