Fix ARM & ARM64 UnsafeGetObject intrinsics with read barriers.
The implementation was incorrectly interpreting the 'offset'
input as an index in a (4-byte) object reference array,
whereas it is a (1-byte) offset to an object reference field
within the 'base' (object) input.
Bug: 29516905
Change-Id: I4da5be0193217965f25e5d141c242592dea6ffe8
Test: Covered by test/004-UnsafeTest.
diff --git a/compiler/optimizing/code_generator_arm.cc b/compiler/optimizing/code_generator_arm.cc
index eca9e2c..2b63f3d 100644
--- a/compiler/optimizing/code_generator_arm.cc
+++ b/compiler/optimizing/code_generator_arm.cc
@@ -6085,8 +6085,9 @@
// /* HeapReference<Object> */ ref = *(obj + offset)
Location no_index = Location::NoLocation();
+ ScaleFactor no_scale_factor = TIMES_1;
GenerateReferenceLoadWithBakerReadBarrier(
- instruction, ref, obj, offset, no_index, temp, needs_null_check);
+ instruction, ref, obj, offset, no_index, no_scale_factor, temp, needs_null_check);
}
void CodeGeneratorARM::GenerateArrayLoadWithBakerReadBarrier(HInstruction* instruction,
@@ -6099,10 +6100,14 @@
DCHECK(kEmitCompilerReadBarrier);
DCHECK(kUseBakerReadBarrier);
+ static_assert(
+ sizeof(mirror::HeapReference<mirror::Object>) == sizeof(int32_t),
+ "art::mirror::HeapReference<art::mirror::Object> and int32_t have different sizes.");
// /* HeapReference<Object> */ ref =
// *(obj + data_offset + index * sizeof(HeapReference<Object>))
+ ScaleFactor scale_factor = TIMES_4;
GenerateReferenceLoadWithBakerReadBarrier(
- instruction, ref, obj, data_offset, index, temp, needs_null_check);
+ instruction, ref, obj, data_offset, index, scale_factor, temp, needs_null_check);
}
void CodeGeneratorARM::GenerateReferenceLoadWithBakerReadBarrier(HInstruction* instruction,
@@ -6110,6 +6115,7 @@
Register obj,
uint32_t offset,
Location index,
+ ScaleFactor scale_factor,
Location temp,
bool needs_null_check) {
DCHECK(kEmitCompilerReadBarrier);
@@ -6164,17 +6170,22 @@
// The actual reference load.
if (index.IsValid()) {
- static_assert(
- sizeof(mirror::HeapReference<mirror::Object>) == sizeof(int32_t),
- "art::mirror::HeapReference<art::mirror::Object> and int32_t have different sizes.");
- // /* HeapReference<Object> */ ref =
- // *(obj + offset + index * sizeof(HeapReference<Object>))
+ // Load types involving an "index": ArrayGet and
+ // UnsafeGetObject/UnsafeGetObjectVolatile intrinsics.
+ // /* HeapReference<Object> */ ref = *(obj + offset + (index << scale_factor))
if (index.IsConstant()) {
size_t computed_offset =
- (index.GetConstant()->AsIntConstant()->GetValue() << TIMES_4) + offset;
+ (index.GetConstant()->AsIntConstant()->GetValue() << scale_factor) + offset;
__ LoadFromOffset(kLoadWord, ref_reg, obj, computed_offset);
} else {
- __ add(IP, obj, ShifterOperand(index.AsRegister<Register>(), LSL, TIMES_4));
+ // Handle the special case of the
+ // UnsafeGetObject/UnsafeGetObjectVolatile intrinsics, which use
+ // a register pair as index ("long offset"), of which only the low
+ // part contains data.
+ Register index_reg = index.IsRegisterPair()
+ ? index.AsRegisterPairLow<Register>()
+ : index.AsRegister<Register>();
+ __ add(IP, obj, ShifterOperand(index_reg, LSL, scale_factor));
__ LoadFromOffset(kLoadWord, ref_reg, IP, offset);
}
} else {