diff options
73 files changed, 1437 insertions, 428 deletions
diff --git a/build/Android.common_build.mk b/build/Android.common_build.mk index f5a95fa0cf..ad551a13e9 100644 --- a/build/Android.common_build.mk +++ b/build/Android.common_build.mk @@ -46,8 +46,13 @@ ifeq ($(ART_BUILD_HOST_DEBUG),false) $(info Disabling ART_BUILD_HOST_DEBUG) endif +ifeq ($(ART_USE_RESTRICTED_MODE),true) +# TODO(Simulator): Support read barriers. +ART_USE_READ_BARRIER := false +else # Enable the read barrier by default. ART_USE_READ_BARRIER ?= true +endif ART_CPP_EXTENSION := .cc diff --git a/build/art.go b/build/art.go index 3aaa3eee21..c4df20d3d7 100644 --- a/build/art.go +++ b/build/art.go @@ -44,6 +44,14 @@ func globalFlags(ctx android.LoadHookContext) ([]string, []string) { tlab = true } + if ctx.Config().IsEnvTrue("ART_USE_RESTRICTED_MODE") { + cflags = append(cflags, "-DART_USE_RESTRICTED_MODE=1") + asflags = append(asflags, "-DART_USE_RESTRICTED_MODE=1") + + // TODO(Simulator): Support other GC types. + gcType = "MS" + } + cflags = append(cflags, "-DART_DEFAULT_GC_TYPE_IS_"+gcType) if ctx.Config().IsEnvTrue("ART_HEAP_POISONING") { @@ -56,7 +64,8 @@ func globalFlags(ctx android.LoadHookContext) ([]string, []string) { // TODO: deprecate and then eventually remove ART_USE_GENERATIONAL_CC in favor of // ART_USE_GENERATIONAL_GC - if !ctx.Config().IsEnvFalse("ART_USE_READ_BARRIER") && ctx.Config().ArtUseReadBarrier() { + if !ctx.Config().IsEnvFalse("ART_USE_READ_BARRIER") && ctx.Config().ArtUseReadBarrier() && + !ctx.Config().IsEnvTrue("ART_USE_RESTRICTED_MODE") { // Used to change the read barrier type. Valid values are BAKER, TABLELOOKUP. // The default is BAKER. barrierType := ctx.Config().GetenvWithDefault("ART_READ_BARRIER_TYPE", "BAKER") diff --git a/compiler/Android.bp b/compiler/Android.bp index ef56e7f3ad..73994f7d95 100644 --- a/compiler/Android.bp +++ b/compiler/Android.bp @@ -438,7 +438,6 @@ art_cc_defaults { name: "art_compiler_tests_defaults", device_common_data: [ ":art-gtest-jars-ExceptionHandle", - ":art-gtest-jars-Interfaces", ":art-gtest-jars-Main", ":art-gtest-jars-MyClassNatives", ], @@ -489,8 +488,6 @@ art_cc_defaults { "optimizing/stack_map_test.cc", "optimizing/superblock_cloner_test.cc", "optimizing/suspend_check_test.cc", - "utils/atomic_dex_ref_map_test.cc", - "utils/dedupe_set_test.cc", "optimizing/codegen_test.cc", "optimizing/instruction_simplifier_test.cc", diff --git a/compiler/art_standalone_compiler_tests.xml b/compiler/art_standalone_compiler_tests.xml index c2065dd766..813a6f10d0 100644 --- a/compiler/art_standalone_compiler_tests.xml +++ b/compiler/art_standalone_compiler_tests.xml @@ -26,7 +26,6 @@ <target_preparer class="com.android.compatibility.common.tradefed.targetprep.FilePusher"> <option name="cleanup" value="true" /> <option name="push" value="art-gtest-jars-ExceptionHandle.jar->/data/local/tmp/art_standalone_compiler_tests/art-gtest-jars-ExceptionHandle.jar" /> - <option name="push" value="art-gtest-jars-Interfaces.jar->/data/local/tmp/art_standalone_compiler_tests/art-gtest-jars-Interfaces.jar" /> <option name="push" value="art-gtest-jars-Main.jar->/data/local/tmp/art_standalone_compiler_tests/art-gtest-jars-Main.jar" /> <option name="push" value="art-gtest-jars-MyClassNatives.jar->/data/local/tmp/art_standalone_compiler_tests/art-gtest-jars-MyClassNatives.jar" /> </target_preparer> diff --git a/compiler/common_compiler_test.cc b/compiler/common_compiler_test.cc index 26800637fa..e54c85f747 100644 --- a/compiler/common_compiler_test.cc +++ b/compiler/common_compiler_test.cc @@ -41,7 +41,6 @@ #include "oat/oat_quick_method_header.h" #include "scoped_thread_state_change-inl.h" #include "thread-current-inl.h" -#include "utils/atomic_dex_ref_map-inl.h" namespace art HIDDEN { diff --git a/compiler/driver/compiler_options.h b/compiler/driver/compiler_options.h index 36ecf88199..a3957ce232 100644 --- a/compiler/driver/compiler_options.h +++ b/compiler/driver/compiler_options.h @@ -101,7 +101,13 @@ class CompilerOptions final { } bool IsJniCompilationEnabled() const { +#ifdef ART_USE_RESTRICTED_MODE + // TODO(Simulator): Support JNICompiler. + // Without the JNI compiler, GenericJNITrampoline will be used for JNI calls. + return false; +#else return CompilerFilter::IsJniCompilationEnabled(compiler_filter_); +#endif } bool IsVerificationEnabled() const { diff --git a/compiler/optimizing/code_generator.cc b/compiler/optimizing/code_generator.cc index e84cfcbe80..5c2e4dbc51 100644 --- a/compiler/optimizing/code_generator.cc +++ b/compiler/optimizing/code_generator.cc @@ -1569,6 +1569,7 @@ bool CodeGenerator::CanMoveNullCheckToUser(HNullCheck* null_check) { void CodeGenerator::MaybeRecordImplicitNullCheck(HInstruction* instr) { HNullCheck* null_check = instr->GetImplicitNullCheck(); if (null_check != nullptr) { + DCHECK(compiler_options_.GetImplicitNullChecks()); RecordPcInfo(null_check); } } diff --git a/compiler/optimizing/intrinsics_arm64.cc b/compiler/optimizing/intrinsics_arm64.cc index 31e617baec..98aa5600b4 100644 --- a/compiler/optimizing/intrinsics_arm64.cc +++ b/compiler/optimizing/intrinsics_arm64.cc @@ -200,12 +200,18 @@ class InvokePolymorphicSlowPathARM64 : public SlowPathCodeARM64 { #undef __ bool IntrinsicLocationsBuilderARM64::TryDispatch(HInvoke* invoke) { +#ifdef ART_USE_RESTRICTED_MODE + // TODO(Simulator): support intrinsics. + USE(invoke); + return false; +#else Dispatch(invoke); LocationSummary* res = invoke->GetLocations(); if (res == nullptr) { return false; } return res->Intrinsified(); +#endif // ART_USE_RESTRICTED_MODE } #define __ masm-> diff --git a/compiler/optimizing/optimizing_compiler.cc b/compiler/optimizing/optimizing_compiler.cc index ef84827653..970771424b 100644 --- a/compiler/optimizing/optimizing_compiler.cc +++ b/compiler/optimizing/optimizing_compiler.cc @@ -759,6 +759,51 @@ CompiledMethod* OptimizingCompiler::Emit(ArenaAllocator* allocator, return compiled_method; } +#ifdef ART_USE_RESTRICTED_MODE + +// This class acts as a filter and enables gradual enablement of ART Simulator work - we +// compile (and hence simulate) only limited types of methods. +class CompilationFilterForRestrictedMode : public HGraphDelegateVisitor { + public: + explicit CompilationFilterForRestrictedMode(HGraph* graph) + : HGraphDelegateVisitor(graph), + has_unsupported_instructions_(false) {} + + // Returns true if the graph contains instructions which are not currently supported in + // the restricted mode. + bool GraphRejected() const { return has_unsupported_instructions_; } + + private: + void VisitInstruction(HInstruction*) override { + // Currently we don't support compiling methods unless they were annotated with $compile$. + RejectGraph(); + } + void RejectGraph() { + has_unsupported_instructions_ = true; + } + + bool has_unsupported_instructions_; +}; + +// Returns whether an ArtMethod, specified by a name, should be compiled. Used in restricted +// mode. +// +// In restricted mode, the simulator will execute only those methods which are compiled; thus +// this is going to be an effective filter for methods to be simulated. +// +// TODO(Simulator): compile and simulate all the methods as in regular host mode. +bool ShouldMethodBeCompiled(HGraph* graph, const std::string& method_name) { + if (method_name.find("$compile$") != std::string::npos) { + return true; + } + + CompilationFilterForRestrictedMode filter_visitor(graph); + filter_visitor.VisitReversePostOrder(); + + return !filter_visitor.GraphRejected(); +} +#endif // ART_USE_RESTRICTED_MODE + CodeGenerator* OptimizingCompiler::TryCompile(ArenaAllocator* allocator, ArenaStack* arena_stack, const DexCompilationUnit& dex_compilation_unit, @@ -958,6 +1003,17 @@ CodeGenerator* OptimizingCompiler::TryCompile(ArenaAllocator* allocator, return nullptr; } +#ifdef ART_USE_RESTRICTED_MODE + // Check whether the method should be compiled according to the compilation filter. Note: this + // relies on a LocationSummary being available for each instruction so should take place after + // register allocation does liveness analysis. + // TODO(Simulator): support and compile all methods. + std::string method_name = dex_file.PrettyMethod(method_idx); + if (!ShouldMethodBeCompiled(graph, method_name)) { + return nullptr; + } +#endif // ART_USE_RESTRICTED_MODE + codegen->Compile(); pass_observer.DumpDisassembly(); @@ -977,6 +1033,11 @@ CodeGenerator* OptimizingCompiler::TryCompileIntrinsic( const DexFile& dex_file = *dex_compilation_unit.GetDexFile(); uint32_t method_idx = dex_compilation_unit.GetDexMethodIndex(); + // TODO(Simulator): Reenable compilation of intrinsics. +#ifdef ART_USE_RESTRICTED_MODE + return nullptr; +#endif // ART_USE_RESTRICTED_MODE + // Always use the Thumb-2 assembler: some runtime functionality // (like implicit stack overflow checks) assume Thumb-2. DCHECK_NE(instruction_set, InstructionSet::kArm); @@ -1149,6 +1210,8 @@ CompiledMethod* OptimizingCompiler::Compile(const dex::CodeItem* code_item, } } + // TODO(Simulator): Check for $opt$ in method name and that such method is compiled. +#ifndef ART_USE_RESTRICTED_MODE if (kIsDebugBuild && compiler_options.CompileArtTest() && IsInstructionSetSupported(compiler_options.GetInstructionSet())) { @@ -1160,6 +1223,7 @@ CompiledMethod* OptimizingCompiler::Compile(const dex::CodeItem* code_item, bool shouldCompile = method_name.find("$opt$") != std::string::npos; DCHECK_IMPLIES(compiled_method == nullptr, !shouldCompile) << "Didn't compile " << method_name; } +#endif // #ifndef ART_USE_RESTRICTED_MODE return compiled_method; } diff --git a/dex2oat/Android.bp b/dex2oat/Android.bp index 5df1fbbad2..f97aef2deb 100644 --- a/dex2oat/Android.bp +++ b/dex2oat/Android.bp @@ -421,6 +421,7 @@ art_cc_defaults { ":art-gtest-jars-Dex2oatVdexTestDex", ":art-gtest-jars-ImageLayoutA", ":art-gtest-jars-ImageLayoutB", + ":art-gtest-jars-Interfaces", ":art-gtest-jars-LinkageTest", ":art-gtest-jars-Main", ":art-gtest-jars-MainEmptyUncompressed", @@ -466,6 +467,8 @@ art_cc_defaults { "linker/oat_writer_test.cc", "transaction_test.cc", "verifier_deps_test.cc", + "utils/atomic_dex_ref_map_test.cc", + "utils/dedupe_set_test.cc", "utils/swap_space_test.cc", ], target: { diff --git a/dex2oat/art_standalone_dex2oat_tests.xml b/dex2oat/art_standalone_dex2oat_tests.xml index d86eb154bb..d58d691556 100644 --- a/dex2oat/art_standalone_dex2oat_tests.xml +++ b/dex2oat/art_standalone_dex2oat_tests.xml @@ -33,6 +33,7 @@ <option name="push" value="art-gtest-jars-Dex2oatVdexTestDex.jar->/data/local/tmp/art_standalone_dex2oat_tests/art-gtest-jars-Dex2oatVdexTestDex.jar" /> <option name="push" value="art-gtest-jars-ImageLayoutA.jar->/data/local/tmp/art_standalone_dex2oat_tests/art-gtest-jars-ImageLayoutA.jar" /> <option name="push" value="art-gtest-jars-ImageLayoutB.jar->/data/local/tmp/art_standalone_dex2oat_tests/art-gtest-jars-ImageLayoutB.jar" /> + <option name="push" value="art-gtest-jars-Interfaces.jar->/data/local/tmp/art_standalone_dex2oat_tests/art-gtest-jars-Interfaces.jar" /> <option name="push" value="art-gtest-jars-LinkageTest.dex->/data/local/tmp/art_standalone_dex2oat_tests/art-gtest-jars-LinkageTest.dex" /> <option name="push" value="art-gtest-jars-Main.jar->/data/local/tmp/art_standalone_dex2oat_tests/art-gtest-jars-Main.jar" /> <option name="push" value="art-gtest-jars-MainEmptyUncompressed.jar->/data/local/tmp/art_standalone_dex2oat_tests/art-gtest-jars-MainEmptyUncompressed.jar" /> diff --git a/dex2oat/dex2oat.cc b/dex2oat/dex2oat.cc index 344c9406f2..7564bf0b68 100644 --- a/dex2oat/dex2oat.cc +++ b/dex2oat/dex2oat.cc @@ -878,6 +878,12 @@ class Dex2Oat final { break; } +#ifdef ART_USE_RESTRICTED_MODE + // TODO(Simulator): support signal handling and implicit checks. + compiler_options_->implicit_suspend_checks_ = false; + compiler_options_->implicit_null_checks_ = false; +#endif // ART_USE_RESTRICTED_MODE + // Done with usage checks, enable watchdog if requested if (parser_options->watch_dog_enabled) { int64_t timeout = parser_options->watch_dog_timeout_in_ms > 0 @@ -2198,11 +2204,17 @@ class Dex2Oat final { } elf_writer->WriteDynamicSection(); - elf_writer->WriteDebugInfo(oat_writer->GetDebugInfo()); + { + TimingLogger::ScopedTiming t_wdi("Write DebugInfo", timings_); + elf_writer->WriteDebugInfo(oat_writer->GetDebugInfo()); + } - if (!elf_writer->End()) { - LOG(ERROR) << "Failed to write ELF file " << oat_file->GetPath(); - return false; + { + TimingLogger::ScopedTiming t_end("Write ELF End", timings_); + if (!elf_writer->End()) { + LOG(ERROR) << "Failed to write ELF file " << oat_file->GetPath(); + return false; + } } if (!FlushOutputFile(&vdex_files_[i]) || !FlushOutputFile(&oat_files_[i])) { @@ -2211,7 +2223,10 @@ class Dex2Oat final { VLOG(compiler) << "Oat file written successfully: " << oat_filenames_[i]; - oat_writer.reset(); + { + TimingLogger::ScopedTiming t_dow("Destroy OatWriter", timings_); + oat_writer.reset(); + } // We may still need the ELF writer later for stripping. } } diff --git a/dex2oat/driver/compiler_driver-inl.h b/dex2oat/driver/compiler_driver-inl.h index a928b6dd02..e416770f91 100644 --- a/dex2oat/driver/compiler_driver-inl.h +++ b/dex2oat/driver/compiler_driver-inl.h @@ -29,6 +29,7 @@ #include "mirror/dex_cache-inl.h" #include "runtime.h" #include "scoped_thread_state_change-inl.h" +#include "utils/atomic_dex_ref_map-inl.h" namespace art { @@ -99,6 +100,11 @@ inline std::pair<bool, bool> CompilerDriver::IsFastInstanceField( return std::make_pair(fast_get, fast_put); } +inline const CompilerDriver::CompiledMethodArray* CompilerDriver::GetCompiledMethods( + const DexFile* dex_file) const { + return compiled_methods_.GetArray(dex_file); +} + } // namespace art #endif // ART_DEX2OAT_DRIVER_COMPILER_DRIVER_INL_H_ diff --git a/dex2oat/driver/compiler_driver.cc b/dex2oat/driver/compiler_driver.cc index ddf9bbda84..7f4ec13f1b 100644 --- a/dex2oat/driver/compiler_driver.cc +++ b/dex2oat/driver/compiler_driver.cc @@ -565,6 +565,7 @@ static void CompileMethodQuick( void CompilerDriver::Resolve(jobject class_loader, const std::vector<const DexFile*>& dex_files, TimingLogger* timings) { + TimingLogger::ScopedTiming t("Resolve Types", timings); // Resolution allocates classes and needs to run single-threaded to be deterministic. bool force_determinism = GetCompilerOptions().IsForceDeterminism(); ThreadPool* resolve_thread_pool = force_determinism @@ -586,6 +587,7 @@ void CompilerDriver::Resolve(jobject class_loader, void CompilerDriver::ResolveConstStrings(const std::vector<const DexFile*>& dex_files, bool only_startup_strings, TimingLogger* timings) { + TimingLogger::ScopedTiming t("Resolve const-string Strings", timings); const ProfileCompilationInfo* profile_compilation_info = GetCompilerOptions().GetProfileCompilationInfo(); if (only_startup_strings && profile_compilation_info == nullptr) { @@ -601,7 +603,6 @@ void CompilerDriver::ResolveConstStrings(const std::vector<const DexFile*>& dex_ for (const DexFile* dex_file : dex_files) { dex_cache.Assign(class_linker->FindDexCache(soa.Self(), *dex_file)); - TimingLogger::ScopedTiming t("Resolve const-string Strings", timings); ProfileCompilationInfo::ProfileIndexType profile_index = ProfileCompilationInfo::MaxProfileIndex(); @@ -1767,7 +1768,7 @@ void CompilerDriver::ResolveDexFile(jobject class_loader, size_t thread_count, TimingLogger* timings) { ScopedTrace trace(__FUNCTION__); - TimingLogger::ScopedTiming t("Resolve Types", timings); + TimingLogger::ScopedTiming t("Resolve Dex File", timings); ClassLinker* class_linker = Runtime::Current()->GetClassLinker(); // TODO: we could resolve strings here, although the string table is largely filled with class @@ -1932,18 +1933,21 @@ void CompilerDriver::Verify(jobject jclass_loader, } } - // Verification updates VerifierDeps and needs to run single-threaded to be deterministic. - bool force_determinism = GetCompilerOptions().IsForceDeterminism(); - ThreadPool* verify_thread_pool = - force_determinism ? single_thread_pool_.get() : parallel_thread_pool_.get(); - size_t verify_thread_count = force_determinism ? 1U : parallel_thread_count_; - for (const DexFile* dex_file : dex_files) { - CHECK(dex_file != nullptr); - VerifyDexFile(jclass_loader, - *dex_file, - verify_thread_pool, - verify_thread_count, - timings); + { + TimingLogger::ScopedTiming t("Verify Classes", timings); + // Verification updates VerifierDeps and needs to run single-threaded to be deterministic. + bool force_determinism = GetCompilerOptions().IsForceDeterminism(); + ThreadPool* verify_thread_pool = + force_determinism ? single_thread_pool_.get() : parallel_thread_pool_.get(); + size_t verify_thread_count = force_determinism ? 1U : parallel_thread_count_; + for (const DexFile* dex_file : dex_files) { + CHECK(dex_file != nullptr); + VerifyDexFile(jclass_loader, + *dex_file, + verify_thread_pool, + verify_thread_count, + timings); + } } if (main_verifier_deps != nullptr) { @@ -2609,7 +2613,7 @@ class InitializeClassVisitor : public CompilationVisitor { void CompilerDriver::InitializeClasses(jobject jni_class_loader, const DexFile& dex_file, TimingLogger* timings) { - TimingLogger::ScopedTiming t("InitializeNoClinit", timings); + TimingLogger::ScopedTiming t("Initialize Classes Dex File", timings); // Initialization allocates objects and needs to run single-threaded to be deterministic. bool force_determinism = GetCompilerOptions().IsForceDeterminism(); @@ -2640,6 +2644,7 @@ void CompilerDriver::InitializeClasses(jobject jni_class_loader, void CompilerDriver::InitializeClasses(jobject class_loader, const std::vector<const DexFile*>& dex_files, TimingLogger* timings) { + TimingLogger::ScopedTiming t("Initialize Classes", timings); for (const DexFile* dex_file : dex_files) { CHECK(dex_file != nullptr); InitializeClasses(class_loader, *dex_file, timings); @@ -2741,6 +2746,7 @@ static void CompileDexFile(CompilerDriver* driver, void CompilerDriver::Compile(jobject class_loader, const std::vector<const DexFile*>& dex_files, TimingLogger* timings) { + TimingLogger::ScopedTiming t("Compile Methods", timings); if (kDebugProfileGuidedCompilation) { const ProfileCompilationInfo* profile_compilation_info = GetCompilerOptions().GetProfileCompilationInfo(); diff --git a/dex2oat/driver/compiler_driver.h b/dex2oat/driver/compiler_driver.h index db03ab672e..06c81fb7b7 100644 --- a/dex2oat/driver/compiler_driver.h +++ b/dex2oat/driver/compiler_driver.h @@ -135,7 +135,11 @@ class CompilerDriver { ClassStatus GetClassStatus(const ClassReference& ref) const; bool GetCompiledClass(const ClassReference& ref, ClassStatus* status) const; + using CompiledMethodArray = dchecked_vector<Atomic<CompiledMethod*>>; + const CompiledMethodArray* GetCompiledMethods(const DexFile* dex_file) const; + CompiledMethod* GetCompiledMethod(MethodReference ref) const; + // Add a compiled method. void AddCompiledMethod(const MethodReference& method_ref, CompiledMethod* const compiled_method); CompiledMethod* RemoveCompiledMethod(const MethodReference& method_ref); diff --git a/dex2oat/linker/oat_writer.cc b/dex2oat/linker/oat_writer.cc index 7415604ab6..20787ddc8f 100644 --- a/dex2oat/linker/oat_writer.cc +++ b/dex2oat/linker/oat_writer.cc @@ -602,6 +602,11 @@ void OatWriter::PrepareLayout(MultiOatRelativePatcher* relative_patcher) { CHECK_EQ(instruction_set, oat_header_->GetInstructionSet()); { + TimingLogger::ScopedTiming split("InitBssAndRelRoData", timings_); + InitBssAndRelRoData(); + } + + { TimingLogger::ScopedTiming split("InitBssLayout", timings_); InitBssLayout(instruction_set); } @@ -732,88 +737,84 @@ static bool HasCompiledCode(const CompiledMethod* method) { return method != nullptr && !method->GetQuickCode().empty(); } -class OatWriter::InitBssLayoutMethodVisitor : public DexMethodVisitor { - public: - explicit InitBssLayoutMethodVisitor(OatWriter* writer) - : DexMethodVisitor(writer, /* offset */ 0u) {} - - bool VisitMethod([[maybe_unused]] size_t class_def_method_index, - const ClassAccessor::Method& method) override { - // Look for patches with .bss references and prepare maps with placeholders for their offsets. - CompiledMethod* compiled_method = writer_->compiler_driver_->GetCompiledMethod( - MethodReference(dex_file_, method.GetIndex())); - if (HasCompiledCode(compiled_method)) { +void OatWriter::InitBssAndRelRoData() { + for (const DexFile* dex_file : *dex_files_) { + const dchecked_vector<Atomic<CompiledMethod*>>* compiled_methods = + compiler_driver_->GetCompiledMethods(dex_file); + if (compiled_methods == nullptr) { + continue; + } + for (const Atomic<CompiledMethod*>& entry : *compiled_methods) { + CompiledMethod* compiled_method = entry.load(std::memory_order_relaxed); + if (compiled_method == nullptr) { + continue; + } + DCHECK_IMPLIES(!compiled_method->GetPatches().empty(), HasCompiledCode(compiled_method)); for (const LinkerPatch& patch : compiled_method->GetPatches()) { if (patch.GetType() == LinkerPatch::Type::kBootImageRelRo) { - writer_->boot_image_rel_ro_entries_.Overwrite(patch.BootImageOffset(), - /* placeholder */ 0u); + boot_image_rel_ro_entries_.Overwrite(patch.BootImageOffset(), /* placeholder */ 0u); } else if (patch.GetType() == LinkerPatch::Type::kMethodAppImageRelRo) { MethodReference target_method = patch.TargetMethod(); - writer_->app_image_rel_ro_method_entries_.Overwrite(target_method, /* placeholder */ 0u); + app_image_rel_ro_method_entries_.Overwrite(target_method, /* placeholder */ 0u); } else if (patch.GetType() == LinkerPatch::Type::kMethodBssEntry) { MethodReference target_method = patch.TargetMethod(); AddBssReference(target_method, target_method.dex_file->NumMethodIds(), - &writer_->bss_method_entry_references_); - writer_->bss_method_entries_.Overwrite(target_method, /* placeholder */ 0u); + &bss_method_entry_references_); + 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); + 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, target_type.dex_file->NumTypeIds(), - &writer_->bss_type_entry_references_); - writer_->bss_type_entries_.Overwrite(target_type, /* placeholder */ 0u); + &bss_type_entry_references_); + bss_type_entries_.Overwrite(target_type, /* placeholder */ 0u); } else if (patch.GetType() == LinkerPatch::Type::kPublicTypeBssEntry) { TypeReference target_type = patch.TargetType(); AddBssReference(target_type, target_type.dex_file->NumTypeIds(), - &writer_->bss_public_type_entry_references_); - writer_->bss_public_type_entries_.Overwrite(target_type, /* placeholder */ 0u); + &bss_public_type_entry_references_); + bss_public_type_entries_.Overwrite(target_type, /* placeholder */ 0u); } else if (patch.GetType() == LinkerPatch::Type::kPackageTypeBssEntry) { TypeReference target_type = patch.TargetType(); AddBssReference(target_type, target_type.dex_file->NumTypeIds(), - &writer_->bss_package_type_entry_references_); - writer_->bss_package_type_entries_.Overwrite(target_type, /* placeholder */ 0u); + &bss_package_type_entry_references_); + bss_package_type_entries_.Overwrite(target_type, /* placeholder */ 0u); } else if (patch.GetType() == LinkerPatch::Type::kStringBssEntry) { StringReference target_string = patch.TargetString(); AddBssReference(target_string, target_string.dex_file->NumStringIds(), - &writer_->bss_string_entry_references_); - writer_->bss_string_entries_.Overwrite(target_string, /* placeholder */ 0u); + &bss_string_entry_references_); + bss_string_entries_.Overwrite(target_string, /* placeholder */ 0u); } else if (patch.GetType() == LinkerPatch::Type::kMethodTypeBssEntry) { ProtoReference target_proto = patch.TargetProto(); AddBssReference(target_proto, target_proto.dex_file->NumProtoIds(), - &writer_->bss_method_type_entry_references_); - writer_->bss_method_type_entries_.Overwrite(target_proto, /* placeholder */ 0u); + &bss_method_type_entry_references_); + bss_method_type_entries_.Overwrite(target_proto, /* placeholder */ 0u); } } - } else { - DCHECK(compiled_method == nullptr || compiled_method->GetPatches().empty()); } - return true; } +} - private: - void AddBssReference(const DexFileReference& ref, - size_t number_of_indexes, - /*inout*/ SafeMap<const DexFile*, BitVector>* references) { - DCHECK(ContainsElement(*writer_->dex_files_, ref.dex_file) || - ContainsElement(Runtime::Current()->GetClassLinker()->GetBootClassPath(), ref.dex_file)); - DCHECK_LT(ref.index, number_of_indexes); - - auto refs_it = references->find(ref.dex_file); - if (refs_it == references->end()) { - refs_it = references->Put( - ref.dex_file, - BitVector(number_of_indexes, /* expandable */ false, Allocator::GetCallocAllocator())); - } - refs_it->second.SetBit(ref.index); +inline void OatWriter::AddBssReference(const DexFileReference& ref, + size_t number_of_indexes, + /*inout*/ SafeMap<const DexFile*, BitVector>* references) { + DCHECK(ContainsElement(*dex_files_, ref.dex_file) || + ContainsElement(Runtime::Current()->GetClassLinker()->GetBootClassPath(), ref.dex_file)); + DCHECK_LT(ref.index, number_of_indexes); + + auto refs_it = references->find(ref.dex_file); + if (refs_it == references->end()) { + refs_it = references->Put( + ref.dex_file, + BitVector(number_of_indexes, /* expandable */ false, Allocator::GetCallocAllocator())); } -}; + refs_it->second.SetBit(ref.index); +} class OatWriter::InitOatClassesMethodVisitor : public DexMethodVisitor { public: @@ -2432,12 +2433,6 @@ size_t OatWriter::InitDataImgRelRoLayout(size_t offset) { } void OatWriter::InitBssLayout(InstructionSet instruction_set) { - { - InitBssLayoutMethodVisitor visitor(this); - bool success = VisitDexMethods(&visitor); - DCHECK(success); - } - DCHECK_EQ(bss_size_, 0u); if (bss_method_entries_.empty() && bss_type_entries_.empty() && @@ -2494,6 +2489,7 @@ void OatWriter::InitBssLayout(InstructionSet instruction_set) { } bool OatWriter::WriteRodata(OutputStream* out) { + TimingLogger::ScopedTiming split("WriteRodata", timings_); CHECK(write_state_ == WriteState::kWriteRoData); size_t file_offset = oat_data_offset_; @@ -2585,6 +2581,7 @@ void OatWriter::WriteVerifierDeps(verifier::VerifierDeps* verifier_deps, } bool OatWriter::WriteCode(OutputStream* out) { + TimingLogger::ScopedTiming split("WriteCode", timings_); CHECK(write_state_ == WriteState::kWriteText); // Wrap out to update checksum with each write. @@ -2621,6 +2618,7 @@ bool OatWriter::WriteCode(OutputStream* out) { } bool OatWriter::WriteDataImgRelRo(OutputStream* out) { + TimingLogger::ScopedTiming split("WriteDataImgRelRo", timings_); CHECK(write_state_ == WriteState::kWriteDataImgRelRo); // Wrap out to update checksum with each write. @@ -2747,6 +2745,8 @@ bool OatWriter::CheckOatSize(OutputStream* out, size_t file_offset, size_t relat } bool OatWriter::WriteHeader(OutputStream* out) { + TimingLogger::ScopedTiming split("WriteHeader", timings_); + CHECK(write_state_ == WriteState::kWriteHeader); // Update checksum with header data. @@ -3040,7 +3040,6 @@ size_t OatWriter::WriteIndexBssMappingsHelper(OutputStream* out, size_t OatWriter::WriteIndexBssMappings(OutputStream* out, size_t file_offset, size_t relative_offset) { - TimingLogger::ScopedTiming split("WriteMethodBssMappings", timings_); if (bss_method_entry_references_.empty() && bss_type_entry_references_.empty() && bss_public_type_entry_references_.empty() && @@ -3110,8 +3109,6 @@ size_t OatWriter::WriteIndexBssMappings(OutputStream* out, } size_t OatWriter::WriteOatDexFiles(OutputStream* out, size_t file_offset, size_t relative_offset) { - TimingLogger::ScopedTiming split("WriteOatDexFiles", timings_); - for (size_t i = 0, size = oat_dex_files_.size(); i != size; ++i) { OatDexFile* oat_dex_file = &oat_dex_files_[i]; DCHECK_EQ(relative_offset, oat_dex_file->offset_); @@ -3128,8 +3125,6 @@ size_t OatWriter::WriteOatDexFiles(OutputStream* out, size_t file_offset, size_t } size_t OatWriter::WriteBcpBssInfo(OutputStream* out, size_t file_offset, size_t relative_offset) { - TimingLogger::ScopedTiming split("WriteBcpBssInfo", timings_); - const uint32_t number_of_bcp_dexfiles = bcp_bss_info_.size(); // We skip adding the number of DexFiles if we have no .bss mappings. if (number_of_bcp_dexfiles == 0) { diff --git a/dex2oat/linker/oat_writer.h b/dex2oat/linker/oat_writer.h index 5166887963..5a775fa517 100644 --- a/dex2oat/linker/oat_writer.h +++ b/dex2oat/linker/oat_writer.h @@ -261,7 +261,6 @@ class OatWriter { // to actually write it. class DexMethodVisitor; class OatDexMethodVisitor; - class InitBssLayoutMethodVisitor; class InitOatClassesMethodVisitor; class LayoutCodeMethodVisitor; class LayoutReserveOffsetCodeMethodVisitor; @@ -302,7 +301,11 @@ class OatWriter { size_t InitOatCode(size_t offset); size_t InitOatCodeDexFiles(size_t offset); size_t InitDataImgRelRoLayout(size_t offset); + void InitBssAndRelRoData(); void InitBssLayout(InstructionSet instruction_set); + void AddBssReference(const DexFileReference& ref, + size_t number_of_indexes, + /*inout*/ SafeMap<const DexFile*, BitVector>* references); size_t WriteClassOffsets(OutputStream* out, size_t file_offset, size_t relative_offset); size_t WriteClasses(OutputStream* out, size_t file_offset, size_t relative_offset); diff --git a/compiler/utils/atomic_dex_ref_map-inl.h b/dex2oat/utils/atomic_dex_ref_map-inl.h index 653d21b3ea..b36671c347 100644 --- a/compiler/utils/atomic_dex_ref_map-inl.h +++ b/dex2oat/utils/atomic_dex_ref_map-inl.h @@ -14,8 +14,8 @@ * limitations under the License. */ -#ifndef ART_COMPILER_UTILS_ATOMIC_DEX_REF_MAP_INL_H_ -#define ART_COMPILER_UTILS_ATOMIC_DEX_REF_MAP_INL_H_ +#ifndef ART_DEX2OAT_UTILS_ATOMIC_DEX_REF_MAP_INL_H_ +#define ART_DEX2OAT_UTILS_ATOMIC_DEX_REF_MAP_INL_H_ #include "atomic_dex_ref_map.h" @@ -147,4 +147,4 @@ inline std::vector<const DexFile*> AtomicDexRefMap<DexFileReferenceType, Value>: } // namespace art -#endif // ART_COMPILER_UTILS_ATOMIC_DEX_REF_MAP_INL_H_ +#endif // ART_DEX2OAT_UTILS_ATOMIC_DEX_REF_MAP_INL_H_ diff --git a/compiler/utils/atomic_dex_ref_map.h b/dex2oat/utils/atomic_dex_ref_map.h index b10fef50c5..59ecd3c504 100644 --- a/compiler/utils/atomic_dex_ref_map.h +++ b/dex2oat/utils/atomic_dex_ref_map.h @@ -14,8 +14,8 @@ * limitations under the License. */ -#ifndef ART_COMPILER_UTILS_ATOMIC_DEX_REF_MAP_H_ -#define ART_COMPILER_UTILS_ATOMIC_DEX_REF_MAP_H_ +#ifndef ART_DEX2OAT_UTILS_ATOMIC_DEX_REF_MAP_H_ +#define ART_DEX2OAT_UTILS_ATOMIC_DEX_REF_MAP_H_ #include "base/atomic.h" #include "base/dchecked_vector.h" @@ -31,6 +31,9 @@ class DexFile; template <typename DexFileReferenceType, typename Value> class AtomicDexRefMap { public: + // Verified methods. The method array is fixed to avoid needing a lock to extend it. + using ElementArray = dchecked_vector<Atomic<Value>>; + AtomicDexRefMap() {} ~AtomicDexRefMap() {} @@ -68,14 +71,12 @@ class AtomicDexRefMap { void ClearEntries(); - private: - // Verified methods. The method array is fixed to avoid needing a lock to extend it. - using ElementArray = dchecked_vector<Atomic<Value>>; - using DexFileArrays = SafeMap<const DexFile*, ElementArray>; - const ElementArray* GetArray(const DexFile* dex_file) const; ElementArray* GetArray(const DexFile* dex_file); + private: + using DexFileArrays = SafeMap<const DexFile*, ElementArray>; + static size_t NumberOfDexIndices(const DexFile* dex_file); DexFileArrays arrays_; @@ -83,4 +84,4 @@ class AtomicDexRefMap { } // namespace art -#endif // ART_COMPILER_UTILS_ATOMIC_DEX_REF_MAP_H_ +#endif // ART_DEX2OAT_UTILS_ATOMIC_DEX_REF_MAP_H_ diff --git a/compiler/utils/atomic_dex_ref_map_test.cc b/dex2oat/utils/atomic_dex_ref_map_test.cc index 329735b796..329735b796 100644 --- a/compiler/utils/atomic_dex_ref_map_test.cc +++ b/dex2oat/utils/atomic_dex_ref_map_test.cc diff --git a/compiler/utils/dedupe_set-inl.h b/dex2oat/utils/dedupe_set-inl.h index db744c53f7..5de82c534b 100644 --- a/compiler/utils/dedupe_set-inl.h +++ b/dex2oat/utils/dedupe_set-inl.h @@ -14,8 +14,8 @@ * limitations under the License. */ -#ifndef ART_COMPILER_UTILS_DEDUPE_SET_INL_H_ -#define ART_COMPILER_UTILS_DEDUPE_SET_INL_H_ +#ifndef ART_DEX2OAT_UTILS_DEDUPE_SET_INL_H_ +#define ART_DEX2OAT_UTILS_DEDUPE_SET_INL_H_ #include "dedupe_set.h" @@ -272,4 +272,4 @@ std::string DedupeSet<InKey, StoreKey, Alloc, HashType, HashFunc, kShard>::DumpS } // namespace art -#endif // ART_COMPILER_UTILS_DEDUPE_SET_INL_H_ +#endif // ART_DEX2OAT_UTILS_DEDUPE_SET_INL_H_ diff --git a/compiler/utils/dedupe_set.h b/dex2oat/utils/dedupe_set.h index 42db8e3ca0..2b8d713cb8 100644 --- a/compiler/utils/dedupe_set.h +++ b/dex2oat/utils/dedupe_set.h @@ -14,8 +14,8 @@ * limitations under the License. */ -#ifndef ART_COMPILER_UTILS_DEDUPE_SET_H_ -#define ART_COMPILER_UTILS_DEDUPE_SET_H_ +#ifndef ART_DEX2OAT_UTILS_DEDUPE_SET_H_ +#define ART_DEX2OAT_UTILS_DEDUPE_SET_H_ #include <stdint.h> #include <memory> @@ -61,4 +61,4 @@ class DedupeSet { } // namespace art -#endif // ART_COMPILER_UTILS_DEDUPE_SET_H_ +#endif // ART_DEX2OAT_UTILS_DEDUPE_SET_H_ diff --git a/compiler/utils/dedupe_set_test.cc b/dex2oat/utils/dedupe_set_test.cc index 89385e7c82..89385e7c82 100644 --- a/compiler/utils/dedupe_set_test.cc +++ b/dex2oat/utils/dedupe_set_test.cc diff --git a/dexdump/dexdump.cc b/dexdump/dexdump.cc index 43ed224b01..dd90d90cf2 100644 --- a/dexdump/dexdump.cc +++ b/dexdump/dexdump.cc @@ -1341,7 +1341,7 @@ static void dumpCode(const DexFile* pDexFile, u4 idx, u4 flags, static std::string GetHiddenapiFlagStr(uint32_t hiddenapi_flags) { std::stringstream ss; - hiddenapi::ApiList api_list(hiddenapi_flags); + hiddenapi::ApiList api_list = hiddenapi::ApiList::FromDexFlags(hiddenapi_flags); api_list.Dump(ss); std::string str_api_list = ss.str(); std::transform(str_api_list.begin(), str_api_list.end(), str_api_list.begin(), ::toupper); diff --git a/libartbase/Android.bp b/libartbase/Android.bp index 91c9c5b4f8..2d8194c46e 100644 --- a/libartbase/Android.bp +++ b/libartbase/Android.bp @@ -108,7 +108,7 @@ cc_defaults { ], export_shared_lib_headers: ["libbase"], // ART's macros.h depends on libbase's macros.h. }, - linux_glibc: { + host_linux: { static_libs: [ "libcap", ], @@ -180,7 +180,7 @@ cc_defaults { "libcap", ], }, - linux_glibc: { + host_linux: { whole_static_libs: [ "libcap", ], diff --git a/libartbase/base/hiddenapi_flags.h b/libartbase/base/hiddenapi_flags.h index 0d7938aca1..9ab8c759c3 100644 --- a/libartbase/base/hiddenapi_flags.h +++ b/libartbase/base/hiddenapi_flags.h @@ -17,15 +17,15 @@ #ifndef ART_LIBARTBASE_BASE_HIDDENAPI_FLAGS_H_ #define ART_LIBARTBASE_BASE_HIDDENAPI_FLAGS_H_ -#include "sdk_version.h" +#include <android-base/logging.h> #include <vector> -#include "android-base/logging.h" #include "base/bit_utils.h" #include "base/dumpable.h" -#include "base/macros.h" #include "base/hiddenapi_stubs.h" +#include "base/macros.h" +#include "sdk_version.h" namespace art { namespace hiddenapi { @@ -80,15 +80,20 @@ namespace helper { */ class ApiList { private: + // The representation in dex_flags_ is a combination of a Value in the lowest + // kValueBitSize bits, and bit flags corresponding to DomainApi in bits above + // that. + uint32_t dex_flags_; + // Number of bits reserved for Value in dex flags, and the corresponding bit mask. static constexpr uint32_t kValueBitSize = 4; static constexpr uint32_t kValueBitMask = helper::BitMask(kValueBitSize); enum class Value : uint32_t { // Values independent of target SDK version of app - kSdk = 0, - kUnsupported = 1, - kBlocked = 2, + kSdk = 0, + kUnsupported = 1, // @UnsupportedAppUsage + kBlocked = 2, // Values dependent on target SDK version of app. Put these last as // their list will be extended in future releases. @@ -100,21 +105,24 @@ class ApiList { kMaxTargetR = 6, kMaxTargetS = 7, - // Special values - kInvalid = (static_cast<uint32_t>(-1) & kValueBitMask), - kMin = kSdk, - kMax = kMaxTargetS, + // Invalid value. Does not imply the DomainApi is invalid. + kInvalid = (static_cast<uint32_t>(-1) & kValueBitMask), + + kMin = kSdk, + kMax = kMaxTargetS, + kFuture = kMax + 1, // Only for testing }; - // Additional bit flags after the first kValueBitSize bits in dex flags. - // These are used for domain-specific API. + // Additional bit flags after the first kValueBitSize bits in dex flags. These + // are used for domain-specific APIs. The app domain is the default when no + // bits are set. enum class DomainApi : uint32_t { kCorePlatformApi = kValueBitSize, kTestApi = kValueBitSize + 1, // Special values - kMin = kCorePlatformApi, - kMax = kTestApi, + kMin = kCorePlatformApi, + kMax = kTestApi, }; // Bit mask of all domain API flags. @@ -124,12 +132,22 @@ class ApiList { static_assert(kValueBitSize >= MinimumBitsToStore(helper::ToUint(Value::kMax)), "Not enough bits to store all ApiList values"); - // Checks that all Values are covered by kValueBitMask. + // Check that all Values are covered by kValueBitMask. static_assert(helper::MatchesBitMask(Value::kMin, kValueBitMask)); static_assert(helper::MatchesBitMask(Value::kMax, kValueBitMask)); + static_assert(helper::MatchesBitMask(Value::kFuture, kValueBitMask)); + static_assert(helper::MatchesBitMask(Value::kInvalid, kValueBitMask)); + + // Check that there's no offset between Values and the corresponding uint32 + // dex flags, so they can be converted between each other without any change. + static_assert(helper::ToUint(Value::kMin) == 0); - // Assert that Value::kInvalid is larger than the maximum Value. - static_assert(helper::ToUint(Value::kMax) < helper::ToUint(Value::kInvalid)); + // Check that Value::kInvalid is larger than kFuture (which is larger than kMax). + static_assert(helper::ToUint(Value::kFuture) < helper::ToUint(Value::kInvalid)); + + // Check that no DomainApi bit flag is covered by kValueBitMask. + static_assert((helper::ToBit(DomainApi::kMin) & kValueBitMask) == 0); + static_assert((helper::ToBit(DomainApi::kMax) & kValueBitMask) == 0); // Names corresponding to Values. static constexpr const char* kValueNames[] = { @@ -165,13 +183,32 @@ class ApiList { /* max-target-s */ SdkVersion::kS, }; - explicit ApiList(Value val, uint32_t domain_apis = 0u) - : dex_flags_(helper::ToUint(val) | domain_apis) { - DCHECK(GetValue() == val); - DCHECK_EQ(GetDomainApis(), domain_apis); + explicit ApiList(uint32_t dex_flags) : dex_flags_(dex_flags) { + DCHECK_EQ(dex_flags_, (dex_flags_ & kValueBitMask) | (dex_flags_ & kDomainApiBitMask)); } - explicit ApiList(DomainApi val) : ApiList(Value::kInvalid, helper::ToBit(val)) {} + static ApiList FromValue(Value val) { + ApiList api_list(helper::ToUint(val)); + DCHECK(api_list.GetValue() == val); + DCHECK_EQ(api_list.GetDomainApis(), 0u); + return api_list; + } + + // Returns an ApiList with only a DomainApi bit set - the Value is invalid. It + // can be Combine'd with another ApiList with a Value to produce a valid combination. + static ApiList FromDomainApi(DomainApi domain_api) { + ApiList api_list(helper::ToUint(Value::kInvalid) | helper::ToBit(domain_api)); + DCHECK(api_list.GetValue() == Value::kInvalid); + DCHECK_EQ(api_list.GetDomainApis(), helper::ToBit(domain_api)); + return api_list; + } + + static ApiList FromValueAndDomainApis(Value val, uint32_t domain_apis) { + ApiList api_list(helper::ToUint(val) | domain_apis); + DCHECK(api_list.GetValue() == val); + DCHECK_EQ(api_list.GetDomainApis(), domain_apis); + return api_list; + } Value GetValue() const { uint32_t value = (dex_flags_ & kValueBitMask); @@ -190,47 +227,80 @@ class ApiList { uint32_t GetDomainApis() const { return (dex_flags_ & kDomainApiBitMask); } - uint32_t dex_flags_; - - public: - ApiList() : ApiList(Value::kInvalid) {} + // In order to correctly handle flagged changes from Unsupported to the Sdk, where both will be + // set when the flag is enabled, consider Sdk to take precedence over any form of unsupported. + // Note, this is not necessary in the inverse direction, because API flagging does not currently + // support API removal. Moving from the blocklist to unsupported is also a case we don't have to + // consider. + // If this is true, the conflict resolves to Value::kSdk. + static bool IsConflictingFlagsAcceptable(Value x, Value y) { + const auto predicate_non_symmetric = [](auto l, auto r) { + if (l != Value::kSdk) { + return false; + } + switch (r) { + case Value::kSdk: + case Value::kUnsupported: + case Value::kMaxTargetO: + case Value::kMaxTargetP: + case Value::kMaxTargetQ: + case Value::kMaxTargetR: + case Value::kMaxTargetS: + return true; + default: + return false; + } + }; + return predicate_non_symmetric(x, y) || predicate_non_symmetric(y, x); + } - explicit ApiList(uint32_t dex_flags) : dex_flags_(dex_flags) { - DCHECK_EQ(dex_flags_, (dex_flags_ & kValueBitMask) | (dex_flags_ & kDomainApiBitMask)); + // Returns true if combining this ApiList with `other` will succeed. + bool CanCombineWith(const ApiList& other) const { + const Value val1 = GetValue(); + const Value val2 = other.GetValue(); + return (val1 == val2) || (val1 == Value::kInvalid) || (val2 == Value::kInvalid) || + IsConflictingFlagsAcceptable(val1, val2); } + public: // Helpers for conveniently constructing ApiList instances. - static ApiList Sdk() { return ApiList(Value::kSdk); } - static ApiList Unsupported() { return ApiList(Value::kUnsupported); } - static ApiList Blocked() { return ApiList(Value::kBlocked); } - static ApiList MaxTargetO() { return ApiList(Value::kMaxTargetO); } - static ApiList MaxTargetP() { return ApiList(Value::kMaxTargetP); } - static ApiList MaxTargetQ() { return ApiList(Value::kMaxTargetQ); } - static ApiList MaxTargetR() { return ApiList(Value::kMaxTargetR); } - static ApiList MaxTargetS() { return ApiList(Value::kMaxTargetS); } - static ApiList CorePlatformApi() { return ApiList(DomainApi::kCorePlatformApi); } - static ApiList TestApi() { return ApiList(DomainApi::kTestApi); } + static ApiList Sdk() { return FromValue(Value::kSdk); } + static ApiList Unsupported() { return FromValue(Value::kUnsupported); } + static ApiList Blocked() { return FromValue(Value::kBlocked); } + static ApiList MaxTargetO() { return FromValue(Value::kMaxTargetO); } + static ApiList MaxTargetP() { return FromValue(Value::kMaxTargetP); } + static ApiList MaxTargetQ() { return FromValue(Value::kMaxTargetQ); } + static ApiList MaxTargetR() { return FromValue(Value::kMaxTargetR); } + static ApiList MaxTargetS() { return FromValue(Value::kMaxTargetS); } + static ApiList Invalid() { return FromValue(Value::kInvalid); } + static ApiList CorePlatformApi() { return FromDomainApi(DomainApi::kCorePlatformApi); } + static ApiList TestApi() { return FromDomainApi(DomainApi::kTestApi); } uint32_t GetDexFlags() const { return dex_flags_; } - uint32_t GetIntValue() const { return helper::ToUint(GetValue()) - helper::ToUint(Value::kMin); } + uint32_t GetIntValue() const { return helper::ToUint(GetValue()); } + + static ApiList FromDexFlags(uint32_t dex_flags) { return ApiList(dex_flags); } + + static ApiList FromIntValue(uint32_t int_val) { + return FromValue(helper::GetEnumAt<Value>(int_val)); + } // Returns the ApiList with a flag of a given name, or an empty ApiList if not matched. static ApiList FromName(const std::string& str) { for (uint32_t i = 0; i < kValueCount; ++i) { if (str == kValueNames[i]) { - return ApiList(helper::GetEnumAt<Value>(i)); + return FromIntValue(i); } } for (uint32_t i = 0; i < kDomainApiCount; ++i) { if (str == kDomainApiNames[i]) { - return ApiList(helper::GetEnumAt<DomainApi>(i)); + return FromDomainApi(helper::GetEnumAt<DomainApi>(i)); } } if (str == kFutureValueName) { - static_assert(helper::ToUint(Value::kMax) + 1 < helper::ToUint(Value::kInvalid)); - return ApiList(helper::ToUint(Value::kMax) + 1); + return FromValue(Value::kFuture); } - return ApiList(); + return Invalid(); } // Parses a vector of flag names into a single ApiList value. If successful, @@ -238,7 +308,7 @@ class ApiList { static bool FromNames(std::vector<std::string>::iterator begin, std::vector<std::string>::iterator end, /* out */ ApiList* out_api_list) { - ApiList api_list; + ApiList api_list = Invalid(); for (std::vector<std::string>::iterator it = begin; it != end; it++) { ApiList current = FromName(*it); if (current.IsEmpty() || !api_list.CanCombineWith(current)) { @@ -250,7 +320,7 @@ class ApiList { } return false; } - api_list |= current; + api_list = Combine(api_list, current); } if (out_api_list != nullptr) { *out_api_list = api_list; @@ -260,71 +330,36 @@ class ApiList { bool operator==(const ApiList& other) const { return dex_flags_ == other.dex_flags_; } bool operator!=(const ApiList& other) const { return !(*this == other); } - bool operator<(const ApiList& other) const { return dex_flags_ < other.dex_flags_; } - bool operator>(const ApiList& other) const { return dex_flags_ > other.dex_flags_; } - // In order to correctly handle flagged changes from Unsupported to the Sdk, where both will be - // set when the flag is enabled, consider Sdk to take precedence over any form of unsupported. - // Note, this is not necessary in the inverse direction, because API flagging does not currently - // support API removal. Moving from the blocklist to unsupported is also a case we don't have to - // consider. - // If this is true, the conflict resolves to Value::kSdk. - static bool is_conflicting_flags_acceptable(Value x, Value y) { - const auto predicate_non_symmetric = [] (auto l, auto r) { - if (l != Value::kSdk) return false; - switch (r) { - case Value::kSdk: - case Value::kUnsupported: - case Value::kMaxTargetO: - case Value::kMaxTargetP: - case Value::kMaxTargetQ: - case Value::kMaxTargetR: - case Value::kMaxTargetS: - return true; - default: - return false; - } - }; - return predicate_non_symmetric(x, y) || predicate_non_symmetric(y, x); - } - - // Returns true if combining this ApiList with `other` will succeed. - bool CanCombineWith(const ApiList& other) const { - const Value val1 = GetValue(); - const Value val2 = other.GetValue(); - return (val1 == val2) || (val1 == Value::kInvalid) || (val2 == Value::kInvalid) || - is_conflicting_flags_acceptable(val1, val2); - } + // The order doesn't have any significance - only for ordering in containers. + bool operator<(const ApiList& other) const { return dex_flags_ < other.dex_flags_; } - // Combine two ApiList instances. - ApiList operator|(const ApiList& other) { + // Combine two ApiList instances. The returned value has the union of the API + // domains. Values are mutually exclusive, so they either have to be identical + // or one of them can be safely ignored, which includes being kInvalid. + static ApiList Combine(const ApiList& api1, const ApiList& api2) { // DomainApis are not mutually exclusive. Simply OR them. - const uint32_t domain_apis = GetDomainApis() | other.GetDomainApis(); + // TODO: This is suspect since the app domain doesn't have any bit and hence + // implicitly disappears if OR'ed with any other domain. + const uint32_t domain_apis = api1.GetDomainApis() | api2.GetDomainApis(); - // Values are mutually exclusive. Check if `this` and `other` have the same Value - // or if at most one is set. - const Value val1 = GetValue(); - const Value val2 = other.GetValue(); + const Value val1 = api1.GetValue(); + const Value val2 = api2.GetValue(); if (val1 == val2) { - return ApiList(val1, domain_apis); + return FromValueAndDomainApis(val1, domain_apis); } else if (val1 == Value::kInvalid) { - return ApiList(val2, domain_apis); + return FromValueAndDomainApis(val2, domain_apis); } else if (val2 == Value::kInvalid) { - return ApiList(val1, domain_apis); - } else if (is_conflicting_flags_acceptable(val1, val2)) { - return ApiList(Value::kSdk, domain_apis); + return FromValueAndDomainApis(val1, domain_apis); + } else if (IsConflictingFlagsAcceptable(val1, val2)) { + return FromValueAndDomainApis(Value::kSdk, domain_apis); } else { - LOG(FATAL) << "Invalid combination of values " << Dumpable(ApiList(val1)) - << " and " << Dumpable(ApiList(val2)); + LOG(FATAL) << "Invalid combination of values " << Dumpable(FromValue(val1)) << " and " + << Dumpable(FromValue(val2)); UNREACHABLE(); } } - const ApiList& operator|=(const ApiList& other) { - (*this) = (*this) | other; - return *this; - } - // Returns true if all flags set in `other` are also set in `this`. bool Contains(const ApiList& other) const { return ((other.GetValue() == Value::kInvalid) || (GetValue() == other.GetValue())) && @@ -338,13 +373,9 @@ class ApiList { bool IsEmpty() const { return (GetValue() == Value::kInvalid) && (GetDomainApis() == 0); } // Returns true if the ApiList is on blocklist. - bool IsBlocked() const { - return GetValue() == Value::kBlocked; - } + bool IsBlocked() const { return GetValue() == Value::kBlocked; } - bool IsSdkApi() const { - return GetValue() == Value::kSdk; - } + bool IsSdkApi() const { return GetValue() == Value::kSdk; } // Returns true if the ApiList is a test API. bool IsTestApi() const { diff --git a/libartservice/service/Android.bp b/libartservice/service/Android.bp index 9df3d4ef7b..e1a16197b0 100644 --- a/libartservice/service/Android.bp +++ b/libartservice/service/Android.bp @@ -102,6 +102,7 @@ java_defaults { ], static_libs: [ "android.content.pm.flags-aconfig-java-export", + "android.os.flags-aconfig-java-export", "art-statslog-art-java", "artd-aidl-java", "dexopt_chroot_setup-aidl-java", @@ -256,6 +257,7 @@ android_test { "androidx.test.ext.truth", "androidx.test.runner", "artd-aidl-java", + "flag-junit", "framework-annotations-lib", // We need ExtendedMockito to mock static methods. "mockito-target-extended-minus-junit4", diff --git a/libartservice/service/README.internal.md b/libartservice/service/README.internal.md new file mode 100644 index 0000000000..690b25355f --- /dev/null +++ b/libartservice/service/README.internal.md @@ -0,0 +1,56 @@ +# ART Service internal doc + +Warning: The contents in this doc can become stale while the code evolves. + +## Pre-reboot Dexopt + +Pre-reboot Dexopt is a successor of otapreopt, available on Android V+. + +### On Mainline update + +On Mainline update, `ArtManagerLocal.onApexStaged` is called. The method +schedules an asynchronous job and returns immediately. Later, when the device is +idle and charging, the job will be run by the job scheduler. + +### On OTA update + +On Mainline update, the shell command `pm art on-ota-staged` is called. The +behavior depends on the platform version and the configuration. + +- On Android V + + By default, Pre-reboot Dexopt runs in synchronous mode from the postinstall + script while update_engine keeps the snapshot devices mapped. The command + blocks until Pre-reboot Dexopt finishes. + +- On Android V, with asynchronous mode enabled + + The asynchronous mode can be enabled by + `dalvik.vm.pr_dexopt_async_for_ota=true`. In this case, the command schedules + an asynchronous job and returns immediately. Later, when the device is idle + and charging, the job will be run by the job scheduler. The job uses + `snapshotctl` to map snapshot devices. + + Note that this mode has a risk of racing with update_engine on snapshot + devices. Particularly, if update_engine wants to unmap snapshot devices, to + revoke an OTA update, the job may be running and preventing update_engine from + successfully doing so. + +- On Android B+ + + Pre-reboot Dexopt is always in asynchronous mode. The command schedules an + asynchronous job and returns immediately. Later, when the device is idle and + charging, the job will be run by the job scheduler. The job will call + `UpdateEngine.triggerPostinstall` to ask update_engine to map snapshot + devices, and update_engine will call this command again with '--start' through + the postinstall script, to notify the job that the snapshot devices are ready. + +### On shell command + +Pre-reboot Dexopt can be triggered by a shell command `pm art pr-dexopt-job`. +It is synchronous if called with `--run`, and is asynchronous if called with +`--schedule`. + +Regardless of being synchronous or asynchronous, it always tries to map snapshot +devices if called with `--slot`. On Android V, it does so through `snapshotctl`, +and on Android B+, it does so through `UpdateEngine.triggerPostinstall`. diff --git a/libartservice/service/jarjar-rules.txt b/libartservice/service/jarjar-rules.txt index 014ff22579..cb25669b72 100644 --- a/libartservice/service/jarjar-rules.txt +++ b/libartservice/service/jarjar-rules.txt @@ -4,5 +4,10 @@ rule android.content.pm.FakeFeatureFlagsImpl com.android.server.art.jarjar.@0 rule android.content.pm.FeatureFlags com.android.server.art.jarjar.@0 rule android.content.pm.FeatureFlagsImpl com.android.server.art.jarjar.@0 rule android.content.pm.Flags com.android.server.art.jarjar.@0 +rule android.os.CustomFeatureFlags com.android.server.art.jarjar.@0 +rule android.os.FakeFeatureFlagsImpl com.android.server.art.jarjar.@0 +rule android.os.FeatureFlags com.android.server.art.jarjar.@0 +rule android.os.FeatureFlagsImpl com.android.server.art.jarjar.@0 +rule android.os.Flags com.android.server.art.jarjar.@0 rule com.android.modules.utils.** com.android.server.art.jarjar.@0 rule com.google.protobuf.** com.android.server.art.jarjar.@0 diff --git a/libartservice/service/java/com/android/server/art/ArtManagedInstallFileHelper.java b/libartservice/service/java/com/android/server/art/ArtManagedInstallFileHelper.java index dd040165e3..53d88c2588 100644 --- a/libartservice/service/java/com/android/server/art/ArtManagedInstallFileHelper.java +++ b/libartservice/service/java/com/android/server/art/ArtManagedInstallFileHelper.java @@ -25,9 +25,12 @@ import androidx.annotation.RequiresApi; import com.android.art.flags.Flags; +import java.nio.file.Path; +import java.nio.file.Paths; import java.util.List; import java.util.Set; import java.util.stream.Collectors; +import java.util.stream.Stream; /** * Helper class for <i>ART-managed install files</i> (files installed by Package Manager @@ -41,6 +44,11 @@ import java.util.stream.Collectors; public final class ArtManagedInstallFileHelper { private static final List<String> FILE_TYPES = List.of(ArtConstants.DEX_METADATA_FILE_EXT, ArtConstants.PROFILE_FILE_EXT, ArtConstants.SECURE_DEX_METADATA_FILE_EXT); + private static final List<String> SDM_SUFFIXES = + Utils.getNativeIsas() + .stream() + .map(isa -> "." + isa + ArtConstants.SECURE_DEX_METADATA_FILE_EXT) + .toList(); private ArtManagedInstallFileHelper() {} @@ -64,9 +72,14 @@ public final class ArtManagedInstallFileHelper { @FlaggedApi(Flags.FLAG_ART_SERVICE_V3) public static @NonNull List<String> filterPathsForApk( @NonNull List<String> paths, @NonNull String apkPath) { - Set<String> candidates = FILE_TYPES.stream() - .map(ext -> Utils.replaceFileExtension(apkPath, ext)) - .collect(Collectors.toSet()); + Set<String> candidates = + FILE_TYPES.stream() + .flatMap(ext + -> ext.equals(ArtConstants.SECURE_DEX_METADATA_FILE_EXT) + ? SDM_SUFFIXES.stream().map(suffix + -> Utils.replaceFileExtension(apkPath, suffix)) + : Stream.of(Utils.replaceFileExtension(apkPath, ext))) + .collect(Collectors.toSet()); return paths.stream().filter(path -> candidates.contains(path)).toList(); } @@ -84,10 +97,23 @@ public final class ArtManagedInstallFileHelper { public static @NonNull String getTargetPathForApk( @NonNull String originalPath, @NonNull String apkPath) { for (String ext : FILE_TYPES) { - if (originalPath.endsWith(ext)) { + if (!ext.equals(ArtConstants.SECURE_DEX_METADATA_FILE_EXT) + && originalPath.endsWith(ext)) { return Utils.replaceFileExtension(apkPath, ext); } } + if (originalPath.endsWith(ArtConstants.SECURE_DEX_METADATA_FILE_EXT)) { + for (String suffix : SDM_SUFFIXES) { + if (originalPath.endsWith(suffix)) { + return Utils.replaceFileExtension(apkPath, suffix); + } + } + AsLog.w("SDM filename '" + originalPath + + "' does not contain a valid instruction set name"); + Path dirname = Paths.get(apkPath).getParent(); + Path basename = Paths.get(originalPath).getFileName(); + return (dirname != null ? dirname.resolve(basename) : basename).toString(); + } throw new IllegalArgumentException( "Illegal ART managed install file path '" + originalPath + "'"); } diff --git a/libartservice/service/java/com/android/server/art/ArtManagerLocal.java b/libartservice/service/java/com/android/server/art/ArtManagerLocal.java index e202c4fecc..d8a508eb4e 100644 --- a/libartservice/service/java/com/android/server/art/ArtManagerLocal.java +++ b/libartservice/service/java/com/android/server/art/ArtManagerLocal.java @@ -1014,7 +1014,7 @@ public final class ArtManagerLocal { public void dump(@NonNull PrintWriter pw, @NonNull PackageManagerLocal.FilteredSnapshot snapshot, boolean verifySdmSignatures) { try (var pin = mInjector.createArtdPin()) { - new DumpHelper(this).dump(pw, snapshot, verifySdmSignatures); + new DumpHelper(this, verifySdmSignatures).dump(pw, snapshot); } } @@ -1043,8 +1043,8 @@ public final class ArtManagerLocal { @NonNull PackageManagerLocal.FilteredSnapshot snapshot, @NonNull String packageName, boolean verifySdmSignatures) { try (var pin = mInjector.createArtdPin()) { - new DumpHelper(this).dumpPackage(pw, snapshot, - Utils.getPackageStateOrThrow(snapshot, packageName), verifySdmSignatures); + new DumpHelper(this, verifySdmSignatures) + .dumpPackage(pw, snapshot, Utils.getPackageStateOrThrow(snapshot, packageName)); } } diff --git a/libartservice/service/java/com/android/server/art/ArtShellCommand.java b/libartservice/service/java/com/android/server/art/ArtShellCommand.java index 5db645ad19..0bbb6213d2 100644 --- a/libartservice/service/java/com/android/server/art/ArtShellCommand.java +++ b/libartservice/service/java/com/android/server/art/ArtShellCommand.java @@ -690,6 +690,7 @@ public final class ArtShellCommand extends BasicShellCommandHandler { throw new SecurityException("Only root can call 'on-ota-staged'"); } + String mode = null; String otaSlot = null; String opt; @@ -698,23 +699,33 @@ public final class ArtShellCommand extends BasicShellCommandHandler { case "--slot": otaSlot = getNextArgRequired(); break; + case "--start": + mode = opt; + break; default: pw.println("Error: Unknown option: " + opt); return 1; } } - if (otaSlot == null) { - pw.println("Error: '--slot' must be specified"); - return 1; - } - - if (mInjector.getArtManagerLocal().getPreRebootDexoptJob().isAsyncForOta()) { - return handleSchedulePrDexoptJob(pw, otaSlot); + if ("--start".equals(mode)) { + if (otaSlot != null) { + pw.println("Error: '--slot' cannot be specified together with '--start'"); + return 1; + } + return handleOnOtaStagedStart(pw); } else { - // Don't map snapshots when running synchronously. `update_engine` maps snapshots - // for us. - return handleRunPrDexoptJob(pw, otaSlot, false /* mapSnapshotsForOta */); + if (otaSlot == null) { + pw.println("Error: '--slot' must be specified"); + return 1; + } + + if (mInjector.getArtManagerLocal().getPreRebootDexoptJob().isAsyncForOta()) { + return handleSchedulePrDexoptJob(pw, otaSlot); + } else { + // In the synchronous case, `update_engine` has already mapped snapshots for us. + return handleRunPrDexoptJob(pw, otaSlot, true /* isUpdateEngineReady */); + } } } @@ -766,7 +777,11 @@ public final class ArtShellCommand extends BasicShellCommandHandler { case "--test": return handleTestPrDexoptJob(pw); case "--run": - return handleRunPrDexoptJob(pw, otaSlot, true /* mapSnapshotsForOta */); + // Passing isUpdateEngineReady=false will make the job call update_engine's + // triggerPostinstall to map the snapshot devices if the API is available. + // It's always safe to do so because triggerPostinstall can be called at any time + // any number of times to map the snapshots if any are available. + return handleRunPrDexoptJob(pw, otaSlot, false /* isUpdateEngineReady */); case "--schedule": return handleSchedulePrDexoptJob(pw, otaSlot); case "--cancel": @@ -792,15 +807,41 @@ public final class ArtShellCommand extends BasicShellCommandHandler { @RequiresApi(Build.VERSION_CODES.VANILLA_ICE_CREAM) private int handleRunPrDexoptJob( - @NonNull PrintWriter pw, @Nullable String otaSlot, boolean mapSnapshotsForOta) { + @NonNull PrintWriter pw, @Nullable String otaSlot, boolean isUpdateEngineReady) { PreRebootDexoptJob job = mInjector.getArtManagerLocal().getPreRebootDexoptJob(); - CompletableFuture<Void> future = job.onUpdateReadyStartNow(otaSlot, mapSnapshotsForOta); + CompletableFuture<Void> future = job.onUpdateReadyStartNow(otaSlot, isUpdateEngineReady); if (future == null) { pw.println("Job disabled by system property"); return 1; } + return handlePrDexoptJobRunning(pw, future); + } + + @RequiresApi(Build.VERSION_CODES.VANILLA_ICE_CREAM) + private int handleOnOtaStagedStart(@NonNull PrintWriter pw) { + PreRebootDexoptJob job = mInjector.getArtManagerLocal().getPreRebootDexoptJob(); + + // We assume we're being invoked from within `UpdateEngine.triggerPostinstall` in + // `PreRebootDexoptJob.triggerUpdateEnginePostinstallAndWait`, so a Pre-reboot Dexopt job is + // waiting. + CompletableFuture<Void> future = job.notifyUpdateEngineReady(); + if (future == null) { + pw.println("No waiting job found"); + return 1; + } + + return handlePrDexoptJobRunning(pw, future); + } + + @RequiresApi(Build.VERSION_CODES.VANILLA_ICE_CREAM) + private int handlePrDexoptJobRunning( + @NonNull PrintWriter pw, @NonNull CompletableFuture<Void> future) { + PreRebootDexoptJob job = mInjector.getArtManagerLocal().getPreRebootDexoptJob(); + + // Read stdin and cancel on broken pipe, to detect if the caller (e.g. update_engine) has + // killed the postinstall script. // Put the read in a separate thread because there isn't an easy way in Java to wait for // both the `Future` and the read. var readThread = new Thread(() -> { @@ -1059,7 +1100,7 @@ public final class ArtShellCommand extends BasicShellCommandHandler { 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(" on-ota-staged [--slot SLOT | --start]"); pw.println(" Notify ART Service that an OTA update is staged. ART Service decides what"); pw.println(" to do with this notification:"); pw.println(" - If Pre-reboot Dexopt is disabled or unsupported, the command returns"); @@ -1072,6 +1113,9 @@ public final class ArtShellCommand extends BasicShellCommandHandler { pw.println(" then run by the job scheduler when the device is idle and charging."); pw.println(" Options:"); pw.println(" --slot SLOT The slot that contains the OTA update, '_a' or '_b'."); + pw.println(" --start Notify the asynchronous job that the snapshot devices are"); + pw.println(" ready. The command blocks until the job finishes, and returns zero no"); + pw.println(" matter it succeeds or not."); pw.println(" Note: This command is only supposed to be used by the system. To manually"); pw.println(" control the Pre-reboot Dexopt job, use 'pr-dexopt-job' instead."); pw.println(); diff --git a/libartservice/service/java/com/android/server/art/BackgroundDexoptJobService.java b/libartservice/service/java/com/android/server/art/BackgroundDexoptJobService.java index dcea7084fa..7cfb4ee1ac 100644 --- a/libartservice/service/java/com/android/server/art/BackgroundDexoptJobService.java +++ b/libartservice/service/java/com/android/server/art/BackgroundDexoptJobService.java @@ -31,6 +31,9 @@ import com.android.server.art.model.ArtServiceJobInterface; * Entry point for the callback from the job scheduler. This class is instantiated by the system * automatically. * + * This class is for all ART Service jobs, not only the background dexopt job but also the + * Pre-reboot Dexopt job. We cannot change its name because its hardcoded on the platform side. + * * @hide */ @RequiresApi(Build.VERSION_CODES.UPSIDE_DOWN_CAKE) diff --git a/libartservice/service/java/com/android/server/art/DexUseManagerLocal.java b/libartservice/service/java/com/android/server/art/DexUseManagerLocal.java index fc30e359f7..8d47cb6ba0 100644 --- a/libartservice/service/java/com/android/server/art/DexUseManagerLocal.java +++ b/libartservice/service/java/com/android/server/art/DexUseManagerLocal.java @@ -413,7 +413,7 @@ public class DexUseManagerLocal { // TODO(jiakaiz): Investigate whether it should also be considered as isolated process if // `Process.isSdkSandboxUid` returns true. - boolean isolatedProcess = Process.isIsolatedUid(mInjector.getCallingUid()); + boolean isolatedProcess = mInjector.isIsolatedUid(mInjector.getCallingUid()); long lastUsedAtMs = mInjector.getCurrentTimeMillis(); for (var entry : classLoaderContextByDexContainerFile.entrySet()) { @@ -1400,5 +1400,9 @@ public class DexUseManagerLocal { public int getCallingUid() { return Binder.getCallingUid(); } + + public boolean isIsolatedUid(int uid) { + return Process.isIsolatedUid(uid); + } } } diff --git a/libartservice/service/java/com/android/server/art/DumpHelper.java b/libartservice/service/java/com/android/server/art/DumpHelper.java index 9578652120..77cc37f4b3 100644 --- a/libartservice/service/java/com/android/server/art/DumpHelper.java +++ b/libartservice/service/java/com/android/server/art/DumpHelper.java @@ -60,24 +60,26 @@ import java.util.stream.Collectors; @RequiresApi(Build.VERSION_CODES.UPSIDE_DOWN_CAKE) public class DumpHelper { @NonNull private final Injector mInjector; + private final boolean mVerifySdmSignatures; - public DumpHelper(@NonNull ArtManagerLocal artManagerLocal) { - this(new Injector(artManagerLocal)); + public DumpHelper(@NonNull ArtManagerLocal artManagerLocal, boolean verifySdmSignatures) { + this(new Injector(artManagerLocal), verifySdmSignatures); } @VisibleForTesting - public DumpHelper(@NonNull Injector injector) { + public DumpHelper(@NonNull Injector injector, boolean verifySdmSignatures) { mInjector = injector; + mVerifySdmSignatures = verifySdmSignatures; } /** Handles {@link ArtManagerLocal#dump(PrintWriter, PackageManagerLocal.FilteredSnapshot)}. */ - public void dump(@NonNull PrintWriter pw, - @NonNull PackageManagerLocal.FilteredSnapshot snapshot, boolean verifySdmSignatures) { + public void dump( + @NonNull PrintWriter pw, @NonNull PackageManagerLocal.FilteredSnapshot snapshot) { snapshot.getPackageStates() .values() .stream() .sorted(Comparator.comparing(PackageState::getPackageName)) - .forEach(pkgState -> dumpPackage(pw, snapshot, pkgState, verifySdmSignatures)); + .forEach(pkgState -> dumpPackage(pw, snapshot, pkgState)); pw.printf("\nCurrent GC: %s\n", ArtJni.getGarbageCollector()); } @@ -86,8 +88,8 @@ public class DumpHelper { * ArtManagerLocal#dumpPackage(PrintWriter, PackageManagerLocal.FilteredSnapshot, String)}. */ public void dumpPackage(@NonNull PrintWriter pw, - @NonNull PackageManagerLocal.FilteredSnapshot snapshot, @NonNull PackageState pkgState, - boolean verifySdmSignatures) { + @NonNull PackageManagerLocal.FilteredSnapshot snapshot, + @NonNull PackageState pkgState) { if (pkgState.isApex() || pkgState.getAndroidPackage() == null) { return; } @@ -130,7 +132,7 @@ public class DumpHelper { ipw.increaseIndent(); for (List<DexContainerFileDexoptStatus> fileStatuses : primaryStatusesByDexPath.values()) { - dumpPrimaryDex(ipw, snapshot, fileStatuses, packageName, verifySdmSignatures); + dumpPrimaryDex(ipw, snapshot, fileStatuses, packageName); } if (!secondaryStatusesByDexPath.isEmpty()) { ipw.println("known secondary dex files:"); @@ -147,8 +149,7 @@ public class DumpHelper { private void dumpPrimaryDex(@NonNull IndentingPrintWriter ipw, @NonNull PackageManagerLocal.FilteredSnapshot snapshot, - List<DexContainerFileDexoptStatus> fileStatuses, @NonNull String packageName, - boolean verifySdmSignatures) { + List<DexContainerFileDexoptStatus> fileStatuses, @NonNull String packageName) { String dexPath = fileStatuses.get(0).getDexContainerFile(); ipw.printf("path: %s\n", dexPath); ipw.increaseIndent(); @@ -156,7 +157,6 @@ public class DumpHelper { dumpUsedByOtherApps(ipw, snapshot, mInjector.getDexUseManager().getPrimaryDexLoaders(packageName, dexPath), packageName); - dumpSdmStatus(ipw, dexPath, verifySdmSignatures); ipw.decreaseIndent(); } @@ -199,12 +199,15 @@ public class DumpHelper { private void dumpFileStatuses( @NonNull IndentingPrintWriter ipw, List<DexContainerFileDexoptStatus> fileStatuses) { for (DexContainerFileDexoptStatus fileStatus : fileStatuses) { - ipw.printf("%s: [status=%s] [reason=%s]%s\n", - VMRuntime.getInstructionSet(fileStatus.getAbi()), - fileStatus.getCompilerFilter(), fileStatus.getCompilationReason(), + String isa = VMRuntime.getInstructionSet(fileStatus.getAbi()); + ipw.printf("%s: [status=%s] [reason=%s]%s\n", isa, fileStatus.getCompilerFilter(), + fileStatus.getCompilationReason(), fileStatus.isPrimaryAbi() ? " [primary-abi]" : ""); ipw.increaseIndent(); ipw.printf("[location is %s]\n", fileStatus.getLocationDebugString()); + if (fileStatus.isPrimaryDex()) { + dumpSdmStatus(ipw, fileStatus.getDexContainerFile(), isa); + } ipw.decreaseIndent(); } } @@ -225,13 +228,13 @@ public class DumpHelper { } } - private void dumpSdmStatus(@NonNull IndentingPrintWriter ipw, @NonNull String dexPath, - boolean verifySdmSignatures) { + private void dumpSdmStatus( + @NonNull IndentingPrintWriter ipw, @NonNull String dexPath, @NonNull String isa) { if (!android.content.pm.Flags.cloudCompilationPm()) { return; } - String sdmPath = getSdmPath(dexPath); + String sdmPath = getSdmPath(dexPath, isa); String status = ""; String signature = "skipped"; if (mInjector.fileExists(sdmPath)) { @@ -239,7 +242,7 @@ public class DumpHelper { // because SDM files are not supported yet. status = "pending"; // This operation is expensive, so hide it behind a flag. - if (verifySdmSignatures) { + if (mVerifySdmSignatures) { signature = getSdmSignatureStatus(dexPath, sdmPath); } } @@ -283,8 +286,9 @@ public class DumpHelper { } @NonNull - private static String getSdmPath(@NonNull String dexPath) { - return Utils.replaceFileExtension(dexPath, ArtConstants.SECURE_DEX_METADATA_FILE_EXT); + private static String getSdmPath(@NonNull String dexPath, @NonNull String isa) { + return Utils.replaceFileExtension( + dexPath, "." + isa + ArtConstants.SECURE_DEX_METADATA_FILE_EXT); } @NonNull diff --git a/libartservice/service/java/com/android/server/art/PreRebootDexoptJob.java b/libartservice/service/java/com/android/server/art/PreRebootDexoptJob.java index 51a01b363d..093ea40ec7 100644 --- a/libartservice/service/java/com/android/server/art/PreRebootDexoptJob.java +++ b/libartservice/service/java/com/android/server/art/PreRebootDexoptJob.java @@ -22,6 +22,7 @@ import static com.android.server.art.proto.PreRebootStats.Status; import android.annotation.NonNull; import android.annotation.Nullable; +import android.annotation.SuppressLint; import android.app.job.JobInfo; import android.app.job.JobParameters; import android.app.job.JobScheduler; @@ -34,6 +35,7 @@ import android.os.PersistableBundle; import android.os.RemoteException; import android.os.ServiceSpecificException; import android.os.SystemProperties; +import android.os.UpdateEngine; import android.provider.DeviceConfig; import androidx.annotation.RequiresApi; @@ -72,12 +74,20 @@ public class PreRebootDexoptJob implements ArtServiceJobInterface { /** An arbitrary number. Must be unique among all jobs owned by the system uid. */ public static final int JOB_ID = 27873781; + private static final long UPDATE_ENGINE_TIMEOUT_MS = 10000; + @NonNull private final Injector mInjector; - // Job state variables. The monitor of `this` is notified when `mRunningJob` is changed. + // Job state variables. + // The monitor of `this` is notified when `mRunningJob` or `mIsUpdateEngineReady` is changed. + // Also, an optimization to make `triggerUpdateEnginePostinstallAndWait` return early, if + // `mCancellationSignal` is fired **before `triggerUpdateEnginePostinstallAndWait` returns**, it + // should be guaranteed that the monitor of `this` is notified when it happens. // `mRunningJob` and `mCancellationSignal` have the same nullness. @GuardedBy("this") @Nullable private CompletableFuture<Void> mRunningJob = null; @GuardedBy("this") @Nullable private CancellationSignal mCancellationSignal = null; + /** Whether update_engine has mapped snapshot devices. Only applicable to an OTA update. */ + @GuardedBy("this") private boolean mIsUpdateEngineReady = false; /** Whether `mRunningJob` is running from the job scheduler's perspective. */ @GuardedBy("this") private boolean mIsRunningJobKnownByJobScheduler = false; @@ -85,7 +95,10 @@ public class PreRebootDexoptJob implements ArtServiceJobInterface { /** The slot that contains the OTA update, "_a" or "_b", or null for a Mainline update. */ @GuardedBy("this") @Nullable private String mOtaSlot = null; - /** Whether to map/unmap snapshots. Only applicable to an OTA update. */ + /** + * Whether to map/unmap snapshots ourselves rather than using update_engine. Only applicable to + * an OTA update. For legacy use only. + */ @GuardedBy("this") private boolean mMapSnapshotsForOta = false; /** @@ -154,12 +167,19 @@ public class PreRebootDexoptJob implements ArtServiceJobInterface { Runnable onJobFinishedLocked = () -> { Utils.check(mIsRunningJobKnownByJobScheduler); mIsRunningJobKnownByJobScheduler = false; - // 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. + // There can be four cases when we reach here: + // 1. The job has completed: No need to reschedule. + // 2. The job failed: It means something went wrong, so we don't reschedule the job + // because it will likely fail again. + // 3. The job was killed by update_engine, probably because the OTA was revoked: We + // should definitely give up. + // 4. The job was cancelled by the job scheduler: The job will be rescheduled regardless + // of the arguments we pass here because the return value of `onStopJob` will be + // respected, and this call will be ignored. + // Therefore, we can always pass `false` to the `wantsReschedule` parameter. jobService.jobFinished(params, false /* wantsReschedule */); }; - startLocked(onJobFinishedLocked).exceptionally(t -> { + startLocked(onJobFinishedLocked, false /* isUpdateEngineReady */).exceptionally(t -> { AsLog.e("Fatal error", t); return null; }); @@ -196,7 +216,10 @@ public class PreRebootDexoptJob implements ArtServiceJobInterface { cancelAnyLocked(); resetLocked(); updateOtaSlotLocked(otaSlot); - mMapSnapshotsForOta = true; + // If we can't call update_engine to map snapshot devices, then we have to map snapshot + // devices ourselves. This only happens on a few OEM devices that have + // "dalvik.vm.pr_dexopt_async_for_ota=true" and only on Android V. + mMapSnapshotsForOta = !android.os.Flags.updateEngineApi(); return scheduleLocked(); } @@ -204,23 +227,28 @@ public class PreRebootDexoptJob implements ArtServiceJobInterface { * Same as {@link #onUpdateReady}, but starts the job immediately, instead of going through the * job scheduler. * - * @param mapSnapshotsForOta whether to map/unmap snapshots. Only applicable to an OTA update. + * @param isUpdateEngineReady whether update_engine has mapped snapshot devices. Only applicable + * to an OTA update. * @return The future of the job, or null if Pre-reboot Dexopt is not enabled. */ @Nullable public synchronized CompletableFuture<Void> onUpdateReadyStartNow( - @Nullable String otaSlot, boolean mapSnapshotsForOta) { + @Nullable String otaSlot, boolean isUpdateEngineReady) { cancelAnyLocked(); resetLocked(); updateOtaSlotLocked(otaSlot); - mMapSnapshotsForOta = mapSnapshotsForOta; + // If update_engine hasn't mapped snapshot devices and we can't call update_engine to map + // snapshot devices, then we have to map snapshot devices ourselves. This only happens on + // the `pm art pr-dexopt-job --run` command for local development purposes and only on + // Android V. + mMapSnapshotsForOta = !isUpdateEngineReady && !android.os.Flags.updateEngineApi(); if (!isEnabled()) { mInjector.getStatsReporter().recordJobNotScheduled( Status.STATUS_NOT_SCHEDULED_DISABLED, isOtaUpdate()); return null; } mInjector.getStatsReporter().recordJobScheduled(false /* isAsync */, isOtaUpdate()); - return startLocked(null /* onJobFinishedLocked */); + return startLocked(null /* onJobFinishedLocked */, isUpdateEngineReady); } public synchronized void test() { @@ -351,20 +379,38 @@ public class PreRebootDexoptJob implements ArtServiceJobInterface { */ @GuardedBy("this") @NonNull - private CompletableFuture<Void> startLocked(@Nullable Runnable onJobFinishedLocked) { + private CompletableFuture<Void> startLocked( + @Nullable Runnable onJobFinishedLocked, boolean isUpdateEngineReady) { Utils.check(mRunningJob == null); String otaSlot = mOtaSlot; boolean mapSnapshotsForOta = mMapSnapshotsForOta; var cancellationSignal = mCancellationSignal = new CancellationSignal(); + mIsUpdateEngineReady = isUpdateEngineReady; mRunningJob = new CompletableFuture().runAsync(() -> { markHasStarted(true); PreRebootStatsReporter statsReporter = mInjector.getStatsReporter(); try { statsReporter.recordJobStarted(); + if (otaSlot != null && !isUpdateEngineReady && !mapSnapshotsForOta) { + triggerUpdateEnginePostinstallAndWait(); + synchronized (this) { + // This check is not strictly necessary, but is an optimization to return + // early. + if (mCancellationSignal.isCanceled()) { + // The stats reporter translates success=true to STATUS_CANCELLED. + statsReporter.recordJobEnded(new PreRebootResult(true /* success */)); + return; + } + Utils.check(mIsUpdateEngineReady); + } + } PreRebootResult result = mInjector.getPreRebootDriver().run( otaSlot, mapSnapshotsForOta, cancellationSignal); statsReporter.recordJobEnded(result); + } catch (UpdateEngineException e) { + AsLog.e("update_engine error", e); + statsReporter.recordJobEnded(new PreRebootResult(false /* success */)); } catch (RuntimeException e) { statsReporter.recordJobEnded(new PreRebootResult(false /* success */)); throw e; @@ -379,6 +425,7 @@ public class PreRebootDexoptJob implements ArtServiceJobInterface { } mRunningJob = null; mCancellationSignal = null; + mIsUpdateEngineReady = false; this.notifyAll(); } } @@ -387,6 +434,60 @@ public class PreRebootDexoptJob implements ArtServiceJobInterface { return mRunningJob; } + // The new API usage is safe because it's guarded by a flag. The "NewApi" lint is wrong because + // it's meaningless (b/380891026). We can't change the flag check to `isAtLeastB` because we use + // `SetFlagsRule` in tests to test the behavior with and without the API support. + @SuppressLint("NewApi") + private void triggerUpdateEnginePostinstallAndWait() throws UpdateEngineException { + if (!android.os.Flags.updateEngineApi()) { + // Should never happen. + throw new UnsupportedOperationException(); + } + // When we need snapshot devices, we trigger update_engine postinstall. update_engine will + // map the snapshot devices for us and run the postinstall script, which will call + // `pm art on-ota-staged --start` to notify us that the snapshot device are ready. + // See art/libartservice/service/README.internal.md for typical flows. + AsLog.i("Waiting for update_engine to map snapshots..."); + try { + mInjector.getUpdateEngine().triggerPostinstall("system" /* partition */); + } catch (ServiceSpecificException e) { + throw new UpdateEngineException("Failed to trigger postinstall: " + e.getMessage()); + } + long startTime = System.currentTimeMillis(); + synchronized (this) { + while (true) { + if (mIsUpdateEngineReady || mCancellationSignal.isCanceled()) { + return; + } + long remainingTime = + UPDATE_ENGINE_TIMEOUT_MS - (System.currentTimeMillis() - startTime); + if (remainingTime <= 0) { + throw new UpdateEngineException("Timed out while waiting for update_engine"); + } + try { + this.wait(remainingTime); + } catch (InterruptedException e) { + AsLog.wtf("Interrupted", e); + } + } + } + } + + @Nullable + public CompletableFuture<Void> notifyUpdateEngineReady() { + synchronized (this) { + if (mRunningJob == null) { + AsLog.e("No waiting job found"); + return null; + } + AsLog.i("update_engine finished mapping snapshots"); + mIsUpdateEngineReady = true; + // Notify triggerUpdateEnginePostinstallAndWait to stop waiting. + this.notifyAll(); + return mRunningJob; + } + } + /** * Cancels the given job and waits for it to exit, if it's running. Temporarily releases the * lock when waiting for the job to exit. @@ -402,6 +503,9 @@ public class PreRebootDexoptJob implements ArtServiceJobInterface { while (mRunningJob == job) { if (!mCancellationSignal.isCanceled()) { mCancellationSignal.cancel(); + // This is not strictly necessary, but is an optimization to make + // `triggerUpdateEnginePostinstallAndWait` return early. + this.notifyAll(); AsLog.i("Job cancelled"); } try { @@ -428,6 +532,9 @@ public class PreRebootDexoptJob implements ArtServiceJobInterface { while (mRunningJob != null) { if (!mCancellationSignal.isCanceled()) { mCancellationSignal.cancel(); + // This is not strictly necessary, but is an optimization to make + // `triggerUpdateEnginePostinstallAndWait` return early. + this.notifyAll(); AsLog.i("Job cancelled"); } try { @@ -478,6 +585,10 @@ public class PreRebootDexoptJob implements ArtServiceJobInterface { } public boolean isAsyncForOta() { + if (android.os.Flags.updateEngineApi()) { + return true; + } + // Legacy flag in Android V. return SystemProperties.getBoolean("dalvik.vm.pr_dexopt_async_for_ota", false /* def */); } @@ -513,6 +624,12 @@ public class PreRebootDexoptJob implements ArtServiceJobInterface { return mOtaSlot != null; } + private static class UpdateEngineException extends Exception { + public UpdateEngineException(@NonNull String message) { + super(message); + } + } + /** * Injector pattern for testing purpose. * @@ -555,5 +672,10 @@ public class PreRebootDexoptJob implements ArtServiceJobInterface { @NonNull String namespace, @NonNull String name, boolean defaultValue) { return DeviceConfig.getBoolean(namespace, name, defaultValue); } + + @NonNull + public UpdateEngine getUpdateEngine() { + return new UpdateEngine(); + } } } diff --git a/libartservice/service/java/com/android/server/art/Utils.java b/libartservice/service/java/com/android/server/art/Utils.java index 07168aa07e..3dcbc54141 100644 --- a/libartservice/service/java/com/android/server/art/Utils.java +++ b/libartservice/service/java/com/android/server/art/Utils.java @@ -67,6 +67,7 @@ import java.util.Collection; import java.util.Collections; import java.util.Comparator; import java.util.List; +import java.util.Objects; import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; @@ -204,6 +205,14 @@ public final class Utils { || abiName.equals(Constants.getNative32BitAbi()); } + public static List<String> getNativeIsas() { + return Arrays.asList(Constants.getNative64BitAbi(), Constants.getNative32BitAbi()) + .stream() + .filter(Objects::nonNull) + .map(VMRuntime::getInstructionSet) + .toList(); + } + /** * Returns whether the artifacts of the primary dex files should be in the global dalvik-cache * directory. diff --git a/libartservice/service/javatests/com/android/server/art/ArtManagedInstallFileHelperTest.java b/libartservice/service/javatests/com/android/server/art/ArtManagedInstallFileHelperTest.java index 4b13716eaa..b0b52fae9e 100644 --- a/libartservice/service/javatests/com/android/server/art/ArtManagedInstallFileHelperTest.java +++ b/libartservice/service/javatests/com/android/server/art/ArtManagedInstallFileHelperTest.java @@ -19,10 +19,15 @@ package com.android.server.art; import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertThrows; +import static org.mockito.Mockito.lenient; import androidx.test.filters.SmallTest; import androidx.test.runner.AndroidJUnit4; +import com.android.server.art.testing.StaticMockitoRule; + +import org.junit.Before; +import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; @@ -31,33 +36,45 @@ import java.util.List; @SmallTest @RunWith(AndroidJUnit4.class) public class ArtManagedInstallFileHelperTest { + @Rule public StaticMockitoRule mockitoRule = new StaticMockitoRule(Constants.class); + + @Before + public void setUp() throws Exception { + lenient().when(Constants.getNative64BitAbi()).thenReturn("arm64-v8a"); + lenient().when(Constants.getNative32BitAbi()).thenReturn("armeabi-v7a"); + } + @Test public void testIsArtManaged() throws Exception { assertThat(ArtManagedInstallFileHelper.isArtManaged("/foo/bar.dm")).isTrue(); assertThat(ArtManagedInstallFileHelper.isArtManaged("/foo/bar.prof")).isTrue(); assertThat(ArtManagedInstallFileHelper.isArtManaged("/foo/bar.sdm")).isTrue(); + assertThat(ArtManagedInstallFileHelper.isArtManaged("/foo/bar.arm.sdm")).isTrue(); + assertThat(ArtManagedInstallFileHelper.isArtManaged("/foo/bar.arm64.sdm")).isTrue(); assertThat(ArtManagedInstallFileHelper.isArtManaged("/foo/bar.abc")).isFalse(); } @Test public void testFilterPathsForApk() throws Exception { assertThat(ArtManagedInstallFileHelper.filterPathsForApk( - List.of("/foo/bar.dm", "/foo/bar.prof", "/foo/bar.sdm", "/foo/bar.abc", - "/foo/baz.dm"), + List.of("/foo/bar.dm", "/foo/bar.prof", "/foo/bar.sdm", + "/foo/bar.x86_64.sdm", "/foo/bar.arm.sdm", "/foo/bar.arm64.sdm", + "/foo/bar.abc", "/foo/baz.dm"), "/foo/bar.apk")) - .containsExactly("/foo/bar.dm", "/foo/bar.prof", "/foo/bar.sdm"); + .containsExactly( + "/foo/bar.dm", "/foo/bar.prof", "/foo/bar.arm.sdm", "/foo/bar.arm64.sdm"); // Filenames don't match. assertThat(ArtManagedInstallFileHelper.filterPathsForApk( - List.of("/foo/bar.dm", "/foo/bar.prof", "/foo/bar.sdm", "/foo/bar.abc", - "/foo/baz.dm"), + List.of("/foo/bar.dm", "/foo/bar.prof", "/foo/bar.arm64.sdm", + "/foo/bar.abc", "/foo/baz.dm"), "/foo/qux.apk")) .isEmpty(); // Directories don't match. assertThat(ArtManagedInstallFileHelper.filterPathsForApk( - List.of("/foo/bar.dm", "/foo/bar.prof", "/foo/bar.sdm", "/foo/bar.abc", - "/foo/baz.dm"), + List.of("/foo/bar.dm", "/foo/bar.prof", "/foo/bar.arm64.sdm", + "/foo/bar.abc", "/foo/baz.dm"), "/quz/bar.apk")) .isEmpty(); } @@ -71,8 +88,22 @@ public class ArtManagedInstallFileHelperTest { "/foo/bar.prof", "/somewhere/base.apk")) .isEqualTo("/somewhere/base.prof"); assertThat(ArtManagedInstallFileHelper.getTargetPathForApk( + "/foo/bar.arm.sdm", "/somewhere/base.apk")) + .isEqualTo("/somewhere/base.arm.sdm"); + assertThat(ArtManagedInstallFileHelper.getTargetPathForApk( + "/foo/bar.arm64.sdm", "/somewhere/base.apk")) + .isEqualTo("/somewhere/base.arm64.sdm"); + + // None or invalid ISA. + assertThat(ArtManagedInstallFileHelper.getTargetPathForApk( "/foo/bar.sdm", "/somewhere/base.apk")) - .isEqualTo("/somewhere/base.sdm"); + .isEqualTo("/somewhere/bar.sdm"); + assertThat(ArtManagedInstallFileHelper.getTargetPathForApk( + "/foo/bar.x86_64.sdm", "/somewhere/base.apk")) + .isEqualTo("/somewhere/bar.x86_64.sdm"); + assertThat(ArtManagedInstallFileHelper.getTargetPathForApk( + "/foo/bar.invalid-isa.sdm", "/somewhere/base.apk")) + .isEqualTo("/somewhere/bar.invalid-isa.sdm"); assertThrows(IllegalArgumentException.class, () -> { ArtManagedInstallFileHelper.getTargetPathForApk("/foo/bar.abc", "/somewhere/base.apk"); diff --git a/libartservice/service/javatests/com/android/server/art/ArtShellCommandTest.java b/libartservice/service/javatests/com/android/server/art/ArtShellCommandTest.java index 7b934bd7e4..cb8d1d57cb 100644 --- a/libartservice/service/javatests/com/android/server/art/ArtShellCommandTest.java +++ b/libartservice/service/javatests/com/android/server/art/ArtShellCommandTest.java @@ -28,6 +28,8 @@ import static org.mockito.Mockito.eq; import static org.mockito.Mockito.isNull; import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import android.app.job.JobInfo; @@ -36,6 +38,10 @@ import android.app.job.JobScheduler; import android.os.CancellationSignal; import android.os.Process; import android.os.SystemProperties; +import android.os.UpdateEngine; +import android.platform.test.annotations.DisableFlags; +import android.platform.test.annotations.EnableFlags; +import android.platform.test.flag.junit.SetFlagsRule; import androidx.test.filters.SmallTest; @@ -65,11 +71,13 @@ public class ArtShellCommandTest { @Rule public StaticMockitoRule mockitoRule = new StaticMockitoRule( SystemProperties.class, BackgroundDexoptJobService.class, ArtJni.class); + @Rule public final SetFlagsRule mSetFlagsRule = new SetFlagsRule(); @Mock private BackgroundDexoptJobService mJobService; @Mock private PreRebootDriver mPreRebootDriver; @Mock private PreRebootStatsReporter mPreRebootStatsReporter; @Mock private JobScheduler mJobScheduler; + @Mock private UpdateEngine mUpdateEngine; @Mock private PreRebootDexoptJob.Injector mPreRebootDexoptJobInjector; @Mock private ArtManagerLocal.Injector mArtManagerLocalInjector; @Mock private PackageManagerLocal mPackageManagerLocal; @@ -114,6 +122,7 @@ public class ArtShellCommandTest { .when(mPreRebootDexoptJobInjector.getStatsReporter()) .thenReturn(mPreRebootStatsReporter); lenient().when(mPreRebootDexoptJobInjector.getJobScheduler()).thenReturn(mJobScheduler); + lenient().when(mPreRebootDexoptJobInjector.getUpdateEngine()).thenReturn(mUpdateEngine); mPreRebootDexoptJob = new PreRebootDexoptJob(mPreRebootDexoptJobInjector); lenient().when(BackgroundDexoptJobService.getJob(JOB_ID)).thenReturn(mPreRebootDexoptJob); @@ -141,6 +150,7 @@ public class ArtShellCommandTest { } @Test + @DisableFlags({android.os.Flags.FLAG_UPDATE_ENGINE_API}) public void testOnOtaStagedSync() throws Exception { when(mInjector.getCallingUid()).thenReturn(Process.ROOT_UID); @@ -157,6 +167,7 @@ public class ArtShellCommandTest { } @Test + @DisableFlags({android.os.Flags.FLAG_UPDATE_ENGINE_API}) public void testOnOtaStagedSyncFatalError() throws Exception { when(mInjector.getCallingUid()).thenReturn(Process.ROOT_UID); @@ -173,6 +184,7 @@ public class ArtShellCommandTest { } @Test + @DisableFlags({android.os.Flags.FLAG_UPDATE_ENGINE_API}) public void testOnOtaStagedSyncCancelledByCommand() throws Exception { when(mInjector.getCallingUid()).thenReturn(Process.ROOT_UID); @@ -205,6 +217,7 @@ public class ArtShellCommandTest { } @Test + @DisableFlags({android.os.Flags.FLAG_UPDATE_ENGINE_API}) public void testOnOtaStagedSyncCancelledByBrokenPipe() throws Exception { when(mInjector.getCallingUid()).thenReturn(Process.ROOT_UID); @@ -231,6 +244,188 @@ public class ArtShellCommandTest { } @Test + @EnableFlags({android.os.Flags.FLAG_UPDATE_ENGINE_API}) + public void testOnOtaStagedAsync() throws Exception { + when(mInjector.getCallingUid()).thenReturn(Process.ROOT_UID); + + try (var execution = new CommandExecution( + createHandler(), "art", "on-ota-staged", "--slot", "_b")) { + int exitCode = execution.waitAndGetExitCode(); + String outputs = getOutputs(execution); + assertWithMessage(outputs).that(exitCode).isEqualTo(0); + assertThat(outputs).contains("Pre-reboot Dexopt job scheduled"); + } + + when(mPreRebootDriver.run(eq("_b"), eq(false) /* mapSnapshotsForOta */, any())) + .thenReturn(new PreRebootResult(true /* success */)); + + mPreRebootDexoptJob.onStartJobImpl(mJobService, mJobParameters); + + try (var execution = + new CommandExecution(createHandler(), "art", "on-ota-staged", "--start")) { + int exitCode = execution.waitAndGetExitCode(); + String outputs = getOutputs(execution); + assertWithMessage(outputs).that(exitCode).isEqualTo(0); + assertThat(outputs).contains("Job finished. See logs for details"); + } + } + + @Test + @EnableFlags({android.os.Flags.FLAG_UPDATE_ENGINE_API}) + public void testOnOtaStagedAsyncFatalError() throws Exception { + when(mInjector.getCallingUid()).thenReturn(Process.ROOT_UID); + + try (var execution = new CommandExecution( + createHandler(), "art", "on-ota-staged", "--slot", "_b")) { + int exitCode = execution.waitAndGetExitCode(); + String outputs = getOutputs(execution); + assertWithMessage(outputs).that(exitCode).isEqualTo(0); + assertThat(outputs).contains("Pre-reboot Dexopt job scheduled"); + } + + when(mPreRebootDriver.run(eq("_b"), eq(false) /* mapSnapshotsForOta */, any())) + .thenThrow(RuntimeException.class); + + mPreRebootDexoptJob.onStartJobImpl(mJobService, mJobParameters); + + try (var execution = + new CommandExecution(createHandler(), "art", "on-ota-staged", "--start")) { + int exitCode = execution.waitAndGetExitCode(); + String outputs = getOutputs(execution); + assertWithMessage(outputs).that(exitCode).isEqualTo(0); + assertThat(outputs).contains("Job encountered a fatal error"); + } + } + + @Test + @EnableFlags({android.os.Flags.FLAG_UPDATE_ENGINE_API}) + public void testOnOtaStagedAsyncCancelledByCommand() throws Exception { + when(mInjector.getCallingUid()).thenReturn(Process.ROOT_UID); + + try (var execution = new CommandExecution( + createHandler(), "art", "on-ota-staged", "--slot", "_b")) { + int exitCode = execution.waitAndGetExitCode(); + String outputs = getOutputs(execution); + assertWithMessage(outputs).that(exitCode).isEqualTo(0); + assertThat(outputs).contains("Pre-reboot Dexopt job scheduled"); + } + + Semaphore dexoptStarted = new Semaphore(0); + + when(mPreRebootDriver.run(eq("_b"), eq(false) /* mapSnapshotsForOta */, any())) + .thenAnswer(invocation -> { + // Step 2. + dexoptStarted.release(); + + Semaphore dexoptCancelled = new Semaphore(0); + var cancellationSignal = invocation.<CancellationSignal>getArgument(2); + cancellationSignal.setOnCancelListener(() -> dexoptCancelled.release()); + assertThat(dexoptCancelled.tryAcquire(TIMEOUT_SEC, TimeUnit.SECONDS)).isTrue(); + + // Step 4. + return new PreRebootResult(true /* success */); + }); + + mPreRebootDexoptJob.onStartJobImpl(mJobService, mJobParameters); + + // Step 1. + try (var execution = + new CommandExecution(createHandler(), "art", "on-ota-staged", "--start")) { + assertThat(execution.getStdout().readLine()).contains("Job running..."); + + assertThat(dexoptStarted.tryAcquire(TIMEOUT_SEC, TimeUnit.SECONDS)).isTrue(); + + // Step 3. + try (var execution2 = new CommandExecution( + createHandler(), "art", "pr-dexopt-job", "--cancel")) { + int exitCode2 = execution2.waitAndGetExitCode(); + String outputs2 = getOutputs(execution2); + assertWithMessage(outputs2).that(exitCode2).isEqualTo(0); + assertThat(outputs2).contains("Pre-reboot Dexopt job cancelled"); + } + + int exitCode = execution.waitAndGetExitCode(); + + // Step 5. + String outputs = getOutputs(execution); + assertWithMessage(outputs).that(exitCode).isEqualTo(0); + assertThat(outputs).contains("Job finished. See logs for details"); + } + } + + @Test + @EnableFlags({android.os.Flags.FLAG_UPDATE_ENGINE_API}) + public void testOnOtaStagedAsyncCancelledByBrokenPipe() throws Exception { + when(mInjector.getCallingUid()).thenReturn(Process.ROOT_UID); + + try (var execution = new CommandExecution( + createHandler(), "art", "on-ota-staged", "--slot", "_b")) { + int exitCode = execution.waitAndGetExitCode(); + String outputs = getOutputs(execution); + assertWithMessage(outputs).that(exitCode).isEqualTo(0); + assertThat(outputs).contains("Pre-reboot Dexopt job scheduled"); + } + + Semaphore dexoptStarted = new Semaphore(0); + + when(mPreRebootDriver.run(eq("_b"), eq(false) /* mapSnapshotsForOta */, any())) + .thenAnswer(invocation -> { + // Step 2. + dexoptStarted.release(); + + Semaphore dexoptCancelled = new Semaphore(0); + var cancellationSignal = invocation.<CancellationSignal>getArgument(2); + cancellationSignal.setOnCancelListener(() -> dexoptCancelled.release()); + assertThat(dexoptCancelled.tryAcquire(TIMEOUT_SEC, TimeUnit.SECONDS)).isTrue(); + + // Step 4. + return new PreRebootResult(true /* success */); + }); + + mPreRebootDexoptJob.onStartJobImpl(mJobService, mJobParameters); + + // Step 1. + try (var execution = + new CommandExecution(createHandler(), "art", "on-ota-staged", "--start")) { + assertThat(execution.getStdout().readLine()).contains("Job running..."); + + assertThat(dexoptStarted.tryAcquire(TIMEOUT_SEC, TimeUnit.SECONDS)).isTrue(); + + // Step 3. + execution.closeStdin(); + + int exitCode = execution.waitAndGetExitCode(); + + // Step 5. + String outputs = getOutputs(execution); + assertWithMessage(outputs).that(exitCode).isEqualTo(0); + assertThat(outputs).contains("Job finished. See logs for details"); + } + } + + @Test + @EnableFlags({android.os.Flags.FLAG_UPDATE_ENGINE_API}) + public void testOnOtaStagedAsyncCancelledByJobScheduler() throws Exception { + when(mInjector.getCallingUid()).thenReturn(Process.ROOT_UID); + + try (var execution = new CommandExecution( + createHandler(), "art", "on-ota-staged", "--slot", "_b")) { + int exitCode = execution.waitAndGetExitCode(); + String outputs = getOutputs(execution); + assertWithMessage(outputs).that(exitCode).isEqualTo(0); + assertThat(outputs).contains("Pre-reboot Dexopt job scheduled"); + } + + mPreRebootDexoptJob.onStartJobImpl(mJobService, mJobParameters); + mPreRebootDexoptJob.onStopJobImpl(mJobParameters); + + mPreRebootDexoptJob.waitForRunningJob(); + verify(mUpdateEngine).triggerPostinstall("system"); + verify(mPreRebootDriver, never()).run(any(), anyBoolean(), any()); + } + + @Test + @DisableFlags({android.os.Flags.FLAG_UPDATE_ENGINE_API}) public void testOnOtaStagedAsyncLegacy() throws Exception { when(mInjector.getCallingUid()).thenReturn(Process.ROOT_UID); @@ -253,6 +448,19 @@ public class ArtShellCommandTest { } @Test + public void testOnOtaStagedStartJobNotFound() throws Exception { + when(mInjector.getCallingUid()).thenReturn(Process.ROOT_UID); + + try (var execution = + new CommandExecution(createHandler(), "art", "on-ota-staged", "--start")) { + int exitCode = execution.waitAndGetExitCode(); + String outputs = getOutputs(execution); + assertWithMessage(outputs).that(exitCode).isEqualTo(1); + assertThat(outputs).contains("No waiting job found"); + } + } + + @Test public void testPrDexoptJobRunMainline() throws Exception { when(mInjector.getCallingUid()).thenReturn(Process.SHELL_UID); @@ -283,7 +491,8 @@ public class ArtShellCommandTest { } @Test - public void testPrDexoptJobRunOta() throws Exception { + @DisableFlags({android.os.Flags.FLAG_UPDATE_ENGINE_API}) + public void testPrDexoptJobRunOtaLegacy() throws Exception { when(mInjector.getCallingUid()).thenReturn(Process.ROOT_UID); when(mPreRebootDriver.run(eq("_b"), eq(true) /* mapSnapshotsForOta */, any())) @@ -299,6 +508,33 @@ public class ArtShellCommandTest { } @Test + @EnableFlags({android.os.Flags.FLAG_UPDATE_ENGINE_API}) + public void testPrDexoptJobRunOta() throws Exception { + when(mInjector.getCallingUid()).thenReturn(Process.ROOT_UID); + + when(mPreRebootDriver.run(eq("_b"), eq(false) /* mapSnapshotsForOta */, any())) + .thenReturn(new PreRebootResult(true /* success */)); + + try (var execution = new CommandExecution( + createHandler(), "art", "pr-dexopt-job", "--run", "--slot", "_b")) { + assertThat(execution.getStdout().readLine()).contains("Job running..."); + + try (var execution2 = new CommandExecution( + createHandler(), "art", "on-ota-staged", "--start")) { + int exitCode2 = execution2.waitAndGetExitCode(); + String outputs2 = getOutputs(execution2); + assertWithMessage(outputs2).that(exitCode2).isEqualTo(0); + assertThat(outputs2).contains("Job finished. See logs for details"); + } + + int exitCode = execution.waitAndGetExitCode(); + String outputs = getOutputs(execution); + assertWithMessage(outputs).that(exitCode).isEqualTo(0); + assertThat(outputs).contains("Job finished. See logs for details"); + } + } + + @Test public void testPrDexoptJobScheduleMainline() throws Exception { when(mInjector.getCallingUid()).thenReturn(Process.SHELL_UID); @@ -332,7 +568,8 @@ public class ArtShellCommandTest { } @Test - public void testPrDexoptJobScheduleOta() throws Exception { + @DisableFlags({android.os.Flags.FLAG_UPDATE_ENGINE_API}) + public void testPrDexoptJobScheduleOtaLegacy() throws Exception { when(mInjector.getCallingUid()).thenReturn(Process.ROOT_UID); try (var execution = new CommandExecution( @@ -351,6 +588,35 @@ public class ArtShellCommandTest { } @Test + @EnableFlags({android.os.Flags.FLAG_UPDATE_ENGINE_API}) + public void testPrDexoptJobScheduleOta() throws Exception { + when(mInjector.getCallingUid()).thenReturn(Process.ROOT_UID); + + try (var execution = new CommandExecution( + createHandler(), "art", "pr-dexopt-job", "--schedule", "--slot", "_b")) { + int exitCode = execution.waitAndGetExitCode(); + String outputs = getOutputs(execution); + assertWithMessage(outputs).that(exitCode).isEqualTo(0); + assertThat(outputs).contains("Pre-reboot Dexopt job scheduled"); + } + + when(mPreRebootDriver.run(eq("_b"), eq(false) /* mapSnapshotsForOta */, any())) + .thenReturn(new PreRebootResult(true /* success */)); + + mPreRebootDexoptJob.onStartJobImpl(mJobService, mJobParameters); + + try (var execution = + new CommandExecution(createHandler(), "art", "on-ota-staged", "--start")) { + int exitCode = execution.waitAndGetExitCode(); + String outputs = getOutputs(execution); + assertWithMessage(outputs).that(exitCode).isEqualTo(0); + assertThat(outputs).contains("Job finished. See logs for details"); + } + + mPreRebootDexoptJob.waitForRunningJob(); + } + + @Test public void testPrDexoptJobCancelJobNotFound() throws Exception { when(mInjector.getCallingUid()).thenReturn(Process.ROOT_UID); diff --git a/libartservice/service/javatests/com/android/server/art/DexUseManagerTest.java b/libartservice/service/javatests/com/android/server/art/DexUseManagerTest.java index 3ea3509b82..d404b4cbda 100644 --- a/libartservice/service/javatests/com/android/server/art/DexUseManagerTest.java +++ b/libartservice/service/javatests/com/android/server/art/DexUseManagerTest.java @@ -36,7 +36,6 @@ import android.content.Context; import android.content.Intent; import android.os.Binder; import android.os.Environment; -import android.os.Process; import android.os.SystemProperties; import android.os.UserHandle; import android.os.storage.StorageManager; @@ -88,7 +87,7 @@ public class DexUseManagerTest { @Rule public StaticMockitoRule mockitoRule = new StaticMockitoRule( - SystemProperties.class, Constants.class, Process.class, ArtJni.class); + SystemProperties.class, Constants.class, ArtJni.class); private final UserHandle mUserHandle = UserHandle.of(1); @@ -125,8 +124,6 @@ public class DexUseManagerTest { lenient().when(Constants.getNative64BitAbi()).thenReturn("arm64-v8a"); lenient().when(Constants.getNative32BitAbi()).thenReturn("armeabi-v7a"); - lenient().when(Process.isIsolatedUid(anyInt())).thenReturn(false); - // Use a LinkedHashMap so that we can control the iteration order. mPackageStates = new LinkedHashMap<>(); @@ -187,6 +184,7 @@ public class DexUseManagerTest { lenient().when(mInjector.getPackageManagerLocal()).thenReturn(mPackageManagerLocal); lenient().when(mInjector.getCallingUserHandle()).thenReturn(mUserHandle); lenient().when(mInjector.getCallingUid()).thenReturn(110001); + lenient().when(mInjector.isIsolatedUid(anyInt())).thenReturn(false); mDexUseManager = new DexUseManagerLocal(mInjector); mDexUseManager.systemReady(); @@ -208,7 +206,7 @@ public class DexUseManagerTest { @Test public void testPrimaryDexOwnedIsolated() { - when(Process.isIsolatedUid(anyInt())).thenReturn(true); + when(mInjector.isIsolatedUid(anyInt())).thenReturn(true); mDexUseManager.notifyDexContainersLoaded( mSnapshot, OWNING_PKG_NAME, Map.of(BASE_APK, "CLC")); @@ -223,7 +221,7 @@ public class DexUseManagerTest { @Test public void testPrimaryDexOwnedSplitIsolated() { - when(Process.isIsolatedUid(anyInt())).thenReturn(true); + when(mInjector.isIsolatedUid(anyInt())).thenReturn(true); mDexUseManager.notifyDexContainersLoaded( mSnapshot, OWNING_PKG_NAME, Map.of(SPLIT_APK, "CLC")); @@ -317,7 +315,7 @@ public class DexUseManagerTest { mDexUseManager.notifyDexContainersLoaded( mSnapshot, LOADING_PKG_NAME, Map.of(BASE_APK, "CLC")); - when(Process.isIsolatedUid(anyInt())).thenReturn(true); + when(mInjector.isIsolatedUid(anyInt())).thenReturn(true); mDexUseManager.notifyDexContainersLoaded( mSnapshot, OWNING_PKG_NAME, Map.of(BASE_APK, "CLC")); when(mInjector.getCurrentTimeMillis()).thenReturn(2000l); @@ -367,7 +365,7 @@ public class DexUseManagerTest { @Test public void testSecondaryDexOwnedIsolated() { - when(Process.isIsolatedUid(anyInt())).thenReturn(true); + when(mInjector.isIsolatedUid(anyInt())).thenReturn(true); mDexUseManager.notifyDexContainersLoaded( mSnapshot, OWNING_PKG_NAME, Map.of(mDeDir + "/foo.apk", "CLC")); @@ -529,7 +527,7 @@ public class DexUseManagerTest { mDexUseManager.notifyDexContainersLoaded(mSnapshot, OWNING_PKG_NAME, Map.of(mCeDir + "/baz.apk", SecondaryDexInfo.UNSUPPORTED_CLASS_LOADER_CONTEXT)); - when(Process.isIsolatedUid(anyInt())).thenReturn(true); + when(mInjector.isIsolatedUid(anyInt())).thenReturn(true); mDexUseManager.notifyDexContainersLoaded( mSnapshot, OWNING_PKG_NAME, Map.of(mCeDir + "/foo.apk", "CLC")); when(mInjector.getCurrentTimeMillis()).thenReturn(2000l); @@ -618,7 +616,7 @@ public class DexUseManagerTest { mDexUseManager.notifyDexContainersLoaded( mSnapshot, LOADING_PKG_NAME, Map.of(mCeDir + "/foo.apk", "CLC")); - when(Process.isIsolatedUid(anyInt())).thenReturn(true); + when(mInjector.isIsolatedUid(anyInt())).thenReturn(true); mDexUseManager.notifyDexContainersLoaded( mSnapshot, OWNING_PKG_NAME, Map.of(mCeDir + "/foo.apk", "CLC")); @@ -666,7 +664,7 @@ public class DexUseManagerTest { mDexUseManager.notifyDexContainersLoaded( mSnapshot, LOADING_PKG_NAME, Map.of(mCeDir + "/foo.apk", "CLC")); - when(Process.isIsolatedUid(anyInt())).thenReturn(true); + when(mInjector.isIsolatedUid(anyInt())).thenReturn(true); mDexUseManager.notifyDexContainersLoaded( mSnapshot, OWNING_PKG_NAME, Map.of(mCeDir + "/foo.apk", "CLC")); diff --git a/libartservice/service/javatests/com/android/server/art/DumpHelperTest.java b/libartservice/service/javatests/com/android/server/art/DumpHelperTest.java index c063260f86..9dbecaf49c 100644 --- a/libartservice/service/javatests/com/android/server/art/DumpHelperTest.java +++ b/libartservice/service/javatests/com/android/server/art/DumpHelperTest.java @@ -80,8 +80,6 @@ public class DumpHelperTest { @Mock private SigningInfo mSigningInfoA; @Mock private SigningInfo mSigningInfoB; - private DumpHelper mDumpHelper; - @Before public void setUp() throws Exception { lenient().when(Constants.getPreferredAbi()).thenReturn("arm64-v8a"); @@ -112,8 +110,6 @@ public class DumpHelperTest { lenient().when(mSigningInfoA.signersMatchExactly(mSigningInfoB)).thenReturn(false); lenient().when(mSigningInfoB.signersMatchExactly(mSigningInfoB)).thenReturn(true); lenient().when(mSigningInfoB.signersMatchExactly(mSigningInfoA)).thenReturn(false); - - mDumpHelper = new DumpHelper(mInjector); } @Test @@ -161,7 +157,8 @@ public class DumpHelperTest { + "Current GC: CollectorTypeCMC\n"; var stringWriter = new StringWriter(); - mDumpHelper.dump(new PrintWriter(stringWriter), mSnapshot, false /* verifySdmSignatures */); + createDumpHelper(false /* verifySdmSignatures */) + .dump(new PrintWriter(stringWriter), mSnapshot); assertThat(stringWriter.toString()).isEqualTo(expected); } @@ -170,73 +167,110 @@ public class DumpHelperTest { when(mInjector.fileExists(any())).thenReturn(false); var stringWriter = new StringWriter(); - mDumpHelper.dumpPackage(new PrintWriter(stringWriter), mSnapshot, - getPackageState(PKG_NAME_BAR), true /* verifySdmSignatures */); + createDumpHelper(true /* verifySdmSignatures */) + .dumpPackage( + new PrintWriter(stringWriter), mSnapshot, getPackageState(PKG_NAME_BAR)); assertThat(stringWriter.toString()).doesNotContain("sdm:"); } @Test public void testDumpSdmStatusInvalidSdmSignature() throws Exception { - when(mInjector.fileExists("/somewhere/app/bar/base.sdm")).thenReturn(true); - when(mInjector.getVerifiedSigningInfo(eq("/somewhere/app/bar/base.sdm"), anyInt())) + doReturn(false).when(mInjector).fileExists("/somewhere/app/bar/base.arm.sdm"); + doReturn(true).when(mInjector).fileExists("/somewhere/app/bar/base.arm64.sdm"); + when(mInjector.getVerifiedSigningInfo(eq("/somewhere/app/bar/base.arm64.sdm"), anyInt())) .thenThrow(SigningInfoException.class); var stringWriter = new StringWriter(); - mDumpHelper.dumpPackage(new PrintWriter(stringWriter), mSnapshot, - getPackageState(PKG_NAME_BAR), true /* verifySdmSignatures */); + createDumpHelper(true /* verifySdmSignatures */) + .dumpPackage( + new PrintWriter(stringWriter), mSnapshot, getPackageState(PKG_NAME_BAR)); assertThat(stringWriter.toString()) .contains("sdm: [sdm-status=pending] [sdm-signature=invalid-sdm-signature]"); } @Test public void testDumpSdmStatusInvalidApkSignature() throws Exception { - when(mInjector.fileExists("/somewhere/app/bar/base.sdm")).thenReturn(true); + doReturn(false).when(mInjector).fileExists("/somewhere/app/bar/base.arm.sdm"); + doReturn(true).when(mInjector).fileExists("/somewhere/app/bar/base.arm64.sdm"); doReturn(mSigningInfoA) .when(mInjector) - .getVerifiedSigningInfo(eq("/somewhere/app/bar/base.sdm"), anyInt()); + .getVerifiedSigningInfo(eq("/somewhere/app/bar/base.arm64.sdm"), anyInt()); doThrow(SigningInfoException.class) .when(mInjector) .getVerifiedSigningInfo(eq("/somewhere/app/bar/base.apk"), anyInt()); var stringWriter = new StringWriter(); - mDumpHelper.dumpPackage(new PrintWriter(stringWriter), mSnapshot, - getPackageState(PKG_NAME_BAR), true /* verifySdmSignatures */); + createDumpHelper(true /* verifySdmSignatures */) + .dumpPackage( + new PrintWriter(stringWriter), mSnapshot, getPackageState(PKG_NAME_BAR)); assertThat(stringWriter.toString()) .contains("sdm: [sdm-status=pending] [sdm-signature=invalid-apk-signature]"); } @Test public void testDumpSdmStatusSignersNotMatch() throws Exception { - when(mInjector.fileExists("/somewhere/app/bar/base.sdm")).thenReturn(true); + doReturn(false).when(mInjector).fileExists("/somewhere/app/bar/base.arm.sdm"); + doReturn(true).when(mInjector).fileExists("/somewhere/app/bar/base.arm64.sdm"); doReturn(mSigningInfoA) .when(mInjector) - .getVerifiedSigningInfo(eq("/somewhere/app/bar/base.sdm"), anyInt()); + .getVerifiedSigningInfo(eq("/somewhere/app/bar/base.arm64.sdm"), anyInt()); doReturn(mSigningInfoB) .when(mInjector) .getVerifiedSigningInfo(eq("/somewhere/app/bar/base.apk"), anyInt()); var stringWriter = new StringWriter(); - mDumpHelper.dumpPackage(new PrintWriter(stringWriter), mSnapshot, - getPackageState(PKG_NAME_BAR), true /* verifySdmSignatures */); + createDumpHelper(true /* verifySdmSignatures */) + .dumpPackage( + new PrintWriter(stringWriter), mSnapshot, getPackageState(PKG_NAME_BAR)); assertThat(stringWriter.toString()) .contains("sdm: [sdm-status=pending] [sdm-signature=mismatched-signers]"); } @Test public void testDumpSdmStatusVerified() throws Exception { - when(mInjector.fileExists("/somewhere/app/bar/base.sdm")).thenReturn(true); + doReturn(false).when(mInjector).fileExists("/somewhere/app/bar/base.arm.sdm"); + doReturn(true).when(mInjector).fileExists("/somewhere/app/bar/base.arm64.sdm"); doReturn(mSigningInfoA) .when(mInjector) - .getVerifiedSigningInfo(eq("/somewhere/app/bar/base.sdm"), anyInt()); + .getVerifiedSigningInfo(eq("/somewhere/app/bar/base.arm64.sdm"), anyInt()); doReturn(mSigningInfoA) .when(mInjector) .getVerifiedSigningInfo(eq("/somewhere/app/bar/base.apk"), anyInt()); var stringWriter = new StringWriter(); - mDumpHelper.dumpPackage(new PrintWriter(stringWriter), mSnapshot, - getPackageState(PKG_NAME_BAR), true /* verifySdmSignatures */); + createDumpHelper(true /* verifySdmSignatures */) + .dumpPackage( + new PrintWriter(stringWriter), mSnapshot, getPackageState(PKG_NAME_BAR)); + assertThat(stringWriter.toString()) + .containsMatch(" \\Qpath: /somewhere/app/bar/base.apk\\E\n" + + " arm:.*\n" + + " .*\n" + + " arm64:.*\n" + + " .*\n" + + " \\Qsdm: [sdm-status=pending] " + + "[sdm-signature=verified]\\E\n"); + } + + @Test + public void testDumpSdmStatusMultiArch() throws Exception { + doReturn(true).when(mInjector).fileExists("/somewhere/app/bar/base.arm.sdm"); + doReturn(true).when(mInjector).fileExists("/somewhere/app/bar/base.arm64.sdm"); + doReturn(mSigningInfoA).when(mInjector).getVerifiedSigningInfo(any(), anyInt()); + + var stringWriter = new StringWriter(); + createDumpHelper(true /* verifySdmSignatures */) + .dumpPackage( + new PrintWriter(stringWriter), mSnapshot, getPackageState(PKG_NAME_BAR)); assertThat(stringWriter.toString()) - .contains("sdm: [sdm-status=pending] [sdm-signature=verified]"); + .containsMatch(" \\Qpath: /somewhere/app/bar/base.apk\\E\n" + + " arm:.*\n" + + " .*\n" + + " \\Qsdm: [sdm-status=pending] " + + "[sdm-signature=verified]\\E\n" + + " arm64:.*\n" + + " .*\n" + + " \\Qsdm: [sdm-status=pending] " + + "[sdm-signature=verified]\\E\n"); } private PackageState createPackageState(@NonNull String packageName, int appId, boolean isApex, @@ -411,4 +445,8 @@ public class DumpHelperTest { private PackageState getPackageState(String packageName) { return mSnapshot.getPackageState(packageName); } + + private DumpHelper createDumpHelper(boolean verifySdmSignatures) { + return new DumpHelper(mInjector, verifySdmSignatures); + } } diff --git a/libartservice/service/javatests/com/android/server/art/PreRebootDexoptJobTest.java b/libartservice/service/javatests/com/android/server/art/PreRebootDexoptJobTest.java index 0d99db20e7..9850145702 100644 --- a/libartservice/service/javatests/com/android/server/art/PreRebootDexoptJobTest.java +++ b/libartservice/service/javatests/com/android/server/art/PreRebootDexoptJobTest.java @@ -37,6 +37,10 @@ import android.app.job.JobParameters; import android.app.job.JobScheduler; import android.os.CancellationSignal; import android.os.SystemProperties; +import android.os.UpdateEngine; +import android.platform.test.annotations.DisableFlags; +import android.platform.test.annotations.EnableFlags; +import android.platform.test.flag.junit.SetFlagsRule; import android.provider.DeviceConfig; import androidx.test.filters.SmallTest; @@ -57,6 +61,7 @@ import java.io.File; import java.util.concurrent.CompletableFuture; import java.util.concurrent.Semaphore; import java.util.concurrent.TimeUnit; +import java.util.function.Supplier; @SmallTest @RunWith(MockitoJUnitRunner.StrictStubs.class) @@ -66,11 +71,13 @@ public class PreRebootDexoptJobTest { @Rule public StaticMockitoRule mockitoRule = new StaticMockitoRule( SystemProperties.class, BackgroundDexoptJobService.class, ArtJni.class); + @Rule public final SetFlagsRule mSetFlagsRule = new SetFlagsRule(); @Mock private PreRebootDexoptJob.Injector mInjector; @Mock private JobScheduler mJobScheduler; @Mock private PreRebootDriver mPreRebootDriver; @Mock private BackgroundDexoptJobService mJobService; + @Mock private UpdateEngine mUpdateEngine; @Mock private PreRebootStatsReporter.Injector mPreRebootStatsReporterInjector; private PreRebootDexoptJob mPreRebootDexoptJob; private JobInfo mJobInfo; @@ -101,6 +108,7 @@ public class PreRebootDexoptJobTest { .when(mInjector.getStatsReporter()) .thenAnswer( invocation -> new PreRebootStatsReporter(mPreRebootStatsReporterInjector)); + lenient().when(mInjector.getUpdateEngine()).thenReturn(mUpdateEngine); File tempFile = File.createTempFile("pre-reboot-stats", ".pb"); tempFile.deleteOnExit(); @@ -131,6 +139,14 @@ public class PreRebootDexoptJobTest { mPreRebootDexoptJob = new PreRebootDexoptJob(mInjector); lenient().when(BackgroundDexoptJobService.getJob(JOB_ID)).thenReturn(mPreRebootDexoptJob); + + lenient() + .doAnswer(invocation -> { + CompletableFuture<?> unused = mPreRebootDexoptJob.notifyUpdateEngineReady(); + return null; + }) + .when(mUpdateEngine) + .triggerPostinstall("system"); } @Test @@ -161,7 +177,7 @@ public class PreRebootDexoptJobTest { .thenReturn(true); CompletableFuture<Void> future = mPreRebootDexoptJob.onUpdateReadyStartNow( - null /* otaSlot */, false /* mapSnapshotsForOta */); + null /* otaSlot */, true /* isUpdataEngineReady */); assertThat(future).isNull(); verify(mPreRebootDriver, never()).run(any(), anyBoolean(), any()); @@ -184,7 +200,7 @@ public class PreRebootDexoptJobTest { .thenReturn(false); CompletableFuture<Void> future = mPreRebootDexoptJob.onUpdateReadyStartNow( - null /* otaSlot */, false /* mapSnapshotsForOta */); + null /* otaSlot */, true /* isUpdataEngineReady */); assertThat(future).isNull(); verify(mPreRebootDriver, never()).run(any(), anyBoolean(), any()); @@ -231,10 +247,10 @@ public class PreRebootDexoptJobTest { verify(mJobScheduler).cancel(JOB_ID); } - @Test - public void testStart() throws Exception { + private void checkStart(String otaSlot, Supplier<Boolean> mapSnapshotsForOtaMatcher) + throws Exception { var jobStarted = new Semaphore(0); - when(mPreRebootDriver.run(any(), eq(true) /* mapSnapshotsForOta */, any())) + when(mPreRebootDriver.run(eq(otaSlot), mapSnapshotsForOtaMatcher.get(), any())) .thenAnswer(invocation -> { jobStarted.release(); return new PreRebootResult(true /* success */); @@ -249,7 +265,7 @@ public class PreRebootDexoptJobTest { }); assertThat(mPreRebootDexoptJob.hasStarted()).isFalse(); - mPreRebootDexoptJob.onUpdateReadyImpl(null /* otaSlot */); + mPreRebootDexoptJob.onUpdateReadyImpl(otaSlot); mPreRebootDexoptJob.onStartJobImpl(mJobService, mJobParameters); assertThat(jobStarted.tryAcquire(TIMEOUT_SEC, TimeUnit.SECONDS)).isTrue(); assertThat(mPreRebootDexoptJob.hasStarted()).isTrue(); @@ -258,17 +274,57 @@ public class PreRebootDexoptJobTest { } @Test - public void testSyncStart() throws Exception { - when(mPreRebootDriver.run(any(), eq(false) /* mapSnapshotsForOta */, any())) + @EnableFlags({android.os.Flags.FLAG_UPDATE_ENGINE_API}) + public void testStartWithUpdateEngineApi() throws Exception { + checkStart("_b" /* otaSlot */, () -> eq(false) /* mapSnapshotsForOtaMatcher */); + verify(mUpdateEngine).triggerPostinstall("system"); + } + + @Test + @DisableFlags({android.os.Flags.FLAG_UPDATE_ENGINE_API}) + public void testStartWithoutUpdateEngineApi() throws Exception { + checkStart("_b" /* otaSlot */, () -> eq(true) /* mapSnapshotsForOtaMatcher */); + verify(mUpdateEngine, never()).triggerPostinstall(any()); + } + + @Test + public void testStartMainline() throws Exception { + checkStart(null /* otaSlot */, () -> anyBoolean() /* mapSnapshotsForOtaMatcher */); + verify(mUpdateEngine, never()).triggerPostinstall(any()); + } + + private void checkSyncStart(boolean isUpdateEngineReady, boolean expectedMapSnapshotsForOta) + throws Exception { + when(mPreRebootDriver.run(eq("_b"), eq(expectedMapSnapshotsForOta), any())) .thenReturn(new PreRebootResult(true /* success */)); - CompletableFuture<Void> future = mPreRebootDexoptJob.onUpdateReadyStartNow( - null /* otaSlot */, false /* mapSnapshotsForOta */); + CompletableFuture<Void> future = + mPreRebootDexoptJob.onUpdateReadyStartNow("_b" /* otaSlot */, isUpdateEngineReady); Utils.getFuture(future); } @Test + @EnableFlags({android.os.Flags.FLAG_UPDATE_ENGINE_API}) + public void testSyncStartWithUpdateEngineApi() throws Exception { + checkSyncStart(false /* isUpdataEngineReady */, false /* expectedMapSnapshotsForOta */); + verify(mUpdateEngine).triggerPostinstall("system"); + } + + @Test + @DisableFlags({android.os.Flags.FLAG_UPDATE_ENGINE_API}) + public void testSyncStartWithoutUpdateEngineApi() throws Exception { + checkSyncStart(false /* isUpdataEngineReady */, true /* expectedMapSnapshotsForOta */); + verify(mUpdateEngine, never()).triggerPostinstall(any()); + } + + @Test + public void testSyncStartWithIsUpdateEngineReady() throws Exception { + checkSyncStart(true /* isUpdataEngineReady */, false /* expectedMapSnapshotsForOta */); + verify(mUpdateEngine, never()).triggerPostinstall(any()); + } + + @Test public void testCancel() { Semaphore dexoptCancelled = new Semaphore(0); Semaphore jobExited = new Semaphore(0); @@ -302,7 +358,7 @@ public class PreRebootDexoptJobTest { }); CompletableFuture<Void> future = mPreRebootDexoptJob.onUpdateReadyStartNow( - null /* otaSlot */, false /* mapSnapshotsForOta */); + null /* otaSlot */, true /* isUpdataEngineReady */); mPreRebootDexoptJob.cancelGiven(future, false /* expectInterrupt */); // Check that `cancelGiven` is really blocking. If it wasn't, the check below might still @@ -480,7 +536,7 @@ public class PreRebootDexoptJobTest { // `onUpdateReadyStartNow`, before the job scheduler calls `onStartJob`. JobParameters oldParameters = mJobParameters; CompletableFuture<Void> future = mPreRebootDexoptJob.onUpdateReadyStartNow( - null /* otaSlot */, false /* mapSnapshotsForOta */); + null /* otaSlot */, true /* isUpdataEngineReady */); // The old job should be cancelled at this point. // This cannot be the new job having exited because jobs are serialized. diff --git a/libdexfile/dex/class_accessor-inl.h b/libdexfile/dex/class_accessor-inl.h index 5979f86e8d..663b75cf86 100644 --- a/libdexfile/dex/class_accessor-inl.h +++ b/libdexfile/dex/class_accessor-inl.h @@ -69,7 +69,7 @@ inline void ClassAccessor::Method::Read() { code_off_ = DecodeUnsignedLeb128(&ptr_pos_); if (hiddenapi_ptr_pos_ != nullptr) { hiddenapi_flags_ = DecodeUnsignedLeb128(&hiddenapi_ptr_pos_); - DCHECK(hiddenapi::ApiList(hiddenapi_flags_).IsValid()); + DCHECK(hiddenapi::ApiList::FromDexFlags(hiddenapi_flags_).IsValid()); } } @@ -83,7 +83,7 @@ inline void ClassAccessor::Field::Read() { access_flags_ = DecodeUnsignedLeb128(&ptr_pos_); if (hiddenapi_ptr_pos_ != nullptr) { hiddenapi_flags_ = DecodeUnsignedLeb128(&hiddenapi_ptr_pos_); - DCHECK(hiddenapi::ApiList(hiddenapi_flags_).IsValid()); + DCHECK(hiddenapi::ApiList::FromDexFlags(hiddenapi_flags_).IsValid()); } } diff --git a/libdexfile/dex/dex_file_verifier.cc b/libdexfile/dex/dex_file_verifier.cc index f0fe14fb4d..3f2fd627db 100644 --- a/libdexfile/dex/dex_file_verifier.cc +++ b/libdexfile/dex/dex_file_verifier.cc @@ -2683,7 +2683,7 @@ bool DexFileVerifier::CheckInterHiddenapiClassData() { failure = true; return; } - if (!hiddenapi::ApiList(decoded_flags).IsValid()) { + if (!hiddenapi::ApiList::FromDexFlags(decoded_flags).IsValid()) { ErrorStringPrintf("Hiddenapi class data flags invalid (%u) for %s %i", decoded_flags, member_type, member.GetIndex()); failure = true; diff --git a/runtime/arch/arm/native_entrypoints_arm.S b/runtime/arch/arm/native_entrypoints_arm.S index 1666dc8d4b..1f3aae6392 100644 --- a/runtime/arch/arm/native_entrypoints_arm.S +++ b/runtime/arch/arm/native_entrypoints_arm.S @@ -63,7 +63,12 @@ ENTRY art_jni_dlsym_lookup_stub bic ip, #TAGGED_JNI_SP_MASK // ArtMethod** sp ldr ip, [ip] // ArtMethod* method ldr ip, [ip, #ART_METHOD_ACCESS_FLAGS_OFFSET] // uint32_t access_flags +#ifdef ART_USE_RESTRICTED_MODE + // Critical native methods are disabled and treated as normal native methods instead. + tst ip, #(ACCESS_FLAGS_METHOD_IS_FAST_NATIVE) +#else tst ip, #(ACCESS_FLAGS_METHOD_IS_FAST_NATIVE | ACCESS_FLAGS_METHOD_IS_CRITICAL_NATIVE) +#endif bne .Llookup_stub_fast_or_critical_native blx artFindNativeMethod b .Llookup_stub_continue diff --git a/runtime/arch/arm64/entrypoints_init_arm64.cc b/runtime/arch/arm64/entrypoints_init_arm64.cc index 3292b15503..acc08c4fd7 100644 --- a/runtime/arch/arm64/entrypoints_init_arm64.cc +++ b/runtime/arch/arm64/entrypoints_init_arm64.cc @@ -53,13 +53,12 @@ extern "C" mirror::Object* art_quick_read_barrier_mark_reg09(mirror::Object*); extern "C" mirror::Object* art_quick_read_barrier_mark_reg10(mirror::Object*); extern "C" mirror::Object* art_quick_read_barrier_mark_reg11(mirror::Object*); extern "C" mirror::Object* art_quick_read_barrier_mark_reg12(mirror::Object*); -extern "C" mirror::Object* art_quick_read_barrier_mark_reg12(mirror::Object*); extern "C" mirror::Object* art_quick_read_barrier_mark_reg13(mirror::Object*); extern "C" mirror::Object* art_quick_read_barrier_mark_reg14(mirror::Object*); extern "C" mirror::Object* art_quick_read_barrier_mark_reg15(mirror::Object*); -extern "C" mirror::Object* art_quick_read_barrier_mark_reg16(mirror::Object*); +// extern "C" mirror::Object* art_quick_read_barrier_mark_reg16(mirror::Object*); ip0 is blocked extern "C" mirror::Object* art_quick_read_barrier_mark_reg17(mirror::Object*); -extern "C" mirror::Object* art_quick_read_barrier_mark_reg18(mirror::Object*); +// extern "C" mirror::Object* art_quick_read_barrier_mark_reg18(mirror::Object*); x18 is blocked extern "C" mirror::Object* art_quick_read_barrier_mark_reg19(mirror::Object*); extern "C" mirror::Object* art_quick_read_barrier_mark_reg20(mirror::Object*); extern "C" mirror::Object* art_quick_read_barrier_mark_reg21(mirror::Object*); @@ -130,8 +129,13 @@ void UpdateReadBarrierEntrypoints(QuickEntryPoints* qpoints, bool is_active) { qpoints->SetReadBarrierMarkReg28(is_active ? art_quick_read_barrier_mark_reg28 : nullptr); qpoints->SetReadBarrierMarkReg29(is_active ? art_quick_read_barrier_mark_reg29 : nullptr); - // Check that array switch cases are at appropriate offsets from the introspection entrypoint. DCHECK_ALIGNED(art_quick_read_barrier_mark_introspection, 512u); + + // TODO(Simulator): the introspection entrypoints are not currently used in the simulator and + // they are not aligned correctly due to the veneer used in CALL_SYMBOL and BRANCH_SYMBOL. + // Re-enable these checks when the introspection entrypoints are used and tested. +#ifndef ART_USE_RESTRICTED_MODE + // Check that array switch cases are at appropriate offsets from the introspection entrypoint. intptr_t array_diff = reinterpret_cast<intptr_t>(art_quick_read_barrier_mark_introspection_arrays) - reinterpret_cast<intptr_t>(art_quick_read_barrier_mark_introspection); @@ -141,6 +145,7 @@ void UpdateReadBarrierEntrypoints(QuickEntryPoints* qpoints, bool is_active) { reinterpret_cast<intptr_t>(art_quick_read_barrier_mark_introspection_gc_roots) - reinterpret_cast<intptr_t>(art_quick_read_barrier_mark_introspection); DCHECK_EQ(BAKER_MARK_INTROSPECTION_GC_ROOT_ENTRYPOINT_OFFSET, gc_roots_diff); +#endif // ART_USE_RESTRICTED_MODE // The register 16, i.e. IP0, is reserved, so there is no art_quick_read_barrier_mark_reg16. // We're using the entry to hold a pointer to the introspection entrypoint instead. qpoints->SetReadBarrierMarkReg16(is_active ? art_quick_read_barrier_mark_introspection : nullptr); diff --git a/runtime/arch/arm64/native_entrypoints_arm64.S b/runtime/arch/arm64/native_entrypoints_arm64.S index 747e572c97..00558c2f98 100644 --- a/runtime/arch/arm64/native_entrypoints_arm64.S +++ b/runtime/arch/arm64/native_entrypoints_arm64.S @@ -62,7 +62,12 @@ ENTRY art_jni_dlsym_lookup_stub bic xIP0, xIP0, #TAGGED_JNI_SP_MASK // ArtMethod** sp ldr xIP0, [xIP0] // ArtMethod* method ldr xIP0, [xIP0, #ART_METHOD_ACCESS_FLAGS_OFFSET] // uint32_t access_flags +#ifdef ART_USE_RESTRICTED_MODE + // Critical native methods are disabled and treated as normal native methods instead. + mov xIP1, #(ACCESS_FLAGS_METHOD_IS_FAST_NATIVE) +#else mov xIP1, #(ACCESS_FLAGS_METHOD_IS_FAST_NATIVE | ACCESS_FLAGS_METHOD_IS_CRITICAL_NATIVE) +#endif tst xIP0, xIP1 b.ne .Llookup_stub_fast_or_critical_native bl artFindNativeMethod diff --git a/runtime/arch/riscv64/native_entrypoints_riscv64.S b/runtime/arch/riscv64/native_entrypoints_riscv64.S index 24c8205c0f..08d5fc04ae 100644 --- a/runtime/arch/riscv64/native_entrypoints_riscv64.S +++ b/runtime/arch/riscv64/native_entrypoints_riscv64.S @@ -65,7 +65,12 @@ ENTRY art_jni_dlsym_lookup_stub andi t0, t0, ~TAGGED_JNI_SP_MASK // ArtMethod** sp ld t0, (t0) // ArtMethod* method lw t0, ART_METHOD_ACCESS_FLAGS_OFFSET(t0) // uint32_t access_flags +#ifdef ART_USE_RESTRICTED_MODE + // Critical native methods are disabled and treated as normal native methods instead. + li t1, (ACCESS_FLAGS_METHOD_IS_FAST_NATIVE) +#else li t1, (ACCESS_FLAGS_METHOD_IS_FAST_NATIVE | ACCESS_FLAGS_METHOD_IS_CRITICAL_NATIVE) +#endif and t0, t0, t1 bnez t0, .Llookup_stub_fast_or_critical_native call artFindNativeMethod diff --git a/runtime/arch/x86/native_entrypoints_x86.S b/runtime/arch/x86/native_entrypoints_x86.S index 9d1c41a069..a676b0f664 100644 --- a/runtime/arch/x86/native_entrypoints_x86.S +++ b/runtime/arch/x86/native_entrypoints_x86.S @@ -65,8 +65,13 @@ DEFINE_FUNCTION art_jni_dlsym_lookup_stub movl THREAD_TOP_QUICK_FRAME_OFFSET(%eax), %eax // uintptr_t tagged_quick_frame andl LITERAL(TAGGED_JNI_SP_MASK_TOGGLED32), %eax // ArtMethod** sp movl (%eax), %eax // ArtMethod* method +#ifdef ART_USE_RESTRICTED_MODE + // Critical native methods are disabled and treated as normal native methods instead. + testl LITERAL(ACCESS_FLAGS_METHOD_IS_FAST_NATIVE), ART_METHOD_ACCESS_FLAGS_OFFSET(%eax) +#else testl LITERAL(ACCESS_FLAGS_METHOD_IS_FAST_NATIVE | ACCESS_FLAGS_METHOD_IS_CRITICAL_NATIVE), \ ART_METHOD_ACCESS_FLAGS_OFFSET(%eax) +#endif jne .Llookup_stub_fast_or_critical_native call SYMBOL(artFindNativeMethod) // (Thread*) jmp .Llookup_stub_continue diff --git a/runtime/arch/x86_64/native_entrypoints_x86_64.S b/runtime/arch/x86_64/native_entrypoints_x86_64.S index 12194ef97c..b42981f030 100644 --- a/runtime/arch/x86_64/native_entrypoints_x86_64.S +++ b/runtime/arch/x86_64/native_entrypoints_x86_64.S @@ -73,8 +73,14 @@ DEFINE_FUNCTION art_jni_dlsym_lookup_stub movq THREAD_TOP_QUICK_FRAME_OFFSET(%rdi), %rax // uintptr_t tagged_quick_frame andq LITERAL(TAGGED_JNI_SP_MASK_TOGGLED64), %rax // ArtMethod** sp movq (%rax), %rax // ArtMethod* method +#ifdef ART_USE_RESTRICTED_MODE + // Critical native methods are disabled and treated as normal native methods instead. + testl LITERAL(ACCESS_FLAGS_METHOD_IS_FAST_NATIVE), \ + ART_METHOD_ACCESS_FLAGS_OFFSET(%rax) +#else testl LITERAL(ACCESS_FLAGS_METHOD_IS_FAST_NATIVE | ACCESS_FLAGS_METHOD_IS_CRITICAL_NATIVE), \ ART_METHOD_ACCESS_FLAGS_OFFSET(%rax) +#endif jne .Llookup_stub_fast_or_critical_native call SYMBOL(artFindNativeMethod) // (Thread*) jmp .Llookup_stub_continue diff --git a/runtime/art_method.h b/runtime/art_method.h index ad55547c3b..1884d715e1 100644 --- a/runtime/art_method.h +++ b/runtime/art_method.h @@ -470,12 +470,19 @@ class EXPORT ArtMethod final { return IsCriticalNative(GetAccessFlags()); } - static bool IsCriticalNative(uint32_t access_flags) { + static bool IsCriticalNative([[maybe_unused]] uint32_t access_flags) { +#ifdef ART_USE_RESTRICTED_MODE + // Return false to treat all critical native methods as normal native methods instead, i.e.: + // will use the generic JNI trampoline instead. + // TODO(Simulator): support critical native methods + return false; +#else // The presence of the annotation is checked by ClassLinker and recorded in access flags. // The kAccCriticalNative flag value is used with a different meaning for non-native methods, // so we need to check the kAccNative flag as well. constexpr uint32_t mask = kAccCriticalNative | kAccNative; return (access_flags & mask) == mask; +#endif } // Returns true if the method is managed (not native). diff --git a/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc b/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc index 767b0c7b8b..f7f3673259 100644 --- a/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc +++ b/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc @@ -1930,8 +1930,17 @@ class BuildGenericJniFrameVisitor final : public QuickArgumentVisitor { fsc.GetStartFprRegs(reserved_area), out_args_sp); + bool uses_critical_args = critical_native; + +#ifdef ART_USE_RESTRICTED_MODE + // IsCriticalNative() always returns false so check if the method is actually a critical native + // method. If it is then it won't need the JNI environment or jclass arguments. + constexpr uint32_t mask = kAccCriticalNative | kAccNative; + uses_critical_args = (method->GetAccessFlags() & mask) == mask; +#endif + // First 2 parameters are always excluded for CriticalNative methods. - if (LIKELY(!critical_native)) { + if (LIKELY(!uses_critical_args)) { // jni environment is always first argument sm_.AdvancePointer(self->GetJniEnv()); diff --git a/runtime/gc/collector/mark_compact.cc b/runtime/gc/collector/mark_compact.cc index 4d220e331e..fafd8b55b4 100644 --- a/runtime/gc/collector/mark_compact.cc +++ b/runtime/gc/collector/mark_compact.cc @@ -4538,7 +4538,14 @@ void MarkCompact::ScanObject(mirror::Object* obj) { } if (klass == nullptr) { // It must be heap corruption. - LOG(FATAL_WITHOUT_ABORT) << "klass pointer for obj: " << obj << " found to be null."; + LOG(FATAL_WITHOUT_ABORT) << "klass pointer for obj: " << obj << " found to be null." + << " black_dense_end: " << static_cast<void*>(black_dense_end_) + << " mid_gen_end: " << static_cast<void*>(mid_gen_end_) + << " prev_post_compact_end: " << prev_post_compact_end_ + << " prev_black_allocations_begin: " << prev_black_allocations_begin_ + << " prev_black_dense_end: " << prev_black_dense_end_ + << " prev_gc_young: " << prev_gc_young_ + << " prev_gc_performed_comaction: " << prev_gc_performed_compaction_; heap_->GetVerification()->LogHeapCorruption( obj, mirror::Object::ClassOffset(), klass, /*fatal=*/true); } @@ -4817,6 +4824,14 @@ void MarkCompact::FinishPhase(bool performed_compaction) { compacting_ = false; marking_done_ = false; uint8_t* mark_bitmap_clear_end = black_dense_end_; + // Retain values of some fields for logging in next GC cycle, in case there is + // a memory corruption detected. + prev_black_allocations_begin_ = static_cast<void*>(black_allocations_begin_); + prev_black_dense_end_ = static_cast<void*>(black_dense_end_); + prev_post_compact_end_ = static_cast<void*>(post_compact_end_); + prev_gc_young_ = young_gen_; + prev_gc_performed_compaction_ = performed_compaction; + // Whether compaction is performend or not, we always set post_compact_end_ // before reaching here. CHECK_NE(post_compact_end_, nullptr); diff --git a/runtime/gc/collector/mark_compact.h b/runtime/gc/collector/mark_compact.h index 8392bfec96..d9083fcb2c 100644 --- a/runtime/gc/collector/mark_compact.h +++ b/runtime/gc/collector/mark_compact.h @@ -931,6 +931,12 @@ class MarkCompact final : public GarbageCollector { // is incorporated. void* stack_high_addr_; void* stack_low_addr_; + // Following values for logging purposes + void* prev_post_compact_end_; + void* prev_black_dense_end_; + void* prev_black_allocations_begin_; + bool prev_gc_young_; + bool prev_gc_performed_compaction_; class FlipCallback; class ThreadFlipVisitor; diff --git a/runtime/gc/collector_type.h b/runtime/gc/collector_type.h index 3c19079c08..4267a7763c 100644 --- a/runtime/gc/collector_type.h +++ b/runtime/gc/collector_type.h @@ -75,6 +75,8 @@ static constexpr CollectorType kCollectorTypeDefault = kCollectorTypeSS #elif ART_DEFAULT_GC_TYPE_IS_CMS kCollectorTypeCMS +#elif ART_DEFAULT_GC_TYPE_IS_MS + kCollectorTypeMS #else #error "ART default GC type must be set" #endif diff --git a/runtime/hidden_api.cc b/runtime/hidden_api.cc index c7fb3ef7ff..0dc0b352f9 100644 --- a/runtime/hidden_api.cc +++ b/runtime/hidden_api.cc @@ -354,7 +354,7 @@ void MemberSignature::Dump(std::ostream& os) const { } void MemberSignature::LogAccessToLogcat(AccessMethod access_method, - hiddenapi::ApiList list, + ApiList api_list, bool access_denied, uint32_t runtime_flags, const AccessContext& caller_context, @@ -368,10 +368,10 @@ void MemberSignature::LogAccessToLogcat(AccessMethod access_method, << "hiddenapi: Accessing hidden " << (type_ == kField ? "field " : "method ") << Dumpable<MemberSignature>(*this) << " (runtime_flags=" << FormatHiddenApiRuntimeFlags(runtime_flags) - << ", domain=" << callee_context.GetDomain() << ", api=" << list << ") from " + << ", domain=" << callee_context.GetDomain() << ", api=" << api_list << ") from " << caller_context << " (domain=" << caller_context.GetDomain() << ") using " << access_method << (access_denied ? ": denied" : ": allowed"); - if (access_denied && list.IsTestApi()) { + if (access_denied && api_list.IsTestApi()) { // see b/177047045 for more details about test api access getting denied LOG(WARNING) << "hiddenapi: If this is a platform test consider enabling " << "VMRuntime.ALLOW_TEST_API_ACCESS change id for this package."; @@ -533,8 +533,7 @@ uint32_t GetDexFlags(T* member) REQUIRES_SHARED(Locks::mutator_lock_) { ObjPtr<mirror::Class> declaring_class = member->GetDeclaringClass(); DCHECK(!declaring_class.IsNull()) << "Attempting to access a runtime method"; - ApiList flags; - DCHECK(!flags.IsValid()); + ApiList flags = ApiList::Invalid(); // Check if the declaring class has ClassExt allocated. If it does, check if // the pre-JVMTI redefine dex file has been set to determine if the declaring @@ -556,7 +555,7 @@ uint32_t GetDexFlags(T* member) REQUIRES_SHARED(Locks::mutator_lock_) { uint32_t member_index = GetMemberDexIndex(member); auto fn_visit = [&](const AccessorType& dex_member) { if (dex_member.GetIndex() == member_index) { - flags = ApiList(dex_member.GetHiddenapiFlags()); + flags = ApiList::FromDexFlags(dex_member.GetHiddenapiFlags()); } }; VisitMembers(declaring_class->GetDexFile(), *class_def, fn_visit); @@ -576,7 +575,7 @@ uint32_t GetDexFlags(T* member) REQUIRES_SHARED(Locks::mutator_lock_) { MemberSignature cur_signature(dex_member); if (member_signature.MemberNameAndTypeMatch(cur_signature)) { DCHECK(member_signature.Equals(cur_signature)); - flags = ApiList(dex_member.GetHiddenapiFlags()); + flags = ApiList::FromDexFlags(dex_member.GetHiddenapiFlags()); } }; VisitMembers(*original_dex, original_class_def, fn_visit); @@ -811,7 +810,7 @@ bool ShouldDenyAccessToMember(T* member, // Decode hidden API access flags from the dex file. // This is an O(N) operation scaling with the number of fields/methods // in the class. Only do this on slow path and only do it once. - ApiList api_list(detail::GetDexFlags(member)); + ApiList api_list = ApiList::FromDexFlags(detail::GetDexFlags(member)); DCHECK(api_list.IsValid()); // Member is hidden and caller is not exempted. Enter slow path. diff --git a/runtime/hidden_api.h b/runtime/hidden_api.h index d2a6cdcbd3..7d7bd75fc4 100644 --- a/runtime/hidden_api.h +++ b/runtime/hidden_api.h @@ -266,7 +266,7 @@ inline ArtMethod* GetInterfaceMemberIfProxy(ArtMethod* method) ALWAYS_INLINE inline uint32_t CreateRuntimeFlags_Impl(uint32_t dex_flags) { uint32_t runtime_flags = 0u; - ApiList api_list(dex_flags); + ApiList api_list = ApiList::FromDexFlags(dex_flags); DCHECK(api_list.IsValid()); if (api_list.Contains(ApiList::Sdk())) { diff --git a/runtime/hidden_api_test.cc b/runtime/hidden_api_test.cc index 5a2ee55723..39cca7cce6 100644 --- a/runtime/hidden_api_test.cc +++ b/runtime/hidden_api_test.cc @@ -202,6 +202,11 @@ class HiddenApiTest : public CommonRuntimeTest { hiddenapi::AccessMethod::kCheck); } + bool ShouldDenyAccess(hiddenapi::ApiList list1, hiddenapi::ApiList list2) + REQUIRES_SHARED(Locks::mutator_lock_) { + return ShouldDenyAccess(hiddenapi::ApiList::Combine(list1, list2)); + } + void TestLocation(const std::string& location, hiddenapi::Domain expected_domain) { // Create a temp file with a unique name based on `location` to isolate tests // that may run in parallel. b/238730923 @@ -382,59 +387,63 @@ TEST_F(HiddenApiTest, CheckTestApiEnforcement) { runtime_->SetTargetSdkVersion( static_cast<uint32_t>(hiddenapi::ApiList::MaxTargetR().GetMaxAllowedSdkVersion()) + 1); + // clang-format off + // Default case where all TestApis are treated like non-TestApi. runtime_->SetTestApiEnforcementPolicy(hiddenapi::EnforcementPolicy::kEnabled); SetChangeIdState(kAllowTestApiAccess, false); ASSERT_EQ( - ShouldDenyAccess(hiddenapi::ApiList::TestApi() | hiddenapi::ApiList::Sdk()), false); + ShouldDenyAccess(hiddenapi::ApiList::TestApi(), hiddenapi::ApiList::Sdk()), false); ASSERT_EQ( - ShouldDenyAccess(hiddenapi::ApiList::TestApi() | hiddenapi::ApiList::Unsupported()), false); + ShouldDenyAccess(hiddenapi::ApiList::TestApi(), hiddenapi::ApiList::Unsupported()), false); ASSERT_EQ( - ShouldDenyAccess(hiddenapi::ApiList::TestApi() | hiddenapi::ApiList::MaxTargetR()), true); + ShouldDenyAccess(hiddenapi::ApiList::TestApi(), hiddenapi::ApiList::MaxTargetR()), true); ASSERT_EQ( - ShouldDenyAccess(hiddenapi::ApiList::TestApi() | hiddenapi::ApiList::MaxTargetQ()), true); + ShouldDenyAccess(hiddenapi::ApiList::TestApi(), hiddenapi::ApiList::MaxTargetQ()), true); ASSERT_EQ( - ShouldDenyAccess(hiddenapi::ApiList::TestApi() | hiddenapi::ApiList::MaxTargetP()), true); + ShouldDenyAccess(hiddenapi::ApiList::TestApi(), hiddenapi::ApiList::MaxTargetP()), true); ASSERT_EQ( - ShouldDenyAccess(hiddenapi::ApiList::TestApi() | hiddenapi::ApiList::MaxTargetO()), true); + ShouldDenyAccess(hiddenapi::ApiList::TestApi(), hiddenapi::ApiList::MaxTargetO()), true); ASSERT_EQ( - ShouldDenyAccess(hiddenapi::ApiList::TestApi() | hiddenapi::ApiList::Blocked()), true); + ShouldDenyAccess(hiddenapi::ApiList::TestApi(), hiddenapi::ApiList::Blocked()), true); // A case where we want to allow access to TestApis. runtime_->SetTestApiEnforcementPolicy(hiddenapi::EnforcementPolicy::kDisabled); SetChangeIdState(kAllowTestApiAccess, false); ASSERT_EQ( - ShouldDenyAccess(hiddenapi::ApiList::TestApi() | hiddenapi::ApiList::Sdk()), false); + ShouldDenyAccess(hiddenapi::ApiList::TestApi(), hiddenapi::ApiList::Sdk()), false); ASSERT_EQ( - ShouldDenyAccess(hiddenapi::ApiList::TestApi() | hiddenapi::ApiList::Unsupported()), false); + ShouldDenyAccess(hiddenapi::ApiList::TestApi(), hiddenapi::ApiList::Unsupported()), false); ASSERT_EQ( - ShouldDenyAccess(hiddenapi::ApiList::TestApi() | hiddenapi::ApiList::MaxTargetR()), false); + ShouldDenyAccess(hiddenapi::ApiList::TestApi(), hiddenapi::ApiList::MaxTargetR()), false); ASSERT_EQ( - ShouldDenyAccess(hiddenapi::ApiList::TestApi() | hiddenapi::ApiList::MaxTargetQ()), false); + ShouldDenyAccess(hiddenapi::ApiList::TestApi(), hiddenapi::ApiList::MaxTargetQ()), false); ASSERT_EQ( - ShouldDenyAccess(hiddenapi::ApiList::TestApi() | hiddenapi::ApiList::MaxTargetP()), false); + ShouldDenyAccess(hiddenapi::ApiList::TestApi(), hiddenapi::ApiList::MaxTargetP()), false); ASSERT_EQ( - ShouldDenyAccess(hiddenapi::ApiList::TestApi() | hiddenapi::ApiList::MaxTargetO()), false); + ShouldDenyAccess(hiddenapi::ApiList::TestApi(), hiddenapi::ApiList::MaxTargetO()), false); ASSERT_EQ( - ShouldDenyAccess(hiddenapi::ApiList::TestApi() | hiddenapi::ApiList::Blocked()), false); + ShouldDenyAccess(hiddenapi::ApiList::TestApi(), hiddenapi::ApiList::Blocked()), false); // A second case where we want to allow access to TestApis. runtime_->SetTestApiEnforcementPolicy(hiddenapi::EnforcementPolicy::kEnabled); SetChangeIdState(kAllowTestApiAccess, true); ASSERT_EQ( - ShouldDenyAccess(hiddenapi::ApiList::TestApi() | hiddenapi::ApiList::Sdk()), false); + ShouldDenyAccess(hiddenapi::ApiList::TestApi(), hiddenapi::ApiList::Sdk()), false); ASSERT_EQ( - ShouldDenyAccess(hiddenapi::ApiList::TestApi() | hiddenapi::ApiList::Unsupported()), false); + ShouldDenyAccess(hiddenapi::ApiList::TestApi(), hiddenapi::ApiList::Unsupported()), false); ASSERT_EQ( - ShouldDenyAccess(hiddenapi::ApiList::TestApi() | hiddenapi::ApiList::MaxTargetR()), false); + ShouldDenyAccess(hiddenapi::ApiList::TestApi(), hiddenapi::ApiList::MaxTargetR()), false); ASSERT_EQ( - ShouldDenyAccess(hiddenapi::ApiList::TestApi() | hiddenapi::ApiList::MaxTargetQ()), false); + ShouldDenyAccess(hiddenapi::ApiList::TestApi(), hiddenapi::ApiList::MaxTargetQ()), false); ASSERT_EQ( - ShouldDenyAccess(hiddenapi::ApiList::TestApi() | hiddenapi::ApiList::MaxTargetP()), false); + ShouldDenyAccess(hiddenapi::ApiList::TestApi(), hiddenapi::ApiList::MaxTargetP()), false); ASSERT_EQ( - ShouldDenyAccess(hiddenapi::ApiList::TestApi() | hiddenapi::ApiList::MaxTargetO()), false); + ShouldDenyAccess(hiddenapi::ApiList::TestApi(), hiddenapi::ApiList::MaxTargetO()), false); ASSERT_EQ( - ShouldDenyAccess(hiddenapi::ApiList::TestApi() | hiddenapi::ApiList::Blocked()), false); + ShouldDenyAccess(hiddenapi::ApiList::TestApi(), hiddenapi::ApiList::Blocked()), false); + + // clang-format on } TEST_F(HiddenApiTest, CheckMembersRead) { diff --git a/runtime/interpreter/mterp/nterp.cc b/runtime/interpreter/mterp/nterp.cc index b929444fc6..fc6168c70d 100644 --- a/runtime/interpreter/mterp/nterp.cc +++ b/runtime/interpreter/mterp/nterp.cc @@ -35,6 +35,12 @@ namespace art HIDDEN { namespace interpreter { bool IsNterpSupported() { +#ifdef ART_USE_RESTRICTED_MODE + // TODO(Simulator): Support Nterp. + // Nterp uses the native stack and quick stack frame layout; this will be a complication + // for the simulator mode. We should use switch interpreter only for now. + return false; +#else switch (kRuntimeQuickCodeISA) { case InstructionSet::kArm: case InstructionSet::kThumb2: @@ -48,6 +54,7 @@ bool IsNterpSupported() { default: return false; } +#endif // #ifdef ART_USE_RESTRICTED_MODE } bool CanRuntimeUseNterp() REQUIRES_SHARED(Locks::mutator_lock_) { diff --git a/runtime/mirror/class.cc b/runtime/mirror/class.cc index 7bc0826424..fe923cbaa6 100644 --- a/runtime/mirror/class.cc +++ b/runtime/mirror/class.cc @@ -2298,7 +2298,8 @@ static bool IsInterfaceMethodAccessible(ArtMethod* interface_method) REQUIRES_SHARED(Locks::mutator_lock_) { // If the interface method is part of the public SDK, return it. if ((hiddenapi::GetRuntimeFlags(interface_method) & kAccPublicApi) != 0) { - hiddenapi::ApiList api_list(hiddenapi::detail::GetDexFlags(interface_method)); + hiddenapi::ApiList api_list = + hiddenapi::ApiList::FromDexFlags(hiddenapi::detail::GetDexFlags(interface_method)); // The kAccPublicApi flag is also used as an optimization to avoid // other hiddenapi checks to always go on the slow path. Therefore, we // need to check here if the method is in the SDK list. diff --git a/runtime/runtime.cc b/runtime/runtime.cc index 0186740015..23e06ab792 100644 --- a/runtime/runtime.cc +++ b/runtime/runtime.cc @@ -1441,6 +1441,40 @@ static inline void CreatePreAllocatedException(Thread* self, detailMessageField->SetObject</* kTransactionActive= */ false>(exception->Read(), message); } +inline void Runtime::CreatePreAllocatedExceptions(Thread* self) { + // Pre-allocate an OutOfMemoryError for the case when we fail to + // allocate the exception to be thrown. + CreatePreAllocatedException(self, + this, + &pre_allocated_OutOfMemoryError_when_throwing_exception_, + "Ljava/lang/OutOfMemoryError;", + "OutOfMemoryError thrown while trying to throw an exception; " + "no stack trace available"); + // Pre-allocate an OutOfMemoryError for the double-OOME case. + CreatePreAllocatedException(self, + this, + &pre_allocated_OutOfMemoryError_when_throwing_oome_, + "Ljava/lang/OutOfMemoryError;", + "OutOfMemoryError thrown while trying to throw OutOfMemoryError; " + "no stack trace available"); + // Pre-allocate an OutOfMemoryError for the case when we fail to + // allocate while handling a stack overflow. + CreatePreAllocatedException(self, + this, + &pre_allocated_OutOfMemoryError_when_handling_stack_overflow_, + "Ljava/lang/OutOfMemoryError;", + "OutOfMemoryError thrown while trying to handle a stack overflow; " + "no stack trace available"); + // Pre-allocate a NoClassDefFoundError for the common case of failing to find a system class + // ahead of checking the application's class loader. + CreatePreAllocatedException(self, + this, + &pre_allocated_NoClassDefFoundError_, + "Ljava/lang/NoClassDefFoundError;", + "Class not found using the boot class loader; " + "no stack trace available"); +} + std::string Runtime::GetApexVersions(ArrayRef<const std::string> boot_class_path_locations) { std::vector<std::string_view> bcp_apexes; for (std::string_view jar : boot_class_path_locations) { @@ -1892,6 +1926,12 @@ bool Runtime::Init(RuntimeArgumentMap&& runtime_options_in) { break; } +#ifdef ART_USE_RESTRICTED_MODE + // TODO(Simulator): support signal handling and implicit checks. + implicit_suspend_checks_ = false; + implicit_null_checks_ = false; +#endif // ART_USE_RESTRICTED_MODE + fault_manager.Init(!no_sig_chain_); if (!no_sig_chain_) { if (HandlesSignalsInCompiledCode()) { @@ -2076,38 +2116,7 @@ bool Runtime::Init(RuntimeArgumentMap&& runtime_options_in) { DCHECK(pre_allocated_NoClassDefFoundError_.Read()->GetClass() ->DescriptorEquals("Ljava/lang/NoClassDefFoundError;")); } else { - // Pre-allocate an OutOfMemoryError for the case when we fail to - // allocate the exception to be thrown. - CreatePreAllocatedException(self, - this, - &pre_allocated_OutOfMemoryError_when_throwing_exception_, - "Ljava/lang/OutOfMemoryError;", - "OutOfMemoryError thrown while trying to throw an exception; " - "no stack trace available"); - // Pre-allocate an OutOfMemoryError for the double-OOME case. - CreatePreAllocatedException(self, - this, - &pre_allocated_OutOfMemoryError_when_throwing_oome_, - "Ljava/lang/OutOfMemoryError;", - "OutOfMemoryError thrown while trying to throw OutOfMemoryError; " - "no stack trace available"); - // Pre-allocate an OutOfMemoryError for the case when we fail to - // allocate while handling a stack overflow. - CreatePreAllocatedException(self, - this, - &pre_allocated_OutOfMemoryError_when_handling_stack_overflow_, - "Ljava/lang/OutOfMemoryError;", - "OutOfMemoryError thrown while trying to handle a stack overflow; " - "no stack trace available"); - - // Pre-allocate a NoClassDefFoundError for the common case of failing to find a system class - // ahead of checking the application's class loader. - CreatePreAllocatedException(self, - this, - &pre_allocated_NoClassDefFoundError_, - "Ljava/lang/NoClassDefFoundError;", - "Class not found using the boot class loader; " - "no stack trace available"); + CreatePreAllocatedExceptions(self); } // Class-roots are setup, we can now finish initializing the JniIdManager. diff --git a/runtime/runtime.h b/runtime/runtime.h index 9661f9e514..8f781cff34 100644 --- a/runtime/runtime.h +++ b/runtime/runtime.h @@ -1118,6 +1118,8 @@ class Runtime { // See Flags::ReloadAllFlags as well. static void ReloadAllFlags(const std::string& caller); + inline void CreatePreAllocatedExceptions(Thread* self) REQUIRES_SHARED(Locks::mutator_lock_); + // Parses /apex/apex-info-list.xml to build a string containing apex versions of boot classpath // jars, which is encoded into .oat files. static std::string GetApexVersions(ArrayRef<const std::string> boot_class_path_locations); diff --git a/runtime/runtime_globals.h b/runtime/runtime_globals.h index d3963a52e1..cdab9e0deb 100644 --- a/runtime/runtime_globals.h +++ b/runtime/runtime_globals.h @@ -110,7 +110,12 @@ static constexpr ALWAYS_INLINE size_t ModuloPageSize(size_t num) { // Returns whether the given memory offset can be used for generating // an implicit null check. static inline bool CanDoImplicitNullCheckOn(uintptr_t offset) { +#ifdef ART_USE_RESTRICTED_MODE + UNUSED(offset); + return false; +#else return offset < gPageSize; +#endif // ART_USE_RESTRICTED_MODE } // Required object alignment diff --git a/runtime/trace_profile.cc b/runtime/trace_profile.cc index 2fcf475b80..4c0abbf1da 100644 --- a/runtime/trace_profile.cc +++ b/runtime/trace_profile.cc @@ -76,6 +76,18 @@ void TraceData::AddTracedThread(Thread* thread) { traced_threads_.emplace(thread_id, thread_name); } +void TraceData::MaybeWaitForTraceDumpToFinish() { + if (!trace_dump_in_progress_) { + return; + } + trace_dump_condition_.Wait(Thread::Current()); +} + +void TraceData::SignalTraceDumpComplete() { + trace_dump_in_progress_ = false; + trace_dump_condition_.Broadcast(Thread::Current()); +} + void TraceProfiler::AllocateBuffer(Thread* thread) { if (!art_flags::always_enable_profile_code()) { return; @@ -299,6 +311,10 @@ void TraceProfiler::StopLocked() { return; } + // We should not delete trace_data_ when there is an ongoing trace dump. So + // wait for any in progress trace dump to finish. + trace_data_->MaybeWaitForTraceDumpToFinish(); + static FunctionClosure reset_buffer([](Thread* thread) { auto buffer = thread->GetMethodTraceBuffer(); if (buffer != nullptr) { @@ -393,25 +409,33 @@ void TraceProfiler::Dump(const char* filename) { } void TraceProfiler::Dump(std::unique_ptr<File>&& trace_file, std::ostringstream& os) { - MutexLock mu(Thread::Current(), *Locks::trace_lock_); - if (!profile_in_progress_) { - if (trace_file != nullptr && !trace_file->Close()) { - PLOG(WARNING) << "Failed to close file."; - } - return; - } - Thread* self = Thread::Current(); - // Collect long running methods from all the threads; Runtime* runtime = Runtime::Current(); - TraceDumpCheckpoint checkpoint(trace_data_, trace_file); - size_t threads_running_checkpoint = runtime->GetThreadList()->RunCheckpoint(&checkpoint); + size_t threads_running_checkpoint = 0; + std::unique_ptr<TraceDumpCheckpoint> checkpoint; + { + MutexLock mu(self, *Locks::trace_lock_); + if (!profile_in_progress_ || trace_data_->IsTraceDumpInProgress()) { + if (trace_file != nullptr && !trace_file->Close()) { + PLOG(WARNING) << "Failed to close file."; + } + return; + } + + trace_data_->SetTraceDumpInProgress(); + + // Collect long running methods from all the threads; + checkpoint.reset(new TraceDumpCheckpoint(trace_data_, trace_file)); + threads_running_checkpoint = runtime->GetThreadList()->RunCheckpoint(checkpoint.get()); + } + + // Wait for all threads to dump their data. if (threads_running_checkpoint != 0) { - checkpoint.WaitForThreadsToRunThroughCheckpoint(threads_running_checkpoint); + checkpoint->WaitForThreadsToRunThroughCheckpoint(threads_running_checkpoint); } + checkpoint->FinishTraceDump(os); - trace_data_->DumpData(os); if (trace_file != nullptr) { std::string info = os.str(); if (!trace_file->WriteFully(info.c_str(), info.length())) { @@ -591,16 +615,35 @@ void TraceDumpCheckpoint::WaitForThreadsToRunThroughCheckpoint(size_t threads_ru barrier_.Increment(self, threads_running_checkpoint); } +void TraceDumpCheckpoint::FinishTraceDump(std::ostringstream& os) { + // Dump all the data. + trace_data_->DumpData(os); + + // Any trace stop requests will be blocked while a dump is in progress. So + // broadcast the completion condition for any waiting requests. + MutexLock mu(Thread::Current(), *Locks::trace_lock_); + trace_data_->SignalTraceDumpComplete(); +} + void TraceData::DumpData(std::ostringstream& os) { - MutexLock mu(Thread::Current(), trace_data_lock_); - if (long_running_methods_.length() > 0) { - os << long_running_methods_; + std::unordered_set<ArtMethod*> methods; + std::unordered_map<size_t, std::string> threads; + { + // We cannot dump method information while holding trace_lock_, since we have to also + // acquire a mutator lock. Take a snapshot of thread and method information. + MutexLock mu(Thread::Current(), trace_data_lock_); + if (long_running_methods_.length() > 0) { + os << long_running_methods_; + } + + methods = traced_methods_; + threads = traced_threads_; } // Dump the information about traced_methods and threads { ScopedObjectAccess soa(Thread::Current()); - DumpThreadMethodInfo(traced_threads_, traced_methods_, os); + DumpThreadMethodInfo(threads, methods, os); } } diff --git a/runtime/trace_profile.h b/runtime/trace_profile.h index 24883b217e..df8ddec387 100644 --- a/runtime/trace_profile.h +++ b/runtime/trace_profile.h @@ -48,6 +48,8 @@ class TraceData { explicit TraceData(LowOverheadTraceType trace_type) : trace_type_(trace_type), trace_end_time_(0), + trace_dump_in_progress_(false), + trace_dump_condition_("trace dump condition", *Locks::trace_lock_), trace_data_lock_("Trace Data lock", LockLevel::kGenericBottomLock) {} LowOverheadTraceType GetTraceType() const { @@ -83,6 +85,22 @@ class TraceData { void AddTracedThread(Thread* thread); + // If there is no trace dump in progress this returns immediately. Otherwise + // it waits on a condition variable waiting for the trace dump to finish. + void MaybeWaitForTraceDumpToFinish() REQUIRES(Locks::trace_lock_); + + // Called when a trace dump is finished to notify any waiting requests. This + // also resets the trace_dump_in_progress_ to false. + void SignalTraceDumpComplete() REQUIRES(Locks::trace_lock_); + + void SetTraceDumpInProgress() REQUIRES(Locks::trace_lock_) { + trace_dump_in_progress_ = true; + } + + bool IsTraceDumpInProgress() const REQUIRES(Locks::trace_lock_) { + return trace_dump_in_progress_; + } + private: // This is used to hold the initial methods on stack and also long running methods when there is a // buffer overflow. @@ -100,6 +118,14 @@ class TraceData { // thread. std::unordered_map<size_t, std::string> traced_threads_ GUARDED_BY(trace_data_lock_); + // This specifies if a trace dump is in progress. We release the trace_lock_ + // when waiting for the checkpoints to finish. We shouldn't delete trace data + // when a dump is in progress. trace_dump_in_progress_ and + // trace_dump_condition_ are used to make sure we wait for any in progress + // trace dumps to finish before deleting the trace data. + bool trace_dump_in_progress_ GUARDED_BY(Locks::trace_lock_); + ConditionVariable trace_dump_condition_ GUARDED_BY(Locks::trace_lock_); + // Lock to synchronize access to traced_methods_, traced_threads_ and long_running_methods_ which // can be accessed simultaneously by multiple threads when running TraceDumpCheckpoint. Mutex trace_data_lock_; @@ -115,6 +141,7 @@ class TraceDumpCheckpoint final : public Closure { void Run(Thread* thread) override REQUIRES_SHARED(Locks::mutator_lock_); void WaitForThreadsToRunThroughCheckpoint(size_t threads_running_checkpoint); + void FinishTraceDump(std::ostringstream& os); private: // The barrier to be passed through and for the requestor to wait upon. diff --git a/test/knownfailures.json b/test/knownfailures.json index f764fe9fce..4d2d8ae654 100644 --- a/test/knownfailures.json +++ b/test/knownfailures.json @@ -1623,5 +1623,19 @@ "The ability to destroy a thread group and the concept of a destroyed ", "thread group no longer exists. A thread group is eligible to be GC'ed ", "when there are no live threads in the group and it is otherwise unreachable."] + }, + { + "tests": ["004-StackWalk", + "141-class-unload", + "178-app-image-native-method", + "597-deopt-busy-loop", + "629-vdex-speed", + "638-checker-inline-cache-intrinsic", + "661-oat-writer-layout", + "692-vdex-secondary-loader", + "850-checker-branches", + "2042-reference-processing"], + "env_vars": {"ART_USE_RESTRICTED_MODE": "true"}, + "description": ["Test failures when using the restricted mode for simulator."] } ] diff --git a/test/run-test b/test/run-test index a3f1f60504..1117e07a10 100755 --- a/test/run-test +++ b/test/run-test @@ -759,6 +759,11 @@ if True: if prebuild_mode: run_checker = True + if os.environ.get("ART_USE_RESTRICTED_MODE") == "true": + # TODO(Simulator): support checker runs. + run_checker = False + cfg_output_dir = tmp_dir + if not target_mode: cfg_output_dir = tmp_dir checker_args = f"--arch={host_arch_name.upper()}" diff --git a/tools/hiddenapi/hiddenapi.cc b/tools/hiddenapi/hiddenapi.cc index f5a8d58374..f5d59b9f39 100644 --- a/tools/hiddenapi/hiddenapi.cc +++ b/tools/hiddenapi/hiddenapi.cc @@ -898,7 +898,7 @@ class HiddenApi final { CHECK(!force_assign_all_ || api_list_found) << "Could not find hiddenapi flags for dex entry: " << signature; if (api_list_found && it->second.GetIntValue() > max_hiddenapi_level_.GetIntValue()) { - ApiList without_domain(it->second.GetIntValue()); + ApiList without_domain = ApiList::FromDexFlags(it->second.GetIntValue()); LOG(ERROR) << "Hidden api flag " << without_domain << " for member " << signature << " in " << input_path << " exceeds maximum allowable flag " << max_hiddenapi_level_; @@ -956,7 +956,7 @@ class HiddenApi final { CHECK(api_flag_map.find(signature) == api_flag_map.end()) << path << ":" << line_number << ": Duplicate entry: " << signature << kErrorHelp; - ApiList membership; + ApiList membership = ApiList::Invalid(); std::vector<std::string>::iterator apiListBegin = values.begin() + 1; std::vector<std::string>::iterator apiListEnd = values.end(); @@ -1098,7 +1098,7 @@ class HiddenApi final { // // By default this returns a GetIntValue() that is guaranteed to be bigger than // any valid value returned by GetIntValue(). - ApiList max_hiddenapi_level_; + ApiList max_hiddenapi_level_ = ApiList::Invalid(); // Whether the input is only a fragment of the whole bootclasspath and may // not include a complete set of classes. That requires the tool to ignore missing diff --git a/tools/hiddenapi/hiddenapi_test.cc b/tools/hiddenapi/hiddenapi_test.cc index 3236dddfd3..940a2630d9 100644 --- a/tools/hiddenapi/hiddenapi_test.cc +++ b/tools/hiddenapi/hiddenapi_test.cc @@ -190,7 +190,7 @@ class HiddenApiTest : public CommonRuntimeTest { const uint32_t actual_visibility = field.GetAccessFlags() & kAccVisibilityFlags; CHECK_EQ(actual_visibility, expected_visibility) << "Field " << name << " in class " << accessor.GetDescriptorView(); - return hiddenapi::ApiList(field.GetHiddenapiFlags()); + return hiddenapi::ApiList::FromDexFlags(field.GetHiddenapiFlags()); } } @@ -219,7 +219,7 @@ class HiddenApiTest : public CommonRuntimeTest { const uint32_t actual_visibility = method.GetAccessFlags() & kAccVisibilityFlags; CHECK_EQ(actual_visibility, expected_visibility) << "Method " << name << " in class " << accessor.GetDescriptorView(); - return hiddenapi::ApiList(method.GetHiddenapiFlags()); + return hiddenapi::ApiList::FromDexFlags(method.GetHiddenapiFlags()); } } @@ -700,8 +700,9 @@ TEST_F(HiddenApiTest, InstanceFieldCorePlatformApiMatch) { << "LMain;->ifield:I,unsupported,core-platform-api" << std::endl; auto dex_file = RunHiddenapiEncode(flags_csv, {}, dex); ASSERT_NE(dex_file.get(), nullptr); - ASSERT_EQ(hiddenapi::ApiList::CorePlatformApi() | - hiddenapi::ApiList::Unsupported(), GetIFieldHiddenFlags(*dex_file)); + ASSERT_EQ(hiddenapi::ApiList::Combine(hiddenapi::ApiList::CorePlatformApi(), + hiddenapi::ApiList::Unsupported()), + GetIFieldHiddenFlags(*dex_file)); } TEST_F(HiddenApiTest, InstanceFieldTestApiMatch) { @@ -712,8 +713,9 @@ TEST_F(HiddenApiTest, InstanceFieldTestApiMatch) { << "LMain;->ifield:I,unsupported,test-api" << std::endl; auto dex_file = RunHiddenapiEncode(flags_csv, {}, dex); ASSERT_NE(dex_file.get(), nullptr); - ASSERT_EQ(hiddenapi::ApiList::TestApi() - | hiddenapi::ApiList::Unsupported(), GetIFieldHiddenFlags(*dex_file)); + ASSERT_EQ( + hiddenapi::ApiList::Combine(hiddenapi::ApiList::TestApi(), hiddenapi::ApiList::Unsupported()), + GetIFieldHiddenFlags(*dex_file)); } TEST_F(HiddenApiTest, InstanceFieldUnknownFlagMatch) { diff --git a/tools/veridex/api_list_filter.h b/tools/veridex/api_list_filter.h index 58065db2b2..c8e6aaa378 100644 --- a/tools/veridex/api_list_filter.h +++ b/tools/veridex/api_list_filter.h @@ -17,9 +17,10 @@ #ifndef ART_TOOLS_VERIDEX_API_LIST_FILTER_H_ #define ART_TOOLS_VERIDEX_API_LIST_FILTER_H_ -#include <algorithm> #include <android-base/strings.h> +#include <set> + #include "base/hiddenapi_flags.h" namespace art { @@ -46,10 +47,10 @@ class ApiListFilter { } if (include_invalid_list) { - lists_.push_back(hiddenapi::ApiList()); + lists_.push_back(hiddenapi::ApiList::Invalid()); } for (size_t i = 0; i < hiddenapi::ApiList::kValueCount; ++i) { - hiddenapi::ApiList list = hiddenapi::ApiList(i); + hiddenapi::ApiList list = hiddenapi::ApiList::FromIntValue(i); if (exclude_set.find(list) == exclude_set.end()) { lists_.push_back(list); } diff --git a/tools/veridex/hidden_api.cc b/tools/veridex/hidden_api.cc index 4156aa954b..1e0e303c57 100644 --- a/tools/veridex/hidden_api.cc +++ b/tools/veridex/hidden_api.cc @@ -34,7 +34,7 @@ HiddenApi::HiddenApi(const char* filename, const ApiListFilter& api_list_filter) std::vector<std::string> values = android::base::Split(str, ","); const std::string& signature = values[0]; - hiddenapi::ApiList membership; + hiddenapi::ApiList membership = hiddenapi::ApiList::Invalid(); bool success = hiddenapi::ApiList::FromNames(values.begin() + 1, values.end(), &membership); if (!success) { LOG(ERROR) << "Unknown ApiList flag: " << str; diff --git a/tools/veridex/hidden_api.h b/tools/veridex/hidden_api.h index a8301743aa..12082c3c70 100644 --- a/tools/veridex/hidden_api.h +++ b/tools/veridex/hidden_api.h @@ -45,7 +45,7 @@ class HiddenApi { hiddenapi::ApiList GetApiList(const std::string& name) const { auto it = api_list_.find(name); - return (it == api_list_.end()) ? hiddenapi::ApiList() : it->second; + return (it == api_list_.end()) ? hiddenapi::ApiList::Invalid() : it->second; } bool ShouldReport(const std::string& signature) const { diff --git a/tools/veridex/veridex.cc b/tools/veridex/veridex.cc index 9c7bd78cff..ec2e96168d 100644 --- a/tools/veridex/veridex.cc +++ b/tools/veridex/veridex.cc @@ -270,9 +270,9 @@ class Veridex { os << stats.count << " hidden API(s) used: " << stats.linking_count << " linked against, " << stats.reflection_count << " through reflection" << std::endl; - DumpApiListStats(os, stats, hiddenapi::ApiList(), api_list_filter); + DumpApiListStats(os, stats, hiddenapi::ApiList::Invalid(), api_list_filter); for (size_t i = 0; i < hiddenapi::ApiList::kValueCount; ++i) { - DumpApiListStats(os, stats, hiddenapi::ApiList(i), api_list_filter); + DumpApiListStats(os, stats, hiddenapi::ApiList::FromIntValue(i), api_list_filter); } } |