Report detailed bad profile error from artd.
Bug: 278080573
Test: atest art_standalone_artd_tests
Test: atest ArtServiceTests
Change-Id: Iab558a03378f7b1121b3e28fb67b26436a3cc45a
Merged-In: Iab558a03378f7b1121b3e28fb67b26436a3cc45a
diff --git a/artd/artd.cc b/artd/artd.cc
index 502942e..a4e0f32 100644
--- a/artd/artd.cc
+++ b/artd/artd.cc
@@ -56,11 +56,13 @@
#include "android/binder_manager.h"
#include "android/binder_process.h"
#include "base/compiler_filter.h"
+#include "base/file_magic.h"
#include "base/file_utils.h"
#include "base/globals.h"
#include "base/logging.h"
#include "base/macros.h"
#include "base/os.h"
+#include "base/zip_archive.h"
#include "cmdline_types.h"
#include "exec_utils.h"
#include "file_utils.h"
@@ -80,6 +82,7 @@
using ::aidl::com::android::server::art::ArtdDexoptResult;
using ::aidl::com::android::server::art::ArtifactsPath;
+using ::aidl::com::android::server::art::CopyAndRewriteProfileResult;
using ::aidl::com::android::server::art::DexMetadataPath;
using ::aidl::com::android::server::art::DexoptOptions;
using ::aidl::com::android::server::art::DexoptTrigger;
@@ -345,6 +348,52 @@
return {};
}
+CopyAndRewriteProfileResult AnalyzeCopyAndRewriteProfileFailure(
+ File* src, ProfmanResult::CopyAndUpdateResult result) {
+ DCHECK(result == ProfmanResult::kCopyAndUpdateNoMatch ||
+ result == ProfmanResult::kCopyAndUpdateErrorFailedToLoadProfile);
+
+ auto bad_profile = [&](std::string_view error_msg) {
+ return CopyAndRewriteProfileResult{
+ .status = CopyAndRewriteProfileResult::Status::BAD_PROFILE,
+ .errorMsg = ART_FORMAT("Failed to load profile '{}': {}", src->GetPath(), error_msg)};
+ };
+ CopyAndRewriteProfileResult no_profile{.status = CopyAndRewriteProfileResult::Status::NO_PROFILE,
+ .errorMsg = ""};
+
+ int64_t length = src->GetLength();
+ if (length < 0) {
+ return bad_profile(strerror(-length));
+ }
+ if (length == 0) {
+ return no_profile;
+ }
+
+ std::string error_msg;
+ uint32_t magic;
+ if (!ReadMagicAndReset(src->Fd(), &magic, &error_msg)) {
+ return bad_profile(error_msg);
+ }
+ if (IsZipMagic(magic)) {
+ std::unique_ptr<ZipArchive> zip_archive(
+ ZipArchive::OpenFromOwnedFd(src->Fd(), src->GetPath().c_str(), &error_msg));
+ if (zip_archive == nullptr) {
+ return bad_profile(error_msg);
+ }
+ std::unique_ptr<ZipEntry> zip_entry(zip_archive->Find("primary.prof", &error_msg));
+ if (zip_entry == nullptr || zip_entry->GetUncompressedLength() == 0) {
+ return no_profile;
+ }
+ }
+
+ if (result == ProfmanResult::kCopyAndUpdateNoMatch) {
+ return bad_profile(
+ "The profile does not match the APK (The checksums in the profile do not match the "
+ "checksums of the .dex files in the APK)");
+ }
+ return bad_profile("The profile is in the wrong format or an I/O error has occurred");
+}
+
class FdLogger {
public:
void Add(const NewFile& file) { fd_mapping_.emplace_back(file.Fd(), file.TempPath()); }
@@ -495,7 +544,7 @@
ndk::ScopedAStatus Artd::copyAndRewriteProfile(const ProfilePath& in_src,
OutputProfile* in_dst,
const std::string& in_dexFile,
- bool* _aidl_return) {
+ CopyAndRewriteProfileResult* _aidl_return) {
std::string src_path = OR_RETURN_FATAL(BuildProfileOrDmPath(in_src));
std::string dst_path = OR_RETURN_FATAL(BuildFinalProfilePath(in_dst->profilePath));
OR_RETURN_FATAL(ValidateDexPath(in_dexFile));
@@ -511,7 +560,7 @@
Result<std::unique_ptr<File>> src = OpenFileForReading(src_path);
if (!src.ok()) {
if (src.error().code() == ENOENT) {
- *_aidl_return = false;
+ _aidl_return->status = CopyAndRewriteProfileResult::Status::NO_PROFILE;
return ScopedAStatus::ok();
}
return NonFatal(
@@ -541,8 +590,10 @@
LOG(INFO) << ART_FORMAT("profman returned code {}", result.value());
- if (result.value() == ProfmanResult::kCopyAndUpdateNoMatch) {
- *_aidl_return = false;
+ if (result.value() == ProfmanResult::kCopyAndUpdateNoMatch ||
+ result.value() == ProfmanResult::kCopyAndUpdateErrorFailedToLoadProfile) {
+ *_aidl_return = AnalyzeCopyAndRewriteProfileFailure(
+ src->get(), static_cast<ProfmanResult::CopyAndUpdateResult>(result.value()));
return ScopedAStatus::ok();
}
@@ -551,7 +602,7 @@
}
OR_RETURN_NON_FATAL(dst->Keep());
- *_aidl_return = true;
+ _aidl_return->status = CopyAndRewriteProfileResult::Status::SUCCESS;
in_dst->profilePath.id = dst->TempId();
in_dst->profilePath.tmpPath = dst->TempPath();
return ScopedAStatus::ok();
diff --git a/artd/artd.h b/artd/artd.h
index a4012c6..774f11a 100644
--- a/artd/artd.h
+++ b/artd/artd.h
@@ -99,7 +99,7 @@
const aidl::com::android::server::art::ProfilePath& in_src,
aidl::com::android::server::art::OutputProfile* in_dst,
const std::string& in_dexFile,
- bool* _aidl_return) override;
+ aidl::com::android::server::art::CopyAndRewriteProfileResult* _aidl_return) override;
ndk::ScopedAStatus commitTmpProfile(
const aidl::com::android::server::art::ProfilePath::TmpProfilePath& in_profile) override;
diff --git a/artd/artd_test.cc b/artd/artd_test.cc
index a581b18..ae18b1a 100644
--- a/artd/artd_test.cc
+++ b/artd/artd_test.cc
@@ -25,6 +25,7 @@
#include <chrono>
#include <condition_variable>
#include <csignal>
+#include <cstdio>
#include <filesystem>
#include <functional>
#include <memory>
@@ -61,6 +62,7 @@
#include "profman/profman_result.h"
#include "testing.h"
#include "tools/system_properties.h"
+#include "ziparchive/zip_writer.h"
namespace art {
namespace artd {
@@ -69,6 +71,7 @@
using ::aidl::com::android::server::art::ArtConstants;
using ::aidl::com::android::server::art::ArtdDexoptResult;
using ::aidl::com::android::server::art::ArtifactsPath;
+using ::aidl::com::android::server::art::CopyAndRewriteProfileResult;
using ::aidl::com::android::server::art::DexMetadataPath;
using ::aidl::com::android::server::art::DexoptOptions;
using ::aidl::com::android::server::art::FileVisibility;
@@ -373,15 +376,15 @@
clc_2_ = GetTestDexFileName("Nested");
class_loader_context_ = ART_FORMAT("PCL[{}:{}]", clc_1_, clc_2_);
compiler_filter_ = "speed";
- TmpProfilePath tmp_profile_path{
- .finalPath =
- PrimaryRefProfilePath{.packageName = "com.android.foo", .profileName = "primary"},
- .id = "12345"};
- profile_path_ = tmp_profile_path;
+ tmp_profile_path_ =
+ TmpProfilePath{.finalPath = PrimaryRefProfilePath{.packageName = "com.android.foo",
+ .profileName = "primary"},
+ .id = "12345"};
+ profile_path_ = tmp_profile_path_;
vdex_path_ = artifacts_path_;
dm_path_ = DexMetadataPath{.dexPath = dex_file_};
std::filesystem::create_directories(
- std::filesystem::path(OR_FATAL(BuildFinalProfilePath(tmp_profile_path))).parent_path());
+ std::filesystem::path(OR_FATAL(BuildFinalProfilePath(tmp_profile_path_))).parent_path());
}
void TearDown() override {
@@ -425,12 +428,48 @@
}
}
+ // Runs `copyAndRewriteProfile` with `tmp_profile_path_` and `dex_file_`.
+ template <bool kExpectOk = true>
+ Result<std::pair<std::conditional_t<kExpectOk, CopyAndRewriteProfileResult, ndk::ScopedAStatus>,
+ OutputProfile>>
+ RunCopyAndRewriteProfile() {
+ OutputProfile dst{.profilePath = tmp_profile_path_,
+ .fsPermission = FsPermission{.uid = -1, .gid = -1}};
+ dst.profilePath.id = "";
+ dst.profilePath.tmpPath = "";
+
+ CopyAndRewriteProfileResult result;
+ ndk::ScopedAStatus status =
+ artd_->copyAndRewriteProfile(tmp_profile_path_, &dst, dex_file_, &result);
+ if constexpr (kExpectOk) {
+ if (!status.isOk()) {
+ return Error() << status.getMessage();
+ }
+ return std::make_pair(std::move(result), std::move(dst));
+ } else {
+ return std::make_pair(std::move(status), std::move(dst));
+ }
+ }
+
void CreateFile(const std::string& filename, const std::string& content = "") {
std::filesystem::path path(filename);
std::filesystem::create_directories(path.parent_path());
ASSERT_TRUE(WriteStringToFile(content, filename));
}
+ void CreateZipWithSingleEntry(const std::string& filename,
+ const std::string& entry_name,
+ const std::string& content = "") {
+ std::unique_ptr<File> file(OS::CreateEmptyFileWriteOnly(filename.c_str()));
+ ASSERT_NE(file, nullptr);
+ file->MarkUnchecked(); // `writer.Finish()` flushes the file and the destructor closes it.
+ ZipWriter writer(fdopen(file->Fd(), "wb"));
+ ASSERT_EQ(writer.StartEntry(entry_name, /*flags=*/0), 0);
+ ASSERT_EQ(writer.WriteBytes(content.c_str(), content.size()), 0);
+ ASSERT_EQ(writer.FinishEntry(), 0);
+ ASSERT_EQ(writer.Finish(), 0);
+ }
+
std::shared_ptr<Artd> artd_;
std::unique_ptr<ScratchDir> scratch_dir_;
std::string scratch_path_;
@@ -460,6 +499,7 @@
PriorityClass priority_class_ = PriorityClass::BACKGROUND;
DexoptOptions dexopt_options_;
std::optional<ProfilePath> profile_path_;
+ TmpProfilePath tmp_profile_path_;
bool dex_file_other_readable_ = true;
bool profile_other_readable_ = true;
@@ -1309,13 +1349,9 @@
EXPECT_THAT(status.getMessage(), HasSubstr("profman returned an unexpected code: 100"));
}
-TEST_F(ArtdTest, copyAndRewriteProfile) {
- const TmpProfilePath& src = profile_path_->get<ProfilePath::tmpProfilePath>();
- std::string src_file = OR_FATAL(BuildTmpProfilePath(src));
- CreateFile(src_file, "abc");
- OutputProfile dst{.profilePath = src, .fsPermission = FsPermission{.uid = -1, .gid = -1}};
- dst.profilePath.id = "";
- dst.profilePath.tmpPath = "";
+TEST_F(ArtdTest, copyAndRewriteProfileSuccess) {
+ std::string src_file = OR_FATAL(BuildTmpProfilePath(tmp_profile_path_));
+ CreateFile(src_file, "valid_profile");
CreateFile(dex_file_);
@@ -1335,64 +1371,160 @@
.WillOnce(DoAll(WithArg<0>(WriteToFdFlag("--reference-profile-file-fd=", "def")),
Return(ProfmanResult::kCopyAndUpdateSuccess)));
- bool result;
- EXPECT_TRUE(artd_->copyAndRewriteProfile(src, &dst, dex_file_, &result).isOk());
- EXPECT_TRUE(result);
+ auto [result, dst] = OR_FAIL(RunCopyAndRewriteProfile());
+
+ EXPECT_EQ(result.status, CopyAndRewriteProfileResult::Status::SUCCESS);
EXPECT_THAT(dst.profilePath.id, Not(IsEmpty()));
std::string real_path = OR_FATAL(BuildTmpProfilePath(dst.profilePath));
EXPECT_EQ(dst.profilePath.tmpPath, real_path);
CheckContent(real_path, "def");
}
-TEST_F(ArtdTest, copyAndRewriteProfileFalse) {
- const TmpProfilePath& src = profile_path_->get<ProfilePath::tmpProfilePath>();
- std::string src_file = OR_FATAL(BuildTmpProfilePath(src));
- CreateFile(src_file, "abc");
- OutputProfile dst{.profilePath = src, .fsPermission = FsPermission{.uid = -1, .gid = -1}};
- dst.profilePath.id = "";
- dst.profilePath.tmpPath = "";
+// The input is a plain profile file in the wrong format.
+TEST_F(ArtdTest, copyAndRewriteProfileBadProfileWrongFormat) {
+ std::string src_file = OR_FATAL(BuildTmpProfilePath(tmp_profile_path_));
+ CreateFile(src_file, "wrong_format");
+
+ CreateFile(dex_file_);
+
+ EXPECT_CALL(*mock_exec_utils_, DoExecAndReturnCode(_, _, _))
+ .WillOnce(Return(ProfmanResult::kCopyAndUpdateErrorFailedToLoadProfile));
+
+ auto [result, dst] = OR_FAIL(RunCopyAndRewriteProfile());
+
+ EXPECT_EQ(result.status, CopyAndRewriteProfileResult::Status::BAD_PROFILE);
+ EXPECT_THAT(result.errorMsg,
+ HasSubstr("The profile is in the wrong format or an I/O error has occurred"));
+ EXPECT_THAT(dst.profilePath.id, IsEmpty());
+ EXPECT_THAT(dst.profilePath.tmpPath, IsEmpty());
+}
+
+// The input is a plain profile file that doesn't match the APK.
+TEST_F(ArtdTest, copyAndRewriteProfileBadProfileNoMatch) {
+ std::string src_file = OR_FATAL(BuildTmpProfilePath(tmp_profile_path_));
+ CreateFile(src_file, "no_match");
CreateFile(dex_file_);
EXPECT_CALL(*mock_exec_utils_, DoExecAndReturnCode(_, _, _))
.WillOnce(Return(ProfmanResult::kCopyAndUpdateNoMatch));
- bool result;
- EXPECT_TRUE(artd_->copyAndRewriteProfile(src, &dst, dex_file_, &result).isOk());
- EXPECT_FALSE(result);
+ auto [result, dst] = OR_FAIL(RunCopyAndRewriteProfile());
+
+ EXPECT_EQ(result.status, CopyAndRewriteProfileResult::Status::BAD_PROFILE);
+ EXPECT_THAT(result.errorMsg, HasSubstr("The profile does not match the APK"));
EXPECT_THAT(dst.profilePath.id, IsEmpty());
EXPECT_THAT(dst.profilePath.tmpPath, IsEmpty());
}
-TEST_F(ArtdTest, copyAndRewriteProfileNotFound) {
+// The input is a plain profile file that is empty.
+TEST_F(ArtdTest, copyAndRewriteProfileNoProfileEmpty) {
+ std::string src_file = OR_FATAL(BuildTmpProfilePath(tmp_profile_path_));
+ CreateFile(src_file, "");
+
CreateFile(dex_file_);
- const TmpProfilePath& src = profile_path_->get<ProfilePath::tmpProfilePath>();
- OutputProfile dst{.profilePath = src, .fsPermission = FsPermission{.uid = -1, .gid = -1}};
- dst.profilePath.id = "";
- dst.profilePath.tmpPath = "";
+ EXPECT_CALL(*mock_exec_utils_, DoExecAndReturnCode(_, _, _))
+ .WillOnce(Return(ProfmanResult::kCopyAndUpdateNoMatch));
- bool result;
- EXPECT_TRUE(artd_->copyAndRewriteProfile(src, &dst, dex_file_, &result).isOk());
- EXPECT_FALSE(result);
+ auto [result, dst] = OR_FAIL(RunCopyAndRewriteProfile());
+
+ EXPECT_EQ(result.status, CopyAndRewriteProfileResult::Status::NO_PROFILE);
EXPECT_THAT(dst.profilePath.id, IsEmpty());
EXPECT_THAT(dst.profilePath.tmpPath, IsEmpty());
}
-TEST_F(ArtdTest, copyAndRewriteProfileFailed) {
- const TmpProfilePath& src = profile_path_->get<ProfilePath::tmpProfilePath>();
- std::string src_file = OR_FATAL(BuildTmpProfilePath(src));
- CreateFile(src_file, "abc");
- OutputProfile dst{.profilePath = src, .fsPermission = FsPermission{.uid = -1, .gid = -1}};
- dst.profilePath.id = "";
- dst.profilePath.tmpPath = "";
+// The input does not exist.
+TEST_F(ArtdTest, copyAndRewriteProfileNoProfileNoFile) {
+ CreateFile(dex_file_);
+
+ auto [result, dst] = OR_FAIL(RunCopyAndRewriteProfile());
+
+ EXPECT_EQ(result.status, CopyAndRewriteProfileResult::Status::NO_PROFILE);
+ EXPECT_THAT(dst.profilePath.id, IsEmpty());
+ EXPECT_THAT(dst.profilePath.tmpPath, IsEmpty());
+}
+
+// The input is a dm file with a profile entry in the wrong format.
+TEST_F(ArtdTest, copyAndRewriteProfileNoProfileDmWrongFormat) {
+ std::string src_file = OR_FATAL(BuildTmpProfilePath(tmp_profile_path_));
+ CreateZipWithSingleEntry(src_file, "primary.prof", "wrong_format");
+
+ CreateFile(dex_file_);
+
+ EXPECT_CALL(*mock_exec_utils_, DoExecAndReturnCode(_, _, _))
+ .WillOnce(Return(ProfmanResult::kCopyAndUpdateErrorFailedToLoadProfile));
+
+ auto [result, dst] = OR_FAIL(RunCopyAndRewriteProfile());
+
+ EXPECT_EQ(result.status, CopyAndRewriteProfileResult::Status::BAD_PROFILE);
+ EXPECT_THAT(result.errorMsg,
+ HasSubstr("The profile is in the wrong format or an I/O error has occurred"));
+ EXPECT_THAT(dst.profilePath.id, IsEmpty());
+ EXPECT_THAT(dst.profilePath.tmpPath, IsEmpty());
+}
+
+// The input is a dm file with a profile entry that doesn't match the APK.
+TEST_F(ArtdTest, copyAndRewriteProfileNoProfileDmNoMatch) {
+ std::string src_file = OR_FATAL(BuildTmpProfilePath(tmp_profile_path_));
+ CreateZipWithSingleEntry(src_file, "primary.prof", "no_match");
+
+ CreateFile(dex_file_);
+
+ EXPECT_CALL(*mock_exec_utils_, DoExecAndReturnCode(_, _, _))
+ .WillOnce(Return(ProfmanResult::kCopyAndUpdateNoMatch));
+
+ auto [result, dst] = OR_FAIL(RunCopyAndRewriteProfile());
+
+ EXPECT_EQ(result.status, CopyAndRewriteProfileResult::Status::BAD_PROFILE);
+ EXPECT_THAT(result.errorMsg, HasSubstr("The profile does not match the APK"));
+ EXPECT_THAT(dst.profilePath.id, IsEmpty());
+ EXPECT_THAT(dst.profilePath.tmpPath, IsEmpty());
+}
+
+// The input is a dm file with a profile entry that is empty.
+TEST_F(ArtdTest, copyAndRewriteProfileNoProfileDmEmpty) {
+ std::string src_file = OR_FATAL(BuildTmpProfilePath(tmp_profile_path_));
+ CreateZipWithSingleEntry(src_file, "primary.prof");
+
+ CreateFile(dex_file_);
+
+ EXPECT_CALL(*mock_exec_utils_, DoExecAndReturnCode(_, _, _))
+ .WillOnce(Return(ProfmanResult::kCopyAndUpdateNoMatch));
+
+ auto [result, dst] = OR_FAIL(RunCopyAndRewriteProfile());
+
+ EXPECT_EQ(result.status, CopyAndRewriteProfileResult::Status::NO_PROFILE);
+ EXPECT_THAT(dst.profilePath.id, IsEmpty());
+ EXPECT_THAT(dst.profilePath.tmpPath, IsEmpty());
+}
+
+// The input is a dm file without a profile entry.
+TEST_F(ArtdTest, copyAndRewriteProfileNoProfileDmNoEntry) {
+ std::string src_file = OR_FATAL(BuildTmpProfilePath(tmp_profile_path_));
+ CreateZipWithSingleEntry(src_file, "primary.vdex");
+
+ CreateFile(dex_file_);
+
+ EXPECT_CALL(*mock_exec_utils_, DoExecAndReturnCode(_, _, _))
+ .WillOnce(Return(ProfmanResult::kCopyAndUpdateNoMatch));
+
+ auto [result, dst] = OR_FAIL(RunCopyAndRewriteProfile());
+
+ EXPECT_EQ(result.status, CopyAndRewriteProfileResult::Status::NO_PROFILE);
+ EXPECT_THAT(dst.profilePath.id, IsEmpty());
+ EXPECT_THAT(dst.profilePath.tmpPath, IsEmpty());
+}
+
+TEST_F(ArtdTest, copyAndRewriteProfileException) {
+ std::string src_file = OR_FATAL(BuildTmpProfilePath(tmp_profile_path_));
+ CreateFile(src_file, "valid_profile");
CreateFile(dex_file_);
EXPECT_CALL(*mock_exec_utils_, DoExecAndReturnCode(_, _, _)).WillOnce(Return(100));
- bool result;
- ndk::ScopedAStatus status = artd_->copyAndRewriteProfile(src, &dst, dex_file_, &result);
+ auto [status, dst] = OR_FAIL(RunCopyAndRewriteProfile</*kExpectOk=*/false>());
EXPECT_FALSE(status.isOk());
EXPECT_EQ(status.getExceptionCode(), EX_SERVICE_SPECIFIC);
@@ -1402,19 +1534,17 @@
}
TEST_F(ArtdTest, commitTmpProfile) {
- const TmpProfilePath& tmp_profile_path = profile_path_->get<ProfilePath::tmpProfilePath>();
- std::string tmp_profile_file = OR_FATAL(BuildTmpProfilePath(tmp_profile_path));
+ std::string tmp_profile_file = OR_FATAL(BuildTmpProfilePath(tmp_profile_path_));
CreateFile(tmp_profile_file);
- EXPECT_TRUE(artd_->commitTmpProfile(tmp_profile_path).isOk());
+ EXPECT_TRUE(artd_->commitTmpProfile(tmp_profile_path_).isOk());
EXPECT_FALSE(std::filesystem::exists(tmp_profile_file));
- EXPECT_TRUE(std::filesystem::exists(OR_FATAL(BuildFinalProfilePath(tmp_profile_path))));
+ EXPECT_TRUE(std::filesystem::exists(OR_FATAL(BuildFinalProfilePath(tmp_profile_path_))));
}
TEST_F(ArtdTest, commitTmpProfileFailed) {
- const TmpProfilePath& tmp_profile_path = profile_path_->get<ProfilePath::tmpProfilePath>();
- ndk::ScopedAStatus status = artd_->commitTmpProfile(tmp_profile_path);
+ ndk::ScopedAStatus status = artd_->commitTmpProfile(tmp_profile_path_);
EXPECT_FALSE(status.isOk());
EXPECT_EQ(status.getExceptionCode(), EX_SERVICE_SPECIFIC);
@@ -1422,7 +1552,7 @@
status.getMessage(),
ContainsRegex(R"re(Failed to move .*primary\.prof\.12345\.tmp.* to .*primary\.prof)re"));
- EXPECT_FALSE(std::filesystem::exists(OR_FATAL(BuildFinalProfilePath(tmp_profile_path))));
+ EXPECT_FALSE(std::filesystem::exists(OR_FATAL(BuildFinalProfilePath(tmp_profile_path_))));
}
TEST_F(ArtdTest, deleteProfile) {
@@ -1589,7 +1719,7 @@
}
TEST_F(ArtdTest, mergeProfiles) {
- const TmpProfilePath& reference_profile_path = profile_path_->get<ProfilePath::tmpProfilePath>();
+ const TmpProfilePath& reference_profile_path = tmp_profile_path_;
std::string reference_profile_file = OR_FATAL(BuildTmpProfilePath(reference_profile_path));
CreateFile(reference_profile_file, "abc");
@@ -1655,7 +1785,7 @@
std::string profile_0_file = OR_FATAL(BuildPrimaryCurProfilePath(profile_0_path));
CreateFile(profile_0_file, "def");
- OutputProfile output_profile{.profilePath = profile_path_->get<ProfilePath::tmpProfilePath>(),
+ OutputProfile output_profile{.profilePath = tmp_profile_path_,
.fsPermission = FsPermission{.uid = -1, .gid = -1}};
output_profile.profilePath.id = "";
output_profile.profilePath.tmpPath = "";
@@ -1691,7 +1821,7 @@
}
TEST_F(ArtdTest, mergeProfilesProfilesDontExist) {
- const TmpProfilePath& reference_profile_path = profile_path_->get<ProfilePath::tmpProfilePath>();
+ const TmpProfilePath& reference_profile_path = tmp_profile_path_;
std::string reference_profile_file = OR_FATAL(BuildTmpProfilePath(reference_profile_path));
CreateFile(reference_profile_file, "abc");
@@ -1734,7 +1864,7 @@
std::string profile_0_file = OR_FATAL(BuildPrimaryCurProfilePath(profile_0_path));
CreateFile(profile_0_file, "def");
- OutputProfile output_profile{.profilePath = profile_path_->get<ProfilePath::tmpProfilePath>(),
+ OutputProfile output_profile{.profilePath = tmp_profile_path_,
.fsPermission = FsPermission{.uid = -1, .gid = -1}};
output_profile.profilePath.id = "";
output_profile.profilePath.tmpPath = "";
@@ -1769,7 +1899,7 @@
std::string profile_0_file = OR_FATAL(BuildPrimaryCurProfilePath(profile_0_path));
CreateFile(profile_0_file, "def");
- OutputProfile output_profile{.profilePath = profile_path_->get<ProfilePath::tmpProfilePath>(),
+ OutputProfile output_profile{.profilePath = tmp_profile_path_,
.fsPermission = FsPermission{.uid = -1, .gid = -1}};
output_profile.profilePath.id = "";
output_profile.profilePath.tmpPath = "";
@@ -1808,7 +1938,7 @@
std::string profile_0_file = OR_FATAL(BuildPrimaryCurProfilePath(profile_0_path));
CreateFile(profile_0_file, "def");
- OutputProfile output_profile{.profilePath = profile_path_->get<ProfilePath::tmpProfilePath>(),
+ OutputProfile output_profile{.profilePath = tmp_profile_path_,
.fsPermission = FsPermission{.uid = -1, .gid = -1}};
output_profile.profilePath.id = "";
output_profile.profilePath.tmpPath = "";
diff --git a/artd/binder/com/android/server/art/CopyAndRewriteProfileResult.aidl b/artd/binder/com/android/server/art/CopyAndRewriteProfileResult.aidl
new file mode 100644
index 0000000..37b7a9f
--- /dev/null
+++ b/artd/binder/com/android/server/art/CopyAndRewriteProfileResult.aidl
@@ -0,0 +1,39 @@
+/*
+ * Copyright (C) 2023 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.
+ */
+
+package com.android.server.art;
+
+/**
+ * The result of {@code IArtd.copyAndRewriteProfileResult}.
+ *
+ * @hide
+ */
+parcelable CopyAndRewriteProfileResult {
+ /** The status code. */
+ Status status;
+ /** The error message, if `status` is `BAD_PROFILE`. */
+ @utf8InCpp String errorMsg;
+
+ @Backing(type="int")
+ enum Status {
+ /** The operation succeeded. */
+ SUCCESS = 0,
+ /** The input does not exist or is empty. This is not considered as an error. */
+ NO_PROFILE = 1,
+ /** The input is a bad profile. */
+ BAD_PROFILE = 2,
+ }
+}
diff --git a/artd/binder/com/android/server/art/IArtd.aidl b/artd/binder/com/android/server/art/IArtd.aidl
index ec57bd4..3b55297 100644
--- a/artd/binder/com/android/server/art/IArtd.aidl
+++ b/artd/binder/com/android/server/art/IArtd.aidl
@@ -46,13 +46,14 @@
@utf8InCpp String dexFile);
/**
- * Copies the profile and rewrites it for the given dex file. Returns true and fills
+ * Copies the profile and rewrites it for the given dex file. Returns `SUCCESS` and fills
* `dst.profilePath.id` if the operation succeeds and `src` exists and contains entries that
* match the given dex file.
*
- * Throws fatal and non-fatal errors.
+ * Throws fatal and non-fatal errors, except if the input is a bad profile.
*/
- boolean copyAndRewriteProfile(in com.android.server.art.ProfilePath src,
+ com.android.server.art.CopyAndRewriteProfileResult copyAndRewriteProfile(
+ in com.android.server.art.ProfilePath src,
inout com.android.server.art.OutputProfile dst, @utf8InCpp String dexFile);
/**
diff --git a/artd/testing.h b/artd/testing.h
index df01a9a..8bdbe89 100644
--- a/artd/testing.h
+++ b/artd/testing.h
@@ -21,7 +21,7 @@
// mismatch. This is only to be used in a gMock matcher.
#define OR_MISMATCH(expr) \
({ \
- decltype(expr)&& tmp__ = (expr); \
+ auto&& tmp__ = (expr); \
if (!tmp__.ok()) { \
*result_listener << tmp__.error().message(); \
return false; \
@@ -32,7 +32,7 @@
// Returns the value of the given `android::base::Result`, or fails the GoogleTest.
#define OR_FAIL(expr) \
({ \
- decltype(expr)&& tmp__ = (expr); \
+ auto&& tmp__ = (expr); \
ASSERT_TRUE(tmp__.ok()) << tmp__.error().message(); \
std::move(tmp__).value(); \
})
diff --git a/libartservice/service/java/com/android/server/art/Utils.java b/libartservice/service/java/com/android/server/art/Utils.java
index 6d48c45..273d8dd 100644
--- a/libartservice/service/java/com/android/server/art/Utils.java
+++ b/libartservice/service/java/com/android/server/art/Utils.java
@@ -412,7 +412,10 @@
// build time and is correctly set in the profile header. However, the APK can also
// be an installed one, in which case partners may place a profile file next to the
// APK at install time. Rewriting the profile in the latter case is necessary.
- if (artd.copyAndRewriteProfile(profile, output, dexPath)) {
+ // TODO(b/278080573): Make use of the detailed result.
+ CopyAndRewriteProfileResult result =
+ artd.copyAndRewriteProfile(profile, output, dexPath);
+ if (result.status == CopyAndRewriteProfileResult.Status.SUCCESS) {
return ProfilePath.tmpProfilePath(output.profilePath);
}
} catch (ServiceSpecificException e) {
diff --git a/libartservice/service/javatests/com/android/server/art/ArtManagerLocalTest.java b/libartservice/service/javatests/com/android/server/art/ArtManagerLocalTest.java
index 747e516..983bc9c 100644
--- a/libartservice/service/javatests/com/android/server/art/ArtManagerLocalTest.java
+++ b/libartservice/service/javatests/com/android/server/art/ArtManagerLocalTest.java
@@ -228,7 +228,9 @@
// By default, none of the profiles are usable.
lenient().when(mArtd.isProfileUsable(any(), any())).thenReturn(false);
- lenient().when(mArtd.copyAndRewriteProfile(any(), any(), any())).thenReturn(false);
+ lenient()
+ .when(mArtd.copyAndRewriteProfile(any(), any(), any()))
+ .thenReturn(TestingUtils.createCopyAndRewriteProfileNoProfile());
mArtManagerLocal = new ArtManagerLocal(mInjector);
}
@@ -743,7 +745,7 @@
.thenAnswer(invocation -> {
var output = invocation.<OutputProfile>getArgument(1);
output.profilePath.tmpPath = tempPathForRef;
- return true;
+ return TestingUtils.createCopyAndRewriteProfileSuccess();
});
// Verify that the reference file initialized from the DM file is used.
diff --git a/libartservice/service/javatests/com/android/server/art/PrimaryDexopterTest.java b/libartservice/service/javatests/com/android/server/art/PrimaryDexopterTest.java
index 554b8c6..1525192 100644
--- a/libartservice/service/javatests/com/android/server/art/PrimaryDexopterTest.java
+++ b/libartservice/service/javatests/com/android/server/art/PrimaryDexopterTest.java
@@ -109,7 +109,9 @@
// By default, none of the profiles are usable.
lenient().when(mArtd.isProfileUsable(any(), any())).thenReturn(false);
- lenient().when(mArtd.copyAndRewriteProfile(any(), any(), any())).thenReturn(false);
+ lenient()
+ .when(mArtd.copyAndRewriteProfile(any(), any(), any()))
+ .thenReturn(TestingUtils.createCopyAndRewriteProfileNoProfile());
// By default, no DM file exists.
lenient().when(mArtd.getDmFileVisibility(any())).thenReturn(FileVisibility.NOT_FOUND);
@@ -691,7 +693,7 @@
.when(mArtd.copyAndRewriteProfile(deepEq(profile), any(), any()))
.thenAnswer(invocation -> {
mUsedProfiles.add(invocation.<ProfilePath>getArgument(0));
- return true;
+ return TestingUtils.createCopyAndRewriteProfileSuccess();
});
}
@@ -699,6 +701,6 @@
lenient().when(mArtd.isProfileUsable(deepEq(profile), any())).thenReturn(false);
lenient()
.when(mArtd.copyAndRewriteProfile(deepEq(profile), any(), any()))
- .thenReturn(false);
+ .thenReturn(TestingUtils.createCopyAndRewriteProfileNoProfile());
}
}
diff --git a/libartservice/service/javatests/com/android/server/art/testing/TestingUtils.java b/libartservice/service/javatests/com/android/server/art/testing/TestingUtils.java
index 5ee0a57..ee55170 100644
--- a/libartservice/service/javatests/com/android/server/art/testing/TestingUtils.java
+++ b/libartservice/service/javatests/com/android/server/art/testing/TestingUtils.java
@@ -22,6 +22,8 @@
import android.annotation.Nullable;
import android.util.Log;
+import com.android.server.art.CopyAndRewriteProfileResult;
+
import com.google.common.truth.Correspondence;
import com.google.common.truth.Truth;
@@ -159,6 +161,26 @@
});
}
+ public static CopyAndRewriteProfileResult createCopyAndRewriteProfileSuccess() {
+ var result = new CopyAndRewriteProfileResult();
+ result.status = CopyAndRewriteProfileResult.Status.SUCCESS;
+ return result;
+ }
+
+ public static CopyAndRewriteProfileResult createCopyAndRewriteProfileNoProfile() {
+ var result = new CopyAndRewriteProfileResult();
+ result.status = CopyAndRewriteProfileResult.Status.NO_PROFILE;
+ return result;
+ }
+
+ public static CopyAndRewriteProfileResult createCopyAndRewriteProfileBadProfile(
+ String errorMsg) {
+ var result = new CopyAndRewriteProfileResult();
+ result.status = CopyAndRewriteProfileResult.Status.BAD_PROFILE;
+ result.errorMsg = errorMsg;
+ return result;
+ }
+
private static boolean listDeepEquals(
@NonNull List<?> a, @NonNull List<?> b, @NonNull StringBuilder errorMsg) {
if (a.size() != b.size()) {