RFR: 8241613: Suspicious calls to MacroAssembler::null_check(Register, offset) [v2]

[Andrew Dinn]

On Tue, 21 Mar 2023 11:57:04 GMT, Frederic Parain <fparain at openjdk.org> wrote:

> But my point was not about the method itself, it was about cases where this method is used when it is provable that there's no need to insert an explicit null check because the signal handler is able to handle the case.

Ah, ok, apologies for any confusion. I see your point.

I have always understood that the presence of, for example, both `load_klass` and `load_klass_null_check` arises because they predate employment of the signal handler/read-protection for page 0. Without that protection it is necessary to do a `load_klass_null_check` on a path where an oop has not previously been null checked but a `load_klass` will suffice on a path where it is known to be non-null. I am not sure that is the actual history but I believe it does characterize the intended use of the two method variants.

This distinction may be irrelevant given the current state of affairs but it would matter if for some reason we had to decommission the SEGV handler, for example if OpenJDK were ported to an OS which does not support read-protection for page 0.

I don't suppose we are likely to encounter such a circumstance so it might be reasonable to collapse all calls to `load_klass_null_check` into plain calls to `load_klass`.

That said, there is no good reason for this PR to modify `load_klass_null_check` to always perform a null check (i.e. the call to `null_check(Register)`). Arguably, the existing call to `null_check(Register, int)` could be removed.

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

PR Comment: https://git.openjdk.org/jdk/pull/13026#issuecomment-1479579651