Fix longstanding bug around implicit NPEs and GC, version 2.
The TODO has been there since M (so forever :)):
https://android-review.googlesource.com/c/platform/art/+/122794/13//COMMIT_MSG#13
We hardly see the issue in our tests as we need to have:
1) A GC happening while creating the NPE object.
2) ParallelMoves between the NullCheck and implicit null check operation
that moves references.
The CL piggy backs on the "IsEmittedAtUseSite" flag, to set implicit
null checks with it. The liveness analysis then special cases implicit
null checks to record environment uses at the location of the actual
instruction that will do the implicit null check.
Test: test.py --gcstress
Test: run-libcore-tests --gcstress
bug: 111545159
Change-Id: I3ecea4fe0d7e483e93db83281ca10db47da228c5
diff --git a/compiler/optimizing/code_generator.cc b/compiler/optimizing/code_generator.cc
index a13efca..a90ff3f 100644
--- a/compiler/optimizing/code_generator.cc
+++ b/compiler/optimizing/code_generator.cc
@@ -1394,37 +1394,12 @@
}
bool CodeGenerator::CanMoveNullCheckToUser(HNullCheck* null_check) {
- HInstruction* first_next_not_move = null_check->GetNextDisregardingMoves();
-
- return (first_next_not_move != nullptr)
- && first_next_not_move->CanDoImplicitNullCheckOn(null_check->InputAt(0));
+ return null_check->IsEmittedAtUseSite();
}
void CodeGenerator::MaybeRecordImplicitNullCheck(HInstruction* instr) {
- if (!compiler_options_.GetImplicitNullChecks()) {
- return;
- }
-
- // If we are from a static path don't record the pc as we can't throw NPE.
- // NB: having the checks here makes the code much less verbose in the arch
- // specific code generators.
- if (instr->IsStaticFieldSet() || instr->IsStaticFieldGet()) {
- return;
- }
-
- if (!instr->CanDoImplicitNullCheckOn(instr->InputAt(0))) {
- return;
- }
-
- // Find the first previous instruction which is not a move.
- HInstruction* first_prev_not_move = instr->GetPreviousDisregardingMoves();
-
- // If the instruction is a null check it means that `instr` is the first user
- // and needs to record the pc.
- if (first_prev_not_move != nullptr && first_prev_not_move->IsNullCheck()) {
- HNullCheck* null_check = first_prev_not_move->AsNullCheck();
- // TODO: The parallel moves modify the environment. Their changes need to be
- // reverted otherwise the stack maps at the throw point will not be correct.
+ HNullCheck* null_check = instr->GetImplicitNullCheck();
+ if (null_check != nullptr) {
RecordPcInfo(null_check, null_check->GetDexPc());
}
}