diff options
author | 2023-07-14 16:11:16 +0100 | |
---|---|---|
committer | 2023-07-20 08:00:21 +0000 | |
commit | fb20a36bf38f30e53c26b42f698ee3063228b460 (patch) | |
tree | f358965740c7d46a696aeb9334b3b0cb1f0af947 | |
parent | a6507b5c18c54b77dad169b37fc3b6ec3c0bc608 (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.cc | 30 | ||||
-rw-r--r-- | runtime/gc/heap.cc | 3 | ||||
-rw-r--r-- | runtime/gc/space/image_space.cc | 52 | ||||
-rw-r--r-- | runtime/gc/space/image_space.h | 48 | ||||
-rw-r--r-- | runtime/gc/space/image_space_test.cc | 30 |
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"; |