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

[Frederic Parain]

On Thu, 16 Mar 2023 20:02:56 GMT, Matias Saavedra Silva <matsaave at openjdk.org> wrote:

>> In several places in HotSpot, the method MacroAssembler::null_check(Register, offset) is called in a way that never produces any null check in the assembly code. The method null_check(Register, offset) calls needs_explicit_null_check(offset) to determine if it must emit a null check in the assembly code or not.
>> 
>> needs_explicit_null_check(offset) returns true only if the offset is negative or bigger than the os page size. 
>> the offset being passed is the offset of a field in the header of Java object or a Java array. In both cases, the offset is always positive and smaller than an os page size. A null_check() call with a single parameter will always produce a null check in assembly.
>> 
>> The cases suggested in the issue have been addressed by either removing or preserving the null_check. Verified with tier 1-3 tests.
>
> Matias Saavedra Silva has updated the pull request incrementally with one additional commit since the last revision:
> 
>   load_klass now emits null check and other platforms supported

The nulll_check() method is perfectly fine, and complements the work done in the signal handler for segmentation fault in the first page. 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. It happens when the offset is statically known at VM compilation time, and this offset is smaller than a page size: the klass pointer offset and the array's length.
But if you consider that the code is safer as written in its current form, I'll close the bug as not-an-issue. Anyway, it doesn't change the code being generated.

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

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