[jdk18] RFR: 8272058: 25 Null pointer dereference defect groups in 4 files [v4]

[Patric Hedlin]

On Tue, 4 Jan 2022 21:38:27 GMT, Daniel D. Daugherty <dcubed at openjdk.org> wrote:

>> src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp line 286:
>> 
>>> 284:                    + (uint64_t(Instruction_aarch64::extract(insns[1], 20, 5)) << 16)
>>> 285:                    + (uint64_t(Instruction_aarch64::extract(insns[2], 20, 5)) << 32));
>>> 286:   } else if (Instruction_aarch64::extract(insn, 31, 22) == 0b1011100101 &&
>> 
>> It must be a check for safePoint poll: ldrw zr, polling_page. Technically, the target address for this instruction is zero and the function is correct. I am not sure Parfait is right in his complaints. After all, does this code ever works? That is, if you remove it, will the SholdNotReachHere below fire?
>
> @bulasevich - I've made another pass through the code and checked the
> calls to `MacroAssembler::target_addr_for_insn()` and I don't see any of
> those calls in areas related to safepoint polling. From the other POV, I
> took a look at the aarch64 safepoint polling code and I don't see a way
> into `MacroAssembler::target_addr_for_insn()` .
> 
> It is entirely possible that I'm missing something here and would appreciate
> it if you could sanity check my conclusions here.

I believe this code is dead, in both `MacroAssembler::pd_patch_instruction()` and `MacroAssembler::target_addr_for_insn()`, since the poll read is PIC, we are not patching and should not need to acquire any target to update.

The associated `fix_relocation_after_move()` will not invoke any of the above since the `maybe_cpool_ref()` condition is used:

void poll_Relocation::fix_relocation_after_move(const CodeBuffer* src, CodeBuffer* dest) {
  if (NativeInstruction::maybe_cpool_ref(addr())) {
    address old_addr = old_addr_for(addr(), src, dest);
    MacroAssembler::pd_patch_instruction(addr(), MacroAssembler::target_addr_for_insn(old_addr));
  }
}

Should be sufficient to simply remove these cases, possibly you could move an assert into the "unreachable" branch (for "precision").

} else {
  assert(!NativeInstruction::is_ldrw_to_zr(<addr>), "Unexpected poll read");
  ShouldNotReachHere();
}

(The code above in `fix_relocation_after_move()` might also be dead...)

Chip in @theRealAph ?

-------------

PR: https://git.openjdk.java.net/jdk18/pull/51