Fix null checks on volatile reference field loads on ARM64.
ART's compiler adds a null check HIR instruction before each field
load HIR instruction created in the instruction builder phase. When
implicit null checks are allowed, the compiler elides the null check
if it can be turned into an implicit one (i.e. if the offset is within
a system page range).
On ARM64, the Baker read barrier thunk built for field reference loads
needs to check the lock word of the holder of the field, and thus
includes an explicit null check if no null check has been done before.
However, this was not done for volatile loads (implemented with a
load-acquire instruction on ARM64). This change adds this missing null
check.
Test: art/test/testrunner/testrunner.py --target --64 -t 1004-checker-volatile-ref-load
Bug: 140507091
Bug: 36141117
Change-Id: Ie94f2e73d2f439ae4460549d7b71848401602a21
diff --git a/compiler/optimizing/code_generator_arm64.cc b/compiler/optimizing/code_generator_arm64.cc
index 502ff60..adc98ab 100644
--- a/compiler/optimizing/code_generator_arm64.cc
+++ b/compiler/optimizing/code_generator_arm64.cc
@@ -6341,12 +6341,20 @@
CheckValidReg(holder_reg.GetCode());
UseScratchRegisterScope temps(assembler.GetVIXLAssembler());
temps.Exclude(ip0, ip1);
- // If base_reg differs from holder_reg, the offset was too large and we must have emitted
- // an explicit null check before the load. Otherwise, for implicit null checks, we need to
- // null-check the holder as we do not necessarily do that check before going to the thunk.
+ // In the case of a field load (with relaxed semantic), if `base_reg` differs from
+ // `holder_reg`, the offset was too large and we must have emitted (during the construction
+ // of the HIR graph, see `art::HInstructionBuilder::BuildInstanceFieldAccess`) and preserved
+ // (see `art::PrepareForRegisterAllocation::VisitNullCheck`) an explicit null check before
+ // the load. Otherwise, for implicit null checks, we need to null-check the holder as we do
+ // not necessarily do that check before going to the thunk.
+ //
+ // In the case of a field load with load-acquire semantics (where `base_reg` always differs
+ // from `holder_reg`), we also need an explicit null check when implicit null checks are
+ // allowed, as we do not emit one before going to the thunk.
vixl::aarch64::Label throw_npe_label;
vixl::aarch64::Label* throw_npe = nullptr;
- if (GetCompilerOptions().GetImplicitNullChecks() && holder_reg.Is(base_reg)) {
+ if (GetCompilerOptions().GetImplicitNullChecks() &&
+ (holder_reg.Is(base_reg) || (kind == BakerReadBarrierKind::kAcquire))) {
throw_npe = &throw_npe_label;
__ Cbz(holder_reg.W(), throw_npe);
}