diff options
37 files changed, 810 insertions, 564 deletions
diff --git a/compiler/driver/compiler_driver.cc b/compiler/driver/compiler_driver.cc index 5c649855b6..3720dda0f8 100644 --- a/compiler/driver/compiler_driver.cc +++ b/compiler/driver/compiler_driver.cc @@ -878,14 +878,6 @@ void CompilerDriver::PreCompile(jobject class_loader, TimingLogger* timings) { CheckThreadPools(); - if (kUseBitstringTypeCheck && - !compiler_options_->IsBootImage() && - compiler_options_->IsAotCompilationEnabled()) { - RecordBootImageClassesWithAssignedBitstring(); - VLOG(compiler) << "RecordBootImageClassesWithAssignedBitstring: " - << GetMemoryUsageString(false); - } - LoadImageClasses(timings); VLOG(compiler) << "LoadImageClasses: " << GetMemoryUsageString(false); @@ -954,43 +946,6 @@ void CompilerDriver::PreCompile(jobject class_loader, } } -void CompilerDriver::RecordBootImageClassesWithAssignedBitstring() { - if (boot_image_classes_with_assigned_bitstring_ != nullptr) { - return; // Already recorded. (Happens because of class unloading between dex files.) - } - - class Visitor : public ClassVisitor { - public: - explicit Visitor(std::unordered_set<mirror::Class*>* recorded_classes) - : recorded_classes_(recorded_classes) {} - - bool operator()(ObjPtr<mirror::Class> klass) OVERRIDE - REQUIRES(Locks::subtype_check_lock_) REQUIRES_SHARED(Locks::mutator_lock_) { - DCHECK(klass != nullptr); - SubtypeCheckInfo::State state = SubtypeCheck<ObjPtr<mirror::Class>>::GetState(klass); - if (state == SubtypeCheckInfo::kAssigned) { - recorded_classes_->insert(klass.Ptr()); - } - return true; - } - - private: - std::unordered_set<mirror::Class*>* const recorded_classes_; - }; - - boot_image_classes_with_assigned_bitstring_.reset(new std::unordered_set<mirror::Class*>()); - Visitor visitor(boot_image_classes_with_assigned_bitstring_.get()); - ClassLinker* class_linker = Runtime::Current()->GetClassLinker(); - ScopedObjectAccess soa(Thread::Current()); - MutexLock subtype_check_lock(soa.Self(), *Locks::subtype_check_lock_); - class_linker->VisitClasses(&visitor); -} - -bool CompilerDriver::IsBootImageClassWithAssignedBitstring(ObjPtr<mirror::Class> klass) { - DCHECK(boot_image_classes_with_assigned_bitstring_ != nullptr); - return boot_image_classes_with_assigned_bitstring_->count(klass.Ptr()) != 0u; -} - bool CompilerDriver::IsImageClass(const char* descriptor) const { if (image_classes_ != nullptr) { // If we have a set of image classes, use those. diff --git a/compiler/driver/compiler_driver.h b/compiler/driver/compiler_driver.h index fc77656761..18b1e0ea3c 100644 --- a/compiler/driver/compiler_driver.h +++ b/compiler/driver/compiler_driver.h @@ -385,17 +385,12 @@ class CompilerDriver { return dex_to_dex_compiler_; } - bool IsBootImageClassWithAssignedBitstring(ObjPtr<mirror::Class> klass) - REQUIRES_SHARED(Locks::mutator_lock_); - private: void PreCompile(jobject class_loader, const std::vector<const DexFile*>& dex_files, TimingLogger* timings) REQUIRES(!Locks::mutator_lock_); - void RecordBootImageClassesWithAssignedBitstring() REQUIRES(!Locks::mutator_lock_); - void LoadImageClasses(TimingLogger* timings) REQUIRES(!Locks::mutator_lock_); // Attempt to resolve all type, methods, fields, and strings @@ -518,12 +513,6 @@ class CompilerDriver { // This option may be restricted to the boot image, depending on a flag in the implementation. std::unique_ptr<std::unordered_set<std::string>> methods_to_compile_; - // For AOT app compilation, we keep the set of boot image classes with assigned type check - // bitstring. We need to retrieve this set before we initialize app image classes as the - // initialization can cause more boot image bitstrings to be assigned. - // Note that boot image classes are non-moveable, so it's OK to keep raw pointers. - std::unique_ptr<std::unordered_set<mirror::Class*>> boot_image_classes_with_assigned_bitstring_; - std::atomic<uint32_t> number_of_soft_verifier_failures_; bool had_hard_verifier_failure_; diff --git a/compiler/optimizing/load_store_elimination.cc b/compiler/optimizing/load_store_elimination.cc index aae94b227c..88326d321b 100644 --- a/compiler/optimizing/load_store_elimination.cc +++ b/compiler/optimizing/load_store_elimination.cc @@ -25,51 +25,6 @@ #include <iostream> -/** - * The general algorithm of load-store elimination (LSE). - * Load-store analysis in the previous pass collects a list of heap locations - * and does alias analysis of those heap locations. - * LSE keeps track of a list of heap values corresponding to the heap - * locations. It visits basic blocks in reverse post order and for - * each basic block, visits instructions sequentially, and processes - * instructions as follows: - * - If the instruction is a load, and the heap location for that load has a - * valid heap value, the load can be eliminated. In order to maintain the - * validity of all heap locations during the optimization phase, the real - * elimination is delayed till the end of LSE. - * - If the instruction is a store, it updates the heap value for the heap - * location of the store with the store instruction. The real heap value - * can be fetched from the store instruction. Heap values are invalidated - * for heap locations that may alias with the store instruction's heap - * location. The store instruction can be eliminated unless the value stored - * is later needed e.g. by a load from the same/aliased heap location or - * the heap location persists at method return/deoptimization. - * The store instruction is also needed if it's not used to track the heap - * value anymore, e.g. when it fails to merge with the heap values from other - * predecessors. - * - A store that stores the same value as the heap value is eliminated. - * - The list of heap values are merged at basic block entry from the basic - * block's predecessors. The algorithm is single-pass, so loop side-effects is - * used as best effort to decide if a heap location is stored inside the loop. - * - A special type of objects called singletons are instantiated in the method - * and have a single name, i.e. no aliases. Singletons have exclusive heap - * locations since they have no aliases. Singletons are helpful in narrowing - * down the life span of a heap location such that they do not always - * need to participate in merging heap values. Allocation of a singleton - * can be eliminated if that singleton is not used and does not persist - * at method return/deoptimization. - * - For newly instantiated instances, their heap values are initialized to - * language defined default values. - * - Some instructions such as invokes are treated as loading and invalidating - * all the heap values, depending on the instruction's side effects. - * - Finalizable objects are considered as persisting at method - * return/deoptimization. - * - Currently this LSE algorithm doesn't handle SIMD graph, e.g. with VecLoad - * and VecStore instructions. - * - Currently this LSE algorithm doesn't handle graph with try-catch, due to - * the special block merging structure. - */ - namespace art { // An unknown heap value. Loads with such a value in the heap location cannot be eliminated. @@ -104,7 +59,8 @@ class LSEVisitor : public HGraphDelegateVisitor { removed_loads_(allocator_.Adapter(kArenaAllocLSE)), substitute_instructions_for_loads_(allocator_.Adapter(kArenaAllocLSE)), possibly_removed_stores_(allocator_.Adapter(kArenaAllocLSE)), - singleton_new_instances_(allocator_.Adapter(kArenaAllocLSE)) { + singleton_new_instances_(allocator_.Adapter(kArenaAllocLSE)), + singleton_new_arrays_(allocator_.Adapter(kArenaAllocLSE)) { } void VisitBasicBlock(HBasicBlock* block) OVERRIDE { @@ -132,26 +88,19 @@ class LSEVisitor : public HGraphDelegateVisitor { return type_conversion; } - // Find an instruction's substitute if it's a removed load. + // Find an instruction's substitute if it should be removed. // Return the same instruction if it should not be removed. HInstruction* FindSubstitute(HInstruction* instruction) { - if (!IsLoad(instruction)) { - return instruction; - } size_t size = removed_loads_.size(); for (size_t i = 0; i < size; i++) { if (removed_loads_[i] == instruction) { - HInstruction* substitute = substitute_instructions_for_loads_[i]; - // The substitute list is a flat hierarchy. - DCHECK_EQ(FindSubstitute(substitute), substitute); - return substitute; + return substitute_instructions_for_loads_[i]; } } return instruction; } void AddRemovedLoad(HInstruction* load, HInstruction* heap_value) { - DCHECK(IsLoad(load)); DCHECK_EQ(FindSubstitute(heap_value), heap_value) << "Unexpected heap_value that has a substitute " << heap_value->DebugName(); removed_loads_.push_back(load); @@ -258,59 +207,28 @@ class LSEVisitor : public HGraphDelegateVisitor { new_instance->GetBlock()->RemoveInstruction(new_instance); } } - } - - private: - static bool IsLoad(HInstruction* instruction) { - if (instruction == kUnknownHeapValue || instruction == kDefaultHeapValue) { - return false; - } - // Unresolved load is not treated as a load. - return instruction->IsInstanceFieldGet() || - instruction->IsStaticFieldGet() || - instruction->IsArrayGet(); - } - - static bool IsStore(HInstruction* instruction) { - if (instruction == kUnknownHeapValue || instruction == kDefaultHeapValue) { - return false; - } - // Unresolved store is not treated as a store. - return instruction->IsInstanceFieldSet() || - instruction->IsArraySet() || - instruction->IsStaticFieldSet(); - } - - // Returns the real heap value by finding its substitute or by "peeling" - // a store instruction. - HInstruction* GetRealHeapValue(HInstruction* heap_value) { - if (IsLoad(heap_value)) { - return FindSubstitute(heap_value); - } - if (!IsStore(heap_value)) { - return heap_value; - } + for (HInstruction* new_array : singleton_new_arrays_) { + size_t removed = HConstructorFence::RemoveConstructorFences(new_array); + MaybeRecordStat(stats_, + MethodCompilationStat::kConstructorFenceRemovedLSE, + removed); - // We keep track of store instructions as the heap values which might be - // eliminated if the stores are later found not necessary. The real stored - // value needs to be fetched from the store instruction. - if (heap_value->IsInstanceFieldSet()) { - heap_value = heap_value->AsInstanceFieldSet()->GetValue(); - } else if (heap_value->IsStaticFieldSet()) { - heap_value = heap_value->AsStaticFieldSet()->GetValue(); - } else { - DCHECK(heap_value->IsArraySet()); - heap_value = heap_value->AsArraySet()->GetValue(); + if (!new_array->HasNonEnvironmentUses()) { + new_array->RemoveEnvironmentUsers(); + new_array->GetBlock()->RemoveInstruction(new_array); + } } - // heap_value may already be a removed load. - return FindSubstitute(heap_value); } - // If heap_value is a store, need to keep the store. - // This is necessary if a heap value is killed or replaced by another value, - // so that the store is no longer used to track heap value. + private: + // If heap_values[index] is an instance field store, need to keep the store. + // This is necessary if a heap value is killed due to merging, or loop side + // effects (which is essentially merging also), since a load later from the + // location won't be eliminated. void KeepIfIsStore(HInstruction* heap_value) { - if (!IsStore(heap_value)) { + if (heap_value == kDefaultHeapValue || + heap_value == kUnknownHeapValue || + !(heap_value->IsInstanceFieldSet() || heap_value->IsArraySet())) { return; } auto idx = std::find(possibly_removed_stores_.begin(), @@ -321,41 +239,26 @@ class LSEVisitor : public HGraphDelegateVisitor { } } - // If a heap location X may alias with heap location at `loc_index` - // and heap_values of that heap location X holds a store, keep that store. - // It's needed for a dependent load that's not eliminated since any store - // that may put value into the load's heap location needs to be kept. - void KeepStoresIfAliasedToLocation(ScopedArenaVector<HInstruction*>& heap_values, - size_t loc_index) { - for (size_t i = 0; i < heap_values.size(); i++) { - if ((i == loc_index) || heap_location_collector_.MayAlias(i, loc_index)) { - KeepIfIsStore(heap_values[i]); - } - } - } - void HandleLoopSideEffects(HBasicBlock* block) { DCHECK(block->IsLoopHeader()); int block_id = block->GetBlockId(); ScopedArenaVector<HInstruction*>& heap_values = heap_values_for_[block_id]; - HBasicBlock* pre_header = block->GetLoopInformation()->GetPreHeader(); - ScopedArenaVector<HInstruction*>& pre_header_heap_values = - heap_values_for_[pre_header->GetBlockId()]; - // Don't eliminate loads in irreducible loops. - // Also keep the stores before the loop. + // Don't eliminate loads in irreducible loops. This is safe for singletons, because + // they are always used by the non-eliminated loop-phi. if (block->GetLoopInformation()->IsIrreducible()) { if (kIsDebugBuild) { for (size_t i = 0; i < heap_values.size(); i++) { DCHECK_EQ(heap_values[i], kUnknownHeapValue); } } - for (size_t i = 0; i < heap_values.size(); i++) { - KeepIfIsStore(pre_header_heap_values[i]); - } return; } + HBasicBlock* pre_header = block->GetLoopInformation()->GetPreHeader(); + ScopedArenaVector<HInstruction*>& pre_header_heap_values = + heap_values_for_[pre_header->GetBlockId()]; + // Inherit the values from pre-header. for (size_t i = 0; i < heap_values.size(); i++) { heap_values[i] = pre_header_heap_values[i]; @@ -367,17 +270,18 @@ class LSEVisitor : public HGraphDelegateVisitor { for (size_t i = 0; i < heap_values.size(); i++) { HeapLocation* location = heap_location_collector_.GetHeapLocation(i); ReferenceInfo* ref_info = location->GetReferenceInfo(); - if (ref_info->IsSingleton() && !location->IsValueKilledByLoopSideEffects()) { - // A singleton's field that's not stored into inside a loop is + if (ref_info->IsSingletonAndRemovable() && + !location->IsValueKilledByLoopSideEffects()) { + // A removable singleton's field that's not stored into inside a loop is // invariant throughout the loop. Nothing to do. } else { - // heap value is killed by loop side effects. + // heap value is killed by loop side effects (stored into directly, or + // due to aliasing). Or the heap value may be needed after method return + // or deoptimization. KeepIfIsStore(pre_header_heap_values[i]); heap_values[i] = kUnknownHeapValue; } } - } else { - // The loop doesn't kill any value. } } @@ -396,73 +300,45 @@ class LSEVisitor : public HGraphDelegateVisitor { ScopedArenaVector<HInstruction*>& heap_values = heap_values_for_[block->GetBlockId()]; for (size_t i = 0; i < heap_values.size(); i++) { HInstruction* merged_value = nullptr; - // If we can merge the store itself from the predecessors, we keep - // the store as the heap value as long as possible. In case we cannot - // merge the store, we try to merge the values of the stores. - HInstruction* merged_store_value = nullptr; // Whether merged_value is a result that's merged from all predecessors. bool from_all_predecessors = true; ReferenceInfo* ref_info = heap_location_collector_.GetHeapLocation(i)->GetReferenceInfo(); - HInstruction* ref = ref_info->GetReference(); HInstruction* singleton_ref = nullptr; if (ref_info->IsSingleton()) { - // We do more analysis based on singleton's liveness when merging - // heap values for such cases. - singleton_ref = ref; + // We do more analysis of liveness when merging heap values for such + // cases since stores into such references may potentially be eliminated. + singleton_ref = ref_info->GetReference(); } for (HBasicBlock* predecessor : predecessors) { HInstruction* pred_value = heap_values_for_[predecessor->GetBlockId()][i]; - if (!IsStore(pred_value)) { - pred_value = FindSubstitute(pred_value); - } - DCHECK(pred_value != nullptr); - HInstruction* pred_store_value = GetRealHeapValue(pred_value); if ((singleton_ref != nullptr) && !singleton_ref->GetBlock()->Dominates(predecessor)) { - // singleton_ref is not live in this predecessor. No need to merge - // since singleton_ref is not live at the beginning of this block. + // singleton_ref is not live in this predecessor. Skip this predecessor since + // it does not really have the location. DCHECK_EQ(pred_value, kUnknownHeapValue); from_all_predecessors = false; - break; + continue; } if (merged_value == nullptr) { // First seen heap value. - DCHECK(pred_value != nullptr); merged_value = pred_value; } else if (pred_value != merged_value) { // There are conflicting values. merged_value = kUnknownHeapValue; - // We may still be able to merge store values. - } - - // Conflicting stores may be storing the same value. We do another merge - // of real stored values. - if (merged_store_value == nullptr) { - // First seen store value. - DCHECK(pred_store_value != nullptr); - merged_store_value = pred_store_value; - } else if (pred_store_value != merged_store_value) { - // There are conflicting store values. - merged_store_value = kUnknownHeapValue; - // There must be conflicting stores also. - DCHECK_EQ(merged_value, kUnknownHeapValue); - // No need to merge anymore. break; } } - if (merged_value == nullptr) { - DCHECK(!from_all_predecessors); - DCHECK(singleton_ref != nullptr); - } - if (from_all_predecessors) { - if (ref_info->IsSingletonAndRemovable() && - block->IsSingleReturnOrReturnVoidAllowingPhis()) { - // Values in the singleton are not needed anymore. - } else if (!IsStore(merged_value)) { - // We don't track merged value as a store anymore. We have to - // hold the stores in predecessors live here. + if (ref_info->IsSingleton()) { + if (ref_info->IsSingletonAndNonRemovable() || + (merged_value == kUnknownHeapValue && + !block->IsSingleReturnOrReturnVoidAllowingPhis())) { + // The heap value may be needed after method return or deoptimization, + // or there are conflicting heap values from different predecessors and + // this block is not a single return, + // keep the last store in each predecessor since future loads may not + // be eliminated. for (HBasicBlock* predecessor : predecessors) { ScopedArenaVector<HInstruction*>& pred_values = heap_values_for_[predecessor->GetBlockId()]; @@ -470,33 +346,18 @@ class LSEVisitor : public HGraphDelegateVisitor { } } } else { - DCHECK(singleton_ref != nullptr); - // singleton_ref is non-existing at the beginning of the block. There is - // no need to keep the stores. + // Currenctly we don't eliminate stores to non-singletons. } - if (!from_all_predecessors) { + if ((merged_value == nullptr) || !from_all_predecessors) { DCHECK(singleton_ref != nullptr); DCHECK((singleton_ref->GetBlock() == block) || - !singleton_ref->GetBlock()->Dominates(block)) - << "method: " << GetGraph()->GetMethodName(); + !singleton_ref->GetBlock()->Dominates(block)); // singleton_ref is not defined before block or defined only in some of its // predecessors, so block doesn't really have the location at its entry. heap_values[i] = kUnknownHeapValue; - } else if (predecessors.size() == 1) { - // Inherit heap value from the single predecessor. - DCHECK_EQ(heap_values_for_[predecessors[0]->GetBlockId()][i], merged_value); - heap_values[i] = merged_value; } else { - DCHECK(merged_value == kUnknownHeapValue || - merged_value == kDefaultHeapValue || - merged_value->GetBlock()->Dominates(block)); - if (merged_value != kUnknownHeapValue) { - heap_values[i] = merged_value; - } else { - // Stores in different predecessors may be storing the same value. - heap_values[i] = merged_store_value; - } + heap_values[i] = merged_value; } } } @@ -562,12 +423,23 @@ class LSEVisitor : public HGraphDelegateVisitor { heap_values[idx] = constant; return; } - heap_value = GetRealHeapValue(heap_value); + if (heap_value != kUnknownHeapValue) { + if (heap_value->IsInstanceFieldSet() || heap_value->IsArraySet()) { + HInstruction* store = heap_value; + // This load must be from a singleton since it's from the same + // field/element that a "removed" store puts the value. That store + // must be to a singleton's field/element. + DCHECK(ref_info->IsSingleton()); + // Get the real heap value of the store. + heap_value = heap_value->IsInstanceFieldSet() ? store->InputAt(1) : store->InputAt(2); + // heap_value may already have a substitute. + heap_value = FindSubstitute(heap_value); + } + } if (heap_value == kUnknownHeapValue) { // Load isn't eliminated. Put the load as the value into the HeapLocation. // This acts like GVN but with better aliasing analysis. heap_values[idx] = instruction; - KeepStoresIfAliasedToLocation(heap_values, idx); } else { if (DataType::Kind(heap_value->GetType()) != DataType::Kind(instruction->GetType())) { // The only situation where the same heap location has different type is when @@ -580,10 +452,6 @@ class LSEVisitor : public HGraphDelegateVisitor { DCHECK(heap_value->IsArrayGet()) << heap_value->DebugName(); DCHECK(instruction->IsArrayGet()) << instruction->DebugName(); } - // Load isn't eliminated. Put the load as the value into the HeapLocation. - // This acts like GVN but with better aliasing analysis. - heap_values[idx] = instruction; - KeepStoresIfAliasedToLocation(heap_values, idx); return; } AddRemovedLoad(instruction, heap_value); @@ -592,21 +460,12 @@ class LSEVisitor : public HGraphDelegateVisitor { } bool Equal(HInstruction* heap_value, HInstruction* value) { - DCHECK(!IsStore(value)) << value->DebugName(); - if (heap_value == kUnknownHeapValue) { - // Don't compare kUnknownHeapValue with other values. - return false; - } if (heap_value == value) { return true; } if (heap_value == kDefaultHeapValue && GetDefaultValue(value->GetType()) == value) { return true; } - HInstruction* real_heap_value = GetRealHeapValue(heap_value); - if (real_heap_value != heap_value) { - return Equal(real_heap_value, value); - } return false; } @@ -617,7 +476,6 @@ class LSEVisitor : public HGraphDelegateVisitor { size_t vector_length, int16_t declaring_class_def_index, HInstruction* value) { - DCHECK(!IsStore(value)) << value->DebugName(); // value may already have a substitute. value = FindSubstitute(value); HInstruction* original_ref = heap_location_collector_.HuntForOriginalReference(ref); @@ -628,47 +486,59 @@ class LSEVisitor : public HGraphDelegateVisitor { ScopedArenaVector<HInstruction*>& heap_values = heap_values_for_[instruction->GetBlock()->GetBlockId()]; HInstruction* heap_value = heap_values[idx]; + bool same_value = false; bool possibly_redundant = false; - if (Equal(heap_value, value)) { // Store into the heap location with the same value. - // This store can be eliminated right away. - instruction->GetBlock()->RemoveInstruction(instruction); - return; - } else { + same_value = true; + } else if (index != nullptr && + heap_location_collector_.GetHeapLocation(idx)->HasAliasedLocations()) { + // For array element, don't eliminate stores if the location can be aliased + // (due to either ref or index aliasing). + } else if (ref_info->IsSingleton()) { + // Store into a field/element of a singleton. The value cannot be killed due to + // aliasing/invocation. It can be redundant since future loads can + // directly get the value set by this instruction. The value can still be killed due to + // merging or loop side effects. Stores whose values are killed due to merging/loop side + // effects later will be removed from possibly_removed_stores_ when that is detected. + // Stores whose values may be needed after method return or deoptimization + // are also removed from possibly_removed_stores_ when that is detected. + possibly_redundant = true; HLoopInformation* loop_info = instruction->GetBlock()->GetLoopInformation(); - if (loop_info == nullptr) { - // Store is not in a loop. We try to precisely track the heap value by - // the store. - possibly_redundant = true; - } else if (!loop_info->IsIrreducible()) { - // instruction is a store in the loop so the loop must do write. + if (loop_info != nullptr) { + // instruction is a store in the loop so the loop must does write. DCHECK(side_effects_.GetLoopEffects(loop_info->GetHeader()).DoesAnyWrite()); - if (ref_info->IsSingleton() && !loop_info->IsDefinedOutOfTheLoop(original_ref)) { - // original_ref is created inside the loop. Value stored to it isn't needed at - // the loop header. This is true for outer loops also. - possibly_redundant = true; - } else { + + if (loop_info->IsDefinedOutOfTheLoop(original_ref)) { + DCHECK(original_ref->GetBlock()->Dominates(loop_info->GetPreHeader())); // Keep the store since its value may be needed at the loop header. + possibly_redundant = false; + } else { + // The singleton is created inside the loop. Value stored to it isn't needed at + // the loop header. This is true for outer loops also. } - } else { - // Keep the store inside irreducible loops. } } - if (possibly_redundant) { + if (same_value || possibly_redundant) { possibly_removed_stores_.push_back(instruction); } - // Put the store as the heap value. If the value is loaded or needed after - // return/deoptimization later, this store isn't really redundant. - heap_values[idx] = instruction; - + if (!same_value) { + if (possibly_redundant) { + DCHECK(instruction->IsInstanceFieldSet() || instruction->IsArraySet()); + // Put the store as the heap value. If the value is loaded from heap + // by a load later, this store isn't really redundant. + heap_values[idx] = instruction; + } else { + heap_values[idx] = value; + } + } // This store may kill values in other heap locations due to aliasing. for (size_t i = 0; i < heap_values.size(); i++) { if (i == idx) { continue; } - if (Equal(heap_values[i], value)) { + if (heap_values[i] == value) { // Same value should be kept even if aliasing happens. continue; } @@ -677,9 +547,7 @@ class LSEVisitor : public HGraphDelegateVisitor { continue; } if (heap_location_collector_.MayAlias(i, idx)) { - // Kill heap locations that may alias and as a result if the heap value - // is a store, the store needs to be kept. - KeepIfIsStore(heap_values[i]); + // Kill heap locations that may alias. heap_values[i] = kUnknownHeapValue; } } @@ -765,35 +633,24 @@ class LSEVisitor : public HGraphDelegateVisitor { const ScopedArenaVector<HInstruction*>& heap_values = heap_values_for_[instruction->GetBlock()->GetBlockId()]; for (HInstruction* heap_value : heap_values) { + // Filter out fake instructions before checking instruction kind below. + if (heap_value == kUnknownHeapValue || heap_value == kDefaultHeapValue) { + continue; + } // A store is kept as the heap value for possibly removed stores. - // That value stored is generally observeable after deoptimization, except - // for singletons that don't escape after deoptimization. - if (IsStore(heap_value)) { - if (heap_value->IsStaticFieldSet()) { - KeepIfIsStore(heap_value); - continue; - } + if (heap_value->IsInstanceFieldSet() || heap_value->IsArraySet()) { + // Check whether the reference for a store is used by an environment local of + // HDeoptimize. HInstruction* reference = heap_value->InputAt(0); - if (heap_location_collector_.FindReferenceInfoOf(reference)->IsSingleton()) { - if (reference->IsNewInstance() && reference->AsNewInstance()->IsFinalizable()) { - // Finalizable objects alway escape. + DCHECK(heap_location_collector_.FindReferenceInfoOf(reference)->IsSingleton()); + for (const HUseListNode<HEnvironment*>& use : reference->GetEnvUses()) { + HEnvironment* user = use.GetUser(); + if (user->GetHolder() == instruction) { + // The singleton for the store is visible at this deoptimization + // point. Need to keep the store so that the heap value is + // seen by the interpreter. KeepIfIsStore(heap_value); - continue; } - // Check whether the reference for a store is used by an environment local of - // HDeoptimize. If not, the singleton is not observed after - // deoptimizion. - for (const HUseListNode<HEnvironment*>& use : reference->GetEnvUses()) { - HEnvironment* user = use.GetUser(); - if (user->GetHolder() == instruction) { - // The singleton for the store is visible at this deoptimization - // point. Need to keep the store so that the heap value is - // seen by the interpreter. - KeepIfIsStore(heap_value); - } - } - } else { - KeepIfIsStore(heap_value); } } } @@ -901,7 +758,7 @@ class LSEVisitor : public HGraphDelegateVisitor { return; } if (ref_info->IsSingletonAndRemovable()) { - singleton_new_instances_.push_back(new_array); + singleton_new_arrays_.push_back(new_array); } ScopedArenaVector<HInstruction*>& heap_values = heap_values_for_[new_array->GetBlock()->GetBlockId()]; @@ -934,6 +791,7 @@ class LSEVisitor : public HGraphDelegateVisitor { ScopedArenaVector<HInstruction*> possibly_removed_stores_; ScopedArenaVector<HInstruction*> singleton_new_instances_; + ScopedArenaVector<HInstruction*> singleton_new_arrays_; DISALLOW_COPY_AND_ASSIGN(LSEVisitor); }; diff --git a/compiler/optimizing/sharpening.cc b/compiler/optimizing/sharpening.cc index 12319df2e8..dffef17587 100644 --- a/compiler/optimizing/sharpening.cc +++ b/compiler/optimizing/sharpening.cc @@ -253,9 +253,9 @@ static inline bool CanUseTypeCheckBitstring(ObjPtr<mirror::Class> klass, // If the target is a boot image class, try to assign a type check bitstring (fall through). // (If --force-determinism, this was already done; repeating is OK and yields the same result.) } else { - // For AOT app compilation we can use the bitstring iff the target class is - // a boot image class with a bitstring already assigned in the boot image. - return compiler_driver->IsBootImageClassWithAssignedBitstring(klass); + // TODO: Use the bitstring also for AOT app compilation if the target class has a bitstring + // already assigned in the boot image. + return false; } // Try to assign a type check bitstring. diff --git a/dex2oat/dex2oat.cc b/dex2oat/dex2oat.cc index a836d75a72..8555abf9fd 100644 --- a/dex2oat/dex2oat.cc +++ b/dex2oat/dex2oat.cc @@ -2404,7 +2404,7 @@ class Dex2Oat FINAL { bool AddDexFileSources() { TimingLogger::ScopedTiming t2("AddDexFileSources", timings_); - if (input_vdex_file_ != nullptr) { + if (input_vdex_file_ != nullptr && input_vdex_file_->HasDexSection()) { DCHECK_EQ(oat_writers_.size(), 1u); const std::string& name = zip_location_.empty() ? dex_locations_[0] : zip_location_; DCHECK(!name.empty()); diff --git a/profman/profman.cc b/profman/profman.cc index 9f3e3b6ac5..ffc3c0170f 100644 --- a/profman/profman.cc +++ b/profman/profman.cc @@ -149,6 +149,10 @@ NO_RETURN static void Usage(const char *fmt, ...) { UsageError(" --boot-image-sampled-method-threshold=<value>: minimum number of profiles a"); UsageError(" non-hot method needs to be in order to be hot in the output profile. The"); UsageError(" default is max int."); + UsageError(" --copy-and-update-profile-key: if present, profman will copy the profile from"); + UsageError(" the file passed with --profile-fd(file) to the profile passed with"); + UsageError(" --reference-profile-fd(file) and update at the same time the profile-key"); + UsageError(" of entries corresponding to the apks passed with --apk(-fd)."); UsageError(""); exit(EXIT_FAILURE); @@ -186,7 +190,8 @@ class ProfMan FINAL { test_profile_method_percerntage_(kDefaultTestProfileMethodPercentage), test_profile_class_percentage_(kDefaultTestProfileClassPercentage), test_profile_seed_(NanoTime()), - start_ns_(NanoTime()) {} + start_ns_(NanoTime()), + copy_and_update_profile_key_(false) {} ~ProfMan() { LogCompletionTime(); @@ -302,11 +307,13 @@ class ProfMan FINAL { "should only be used together"); } ProfileAssistant::ProcessingResult result; + if (profile_files_.empty()) { // The file doesn't need to be flushed here (ProcessProfiles will do it) // so don't check the usage. File file(reference_profile_file_fd_, false); - result = ProfileAssistant::ProcessProfiles(profile_files_fd_, reference_profile_file_fd_); + result = ProfileAssistant::ProcessProfiles(profile_files_fd_, + reference_profile_file_fd_); CloseAllFds(profile_files_fd_, "profile_files_fd_"); } else { result = ProfileAssistant::ProcessProfiles(profile_files_, reference_profile_file_); @@ -314,7 +321,7 @@ class ProfMan FINAL { return result; } - void OpenApkFilesFromLocations(std::vector<std::unique_ptr<const DexFile>>* dex_files) { + void OpenApkFilesFromLocations(std::vector<std::unique_ptr<const DexFile>>* dex_files) const { bool use_apk_fd_list = !apks_fd_.empty(); if (use_apk_fd_list) { // Get the APKs from the collection of FDs. @@ -1070,6 +1077,42 @@ class ProfMan FINAL { return !test_profile_.empty(); } + bool ShouldCopyAndUpdateProfileKey() const { + return copy_and_update_profile_key_; + } + + bool CopyAndUpdateProfileKey() const { + // Validate that at least one profile file was passed, as well as a reference profile. + if (!(profile_files_.size() == 1 ^ profile_files_fd_.size() == 1)) { + Usage("Only one profile file should be specified."); + } + if (reference_profile_file_.empty() && !FdIsValid(reference_profile_file_fd_)) { + Usage("No reference profile file specified."); + } + + if (apk_files_.empty() && apks_fd_.empty()) { + Usage("No apk files specified"); + } + + bool use_fds = profile_files_fd_.size() == 1; + + ProfileCompilationInfo profile; + // Do not clear if invalid. The input might be an archive. + if (profile.Load(profile_files_[0], /*clear_if_invalid*/ false)) { + // Open the dex files to look up classes and methods. + std::vector<std::unique_ptr<const DexFile>> dex_files; + OpenApkFilesFromLocations(&dex_files); + if (!profile.UpdateProfileKeys(dex_files)) { + return false; + } + return use_fds + ? profile.Save(reference_profile_file_fd_) + : profile.Save(reference_profile_file_, /*bytes_written*/ nullptr); + } else { + return false; + } + } + private: static void ParseFdForCollection(const StringPiece& option, const char* arg_name, @@ -1114,6 +1157,7 @@ class ProfMan FINAL { uint16_t test_profile_class_percentage_; uint32_t test_profile_seed_; uint64_t start_ns_; + bool copy_and_update_profile_key_; }; // See ProfileAssistant::ProcessingResult for return codes. diff --git a/runtime/entrypoints/quick/quick_throw_entrypoints.cc b/runtime/entrypoints/quick/quick_throw_entrypoints.cc index db4891ebd8..4b26beece3 100644 --- a/runtime/entrypoints/quick/quick_throw_entrypoints.cc +++ b/runtime/entrypoints/quick/quick_throw_entrypoints.cc @@ -126,14 +126,6 @@ extern "C" NO_RETURN void artThrowClassCastException(mirror::Class* dest_type, dex::TypeIndex type_index(check_cast.VRegB_21c()); ClassLinker* linker = Runtime::Current()->GetClassLinker(); dest_type = linker->LookupResolvedType(type_index, visitor.caller).Ptr(); - if (UNLIKELY(dest_type == nullptr)) { - // This class must have been resolved to the boot image at AOT compile time - // but it's not yet resolved in the app's class loader. Just look it up in - // the boot class path loader. - DCHECK(visitor.caller->GetClassLoader() != nullptr); - dest_type = linker->LookupResolvedType( - type_index, visitor.caller->GetDexCache(), /* class_loader */ nullptr).Ptr(); - } CHECK(dest_type != nullptr) << "Target class should have been previously resolved: " << visitor.caller->GetDexFile()->PrettyType(type_index); } diff --git a/runtime/gc/heap.h b/runtime/gc/heap.h index 57d3d506f0..faa6195259 100644 --- a/runtime/gc/heap.h +++ b/runtime/gc/heap.h @@ -1293,10 +1293,6 @@ class Heap { // Parallel GC data structures. std::unique_ptr<ThreadPool> thread_pool_; - // Estimated allocation rate (bytes / second). Computed between the time of the last GC cycle - // and the start of the current one. - uint64_t allocation_rate_; - // For a GC cycle, a bitmap that is set corresponding to the std::unique_ptr<accounting::HeapBitmap> live_bitmap_ GUARDED_BY(Locks::heap_bitmap_lock_); std::unique_ptr<accounting::HeapBitmap> mark_bitmap_ GUARDED_BY(Locks::heap_bitmap_lock_); diff --git a/runtime/hidden_api.h b/runtime/hidden_api.h new file mode 100644 index 0000000000..de3a51a2ac --- /dev/null +++ b/runtime/hidden_api.h @@ -0,0 +1,170 @@ +/* + * Copyright (C) 2018 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef ART_RUNTIME_HIDDEN_API_H_ +#define ART_RUNTIME_HIDDEN_API_H_ + +#include "hidden_api_access_flags.h" +#include "reflection.h" +#include "runtime.h" + +namespace art { +namespace hiddenapi { + +// Returns true if member with `access flags` should only be accessed from +// boot class path. +inline bool IsMemberHidden(uint32_t access_flags) { + if (!Runtime::Current()->AreHiddenApiChecksEnabled()) { + return false; + } + + switch (HiddenApiAccessFlags::DecodeFromRuntime(access_flags)) { + case HiddenApiAccessFlags::kWhitelist: + case HiddenApiAccessFlags::kLightGreylist: + case HiddenApiAccessFlags::kDarkGreylist: + return false; + case HiddenApiAccessFlags::kBlacklist: + return true; + } +} + +// Returns true if we should warn about non-boot class path accessing member +// with `access_flags`. +inline bool ShouldWarnAboutMember(uint32_t access_flags) { + if (!Runtime::Current()->AreHiddenApiChecksEnabled()) { + return false; + } + + switch (HiddenApiAccessFlags::DecodeFromRuntime(access_flags)) { + case HiddenApiAccessFlags::kWhitelist: + return false; + case HiddenApiAccessFlags::kLightGreylist: + case HiddenApiAccessFlags::kDarkGreylist: + return true; + case HiddenApiAccessFlags::kBlacklist: + // We should never access a blacklisted member from non-boot class path, + // but this function is called before we establish the origin of the access. + // Return false here, we do not want to warn when boot class path accesses + // a blacklisted member. + return false; + } +} + +// Returns true if caller `num_frames` up the stack is in boot class path. +inline bool IsCallerInBootClassPath(Thread* self, size_t num_frames) + REQUIRES_SHARED(Locks::mutator_lock_) { + ObjPtr<mirror::Class> klass = GetCallingClass(self, num_frames); + if (klass == nullptr) { + // Unattached native thread. Assume that this is *not* boot class path. + return false; + } + return klass->IsBootStrapClassLoaded(); +} + +// Returns true if `caller` should not be allowed to access member with `access_flags`. +inline bool ShouldBlockAccessToMember(uint32_t access_flags, mirror::Class* caller) + REQUIRES_SHARED(Locks::mutator_lock_) { + return IsMemberHidden(access_flags) && + !caller->IsBootStrapClassLoaded(); +} + +// Returns true if `caller` should not be allowed to access `member`. +template<typename T> +inline bool ShouldBlockAccessToMember(T* member, ArtMethod* caller) + REQUIRES_SHARED(Locks::mutator_lock_) { + DCHECK(member != nullptr); + DCHECK(!caller->IsRuntimeMethod()); + return ShouldBlockAccessToMember(member->GetAccessFlags(), caller->GetDeclaringClass()); +} + +// Returns true if the caller `num_frames` up the stack should not be allowed +// to access `member`. +template<typename T> +inline bool ShouldBlockAccessToMember(T* member, Thread* self, size_t num_frames) + REQUIRES_SHARED(Locks::mutator_lock_) { + DCHECK(member != nullptr); + return IsMemberHidden(member->GetAccessFlags()) && + !IsCallerInBootClassPath(self, num_frames); // This is expensive. Save it for last. +} + +// Issue a warning about field access. +inline void WarnAboutMemberAccess(ArtField* field) REQUIRES_SHARED(Locks::mutator_lock_) { + Runtime::Current()->SetPendingHiddenApiWarning(true); + LOG(WARNING) << "Access to hidden field " << field->PrettyField(); +} + +// Issue a warning about method access. +inline void WarnAboutMemberAccess(ArtMethod* method) REQUIRES_SHARED(Locks::mutator_lock_) { + Runtime::Current()->SetPendingHiddenApiWarning(true); + LOG(WARNING) << "Access to hidden method " << method->PrettyMethod(); +} + +// Set access flags of `member` to be in hidden API whitelist. This can be disabled +// with a Runtime::SetDedupHiddenApiWarnings. +template<typename T> +inline void MaybeWhitelistMember(T* member) REQUIRES_SHARED(Locks::mutator_lock_) { + if (Runtime::Current()->ShouldDedupeHiddenApiWarnings()) { + member->SetAccessFlags(HiddenApiAccessFlags::EncodeForRuntime( + member->GetAccessFlags(), HiddenApiAccessFlags::kWhitelist)); + DCHECK(!ShouldWarnAboutMember(member->GetAccessFlags())); + } +} + +// Check if `caller` should be allowed to access `member` and warn if not. +template<typename T> +inline void MaybeWarnAboutMemberAccess(T* member, ArtMethod* caller) + REQUIRES_SHARED(Locks::mutator_lock_) { + DCHECK(member != nullptr); + DCHECK(!caller->IsRuntimeMethod()); + if (!Runtime::Current()->AreHiddenApiChecksEnabled() || + member == nullptr || + !ShouldWarnAboutMember(member->GetAccessFlags()) || + caller->GetDeclaringClass()->IsBootStrapClassLoaded()) { + return; + } + + WarnAboutMember(member); + MaybeWhitelistMember(member); +} + +// Check if the caller `num_frames` up the stack should be allowed to access +// `member` and warn if not. +template<typename T> +inline void MaybeWarnAboutMemberAccess(T* member, Thread* self, size_t num_frames) + REQUIRES_SHARED(Locks::mutator_lock_) { + if (!Runtime::Current()->AreHiddenApiChecksEnabled() || + member == nullptr || + !ShouldWarnAboutMember(member->GetAccessFlags())) { + return; + } + + // Walk the stack to find the caller. This is *very* expensive. Save it for last. + ObjPtr<mirror::Class> klass = GetCallingClass(self, num_frames); + if (klass == nullptr) { + // Unattached native thread, assume that this is *not* boot class path + // and enforce the rules. + } else if (klass->IsBootStrapClassLoaded()) { + return; + } + + WarnAboutMemberAccess(member); + MaybeWhitelistMember(member); +} + +} // namespace hiddenapi +} // namespace art + +#endif // ART_RUNTIME_HIDDEN_API_H_ diff --git a/runtime/interpreter/unstarted_runtime.cc b/runtime/interpreter/unstarted_runtime.cc index d1436fa9cf..b9b00519d1 100644 --- a/runtime/interpreter/unstarted_runtime.cc +++ b/runtime/interpreter/unstarted_runtime.cc @@ -38,6 +38,7 @@ #include "entrypoints/entrypoint_utils-inl.h" #include "gc/reference_processor.h" #include "handle_scope-inl.h" +#include "hidden_api.h" #include "interpreter/interpreter_common.h" #include "jvalue-inl.h" #include "mirror/array-inl.h" @@ -265,7 +266,11 @@ void UnstartedRuntime::UnstartedClassNewInstance( bool ok = false; auto* cl = Runtime::Current()->GetClassLinker(); if (cl->EnsureInitialized(self, h_klass, true, true)) { - auto* cons = h_klass->FindConstructor("()V", cl->GetImagePointerSize()); + ArtMethod* cons = h_klass->FindConstructor("()V", cl->GetImagePointerSize()); + if (cons != nullptr && + hiddenapi::ShouldBlockAccessToMember(cons, shadow_frame->GetMethod())) { + cons = nullptr; + } if (cons != nullptr) { Handle<mirror::Object> h_obj(hs.NewHandle(klass->AllocObject(self))); CHECK(h_obj != nullptr); // We don't expect OOM at compile-time. @@ -308,6 +313,10 @@ void UnstartedRuntime::UnstartedClassGetDeclaredField( } } } + if (found != nullptr && + hiddenapi::ShouldBlockAccessToMember(found, shadow_frame->GetMethod())) { + found = nullptr; + } if (found == nullptr) { AbortTransactionOrFail(self, "Failed to find field in Class.getDeclaredField in un-started " " runtime. name=%s class=%s", name2->ToModifiedUtf8().c_str(), @@ -370,6 +379,10 @@ void UnstartedRuntime::UnstartedClassGetDeclaredMethod( self, klass, name, args); } } + if (method != nullptr && + hiddenapi::ShouldBlockAccessToMember(method->GetArtMethod(), shadow_frame->GetMethod())) { + method = nullptr; + } result->SetL(method); } @@ -404,6 +417,11 @@ void UnstartedRuntime::UnstartedClassGetDeclaredConstructor( false>(self, klass, args); } } + if (constructor != nullptr && + hiddenapi::ShouldBlockAccessToMember( + constructor->GetArtMethod(), shadow_frame->GetMethod())) { + constructor = nullptr; + } result->SetL(constructor); } diff --git a/runtime/jit/profile_compilation_info.cc b/runtime/jit/profile_compilation_info.cc index 33fa0d6a26..4bf2895723 100644 --- a/runtime/jit/profile_compilation_info.cc +++ b/runtime/jit/profile_compilation_info.cc @@ -2030,4 +2030,28 @@ bool ProfileCompilationInfo::IsProfileFile(int fd) { return memcmp(buffer, kProfileMagic, byte_count) == 0; } +bool ProfileCompilationInfo::UpdateProfileKeys( + const std::vector<std::unique_ptr<const DexFile>>& dex_files) { + for (const std::unique_ptr<const DexFile>& dex_file : dex_files) { + for (DexFileData* dex_data : info_) { + if (dex_data->checksum == dex_file->GetLocationChecksum() + && dex_data->num_method_ids == dex_file->NumMethodIds()) { + std::string new_profile_key = GetProfileDexFileKey(dex_file->GetLocation()); + if (dex_data->profile_key != new_profile_key) { + if (profile_key_map_.find(new_profile_key) != profile_key_map_.end()) { + // We can't update the key if the new key belongs to a different dex file. + LOG(ERROR) << "Cannot update profile key to " << new_profile_key + << " because the new key belongs to another dex file."; + return false; + } + profile_key_map_.erase(dex_data->profile_key); + profile_key_map_.Put(new_profile_key, dex_data->profile_index); + dex_data->profile_key = new_profile_key; + } + } + } + } + return true; +} + } // namespace art diff --git a/runtime/jit/profile_compilation_info.h b/runtime/jit/profile_compilation_info.h index 29a4c115fa..350ce9ed8d 100644 --- a/runtime/jit/profile_compilation_info.h +++ b/runtime/jit/profile_compilation_info.h @@ -416,6 +416,17 @@ class ProfileCompilationInfo { // Return true if the fd points to a profile file. bool IsProfileFile(int fd); + // Update the profile keys corresponding to the given dex files based on their current paths. + // This method allows fix-ups in the profile for dex files that might have been renamed. + // The new profile key will be constructed based on the current dex location. + // + // The matching [profile key <-> dex_file] is done based on the dex checksum and the number of + // methods ids. If neither is a match then the profile key is not updated. + // + // If the new profile key would collide with an existing key (for a different dex) + // the method returns false. Otherwise it returns true. + bool UpdateProfileKeys(const std::vector<std::unique_ptr<const DexFile>>& dex_files); + private: enum ProfileLoadStatus { kProfileLoadWouldOverwiteData, diff --git a/runtime/jit/profile_compilation_info_test.cc b/runtime/jit/profile_compilation_info_test.cc index 55989d8c52..b4265d1a28 100644 --- a/runtime/jit/profile_compilation_info_test.cc +++ b/runtime/jit/profile_compilation_info_test.cc @@ -22,6 +22,7 @@ #include "class_linker-inl.h" #include "common_runtime_test.h" #include "dex/dex_file.h" +#include "dex/dex_file_loader.h" #include "handle_scope-inl.h" #include "jit/profile_compilation_info.h" #include "linear_alloc.h" @@ -1038,4 +1039,89 @@ TEST_F(ProfileCompilationInfoTest, LoadFromZipFailBadProfile) { ASSERT_FALSE(loaded_info.Load(GetFd(zip))); } +TEST_F(ProfileCompilationInfoTest, UpdateProfileKeyOk) { + std::vector<std::unique_ptr<const DexFile>> dex_files = OpenTestDexFiles("MultiDex"); + + ProfileCompilationInfo info; + for (const std::unique_ptr<const DexFile>& dex : dex_files) { + // Create the profile with a different location so that we can update it to the + // real dex location later. + std::string base_location = DexFileLoader::GetBaseLocation(dex->GetLocation()); + std::string multidex_suffix = DexFileLoader::GetMultiDexSuffix(dex->GetLocation()); + std::string old_name = base_location + "-old" + multidex_suffix; + info.AddMethodIndex(Hotness::kFlagHot, + old_name, + dex->GetLocationChecksum(), + /* method_idx */ 0, + dex->NumMethodIds()); + } + + // Update the profile keys based on the original dex files + ASSERT_TRUE(info.UpdateProfileKeys(dex_files)); + + // Verify that we find the methods when searched with the original dex files. + for (const std::unique_ptr<const DexFile>& dex : dex_files) { + std::unique_ptr<ProfileCompilationInfo::OfflineProfileMethodInfo> loaded_pmi = + info.GetMethod(dex->GetLocation(), dex->GetLocationChecksum(), /* method_idx */ 0); + ASSERT_TRUE(loaded_pmi != nullptr); + } +} + +TEST_F(ProfileCompilationInfoTest, UpdateProfileKeyOkButNoUpdate) { + std::vector<std::unique_ptr<const DexFile>> dex_files = OpenTestDexFiles("MultiDex"); + + ProfileCompilationInfo info; + info.AddMethodIndex(Hotness::kFlagHot, + "my.app", + /* checksum */ 123, + /* method_idx */ 0, + /* num_method_ids */ 10); + + // Update the profile keys based on the original dex files + ASSERT_TRUE(info.UpdateProfileKeys(dex_files)); + + // Verify that we did not perform any update and that we cannot find anything with the new + // location. + for (const std::unique_ptr<const DexFile>& dex : dex_files) { + std::unique_ptr<ProfileCompilationInfo::OfflineProfileMethodInfo> loaded_pmi = + info.GetMethod(dex->GetLocation(), dex->GetLocationChecksum(), /* method_idx */ 0); + ASSERT_TRUE(loaded_pmi == nullptr); + } + + // Verify that we can find the original entry. + std::unique_ptr<ProfileCompilationInfo::OfflineProfileMethodInfo> loaded_pmi = + info.GetMethod("my.app", /* checksum */ 123, /* method_idx */ 0); + ASSERT_TRUE(loaded_pmi != nullptr); +} + +TEST_F(ProfileCompilationInfoTest, UpdateProfileKeyFail) { + std::vector<std::unique_ptr<const DexFile>> dex_files = OpenTestDexFiles("MultiDex"); + + + ProfileCompilationInfo info; + // Add all dex + for (const std::unique_ptr<const DexFile>& dex : dex_files) { + // Create the profile with a different location so that we can update it to the + // real dex location later. + std::string base_location = DexFileLoader::GetBaseLocation(dex->GetLocation()); + std::string multidex_suffix = DexFileLoader::GetMultiDexSuffix(dex->GetLocation()); + std::string old_name = base_location + "-old" + multidex_suffix; + info.AddMethodIndex(Hotness::kFlagHot, + old_name, + dex->GetLocationChecksum(), + /* method_idx */ 0, + dex->NumMethodIds()); + } + + // Add a method index using the location we want to rename to. + // This will cause the rename to fail because an existing entry would already have that name. + info.AddMethodIndex(Hotness::kFlagHot, + dex_files[0]->GetLocation(), + /* checksum */ 123, + /* method_idx */ 0, + dex_files[0]->NumMethodIds()); + + ASSERT_FALSE(info.UpdateProfileKeys(dex_files)); +} + } // namespace art diff --git a/runtime/jni_internal.cc b/runtime/jni_internal.cc index b8e6ebe8d8..c40360f612 100644 --- a/runtime/jni_internal.cc +++ b/runtime/jni_internal.cc @@ -34,6 +34,7 @@ #include "class_linker-inl.h" #include "dex/dex_file-inl.h" #include "fault_handler.h" +#include "hidden_api.h" #include "gc/accounting/card_table-inl.h" #include "gc_root.h" #include "indirect_reference_table-inl.h" @@ -79,15 +80,17 @@ namespace art { // things not rendering correctly. E.g. b/16858794 static constexpr bool kWarnJniAbort = false; -// Helpers to call instrumentation functions for fields. These take jobjects so we don't need to set -// up handles for the rare case where these actually do something. Once these functions return it is -// possible there will be a pending exception if the instrumentation happens to throw one. +// Helpers to check if we need to warn about accessing hidden API fields and to call instrumentation +// functions for them. These take jobjects so we don't need to set up handles for the rare case +// where these actually do something. Once these functions return it is possible there will be +// a pending exception if the instrumentation happens to throw one. static void NotifySetObjectField(ArtField* field, jobject obj, jobject jval) REQUIRES_SHARED(Locks::mutator_lock_) { DCHECK_EQ(field->GetTypeAsPrimitiveType(), Primitive::kPrimNot); + Thread* self = Thread::Current(); + hiddenapi::MaybeWarnAboutMemberAccess(field, self, /* num_frames */ 1); instrumentation::Instrumentation* instrumentation = Runtime::Current()->GetInstrumentation(); if (UNLIKELY(instrumentation->HasFieldWriteListeners())) { - Thread* self = Thread::Current(); ArtMethod* cur_method = self->GetCurrentMethod(/*dex_pc*/ nullptr, /*check_suspended*/ true, /*abort_on_error*/ false); @@ -112,9 +115,10 @@ static void NotifySetObjectField(ArtField* field, jobject obj, jobject jval) static void NotifySetPrimitiveField(ArtField* field, jobject obj, JValue val) REQUIRES_SHARED(Locks::mutator_lock_) { DCHECK_NE(field->GetTypeAsPrimitiveType(), Primitive::kPrimNot); + Thread* self = Thread::Current(); + hiddenapi::MaybeWarnAboutMemberAccess(field, self, /* num_frames */ 1); instrumentation::Instrumentation* instrumentation = Runtime::Current()->GetInstrumentation(); if (UNLIKELY(instrumentation->HasFieldWriteListeners())) { - Thread* self = Thread::Current(); ArtMethod* cur_method = self->GetCurrentMethod(/*dex_pc*/ nullptr, /*check_suspended*/ true, /*abort_on_error*/ false); @@ -136,9 +140,10 @@ static void NotifySetPrimitiveField(ArtField* field, jobject obj, JValue val) static void NotifyGetField(ArtField* field, jobject obj) REQUIRES_SHARED(Locks::mutator_lock_) { + Thread* self = Thread::Current(); + hiddenapi::MaybeWarnAboutMemberAccess(field, self, /* num_frames */ 1); instrumentation::Instrumentation* instrumentation = Runtime::Current()->GetInstrumentation(); if (UNLIKELY(instrumentation->HasFieldReadListeners())) { - Thread* self = Thread::Current(); ArtMethod* cur_method = self->GetCurrentMethod(/*dex_pc*/ nullptr, /*check_suspended*/ true, /*abort_on_error*/ false); @@ -238,6 +243,10 @@ static jmethodID FindMethodID(ScopedObjectAccess& soa, jclass jni_class, } else { method = c->FindClassMethod(name, sig, pointer_size); } + if (method != nullptr && + hiddenapi::ShouldBlockAccessToMember(method, soa.Self(), /* num_frames */ 1)) { + method = nullptr; + } if (method == nullptr || method->IsStatic() != is_static) { ThrowNoSuchMethodError(soa, c, name, sig, is_static ? "static" : "non-static"); return nullptr; @@ -314,6 +323,10 @@ static jfieldID FindFieldID(const ScopedObjectAccess& soa, jclass jni_class, con } else { field = c->FindInstanceField(name, field_type->GetDescriptor(&temp)); } + if (field != nullptr && + hiddenapi::ShouldBlockAccessToMember(field, soa.Self(), /* num_frames */ 1)) { + field = nullptr; + } if (field == nullptr) { soa.Self()->ThrowNewExceptionF("Ljava/lang/NoSuchFieldError;", "no \"%s\" field \"%s\" in class \"%s\" or its superclasses", diff --git a/runtime/mirror/class-inl.h b/runtime/mirror/class-inl.h index cd313b32ab..36388eb3aa 100644 --- a/runtime/mirror/class-inl.h +++ b/runtime/mirror/class-inl.h @@ -30,6 +30,7 @@ #include "dex/invoke_type.h" #include "dex_cache.h" #include "gc/heap-inl.h" +#include "hidden_api.h" #include "iftable.h" #include "subtype_check.h" #include "object-inl.h" @@ -1143,6 +1144,10 @@ inline bool Class::CanAccessMember(ObjPtr<Class> access_to, uint32_t member_flag if (this == access_to) { return true; } + // Do not allow non-boot class path classes access hidden APIs. + if (hiddenapi::ShouldBlockAccessToMember(member_flags, this)) { + return false; + } // Public members are trivially accessible if (member_flags & kAccPublic) { return true; diff --git a/runtime/native/dalvik_system_ZygoteHooks.cc b/runtime/native/dalvik_system_ZygoteHooks.cc index e22726b79b..2892967a51 100644 --- a/runtime/native/dalvik_system_ZygoteHooks.cc +++ b/runtime/native/dalvik_system_ZygoteHooks.cc @@ -286,7 +286,7 @@ static void ZygoteHooks_nativePostForkChild(JNIEnv* env, } if ((runtime_flags & DISABLE_HIDDEN_API_CHECKS) != 0) { - Runtime::Current()->DisableHiddenApiChecks(); + Runtime::Current()->SetHiddenApiChecksEnabled(false); runtime_flags &= ~DISABLE_HIDDEN_API_CHECKS; } diff --git a/runtime/native/java_lang_Class.cc b/runtime/native/java_lang_Class.cc index 7b999c04af..5544275984 100644 --- a/runtime/native/java_lang_Class.cc +++ b/runtime/native/java_lang_Class.cc @@ -25,6 +25,7 @@ #include "common_throws.h" #include "dex/dex_file-inl.h" #include "dex/dex_file_annotations.h" +#include "hidden_api.h" #include "jni_internal.h" #include "mirror/class-inl.h" #include "mirror/class_loader.h" @@ -47,6 +48,75 @@ namespace art { +ALWAYS_INLINE static bool ShouldEnforceHiddenApi(Thread* self) + REQUIRES_SHARED(Locks::mutator_lock_) { + if (!Runtime::Current()->AreHiddenApiChecksEnabled()) { + return false; + } + + // Walk the stack and find the first frame not from java.lang.Class. + // This is very expensive. Save this till the last. + struct FirstNonClassClassCallerVisitor : public StackVisitor { + explicit FirstNonClassClassCallerVisitor(Thread* thread) + : StackVisitor(thread, nullptr, StackVisitor::StackWalkKind::kIncludeInlinedFrames), + caller(nullptr) { + } + + bool VisitFrame() REQUIRES_SHARED(Locks::mutator_lock_) { + ArtMethod *m = GetMethod(); + if (m == nullptr) { + // Attached native thread. Assume this is *not* boot class path. + caller = nullptr; + return false; + } else if (m->IsRuntimeMethod()) { + // Internal runtime method, continue walking the stack. + return true; + } else if (m->GetDeclaringClass()->IsClassClass()) { + return true; + } else { + caller = m; + return false; + } + } + + ArtMethod* caller; + }; + + FirstNonClassClassCallerVisitor visitor(self); + visitor.WalkStack(); + return visitor.caller == nullptr || + !visitor.caller->GetDeclaringClass()->IsBootStrapClassLoaded(); +} + +// Returns true if the first non-ClassClass caller up the stack should not be +// allowed access to `member`. +template<typename T> +ALWAYS_INLINE static bool ShouldBlockAccessToMember(T* member, Thread* self) + REQUIRES_SHARED(Locks::mutator_lock_) { + DCHECK(member != nullptr); + return hiddenapi::IsMemberHidden(member->GetAccessFlags()) && + ShouldEnforceHiddenApi(self); +} + +// Returns true if a class member should be discoverable with reflection given +// the criteria. Some reflection calls only return public members +// (public_only == true), some members should be hidden from non-boot class path +// callers (enforce_hidden_api == true). +ALWAYS_INLINE static bool IsDiscoverable(bool public_only, + bool enforce_hidden_api, + uint32_t access_flags) { + if (public_only && ((access_flags & kAccPublic) == 0)) { + return false; + } + + if (enforce_hidden_api && hiddenapi::IsMemberHidden(access_flags)) { + return false; + } + + return true; +} + + ALWAYS_INLINE static inline ObjPtr<mirror::Class> DecodeClass( const ScopedFastNativeObjectAccess& soa, jobject java_class) REQUIRES_SHARED(Locks::mutator_lock_) { @@ -164,17 +234,16 @@ static mirror::ObjectArray<mirror::Field>* GetDeclaredFields( IterationRange<StrideIterator<ArtField>> ifields = klass->GetIFields(); IterationRange<StrideIterator<ArtField>> sfields = klass->GetSFields(); size_t array_size = klass->NumInstanceFields() + klass->NumStaticFields(); - if (public_only) { - // Lets go subtract all the non public fields. - for (ArtField& field : ifields) { - if (!field.IsPublic()) { - --array_size; - } + bool enforce_hidden_api = ShouldEnforceHiddenApi(self); + // Lets go subtract all the non discoverable fields. + for (ArtField& field : ifields) { + if (!IsDiscoverable(public_only, enforce_hidden_api, field.GetAccessFlags())) { + --array_size; } - for (ArtField& field : sfields) { - if (!field.IsPublic()) { - --array_size; - } + } + for (ArtField& field : sfields) { + if (!IsDiscoverable(public_only, enforce_hidden_api, field.GetAccessFlags())) { + --array_size; } } size_t array_idx = 0; @@ -184,7 +253,7 @@ static mirror::ObjectArray<mirror::Field>* GetDeclaredFields( return nullptr; } for (ArtField& field : ifields) { - if (!public_only || field.IsPublic()) { + if (IsDiscoverable(public_only, enforce_hidden_api, field.GetAccessFlags())) { auto* reflect_field = mirror::Field::CreateFromArtField<kRuntimePointerSize>(self, &field, force_resolve); @@ -199,7 +268,7 @@ static mirror::ObjectArray<mirror::Field>* GetDeclaredFields( } } for (ArtField& field : sfields) { - if (!public_only || field.IsPublic()) { + if (IsDiscoverable(public_only, enforce_hidden_api, field.GetAccessFlags())) { auto* reflect_field = mirror::Field::CreateFromArtField<kRuntimePointerSize>(self, &field, force_resolve); @@ -354,8 +423,13 @@ static jobject Class_getPublicFieldRecursive(JNIEnv* env, jobject javaThis, jstr ThrowNullPointerException("name == null"); return nullptr; } - return soa.AddLocalReference<jobject>( - GetPublicFieldRecursive(soa.Self(), DecodeClass(soa, javaThis), name_string)); + + mirror::Field* field = GetPublicFieldRecursive( + soa.Self(), DecodeClass(soa, javaThis), name_string); + if (field == nullptr || ShouldBlockAccessToMember(field->GetArtField(), soa.Self())) { + return nullptr; + } + return soa.AddLocalReference<jobject>(field); } static jobject Class_getDeclaredField(JNIEnv* env, jobject javaThis, jstring name) { @@ -369,7 +443,7 @@ static jobject Class_getDeclaredField(JNIEnv* env, jobject javaThis, jstring nam Handle<mirror::Class> h_klass = hs.NewHandle(DecodeClass(soa, javaThis)); Handle<mirror::Field> result = hs.NewHandle(GetDeclaredField(soa.Self(), h_klass.Get(), h_string.Get())); - if (result == nullptr) { + if (result == nullptr || ShouldBlockAccessToMember(result->GetArtField(), soa.Self())) { std::string name_str = h_string->ToModifiedUtf8(); if (name_str == "value" && h_klass->IsStringClass()) { // We log the error for this specific case, as the user might just swallow the exception. @@ -399,24 +473,32 @@ static jobject Class_getDeclaredConstructorInternal( soa.Self(), DecodeClass(soa, javaThis), soa.Decode<mirror::ObjectArray<mirror::Class>>(args)); + if (result == nullptr || ShouldBlockAccessToMember(result->GetArtMethod(), soa.Self())) { + return nullptr; + } return soa.AddLocalReference<jobject>(result); } -static ALWAYS_INLINE inline bool MethodMatchesConstructor(ArtMethod* m, bool public_only) +static ALWAYS_INLINE inline bool MethodMatchesConstructor( + ArtMethod* m, bool public_only, bool enforce_hidden_api) REQUIRES_SHARED(Locks::mutator_lock_) { DCHECK(m != nullptr); - return (!public_only || m->IsPublic()) && !m->IsStatic() && m->IsConstructor(); + return m->IsConstructor() && + !m->IsStatic() && + IsDiscoverable(public_only, enforce_hidden_api, m->GetAccessFlags()); } static jobjectArray Class_getDeclaredConstructorsInternal( JNIEnv* env, jobject javaThis, jboolean publicOnly) { ScopedFastNativeObjectAccess soa(env); StackHandleScope<2> hs(soa.Self()); + bool public_only = (publicOnly != JNI_FALSE); + bool enforce_hidden_api = ShouldEnforceHiddenApi(soa.Self()); Handle<mirror::Class> h_klass = hs.NewHandle(DecodeClass(soa, javaThis)); size_t constructor_count = 0; // Two pass approach for speed. for (auto& m : h_klass->GetDirectMethods(kRuntimePointerSize)) { - constructor_count += MethodMatchesConstructor(&m, publicOnly != JNI_FALSE) ? 1u : 0u; + constructor_count += MethodMatchesConstructor(&m, public_only, enforce_hidden_api) ? 1u : 0u; } auto h_constructors = hs.NewHandle(mirror::ObjectArray<mirror::Constructor>::Alloc( soa.Self(), mirror::Constructor::ArrayClass(), constructor_count)); @@ -426,7 +508,7 @@ static jobjectArray Class_getDeclaredConstructorsInternal( } constructor_count = 0; for (auto& m : h_klass->GetDirectMethods(kRuntimePointerSize)) { - if (MethodMatchesConstructor(&m, publicOnly != JNI_FALSE)) { + if (MethodMatchesConstructor(&m, public_only, enforce_hidden_api)) { DCHECK_EQ(Runtime::Current()->GetClassLinker()->GetImagePointerSize(), kRuntimePointerSize); DCHECK(!Runtime::Current()->IsActiveTransaction()); auto* constructor = mirror::Constructor::CreateFromArtMethod<kRuntimePointerSize, false>( @@ -452,6 +534,9 @@ static jobject Class_getDeclaredMethodInternal(JNIEnv* env, jobject javaThis, DecodeClass(soa, javaThis), soa.Decode<mirror::String>(name), soa.Decode<mirror::ObjectArray<mirror::Class>>(args)); + if (result == nullptr || ShouldBlockAccessToMember(result->GetArtMethod(), soa.Self())) { + return nullptr; + } return soa.AddLocalReference<jobject>(result); } @@ -459,13 +544,17 @@ static jobjectArray Class_getDeclaredMethodsUnchecked(JNIEnv* env, jobject javaT jboolean publicOnly) { ScopedFastNativeObjectAccess soa(env); StackHandleScope<2> hs(soa.Self()); + + bool enforce_hidden_api = ShouldEnforceHiddenApi(soa.Self()); + bool public_only = (publicOnly != JNI_FALSE); + Handle<mirror::Class> klass = hs.NewHandle(DecodeClass(soa, javaThis)); size_t num_methods = 0; - for (auto& m : klass->GetDeclaredMethods(kRuntimePointerSize)) { - auto modifiers = m.GetAccessFlags(); + for (ArtMethod& m : klass->GetDeclaredMethods(kRuntimePointerSize)) { + uint32_t modifiers = m.GetAccessFlags(); // Add non-constructor declared methods. - if ((publicOnly == JNI_FALSE || (modifiers & kAccPublic) != 0) && - (modifiers & kAccConstructor) == 0) { + if ((modifiers & kAccConstructor) == 0 && + IsDiscoverable(public_only, enforce_hidden_api, modifiers)) { ++num_methods; } } @@ -476,10 +565,10 @@ static jobjectArray Class_getDeclaredMethodsUnchecked(JNIEnv* env, jobject javaT return nullptr; } num_methods = 0; - for (auto& m : klass->GetDeclaredMethods(kRuntimePointerSize)) { - auto modifiers = m.GetAccessFlags(); - if ((publicOnly == JNI_FALSE || (modifiers & kAccPublic) != 0) && - (modifiers & kAccConstructor) == 0) { + for (ArtMethod& m : klass->GetDeclaredMethods(kRuntimePointerSize)) { + uint32_t modifiers = m.GetAccessFlags(); + if ((modifiers & kAccConstructor) == 0 && + IsDiscoverable(public_only, enforce_hidden_api, modifiers)) { DCHECK_EQ(Runtime::Current()->GetClassLinker()->GetImagePointerSize(), kRuntimePointerSize); DCHECK(!Runtime::Current()->IsActiveTransaction()); auto* method = @@ -693,11 +782,11 @@ static jobject Class_newInstance(JNIEnv* env, jobject javaThis) { return nullptr; } } - auto* constructor = klass->GetDeclaredConstructor( + ArtMethod* constructor = klass->GetDeclaredConstructor( soa.Self(), ScopedNullHandle<mirror::ObjectArray<mirror::Class>>(), kRuntimePointerSize); - if (UNLIKELY(constructor == nullptr)) { + if (UNLIKELY(constructor == nullptr) || ShouldBlockAccessToMember(constructor, soa.Self())) { soa.Self()->ThrowNewExceptionF("Ljava/lang/InstantiationException;", "%s has no zero argument constructor", klass->PrettyClass().c_str()); @@ -742,6 +831,7 @@ static jobject Class_newInstance(JNIEnv* env, jobject javaThis) { return nullptr; } } + hiddenapi::MaybeWarnAboutMemberAccess(constructor, soa.Self(), /* num_frames */ 1); // Invoke the constructor. JValue result; uint32_t args[1] = { static_cast<uint32_t>(reinterpret_cast<uintptr_t>(receiver.Get())) }; diff --git a/runtime/native/java_lang_reflect_Field.cc b/runtime/native/java_lang_reflect_Field.cc index f990c0421d..db7f4bb18c 100644 --- a/runtime/native/java_lang_reflect_Field.cc +++ b/runtime/native/java_lang_reflect_Field.cc @@ -25,6 +25,7 @@ #include "common_throws.h" #include "dex/dex_file-inl.h" #include "dex/dex_file_annotations.h" +#include "hidden_api.h" #include "jni_internal.h" #include "mirror/class-inl.h" #include "mirror/field-inl.h" @@ -161,6 +162,9 @@ static jobject Field_get(JNIEnv* env, jobject javaField, jobject javaObj) { DCHECK(soa.Self()->IsExceptionPending()); return nullptr; } + + hiddenapi::MaybeWarnAboutMemberAccess(f->GetArtField(), soa.Self(), /* num_frames */ 1); + // We now don't expect suspension unless an exception is thrown. // Get the field's value, boxing if necessary. Primitive::Type field_type = f->GetTypeAsPrimitiveType(); @@ -183,13 +187,14 @@ ALWAYS_INLINE inline static JValue GetPrimitiveField(JNIEnv* env, DCHECK(soa.Self()->IsExceptionPending()); return JValue(); } - // If field is not set to be accessible, verify it can be accessed by the caller. if (!f->IsAccessible() && !VerifyFieldAccess<false>(soa.Self(), f, o)) { DCHECK(soa.Self()->IsExceptionPending()); return JValue(); } + hiddenapi::MaybeWarnAboutMemberAccess(f->GetArtField(), soa.Self(), /* num_frames */ 1); + // We now don't expect suspension unless an exception is thrown. // Read the value. Primitive::Type field_type = f->GetTypeAsPrimitiveType(); @@ -351,11 +356,15 @@ static void Field_set(JNIEnv* env, jobject javaField, jobject javaObj, jobject j DCHECK(soa.Self()->IsExceptionPending()); return; } + // If field is not set to be accessible, verify it can be accessed by the caller. if (!f->IsAccessible() && !VerifyFieldAccess<true>(soa.Self(), f, o)) { DCHECK(soa.Self()->IsExceptionPending()); return; } + + hiddenapi::MaybeWarnAboutMemberAccess(f->GetArtField(), soa.Self(), /* num_frames */ 1); + SetFieldValue(o, f, field_prim_type, true, unboxed_value); } @@ -391,6 +400,8 @@ static void SetPrimitiveField(JNIEnv* env, return; } + hiddenapi::MaybeWarnAboutMemberAccess(f->GetArtField(), soa.Self(), /* num_frames */ 1); + // Write the value. SetFieldValue(o, f, field_type, false, wide_value); } diff --git a/runtime/reflection.cc b/runtime/reflection.cc index 635a03afe0..6ffafe02f1 100644 --- a/runtime/reflection.cc +++ b/runtime/reflection.cc @@ -465,6 +465,9 @@ JValue InvokeWithVarArgs(const ScopedObjectAccessAlreadyRunnable& soa, jobject o } ArtMethod* method = jni::DecodeArtMethod(mid); + + hiddenapi::MaybeWarnAboutMemberAccess(method, soa.Self(), /* num_frames */ 1); + bool is_string_init = method->GetDeclaringClass()->IsStringClass() && method->IsConstructor(); if (is_string_init) { // Replace calls to String.<init> with equivalent StringFactory call. @@ -496,6 +499,9 @@ JValue InvokeWithJValues(const ScopedObjectAccessAlreadyRunnable& soa, jobject o } ArtMethod* method = jni::DecodeArtMethod(mid); + + hiddenapi::MaybeWarnAboutMemberAccess(method, soa.Self(), /* num_frames */ 1); + bool is_string_init = method->GetDeclaringClass()->IsStringClass() && method->IsConstructor(); if (is_string_init) { // Replace calls to String.<init> with equivalent StringFactory call. @@ -528,6 +534,9 @@ JValue InvokeVirtualOrInterfaceWithJValues(const ScopedObjectAccessAlreadyRunnab ObjPtr<mirror::Object> receiver = soa.Decode<mirror::Object>(obj); ArtMethod* method = FindVirtualMethod(receiver, jni::DecodeArtMethod(mid)); + + hiddenapi::MaybeWarnAboutMemberAccess(method, soa.Self(), /* num_frames */ 1); + bool is_string_init = method->GetDeclaringClass()->IsStringClass() && method->IsConstructor(); if (is_string_init) { // Replace calls to String.<init> with equivalent StringFactory call. @@ -560,6 +569,9 @@ JValue InvokeVirtualOrInterfaceWithVarArgs(const ScopedObjectAccessAlreadyRunnab ObjPtr<mirror::Object> receiver = soa.Decode<mirror::Object>(obj); ArtMethod* method = FindVirtualMethod(receiver, jni::DecodeArtMethod(mid)); + + hiddenapi::MaybeWarnAboutMemberAccess(method, soa.Self(), /* num_frames */ 1); + bool is_string_init = method->GetDeclaringClass()->IsStringClass() && method->IsConstructor(); if (is_string_init) { // Replace calls to String.<init> with equivalent StringFactory call. @@ -604,6 +616,8 @@ jobject InvokeMethod(const ScopedObjectAccessAlreadyRunnable& soa, jobject javaM } } + hiddenapi::MaybeWarnAboutMemberAccess(m, soa.Self(), num_frames); + ObjPtr<mirror::Object> receiver; if (!m->IsStatic()) { // Replace calls to String.<init> with equivalent StringFactory call. diff --git a/runtime/runtime.cc b/runtime/runtime.cc index 33bebe0887..6d065d6146 100644 --- a/runtime/runtime.cc +++ b/runtime/runtime.cc @@ -266,6 +266,8 @@ Runtime::Runtime() is_low_memory_mode_(false), safe_mode_(false), do_hidden_api_checks_(false), + pending_hidden_api_warning_(false), + dedupe_hidden_api_warnings_(true), dump_native_stack_on_sig_quit_(true), pruned_dalvik_cache_(false), // Initially assume we perceive jank in case the process state is never updated. diff --git a/runtime/runtime.h b/runtime/runtime.h index 022a1be124..184e4e5b91 100644 --- a/runtime/runtime.h +++ b/runtime/runtime.h @@ -520,14 +520,30 @@ class Runtime { bool IsVerificationEnabled() const; bool IsVerificationSoftFail() const; - void DisableHiddenApiChecks() { - do_hidden_api_checks_ = false; + void SetHiddenApiChecksEnabled(bool value) { + do_hidden_api_checks_ = value; } bool AreHiddenApiChecksEnabled() const { return do_hidden_api_checks_; } + void SetPendingHiddenApiWarning(bool value) { + pending_hidden_api_warning_ = value; + } + + bool HasPendingHiddenApiWarning() const { + return pending_hidden_api_warning_; + } + + void SetDedupeHiddenApiWarnings(bool value) { + dedupe_hidden_api_warnings_ = value; + } + + bool ShouldDedupeHiddenApiWarnings() { + return dedupe_hidden_api_warnings_; + } + bool IsDexFileFallbackEnabled() const { return allow_dex_file_fallback_; } @@ -968,6 +984,14 @@ class Runtime { // Whether access checks on hidden API should be performed. bool do_hidden_api_checks_; + // Whether the application has used an API which is not restricted but we + // should issue a warning about it. + bool pending_hidden_api_warning_; + + // Do not warn about the same hidden API access violation twice. + // This is only used for testing. + bool dedupe_hidden_api_warnings_; + // Whether threads should dump their native stack on SIGQUIT. bool dump_native_stack_on_sig_quit_; diff --git a/runtime/vdex_file.h b/runtime/vdex_file.h index e8d66ace8e..b9fd467017 100644 --- a/runtime/vdex_file.h +++ b/runtime/vdex_file.h @@ -223,6 +223,10 @@ class VdexFile { ArrayRef<const uint8_t> GetQuickenedInfoOf(const DexFile& dex_file, uint32_t dex_method_idx) const; + bool HasDexSection() const { + return GetHeader().GetDexSize() != 0; + } + private: uint32_t GetQuickeningInfoTableOffset(const uint8_t* source_dex_begin) const; @@ -240,10 +244,6 @@ class VdexFile { uint32_t num_method_ids, const ArrayRef<const uint8_t>& quickening_info) const; - bool HasDexSection() const { - return GetHeader().GetDexSize() != 0; - } - bool ContainsDexFile(const DexFile& dex_file) const; const uint8_t* DexBegin() const { diff --git a/runtime/verifier/reg_type-inl.h b/runtime/verifier/reg_type-inl.h index f719782727..9e12d636d4 100644 --- a/runtime/verifier/reg_type-inl.h +++ b/runtime/verifier/reg_type-inl.h @@ -48,9 +48,6 @@ inline bool RegType::CanAccess(const RegType& other) const { inline bool RegType::CanAccessMember(ObjPtr<mirror::Class> klass, uint32_t access_flags) const { DCHECK(IsReferenceTypes()); - if ((access_flags & kAccPublic) != 0) { - return true; - } if (IsNull()) { return true; } diff --git a/test/530-checker-lse/src/Main.java b/test/530-checker-lse/src/Main.java index 98838c5089..f6332b5503 100644 --- a/test/530-checker-lse/src/Main.java +++ b/test/530-checker-lse/src/Main.java @@ -398,6 +398,7 @@ public class Main { /// CHECK-START: int Main.test15() load_store_elimination (after) /// CHECK: <<Const2:i\d+>> IntConstant 2 /// CHECK: StaticFieldSet + /// CHECK: StaticFieldSet /// CHECK-NOT: StaticFieldGet /// CHECK: Return [<<Const2>>] @@ -772,127 +773,6 @@ public class Main { return obj; } - /// CHECK-START: void Main.testStoreStore2(TestClass2) load_store_elimination (before) - /// CHECK: InstanceFieldSet - /// CHECK: InstanceFieldSet - /// CHECK: InstanceFieldSet - /// CHECK: InstanceFieldSet - - /// CHECK-START: void Main.testStoreStore2(TestClass2) load_store_elimination (after) - /// CHECK: InstanceFieldSet - /// CHECK: InstanceFieldSet - /// CHECK-NOT: InstanceFieldSet - - private static void testStoreStore2(TestClass2 obj) { - obj.i = 41; - obj.j = 42; - obj.i = 43; - obj.j = 44; - } - - /// CHECK-START: void Main.testStoreStore3(TestClass2, boolean) load_store_elimination (before) - /// CHECK: InstanceFieldSet - /// CHECK: InstanceFieldSet - /// CHECK: InstanceFieldSet - /// CHECK: InstanceFieldSet - - /// CHECK-START: void Main.testStoreStore3(TestClass2, boolean) load_store_elimination (after) - /// CHECK: InstanceFieldSet - /// CHECK: InstanceFieldSet - /// CHECK: InstanceFieldSet - /// CHECK-NOT: InstanceFieldSet - - private static void testStoreStore3(TestClass2 obj, boolean flag) { - obj.i = 41; - obj.j = 42; // redundant since it's overwritten in both branches below. - if (flag) { - obj.j = 43; - } else { - obj.j = 44; - } - } - - /// CHECK-START: void Main.testStoreStore4() load_store_elimination (before) - /// CHECK: StaticFieldSet - /// CHECK: StaticFieldSet - - /// CHECK-START: void Main.testStoreStore4() load_store_elimination (after) - /// CHECK: StaticFieldSet - /// CHECK-NOT: StaticFieldSet - - private static void testStoreStore4() { - TestClass.si = 61; - TestClass.si = 62; - } - - /// CHECK-START: int Main.testStoreStore5(TestClass2, TestClass2) load_store_elimination (before) - /// CHECK: InstanceFieldSet - /// CHECK: InstanceFieldGet - /// CHECK: InstanceFieldSet - - /// CHECK-START: int Main.testStoreStore5(TestClass2, TestClass2) load_store_elimination (after) - /// CHECK: InstanceFieldSet - /// CHECK: InstanceFieldGet - /// CHECK: InstanceFieldSet - - private static int testStoreStore5(TestClass2 obj1, TestClass2 obj2) { - obj1.i = 71; // This store is needed since obj2.i may load from it. - int i = obj2.i; - obj1.i = 72; - return i; - } - - /// CHECK-START: int Main.testStoreStore6(TestClass2, TestClass2) load_store_elimination (before) - /// CHECK: InstanceFieldSet - /// CHECK: InstanceFieldGet - /// CHECK: InstanceFieldSet - - /// CHECK-START: int Main.testStoreStore6(TestClass2, TestClass2) load_store_elimination (after) - /// CHECK-NOT: InstanceFieldSet - /// CHECK: InstanceFieldGet - /// CHECK: InstanceFieldSet - - private static int testStoreStore6(TestClass2 obj1, TestClass2 obj2) { - obj1.i = 81; // This store is not needed since obj2.j cannot load from it. - int j = obj2.j; - obj1.i = 82; - return j; - } - - /// CHECK-START: int Main.testNoSideEffects(int[]) load_store_elimination (before) - /// CHECK: ArraySet - /// CHECK: ArraySet - /// CHECK: ArraySet - /// CHECK: ArrayGet - - /// CHECK-START: int Main.testNoSideEffects(int[]) load_store_elimination (after) - /// CHECK: ArraySet - /// CHECK: ArraySet - /// CHECK-NOT: ArraySet - /// CHECK-NOT: ArrayGet - - private static int testNoSideEffects(int[] array) { - array[0] = 101; - array[1] = 102; - int bitCount = Integer.bitCount(0x3456); - array[1] = 103; - return array[0] + bitCount; - } - - /// CHECK-START: void Main.testThrow(TestClass2, java.lang.Exception) load_store_elimination (before) - /// CHECK: InstanceFieldSet - /// CHECK: Throw - - /// CHECK-START: void Main.testThrow(TestClass2, java.lang.Exception) load_store_elimination (after) - /// CHECK: InstanceFieldSet - /// CHECK: Throw - - // Make sure throw keeps the store. - private static void testThrow(TestClass2 obj, Exception e) throws Exception { - obj.i = 55; - throw e; - } - /// CHECK-START: int Main.testStoreStoreWithDeoptimize(int[]) load_store_elimination (before) /// CHECK: NewInstance /// CHECK: InstanceFieldSet @@ -934,6 +814,23 @@ public class Main { return arr[0] + arr[1] + arr[2] + arr[3]; } + /// CHECK-START: int Main.testNoSideEffects(int[]) load_store_elimination (before) + /// CHECK: ArraySet + /// CHECK: ArraySet + /// CHECK: ArrayGet + + /// CHECK-START: int Main.testNoSideEffects(int[]) load_store_elimination (after) + /// CHECK: ArraySet + /// CHECK: ArraySet + /// CHECK-NOT: ArrayGet + + private static int testNoSideEffects(int[] array) { + array[0] = 101; + int bitCount = Integer.bitCount(0x3456); + array[1] = array[0] + 1; + return array[0] + bitCount; + } + /// CHECK-START: double Main.getCircleArea(double, boolean) load_store_elimination (before) /// CHECK: NewInstance @@ -1208,46 +1105,16 @@ public class Main { assertIntEquals(testStoreStore().i, 41); assertIntEquals(testStoreStore().j, 43); + assertIntEquals(testStoreStoreWithDeoptimize(new int[4]), 4); assertIntEquals(testExitMerge(true), 2); assertIntEquals(testExitMerge2(true), 2); assertIntEquals(testExitMerge2(false), 2); - TestClass2 testclass2 = new TestClass2(); - testStoreStore2(testclass2); - assertIntEquals(testclass2.i, 43); - assertIntEquals(testclass2.j, 44); - - testStoreStore3(testclass2, true); - assertIntEquals(testclass2.i, 41); - assertIntEquals(testclass2.j, 43); - testStoreStore3(testclass2, false); - assertIntEquals(testclass2.i, 41); - assertIntEquals(testclass2.j, 44); - - testStoreStore4(); - assertIntEquals(TestClass.si, 62); - - int ret = testStoreStore5(testclass2, testclass2); - assertIntEquals(testclass2.i, 72); - assertIntEquals(ret, 71); - - testclass2.j = 88; - ret = testStoreStore6(testclass2, testclass2); - assertIntEquals(testclass2.i, 82); - assertIntEquals(ret, 88); - - ret = testNoSideEffects(iarray); + int ret = testNoSideEffects(iarray); assertIntEquals(iarray[0], 101); - assertIntEquals(iarray[1], 103); + assertIntEquals(iarray[1], 102); assertIntEquals(ret, 108); - - try { - testThrow(testclass2, new Exception()); - } catch (Exception e) {} - assertIntEquals(testclass2.i, 55); - - assertIntEquals(testStoreStoreWithDeoptimize(new int[4]), 4); } static boolean sFlag; diff --git a/test/552-checker-sharpening/src/Main.java b/test/552-checker-sharpening/src/Main.java index 0623a22d64..3173afdfcd 100644 --- a/test/552-checker-sharpening/src/Main.java +++ b/test/552-checker-sharpening/src/Main.java @@ -14,17 +14,8 @@ * limitations under the License. */ -import java.io.ByteArrayInputStream; -import java.io.InputStream; - public class Main { - public static void assertBooleanEquals(boolean expected, boolean result) { - if (expected != result) { - throw new Error("Expected: " + expected + ", found: " + result); - } - } - public static void assertIntEquals(int expected, int result) { if (expected != result) { throw new Error("Expected: " + expected + ", found: " + result); @@ -204,14 +195,6 @@ public class Main { return Other.class; } - /// CHECK-START: boolean Main.$noinline$instanceOfInputStream(java.lang.Object) builder (after) - /// CHECK-NOT: LoadClass - /// CHECK: InstanceOf check_kind:bitstring_check - public static boolean $noinline$instanceOfInputStream(Object o) { - // InputStream is known to be in the core image with an initialized type check bitstring. - return o instanceof InputStream; - } - public static void main(String[] args) { assertIntEquals(1, testSimple(1)); assertIntEquals(1, testDiamond(false, 1)); @@ -225,10 +208,6 @@ public class Main { assertStringEquals("non-boot-image-string", $noinline$getNonBootImageString()); assertClassEquals(String.class, $noinline$getStringClass()); assertClassEquals(Other.class, $noinline$getOtherClass()); - assertBooleanEquals(false, $noinline$instanceOfInputStream(null)); - assertBooleanEquals(false, $noinline$instanceOfInputStream(new Integer(1))); - assertBooleanEquals(true, - $noinline$instanceOfInputStream(new ByteArrayInputStream(new byte[10]))); } } diff --git a/test/608-checker-unresolved-lse/src/Main.java b/test/608-checker-unresolved-lse/src/Main.java index a39dd51bdf..c6f8854b49 100644 --- a/test/608-checker-unresolved-lse/src/Main.java +++ b/test/608-checker-unresolved-lse/src/Main.java @@ -88,6 +88,7 @@ public class Main extends MissingSuperClass { /// CHECK-START: void Main.staticFieldTest() load_store_elimination (after) /// CHECK: StaticFieldSet + /// CHECK: StaticFieldSet /// CHECK: UnresolvedStaticFieldGet public static void staticFieldTest() { // Ensure Foo is initialized. diff --git a/test/639-checker-code-sinking/expected.txt b/test/639-checker-code-sinking/expected.txt index 5d4833aca8..52e756c231 100644 --- a/test/639-checker-code-sinking/expected.txt +++ b/test/639-checker-code-sinking/expected.txt @@ -1,3 +1,3 @@ 0 class java.lang.Object -42 +43 diff --git a/test/639-checker-code-sinking/src/Main.java b/test/639-checker-code-sinking/src/Main.java index a1c30f7b4e..7496925adc 100644 --- a/test/639-checker-code-sinking/src/Main.java +++ b/test/639-checker-code-sinking/src/Main.java @@ -337,7 +337,7 @@ public class Main { public static void testStoreStore(boolean doThrow) { Main m = new Main(); m.intField = 42; - m.intField2 = 43; + m.intField = 43; if (doThrow) { throw new Error(m.$opt$noinline$toString()); } @@ -349,7 +349,6 @@ public class Main { volatile int volatileField; int intField; - int intField2; Object objectField; static boolean doThrow; static boolean doLoop; diff --git a/test/674-hiddenapi/hiddenapi.cc b/test/674-hiddenapi/hiddenapi.cc index 672079b9d8..baff6f758d 100644 --- a/test/674-hiddenapi/hiddenapi.cc +++ b/test/674-hiddenapi/hiddenapi.cc @@ -25,6 +25,11 @@ namespace art { namespace Test674HiddenApi { +extern "C" JNIEXPORT void JNICALL Java_Main_init(JNIEnv*, jclass) { + Runtime::Current()->SetHiddenApiChecksEnabled(true); + Runtime::Current()->SetDedupeHiddenApiWarnings(false); +} + extern "C" JNIEXPORT void JNICALL Java_Main_appendToBootClassLoader( JNIEnv* env, jclass, jstring jpath) { ScopedUtfChars utf(env, jpath); @@ -280,11 +285,11 @@ extern "C" JNIEXPORT jint JNICALL Java_Reflection_getHiddenApiAccessFlags(JNIEnv } extern "C" JNIEXPORT jboolean JNICALL Java_ChildClass_hasPendingWarning(JNIEnv*, jclass) { - return false; + return Runtime::Current()->HasPendingHiddenApiWarning(); } extern "C" JNIEXPORT void JNICALL Java_ChildClass_clearWarning(JNIEnv*, jclass) { - return; + Runtime::Current()->SetPendingHiddenApiWarning(false); } } // namespace Test674HiddenApi diff --git a/test/674-hiddenapi/src-art/Main.java b/test/674-hiddenapi/src-art/Main.java index 56a644f637..a808e946a9 100644 --- a/test/674-hiddenapi/src-art/Main.java +++ b/test/674-hiddenapi/src-art/Main.java @@ -31,6 +31,9 @@ public class Main { System.loadLibrary(args[0]); prepareNativeLibFileName(args[0]); + // Enable hidden API checks in case they are disabled by default. + init(); + // Run test with both parent and child dex files loaded with class loaders. // The expectation is that hidden members in parent should be visible to // the child. @@ -150,4 +153,5 @@ public class Main { private static ClassLoader BOOT_CLASS_LOADER = Object.class.getClassLoader(); private static native void appendToBootClassLoader(String dexPath); + private static native void init(); } diff --git a/test/674-hiddenapi/src-ex/ChildClass.java b/test/674-hiddenapi/src-ex/ChildClass.java index af615bfd5f..babd88359b 100644 --- a/test/674-hiddenapi/src-ex/ChildClass.java +++ b/test/674-hiddenapi/src-ex/ChildClass.java @@ -93,9 +93,18 @@ public class ChildClass { "in boot class path"); } + boolean isSameBoot = (isParentInBoot == isChildInBoot); + // Run meaningful combinations of access flags. for (Hiddenness hiddenness : Hiddenness.values()) { - final Behaviour expected = Behaviour.Granted; + final Behaviour expected; + if (isSameBoot || hiddenness == Hiddenness.Whitelist) { + expected = Behaviour.Granted; + } else if (hiddenness == Hiddenness.Blacklist) { + expected = Behaviour.Denied; + } else { + expected = Behaviour.Warning; + } for (boolean isStatic : booleanValues) { String suffix = (isStatic ? "Static" : "") + hiddenness.name(); @@ -380,7 +389,7 @@ public class ChildClass { private static void checkLinking(String className, boolean takesParameter, Behaviour behaviour) throws Exception { boolean canAccess = (behaviour != Behaviour.Denied); - boolean setsWarning = (behaviour == Behaviour.Warning); + boolean setsWarning = false; // we do not set the flag in verifier or at runtime clearWarning(); if (Linking.canAccess(className, takesParameter) != canAccess) { diff --git a/test/674-vdex-uncompress/build b/test/674-vdex-uncompress/build new file mode 100755 index 0000000000..7b1804d3e0 --- /dev/null +++ b/test/674-vdex-uncompress/build @@ -0,0 +1,19 @@ +#!/bin/bash +# +# Copyright 2018 The Android Open Source Project +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# Uncompress and align the dex files so that dex2oat will not copy the dex +# code to the .vdex file. +./default-build "$@" --zip-compression-method store --zip-align 4 diff --git a/test/674-vdex-uncompress/expected.txt b/test/674-vdex-uncompress/expected.txt new file mode 100644 index 0000000000..d0f61f692c --- /dev/null +++ b/test/674-vdex-uncompress/expected.txt @@ -0,0 +1,2 @@ +In foo +In foo diff --git a/test/674-vdex-uncompress/info.txt b/test/674-vdex-uncompress/info.txt new file mode 100644 index 0000000000..6aa6f7b0d5 --- /dev/null +++ b/test/674-vdex-uncompress/info.txt @@ -0,0 +1,2 @@ +Test that dex2oat can compile an APK that has uncompressed dex files, +and where --input-vdex is passed. diff --git a/test/674-vdex-uncompress/run b/test/674-vdex-uncompress/run new file mode 100644 index 0000000000..edf699f842 --- /dev/null +++ b/test/674-vdex-uncompress/run @@ -0,0 +1,17 @@ +#!/bin/bash +# +# Copyright (C) 2018 The Android Open Source Project +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +exec ${RUN} -Xcompiler-option --compiler-filter=verify --vdex "${@}" diff --git a/test/674-vdex-uncompress/src/Main.java b/test/674-vdex-uncompress/src/Main.java new file mode 100644 index 0000000000..0a25b564fe --- /dev/null +++ b/test/674-vdex-uncompress/src/Main.java @@ -0,0 +1,37 @@ +/* + * Copyright (C) 2018 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +public class Main { + Main() { + // Will be quickened with RETURN_VOID_NO_BARRIER. + } + + public static void main(String[] args) { + Main m = new Main(); + Object o = m; + // The call and field accesses will be quickened. + m.foo(m.a); + + // The checkcast will be quickened. + m.foo(((Main)o).a); + } + + int a; + void foo(int a) { + System.out.println("In foo"); + } +} + diff --git a/tools/prebuilt_libjdwp_art_failures.txt b/tools/prebuilt_libjdwp_art_failures.txt index 7694a4c7e4..6052f237a7 100644 --- a/tools/prebuilt_libjdwp_art_failures.txt +++ b/tools/prebuilt_libjdwp_art_failures.txt @@ -118,5 +118,11 @@ result: EXEC_FAILED, bug: 69591477, name: "org.apache.harmony.jpda.tests.jdwp.VirtualMachine.ExitTest#testExit001" +}, +{ + description: "Test is flakey", + result: EXEC_FAILED, + bug: 70958370, + name: "org.apache.harmony.jpda.tests.jdwp.ObjectReference.EnableCollectionTest#testEnableCollection001" } ] |