summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author Jiakai Zhang <jiakaiz@google.com> 2023-07-14 16:11:16 +0100
committer Treehugger Robot <treehugger-gerrit@google.com> 2023-07-20 08:00:21 +0000
commitfb20a36bf38f30e53c26b42f698ee3063228b460 (patch)
treef358965740c7d46a696aeb9334b3b0cb1f0af947
parenta6507b5c18c54b77dad169b37fc3b6ec3c0bc608 (diff)
Force LoadBootImage to take APEX versions from an argument.
When a boot image is being loaded, the APEX versions on device are checked against the APEX versions stored in the boot image. Before this change, the APEX versions on device were taken from the runtime instance. However, some tests use LoadBootImage with a bootclasspath different from the one that the runtime uses, so the APEX versions calculated by the runtime shouldn't be used. The APEX version check happened to pass because of many coincidences, but it will no longer pass once we add core-icu4j and consrypt to the boot image. This CL fixes the issue with APEX versions, to prepare for the boot image change. Bug: 290583827 Test: atest ArtGtestsTargetChroot Change-Id: Ia7680ac328cf7d996b8fefd1816f1175268918c9
-rw-r--r--dex2oat/dex2oat_image_test.cc30
-rw-r--r--runtime/gc/heap.cc3
-rw-r--r--runtime/gc/space/image_space.cc52
-rw-r--r--runtime/gc/space/image_space.h48
-rw-r--r--runtime/gc/space/image_space_test.cc30
5 files changed, 84 insertions, 79 deletions
diff --git a/dex2oat/dex2oat_image_test.cc b/dex2oat/dex2oat_image_test.cc
index dae1186cf7..507cd541d2 100644
--- a/dex2oat/dex2oat_image_test.cc
+++ b/dex2oat/dex2oat_image_test.cc
@@ -421,20 +421,22 @@ TEST_F(Dex2oatImageTest, TestExtension) {
boot_image_spaces.clear();
extra_reservation = MemMap::Invalid();
ScopedObjectAccess soa(Thread::Current());
- return gc::space::ImageSpace::LoadBootImage(/*boot_class_path=*/ boot_class_path,
- /*boot_class_path_locations=*/ libcore_dex_files,
- /*boot_class_path_fds=*/ std::vector<int>(),
- /*boot_class_path_image_fds=*/ std::vector<int>(),
- /*boot_class_path_vdex_fds=*/ std::vector<int>(),
- /*boot_class_path_oat_fds=*/ std::vector<int>(),
- android::base::Split(image_location, ":"),
- kRuntimeISA,
- relocate,
- /*executable=*/ true,
- /*extra_reservation_size=*/ 0u,
- /*allow_in_memory_compilation=*/ true,
- &boot_image_spaces,
- &extra_reservation);
+ return gc::space::ImageSpace::LoadBootImage(
+ /*boot_class_path=*/boot_class_path,
+ /*boot_class_path_locations=*/libcore_dex_files,
+ /*boot_class_path_fds=*/std::vector<int>(),
+ /*boot_class_path_image_fds=*/std::vector<int>(),
+ /*boot_class_path_vdex_fds=*/std::vector<int>(),
+ /*boot_class_path_oat_fds=*/std::vector<int>(),
+ android::base::Split(image_location, ":"),
+ kRuntimeISA,
+ relocate,
+ /*executable=*/true,
+ /*extra_reservation_size=*/0u,
+ /*allow_in_memory_compilation=*/true,
+ Runtime::GetApexVersions(ArrayRef<const std::string>(libcore_dex_files)),
+ &boot_image_spaces,
+ &extra_reservation);
};
auto silent_load = [&](const std::string& image_location) {
ScopedLogSeverity quiet(LogSeverity::FATAL);
diff --git a/runtime/gc/heap.cc b/runtime/gc/heap.cc
index f19395ce12..2ce7666be0 100644
--- a/runtime/gc/heap.cc
+++ b/runtime/gc/heap.cc
@@ -500,9 +500,10 @@ Heap::Heap(size_t initial_size,
image_file_names,
image_instruction_set,
runtime->ShouldRelocate(),
- /*executable=*/ !runtime->IsAotCompiler(),
+ /*executable=*/!runtime->IsAotCompiler(),
heap_reservation_size,
runtime->AllowInMemoryCompilation(),
+ runtime->GetApexVersions(),
&boot_image_spaces,
&heap_reservation)) {
DCHECK_EQ(heap_reservation_size, heap_reservation.IsValid() ? heap_reservation.Size() : 0u);
diff --git a/runtime/gc/space/image_space.cc b/runtime/gc/space/image_space.cc
index 0352c05505..124fed7ac2 100644
--- a/runtime/gc/space/image_space.cc
+++ b/runtime/gc/space/image_space.cc
@@ -2198,6 +2198,8 @@ bool ImageSpace::BootImageLayout::LoadFromSystem(InstructionSet image_isa,
class ImageSpace::BootImageLoader {
public:
+ // Creates an instance.
+ // `apex_versions` is created from `Runtime::GetApexVersions` and must outlive this instance.
BootImageLoader(const std::vector<std::string>& boot_class_path,
const std::vector<std::string>& boot_class_path_locations,
const std::vector<int>& boot_class_path_fds,
@@ -2207,7 +2209,8 @@ class ImageSpace::BootImageLoader {
const std::vector<std::string>& image_locations,
InstructionSet image_isa,
bool relocate,
- bool executable)
+ bool executable,
+ const std::string* apex_versions)
: boot_class_path_(boot_class_path),
boot_class_path_locations_(boot_class_path_locations),
boot_class_path_fds_(boot_class_path_fds),
@@ -2218,8 +2221,8 @@ class ImageSpace::BootImageLoader {
image_isa_(image_isa),
relocate_(relocate),
executable_(executable),
- has_system_(false) {
- }
+ has_system_(false),
+ apex_versions_(apex_versions) {}
void FindImageFiles() {
BootImageLayout layout(image_locations_,
@@ -2228,7 +2231,8 @@ class ImageSpace::BootImageLoader {
boot_class_path_fds_,
boot_class_path_image_fds_,
boot_class_path_vdex_fds_,
- boot_class_path_oat_fds_);
+ boot_class_path_oat_fds_,
+ apex_versions_);
std::string image_location = layout.GetPrimaryImageLocation();
std::string system_filename;
bool found_image = FindImageFilenameImpl(image_location.c_str(),
@@ -3205,6 +3209,7 @@ class ImageSpace::BootImageLoader {
const bool relocate_;
const bool executable_;
bool has_system_;
+ const std::string* apex_versions_;
};
bool ImageSpace::BootImageLoader::LoadFromSystem(
@@ -3221,7 +3226,8 @@ bool ImageSpace::BootImageLoader::LoadFromSystem(
boot_class_path_fds_,
boot_class_path_image_fds_,
boot_class_path_vdex_fds_,
- boot_class_path_oat_fds_);
+ boot_class_path_oat_fds_,
+ apex_versions_);
if (!layout.LoadFromSystem(image_isa_, allow_in_memory_compilation, error_msg)) {
return false;
}
@@ -3254,7 +3260,8 @@ bool ImageSpace::IsBootClassPathOnDisk(InstructionSet image_isa) {
ArrayRef<const int>(runtime->GetBootClassPathFds()),
ArrayRef<const int>(runtime->GetBootClassPathImageFds()),
ArrayRef<const int>(runtime->GetBootClassPathVdexFds()),
- ArrayRef<const int>(runtime->GetBootClassPathOatFds()));
+ ArrayRef<const int>(runtime->GetBootClassPathOatFds()),
+ &runtime->GetApexVersions());
const std::string image_location = layout.GetPrimaryImageLocation();
std::unique_ptr<ImageHeader> image_header;
std::string error_msg;
@@ -3273,21 +3280,21 @@ bool ImageSpace::IsBootClassPathOnDisk(InstructionSet image_isa) {
return image_header != nullptr;
}
-bool ImageSpace::LoadBootImage(
- const std::vector<std::string>& boot_class_path,
- const std::vector<std::string>& boot_class_path_locations,
- const std::vector<int>& boot_class_path_fds,
- const std::vector<int>& boot_class_path_image_fds,
- const std::vector<int>& boot_class_path_vdex_fds,
- const std::vector<int>& boot_class_path_odex_fds,
- const std::vector<std::string>& image_locations,
- const InstructionSet image_isa,
- bool relocate,
- bool executable,
- size_t extra_reservation_size,
- bool allow_in_memory_compilation,
- /*out*/std::vector<std::unique_ptr<ImageSpace>>* boot_image_spaces,
- /*out*/MemMap* extra_reservation) {
+bool ImageSpace::LoadBootImage(const std::vector<std::string>& boot_class_path,
+ const std::vector<std::string>& boot_class_path_locations,
+ const std::vector<int>& boot_class_path_fds,
+ const std::vector<int>& boot_class_path_image_fds,
+ const std::vector<int>& boot_class_path_vdex_fds,
+ const std::vector<int>& boot_class_path_odex_fds,
+ const std::vector<std::string>& image_locations,
+ const InstructionSet image_isa,
+ bool relocate,
+ bool executable,
+ size_t extra_reservation_size,
+ bool allow_in_memory_compilation,
+ const std::string& apex_versions,
+ /*out*/ std::vector<std::unique_ptr<ImageSpace>>* boot_image_spaces,
+ /*out*/ MemMap* extra_reservation) {
ScopedTrace trace(__FUNCTION__);
DCHECK(boot_image_spaces != nullptr);
@@ -3309,7 +3316,8 @@ bool ImageSpace::LoadBootImage(
image_locations,
image_isa,
relocate,
- executable);
+ executable,
+ &apex_versions);
loader.FindImageFiles();
// Collect all the errors.
diff --git a/runtime/gc/space/image_space.h b/runtime/gc/space/image_space.h
index 1a85456961..9ab2074cc2 100644
--- a/runtime/gc/space/image_space.h
+++ b/runtime/gc/space/image_space.h
@@ -133,21 +133,22 @@ class ImageSpace : public MemMapSpace {
// extension is not found or broken compile it in memory using
// the specified profile file in the BCP component path, each
// extension is compiled only against the primary boot image.
- static bool LoadBootImage(
- const std::vector<std::string>& boot_class_path,
- const std::vector<std::string>& boot_class_path_locations,
- const std::vector<int>& boot_class_path_fds,
- const std::vector<int>& boot_class_path_image_fds,
- const std::vector<int>& boot_class_path_vdex_fds,
- const std::vector<int>& boot_class_path_oat_fds,
- const std::vector<std::string>& image_locations,
- const InstructionSet image_isa,
- bool relocate,
- bool executable,
- size_t extra_reservation_size,
- bool allow_in_memory_compilation,
- /*out*/std::vector<std::unique_ptr<ImageSpace>>* boot_image_spaces,
- /*out*/MemMap* extra_reservation) REQUIRES_SHARED(Locks::mutator_lock_);
+ static bool LoadBootImage(const std::vector<std::string>& boot_class_path,
+ const std::vector<std::string>& boot_class_path_locations,
+ const std::vector<int>& boot_class_path_fds,
+ const std::vector<int>& boot_class_path_image_fds,
+ const std::vector<int>& boot_class_path_vdex_fds,
+ const std::vector<int>& boot_class_path_oat_fds,
+ const std::vector<std::string>& image_locations,
+ const InstructionSet image_isa,
+ bool relocate,
+ bool executable,
+ size_t extra_reservation_size,
+ bool allow_in_memory_compilation,
+ const std::string& apex_versions,
+ /*out*/ std::vector<std::unique_ptr<ImageSpace>>* boot_image_spaces,
+ /*out*/ MemMap* extra_reservation)
+ REQUIRES_SHARED(Locks::mutator_lock_);
// Try to open an existing app image space for an oat file,
// using the boot image spaces from the current Runtime.
@@ -341,6 +342,8 @@ class ImageSpace : public MemMapSpace {
mutable android::base::unique_fd oat_fd;
};
+ // Creates an instance.
+ // `apex_versions` is created from `Runtime::GetApexVersions` and must outlive this instance.
BootImageLayout(ArrayRef<const std::string> image_locations,
ArrayRef<const std::string> boot_class_path,
ArrayRef<const std::string> boot_class_path_locations,
@@ -348,7 +351,7 @@ class ImageSpace : public MemMapSpace {
ArrayRef<const int> boot_class_path_image_fds,
ArrayRef<const int> boot_class_path_vdex_fds,
ArrayRef<const int> boot_class_path_oat_fds,
- const std::string* apex_versions = nullptr)
+ const std::string* apex_versions)
: image_locations_(image_locations),
boot_class_path_(boot_class_path),
boot_class_path_locations_(boot_class_path_locations),
@@ -356,7 +359,7 @@ class ImageSpace : public MemMapSpace {
boot_class_path_image_fds_(boot_class_path_image_fds),
boot_class_path_vdex_fds_(boot_class_path_vdex_fds),
boot_class_path_oat_fds_(boot_class_path_oat_fds),
- apex_versions_(GetApexVersions(apex_versions)) {}
+ apex_versions_(*apex_versions) {}
std::string GetPrimaryImageLocation();
@@ -453,17 +456,6 @@ class ImageSpace : public MemMapSpace {
bool allow_in_memory_compilation,
/*out*/ std::string* error_msg);
- // This function prefers taking APEX versions from the input instead of from the runtime if
- // possible. If the input is present, `ValidateFromSystem` can work without an active runtime.
- static const std::string& GetApexVersions(const std::string* apex_versions) {
- if (apex_versions == nullptr) {
- DCHECK(Runtime::Current() != nullptr);
- return Runtime::Current()->GetApexVersions();
- } else {
- return *apex_versions;
- }
- }
-
ArrayRef<const std::string> image_locations_;
ArrayRef<const std::string> boot_class_path_;
ArrayRef<const std::string> boot_class_path_locations_;
diff --git a/runtime/gc/space/image_space_test.cc b/runtime/gc/space/image_space_test.cc
index 2a67f16b90..fa9a255ccb 100644
--- a/runtime/gc/space/image_space_test.cc
+++ b/runtime/gc/space/image_space_test.cc
@@ -136,20 +136,22 @@ TEST_F(ImageSpaceTest, StringDeduplication) {
auto load_boot_image = [&]() REQUIRES_SHARED(Locks::mutator_lock_) {
boot_image_spaces.clear();
extra_reservation = MemMap::Invalid();
- return ImageSpace::LoadBootImage(bcp,
- bcp_locations,
- /*boot_class_path_fds=*/std::vector<int>(),
- /*boot_class_path_image_fds=*/std::vector<int>(),
- /*boot_class_path_vdex_fds=*/std::vector<int>(),
- /*boot_class_path_oat_fds=*/std::vector<int>(),
- full_image_locations,
- kRuntimeISA,
- /*relocate=*/false,
- /*executable=*/true,
- /*extra_reservation_size=*/0u,
- /*allow_in_memory_compilation=*/false,
- &boot_image_spaces,
- &extra_reservation);
+ return ImageSpace::LoadBootImage(
+ bcp,
+ bcp_locations,
+ /*boot_class_path_fds=*/std::vector<int>(),
+ /*boot_class_path_image_fds=*/std::vector<int>(),
+ /*boot_class_path_vdex_fds=*/std::vector<int>(),
+ /*boot_class_path_oat_fds=*/std::vector<int>(),
+ full_image_locations,
+ kRuntimeISA,
+ /*relocate=*/false,
+ /*executable=*/true,
+ /*extra_reservation_size=*/0u,
+ /*allow_in_memory_compilation=*/false,
+ Runtime::GetApexVersions(ArrayRef<const std::string>(bcp_locations)),
+ &boot_image_spaces,
+ &extra_reservation);
};
const char test_string[] = "SharedBootImageExtensionTestString";