diff options
| author | Android Build Coastguard Worker <android-build-coastguard-worker@google.com> | 2024-05-07 23:00:38 +0000 |
|---|---|---|
| committer | Android Build Coastguard Worker <android-build-coastguard-worker@google.com> | 2024-05-07 23:00:38 +0000 |
| commit | 0848218580b28ed3bdcb4e680ec47a50eee86385 (patch) | |
| tree | 7bc33df1846aa6838babe2f1f5b6c60137a7eb71 | |
| parent | 47992e6060de07d86436e2fb2e9c23fe030e8e0d (diff) | |
| parent | 488413f47e7552d067edf9cfd5ceda321fc12f88 (diff) | |
| download | art-0848218580b28ed3bdcb4e680ec47a50eee86385.tar.gz | |
Snap for 11812660 from 488413f47e7552d067edf9cfd5ceda321fc12f88 to sdk-release
Change-Id: I6884ed134a5f5f3d4df1f6680082455c2293b083
106 files changed, 2656 insertions, 769 deletions
diff --git a/artd/artd.cc b/artd/artd.cc index 6154c8207b..3eef99d7a8 100644 --- a/artd/artd.cc +++ b/artd/artd.cc @@ -117,6 +117,7 @@ using ::android::base::ReadFileToString; using ::android::base::Result; using ::android::base::Split; using ::android::base::StringReplace; +using ::android::base::Tokenize; using ::android::base::WriteStringToFd; using ::android::base::WriteStringToFile; using ::android::fs_mgr::FstabEntry; @@ -129,6 +130,7 @@ using ::art::tools::NonFatal; using ::ndk::ScopedAStatus; using TmpProfilePath = ProfilePath::TmpProfilePath; +using WritableProfilePath = ProfilePath::WritableProfilePath; constexpr const char* kServiceName = "artd"; constexpr const char* kPreRebootServiceName = "artd_pre_reboot"; @@ -604,8 +606,7 @@ ndk::ScopedAStatus Artd::isProfileUsable(const ProfilePath& in_profile, FdLogger fd_logger; - CmdlineBuilder art_exec_args; - art_exec_args.Add(OR_RETURN_FATAL(GetArtExec())).Add("--drop-capabilities"); + CmdlineBuilder art_exec_args = OR_RETURN_FATAL(GetArtExecCmdlineBuilder()); CmdlineBuilder args; args.Add(OR_RETURN_FATAL(GetProfman())); @@ -657,8 +658,7 @@ ndk::ScopedAStatus Artd::CopyAndRewriteProfileImpl(File src, FdLogger fd_logger; - CmdlineBuilder art_exec_args; - art_exec_args.Add(OR_RETURN_FATAL(GetArtExec())).Add("--drop-capabilities"); + CmdlineBuilder art_exec_args = OR_RETURN_FATAL(GetArtExecCmdlineBuilder()); CmdlineBuilder args; args.Add(OR_RETURN_FATAL(GetProfman())).Add("--copy-and-update-profile-key"); @@ -831,8 +831,7 @@ ndk::ScopedAStatus Artd::mergeProfiles(const std::vector<ProfilePath>& in_profil FdLogger fd_logger; - CmdlineBuilder art_exec_args; - art_exec_args.Add(OR_RETURN_FATAL(GetArtExec())).Add("--drop-capabilities"); + CmdlineBuilder art_exec_args = OR_RETURN_FATAL(GetArtExecCmdlineBuilder()); CmdlineBuilder args; args.Add(OR_RETURN_FATAL(GetProfman())); @@ -1026,8 +1025,7 @@ ndk::ScopedAStatus Artd::dexopt( FdLogger fd_logger; - CmdlineBuilder art_exec_args; - art_exec_args.Add(OR_RETURN_FATAL(GetArtExec())).Add("--drop-capabilities"); + CmdlineBuilder art_exec_args = OR_RETURN_FATAL(GetArtExecCmdlineBuilder()); CmdlineBuilder args; args.Add(OR_RETURN_FATAL(GetDex2Oat())); @@ -1248,6 +1246,7 @@ ScopedAStatus Artd::cleanup(const std::vector<ProfilePath>& in_profilesToKeep, const std::vector<ArtifactsPath>& in_artifactsToKeep, const std::vector<VdexPath>& in_vdexFilesToKeep, const std::vector<RuntimeArtifactsPath>& in_runtimeArtifactsToKeep, + bool in_keepPreRebootStagedFiles, int64_t* _aidl_return) { RETURN_FATAL_IF_PRE_REBOOT(options_); std::unordered_set<std::string> files_to_keep; @@ -1276,7 +1275,8 @@ ScopedAStatus Artd::cleanup(const std::vector<ProfilePath>& in_profilesToKeep, } *_aidl_return = 0; for (const std::string& file : ListManagedFiles(android_data, android_expand)) { - if (files_to_keep.find(file) == files_to_keep.end()) { + if (files_to_keep.find(file) == files_to_keep.end() && + (!in_keepPreRebootStagedFiles || !IsPreRebootStagedFile(file))) { LOG(INFO) << ART_FORMAT("Cleaning up obsolete file '{}'", file); *_aidl_return += GetSizeAndDeleteFile(file); } @@ -1370,6 +1370,61 @@ ScopedAStatus Artd::getProfileSize(const ProfilePath& in_profile, int64_t* _aidl return ScopedAStatus::ok(); } +ScopedAStatus Artd::commitPreRebootStagedFiles( + const std::vector<ArtifactsPath>& in_artifacts, + const std::vector<WritableProfilePath>& in_profiles) { + RETURN_FATAL_IF_PRE_REBOOT(options_); + + std::vector<std::pair<std::string, std::string>> files_to_move; + std::vector<std::string> files_to_remove; + + for (const ArtifactsPath& artifacts : in_artifacts) { + RETURN_FATAL_IF_ARG_IS_PRE_REBOOT(artifacts, "artifacts"); + + ArtifactsPath pre_reboot_artifacts = artifacts; + pre_reboot_artifacts.isPreReboot = true; + + auto src_artifacts = std::make_unique<RawArtifactsPath>( + OR_RETURN_FATAL(BuildArtifactsPath(pre_reboot_artifacts))); + auto dst_artifacts = + std::make_unique<RawArtifactsPath>(OR_RETURN_FATAL(BuildArtifactsPath(artifacts))); + + if (OS::FileExists(src_artifacts->oat_path.c_str())) { + files_to_move.emplace_back(src_artifacts->oat_path, dst_artifacts->oat_path); + files_to_move.emplace_back(src_artifacts->vdex_path, dst_artifacts->vdex_path); + if (OS::FileExists(src_artifacts->art_path.c_str())) { + files_to_move.emplace_back(src_artifacts->art_path, dst_artifacts->art_path); + } else { + files_to_remove.push_back(dst_artifacts->art_path); + } + } + } + + for (const WritableProfilePath& profile : in_profiles) { + RETURN_FATAL_IF_ARG_IS_PRE_REBOOT(profile, "profiles"); + + WritableProfilePath pre_reboot_profile = profile; + PreRebootFlag(pre_reboot_profile) = true; + + auto src_profile = std::make_unique<std::string>( + OR_RETURN_FATAL(BuildWritableProfilePath(pre_reboot_profile))); + auto dst_profile = + std::make_unique<std::string>(OR_RETURN_FATAL(BuildWritableProfilePath(profile))); + + if (OS::FileExists(src_profile->c_str())) { + files_to_move.emplace_back(*src_profile, *dst_profile); + } + } + + OR_RETURN_NON_FATAL(MoveAllOrAbandon(files_to_move, files_to_remove)); + + for (const auto& [src_path, dst_path] : files_to_move) { + LOG(INFO) << ART_FORMAT("Committed Pre-reboot staged file '{}' to '{}'", src_path, dst_path); + } + + return ScopedAStatus::ok(); +} + Result<void> Artd::Start() { OR_RETURN(SetLogVerbosity()); MemMap::Init(); @@ -1490,7 +1545,13 @@ bool Artd::DenyArtApexDataFilesLocked() { Result<std::string> Artd::GetProfman() { return BuildArtBinPath("profman"); } -Result<std::string> Artd::GetArtExec() { return BuildArtBinPath("art_exec"); } +Result<CmdlineBuilder> Artd::GetArtExecCmdlineBuilder() { + CmdlineBuilder args; + args.Add(OR_RETURN(BuildArtBinPath("art_exec"))) + .Add("--drop-capabilities") + .AddIf(options_.is_pre_reboot, "--process-name-suffix=Pre-reboot Dexopt chroot"); + return args; +} bool Artd::ShouldUseDex2Oat64() { return !props_->GetOrEmpty("ro.product.cpu.abilist64").empty() && @@ -1538,8 +1599,7 @@ void Artd::AddCompilerConfigFlags(const std::string& instruction_set, props_->GetOrEmpty("dalvik.vm.dex2oat-very-large")) .AddIfNonEmpty( "--resolve-startup-const-strings=%s", - props_->GetOrEmpty("persist.device_config.runtime.dex2oat_resolve_startup_strings", - "dalvik.vm.dex2oat-resolve-startup-strings")); + props_->GetOrEmpty("dalvik.vm.dex2oat-resolve-startup-strings")); args.AddIf(dexopt_options.debuggable, "--debuggable") .AddIf(props_->GetBool("debug.generate-debug-info", /*default_value=*/false), @@ -1589,6 +1649,11 @@ void Artd::AddPerfConfigFlags(PriorityClass priority_class, // It takes longer but reduces the memory footprint. dex2oat_args.AddIf(props_->GetBool("ro.config.low_ram", /*default_value=*/false), "--compile-individually"); + + for (const std::string& flag : + Tokenize(props_->GetOrEmpty("dalvik.vm.dex2oat-flags"), /*delimiters=*/" ")) { + dex2oat_args.AddIfNonEmpty("%s", flag); + } } Result<int> Artd::ExecAndReturnCode(const std::vector<std::string>& args, @@ -1719,10 +1784,8 @@ Result<void> Artd::PreRebootInitDeriveClasspath(const std::string& path) { return ErrnoErrorf("Failed to create '{}'", path); } - CmdlineBuilder args; - args.Add(OR_RETURN(GetArtExec())) - .Add("--drop-capabilities") - .Add("--keep-fds=%d", output->Fd()) + CmdlineBuilder args = OR_RETURN(GetArtExecCmdlineBuilder()); + args.Add("--keep-fds=%d", output->Fd()) .Add("--") .Add("/apex/com.android.sdkext/bin/derive_classpath") .Add("/proc/self/fd/%d", output->Fd()); @@ -1748,10 +1811,8 @@ Result<void> Artd::PreRebootInitDeriveClasspath(const std::string& path) { } Result<void> Artd::PreRebootInitBootImages() { - CmdlineBuilder args; - args.Add(OR_RETURN(GetArtExec())) - .Add("--drop-capabilities") - .Add("--") + CmdlineBuilder args = OR_RETURN(GetArtExecCmdlineBuilder()); + args.Add("--") .Add(OR_RETURN(BuildArtBinPath("odrefresh"))) .Add("--only-boot-images") .Add("--compile"); diff --git a/artd/artd.h b/artd/artd.h index a1f3541fc0..a926dbc652 100644 --- a/artd/artd.h +++ b/artd/artd.h @@ -199,6 +199,7 @@ class Artd : public aidl::com::android::server::art::BnArtd { const std::vector<aidl::com::android::server::art::VdexPath>& in_vdexFilesToKeep, const std::vector<aidl::com::android::server::art::RuntimeArtifactsPath>& in_runtimeArtifactsToKeep, + bool in_keepPreRebootStagedFiles, int64_t* _aidl_return) override; ndk::ScopedAStatus isInDalvikCache(const std::string& in_dexFile, bool* _aidl_return) override; @@ -221,6 +222,11 @@ class Artd : public aidl::com::android::server::art::BnArtd { ndk::ScopedAStatus getProfileSize(const aidl::com::android::server::art::ProfilePath& in_profile, int64_t* _aidl_return) override; + ndk::ScopedAStatus commitPreRebootStagedFiles( + const std::vector<aidl::com::android::server::art::ArtifactsPath>& in_artifacts, + const std::vector<aidl::com::android::server::art::ProfilePath::WritableProfilePath>& + in_profiles) override; + ndk::ScopedAStatus preRebootInit() override; ndk::ScopedAStatus validateDexPath(const std::string& in_dexFile, @@ -257,7 +263,7 @@ class Artd : public aidl::com::android::server::art::BnArtd { android::base::Result<std::string> GetProfman(); - android::base::Result<std::string> GetArtExec(); + android::base::Result<tools::CmdlineBuilder> GetArtExecCmdlineBuilder(); bool ShouldUseDex2Oat64(); diff --git a/artd/artd_test.cc b/artd/artd_test.cc index fccf091464..1c71ab2fbd 100644 --- a/artd/artd_test.cc +++ b/artd/artd_test.cc @@ -943,6 +943,8 @@ TEST_F(ArtdTest, dexoptFlagsFromSystemProps) { EXPECT_CALL(*mock_props_, GetProperty("ro.config.low_ram")).WillOnce(Return("1")); EXPECT_CALL(*mock_props_, GetProperty("dalvik.vm.appimageformat")).WillOnce(Return("imgfmt")); EXPECT_CALL(*mock_props_, GetProperty("dalvik.vm.boot-image")).WillOnce(Return("boot-image")); + EXPECT_CALL(*mock_props_, GetProperty("dalvik.vm.dex2oat-flags")) + .WillOnce(Return("--flag1 --flag2 --flag3")); EXPECT_CALL(*mock_exec_utils_, DoExecAndReturnCode( @@ -962,7 +964,10 @@ TEST_F(ArtdTest, dexoptFlagsFromSystemProps) { Contains("--compile-individually"), Contains(Flag("--image-format=", "imgfmt")), Not(Contains("--force-jit-zygote")), - Contains(Flag("--boot-image=", "boot-image")))), + Contains(Flag("--boot-image=", "boot-image")), + Contains("--flag1"), + Contains("--flag2"), + Contains("--flag3"))), _, _)) .WillOnce(Return(0)); @@ -2089,138 +2094,165 @@ TEST_F(ArtdTest, mergeProfilesWithOptionsDumpClassesAndMethods) { CheckContent(output_profile.profilePath.tmpPath, "dump"); } -TEST_F(ArtdTest, cleanup) { - std::vector<std::string> gc_removed_files; - std::vector<std::string> gc_kept_files; +class ArtdCleanupTest : public ArtdTest { + protected: + void SetUp() override { + ArtdTest::SetUp(); - auto CreateGcRemovedFile = [&](const std::string& path) { + // Unmanaged files. + CreateGcKeptFile(android_data_ + "/user_de/0/com.android.foo/1.odex"); + CreateGcKeptFile(android_data_ + "/user_de/0/com.android.foo/oat/1.odex"); + CreateGcKeptFile(android_data_ + "/user_de/0/com.android.foo/oat/1.txt"); + CreateGcKeptFile(android_data_ + "/user_de/0/com.android.foo/oat/arm64/1.txt"); + CreateGcKeptFile(android_data_ + "/user_de/0/com.android.foo/oat/arm64/1.tmp"); + + // Files to keep. + CreateGcKeptFile(android_data_ + "/misc/profiles/cur/1/com.android.foo/primary.prof"); + CreateGcKeptFile(android_data_ + "/misc/profiles/cur/3/com.android.foo/primary.prof"); + CreateGcKeptFile(android_data_ + "/dalvik-cache/arm64/system@app@Foo@Foo.apk@classes.dex"); + CreateGcKeptFile(android_data_ + "/dalvik-cache/arm64/system@app@Foo@Foo.apk@classes.vdex"); + CreateGcKeptFile(android_data_ + "/dalvik-cache/arm64/system@app@Foo@Foo.apk@classes.art"); + CreateGcKeptFile(android_data_ + "/user_de/0/com.android.foo/aaa/oat/arm64/1.vdex"); + CreateGcKeptFile( + android_expand_ + + "/123456-7890/app/~~nkfeankfna==/com.android.bar-jfoeaofiew==/oat/arm64/base.odex"); + CreateGcKeptFile( + android_expand_ + + "/123456-7890/app/~~nkfeankfna==/com.android.bar-jfoeaofiew==/oat/arm64/base.vdex"); + CreateGcKeptFile( + android_expand_ + + "/123456-7890/app/~~nkfeankfna==/com.android.bar-jfoeaofiew==/oat/arm64/base.art"); + CreateGcKeptFile(android_data_ + "/user_de/0/com.android.foo/aaa/oat/arm64/2.odex"); + CreateGcKeptFile(android_data_ + "/user_de/0/com.android.foo/aaa/oat/arm64/2.vdex"); + CreateGcKeptFile(android_data_ + "/user_de/0/com.android.foo/aaa/oat/arm64/2.art"); + CreateGcKeptFile(android_data_ + "/user_de/0/com.android.foo/cache/oat_primary/arm64/base.art"); + CreateGcKeptFile(android_data_ + "/user/0/com.android.foo/cache/oat_primary/arm64/base.art"); + CreateGcKeptFile(android_data_ + "/user/1/com.android.foo/cache/oat_primary/arm64/base.art"); + CreateGcKeptFile(android_expand_ + + "/123456-7890/user/1/com.android.foo/cache/oat_primary/arm64/base.art"); + CreateGcKeptFile(android_data_ + + "/user/0/com.android.foo/cache/not_oat_dir/oat_primary/arm64/base.art"); + + // Files to remove. + CreateGcRemovedFile(android_data_ + "/misc/profiles/ref/com.android.foo/primary.prof"); + CreateGcRemovedFile(android_data_ + "/misc/profiles/cur/2/com.android.foo/primary.prof"); + CreateGcRemovedFile(android_data_ + "/misc/profiles/cur/3/com.android.bar/primary.prof"); + CreateGcRemovedFile(android_data_ + "/dalvik-cache/arm64/extra.odex"); + CreateGcRemovedFile(android_data_ + "/dalvik-cache/arm64/system@app@Bar@Bar.apk@classes.dex"); + CreateGcRemovedFile(android_data_ + "/dalvik-cache/arm64/system@app@Bar@Bar.apk@classes.vdex"); + CreateGcRemovedFile(android_data_ + "/dalvik-cache/arm64/system@app@Bar@Bar.apk@classes.art"); + CreateGcRemovedFile( + android_expand_ + + "/123456-7890/app/~~daewfweaf==/com.android.foo-fjuwidhia==/oat/arm64/base.odex"); + CreateGcRemovedFile( + android_expand_ + + "/123456-7890/app/~~daewfweaf==/com.android.foo-fjuwidhia==/oat/arm64/base.vdex"); + CreateGcRemovedFile( + android_expand_ + + "/123456-7890/app/~~daewfweaf==/com.android.foo-fjuwidhia==/oat/arm64/base.art"); + CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/oat/1.prof"); + CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/oat/1.prof.123456.tmp"); + CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/oat/arm64/1.odex"); + CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/oat/arm64/1.vdex"); + CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/oat/arm64/1.art"); + CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/oat/arm64/1.odex.123456.tmp"); + CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/oat/arm64/2.odex.123456.tmp"); + CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/aaa/oat/arm64/1.odex"); + CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/aaa/oat/arm64/1.art"); + CreateGcRemovedFile(android_data_ + + "/user_de/0/com.android.foo/aaa/oat/arm64/1.vdex.123456.tmp"); + CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/aaa/bbb/oat/arm64/1.odex"); + CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/aaa/bbb/oat/arm64/1.vdex"); + CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/aaa/bbb/oat/arm64/1.art"); + CreateGcRemovedFile(android_data_ + + "/user_de/0/com.android.foo/aaa/bbb/oat/arm64/1.art.123456.tmp"); + CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.bar/aaa/oat/arm64/1.vdex"); + CreateGcRemovedFile(android_data_ + + "/user/0/com.android.different_package/cache/oat_primary/arm64/base.art"); + CreateGcRemovedFile(android_data_ + + "/user/0/com.android.foo/cache/oat_primary/arm64/different_dex.art"); + CreateGcRemovedFile(android_data_ + + "/user/0/com.android.foo/cache/oat_primary/different_isa/base.art"); + } + + void CreateGcRemovedFile(const std::string& path) { CreateFile(path); - gc_removed_files.push_back(path); - }; + gc_removed_files_.push_back(path); + } - auto CreateGcKeptFile = [&](const std::string& path) { + void CreateGcKeptFile(const std::string& path) { CreateFile(path); - gc_kept_files.push_back(path); - }; + gc_kept_files_.push_back(path); + } - // Unmanaged files. - CreateGcKeptFile(android_data_ + "/user_de/0/com.android.foo/1.odex"); - CreateGcKeptFile(android_data_ + "/user_de/0/com.android.foo/oat/1.odex"); - CreateGcKeptFile(android_data_ + "/user_de/0/com.android.foo/oat/1.txt"); - CreateGcKeptFile(android_data_ + "/user_de/0/com.android.foo/oat/arm64/1.txt"); - CreateGcKeptFile(android_data_ + "/user_de/0/com.android.foo/oat/arm64/1.tmp"); - - // Files to keep. - CreateGcKeptFile(android_data_ + "/misc/profiles/cur/1/com.android.foo/primary.prof"); - CreateGcKeptFile(android_data_ + "/misc/profiles/cur/3/com.android.foo/primary.prof"); - CreateGcKeptFile(android_data_ + "/dalvik-cache/arm64/system@app@Foo@Foo.apk@classes.dex"); - CreateGcKeptFile(android_data_ + "/dalvik-cache/arm64/system@app@Foo@Foo.apk@classes.vdex"); - CreateGcKeptFile(android_data_ + "/dalvik-cache/arm64/system@app@Foo@Foo.apk@classes.art"); - CreateGcKeptFile(android_data_ + "/user_de/0/com.android.foo/aaa/oat/arm64/1.vdex"); - CreateGcKeptFile( - android_expand_ + - "/123456-7890/app/~~nkfeankfna==/com.android.bar-jfoeaofiew==/oat/arm64/base.odex"); - CreateGcKeptFile( - android_expand_ + - "/123456-7890/app/~~nkfeankfna==/com.android.bar-jfoeaofiew==/oat/arm64/base.vdex"); + void RunCleanup(bool keepPreRebootStagedFiles) { + int64_t aidl_return; + ASSERT_STATUS_OK(artd_->cleanup( + { + PrimaryCurProfilePath{ + .userId = 1, .packageName = "com.android.foo", .profileName = "primary"}, + PrimaryCurProfilePath{ + .userId = 3, .packageName = "com.android.foo", .profileName = "primary"}, + }, + { + ArtifactsPath{ + .dexPath = "/system/app/Foo/Foo.apk", .isa = "arm64", .isInDalvikCache = true}, + ArtifactsPath{ + .dexPath = android_expand_ + + "/123456-7890/app/~~nkfeankfna==/com.android.bar-jfoeaofiew==/base.apk", + .isa = "arm64", + .isInDalvikCache = false}, + ArtifactsPath{.dexPath = android_data_ + "/user_de/0/com.android.foo/aaa/2.apk", + .isa = "arm64", + .isInDalvikCache = false}, + }, + { + VdexPath{ + ArtifactsPath{.dexPath = android_data_ + "/user_de/0/com.android.foo/aaa/1.apk", + .isa = "arm64", + .isInDalvikCache = false}}, + }, + { + RuntimeArtifactsPath{ + .packageName = "com.android.foo", .dexPath = "/a/b/base.apk", .isa = "arm64"}, + }, + keepPreRebootStagedFiles, + &aidl_return)); + } + + void Verify() { + for (const std::string& path : gc_removed_files_) { + EXPECT_FALSE(std::filesystem::exists(path)) << ART_FORMAT("'{}' should be removed", path); + } + + for (const std::string& path : gc_kept_files_) { + EXPECT_TRUE(std::filesystem::exists(path)) << ART_FORMAT("'{}' should be kept", path); + } + } + + private: + std::vector<std::string> gc_removed_files_; + std::vector<std::string> gc_kept_files_; +}; + +TEST_F(ArtdCleanupTest, cleanupKeepingPreRebootStagedFiles) { CreateGcKeptFile( android_expand_ + - "/123456-7890/app/~~nkfeankfna==/com.android.bar-jfoeaofiew==/oat/arm64/base.art"); - CreateGcKeptFile(android_data_ + "/user_de/0/com.android.foo/aaa/oat/arm64/2.odex"); - CreateGcKeptFile(android_data_ + "/user_de/0/com.android.foo/aaa/oat/arm64/2.vdex"); - CreateGcKeptFile(android_data_ + "/user_de/0/com.android.foo/aaa/oat/arm64/2.art"); - CreateGcKeptFile(android_data_ + "/user_de/0/com.android.foo/cache/oat_primary/arm64/base.art"); - CreateGcKeptFile(android_data_ + "/user/0/com.android.foo/cache/oat_primary/arm64/base.art"); - CreateGcKeptFile(android_data_ + "/user/1/com.android.foo/cache/oat_primary/arm64/base.art"); - CreateGcKeptFile(android_expand_ + - "/123456-7890/user/1/com.android.foo/cache/oat_primary/arm64/base.art"); - CreateGcKeptFile(android_data_ + - "/user/0/com.android.foo/cache/not_oat_dir/oat_primary/arm64/base.art"); - - // Files to remove. - CreateGcRemovedFile(android_data_ + "/misc/profiles/ref/com.android.foo/primary.prof"); - CreateGcRemovedFile(android_data_ + "/misc/profiles/cur/2/com.android.foo/primary.prof"); - CreateGcRemovedFile(android_data_ + "/misc/profiles/cur/3/com.android.bar/primary.prof"); - CreateGcRemovedFile(android_data_ + "/dalvik-cache/arm64/extra.odex"); - CreateGcRemovedFile(android_data_ + "/dalvik-cache/arm64/system@app@Bar@Bar.apk@classes.dex"); - CreateGcRemovedFile(android_data_ + "/dalvik-cache/arm64/system@app@Bar@Bar.apk@classes.vdex"); - CreateGcRemovedFile(android_data_ + "/dalvik-cache/arm64/system@app@Bar@Bar.apk@classes.art"); - CreateGcRemovedFile( - android_expand_ + - "/123456-7890/app/~~daewfweaf==/com.android.foo-fjuwidhia==/oat/arm64/base.odex"); - CreateGcRemovedFile( - android_expand_ + - "/123456-7890/app/~~daewfweaf==/com.android.foo-fjuwidhia==/oat/arm64/base.vdex"); - CreateGcRemovedFile( - android_expand_ + - "/123456-7890/app/~~daewfweaf==/com.android.foo-fjuwidhia==/oat/arm64/base.art"); - CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/oat/1.prof"); - CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/oat/1.prof.123456.tmp"); - CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/oat/arm64/1.odex"); - CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/oat/arm64/1.vdex"); - CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/oat/arm64/1.art"); - CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/oat/arm64/1.odex.123456.tmp"); - CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/oat/arm64/2.odex.123456.tmp"); - CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/aaa/oat/arm64/1.odex"); - CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/aaa/oat/arm64/1.art"); - CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/aaa/oat/arm64/1.vdex.123456.tmp"); - CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/aaa/bbb/oat/arm64/1.odex"); - CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/aaa/bbb/oat/arm64/1.vdex"); - CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/aaa/bbb/oat/arm64/1.art"); - CreateGcRemovedFile(android_data_ + - "/user_de/0/com.android.foo/aaa/bbb/oat/arm64/1.art.123456.tmp"); - CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.bar/aaa/oat/arm64/1.vdex"); - CreateGcRemovedFile(android_data_ + - "/user/0/com.android.different_package/cache/oat_primary/arm64/base.art"); - CreateGcRemovedFile(android_data_ + - "/user/0/com.android.foo/cache/oat_primary/arm64/different_dex.art"); - CreateGcRemovedFile(android_data_ + - "/user/0/com.android.foo/cache/oat_primary/different_isa/base.art"); + "/123456-7890/app/~~nkfeankfna==/com.android.bar-jfoeaofiew==/oat/arm64/base.odex.staged"); + CreateGcKeptFile(android_data_ + "/user_de/0/com.android.foo/aaa/oat/arm64/2.odex.staged"); - int64_t aidl_return; - ASSERT_TRUE( - artd_ - ->cleanup( - { - PrimaryCurProfilePath{ - .userId = 1, .packageName = "com.android.foo", .profileName = "primary"}, - PrimaryCurProfilePath{ - .userId = 3, .packageName = "com.android.foo", .profileName = "primary"}, - }, - { - ArtifactsPath{.dexPath = "/system/app/Foo/Foo.apk", - .isa = "arm64", - .isInDalvikCache = true}, - ArtifactsPath{ - .dexPath = - android_expand_ + - "/123456-7890/app/~~nkfeankfna==/com.android.bar-jfoeaofiew==/base.apk", - .isa = "arm64", - .isInDalvikCache = false}, - ArtifactsPath{.dexPath = android_data_ + "/user_de/0/com.android.foo/aaa/2.apk", - .isa = "arm64", - .isInDalvikCache = false}, - }, - { - VdexPath{ArtifactsPath{ - .dexPath = android_data_ + "/user_de/0/com.android.foo/aaa/1.apk", - .isa = "arm64", - .isInDalvikCache = false}}, - }, - { - RuntimeArtifactsPath{ - .packageName = "com.android.foo", .dexPath = "/a/b/base.apk", .isa = "arm64"}, - }, - &aidl_return) - .isOk()); + ASSERT_NO_FATAL_FAILURE(RunCleanup(/*keepPreRebootStagedFiles=*/true)); + Verify(); +} - for (const std::string& path : gc_removed_files) { - EXPECT_FALSE(std::filesystem::exists(path)) << ART_FORMAT("'{}' should be removed", path); - } +TEST_F(ArtdCleanupTest, cleanupRemovingPreRebootStagedFiles) { + CreateGcRemovedFile( + android_expand_ + + "/123456-7890/app/~~nkfeankfna==/com.android.bar-jfoeaofiew==/oat/arm64/base.odex.staged"); + CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/aaa/oat/arm64/2.odex.staged"); - for (const std::string& path : gc_kept_files) { - EXPECT_TRUE(std::filesystem::exists(path)) << ART_FORMAT("'{}' should be kept", path); - } + ASSERT_NO_FATAL_FAILURE(RunCleanup(/*keepPreRebootStagedFiles=*/false)); + Verify(); } TEST_F(ArtdTest, isInDalvikCache) { @@ -2446,6 +2478,93 @@ TEST_F(ArtdTest, getProfileSize) { EXPECT_EQ(aidl_return, 1); } +TEST_F(ArtdTest, commitPreRebootStagedFiles) { + CreateFile(android_data_ + "/dalvik-cache/arm64/system@app@Foo@Foo.apk@classes.dex.staged", + "new_odex_1"); + CreateFile(android_data_ + "/dalvik-cache/arm64/system@app@Foo@Foo.apk@classes.vdex.staged", + "new_vdex_1"); + CreateFile(android_data_ + "/dalvik-cache/arm64/system@app@Foo@Foo.apk@classes.art.staged", + "new_art_1"); + + CreateFile(android_data_ + "/dalvik-cache/arm64/system@app@Foo@Foo.apk@classes.dex", + "old_odex_1"); + CreateFile(android_data_ + "/dalvik-cache/arm64/system@app@Foo@Foo.apk@classes.vdex", + "old_vdex_1"); + CreateFile(android_data_ + "/dalvik-cache/arm64/system@app@Foo@Foo.apk@classes.art", "old_art_1"); + + CreateFile(android_data_ + "/app/com.android.foo/oat/arm64/base.odex", "old_odex_2"); + CreateFile(android_data_ + "/app/com.android.foo/oat/arm64/base.vdex", "old_vdex_2"); + CreateFile(android_data_ + "/app/com.android.foo/oat/arm64/base.art", "old_art_2"); + + CreateFile(android_data_ + "/app/com.android.foo/oat/arm64/base.odex.staged", "new_odex_2"); + CreateFile(android_data_ + "/app/com.android.foo/oat/arm64/base.vdex.staged", "new_vdex_2"); + + CreateFile(android_data_ + "/app/com.android.foo/oat/arm/base.odex", "old_odex_3"); + CreateFile(android_data_ + "/app/com.android.foo/oat/arm/base.vdex", "old_vdex_3"); + CreateFile(android_data_ + "/app/com.android.foo/oat/arm/base.art", "old_art_3"); + + CreateFile(android_data_ + "/misc/profiles/ref/com.android.foo/primary.prof", "old_prof_1"); + CreateFile(android_data_ + "/misc/profiles/ref/com.android.foo/primary.prof.staged", + "new_prof_1"); + + CreateFile(android_data_ + "/misc/profiles/ref/com.android.bar/primary.prof", "old_prof_2"); + + ASSERT_STATUS_OK(artd_->commitPreRebootStagedFiles( + { + // Has all new files. All old files should be replaced. + ArtifactsPath{ + .dexPath = "/system/app/Foo/Foo.apk", .isa = "arm64", .isInDalvikCache = true}, + // Has new files but not ".art" file. Old ".odex" and ".vdex" files should be replaced, + // and old ".art" file should be removed. + ArtifactsPath{.dexPath = android_data_ + "/app/com.android.foo/base.apk", + .isa = "arm64", + .isInDalvikCache = false}, + // Has no new file. All old files should be kept. + ArtifactsPath{.dexPath = android_data_ + "/app/com.android.foo/base.apk", + .isa = "arm", + .isInDalvikCache = false}, + }, + { + // Has new file. + PrimaryRefProfilePath{.packageName = "com.android.foo", .profileName = "primary"}, + // Has no new file. + PrimaryRefProfilePath{.packageName = "com.android.bar", .profileName = "primary"}, + })); + + CheckContent(android_data_ + "/dalvik-cache/arm64/system@app@Foo@Foo.apk@classes.dex", + "new_odex_1"); + CheckContent(android_data_ + "/dalvik-cache/arm64/system@app@Foo@Foo.apk@classes.vdex", + "new_vdex_1"); + CheckContent(android_data_ + "/dalvik-cache/arm64/system@app@Foo@Foo.apk@classes.art", + "new_art_1"); + + CreateFile(android_data_ + "/app/com.android.foo/oat/arm64/base.odex", "new_odex_2"); + CreateFile(android_data_ + "/app/com.android.foo/oat/arm64/base.vdex", "new_vdex_2"); + EXPECT_FALSE(std::filesystem::exists(android_data_ + "/app/com.android.foo/oat/arm64/base.art")); + + CheckContent(android_data_ + "/app/com.android.foo/oat/arm/base.odex", "old_odex_3"); + CheckContent(android_data_ + "/app/com.android.foo/oat/arm/base.vdex", "old_vdex_3"); + CheckContent(android_data_ + "/app/com.android.foo/oat/arm/base.art", "old_art_3"); + + CheckContent(android_data_ + "/misc/profiles/ref/com.android.foo/primary.prof", "new_prof_1"); + + CheckContent(android_data_ + "/misc/profiles/ref/com.android.bar/primary.prof", "old_prof_2"); + + // All staged files are gone. + EXPECT_FALSE(std::filesystem::exists( + android_data_ + "/dalvik-cache/arm64/system@app@Foo@Foo.apk@classes.dex.staged")); + EXPECT_FALSE(std::filesystem::exists( + android_data_ + "/dalvik-cache/arm64/system@app@Foo@Foo.apk@classes.vdex.staged")); + EXPECT_FALSE(std::filesystem::exists( + android_data_ + "/dalvik-cache/arm64/system@app@Foo@Foo.apk@classes.art.staged")); + EXPECT_FALSE( + std::filesystem::exists(android_data_ + "/app/com.android.foo/oat/arm64/base.odex.staged")); + EXPECT_FALSE( + std::filesystem::exists(android_data_ + "/app/com.android.foo/oat/arm64/base.vdex.staged")); + EXPECT_FALSE(std::filesystem::exists(android_data_ + + "/misc/profiles/ref/com.android.foo/primary.prof.staged")); +} + class ArtdPreRebootTest : public ArtdTest { protected: void SetUp() override { diff --git a/artd/binder/com/android/server/art/IArtd.aidl b/artd/binder/com/android/server/art/IArtd.aidl index 7f0bc5690c..54f55f8460 100644 --- a/artd/binder/com/android/server/art/IArtd.aidl +++ b/artd/binder/com/android/server/art/IArtd.aidl @@ -188,7 +188,8 @@ interface IArtd { long cleanup(in List<com.android.server.art.ProfilePath> profilesToKeep, in List<com.android.server.art.ArtifactsPath> artifactsToKeep, in List<com.android.server.art.VdexPath> vdexFilesToKeep, - in List<com.android.server.art.RuntimeArtifactsPath> runtimeArtifactsToKeep); + in List<com.android.server.art.RuntimeArtifactsPath> runtimeArtifactsToKeep, + boolean keepPreRebootStagedFiles); /** * Returns whether the artifacts of the primary dex files should be in the global dalvik-cache @@ -251,6 +252,22 @@ interface IArtd { */ long getProfileSize(in com.android.server.art.ProfilePath profile); + /** + * Moves the staged files of the given artifacts and profiles to the permanent locations, + * replacing old files if they exist. Removes the staged files and restores the old files at + * best effort if any error occurs. + * + * This is intended to be called for a superset of the packages that we actually expect to have + * staged files, so missing files are expected. + * + * Not supported in Pre-reboot Dexopt mode. + * + * Throws fatal and non-fatal errors. + */ + void commitPreRebootStagedFiles( + in List<com.android.server.art.ArtifactsPath> artifacts, + in List<com.android.server.art.ProfilePath.WritableProfilePath> profiles); + // The methods below are only for Pre-reboot Dexopt and only supported in Pre-reboot Dexopt // mode. diff --git a/artd/file_utils.cc b/artd/file_utils.cc index 1415efbdb3..195daf6162 100644 --- a/artd/file_utils.cc +++ b/artd/file_utils.cc @@ -230,6 +230,20 @@ Result<void> MoveAllOrAbandon( return {}; } +android::base::Result<void> MoveAllOrAbandon( + const std::vector<std::pair<std::string, std::string>>& files_to_move, + const std::vector<std::string>& files_to_remove) { + std::vector<std::pair<std::string_view, std::string_view>> files_to_move_view; + std::vector<std::string_view> files_to_remove_view; + for (const auto& [src, dst] : files_to_move) { + files_to_move_view.emplace_back(src, dst); + } + for (const std::string& file : files_to_remove) { + files_to_remove_view.emplace_back(file); + } + return MoveAllOrAbandon(files_to_move_view, files_to_remove_view); +} + std::string NewFile::BuildTempPath(std::string_view final_path, const std::string& id) { return ART_FORMAT("{}.{}.tmp", final_path, id); } diff --git a/artd/file_utils.h b/artd/file_utils.h index a97f52c42a..77c7ffb19c 100644 --- a/artd/file_utils.h +++ b/artd/file_utils.h @@ -20,6 +20,7 @@ #include <sys/types.h> #include <memory> +#include <string> #include <string_view> #include <utility> #include <vector> @@ -147,6 +148,11 @@ android::base::Result<void> MoveAllOrAbandon( const std::vector<std::pair<std::string_view, std::string_view>>& files_to_move, const std::vector<std::string_view>& files_to_remove = {}); +// Same as above, but takes `std::string`s. +android::base::Result<void> MoveAllOrAbandon( + const std::vector<std::pair<std::string, std::string>>& files_to_move, + const std::vector<std::string>& files_to_remove = {}); + } // namespace artd } // namespace art diff --git a/artd/path_utils.cc b/artd/path_utils.cc index d3eb6d4dfe..5c6ad9dc02 100644 --- a/artd/path_utils.cc +++ b/artd/path_utils.cc @@ -23,6 +23,7 @@ #include "aidl/com/android/server/art/BnArtd.h" #include "android-base/errors.h" #include "android-base/result.h" +#include "android-base/strings.h" #include "arch/instruction_set.h" #include "base/file_utils.h" #include "base/macros.h" @@ -44,6 +45,7 @@ using ::aidl::com::android::server::art::OutputProfile; using ::aidl::com::android::server::art::ProfilePath; using ::aidl::com::android::server::art::RuntimeArtifactsPath; using ::aidl::com::android::server::art::VdexPath; +using ::android::base::EndsWith; using ::android::base::Error; using ::android::base::Result; using ::art::service::ValidateDexPath; @@ -111,11 +113,11 @@ std::vector<std::string> ListManagedFiles(const std::string& android_data, // Profiles and artifacts for secondary dex files. Those files are in app data directories, so // we use more granular patterns to avoid accidentally deleting apps' files. std::string secondary_oat_dir = data_dir + "/**/oat"; - for (const char* maybe_tmp_suffix : {"", ".*.tmp"}) { - patterns.push_back(secondary_oat_dir + "/*.prof" + maybe_tmp_suffix); - patterns.push_back(secondary_oat_dir + "/*/*.odex" + maybe_tmp_suffix); - patterns.push_back(secondary_oat_dir + "/*/*.vdex" + maybe_tmp_suffix); - patterns.push_back(secondary_oat_dir + "/*/*.art" + maybe_tmp_suffix); + for (const char* suffix : {"", ".*.tmp", kPreRebootSuffix}) { + patterns.push_back(secondary_oat_dir + "/*.prof" + suffix); + patterns.push_back(secondary_oat_dir + "/*/*.odex" + suffix); + patterns.push_back(secondary_oat_dir + "/*/*.vdex" + suffix); + patterns.push_back(secondary_oat_dir + "/*/*.art" + suffix); } // Runtime image files. patterns.push_back(RuntimeImage::GetRuntimeImageDir(data_dir) + "**"); @@ -240,18 +242,21 @@ Result<std::string> BuildSecondaryCurProfilePath( "{}/oat/{}.cur.prof", dex_path.parent_path().string(), dex_path.filename().string()); } -Result<std::string> BuildFinalProfilePath(const TmpProfilePath& tmp_profile_path) { - const WritableProfilePath& final_path = tmp_profile_path.finalPath; - switch (final_path.getTag()) { +Result<std::string> BuildWritableProfilePath(const WritableProfilePath& profile_path) { + switch (profile_path.getTag()) { case WritableProfilePath::forPrimary: - return BuildPrimaryRefProfilePath(final_path.get<WritableProfilePath::forPrimary>()); + return BuildPrimaryRefProfilePath(profile_path.get<WritableProfilePath::forPrimary>()); case WritableProfilePath::forSecondary: - return BuildSecondaryRefProfilePath(final_path.get<WritableProfilePath::forSecondary>()); + return BuildSecondaryRefProfilePath(profile_path.get<WritableProfilePath::forSecondary>()); // No default. All cases should be explicitly handled, or the compilation will fail. } // This should never happen. Just in case we get a non-enumerator value. LOG(FATAL) << ART_FORMAT("Unexpected writable profile path type {}", - fmt::underlying(final_path.getTag())); + fmt::underlying(profile_path.getTag())); +} + +Result<std::string> BuildFinalProfilePath(const TmpProfilePath& tmp_profile_path) { + return BuildWritableProfilePath(tmp_profile_path.finalPath); } Result<std::string> BuildTmpProfilePath(const TmpProfilePath& tmp_profile_path) { @@ -329,6 +334,10 @@ bool PreRebootFlag(const VdexPath& vdex_path) { return PreRebootFlag(vdex_path.get<VdexPath::artifactsPath>()); } +bool IsPreRebootStagedFile(std::string_view filename) { + return EndsWith(filename, kPreRebootSuffix); +} + void TestOnlySetListRootDir(std::string_view root_dir) { gListRootDir = root_dir; } } // namespace artd diff --git a/artd/path_utils.h b/artd/path_utils.h index a4a6e4f2a7..1528d0610b 100644 --- a/artd/path_utils.h +++ b/artd/path_utils.h @@ -78,6 +78,9 @@ android::base::Result<std::string> BuildSecondaryCurProfilePath( const aidl::com::android::server::art::ProfilePath::SecondaryCurProfilePath& secondary_cur_profile_path); +android::base::Result<std::string> BuildWritableProfilePath( + const aidl::com::android::server::art::ProfilePath::WritableProfilePath& profile_path); + android::base::Result<std::string> BuildFinalProfilePath( const aidl::com::android::server::art::ProfilePath::TmpProfilePath& tmp_profile_path); @@ -125,6 +128,8 @@ bool PreRebootFlag(const aidl::com::android::server::art::ArtifactsPath& artifac bool PreRebootFlag(const aidl::com::android::server::art::OutputArtifacts& artifacts); bool PreRebootFlag(const aidl::com::android::server::art::VdexPath& vdex_path); +bool IsPreRebootStagedFile(std::string_view filename); + // Sets the root dir for `ListManagedFiles` and `ListRuntimeImageFiles`. // The passed string must be alive until the test ends. // For testing use only. diff --git a/build/apex/art.rc b/build/apex/art.rc index bab7fe30a3..8a98a5f670 100644 --- a/build/apex/art.rc +++ b/build/apex/art.rc @@ -26,7 +26,12 @@ service artd /apex/com.android.art/bin/artd # Same as above, but for Pre-reboot Dexopt. It runs in a chroot environment that # is set up by dexopt_chroot_setup. It's a lazy service that is started and # stopped dynamically as needed. -service artd_pre_reboot /apex/com.android.art/bin/art_exec --chroot=/mnt/pre_reboot_dexopt/chroot -- /apex/com.android.art/bin/artd --pre-reboot +service artd_pre_reboot /apex/com.android.art/bin/art_exec \ + --chroot=/mnt/pre_reboot_dexopt/chroot \ + --process-name-suffix="Pre-reboot Dexopt chroot" \ + -- \ + /apex/com.android.art/bin/artd \ + --pre-reboot interface aidl artd_pre_reboot disabled # Prevents the service from automatically starting at boot. oneshot # Prevents the service from automatically restarting each time it is stopped. diff --git a/compiler/dex/inline_method_analyser.cc b/compiler/dex/inline_method_analyser.cc index 85cf83c099..0567fe12c8 100644 --- a/compiler/dex/inline_method_analyser.cc +++ b/compiler/dex/inline_method_analyser.cc @@ -147,8 +147,11 @@ ArtMethod* GetTargetConstructor(ArtMethod* method, const Instruction* invoke_dir accessor.RegistersSize() - accessor.InsSize()); } uint32_t method_index = invoke_direct->VRegB_35c(); + StackHandleScope<2> hs(Thread::Current()); + Handle<mirror::DexCache> h_dex_cache = hs.NewHandle(method->GetDexCache()); + Handle<mirror::ClassLoader> h_class_loader = hs.NewHandle(method->GetClassLoader()); ArtMethod* target_method = Runtime::Current()->GetClassLinker()->LookupResolvedMethod( - method_index, method->GetDexCache(), method->GetClassLoader()); + method_index, h_dex_cache, h_class_loader); if (kIsDebugBuild && target_method != nullptr) { CHECK(!target_method->IsStatic()); CHECK(target_method->IsConstructor()); diff --git a/compiler/linker/linker_patch.h b/compiler/linker/linker_patch.h index 19ee0e640c..b061e042f0 100644 --- a/compiler/linker/linker_patch.h +++ b/compiler/linker/linker_patch.h @@ -54,6 +54,7 @@ class LinkerPatch { kJniEntrypointRelative, kCallRelative, kTypeRelative, + kTypeAppImageRelRo, kTypeBssEntry, kPublicTypeBssEntry, kPackageTypeBssEntry, @@ -130,6 +131,16 @@ class LinkerPatch { return patch; } + static LinkerPatch TypeAppImageRelRoPatch(size_t literal_offset, + const DexFile* target_dex_file, + uint32_t pc_insn_offset, + uint32_t target_type_idx) { + LinkerPatch patch(literal_offset, Type::kTypeAppImageRelRo, target_dex_file); + patch.type_idx_ = target_type_idx; + patch.pc_insn_offset_ = pc_insn_offset; + return patch; + } + static LinkerPatch TypeBssEntryPatch(size_t literal_offset, const DexFile* target_dex_file, uint32_t pc_insn_offset, @@ -241,6 +252,7 @@ class LinkerPatch { TypeReference TargetType() const { DCHECK(patch_type_ == Type::kTypeRelative || + patch_type_ == Type::kTypeAppImageRelRo || patch_type_ == Type::kTypeBssEntry || patch_type_ == Type::kPublicTypeBssEntry || patch_type_ == Type::kPackageTypeBssEntry); @@ -265,6 +277,7 @@ class LinkerPatch { patch_type_ == Type::kMethodBssEntry || patch_type_ == Type::kJniEntrypointRelative || patch_type_ == Type::kTypeRelative || + patch_type_ == Type::kTypeAppImageRelRo || patch_type_ == Type::kTypeBssEntry || patch_type_ == Type::kPublicTypeBssEntry || patch_type_ == Type::kPackageTypeBssEntry || diff --git a/compiler/optimizing/code_generator_arm64.cc b/compiler/optimizing/code_generator_arm64.cc index cfa28eda25..988809ee48 100644 --- a/compiler/optimizing/code_generator_arm64.cc +++ b/compiler/optimizing/code_generator_arm64.cc @@ -1011,6 +1011,7 @@ CodeGeneratorARM64::CodeGeneratorARM64(HGraph* graph, boot_image_method_patches_(graph->GetAllocator()->Adapter(kArenaAllocCodeGenerator)), method_bss_entry_patches_(graph->GetAllocator()->Adapter(kArenaAllocCodeGenerator)), boot_image_type_patches_(graph->GetAllocator()->Adapter(kArenaAllocCodeGenerator)), + app_image_type_patches_(graph->GetAllocator()->Adapter(kArenaAllocCodeGenerator)), type_bss_entry_patches_(graph->GetAllocator()->Adapter(kArenaAllocCodeGenerator)), public_type_bss_entry_patches_(graph->GetAllocator()->Adapter(kArenaAllocCodeGenerator)), package_type_bss_entry_patches_(graph->GetAllocator()->Adapter(kArenaAllocCodeGenerator)), @@ -5155,6 +5156,13 @@ vixl::aarch64::Label* CodeGeneratorARM64::NewBootImageTypePatch( return NewPcRelativePatch(&dex_file, type_index.index_, adrp_label, &boot_image_type_patches_); } +vixl::aarch64::Label* CodeGeneratorARM64::NewAppImageTypePatch( + const DexFile& dex_file, + dex::TypeIndex type_index, + vixl::aarch64::Label* adrp_label) { + return NewPcRelativePatch(&dex_file, type_index.index_, adrp_label, &app_image_type_patches_); +} + vixl::aarch64::Label* CodeGeneratorARM64::NewBssEntryTypePatch( HLoadClass* load_class, vixl::aarch64::Label* adrp_label) { @@ -5365,6 +5373,7 @@ void CodeGeneratorARM64::EmitLinkerPatches(ArenaVector<linker::LinkerPatch>* lin boot_image_method_patches_.size() + method_bss_entry_patches_.size() + boot_image_type_patches_.size() + + app_image_type_patches_.size() + type_bss_entry_patches_.size() + public_type_bss_entry_patches_.size() + package_type_bss_entry_patches_.size() + @@ -5387,12 +5396,15 @@ void CodeGeneratorARM64::EmitLinkerPatches(ArenaVector<linker::LinkerPatch>* lin DCHECK(boot_image_type_patches_.empty()); DCHECK(boot_image_string_patches_.empty()); } + DCHECK_IMPLIES(!GetCompilerOptions().IsAppImage(), app_image_type_patches_.empty()); if (GetCompilerOptions().IsBootImage()) { EmitPcRelativeLinkerPatches<NoDexFileAdapter<linker::LinkerPatch::IntrinsicReferencePatch>>( boot_image_other_patches_, linker_patches); } else { EmitPcRelativeLinkerPatches<NoDexFileAdapter<linker::LinkerPatch::BootImageRelRoPatch>>( boot_image_other_patches_, linker_patches); + EmitPcRelativeLinkerPatches<linker::LinkerPatch::TypeAppImageRelRoPatch>( + app_image_type_patches_, linker_patches); } EmitPcRelativeLinkerPatches<linker::LinkerPatch::MethodBssEntryPatch>( method_bss_entry_patches_, linker_patches); @@ -5504,6 +5516,7 @@ HLoadClass::LoadKind CodeGeneratorARM64::GetSupportedLoadClassKind( break; case HLoadClass::LoadKind::kBootImageLinkTimePcRelative: case HLoadClass::LoadKind::kBootImageRelRo: + case HLoadClass::LoadKind::kAppImageRelRo: case HLoadClass::LoadKind::kBssEntry: case HLoadClass::LoadKind::kBssEntryPublic: case HLoadClass::LoadKind::kBssEntryPackage: @@ -5613,6 +5626,20 @@ void InstructionCodeGeneratorARM64::VisitLoadClass(HLoadClass* cls) NO_THREAD_SA codegen_->LoadBootImageRelRoEntry(out.W(), boot_image_offset); break; } + case HLoadClass::LoadKind::kAppImageRelRo: { + DCHECK(codegen_->GetCompilerOptions().IsAppImage()); + DCHECK_EQ(read_barrier_option, kWithoutReadBarrier); + // Add ADRP with its PC-relative type patch. + const DexFile& dex_file = cls->GetDexFile(); + dex::TypeIndex type_index = cls->GetTypeIndex(); + vixl::aarch64::Label* adrp_label = codegen_->NewAppImageTypePatch(dex_file, type_index); + codegen_->EmitAdrpPlaceholder(adrp_label, out.X()); + // Add LDR with its PC-relative type patch. + vixl::aarch64::Label* ldr_label = + codegen_->NewAppImageTypePatch(dex_file, type_index, adrp_label); + codegen_->EmitLdrOffsetPlaceholder(ldr_label, out.W(), out.X()); + break; + } case HLoadClass::LoadKind::kBssEntry: case HLoadClass::LoadKind::kBssEntryPublic: case HLoadClass::LoadKind::kBssEntryPackage: { diff --git a/compiler/optimizing/code_generator_arm64.h b/compiler/optimizing/code_generator_arm64.h index 32a69a9df7..78049c5675 100644 --- a/compiler/optimizing/code_generator_arm64.h +++ b/compiler/optimizing/code_generator_arm64.h @@ -817,6 +817,14 @@ class CodeGeneratorARM64 : public CodeGenerator { dex::TypeIndex type_index, vixl::aarch64::Label* adrp_label = nullptr); + // Add a new app image type patch for an instruction and return the label + // to be bound before the instruction. The instruction will be either the + // ADRP (pass `adrp_label = null`) or the LDR (pass `adrp_label` pointing + // to the associated ADRP patch label). + vixl::aarch64::Label* NewAppImageTypePatch(const DexFile& dex_file, + dex::TypeIndex type_index, + vixl::aarch64::Label* adrp_label = nullptr); + // Add a new .bss entry type patch for an instruction and return the label // to be bound before the instruction. The instruction will be either the // ADRP (pass `adrp_label = null`) or the ADD (pass `adrp_label` pointing @@ -1150,6 +1158,8 @@ class CodeGeneratorARM64 : public CodeGenerator { ArenaDeque<PcRelativePatchInfo> method_bss_entry_patches_; // PC-relative type patch info for kBootImageLinkTimePcRelative. ArenaDeque<PcRelativePatchInfo> boot_image_type_patches_; + // PC-relative type patch info for kAppImageRelRo. + ArenaDeque<PcRelativePatchInfo> app_image_type_patches_; // PC-relative type patch info for kBssEntry. ArenaDeque<PcRelativePatchInfo> type_bss_entry_patches_; // PC-relative public type patch info for kBssEntryPublic. diff --git a/compiler/optimizing/code_generator_arm_vixl.cc b/compiler/optimizing/code_generator_arm_vixl.cc index a7cc5a6d12..7c1a9b21f2 100644 --- a/compiler/optimizing/code_generator_arm_vixl.cc +++ b/compiler/optimizing/code_generator_arm_vixl.cc @@ -1946,6 +1946,7 @@ CodeGeneratorARMVIXL::CodeGeneratorARMVIXL(HGraph* graph, boot_image_method_patches_(graph->GetAllocator()->Adapter(kArenaAllocCodeGenerator)), method_bss_entry_patches_(graph->GetAllocator()->Adapter(kArenaAllocCodeGenerator)), boot_image_type_patches_(graph->GetAllocator()->Adapter(kArenaAllocCodeGenerator)), + app_image_type_patches_(graph->GetAllocator()->Adapter(kArenaAllocCodeGenerator)), type_bss_entry_patches_(graph->GetAllocator()->Adapter(kArenaAllocCodeGenerator)), public_type_bss_entry_patches_(graph->GetAllocator()->Adapter(kArenaAllocCodeGenerator)), package_type_bss_entry_patches_(graph->GetAllocator()->Adapter(kArenaAllocCodeGenerator)), @@ -7657,6 +7658,7 @@ HLoadClass::LoadKind CodeGeneratorARMVIXL::GetSupportedLoadClassKind( break; case HLoadClass::LoadKind::kBootImageLinkTimePcRelative: case HLoadClass::LoadKind::kBootImageRelRo: + case HLoadClass::LoadKind::kAppImageRelRo: case HLoadClass::LoadKind::kBssEntry: case HLoadClass::LoadKind::kBssEntryPublic: case HLoadClass::LoadKind::kBssEntryPackage: @@ -7760,6 +7762,15 @@ void InstructionCodeGeneratorARMVIXL::VisitLoadClass(HLoadClass* cls) NO_THREAD_ codegen_->LoadBootImageRelRoEntry(out, boot_image_offset); break; } + case HLoadClass::LoadKind::kAppImageRelRo: { + DCHECK(codegen_->GetCompilerOptions().IsAppImage()); + DCHECK_EQ(read_barrier_option, kWithoutReadBarrier); + CodeGeneratorARMVIXL::PcRelativePatchInfo* labels = + codegen_->NewAppImageTypePatch(cls->GetDexFile(), cls->GetTypeIndex()); + codegen_->EmitMovwMovtPlaceholder(labels, out); + __ Ldr(out, MemOperand(out, /*offset=*/ 0)); + break; + } case HLoadClass::LoadKind::kBssEntry: case HLoadClass::LoadKind::kBssEntryPublic: case HLoadClass::LoadKind::kBssEntryPackage: { @@ -9691,6 +9702,11 @@ CodeGeneratorARMVIXL::PcRelativePatchInfo* CodeGeneratorARMVIXL::NewBootImageTyp return NewPcRelativePatch(&dex_file, type_index.index_, &boot_image_type_patches_); } +CodeGeneratorARMVIXL::PcRelativePatchInfo* CodeGeneratorARMVIXL::NewAppImageTypePatch( + const DexFile& dex_file, dex::TypeIndex type_index) { + return NewPcRelativePatch(&dex_file, type_index.index_, &app_image_type_patches_); +} + CodeGeneratorARMVIXL::PcRelativePatchInfo* CodeGeneratorARMVIXL::NewTypeBssEntryPatch( HLoadClass* load_class) { const DexFile& dex_file = load_class->GetDexFile(); @@ -9877,6 +9893,7 @@ void CodeGeneratorARMVIXL::EmitLinkerPatches(ArenaVector<linker::LinkerPatch>* l /* MOVW+MOVT for each entry */ 2u * boot_image_method_patches_.size() + /* MOVW+MOVT for each entry */ 2u * method_bss_entry_patches_.size() + /* MOVW+MOVT for each entry */ 2u * boot_image_type_patches_.size() + + /* MOVW+MOVT for each entry */ 2u * app_image_type_patches_.size() + /* MOVW+MOVT for each entry */ 2u * type_bss_entry_patches_.size() + /* MOVW+MOVT for each entry */ 2u * public_type_bss_entry_patches_.size() + /* MOVW+MOVT for each entry */ 2u * package_type_bss_entry_patches_.size() + @@ -9898,12 +9915,15 @@ void CodeGeneratorARMVIXL::EmitLinkerPatches(ArenaVector<linker::LinkerPatch>* l DCHECK(boot_image_type_patches_.empty()); DCHECK(boot_image_string_patches_.empty()); } + DCHECK_IMPLIES(!GetCompilerOptions().IsAppImage(), app_image_type_patches_.empty()); if (GetCompilerOptions().IsBootImage()) { EmitPcRelativeLinkerPatches<NoDexFileAdapter<linker::LinkerPatch::IntrinsicReferencePatch>>( boot_image_other_patches_, linker_patches); } else { EmitPcRelativeLinkerPatches<NoDexFileAdapter<linker::LinkerPatch::BootImageRelRoPatch>>( boot_image_other_patches_, linker_patches); + EmitPcRelativeLinkerPatches<linker::LinkerPatch::TypeAppImageRelRoPatch>( + app_image_type_patches_, linker_patches); } EmitPcRelativeLinkerPatches<linker::LinkerPatch::MethodBssEntryPatch>( method_bss_entry_patches_, linker_patches); diff --git a/compiler/optimizing/code_generator_arm_vixl.h b/compiler/optimizing/code_generator_arm_vixl.h index a62097d08f..51bee1cd77 100644 --- a/compiler/optimizing/code_generator_arm_vixl.h +++ b/compiler/optimizing/code_generator_arm_vixl.h @@ -709,6 +709,7 @@ class CodeGeneratorARMVIXL : public CodeGenerator { PcRelativePatchInfo* NewBootImageMethodPatch(MethodReference target_method); PcRelativePatchInfo* NewMethodBssEntryPatch(MethodReference target_method); PcRelativePatchInfo* NewBootImageTypePatch(const DexFile& dex_file, dex::TypeIndex type_index); + PcRelativePatchInfo* NewAppImageTypePatch(const DexFile& dex_file, dex::TypeIndex type_index); PcRelativePatchInfo* NewTypeBssEntryPatch(HLoadClass* load_class); PcRelativePatchInfo* NewBootImageStringPatch(const DexFile& dex_file, dex::StringIndex string_index); @@ -1029,6 +1030,8 @@ class CodeGeneratorARMVIXL : public CodeGenerator { ArenaDeque<PcRelativePatchInfo> method_bss_entry_patches_; // PC-relative type patch info for kBootImageLinkTimePcRelative. ArenaDeque<PcRelativePatchInfo> boot_image_type_patches_; + // PC-relative type patch info for kAppImageRelRo. + ArenaDeque<PcRelativePatchInfo> app_image_type_patches_; // PC-relative type patch info for kBssEntry. ArenaDeque<PcRelativePatchInfo> type_bss_entry_patches_; // PC-relative public type patch info for kBssEntryPublic. diff --git a/compiler/optimizing/code_generator_riscv64.cc b/compiler/optimizing/code_generator_riscv64.cc index c870db662c..9b499a08a2 100644 --- a/compiler/optimizing/code_generator_riscv64.cc +++ b/compiler/optimizing/code_generator_riscv64.cc @@ -3382,7 +3382,7 @@ TypeCheckKind type_check_kind = instruction->GetTypeCheckKind(); kWithoutReadBarrier); XRegister temp2 = maybe_temp2_loc.AsRegister<XRegister>(); XRegister temp3 = maybe_temp3_loc.AsRegister<XRegister>(); - // Iftable is never null. + // Load the size of the `IfTable`. The `Class::iftable_` is never null. __ Loadw(temp2, temp, array_length_offset); // Loop through the iftable and check if any class matches. Riscv64Label loop; @@ -3841,15 +3841,16 @@ void LocationsBuilderRISCV64::VisitInstanceOf(HInstanceOf* instruction) { case TypeCheckKind::kExactCheck: case TypeCheckKind::kAbstractClassCheck: case TypeCheckKind::kClassHierarchyCheck: - case TypeCheckKind::kArrayObjectCheck: { + case TypeCheckKind::kArrayObjectCheck: + case TypeCheckKind::kInterfaceCheck: { bool needs_read_barrier = codegen_->InstanceOfNeedsReadBarrier(instruction); call_kind = needs_read_barrier ? LocationSummary::kCallOnSlowPath : LocationSummary::kNoCall; - baker_read_barrier_slow_path = kUseBakerReadBarrier && needs_read_barrier; + baker_read_barrier_slow_path = (kUseBakerReadBarrier && needs_read_barrier) && + (type_check_kind != TypeCheckKind::kInterfaceCheck); break; } case TypeCheckKind::kArrayCheck: case TypeCheckKind::kUnresolvedCheck: - case TypeCheckKind::kInterfaceCheck: call_kind = LocationSummary::kCallOnSlowPath; break; case TypeCheckKind::kBitstringCheck: @@ -3889,10 +3890,14 @@ void InstructionCodeGeneratorRISCV64::VisitInstanceOf(HInstanceOf* instruction) const size_t num_temps = NumberOfInstanceOfTemps(codegen_->EmitReadBarrier(), type_check_kind); DCHECK_LE(num_temps, 1u); Location maybe_temp_loc = (num_temps >= 1) ? locations->GetTemp(0) : Location::NoLocation(); - uint32_t class_offset = mirror::Object::ClassOffset().Int32Value(); - uint32_t super_offset = mirror::Class::SuperClassOffset().Int32Value(); - uint32_t component_offset = mirror::Class::ComponentTypeOffset().Int32Value(); - uint32_t primitive_offset = mirror::Class::PrimitiveTypeOffset().Int32Value(); + const uint32_t class_offset = mirror::Object::ClassOffset().Int32Value(); + const uint32_t super_offset = mirror::Class::SuperClassOffset().Int32Value(); + const uint32_t component_offset = mirror::Class::ComponentTypeOffset().Int32Value(); + const uint32_t primitive_offset = mirror::Class::PrimitiveTypeOffset().Int32Value(); + const uint32_t iftable_offset = mirror::Class::IfTableOffset().Uint32Value(); + const uint32_t array_length_offset = mirror::Array::LengthOffset().Uint32Value(); + const uint32_t object_array_data_offset = + mirror::Array::DataOffset(kHeapReferenceSize).Uint32Value(); Riscv64Label done; SlowPathCodeRISCV64* slow_path = nullptr; @@ -3999,11 +4004,51 @@ void InstructionCodeGeneratorRISCV64::VisitInstanceOf(HInstanceOf* instruction) break; } - case TypeCheckKind::kUnresolvedCheck: case TypeCheckKind::kInterfaceCheck: { + if (codegen_->InstanceOfNeedsReadBarrier(instruction)) { + DCHECK(locations->OnlyCallsOnSlowPath()); + slow_path = new (codegen_->GetScopedAllocator()) TypeCheckSlowPathRISCV64( + instruction, /* is_fatal= */ false); + codegen_->AddSlowPath(slow_path); + if (codegen_->EmitNonBakerReadBarrier()) { + __ J(slow_path->GetEntryLabel()); + break; + } + // For Baker read barrier, take the slow path while marking. + __ Loadw(out, TR, Thread::IsGcMarkingOffset<kRiscv64PointerSize>().Int32Value()); + __ Bnez(out, slow_path->GetEntryLabel()); + } + + // Fast-path without read barriers. + ScratchRegisterScope srs(GetAssembler()); + XRegister temp = srs.AllocateXRegister(); + // /* HeapReference<Class> */ temp = obj->klass_ + __ Loadwu(temp, obj, class_offset); + codegen_->MaybeUnpoisonHeapReference(temp); + // /* HeapReference<Class> */ temp = temp->iftable_ + __ Loadwu(temp, temp, iftable_offset); + codegen_->MaybeUnpoisonHeapReference(temp); + // Load the size of the `IfTable`. The `Class::iftable_` is never null. + __ Loadw(out, temp, array_length_offset); + // Loop through the `IfTable` and check if any class matches. + Riscv64Label loop; + XRegister temp2 = srs.AllocateXRegister(); + __ Bind(&loop); + __ Beqz(out, &done); // If taken, the result in `out` is already 0 (false). + __ Loadwu(temp2, temp, object_array_data_offset); + codegen_->MaybeUnpoisonHeapReference(temp2); + // Go to next interface. + __ Addi(temp, temp, 2 * kHeapReferenceSize); + __ Addi(out, out, -2); + // Compare the classes and continue the loop if they do not match. + __ Bne(cls.AsRegister<XRegister>(), temp2, &loop); + __ LoadConst32(out, 1); + break; + } + + case TypeCheckKind::kUnresolvedCheck: { // Note that we indeed only call on slow path, but we always go - // into the slow path for the unresolved and interface check - // cases. + // into the slow path for the unresolved check case. // // We cannot directly call the InstanceofNonTrivial runtime // entry point without resorting to a type checking slow path @@ -4328,6 +4373,18 @@ void InstructionCodeGeneratorRISCV64::VisitLoadClass(HLoadClass* instruction) codegen_->LoadBootImageRelRoEntry(out, boot_image_offset); break; } + case HLoadClass::LoadKind::kAppImageRelRo: { + DCHECK(codegen_->GetCompilerOptions().IsAppImage()); + DCHECK_EQ(read_barrier_option, kWithoutReadBarrier); + CodeGeneratorRISCV64::PcRelativePatchInfo* info_high = + codegen_->NewAppImageTypePatch(instruction->GetDexFile(), instruction->GetTypeIndex()); + codegen_->EmitPcRelativeAuipcPlaceholder(info_high, out); + CodeGeneratorRISCV64::PcRelativePatchInfo* info_low = + codegen_->NewAppImageTypePatch( + instruction->GetDexFile(), instruction->GetTypeIndex(), info_high); + codegen_->EmitPcRelativeLwuPlaceholder(info_low, out, out); + break; + } case HLoadClass::LoadKind::kBssEntry: case HLoadClass::LoadKind::kBssEntryPublic: case HLoadClass::LoadKind::kBssEntryPackage: { @@ -5775,6 +5832,7 @@ CodeGeneratorRISCV64::CodeGeneratorRISCV64(HGraph* graph, boot_image_method_patches_(graph->GetAllocator()->Adapter(kArenaAllocCodeGenerator)), method_bss_entry_patches_(graph->GetAllocator()->Adapter(kArenaAllocCodeGenerator)), boot_image_type_patches_(graph->GetAllocator()->Adapter(kArenaAllocCodeGenerator)), + app_image_type_patches_(graph->GetAllocator()->Adapter(kArenaAllocCodeGenerator)), type_bss_entry_patches_(graph->GetAllocator()->Adapter(kArenaAllocCodeGenerator)), public_type_bss_entry_patches_(graph->GetAllocator()->Adapter(kArenaAllocCodeGenerator)), package_type_bss_entry_patches_(graph->GetAllocator()->Adapter(kArenaAllocCodeGenerator)), @@ -6383,6 +6441,7 @@ HLoadClass::LoadKind CodeGeneratorRISCV64::GetSupportedLoadClassKind( break; case HLoadClass::LoadKind::kBootImageLinkTimePcRelative: case HLoadClass::LoadKind::kBootImageRelRo: + case HLoadClass::LoadKind::kAppImageRelRo: case HLoadClass::LoadKind::kBssEntry: case HLoadClass::LoadKind::kBssEntryPublic: case HLoadClass::LoadKind::kBssEntryPackage: @@ -6434,6 +6493,11 @@ CodeGeneratorRISCV64::PcRelativePatchInfo* CodeGeneratorRISCV64::NewBootImageTyp return NewPcRelativePatch(&dex_file, type_index.index_, info_high, &boot_image_type_patches_); } +CodeGeneratorRISCV64::PcRelativePatchInfo* CodeGeneratorRISCV64::NewAppImageTypePatch( + const DexFile& dex_file, dex::TypeIndex type_index, const PcRelativePatchInfo* info_high) { + return NewPcRelativePatch(&dex_file, type_index.index_, info_high, &app_image_type_patches_); +} + CodeGeneratorRISCV64::PcRelativePatchInfo* CodeGeneratorRISCV64::NewBootImageJniEntrypointPatch( MethodReference target_method, const PcRelativePatchInfo* info_high) { return NewPcRelativePatch( @@ -6597,6 +6661,7 @@ void CodeGeneratorRISCV64::EmitLinkerPatches(ArenaVector<linker::LinkerPatch>* l boot_image_method_patches_.size() + method_bss_entry_patches_.size() + boot_image_type_patches_.size() + + app_image_type_patches_.size() + type_bss_entry_patches_.size() + public_type_bss_entry_patches_.size() + package_type_bss_entry_patches_.size() + @@ -6617,12 +6682,15 @@ void CodeGeneratorRISCV64::EmitLinkerPatches(ArenaVector<linker::LinkerPatch>* l DCHECK(boot_image_type_patches_.empty()); DCHECK(boot_image_string_patches_.empty()); } + DCHECK_IMPLIES(!GetCompilerOptions().IsAppImage(), app_image_type_patches_.empty()); if (GetCompilerOptions().IsBootImage()) { EmitPcRelativeLinkerPatches<NoDexFileAdapter<linker::LinkerPatch::IntrinsicReferencePatch>>( boot_image_other_patches_, linker_patches); } else { EmitPcRelativeLinkerPatches<NoDexFileAdapter<linker::LinkerPatch::BootImageRelRoPatch>>( boot_image_other_patches_, linker_patches); + EmitPcRelativeLinkerPatches<linker::LinkerPatch::TypeAppImageRelRoPatch>( + app_image_type_patches_, linker_patches); } EmitPcRelativeLinkerPatches<linker::LinkerPatch::MethodBssEntryPatch>( method_bss_entry_patches_, linker_patches); diff --git a/compiler/optimizing/code_generator_riscv64.h b/compiler/optimizing/code_generator_riscv64.h index c49e2b4771..227882e7b8 100644 --- a/compiler/optimizing/code_generator_riscv64.h +++ b/compiler/optimizing/code_generator_riscv64.h @@ -71,7 +71,6 @@ static constexpr int32_t kFClassNaNMinValue = 0x100; V(FP16LessEquals) \ V(FP16Min) \ V(FP16Max) \ - V(StringGetCharsNoCheck) \ V(StringStringIndexOf) \ V(StringStringIndexOfAfter) \ V(StringBufferAppend) \ @@ -590,6 +589,9 @@ class CodeGeneratorRISCV64 : public CodeGenerator { PcRelativePatchInfo* NewBootImageTypePatch(const DexFile& dex_file, dex::TypeIndex type_index, const PcRelativePatchInfo* info_high = nullptr); + PcRelativePatchInfo* NewAppImageTypePatch(const DexFile& dex_file, + dex::TypeIndex type_index, + const PcRelativePatchInfo* info_high = nullptr); PcRelativePatchInfo* NewTypeBssEntryPatch(HLoadClass* load_class, const PcRelativePatchInfo* info_high = nullptr); PcRelativePatchInfo* NewBootImageStringPatch(const DexFile& dex_file, @@ -820,6 +822,8 @@ class CodeGeneratorRISCV64 : public CodeGenerator { ArenaDeque<PcRelativePatchInfo> method_bss_entry_patches_; // PC-relative type patch info for kBootImageLinkTimePcRelative. ArenaDeque<PcRelativePatchInfo> boot_image_type_patches_; + // PC-relative type patch info for kAppImageRelRo. + ArenaDeque<PcRelativePatchInfo> app_image_type_patches_; // PC-relative type patch info for kBssEntry. ArenaDeque<PcRelativePatchInfo> type_bss_entry_patches_; // PC-relative public type patch info for kBssEntryPublic. diff --git a/compiler/optimizing/code_generator_x86.cc b/compiler/optimizing/code_generator_x86.cc index 5ad818dd53..4329e40efc 100644 --- a/compiler/optimizing/code_generator_x86.cc +++ b/compiler/optimizing/code_generator_x86.cc @@ -1173,6 +1173,7 @@ CodeGeneratorX86::CodeGeneratorX86(HGraph* graph, boot_image_method_patches_(graph->GetAllocator()->Adapter(kArenaAllocCodeGenerator)), method_bss_entry_patches_(graph->GetAllocator()->Adapter(kArenaAllocCodeGenerator)), boot_image_type_patches_(graph->GetAllocator()->Adapter(kArenaAllocCodeGenerator)), + app_image_type_patches_(graph->GetAllocator()->Adapter(kArenaAllocCodeGenerator)), type_bss_entry_patches_(graph->GetAllocator()->Adapter(kArenaAllocCodeGenerator)), public_type_bss_entry_patches_(graph->GetAllocator()->Adapter(kArenaAllocCodeGenerator)), package_type_bss_entry_patches_(graph->GetAllocator()->Adapter(kArenaAllocCodeGenerator)), @@ -5715,6 +5716,14 @@ void CodeGeneratorX86::RecordBootImageTypePatch(HLoadClass* load_class) { __ Bind(&boot_image_type_patches_.back().label); } +void CodeGeneratorX86::RecordAppImageTypePatch(HLoadClass* load_class) { + HX86ComputeBaseMethodAddress* method_address = + load_class->InputAt(0)->AsX86ComputeBaseMethodAddress(); + app_image_type_patches_.emplace_back( + method_address, &load_class->GetDexFile(), load_class->GetTypeIndex().index_); + __ Bind(&app_image_type_patches_.back().label); +} + Label* CodeGeneratorX86::NewTypeBssEntryPatch(HLoadClass* load_class) { HX86ComputeBaseMethodAddress* method_address = load_class->InputAt(0)->AsX86ComputeBaseMethodAddress(); @@ -5844,6 +5853,7 @@ void CodeGeneratorX86::EmitLinkerPatches(ArenaVector<linker::LinkerPatch>* linke boot_image_method_patches_.size() + method_bss_entry_patches_.size() + boot_image_type_patches_.size() + + app_image_type_patches_.size() + type_bss_entry_patches_.size() + public_type_bss_entry_patches_.size() + package_type_bss_entry_patches_.size() + @@ -5864,12 +5874,15 @@ void CodeGeneratorX86::EmitLinkerPatches(ArenaVector<linker::LinkerPatch>* linke DCHECK(boot_image_type_patches_.empty()); DCHECK(boot_image_string_patches_.empty()); } + DCHECK_IMPLIES(!GetCompilerOptions().IsAppImage(), app_image_type_patches_.empty()); if (GetCompilerOptions().IsBootImage()) { EmitPcRelativeLinkerPatches<NoDexFileAdapter<linker::LinkerPatch::IntrinsicReferencePatch>>( boot_image_other_patches_, linker_patches); } else { EmitPcRelativeLinkerPatches<NoDexFileAdapter<linker::LinkerPatch::BootImageRelRoPatch>>( boot_image_other_patches_, linker_patches); + EmitPcRelativeLinkerPatches<linker::LinkerPatch::TypeAppImageRelRoPatch>( + app_image_type_patches_, linker_patches); } EmitPcRelativeLinkerPatches<linker::LinkerPatch::MethodBssEntryPatch>( method_bss_entry_patches_, linker_patches); @@ -7283,6 +7296,7 @@ HLoadClass::LoadKind CodeGeneratorX86::GetSupportedLoadClassKind( break; case HLoadClass::LoadKind::kBootImageLinkTimePcRelative: case HLoadClass::LoadKind::kBootImageRelRo: + case HLoadClass::LoadKind::kAppImageRelRo: case HLoadClass::LoadKind::kBssEntry: case HLoadClass::LoadKind::kBssEntryPublic: case HLoadClass::LoadKind::kBssEntryPackage: @@ -7396,6 +7410,14 @@ void InstructionCodeGeneratorX86::VisitLoadClass(HLoadClass* cls) NO_THREAD_SAFE CodeGenerator::GetBootImageOffset(cls)); break; } + case HLoadClass::LoadKind::kAppImageRelRo: { + DCHECK(codegen_->GetCompilerOptions().IsAppImage()); + DCHECK_EQ(read_barrier_option, kWithoutReadBarrier); + Register method_address = locations->InAt(0).AsRegister<Register>(); + __ movl(out, Address(method_address, CodeGeneratorX86::kPlaceholder32BitOffset)); + codegen_->RecordAppImageTypePatch(cls); + break; + } case HLoadClass::LoadKind::kBssEntry: case HLoadClass::LoadKind::kBssEntryPublic: case HLoadClass::LoadKind::kBssEntryPackage: { diff --git a/compiler/optimizing/code_generator_x86.h b/compiler/optimizing/code_generator_x86.h index ee87947ca0..93f8e6ed9b 100644 --- a/compiler/optimizing/code_generator_x86.h +++ b/compiler/optimizing/code_generator_x86.h @@ -539,6 +539,7 @@ class CodeGeneratorX86 : public CodeGenerator { void RecordBootImageMethodPatch(HInvoke* invoke); void RecordMethodBssEntryPatch(HInvoke* invoke); void RecordBootImageTypePatch(HLoadClass* load_class); + void RecordAppImageTypePatch(HLoadClass* load_class); Label* NewTypeBssEntryPatch(HLoadClass* load_class); void RecordBootImageStringPatch(HLoadString* load_string); Label* NewStringBssEntryPatch(HLoadString* load_string); @@ -775,6 +776,8 @@ class CodeGeneratorX86 : public CodeGenerator { ArenaDeque<X86PcRelativePatchInfo> method_bss_entry_patches_; // PC-relative type patch info for kBootImageLinkTimePcRelative. ArenaDeque<X86PcRelativePatchInfo> boot_image_type_patches_; + // PC-relative type patch info for kAppImageRelRo. + ArenaDeque<X86PcRelativePatchInfo> app_image_type_patches_; // PC-relative type patch info for kBssEntry. ArenaDeque<X86PcRelativePatchInfo> type_bss_entry_patches_; // PC-relative public type patch info for kBssEntryPublic. diff --git a/compiler/optimizing/code_generator_x86_64.cc b/compiler/optimizing/code_generator_x86_64.cc index 4855b086ae..5b4f1b8b25 100644 --- a/compiler/optimizing/code_generator_x86_64.cc +++ b/compiler/optimizing/code_generator_x86_64.cc @@ -1328,6 +1328,12 @@ void CodeGeneratorX86_64::RecordBootImageTypePatch(const DexFile& dex_file, __ Bind(&boot_image_type_patches_.back().label); } +void CodeGeneratorX86_64::RecordAppImageTypePatch(const DexFile& dex_file, + dex::TypeIndex type_index) { + app_image_type_patches_.emplace_back(&dex_file, type_index.index_); + __ Bind(&app_image_type_patches_.back().label); +} + Label* CodeGeneratorX86_64::NewTypeBssEntryPatch(HLoadClass* load_class) { ArenaDeque<PatchInfo<Label>>* patches = nullptr; switch (load_class->GetLoadKind()) { @@ -1448,6 +1454,7 @@ void CodeGeneratorX86_64::EmitLinkerPatches(ArenaVector<linker::LinkerPatch>* li boot_image_method_patches_.size() + method_bss_entry_patches_.size() + boot_image_type_patches_.size() + + app_image_type_patches_.size() + type_bss_entry_patches_.size() + public_type_bss_entry_patches_.size() + package_type_bss_entry_patches_.size() + @@ -1469,12 +1476,15 @@ void CodeGeneratorX86_64::EmitLinkerPatches(ArenaVector<linker::LinkerPatch>* li DCHECK(boot_image_type_patches_.empty()); DCHECK(boot_image_string_patches_.empty()); } + DCHECK_IMPLIES(!GetCompilerOptions().IsAppImage(), app_image_type_patches_.empty()); if (GetCompilerOptions().IsBootImage()) { EmitPcRelativeLinkerPatches<NoDexFileAdapter<linker::LinkerPatch::IntrinsicReferencePatch>>( boot_image_other_patches_, linker_patches); } else { EmitPcRelativeLinkerPatches<NoDexFileAdapter<linker::LinkerPatch::BootImageRelRoPatch>>( boot_image_other_patches_, linker_patches); + EmitPcRelativeLinkerPatches<linker::LinkerPatch::TypeAppImageRelRoPatch>( + app_image_type_patches_, linker_patches); } EmitPcRelativeLinkerPatches<linker::LinkerPatch::MethodBssEntryPatch>( method_bss_entry_patches_, linker_patches); @@ -1607,6 +1617,7 @@ CodeGeneratorX86_64::CodeGeneratorX86_64(HGraph* graph, boot_image_method_patches_(graph->GetAllocator()->Adapter(kArenaAllocCodeGenerator)), method_bss_entry_patches_(graph->GetAllocator()->Adapter(kArenaAllocCodeGenerator)), boot_image_type_patches_(graph->GetAllocator()->Adapter(kArenaAllocCodeGenerator)), + app_image_type_patches_(graph->GetAllocator()->Adapter(kArenaAllocCodeGenerator)), type_bss_entry_patches_(graph->GetAllocator()->Adapter(kArenaAllocCodeGenerator)), public_type_bss_entry_patches_(graph->GetAllocator()->Adapter(kArenaAllocCodeGenerator)), package_type_bss_entry_patches_(graph->GetAllocator()->Adapter(kArenaAllocCodeGenerator)), @@ -6620,6 +6631,7 @@ HLoadClass::LoadKind CodeGeneratorX86_64::GetSupportedLoadClassKind( break; case HLoadClass::LoadKind::kBootImageLinkTimePcRelative: case HLoadClass::LoadKind::kBootImageRelRo: + case HLoadClass::LoadKind::kAppImageRelRo: case HLoadClass::LoadKind::kBssEntry: case HLoadClass::LoadKind::kBssEntryPublic: case HLoadClass::LoadKind::kBssEntryPackage: @@ -6732,6 +6744,14 @@ void InstructionCodeGeneratorX86_64::VisitLoadClass(HLoadClass* cls) NO_THREAD_S codegen_->RecordBootImageRelRoPatch(CodeGenerator::GetBootImageOffset(cls)); break; } + case HLoadClass::LoadKind::kAppImageRelRo: { + DCHECK(codegen_->GetCompilerOptions().IsAppImage()); + DCHECK_EQ(read_barrier_option, kWithoutReadBarrier); + __ movl(out, + Address::Absolute(CodeGeneratorX86_64::kPlaceholder32BitOffset, /* no_rip= */ false)); + codegen_->RecordAppImageTypePatch(cls->GetDexFile(), cls->GetTypeIndex()); + break; + } case HLoadClass::LoadKind::kBssEntry: case HLoadClass::LoadKind::kBssEntryPublic: case HLoadClass::LoadKind::kBssEntryPackage: { diff --git a/compiler/optimizing/code_generator_x86_64.h b/compiler/optimizing/code_generator_x86_64.h index b8e2456381..7a5e4aa894 100644 --- a/compiler/optimizing/code_generator_x86_64.h +++ b/compiler/optimizing/code_generator_x86_64.h @@ -532,6 +532,7 @@ class CodeGeneratorX86_64 : public CodeGenerator { void RecordBootImageMethodPatch(HInvoke* invoke); void RecordMethodBssEntryPatch(HInvoke* invoke); void RecordBootImageTypePatch(const DexFile& dex_file, dex::TypeIndex type_index); + void RecordAppImageTypePatch(const DexFile& dex_file, dex::TypeIndex type_index); Label* NewTypeBssEntryPatch(HLoadClass* load_class); void RecordBootImageStringPatch(HLoadString* load_string); Label* NewStringBssEntryPatch(HLoadString* load_string); @@ -738,6 +739,8 @@ class CodeGeneratorX86_64 : public CodeGenerator { ArenaDeque<PatchInfo<Label>> method_bss_entry_patches_; // PC-relative type patch info for kBootImageLinkTimePcRelative. ArenaDeque<PatchInfo<Label>> boot_image_type_patches_; + // PC-relative type patch info for kAppImageRelRo. + ArenaDeque<PatchInfo<Label>> app_image_type_patches_; // PC-relative type patch info for kBssEntry. ArenaDeque<PatchInfo<Label>> type_bss_entry_patches_; // PC-relative public type patch info for kBssEntryPublic. diff --git a/compiler/optimizing/intrinsics_riscv64.cc b/compiler/optimizing/intrinsics_riscv64.cc index 4e248a2b7c..9aa4b9415b 100644 --- a/compiler/optimizing/intrinsics_riscv64.cc +++ b/compiler/optimizing/intrinsics_riscv64.cc @@ -5043,6 +5043,185 @@ void IntrinsicCodeGeneratorRISCV64::VisitMathMultiplyHigh(HInvoke* invoke) { __ Mulh(out, x, y); } +void IntrinsicLocationsBuilderRISCV64::VisitStringGetCharsNoCheck(HInvoke* invoke) { + LocationSummary* locations = + new (allocator_) LocationSummary(invoke, LocationSummary::kNoCall, kIntrinsified); + + locations->SetInAt(0, Location::RequiresRegister()); + locations->SetInAt(1, Location::RequiresRegister()); + locations->SetInAt(2, Location::RequiresRegister()); + locations->SetInAt(3, Location::RequiresRegister()); + locations->SetInAt(4, Location::RequiresRegister()); + + locations->AddTemp(Location::RequiresRegister()); + locations->AddTemp(Location::RequiresRegister()); + locations->AddTemp(Location::RequiresRegister()); +} + +void IntrinsicCodeGeneratorRISCV64::VisitStringGetCharsNoCheck(HInvoke* invoke) { + Riscv64Assembler* assembler = GetAssembler(); + LocationSummary* locations = invoke->GetLocations(); + + // In Java sizeof(Char) is 2. + constexpr size_t char_size = DataType::Size(DataType::Type::kUint16); + static_assert(char_size == 2u); + + // Location of data in the destination char array buffer. + const uint32_t array_data_offset = mirror::Array::DataOffset(char_size).Uint32Value(); + + // Location of char array data in the source string. + const uint32_t string_value_offset = mirror::String::ValueOffset().Uint32Value(); + + // void getCharsNoCheck(int srcBegin, int srcEnd, char[] dst, int dstBegin); + + // The source string. + XRegister source_string_object = locations->InAt(0).AsRegister<XRegister>(); + // Index of the first character. + XRegister source_begin_index = locations->InAt(1).AsRegister<XRegister>(); + // Index that immediately follows the last character. + XRegister source_end_index = locations->InAt(2).AsRegister<XRegister>(); + // The destination array. + XRegister destination_array_object = locations->InAt(3).AsRegister<XRegister>(); + // The start offset in the destination array. + XRegister destination_begin_offset = locations->InAt(4).AsRegister<XRegister>(); + + XRegister source_ptr = locations->GetTemp(0).AsRegister<XRegister>(); + XRegister destination_ptr = locations->GetTemp(1).AsRegister<XRegister>(); + XRegister number_of_chars = locations->GetTemp(2).AsRegister<XRegister>(); + + ScratchRegisterScope temps(assembler); + XRegister tmp = temps.AllocateXRegister(); + + Riscv64Label done; + + // Calculate the length(number_of_chars) of the string. + __ Subw(number_of_chars, source_end_index, source_begin_index); + + // If the string has zero length then exit. + __ Beqz(number_of_chars, &done); + + // Prepare a register with the destination address + // to start copying to the address: + // 1. set the address from which the data in the + // destination array begins (destination_array_object + array_data_offset); + __ Addi(destination_ptr, destination_array_object, array_data_offset); + // 2. it is necessary to add the start offset relative to the beginning + // of the data in the destination array, + // yet, due to sizeof(Char) being 2, formerly scaling must be performed + // (destination_begin_offset * 2 that equals to destination_begin_offset << 1); + __ Sh1Add(destination_ptr, destination_begin_offset, destination_ptr); + + // Prepare a register with the source address + // to start copying from the address: + // 1. set the address from which the data in the + // source string begins (source_string_object + string_value_offset). + // Other manipulations will be performed later, + // since they depend on whether the string is compressed or not. + __ Addi(source_ptr, source_string_object, string_value_offset); + + // The string can be compressed. It is a way to store strings more compactly. + // In this instance, every character is located in one byte (instead of two). + Riscv64Label compressed_string_preloop; + + // Information about whether the string is compressed or not is located + // in the area intended for storing the length of the string. + // The least significant bit of the string's length is used + // as the compression flag if STRING_COMPRESSION_ENABLED. + if (mirror::kUseStringCompression) { + // Location of count in string. + const uint32_t count_offset = mirror::String::CountOffset().Uint32Value(); + // String's length. + __ Loadwu(tmp, source_string_object, count_offset); + + // Checking the string for compression. + // If so, move to the "compressed_string_preloop". + __ Andi(tmp, tmp, 0x1); + __ Beqz(tmp, &compressed_string_preloop); + } + + // Continue preparing the source register: + // proceed similarly to what was done for the destination register. + __ Sh1Add(source_ptr, source_begin_index, source_ptr); + + // If the string is not compressed, then perform ordinary copying. + // Copying will occur 4 characters (8 bytes) at a time, immediately after there are + // less than 4 characters left, move to the "remainder_loop" and copy the remaining + // characters one character (2 bytes) at a time. + // Note: Unaligned addresses are acceptable here and it is not required to embed + // additional code to correct them. + Riscv64Label main_loop; + Riscv64Label remainder_loop; + + // If initially there are less than 4 characters, + // then we directly calculate the remainder. + __ Addi(tmp, number_of_chars, -4); + __ Bltz(tmp, &remainder_loop); + + // Otherwise, save the value to the counter and continue. + __ Mv(number_of_chars, tmp); + + // Main loop. Loads and stores 4 16-bit Java characters at a time. + __ Bind(&main_loop); + + __ Loadd(tmp, source_ptr, 0); + __ Addi(source_ptr, source_ptr, char_size * 4); + __ Stored(tmp, destination_ptr, 0); + __ Addi(destination_ptr, destination_ptr, char_size * 4); + + __ Addi(number_of_chars, number_of_chars, -4); + + __ Bgez(number_of_chars, &main_loop); + + // Restore the previous counter value. + __ Addi(number_of_chars, number_of_chars, 4); + __ Beqz(number_of_chars, &done); + + // Remainder loop for < 4 characters case and remainder handling. + // Loads and stores one 16-bit Java character at a time. + __ Bind(&remainder_loop); + + __ Loadhu(tmp, source_ptr, 0); + __ Addi(source_ptr, source_ptr, char_size); + + __ Storeh(tmp, destination_ptr, 0); + __ Addi(destination_ptr, destination_ptr, char_size); + + __ Addi(number_of_chars, number_of_chars, -1); + __ Bgtz(number_of_chars, &remainder_loop); + + Riscv64Label compressed_string_loop; + if (mirror::kUseStringCompression) { + __ J(&done); + + // Below is the copying under the string compression circumstance mentioned above. + // Every character in the source string occupies only one byte (instead of two). + constexpr size_t compressed_char_size = DataType::Size(DataType::Type::kInt8); + static_assert(compressed_char_size == 1u); + + __ Bind(&compressed_string_preloop); + + // Continue preparing the source register: + // proceed identically to what was done for the destination register, + // yet take into account that only one byte yields for every source character, + // hence we need to extend it to two ones when copying it to the destination address. + // Against this background scaling for source_begin_index is not needed. + __ Add(source_ptr, source_ptr, source_begin_index); + + // Copy loop for compressed strings. Copying one 8-bit character to 16-bit one at a time. + __ Bind(&compressed_string_loop); + + __ Loadbu(tmp, source_ptr, 0); + __ Addi(source_ptr, source_ptr, compressed_char_size); + __ Storeh(tmp, destination_ptr, 0); + __ Addi(destination_ptr, destination_ptr, char_size); + + __ Addi(number_of_chars, number_of_chars, -1); + __ Bgtz(number_of_chars, &compressed_string_loop); + } + + __ Bind(&done); +} + #define MARK_UNIMPLEMENTED(Name) UNIMPLEMENTED_INTRINSIC(RISCV64, Name) UNIMPLEMENTED_INTRINSIC_LIST_RISCV64(MARK_UNIMPLEMENTED); #undef MARK_UNIMPLEMENTED diff --git a/compiler/optimizing/nodes.h b/compiler/optimizing/nodes.h index 90fc5db02e..33ffc07ba8 100644 --- a/compiler/optimizing/nodes.h +++ b/compiler/optimizing/nodes.h @@ -6743,6 +6743,10 @@ class HLoadClass final : public HInstruction { // Used for boot image classes referenced by apps in AOT-compiled code. kBootImageRelRo, + // Load from an app image entry in the .data.img.rel.ro using a PC-relative load. + // Used for app image classes referenced by apps in AOT-compiled code. + kAppImageRelRo, + // Load from an entry in the .bss section using a PC-relative load. // Used for classes outside boot image referenced by AOT-compiled app and boot image code. kBssEntry, @@ -6814,6 +6818,7 @@ class HLoadClass final : public HInstruction { bool HasPcRelativeLoadKind() const { return GetLoadKind() == LoadKind::kBootImageLinkTimePcRelative || GetLoadKind() == LoadKind::kBootImageRelRo || + GetLoadKind() == LoadKind::kAppImageRelRo || GetLoadKind() == LoadKind::kBssEntry || GetLoadKind() == LoadKind::kBssEntryPublic || GetLoadKind() == LoadKind::kBssEntryPackage; @@ -6933,6 +6938,7 @@ class HLoadClass final : public HInstruction { static bool HasTypeReference(LoadKind load_kind) { return load_kind == LoadKind::kReferrersClass || load_kind == LoadKind::kBootImageLinkTimePcRelative || + load_kind == LoadKind::kAppImageRelRo || load_kind == LoadKind::kBssEntry || load_kind == LoadKind::kBssEntryPublic || load_kind == LoadKind::kBssEntryPackage || @@ -6978,6 +6984,7 @@ inline void HLoadClass::AddSpecialInput(HInstruction* special_input) { // including literal pool loads, which are PC-relative too. DCHECK(GetLoadKind() == LoadKind::kBootImageLinkTimePcRelative || GetLoadKind() == LoadKind::kBootImageRelRo || + GetLoadKind() == LoadKind::kAppImageRelRo || GetLoadKind() == LoadKind::kBssEntry || GetLoadKind() == LoadKind::kBssEntryPublic || GetLoadKind() == LoadKind::kBssEntryPackage || diff --git a/compiler/optimizing/optimizing_compiler.cc b/compiler/optimizing/optimizing_compiler.cc index 45d534a9ec..a75ac9239b 100644 --- a/compiler/optimizing/optimizing_compiler.cc +++ b/compiler/optimizing/optimizing_compiler.cc @@ -1183,14 +1183,14 @@ CompiledMethod* OptimizingCompiler::JniCompile(uint32_t access_flags, const CompilerOptions& compiler_options = GetCompilerOptions(); if (compiler_options.IsBootImage()) { ScopedObjectAccess soa(Thread::Current()); - ArtMethod* method = runtime->GetClassLinker()->LookupResolvedMethod( - method_idx, dex_cache.Get(), /*class_loader=*/ nullptr); + VariableSizedHandleScope handles(soa.Self()); + ScopedNullHandle<mirror::ClassLoader> class_loader; // null means boot class path loader. + ArtMethod* method = + runtime->GetClassLinker()->LookupResolvedMethod(method_idx, dex_cache, class_loader); // Try to compile a fully intrinsified implementation. Do not try to do this for // signature polymorphic methods as the InstructionBuilder cannot handle them; // and it would be useless as they always have a slow path for type conversions. if (method != nullptr && UNLIKELY(method->IsIntrinsic()) && !method->IsSignaturePolymorphic()) { - VariableSizedHandleScope handles(soa.Self()); - ScopedNullHandle<mirror::ClassLoader> class_loader; // null means boot class path loader. Handle<mirror::Class> compiling_class = handles.NewHandle(method->GetDeclaringClass()); DexCompilationUnit dex_compilation_unit( class_loader, diff --git a/compiler/optimizing/sharpening.cc b/compiler/optimizing/sharpening.cc index aa75c2464b..cb94491b8e 100644 --- a/compiler/optimizing/sharpening.cc +++ b/compiler/optimizing/sharpening.cc @@ -277,7 +277,7 @@ HLoadClass::LoadKind HSharpening::ComputeLoadClassKind( } else if (compiler_options.IsAppImage() && is_class_in_current_image()) { // AOT app compilation, app image class. is_in_image = true; - desired_load_kind = HLoadClass::LoadKind::kBssEntry; + desired_load_kind = HLoadClass::LoadKind::kAppImageRelRo; } else { // Not JIT and the klass is not in boot image or app image. desired_load_kind = HLoadClass::LoadKind::kBssEntry; diff --git a/dex2oat/dex2oat.cc b/dex2oat/dex2oat.cc index c2c4fb27a1..9529e6d28b 100644 --- a/dex2oat/dex2oat.cc +++ b/dex2oat/dex2oat.cc @@ -1966,6 +1966,9 @@ class Dex2Oat final { } } } + if (IsAppImage()) { + AotClassLinker::SetAppImageDexFiles(&compiler_options_->GetDexFilesForOatFile()); + } // Register dex caches and key them to the class loader so that they only unload when the // class loader unloads. @@ -2127,6 +2130,7 @@ class Dex2Oat final { elf_writer->PrepareDynamicSection(oat_writer->GetOatHeader().GetExecutableOffset(), oat_writer->GetCodeSize(), oat_writer->GetDataImgRelRoSize(), + oat_writer->GetDataImgRelRoAppImageOffset(), oat_writer->GetBssSize(), oat_writer->GetBssMethodsOffset(), oat_writer->GetBssRootsOffset(), diff --git a/dex2oat/driver/compiler_driver.cc b/dex2oat/driver/compiler_driver.cc index 329c0d76a1..f7f86ed4fc 100644 --- a/dex2oat/driver/compiler_driver.cc +++ b/dex2oat/driver/compiler_driver.cc @@ -43,6 +43,7 @@ #include "base/timing_logger.h" #include "class_linker-inl.h" #include "class_root-inl.h" +#include "common_throws.h" #include "compiled_method-inl.h" #include "compiler.h" #include "compiler_callbacks.h" @@ -82,7 +83,6 @@ #include "thread_list.h" #include "thread_pool.h" #include "trampolines/trampoline_compiler.h" -#include "transaction.h" #include "utils/atomic_dex_ref_map-inl.h" #include "utils/swap_space.h" #include "vdex_file.h" @@ -889,6 +889,12 @@ void CompilerDriver::PreCompile(jobject class_loader, << "Please check the log."; _exit(1); } + + if (GetCompilerOptions().IsAppImage() && had_hard_verifier_failure_) { + // Prune erroneous classes and classes that depend on them. + UpdateImageClasses(timings, image_classes); + VLOG(compiler) << "verify/UpdateImageClasses: " << GetMemoryUsageString(false); + } } if (GetCompilerOptions().IsGeneratingImage()) { @@ -911,8 +917,10 @@ void CompilerDriver::PreCompile(jobject class_loader, visitor.FillAllIMTAndConflictTables(); } - UpdateImageClasses(timings, image_classes); - VLOG(compiler) << "UpdateImageClasses: " << GetMemoryUsageString(false); + if (GetCompilerOptions().IsBootImage() || GetCompilerOptions().IsBootImageExtension()) { + UpdateImageClasses(timings, image_classes); + VLOG(compiler) << "UpdateImageClasses: " << GetMemoryUsageString(false); + } if (kBitstringSubtypeCheckEnabled && GetCompilerOptions().IsForceDeterminism() && GetCompilerOptions().IsBootImage()) { @@ -1380,7 +1388,8 @@ class ClinitImageUpdate { bool operator()(ObjPtr<mirror::Class> klass) override REQUIRES_SHARED(Locks::mutator_lock_) { bool resolved = klass->IsResolved(); DCHECK(resolved || klass->IsErroneousUnresolved()); - bool can_include_in_image = LIKELY(resolved) && CanIncludeInCurrentImage(klass); + bool can_include_in_image = + LIKELY(resolved) && LIKELY(!klass->IsErroneous()) && CanIncludeInCurrentImage(klass); std::string temp; std::string_view descriptor(klass->GetDescriptor(&temp)); auto it = data_->image_class_descriptors_->find(descriptor); @@ -1451,17 +1460,16 @@ class ClinitImageUpdate { void CompilerDriver::UpdateImageClasses(TimingLogger* timings, /*inout*/ HashSet<std::string>* image_classes) { - if (GetCompilerOptions().IsBootImage() || GetCompilerOptions().IsBootImageExtension()) { - TimingLogger::ScopedTiming t("UpdateImageClasses", timings); + DCHECK(GetCompilerOptions().IsGeneratingImage()); + TimingLogger::ScopedTiming t("UpdateImageClasses", timings); - // Suspend all threads. - ScopedSuspendAll ssa(__FUNCTION__); + // Suspend all threads. + ScopedSuspendAll ssa(__FUNCTION__); - ClinitImageUpdate update(image_classes, Thread::Current()); + ClinitImageUpdate update(image_classes, Thread::Current()); - // Do the marking. - update.Walk(); - } + // Do the marking. + update.Walk(); } void CompilerDriver::ProcessedInstanceField(bool resolved) { @@ -1641,7 +1649,7 @@ class ParallelCompilationManager { static bool SkipClass(jobject class_loader, const DexFile& dex_file, ObjPtr<mirror::Class> klass) REQUIRES_SHARED(Locks::mutator_lock_) { DCHECK(klass != nullptr); - const DexFile& original_dex_file = *klass->GetDexCache()->GetDexFile(); + const DexFile& original_dex_file = klass->GetDexFile(); if (&dex_file != &original_dex_file) { if (class_loader == nullptr) { LOG(WARNING) << "Skipping class " << klass->PrettyDescriptor() << " from " @@ -2029,17 +2037,18 @@ class VerifyClassVisitor : public CompilationVisitor { break; } } - } else if (&klass->GetDexFile() != &dex_file) { + } else if (SkipClass(jclass_loader, dex_file, klass.Get())) { // Skip a duplicate class (as the resolved class is from another, earlier dex file). return; // Do not update state. - } else if (!SkipClass(jclass_loader, dex_file, klass.Get())) { + } else { CHECK(klass->IsResolved()) << klass->PrettyClass(); failure_kind = class_linker->VerifyClass(soa.Self(), soa.Self()->GetVerifierDeps(), klass, log_level_); - if (klass->IsErroneous()) { + DCHECK_EQ(klass->IsErroneous(), failure_kind == verifier::FailureKind::kHardFailure); + if (failure_kind == verifier::FailureKind::kHardFailure) { // ClassLinker::VerifyClass throws, which isn't useful in the compiler. CHECK(soa.Self()->IsExceptionPending()); soa.Self()->ClearException(); @@ -2092,9 +2101,6 @@ class VerifyClassVisitor : public CompilationVisitor { DCHECK_EQ(failure_kind, verifier::FailureKind::kHardFailure); } } - } else { - // Make the skip a soft failure, essentially being considered as verify at runtime. - failure_kind = verifier::FailureKind::kSoftFailure; } verifier::VerifierDeps::MaybeRecordVerificationStatus(soa.Self()->GetVerifierDeps(), dex_file, @@ -2332,10 +2338,8 @@ class InitializeClassVisitor : public CompilationVisitor { // the transaction aborts and cannot resolve the type. // TransactionAbortError is not initialized ant not in boot image, needed only by // compiler and will be pruned by ImageWriter. - Handle<mirror::Class> exception_class = - hs.NewHandle(class_linker->FindClass(self, - Transaction::kAbortExceptionDescriptor, - class_loader)); + Handle<mirror::Class> exception_class = hs.NewHandle( + class_linker->FindClass(self, kTransactionAbortErrorDescriptor, class_loader)); bool exception_initialized = class_linker->EnsureInitialized(self, exception_class, true, true); DCHECK(exception_initialized); @@ -2708,8 +2712,6 @@ static void CompileDexFile(CompilerDriver* driver, soa.Self()->ClearException(); dex_cache = hs.NewHandle(class_linker->FindDexCache(soa.Self(), dex_file)); } else if (SkipClass(jclass_loader, dex_file, klass.Get())) { - return; - } else if (&klass->GetDexFile() != &dex_file) { // Skip a duplicate class (as the resolved class is from another, earlier dex file). return; // Do not update state. } else { diff --git a/dex2oat/linker/arm64/relative_patcher_arm64.cc b/dex2oat/linker/arm64/relative_patcher_arm64.cc index 8772974c4b..2956285545 100644 --- a/dex2oat/linker/arm64/relative_patcher_arm64.cc +++ b/dex2oat/linker/arm64/relative_patcher_arm64.cc @@ -66,6 +66,7 @@ inline bool IsAdrpPatch(const LinkerPatch& patch) { case LinkerPatch::Type::kMethodBssEntry: case LinkerPatch::Type::kJniEntrypointRelative: case LinkerPatch::Type::kTypeRelative: + case LinkerPatch::Type::kTypeAppImageRelRo: case LinkerPatch::Type::kTypeBssEntry: case LinkerPatch::Type::kPublicTypeBssEntry: case LinkerPatch::Type::kPackageTypeBssEntry: @@ -274,6 +275,7 @@ void Arm64RelativePatcher::PatchPcRelativeReference(std::vector<uint8_t>* code, DCHECK(patch.GetType() == LinkerPatch::Type::kBootImageRelRo || patch.GetType() == LinkerPatch::Type::kMethodBssEntry || patch.GetType() == LinkerPatch::Type::kJniEntrypointRelative || + patch.GetType() == LinkerPatch::Type::kTypeAppImageRelRo || patch.GetType() == LinkerPatch::Type::kTypeBssEntry || patch.GetType() == LinkerPatch::Type::kPublicTypeBssEntry || patch.GetType() == LinkerPatch::Type::kPackageTypeBssEntry || diff --git a/dex2oat/linker/elf_writer.h b/dex2oat/linker/elf_writer.h index a1d0ece9de..35e3565592 100644 --- a/dex2oat/linker/elf_writer.h +++ b/dex2oat/linker/elf_writer.h @@ -61,6 +61,7 @@ class ElfWriter { virtual void PrepareDynamicSection(size_t rodata_size, size_t text_size, size_t data_img_rel_ro_size, + size_t data_img_rel_ro_app_image_offset, size_t bss_size, size_t bss_methods_offset, size_t bss_roots_offset, diff --git a/dex2oat/linker/elf_writer_quick.cc b/dex2oat/linker/elf_writer_quick.cc index 373c46ce2c..f87ca6d81e 100644 --- a/dex2oat/linker/elf_writer_quick.cc +++ b/dex2oat/linker/elf_writer_quick.cc @@ -92,6 +92,7 @@ class ElfWriterQuick final : public ElfWriter { void PrepareDynamicSection(size_t rodata_size, size_t text_size, size_t data_img_rel_ro_size, + size_t data_img_rel_ro_app_image_offset, size_t bss_size, size_t bss_methods_offset, size_t bss_roots_offset, @@ -173,6 +174,7 @@ template <typename ElfTypes> void ElfWriterQuick<ElfTypes>::PrepareDynamicSection(size_t rodata_size, size_t text_size, size_t data_img_rel_ro_size, + size_t data_img_rel_ro_app_image_offset, size_t bss_size, size_t bss_methods_offset, size_t bss_roots_offset, @@ -191,6 +193,7 @@ void ElfWriterQuick<ElfTypes>::PrepareDynamicSection(size_t rodata_size, rodata_size_, text_size_, data_img_rel_ro_size_, + data_img_rel_ro_app_image_offset, bss_size_, bss_methods_offset, bss_roots_offset, diff --git a/dex2oat/linker/image_test.h b/dex2oat/linker/image_test.h index 9f553c1c74..f4a87bb84d 100644 --- a/dex2oat/linker/image_test.h +++ b/dex2oat/linker/image_test.h @@ -309,6 +309,7 @@ inline void ImageTest::DoCompile(ImageHeader::StorageMode storage_mode, elf_writer->PrepareDynamicSection(oat_writer->GetOatHeader().GetExecutableOffset(), oat_writer->GetCodeSize(), oat_writer->GetDataImgRelRoSize(), + oat_writer->GetDataImgRelRoAppImageOffset(), oat_writer->GetBssSize(), oat_writer->GetBssMethodsOffset(), oat_writer->GetBssRootsOffset(), diff --git a/dex2oat/linker/image_writer.cc b/dex2oat/linker/image_writer.cc index 63ac71fc12..abc3354421 100644 --- a/dex2oat/linker/image_writer.cc +++ b/dex2oat/linker/image_writer.cc @@ -1101,7 +1101,17 @@ bool ImageWriter::KeepClass(ObjPtr<mirror::Class> klass) { // the boot image spaces since these may have already been loaded at // run time when this image is loaded. Keep classes in the boot image // spaces we're compiling against since we don't want to re-resolve these. - return !PruneImageClass(klass); + // FIXME: Update image classes in the `CompilerOptions` after initializing classes + // with `--initialize-app-image-classes=true`. This experimental flag can currently + // cause an inconsistency between `CompilerOptions::IsImageClass()` and what actually + // ends up in the app image as seen in the run-test `660-clinit` where the class + // `ObjectRef` is considered an app image class during compilation but in the end + // it's pruned here. This inconsistency should be fixed if we want to properly + // initialize app image classes. b/38313278 + bool keep = !PruneImageClass(klass); + CHECK_IMPLIES(!compiler_options_.InitializeAppImageClasses(), keep) + << klass->PrettyDescriptor(); + return keep; } return true; } diff --git a/dex2oat/linker/image_writer.h b/dex2oat/linker/image_writer.h index 4d45529871..8ffe226bec 100644 --- a/dex2oat/linker/image_writer.h +++ b/dex2oat/linker/image_writer.h @@ -128,6 +128,15 @@ class ImageWriter final { } } + uint32_t GetGlobalImageOffset(mirror::Object* object) const REQUIRES_SHARED(Locks::mutator_lock_) { + DCHECK(object != nullptr); + DCHECK(!IsInBootImage(object)); + size_t oat_index = GetOatIndex(object); + const ImageInfo& image_info = GetImageInfo(oat_index); + return dchecked_integral_cast<uint32_t>( + image_info.image_begin_ + GetImageOffset(object, oat_index) - global_image_begin_); + } + ArtMethod* GetImageMethodAddress(ArtMethod* method) REQUIRES_SHARED(Locks::mutator_lock_); const void* GetIntrinsicReferenceAddress(uint32_t intrinsic_data) REQUIRES_SHARED(Locks::mutator_lock_); diff --git a/dex2oat/linker/oat_writer.cc b/dex2oat/linker/oat_writer.cc index 0dbb4bb05b..7c2f8d6249 100644 --- a/dex2oat/linker/oat_writer.cc +++ b/dex2oat/linker/oat_writer.cc @@ -373,6 +373,7 @@ OatWriter::OatWriter(const CompilerOptions& compiler_options, oat_size_(0u), data_img_rel_ro_start_(0u), data_img_rel_ro_size_(0u), + data_img_rel_ro_app_image_offset_(0u), bss_start_(0u), bss_size_(0u), bss_methods_offset_(0u), @@ -380,6 +381,7 @@ OatWriter::OatWriter(const CompilerOptions& compiler_options, boot_image_rel_ro_entries_(), bss_method_entry_references_(), bss_method_entries_(), + app_image_rel_ro_type_entries_(), bss_type_entries_(), bss_public_type_entries_(), bss_package_type_entries_(), @@ -754,6 +756,9 @@ class OatWriter::InitBssLayoutMethodVisitor : public DexMethodVisitor { target_method.dex_file->NumMethodIds(), &writer_->bss_method_entry_references_); writer_->bss_method_entries_.Overwrite(target_method, /* placeholder */ 0u); + } else if (patch.GetType() == LinkerPatch::Type::kTypeAppImageRelRo) { + writer_->app_image_rel_ro_type_entries_.Overwrite(patch.TargetType(), + /* placeholder */ 0u); } else if (patch.GetType() == LinkerPatch::Type::kTypeBssEntry) { TypeReference target_type = patch.TargetType(); AddBssReference(target_type, @@ -1710,6 +1715,16 @@ class OatWriter::WriteCodeMethodVisitor : public OrderedMethodVisitor { target_offset); break; } + case LinkerPatch::Type::kTypeAppImageRelRo: { + uint32_t target_offset = + writer_->data_img_rel_ro_start_ + + writer_->app_image_rel_ro_type_entries_.Get(patch.TargetType()); + writer_->relative_patcher_->PatchPcRelativeReference(&patched_code_, + patch, + offset_ + literal_offset, + target_offset); + break; + } case LinkerPatch::Type::kTypeBssEntry: { uint32_t target_offset = writer_->bss_start_ + writer_->bss_type_entries_.Get(patch.TargetType()); @@ -1822,11 +1837,12 @@ class OatWriter::WriteCodeMethodVisitor : public OrderedMethodVisitor { ArtMethod* GetTargetMethod(const LinkerPatch& patch) REQUIRES_SHARED(Locks::mutator_lock_) { MethodReference ref = patch.TargetMethod(); - ObjPtr<mirror::DexCache> dex_cache = - (dex_file_ == ref.dex_file) ? dex_cache_ : class_linker_->FindDexCache( - Thread::Current(), *ref.dex_file); - ArtMethod* method = - class_linker_->LookupResolvedMethod(ref.index, dex_cache, class_loader_); + StackHandleScope<3> hs(Thread::Current()); + Handle<mirror::DexCache> h_dex_cache = hs.NewHandle(GetDexCache(ref.dex_file)); + // We must save both ObjPtr since they might be obsolete after LookupResolvedMethod. + HandleWrapperObjPtr<mirror::DexCache> h_original_dex_cache(hs.NewHandleWrapper(&dex_cache_)); + HandleWrapperObjPtr<mirror::ClassLoader> h_class_loader(hs.NewHandleWrapper(&class_loader_)); + ArtMethod* method = class_linker_->LookupResolvedMethod(ref.index, h_dex_cache, h_class_loader); CHECK(method != nullptr); return method; } @@ -2363,7 +2379,7 @@ size_t OatWriter::InitOatCodeDexFiles(size_t offset) { size_t OatWriter::InitDataImgRelRoLayout(size_t offset) { DCHECK_EQ(data_img_rel_ro_size_, 0u); - if (boot_image_rel_ro_entries_.empty()) { + if (boot_image_rel_ro_entries_.empty() && app_image_rel_ro_type_entries_.empty()) { // Nothing to put to the .data.img.rel.ro section. return offset; } @@ -2376,6 +2392,14 @@ size_t OatWriter::InitDataImgRelRoLayout(size_t offset) { data_img_rel_ro_size_ += sizeof(uint32_t); } + data_img_rel_ro_app_image_offset_ = data_img_rel_ro_size_; + + for (auto& entry : app_image_rel_ro_type_entries_) { + size_t& entry_offset = entry.second; + entry_offset = data_img_rel_ro_size_; + data_img_rel_ro_size_ += sizeof(uint32_t); + } + offset = data_img_rel_ro_start_ + data_img_rel_ro_size_; return offset; } @@ -3176,18 +3200,39 @@ size_t OatWriter::WriteCodeDexFiles(OutputStream* out, size_t OatWriter::WriteDataImgRelRo(OutputStream* out, size_t file_offset, size_t relative_offset) { - if (boot_image_rel_ro_entries_.empty()) { + if (boot_image_rel_ro_entries_.empty() && app_image_rel_ro_type_entries_.empty()) { return relative_offset; } // Write the entire .data.img.rel.ro with a single WriteFully(). std::vector<uint32_t> data; - data.reserve(boot_image_rel_ro_entries_.size()); + data.reserve(boot_image_rel_ro_entries_.size() + app_image_rel_ro_type_entries_.size()); for (const auto& entry : boot_image_rel_ro_entries_) { uint32_t boot_image_offset = entry.first; data.push_back(boot_image_offset); } - DCHECK_EQ(data.size(), boot_image_rel_ro_entries_.size()); + if (!app_image_rel_ro_type_entries_.empty()) { + DCHECK(GetCompilerOptions().IsAppImage()); + ClassLinker* class_linker = Runtime::Current()->GetClassLinker(); + ScopedObjectAccess soa(Thread::Current()); + const DexFile* last_dex_file = nullptr; + ObjPtr<mirror::DexCache> dex_cache = nullptr; + ObjPtr<mirror::ClassLoader> class_loader = nullptr; + for (const auto& entry : app_image_rel_ro_type_entries_) { + TypeReference target_type = entry.first; + if (target_type.dex_file != last_dex_file) { + dex_cache = class_linker->FindDexCache(soa.Self(), *target_type.dex_file); + class_loader = dex_cache->GetClassLoader(); + last_dex_file = target_type.dex_file; + } + ObjPtr<mirror::Class> type = + class_linker->LookupResolvedType(target_type.TypeIndex(), dex_cache, class_loader); + CHECK(type != nullptr); + uint32_t app_image_offset = image_writer_->GetGlobalImageOffset(type.Ptr()); + data.push_back(app_image_offset); + } + } + DCHECK_EQ(data.size(), boot_image_rel_ro_entries_.size() + app_image_rel_ro_type_entries_.size()); DCHECK_OFFSET(); if (!out->WriteFully(data.data(), data.size() * sizeof(data[0]))) { PLOG(ERROR) << "Failed to write .data.img.rel.ro in " << out->GetLocation(); diff --git a/dex2oat/linker/oat_writer.h b/dex2oat/linker/oat_writer.h index 4e2332e3cc..43c9bc77ff 100644 --- a/dex2oat/linker/oat_writer.h +++ b/dex2oat/linker/oat_writer.h @@ -215,6 +215,10 @@ class OatWriter { return data_img_rel_ro_size_; } + size_t GetDataImgRelRoAppImageOffset() const { + return data_img_rel_ro_app_image_offset_; + } + size_t GetBssSize() const { return bss_size_; } @@ -419,6 +423,9 @@ class OatWriter { // The size of the optional .data.img.rel.ro section holding the image relocations. size_t data_img_rel_ro_size_; + // The start of app image relocations in the .data.img.rel.ro section. + size_t data_img_rel_ro_app_image_offset_; + // The start of the optional .bss section. size_t bss_start_; @@ -462,6 +469,11 @@ class OatWriter { // The value is the target offset for patching, starting at `bss_start_ + bss_methods_offset_`. SafeMap<MethodReference, size_t, MethodReferenceValueComparator> bss_method_entries_; + // Map for allocating app image Class entries in .data.img.rel.ro. Indexed by TypeReference for + // the source type in the dex file with the "type value comparator" for deduplication. The value + // is the target offset for patching, starting at `data_img_rel_ro_start_`. + SafeMap<TypeReference, size_t, TypeReferenceValueComparator> app_image_rel_ro_type_entries_; + // Map for allocating Class entries in .bss. Indexed by TypeReference for the source // type in the dex file with the "type value comparator" for deduplication. The value // is the target offset for patching, starting at `bss_start_ + bss_roots_offset_`. diff --git a/dex2oat/linker/oat_writer_test.cc b/dex2oat/linker/oat_writer_test.cc index 0972e5d14a..f2a6ee47f0 100644 --- a/dex2oat/linker/oat_writer_test.cc +++ b/dex2oat/linker/oat_writer_test.cc @@ -211,6 +211,7 @@ class OatTest : public CommonCompilerDriverTest { elf_writer->PrepareDynamicSection(oat_writer.GetOatHeader().GetExecutableOffset(), oat_writer.GetCodeSize(), oat_writer.GetDataImgRelRoSize(), + oat_writer.GetDataImgRelRoAppImageOffset(), oat_writer.GetBssSize(), oat_writer.GetBssMethodsOffset(), oat_writer.GetBssRootsOffset(), diff --git a/dexopt_chroot_setup/dexopt_chroot_setup.cc b/dexopt_chroot_setup/dexopt_chroot_setup.cc index 967cb20c1e..f7d1ea4d37 100644 --- a/dexopt_chroot_setup/dexopt_chroot_setup.cc +++ b/dexopt_chroot_setup/dexopt_chroot_setup.cc @@ -97,13 +97,17 @@ Result<void> Run(std::string_view log_name, const std::vector<std::string>& args return {}; } -Result<std::string> GetArtExec() { +Result<CmdlineBuilder> GetArtExecCmdlineBuilder() { std::string error_msg; std::string art_root = GetArtRootSafe(&error_msg); if (!error_msg.empty()) { return Error() << error_msg; } - return art_root + "/bin/art_exec"; + CmdlineBuilder args; + args.Add(art_root + "/bin/art_exec") + .Add("--chroot=%s", DexoptChrootSetup::CHROOT_DIR) + .Add("--process-name-suffix=Pre-reboot Dexopt chroot"); + return args; } Result<void> CreateDir(const std::string& path) { @@ -408,19 +412,15 @@ Result<void> DexoptChrootSetup::SetUpChroot(const std::optional<std::string>& ot PLOG(WARNING) << "Failed to generate empty linker config to suppress warnings"; } - CmdlineBuilder args; - args.Add(OR_RETURN(GetArtExec())) - .Add("--chroot=%s", CHROOT_DIR) - .Add("--") + CmdlineBuilder args = OR_RETURN(GetArtExecCmdlineBuilder()); + args.Add("--") .Add("/system/bin/apexd") .Add("--otachroot-bootstrap") .AddIf(!IsOtaUpdate(ota_slot), "--also-include-staged-apexes"); OR_RETURN(Run("apexd", args.Get())); - args = CmdlineBuilder(); - args.Add(OR_RETURN(GetArtExec())) - .Add("--chroot=%s", CHROOT_DIR) - .Add("--drop-capabilities") + args = OR_RETURN(GetArtExecCmdlineBuilder()); + args.Add("--drop-capabilities") .Add("--") .Add("/apex/com.android.runtime/bin/linkerconfig") .Add("--target") @@ -432,10 +432,8 @@ Result<void> DexoptChrootSetup::SetUpChroot(const std::optional<std::string>& ot Result<void> DexoptChrootSetup::TearDownChroot() const { if (OS::FileExists(PathInChroot("/system/bin/apexd").c_str())) { - CmdlineBuilder args; - args.Add(OR_RETURN(GetArtExec())) - .Add("--chroot=%s", CHROOT_DIR) - .Add("--") + CmdlineBuilder args = OR_RETURN(GetArtExecCmdlineBuilder()); + args.Add("--") .Add("/system/bin/apexd") .Add("--unmount-all") .Add("--also-include-staged-apexes"); diff --git a/libartservice/service/Android.bp b/libartservice/service/Android.bp index fe0c3af5e0..a9a0a1a206 100644 --- a/libartservice/service/Android.bp +++ b/libartservice/service/Android.bp @@ -82,6 +82,7 @@ java_defaults { libs: [ "androidx.annotation_annotation", "auto_value_annotations", + "sdk_module-lib_current_framework-configinfrastructure", "sdk_module-lib_current_framework-permission-s", // TODO(b/256866172): Transitive dependency, for r8 only. "framework-statsd.stubs.module_lib", @@ -235,6 +236,10 @@ android_test { "art_module_source_build_java_defaults", ], + libs: [ + "sdk_module-lib_current_framework-configinfrastructure", + ], + static_libs: [ "androidx.test.ext.junit", "androidx.test.ext.truth", diff --git a/libartservice/service/java/com/android/server/art/AidlUtils.java b/libartservice/service/java/com/android/server/art/AidlUtils.java index d42a6539c4..6531e61668 100644 --- a/libartservice/service/java/com/android/server/art/AidlUtils.java +++ b/libartservice/service/java/com/android/server/art/AidlUtils.java @@ -208,6 +208,19 @@ public final class AidlUtils { } @NonNull + public static WritableProfilePath toWritableProfilePath(@NonNull ProfilePath profile) { + switch (profile.getTag()) { + case ProfilePath.primaryRefProfilePath: + return WritableProfilePath.forPrimary(profile.getPrimaryRefProfilePath()); + case ProfilePath.secondaryRefProfilePath: + return WritableProfilePath.forSecondary(profile.getSecondaryRefProfilePath()); + default: + throw new IllegalStateException("ProfilePath tag " + profile.getTag() + + " does not represent a writable type"); + } + } + + @NonNull public static String toString(@NonNull PrimaryRefProfilePath profile) { return String.format( "PrimaryRefProfilePath[packageName = %s, profileName = %s, isPreReboot = %b]", diff --git a/libartservice/service/java/com/android/server/art/ArtFileManager.java b/libartservice/service/java/com/android/server/art/ArtFileManager.java index 234ca01f7c..754b9ec1dd 100644 --- a/libartservice/service/java/com/android/server/art/ArtFileManager.java +++ b/libartservice/service/java/com/android/server/art/ArtFileManager.java @@ -29,7 +29,6 @@ import android.os.RemoteException; import android.os.ServiceSpecificException; import android.os.UserHandle; import android.os.UserManager; -import android.util.Log; import android.util.Pair; import androidx.annotation.RequiresApi; @@ -57,8 +56,6 @@ import java.util.Objects; */ @RequiresApi(Build.VERSION_CODES.UPSIDE_DOWN_CAKE) public class ArtFileManager { - private static final String TAG = ArtManagerLocal.TAG; - @NonNull private final Injector mInjector; public ArtFileManager(@NonNull Context context) { @@ -170,8 +167,7 @@ public class ArtFileManager { } } } catch (ServiceSpecificException e) { - Log.e(TAG, - String.format( + AsLog.e(String.format( "Failed to get dexopt status [packageName = %s, dexPath = %s, " + "isa = %s, classLoaderContext = %s]", pkgState.getPackageName(), dexInfo.dexPath(), abi.isa(), diff --git a/libartservice/service/java/com/android/server/art/ArtJni.java b/libartservice/service/java/com/android/server/art/ArtJni.java index 61384ca5f6..d46b889460 100644 --- a/libartservice/service/java/com/android/server/art/ArtJni.java +++ b/libartservice/service/java/com/android/server/art/ArtJni.java @@ -20,7 +20,6 @@ import android.annotation.NonNull; import android.annotation.Nullable; import android.os.Build; import android.os.RemoteException; -import android.util.Log; import androidx.annotation.RequiresApi; diff --git a/libartservice/service/java/com/android/server/art/ArtManagerLocal.java b/libartservice/service/java/com/android/server/art/ArtManagerLocal.java index 276e6eb7f5..30decda68b 100644 --- a/libartservice/service/java/com/android/server/art/ArtManagerLocal.java +++ b/libartservice/service/java/com/android/server/art/ArtManagerLocal.java @@ -20,8 +20,10 @@ import static com.android.server.art.ArtFileManager.ProfileLists; import static com.android.server.art.ArtFileManager.UsableArtifactLists; import static com.android.server.art.ArtFileManager.WritableArtifactLists; import static com.android.server.art.DexMetadataHelper.DexMetadataInfo; +import static com.android.server.art.DexUseManagerLocal.SecondaryDexInfo; import static com.android.server.art.PrimaryDexUtils.DetailedPrimaryDexInfo; import static com.android.server.art.PrimaryDexUtils.PrimaryDexInfo; +import static com.android.server.art.ProfilePath.WritableProfilePath; import static com.android.server.art.ReasonMapping.BatchDexoptReason; import static com.android.server.art.ReasonMapping.BootReason; import static com.android.server.art.Utils.Abi; @@ -39,7 +41,10 @@ import android.annotation.SystemApi; import android.annotation.SystemService; import android.app.job.JobInfo; import android.apphibernation.AppHibernationManager; +import android.content.BroadcastReceiver; import android.content.Context; +import android.content.Intent; +import android.content.IntentFilter; import android.os.Binder; import android.os.Build; import android.os.CancellationSignal; @@ -52,12 +57,12 @@ import android.os.UserHandle; import android.os.UserManager; import android.os.storage.StorageManager; import android.text.TextUtils; -import android.util.Log; import android.util.Pair; import androidx.annotation.RequiresApi; import com.android.internal.annotations.VisibleForTesting; +import com.android.modules.utils.build.SdkLevel; import com.android.server.LocalManagerRegistry; import com.android.server.art.model.ArtFlags; import com.android.server.art.model.ArtManagedFileStats; @@ -115,9 +120,6 @@ import java.util.stream.Stream; */ @SystemApi(client = SystemApi.Client.SYSTEM_SERVER) public final class ArtManagerLocal { - /** @hide */ - public static final String TAG = "ArtService"; - private static final String[] CLASSPATHS_FOR_BOOT_IMAGE_PROFILE = { "BOOTCLASSPATH", "SYSTEMSERVERCLASSPATH", "STANDALONE_SYSTEMSERVER_JARS"}; @@ -126,6 +128,8 @@ public final class ArtManagerLocal { @NonNull private final Injector mInjector; + private boolean mShouldCommitPreRebootStagedFiles = false; + @Deprecated public ArtManagerLocal() { mInjector = new Injector(); @@ -482,8 +486,7 @@ public final class ArtManagerLocal { dexoptResults.put(ArtFlags.PASS_DOWNGRADE, downgradeResult); } } - Log.i(TAG, - "Dexopting " + params.getPackages().size() + " packages with reason=" + reason); + AsLog.i("Dexopting " + params.getPackages().size() + " packages with reason=" + reason); DexoptResult mainResult = mInjector.getDexoptHelper().dexopt(snapshot, params.getPackages(), params.getDexoptParams(), cancellationSignal, dexoptExecutor, progressCallbackExecutor, @@ -752,9 +755,8 @@ public final class ArtManagerLocal { PrimaryDexUtils.buildOutputProfile(pkgState, dexInfo, Process.SYSTEM_UID, Process.SYSTEM_UID, false /* isPublic */, false /* isPreReboot */)); if (!result.externalProfileErrors().isEmpty()) { - Log.e(TAG, - "Error occurred when initializing from external profiles: " - + result.externalProfileErrors()); + AsLog.e("Error occurred when initializing from external profiles: " + + result.externalProfileErrors()); } ProfilePath refProfile = result.profile(); @@ -864,12 +866,49 @@ public final class ArtManagerLocal { @Nullable @CallbackExecutor Executor progressCallbackExecutor, @Nullable Consumer<OperationProgress> progressCallback) { try (var snapshot = mInjector.getPackageManagerLocal().withFilteredSnapshot()) { + if ((bootReason.equals(ReasonMapping.REASON_BOOT_AFTER_OTA) + || bootReason.equals(ReasonMapping.REASON_BOOT_AFTER_MAINLINE_UPDATE)) + && SdkLevel.isAtLeastV()) { + // The staged files have to be committed in two phases, one during boot, for primary + // dex files, and another after boot complete, for secondary dex files. We need to + // commit files for primary dex files early because apps will start using them as + // soon as the package manager is initialized. We need to wait until boot complete + // to commit files for secondary dex files because they are not decrypted before + // then. + mShouldCommitPreRebootStagedFiles = true; + commitPreRebootStagedFiles(snapshot, false /* forSecondary */); + } dexoptPackages(snapshot, bootReason, new CancellationSignal(), progressCallbackExecutor, progressCallback != null ? Map.of(ArtFlags.PASS_MAIN, progressCallback) : null); } } /** + * Notifies this class that {@link Context#registerReceiver} is ready for use. + * + * Should be used by {@link DexUseManagerLocal} ONLY. + * + * @hide + */ + @RequiresApi(Build.VERSION_CODES.UPSIDE_DOWN_CAKE) + void systemReady() { + if (mShouldCommitPreRebootStagedFiles) { + mInjector.getContext().registerReceiver(new BroadcastReceiver() { + @Override + public void onReceive(Context context, Intent intent) { + context.unregisterReceiver(this); + if (!SdkLevel.isAtLeastV()) { + throw new IllegalStateException("Broadcast receiver unexpectedly called"); + } + try (var snapshot = mInjector.getPackageManagerLocal().withFilteredSnapshot()) { + commitPreRebootStagedFiles(snapshot, true /* forSecondary */); + } + } + }, new IntentFilter(Intent.ACTION_BOOT_COMPLETED)); + } + } + + /** * Notifies ART Service that there are apexes staged for installation on next reboot (see * <a href="https://source.android.com/docs/core/ota/apex#apex-manager">the update sequence of * an APEX</a>). ART Service may use this to schedule a pre-reboot dexopt job. This might change @@ -885,6 +924,11 @@ public final class ArtManagerLocal { @RequiresApi(Build.VERSION_CODES.VANILLA_ICE_CREAM) public void onApexStaged(@NonNull String[] stagedApexModuleNames) { // TODO(b/311377497): Check system requirements. + mInjector.getPreRebootDexoptJob().unschedule(); + // Although `unschedule` implies `cancel`, we explicitly call `cancel` here to wait for + // the job to exit, if it's running. + mInjector.getPreRebootDexoptJob().cancel(true /* blocking */); + mInjector.getPreRebootDexoptJob().updateOtaSlot(null); mInjector.getPreRebootDexoptJob().schedule(); } @@ -1048,14 +1092,64 @@ public final class ArtManagerLocal { runtimeArtifactsToKeep.addAll(artifactLists.runtimeArtifacts()); } } - return mInjector.getArtd().cleanup( - profilesToKeep, artifactsToKeep, vdexFilesToKeep, runtimeArtifactsToKeep); + return mInjector.getArtd().cleanup(profilesToKeep, artifactsToKeep, vdexFilesToKeep, + runtimeArtifactsToKeep, + SdkLevel.isAtLeastV() && mInjector.getPreRebootDexoptJob().hasStarted()); } catch (RemoteException e) { Utils.logArtdException(e); return 0; } } + /** @param forSecondary true for secondary dex files; false for primary dex files. */ + @RequiresApi(Build.VERSION_CODES.VANILLA_ICE_CREAM) + private void commitPreRebootStagedFiles( + @NonNull PackageManagerLocal.FilteredSnapshot snapshot, boolean forSecondary) { + try { + // Because we don't know for which packages the Pre-reboot Dexopt job has generated + // staged files, we call artd for all dexoptable packages, which is a superset of the + // packages that we actually expect to have staged files. + for (PackageState pkgState : snapshot.getPackageStates().values()) { + if (!Utils.canDexoptPackage(pkgState, null /* appHibernationManager */)) { + continue; + } + AndroidPackage pkg = Utils.getPackageOrThrow(pkgState); + var options = ArtFileManager.Options.builder() + .setForPrimaryDex(!forSecondary) + .setForSecondaryDex(forSecondary) + .setExcludeForObsoleteDexesAndLoaders(true) + .build(); + List<ArtifactsPath> artifacts = + mInjector.getArtFileManager() + .getWritableArtifacts(pkgState, pkg, options) + .artifacts(); + List<WritableProfilePath> profiles = mInjector.getArtFileManager() + .getProfiles(pkgState, pkg, options) + .refProfiles() + .stream() + .map(AidlUtils::toWritableProfilePath) + .collect(Collectors.toList()); + try { + // The artd method commits all files somewhat transactionally. Here, we are + // committing files transactionally at the package level just for simplicity. In + // fact, we only need transaction on the split level: the artifacts and the + // profile of the same split must be committed transactionally. Consider the + // case where the staged artifacts and profile have less methods than the active + // ones generated by background dexopt, committing the artifacts while failing + // to commit the profile can potentially cause a permanent performance + // regression. + mInjector.getArtd().commitPreRebootStagedFiles(artifacts, profiles); + } catch (ServiceSpecificException e) { + AsLog.e("Failed to commit Pre-reboot staged files for package '" + + pkgState.getPackageName() + "'", + e); + } + } + } catch (RemoteException e) { + Utils.logArtdException(e); + } + } + /** * Should be used by {@link BackgroundDexoptJobService} ONLY. * @@ -1092,15 +1186,14 @@ public final class ArtManagerLocal { .filter(pkg -> !excludedPackages.contains(pkg)) .collect(Collectors.toList()); if (!packages.isEmpty()) { - Log.i(TAG, "Storage is low. Downgrading " + packages.size() + " inactive packages"); + AsLog.i("Storage is low. Downgrading " + packages.size() + " inactive packages"); DexoptParams params = new DexoptParams.Builder(ReasonMapping.REASON_INACTIVE).build(); return mInjector.getDexoptHelper().dexopt(snapshot, packages, params, cancellationSignal, executor, progressCallbackExecutor, progressCallback); } else { - Log.i(TAG, - "Storage is low, but downgrading is disabled or there's nothing to " - + "downgrade"); + AsLog.i("Storage is low, but downgrading is disabled or there's nothing to " + + "downgrade"); } } return null; @@ -1112,7 +1205,7 @@ public final class ArtManagerLocal { return mInjector.getStorageManager().getAllocatableBytes(StorageManager.UUID_DEFAULT) < DOWNGRADE_THRESHOLD_ABOVE_LOW_BYTES; } catch (IOException e) { - Log.e(TAG, "Failed to check storage. Assuming storage not low", e); + AsLog.e("Failed to check storage. Assuming storage not low", e); return false; } } @@ -1152,9 +1245,8 @@ public final class ArtManagerLocal { ArtFlags.FLAG_FORCE_MERGE_PROFILE) .build(); - Log.i(TAG, - "Dexopting " + packageNames.size() + " packages with reason=" - + dexoptParams.getReason() + " (supplementary pass)"); + AsLog.i("Dexopting " + packageNames.size() + + " packages with reason=" + dexoptParams.getReason() + " (supplementary pass)"); return mInjector.getDexoptHelper().dexopt(snapshot, packageNames, dexoptParams, cancellationSignal, dexoptExecutor, progressCallbackExecutor, progressCallback); } diff --git a/libartservice/service/java/com/android/server/art/ArtShellCommand.java b/libartservice/service/java/com/android/server/art/ArtShellCommand.java index 9203143abe..f3779eb93a 100644 --- a/libartservice/service/java/com/android/server/art/ArtShellCommand.java +++ b/libartservice/service/java/com/android/server/art/ArtShellCommand.java @@ -40,7 +40,6 @@ import android.os.Process; import android.system.ErrnoException; import android.system.Os; import android.system.StructStat; -import android.util.Log; import androidx.annotation.RequiresApi; @@ -88,8 +87,6 @@ import java.util.stream.Collectors; */ @RequiresApi(Build.VERSION_CODES.UPSIDE_DOWN_CAKE) public final class ArtShellCommand extends BasicShellCommandHandler { - private static final String TAG = ArtManagerLocal.TAG; - /** The default location for profile dumps. */ private final static String PROFILE_DEBUG_LOCATION = "/data/misc/profman"; @@ -190,6 +187,9 @@ public final class ArtShellCommand extends BasicShellCommandHandler { // TODO(b/311377497): Remove this command once the development is done. return handlePreRebootDexopt(pw); } + case "on-ota-staged": { + return handleOnOtaStaged(pw); + } default: pw.printf("Error: Unknown 'art' sub-command '%s'\n", subcmd); pw.println("See 'pm help' for help"); @@ -667,6 +667,67 @@ public final class ArtShellCommand extends BasicShellCommandHandler { } } + private int handleOnOtaStaged(@NonNull PrintWriter pw) { + if (!SdkLevel.isAtLeastV()) { + pw.println("Error: Unsupported command 'on-ota-staged'"); + return 1; + } + + int uid = Binder.getCallingUid(); + if (uid != Process.ROOT_UID) { + throw new SecurityException("Only root can call 'on-ota-staged'"); + } + + String otaSlot = null; + + String opt; + while ((opt = getNextOption()) != null) { + switch (opt) { + case "--slot": + otaSlot = getNextArgRequired(); + break; + default: + pw.println("Error: Unknown option: " + opt); + return 1; + } + } + + if (otaSlot == null) { + pw.println("Error: '--slot' must be specified"); + return 1; + } + + int code; + + // This operation requires the uid to be "system" (1000). + long identityToken = Binder.clearCallingIdentity(); + try { + mArtManagerLocal.getPreRebootDexoptJob().unschedule(); + // Although `unschedule` implies `cancel`, we explicitly call `cancel` here to wait for + // the job to exit, if it's running. + mArtManagerLocal.getPreRebootDexoptJob().cancel(true /* blocking */); + mArtManagerLocal.getPreRebootDexoptJob().updateOtaSlot(otaSlot); + code = mArtManagerLocal.getPreRebootDexoptJob().schedule(); + } finally { + Binder.restoreCallingIdentity(identityToken); + } + + switch (code) { + case ArtFlags.SCHEDULE_SUCCESS: + pw.println("Job scheduled"); + return 0; + case ArtFlags.SCHEDULE_DISABLED_BY_SYSPROP: + pw.println("Job disabled by system property"); + return 1; + case ArtFlags.SCHEDULE_JOB_SCHEDULER_FAILURE: + pw.println("Failed to schedule job"); + return 1; + default: + // Can't happen. + throw new IllegalStateException("Unknown result code: " + code); + } + } + @Override public void onHelp() { // No one should call this. The help text should be printed by the `onHelp` handler of `cmd @@ -816,6 +877,12 @@ public final class ArtShellCommand extends BasicShellCommandHandler { pw.println(" This command is different from 'pm compile -r REASON -a'. For example, it"); pw.println(" only dexopts a subset of apps, and it runs dexopt in parallel. See the"); pw.println(" API documentation for 'ArtManagerLocal.dexoptPackages' for details."); + pw.println(); + pw.println(" on-ota-staged --slot SLOT"); + pw.println(" Notifies ART Service that an OTA update is staged. A Pre-reboot Dexopt"); + pw.println(" job is scheduled for it."); + pw.println(" Options:"); + pw.println(" --slot SLOT The slot that contains the OTA update, '_a' or '_b'."); } private void enforceRootOrShell() { diff --git a/libartservice/service/java/com/android/server/art/ArtdRefCache.java b/libartservice/service/java/com/android/server/art/ArtdRefCache.java index c4ed71203c..148e4759b7 100644 --- a/libartservice/service/java/com/android/server/art/ArtdRefCache.java +++ b/libartservice/service/java/com/android/server/art/ArtdRefCache.java @@ -44,7 +44,6 @@ import java.util.concurrent.ScheduledExecutorService; */ @RequiresApi(Build.VERSION_CODES.UPSIDE_DOWN_CAKE) public class ArtdRefCache { - private static final String TAG = ArtManagerLocal.TAG; // The 15s timeout is arbitrarily picked. // TODO(jiakaiz): Revisit this based on real CUJs. @VisibleForTesting public static final long CACHE_TIMEOUT_MS = 15_000; diff --git a/libartservice/service/java/com/android/server/art/AsLog.java b/libartservice/service/java/com/android/server/art/AsLog.java new file mode 100644 index 0000000000..ac6ae472cc --- /dev/null +++ b/libartservice/service/java/com/android/server/art/AsLog.java @@ -0,0 +1,89 @@ +/* + * Copyright (C) 2024 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; + +import android.annotation.NonNull; +import android.annotation.Nullable; +import android.os.Build; +import android.util.Log; +import android.util.Slog; + +import androidx.annotation.RequiresApi; + +/** + * A log wrapper that logs messages with the appropriate tag. + * + * @hide + */ +@RequiresApi(Build.VERSION_CODES.UPSIDE_DOWN_CAKE) +public class AsLog { + private static final String TAG = "ArtService"; + private static final String PRE_REBOOT_TAG = "ArtServicePreReboot"; + + @NonNull + public static String getTag() { + return GlobalInjector.getInstance().isPreReboot() ? PRE_REBOOT_TAG : TAG; + } + + public static void v(@NonNull String msg) { + Log.v(getTag(), msg); + } + + public static void v(@NonNull String msg, @Nullable Throwable tr) { + Log.v(getTag(), msg, tr); + } + + public static void d(@NonNull String msg) { + Log.d(getTag(), msg); + } + + public static void d(@NonNull String msg, @Nullable Throwable tr) { + Log.d(getTag(), msg, tr); + } + + public static void i(@NonNull String msg) { + Log.i(getTag(), msg); + } + + public static void i(@NonNull String msg, @Nullable Throwable tr) { + Log.i(getTag(), msg, tr); + } + + public static void w(@NonNull String msg) { + Log.w(getTag(), msg); + } + + public static void w(@NonNull String msg, @Nullable Throwable tr) { + Log.w(getTag(), msg, tr); + } + + public static void e(@NonNull String msg) { + Log.e(getTag(), msg); + } + + public static void e(@NonNull String msg, @Nullable Throwable tr) { + Log.e(getTag(), msg, tr); + } + + public static void wtf(@NonNull String msg) { + Slog.wtf(getTag(), msg); + } + + public static void wtf(@NonNull String msg, @Nullable Throwable tr) { + Slog.wtf(getTag(), msg, tr); + } +} diff --git a/libartservice/service/java/com/android/server/art/BackgroundDexoptJob.java b/libartservice/service/java/com/android/server/art/BackgroundDexoptJob.java index 289c7cd235..6dda5c9980 100644 --- a/libartservice/service/java/com/android/server/art/BackgroundDexoptJob.java +++ b/libartservice/service/java/com/android/server/art/BackgroundDexoptJob.java @@ -32,8 +32,6 @@ import android.os.Build; import android.os.CancellationSignal; import android.os.SystemClock; import android.os.SystemProperties; -import android.util.Log; -import android.util.Slog; import androidx.annotation.RequiresApi; @@ -61,8 +59,6 @@ import java.util.function.Consumer; /** @hide */ @RequiresApi(Build.VERSION_CODES.UPSIDE_DOWN_CAKE) public class BackgroundDexoptJob implements ArtServiceJobInterface { - private static final String TAG = ArtManagerLocal.TAG; - /** * "android" is the package name for a <service> declared in * frameworks/base/core/res/AndroidManifest.xml @@ -98,7 +94,7 @@ public class BackgroundDexoptJob implements ArtServiceJobInterface { writeStats(result); } catch (RuntimeException e) { // Not expected. Log wtf to surface it. - Slog.wtf(TAG, "Failed to write stats", e); + AsLog.wtf("Failed to write stats", e); } // This is a periodic job, where the interval is specified in the `JobInfo`. "true" @@ -132,7 +128,7 @@ public class BackgroundDexoptJob implements ArtServiceJobInterface { } if (SystemProperties.getBoolean("pm.dexopt.disable_bg_dexopt", false /* def */)) { - Log.i(TAG, "Job is disabled by system property 'pm.dexopt.disable_bg_dexopt'"); + AsLog.i("Job is disabled by system property 'pm.dexopt.disable_bg_dexopt'"); return ArtFlags.SCHEDULE_DISABLED_BY_SYSPROP; } @@ -177,17 +173,17 @@ public class BackgroundDexoptJob implements ArtServiceJobInterface { @NonNull public synchronized CompletableFuture<Result> start() { if (mRunningJob != null) { - Log.i(TAG, "Job is already running"); + AsLog.i("Job is already running"); return mRunningJob; } mCancellationSignal = new CancellationSignal(); mLastStopReason = Optional.empty(); mRunningJob = new CompletableFuture().supplyAsync(() -> { - try (var tracing = new Utils.TracingWithTimingLogging(TAG, "jobExecution")) { + try (var tracing = new Utils.TracingWithTimingLogging(AsLog.getTag(), "jobExecution")) { return run(mCancellationSignal); } catch (RuntimeException e) { - Log.e(TAG, "Fatal error", e); + AsLog.e("Fatal error", e); return new FatalErrorResult(); } finally { synchronized (this) { @@ -201,12 +197,12 @@ public class BackgroundDexoptJob implements ArtServiceJobInterface { public synchronized void cancel() { if (mRunningJob == null) { - Log.i(TAG, "Job is not running"); + AsLog.i("Job is not running"); return; } mCancellationSignal.cancel(); - Log.i(TAG, "Job cancelled"); + AsLog.i("Job cancelled"); } @Nullable @@ -247,7 +243,7 @@ public class BackgroundDexoptJob implements ArtServiceJobInterface { // lose some chance to dexopt when the storage is very low, but it's fine because we // can still dexopt in the next run. long freedBytes = mInjector.getArtManagerLocal().cleanup(snapshot); - Log.i(TAG, String.format("Freed %d bytes", freedBytes)); + AsLog.i(String.format("Freed %d bytes", freedBytes)); } } return CompletedResult.create(dexoptResultByPass, durationMsByPass); diff --git a/libartservice/service/java/com/android/server/art/DexMetadataHelper.java b/libartservice/service/java/com/android/server/art/DexMetadataHelper.java index 1252123479..99d92083d3 100644 --- a/libartservice/service/java/com/android/server/art/DexMetadataHelper.java +++ b/libartservice/service/java/com/android/server/art/DexMetadataHelper.java @@ -19,7 +19,6 @@ package com.android.server.art; import android.annotation.NonNull; import android.annotation.Nullable; import android.os.Build; -import android.util.Log; import androidx.annotation.RequiresApi; @@ -43,8 +42,6 @@ import java.util.zip.ZipFile; */ @RequiresApi(Build.VERSION_CODES.UPSIDE_DOWN_CAKE) public class DexMetadataHelper { - private static final String TAG = ArtManagerLocal.TAG; - @NonNull private final Injector mInjector; public DexMetadataHelper() { @@ -73,7 +70,7 @@ public class DexMetadataHelper { } } catch (IOException e) { if (!(e instanceof FileNotFoundException || e instanceof NoSuchFileException)) { - Log.e(TAG, String.format("Failed to read dm file '%s'", realDmPath), e); + AsLog.e(String.format("Failed to read dm file '%s'", realDmPath), e); } return getDefaultDexMetadataInfo(); } diff --git a/libartservice/service/java/com/android/server/art/DexUseManagerLocal.java b/libartservice/service/java/com/android/server/art/DexUseManagerLocal.java index c6394d3bd6..9ef804919a 100644 --- a/libartservice/service/java/com/android/server/art/DexUseManagerLocal.java +++ b/libartservice/service/java/com/android/server/art/DexUseManagerLocal.java @@ -30,7 +30,6 @@ import android.os.Process; import android.os.RemoteException; import android.os.ServiceSpecificException; import android.os.UserHandle; -import android.util.Log; import androidx.annotation.RequiresApi; @@ -94,7 +93,6 @@ import java.util.stream.Collectors; @SystemApi(client = SystemApi.Client.SYSTEM_SERVER) @RequiresApi(Build.VERSION_CODES.UPSIDE_DOWN_CAKE) public class DexUseManagerLocal { - private static final String TAG = ArtManagerLocal.TAG; private static final String FILENAME = "/data/system/package-dex-usage.pb"; /** @@ -169,6 +167,7 @@ public class DexUseManagerLocal { /** Notifies dex use manager that {@link Context#registerReceiver} is ready for use. */ public void systemReady() { Utils.check(!mInjector.isPreReboot()); + mInjector.getArtManagerLocal().systemReady(); // Save the data when the device is being shut down. The receiver is blocking, with a // 10s timeout. mInjector.getContext().registerReceiver(new BroadcastReceiver() { @@ -556,7 +555,7 @@ public class DexUseManagerLocal { } } } catch (IOException e) { - Log.e(TAG, "Failed to save dex use data", e); + AsLog.e("Failed to save dex use data", e); } finally { Utils.deleteIfExistsSafe(tempFile); } @@ -574,7 +573,7 @@ public class DexUseManagerLocal { proto = DexUseProto.parseFrom(in); } catch (IOException e) { // Nothing else we can do but to start from scratch. - Log.e(TAG, "Failed to load dex use data", e); + AsLog.e("Failed to load dex use data", e); } synchronized (mLock) { if (mDexUse != null) { @@ -635,7 +634,7 @@ public class DexUseManagerLocal { try { return mInjector.getArtd().getDexFileVisibility(dexPath); } catch (ServiceSpecificException | RemoteException e) { - Log.e(TAG, "Failed to get visibility of " + dexPath, e); + AsLog.e("Failed to get visibility of " + dexPath, e); return FileVisibility.NOT_FOUND; } } @@ -953,7 +952,7 @@ public class DexUseManagerLocal { // Skip invalid dex paths persisted by previous versions. String errorMsg = validateDexPath.apply(dexFile); if (errorMsg != null) { - Log.e(TAG, errorMsg); + AsLog.e(errorMsg); continue; } @@ -1017,7 +1016,7 @@ public class DexUseManagerLocal { String errorMsg = validateClassLoaderContext.apply( Utils.assertNonEmpty(recordProto.getClassLoaderContext())); if (errorMsg != null) { - Log.e(TAG, errorMsg); + AsLog.e(errorMsg); continue; } @@ -1026,10 +1025,9 @@ public class DexUseManagerLocal { if (!Utils.isNativeAbi(record.mAbiName)) { // The native ABI set has changed by an OTA since the ABI name was recorded. - Log.i(TAG, - String.format("Ignoring secondary dex use record with non-native ABI " - + "'%s' for '%s'", - record.mAbiName, proto.getDexFile())); + AsLog.i(String.format("Ignoring secondary dex use record with non-native ABI " + + "'%s' for '%s'", + record.mAbiName, proto.getDexFile())); continue; } @@ -1221,5 +1219,10 @@ public class DexUseManagerLocal { return Objects.requireNonNull( LocalManagerRegistry.getManager(PackageManagerLocal.class)); } + + @NonNull + public ArtManagerLocal getArtManagerLocal() { + return Objects.requireNonNull(LocalManagerRegistry.getManager(ArtManagerLocal.class)); + } } } diff --git a/libartservice/service/java/com/android/server/art/DexoptHelper.java b/libartservice/service/java/com/android/server/art/DexoptHelper.java index 8f402918f6..76edfd503f 100644 --- a/libartservice/service/java/com/android/server/art/DexoptHelper.java +++ b/libartservice/service/java/com/android/server/art/DexoptHelper.java @@ -69,8 +69,6 @@ import java.util.stream.Collectors; */ @RequiresApi(Build.VERSION_CODES.UPSIDE_DOWN_CAKE) public class DexoptHelper { - private static final String TAG = ArtManagerLocal.TAG; - @NonNull private final Injector mInjector; public DexoptHelper(@NonNull Context context, @NonNull Config config) { diff --git a/libartservice/service/java/com/android/server/art/Dexopter.java b/libartservice/service/java/com/android/server/art/Dexopter.java index 69b861527e..f66dde81fe 100644 --- a/libartservice/service/java/com/android/server/art/Dexopter.java +++ b/libartservice/service/java/com/android/server/art/Dexopter.java @@ -40,7 +40,6 @@ import android.os.SystemProperties; import android.os.UserManager; import android.os.storage.StorageManager; import android.text.TextUtils; -import android.util.Log; import android.util.Pair; import androidx.annotation.RequiresApi; @@ -69,7 +68,6 @@ import java.util.Objects; /** @hide */ @RequiresApi(Build.VERSION_CODES.UPSIDE_DOWN_CAKE) public abstract class Dexopter<DexInfoType extends DetailedDexInfo> { - private static final String TAG = ArtManagerLocal.TAG; private static final List<String> ART_PACKAGE_NAMES = List.of("com.google.android.art", "com.android.art", "com.google.android.go.art"); @@ -102,7 +100,7 @@ public abstract class Dexopter<DexInfoType extends DetailedDexInfo> { @NonNull public final List<DexContainerFileDexoptResult> dexopt() throws RemoteException { if (SystemProperties.getBoolean("dalvik.vm.disable-art-service-dexopt", false /* def */)) { - Log.i(TAG, "Dexopt skipped because it's disabled by system property"); + AsLog.i("Dexopt skipped because it's disabled by system property"); return List.of(); } @@ -135,7 +133,7 @@ public abstract class Dexopter<DexInfoType extends DetailedDexInfo> { if (!dmInfo.config().getEnableEmbeddedProfile()) { String dmPath = DexMetadataHelper.getDmPath( Objects.requireNonNull(dmInfo.dmPath())); - Log.i(TAG, "Embedded profile disabled by config in the dm file " + dmPath); + AsLog.i("Embedded profile disabled by config in the dm file " + dmPath); } if (needsToBeShared) { @@ -228,7 +226,7 @@ public abstract class Dexopter<DexInfoType extends DetailedDexInfo> { continue; } } catch (IOException e) { - Log.e(TAG, "Failed to check storage. Assuming storage not low", e); + AsLog.e("Failed to check storage. Assuming storage not low", e); } IArtdCancellationSignal artdCancellationSignal = @@ -237,8 +235,7 @@ public abstract class Dexopter<DexInfoType extends DetailedDexInfo> { try { artdCancellationSignal.cancel(); } catch (RemoteException e) { - Log.e(TAG, "An error occurred when sending a cancellation signal", - e); + AsLog.e("An error occurred when sending a cancellation signal", e); } }); @@ -257,8 +254,7 @@ public abstract class Dexopter<DexInfoType extends DetailedDexInfo> { } } catch (ServiceSpecificException e) { // Log the error and continue. - Log.e(TAG, - String.format("Failed to dexopt [packageName = %s, dexPath = %s, " + AsLog.e(String.format("Failed to dexopt [packageName = %s, dexPath = %s, " + "isa = %s, classLoaderContext = %s]", mPkgState.getPackageName(), dexInfo.dexPath(), abi.isa(), dexInfo.classLoaderContext()), @@ -272,9 +268,8 @@ public abstract class Dexopter<DexInfoType extends DetailedDexInfo> { abi.isPrimaryAbi(), abi.name(), compilerFilter, status, wallTimeMs, cpuTimeMs, sizeBytes, sizeBeforeBytes, extendedStatusFlags, externalProfileErrors); - Log.i(TAG, - String.format("Dexopt result: [packageName = %s] %s", - mPkgState.getPackageName(), result)); + AsLog.i(String.format("Dexopt result: [packageName = %s] %s", + mPkgState.getPackageName(), result)); results.add(result); if (status != DexoptResult.DEXOPT_SKIPPED && status != DexoptResult.DEXOPT_PERFORMED) { @@ -568,8 +563,7 @@ public abstract class Dexopter<DexInfoType extends DetailedDexInfo> { mInjector.getArtd().commitTmpProfile(profile); return true; } catch (ServiceSpecificException e) { - Log.e(TAG, "Failed to commit profile changes " + AidlUtils.toString(profile.finalPath), - e); + AsLog.e("Failed to commit profile changes " + AidlUtils.toString(profile.finalPath), e); return false; } } @@ -588,8 +582,7 @@ public abstract class Dexopter<DexInfoType extends DetailedDexInfo> { return ProfilePath.tmpProfilePath(output.profilePath); } } catch (ServiceSpecificException e) { - Log.e(TAG, - "Failed to merge profiles " + AidlUtils.toString(output.profilePath.finalPath), + AsLog.e("Failed to merge profiles " + AidlUtils.toString(output.profilePath.finalPath), e); } diff --git a/libartservice/service/java/com/android/server/art/DumpHelper.java b/libartservice/service/java/com/android/server/art/DumpHelper.java index 8bd2210f33..31c159c92c 100644 --- a/libartservice/service/java/com/android/server/art/DumpHelper.java +++ b/libartservice/service/java/com/android/server/art/DumpHelper.java @@ -24,7 +24,6 @@ import android.annotation.NonNull; import android.os.Build; import android.os.RemoteException; import android.os.ServiceSpecificException; -import android.util.Log; import androidx.annotation.RequiresApi; @@ -54,8 +53,6 @@ import java.util.stream.Collectors; */ @RequiresApi(Build.VERSION_CODES.UPSIDE_DOWN_CAKE) public class DumpHelper { - private static final String TAG = ArtManagerLocal.TAG; - @NonNull private final Injector mInjector; public DumpHelper(@NonNull ArtManagerLocal artManagerLocal) { diff --git a/libartservice/service/java/com/android/server/art/PreRebootDexoptJob.java b/libartservice/service/java/com/android/server/art/PreRebootDexoptJob.java index b8a55af7e0..eca05c995b 100644 --- a/libartservice/service/java/com/android/server/art/PreRebootDexoptJob.java +++ b/libartservice/service/java/com/android/server/art/PreRebootDexoptJob.java @@ -19,15 +19,27 @@ package com.android.server.art; import static com.android.server.art.model.ArtFlags.ScheduleStatus; import android.annotation.NonNull; +import android.annotation.Nullable; +import android.app.job.JobInfo; import android.app.job.JobParameters; +import android.app.job.JobScheduler; +import android.content.ComponentName; import android.content.Context; import android.os.Build; +import android.os.CancellationSignal; +import android.os.SystemProperties; +import android.provider.DeviceConfig; import androidx.annotation.RequiresApi; +import com.android.internal.annotations.GuardedBy; import com.android.internal.annotations.VisibleForTesting; import com.android.server.art.model.ArtFlags; import com.android.server.art.model.ArtServiceJobInterface; +import com.android.server.art.prereboot.PreRebootDriver; + +import java.util.Objects; +import java.util.concurrent.CompletableFuture; /** * The Pre-reboot Dexopt job. @@ -38,8 +50,6 @@ import com.android.server.art.model.ArtServiceJobInterface; */ @RequiresApi(Build.VERSION_CODES.VANILLA_ICE_CREAM) public class PreRebootDexoptJob implements ArtServiceJobInterface { - private static final String TAG = ArtManagerLocal.TAG; - /** * "android" is the package name for a <service> declared in * frameworks/base/core/res/AndroidManifest.xml @@ -50,6 +60,23 @@ public class PreRebootDexoptJob implements ArtServiceJobInterface { @NonNull private final Injector mInjector; + // Job state variables. + @GuardedBy("this") @Nullable private CompletableFuture<Void> mRunningJob = null; + @GuardedBy("this") @Nullable private CancellationSignal mCancellationSignal = null; + + /** The slot that contains the OTA update, "_a" or "_b", or null for a Mainline update. */ + @GuardedBy("this") @Nullable private String mOtaSlot = null; + + /** + * Whether the job has started at least once, meaning the device is expected to have staged + * files, no matter it succeed, failed, or cancelled. + * + * Note that this flag is not persisted across system server restarts. It's possible that the + * value is lost due to a system server restart caused by a crash, but this is a minor case, so + * we don't handle it here for simplicity. + */ + @GuardedBy("this") private boolean mHasStarted = false; + public PreRebootDexoptJob(@NonNull Context context) { this(new Injector(context)); } @@ -62,19 +89,138 @@ public class PreRebootDexoptJob implements ArtServiceJobInterface { @Override public boolean onStartJob( @NonNull BackgroundDexoptJobService jobService, @NonNull JobParameters params) { + // No need to handle exceptions thrown by the future because exceptions are handled inside + // the future itself. + var unused = start().thenRunAsync(() -> { + try { + // If it failed, it means something went wrong, so we don't reschedule the job + // because it will likely fail again. If it's cancelled, the job will be rescheduled + // because the return value of `onStopJob` will be respected, and this call will be + // ignored. + jobService.jobFinished(params, false /* wantsReschedule */); + } catch (RuntimeException e) { + AsLog.wtf("Unexpected exception", e); + } + }); // "true" means the job will continue running until `jobFinished` is called. - return false; + return true; } @Override public boolean onStopJob(@NonNull JobParameters params) { - // "true" means to execute again in the same interval with the default retry policy. + cancel(false /* blocking */); + // "true" means to execute again with the default retry policy. return true; } public @ScheduleStatus int schedule() { - // TODO(b/311377497): Schedule the job. - return ArtFlags.SCHEDULE_SUCCESS; + if (this != BackgroundDexoptJobService.getJob(JOB_ID)) { + throw new IllegalStateException("This job cannot be scheduled"); + } + + if (!SystemProperties.getBoolean("dalvik.vm.enable_pr_dexopt", false /* def */) + && !DeviceConfig.getBoolean(DeviceConfig.NAMESPACE_RUNTIME, "enable_pr_dexopt", + false /* defaultValue */)) { + AsLog.i("Pre-reboot Dexopt Job is not enabled by system property"); + return ArtFlags.SCHEDULE_DISABLED_BY_SYSPROP; + } + + // If `pm.dexopt.disable_bg_dexopt` is set, the user probably means to disable any dexopt + // jobs in the background. + if (SystemProperties.getBoolean("pm.dexopt.disable_bg_dexopt", false /* def */)) { + AsLog.i("Pre-reboot Dexopt Job is disabled by system property " + + "'pm.dexopt.disable_bg_dexopt'"); + return ArtFlags.SCHEDULE_DISABLED_BY_SYSPROP; + } + + JobInfo info = new JobInfo + .Builder(JOB_ID, + new ComponentName(JOB_PKG_NAME, + BackgroundDexoptJobService.class.getName())) + .setRequiresDeviceIdle(true) + .setRequiresCharging(true) + .setRequiresBatteryNotLow(true) + .build(); + + /* @JobScheduler.Result */ int result = mInjector.getJobScheduler().schedule(info); + if (result == JobScheduler.RESULT_SUCCESS) { + AsLog.i("Pre-reboot Dexopt Job scheduled"); + return ArtFlags.SCHEDULE_SUCCESS; + } else { + AsLog.i("Failed to schedule Pre-reboot Dexopt Job"); + return ArtFlags.SCHEDULE_JOB_SCHEDULER_FAILURE; + } + } + + public void unschedule() { + if (this != BackgroundDexoptJobService.getJob(JOB_ID)) { + throw new IllegalStateException("This job cannot be unscheduled"); + } + + mInjector.getJobScheduler().cancel(JOB_ID); + } + + @NonNull + public synchronized CompletableFuture<Void> start() { + if (mRunningJob != null) { + AsLog.i("Job is already running"); + return mRunningJob; + } + + String otaSlot = mOtaSlot; + var cancellationSignal = mCancellationSignal = new CancellationSignal(); + mHasStarted = true; + mRunningJob = new CompletableFuture().runAsync(() -> { + try { + // TODO(b/336239721): Consume the result and report metrics. + mInjector.getPreRebootDriver().run(otaSlot, cancellationSignal); + } catch (RuntimeException e) { + AsLog.e("Fatal error", e); + } finally { + synchronized (this) { + mRunningJob = null; + mCancellationSignal = null; + } + } + }); + return mRunningJob; + } + + /** + * Cancels the job. + * + * @param blocking whether to wait for the job to exit. + */ + public void cancel(boolean blocking) { + CompletableFuture<Void> runningJob = null; + synchronized (this) { + if (mRunningJob == null) { + return; + } + + mCancellationSignal.cancel(); + AsLog.i("Job cancelled"); + runningJob = mRunningJob; + } + if (blocking) { + Utils.getFuture(runningJob); + } + } + + public synchronized void updateOtaSlot(@NonNull String value) { + Utils.check(value == null || value.equals("_a") || value.equals("_b")); + // It's not possible that this method is called with two different slots. + Utils.check(mOtaSlot == null || value == null || Objects.equals(mOtaSlot, value)); + // An OTA update has a higher priority than a Mainline update. When there are both a pending + // OTA update and a pending Mainline update, the system discards the Mainline update on the + // reboot. + if (mOtaSlot == null && value != null) { + mOtaSlot = value; + } + } + + public synchronized boolean hasStarted() { + return mHasStarted; } /** @@ -89,5 +235,15 @@ public class PreRebootDexoptJob implements ArtServiceJobInterface { Injector(@NonNull Context context) { mContext = context; } + + @NonNull + public JobScheduler getJobScheduler() { + return Objects.requireNonNull(mContext.getSystemService(JobScheduler.class)); + } + + @NonNull + public PreRebootDriver getPreRebootDriver() { + return new PreRebootDriver(mContext); + } } } diff --git a/libartservice/service/java/com/android/server/art/PrimaryDexopter.java b/libartservice/service/java/com/android/server/art/PrimaryDexopter.java index 6ac7988f9e..7856dbeacd 100644 --- a/libartservice/service/java/com/android/server/art/PrimaryDexopter.java +++ b/libartservice/service/java/com/android/server/art/PrimaryDexopter.java @@ -31,7 +31,6 @@ import android.os.RemoteException; import android.os.ServiceSpecificException; import android.os.UserHandle; import android.text.TextUtils; -import android.util.Log; import androidx.annotation.RequiresApi; @@ -56,8 +55,6 @@ import java.util.Objects; /** @hide */ @RequiresApi(Build.VERSION_CODES.UPSIDE_DOWN_CAKE) public class PrimaryDexopter extends Dexopter<DetailedPrimaryDexInfo> { - private static final String TAG = ArtManagerLocal.TAG; - private final int mSharedGid; public PrimaryDexopter(@NonNull Context context, @NonNull Config config, diff --git a/libartservice/service/java/com/android/server/art/SecondaryDexopter.java b/libartservice/service/java/com/android/server/art/SecondaryDexopter.java index fe0cdb0091..9a158042c4 100644 --- a/libartservice/service/java/com/android/server/art/SecondaryDexopter.java +++ b/libartservice/service/java/com/android/server/art/SecondaryDexopter.java @@ -40,8 +40,6 @@ import java.util.List; /** @hide */ @RequiresApi(Build.VERSION_CODES.UPSIDE_DOWN_CAKE) public class SecondaryDexopter extends Dexopter<CheckedSecondaryDexInfo> { - private static final String TAG = ArtManagerLocal.TAG; - public SecondaryDexopter(@NonNull Context context, @NonNull Config config, @NonNull PackageState pkgState, @NonNull AndroidPackage pkg, @NonNull DexoptParams params, @NonNull CancellationSignal cancellationSignal) { diff --git a/libartservice/service/java/com/android/server/art/Utils.java b/libartservice/service/java/com/android/server/art/Utils.java index 6c01bbcb40..8132dbbab2 100644 --- a/libartservice/service/java/com/android/server/art/Utils.java +++ b/libartservice/service/java/com/android/server/art/Utils.java @@ -37,7 +37,6 @@ import android.os.UserManager; import android.text.TextUtils; import android.util.Log; import android.util.Pair; -import android.util.Slog; import android.util.SparseArray; import androidx.annotation.RequiresApi; @@ -73,7 +72,6 @@ import java.util.stream.Collectors; /** @hide */ @RequiresApi(Build.VERSION_CODES.UPSIDE_DOWN_CAKE) public final class Utils { - public static final String TAG = ArtManagerLocal.TAG; public static final String PLATFORM_PACKAGE_NAME = "android"; /** A copy of {@link android.os.Trace.TRACE_TAG_DALVIK}. */ @@ -217,7 +215,7 @@ public final class Utils { // This should never happen. Ignore the error and conservatively use dalvik-cache to // minimize the risk. // TODO(jiakaiz): Throw the error instead of ignoring it. - Log.e(TAG, "Failed to determine the location of the artifacts", e); + AsLog.e("Failed to determine the location of the artifacts", e); return true; } } @@ -343,7 +341,7 @@ public final class Utils { try { Files.deleteIfExists(path); } catch (IOException e) { - Log.e(TAG, "Failed to delete file '" + path + "'", e); + AsLog.e("Failed to delete file '" + path + "'", e); } } @@ -390,8 +388,7 @@ public final class Utils { refProfile, isOtherReadable, List.of() /* externalProfileErrors */); } } catch (ServiceSpecificException e) { - Log.e(TAG, - "Failed to use the existing reference profile " + AsLog.e("Failed to use the existing reference profile " + AidlUtils.toString(refProfile), e); } @@ -440,7 +437,7 @@ public final class Utils { externalProfileErrors.add(result.errorMsg); } } catch (ServiceSpecificException e) { - Log.e(TAG, "Failed to initialize profile from " + pair.first, e); + AsLog.e("Failed to initialize profile from " + pair.first, e); } } @@ -457,10 +454,10 @@ public final class Utils { // exception is expected. // In either case, we don't need to surface the exception from here. // The Java stack trace is intentionally omitted because it's not helpful. - Log.e(TAG, message); + AsLog.e(message); } else { // Not expected. Log wtf to surface it. - Slog.wtf(TAG, message, e); + AsLog.wtf(message, e); } } @@ -473,7 +470,7 @@ public final class Utils { try { Thread.sleep(millis); } catch (InterruptedException e) { - Slog.wtf(TAG, "Sleep interrupted", e); + AsLog.wtf("Sleep interrupted", e); } } diff --git a/libartservice/service/java/com/android/server/art/prereboot/PreRebootDriver.java b/libartservice/service/java/com/android/server/art/prereboot/PreRebootDriver.java index 2e43383161..44809b1c50 100644 --- a/libartservice/service/java/com/android/server/art/prereboot/PreRebootDriver.java +++ b/libartservice/service/java/com/android/server/art/prereboot/PreRebootDriver.java @@ -26,14 +26,13 @@ import android.os.Build; import android.os.CancellationSignal; import android.os.RemoteException; import android.os.ServiceSpecificException; -import android.util.Log; -import android.util.Slog; import androidx.annotation.RequiresApi; import com.android.internal.annotations.VisibleForTesting; import com.android.server.art.ArtManagerLocal; import com.android.server.art.ArtModuleServiceInitializer; +import com.android.server.art.AsLog; import com.android.server.art.GlobalInjector; import com.android.server.art.IDexoptChrootSetup; import com.android.server.art.Utils; @@ -49,8 +48,6 @@ import dalvik.system.DelegateLastClassLoader; */ @RequiresApi(Build.VERSION_CODES.VANILLA_ICE_CREAM) public class PreRebootDriver { - private static final String TAG = ArtManagerLocal.TAG; - @NonNull private final Injector mInjector; public PreRebootDriver(@NonNull Context context) { @@ -76,9 +73,9 @@ public class PreRebootDriver { } catch (RemoteException e) { Utils.logArtdException(e); } catch (ServiceSpecificException e) { - Log.e(TAG, "Failed to set up chroot", e); + AsLog.e("Failed to set up chroot", e); } catch (ReflectiveOperationException e) { - Log.e(TAG, "Failed to run pre-reboot dexopt", e); + AsLog.e("Failed to run pre-reboot dexopt", e); } finally { tearDown(); } @@ -107,14 +104,14 @@ public class PreRebootDriver { } catch (RemoteException e) { Utils.logArtdException(e); } catch (ServiceSpecificException e) { - Log.e(TAG, "Failed to tear down chroot", e); + AsLog.e("Failed to tear down chroot", e); } catch (IllegalStateException e) { // Not expected, but we still want retries in such an extreme case. - Slog.wtf(TAG, "Unexpected exception", e); + AsLog.wtf("Unexpected exception", e); } if (--numRetries > 0) { - Log.i(TAG, "Retrying...."); + AsLog.i("Retrying...."); Utils.sleep(30000); } } diff --git a/libartservice/service/javatests/com/android/server/art/ArtManagerLocalTest.java b/libartservice/service/javatests/com/android/server/art/ArtManagerLocalTest.java index fa217ab89d..1fd7e4bcfc 100644 --- a/libartservice/service/javatests/com/android/server/art/ArtManagerLocalTest.java +++ b/libartservice/service/javatests/com/android/server/art/ArtManagerLocalTest.java @@ -48,6 +48,9 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import android.apphibernation.AppHibernationManager; +import android.content.BroadcastReceiver; +import android.content.Context; +import android.content.Intent; import android.os.CancellationSignal; import android.os.ParcelFileDescriptor; import android.os.Process; @@ -83,6 +86,7 @@ import org.junit.runner.RunWith; import org.junit.runners.Parameterized; import org.junit.runners.Parameterized.Parameter; import org.junit.runners.Parameterized.Parameters; +import org.mockito.ArgumentCaptor; import org.mockito.Mock; import java.io.File; @@ -132,12 +136,15 @@ public class ArtManagerLocalTest { @Mock private StorageManager mStorageManager; @Mock private ArtdRefCache.Pin mArtdPin; @Mock private DexMetadataHelper.Injector mDexMetadataHelperInjector; + @Mock private Context mContext; + @Mock private PreRebootDexoptJob mPreRebootDexoptJob; private PackageState mPkgState1; private AndroidPackage mPkg1; private CheckedSecondaryDexInfo mPkg1SecondaryDexInfo1; private CheckedSecondaryDexInfo mPkg1SecondaryDexInfoNotFound; private Config mConfig; private DexMetadataHelper mDexMetadataHelper; + private ArgumentCaptor<BroadcastReceiver> mBroadcastReceiverCaptor; // True if the artifacts should be in dalvik-cache. @Parameter(0) public boolean mIsInDalvikCache; @@ -173,6 +180,8 @@ public class ArtManagerLocalTest { .when(mInjector.getArtFileManager()) .thenReturn(new ArtFileManager(mArtFileManagerInjector)); lenient().when(mInjector.getDexMetadataHelper()).thenReturn(mDexMetadataHelper); + lenient().when(mInjector.getContext()).thenReturn(mContext); + lenient().when(mInjector.getPreRebootDexoptJob()).thenReturn(mPreRebootDexoptJob); lenient().when(mArtFileManagerInjector.getArtd()).thenReturn(mArtd); lenient().when(mArtFileManagerInjector.getUserManager()).thenReturn(mUserManager); @@ -186,6 +195,7 @@ public class ArtManagerLocalTest { lenient().when(SystemProperties.get(eq("pm.dexopt.install"))).thenReturn("speed-profile"); lenient().when(SystemProperties.get(eq("pm.dexopt.bg-dexopt"))).thenReturn("speed-profile"); lenient().when(SystemProperties.get(eq("pm.dexopt.first-boot"))).thenReturn("verify"); + lenient().when(SystemProperties.get(eq("pm.dexopt.boot-after-ota"))).thenReturn("verify"); lenient() .when(SystemProperties.get(eq("pm.dexopt.boot-after-mainline-update"))) .thenReturn("verify"); @@ -269,6 +279,11 @@ public class ArtManagerLocalTest { .when(mDexMetadataHelperInjector.openZipFile(any())) .thenThrow(NoSuchFileException.class); + mBroadcastReceiverCaptor = ArgumentCaptor.forClass(BroadcastReceiver.class); + lenient() + .when(mContext.registerReceiver(mBroadcastReceiverCaptor.capture(), any())) + .thenReturn(mock(Intent.class)); + mArtManagerLocal = new ArtManagerLocal(mInjector); } @@ -1119,7 +1134,18 @@ public class ArtManagerLocalTest { } @Test - public void testCleanup() throws Exception { + public void testCleanupKeepPreRebootStagedFiles() throws Exception { + when(mPreRebootDexoptJob.hasStarted()).thenReturn(true); + testCleanup(true /* keepPreRebootStagedFiles */); + } + + @Test + public void testCleanupRemovePreRebootStagedFiles() throws Exception { + when(mPreRebootDexoptJob.hasStarted()).thenReturn(false); + testCleanup(false /* keepPreRebootStagedFiles */); + } + + private void testCleanup(boolean keepPreRebootStagedFiles) throws Exception { // It should keep all artifacts, but not runtime images. doReturn(createGetDexoptStatusResult( "speed-profile", "bg-dexopt", "location", ArtifactsLocation.NEXT_TO_DEX)) @@ -1177,7 +1203,8 @@ public class ArtManagerLocalTest { inAnyOrderDeepEquals(AidlUtils.buildRuntimeArtifactsPath( PKG_NAME_1, "/somewhere/app/foo/split_0.apk", "arm64"), AidlUtils.buildRuntimeArtifactsPath( - PKG_NAME_1, "/somewhere/app/foo/split_0.apk", "arm"))); + PKG_NAME_1, "/somewhere/app/foo/split_0.apk", "arm")), + eq(keepPreRebootStagedFiles)); } @Test @@ -1330,6 +1357,56 @@ public class ArtManagerLocalTest { verify(mArtd, times(expectedGetProfileSizeCalls)).getProfileSize(any()); } + @Test + public void testCommitPreRebootStagedFiles() throws Exception { + when(mSnapshot.getPackageStates()).thenReturn(Map.of(PKG_NAME_1, mPkgState1)); + + mArtManagerLocal.onBoot(ReasonMapping.REASON_BOOT_AFTER_OTA, + null /* progressCallbackExecutor */, null /* progressCallback */); + + // It should commit files for primary dex files on boot. + verify(mArtd).commitPreRebootStagedFiles( + inAnyOrderDeepEquals( + AidlUtils.buildArtifactsPathAsInput( + "/somewhere/app/foo/base.apk", "arm64", mIsInDalvikCache), + AidlUtils.buildArtifactsPathAsInput( + "/somewhere/app/foo/base.apk", "arm", mIsInDalvikCache), + AidlUtils.buildArtifactsPathAsInput( + "/somewhere/app/foo/split_0.apk", "arm64", mIsInDalvikCache), + AidlUtils.buildArtifactsPathAsInput( + "/somewhere/app/foo/split_0.apk", "arm", mIsInDalvikCache)), + inAnyOrderDeepEquals(AidlUtils.toWritableProfilePath( + AidlUtils.buildProfilePathForPrimaryRefAsInput( + PKG_NAME_1, "primary")), + AidlUtils.toWritableProfilePath( + AidlUtils.buildProfilePathForPrimaryRefAsInput( + PKG_NAME_1, "split_0.split")))); + verify(mArtd, times(1)).commitPreRebootStagedFiles(any(), any()); + + mArtManagerLocal.systemReady(); + + // It should not commit anything on system ready. + verify(mArtd, times(1)).commitPreRebootStagedFiles(any(), any()); + + mBroadcastReceiverCaptor.getValue().onReceive(mContext, mock(Intent.class)); + + // It should commit files for secondary dex files on boot complete. + verify(mArtd).commitPreRebootStagedFiles( + inAnyOrderDeepEquals(AidlUtils.buildArtifactsPathAsInput( + "/data/user/0/foo/1.apk", "arm64", false /* isInDalvikCache */)), + inAnyOrderDeepEquals(AidlUtils.toWritableProfilePath( + AidlUtils.buildProfilePathForSecondaryRefAsInput( + "/data/user/0/foo/1.apk")))); + verify(mArtd, times(2)).commitPreRebootStagedFiles(any(), any()); + } + + @Test + public void testCommitPreRebootStagedFilesOnBootNotCalled() throws Exception { + mArtManagerLocal.systemReady(); + + verify(mContext, never()).registerReceiver(any(), any()); + } + private AndroidPackage createPackage(boolean multiSplit) { AndroidPackage pkg = mock(AndroidPackage.class); diff --git a/libartservice/service/javatests/com/android/server/art/DexUseManagerTest.java b/libartservice/service/javatests/com/android/server/art/DexUseManagerTest.java index 11810dfb35..ffc702f338 100644 --- a/libartservice/service/javatests/com/android/server/art/DexUseManagerTest.java +++ b/libartservice/service/javatests/com/android/server/art/DexUseManagerTest.java @@ -28,6 +28,7 @@ import static org.mockito.Mockito.argThat; import static org.mockito.Mockito.eq; import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import android.content.BroadcastReceiver; @@ -97,6 +98,7 @@ public class DexUseManagerTest { @Mock private DexUseManagerLocal.Injector mInjector; @Mock private IArtd mArtd; @Mock private Context mContext; + @Mock private ArtManagerLocal mArtManagerLocal; private DexUseManagerLocal mDexUseManager; private String mCeDir; private String mDeDir; @@ -167,6 +169,7 @@ public class DexUseManagerTest { lenient().when(mInjector.getContext()).thenReturn(mContext); lenient().when(mInjector.getAllPackageNames()).thenReturn(mPackageStates.keySet()); lenient().when(mInjector.isPreReboot()).thenReturn(false); + lenient().when(mInjector.getArtManagerLocal()).thenReturn(mArtManagerLocal); mDexUseManager = new DexUseManagerLocal(mInjector); mDexUseManager.systemReady(); @@ -831,6 +834,11 @@ public class DexUseManagerTest { true /* isUsedByOtherApps */, mDefaultFileVisibility)); } + @Test + public void testSystemReady() { + verify(mArtManagerLocal).systemReady(); + } + @Test(expected = IllegalStateException.class) public void testPreRebootNoUpdate() throws Exception { when(mInjector.isPreReboot()).thenReturn(true); diff --git a/libartservice/service/javatests/com/android/server/art/PreRebootDexoptJobTest.java b/libartservice/service/javatests/com/android/server/art/PreRebootDexoptJobTest.java new file mode 100644 index 0000000000..ba4e7bab52 --- /dev/null +++ b/libartservice/service/javatests/com/android/server/art/PreRebootDexoptJobTest.java @@ -0,0 +1,260 @@ +/* + * Copyright (C) 2024 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; + +import static com.android.server.art.PreRebootDexoptJob.JOB_ID; + +import static com.google.common.truth.Truth.assertThat; + +import static org.mockito.Mockito.any; +import static org.mockito.Mockito.anyBoolean; +import static org.mockito.Mockito.eq; +import static org.mockito.Mockito.isNull; +import static org.mockito.Mockito.lenient; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import android.app.job.JobInfo; +import android.app.job.JobScheduler; +import android.os.CancellationSignal; +import android.os.SystemProperties; +import android.provider.DeviceConfig; + +import androidx.test.filters.SmallTest; + +import com.android.server.art.model.ArtFlags; +import com.android.server.art.prereboot.PreRebootDriver; +import com.android.server.art.testing.StaticMockitoRule; + +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; + +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.Future; +import java.util.concurrent.Semaphore; +import java.util.concurrent.TimeUnit; + +@SmallTest +@RunWith(MockitoJUnitRunner.StrictStubs.class) +public class PreRebootDexoptJobTest { + private static final long TIMEOUT_SEC = 10; + + @Rule + public StaticMockitoRule mockitoRule = new StaticMockitoRule( + SystemProperties.class, BackgroundDexoptJobService.class, DeviceConfig.class); + + @Mock private PreRebootDexoptJob.Injector mInjector; + @Mock private JobScheduler mJobScheduler; + @Mock private PreRebootDriver mPreRebootDriver; + private PreRebootDexoptJob mPreRebootDexoptJob; + + @Before + public void setUp() throws Exception { + // By default, the job is enabled by a build-time flag. + lenient() + .when(SystemProperties.getBoolean(eq("pm.dexopt.disable_bg_dexopt"), anyBoolean())) + .thenReturn(false); + lenient() + .when(SystemProperties.getBoolean(eq("dalvik.vm.enable_pr_dexopt"), anyBoolean())) + .thenReturn(true); + lenient() + .when(DeviceConfig.getBoolean( + eq(DeviceConfig.NAMESPACE_RUNTIME), eq("enable_pr_dexopt"), anyBoolean())) + .thenReturn(false); + + lenient().when(mInjector.getJobScheduler()).thenReturn(mJobScheduler); + lenient().when(mInjector.getPreRebootDriver()).thenReturn(mPreRebootDriver); + + mPreRebootDexoptJob = new PreRebootDexoptJob(mInjector); + lenient().when(BackgroundDexoptJobService.getJob(JOB_ID)).thenReturn(mPreRebootDexoptJob); + } + + @Test + public void testSchedule() throws Exception { + var captor = ArgumentCaptor.forClass(JobInfo.class); + when(mJobScheduler.schedule(captor.capture())).thenReturn(JobScheduler.RESULT_SUCCESS); + + assertThat(mPreRebootDexoptJob.schedule()).isEqualTo(ArtFlags.SCHEDULE_SUCCESS); + + JobInfo jobInfo = captor.getValue(); + assertThat(jobInfo.isPeriodic()).isFalse(); + assertThat(jobInfo.isRequireDeviceIdle()).isTrue(); + assertThat(jobInfo.isRequireCharging()).isTrue(); + assertThat(jobInfo.isRequireBatteryNotLow()).isTrue(); + } + + @Test + public void testScheduleDisabled() { + when(SystemProperties.getBoolean(eq("pm.dexopt.disable_bg_dexopt"), anyBoolean())) + .thenReturn(true); + + assertThat(mPreRebootDexoptJob.schedule()).isEqualTo(ArtFlags.SCHEDULE_DISABLED_BY_SYSPROP); + + verify(mJobScheduler, never()).schedule(any()); + } + + @Test + public void testScheduleNotEnabled() { + when(SystemProperties.getBoolean(eq("dalvik.vm.enable_pr_dexopt"), anyBoolean())) + .thenReturn(false); + + assertThat(mPreRebootDexoptJob.schedule()).isEqualTo(ArtFlags.SCHEDULE_DISABLED_BY_SYSPROP); + + verify(mJobScheduler, never()).schedule(any()); + } + + @Test + public void testScheduleEnabledByPhenotypeFlag() { + lenient() + .when(SystemProperties.getBoolean(eq("dalvik.vm.enable_pr_dexopt"), anyBoolean())) + .thenReturn(false); + lenient() + .when(DeviceConfig.getBoolean( + eq(DeviceConfig.NAMESPACE_RUNTIME), eq("enable_pr_dexopt"), anyBoolean())) + .thenReturn(true); + when(mJobScheduler.schedule(any())).thenReturn(JobScheduler.RESULT_SUCCESS); + + assertThat(mPreRebootDexoptJob.schedule()).isEqualTo(ArtFlags.SCHEDULE_SUCCESS); + + verify(mJobScheduler).schedule(any()); + } + + @Test + public void testUnschedule() { + mPreRebootDexoptJob.unschedule(); + verify(mJobScheduler).cancel(JOB_ID); + } + + @Test + public void testStart() { + when(mPreRebootDriver.run(any(), any())).thenReturn(true); + + assertThat(mPreRebootDexoptJob.hasStarted()).isFalse(); + Future<Void> future = mPreRebootDexoptJob.start(); + assertThat(mPreRebootDexoptJob.hasStarted()).isTrue(); + + Utils.getFuture(future); + } + + @Test + public void testStartAlreadyRunning() { + Semaphore dexoptDone = new Semaphore(0); + when(mPreRebootDriver.run(any(), any())).thenAnswer(invocation -> { + assertThat(dexoptDone.tryAcquire(TIMEOUT_SEC, TimeUnit.SECONDS)).isTrue(); + return true; + }); + + Future<Void> future1 = mPreRebootDexoptJob.start(); + Future<Void> future2 = mPreRebootDexoptJob.start(); + assertThat(future1).isSameInstanceAs(future2); + + dexoptDone.release(); + Utils.getFuture(future1); + + verify(mPreRebootDriver, times(1)).run(any(), any()); + } + + @Test + public void testStartAnother() { + when(mPreRebootDriver.run(any(), any())).thenReturn(true); + + Future<Void> future1 = mPreRebootDexoptJob.start(); + Utils.getFuture(future1); + Future<Void> future2 = mPreRebootDexoptJob.start(); + Utils.getFuture(future2); + assertThat(future1).isNotSameInstanceAs(future2); + } + + @Test + public void testCancel() { + Semaphore dexoptCancelled = new Semaphore(0); + Semaphore jobExited = new Semaphore(0); + when(mPreRebootDriver.run(any(), any())).thenAnswer(invocation -> { + assertThat(dexoptCancelled.tryAcquire(TIMEOUT_SEC, TimeUnit.SECONDS)).isTrue(); + var cancellationSignal = invocation.<CancellationSignal>getArgument(1); + assertThat(cancellationSignal.isCanceled()).isTrue(); + jobExited.release(); + return true; + }); + + var unused = mPreRebootDexoptJob.start(); + Future<Void> future = new CompletableFuture().runAsync( + () -> { mPreRebootDexoptJob.cancel(true /* blocking */); }); + dexoptCancelled.release(); + Utils.getFuture(future); + // Check that `cancel` is really blocking. + assertThat(jobExited.tryAcquire()).isTrue(); + } + + @Test + public void testUpdateOtaSlotOtaThenMainline() { + mPreRebootDexoptJob.updateOtaSlot("_b"); + mPreRebootDexoptJob.updateOtaSlot(null); + + when(mPreRebootDriver.run(eq("_b"), any())).thenReturn(true); + + Utils.getFuture(mPreRebootDexoptJob.start()); + } + + @Test + public void testUpdateOtaSlotMainlineThenOta() { + mPreRebootDexoptJob.updateOtaSlot(null); + mPreRebootDexoptJob.updateOtaSlot("_a"); + + when(mPreRebootDriver.run(eq("_a"), any())).thenReturn(true); + + Utils.getFuture(mPreRebootDexoptJob.start()); + } + + @Test + public void testUpdateOtaSlotMainlineThenMainline() { + mPreRebootDexoptJob.updateOtaSlot(null); + mPreRebootDexoptJob.updateOtaSlot(null); + + when(mPreRebootDriver.run(isNull(), any())).thenReturn(true); + + Utils.getFuture(mPreRebootDexoptJob.start()); + } + + @Test + public void testUpdateOtaSlotOtaThenOta() { + mPreRebootDexoptJob.updateOtaSlot("_b"); + mPreRebootDexoptJob.updateOtaSlot("_b"); + + when(mPreRebootDriver.run(eq("_b"), any())).thenReturn(true); + + Utils.getFuture(mPreRebootDexoptJob.start()); + } + + @Test(expected = IllegalStateException.class) + public void testUpdateOtaSlotOtaThenOtaDifferentSlots() { + mPreRebootDexoptJob.updateOtaSlot("_b"); + mPreRebootDexoptJob.updateOtaSlot("_a"); + } + + @Test(expected = IllegalStateException.class) + public void testUpdateOtaSlotOtaBogusSlot() { + mPreRebootDexoptJob.updateOtaSlot("_bogus"); + } +} diff --git a/libarttools/art_exec.cc b/libarttools/art_exec.cc index 704b25dddb..dfd0ca5bf9 100644 --- a/libarttools/art_exec.cc +++ b/libarttools/art_exec.cc @@ -67,6 +67,8 @@ Supported options: --keep-fds=FILE_DESCRIPTORS: A semicolon-separated list of file descriptors to keep open. --env=KEY=VALUE: Set an environment variable. This flag can be passed multiple times to set multiple environment variables. + --process-name-suffix=SUFFIX: Add a suffix in parentheses to argv[0] when calling `execv`. This + suffix will show up as part of the process name in tombstone when the process crashes. )"; constexpr int kErrorUsage = 100; diff --git a/libelffile/elf/elf_builder.h b/libelffile/elf/elf_builder.h index 908ad5cb89..bb82b109bd 100644 --- a/libelffile/elf/elf_builder.h +++ b/libelffile/elf/elf_builder.h @@ -641,6 +641,7 @@ class ElfBuilder final { Elf_Word rodata_size, Elf_Word text_size, Elf_Word data_img_rel_ro_size, + Elf_Word data_img_rel_ro_app_image_offset, Elf_Word bss_size, Elf_Word bss_methods_offset, Elf_Word bss_roots_offset, @@ -682,6 +683,7 @@ class ElfBuilder final { Elf_Word oatlastword_address = rodata_.GetAddress() + rodata_size - 4; dynsym_.Add(oatlastword, &rodata_, oatlastword_address, 4, STB_GLOBAL, STT_OBJECT); } + DCHECK_LE(data_img_rel_ro_app_image_offset, data_img_rel_ro_size); if (data_img_rel_ro_size != 0u) { Elf_Word oatdataimgrelro = dynstr_.Add("oatdataimgrelro"); dynsym_.Add(oatdataimgrelro, @@ -691,14 +693,21 @@ class ElfBuilder final { STB_GLOBAL, STT_OBJECT); Elf_Word oatdataimgrelrolastword = dynstr_.Add("oatdataimgrelrolastword"); - Elf_Word oatdataimgrelrolastword_address = - data_img_rel_ro_.GetAddress() + data_img_rel_ro_size - 4; dynsym_.Add(oatdataimgrelrolastword, &data_img_rel_ro_, - oatdataimgrelrolastword_address, + data_img_rel_ro_.GetAddress() + data_img_rel_ro_size - 4, 4, STB_GLOBAL, STT_OBJECT); + if (data_img_rel_ro_app_image_offset != data_img_rel_ro_size) { + Elf_Word oatdataimgrelroappimage = dynstr_.Add("oatdataimgrelroappimage"); + dynsym_.Add(oatdataimgrelroappimage, + &data_img_rel_ro_, + data_img_rel_ro_.GetAddress() + data_img_rel_ro_app_image_offset, + data_img_rel_ro_app_image_offset, + STB_GLOBAL, + STT_OBJECT); + } } DCHECK_LE(bss_roots_offset, bss_size); if (bss_size != 0u) { diff --git a/oatdump/oatdump.cc b/oatdump/oatdump.cc index 2c17845417..983316e9a2 100644 --- a/oatdump/oatdump.cc +++ b/oatdump/oatdump.cc @@ -183,6 +183,7 @@ class OatSymbolizer final { rodata_size, text_size, oat_file_->DataImgRelRoSize(), + oat_file_->DataImgRelRoAppImageOffset(), oat_file_->BssSize(), oat_file_->BssMethodsOffset(), oat_file_->BssRootsOffset(), diff --git a/openjdkjvmti/ti_redefine.cc b/openjdkjvmti/ti_redefine.cc index d0ce6a1f49..60e8193f5d 100644 --- a/openjdkjvmti/ti_redefine.cc +++ b/openjdkjvmti/ti_redefine.cc @@ -2550,9 +2550,6 @@ void Redefiner::ClassRedefinition::UpdateMethods(art::ObjPtr<art::mirror::Class> const art::DexFile& old_dex_file = mclass->GetDexFile(); // Update methods. for (art::ArtMethod& method : mclass->GetDeclaredMethods(image_pointer_size)) { - // Reinitialize the method by calling `CopyFrom`. This ensures for example - // the entrypoint and the hotness are reset. - method.CopyFrom(&method, image_pointer_size); const art::dex::StringId* new_name_id = dex_file_->FindStringId(method.GetName()); art::dex::TypeIndex method_return_idx = dex_file_->GetIndexForTypeId(*dex_file_->FindTypeId(method.GetReturnTypeDescriptor())); diff --git a/runtime/art_method.cc b/runtime/art_method.cc index 0b6b883bab..c8b16990b2 100644 --- a/runtime/art_method.cc +++ b/runtime/art_method.cc @@ -795,18 +795,16 @@ void ArtMethod::CopyFrom(ArtMethod* src, PointerSize image_pointer_size) { const void* entry_point = GetEntryPointFromQuickCompiledCodePtrSize(image_pointer_size); if (runtime->UseJitCompilation()) { if (runtime->GetJit()->GetCodeCache()->ContainsPc(entry_point)) { - SetNativePointer(EntryPointFromQuickCompiledCodeOffset(image_pointer_size), - src->IsNative() ? GetQuickGenericJniStub() : GetQuickToInterpreterBridge(), - image_pointer_size); + SetEntryPointFromQuickCompiledCodePtrSize( + src->IsNative() ? GetQuickGenericJniStub() : GetQuickToInterpreterBridge(), + image_pointer_size); } } ClassLinker* class_linker = Runtime::Current()->GetClassLinker(); if (interpreter::IsNterpSupported() && class_linker->IsNterpEntryPoint(entry_point)) { // If the entrypoint is nterp, it's too early to check if the new method // will support it. So for simplicity, use the interpreter bridge. - SetNativePointer(EntryPointFromQuickCompiledCodeOffset(image_pointer_size), - GetQuickToInterpreterBridge(), - image_pointer_size); + SetEntryPointFromQuickCompiledCodePtrSize(GetQuickToInterpreterBridge(), image_pointer_size); } // Clear the data pointer, it will be set if needed by the caller. @@ -924,20 +922,4 @@ ALWAYS_INLINE static inline void DoGetAccessFlagsHelper(ArtMethod* method) method->GetDeclaringClass<kReadBarrierOption>()->IsErroneous()); } -void ArtMethod::SetEntryPointFromQuickCompiledCodePtrSize( - const void* entry_point_from_quick_compiled_code, PointerSize pointer_size) { - const void* current_entry_point = GetEntryPointFromQuickCompiledCodePtrSize(pointer_size); - if (current_entry_point == entry_point_from_quick_compiled_code) { - return; - } - jit::Jit* jit = Runtime::Current()->GetJit(); - SetNativePointer(EntryPointFromQuickCompiledCodeOffset(pointer_size), - entry_point_from_quick_compiled_code, - pointer_size); - if (jit != nullptr && - jit->GetCodeCache()->ContainsPc(current_entry_point)) { - jit->GetCodeCache()->AddZombieCode(this, current_entry_point); - } -} - } // namespace art diff --git a/runtime/art_method.h b/runtime/art_method.h index 97e1e6fa25..a612c81b75 100644 --- a/runtime/art_method.h +++ b/runtime/art_method.h @@ -766,7 +766,11 @@ class EXPORT ArtMethod final { } ALWAYS_INLINE void SetEntryPointFromQuickCompiledCodePtrSize( const void* entry_point_from_quick_compiled_code, PointerSize pointer_size) - REQUIRES_SHARED(Locks::mutator_lock_); + REQUIRES_SHARED(Locks::mutator_lock_) { + SetNativePointer(EntryPointFromQuickCompiledCodeOffset(pointer_size), + entry_point_from_quick_compiled_code, + pointer_size); + } static constexpr MemberOffset DataOffset(PointerSize pointer_size) { return MemberOffset(PtrSizedFieldsOffset(pointer_size) + OFFSETOF_MEMBER( @@ -1186,8 +1190,6 @@ class EXPORT ArtMethod final { // Used by GetName and GetNameView to share common code. const char* GetRuntimeMethodName() REQUIRES_SHARED(Locks::mutator_lock_); - friend class RuntimeImageHelper; // For SetNativePointer. - DISALLOW_COPY_AND_ASSIGN(ArtMethod); // Need to use CopyFrom to deal with 32 vs 64 bits. }; diff --git a/runtime/class_linker-inl.h b/runtime/class_linker-inl.h index 6951e35791..1f5272c7ce 100644 --- a/runtime/class_linker-inl.h +++ b/runtime/class_linker-inl.h @@ -271,16 +271,31 @@ inline bool ClassLinker::CheckInvokeClassMismatch(ObjPtr<mirror::DexCache> dex_c } inline ArtMethod* ClassLinker::LookupResolvedMethod(uint32_t method_idx, - ObjPtr<mirror::DexCache> dex_cache, - ObjPtr<mirror::ClassLoader> class_loader) { - DCHECK(dex_cache->GetClassLoader() == class_loader); + Handle<mirror::DexCache> dex_cache, + Handle<mirror::ClassLoader> class_loader) { + DCHECK(dex_cache->GetClassLoader() == class_loader.Get()); ArtMethod* resolved = dex_cache->GetResolvedMethod(method_idx); if (resolved == nullptr) { const DexFile& dex_file = *dex_cache->GetDexFile(); const dex::MethodId& method_id = dex_file.GetMethodId(method_idx); - ObjPtr<mirror::Class> klass = LookupResolvedType(method_id.class_idx_, dex_cache, class_loader); + ObjPtr<mirror::Class> klass = + LookupResolvedType(method_id.class_idx_, dex_cache.Get(), class_loader.Get()); + + if (UNLIKELY(klass == nullptr)) { + // We normaly should not end up here. However the verifier currently doesn't guarantee + // the invariant of having the klass in the class table. b/73760543 b/336842546 + klass = ResolveType(method_id.class_idx_, dex_cache, class_loader); + if (UNLIKELY(klass == nullptr)) { + // This can only happen if the current thread is not allowed to load + // classes. + DCHECK(!Thread::Current()->CanLoadClasses()); + DCHECK(Thread::Current()->IsExceptionPending()); + return nullptr; + } + } + if (klass != nullptr) { - resolved = FindResolvedMethod(klass, dex_cache, class_loader, method_idx); + resolved = FindResolvedMethod(klass, dex_cache.Get(), class_loader.Get(), method_idx); } } return resolved; diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc index 5b00a87217..08cc153d84 100644 --- a/runtime/class_linker.cc +++ b/runtime/class_linker.cc @@ -11166,6 +11166,27 @@ void ClassLinker::SetEnablePublicSdkChecks([[maybe_unused]] bool enabled) { UNREACHABLE(); } +bool ClassLinker::TransactionWriteConstraint( + [[maybe_unused]] Thread* self, [[maybe_unused]] ObjPtr<mirror::Object> obj) const { + // Should not be called on ClassLinker, only on AotClassLinker that overrides this. + LOG(FATAL) << "UNREACHABLE"; + UNREACHABLE(); +} + +bool ClassLinker::TransactionWriteValueConstraint( + [[maybe_unused]] Thread* self, [[maybe_unused]] ObjPtr<mirror::Object> value) const { + // Should not be called on ClassLinker, only on AotClassLinker that overrides this. + LOG(FATAL) << "UNREACHABLE"; + UNREACHABLE(); +} + +bool ClassLinker::TransactionAllocationConstraint( + [[maybe_unused]] Thread* self, [[maybe_unused]] ObjPtr<mirror::Class> klass) const { + // Should not be called on ClassLinker, only on AotClassLinker that overrides this. + LOG(FATAL) << "UNREACHABLE"; + UNREACHABLE(); +} + void ClassLinker::RemoveDexFromCaches(const DexFile& dex_file) { ReaderMutexLock mu(Thread::Current(), *Locks::dex_lock_); diff --git a/runtime/class_linker.h b/runtime/class_linker.h index f2c08b93c5..7f53ca6e93 100644 --- a/runtime/class_linker.h +++ b/runtime/class_linker.h @@ -339,8 +339,8 @@ class EXPORT ClassLinker { // Look up a previously resolved method with the given index. ArtMethod* LookupResolvedMethod(uint32_t method_idx, - ObjPtr<mirror::DexCache> dex_cache, - ObjPtr<mirror::ClassLoader> class_loader) + Handle<mirror::DexCache> dex_cache, + Handle<mirror::ClassLoader> class_loader) REQUIRES_SHARED(Locks::mutator_lock_); // Find a method with the given index from class `klass`, and update the dex cache. @@ -890,6 +890,15 @@ class EXPORT ClassLinker { virtual bool DenyAccessBasedOnPublicSdk(const char* type_descriptor) const; // Enable or disable public sdk checks. virtual void SetEnablePublicSdkChecks(bool enabled); + + // Transaction constraint checks for AOT compilation. + virtual bool TransactionWriteConstraint(Thread* self, ObjPtr<mirror::Object> obj) const + REQUIRES_SHARED(Locks::mutator_lock_); + virtual bool TransactionWriteValueConstraint(Thread* self, ObjPtr<mirror::Object> value) const + REQUIRES_SHARED(Locks::mutator_lock_); + virtual bool TransactionAllocationConstraint(Thread* self, ObjPtr<mirror::Class> klass) const + REQUIRES_SHARED(Locks::mutator_lock_); + void RemoveDexFromCaches(const DexFile& dex_file); ClassTable* GetBootClassTable() REQUIRES_SHARED(Locks::classlinker_classes_lock_) { return boot_class_table_.get(); diff --git a/runtime/common_throws.h b/runtime/common_throws.h index aeba3bb1bb..71b0968fc0 100644 --- a/runtime/common_throws.h +++ b/runtime/common_throws.h @@ -35,6 +35,9 @@ class DexFile; enum InvokeType : uint32_t; class Signature; +// The descriptor of the transaction abort exception. +constexpr const char kTransactionAbortErrorDescriptor[] = "Ldalvik/system/TransactionAbortError;"; + // AbstractMethodError void ThrowAbstractMethodError(ArtMethod* method) diff --git a/runtime/entrypoints/entrypoint_utils-inl.h b/runtime/entrypoints/entrypoint_utils-inl.h index b0d0ab4b07..e1efa4a0ac 100644 --- a/runtime/entrypoints/entrypoint_utils-inl.h +++ b/runtime/entrypoints/entrypoint_utils-inl.h @@ -131,50 +131,60 @@ inline ArtMethod* GetResolvedMethod(ArtMethod* outer_method, ClassLinker* class_linker = Runtime::Current()->GetClassLinker(); ArtMethod* method = outer_method; for (InlineInfo inline_info : inline_infos) { + StackHandleScope<2> hs(Thread::Current()); + Handle<mirror::DexCache> h_dex_cache; + Handle<mirror::ClassLoader> h_class_loader; DCHECK(!inline_info.EncodesArtMethod()); DCHECK_NE(inline_info.GetDexPc(), static_cast<uint32_t>(-1)); MethodInfo method_info = code_info.GetMethodInfoOf(inline_info); uint32_t method_index = method_info.GetMethodIndex(); const uint32_t dex_file_index = method_info.GetDexFileIndex(); ArtMethod* inlined_method = nullptr; - ObjPtr<mirror::DexCache> dex_cache = nullptr; if (method_info.HasDexFileIndex()) { if (method_info.GetDexFileIndexKind() == MethodInfo::kKindBCP) { ArrayRef<const DexFile* const> bcp_dex_files(class_linker->GetBootClassPath()); DCHECK_LT(dex_file_index, bcp_dex_files.size()) << "OOB access to bcp_dex_files. Dumping info: " - << GetResolvedMethodErrorString( - class_linker, inlined_method, method, outer_method, dex_cache, method_info); + << GetResolvedMethodErrorString(class_linker, + inlined_method, + method, + outer_method, + /*dex_cache=*/ nullptr, + method_info); const DexFile* dex_file = bcp_dex_files[dex_file_index]; DCHECK_NE(dex_file, nullptr); - dex_cache = class_linker->FindDexCache(Thread::Current(), *dex_file); + h_dex_cache = hs.NewHandle(class_linker->FindDexCache(Thread::Current(), *dex_file)); } else { ArrayRef<const OatDexFile* const> oat_dex_files( outer_method->GetDexFile()->GetOatDexFile()->GetOatFile()->GetOatDexFiles()); DCHECK_LT(dex_file_index, oat_dex_files.size()) << "OOB access to oat_dex_files. Dumping info: " - << GetResolvedMethodErrorString( - class_linker, inlined_method, method, outer_method, dex_cache, method_info); + << GetResolvedMethodErrorString(class_linker, + inlined_method, + method, + outer_method, + /*dex_cache=*/ nullptr, + method_info); const OatDexFile* odf = oat_dex_files[dex_file_index]; DCHECK_NE(odf, nullptr); - dex_cache = class_linker->FindDexCache(Thread::Current(), *odf); + h_dex_cache = hs.NewHandle(class_linker->FindDexCache(Thread::Current(), *odf)); } } else { - dex_cache = outer_method->GetDexCache(); + h_dex_cache = hs.NewHandle(outer_method->GetDexCache()); } - inlined_method = - class_linker->LookupResolvedMethod(method_index, dex_cache, dex_cache->GetClassLoader()); + h_class_loader = hs.NewHandle(h_dex_cache->GetClassLoader()); + inlined_method = class_linker->LookupResolvedMethod(method_index, h_dex_cache, h_class_loader); if (UNLIKELY(inlined_method == nullptr)) { LOG(FATAL) << GetResolvedMethodErrorString( - class_linker, inlined_method, method, outer_method, dex_cache, method_info); + class_linker, inlined_method, method, outer_method, h_dex_cache.Get(), method_info); UNREACHABLE(); } DCHECK(!inlined_method->IsRuntimeMethod()); DCHECK_EQ(inlined_method->GetDexFile() == outer_method->GetDexFile(), dex_file_index == MethodInfo::kSameDexFile) << GetResolvedMethodErrorString( - class_linker, inlined_method, method, outer_method, dex_cache, method_info); + class_linker, inlined_method, method, outer_method, h_dex_cache.Get(), method_info); method = inlined_method; } diff --git a/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc b/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc index b53339acd9..85e5d4e1f4 100644 --- a/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc +++ b/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc @@ -1114,9 +1114,10 @@ static void DumpB74410240DebugData(ArtMethod** sp) REQUIRES_SHARED(Locks::mutato caller = WellKnownClasses::java_lang_String_charAt; CHECK_EQ(caller->GetDexMethodIndex(), method_index); } else { - ObjPtr<mirror::DexCache> dex_cache = caller->GetDexCache(); - ObjPtr<mirror::ClassLoader> class_loader = caller->GetClassLoader(); - caller = class_linker->LookupResolvedMethod(method_index, dex_cache, class_loader); + StackHandleScope<2> hs(Thread::Current()); + Handle<mirror::DexCache> h_dex_cache(hs.NewHandle(caller->GetDexCache())); + Handle<mirror::ClassLoader> h_class_loader(hs.NewHandle(caller->GetClassLoader())); + caller = class_linker->LookupResolvedMethod(method_index, h_dex_cache, h_class_loader); CHECK(caller != nullptr); } } diff --git a/runtime/interpreter/active_transaction_checker.h b/runtime/interpreter/active_transaction_checker.h new file mode 100644 index 0000000000..16fa6b1829 --- /dev/null +++ b/runtime/interpreter/active_transaction_checker.h @@ -0,0 +1,87 @@ +/* + * Copyright (C) 2024 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. + */ + +#ifndef ART_RUNTIME_INTERPRETER_ACTIVE_TRANSACTION_CHECKER_H_ +#define ART_RUNTIME_INTERPRETER_ACTIVE_TRANSACTION_CHECKER_H_ + +#include "base/macros.h" +#include "gc/heap.h" +#include "mirror/object-inl.h" +#include "runtime.h" +#include "transaction.h" + +namespace art HIDDEN { +namespace interpreter { + +class ActiveTransactionChecker { + public: + static inline bool WriteConstraint(Thread* self, ObjPtr<mirror::Object> obj) + REQUIRES_SHARED(Locks::mutator_lock_) { + Runtime* runtime = Runtime::Current(); + if (runtime->GetTransaction()->WriteConstraint(obj)) { + DCHECK(runtime->GetHeap()->ObjectIsInBootImageSpace(obj) || obj->IsClass()); + const char* extra = runtime->GetHeap()->ObjectIsInBootImageSpace(obj) ? "boot image " : ""; + runtime->AbortTransactionF( + self, "Can't set fields of %s%s", extra, obj->PrettyTypeOf().c_str()); + return true; + } + return false; + } + + static inline bool WriteValueConstraint(Thread* self, ObjPtr<mirror::Object> value) + REQUIRES_SHARED(Locks::mutator_lock_) { + Runtime* runtime = Runtime::Current(); + if (runtime->GetTransaction()->WriteValueConstraint(value)) { + DCHECK(value != nullptr); + const char* description = value->IsClass() ? "class" : "instance of"; + ObjPtr<mirror::Class> klass = value->IsClass() ? value->AsClass() : value->GetClass(); + runtime->AbortTransactionF( + self, "Can't store reference to %s %s", description, klass->PrettyDescriptor().c_str()); + return true; + } + return false; + } + + static inline bool ReadConstraint(Thread* self, ObjPtr<mirror::Object> obj) + REQUIRES_SHARED(Locks::mutator_lock_) { + DCHECK(obj->IsClass()); + Runtime* runtime = Runtime::Current(); + if (runtime->GetTransaction()->ReadConstraint(obj)) { + runtime->AbortTransactionF( + self, + "Can't read static fields of %s since it does not belong to clinit's class.", + obj->PrettyTypeOf().c_str()); + return true; + } + return false; + } + + static inline bool AllocationConstraint(Thread* self, ObjPtr<mirror::Class> klass) + REQUIRES_SHARED(Locks::mutator_lock_) { + if (klass->IsFinalizable()) { + Runtime::Current()->AbortTransactionF(self, + "Allocating finalizable object in transaction: %s", + klass->PrettyDescriptor().c_str()); + return true; + } + return false; + } +}; + +} // namespace interpreter +} // namespace art + +#endif // ART_RUNTIME_INTERPRETER_ACTIVE_TRANSACTION_CHECKER_H_ diff --git a/runtime/interpreter/interpreter_common.cc b/runtime/interpreter/interpreter_common.cc index 737e80b8ca..080b32f3d5 100644 --- a/runtime/interpreter/interpreter_common.cc +++ b/runtime/interpreter/interpreter_common.cc @@ -46,7 +46,6 @@ #include "shadow_frame-inl.h" #include "stack.h" #include "thread-inl.h" -#include "transaction.h" #include "var_handles.h" #include "well_known_classes.h" @@ -210,22 +209,6 @@ void UnexpectedOpcode(const Instruction* inst, const ShadowFrame& shadow_frame) UNREACHABLE(); } -void AbortTransactionF(Thread* self, const char* fmt, ...) { - va_list args; - va_start(args, fmt); - AbortTransactionV(self, fmt, args); - va_end(args); -} - -void AbortTransactionV(Thread* self, const char* fmt, va_list args) { - CHECK(Runtime::Current()->IsActiveTransaction()); - // Constructs abort message. - std::string abort_msg; - android::base::StringAppendV(&abort_msg, fmt, args); - // Throws an exception so we can abort the transaction and rollback every change. - Runtime::Current()->AbortTransactionAndThrowAbortError(self, abort_msg); -} - // START DECLARATIONS : // // These additional declarations are required because clang complains diff --git a/runtime/interpreter/interpreter_common.h b/runtime/interpreter/interpreter_common.h index e6548ef82b..24fb8a83c4 100644 --- a/runtime/interpreter/interpreter_common.h +++ b/runtime/interpreter/interpreter_common.h @@ -20,7 +20,6 @@ #include "android-base/macros.h" #include "instrumentation.h" #include "interpreter.h" -#include "transaction.h" #include <math.h> @@ -67,6 +66,38 @@ namespace art HIDDEN { namespace interpreter { +// We declare the helper class for transaction checks here but it shall be defined +// only when compiling the transactional interpreter. +class ActiveTransactionChecker; + +// Define the helper class that does not do any transaction checks. +class InactiveTransactionChecker { + public: + ALWAYS_INLINE static bool WriteConstraint([[maybe_unused]] Thread* self, + [[maybe_unused]] ObjPtr<mirror::Object> obj) + REQUIRES_SHARED(Locks::mutator_lock_) { + return false; + } + + ALWAYS_INLINE static bool WriteValueConstraint([[maybe_unused]] Thread* self, + [[maybe_unused]] ObjPtr<mirror::Object> value) + REQUIRES_SHARED(Locks::mutator_lock_) { + return false; + } + + ALWAYS_INLINE static bool ReadConstraint([[maybe_unused]] Thread* self, + [[maybe_unused]] ObjPtr<mirror::Object> value) + REQUIRES_SHARED(Locks::mutator_lock_) { + return false; + } + + ALWAYS_INLINE static bool AllocationConstraint([[maybe_unused]] Thread* self, + [[maybe_unused]] ObjPtr<mirror::Class> klass) + REQUIRES_SHARED(Locks::mutator_lock_) { + return false; + } +}; + void ThrowNullPointerExceptionFromInterpreter() REQUIRES_SHARED(Locks::mutator_lock_); @@ -111,13 +142,6 @@ static inline bool DoMonitorCheckOnExit(Thread* self, ShadowFrame* frame) return true; } -void AbortTransactionF(Thread* self, const char* fmt, ...) - __attribute__((__format__(__printf__, 2, 3))) - REQUIRES_SHARED(Locks::mutator_lock_); - -void AbortTransactionV(Thread* self, const char* fmt, va_list args) - REQUIRES_SHARED(Locks::mutator_lock_); - void RecordArrayElementsInTransaction(ObjPtr<mirror::Array> array, int32_t count) REQUIRES_SHARED(Locks::mutator_lock_); @@ -378,12 +402,10 @@ ALWAYS_INLINE bool DoFieldGet(Thread* self, ObjPtr<mirror::Object> obj; if (is_static) { obj = field->GetDeclaringClass(); - if (transaction_active) { - if (Runtime::Current()->GetTransaction()->ReadConstraint(obj)) { - Runtime::Current()->AbortTransactionAndThrowAbortError(self, "Can't read static fields of " - + obj->PrettyTypeOf() + " since it does not belong to clinit's class."); - return false; - } + using TransactionChecker = typename std::conditional_t< + transaction_active, ActiveTransactionChecker, InactiveTransactionChecker>; + if (TransactionChecker::ReadConstraint(self, obj)) { + return false; } } else { obj = shadow_frame.GetVRegReference(inst->VRegB_22c(inst_data)); @@ -446,34 +468,6 @@ ALWAYS_INLINE bool DoFieldGet(Thread* self, return true; } -static inline bool CheckWriteConstraint(Thread* self, ObjPtr<mirror::Object> obj) - REQUIRES_SHARED(Locks::mutator_lock_) { - Runtime* runtime = Runtime::Current(); - if (runtime->GetTransaction()->WriteConstraint(obj)) { - DCHECK(runtime->GetHeap()->ObjectIsInBootImageSpace(obj) || obj->IsClass()); - const char* base_msg = runtime->GetHeap()->ObjectIsInBootImageSpace(obj) - ? "Can't set fields of boot image " - : "Can't set fields of "; - runtime->AbortTransactionAndThrowAbortError(self, base_msg + obj->PrettyTypeOf()); - return false; - } - return true; -} - -static inline bool CheckWriteValueConstraint(Thread* self, ObjPtr<mirror::Object> value) - REQUIRES_SHARED(Locks::mutator_lock_) { - Runtime* runtime = Runtime::Current(); - if (runtime->GetTransaction()->WriteValueConstraint(value)) { - DCHECK(value != nullptr); - std::string msg = value->IsClass() - ? "Can't store reference to class " + value->AsClass()->PrettyDescriptor() - : "Can't store reference to instance of " + value->GetClass()->PrettyDescriptor(); - runtime->AbortTransactionAndThrowAbortError(self, msg); - return false; - } - return true; -} - // Handles iput-XXX and sput-XXX instructions. // Returns true on success, otherwise throws an exception and returns false. template<FindFieldType find_type, Primitive::Type field_type, bool transaction_active> @@ -523,15 +517,16 @@ ALWAYS_INLINE bool DoFieldPut(Thread* self, obj = shadow_frame.GetVRegReference(inst->VRegB_22c(inst_data)); } } - if (transaction_active && !CheckWriteConstraint(self, obj)) { + using TransactionChecker = typename std::conditional_t< + transaction_active, ActiveTransactionChecker, InactiveTransactionChecker>; + if (TransactionChecker::WriteConstraint(self, obj)) { return false; } JValue value = GetFieldValue<field_type>(shadow_frame, vregA); - if (transaction_active && - field_type == Primitive::kPrimNot && - !CheckWriteValueConstraint(self, value.GetL())) { + if (field_type == Primitive::kPrimNot && + TransactionChecker::WriteValueConstraint(self, value.GetL())) { return false; } if (should_report) { diff --git a/runtime/interpreter/interpreter_switch_impl-inl.h b/runtime/interpreter/interpreter_switch_impl-inl.h index 8b23715ba6..8ff6a897f8 100644 --- a/runtime/interpreter/interpreter_switch_impl-inl.h +++ b/runtime/interpreter/interpreter_switch_impl-inl.h @@ -23,6 +23,7 @@ #include "base/memory_tool.h" #include "base/pointer_size.h" #include "base/quasi_atomic.h" +#include "common_throws.h" #include "dex/dex_file_types.h" #include "dex/dex_instruction_list.h" #include "experimental_flags.h" @@ -53,6 +54,9 @@ namespace interpreter { template<bool transaction_active, Instruction::Format kFormat> class InstructionHandler { public: + using TransactionChecker = typename std::conditional_t< + transaction_active, ActiveTransactionChecker, InactiveTransactionChecker>; + #define HANDLER_ATTRIBUTES ALWAYS_INLINE FLATTEN WARN_UNUSED REQUIRES_SHARED(Locks::mutator_lock_) HANDLER_ATTRIBUTES bool CheckTransactionAbort() { @@ -62,7 +66,7 @@ class InstructionHandler { StackHandleScope<1u> hs(Self()); Handle<mirror::Throwable> abort_exception = hs.NewHandle(Self()->GetException()); DCHECK(abort_exception != nullptr); - DCHECK(abort_exception->GetClass()->DescriptorEquals(Transaction::kAbortExceptionDescriptor)); + DCHECK(abort_exception->GetClass()->DescriptorEquals(kTransactionAbortErrorDescriptor)); Self()->ClearException(); PerformNonStandardReturn( Self(), shadow_frame_, ctx_->result, Instrumentation(), Accessor().InsSize()); @@ -332,7 +336,7 @@ class InstructionHandler { if (UNLIKELY(!array->CheckIsValidIndex(index))) { return false; // Pending exception. } - if (transaction_active && !CheckWriteConstraint(Self(), array)) { + if (TransactionChecker::WriteConstraint(Self(), array)) { return false; } array->template SetWithoutChecks<transaction_active>(index, value); @@ -669,10 +673,7 @@ class InstructionHandler { if (LIKELY(c != nullptr)) { // Don't allow finalizable objects to be allocated during a transaction since these can't // be finalized without a started runtime. - if (transaction_active && c->IsFinalizable()) { - AbortTransactionF(Self(), - "Allocating finalizable object in transaction: %s", - c->PrettyDescriptor().c_str()); + if (TransactionChecker::AllocationConstraint(Self(), c)) { return false; // Pending exception. } gc::AllocatorType allocator_type = Runtime::Current()->GetHeap()->GetCurrentAllocator(); @@ -898,8 +899,8 @@ class InstructionHandler { ObjPtr<mirror::Object> val = GetVRegReference(A()); ObjPtr<mirror::ObjectArray<mirror::Object>> array = a->AsObjectArray<mirror::Object>(); if (array->CheckIsValidIndex(index) && array->CheckAssignable(val)) { - if (transaction_active && - (!CheckWriteConstraint(Self(), array) || !CheckWriteValueConstraint(Self(), val))) { + if (TransactionChecker::WriteConstraint(Self(), array) || + TransactionChecker::WriteValueConstraint(Self(), val)) { return false; } array->SetWithoutChecks<transaction_active>(index, val); diff --git a/runtime/interpreter/interpreter_switch_impl1.cc b/runtime/interpreter/interpreter_switch_impl1.cc index b9033d926e..e56c58c2bc 100644 --- a/runtime/interpreter/interpreter_switch_impl1.cc +++ b/runtime/interpreter/interpreter_switch_impl1.cc @@ -19,6 +19,8 @@ #include "interpreter_switch_impl-inl.h" +#include "active_transaction_checker.h" + namespace art HIDDEN { namespace interpreter { diff --git a/runtime/interpreter/unstarted_runtime.cc b/runtime/interpreter/unstarted_runtime.cc index 7309391b32..d865327473 100644 --- a/runtime/interpreter/unstarted_runtime.cc +++ b/runtime/interpreter/unstarted_runtime.cc @@ -62,7 +62,6 @@ #include "nth_caller_visitor.h" #include "reflection.h" #include "thread-inl.h" -#include "transaction.h" #include "unstarted_runtime_list.h" #include "well_known_classes-inl.h" @@ -78,9 +77,10 @@ static void AbortTransactionOrFail(Thread* self, const char* fmt, ...) static void AbortTransactionOrFail(Thread* self, const char* fmt, ...) { va_list args; - if (Runtime::Current()->IsActiveTransaction()) { + Runtime* runtime = Runtime::Current(); + if (runtime->IsActiveTransaction()) { va_start(args, fmt); - AbortTransactionV(self, fmt, args); + runtime->AbortTransactionV(self, fmt, args); va_end(args); } else { va_start(args, fmt); @@ -166,8 +166,7 @@ static void CheckExceptionGenerateClassNotFound(Thread* self) if (self->IsExceptionPending()) { Runtime* runtime = Runtime::Current(); DCHECK_EQ(runtime->IsTransactionAborted(), - self->GetException()->GetClass()->DescriptorEquals( - Transaction::kAbortExceptionDescriptor)) + self->GetException()->GetClass()->DescriptorEquals(kTransactionAbortErrorDescriptor)) << self->GetException()->GetClass()->PrettyDescriptor(); if (runtime->IsActiveTransaction()) { // The boot class path at run time may contain additional dex files with @@ -176,7 +175,7 @@ static void CheckExceptionGenerateClassNotFound(Thread* self) // initialize a class differently than when executing at run time. // If we're not aborting the transaction yet, abort now. b/183691501 if (!runtime->IsTransactionAborted()) { - AbortTransactionF(self, "ClassNotFoundException"); + runtime->AbortTransactionF(self, "ClassNotFoundException"); } } else { // If not in a transaction, it cannot be the transaction abort exception. Wrap it. @@ -301,12 +300,11 @@ void UnstartedRuntime::UnstartedClassNewInstance( } // If we're in a transaction, class must not be finalizable (it or a superclass has a finalizer). - if (Runtime::Current()->IsActiveTransaction()) { - if (h_klass->IsFinalizable()) { - AbortTransactionF(self, "Class for newInstance is finalizable: '%s'", - h_klass->PrettyClass().c_str()); - return; - } + Runtime* runtime = Runtime::Current(); + if (runtime->IsActiveTransaction() && + runtime->GetClassLinker()->TransactionAllocationConstraint(self, h_klass.Get())) { + DCHECK(self->IsExceptionPending()); + return; } // There are two situations in which we'll abort this run. @@ -314,7 +312,7 @@ void UnstartedRuntime::UnstartedClassNewInstance( // 2) If we can't find the default constructor. We'll postpone the exception to runtime. // Note that 2) could likely be handled here, but for safety abort the transaction. bool ok = false; - auto* cl = Runtime::Current()->GetClassLinker(); + auto* cl = runtime->GetClassLinker(); if (cl->EnsureInitialized(self, h_klass, true, true)) { ArtMethod* cons = h_klass->FindConstructor("()V", cl->GetImagePointerSize()); if (cons != nullptr && ShouldDenyAccessToMember(cons, shadow_frame)) { @@ -764,14 +762,13 @@ void UnstartedRuntime::UnstartedVmClassLoaderFindLoadedClass( if (self->IsExceptionPending()) { Runtime* runtime = Runtime::Current(); DCHECK_EQ(runtime->IsTransactionAborted(), - self->GetException()->GetClass()->DescriptorEquals( - Transaction::kAbortExceptionDescriptor)) + self->GetException()->GetClass()->DescriptorEquals(kTransactionAbortErrorDescriptor)) << self->GetException()->GetClass()->PrettyDescriptor(); if (runtime->IsActiveTransaction()) { // If we're not aborting the transaction yet, abort now. b/183691501 // See CheckExceptionGenerateClassNotFound() for more detailed explanation. if (!runtime->IsTransactionAborted()) { - AbortTransactionF(self, "ClassNotFoundException"); + runtime->AbortTransactionF(self, "ClassNotFoundException"); } } else { // If not in a transaction, it cannot be the transaction abort exception. Clear it. @@ -855,7 +852,9 @@ void UnstartedRuntime::UnstartedSystemArraycopy(Thread* self, return; } - if (Runtime::Current()->IsActiveTransaction() && !CheckWriteConstraint(self, dst_obj)) { + Runtime* runtime = Runtime::Current(); + if (runtime->IsActiveTransaction() && + runtime->GetClassLinker()->TransactionWriteConstraint(self, dst_obj)) { DCHECK(self->IsExceptionPending()); return; } @@ -1589,8 +1588,9 @@ void UnstartedRuntime::UnstartedJdkUnsafeCompareAndSwapLong( int64_t newValue = shadow_frame->GetVRegLong(arg_offset + 6); bool success; // Check whether we're in a transaction, call accordingly. - if (Runtime::Current()->IsActiveTransaction()) { - if (!CheckWriteConstraint(self, obj)) { + Runtime* runtime = Runtime::Current(); + if (runtime->IsActiveTransaction()) { + if (runtime->GetClassLinker()->TransactionWriteConstraint(self, obj)) { DCHECK(self->IsExceptionPending()); return; } @@ -1635,8 +1635,10 @@ void UnstartedRuntime::UnstartedJdkUnsafeCompareAndSwapObject( } bool success; // Check whether we're in a transaction, call accordingly. - if (Runtime::Current()->IsActiveTransaction()) { - if (!CheckWriteConstraint(self, obj) || !CheckWriteValueConstraint(self, new_value)) { + Runtime* runtime = Runtime::Current(); + if (runtime->IsActiveTransaction()) { + if (runtime->GetClassLinker()->TransactionWriteConstraint(self, obj) || + runtime->GetClassLinker()->TransactionWriteValueConstraint(self, new_value)) { DCHECK(self->IsExceptionPending()); return; } @@ -1682,8 +1684,10 @@ void UnstartedRuntime::UnstartedJdkUnsafePutReferenceVolatile(Thread* self, } int64_t offset = shadow_frame->GetVRegLong(arg_offset + 2); mirror::Object* value = shadow_frame->GetVRegReference(arg_offset + 4); - if (Runtime::Current()->IsActiveTransaction()) { - if (!CheckWriteConstraint(self, obj) || !CheckWriteValueConstraint(self, value)) { + Runtime* runtime = Runtime::Current(); + if (runtime->IsActiveTransaction()) { + if (runtime->GetClassLinker()->TransactionWriteConstraint(self, obj) || + runtime->GetClassLinker()->TransactionWriteValueConstraint(self, value)) { DCHECK(self->IsExceptionPending()); return; } @@ -1707,8 +1711,10 @@ void UnstartedRuntime::UnstartedJdkUnsafePutOrderedObject(Thread* self, int64_t offset = shadow_frame->GetVRegLong(arg_offset + 2); mirror::Object* new_value = shadow_frame->GetVRegReference(arg_offset + 4); std::atomic_thread_fence(std::memory_order_release); - if (Runtime::Current()->IsActiveTransaction()) { - if (!CheckWriteConstraint(self, obj) || !CheckWriteValueConstraint(self, new_value)) { + Runtime* runtime = Runtime::Current(); + if (runtime->IsActiveTransaction()) { + if (runtime->GetClassLinker()->TransactionWriteConstraint(self, obj) || + runtime->GetClassLinker()->TransactionWriteValueConstraint(self, new_value)) { DCHECK(self->IsExceptionPending()); return; } @@ -2154,8 +2160,9 @@ void UnstartedRuntime::UnstartedJNIJdkUnsafeCompareAndSwapInt( jint expectedValue = args[3]; jint newValue = args[4]; bool success; - if (Runtime::Current()->IsActiveTransaction()) { - if (!CheckWriteConstraint(self, obj)) { + Runtime* runtime = Runtime::Current(); + if (runtime->IsActiveTransaction()) { + if (runtime->GetClassLinker()->TransactionWriteConstraint(self, obj)) { DCHECK(self->IsExceptionPending()); return; } @@ -2211,8 +2218,10 @@ void UnstartedRuntime::UnstartedJNIJdkUnsafePutReference(Thread* self, } jlong offset = (static_cast<uint64_t>(args[2]) << 32) | args[1]; ObjPtr<mirror::Object> new_value = reinterpret_cast32<mirror::Object*>(args[3]); - if (Runtime::Current()->IsActiveTransaction()) { - if (!CheckWriteConstraint(self, obj) || !CheckWriteValueConstraint(self, new_value)) { + Runtime* runtime = Runtime::Current(); + if (runtime->IsActiveTransaction()) { + if (runtime->GetClassLinker()->TransactionWriteConstraint(self, obj) || + runtime->GetClassLinker()->TransactionWriteValueConstraint(self, new_value)) { DCHECK(self->IsExceptionPending()); return; } @@ -2420,12 +2429,16 @@ void UnstartedRuntime::Jni(Thread* self, ArtMethod* method, mirror::Object* rece // Clear out the result in case it's not zeroed out. result->SetL(nullptr); (*iter->second)(self, method, receiver, args, result); - } else if (Runtime::Current()->IsActiveTransaction()) { - AbortTransactionF(self, "Attempt to invoke native method in non-started runtime: %s", - ArtMethod::PrettyMethod(method).c_str()); } else { - LOG(FATAL) << "Calling native method " << ArtMethod::PrettyMethod(method) << " in an unstarted " - "non-transactional runtime"; + Runtime* runtime = Runtime::Current(); + if (runtime->IsActiveTransaction()) { + runtime->AbortTransactionF(self, + "Attempt to invoke native method in non-started runtime: %s", + ArtMethod::PrettyMethod(method).c_str()); + } else { + LOG(FATAL) << "Calling native method " << ArtMethod::PrettyMethod(method) + << " in an unstarted non-transactional runtime"; + } } } diff --git a/runtime/interpreter/unstarted_runtime_test.cc b/runtime/interpreter/unstarted_runtime_test.cc index 494eb756a4..44c1e2627a 100644 --- a/runtime/interpreter/unstarted_runtime_test.cc +++ b/runtime/interpreter/unstarted_runtime_test.cc @@ -25,6 +25,7 @@ #include "class_linker.h" #include "class_root-inl.h" #include "common_runtime_test.h" +#include "common_throws.h" #include "dex/descriptors_names.h" #include "dex/dex_instruction.h" #include "handle.h" @@ -42,7 +43,6 @@ #include "scoped_thread_state_change-inl.h" #include "shadow_frame-inl.h" #include "thread.h" -#include "transaction.h" #include "unstarted_runtime_list.h" namespace art HIDDEN { @@ -216,7 +216,7 @@ class UnstartedRuntimeTest : public CommonRuntimeTest { void PrepareForAborts() REQUIRES_SHARED(Locks::mutator_lock_) { ObjPtr<mirror::Object> result = Runtime::Current()->GetClassLinker()->FindClass( Thread::Current(), - Transaction::kAbortExceptionDescriptor, + kTransactionAbortErrorDescriptor, ScopedNullHandle<mirror::ClassLoader>()); CHECK(result != nullptr); } diff --git a/runtime/jit/jit_code_cache.cc b/runtime/jit/jit_code_cache.cc index 198ffcad25..7ea9efb0f9 100644 --- a/runtime/jit/jit_code_cache.cc +++ b/runtime/jit/jit_code_cache.cc @@ -342,8 +342,6 @@ const void* JitCodeCache::GetSavedEntryPointOfPreCompiledMethod(ArtMethod* metho auto it = saved_compiled_methods_map_.find(method); if (it != saved_compiled_methods_map_.end()) { code_ptr = it->second; - // Now that we're using the saved entrypoint, remove it from the saved map. - saved_compiled_methods_map_.erase(it); } } if (code_ptr != nullptr) { @@ -491,36 +489,33 @@ void JitCodeCache::FreeAllMethodHeaders( ->RemoveDependentsWithMethodHeaders(method_headers); } - { - ScopedCodeCacheWrite scc(private_region_); - for (const OatQuickMethodHeader* method_header : method_headers) { - FreeCodeAndData(method_header->GetCode()); - } - - // We have potentially removed a lot of debug info. Do maintenance pass to save space. - RepackNativeDebugInfoForJit(); + ScopedCodeCacheWrite scc(private_region_); + for (const OatQuickMethodHeader* method_header : method_headers) { + FreeCodeAndData(method_header->GetCode()); } + // We have potentially removed a lot of debug info. Do maintenance pass to save space. + RepackNativeDebugInfoForJit(); + // Check that the set of compiled methods exactly matches native debug information. // Does not check zygote methods since they can change concurrently. if (kIsDebugBuild && !Runtime::Current()->IsZygote()) { std::map<const void*, ArtMethod*> compiled_methods; - std::set<const void*> debug_info; VisitAllMethods([&](const void* addr, ArtMethod* method) { if (!IsInZygoteExecSpace(addr)) { CHECK(addr != nullptr && method != nullptr); compiled_methods.emplace(addr, method); } }); + std::set<const void*> debug_info; ForEachNativeDebugSymbol([&](const void* addr, size_t, const char* name) { addr = AlignDown(addr, GetInstructionSetInstructionAlignment(kRuntimeISA)); // Thumb-bit. - bool res = debug_info.emplace(addr).second; - CHECK(res) << "Duplicate debug info: " << addr << " " << name; + CHECK(debug_info.emplace(addr).second) << "Duplicate debug info: " << addr << " " << name; CHECK_EQ(compiled_methods.count(addr), 1u) << "Extra debug info: " << addr << " " << name; }); if (!debug_info.empty()) { // If debug-info generation is enabled. for (auto it : compiled_methods) { - CHECK_EQ(debug_info.count(it.first), 1u) << "Missing debug info"; + CHECK_EQ(debug_info.count(it.first), 1u) << "No debug info: " << it.second->PrettyMethod(); } CHECK_EQ(compiled_methods.size(), debug_info.size()); } @@ -535,67 +530,52 @@ void JitCodeCache::RemoveMethodsIn(Thread* self, const LinearAlloc& alloc) { // for entries in this set. And it's more efficient to iterate through // the CHA dependency map just once with an unordered_set. std::unordered_set<OatQuickMethodHeader*> method_headers; - MutexLock mu(self, *Locks::jit_lock_); - // We do not check if a code cache GC is in progress, as this method comes - // with the classlinker_classes_lock_ held, and suspending ourselves could - // lead to a deadlock. { - for (auto it = jni_stubs_map_.begin(); it != jni_stubs_map_.end();) { - it->second.RemoveMethodsIn(alloc); - if (it->second.GetMethods().empty()) { - method_headers.insert(OatQuickMethodHeader::FromCodePointer(it->second.GetCode())); - it = jni_stubs_map_.erase(it); - } else { - it->first.UpdateShorty(it->second.GetMethods().front()); - ++it; + MutexLock mu(self, *Locks::jit_lock_); + // We do not check if a code cache GC is in progress, as this method comes + // with the classlinker_classes_lock_ held, and suspending ourselves could + // lead to a deadlock. + { + for (auto it = jni_stubs_map_.begin(); it != jni_stubs_map_.end();) { + it->second.RemoveMethodsIn(alloc); + if (it->second.GetMethods().empty()) { + method_headers.insert(OatQuickMethodHeader::FromCodePointer(it->second.GetCode())); + it = jni_stubs_map_.erase(it); + } else { + it->first.UpdateShorty(it->second.GetMethods().front()); + ++it; + } } - } - for (auto it = zombie_jni_code_.begin(); it != zombie_jni_code_.end();) { - if (alloc.ContainsUnsafe(*it)) { - it = zombie_jni_code_.erase(it); - } else { - ++it; + for (auto it = method_code_map_.begin(); it != method_code_map_.end();) { + if (alloc.ContainsUnsafe(it->second)) { + method_headers.insert(OatQuickMethodHeader::FromCodePointer(it->first)); + VLOG(jit) << "JIT removed " << it->second->PrettyMethod() << ": " << it->first; + it = method_code_map_.erase(it); + } else { + ++it; + } } } - for (auto it = processed_zombie_jni_code_.begin(); it != processed_zombie_jni_code_.end();) { - if (alloc.ContainsUnsafe(*it)) { - it = processed_zombie_jni_code_.erase(it); + for (auto it = osr_code_map_.begin(); it != osr_code_map_.end();) { + if (alloc.ContainsUnsafe(it->first)) { + // Note that the code has already been pushed to method_headers in the loop + // above and is going to be removed in FreeCode() below. + it = osr_code_map_.erase(it); } else { ++it; } } - for (auto it = method_code_map_.begin(); it != method_code_map_.end();) { - if (alloc.ContainsUnsafe(it->second)) { - method_headers.insert(OatQuickMethodHeader::FromCodePointer(it->first)); - VLOG(jit) << "JIT removed " << it->second->PrettyMethod() << ": " << it->first; - it = method_code_map_.erase(it); - zombie_code_.erase(it->first); - processed_zombie_code_.erase(it->first); + for (auto it = profiling_infos_.begin(); it != profiling_infos_.end();) { + ProfilingInfo* info = it->second; + if (alloc.ContainsUnsafe(info->GetMethod())) { + private_region_.FreeWritableData(reinterpret_cast<uint8_t*>(info)); + it = profiling_infos_.erase(it); } else { ++it; } } + FreeAllMethodHeaders(method_headers); } - for (auto it = osr_code_map_.begin(); it != osr_code_map_.end();) { - DCHECK(!ContainsElement(zombie_code_, it->second)); - if (alloc.ContainsUnsafe(it->first)) { - // Note that the code has already been pushed to method_headers in the loop - // above and is going to be removed in FreeCode() below. - it = osr_code_map_.erase(it); - } else { - ++it; - } - } - for (auto it = profiling_infos_.begin(); it != profiling_infos_.end();) { - ProfilingInfo* info = it->second; - if (alloc.ContainsUnsafe(info->GetMethod())) { - private_region_.FreeWritableData(reinterpret_cast<uint8_t*>(info)); - it = profiling_infos_.erase(it); - } else { - ++it; - } - } - FreeAllMethodHeaders(method_headers); } bool JitCodeCache::IsWeakAccessEnabled(Thread* self) const { @@ -661,6 +641,18 @@ static void ClearMethodCounter(ArtMethod* method, bool was_warm) method->UpdateCounter(/* new_samples= */ 1); } +void JitCodeCache::WaitForPotentialCollectionToCompleteRunnable(Thread* self) { + while (collection_in_progress_) { + Locks::jit_lock_->Unlock(self); + { + ScopedThreadSuspension sts(self, ThreadState::kSuspended); + MutexLock mu(self, *Locks::jit_lock_); + WaitForPotentialCollectionToComplete(self); + } + Locks::jit_lock_->Lock(self); + } +} + bool JitCodeCache::Commit(Thread* self, JitMemoryRegion* region, ArtMethod* method, @@ -688,6 +680,9 @@ bool JitCodeCache::Commit(Thread* self, OatQuickMethodHeader* method_header = nullptr; { MutexLock mu(self, *Locks::jit_lock_); + // We need to make sure that there will be no jit-gcs going on and wait for any ongoing one to + // finish. + WaitForPotentialCollectionToCompleteRunnable(self); const uint8_t* code_ptr = region->CommitCode(reserved_code, code, stack_map_data); if (code_ptr == nullptr) { return false; @@ -787,6 +782,11 @@ bool JitCodeCache::Commit(Thread* self, method, method_header->GetEntryPoint()); } } + if (collection_in_progress_) { + // We need to update the live bitmap if there is a GC to ensure it sees this new + // code. + GetLiveBitmap()->AtomicTestAndSet(FromCodeToAllocation(code_ptr)); + } VLOG(jit) << "JIT added (kind=" << compilation_kind << ") " << ArtMethod::PrettyMethod(method) << "@" << method @@ -855,7 +855,6 @@ bool JitCodeCache::RemoveMethodLocked(ArtMethod* method, bool release_memory) { FreeCodeAndData(it->second.GetCode()); } jni_stubs_map_.erase(it); - zombie_jni_code_.erase(method); } else { it->first.UpdateShorty(it->second.GetMethods().front()); } @@ -986,6 +985,7 @@ bool JitCodeCache::Reserve(Thread* self, { ScopedThreadSuspension sts(self, ThreadState::kSuspended); MutexLock mu(self, *Locks::jit_lock_); + WaitForPotentialCollectionToComplete(self); ScopedCodeCacheWrite ccw(*region); code = region->AllocateCode(code_size); data = region->AllocateData(data_size); @@ -1002,8 +1002,8 @@ bool JitCodeCache::Reserve(Thread* self, << PrettySize(data_size); return false; } - // Increase the capacity and try again. - IncreaseCodeCacheCapacity(self); + // Run a code cache collection and try again. + GarbageCollectCache(self); } *reserved_code = ArrayRef<const uint8_t>(code, code_size); @@ -1081,6 +1081,11 @@ class MarkCodeClosure final : public Closure { Barrier* const barrier_; }; +void JitCodeCache::NotifyCollectionDone(Thread* self) { + collection_in_progress_ = false; + lock_cond_.Broadcast(self); +} + void JitCodeCache::MarkCompiledCodeOnThreadStacks(Thread* self) { Barrier barrier(0); size_t threads_running_checkpoint = 0; @@ -1098,111 +1103,86 @@ bool JitCodeCache::IsAtMaxCapacity() const { return private_region_.GetCurrentCapacity() == private_region_.GetMaxCapacity(); } -void JitCodeCache::IncreaseCodeCacheCapacity(Thread* self) { - MutexLock mu(self, *Locks::jit_lock_); - private_region_.IncreaseCodeCacheCapacity(); -} - -void JitCodeCache::RemoveUnmarkedCode(Thread* self) { +void JitCodeCache::GarbageCollectCache(Thread* self) { ScopedTrace trace(__FUNCTION__); - std::unordered_set<OatQuickMethodHeader*> method_headers; - ScopedDebugDisallowReadBarriers sddrb(self); - MutexLock mu(self, *Locks::jit_lock_); - // Iterate over all zombie code and remove entries that are not marked. - for (auto it = processed_zombie_code_.begin(); it != processed_zombie_code_.end();) { - const void* code_ptr = *it; - uintptr_t allocation = FromCodeToAllocation(code_ptr); - DCHECK(!IsInZygoteExecSpace(code_ptr)); - if (GetLiveBitmap()->Test(allocation)) { - ++it; - } else { - OatQuickMethodHeader* header = OatQuickMethodHeader::FromCodePointer(code_ptr); - method_headers.insert(header); - method_code_map_.erase(header->GetCode()); - VLOG(jit) << "JIT removed " << *it; - it = processed_zombie_code_.erase(it); - } - } - for (auto it = processed_zombie_jni_code_.begin(); it != processed_zombie_jni_code_.end();) { - ArtMethod* method = *it; - auto stub = jni_stubs_map_.find(JniStubKey(method)); - if (stub == jni_stubs_map_.end()) { - it = processed_zombie_jni_code_.erase(it); - continue; - } - JniStubData& data = stub->second; - if (!data.IsCompiled() || !ContainsElement(data.GetMethods(), method)) { - it = processed_zombie_jni_code_.erase(it); - } else if (method->GetEntryPointFromQuickCompiledCode() == - OatQuickMethodHeader::FromCodePointer(data.GetCode())->GetEntryPoint()) { - // The stub got reused for this method, remove ourselves from the zombie - // list. - it = processed_zombie_jni_code_.erase(it); - } else if (!GetLiveBitmap()->Test(FromCodeToAllocation(data.GetCode()))) { - data.RemoveMethod(method); - if (data.GetMethods().empty()) { - OatQuickMethodHeader* header = OatQuickMethodHeader::FromCodePointer(data.GetCode()); - method_headers.insert(header); - CHECK(ContainsPc(header)); - VLOG(jit) << "JIT removed native code of" << method->PrettyMethod(); - jni_stubs_map_.erase(stub); - } else { - stub->first.UpdateShorty(stub->second.GetMethods().front()); - } - it = processed_zombie_jni_code_.erase(it); + // Wait for an existing collection, or let everyone know we are starting one. + { + ScopedThreadSuspension sts(self, ThreadState::kSuspended); + MutexLock mu(self, *Locks::jit_lock_); + if (!garbage_collect_code_) { + private_region_.IncreaseCodeCacheCapacity(); + return; + } else if (WaitForPotentialCollectionToComplete(self)) { + return; } else { - ++it; + number_of_collections_++; + live_bitmap_.reset(CodeCacheBitmap::Create( + "code-cache-bitmap", + reinterpret_cast<uintptr_t>(private_region_.GetExecPages()->Begin()), + reinterpret_cast<uintptr_t>( + private_region_.GetExecPages()->Begin() + private_region_.GetCurrentCapacity() / 2))); + collection_in_progress_ = true; } } - FreeAllMethodHeaders(method_headers); -} -void JitCodeCache::AddZombieCode(ArtMethod* method, const void* entry_point) { - CHECK(ContainsPc(entry_point)); - CHECK(method->IsNative() || (method->GetEntryPointFromQuickCompiledCode() != entry_point)); - const void* code_ptr = OatQuickMethodHeader::FromEntryPoint(entry_point)->GetCode(); - if (!IsInZygoteExecSpace(code_ptr)) { - Thread* self = Thread::Current(); - if (Locks::jit_lock_->IsExclusiveHeld(self)) { - AddZombieCodeInternal(method, code_ptr); - } else { - MutexLock mu(Thread::Current(), *Locks::jit_lock_); - AddZombieCodeInternal(method, code_ptr); - } - } -} + TimingLogger logger("JIT code cache timing logger", true, VLOG_IS_ON(jit)); + { + TimingLogger::ScopedTiming st("Code cache collection", &logger); + VLOG(jit) << "Do code cache collection, code=" + << PrettySize(CodeCacheSize()) + << ", data=" << PrettySize(DataCacheSize()); -class JitGcTask final : public Task { - public: - JitGcTask() {} + DoCollection(self); - void Run(Thread* self) override { - Runtime::Current()->GetJit()->GetCodeCache()->DoCollection(self); - } + VLOG(jit) << "After code cache collection, code=" + << PrettySize(CodeCacheSize()) + << ", data=" << PrettySize(DataCacheSize()); - void Finalize() override { - delete this; + { + MutexLock mu(self, *Locks::jit_lock_); + private_region_.IncreaseCodeCacheCapacity(); + live_bitmap_.reset(nullptr); + NotifyCollectionDone(self); + } } -}; + Runtime::Current()->GetJit()->AddTimingLogger(logger); +} -void JitCodeCache::AddZombieCodeInternal(ArtMethod* method, const void* code_ptr) { - if (method->IsNative()) { - CHECK(jni_stubs_map_.find(JniStubKey(method)) != jni_stubs_map_.end()); - zombie_jni_code_.insert(method); - } else { - zombie_code_.insert(code_ptr); - } - // Arbitrary threshold of number of zombie code before doing a GC. - static constexpr size_t kNumberOfZombieCodeThreshold = kIsDebugBuild ? 1 : 1000; - size_t number_of_code_to_delete = - zombie_code_.size() + zombie_jni_code_.size() + osr_code_map_.size(); - if (number_of_code_to_delete >= kNumberOfZombieCodeThreshold) { - JitThreadPool* pool = Runtime::Current()->GetJit()->GetThreadPool(); - if (pool != nullptr && !gc_task_scheduled_) { - gc_task_scheduled_ = true; - pool->AddTask(Thread::Current(), new JitGcTask()); +void JitCodeCache::RemoveUnmarkedCode(Thread* self) { + ScopedTrace trace(__FUNCTION__); + ScopedDebugDisallowReadBarriers sddrb(self); + std::unordered_set<OatQuickMethodHeader*> method_headers; + { + MutexLock mu(self, *Locks::jit_lock_); + // Iterate over all compiled code and remove entries that are not marked. + for (auto it = jni_stubs_map_.begin(); it != jni_stubs_map_.end();) { + JniStubData* data = &it->second; + if (IsInZygoteExecSpace(data->GetCode()) || + !data->IsCompiled() || + GetLiveBitmap()->Test(FromCodeToAllocation(data->GetCode()))) { + ++it; + } else { + method_headers.insert(OatQuickMethodHeader::FromCodePointer(data->GetCode())); + for (ArtMethod* method : data->GetMethods()) { + VLOG(jit) << "JIT removed (JNI) " << method->PrettyMethod() << ": " << data->GetCode(); + } + it = jni_stubs_map_.erase(it); + } + } + for (auto it = method_code_map_.begin(); it != method_code_map_.end();) { + const void* code_ptr = it->first; + uintptr_t allocation = FromCodeToAllocation(code_ptr); + if (IsInZygoteExecSpace(code_ptr) || GetLiveBitmap()->Test(allocation)) { + ++it; + } else { + OatQuickMethodHeader* header = OatQuickMethodHeader::FromCodePointer(code_ptr); + method_headers.insert(header); + VLOG(jit) << "JIT removed " << it->second->PrettyMethod() << ": " << it->first; + it = method_code_map_.erase(it); + } } + FreeAllMethodHeaders(method_headers); } } @@ -1254,58 +1234,51 @@ void JitCodeCache::ResetHotnessCounter(ArtMethod* method, Thread* self) { void JitCodeCache::DoCollection(Thread* self) { ScopedTrace trace(__FUNCTION__); - { ScopedDebugDisallowReadBarriers sddrb(self); MutexLock mu(self, *Locks::jit_lock_); - if (!garbage_collect_code_) { - return; - } else if (WaitForPotentialCollectionToComplete(self)) { - return; + // Mark compiled code that are entrypoints of ArtMethods. Compiled code that is not + // an entry point is either: + // - an osr compiled code, that will be removed if not in a thread call stack. + // - discarded compiled code, that will be removed if not in a thread call stack. + for (const auto& entry : jni_stubs_map_) { + const JniStubData& data = entry.second; + const void* code_ptr = data.GetCode(); + if (IsInZygoteExecSpace(code_ptr)) { + continue; + } + const OatQuickMethodHeader* method_header = OatQuickMethodHeader::FromCodePointer(code_ptr); + for (ArtMethod* method : data.GetMethods()) { + if (method_header->GetEntryPoint() == method->GetEntryPointFromQuickCompiledCode()) { + GetLiveBitmap()->AtomicTestAndSet(FromCodeToAllocation(code_ptr)); + break; + } + } } - collection_in_progress_ = true; - number_of_collections_++; - live_bitmap_.reset(CodeCacheBitmap::Create( - "code-cache-bitmap", - reinterpret_cast<uintptr_t>(private_region_.GetExecPages()->Begin()), - reinterpret_cast<uintptr_t>( - private_region_.GetExecPages()->Begin() + private_region_.GetCurrentCapacity() / 2))); - processed_zombie_code_.insert(zombie_code_.begin(), zombie_code_.end()); - zombie_code_.clear(); - processed_zombie_jni_code_.insert(zombie_jni_code_.begin(), zombie_jni_code_.end()); - zombie_jni_code_.clear(); + for (const auto& it : method_code_map_) { + ArtMethod* method = it.second; + const void* code_ptr = it.first; + if (IsInZygoteExecSpace(code_ptr)) { + continue; + } + const OatQuickMethodHeader* method_header = OatQuickMethodHeader::FromCodePointer(code_ptr); + if (method_header->GetEntryPoint() == method->GetEntryPointFromQuickCompiledCode()) { + GetLiveBitmap()->AtomicTestAndSet(FromCodeToAllocation(code_ptr)); + } + } + // Empty osr method map, as osr compiled code will be deleted (except the ones // on thread stacks). - for (auto it = osr_code_map_.begin(); it != osr_code_map_.end(); ++it) { - processed_zombie_code_.insert(it->second); - } osr_code_map_.clear(); } - TimingLogger logger("JIT code cache timing logger", true, VLOG_IS_ON(jit)); - { - TimingLogger::ScopedTiming st("Code cache collection", &logger); - - { - ScopedObjectAccess soa(self); - // Run a checkpoint on all threads to mark the JIT compiled code they are running. - MarkCompiledCodeOnThreadStacks(self); - - // Remove zombie code which hasn't been marked. - RemoveUnmarkedCode(self); - } - - MutexLock mu(self, *Locks::jit_lock_); - live_bitmap_.reset(nullptr); - NotifyCollectionDone(self); - } - Runtime::Current()->GetJit()->AddTimingLogger(logger); -} + // Run a checkpoint on all threads to mark the JIT compiled code they are running. + MarkCompiledCodeOnThreadStacks(self); -void JitCodeCache::NotifyCollectionDone(Thread* self) { - collection_in_progress_ = false; - gc_task_scheduled_ = false; - lock_cond_.Broadcast(self); + // At this point, mutator threads are still running, and entrypoints of methods can + // change. We do know they cannot change to a code cache entry that is not marked, + // therefore we can safely remove those entries. + RemoveUnmarkedCode(self); } OatQuickMethodHeader* JitCodeCache::LookupMethodHeader(uintptr_t pc, ArtMethod* method) { @@ -1409,7 +1382,7 @@ ProfilingInfo* JitCodeCache::AddProfilingInfo(Thread* self, } if (info == nullptr) { - IncreaseCodeCacheCapacity(self); + GarbageCollectCache(self); MutexLock mu(self, *Locks::jit_lock_); info = AddProfilingInfoInternal(self, method, inline_cache_entries, branch_cache_entries); } @@ -1637,6 +1610,11 @@ bool JitCodeCache::NotifyCompilationOf(ArtMethod* method, // as well change them back as this stub shall not be collected anyway and this // can avoid a few expensive GenericJNI calls. data->UpdateEntryPoints(entrypoint); + if (collection_in_progress_) { + if (!IsInZygoteExecSpace(data->GetCode())) { + GetLiveBitmap()->AtomicTestAndSet(FromCodeToAllocation(data->GetCode())); + } + } } return new_compilation; } else { diff --git a/runtime/jit/jit_code_cache.h b/runtime/jit/jit_code_cache.h index 7de29d4024..96fc7e2706 100644 --- a/runtime/jit/jit_code_cache.h +++ b/runtime/jit/jit_code_cache.h @@ -285,9 +285,11 @@ class JitCodeCache { REQUIRES_SHARED(Locks::mutator_lock_) REQUIRES(!Locks::jit_lock_); void FreeLocked(JitMemoryRegion* region, const uint8_t* code, const uint8_t* data) + REQUIRES_SHARED(Locks::mutator_lock_) REQUIRES(Locks::jit_lock_); - void IncreaseCodeCacheCapacity(Thread* self) + // Perform a collection on the code cache. + EXPORT void GarbageCollectCache(Thread* self) REQUIRES(!Locks::jit_lock_) REQUIRES_SHARED(Locks::mutator_lock_); @@ -421,20 +423,9 @@ class JitCodeCache { Thread* self) REQUIRES_SHARED(Locks::mutator_lock_); - // NO_THREAD_SAFETY_ANALYSIS because we may be called with the JIT lock held - // or not. The implementation of this method handles the two cases. - void AddZombieCode(ArtMethod* method, const void* code_ptr) NO_THREAD_SAFETY_ANALYSIS; - - EXPORT void DoCollection(Thread* self) - REQUIRES(!Locks::jit_lock_); - private: JitCodeCache(); - void AddZombieCodeInternal(ArtMethod* method, const void* code_ptr) - REQUIRES(Locks::jit_lock_) - REQUIRES_SHARED(Locks::mutator_lock_); - ProfilingInfo* AddProfilingInfoInternal(Thread* self, ArtMethod* method, const std::vector<uint32_t>& inline_cache_entries, @@ -442,16 +433,20 @@ class JitCodeCache { REQUIRES(Locks::jit_lock_) REQUIRES_SHARED(Locks::mutator_lock_); + // If a collection is in progress, wait for it to finish. Must be called with the mutator lock. + // The non-mutator lock version should be used if possible. This method will release then + // re-acquire the mutator lock. + void WaitForPotentialCollectionToCompleteRunnable(Thread* self) + REQUIRES(Locks::jit_lock_, !Roles::uninterruptible_) REQUIRES_SHARED(Locks::mutator_lock_); + // If a collection is in progress, wait for it to finish. Return // whether the thread actually waited. bool WaitForPotentialCollectionToComplete(Thread* self) - REQUIRES(Locks::jit_lock_) REQUIRES_SHARED(!Locks::mutator_lock_); - - // Notify all waiting threads that a collection is done. - void NotifyCollectionDone(Thread* self) REQUIRES(Locks::jit_lock_); + REQUIRES(Locks::jit_lock_) REQUIRES(!Locks::mutator_lock_); // Remove CHA dependents and underlying allocations for entries in `method_headers`. void FreeAllMethodHeaders(const std::unordered_set<OatQuickMethodHeader*>& method_headers) + REQUIRES_SHARED(Locks::mutator_lock_) REQUIRES(Locks::jit_lock_) REQUIRES(!Locks::cha_lock_); @@ -467,7 +462,8 @@ class JitCodeCache { // Free code and data allocations for `code_ptr`. void FreeCodeAndData(const void* code_ptr) - REQUIRES(Locks::jit_lock_); + REQUIRES(Locks::jit_lock_) + REQUIRES_SHARED(Locks::mutator_lock_); // Number of bytes allocated in the code cache. size_t CodeCacheSize() REQUIRES(!Locks::jit_lock_); @@ -481,9 +477,16 @@ class JitCodeCache { // Number of bytes allocated in the data cache. size_t DataCacheSizeLocked() REQUIRES(Locks::jit_lock_); + // Notify all waiting threads that a collection is done. + void NotifyCollectionDone(Thread* self) REQUIRES(Locks::jit_lock_); + // Return whether the code cache's capacity is at its maximum. bool IsAtMaxCapacity() const REQUIRES(Locks::jit_lock_); + void DoCollection(Thread* self) + REQUIRES(!Locks::jit_lock_) + REQUIRES_SHARED(Locks::mutator_lock_); + void RemoveUnmarkedCode(Thread* self) REQUIRES(!Locks::jit_lock_) REQUIRES_SHARED(Locks::mutator_lock_); @@ -560,28 +563,18 @@ class JitCodeCache { // ProfilingInfo objects we have allocated. SafeMap<ArtMethod*, ProfilingInfo*> profiling_infos_ GUARDED_BY(Locks::jit_lock_); - // Zombie code and JNI methods to consider for collection. - std::set<const void*> zombie_code_ GUARDED_BY(Locks::jit_lock_); - std::set<ArtMethod*> zombie_jni_code_ GUARDED_BY(Locks::jit_lock_); - - std::set<const void*> processed_zombie_code_ GUARDED_BY(Locks::jit_lock_); - std::set<ArtMethod*> processed_zombie_jni_code_ GUARDED_BY(Locks::jit_lock_); - // Methods that the zygote has compiled and can be shared across processes // forked from the zygote. ZygoteMap zygote_map_; // -------------- JIT GC related data structures ----------------------- // - // Condition to wait on during collection and for accessing weak references in inline caches. + // Condition to wait on during collection. ConditionVariable lock_cond_ GUARDED_BY(Locks::jit_lock_); // Whether there is a code cache collection in progress. bool collection_in_progress_ GUARDED_BY(Locks::jit_lock_); - // Whether a GC task is already scheduled. - bool gc_task_scheduled_ GUARDED_BY(Locks::jit_lock_); - // Bitmap for collecting code and data. std::unique_ptr<CodeCacheBitmap> live_bitmap_; diff --git a/runtime/mirror/dex_cache.h b/runtime/mirror/dex_cache.h index 8910dfcbb5..c1d71e1501 100644 --- a/runtime/mirror/dex_cache.h +++ b/runtime/mirror/dex_cache.h @@ -374,7 +374,7 @@ class MANAGED DexCache final : public Object { void SetClassLoader(ObjPtr<ClassLoader> class_loader) REQUIRES_SHARED(Locks::mutator_lock_); - ObjPtr<ClassLoader> GetClassLoader() REQUIRES_SHARED(Locks::mutator_lock_); + EXPORT ObjPtr<ClassLoader> GetClassLoader() REQUIRES_SHARED(Locks::mutator_lock_); template <VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags, ReadBarrierOption kReadBarrierOption = kWithReadBarrier, diff --git a/runtime/oat/aot_class_linker.cc b/runtime/oat/aot_class_linker.cc index 6640214dc0..09fb92452d 100644 --- a/runtime/oat/aot_class_linker.cc +++ b/runtime/oat/aot_class_linker.cc @@ -16,11 +16,13 @@ #include "aot_class_linker.h" +#include "base/stl_util.h" #include "class_status.h" #include "compiler_callbacks.h" #include "dex/class_reference.h" #include "gc/heap.h" #include "handle_scope-inl.h" +#include "interpreter/active_transaction_checker.h" #include "mirror/class-inl.h" #include "runtime.h" #include "verifier/verifier_enums.h" @@ -130,6 +132,12 @@ verifier::FailureKind AotClassLinker::PerformClassVerification( return ClassLinker::PerformClassVerification(self, verifier_deps, klass, log_level, error_msg); } +static const std::vector<const DexFile*>* gAppImageDexFiles = nullptr; + +void AotClassLinker::SetAppImageDexFiles(const std::vector<const DexFile*>* app_image_dex_files) { + gAppImageDexFiles = app_image_dex_files; +} + bool AotClassLinker::CanReferenceInBootImageExtensionOrAppImage( ObjPtr<mirror::Class> klass, gc::Heap* heap) { // Do not allow referencing a class or instance of a class defined in a dex file @@ -159,8 +167,25 @@ bool AotClassLinker::CanReferenceInBootImageExtensionOrAppImage( } } + auto can_reference_dex_cache = [&](ObjPtr<mirror::DexCache> dex_cache) + REQUIRES_SHARED(Locks::mutator_lock_) { + // We cannot reference a boot image dex cache for classes + // that were not themselves in the boot image. + if (heap->ObjectIsInBootImageSpace(dex_cache)) { + return false; + } + // App image compilation can pull in dex files from parent or library class loaders. + // Classes from such dex files cannot be included or referenced in the current app image + // to avoid conflicts with classes in the parent or library class loader's app image. + if (gAppImageDexFiles != nullptr && + !ContainsElement(*gAppImageDexFiles, dex_cache->GetDexFile())) { + return false; + } + return true; + }; + // Check the class itself. - if (heap->ObjectIsInBootImageSpace(klass->GetDexCache())) { + if (!can_reference_dex_cache(klass->GetDexCache())) { return false; } @@ -168,7 +193,7 @@ bool AotClassLinker::CanReferenceInBootImageExtensionOrAppImage( ObjPtr<mirror::Class> superclass = klass->GetSuperClass(); while (!heap->ObjectIsInBootImageSpace(superclass)) { DCHECK(superclass != nullptr); // Cannot skip Object which is in the primary boot image. - if (heap->ObjectIsInBootImageSpace(superclass->GetDexCache())) { + if (!can_reference_dex_cache(superclass->GetDexCache())) { return false; } superclass = superclass->GetSuperClass(); @@ -180,7 +205,7 @@ bool AotClassLinker::CanReferenceInBootImageExtensionOrAppImage( ObjPtr<mirror::Class> interface = if_table->GetInterface(i); DCHECK(interface != nullptr); if (!heap->ObjectIsInBootImageSpace(interface) && - heap->ObjectIsInBootImageSpace(interface->GetDexCache())) { + !can_reference_dex_cache(interface->GetDexCache())) { return false; } } @@ -193,7 +218,7 @@ bool AotClassLinker::CanReferenceInBootImageExtensionOrAppImage( for (auto& m : k->GetVirtualMethods(pointer_size)) { ObjPtr<mirror::Class> declaring_class = m.GetDeclaringClass(); CHECK(heap->ObjectIsInBootImageSpace(declaring_class) || - !heap->ObjectIsInBootImageSpace(declaring_class->GetDexCache())); + can_reference_dex_cache(declaring_class->GetDexCache())); } k = k->GetSuperClass(); } @@ -228,4 +253,21 @@ void AotClassLinker::SetEnablePublicSdkChecks(bool enabled) { } } +bool AotClassLinker::TransactionWriteConstraint(Thread* self, ObjPtr<mirror::Object> obj) const { + DCHECK(Runtime::Current()->IsActiveTransaction()); + return interpreter::ActiveTransactionChecker::WriteConstraint(self, obj); +} + +bool AotClassLinker::TransactionWriteValueConstraint( + Thread* self, ObjPtr<mirror::Object> value) const { + DCHECK(Runtime::Current()->IsActiveTransaction()); + return interpreter::ActiveTransactionChecker::WriteValueConstraint(self, value); +} + +bool AotClassLinker::TransactionAllocationConstraint(Thread* self, + ObjPtr<mirror::Class> klass) const { + DCHECK(Runtime::Current()->IsActiveTransaction()); + return interpreter::ActiveTransactionChecker::AllocationConstraint(self, klass); +} + } // namespace art diff --git a/runtime/oat/aot_class_linker.h b/runtime/oat/aot_class_linker.h index 492674ea10..18689bed7f 100644 --- a/runtime/oat/aot_class_linker.h +++ b/runtime/oat/aot_class_linker.h @@ -35,6 +35,8 @@ class AotClassLinker : public ClassLinker { explicit AotClassLinker(InternTable *intern_table); ~AotClassLinker(); + EXPORT static void SetAppImageDexFiles(const std::vector<const DexFile*>* app_image_dex_files); + EXPORT static bool CanReferenceInBootImageExtensionOrAppImage( ObjPtr<mirror::Class> klass, gc::Heap* heap) REQUIRES_SHARED(Locks::mutator_lock_); @@ -48,6 +50,14 @@ class AotClassLinker : public ClassLinker { bool DenyAccessBasedOnPublicSdk([[maybe_unused]] const char* type_descriptor) const override; void SetEnablePublicSdkChecks(bool enabled) override; + // Transaction constraint checks for AOT compilation. + bool TransactionWriteConstraint(Thread* self, ObjPtr<mirror::Object> obj) const override + REQUIRES_SHARED(Locks::mutator_lock_); + bool TransactionWriteValueConstraint(Thread* self, ObjPtr<mirror::Object> value) const override + REQUIRES_SHARED(Locks::mutator_lock_); + bool TransactionAllocationConstraint(Thread* self, ObjPtr<mirror::Class> klass) const override + REQUIRES_SHARED(Locks::mutator_lock_); + protected: // Overridden version of PerformClassVerification allows skipping verification if the class was // previously verified but unloaded. diff --git a/runtime/oat/elf_file.cc b/runtime/oat/elf_file.cc index 30e0197470..91af410471 100644 --- a/runtime/oat/elf_file.cc +++ b/runtime/oat/elf_file.cc @@ -1025,6 +1025,20 @@ bool ElfFileImpl<ElfTypes>::GetLoadedSize(size_t* size, std::string* error_msg) return GetLoadedAddressRange(&vaddr_begin, size, error_msg); } +template <typename ElfTypes> +size_t ElfFileImpl<ElfTypes>::GetElfSegmentAlignmentFromFile() const { + // Return the alignment of the first loadable program segment. + for (Elf_Word i = 0; i < GetProgramHeaderNum(); i++) { + Elf_Phdr* program_header = GetProgramHeader(i); + if (program_header->p_type != PT_LOAD) { + continue; + } + return program_header->p_align; + } + LOG(ERROR) << "No loadable segment found in ELF file " << file_path_; + return 0; +} + // Base on bionic phdr_table_get_load_size template <typename ElfTypes> bool ElfFileImpl<ElfTypes>::GetLoadedAddressRange(/*out*/uint8_t** vaddr_begin, @@ -1671,6 +1685,10 @@ bool ElfFile::GetLoadedSize(size_t* size, std::string* error_msg) const { DELEGATE_TO_IMPL(GetLoadedSize, size, error_msg); } +size_t ElfFile::GetElfSegmentAlignmentFromFile() const { + DELEGATE_TO_IMPL(GetElfSegmentAlignmentFromFile); +} + bool ElfFile::Strip(File* file, std::string* error_msg) { std::unique_ptr<ElfFile> elf_file(ElfFile::Open(file, true, false, /*low_4gb=*/false, error_msg)); if (elf_file.get() == nullptr) { diff --git a/runtime/oat/elf_file.h b/runtime/oat/elf_file.h index a9218106de..125b4441f1 100644 --- a/runtime/oat/elf_file.h +++ b/runtime/oat/elf_file.h @@ -82,6 +82,8 @@ class ElfFile { bool GetLoadedSize(size_t* size, std::string* error_msg) const; + size_t GetElfSegmentAlignmentFromFile() const; + // Strip an ELF file of unneeded debugging information. // Returns true on success, false on failure. static bool Strip(File* file, std::string* error_msg); diff --git a/runtime/oat/elf_file_impl.h b/runtime/oat/elf_file_impl.h index ffc6a2c081..67c60e906c 100644 --- a/runtime/oat/elf_file_impl.h +++ b/runtime/oat/elf_file_impl.h @@ -116,6 +116,9 @@ class ElfFileImpl { // Retrieves the expected size when the file is loaded at runtime. Returns true if successful. bool GetLoadedSize(size_t* size, std::string* error_msg) const; + // Get the alignment of the first loadable program segment. Return 0 if no loadable segment found. + size_t GetElfSegmentAlignmentFromFile() const; + // Load segments into memory based on PT_LOAD program headers. // executable is true at run time, false at compile time. bool Load(File* file, diff --git a/runtime/oat/oat.h b/runtime/oat/oat.h index 84c169a365..b850fe8dd5 100644 --- a/runtime/oat/oat.h +++ b/runtime/oat/oat.h @@ -44,8 +44,8 @@ std::ostream& operator<<(std::ostream& stream, StubType stub_type); class EXPORT PACKED(4) OatHeader { public: static constexpr std::array<uint8_t, 4> kOatMagic { { 'o', 'a', 't', '\n' } }; - // Last oat version changed reason: BSS mappings for boot image extension. - static constexpr std::array<uint8_t, 4> kOatVersion{{'2', '4', '3', '\0'}}; + // Last oat version changed reason: Implement `HLoadClass::LoadKind::kAppImageRelRo`. + static constexpr std::array<uint8_t, 4> kOatVersion{{'2', '4', '4', '\0'}}; static constexpr const char* kDex2OatCmdLineKey = "dex2oat-cmdline"; static constexpr const char* kDebuggableKey = "debuggable"; diff --git a/runtime/oat/oat_file.cc b/runtime/oat/oat_file.cc index 889b78f015..97db7569cf 100644 --- a/runtime/oat/oat_file.cc +++ b/runtime/oat/oat_file.cc @@ -373,6 +373,11 @@ bool OatFileBase::ComputeFields(const std::string& file_path, std::string* error } // Readjust to be non-inclusive upper bound. data_img_rel_ro_end_ += sizeof(uint32_t); + data_img_rel_ro_app_image_ = + FindDynamicSymbolAddress("oatdataimgrelroappimage", &symbol_error_msg); + if (data_img_rel_ro_app_image_ == nullptr) { + data_img_rel_ro_app_image_ = data_img_rel_ro_end_; + } } bss_begin_ = const_cast<uint8_t*>(FindDynamicSymbolAddress("oatbss", &symbol_error_msg)); @@ -648,10 +653,15 @@ bool OatFileBase::Setup(int zip_fd, if (!IsAligned<sizeof(uint32_t)>(data_img_rel_ro_begin_) || !IsAligned<sizeof(uint32_t)>(data_img_rel_ro_end_) || - data_img_rel_ro_begin_ > data_img_rel_ro_end_) { - *error_msg = ErrorPrintf("unaligned or unordered databimgrelro symbol(s): begin = %p, end = %p", - data_img_rel_ro_begin_, - data_img_rel_ro_end_); + !IsAligned<sizeof(uint32_t)>(data_img_rel_ro_app_image_) || + data_img_rel_ro_begin_ > data_img_rel_ro_end_ || + data_img_rel_ro_begin_ > data_img_rel_ro_app_image_ || + data_img_rel_ro_app_image_ > data_img_rel_ro_end_) { + *error_msg = ErrorPrintf( + "unaligned or unordered databimgrelro symbol(s): begin = %p, end = %p, app_image = %p", + data_img_rel_ro_begin_, + data_img_rel_ro_end_, + data_img_rel_ro_app_image_); return false; } @@ -2015,6 +2025,7 @@ OatFile::OatFile(const std::string& location, bool is_executable) end_(nullptr), data_img_rel_ro_begin_(nullptr), data_img_rel_ro_end_(nullptr), + data_img_rel_ro_app_image_(nullptr), bss_begin_(nullptr), bss_end_(nullptr), bss_methods_(nullptr), @@ -2022,6 +2033,7 @@ OatFile::OatFile(const std::string& location, bool is_executable) is_executable_(is_executable), vdex_begin_(nullptr), vdex_end_(nullptr), + app_image_begin_(nullptr), secondary_lookup_lock_("OatFile secondary lookup lock", kOatFileSecondaryLookupLock) { CHECK(!location_.empty()); } @@ -2054,9 +2066,25 @@ const uint8_t* OatFile::DexEnd() const { ArrayRef<const uint32_t> OatFile::GetBootImageRelocations() const { if (data_img_rel_ro_begin_ != nullptr) { - const uint32_t* relocations = reinterpret_cast<const uint32_t*>(data_img_rel_ro_begin_); - const uint32_t* relocations_end = reinterpret_cast<const uint32_t*>(data_img_rel_ro_end_); - return ArrayRef<const uint32_t>(relocations, relocations_end - relocations); + const uint32_t* boot_image_relocations = + reinterpret_cast<const uint32_t*>(data_img_rel_ro_begin_); + const uint32_t* boot_image_relocations_end = + reinterpret_cast<const uint32_t*>(data_img_rel_ro_app_image_); + return ArrayRef<const uint32_t>( + boot_image_relocations, boot_image_relocations_end - boot_image_relocations); + } else { + return ArrayRef<const uint32_t>(); + } +} + +ArrayRef<const uint32_t> OatFile::GetAppImageRelocations() const { + if (data_img_rel_ro_begin_ != nullptr) { + const uint32_t* app_image_relocations = + reinterpret_cast<const uint32_t*>(data_img_rel_ro_app_image_); + const uint32_t* app_image_relocations_end = + reinterpret_cast<const uint32_t*>(data_img_rel_ro_end_); + return ArrayRef<const uint32_t>( + app_image_relocations, app_image_relocations_end - app_image_relocations); } else { return ArrayRef<const uint32_t>(); } @@ -2476,7 +2504,7 @@ void OatFile::InitializeRelocations() const { DCHECK(IsExecutable()); // Initialize the .data.img.rel.ro section. - if (!GetBootImageRelocations().empty()) { + if (DataImgRelRoEnd() != DataImgRelRoBegin()) { uint8_t* reloc_begin = const_cast<uint8_t*>(DataImgRelRoBegin()); CheckedCall(mprotect, "un-protect boot image relocations", @@ -2487,6 +2515,13 @@ void OatFile::InitializeRelocations() const { for (const uint32_t& relocation : GetBootImageRelocations()) { const_cast<uint32_t&>(relocation) += boot_image_begin; } + if (!GetAppImageRelocations().empty()) { + CHECK(app_image_begin_ != nullptr); + uint32_t app_image_begin = reinterpret_cast32<uint32_t>(app_image_begin_); + for (const uint32_t& relocation : GetAppImageRelocations()) { + const_cast<uint32_t&>(relocation) += app_image_begin; + } + } CheckedCall(mprotect, "protect boot image relocations", reloc_begin, diff --git a/runtime/oat/oat_file.h b/runtime/oat/oat_file.h index e17f4c43ff..579fb049f0 100644 --- a/runtime/oat/oat_file.h +++ b/runtime/oat/oat_file.h @@ -181,6 +181,12 @@ class OatFile { ClassLoaderContext* context, std::string* error_msg); + // Set the start of the app image. + // Needed for initializing app image relocations in the .data.img.rel.ro section. + void SetAppImageBegin(uint8_t* app_image_begin) const { + app_image_begin_ = app_image_begin; + } + // Return whether the `OatFile` uses a vdex-only file. bool IsBackedByVdexOnly() const; @@ -329,6 +335,10 @@ class OatFile { return DataImgRelRoEnd() - DataImgRelRoBegin(); } + size_t DataImgRelRoAppImageOffset() const { + return DataImgRelRoAppImage() - DataImgRelRoBegin(); + } + size_t BssSize() const { return BssEnd() - BssBegin(); } @@ -356,6 +366,7 @@ class OatFile { const uint8_t* DataImgRelRoBegin() const { return data_img_rel_ro_begin_; } const uint8_t* DataImgRelRoEnd() const { return data_img_rel_ro_end_; } + const uint8_t* DataImgRelRoAppImage() const { return data_img_rel_ro_app_image_; } const uint8_t* BssBegin() const { return bss_begin_; } const uint8_t* BssEnd() const { return bss_end_; } @@ -367,6 +378,7 @@ class OatFile { EXPORT const uint8_t* DexEnd() const; EXPORT ArrayRef<const uint32_t> GetBootImageRelocations() const; + EXPORT ArrayRef<const uint32_t> GetAppImageRelocations() const; EXPORT ArrayRef<ArtMethod*> GetBssMethods() const; EXPORT ArrayRef<GcRoot<mirror::Object>> GetBssGcRoots() const; @@ -429,16 +441,20 @@ class OatFile { // Pointer to the end of the .data.img.rel.ro section, if present, otherwise null. const uint8_t* data_img_rel_ro_end_; + // Pointer to the beginning of the app image relocations in the .data.img.rel.ro section, + // if present, otherwise null. + const uint8_t* data_img_rel_ro_app_image_; + // Pointer to the .bss section, if present, otherwise null. uint8_t* bss_begin_; // Pointer to the end of the .bss section, if present, otherwise null. uint8_t* bss_end_; - // Pointer to the beginning of the ArtMethod*s in .bss section, if present, otherwise null. + // Pointer to the beginning of the ArtMethod*s in the .bss section, if present, otherwise null. uint8_t* bss_methods_; - // Pointer to the beginning of the GC roots in .bss section, if present, otherwise null. + // Pointer to the beginning of the GC roots in the .bss section, if present, otherwise null. uint8_t* bss_roots_; // Was this oat_file loaded executable? @@ -450,6 +466,9 @@ class OatFile { // Pointer to the end of the .vdex section, if present, otherwise null. uint8_t* vdex_end_; + // Pointer to the beginning of the app image, if any. + mutable uint8_t* app_image_begin_; + // Owning storage for the OatDexFile objects. std::vector<const OatDexFile*> oat_dex_files_storage_; diff --git a/runtime/oat/oat_file_manager.cc b/runtime/oat/oat_file_manager.cc index 11333b0249..fea361b8ee 100644 --- a/runtime/oat/oat_file_manager.cc +++ b/runtime/oat/oat_file_manager.cc @@ -312,6 +312,7 @@ std::vector<std::unique_ptr<const DexFile>> OatFileManager::OpenDexFilesFromOat( hs.NewHandle(soa.Decode<mirror::ClassLoader>(class_loader))); // Can not load app image without class loader. if (h_loader != nullptr) { + oat_file->SetAppImageBegin(image_space->Begin()); std::string temp_error_msg; // Add image space has a race condition since other threads could be reading from the // spaces array. @@ -344,6 +345,7 @@ std::vector<std::unique_ptr<const DexFile>> OatFileManager::OpenDexFilesFromOat( } } else { LOG(INFO) << "Failed to add image file: " << temp_error_msg; + oat_file->SetAppImageBegin(nullptr); dex_files.clear(); { ScopedThreadSuspension sts(self, ThreadState::kSuspended); diff --git a/runtime/runtime.cc b/runtime/runtime.cc index df66c5e175..3b27fb6faa 100644 --- a/runtime/runtime.cc +++ b/runtime/runtime.cc @@ -3011,6 +3011,22 @@ void Runtime::ThrowTransactionAbortError(Thread* self) { GetTransaction()->ThrowAbortError(self, nullptr); } +void Runtime::AbortTransactionF(Thread* self, const char* fmt, ...) { + va_list args; + va_start(args, fmt); + AbortTransactionV(self, fmt, args); + va_end(args); +} + +void Runtime::AbortTransactionV(Thread* self, const char* fmt, va_list args) { + CHECK(IsActiveTransaction()); + // Constructs abort message. + std::string abort_msg; + android::base::StringAppendV(&abort_msg, fmt, args); + // Throws an exception so we can abort the transaction and rollback every change. + AbortTransactionAndThrowAbortError(self, abort_msg); +} + void Runtime::RecordWriteFieldBoolean(mirror::Object* obj, MemberOffset field_offset, uint8_t value, diff --git a/runtime/runtime.h b/runtime/runtime.h index 1a1c28bff5..a48bb55755 100644 --- a/runtime/runtime.h +++ b/runtime/runtime.h @@ -646,6 +646,13 @@ class Runtime { void ThrowTransactionAbortError(Thread* self) REQUIRES_SHARED(Locks::mutator_lock_); + void AbortTransactionF(Thread* self, const char* fmt, ...) + __attribute__((__format__(__printf__, 3, 4))) + REQUIRES_SHARED(Locks::mutator_lock_); + + void AbortTransactionV(Thread* self, const char* fmt, va_list args) + REQUIRES_SHARED(Locks::mutator_lock_); + void RecordWriteFieldBoolean(mirror::Object* obj, MemberOffset field_offset, uint8_t value, diff --git a/runtime/runtime_image.cc b/runtime/runtime_image.cc index b134c79286..997ea2fde6 100644 --- a/runtime/runtime_image.cc +++ b/runtime/runtime_image.cc @@ -964,9 +964,7 @@ class RuntimeImageHelper { entrypoint = boot_jni_stub; } } - copy->SetNativePointer(ArtMethod::EntryPointFromQuickCompiledCodeOffset(kRuntimePointerSize), - entrypoint, - kRuntimePointerSize); + copy->SetEntryPointFromQuickCompiledCode(entrypoint); if (method->IsNative()) { StubType stub_type = method->IsCriticalNative() diff --git a/runtime/runtime_test.cc b/runtime/runtime_test.cc index 210ad7d711..cd0c862d82 100644 --- a/runtime/runtime_test.cc +++ b/runtime/runtime_test.cc @@ -21,9 +21,14 @@ #include "android-base/logging.h" #include "base/locks.h" #include "base/mutex.h" +#include "oat/elf_file.h" #include "runtime.h" #include "thread-current-inl.h" +#ifdef ART_TARGET_ANDROID +#include "android-base/properties.h" +#endif + namespace art HIDDEN { class RuntimeTest : public CommonRuntimeTest {}; @@ -75,4 +80,33 @@ TEST_F(RuntimeTest, AbortFromUnattachedThread) { }, ::testing::KilledBySignal(SIGABRT), kDeathRegex); } +// It is possible to run tests that validate an existing deployed on-device ART APEX ('standalone' +// tests). If these tests expect to load ELF files with a particular alignment, but those ELF files +// were created with a different alignment, there will be many difficult-to-debug failures. This +// test aims to identify this mismatch, related to whether or not the runtimes were built to be +// page-size agnostic. +TEST_F(RuntimeTest, ElfAlignmentMismatch) { +#ifdef ART_TARGET_ANDROID + bool platform_pga = android::base::GetBoolProperty("ro.product.build.no_bionic_page_size_macro", + false); + if (kPageSizeAgnostic != platform_pga) { + LOG(WARNING) << "Test configured with kPageSizeAgnostic=" << kPageSizeAgnostic << ", but " + << "platform ro.product.build.no_bionic_page_size_macro=" << platform_pga << "."; + } +#endif + // Determine the alignment of the ART APEX by reading the alignment of boot.oat. + std::string core_oat_location = GetSystemImageFilename(GetCoreOatLocation().c_str(), kRuntimeISA); + std::unique_ptr<File> core_oat_file(OS::OpenFileForReading(core_oat_location.c_str())); + ASSERT_TRUE(core_oat_file.get() != nullptr) << core_oat_location; + + std::string error_msg; + std::unique_ptr<ElfFile> elf_file(ElfFile::Open(core_oat_file.get(), + /*writable=*/false, + /*program_header_only=*/true, + /*low_4gb=*/false, + &error_msg)); + ASSERT_TRUE(elf_file != nullptr) << error_msg; + EXPECT_EQ(kElfSegmentAlignment, elf_file->GetElfSegmentAlignmentFromFile()); +} + } // namespace art diff --git a/runtime/transaction.cc b/runtime/transaction.cc index b796df620e..9a8fcd9739 100644 --- a/runtime/transaction.cc +++ b/runtime/transaction.cc @@ -20,6 +20,7 @@ #include "base/mutex-inl.h" #include "base/stl_util.h" +#include "common_throws.h" #include "dex/descriptors_names.h" #include "gc/accounting/card_table-inl.h" #include "gc/heap.h" @@ -104,17 +105,15 @@ void Transaction::Abort(const std::string& abort_message) { void Transaction::ThrowAbortError(Thread* self, const std::string* abort_message) { const bool rethrow = (abort_message == nullptr); if (kIsDebugBuild && rethrow) { - CHECK(IsAborted()) << "Rethrow " << DescriptorToDot(Transaction::kAbortExceptionDescriptor) + CHECK(IsAborted()) << "Rethrow " << DescriptorToDot(kTransactionAbortErrorDescriptor) << " while transaction is not aborted"; } if (rethrow) { // Rethrow an exception with the earlier abort message stored in the transaction. - self->ThrowNewWrappedException(Transaction::kAbortExceptionDescriptor, - GetAbortMessage().c_str()); + self->ThrowNewWrappedException(kTransactionAbortErrorDescriptor, GetAbortMessage().c_str()); } else { // Throw an exception with the given abort message. - self->ThrowNewWrappedException(Transaction::kAbortExceptionDescriptor, - abort_message->c_str()); + self->ThrowNewWrappedException(kTransactionAbortErrorDescriptor, abort_message->c_str()); } } @@ -141,16 +140,15 @@ bool Transaction::WriteValueConstraint(ObjPtr<mirror::Object> value) const { if (value == nullptr) { return false; // We can always store null values. } - gc::Heap* heap = Runtime::Current()->GetHeap(); if (IsStrict()) { // TODO: Should we restrict writes the same way as for boot image extension? return false; - } else if (heap->GetBootImageSpaces().empty()) { + } else if (heap_->GetBootImageSpaces().empty()) { return false; // No constraints for boot image. } else { // Boot image extension. ObjPtr<mirror::Class> klass = value->IsClass() ? value->AsClass() : value->GetClass(); - return !AotClassLinker::CanReferenceInBootImageExtensionOrAppImage(klass, heap); + return !AotClassLinker::CanReferenceInBootImageExtensionOrAppImage(klass, heap_); } } diff --git a/runtime/transaction.h b/runtime/transaction.h index 9f3282f8d9..847120234d 100644 --- a/runtime/transaction.h +++ b/runtime/transaction.h @@ -46,8 +46,6 @@ template<class MirrorType> class ObjPtr; class Transaction final { public: - static constexpr const char* kAbortExceptionDescriptor = "Ldalvik/system/TransactionAbortError;"; - Transaction(bool strict, mirror::Class* root, ArenaStack* arena_stack, ArenaPool* arena_pool); ~Transaction(); diff --git a/runtime/transaction_test.cc b/runtime/transaction_test.cc index c4eb7e808b..db9d123521 100644 --- a/runtime/transaction_test.cc +++ b/runtime/transaction_test.cc @@ -20,6 +20,7 @@ #include "art_method-inl.h" #include "class_linker-inl.h" #include "common_runtime_test.h" +#include "common_throws.h" #include "dex/dex_file.h" #include "mirror/array-alloc-inl.h" #include "mirror/class-alloc-inl.h" @@ -52,8 +53,7 @@ class TransactionTest : public CommonRuntimeTest { class_linker_->EnsureInitialized(soa.Self(), h_klass, true, true); ASSERT_TRUE(h_klass->IsInitialized()); - h_klass.Assign(class_linker_->FindSystemClass(soa.Self(), - Transaction::kAbortExceptionDescriptor)); + h_klass.Assign(class_linker_->FindSystemClass(soa.Self(), kTransactionAbortErrorDescriptor)); ASSERT_TRUE(h_klass != nullptr); class_linker_->EnsureInitialized(soa.Self(), h_klass, true, true); ASSERT_TRUE(h_klass->IsInitialized()); diff --git a/test/536-checker-intrinsic-optimization/src/Main.java b/test/536-checker-intrinsic-optimization/src/Main.java index c809278d8e..6519e5c2ad 100644 --- a/test/536-checker-intrinsic-optimization/src/Main.java +++ b/test/536-checker-intrinsic-optimization/src/Main.java @@ -19,6 +19,10 @@ public class Main { public static String mediumString = generateString(300); public static String largeString = generateString(2000); + public static String smallNonAsciiString = generateNonAsciiString(100); + public static String mediumNonAsciiString = generateNonAsciiString(300); + public static String largeNonAsciiString = generateNonAsciiString(2000); + public static String generateString(int length) { // Generate a string in the ASCII range that will // use string compression. @@ -30,6 +34,14 @@ public class Main { return sb.toString(); } + public static String generateNonAsciiString(int length) { + StringBuilder sb = new StringBuilder(); + for (int i = 0; i < length; i++) { + sb.append(Character.valueOf('\uFFFF')); + } + return sb.toString(); + } + public static void assertIntEquals(int expected, int result) { if (expected != result) { throw new Error("Expected: " + expected + ", found: " + result); @@ -102,6 +114,38 @@ public class Main { // Substring > 8 characters. assertStringEquals(smallString.substring(7, 28), stringGetCharsRange(smallString, 7, 28, 17)); + // Single character. + assertStringEquals("\uFFFF", stringGetCharsAndBack("\uFFFF")); + + // Strings < 8 characters. + assertStringEquals("\uFFFF\uFFFF\uFFFF\uFFFF\uFFFF", + stringGetCharsAndBack("\uFFFF\uFFFF\uFFFF\uFFFF\uFFFF")); + + // Strings > 8 characters of various lengths. + assertStringEquals(smallNonAsciiString, stringGetCharsAndBack(smallNonAsciiString)); + assertStringEquals(mediumNonAsciiString, stringGetCharsAndBack(mediumNonAsciiString)); + assertStringEquals(largeNonAsciiString, stringGetCharsAndBack(largeNonAsciiString)); + + // Get only a substring: + // Substring < 8 characters. + assertStringEquals(smallNonAsciiString.substring(5, 10), + stringGetCharsRange(smallNonAsciiString, 5, 10, 0)); + // Substring > 8 characters. + assertStringEquals(smallNonAsciiString.substring(7, 28), + stringGetCharsRange(smallNonAsciiString, 7, 28, 0)); + + // Get full string with offset in the char array. + assertStringEquals(smallNonAsciiString, + stringGetCharsAndBackOffset(smallNonAsciiString, 17)); + + // Get a substring with an offset in the char array. + // Substring < 8 characters. + assertStringEquals(smallNonAsciiString.substring(5, 10), + stringGetCharsRange(smallNonAsciiString, 5, 10, 17)); + // Substring > 8 characters. + assertStringEquals(smallNonAsciiString.substring(7, 28), + stringGetCharsRange(smallNonAsciiString, 7, 28, 17)); + try { $opt$noinline$stringCharAt("abc", -1); throw new Error("Should throw SIOOB."); diff --git a/test/667-jit-jni-stub/jit_jni_stub_test.cc b/test/667-jit-jni-stub/jit_jni_stub_test.cc index b7946f39ed..2b8e14c03d 100644 --- a/test/667-jit-jni-stub/jit_jni_stub_test.cc +++ b/test/667-jit-jni-stub/jit_jni_stub_test.cc @@ -39,11 +39,9 @@ extern "C" JNIEXPORT void Java_Main_jitGc(JNIEnv*, jclass) { CHECK(Runtime::Current()->GetJit() != nullptr); jit::JitCodeCache* cache = Runtime::Current()->GetJit()->GetCodeCache(); - { - ScopedObjectAccess soa(Thread::Current()); - cache->InvalidateAllCompiledCode(); - } - cache->DoCollection(Thread::Current()); + ScopedObjectAccess soa(Thread::Current()); + cache->InvalidateAllCompiledCode(); + cache->GarbageCollectCache(Thread::Current()); } extern "C" JNIEXPORT diff --git a/test/667-jit-jni-stub/src/Main.java b/test/667-jit-jni-stub/src/Main.java index 05039670d1..c0829b5100 100644 --- a/test/667-jit-jni-stub/src/Main.java +++ b/test/667-jit-jni-stub/src/Main.java @@ -67,6 +67,9 @@ public class Main { assertFalse(hasJitCompiledEntrypoint(Main.class, "callThrough")); assertFalse(hasJitCompiledCode(Main.class, "callThrough")); callThrough(Main.class, "testMixedFramesOnStackStage2"); + // We have just returned through the JIT-compiled JNI stub, so it must still + // be compiled (though not necessarily with the entrypoint pointing to it). + assertTrue(hasJitCompiledCode(Main.class, "callThrough")); // Though the callThrough() is on the stack, that frame is using the GenericJNI // and does not prevent the collection of the JNI stub. testStubCanBeCollected(); @@ -91,6 +94,10 @@ public class Main { } public static void testStubCanBeCollected() { + assertTrue(hasJitCompiledCode(Main.class, "callThrough")); + doJitGcsUntilFullJitGcIsScheduled(); + assertFalse(hasJitCompiledEntrypoint(Main.class, "callThrough")); + assertTrue(hasJitCompiledCode(Main.class, "callThrough")); jitGc(); // JIT GC without callThrough() on the stack should collect the callThrough() stub. assertFalse(hasJitCompiledEntrypoint(Main.class, "callThrough")); assertFalse(hasJitCompiledCode(Main.class, "callThrough")); diff --git a/test/732-checker-app-image/expected-stdout.txt b/test/732-checker-app-image/expected-stdout.txt index a8deeec6de..cb6b036407 100644 --- a/test/732-checker-app-image/expected-stdout.txt +++ b/test/732-checker-app-image/expected-stdout.txt @@ -1,2 +1,5 @@ AppImageClass NonAppImageClass +SecondaryAppImageClass +SecondaryNonAppImageClass +SecondaryClassWithUnmetDependency diff --git a/test/732-checker-app-image/profile b/test/732-checker-app-image/profile index 924c319453..a35f977d7c 100644 --- a/test/732-checker-app-image/profile +++ b/test/732-checker-app-image/profile @@ -5,3 +5,8 @@ SHLMain;->$noinline$getNonAppImageClass()Ljava/lang/Class; SHLMain;->$noinline$callAppImageClassNop()V SHLMain;->$noinline$callAppImageClassWithClinitNop()V SHLMain;->$noinline$callNonAppImageClassNop()V +LSecondaryAppImageClass; +LSecondaryClassWithUnmetDependency; +SHLSecondary;->$noinline$getSecondaryAppImageClass()Ljava/lang/Class; +SHLSecondary;->$noinline$getSecondaryNonAppImageClass()Ljava/lang/Class; +SHLSecondary;->$noinline$getSecondaryClassWithUnmetDependency()Ljava/lang/Class; diff --git a/test/732-checker-app-image/run.py b/test/732-checker-app-image/run.py index 3dc165b8e8..16658852ad 100644 --- a/test/732-checker-app-image/run.py +++ b/test/732-checker-app-image/run.py @@ -16,5 +16,12 @@ def run(ctx, args): - # We need a profile to tell dex2oat to include classes in the final app image - ctx.default_run(args, profile=True) + if args.jvm: + ctx.default_run(args) + else: + ctx.default_run( + args, + # We need a profile to tell dex2oat to include classes in the final app image + profile=True, + # Append graphs for checker tests (we run dex2oat twice) with `--dump-cfg-append`. + Xcompiler_option=["--dump-cfg-append"]) diff --git a/test/732-checker-app-image/src-ex/Secondary.java b/test/732-checker-app-image/src-ex/Secondary.java new file mode 100644 index 0000000000..e544e6c162 --- /dev/null +++ b/test/732-checker-app-image/src-ex/Secondary.java @@ -0,0 +1,51 @@ +/* + * Copyright (C) 2024 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. + */ + +public class Secondary { + public static void main() { + System.out.println($noinline$getSecondaryAppImageClass().getName()); + System.out.println($noinline$getSecondaryNonAppImageClass().getName()); + System.out.println($noinline$getSecondaryClassWithUnmetDependency().getName()); + } + + /// CHECK-START: java.lang.Class Secondary.$noinline$getSecondaryAppImageClass() builder (after) + /// CHECK: LoadClass load_kind:AppImageRelRo in_image:true + public static Class<?> $noinline$getSecondaryAppImageClass() { + return SecondaryAppImageClass.class; + } + + /// CHECK-START: java.lang.Class Secondary.$noinline$getSecondaryNonAppImageClass() builder (after) + /// CHECK: LoadClass load_kind:BssEntry in_image:false + public static Class<?> $noinline$getSecondaryNonAppImageClass() { + return SecondaryNonAppImageClass.class; + } + + /// CHECK-START: java.lang.Class Secondary.$noinline$getSecondaryClassWithUnmetDependency() builder (after) + /// CHECK: LoadClass load_kind:BssEntry in_image:false + public static Class<?> $noinline$getSecondaryClassWithUnmetDependency() { + return SecondaryClassWithUnmetDependency.class; + } +} + +class SecondaryAppImageClass { // Included in the profile. +} + +class SecondaryNonAppImageClass { // Not included in the profile. +} + +// Included in the profile but with interface defined in parent class loader +// and therefore unsuitable for inclusion in the app image. +class SecondaryClassWithUnmetDependency implements PrimaryInterface {} diff --git a/test/732-checker-app-image/src/Main.java b/test/732-checker-app-image/src/Main.java index 200708a20b..815902940d 100644 --- a/test/732-checker-app-image/src/Main.java +++ b/test/732-checker-app-image/src/Main.java @@ -14,18 +14,43 @@ * limitations under the License. */ +import java.lang.reflect.Constructor; +import java.lang.reflect.Method; + public class Main { - public static void main(String args[]) { + public static String TEST_NAME = "732-checker-app-image"; + + public static ClassLoader getSecondaryClassLoader() throws Exception { + String location = System.getenv("DEX_LOCATION"); + try { + Class<?> class_loader_class = Class.forName("dalvik.system.PathClassLoader"); + Constructor<?> ctor = + class_loader_class.getConstructor(String.class, ClassLoader.class); + return (ClassLoader) ctor.newInstance(location + "/" + TEST_NAME + "-ex.jar", + Main.class.getClassLoader()); + } catch (ClassNotFoundException e) { + // Running on RI. Use URLClassLoader. + return new java.net.URLClassLoader( + new java.net.URL[] { new java.net.URL("file://" + location + "/classes-ex/") }); + } + } + + public static void main(String args[]) throws Exception { System.out.println($noinline$getAppImageClass().getName()); System.out.println($noinline$getNonAppImageClass().getName()); $noinline$callAppImageClassNop(); $noinline$callAppImageClassWithClinitNop(); $noinline$callNonAppImageClassNop(); + + ClassLoader secondaryLoader = getSecondaryClassLoader(); + Class<?> secondaryClass = Class.forName("Secondary", true, secondaryLoader); + Method secondaryMain = secondaryClass.getMethod("main"); + secondaryMain.invoke(null); } /// CHECK-START: java.lang.Class Main.$noinline$getAppImageClass() builder (after) - /// CHECK: LoadClass load_kind:BssEntry in_image:true + /// CHECK: LoadClass load_kind:AppImageRelRo in_image:true public static Class<?> $noinline$getAppImageClass() { return AppImageClass.class; } @@ -52,22 +77,20 @@ public class Main { } /// CHECK-START: void Main.$noinline$callAppImageClassWithClinitNop() builder (after) - /// CHECK: LoadClass load_kind:BssEntry in_image:true gen_clinit_check:false + /// CHECK: LoadClass load_kind:AppImageRelRo in_image:true gen_clinit_check:false /// CHECK: ClinitCheck /// CHECK: InvokeStaticOrDirect clinit_check:explicit /// CHECK-START: void Main.$noinline$callAppImageClassWithClinitNop() inliner (after) - /// CHECK: LoadClass load_kind:BssEntry in_image:true gen_clinit_check:false + /// CHECK: LoadClass load_kind:AppImageRelRo in_image:true gen_clinit_check:false /// CHECK: ClinitCheck /// CHECK-START: void Main.$noinline$callAppImageClassWithClinitNop() inliner (after) /// CHECK-NOT: InvokeStaticOrDirect /// CHECK-START: void Main.$noinline$callAppImageClassWithClinitNop() prepare_for_register_allocation (after) - /// CHECK: LoadClass load_kind:BssEntry in_image:true gen_clinit_check:true - - /// CHECK-START: void Main.$noinline$callAppImageClassWithClinitNop() prepare_for_register_allocation (after) - /// CHECK-NOT: ClinitCheck + /// CHECK: LoadClass load_kind:AppImageRelRo in_image:true gen_clinit_check:false + /// CHECK: ClinitCheck public static void $noinline$callAppImageClassWithClinitNop() { AppImageClassWithClinit.$inline$nop(); } diff --git a/test/732-checker-app-image/src/PrimaryInterface.java b/test/732-checker-app-image/src/PrimaryInterface.java new file mode 100644 index 0000000000..4b2e3f4a7e --- /dev/null +++ b/test/732-checker-app-image/src/PrimaryInterface.java @@ -0,0 +1,17 @@ +/* + * Copyright (C) 2024 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. + */ + +public interface PrimaryInterface {} |