RFR: JDK-8318119: Invalid narrow Klass base on aarch64 post 8312018

[Thomas Stuefe]

On Tue, 17 Oct 2023 10:44:07 GMT, Andrew Haley <aph at openjdk.org> wrote:

>> When reserving space for class space or the combined CDS+class space, provide a fallback on aarch64 when all prior attempts to reserve space failed. Now the logic goes like this:
>> 
>> 1) (only if not CDS) attempt to reserve in low-address regions
>> 2) attempt to reserve at a 32G-aligned address at randomized attach points within the whole of the available address space
>> 3) (new) attempt to reserve 4G-aligned by using os::reserve_aligned.
>> 
>> (3) is similar to (2), but (2) may fail if we are very unlucky and the address space is very populated. The difference between (2) and (3) is that with (3), the kernel chooses the placement of the VMA, and we rely on ASLR to provide entropy.
>> 
>> The patch also fixes os::reserve_memory_aligned() to remove the assertion that *size* needs to be aligned. There is no need for that, and for large alignments this will blow up the required over-allocation size even further.
>> 
>> The patch also fixes up CompressedKlassPointers::is_valid_base(). The whole purpose of this function is to filter out possibly invalid base addresses when someone manually specifies SharedBaseAddress. The problem with this function is that the encoding base alone is not sufficient to determine its validity, since for that one needs to know the shift value too, and for that the boundaries of the future encoding range. I changed this function to give the best answer possible at that point, see comment.
>
> src/hotspot/share/memory/metaspace.cpp line 634:
> 
>> 632:     if (result == nullptr) {
>> 633:       // If that failed, attempt to allocate at any properly aligned address. Try first an alignment that
>> 634:       // includes shift (e.g. 32GB). Failing that, try an alignment that does not include the shift (4GB).
> 
> What's the point of preferring a shift? On AArch64, better simply to use a `movk`.

The way movk mode is currently implemented is to apply the right-shifted base to the narrow Klass ID, then to left-shift the thing to get Klass*

https://github.com/openjdk/jdk/blob/d0ea2a51111bd5de5a6465e7de6a4950aae89c71/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp#L4723-L4731

which requires the *right-shifted* base to be movk-able. So it must have the lower 32+shift bits zero:

https://github.com/openjdk/jdk/blob/d0ea2a51111bd5de5a6465e7de6a4950aae89c71/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp#L4656-L4659

hence the 32g alignment.


movw dst, src
movk dst, right-shifted-base to q3
shl dst, dst, 3


Note that this patch is still draft. I wanted to add another movk decoding variant, would work with a 4GB aligned base, just to increase our options. 

It would apply the unshifted base to the left-shifted narrow Klass ID:


shl dst, src, 3
movk dst, unshifted-base


and that would even need one less operation. 

Only problem is that it limits the value range of the narrow Klass ID, since it must not spill over into the third quadrant, so its upper (shift) bits must be zero. For shift=3, it means narrowKlass must be <= 2^(32-3). I think this is fine though, since we still can address 4 GB of class space. In other words, we should have never seen such high narrow Klass IDs anyway.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16215#discussion_r1362277848