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

[Daniel D.Daugherty]

On Mon, 20 Dec 2021 22:26:12 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> A small refactoring to resolve a Parfait complaint about the return value from
>> `MacroAssembler::target_addr_for_insn(address insn_addr, unsigned insn)`
>> on AARCH64. The logic that supports returning `nullptr` as the target addr for
>> a particular instruction is moved from
>> `MacroAssembler::target_addr_for_insn(address insn_addr, unsigned insn)` to
>> `MacroAssembler::target_addr_for_insn_allow_null_ret(address insn_addr, unsigned insn)`.
>> A couple of `target_addr_for_insn()` call sites that can tolerate a `nullptr` are
>> converted to use `target_addr_for_insn_allow_null_ret()`.
>> 
>> This fix has been tested with Mach5 Tier[1-3].
>
> Hi Dan,
> 
> In terms of the refactoring and addressing the warning this seems reasonable. But in terms of semantics this claims that all the remaining callsites for `target_addr_for_insn` can never pass an `insn` that can lead to a NULL address. I can't verify that claim. This needs some Aarch64 experts to chime in - perhaps @theRealAph?
> 
> Thanks,
> David

@dholmes-ora - Thanks for taking a look. I did take the time to look at all the
remaining call sites for `target_addr_for_insn` and, in some cases, their callers
and I didn't see anything where it made sense to run into an `insn` that could
lead to a `nullptr` address. Of course, that's just code inspection on my part
so I also created `guarantee()` traps at all the call-sites and ran those bits
thru Mach5 Tier[1-8] testing without hitting any of the traps. As you pointed
out in the bug report, that kind of testing isn't proof either, but it helps me
believe that I didn't miss anything too obvious.

Having @theRealAph chime in here would be wonderful!

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

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