From eebb821b1adaf2db7662fc1c3ff4e9fcfe59a694 Mon Sep 17 00:00:00 2001 From: Vladimir Marko Date: Tue, 5 Jun 2018 14:57:24 +0100 Subject: Implement Integer.valueOf() intrinsic for PIC. And fix the intrinsic for JIT even in case when someone messes up the IntegerCache using reflection. Two cases are exposed with a regression test (one that previously failed randomly and one that failed 100%) but other crashes were possible; for example, we would need a read barrier for array reads when elements are not guaranteed to be in the boot image. The new approach loads references only from the boot image live objects array which cannot be touched by reflection. The referenced objects and IntegerCache.cache are exposed and can lead to weird behavior but not crashes. On x86, the pc_relative_fixups_86 actually checks the cache an additional time but discrepancies between this check and the location building at the beginning of codegen should be OK as the HIsX86ComputeBaseMethodAddress should be added for PIC regardless of whether pc_relative_fixups_86 thinks the method is intrinsified or not. Test: 717-integer-value-of Test: Pixel 2 XL boots. Test: m test-art-host-gtest Test: testrunner.py --host --optimizing --pictest --npictest Test: testrunner.py --host --jit Test: testrunner.py --target --optimizing --pictest --npictest Test: testrunner.py --target --jit Bug: 71526895 Change-Id: I89b3245a62aba22980c86a99e2af480bfa250af1 --- compiler/optimizing/intrinsics_mips.cc | 37 +++++++++++++--------------------- 1 file changed, 14 insertions(+), 23 deletions(-) (limited to 'compiler/optimizing/intrinsics_mips.cc') diff --git a/compiler/optimizing/intrinsics_mips.cc b/compiler/optimizing/intrinsics_mips.cc index ae248a3e5c..a3eb42b4b7 100644 --- a/compiler/optimizing/intrinsics_mips.cc +++ b/compiler/optimizing/intrinsics_mips.cc @@ -2601,7 +2601,7 @@ void IntrinsicLocationsBuilderMIPS::VisitIntegerValueOf(HInvoke* invoke) { } void IntrinsicCodeGeneratorMIPS::VisitIntegerValueOf(HInvoke* invoke) { - IntrinsicVisitor::IntegerValueOfInfo info = IntrinsicVisitor::ComputeIntegerValueOfInfo(); + IntrinsicVisitor::IntegerValueOfInfo info = IntrinsicVisitor::ComputeIntegerValueOfInfo(invoke); LocationSummary* locations = invoke->GetLocations(); MipsAssembler* assembler = GetAssembler(); InstructionCodeGeneratorMIPS* icodegen = @@ -2609,22 +2609,18 @@ void IntrinsicCodeGeneratorMIPS::VisitIntegerValueOf(HInvoke* invoke) { Register out = locations->Out().AsRegister(); InvokeRuntimeCallingConvention calling_convention; + Register argument = calling_convention.GetRegisterAt(0); if (invoke->InputAt(0)->IsConstant()) { int32_t value = invoke->InputAt(0)->AsIntConstant()->GetValue(); - if (value >= info.low && value <= info.high) { + if (info.value_boot_image_offset != 0u) { // Just embed the j.l.Integer in the code. - ScopedObjectAccess soa(Thread::Current()); - mirror::Object* boxed = info.cache->Get(value + (-info.low)); - DCHECK(boxed != nullptr && Runtime::Current()->GetHeap()->ObjectIsInBootImageSpace(boxed)); - uint32_t address = dchecked_integral_cast(reinterpret_cast(boxed)); - __ LoadConst32(out, address); + codegen_->LoadBootImageAddress(out, info.value_boot_image_offset); } else { + DCHECK(locations->CanCall()); // Allocate and initialize a new j.l.Integer. // TODO: If we JIT, we could allocate the j.l.Integer now, and store it in the // JIT object table. - uint32_t address = - dchecked_integral_cast(reinterpret_cast(info.integer)); - __ LoadConst32(calling_convention.GetRegisterAt(0), address); + codegen_->LoadBootImageAddress(argument, info.integer_boot_image_offset); codegen_->InvokeRuntime(kQuickAllocObjectInitialized, invoke, invoke->GetDexPc()); CheckEntrypointTypes(); __ StoreConstToOffset(kStoreWord, value, out, info.value_offset, TMP); @@ -2633,27 +2629,23 @@ void IntrinsicCodeGeneratorMIPS::VisitIntegerValueOf(HInvoke* invoke) { icodegen->GenerateMemoryBarrier(MemBarrierKind::kStoreStore); } } else { + DCHECK(locations->CanCall()); Register in = locations->InAt(0).AsRegister(); MipsLabel allocate, done; - int32_t count = static_cast(info.high) - info.low + 1; - // Is (info.low <= in) && (in <= info.high)? __ Addiu32(out, in, -info.low); - // As unsigned quantities is out < (info.high - info.low + 1)? - if (IsInt<16>(count)) { - __ Sltiu(AT, out, count); + // As unsigned quantities is out < info.length ? + if (IsUint<15>(info.length)) { + __ Sltiu(AT, out, info.length); } else { - __ LoadConst32(AT, count); + __ LoadConst32(AT, info.length); __ Sltu(AT, out, AT); } - // Branch if out >= (info.high - info.low + 1). - // This means that "in" is outside of the range [info.low, info.high]. + // Branch if out >= info.length. This means that "in" is outside of the valid range. __ Beqz(AT, &allocate); // If the value is within the bounds, load the j.l.Integer directly from the array. - uint32_t data_offset = mirror::Array::DataOffset(kHeapReferenceSize).Uint32Value(); - uint32_t address = dchecked_integral_cast(reinterpret_cast(info.cache)); - __ LoadConst32(TMP, data_offset + address); + codegen_->LoadBootImageAddress(TMP, info.array_data_boot_image_offset); __ ShiftAndAdd(out, out, TMP, TIMES_4); __ Lw(out, out, 0); __ MaybeUnpoisonHeapReference(out); @@ -2661,8 +2653,7 @@ void IntrinsicCodeGeneratorMIPS::VisitIntegerValueOf(HInvoke* invoke) { __ Bind(&allocate); // Otherwise allocate and initialize a new j.l.Integer. - address = dchecked_integral_cast(reinterpret_cast(info.integer)); - __ LoadConst32(calling_convention.GetRegisterAt(0), address); + codegen_->LoadBootImageAddress(argument, info.integer_boot_image_offset); codegen_->InvokeRuntime(kQuickAllocObjectInitialized, invoke, invoke->GetDexPc()); CheckEntrypointTypes(); __ StoreToOffset(kStoreWord, in, out, info.value_offset); -- cgit v1.2.3-59-g8ed1b