diff options
79 files changed, 1316 insertions, 274 deletions
diff --git a/compiler/jni/jni_compiler_test.cc b/compiler/jni/jni_compiler_test.cc index 251dc39864..c4c2399ccd 100644 --- a/compiler/jni/jni_compiler_test.cc +++ b/compiler/jni/jni_compiler_test.cc @@ -98,8 +98,8 @@ class JniCompilerTest : public CommonCompilerTest { CompileForTest(class_loader_, direct, method_name, method_sig); // Start runtime. Thread::Current()->TransitionFromSuspendedToRunnable(); - bool started = runtime_->Start(); android::InitializeNativeLoader(); + bool started = runtime_->Start(); CHECK(started); } // JNI operations after runtime start. diff --git a/compiler/optimizing/code_generator.cc b/compiler/optimizing/code_generator.cc index 3983661143..08670a0d82 100644 --- a/compiler/optimizing/code_generator.cc +++ b/compiler/optimizing/code_generator.cc @@ -50,6 +50,7 @@ #include "mirror/array-inl.h" #include "mirror/object_array-inl.h" #include "mirror/object_reference.h" +#include "mirror/string.h" #include "parallel_move_resolver.h" #include "ssa_liveness_analysis.h" #include "utils/assembler.h" @@ -139,6 +140,12 @@ size_t CodeGenerator::GetCachePointerOffset(uint32_t index) { return pointer_size * index; } +uint32_t CodeGenerator::GetArrayLengthOffset(HArrayLength* array_length) { + return array_length->IsStringLength() + ? mirror::String::CountOffset().Uint32Value() + : mirror::Array::LengthOffset().Uint32Value(); +} + bool CodeGenerator::GoesToNextBlock(HBasicBlock* current, HBasicBlock* next) const { DCHECK_EQ((*block_order_)[current_block_index_], current); return GetNextBlockToEmit() == FirstNonEmptyBlock(next); diff --git a/compiler/optimizing/code_generator.h b/compiler/optimizing/code_generator.h index 6f742d924c..82a54d2ed1 100644 --- a/compiler/optimizing/code_generator.h +++ b/compiler/optimizing/code_generator.h @@ -340,6 +340,11 @@ class CodeGenerator : public DeletableArenaObject<kArenaAllocCodeGenerator> { // Pointer variant for ArtMethod and ArtField arrays. size_t GetCachePointerOffset(uint32_t index); + // Helper that returns the offset of the array's length field. + // Note: Besides the normal arrays, we also use the HArrayLength for + // accessing the String's `count` field in String intrinsics. + static uint32_t GetArrayLengthOffset(HArrayLength* array_length); + void EmitParallelMoves(Location from1, Location to1, Primitive::Type type1, diff --git a/compiler/optimizing/code_generator_arm.cc b/compiler/optimizing/code_generator_arm.cc index 197e473473..e0106628c6 100644 --- a/compiler/optimizing/code_generator_arm.cc +++ b/compiler/optimizing/code_generator_arm.cc @@ -4742,7 +4742,7 @@ void LocationsBuilderARM::VisitArrayLength(HArrayLength* instruction) { void InstructionCodeGeneratorARM::VisitArrayLength(HArrayLength* instruction) { LocationSummary* locations = instruction->GetLocations(); - uint32_t offset = mirror::Array::LengthOffset().Uint32Value(); + uint32_t offset = CodeGenerator::GetArrayLengthOffset(instruction); Register obj = locations->InAt(0).AsRegister<Register>(); Register out = locations->Out().AsRegister<Register>(); __ LoadFromOffset(kLoadWord, out, obj, offset); diff --git a/compiler/optimizing/code_generator_arm64.cc b/compiler/optimizing/code_generator_arm64.cc index 9680f2bf45..261c04f062 100644 --- a/compiler/optimizing/code_generator_arm64.cc +++ b/compiler/optimizing/code_generator_arm64.cc @@ -2118,9 +2118,9 @@ void LocationsBuilderARM64::VisitArrayLength(HArrayLength* instruction) { } void InstructionCodeGeneratorARM64::VisitArrayLength(HArrayLength* instruction) { + uint32_t offset = CodeGenerator::GetArrayLengthOffset(instruction); BlockPoolsScope block_pools(GetVIXLAssembler()); - __ Ldr(OutputRegister(instruction), - HeapOperand(InputRegisterAt(instruction, 0), mirror::Array::LengthOffset())); + __ Ldr(OutputRegister(instruction), HeapOperand(InputRegisterAt(instruction, 0), offset)); codegen_->MaybeRecordImplicitNullCheck(instruction); } diff --git a/compiler/optimizing/code_generator_mips.cc b/compiler/optimizing/code_generator_mips.cc index 12d1164d03..fb50680c91 100644 --- a/compiler/optimizing/code_generator_mips.cc +++ b/compiler/optimizing/code_generator_mips.cc @@ -1803,7 +1803,7 @@ void LocationsBuilderMIPS::VisitArrayLength(HArrayLength* instruction) { void InstructionCodeGeneratorMIPS::VisitArrayLength(HArrayLength* instruction) { LocationSummary* locations = instruction->GetLocations(); - uint32_t offset = mirror::Array::LengthOffset().Uint32Value(); + uint32_t offset = CodeGenerator::GetArrayLengthOffset(instruction); Register obj = locations->InAt(0).AsRegister<Register>(); Register out = locations->Out().AsRegister<Register>(); __ LoadFromOffset(kLoadWord, out, obj, offset); diff --git a/compiler/optimizing/code_generator_mips64.cc b/compiler/optimizing/code_generator_mips64.cc index 56ac38ef84..e67d8d0dc5 100644 --- a/compiler/optimizing/code_generator_mips64.cc +++ b/compiler/optimizing/code_generator_mips64.cc @@ -1426,7 +1426,7 @@ void LocationsBuilderMIPS64::VisitArrayLength(HArrayLength* instruction) { void InstructionCodeGeneratorMIPS64::VisitArrayLength(HArrayLength* instruction) { LocationSummary* locations = instruction->GetLocations(); - uint32_t offset = mirror::Array::LengthOffset().Uint32Value(); + uint32_t offset = CodeGenerator::GetArrayLengthOffset(instruction); GpuRegister obj = locations->InAt(0).AsRegister<GpuRegister>(); GpuRegister out = locations->Out().AsRegister<GpuRegister>(); __ LoadFromOffset(kLoadWord, out, obj, offset); diff --git a/compiler/optimizing/code_generator_x86.cc b/compiler/optimizing/code_generator_x86.cc index 858a79c9c9..bdbafcdf6a 100644 --- a/compiler/optimizing/code_generator_x86.cc +++ b/compiler/optimizing/code_generator_x86.cc @@ -5487,7 +5487,7 @@ void LocationsBuilderX86::VisitArrayLength(HArrayLength* instruction) { void InstructionCodeGeneratorX86::VisitArrayLength(HArrayLength* instruction) { LocationSummary* locations = instruction->GetLocations(); - uint32_t offset = mirror::Array::LengthOffset().Uint32Value(); + uint32_t offset = CodeGenerator::GetArrayLengthOffset(instruction); Register obj = locations->InAt(0).AsRegister<Register>(); Register out = locations->Out().AsRegister<Register>(); __ movl(out, Address(obj, offset)); diff --git a/compiler/optimizing/code_generator_x86_64.cc b/compiler/optimizing/code_generator_x86_64.cc index 80bcec27cd..30eca2c103 100644 --- a/compiler/optimizing/code_generator_x86_64.cc +++ b/compiler/optimizing/code_generator_x86_64.cc @@ -4962,7 +4962,7 @@ void LocationsBuilderX86_64::VisitArrayLength(HArrayLength* instruction) { void InstructionCodeGeneratorX86_64::VisitArrayLength(HArrayLength* instruction) { LocationSummary* locations = instruction->GetLocations(); - uint32_t offset = mirror::Array::LengthOffset().Uint32Value(); + uint32_t offset = CodeGenerator::GetArrayLengthOffset(instruction); CpuRegister obj = locations->InAt(0).AsRegister<CpuRegister>(); CpuRegister out = locations->Out().AsRegister<CpuRegister>(); __ movl(out, Address(obj, offset)); diff --git a/compiler/optimizing/dead_code_elimination.cc b/compiler/optimizing/dead_code_elimination.cc index 5f11024996..49cfff46d8 100644 --- a/compiler/optimizing/dead_code_elimination.cc +++ b/compiler/optimizing/dead_code_elimination.cc @@ -23,7 +23,7 @@ namespace art { static void MarkReachableBlocks(HGraph* graph, ArenaBitVector* visited) { - ArenaVector<HBasicBlock*> worklist(graph->GetArena()->Adapter()); + ArenaVector<HBasicBlock*> worklist(graph->GetArena()->Adapter(kArenaAllocDCE)); constexpr size_t kDefaultWorlistSize = 8; worklist.reserve(kDefaultWorlistSize); visited->SetBit(graph->GetEntryBlock()->GetBlockId()); diff --git a/compiler/optimizing/graph_visualizer.cc b/compiler/optimizing/graph_visualizer.cc index 9efc13f61b..2038c88e55 100644 --- a/compiler/optimizing/graph_visualizer.cc +++ b/compiler/optimizing/graph_visualizer.cc @@ -389,6 +389,11 @@ class HGraphVisualizerPrinter : public HGraphDelegateVisitor { << instance_of->MustDoNullCheck() << std::noboolalpha; } + void VisitArrayLength(HArrayLength* array_length) OVERRIDE { + StartAttributeStream("is_string_length") << std::boolalpha + << array_length->IsStringLength() << std::noboolalpha; + } + void VisitArraySet(HArraySet* array_set) OVERRIDE { StartAttributeStream("value_can_be_null") << std::boolalpha << array_set->GetValueCanBeNull() << std::noboolalpha; @@ -544,26 +549,19 @@ class HGraphVisualizerPrinter : public HGraphDelegateVisitor { } } - if (IsPass(LICM::kLoopInvariantCodeMotionPassName) - || IsPass(HDeadCodeElimination::kFinalDeadCodeEliminationPassName) - || IsPass(HDeadCodeElimination::kInitialDeadCodeEliminationPassName) - || IsPass(BoundsCheckElimination::kBoundsCheckEliminationPassName) - || IsPass(RegisterAllocator::kRegisterAllocatorPassName) - || IsPass(HGraphBuilder::kBuilderPassName)) { - HLoopInformation* info = instruction->GetBlock()->GetLoopInformation(); - if (info == nullptr) { - StartAttributeStream("loop") << "none"; + HLoopInformation* loop_info = instruction->GetBlock()->GetLoopInformation(); + if (loop_info == nullptr) { + StartAttributeStream("loop") << "none"; + } else { + StartAttributeStream("loop") << "B" << loop_info->GetHeader()->GetBlockId(); + HLoopInformation* outer = loop_info->GetPreHeader()->GetLoopInformation(); + if (outer != nullptr) { + StartAttributeStream("outer_loop") << "B" << outer->GetHeader()->GetBlockId(); } else { - StartAttributeStream("loop") << "B" << info->GetHeader()->GetBlockId(); - HLoopInformation* outer = info->GetPreHeader()->GetLoopInformation(); - if (outer != nullptr) { - StartAttributeStream("outer_loop") << "B" << outer->GetHeader()->GetBlockId(); - } else { - StartAttributeStream("outer_loop") << "none"; - } - StartAttributeStream("irreducible") - << std::boolalpha << info->IsIrreducible() << std::noboolalpha; + StartAttributeStream("outer_loop") << "none"; } + StartAttributeStream("irreducible") + << std::boolalpha << loop_info->IsIrreducible() << std::noboolalpha; } if ((IsPass(HGraphBuilder::kBuilderPassName) diff --git a/compiler/optimizing/gvn.cc b/compiler/optimizing/gvn.cc index d0d52bf6cc..1e86b75075 100644 --- a/compiler/optimizing/gvn.cc +++ b/compiler/optimizing/gvn.cc @@ -454,11 +454,16 @@ void GlobalValueNumberer::VisitBasicBlock(HBasicBlock* block) { if (!set->IsEmpty()) { if (block->IsLoopHeader()) { - if (block->GetLoopInformation()->IsIrreducible()) { + if (block->GetLoopInformation()->ContainsIrreducibleLoop()) { // To satisfy our linear scan algorithm, no instruction should flow in an irreducible - // loop header. + // loop header. We clear the set at entry of irreducible loops and any loop containing + // an irreducible loop, as in both cases, GVN can extend the liveness of an instruction + // across the irreducible loop. + // Note that, if we're not compiling OSR, we could still do GVN and introduce + // phis at irreducible loop headers. We decided it was not worth the complexity. set->Clear(); } else { + DCHECK(!block->GetLoopInformation()->IsIrreducible()); DCHECK_EQ(block->GetDominator(), block->GetLoopInformation()->GetPreHeader()); set->Kill(side_effects_.GetLoopEffects(block)); } diff --git a/compiler/optimizing/instruction_builder.cc b/compiler/optimizing/instruction_builder.cc index a834216d0c..aaddc01f1f 100644 --- a/compiler/optimizing/instruction_builder.cc +++ b/compiler/optimizing/instruction_builder.cc @@ -721,6 +721,11 @@ ArtMethod* HInstructionBuilder::ResolveMethod(uint16_t method_idx, InvokeType in DCHECK(Runtime::Current()->IsAotCompiler()); return nullptr; } + if (!methods_class->IsAssignableFrom(compiling_class.Get())) { + // We cannot statically determine the target method. The runtime will throw a + // NoSuchMethodError on this one. + return nullptr; + } ArtMethod* actual_method; if (methods_class->IsInterface()) { actual_method = methods_class->FindVirtualMethodForInterfaceSuper( diff --git a/compiler/optimizing/instruction_simplifier.cc b/compiler/optimizing/instruction_simplifier.cc index d7b3856bf4..fd79901ffc 100644 --- a/compiler/optimizing/instruction_simplifier.cc +++ b/compiler/optimizing/instruction_simplifier.cc @@ -101,6 +101,7 @@ class InstructionSimplifierVisitor : public HGraphDelegateVisitor { void SimplifyCompare(HInvoke* invoke, bool is_signum, Primitive::Type type); void SimplifyIsNaN(HInvoke* invoke); void SimplifyFP2Int(HInvoke* invoke); + void SimplifyStringIsEmptyOrLength(HInvoke* invoke); void SimplifyMemBarrier(HInvoke* invoke, MemBarrierKind barrier_kind); OptimizingCompilerStats* stats_; @@ -1673,6 +1674,27 @@ void InstructionSimplifierVisitor::SimplifyFP2Int(HInvoke* invoke) { invoke->ReplaceWithExceptInReplacementAtIndex(select, 0); // false at index 0 } +void InstructionSimplifierVisitor::SimplifyStringIsEmptyOrLength(HInvoke* invoke) { + HInstruction* str = invoke->InputAt(0); + uint32_t dex_pc = invoke->GetDexPc(); + // We treat String as an array to allow DCE and BCE to seamlessly work on strings, + // so create the HArrayLength. + HArrayLength* length = new (GetGraph()->GetArena()) HArrayLength(str, dex_pc); + length->MarkAsStringLength(); + HInstruction* replacement; + if (invoke->GetIntrinsic() == Intrinsics::kStringIsEmpty) { + // For String.isEmpty(), create the `HEqual` representing the `length == 0`. + invoke->GetBlock()->InsertInstructionBefore(length, invoke); + HIntConstant* zero = GetGraph()->GetIntConstant(0); + HEqual* equal = new (GetGraph()->GetArena()) HEqual(length, zero, dex_pc); + replacement = equal; + } else { + DCHECK_EQ(invoke->GetIntrinsic(), Intrinsics::kStringLength); + replacement = length; + } + invoke->GetBlock()->ReplaceAndRemoveInstructionWith(invoke, replacement); +} + void InstructionSimplifierVisitor::SimplifyMemBarrier(HInvoke* invoke, MemBarrierKind barrier_kind) { uint32_t dex_pc = invoke->GetDexPc(); HMemoryBarrier* mem_barrier = new (GetGraph()->GetArena()) HMemoryBarrier(barrier_kind, dex_pc); @@ -1719,6 +1741,10 @@ void InstructionSimplifierVisitor::VisitInvoke(HInvoke* instruction) { case Intrinsics::kDoubleDoubleToLongBits: SimplifyFP2Int(instruction); break; + case Intrinsics::kStringIsEmpty: + case Intrinsics::kStringLength: + SimplifyStringIsEmptyOrLength(instruction); + break; case Intrinsics::kUnsafeLoadFence: SimplifyMemBarrier(instruction, MemBarrierKind::kLoadAny); break; diff --git a/compiler/optimizing/intrinsics.cc b/compiler/optimizing/intrinsics.cc index 5d4c4e2950..418d59c6cb 100644 --- a/compiler/optimizing/intrinsics.cc +++ b/compiler/optimizing/intrinsics.cc @@ -388,10 +388,8 @@ static Intrinsics GetIntrinsic(InlineMethod method) { case kIntrinsicGetCharsNoCheck: return Intrinsics::kStringGetCharsNoCheck; case kIntrinsicIsEmptyOrLength: - // The inliner can handle these two cases - and this is the preferred approach - // since after inlining the call is no longer visible (as opposed to waiting - // until codegen to handle intrinsic). - return Intrinsics::kNone; + return ((method.d.data & kIntrinsicFlagIsEmpty) == 0) ? + Intrinsics::kStringLength : Intrinsics::kStringIsEmpty; case kIntrinsicIndexOf: return ((method.d.data & kIntrinsicFlagBase0) == 0) ? Intrinsics::kStringIndexOfAfter : Intrinsics::kStringIndexOf; diff --git a/compiler/optimizing/intrinsics.h b/compiler/optimizing/intrinsics.h index 39a1313ba0..214250f337 100644 --- a/compiler/optimizing/intrinsics.h +++ b/compiler/optimizing/intrinsics.h @@ -239,6 +239,8 @@ UNREACHABLE_INTRINSIC(Arch, IntegerCompare) \ UNREACHABLE_INTRINSIC(Arch, LongCompare) \ UNREACHABLE_INTRINSIC(Arch, IntegerSignum) \ UNREACHABLE_INTRINSIC(Arch, LongSignum) \ +UNREACHABLE_INTRINSIC(Arch, StringIsEmpty) \ +UNREACHABLE_INTRINSIC(Arch, StringLength) \ UNREACHABLE_INTRINSIC(Arch, UnsafeLoadFence) \ UNREACHABLE_INTRINSIC(Arch, UnsafeStoreFence) \ UNREACHABLE_INTRINSIC(Arch, UnsafeFullFence) diff --git a/compiler/optimizing/intrinsics_arm.cc b/compiler/optimizing/intrinsics_arm.cc index 146fea1fe0..4e3ace498d 100644 --- a/compiler/optimizing/intrinsics_arm.cc +++ b/compiler/optimizing/intrinsics_arm.cc @@ -1115,15 +1115,15 @@ static void GenerateVisitStringIndexOf(HInvoke* invoke, ArenaAllocator* allocator, bool start_at_zero) { LocationSummary* locations = invoke->GetLocations(); - Register tmp_reg = locations->GetTemp(0).AsRegister<Register>(); // Note that the null check must have been done earlier. DCHECK(!invoke->CanDoImplicitNullCheckOn(invoke->InputAt(0))); // Check for code points > 0xFFFF. Either a slow-path check when we don't know statically, - // or directly dispatch if we have a constant. + // or directly dispatch for a large constant, or omit slow-path for a small constant or a char. SlowPathCode* slow_path = nullptr; - if (invoke->InputAt(1)->IsIntConstant()) { + HInstruction* code_point = invoke->InputAt(1); + if (code_point->IsIntConstant()) { if (static_cast<uint32_t>(invoke->InputAt(1)->AsIntConstant()->GetValue()) > std::numeric_limits<uint16_t>::max()) { // Always needs the slow-path. We could directly dispatch to it, but this case should be @@ -1134,16 +1134,18 @@ static void GenerateVisitStringIndexOf(HInvoke* invoke, __ Bind(slow_path->GetExitLabel()); return; } - } else { + } else if (code_point->GetType() != Primitive::kPrimChar) { Register char_reg = locations->InAt(1).AsRegister<Register>(); - __ LoadImmediate(tmp_reg, std::numeric_limits<uint16_t>::max()); - __ cmp(char_reg, ShifterOperand(tmp_reg)); + // 0xffff is not modified immediate but 0x10000 is, so use `>= 0x10000` instead of `> 0xffff`. + __ cmp(char_reg, + ShifterOperand(static_cast<uint32_t>(std::numeric_limits<uint16_t>::max()) + 1)); slow_path = new (allocator) IntrinsicSlowPathARM(invoke); codegen->AddSlowPath(slow_path); - __ b(slow_path->GetEntryLabel(), HI); + __ b(slow_path->GetEntryLabel(), HS); } if (start_at_zero) { + Register tmp_reg = locations->GetTemp(0).AsRegister<Register>(); DCHECK_EQ(tmp_reg, R2); // Start-index = 0. __ LoadImmediate(tmp_reg, 0); @@ -1170,7 +1172,7 @@ void IntrinsicLocationsBuilderARM::VisitStringIndexOf(HInvoke* invoke) { locations->SetInAt(1, Location::RegisterLocation(calling_convention.GetRegisterAt(1))); locations->SetOut(Location::RegisterLocation(R0)); - // Need a temp for slow-path codepoint compare, and need to send start-index=0. + // Need to send start-index=0. locations->AddTemp(Location::RegisterLocation(calling_convention.GetRegisterAt(2))); } @@ -1190,9 +1192,6 @@ void IntrinsicLocationsBuilderARM::VisitStringIndexOfAfter(HInvoke* invoke) { locations->SetInAt(1, Location::RegisterLocation(calling_convention.GetRegisterAt(1))); locations->SetInAt(2, Location::RegisterLocation(calling_convention.GetRegisterAt(2))); locations->SetOut(Location::RegisterLocation(R0)); - - // Need a temp for slow-path codepoint compare. - locations->AddTemp(Location::RequiresRegister()); } void IntrinsicCodeGeneratorARM::VisitStringIndexOfAfter(HInvoke* invoke) { diff --git a/compiler/optimizing/intrinsics_arm64.cc b/compiler/optimizing/intrinsics_arm64.cc index 1d8229674c..cc5fd65e2e 100644 --- a/compiler/optimizing/intrinsics_arm64.cc +++ b/compiler/optimizing/intrinsics_arm64.cc @@ -1390,15 +1390,15 @@ static void GenerateVisitStringIndexOf(HInvoke* invoke, ArenaAllocator* allocator, bool start_at_zero) { LocationSummary* locations = invoke->GetLocations(); - Register tmp_reg = WRegisterFrom(locations->GetTemp(0)); // Note that the null check must have been done earlier. DCHECK(!invoke->CanDoImplicitNullCheckOn(invoke->InputAt(0))); // Check for code points > 0xFFFF. Either a slow-path check when we don't know statically, - // or directly dispatch if we have a constant. + // or directly dispatch for a large constant, or omit slow-path for a small constant or a char. SlowPathCodeARM64* slow_path = nullptr; - if (invoke->InputAt(1)->IsIntConstant()) { + HInstruction* code_point = invoke->InputAt(1); + if (code_point->IsIntConstant()) { if (static_cast<uint32_t>(invoke->InputAt(1)->AsIntConstant()->GetValue()) > 0xFFFFU) { // Always needs the slow-path. We could directly dispatch to it, but this case should be // rare, so for simplicity just put the full slow-path down and branch unconditionally. @@ -1408,17 +1408,17 @@ static void GenerateVisitStringIndexOf(HInvoke* invoke, __ Bind(slow_path->GetExitLabel()); return; } - } else { + } else if (code_point->GetType() != Primitive::kPrimChar) { Register char_reg = WRegisterFrom(locations->InAt(1)); - __ Mov(tmp_reg, 0xFFFF); - __ Cmp(char_reg, Operand(tmp_reg)); + __ Tst(char_reg, 0xFFFF0000); slow_path = new (allocator) IntrinsicSlowPathARM64(invoke); codegen->AddSlowPath(slow_path); - __ B(hi, slow_path->GetEntryLabel()); + __ B(ne, slow_path->GetEntryLabel()); } if (start_at_zero) { // Start-index = 0. + Register tmp_reg = WRegisterFrom(locations->GetTemp(0)); __ Mov(tmp_reg, 0); } @@ -1442,7 +1442,7 @@ void IntrinsicLocationsBuilderARM64::VisitStringIndexOf(HInvoke* invoke) { locations->SetInAt(1, LocationFrom(calling_convention.GetRegisterAt(1))); locations->SetOut(calling_convention.GetReturnLocation(Primitive::kPrimInt)); - // Need a temp for slow-path codepoint compare, and need to send start_index=0. + // Need to send start_index=0. locations->AddTemp(LocationFrom(calling_convention.GetRegisterAt(2))); } @@ -1462,9 +1462,6 @@ void IntrinsicLocationsBuilderARM64::VisitStringIndexOfAfter(HInvoke* invoke) { locations->SetInAt(1, LocationFrom(calling_convention.GetRegisterAt(1))); locations->SetInAt(2, LocationFrom(calling_convention.GetRegisterAt(2))); locations->SetOut(calling_convention.GetReturnLocation(Primitive::kPrimInt)); - - // Need a temp for slow-path codepoint compare. - locations->AddTemp(Location::RequiresRegister()); } void IntrinsicCodeGeneratorARM64::VisitStringIndexOfAfter(HInvoke* invoke) { diff --git a/compiler/optimizing/intrinsics_list.h b/compiler/optimizing/intrinsics_list.h index dd9294d486..db60238fb4 100644 --- a/compiler/optimizing/intrinsics_list.h +++ b/compiler/optimizing/intrinsics_list.h @@ -107,6 +107,8 @@ V(StringGetCharsNoCheck, kDirect, kNeedsEnvironmentOrCache, kReadSideEffects, kCanThrow) \ V(StringIndexOf, kDirect, kNeedsEnvironmentOrCache, kReadSideEffects, kCanThrow) \ V(StringIndexOfAfter, kDirect, kNeedsEnvironmentOrCache, kReadSideEffects, kCanThrow) \ + V(StringIsEmpty, kDirect, kNeedsEnvironmentOrCache, kReadSideEffects, kNoThrow) \ + V(StringLength, kDirect, kNeedsEnvironmentOrCache, kReadSideEffects, kNoThrow) \ V(StringNewStringFromBytes, kStatic, kNeedsEnvironmentOrCache, kAllSideEffects, kCanThrow) \ V(StringNewStringFromChars, kStatic, kNeedsEnvironmentOrCache, kAllSideEffects, kCanThrow) \ V(StringNewStringFromString, kStatic, kNeedsEnvironmentOrCache, kAllSideEffects, kCanThrow) \ diff --git a/compiler/optimizing/intrinsics_mips.cc b/compiler/optimizing/intrinsics_mips.cc index 46195c104a..20b61f8a1c 100644 --- a/compiler/optimizing/intrinsics_mips.cc +++ b/compiler/optimizing/intrinsics_mips.cc @@ -2067,10 +2067,11 @@ static void GenerateStringIndexOf(HInvoke* invoke, // Note that the null check must have been done earlier. DCHECK(!invoke->CanDoImplicitNullCheckOn(invoke->InputAt(0))); - // Check for code points > 0xFFFF. Either a slow-path check when we - // don't know statically, or directly dispatch if we have a constant. + // Check for code points > 0xFFFF. Either a slow-path check when we don't know statically, + // or directly dispatch for a large constant, or omit slow-path for a small constant or a char. SlowPathCodeMIPS* slow_path = nullptr; - if (invoke->InputAt(1)->IsIntConstant()) { + HInstruction* code_point = invoke->InputAt(1); + if (code_point->IsIntConstant()) { if (!IsUint<16>(invoke->InputAt(1)->AsIntConstant()->GetValue())) { // Always needs the slow-path. We could directly dispatch to it, // but this case should be rare, so for simplicity just put the @@ -2081,7 +2082,7 @@ static void GenerateStringIndexOf(HInvoke* invoke, __ Bind(slow_path->GetExitLabel()); return; } - } else { + } else if (code_point->GetType() != Primitive::kPrimChar) { Register char_reg = locations->InAt(1).AsRegister<Register>(); // The "bltu" conditional branch tests to see if the character value // fits in a valid 16-bit (MIPS halfword) value. If it doesn't then diff --git a/compiler/optimizing/intrinsics_mips64.cc b/compiler/optimizing/intrinsics_mips64.cc index 1524e1e011..7188e1cc75 100644 --- a/compiler/optimizing/intrinsics_mips64.cc +++ b/compiler/optimizing/intrinsics_mips64.cc @@ -1563,10 +1563,11 @@ static void GenerateStringIndexOf(HInvoke* invoke, // Note that the null check must have been done earlier. DCHECK(!invoke->CanDoImplicitNullCheckOn(invoke->InputAt(0))); - // Check for code points > 0xFFFF. Either a slow-path check when we - // don't know statically, or directly dispatch if we have a constant. + // Check for code points > 0xFFFF. Either a slow-path check when we don't know statically, + // or directly dispatch for a large constant, or omit slow-path for a small constant or a char. SlowPathCodeMIPS64* slow_path = nullptr; - if (invoke->InputAt(1)->IsIntConstant()) { + HInstruction* code_point = invoke->InputAt(1); + if (code_point->IsIntConstant()) { if (!IsUint<16>(invoke->InputAt(1)->AsIntConstant()->GetValue())) { // Always needs the slow-path. We could directly dispatch to it, // but this case should be rare, so for simplicity just put the @@ -1577,7 +1578,7 @@ static void GenerateStringIndexOf(HInvoke* invoke, __ Bind(slow_path->GetExitLabel()); return; } - } else { + } else if (code_point->GetType() != Primitive::kPrimChar) { GpuRegister char_reg = locations->InAt(1).AsRegister<GpuRegister>(); __ LoadConst32(tmp_reg, std::numeric_limits<uint16_t>::max()); slow_path = new (allocator) IntrinsicSlowPathMIPS64(invoke); diff --git a/compiler/optimizing/intrinsics_x86.cc b/compiler/optimizing/intrinsics_x86.cc index 86e904acf9..5c4736109f 100644 --- a/compiler/optimizing/intrinsics_x86.cc +++ b/compiler/optimizing/intrinsics_x86.cc @@ -1418,9 +1418,10 @@ static void GenerateStringIndexOf(HInvoke* invoke, DCHECK_EQ(out, EDI); // Check for code points > 0xFFFF. Either a slow-path check when we don't know statically, - // or directly dispatch if we have a constant. + // or directly dispatch for a large constant, or omit slow-path for a small constant or a char. SlowPathCode* slow_path = nullptr; - if (invoke->InputAt(1)->IsIntConstant()) { + HInstruction* code_point = invoke->InputAt(1); + if (code_point->IsIntConstant()) { if (static_cast<uint32_t>(invoke->InputAt(1)->AsIntConstant()->GetValue()) > std::numeric_limits<uint16_t>::max()) { // Always needs the slow-path. We could directly dispatch to it, but this case should be @@ -1431,7 +1432,7 @@ static void GenerateStringIndexOf(HInvoke* invoke, __ Bind(slow_path->GetExitLabel()); return; } - } else { + } else if (code_point->GetType() != Primitive::kPrimChar) { __ cmpl(search_value, Immediate(std::numeric_limits<uint16_t>::max())); slow_path = new (allocator) IntrinsicSlowPathX86(invoke); codegen->AddSlowPath(slow_path); diff --git a/compiler/optimizing/intrinsics_x86_64.cc b/compiler/optimizing/intrinsics_x86_64.cc index 1a7cea07f8..a65e54cf28 100644 --- a/compiler/optimizing/intrinsics_x86_64.cc +++ b/compiler/optimizing/intrinsics_x86_64.cc @@ -1517,9 +1517,10 @@ static void GenerateStringIndexOf(HInvoke* invoke, DCHECK_EQ(out.AsRegister(), RDI); // Check for code points > 0xFFFF. Either a slow-path check when we don't know statically, - // or directly dispatch if we have a constant. + // or directly dispatch for a large constant, or omit slow-path for a small constant or a char. SlowPathCode* slow_path = nullptr; - if (invoke->InputAt(1)->IsIntConstant()) { + HInstruction* code_point = invoke->InputAt(1); + if (code_point->IsIntConstant()) { if (static_cast<uint32_t>(invoke->InputAt(1)->AsIntConstant()->GetValue()) > std::numeric_limits<uint16_t>::max()) { // Always needs the slow-path. We could directly dispatch to it, but this case should be @@ -1530,7 +1531,7 @@ static void GenerateStringIndexOf(HInvoke* invoke, __ Bind(slow_path->GetExitLabel()); return; } - } else { + } else if (code_point->GetType() != Primitive::kPrimChar) { __ cmpl(search_value, Immediate(std::numeric_limits<uint16_t>::max())); slow_path = new (allocator) IntrinsicSlowPathX86_64(invoke); codegen->AddSlowPath(slow_path); diff --git a/compiler/optimizing/licm.cc b/compiler/optimizing/licm.cc index 5a0b89c90a..7543cd6c54 100644 --- a/compiler/optimizing/licm.cc +++ b/compiler/optimizing/licm.cc @@ -101,16 +101,6 @@ void LICM::Run() { SideEffects loop_effects = side_effects_.GetLoopEffects(block); HBasicBlock* pre_header = loop_info->GetPreHeader(); - bool contains_irreducible_loop = false; - if (graph_->HasIrreducibleLoops()) { - for (HBlocksInLoopIterator it_loop(*loop_info); !it_loop.Done(); it_loop.Advance()) { - if (it_loop.Current()->GetLoopInformation()->IsIrreducible()) { - contains_irreducible_loop = true; - break; - } - } - } - for (HBlocksInLoopIterator it_loop(*loop_info); !it_loop.Done(); it_loop.Advance()) { HBasicBlock* inner = it_loop.Current(); DCHECK(inner->IsInLoop()); @@ -123,11 +113,12 @@ void LICM::Run() { visited->SetBit(inner->GetBlockId()); } - if (contains_irreducible_loop) { + if (loop_info->ContainsIrreducibleLoop()) { // We cannot licm in an irreducible loop, or in a natural loop containing an // irreducible loop. continue; } + DCHECK(!loop_info->IsIrreducible()); // We can move an instruction that can throw only if it is the first // throwing instruction in the loop. Note that the first potentially diff --git a/compiler/optimizing/load_store_elimination.cc b/compiler/optimizing/load_store_elimination.cc index 2de41580b6..8a75a90cfd 100644 --- a/compiler/optimizing/load_store_elimination.cc +++ b/compiler/optimizing/load_store_elimination.cc @@ -733,19 +733,14 @@ class LSEVisitor : public HGraphVisitor { if (Primitive::PrimitiveKind(heap_value->GetType()) != Primitive::PrimitiveKind(instruction->GetType())) { // The only situation where the same heap location has different type is when - // we do an array get from a null constant. In order to stay properly typed - // we do not merge the array gets. + // we do an array get on an instruction that originates from the null constant + // (the null could be behind a field access, an array access, a null check or + // a bound type). + // In order to stay properly typed on primitive types, we do not eliminate + // the array gets. if (kIsDebugBuild) { DCHECK(heap_value->IsArrayGet()) << heap_value->DebugName(); DCHECK(instruction->IsArrayGet()) << instruction->DebugName(); - HInstruction* array = instruction->AsArrayGet()->GetArray(); - DCHECK(array->IsNullCheck()) << array->DebugName(); - HInstruction* input = HuntForOriginalReference(array->InputAt(0)); - DCHECK(input->IsNullConstant()) << input->DebugName(); - array = heap_value->AsArrayGet()->GetArray(); - DCHECK(array->IsNullCheck()) << array->DebugName(); - input = HuntForOriginalReference(array->InputAt(0)); - DCHECK(input->IsNullConstant()) << input->DebugName(); } return; } diff --git a/compiler/optimizing/nodes.cc b/compiler/optimizing/nodes.cc index 679c274e62..60329ccff2 100644 --- a/compiler/optimizing/nodes.cc +++ b/compiler/optimizing/nodes.cc @@ -56,9 +56,11 @@ void HGraph::FindBackEdges(ArenaBitVector* visited) { // Nodes that we're currently visiting, indexed by block id. ArenaBitVector visiting(arena_, blocks_.size(), false, kArenaAllocGraphBuilder); // Number of successors visited from a given node, indexed by block id. - ArenaVector<size_t> successors_visited(blocks_.size(), 0u, arena_->Adapter()); + ArenaVector<size_t> successors_visited(blocks_.size(), + 0u, + arena_->Adapter(kArenaAllocGraphBuilder)); // Stack of nodes that we're currently visiting (same as marked in "visiting" above). - ArenaVector<HBasicBlock*> worklist(arena_->Adapter()); + ArenaVector<HBasicBlock*> worklist(arena_->Adapter(kArenaAllocGraphBuilder)); constexpr size_t kDefaultWorklistSize = 8; worklist.reserve(kDefaultWorklistSize); visited->SetBit(entry_block_->GetBlockId()); @@ -206,17 +208,35 @@ HInstruction* HBasicBlock::GetFirstInstructionDisregardMoves() const { return instruction; } +static bool UpdateDominatorOfSuccessor(HBasicBlock* block, HBasicBlock* successor) { + DCHECK(ContainsElement(block->GetSuccessors(), successor)); + + HBasicBlock* old_dominator = successor->GetDominator(); + HBasicBlock* new_dominator = + (old_dominator == nullptr) ? block + : CommonDominator::ForPair(old_dominator, block); + + if (old_dominator == new_dominator) { + return false; + } else { + successor->SetDominator(new_dominator); + return true; + } +} + void HGraph::ComputeDominanceInformation() { DCHECK(reverse_post_order_.empty()); reverse_post_order_.reserve(blocks_.size()); reverse_post_order_.push_back(entry_block_); // Number of visits of a given node, indexed by block id. - ArenaVector<size_t> visits(blocks_.size(), 0u, arena_->Adapter()); + ArenaVector<size_t> visits(blocks_.size(), 0u, arena_->Adapter(kArenaAllocGraphBuilder)); // Number of successors visited from a given node, indexed by block id. - ArenaVector<size_t> successors_visited(blocks_.size(), 0u, arena_->Adapter()); + ArenaVector<size_t> successors_visited(blocks_.size(), + 0u, + arena_->Adapter(kArenaAllocGraphBuilder)); // Nodes for which we need to visit successors. - ArenaVector<HBasicBlock*> worklist(arena_->Adapter()); + ArenaVector<HBasicBlock*> worklist(arena_->Adapter(kArenaAllocGraphBuilder)); constexpr size_t kDefaultWorklistSize = 8; worklist.reserve(kDefaultWorklistSize); worklist.push_back(entry_block_); @@ -228,15 +248,7 @@ void HGraph::ComputeDominanceInformation() { worklist.pop_back(); } else { HBasicBlock* successor = current->GetSuccessors()[successors_visited[current_id]++]; - - if (successor->GetDominator() == nullptr) { - successor->SetDominator(current); - } else { - // The CommonDominator can work for multiple blocks as long as the - // domination information doesn't change. However, since we're changing - // that information here, we can use the finder only for pairs of blocks. - successor->SetDominator(CommonDominator::ForPair(successor->GetDominator(), current)); - } + UpdateDominatorOfSuccessor(current, successor); // Once all the forward edges have been visited, we know the immediate // dominator of the block. We can then start visiting its successors. @@ -248,6 +260,44 @@ void HGraph::ComputeDominanceInformation() { } } + // Check if the graph has back edges not dominated by their respective headers. + // If so, we need to update the dominators of those headers and recursively of + // their successors. We do that with a fix-point iteration over all blocks. + // The algorithm is guaranteed to terminate because it loops only if the sum + // of all dominator chains has decreased in the current iteration. + bool must_run_fix_point = false; + for (HBasicBlock* block : blocks_) { + if (block != nullptr && + block->IsLoopHeader() && + block->GetLoopInformation()->HasBackEdgeNotDominatedByHeader()) { + must_run_fix_point = true; + break; + } + } + if (must_run_fix_point) { + bool update_occurred = true; + while (update_occurred) { + update_occurred = false; + for (HReversePostOrderIterator it(*this); !it.Done(); it.Advance()) { + HBasicBlock* block = it.Current(); + for (HBasicBlock* successor : block->GetSuccessors()) { + update_occurred |= UpdateDominatorOfSuccessor(block, successor); + } + } + } + } + + // Make sure that there are no remaining blocks whose dominator information + // needs to be updated. + if (kIsDebugBuild) { + for (HReversePostOrderIterator it(*this); !it.Done(); it.Advance()) { + HBasicBlock* block = it.Current(); + for (HBasicBlock* successor : block->GetSuccessors()) { + DCHECK(!UpdateDominatorOfSuccessor(block, successor)); + } + } + } + // Populate `dominated_blocks_` information after computing all dominators. // The potential presence of irreducible loops requires to do it after. for (HReversePostOrderIterator it(*this); !it.Done(); it.Advance()) { @@ -396,8 +446,10 @@ void HGraph::SimplifyCFG() { } GraphAnalysisResult HGraph::AnalyzeLoops() const { - // Order does not matter. - for (HReversePostOrderIterator it(*this); !it.Done(); it.Advance()) { + // We iterate post order to ensure we visit inner loops before outer loops. + // `PopulateRecursive` needs this guarantee to know whether a natural loop + // contains an irreducible loop. + for (HPostOrderIterator it(*this); !it.Done(); it.Advance()) { HBasicBlock* block = it.Current(); if (block->IsLoopHeader()) { if (block->IsCatchBlock()) { @@ -530,6 +582,14 @@ void HLoopInformation::PopulateRecursive(HBasicBlock* block) { blocks_.SetBit(block->GetBlockId()); block->SetInLoop(this); + if (block->IsLoopHeader()) { + // We're visiting loops in post-order, so inner loops must have been + // populated already. + DCHECK(block->GetLoopInformation()->IsPopulated()); + if (block->GetLoopInformation()->IsIrreducible()) { + contains_irreducible_loop_ = true; + } + } for (HBasicBlock* predecessor : block->GetPredecessors()) { PopulateRecursive(predecessor); } @@ -595,14 +655,7 @@ void HLoopInformation::Populate() { blocks_.SetBit(header_->GetBlockId()); header_->SetInLoop(this); - bool is_irreducible_loop = false; - for (HBasicBlock* back_edge : GetBackEdges()) { - DCHECK(back_edge->GetDominator() != nullptr); - if (!header_->Dominates(back_edge)) { - is_irreducible_loop = true; - break; - } - } + bool is_irreducible_loop = HasBackEdgeNotDominatedByHeader(); if (is_irreducible_loop) { ArenaBitVector visited(graph->GetArena(), @@ -640,6 +693,7 @@ void HLoopInformation::Populate() { } if (is_irreducible_loop) { irreducible_ = true; + contains_irreducible_loop_ = true; graph->SetHasIrreducibleLoops(true); } } @@ -670,6 +724,16 @@ size_t HLoopInformation::GetLifetimeEnd() const { return last_position; } +bool HLoopInformation::HasBackEdgeNotDominatedByHeader() const { + for (HBasicBlock* back_edge : GetBackEdges()) { + DCHECK(back_edge->GetDominator() != nullptr); + if (!header_->Dominates(back_edge)) { + return true; + } + } + return false; +} + bool HBasicBlock::Dominates(HBasicBlock* other) const { // Walk up the dominator tree from `other`, to find out if `this` // is an ancestor. @@ -2399,6 +2463,7 @@ void HLoadString::SetLoadKindInternal(LoadKind load_kind) { } if (!NeedsEnvironment()) { RemoveEnvironment(); + SetSideEffects(SideEffects::None()); } } diff --git a/compiler/optimizing/nodes.h b/compiler/optimizing/nodes.h index afb995d438..12ea059d3f 100644 --- a/compiler/optimizing/nodes.h +++ b/compiler/optimizing/nodes.h @@ -650,6 +650,7 @@ class HLoopInformation : public ArenaObject<kArenaAllocLoopInfo> { : header_(header), suspend_check_(nullptr), irreducible_(false), + contains_irreducible_loop_(false), back_edges_(graph->GetArena()->Adapter(kArenaAllocLoopInfoBackEdges)), // Make bit vector growable, as the number of blocks may change. blocks_(graph->GetArena(), graph->GetBlocks().size(), true, kArenaAllocLoopInfoBackEdges) { @@ -657,6 +658,7 @@ class HLoopInformation : public ArenaObject<kArenaAllocLoopInfo> { } bool IsIrreducible() const { return irreducible_; } + bool ContainsIrreducibleLoop() const { return contains_irreducible_loop_; } void Dump(std::ostream& os); @@ -725,6 +727,12 @@ class HLoopInformation : public ArenaObject<kArenaAllocLoopInfo> { blocks_.ClearAllBits(); } + bool HasBackEdgeNotDominatedByHeader() const; + + bool IsPopulated() const { + return blocks_.GetHighestBitSet() != -1; + } + private: // Internal recursive implementation of `Populate`. void PopulateRecursive(HBasicBlock* block); @@ -733,6 +741,7 @@ class HLoopInformation : public ArenaObject<kArenaAllocLoopInfo> { HBasicBlock* header_; HSuspendCheck* suspend_check_; bool irreducible_; + bool contains_irreducible_loop_; ArenaVector<HBasicBlock*> back_edges_; ArenaBitVector blocks_; @@ -5226,9 +5235,22 @@ class HArrayLength : public HExpression<1> { return obj == InputAt(0); } + void MarkAsStringLength() { SetPackedFlag<kFlagIsStringLength>(); } + bool IsStringLength() const { return GetPackedFlag<kFlagIsStringLength>(); } + DECLARE_INSTRUCTION(ArrayLength); private: + // We treat a String as an array, creating the HArrayLength from String.length() + // or String.isEmpty() intrinsic in the instruction simplifier. We can always + // determine whether a particular HArrayLength is actually a String.length() by + // looking at the type of the input but that requires holding the mutator lock, so + // we prefer to use a flag, so that code generators don't need to do the locking. + static constexpr size_t kFlagIsStringLength = kNumberOfExpressionPackedBits; + static constexpr size_t kNumberOfArrayLengthPackedBits = kFlagIsStringLength + 1; + static_assert(kNumberOfArrayLengthPackedBits <= HInstruction::kMaxNumberOfPackedBits, + "Too many packed fields."); + DISALLOW_COPY_AND_ASSIGN(HArrayLength); }; @@ -5530,6 +5552,7 @@ class HLoadString : public HExpression<1> { SetPackedFlag<kFlagIsInDexCache>(true); DCHECK(!NeedsEnvironment()); RemoveEnvironment(); + SetSideEffects(SideEffects::None()); } size_t InputCount() const OVERRIDE { diff --git a/compiler/optimizing/register_allocator.cc b/compiler/optimizing/register_allocator.cc index b1f9cbcdfa..4405b803e0 100644 --- a/compiler/optimizing/register_allocator.cc +++ b/compiler/optimizing/register_allocator.cc @@ -1773,7 +1773,9 @@ void RegisterAllocator::ConnectSplitSiblings(LiveInterval* interval, // therefore will not have a location for that instruction for `to`. // Because the instruction is a constant or the ArtMethod, we don't need to // do anything: it will be materialized in the irreducible loop. - DCHECK(IsMaterializableEntryBlockInstructionOfGraphWithIrreducibleLoop(defined_by)); + DCHECK(IsMaterializableEntryBlockInstructionOfGraphWithIrreducibleLoop(defined_by)) + << defined_by->DebugName() << ":" << defined_by->GetId() + << " " << from->GetBlockId() << " -> " << to->GetBlockId(); return; } diff --git a/compiler/optimizing/ssa_builder.cc b/compiler/optimizing/ssa_builder.cc index c2aa0c0075..f96ca321c9 100644 --- a/compiler/optimizing/ssa_builder.cc +++ b/compiler/optimizing/ssa_builder.cc @@ -233,7 +233,7 @@ bool SsaBuilder::UpdatePrimitiveType(HPhi* phi, ArenaVector<HPhi*>* worklist) { } void SsaBuilder::RunPrimitiveTypePropagation() { - ArenaVector<HPhi*> worklist(graph_->GetArena()->Adapter()); + ArenaVector<HPhi*> worklist(graph_->GetArena()->Adapter(kArenaAllocGraphBuilder)); for (HReversePostOrderIterator it(*graph_); !it.Done(); it.Advance()) { HBasicBlock* block = it.Current(); @@ -319,7 +319,7 @@ bool SsaBuilder::FixAmbiguousArrayOps() { // uses (because they are untyped) and environment uses (if --debuggable). // After resolving all ambiguous ArrayGets, we will re-run primitive type // propagation on the Phis which need to be updated. - ArenaVector<HPhi*> worklist(graph_->GetArena()->Adapter()); + ArenaVector<HPhi*> worklist(graph_->GetArena()->Adapter(kArenaAllocGraphBuilder)); { ScopedObjectAccess soa(Thread::Current()); diff --git a/compiler/optimizing/ssa_liveness_analysis.cc b/compiler/optimizing/ssa_liveness_analysis.cc index 5534aeac29..36e0d993d1 100644 --- a/compiler/optimizing/ssa_liveness_analysis.cc +++ b/compiler/optimizing/ssa_liveness_analysis.cc @@ -309,17 +309,8 @@ void SsaLivenessAnalysis::ComputeLiveRanges() { } if (block->IsLoopHeader()) { - if (kIsDebugBuild && block->GetLoopInformation()->IsIrreducible()) { - // To satisfy our liveness algorithm, we need to ensure loop headers of - // irreducible loops do not have any live-in instructions, except constants - // and the current method, which can be trivially re-materialized. - for (uint32_t idx : live_in->Indexes()) { - HInstruction* instruction = GetInstructionFromSsaIndex(idx); - DCHECK(instruction->GetBlock()->IsEntryBlock()) << instruction->DebugName(); - DCHECK(!instruction->IsParameterValue()); - DCHECK(instruction->IsCurrentMethod() || instruction->IsConstant()) - << instruction->DebugName(); - } + if (kIsDebugBuild) { + CheckNoLiveInIrreducibleLoop(*block); } size_t last_position = block->GetLoopInformation()->GetLifetimeEnd(); // For all live_in instructions at the loop header, we need to create a range @@ -344,6 +335,9 @@ void SsaLivenessAnalysis::ComputeLiveInAndLiveOutSets() { // change in this loop), and the live_out set. If the live_out // set does not change, there is no need to update the live_in set. if (UpdateLiveOut(block) && UpdateLiveIn(block)) { + if (kIsDebugBuild) { + CheckNoLiveInIrreducibleLoop(block); + } changed = true; } } diff --git a/compiler/optimizing/ssa_liveness_analysis.h b/compiler/optimizing/ssa_liveness_analysis.h index 1141fd1c76..1fcba8bc77 100644 --- a/compiler/optimizing/ssa_liveness_analysis.h +++ b/compiler/optimizing/ssa_liveness_analysis.h @@ -1260,6 +1260,23 @@ class SsaLivenessAnalysis : public ValueObject { return instruction->GetType() == Primitive::kPrimNot; } + void CheckNoLiveInIrreducibleLoop(const HBasicBlock& block) const { + if (!block.IsLoopHeader() || !block.GetLoopInformation()->IsIrreducible()) { + return; + } + BitVector* live_in = GetLiveInSet(block); + // To satisfy our liveness algorithm, we need to ensure loop headers of + // irreducible loops do not have any live-in instructions, except constants + // and the current method, which can be trivially re-materialized. + for (uint32_t idx : live_in->Indexes()) { + HInstruction* instruction = GetInstructionFromSsaIndex(idx); + DCHECK(instruction->GetBlock()->IsEntryBlock()) << instruction->DebugName(); + DCHECK(!instruction->IsParameterValue()); + DCHECK(instruction->IsCurrentMethod() || instruction->IsConstant()) + << instruction->DebugName(); + } + } + HGraph* const graph_; CodeGenerator* const codegen_; ArenaVector<BlockInfo*> block_infos_; diff --git a/compiler/optimizing/ssa_phi_elimination.cc b/compiler/optimizing/ssa_phi_elimination.cc index 0978ae17f8..c67612e651 100644 --- a/compiler/optimizing/ssa_phi_elimination.cc +++ b/compiler/optimizing/ssa_phi_elimination.cc @@ -17,6 +17,7 @@ #include "ssa_phi_elimination.h" #include "base/arena_containers.h" +#include "base/arena_bit_vector.h" #include "base/bit_vector-inl.h" namespace art { @@ -30,7 +31,7 @@ void SsaDeadPhiElimination::MarkDeadPhis() { // Phis are constructed live and should not be revived if previously marked // dead. This algorithm temporarily breaks that invariant but we DCHECK that // only phis which were initially live are revived. - ArenaSet<HPhi*> initially_live(graph_->GetArena()->Adapter()); + ArenaSet<HPhi*> initially_live(graph_->GetArena()->Adapter(kArenaAllocSsaPhiElimination)); // Add to the worklist phis referenced by non-phi instructions. for (HReversePostOrderIterator it(*graph_); !it.Done(); it.Advance()) { @@ -127,8 +128,11 @@ void SsaRedundantPhiElimination::Run() { } } - ArenaSet<uint32_t> visited_phis_in_cycle(graph_->GetArena()->Adapter()); - ArenaVector<HPhi*> cycle_worklist(graph_->GetArena()->Adapter()); + ArenaBitVector visited_phis_in_cycle(graph_->GetArena(), + graph_->GetCurrentInstructionId(), + /* expandable */ false, + kArenaAllocSsaPhiElimination); + ArenaVector<HPhi*> cycle_worklist(graph_->GetArena()->Adapter(kArenaAllocSsaPhiElimination)); while (!worklist_.empty()) { HPhi* phi = worklist_.back(); @@ -146,11 +150,11 @@ void SsaRedundantPhiElimination::Run() { } HInstruction* candidate = nullptr; - visited_phis_in_cycle.clear(); + visited_phis_in_cycle.ClearAllBits(); cycle_worklist.clear(); cycle_worklist.push_back(phi); - visited_phis_in_cycle.insert(phi->GetId()); + visited_phis_in_cycle.SetBit(phi->GetId()); bool catch_phi_in_cycle = phi->IsCatchPhi(); bool irreducible_loop_phi_in_cycle = phi->IsIrreducibleLoopHeaderPhi(); @@ -182,9 +186,9 @@ void SsaRedundantPhiElimination::Run() { if (input == current) { continue; } else if (input->IsPhi()) { - if (!ContainsElement(visited_phis_in_cycle, input->GetId())) { + if (!visited_phis_in_cycle.IsBitSet(input->GetId())) { cycle_worklist.push_back(input->AsPhi()); - visited_phis_in_cycle.insert(input->GetId()); + visited_phis_in_cycle.SetBit(input->GetId()); catch_phi_in_cycle |= input->AsPhi()->IsCatchPhi(); irreducible_loop_phi_in_cycle |= input->IsIrreducibleLoopHeaderPhi(); } else { @@ -233,7 +237,7 @@ void SsaRedundantPhiElimination::Run() { // for elimination. Add phis that use this phi to the worklist. for (const HUseListNode<HInstruction*>& use : current->GetUses()) { HInstruction* user = use.GetUser(); - if (user->IsPhi() && !ContainsElement(visited_phis_in_cycle, user->GetId())) { + if (user->IsPhi() && !visited_phis_in_cycle.IsBitSet(user->GetId())) { worklist_.push_back(user->AsPhi()); } } diff --git a/profman/profman.cc b/profman/profman.cc index b3454fa2e7..37a560d203 100644 --- a/profman/profman.cc +++ b/profman/profman.cc @@ -219,7 +219,7 @@ class ProfMan FINAL { static constexpr uint64_t kLogThresholdTime = MsToNs(100); // 100ms uint64_t time_taken = NanoTime() - start_ns_; if (time_taken > kLogThresholdTime) { - LOG(WARNING) << "profman took " << PrettyDuration(NanoTime() - start_ns_); + LOG(WARNING) << "profman took " << PrettyDuration(time_taken); } } diff --git a/runtime/arch/x86_64/quick_entrypoints_x86_64.S b/runtime/arch/x86_64/quick_entrypoints_x86_64.S index 562ee2d810..8064ed696f 100644 --- a/runtime/arch/x86_64/quick_entrypoints_x86_64.S +++ b/runtime/arch/x86_64/quick_entrypoints_x86_64.S @@ -894,57 +894,107 @@ DEFINE_FUNCTION art_quick_alloc_object_rosalloc RETURN_IF_RESULT_IS_NON_ZERO_OR_DELIVER // return or deliver exception END_FUNCTION art_quick_alloc_object_rosalloc -// A handle-written override for GENERATE_ALLOC_ENTRYPOINTS_ALLOC_OBJECT(_tlab, TLAB). -DEFINE_FUNCTION art_quick_alloc_object_tlab - // Fast path tlab allocation. - // RDI: uint32_t type_idx, RSI: ArtMethod* - // RDX, RCX, R8, R9: free. RAX: return val. - // TODO: Add read barrier when this function is used. - // Note this function can/should implement read barrier fast path only - // (no read barrier slow path) because this is the fast path of tlab allocation. - // We can fall back to the allocation slow path to do the read barrier slow path. -#if defined(USE_READ_BARRIER) - int3 - int3 -#endif - // Might need a special macro since rsi and edx is 32b/64b mismatched. - movq ART_METHOD_DEX_CACHE_TYPES_OFFSET_64(%rsi), %rdx // Load dex cache resolved types array - // TODO: Add read barrier when this function is used. - // Might need to break down into multiple instructions to get the base address in a register. - // Load the class - movl 0(%rdx, %rdi, COMPRESSED_REFERENCE_SIZE), %edx +// The common fast path code for art_quick_alloc_object_tlab and art_quick_alloc_object_region_tlab. +// +// RDI: type_idx, RSI: ArtMethod*, RDX/EDX: the class, RAX: return value. +// RCX: scratch, r8: Thread::Current(). +MACRO1(ALLOC_OBJECT_TLAB_FAST_PATH, slowPathLabel) testl %edx, %edx // Check null class - jz .Lart_quick_alloc_object_tlab_slow_path + jz RAW_VAR(slowPathLabel) // Check class status. cmpl LITERAL(MIRROR_CLASS_STATUS_INITIALIZED), MIRROR_CLASS_STATUS_OFFSET(%rdx) - jne .Lart_quick_alloc_object_tlab_slow_path - // Check access flags has kAccClassIsFinalizable + jne RAW_VAR(slowPathLabel) + // No fake dependence needed on x86 + // between status and flags load, + // since each load is a load-acquire, + // no loads reordering. + // Check access flags has + // kAccClassIsFinalizable testl LITERAL(ACCESS_FLAGS_CLASS_IS_FINALIZABLE), MIRROR_CLASS_ACCESS_FLAGS_OFFSET(%rdx) - jnz .Lart_quick_alloc_object_tlab_slow_path + jnz RAW_VAR(slowPathLabel) + movq %gs:THREAD_SELF_OFFSET, %r8 // r8 = thread + movq THREAD_LOCAL_END_OFFSET(%r8), %rax // Load thread_local_end. + subq THREAD_LOCAL_POS_OFFSET(%r8), %rax // Compute the remaining buffer size. movl MIRROR_CLASS_OBJECT_SIZE_OFFSET(%rdx), %ecx // Load the object size. + cmpq %rax, %rcx // Check if it fits. OK to do this + // before rounding up the object size + // assuming the buf size alignment. + ja RAW_VAR(slowPathLabel) addl LITERAL(OBJECT_ALIGNMENT_MASK), %ecx // Align the size by 8. (addr + 7) & ~7. andl LITERAL(OBJECT_ALIGNMENT_MASK_TOGGLED), %ecx - movq %gs:THREAD_SELF_OFFSET, %r8 // r8 = thread - movq THREAD_LOCAL_POS_OFFSET(%r8), %rax // Load thread_local_pos. + movq THREAD_LOCAL_POS_OFFSET(%r8), %rax // Load thread_local_pos + // as allocated object. addq %rax, %rcx // Add the object size. - cmpq THREAD_LOCAL_END_OFFSET(%r8), %rcx // Check if it fits. - ja .Lart_quick_alloc_object_tlab_slow_path movq %rcx, THREAD_LOCAL_POS_OFFSET(%r8) // Update thread_local_pos. - addq LITERAL(1), THREAD_LOCAL_OBJECTS_OFFSET(%r8) // Increment thread_local_objects. + addq LITERAL(1), THREAD_LOCAL_OBJECTS_OFFSET(%r8) // Increase thread_local_objects. // Store the class pointer in the header. // No fence needed for x86. + POISON_HEAP_REF edx movl %edx, MIRROR_OBJECT_CLASS_OFFSET(%rax) ret // Fast path succeeded. -.Lart_quick_alloc_object_tlab_slow_path: +END_MACRO + +// The common slow path code for art_quick_alloc_object_tlab and art_quick_alloc_object_region_tlab. +MACRO1(ALLOC_OBJECT_TLAB_SLOW_PATH, cxx_name) SETUP_REFS_ONLY_CALLEE_SAVE_FRAME // save ref containing registers for GC // Outgoing argument set up movq %gs:THREAD_SELF_OFFSET, %rdx // pass Thread::Current() - call SYMBOL(artAllocObjectFromCodeTLAB) // cxx_name(arg0, arg1, Thread*) + call VAR(cxx_name) // cxx_name(arg0, arg1, Thread*) RESTORE_REFS_ONLY_CALLEE_SAVE_FRAME // restore frame up to return address RETURN_IF_RESULT_IS_NON_ZERO_OR_DELIVER // return or deliver exception +END_MACRO + +// A hand-written override for GENERATE_ALLOC_ENTRYPOINTS_ALLOC_OBJECT(_tlab, TLAB). +DEFINE_FUNCTION art_quick_alloc_object_tlab + // Fast path tlab allocation. + // RDI: uint32_t type_idx, RSI: ArtMethod* + // RDX, RCX, R8, R9: free. RAX: return val. +#if defined(USE_READ_BARRIER) + int3 + int3 +#endif + // Might need a special macro since rsi and edx is 32b/64b mismatched. + movq ART_METHOD_DEX_CACHE_TYPES_OFFSET_64(%rsi), %rdx // Load dex cache resolved types array + // Might need to break down into multiple instructions to get the base address in a register. + // Load the class + movl 0(%rdx, %rdi, COMPRESSED_REFERENCE_SIZE), %edx + ALLOC_OBJECT_TLAB_FAST_PATH .Lart_quick_alloc_object_tlab_slow_path +.Lart_quick_alloc_object_tlab_slow_path: + ALLOC_OBJECT_TLAB_SLOW_PATH artAllocObjectFromCodeTLAB END_FUNCTION art_quick_alloc_object_tlab -GENERATE_ALLOC_ENTRYPOINTS_ALLOC_OBJECT(_region_tlab, RegionTLAB) +// A hand-written override for GENERATE_ALLOC_ENTRYPOINTS_ALLOC_OBJECT(_region_tlab, RegionTLAB). +DEFINE_FUNCTION art_quick_alloc_object_region_tlab + // Fast path region tlab allocation. + // RDI: uint32_t type_idx, RSI: ArtMethod* + // RDX, RCX, R8, R9: free. RAX: return val. +#if !defined(USE_READ_BARRIER) + int3 + int3 +#endif + // Might need a special macro since rsi and edx is 32b/64b mismatched. + movq ART_METHOD_DEX_CACHE_TYPES_OFFSET_64(%rsi), %rdx // Load dex cache resolved types array + // Might need to break down into multiple instructions to get the base address in a register. + // Load the class + movl 0(%rdx, %rdi, COMPRESSED_REFERENCE_SIZE), %edx + cmpl LITERAL(0), %gs:THREAD_IS_GC_MARKING_OFFSET + jne .Lart_quick_alloc_object_region_tlab_class_load_read_barrier_slow_path +.Lart_quick_alloc_object_region_tlab_class_load_read_barrier_slow_path_exit: + ALLOC_OBJECT_TLAB_FAST_PATH .Lart_quick_alloc_object_region_tlab_slow_path +.Lart_quick_alloc_object_region_tlab_class_load_read_barrier_slow_path: + // The read barrier slow path. Mark the class. + PUSH rdi + PUSH rsi + // Outgoing argument set up + movq %rdx, %rdi // Pass the class as the first param. + call SYMBOL(artReadBarrierMark) // cxx_name(mirror::Object* obj) + movq %rax, %rdx + POP rsi + POP rdi + jmp .Lart_quick_alloc_object_region_tlab_class_load_read_barrier_slow_path_exit +.Lart_quick_alloc_object_region_tlab_slow_path: + ALLOC_OBJECT_TLAB_SLOW_PATH artAllocObjectFromCodeRegionTLAB +END_FUNCTION art_quick_alloc_object_region_tlab ONE_ARG_DOWNCALL art_quick_resolve_string, artResolveStringFromCode, RETURN_IF_RESULT_IS_NON_ZERO_OR_DELIVER ONE_ARG_DOWNCALL art_quick_initialize_static_storage, artInitializeStaticStorageFromCode, RETURN_IF_RESULT_IS_NON_ZERO_OR_DELIVER diff --git a/runtime/art_method.cc b/runtime/art_method.cc index 06156f5cf8..1790df6be4 100644 --- a/runtime/art_method.cc +++ b/runtime/art_method.cc @@ -253,14 +253,17 @@ void ArtMethod::Invoke(Thread* self, uint32_t* args, uint32_t args_size, JValue* Runtime* runtime = Runtime::Current(); // Call the invoke stub, passing everything as arguments. // If the runtime is not yet started or it is required by the debugger, then perform the - // Invocation by the interpreter. + // Invocation by the interpreter, explicitly forcing interpretation over JIT to prevent + // cycling around the various JIT/Interpreter methods that handle method invocation. if (UNLIKELY(!runtime->IsStarted() || Dbg::IsForcedInterpreterNeededForCalling(self, this))) { if (IsStatic()) { - art::interpreter::EnterInterpreterFromInvoke(self, this, nullptr, args, result); + art::interpreter::EnterInterpreterFromInvoke( + self, this, nullptr, args, result, /*stay_in_interpreter*/ true); } else { mirror::Object* receiver = reinterpret_cast<StackReference<mirror::Object>*>(&args[0])->AsMirrorPtr(); - art::interpreter::EnterInterpreterFromInvoke(self, this, receiver, args + 1, result); + art::interpreter::EnterInterpreterFromInvoke( + self, this, receiver, args + 1, result, /*stay_in_interpreter*/ true); } } else { DCHECK_EQ(runtime->GetClassLinker()->GetImagePointerSize(), sizeof(void*)); diff --git a/runtime/base/histogram.h b/runtime/base/histogram.h index bcb7b3b769..0e3bc8e1b4 100644 --- a/runtime/base/histogram.h +++ b/runtime/base/histogram.h @@ -85,6 +85,10 @@ template <class Value> class Histogram { return max_value_added_; } + Value BucketWidth() const { + return bucket_width_; + } + const std::string& Name() const { return name_; } diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc index 35c40cd219..e9b8643223 100644 --- a/runtime/class_linker.cc +++ b/runtime/class_linker.cc @@ -6970,6 +6970,7 @@ bool ClassLinker::LinkInterfaceMethods( } // Put some random garbage in old methods to help find stale pointers. if (methods != old_methods && old_methods != nullptr) { + WriterMutexLock mu(self, ClassTableForClassLoader(klass->GetClassLoader())->GetLock()); memset(old_methods, 0xFEu, old_size); } } else { diff --git a/runtime/class_linker.h b/runtime/class_linker.h index ece171c9a6..d1c8172630 100644 --- a/runtime/class_linker.h +++ b/runtime/class_linker.h @@ -1020,7 +1020,7 @@ class ClassLinker { // Returns null if not found. ClassTable* ClassTableForClassLoader(mirror::ClassLoader* class_loader) - SHARED_REQUIRES(Locks::mutator_lock_, Locks::classlinker_classes_lock_); + SHARED_REQUIRES(Locks::mutator_lock_); // Insert a new class table if not found. ClassTable* InsertClassTableForClassLoader(mirror::ClassLoader* class_loader) diff --git a/runtime/class_table.h b/runtime/class_table.h index eb784b5c71..686381d35c 100644 --- a/runtime/class_table.h +++ b/runtime/class_table.h @@ -153,6 +153,10 @@ class ClassTable { REQUIRES(!lock_) SHARED_REQUIRES(Locks::mutator_lock_); + ReaderWriterMutex& GetLock() { + return lock_; + } + private: // Lock to guard inserting and removing. mutable ReaderWriterMutex lock_; diff --git a/runtime/common_runtime_test.cc b/runtime/common_runtime_test.cc index 5bdb36cafc..f58af5a8da 100644 --- a/runtime/common_runtime_test.cc +++ b/runtime/common_runtime_test.cc @@ -418,6 +418,26 @@ void CommonRuntimeTestImpl::TearDown() { (*icu_cleanup_fn)(); Runtime::Current()->GetHeap()->VerifyHeap(); // Check for heap corruption after the test + + // Manually closing the JNI libraries. + // Runtime does not support repeatedly doing JNI->CreateVM, thus we need to manually clean up the + // dynamic linking loader so that gtests would not fail. + // Bug: 25785594 + if (runtime_->IsStarted()) { + { + // We retrieve the handle by calling dlopen on the library. To close it, we need to call + // dlclose twice, the first time to undo our dlopen and the second time to actually unload it. + // See man dlopen. + void* handle = dlopen("libjavacore.so", RTLD_LAZY); + dlclose(handle); + CHECK_EQ(0, dlclose(handle)); + } + { + void* handle = dlopen("libopenjdkd.so", RTLD_LAZY); + dlclose(handle); + CHECK_EQ(0, dlclose(handle)); + } + } } static std::string GetDexFileName(const std::string& jar_prefix, bool host) { diff --git a/runtime/dex_file_verifier.cc b/runtime/dex_file_verifier.cc index 21ee4bd528..bbffbbb7b7 100644 --- a/runtime/dex_file_verifier.cc +++ b/runtime/dex_file_verifier.cc @@ -251,6 +251,14 @@ bool DexFileVerifier::CheckValidOffsetAndSize(uint32_t offset, return true; } +bool DexFileVerifier::CheckSizeLimit(uint32_t size, uint32_t limit, const char* label) { + if (size > limit) { + ErrorStringPrintf("Size(%u) should not exceed limit(%u) for %s.", size, limit, label); + return false; + } + return true; +} + bool DexFileVerifier::CheckHeader() { // Check file size from the header. uint32_t expected_size = header_->file_size_; @@ -298,10 +306,12 @@ bool DexFileVerifier::CheckHeader() { header_->type_ids_size_, 4, "type-ids") && + CheckSizeLimit(header_->type_ids_size_, DexFile::kDexNoIndex16, "type-ids") && CheckValidOffsetAndSize(header_->proto_ids_off_, header_->proto_ids_size_, 4, "proto-ids") && + CheckSizeLimit(header_->proto_ids_size_, DexFile::kDexNoIndex16, "proto-ids") && CheckValidOffsetAndSize(header_->field_ids_off_, header_->field_ids_size_, 4, @@ -1786,13 +1796,8 @@ bool DexFileVerifier::CheckInterProtoIdItem() { while (curr_it.HasNext() && prev_it.HasNext()) { uint16_t prev_idx = prev_it.GetTypeIdx(); uint16_t curr_idx = curr_it.GetTypeIdx(); - if (prev_idx == DexFile::kDexNoIndex16) { - break; - } - if (UNLIKELY(curr_idx == DexFile::kDexNoIndex16)) { - ErrorStringPrintf("Out-of-order proto_id arguments"); - return false; - } + DCHECK_NE(prev_idx, DexFile::kDexNoIndex16); + DCHECK_NE(curr_idx, DexFile::kDexNoIndex16); if (prev_idx < curr_idx) { break; @@ -1804,6 +1809,12 @@ bool DexFileVerifier::CheckInterProtoIdItem() { prev_it.Next(); curr_it.Next(); } + if (!curr_it.HasNext()) { + // Either a duplicate ProtoId or a ProtoId with a shorter argument list follows + // a ProtoId with a longer one. Both cases are forbidden by the specification. + ErrorStringPrintf("Out-of-order proto_id arguments"); + return false; + } } } @@ -2609,7 +2620,13 @@ bool DexFileVerifier::CheckMethodAccessFlags(uint32_t method_index, *error_msg = StringPrintf("Constructor %" PRIu32 "(%s) is not flagged correctly wrt/ static.", method_index, GetMethodDescriptionOrError(begin_, header_, method_index).c_str()); - return false; + if (header_->GetVersion() >= DexFile::kDefaultMethodsVersion) { + return false; + } else { + // Allow in older versions, but warn. + LOG(WARNING) << "This dex file is invalid and will be rejected in the future. Error is: " + << *error_msg; + } } } // Check that static and private methods, as well as constructors, are in the direct methods list, @@ -2663,7 +2680,13 @@ bool DexFileVerifier::CheckMethodAccessFlags(uint32_t method_index, *error_msg = StringPrintf("Constructor %u(%s) must not be abstract or native", method_index, GetMethodDescriptionOrError(begin_, header_, method_index).c_str()); - return false; + if (header_->GetVersion() >= DexFile::kDefaultMethodsVersion) { + return false; + } else { + // Allow in older versions, but warn. + LOG(WARNING) << "This dex file is invalid and will be rejected in the future. Error is: " + << *error_msg; + } } if ((method_access_flags & kAccAbstract) != 0) { // Abstract methods are not allowed to have the following flags. diff --git a/runtime/dex_file_verifier.h b/runtime/dex_file_verifier.h index be0e6d83f8..90409db44b 100644 --- a/runtime/dex_file_verifier.h +++ b/runtime/dex_file_verifier.h @@ -49,6 +49,8 @@ class DexFileVerifier { // Checks whether the offset is zero (when size is zero) or that the offset falls within the area // claimed by the file. bool CheckValidOffsetAndSize(uint32_t offset, uint32_t size, size_t alignment, const char* label); + // Checks whether the size is less than the limit. + bool CheckSizeLimit(uint32_t size, uint32_t limit, const char* label); bool CheckIndex(uint32_t field, uint32_t limit, const char* label); bool CheckHeader(); diff --git a/runtime/dex_file_verifier_test.cc b/runtime/dex_file_verifier_test.cc index 84f48daccb..3741c1e76c 100644 --- a/runtime/dex_file_verifier_test.cc +++ b/runtime/dex_file_verifier_test.cc @@ -57,6 +57,13 @@ static const uint8_t kBase64Map[256] = { 255, 255, 255, 255 }; +// Make the Dex file version 37. +static void MakeDexVersion37(DexFile* dex_file) { + size_t offset = OFFSETOF_MEMBER(DexFile::Header, magic_) + 6; + CHECK_EQ(*(dex_file->Begin() + offset), '5'); + *(const_cast<uint8_t*>(dex_file->Begin()) + offset) = '7'; +} + static inline std::unique_ptr<uint8_t[]> DecodeBase64(const char* src, size_t* dst_size) { std::vector<uint8_t> tmp; uint32_t t = 0, y = 0; @@ -438,6 +445,7 @@ TEST_F(DexFileVerifierTest, MethodAccessFlagsConstructors) { kMethodFlagsTestDex, "method_flags_constructor_native_nocode", [&](DexFile* dex_file) { + MakeDexVersion37(dex_file); ApplyMaskToMethodFlags(dex_file, "foo", ~kAccDeclaredSynchronized); ApplyMaskToMethodFlags(dex_file, "bar", ~kAccDeclaredSynchronized); @@ -450,6 +458,7 @@ TEST_F(DexFileVerifierTest, MethodAccessFlagsConstructors) { kMethodFlagsTestDex, "method_flags_constructor_abstract_nocode", [&](DexFile* dex_file) { + MakeDexVersion37(dex_file); ApplyMaskToMethodFlags(dex_file, "foo", ~kAccDeclaredSynchronized); ApplyMaskToMethodFlags(dex_file, "bar", ~kAccDeclaredSynchronized); @@ -510,6 +519,7 @@ TEST_F(DexFileVerifierTest, MethodAccessFlagsConstructors) { kMethodFlagsTestDex, "init_not_allowed_flags", [&](DexFile* dex_file) { + MakeDexVersion37(dex_file); ApplyMaskToMethodFlags(dex_file, "foo", ~kAccDeclaredSynchronized); ApplyMaskToMethodFlags(dex_file, "bar", ~kAccDeclaredSynchronized); @@ -730,13 +740,6 @@ static uint32_t ApplyMaskShifted(uint32_t src_value, uint32_t mask) { return result; } -// Make the Dex file version 37. -static void MakeDexVersion37(DexFile* dex_file) { - size_t offset = OFFSETOF_MEMBER(DexFile::Header, magic_) + 6; - CHECK_EQ(*(dex_file->Begin() + offset), '5'); - *(const_cast<uint8_t*>(dex_file->Begin()) + offset) = '7'; -} - TEST_F(DexFileVerifierTest, MethodAccessFlagsInterfaces) { VerifyModification( kMethodFlagsInterface, @@ -1441,4 +1444,81 @@ TEST_F(DexFileVerifierTest, SectionAlignment) { } } +// Generated from +// +// .class LOverloading; +// +// .super Ljava/lang/Object; +// +// .method public static foo()V +// .registers 1 +// return-void +// .end method +// +// .method public static foo(I)V +// .registers 1 +// return-void +// .end method +static const char kProtoOrderingTestDex[] = + "ZGV4CjAzNQA1L+ABE6voQ9Lr4Ci//efB53oGnDr5PinsAQAAcAAAAHhWNBIAAAAAAAAAAFgBAAAG" + "AAAAcAAAAAQAAACIAAAAAgAAAJgAAAAAAAAAAAAAAAIAAACwAAAAAQAAAMAAAAAMAQAA4AAAAOAA" + "AADjAAAA8gAAAAYBAAAJAQAADQEAAAAAAAABAAAAAgAAAAMAAAADAAAAAwAAAAAAAAAEAAAAAwAA" + "ABQBAAABAAAABQAAAAEAAQAFAAAAAQAAAAAAAAACAAAAAAAAAP////8AAAAASgEAAAAAAAABSQAN" + "TE92ZXJsb2FkaW5nOwASTGphdmEvbGFuZy9PYmplY3Q7AAFWAAJWSQADZm9vAAAAAQAAAAAAAAAA" + "AAAAAAAAAAEAAAAAAAAAAAAAAAEAAAAOAAAAAQABAAAAAAAAAAAAAQAAAA4AAAACAAAJpAIBCbgC" + "AAAMAAAAAAAAAAEAAAAAAAAAAQAAAAYAAABwAAAAAgAAAAQAAACIAAAAAwAAAAIAAACYAAAABQAA" + "AAIAAACwAAAABgAAAAEAAADAAAAAAiAAAAYAAADgAAAAARAAAAEAAAAUAQAAAxAAAAIAAAAcAQAA" + "ASAAAAIAAAAkAQAAACAAAAEAAABKAQAAABAAAAEAAABYAQAA"; + +TEST_F(DexFileVerifierTest, ProtoOrdering) { + { + // The input dex file should be good before modification. + ScratchFile tmp; + std::string error_msg; + std::unique_ptr<const DexFile> raw(OpenDexFileBase64(kProtoOrderingTestDex, + tmp.GetFilename().c_str(), + &error_msg)); + ASSERT_TRUE(raw.get() != nullptr) << error_msg; + } + + // Modify the order of the ProtoIds for two overloads of "foo" with the + // same return type and one having longer parameter list than the other. + for (size_t i = 0; i != 2; ++i) { + VerifyModification( + kProtoOrderingTestDex, + "proto_ordering", + [i](DexFile* dex_file) { + uint32_t method_idx; + const uint8_t* data = FindMethodData(dex_file, "foo", &method_idx); + CHECK(data != nullptr); + // There should be 2 methods called "foo". + CHECK_LT(method_idx + 1u, dex_file->NumMethodIds()); + CHECK_EQ(dex_file->GetMethodId(method_idx).name_idx_, + dex_file->GetMethodId(method_idx + 1).name_idx_); + CHECK_EQ(dex_file->GetMethodId(method_idx).proto_idx_ + 1u, + dex_file->GetMethodId(method_idx + 1).proto_idx_); + // Their return types should be the same. + uint32_t proto1_idx = dex_file->GetMethodId(method_idx).proto_idx_; + const DexFile::ProtoId& proto1 = dex_file->GetProtoId(proto1_idx); + const DexFile::ProtoId& proto2 = dex_file->GetProtoId(proto1_idx + 1u); + CHECK_EQ(proto1.return_type_idx_, proto2.return_type_idx_); + // And the first should not have any parameters while the second should have some. + CHECK(!DexFileParameterIterator(*dex_file, proto1).HasNext()); + CHECK(DexFileParameterIterator(*dex_file, proto2).HasNext()); + if (i == 0) { + // Swap the proto parameters and shorties to break the ordering. + std::swap(const_cast<uint32_t&>(proto1.parameters_off_), + const_cast<uint32_t&>(proto2.parameters_off_)); + std::swap(const_cast<uint32_t&>(proto1.shorty_idx_), + const_cast<uint32_t&>(proto2.shorty_idx_)); + } else { + // Copy the proto parameters and shorty to create duplicate proto id. + const_cast<uint32_t&>(proto1.parameters_off_) = proto2.parameters_off_; + const_cast<uint32_t&>(proto1.shorty_idx_) = proto2.shorty_idx_; + } + }, + "Out-of-order proto_id arguments"); + } +} + } // namespace art diff --git a/runtime/entrypoints/entrypoint_utils-inl.h b/runtime/entrypoints/entrypoint_utils-inl.h index 16fbfaad32..fc6257302a 100644 --- a/runtime/entrypoints/entrypoint_utils-inl.h +++ b/runtime/entrypoints/entrypoint_utils-inl.h @@ -514,12 +514,18 @@ inline ArtMethod* FindMethodFromCode(uint32_t method_idx, mirror::Object** this_ CHECK(self->IsExceptionPending()); return nullptr; } else if (!method_reference_class->IsInterface()) { - // It is not an interface. - mirror::Class* super_class = referring_class->GetSuperClass(); + // It is not an interface. If the referring class is in the class hierarchy of the + // referenced class in the bytecode, we use its super class. Otherwise, we throw + // a NoSuchMethodError. + mirror::Class* super_class = nullptr; + if (method_reference_class->IsAssignableFrom(referring_class)) { + super_class = referring_class->GetSuperClass(); + } uint16_t vtable_index = resolved_method->GetMethodIndex(); if (access_check) { // Check existence of super class. - if (super_class == nullptr || !super_class->HasVTable() || + if (super_class == nullptr || + !super_class->HasVTable() || vtable_index >= static_cast<uint32_t>(super_class->GetVTableLength())) { // Behavior to agree with that of the verifier. ThrowNoSuchMethodError(type, resolved_method->GetDeclaringClass(), @@ -693,8 +699,13 @@ inline ArtMethod* FindMethodFast(uint32_t method_idx, mirror::Object* this_objec // Need to do full type resolution... return nullptr; } else if (!method_reference_class->IsInterface()) { - // It is not an interface. - mirror::Class* super_class = referrer->GetDeclaringClass()->GetSuperClass(); + // It is not an interface. If the referring class is in the class hierarchy of the + // referenced class in the bytecode, we use its super class. Otherwise, we cannot + // resolve the method. + if (!method_reference_class->IsAssignableFrom(referring_class)) { + return nullptr; + } + mirror::Class* super_class = referring_class->GetSuperClass(); if (resolved_method->GetMethodIndex() >= super_class->GetVTableLength()) { // The super class does not have the method. return nullptr; diff --git a/runtime/gc/heap.cc b/runtime/gc/heap.cc index df5aa0a75c..fa540c0f9b 100644 --- a/runtime/gc/heap.cc +++ b/runtime/gc/heap.cc @@ -119,6 +119,8 @@ static constexpr uint32_t kAllocSpaceBeginForDeterministicAoT = 0x40000000; // Dump the rosalloc stats on SIGQUIT. static constexpr bool kDumpRosAllocStatsOnSigQuit = false; +static constexpr size_t kNativeAllocationHistogramBuckets = 16; + static inline bool CareAboutPauseTimes() { return Runtime::Current()->InJankPerceptibleProcessState(); } @@ -186,6 +188,11 @@ Heap::Heap(size_t initial_size, total_objects_freed_ever_(0), num_bytes_allocated_(0), native_bytes_allocated_(0), + native_histogram_lock_("Native allocation lock"), + native_allocation_histogram_("Native allocation sizes", + 1U, + kNativeAllocationHistogramBuckets), + native_free_histogram_("Native free sizes", 1U, kNativeAllocationHistogramBuckets), num_bytes_freed_revoke_(0), verify_missing_card_marks_(false), verify_system_weaks_(false), @@ -1185,6 +1192,20 @@ void Heap::DumpGcPerformanceInfo(std::ostream& os) { rosalloc_space_->DumpStats(os); } + { + MutexLock mu(Thread::Current(), native_histogram_lock_); + if (native_allocation_histogram_.SampleSize() > 0u) { + os << "Histogram of native allocation "; + native_allocation_histogram_.DumpBins(os); + os << " bucket size " << native_allocation_histogram_.BucketWidth() << "\n"; + } + if (native_free_histogram_.SampleSize() > 0u) { + os << "Histogram of native free "; + native_free_histogram_.DumpBins(os); + os << " bucket size " << native_free_histogram_.BucketWidth() << "\n"; + } + } + BaseMutex::DumpAll(os); } @@ -3848,6 +3869,10 @@ void Heap::RunFinalization(JNIEnv* env, uint64_t timeout) { void Heap::RegisterNativeAllocation(JNIEnv* env, size_t bytes) { Thread* self = ThreadForEnv(env); + { + MutexLock mu(self, native_histogram_lock_); + native_allocation_histogram_.AddValue(bytes); + } if (native_need_to_run_finalization_) { RunFinalization(env, kNativeAllocationFinalizeTimeout); UpdateMaxNativeFootprint(); @@ -3892,6 +3917,10 @@ void Heap::RegisterNativeAllocation(JNIEnv* env, size_t bytes) { void Heap::RegisterNativeFree(JNIEnv* env, size_t bytes) { size_t expected_size; + { + MutexLock mu(Thread::Current(), native_histogram_lock_); + native_free_histogram_.AddValue(bytes); + } do { expected_size = native_bytes_allocated_.LoadRelaxed(); if (UNLIKELY(bytes > expected_size)) { diff --git a/runtime/gc/heap.h b/runtime/gc/heap.h index fada1a2212..2a1a4a17ae 100644 --- a/runtime/gc/heap.h +++ b/runtime/gc/heap.h @@ -241,9 +241,9 @@ class Heap { SHARED_REQUIRES(Locks::mutator_lock_); void RegisterNativeAllocation(JNIEnv* env, size_t bytes) - REQUIRES(!*gc_complete_lock_, !*pending_task_lock_); + REQUIRES(!*gc_complete_lock_, !*pending_task_lock_, !native_histogram_lock_); void RegisterNativeFree(JNIEnv* env, size_t bytes) - REQUIRES(!*gc_complete_lock_, !*pending_task_lock_); + REQUIRES(!*gc_complete_lock_, !*pending_task_lock_, !native_histogram_lock_); // Change the allocator, updates entrypoints. void ChangeAllocator(AllocatorType allocator) @@ -532,7 +532,7 @@ class Heap { space::Space* FindSpaceFromObject(const mirror::Object*, bool fail_ok) const SHARED_REQUIRES(Locks::mutator_lock_); - void DumpForSigQuit(std::ostream& os) REQUIRES(!*gc_complete_lock_); + void DumpForSigQuit(std::ostream& os) REQUIRES(!*gc_complete_lock_, !native_histogram_lock_); // Do a pending collector transition. void DoPendingCollectorTransition() REQUIRES(!*gc_complete_lock_); @@ -654,7 +654,8 @@ class Heap { std::string SafePrettyTypeOf(mirror::Object* obj) NO_THREAD_SAFETY_ANALYSIS; // GC performance measuring - void DumpGcPerformanceInfo(std::ostream& os) REQUIRES(!*gc_complete_lock_); + void DumpGcPerformanceInfo(std::ostream& os) + REQUIRES(!*gc_complete_lock_, !native_histogram_lock_); void ResetGcPerformanceInfo() REQUIRES(!*gc_complete_lock_); // Thread pool. @@ -1156,6 +1157,11 @@ class Heap { // Bytes which are allocated and managed by native code but still need to be accounted for. Atomic<size_t> native_bytes_allocated_; + // Native allocation stats. + Mutex native_histogram_lock_; + Histogram<uint64_t> native_allocation_histogram_; + Histogram<uint64_t> native_free_histogram_; + // Number of bytes freed by thread local buffer revokes. This will // cancel out the ahead-of-time bulk counting of bytes allocated in // rosalloc thread-local buffers. It is temporarily accumulated diff --git a/runtime/interpreter/interpreter.cc b/runtime/interpreter/interpreter.cc index 6c630cc48f..1d0e600e4d 100644 --- a/runtime/interpreter/interpreter.cc +++ b/runtime/interpreter/interpreter.cc @@ -264,12 +264,12 @@ JValue ExecuteGotoImpl<false, true>(Thread* self, const DexFile::CodeItem* code_ ShadowFrame& shadow_frame, JValue result_register); #endif -static JValue Execute(Thread* self, const DexFile::CodeItem* code_item, ShadowFrame& shadow_frame, - JValue result_register) - SHARED_REQUIRES(Locks::mutator_lock_); - -static inline JValue Execute(Thread* self, const DexFile::CodeItem* code_item, - ShadowFrame& shadow_frame, JValue result_register) { +static inline JValue Execute( + Thread* self, + const DexFile::CodeItem* code_item, + ShadowFrame& shadow_frame, + JValue result_register, + bool stay_in_interpreter = false) SHARED_REQUIRES(Locks::mutator_lock_) { DCHECK(!shadow_frame.GetMethod()->IsAbstract()); DCHECK(!shadow_frame.GetMethod()->IsNative()); if (LIKELY(shadow_frame.GetDexPC() == 0)) { // Entering the method, but not via deoptimization. @@ -284,19 +284,21 @@ static inline JValue Execute(Thread* self, const DexFile::CodeItem* code_item, method, 0); } - jit::Jit* jit = Runtime::Current()->GetJit(); - if (jit != nullptr) { - jit->MethodEntered(self, shadow_frame.GetMethod()); - if (jit->CanInvokeCompiledCode(method)) { - JValue result; + if (!stay_in_interpreter) { + jit::Jit* jit = Runtime::Current()->GetJit(); + if (jit != nullptr) { + jit->MethodEntered(self, shadow_frame.GetMethod()); + if (jit->CanInvokeCompiledCode(method)) { + JValue result; - // Pop the shadow frame before calling into compiled code. - self->PopShadowFrame(); - ArtInterpreterToCompiledCodeBridge(self, nullptr, code_item, &shadow_frame, &result); - // Push the shadow frame back as the caller will expect it. - self->PushShadowFrame(&shadow_frame); + // Pop the shadow frame before calling into compiled code. + self->PopShadowFrame(); + ArtInterpreterToCompiledCodeBridge(self, nullptr, code_item, &shadow_frame, &result); + // Push the shadow frame back as the caller will expect it. + self->PushShadowFrame(&shadow_frame); - return result; + return result; + } } } } @@ -387,7 +389,8 @@ static inline JValue Execute(Thread* self, const DexFile::CodeItem* code_item, } void EnterInterpreterFromInvoke(Thread* self, ArtMethod* method, Object* receiver, - uint32_t* args, JValue* result) { + uint32_t* args, JValue* result, + bool stay_in_interpreter) { DCHECK_EQ(self, Thread::Current()); bool implicit_check = !Runtime::Current()->ExplicitStackOverflowChecks(); if (UNLIKELY(__builtin_frame_address(0) < self->GetStackEndForInterpreter(implicit_check))) { @@ -462,7 +465,7 @@ void EnterInterpreterFromInvoke(Thread* self, ArtMethod* method, Object* receive } } if (LIKELY(!method->IsNative())) { - JValue r = Execute(self, code_item, *shadow_frame, JValue()); + JValue r = Execute(self, code_item, *shadow_frame, JValue(), stay_in_interpreter); if (result != nullptr) { *result = r; } diff --git a/runtime/interpreter/interpreter.h b/runtime/interpreter/interpreter.h index 6353a9b7bf..bf4bcff856 100644 --- a/runtime/interpreter/interpreter.h +++ b/runtime/interpreter/interpreter.h @@ -33,8 +33,11 @@ class Thread; namespace interpreter { // Called by ArtMethod::Invoke, shadow frames arguments are taken from the args array. +// The optional stay_in_interpreter parameter (false by default) can be used by clients to +// explicitly force interpretation in the remaining path that implements method invocation. extern void EnterInterpreterFromInvoke(Thread* self, ArtMethod* method, - mirror::Object* receiver, uint32_t* args, JValue* result) + mirror::Object* receiver, uint32_t* args, JValue* result, + bool stay_in_interpreter = false) SHARED_REQUIRES(Locks::mutator_lock_); // 'from_code' denotes whether the deoptimization was explicitly triggered by compiled code. diff --git a/runtime/java_vm_ext.cc b/runtime/java_vm_ext.cc index 3de718b397..d983a9fa19 100644 --- a/runtime/java_vm_ext.cc +++ b/runtime/java_vm_ext.cc @@ -74,10 +74,6 @@ class SharedLibrary { if (self != nullptr) { self->GetJniEnv()->DeleteWeakGlobalRef(class_loader_); } - - if (!needs_native_bridge_) { - android::CloseNativeLibrary(handle_); - } } jweak GetClassLoader() const { @@ -275,7 +271,8 @@ class Libraries { REQUIRES(!Locks::jni_libraries_lock_) SHARED_REQUIRES(Locks::mutator_lock_) { ScopedObjectAccessUnchecked soa(Thread::Current()); - std::vector<SharedLibrary*> unload_libraries; + typedef void (*JNI_OnUnloadFn)(JavaVM*, void*); + std::vector<JNI_OnUnloadFn> unload_functions; { MutexLock mu(soa.Self(), *Locks::jni_libraries_lock_); for (auto it = libraries_.begin(); it != libraries_.end(); ) { @@ -286,7 +283,15 @@ class Libraries { // the native libraries of the boot class loader. if (class_loader != nullptr && soa.Self()->IsJWeakCleared(class_loader)) { - unload_libraries.push_back(library); + void* const sym = library->FindSymbol("JNI_OnUnload", nullptr); + if (sym == nullptr) { + VLOG(jni) << "[No JNI_OnUnload found in \"" << library->GetPath() << "\"]"; + } else { + VLOG(jni) << "[JNI_OnUnload found for \"" << library->GetPath() << "\"]"; + JNI_OnUnloadFn jni_on_unload = reinterpret_cast<JNI_OnUnloadFn>(sym); + unload_functions.push_back(jni_on_unload); + } + delete library; it = libraries_.erase(it); } else { ++it; @@ -294,17 +299,9 @@ class Libraries { } } // Do this without holding the jni libraries lock to prevent possible deadlocks. - typedef void (*JNI_OnUnloadFn)(JavaVM*, void*); - for (auto library : unload_libraries) { - void* const sym = library->FindSymbol("JNI_OnUnload", nullptr); - if (sym == nullptr) { - VLOG(jni) << "[No JNI_OnUnload found in \"" << library->GetPath() << "\"]"; - } else { - VLOG(jni) << "[JNI_OnUnload found for \"" << library->GetPath() << "\"]: Calling..."; - JNI_OnUnloadFn jni_on_unload = reinterpret_cast<JNI_OnUnloadFn>(sym); - jni_on_unload(soa.Vm(), nullptr); - } - delete library; + for (JNI_OnUnloadFn fn : unload_functions) { + VLOG(jni) << "Calling JNI_OnUnload"; + (*fn)(soa.Vm(), nullptr); } } @@ -946,6 +943,11 @@ extern "C" jint JNI_CreateJavaVM(JavaVM** p_vm, JNIEnv** p_env, void* vm_args) { if (!Runtime::Create(options, ignore_unrecognized)) { return JNI_ERR; } + + // Initialize native loader. This step makes sure we have + // everything set up before we start using JNI. + android::InitializeNativeLoader(); + Runtime* runtime = Runtime::Current(); bool started = runtime->Start(); if (!started) { @@ -955,10 +957,6 @@ extern "C" jint JNI_CreateJavaVM(JavaVM** p_vm, JNIEnv** p_env, void* vm_args) { return JNI_ERR; } - // Initialize native loader. This step makes sure we have - // everything set up before we start using JNI. - android::InitializeNativeLoader(); - *p_env = Thread::Current()->GetJniEnv(); *p_vm = runtime->GetJavaVM(); return JNI_OK; diff --git a/runtime/jit/jit.cc b/runtime/jit/jit.cc index e9317a5435..dcc6300636 100644 --- a/runtime/jit/jit.cc +++ b/runtime/jit/jit.cc @@ -103,13 +103,13 @@ JitOptions* JitOptions::CreateFromRuntimeArguments(const RuntimeArgumentMap& opt } if (options.Exists(RuntimeArgumentMap::JITInvokeTransitionWeight)) { + jit_options->invoke_transition_weight_ = + *options.Get(RuntimeArgumentMap::JITInvokeTransitionWeight); if (jit_options->invoke_transition_weight_ > jit_options->warmup_threshold_) { LOG(FATAL) << "Invoke transition weight is above the warmup threshold."; } else if (jit_options->invoke_transition_weight_ == 0) { - LOG(FATAL) << "Invoke transition ratio cannot be 0."; + LOG(FATAL) << "Invoke transition weight cannot be 0."; } - jit_options->invoke_transition_weight_ = - *options.Get(RuntimeArgumentMap::JITInvokeTransitionWeight); } else { jit_options->invoke_transition_weight_ = std::max( jit_options->warmup_threshold_ / Jit::kDefaultInvokeTransitionWeightRatio, diff --git a/runtime/native/dalvik_system_DexFile.cc b/runtime/native/dalvik_system_DexFile.cc index 0abe39d872..0126b4d0a4 100644 --- a/runtime/native/dalvik_system_DexFile.cc +++ b/runtime/native/dalvik_system_DexFile.cc @@ -16,6 +16,8 @@ #include "dalvik_system_DexFile.h" +#include <sstream> + #include "base/logging.h" #include "base/stl_util.h" #include "base/stringprintf.h" @@ -27,6 +29,7 @@ #include "mirror/class_loader.h" #include "mirror/object-inl.h" #include "mirror/string.h" +#include "oat_file.h" #include "oat_file_assistant.h" #include "oat_file_manager.h" #include "os.h" @@ -387,6 +390,61 @@ static jint GetDexOptNeeded(JNIEnv* env, return oat_file_assistant.GetDexOptNeeded(filter); } +static jstring DexFile_getDexFileStatus(JNIEnv* env, + jclass, + jstring javaFilename, + jstring javaInstructionSet) { + ScopedUtfChars filename(env, javaFilename); + if (env->ExceptionCheck()) { + return nullptr; + } + + ScopedUtfChars instruction_set(env, javaInstructionSet); + if (env->ExceptionCheck()) { + return nullptr; + } + + const InstructionSet target_instruction_set = GetInstructionSetFromString( + instruction_set.c_str()); + if (target_instruction_set == kNone) { + ScopedLocalRef<jclass> iae(env, env->FindClass("java/lang/IllegalArgumentException")); + std::string message(StringPrintf("Instruction set %s is invalid.", instruction_set.c_str())); + env->ThrowNew(iae.get(), message.c_str()); + return nullptr; + } + + OatFileAssistant oat_file_assistant(filename.c_str(), target_instruction_set, + false /* profile_changed */, + false /* load_executable */); + + std::ostringstream status; + bool oat_file_exists = false; + bool odex_file_exists = false; + if (oat_file_assistant.OatFileExists()) { + oat_file_exists = true; + status << *oat_file_assistant.OatFileName() << " [compilation_filter="; + status << CompilerFilter::NameOfFilter(oat_file_assistant.OatFileCompilerFilter()); + status << ", status=" << oat_file_assistant.OatFileStatus(); + } + + if (oat_file_assistant.OdexFileExists()) { + odex_file_exists = true; + if (oat_file_exists) { + status << "] "; + } + status << *oat_file_assistant.OdexFileName() << " [compilation_filter="; + status << CompilerFilter::NameOfFilter(oat_file_assistant.OdexFileCompilerFilter()); + status << ", status=" << oat_file_assistant.OdexFileStatus(); + } + + if (!oat_file_exists && !odex_file_exists) { + status << "invalid["; + } + + status << "]"; + return env->NewStringUTF(status.str().c_str()); +} + static jint DexFile_getDexOptNeeded(JNIEnv* env, jclass, jstring javaFilename, @@ -481,6 +539,16 @@ static jstring DexFile_getNonProfileGuidedCompilerFilter(JNIEnv* env, return env->NewStringUTF(new_filter_str.c_str()); } +static jboolean DexFile_isBackedByOatFile(JNIEnv* env, jclass, jobject cookie) { + const OatFile* oat_file = nullptr; + std::vector<const DexFile*> dex_files; + if (!ConvertJavaArrayToDexFiles(env, cookie, /*out */ dex_files, /* out */ oat_file)) { + DCHECK(env->ExceptionCheck()); + return false; + } + return oat_file != nullptr; +} + static JNINativeMethod gMethods[] = { NATIVE_METHOD(DexFile, closeDexFile, "(Ljava/lang/Object;)Z"), NATIVE_METHOD(DexFile, @@ -506,6 +574,9 @@ static JNINativeMethod gMethods[] = { NATIVE_METHOD(DexFile, getNonProfileGuidedCompilerFilter, "(Ljava/lang/String;)Ljava/lang/String;"), + NATIVE_METHOD(DexFile, isBackedByOatFile, "(Ljava/lang/Object;)Z"), + NATIVE_METHOD(DexFile, getDexFileStatus, + "(Ljava/lang/String;Ljava/lang/String;)Ljava/lang/String;") }; void register_dalvik_system_DexFile(JNIEnv* env) { diff --git a/runtime/native/java_lang_reflect_Constructor.cc b/runtime/native/java_lang_reflect_Constructor.cc index ddcaadefa3..54b8afd1f3 100644 --- a/runtime/native/java_lang_reflect_Constructor.cc +++ b/runtime/native/java_lang_reflect_Constructor.cc @@ -34,20 +34,38 @@ static jobject Constructor_getAnnotationNative(JNIEnv* env, jobject javaMethod, ScopedFastNativeObjectAccess soa(env); StackHandleScope<1> hs(soa.Self()); ArtMethod* method = ArtMethod::FromReflectedMethod(soa, javaMethod); - Handle<mirror::Class> klass(hs.NewHandle(soa.Decode<mirror::Class*>(annotationType))); - return soa.AddLocalReference<jobject>( - method->GetDexFile()->GetAnnotationForMethod(method, klass)); + if (method->IsProxyMethod()) { + return nullptr; + } else { + Handle<mirror::Class> klass(hs.NewHandle(soa.Decode<mirror::Class*>(annotationType))); + return soa.AddLocalReference<jobject>( + method->GetDexFile()->GetAnnotationForMethod(method, klass)); + } } static jobjectArray Constructor_getDeclaredAnnotations(JNIEnv* env, jobject javaMethod) { ScopedFastNativeObjectAccess soa(env); ArtMethod* method = ArtMethod::FromReflectedMethod(soa, javaMethod); - return soa.AddLocalReference<jobjectArray>(method->GetDexFile()->GetAnnotationsForMethod(method)); + if (method->IsProxyMethod()) { + mirror::Class* class_class = mirror::Class::GetJavaLangClass(); + mirror::Class* class_array_class = + Runtime::Current()->GetClassLinker()->FindArrayClass(soa.Self(), &class_class); + if (class_array_class == nullptr) { + return nullptr; + } + mirror::ObjectArray<mirror::Class>* empty_array = + mirror::ObjectArray<mirror::Class>::Alloc(soa.Self(), class_array_class, 0); + return soa.AddLocalReference<jobjectArray>(empty_array); + } else { + return soa.AddLocalReference<jobjectArray>( + method->GetDexFile()->GetAnnotationsForMethod(method)); + } } static jobjectArray Constructor_getExceptionTypes(JNIEnv* env, jobject javaMethod) { ScopedFastNativeObjectAccess soa(env); - ArtMethod* method = ArtMethod::FromReflectedMethod(soa, javaMethod); + ArtMethod* method = ArtMethod::FromReflectedMethod(soa, javaMethod) + ->GetInterfaceMethodIfProxy(sizeof(void*)); mirror::ObjectArray<mirror::Class>* result_array = method->GetDexFile()->GetExceptionTypesForMethod(method); if (result_array == nullptr) { @@ -69,7 +87,12 @@ static jobjectArray Constructor_getExceptionTypes(JNIEnv* env, jobject javaMetho static jobjectArray Constructor_getParameterAnnotationsNative(JNIEnv* env, jobject javaMethod) { ScopedFastNativeObjectAccess soa(env); ArtMethod* method = ArtMethod::FromReflectedMethod(soa, javaMethod); - return soa.AddLocalReference<jobjectArray>(method->GetDexFile()->GetParameterAnnotations(method)); + if (method->IsProxyMethod()) { + return nullptr; + } else { + return soa.AddLocalReference<jobjectArray>( + method->GetDexFile()->GetParameterAnnotations(method)); + } } static jboolean Constructor_isAnnotationPresentNative(JNIEnv* env, jobject javaMethod, @@ -77,6 +100,10 @@ static jboolean Constructor_isAnnotationPresentNative(JNIEnv* env, jobject javaM ScopedFastNativeObjectAccess soa(env); StackHandleScope<1> hs(soa.Self()); ArtMethod* method = ArtMethod::FromReflectedMethod(soa, javaMethod); + if (method->IsProxyMethod()) { + // Proxies have no annotations. + return false; + } Handle<mirror::Class> klass(hs.NewHandle(soa.Decode<mirror::Class*>(annotationType))); return method->GetDexFile()->IsMethodAnnotationPresent(method, klass); } diff --git a/runtime/oat_file_assistant.cc b/runtime/oat_file_assistant.cc index a508e87c87..713e2f3fa9 100644 --- a/runtime/oat_file_assistant.cc +++ b/runtime/oat_file_assistant.cc @@ -30,6 +30,7 @@ #include "base/logging.h" #include "base/stringprintf.h" +#include "compiler_filter.h" #include "class_linker.h" #include "gc/heap.h" #include "gc/space/image_space.h" @@ -43,6 +44,24 @@ namespace art { +std::ostream& operator << (std::ostream& stream, const OatFileAssistant::OatStatus status) { + switch (status) { + case OatFileAssistant::kOatOutOfDate: + stream << "kOatOutOfDate"; + break; + case OatFileAssistant::kOatUpToDate: + stream << "kOatUpToDate"; + break; + case OatFileAssistant::kOatNeedsRelocation: + stream << "kOatNeedsRelocation"; + break; + default: + UNREACHABLE(); + } + + return stream; +} + OatFileAssistant::OatFileAssistant(const char* dex_location, const InstructionSet isa, bool profile_changed, @@ -377,6 +396,12 @@ bool OatFileAssistant::OdexFileIsUpToDate() { return cached_odex_file_is_up_to_date_; } +CompilerFilter::Filter OatFileAssistant::OdexFileCompilerFilter() { + const OatFile* odex_file = GetOdexFile(); + CHECK(odex_file != nullptr); + + return odex_file->GetCompilerFilter(); +} std::string OatFileAssistant::ArtFileName(const OatFile* oat_file) const { const std::string oat_file_location = oat_file->GetLocation(); // Replace extension with .art @@ -455,6 +480,13 @@ bool OatFileAssistant::OatFileIsUpToDate() { return cached_oat_file_is_up_to_date_; } +CompilerFilter::Filter OatFileAssistant::OatFileCompilerFilter() { + const OatFile* oat_file = GetOatFile(); + CHECK(oat_file != nullptr); + + return oat_file->GetCompilerFilter(); +} + OatFileAssistant::OatStatus OatFileAssistant::GivenOatFileStatus(const OatFile& file) { // TODO: This could cause GivenOatFileIsOutOfDate to be called twice, which // is more work than we need to do. If performance becomes a concern, and diff --git a/runtime/oat_file_assistant.h b/runtime/oat_file_assistant.h index 85f4a47868..f48cdf343c 100644 --- a/runtime/oat_file_assistant.h +++ b/runtime/oat_file_assistant.h @@ -19,6 +19,7 @@ #include <cstdint> #include <memory> +#include <sstream> #include <string> #include "arch/instruction_set.h" @@ -211,6 +212,9 @@ class OatFileAssistant { bool OdexFileIsOutOfDate(); bool OdexFileNeedsRelocation(); bool OdexFileIsUpToDate(); + // Must only be called if the associated odex file exists, i.e, if + // |OdexFileExists() == true|. + CompilerFilter::Filter OdexFileCompilerFilter(); // When the dex files is compiled on the target device, the oat file is the // result. The oat file will have been relocated to some @@ -227,6 +231,9 @@ class OatFileAssistant { bool OatFileIsOutOfDate(); bool OatFileNeedsRelocation(); bool OatFileIsUpToDate(); + // Must only be called if the associated oat file exists, i.e, if + // |OatFileExists() == true|. + CompilerFilter::Filter OatFileCompilerFilter(); // Return image file name. Does not cache since it relies on the oat file. std::string ArtFileName(const OatFile* oat_file) const; @@ -436,6 +443,8 @@ class OatFileAssistant { DISALLOW_COPY_AND_ASSIGN(OatFileAssistant); }; +std::ostream& operator << (std::ostream& stream, const OatFileAssistant::OatStatus status); + } // namespace art #endif // ART_RUNTIME_OAT_FILE_ASSISTANT_H_ diff --git a/runtime/verifier/method_verifier.cc b/runtime/verifier/method_verifier.cc index d05ae42b0e..2b96328f80 100644 --- a/runtime/verifier/method_verifier.cc +++ b/runtime/verifier/method_verifier.cc @@ -4101,8 +4101,8 @@ ArtMethod* MethodVerifier::VerifyInvocationArgs( << " to super " << PrettyMethod(res_method); return nullptr; } - mirror::Class* super_klass = super.GetClass(); - if (res_method->GetMethodIndex() >= super_klass->GetVTableLength()) { + if (!reference_class->IsAssignableFrom(GetDeclaringClass().GetClass()) || + (res_method->GetMethodIndex() >= super.GetClass()->GetVTableLength())) { Fail(VERIFY_ERROR_NO_METHOD) << "invalid invoke-super from " << PrettyMethod(dex_method_idx_, *dex_file_) << " to super " << super diff --git a/test/044-proxy/expected.txt b/test/044-proxy/expected.txt index be7023e49d..2a5f0b90db 100644 --- a/test/044-proxy/expected.txt +++ b/test/044-proxy/expected.txt @@ -95,3 +95,5 @@ Proxy narrowed invocation return type passed 5.8 JNI_OnLoad called callback +Found constructor. +Found constructors with 0 exceptions diff --git a/test/044-proxy/src/ConstructorProxy.java b/test/044-proxy/src/ConstructorProxy.java new file mode 100644 index 0000000000..95d150cbbd --- /dev/null +++ b/test/044-proxy/src/ConstructorProxy.java @@ -0,0 +1,53 @@ +/* + * Copyright 2016 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. + */ + +import java.lang.reflect.Constructor; +import java.lang.reflect.InvocationHandler; +import java.lang.reflect.Method; +import java.lang.reflect.Proxy; + +/** + * Tests proxies when used with constructor methods. + */ +class ConstructorProxy implements InvocationHandler { + public static void main() { + try { + new ConstructorProxy().runTest(); + } catch (Exception e) { + System.out.println("Unexpected failure occured"); + e.printStackTrace(); + } + } + + public void runTest() throws Exception { + Class<?> proxyClass = Proxy.getProxyClass( + getClass().getClassLoader(), + new Class<?>[] { Runnable.class } + ); + Constructor<?> constructor = proxyClass.getConstructor(InvocationHandler.class); + System.out.println("Found constructor."); + // We used to crash when asking the exception types of the constructor, because the runtime was + // not using the non-proxy ArtMethod + Object[] exceptions = constructor.getExceptionTypes(); + System.out.println("Found constructors with " + exceptions.length + " exceptions"); + } + + @Override + public Object invoke(Object proxy, Method method, Object[] args) throws Throwable { + return args[0]; + } +} + diff --git a/test/044-proxy/src/Main.java b/test/044-proxy/src/Main.java index 1f23b95cf0..9dadb7c6ea 100644 --- a/test/044-proxy/src/Main.java +++ b/test/044-proxy/src/Main.java @@ -31,6 +31,7 @@ public class Main { NarrowingTest.main(null); FloatSelect.main(null); NativeProxy.main(args); + ConstructorProxy.main(); } // The following code maps from the actual proxy class names (eg $Proxy2) to their test output diff --git a/test/536-checker-intrinsic-optimization/src/Main.java b/test/536-checker-intrinsic-optimization/src/Main.java index be666e94fa..15a9504acf 100644 --- a/test/536-checker-intrinsic-optimization/src/Main.java +++ b/test/536-checker-intrinsic-optimization/src/Main.java @@ -16,9 +16,69 @@ public class Main { + public static boolean doThrow = false; + + public static void assertIntEquals(int expected, int result) { + if (expected != result) { + throw new Error("Expected: " + expected + ", found: " + result); + } + } + + public static void assertBooleanEquals(boolean expected, boolean result) { + if (expected != result) { + throw new Error("Expected: " + expected + ", found: " + result); + } + } + public static void main(String[] args) { stringEqualsSame(); stringArgumentNotNull("Foo"); + + assertIntEquals(0, $opt$noinline$getStringLength("")); + assertIntEquals(3, $opt$noinline$getStringLength("abc")); + assertIntEquals(10, $opt$noinline$getStringLength("0123456789")); + + assertBooleanEquals(true, $opt$noinline$isStringEmpty("")); + assertBooleanEquals(false, $opt$noinline$isStringEmpty("abc")); + assertBooleanEquals(false, $opt$noinline$isStringEmpty("0123456789")); + } + + /// CHECK-START: int Main.$opt$noinline$getStringLength(java.lang.String) instruction_simplifier (before) + /// CHECK-DAG: <<Length:i\d+>> InvokeVirtual intrinsic:StringLength + /// CHECK-DAG: Return [<<Length>>] + + /// CHECK-START: int Main.$opt$noinline$getStringLength(java.lang.String) instruction_simplifier (after) + /// CHECK-DAG: <<String:l\d+>> ParameterValue + /// CHECK-DAG: <<NullCk:l\d+>> NullCheck [<<String>>] + /// CHECK-DAG: <<Length:i\d+>> ArrayLength [<<NullCk>>] is_string_length:true + /// CHECK-DAG: Return [<<Length>>] + + /// CHECK-START: int Main.$opt$noinline$getStringLength(java.lang.String) instruction_simplifier (after) + /// CHECK-NOT: InvokeVirtual intrinsic:StringLength + + static public int $opt$noinline$getStringLength(String s) { + if (doThrow) { throw new Error(); } + return s.length(); + } + + /// CHECK-START: boolean Main.$opt$noinline$isStringEmpty(java.lang.String) instruction_simplifier (before) + /// CHECK-DAG: <<IsEmpty:z\d+>> InvokeVirtual intrinsic:StringIsEmpty + /// CHECK-DAG: Return [<<IsEmpty>>] + + /// CHECK-START: boolean Main.$opt$noinline$isStringEmpty(java.lang.String) instruction_simplifier (after) + /// CHECK-DAG: <<String:l\d+>> ParameterValue + /// CHECK-DAG: <<Const0:i\d+>> IntConstant 0 + /// CHECK-DAG: <<NullCk:l\d+>> NullCheck [<<String>>] + /// CHECK-DAG: <<Length:i\d+>> ArrayLength [<<NullCk>>] is_string_length:true + /// CHECK-DAG: <<IsEmpty:z\d+>> Equal [<<Length>>,<<Const0>>] + /// CHECK-DAG: Return [<<IsEmpty>>] + + /// CHECK-START: boolean Main.$opt$noinline$isStringEmpty(java.lang.String) instruction_simplifier (after) + /// CHECK-NOT: InvokeVirtual intrinsic:StringIsEmpty + + static public boolean $opt$noinline$isStringEmpty(String s) { + if (doThrow) { throw new Error(); } + return s.isEmpty(); } /// CHECK-START: boolean Main.stringEqualsSame() instruction_simplifier (before) diff --git a/test/586-checker-null-array-get/src/Main.java b/test/586-checker-null-array-get/src/Main.java index 332cfb0a53..e0782bc84d 100644 --- a/test/586-checker-null-array-get/src/Main.java +++ b/test/586-checker-null-array-get/src/Main.java @@ -14,10 +14,20 @@ * limitations under the License. */ +class Test1 { + int[] iarr; +} + +class Test2 { + float[] farr; +} + public class Main { public static Object[] getObjectArray() { return null; } public static long[] getLongArray() { return null; } public static Object getNull() { return null; } + public static Test1 getNullTest1() { return null; } + public static Test2 getNullTest2() { return null; } public static void main(String[] args) { try { @@ -26,13 +36,25 @@ public class Main { } catch (NullPointerException e) { // Expected. } + try { + bar(); + throw new Error("Expected NullPointerException"); + } catch (NullPointerException e) { + // Expected. + } + try { + test1(); + throw new Error("Expected NullPointerException"); + } catch (NullPointerException e) { + // Expected. + } } /// CHECK-START: void Main.foo() load_store_elimination (after) - /// CHECK-DAG: <<Null:l\d+>> NullConstant - /// CHECK-DAG: <<Check:l\d+>> NullCheck [<<Null>>] - /// CHECK-DAG: <<Get1:j\d+>> ArrayGet [<<Check>>,{{i\d+}}] - /// CHECK-DAG: <<Get2:l\d+>> ArrayGet [<<Check>>,{{i\d+}}] + /// CHECK-DAG: <<Null:l\d+>> NullConstant + /// CHECK-DAG: <<Check:l\d+>> NullCheck [<<Null>>] + /// CHECK-DAG: <<Get1:j\d+>> ArrayGet [<<Check>>,{{i\d+}}] + /// CHECK-DAG: <<Get2:l\d+>> ArrayGet [<<Check>>,{{i\d+}}] public static void foo() { longField = getLongArray()[0]; objectField = getObjectArray()[0]; @@ -56,7 +78,7 @@ public class Main { // elimination pass to add a HDeoptimize. Not having the bounds check helped // the load store elimination think it could merge two ArrayGet with different // types. - String[] array = ((String[])getNull()); + String[] array = (String[])getNull(); objectField = array[0]; objectField = array[1]; objectField = array[2]; @@ -68,6 +90,23 @@ public class Main { longField = longArray[3]; } + /// CHECK-START: float Main.test1() load_store_elimination (after) + /// CHECK-DAG: <<Null:l\d+>> NullConstant + /// CHECK-DAG: <<Check1:l\d+>> NullCheck [<<Null>>] + /// CHECK-DAG: <<FieldGet1:l\d+>> InstanceFieldGet [<<Check1>>] field_name:Test1.iarr + /// CHECK-DAG: <<Check2:l\d+>> NullCheck [<<FieldGet1>>] + /// CHECK-DAG: <<ArrayGet1:i\d+>> ArrayGet [<<Check2>>,{{i\d+}}] + /// CHECK-DAG: <<ArrayGet2:f\d+>> ArrayGet [<<Check2>>,{{i\d+}}] + /// CHECK-DAG: Return [<<ArrayGet2>>] + public static float test1() { + Test1 test1 = getNullTest1(); + Test2 test2 = getNullTest2();; + int[] iarr = test1.iarr; + float[] farr = test2.farr; + iarr[0] = iarr[1]; + return farr[0]; + } + public static long longField; public static Object objectField; } diff --git a/test/594-invoke-super/expected.txt b/test/594-invoke-super/expected.txt new file mode 100644 index 0000000000..de26026c99 --- /dev/null +++ b/test/594-invoke-super/expected.txt @@ -0,0 +1,7 @@ +new A +I am A's foo +new B +I am B's foo +new A +new B +passed diff --git a/test/594-invoke-super/info.txt b/test/594-invoke-super/info.txt new file mode 100644 index 0000000000..440d8b8c66 --- /dev/null +++ b/test/594-invoke-super/info.txt @@ -0,0 +1 @@ +Invoke-super on various references. diff --git a/test/594-invoke-super/smali/invoke-super.smali b/test/594-invoke-super/smali/invoke-super.smali new file mode 100644 index 0000000000..6f787dd170 --- /dev/null +++ b/test/594-invoke-super/smali/invoke-super.smali @@ -0,0 +1,31 @@ +# +# Copyright (C) 2016 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. + +.class public LZ; +.super LA; + +.method public constructor <init>()V +.registers 1 + invoke-direct {v0}, LA;-><init>()V + return-void +.end method + +.method public foo()V +.registers 3 + new-instance v0, LY; + invoke-direct {v0}, LY;-><init>()V + invoke-super {v0}, LY;->foo()V + return-void +.end method diff --git a/test/594-invoke-super/src/Main.java b/test/594-invoke-super/src/Main.java new file mode 100644 index 0000000000..53f2bbf67a --- /dev/null +++ b/test/594-invoke-super/src/Main.java @@ -0,0 +1,80 @@ +/* + * Copyright (C) 2016 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. + */ + +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; + +// +// Two classes A and B with method foo(). +// + +class A { + A() { System.out.println("new A"); } + + public void foo() { System.out.println("I am A's foo"); } + + // We previously used to invoke this method with a Y instance, due + // to invoke-super underspecified behavior. + public void bar() { System.out.println("I am A's bar"); } +} + +class B { + B() { System.out.println("new B"); } + + public void foo() { System.out.println("I am B's foo"); } +} + +// +// Two subclasses X and Y that call foo() on super. +// + +class X extends A { + public void foo() { super.foo(); } +} + +class Y extends B { + public void foo() { super.foo(); } +} + +// +// Driver class. +// + +public class Main { + + public static void main(String[] args) throws Exception { + // The normal stuff, X's super goes to A, Y's super goes to B. + new X().foo(); + new Y().foo(); + + // And now it gets interesting. + + // In bytecode, we define a class Z that is a subclass of A, and we call + // invoke-super on an instance of Y. + Class<?> z = Class.forName("Z"); + Method m = z.getMethod("foo"); + try { + m.invoke(z.newInstance()); + throw new Error("Expected InvocationTargetException"); + } catch (InvocationTargetException e) { + if (!(e.getCause() instanceof NoSuchMethodError)) { + throw new Error("Expected NoSuchMethodError"); + } + } + + System.out.println("passed"); + } +} diff --git a/test/594-load-string-regression/expected.txt b/test/594-load-string-regression/expected.txt new file mode 100644 index 0000000000..365b0e168f --- /dev/null +++ b/test/594-load-string-regression/expected.txt @@ -0,0 +1 @@ +String: "" diff --git a/test/594-load-string-regression/info.txt b/test/594-load-string-regression/info.txt new file mode 100644 index 0000000000..6a07ace81b --- /dev/null +++ b/test/594-load-string-regression/info.txt @@ -0,0 +1,2 @@ +Regression test for LoadString listing side effects when it doesn't have any +and triggering a DCHECK() failure when merging ClinitCheck into NewInstance. diff --git a/test/594-load-string-regression/src/Main.java b/test/594-load-string-regression/src/Main.java new file mode 100644 index 0000000000..0b9f7b52a1 --- /dev/null +++ b/test/594-load-string-regression/src/Main.java @@ -0,0 +1,77 @@ +/* + * Copyright (C) 2016 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 { + static boolean doThrow = false; + + // Note: We're not doing checker tests as we cannot do them specifically for a non-PIC + // configuration. The check here would be "prepare_for_register_allocation (before)" + // CHECK: LoadClass + // CHECK-NEXT: ClinitCheck + // CHECK-NEXT: LoadString load_kind:BootImageAddress + // CHECK-NEXT: NewInstance + // and "prepare_for_register_allocation (after)" + // CHECK: LoadString + // CHECK-NEXT: NewInstance + // but the order of instructions for non-PIC mode is different. + public static int $noinline$test() { + if (doThrow) { throw new Error(); } + + int r = 0x12345678; + do { + // LICM pulls the LoadClass and ClinitCheck out of the loop, leaves NewInstance in the loop. + Helper h = new Helper(); + // For non-PIC mode, LICM pulls the boot image LoadString out of the loop. + // (For PIC mode, the LoadString can throw and will not be moved out of the loop.) + String s = ""; // Empty string is known to be in the boot image. + r = r ^ (r >> 5); + h.$noinline$printString(s); + // During DCE after inlining, the loop back-edge disappears and the pre-header is + // merged with the body, leaving consecutive LoadClass, ClinitCheck, LoadString + // and NewInstance in non-PIC mode. The prepare_for_register_allocation pass + // merges the LoadClass and ClinitCheck with the NewInstance and checks that + // there are no instructions with side effects in between. This check used to + // fail because LoadString was always listing SideEffects::CanTriggerGC() even + // when it doesn't really have any side effects, i.e. for direct references to + // boot image Strings or for Strings known to be in the dex cache. + } while ($inline$shouldContinue()); + return r; + } + + static boolean $inline$shouldContinue() { + return false; + } + + public static void main(String[] args) { + assertIntEquals(0x12345678 ^ (0x12345678 >> 5), $noinline$test()); + } + + public static void assertIntEquals(int expected, int result) { + if (expected != result) { + throw new Error("Expected: " + expected + ", found: " + result); + } + } +} + +class Helper { + static boolean doThrow = false; + + public void $noinline$printString(String s) { + if (doThrow) { throw new Error(); } + + System.out.println("String: \"" + s + "\""); + } +} diff --git a/test/598-checker-irreducible-dominance/expected.txt b/test/598-checker-irreducible-dominance/expected.txt new file mode 100644 index 0000000000..e69de29bb2 --- /dev/null +++ b/test/598-checker-irreducible-dominance/expected.txt diff --git a/test/598-checker-irreducible-dominance/info.txt b/test/598-checker-irreducible-dominance/info.txt new file mode 100644 index 0000000000..8ca4e63be9 --- /dev/null +++ b/test/598-checker-irreducible-dominance/info.txt @@ -0,0 +1,2 @@ +Regression test for HGraphBuilder which would compute wrong dominance information +in the presence of irreducible loops.
\ No newline at end of file diff --git a/test/598-checker-irreducible-dominance/smali/IrreducibleLoop.smali b/test/598-checker-irreducible-dominance/smali/IrreducibleLoop.smali new file mode 100644 index 0000000000..4d8b5152fd --- /dev/null +++ b/test/598-checker-irreducible-dominance/smali/IrreducibleLoop.smali @@ -0,0 +1,52 @@ +# Copyright (C) 2016 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. + +.class public LIrreducibleLoop; +.super Ljava/lang/Object; + +# Test case in which `inner_back_edge` is not dominated by `inner_header` and +# causes `outer_back_edge` to not be dominated by `outer_header`. HGraphBuilder +# not do a fix-point iteration and would miss the path to `outer_back_edge` +# through `inner_back_edge` and incorrectly label the outer loop non-irreducible. + +## CHECK-START: int IrreducibleLoop.dominance(int) builder (after) +## CHECK: Add irreducible:true + +.method public static dominance(I)I + .registers 2 + + if-eqz p0, :outer_header + goto :inner_back_edge + + :outer_header + if-eqz p0, :inner_header + + :outer_branch_exit + if-eqz p0, :outer_merge + return p0 + + :inner_header + goto :outer_merge + + :inner_back_edge + goto :inner_header + + :outer_merge + if-eqz p0, :inner_back_edge + + :outer_back_edge + add-int/2addr p0, p0 + goto :outer_header + +.end method diff --git a/test/598-checker-irreducible-dominance/src/Main.java b/test/598-checker-irreducible-dominance/src/Main.java new file mode 100644 index 0000000000..38b2ab4384 --- /dev/null +++ b/test/598-checker-irreducible-dominance/src/Main.java @@ -0,0 +1,25 @@ +/* + * Copyright (C) 2016 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 { + // Workaround for b/18051191. + class InnerClass {} + + public static void main(String[] args) { + // Nothing to run. This regression test merely makes sure the smali test + // case successfully compiles. + } +} diff --git a/test/599-checker-irreducible-loop/expected.txt b/test/599-checker-irreducible-loop/expected.txt new file mode 100644 index 0000000000..573541ac97 --- /dev/null +++ b/test/599-checker-irreducible-loop/expected.txt @@ -0,0 +1 @@ +0 diff --git a/test/599-checker-irreducible-loop/info.txt b/test/599-checker-irreducible-loop/info.txt new file mode 100644 index 0000000000..1e0dd02284 --- /dev/null +++ b/test/599-checker-irreducible-loop/info.txt @@ -0,0 +1,2 @@ +Regression test for optimizing in the presence of +an irreducible loop. diff --git a/test/599-checker-irreducible-loop/smali/IrreducibleLoop.smali b/test/599-checker-irreducible-loop/smali/IrreducibleLoop.smali new file mode 100644 index 0000000000..5331fd6a31 --- /dev/null +++ b/test/599-checker-irreducible-loop/smali/IrreducibleLoop.smali @@ -0,0 +1,56 @@ +# Copyright (C) 2016 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. + +.class public LIrreducibleLoop; + +.super Ljava/lang/Object; + +## CHECK-START: int IrreducibleLoop.test(int) GVN (before) +## CHECK-DAG: LoadClass loop:none +## CHECK-DAG: LoadClass loop:{{B\d+}} outer_loop:none + +## CHECK-START: int IrreducibleLoop.test(int) GVN (after) +## CHECK-DAG: LoadClass loop:none +## CHECK-DAG: LoadClass loop:{{B\d+}} outer_loop:none +.method public static test(I)I + .registers 2 + + sget v0, LIrreducibleLoop;->field1:I + sput v0, LIrreducibleLoop;->field2:I + + if-eqz p0, :loop_entry + goto :exit + + :loop_entry + if-eqz p0, :irreducible_loop_entry + sget v0, LIrreducibleLoop;->field2:I + sput v0, LIrreducibleLoop;->field1:I + if-eqz v0, :exit + goto :irreducible_other_loop_entry + + :irreducible_loop_entry + if-eqz p0, :loop_back_edge + :irreducible_other_loop_entry + if-eqz v0, :loop_back_edge + goto :irreducible_loop_entry + + :loop_back_edge + goto :loop_entry + + :exit + return v0 +.end method + +.field public static field1:I +.field public static field2:I diff --git a/test/599-checker-irreducible-loop/src/Main.java b/test/599-checker-irreducible-loop/src/Main.java new file mode 100644 index 0000000000..b47721f721 --- /dev/null +++ b/test/599-checker-irreducible-loop/src/Main.java @@ -0,0 +1,30 @@ +/* + * Copyright (C) 2016 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. + */ + +import java.lang.reflect.Method; + +public class Main { + // Workaround for b/18051191. + class InnerClass {} + + public static void main(String[] args) throws Exception { + Class<?> c = Class.forName("IrreducibleLoop"); + Method m = c.getMethod("test", int.class); + Object[] arguments = { 42 }; + // Invoke the code just for sanity checking. + System.out.println(m.invoke(null, arguments)); + } +} diff --git a/test/etc/run-test-jar b/test/etc/run-test-jar index 01114b7690..aa45d40cb4 100755 --- a/test/etc/run-test-jar +++ b/test/etc/run-test-jar @@ -473,11 +473,14 @@ if [ "$HOST" = "n" ]; then LD_LIBRARY_PATH=$ANDROID_ROOT/$LIBRARY_DIRECTORY fi + PUBLIC_LIBS=libart.so:libartd.so + # Create a script with the command. The command can get longer than the longest # allowed adb command and there is no way to get the exit status from a adb shell # command. cmdline="cd $DEX_LOCATION && \ export ANDROID_DATA=$DEX_LOCATION && \ + export ANDROID_ADDITIONAL_PUBLIC_LIBRARIES=$PUBLIC_LIBS && \ export DEX_LOCATION=$DEX_LOCATION && \ export ANDROID_ROOT=$ANDROID_ROOT && \ $mkdir_cmdline && \ diff --git a/test/run-test b/test/run-test index 424c54f2c9..2710ea32b1 100755 --- a/test/run-test +++ b/test/run-test @@ -122,10 +122,12 @@ never_clean="no" have_dex2oat="yes" have_patchoat="yes" have_image="yes" -image_suffix="" pic_image_suffix="" multi_image_suffix="" android_root="/system" +# By default we will use optimizing. +image_args="" +image_suffix="-optimizing" while true; do if [ "x$1" = "x--host" ]; then @@ -148,6 +150,7 @@ while true; do elif [ "x$1" = "x--jvm" ]; then target_mode="no" runtime="jvm" + image_args="" prebuild_mode="no" NEED_DEX="false" USE_JACK="false" @@ -248,18 +251,18 @@ while true; do image_suffix="-interpreter" shift elif [ "x$1" = "x--jit" ]; then - run_args="${run_args} --jit" + image_args="--jit" image_suffix="-jit" shift elif [ "x$1" = "x--optimizing" ]; then - run_args="${run_args} -Xcompiler-option --compiler-backend=Optimizing" + image_args="-Xcompiler-option --compiler-backend=Optimizing" image_suffix="-optimizing" shift elif [ "x$1" = "x--no-verify" ]; then run_args="${run_args} --no-verify" shift elif [ "x$1" = "x--verify-soft-fail" ]; then - run_args="${run_args} --verify-soft-fail" + image_args="--verify-soft-fail" image_suffix="-interp-ac" shift elif [ "x$1" = "x--no-optimize" ]; then @@ -348,6 +351,7 @@ while true; do fi done +run_args="${run_args} ${image_args}" # Allocate file descriptor real_stderr and redirect it to the shell's error # output (fd 2). if [ ${BASH_VERSINFO[1]} -ge 4 ] && [ ${BASH_VERSINFO[2]} -ge 1 ]; then @@ -467,7 +471,7 @@ elif [ "$runtime" = "art" ]; then run_args="${run_args} --runtime-option -Djava.library.path=${ANDROID_HOST_OUT}/lib${suffix64}" else guess_target_arch_name - run_args="${run_args} --runtime-option -Djava.library.path=/data/art-test/${target_arch_name}" + run_args="${run_args} --runtime-option -Djava.library.path=/data/art-test/${target_arch_name}:/system/lib${suffix64}" run_args="${run_args} --boot /data/art-test/core${image_suffix}${pic_image_suffix}${multi_image_suffix}.art" fi if [ "$relocate" = "yes" ]; then diff --git a/tools/dmtracedump/tracedump.cc b/tools/dmtracedump/tracedump.cc index f70e2c2207..3afee6fdcb 100644 --- a/tools/dmtracedump/tracedump.cc +++ b/tools/dmtracedump/tracedump.cc @@ -512,10 +512,10 @@ int32_t compareUniqueExclusive(const void* a, const void* b) { void freeDataKeys(DataKeys* pKeys) { if (pKeys == nullptr) return; - free(pKeys->fileData); - free(pKeys->threads); - free(pKeys->methods); - free(pKeys); + delete[] pKeys->fileData; + delete[] pKeys->threads; + delete[] pKeys->methods; + delete pKeys; } /* @@ -822,8 +822,8 @@ void sortMethodList(DataKeys* pKeys) { DataKeys* parseKeys(FILE* fp, int32_t verbose) { int64_t offset; DataKeys* pKeys = new DataKeys(); - memset(pKeys, 0, sizeof(DataKeys)); if (pKeys == nullptr) return nullptr; + memset(pKeys, 0, sizeof(DataKeys)); /* * We load the entire file into memory. We do this, rather than memory- @@ -865,9 +865,13 @@ DataKeys* parseKeys(FILE* fp, int32_t verbose) { return nullptr; } - /* Reduce our allocation now that we know where the end of the key section is. */ - pKeys->fileData = reinterpret_cast<char*>(realloc(pKeys->fileData, offset)); - pKeys->fileLen = offset; + /* + * Although it is tempting to reduce our allocation now that we know where the + * end of the key section is, there is a pitfall. The method names and + * signatures in the method list contain pointers into the fileData area. + * Realloc or free will result in corruption. + */ + /* Leave fp pointing to the beginning of the data section. */ fseek(fp, offset, SEEK_SET); @@ -2607,7 +2611,7 @@ int32_t main(int32_t argc, char** argv) { if (gOptions.graphFileName != nullptr) { createInclusiveProfileGraphNew(dataKeys); } - free(methods); + delete[] methods; } freeDataKeys(dataKeys); diff --git a/tools/libcore_failures.txt b/tools/libcore_failures.txt index dd2cc3140f..f25fb98c4d 100644 --- a/tools/libcore_failures.txt +++ b/tools/libcore_failures.txt @@ -253,11 +253,5 @@ names: ["jsr166.CollectionTest#testEmptyMeansEmpty", "jsr166.Collection8Test#testForEach", "jsr166.Collection8Test#testForEachConcurrentStressTest"] -}, -{ - description: "Unclear why this started to fail", - result: EXEC_FAILED, - bug: 28574453, - names: [ "org.apache.harmony.tests.javax.security.cert.X509CertificateTest#testVerifyPublicKey" ] } ] |