Merge "Installd: Add error message to dexopt"
diff --git a/cmds/installd/InstalldNativeService.cpp b/cmds/installd/InstalldNativeService.cpp
index 68acc69..1d8b52c 100644
--- a/cmds/installd/InstalldNativeService.cpp
+++ b/cmds/installd/InstalldNativeService.cpp
@@ -1909,10 +1909,11 @@
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) {
diff --git a/cmds/installd/dexopt.cpp b/cmds/installd/dexopt.cpp
index 02a6063..57c64c6 100644
--- a/cmds/installd/dexopt.cpp
+++ b/cmds/installd/dexopt.cpp
@@ -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 @@
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 @@
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 @@
// 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 @@
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 @@
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 @@
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 @@
}
}
- ALOGV("DexInv: --- BEGIN '%s' ---\n", dex_path);
+ LOG(VERBOSE) << "DexInv: --- BEGIN '" << dex_path << "' ---";
pid_t pid = fork();
if (pid == 0) {
@@ -1981,7 +1996,7 @@
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 @@
} 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;
}
}
diff --git a/cmds/installd/dexopt.h b/cmds/installd/dexopt.h
index 62f9467..bb6fab3 100644
--- a/cmds/installd/dexopt.h
+++ b/cmds/installd/dexopt.h
@@ -106,7 +106,7 @@
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);
diff --git a/cmds/installd/otapreopt.cpp b/cmds/installd/otapreopt.cpp
index 20e7e49..0fcb7d3 100644
--- a/cmds/installd/otapreopt.cpp
+++ b/cmds/installd/otapreopt.cpp
@@ -568,6 +568,7 @@
// 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 @@
parameters_.target_sdk_version,
parameters_.profile_name,
parameters_.dex_metadata_path,
- parameters_.compilation_reason);
+ parameters_.compilation_reason,
+ &dummy);
}
int RunPreopt() {
diff --git a/cmds/installd/tests/installd_dexopt_test.cpp b/cmds/installd/tests/installd_dexopt_test.cpp
index d640558..7c75713 100644
--- a/cmds/installd/tests/installd_dexopt_test.cpp
+++ b/cmds/installd/tests/installd_dexopt_test.cpp
@@ -249,7 +249,7 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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, 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, 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) {
